Created by: grokify
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
and./bin/security/{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\
. -
Filed the PR against the correct branch: master
,3.1.x
,4.0.x
. Default:master
. -
Copied the technical committee to review the pull request if your PR is targeting a particular programming language.
@antihax @bvwells
There are some open items listed at the end of this post so it is not yet ready for merging. Please review the general approach.
Description of the PR
When generating a client with a v3.0 spec Schema or v2.0 spec Definition named File
, the generator will convert the type to *os.File
instead of the model named File
. This is a generic issue where a model schema/definition name may collide with a typeMapping
key.
An example spec excerpt looks like:
PoolResponseResource:
type: "object"
properties:
# ...
sourceFiles:
type: "array"
items:
$ref: "#/definitions/File"
File:
type: "object"
properties:
sourceFile:
type: "string"
originalFileName:
type: "string"
In the above, PoolResponseResource
will have a property SourceFiles
with type []*os.File
instead of the desired []File
.
This occurs because abstract class AbstractGoCodegen
converts File
to *os.File
using typeMapping
:
typeMapping.put("File", "*os.File");
typeMapping.put("file", "*os.File");
getSchemaType
and getTypeDeclaration
both check against typeMapping
, e.g.:
if (typeMapping.containsKey(openAPIType)) {
type = typeMapping.get(openAPIType);
getSchemaType
gets a baseline type to process from super.getSchemaType
which returns a single string that appears to represent 3 things: (a) definition/schema names, (b) types, and (c) formats. typeMapping
covers the following:
schema type | language type | spec (2.0) |
---|---|---|
integer |
int32 |
type only: "type": "integer"
|
long |
int64 |
type and format : "type": "integer", "format": "int64"
|
File |
*os.File |
$ref only: "$ref": "#/definitions/File"
|
Proposed Fix
This PR includes a minimal fix and test case to consider the value of $ref
when determining the type. If $ref
is present and matches the Spec 2.0/3.0 definition/schema path, typeMapping
is not used.
Notes
The added test, filePropertyTest
in GoModelTest.java
uses the spec 2.0 model reference: #/definitions/File
, however, when building a client, it appears a 2.0 spec using #/definitions/File
is converted to a 3.0 spec #/components/schemas/File
by super.getSchemaType
References
Open Items / Next Steps
-
This update generates client code conditionally if a spec definition with nameFile
(or other schema name with typeMapping collision) is present, so an additional fake PetStore test endpoint will be provided to test the results.filePropertyTest
added toGoModelTest.java
. -
This currently generates a regression issue that needs to be resolved. A struct will be created with nameFixed in e51623bb . Covered inProfileImageInfoUri
but be referenced asProfileImageInfoURI
. This is not covered in the Petstore tests, but appears using the RingCentral spec. Will resolve and look to add a test case for this.filePropertyTest
added toGoModelTest.java
.