-
Notifications
You must be signed in to change notification settings - Fork 12
support root extensions #177
base: master
Are you sure you want to change the base?
Conversation
…s per the swagger specification. root extensions are parsed into the same location as the info extensions.
i believe the tests are failing on a swagger zoo assertion, where the root |
Can you please make a PR on swagger-zoo fixing it? |
You don't need to add a test case here if you fix it there. |
@pksunkara thanks for the advice! i will do that today. |
Reference to previous discussion: #71 (comment) |
This would result in multiple extension elements in the API element, the API Elements consumer has no idea which one is which, and it could be really confusing when there is an extension from path, root and info which are using the same name. For example: info:
aws-foo: foo
paths:
aws-foo: bar
aws-foo: foobar The parse result will contain an API with three extension elements with different values for I think we need to find an alternative solution. Perhaps the href for the extension (https://help.apiary.io/profiles/api-elements/vendor-extensions/) could provide more information on the path, for example:
@abacaphiliac @pksunkara what do you think? |
@kylef i agree with your concern. vendor-extensions links to element-definitions, which provides the following example: {
"element": "extension",
"meta": {
"links": {
"element": "array",
"content": [
{
"element": "link",
"attributes": {
"relation": {
"element": "string",
"content": "profile"
},
"href": {
"element": "string",
"content": "http://example.com/extensions/info/"
}
}
}
]
}
},
"content": {
"element": "object",
"content": [
{
"element": "member",
"content": {
"key": {
"element": "string",
"content": "version"
},
"value": {
"element": "string",
"content": "1.0"
}
}
}
]
}
}
edit: i see now that we're already using |
@kylef href fragment makes sense to me unless there is another more easily parsable location. are there any other attributes or meta that we can use with the link element? the extension element example uses an object element as content. we could put some kind of source information there, but i think that would conflict with the existing extension element content structure. |
I like the href fragment idea. |
…profile link URL fragment and added paths extension assertion as well
@kylef @pksunkara thanks for the feedback. i've updated the parser to add URL fragments for
URL fragment is the only mutable detail on the profile link. is that acceptable? i considered accepting a full URL object and merging overrides onto the existing profile link as a base. is that better, or would that be over the top? |
Code looks good. Can you do a corresponding swagger-zoo PR? |
|
||
this.handleExternalDocs(this.api, swagger.externalDocs); | ||
|
||
this.validateProduces(this.swagger.produces); | ||
this.validateConsumes(this.swagger.consumes); | ||
|
||
const complete = () => { | ||
this.handleSwaggerVendorExtensions(this.api, swagger.paths); | ||
this.handleSwaggerVendorExtensions(this.api, swagger.paths, 'paths'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be a breaking change for tooling looking for these relations so we need to be careful about rolling out this change. @pksunkara perhaps we should serialise them differently in minim 1.0/0.6 serialisers?
We have a task in our backlog about moving all of the links (such as those in annotations) to be in apielements.org (since many of them are actually 404s). What we could do it update them all and create a profile registry over in API elements. Then in the minim 0.6 serialiser we can replace them to be the existing links.
Another thing to think about is I don't think the current profile is correct given the API Elements 1.0 serialisation rules. Since we're placing an un-refracted object directly in the content of the extension which breaks the rules. Given the following extension:
x-extension:
element: string
Tooling would think that this is a string element instead a user-defined object which is some of the problems we was trying to solve in API Elements 1.0. @pksunkara Would love your thoughts, we can probably use this time to do the profile right before wide-adoption of API Elements 1.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative idea, we could also include both the old style and new style extension element. This allows existing tooling to continue to work (we would have to think of a deprecation plan, a way to communicate that the element is deprecated and a time period in which we support the existing element). This allows new tooling to directly utilise the newer format, and old tooling to utilise the existing format until updated. Personally I think my other comment would be better idea long-term for this particular case given API Elements immaturity, but it involves more work short-term (changing minim serialiser and releasing another toolchain version), but it would be less work in the long run than having to maintain older code paths, but it would be a good idea to start thinking about the way forward for deprecating and replacing elements for the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like your first comment on this.
- Move the profiles to apielements.org
- Write an RFC about the content of the profiles and implement it
- Convert it into 0.6 serialization.
If you want to discuss about the problems we need to fix before 1.0, RFC would be a better place to discuss this.
@pksunkara I'd be happy to open a corresponding swagger-zoo PR. Would you prefer I hold off on that until the profile and serializer details settle? Is there any way I can contribute to that discussion or project(s)? |
fix(root-extensions): similar to #174, this will parse root extensions per the swagger specification.
unfortunately, root extensions are parsed into the same location as the info extensions.