ztai
Minnow
Posts: 8
|
Post by ztai on Jun 26, 2020 0:36:27 GMT
Hi, I tried to generate server code using Redfish OpenAPI schema but failed. Wonder if anyone has tried this recently? The error is about multiple object definitions with same name and indirectly reference. [main] INFO o.o.codegen.DefaultGenerator - Model Error not generated since it's a free-form object
Exception in thread "main" java.lang.StackOverflowError
at org.openapitools.codegen.utils.ModelUtils.hasOrInheritsDiscriminator(ModelUtils.java:1327)
at org.openapitools.codegen.utils.ModelUtils.hasOrInheritsDiscriminator(ModelUtils.java:1338)
...The same error can be reproduced with a set of simplified yaml files in github.com/ztai-goog/bugreport/tree/master/openapi_codegen_1and run the generator command below. docker run --rm \
-v $PWD:/local openapitools/openapi-generator-cli generate \
-i /local/petstore.yaml \
-g go-server \
-o /local/go_server
|
|
|
|
Post by mraineri on Jun 26, 2020 13:52:33 GMT
I haven't tried server-side code; most of my experience with the OpenAPI tools have been with documentation generation, which has been successful for me the last time I tried.
The last time I tried any sort of code generation was pretty early on when the tools didn't really have complete support for external references (or so it seemed to me at the time). I can certainly take a closer look at things now that they should be in a more mature state.
From what your describing, is the issue that "TestType" is defined in both A.yaml and C.yaml? Is there any documented restriction about this? I would think that since the references call out the entire path to the definition, there shouldn't be any restrictions like that.
|
|
ztai
Minnow
Posts: 8
|
Post by ztai on Jun 26, 2020 22:04:12 GMT
Hi mraineri,
It'll be great if you can help try the code generation.
The issue as I see is not only because defined in both A and C, but more likely due to the reference of C's definition of TestType from B.yaml.
|
|
|
|
Post by mraineri on Jun 30, 2020 13:49:07 GMT
I think to ask things a bit differently... In the example you have, petstore.yaml makes the following two references: - TestType within A.yaml, which has its full definition there
- TestType within B.yaml, which then references TestType in C.yaml, and the full definition is found there
I'm assuming that the issue is because "TestType" is defined in multiple places. However, all cases, the references are very direct and there's nothing circular about it; ultimately the reference line terminates at a final definition.
What I'd like to understand before addressing this is if there's an issue with the OpenAPI tools in assuming that all definition names are unique, or if this is a requirement in the OpenAPI spec. From a specification perspective, I cannot find any text that requires this. The closest thing I can find is in the JSON Schema spec, which I know OpenAPI heavily uses:
|
|
ztai
Minnow
Posts: 8
|
Post by ztai on Jun 30, 2020 20:53:49 GMT
Hi mraineri,
I don't see the requirement of OpenAPI specification that a definition has to be unique.
In fact, if B.yaml has the definition instead of using ref from C.yaml, the generator will work.
The reason I put together the example yaml files is because the Redfish yaml fails the generator for the same reason.
A fun fact is the swagger generator (which OpenAPI generator forks from) happily generates code with the sample yaml files. The reason is swagger generator doesn't try to generate the model structs.
|
|
|
|
Post by mraineri on Jul 2, 2020 17:57:11 GMT
I've created a branch of our tool that generates our OpenAPI files from JSON Schema files here: github.com/DMTF/Redfish-Tools/tree/OpenAPI-Rename-TypesAt least on my end, I can now generate server-side code with the tool you used in your original post. If you'd like to try it out, you can do the following to have an updated set of OpenAPI files: - Download and extract the schema bundle: www.dmtf.org/sites/default/files/standards/documents/DSP8010_2020.2.zip
- Run the conversion tool: python3 json-to-openapi.py -I <Path to JSON Schema Folder> -O <Local OpenAPI Path>
- Move the generated openapi.yaml file from the tool's directory to <Local OpenAPI Path>
- Inside of <Local OpenAPI Path>, run the following to fix one external reference to a Swordfish definition: find ./ -type f -exec sed -i -e 's%Volume.RAIDType%RAIDType%g' {} \;
- Inside of <Local OpenAPI Path>, run the following to turn all references to be local: find ./ -type f -exec sed -i -e 's%http://redfish.dmtf.org/schemas/v1/%./%g' {} \;
|
|
|
|
Post by mraineri on Jul 7, 2020 13:26:39 GMT
I forgot one other thing that needs to be done; when you extract the 8010 bundle, you will need to delete the Volume and VolumeCollection schemas from the json-schema folder. "rm Volume*" should do the trick. Volume is currently managed by SNIA, which also has several references to other SNIA schemas, which makes the conversion complex for this testing.
|
|
ztai
Minnow
Posts: 8
|
Post by ztai on Jul 8, 2020 3:26:19 GMT
Thanks a lot for the converter change!
The change is basically to prefix file name to the type names so that they won't all have the same name.
Currently it requires a few extra steps before it can be generated.
I understand changing schema isn't an easy thing to do, especially when there are many versions and cases where a type can be one of many types/versions.
Could you please let me know the plan forward to address this issue?
|
|
|
|
Post by mraineri on Jul 8, 2020 12:18:03 GMT
Yes, exactly; the generators seem to make a flat list of everything, so it also seemed like we needed to make all of the definition names unique.
If the changes look good to you and address your issue, we'll take that feedback into the forum and see how we can logistically do something like this for future releases. Ultimately I would prefer users to simply download and use the 8010 bundle as-is and not require them to do further processing of the schema. I just needed you to do that for the time being to make sure the changes I made fixed your issue.
|
|
ztai
Minnow
Posts: 8
|
Post by ztai on Jul 8, 2020 16:43:16 GMT
The generator simply brings in all reference to local schemas section and due to the lack of namespace, a type with reference to another type with same name clashes.
The fix you had is essentially using filename as namespace.
I can use the 8010 bundle as-is for now.
Longer term we'll probably need to sync with upstream schema, which requires new redfish releases.
|
|
ztai
Minnow
Posts: 8
|
Post by ztai on Jul 9, 2020 4:30:13 GMT
One slight issue about the fix is it breaks the golang type declaration. type AccelerationFunction.v1_0_2.AccelerationFunctionType stringError message go/model_acceleration_function_v1_0_2_acceleration_function_type.go:12:26: syntax error: unexpected . in type declarationI have made a fix to replace the dot with underscore in github.com/ztai-goog/Redfish-Tools/commit/61e76ff4999f00a14dbc0c94b1194af38941232a--------- Another issue is related Enum declaration. Running `go build` in the generated server code fails because enums of same name have been declared in multiple files. The error looks like $ go build
# github.com/GIT_USER_ID/GIT_REPO_ID/go
go/model_account_service_v1_7_1_account_provider_types.go:19:2: OEM redeclared in this block
previous declaration at go/model_acceleration_function_v1_0_2_acceleration_function_type.go:23:57
go/model_account_service_v1_7_1_authentication_types.go:19:2: OEM redeclared in this block
previous declaration at go/model_account_service_v1_7_1_account_provider_types.go:19:47
...
... |
|
|
|
Post by mraineri on Jul 9, 2020 12:12:40 GMT
Great, thanks for testing this further. I'll merge your change into my working branch.
The new error confused me a bit... "OEM" isn't the name of the enumeration, but rather it's a value in an enumeration (and one that we reuse in many places). I'm not sure why having the same enumeration value in different enumerations would be bad... I'll do some more digging on this.
|
|
|
|
Post by mraineri on Jul 9, 2020 13:05:30 GMT
Did a little bit of digging; looks like Go's enum structures are not as tightly controlled as other languages. So, you can easily hit collisions like this. It also seems like others are hitting the same problem with their respective projects. I found this issue in the openapi-generator project: github.com/OpenAPITools/openapi-generator/issues/535There's some guidance in that issue to try using the enumClassPrefix option (-p enumClassPrefix=true). |
|
|
|
Post by mraineri on Jul 9, 2020 13:49:51 GMT
And unfortunately it looks like that additional option is only available for generating Go clients...
|
|
ztai
Minnow
Posts: 8
|
Post by ztai on Jul 10, 2020 2:50:22 GMT
It seems the type generation is unlikely to work properly for redfish schema.
I've tried the following languages
Go: Issues we already know
Python: Generator fails for some of the pattern regex strings
Rust: Free-form objects are not generated but they are still referenced by other generated type structs
NodeJs: No type generation, no router wiring.
It's probably safe to say, better avoid type generation for now.
Without automatic type generation or serialization/deserialization, Node/Js is likely an easier choice.
|
|
