Skip to content

Bugfix/3248#3857

Closed
peyerroger wants to merge 2 commits intoOpenAPITools:masterfrom
peyerroger:bugfix/3248
Closed

Bugfix/3248#3857
peyerroger wants to merge 2 commits intoOpenAPITools:masterfrom
peyerroger:bugfix/3248

Conversation

@peyerroger
Copy link
Contributor

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

(details of the change, additional tests that have been done, reference to the issue for tracking, etc)

Copy link
Member

@jimschubert jimschubert left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I've left a few comments.

* @param schema potentially containing a '$ref'
* @return true if it's a model with at least one properties
*/
public static boolean isModel(Schema schema) {
Copy link
Member

Choose a reason for hiding this comment

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

Removing a public helper is a breaking change (these are used by programmatic consumers, and this would therefore need to be deprecated before removal).

// property.baseType = getSimpleRef(p.get$ref());
//}
// --END of revision
property.isModel = ModelUtils.isModel(p);
Copy link
Member

Choose a reason for hiding this comment

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

If you're changing the template to not use isModel, we should keep this as the change affects all generators. See my resolution in #3855, which sets isModel as expected (after the naive attempt done in setNonArrayMapProperty.

}

@Test
public void objectQueryParamIdentifyAsObject() {
Copy link
Member

Choose a reason for hiding this comment

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

NOTE: Removing this test/functionality would be considered a breaking change and would need to be documented as a regression.

@peyerroger peyerroger closed this Sep 8, 2019
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.

2 participants

Comments