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.

Sign up for GitHub

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

[bug][html2] Fix visibility of body/response schemas #5643

Merged
merged 5 commits into from Apr 19, 2020
Merged

[bug][html2] Fix visibility of body/response schemas #5643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d8790d9
1441 fix visibility of body/response schemas
damian-zaleski Mar 20, 2020
949dfd6
Remove non-project files
jimschubert Apr 19, 2020
452029f
[html] Handle schemas with array items
jimschubert Apr 19, 2020
d7f66c2
[html2] Point to template directory in bin script
jimschubert Apr 19, 2020
f7681ca
[html2] Regenerate sample
jimschubert Apr 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/html2-petstore.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ fi

# if you've executed sbt assembly previously it will use that instead.
export JAVA_OPTS="${JAVA_OPTS} -Xmx1024M -DloggerPath=conf/log4j.properties"
ags="generate -i modules/openapi-generator/src/test/resources/2_0/petstore.yaml -g html2 -o samples/documentation/html2 --additional-properties hideGenerationTimestamp=true $@"
ags="generate -i modules/openapi-generator/src/test/resources/2_0/petstore.yaml -g html2 -o samples/documentation/html2 -t modules/openapi-generator/src/main/resources/htmlDocs2/ --additional-properties hideGenerationTimestamp=true $@"

java $JAVA_OPTS -jar $executable $ags
7 changes: 7 additions & 0 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5482,13 +5482,20 @@ public CodegenParameter fromRequestBody(RequestBody body, Set<String> imports, S
setParameterNullable(codegenParameter, codegenProperty);
}

addJsonSchemaForBodyRequestInCaseItsNotPresent(codegenParameter, body);

// set the parameter's example value
// should be overridden by lang codegen
setParameterExampleValue(codegenParameter, body);

return codegenParameter;
}

private void addJsonSchemaForBodyRequestInCaseItsNotPresent(CodegenParameter codegenParameter, RequestBody body){
if(codegenParameter.jsonSchema == null)
codegenParameter.jsonSchema = Json.pretty(body);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if you can share a minimal spec to reproduce the issue fixed by this change?

Copy link
Member

Choose a reason for hiding this comment

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

I commented on my phone, and this was attached at the PR level rather than response here. I'm duplicating my response here in case you didn't see it.

This affects every spec. You can see the current samples, none of which display body parameter information.

}

protected void addOption(String key, String description, String defaultValue) {
CliOption option = new CliOption(key, description);
if (defaultValue != null)
Expand Down
35 changes: 32 additions & 3 deletions modules/openapi-generator/src/main/resources/htmlDocs2/index.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,31 @@
//Convert elements with "marked" class to markdown
processMarked();
});
function findNode(id, currentNode) {
return (Object.keys(currentNode)[0] === id) ? currentNode : findNodeInChildren(id, currentNode);
}
function findNodeInChildren(id, currentNode) {
for (let prop in currentNode) {
if (currentNode.hasOwnProperty(prop)) {
let currentChild = currentNode[prop];
if (id === prop) {
return currentChild;
} else {
// Search in the current child
if (typeof (currentChild) === 'object') {
let result = findNode(id, currentChild);
if (result !== false) {
return result;
}
}
}
}
}
return false;
}
</script>
<style type="text/css">
{{>fonts}}
Expand Down Expand Up @@ -416,9 +441,14 @@
<script>
$(document).ready(function() {
var schemaWrapper = {{{jsonSchema}}};
var schema = schemaWrapper.schema;
var schema = findNode('schema',schemaWrapper).schema;
if (!schema) {
schema = schemaWrapper.schema;
}
if (schema.$ref != null) {
schema = defsParser.$refs.get(schema.$ref);
} else if (schema.items != null && schema.items.$ref != null) {
schema.items = defsParser.$refs.get(schema.items.$ref);
} else {
Copy link

Choose a reason for hiding this comment

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

This does not cover array type, e.g.:

schema:
  type: array
  items:
    ref: '#/components/schemas/Pet'

So I had to add one more if statement:

...
} else if (schema.type === 'array' && schema.items != null && schema.items.$ref != null) {
    schema.items = defsParser.$refs.get(schema.items.$ref);
} else {
    // line 450
}

Copy link
Author

Choose a reason for hiding this comment

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

You're totally right. I tested it on two different versions of the OpenApi generator and on the older one there was no issue with arrays, so I didn't cover it up, but that's a great change.

Copy link
Member

Choose a reason for hiding this comment

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

I've applied the suggested fix to this branch.

schemaWrapper.definitions = Object.assign({}, defs);
$RefParser.dereference(schemaWrapper).catch(function(err) {
Expand Down Expand Up @@ -505,8 +535,7 @@
{{>js_json_stringify_safe}}
{{>js_webfontloader}}
<script>
var schemaWrapper = {};
schemaWrapper.definitions = Object.assign({}, defs);
var schemaWrapper = { "components": { "schemas" : defs}};
defsParser = new $RefParser();
defsParser.dereference(schemaWrapper).catch(function(err) {
console.log(err);
Expand Down
5 changes: 4 additions & 1 deletion modules/openapi-generator/src/main/resources/htmlDocs2/paramB.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
<script>
$(document).ready(function() {
var schemaWrapper = {{{jsonSchema}}};
var schema = schemaWrapper.schema;
var schema = findNode('schema', schemaWrapper).schema;
if (!schema) {
schema = schemaWrapper.schema;
}
if (schema.$ref != null) {
schema = defsParser.$refs.get(schema.$ref);
} else {
Expand Down
2 changes: 1 addition & 1 deletion samples/documentation/html2/.openapi-generator/VERSION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.2.1-SNAPSHOT
4.3.0-SNAPSHOT
Loading