Skip to content
This repository has been archived by the owner on Jan 21, 2024. It is now read-only.

Added annotations to reflect OpenAPI 2.0 properties #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sichvoge
Copy link
Contributor

This PR adds standard annotations that reflect OpenAPI 2.0 properties missing in RAML 1.0.

Copy link

@petrochenko-pavel-a petrochenko-pavel-a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sichvoge as for me some annotations proposed here have more generic meaning so it would be better to declare them as a generic annotations,
for example tags instead of oas-tags.

Also I see potential problem with oas-parameter-collection-format and oas-x- annotations.

Regards,
Pavel


annotationTypes:
oas-summary: !include oas-summary.raml
oas-deprecated: !include oas-deprecated.raml
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for me it is more logical to create generic deprecated annotation, which is not specific to oas.

oas-summary: !include oas-summary.raml
oas-deprecated: !include oas-deprecated.raml
oas-info: !include oas-info.raml
oas-tags: !include oas-tags.raml
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tags - also looks as a pretty generic thing.


type: boolean
default: false
allowedTargets: Method

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we should constrain it to Methods only.

is used to define extensions for the OpenAPI Specification.

type: any
displayName: Extensions (OAS)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for me it does not looks like a good idea to convert all extensions into 1 annotation. (Imagine round trip from RAML to OAS and back, ideally we should convert raml annotations to extensions and then back to same annotations)

@@ -0,0 +1,10 @@
#%RAML 1.0 AnnotationTypeDeclaration

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we have a hidden problem with this annotation. (In RAML we have a strict definition of how arrays of the scalars should be serialised and it seems that it depends from position where we are referencing array type) :-(

oas-original-definition-name: !include oas-original-schema-name.raml
oas-x-: !include oas-extension.raml
oas-responses: !include oas-responses.raml
oas-responses-default: !include oas-responses-default.raml
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oas-responses-default this one also looks pretty generic.

you are allowed to define inside a parameter definition.

type: boolean
displayName: Parameter AllowEmptyValue (OAS)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should add allowedTargets: TypeDeclaration facet to this

you are allowed to define for any operation.

type: string[]
allowedTargets: Method

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it is only allowed to tag methods?

@sichvoge
Copy link
Contributor Author

Hi, Pavel. Thanks for your comments. I agree to the more generic deprecation, but for example tags has specific nodes only to OAS (externalDocs). That's why I suggest to not separate them using the same syntax.

What potential problems do you see with oas-parameter-collection-format and oas-x-?

@petrochenko-pavel-a
Copy link

petrochenko-pavel-a commented Jun 30, 2017

but for example tags has specific nodes only to OAS

that's true, but on other side, let's say that we have a console which would like to group resources by tags, it would be nice if it will be able to understand oas-tags, without additional modifications. One option might be to create a generic tags annotation and request oas->raml to generate both.

What potential problems do you see with oas-parameter-collection-format and oas-x-?

The one with oas-parameter-collection-format. The problem is that serialization of scalar arrays is well defined in RAML and more over it depends from a position. So when you have an array of numbers in OAS which is serialised by csv collection format and is used as queryParameter you should convert it to query parameter of type string and probably add one more annotation which does not exists in this set which describes that this type is actually array of numbers which was converted to string type to overcome limitations of RAML spec.

oas-x-
If I got semantic of this annotation correctly it is a node which gathers all swagger extensions related to the node to which it is applied.

No let's imagine that we have raml->oas->raml conversion chain. I would expect that if my original RAML file has 3 annotations with a names a,b,c I will get same 3 annotations after I will convert my RAML file to OAS and then back to RAML. So as for me semantic of this annotation breaks possibility for doing loss free road trip from RAML to OAS and back

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

Successfully merging this pull request may close these issues.

None yet

2 participants