Make restApiVersion on XContentBuilder final #70878
Conversation
When passing in restApiVersion during creation of XContentBuilder it makes it more clear that this field is final. This prevents accidental change of the version during the xcontent creation. The withCompatibleVersion method can also be removed, since the field only needs to be set in constructor.
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
…h_compatible_version
|
@elasticmachine update branch |
There was a problem hiding this comment.
Changes look good, I like that you were able to find an non-intrusrive solution here.
One nitpick about java doc, and I would have (very) minor preference to expose an additional constructor (i.e. preserve the original and default current API version) so that the cases that don't care about REST API compatibility can remain blissfully ignorant of it.
| @@ -188,13 +198,15 @@ public XContentBuilder(XContentType xContentType, OutputStream bos, Set<String> | |||
| * @param includes the inclusive filters: only fields and objects that match the inclusive filters will be written to the output. | |||
| * @param excludes the exclusive filters: only fields and objects that don't match the exclusive filters will be written to the output. | |||
| * @param responseContentType a content-type header value to be send back on a response | |||
| * @param restApiVersion | |||
| @@ -511,7 +512,7 @@ public static InputStream filterToXContent(HttpRequest request, XContentType xCo | |||
| try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); | |||
| XContentBuilder filteredBuilder = new XContentBuilder(xContentType.xContent(), bos, | |||
| Collections.emptySet(), Collections.singleton(excludeField), | |||
| xContentType.toParsedMediaType())) { | |||
| xContentType.toParsedMediaType(), RestApiVersion.current())) { | |||
There was a problem hiding this comment.
I hate the idea of more constructors ... but I wonder if it makes sense here to de-emphasize that RestApiVersion is non-consequential.
The additional constructor, would be the original constructor prior to the PR that would set RestAPIVersion to current(). That way use cases like this (and enrich) that don't care about REST API compatibility don't need to concern themselves here.
…omulka/elasticsearch into compat/remove_with_compatible_version
When passing in restApiVersion during creation of XContentBuilder
it makes it more clear that this field is final.
This prevents accidental change of the version during the xcontent
creation.
The withCompatibleVersion method can also be removed, since the field
only needs to be set in constructor.
relates #51816