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

[core] Minor schema validations bug + add pattern property to CodegenResponse #4724

Merged

Conversation

jimschubert
Copy link
Member

Found during discussion of #4653 that DefaultCodegen has an openApi property that may remain unset in certain workflows.

This also adds a pattern property to CodegenResponse to demonstrate the concerns discussed in #4653. Rather than introduce all properties defined by OpenAPI Specification for Schema Validation Properties, I've added the single property for pattern. Adding all will change the contract of our response object for templates, and as I've proposed in the issue I think it would be best to rely on an interface which defines the Json Schema Validation Properties and includes getters/setters on targeted Codegen* models.

Current master's state is this:

  • CodegenParameter exposes Json Schema Validation properties as public fields
  • CodegenProperty exposes Json Schema Validation properties as getters/setters
  • CodegenReponse doesn't expose an API explicitly, but these Json Schema Validation properties are accessible via the schema property. For pattern, this poses an issue because the string is not double escaped to be ready for applying to mustache templates as it is for CodegenParameter/CodegenProperty

This PR will fix the case mentioned in the last bullet point, or at least it gives consistency for this property to template users.

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@auto-labeler
Copy link

auto-labeler bot commented Dec 7, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@jimschubert jimschubert changed the title [core] Minor schema validations bug [core] Minor schema validations bug + add pattern property to CodegenResponse Dec 7, 2019
Copy link
Member

@wing328 wing328 left a comment

Choose a reason for hiding this comment

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

LGTM

@wing328 wing328 added this to the 4.2.3 milestone Dec 9, 2019
@wing328 wing328 merged commit ebf9ba0 into OpenAPITools:master Dec 9, 2019
@jimschubert jimschubert deleted the minor-schema-validations-bug branch December 10, 2019 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants