fix(java): stop hardcoding failOnUnknownProperties, and turn it off - #26
Open
gabooh wants to merge 1 commit into
Open
fix(java): stop hardcoding failOnUnknownProperties, and turn it off#26gabooh wants to merge 1 commit into
gabooh wants to merge 1 commit into
Conversation
`templates/Java/libraries/jersey3/JSON.mustache` replaced the generator's
`{{failOnUnknownProperties}}` placeholder with a literal `true`, so the built
client rejects any response property it does not know:
UnrecognizedPropertyException: Unrecognized field
"some_field_from_a_newer_server" (class …model.SearchResponse),
not marked as ignorable
That is not a degraded read — the whole response is lost over one extra field.
Three things in this repository already say it should be otherwise:
* the generator's own default for this option is false, and hardcoding the
value means `--additional-properties failOnUnknownProperties=…` cannot
reach it at all;
* every other Java library template here leaves unknown properties alone —
apache-httpclient, native, restclient, vertx, feign, resteasy,
rest-assured, webclient, google-api-client, and the shared
ApiClient.mustache;
* the schema declares `additionalProperties: true` on `searchResponse`,
`aggBucketsResult` and `aggBucket`. The generated Python client honours it
(`additional_properties`); the Java client contradicts it.
And the README's compatibility table promises that a client stays PARTIALLY
compatible with a newer Manticore Search. With strict deserialization that
state cannot exist: the first response field a newer server adds turns every
search into an exception.
Restore the placeholder on the jersey3 template (the library `build.sh` builds
with) and pass the option explicitly, so the value is chosen in one visible
place rather than frozen in a template. jersey2 and retrofit2 carry the same
hardcoded literal; they are left alone here since nothing builds with them.
This makes an unknown property harmless, not readable — a field absent from the
schema is dropped. Fields that callers need still have to be modelled; this
only stops one of them from costing the response.
Verified by regenerating with OpenAPI Generator 7.17.0 (the version in
./generator-versions), confirming the rendered `JSON.java` now configures the
feature to false, and running the new test against the regenerated output. Both
cases in it throw on the published 10.2.0 client.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
templates/Java/libraries/jersey3/JSON.mustachereplaced the generator's{{failOnUnknownProperties}}placeholder with a literaltrue, so the built client rejects anyresponse property it does not know:
That is not a degraded read. One extra field costs the caller the entire response.
Four things in this repository already say it should be otherwise
false, and hardcoding the value means--additional-properties failOnUnknownProperties=…cannot reach it at all.apache-httpclient,native,restclient,vertx,feign,resteasy,rest-assured,webclient,google-api-client, and the sharedApiClient.mustache. Onlyjersey3(the librarybuild.shactually builds with),
jersey2andretrofit2carry the hardcoded literal.additionalProperties: trueonsearchResponse,aggBucketsResultandaggBucket. The generated Python client honours it (additional_properties), the Go clienthonours it (
AdditionalProperties map[string]interface{}) — the Java client contradicts it.newer Manticore Search. With strict deserialization that state cannot exist: the first response
field a newer server adds turns every search into an exception.
The change
Restore the placeholder on the
jersey3template and pass the option explicitly indo_java(), sothe value is chosen in one visible place rather than frozen in a template.
jersey2andretrofit2carry the same literal and are left alone here, since nothing builds with them — happy to include
them if you would rather they move together.
What this does and does not do
It makes an unknown property harmless, not readable: a field absent from the schema is dropped.
Fields that callers actually need still have to be modelled — which is a separate concern, and the
subject of #25. Neither change makes the other unnecessary: #25 alone leaves the client brittle
against the next new field, and this one alone would silently drop the metric aggregation values
that #25 makes readable.
How it was verified
Regenerated with OpenAPI Generator 7.17.0 (the version in
./generator-versions), confirmed therendered
JSON.javanow configures the feature tofalse, and ran the new test against theregenerated output. Both of its cases throw on the published 10.2.0 client.
out/is deliberatelynot included.