Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#227 Better code completion proposal for name/key of additional properties #402

Merged
merged 28 commits into from
Oct 6, 2017

Conversation

tfesenko
Copy link
Member

@tfesenko tfesenko commented Oct 4, 2017

This PR provides better completion proposals and code templates for properties described as additionalProperties in OAS3 and Swagger v3

OAS3

Code assist

(<element> name) is proposed for elements described with additionalProperties in JSON Schema. When chosen, the new name is selected for editing.
In this screencast, code assist suggests (schema name) for a schema and then (property name) for a schema describing a property:
additionalpropertiesassist2
Similar approach applies to other components (callbacks, links, examples, etc) and other additionalProperties:
screen shot 2017-10-04 at 1 23 35 pm
I am not attaching screenshots for all supported cases as I have provided tests for them.

Tests

OpenApi3ContentAssistProcessorTest provides tests for all major cases:

  • Schema
  • Schema Property
  • All other components (Links, Callbacks, Parameters, Examples, RequestBodies, Headers, SecuritySchemes)
  • Media types
  • Encodings
  • Server variables

Note that some elements still have strange names:

  • Discriminator#mappings: _type_
  • OauthFlow#Scopes: _type_
  • Parameter names in Links: (any name)

Current status is covered by tests.

Implementation

JsonProposalProvider creates proposal for the additionalProperties elements. The label is described by a new TypeDefinition#getLabel() which tried to get needed information from JSON Schema. For the schema property, we can't infer the needed name - property - from the schema, so I added a title property to JSON Schema.

JSON Schema

I had to add one line to the JSON Schema to provide a correct (property name) instead of (schema name)
To do it, I updated our JSON Schema generator. It's generated using commit 83638080c70784ca84fcfbf86af948c6e98bcbba on
https://github.com/RepreZen/gnostic/tree/task/KZOE-227

Code templates

Named schemas and properties now have code templates for major cases. These code templates are generated by Activator.addNamedSchemaTemplatesInSchemas() and addNamedSchemaTemplatesInSchemaProperties() using already existing code templates defined for inline schemas in templates.xml.
I also added code template for a schema ref for schema properties.
additionalpropertiestemplates

Tests

  • I added a new code-template context for schema properties to SchemaObject.yaml. It means that our JUnit tests verify that:
  1. This context exists at a given location
  2. All code templates provided for this context at this location will produce a valid OAS3 documentation.

Known issues

Restore Defaults on KaiZen>OpenAPI v3/Templates preferences page erases synthetic code templates generated for named schema and named property.

Implementation

  • I added a new context for named schema properties
  • Activator.addNamedSchemaTemplatesInSchemas() and addNamedSchemaTemplatesInSchemaProperties() build code templates dynamically
  • Usually we use template variable to define template parts that need to be selected. Variable names don't allow spaces or special characters, and we need it to be something like (schema name). So I added ElementNameResolver to handle it.

Swagger v2

Code assist

Similar code assist experience was implemented for Swagger v2:
screen shot 2017-10-04 at 2 03 24 pm
screen shot 2017-10-04 at 2 03 39 pm
I provided several tests for (schema name) and (property name) in SwaggerProposalProviderTest, but I did not test it as thorough as foer OAS 3.

Code templates

I did NOT provide synthetic code templates for named schemas in Swagger v2. Anyway, only one code template is available for a named schema,

JSON Schema

Again, JSON Schema was modified to provide a correct value for (property name), but as JSON Schema for Swagger v2 is not generated, there is no need to update the schema generator.

… format '(schema name)' and select it for editing
@tfesenko
Copy link
Member Author

tfesenko commented Oct 4, 2017

@ghillairet , @tedepstein , this PR is ready for review.

@tedepstein
Copy link
Collaborator

Looks good, Tanya! I will plan to test it after it has passed code review and been merged.

Copy link
Member

@ghillairet ghillairet left a comment

Choose a reason for hiding this comment

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

Passed QA, all tests passed as well.

I just left a comment about a possible NPE.

Also I noted that there is a content assist proposal _key_ present in responses > content > schema. Not sure why it is there.

screen shot 2017-10-06 at 15 19 39


@Override
public String getLabel() {
return Iterables.getFirst(complexTypes, null).getLabel();
Copy link
Member

Choose a reason for hiding this comment

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

You will get a NPE if complexTypes is null here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Addressed in 9f2cd7d

@tfesenko
Copy link
Member Author

tfesenko commented Oct 6, 2017

@ghillairet , thanks for reporting a case with a _key_ proposal for an anonymous schema in media types. I can't reproduce it - I even created a test for it in 3a4675d.
Can you please double-check it and share the OAS3 file with me?

…perties

Add test to show that _key_ is still present in content assist for response content schema.
@ghillairet
Copy link
Member

@tfesenko See my commit that adds a test to show that key is still present for response content schema.

@tfesenko
Copy link
Member Author

tfesenko commented Oct 6, 2017

@ghillairet , I extracted #411 "_type_ as context assist proposal for context schema" which can be addressed later.
Merging this PR as it passed your code review and QA.

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

Successfully merging this pull request may close these issues.

3 participants