Skip to content
This repository has been archived by the owner on Jun 23, 2019. It is now read-only.

DataStructure is not an ObjectElement #25

Closed
sjuxax opened this issue Apr 24, 2017 · 7 comments
Closed

DataStructure is not an ObjectElement #25

sjuxax opened this issue Apr 24, 2017 · 7 comments

Comments

@sjuxax
Copy link

sjuxax commented Apr 24, 2017

Though the documentation claims that DataStructures are ObjectElements, you can see that they inherit from BaseElement, not ObjectElement, on src/elements/data-structure.js:2 :

class DataStructure extends namespace.BaseElement {

I have tried replacing this manually in the code with ObjectElement and ArrayElement, but there is a big cascade of errors when doing so.

I don't think the DataStructure support is really fleshed out. Instead of using a refracted value as most other tests do, the DataStructure tests are initialized with a simple:

  context('data structure element', () => {
    let struct;

    beforeEach(() => {
      struct = new DataStructure('test');
    });

(from test/api-descriptions.js:381)

I also ran into the issues described in #18 .

@pksunkara
Copy link
Contributor

Thanks for the report @sjuxax. I am looking into this currently.

@kylef
Copy link
Member

kylef commented Apr 28, 2017

Hi @sjuxax,

The problem here lies in Drafter (API Blueprint parser) which is returning an array of any base element. A base API Element should instead be inside the data structure content.

The implementation of minim is correct, however the documentation for minim is incorrect in saying it should be an object element inside the data structure. This isn't true as an array data structures content would be an array element, etc. The documentation in API Elements shows the possibilities: http://api-elements.readthedocs.io/en/latest/element-definitions/#base-data-structure-element

screen shot 2017-04-28 at 14 32 24

Where ref/element pointer can also be there when a data structure is just referencing another.

@pksunkara
Copy link
Contributor

The documentation is wrong here. DataStructue is actually a BaseElement not ObjectElement. I am working on updating it and adding a bit of tests. Let's follow the issue in #18.

What was your use case here? How were you trying to use it?

@sjuxax
Copy link
Author

sjuxax commented Apr 28, 2017

I was trying to walk the tree of an API Blueprint converted to Refract. Everything seems to work except DataStructure elements. This is a problem at least in the serialization pipeline, which produces output that doesn't match the API Elements specification.

I have a work-in-progress patch that produces more-or-less acceptable output given the current serialization format. I will post it up in a little bit, though it's made to address the issues as they stand and not to make things conform to spec.

@pksunkara
Copy link
Contributor

Yeah, we have just realised that dataStructre element produced by our parser (Drafter) is an array element which is wrong. We are working on fixing it and it will be released in a major version in the coming few weeks. I am closing this for now.

sjuxax added a commit to sjuxax/minim-api-description that referenced this issue Apr 28, 2017
Not perfect but somewhat usable at least.
There are several bugs that make the serializer not conform to
specification.

See refractproject#25
@sjuxax
Copy link
Author

sjuxax commented Apr 28, 2017

So you can see my hack above. It produces output that can be used, even though it's not necessarily pretty.

The problems extend beyond just the wrong type of element getting added as a child of the dataStructure. For example, the "expanded Refract" discussed in the API Elements documentation is not resolved by the serializer, though the documentation indicates this should happen.

There are also several malformed Refract elements generated; elements without content, or in some cases, even element keys.

This line https://github.com/sjuxax/minim-api-description/blob/81f38e70006388e0e80efb931f96a2aad3646bef/src/elements/data-structure.js#L17 handles that by referencing the unresolved elements as <?datatype>. The parser can then store this reference and fill in the blank itself, when it gets down to the bottom and reads the dataStructures element that contains the actual information.

I'll add a couple of examples of bad output in a second.

@pksunkara
Copy link
Contributor

If content is not present, then it's null. But yes, we are working on streamlining the serialisation with the upcoming major version.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants