-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
[Protobuf] Fix Discriminator Issue and add capability Enum Extraction #22740
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
base: master
Are you sure you want to change the base?
[Protobuf] Fix Discriminator Issue and add capability Enum Extraction #22740
Conversation
This PR fixes a critical bug in the protobuf schema generator where models using discriminators with �llOf composition were generating invalid import paths when child schemas contained references to other models.
* Improve management of inheritance * Improve management of discriminator * Allow to separate inline enums in external files * Add unit test
…m/Ocsidot/openapi-generator into fix/protobuf-discriminator-issues
Fix issue linked to enum in array when there is inheritance or discriminator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 11 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/test/resources/3_0/protobuf-schema/pet.proto">
<violation number="1" location="modules/openapi-generator/src/test/resources/3_0/protobuf-schema/pet.proto:15">
P1: Imported proto file `models/characteristics.proto` is missing, leaving `Characteristics` unresolved and making the schema uncompilable.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| package openapitools; | ||
|
|
||
| import public "models/characteristics.proto"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Imported proto file models/characteristics.proto is missing, leaving Characteristics unresolved and making the schema uncompilable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/test/resources/3_0/protobuf-schema/pet.proto, line 15:
<comment>Imported proto file `models/characteristics.proto` is missing, leaving `Characteristics` unresolved and making the schema uncompilable.</comment>
<file context>
@@ -12,14 +12,20 @@ syntax = "proto3";
package openapitools;
+import public "models/characteristics.proto";
+
message Pet {
</file context>
Overview
This PR resolves critical a critical issue in the Protobuf schema code generator related to discriminator handling.
Following this fix, during our investigation we also found another issue related on enum management: without extrating enum in separated messages, if enums have similar values, a conflict appears. Thus, we propose to add a new parameter to allow enum extraction, and proper support for enums within array types. The changes ensure correct protobuf generation when models use inheritance, discriminators, and inline/extracted enums.
Issues Fixed
This PR addresses multiple related issues in protobuf code generation:
extractEnumsToSeparateFilesoptionChanges Made
1. Improved Protobuf Generation Core
2. Fixed Enum Value Collision
3. Added Array of Enums Support
property.isArray && property.items.isEnum)ParentModel_FieldName.Enum) to array items4. Fixed Enum Extraction in Complex Scenarios
.EnumsuffixEnum Extraction Pattern
When
extractEnumsToSeparateFilesis enabled, the generator now properly wraps inline enums to prevent naming collisions:Testing
ProtobufSchemaCodegenTestFiles Modified
modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/ProtobufSchemaCodegen.javamodules/openapi-generator/src/test/java/org/openapitools/codegen/languages/protobuf/ProtobufSchemaCodegenTest.javaBackwards Compatibility
✅ Maintains full backwards compatibility:
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)Summary by cubic
Fixes discriminator handling and import paths in the Protobuf schema generator, and adds an option to extract enums to separate files with full support for arrays. This prevents enum name collisions and generates valid schemas for models using inheritance and discriminators.
Bug Fixes
New Features
Written for commit 0902089. Summary will update on new commits.