-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
[Protobuf Schema] Map Field Handling in Composed Schemas #21002
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
Conversation
| @Override | ||
| protected void addProperties(Map<String, Schema> properties, List<String> required, Schema schema, Set<Schema> visitedSchemas){ | ||
| super.addProperties(properties, required, schema, visitedSchemas); | ||
| if(schema.getAdditionalProperties() != null) { |
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.
can you please add some comments explaining what the code block does (e.g. what schema(s) it tries to handle) ?
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.
@wing328 thanks for reviewing the PR. I had added explaining comment on top of the code.
Add explanation to code block
| title: field | ||
| type: string | ||
| additionalProperties: | ||
| title: pet |
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.
According to OpenAPI Specification for Reference Object, the $ref cannot be extended with additional properties, so this is incorrect enhancement. I wonder why it was not solved with a compliant way, such as using dedicated vendor extension?
This PR addresses a corner case where a map defined within an composed schema is being ignored during Protobuf generation.
Previously, the pet map (the field defined via additionalProperties) was not being handled properly, resulting in its omission from the generated proto file. the schema like the following:
With this PR, the generator now correctly detects and processes the pet map, ensuring that it is included in the generated Protobuf message.
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)