Make restApiVersion on XContentBuilder final (#70878)

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

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java

@@ -53,6 +53,24 @@ public final class XContentBuilder implements Closeable, Flushable {
         return new XContentBuilder(xContent, new ByteArrayOutputStream());
     }
 
+    /**
+     * Create a new {@link XContentBuilder} using the given {@link XContent} content and RestApiVersion.
+     * <p>
+     * The builder uses an internal {@link ByteArrayOutputStream} output stream to build the content.
+     * </p>
+     *
+     * @param xContent the {@link XContent}
+     * @return a new {@link XContentBuilder}
+     * @throws IOException if an {@link IOException} occurs while building the content
+     */
+    public static XContentBuilder builder(XContent xContent, RestApiVersion restApiVersion) throws IOException {
+        return new XContentBuilder(xContent, new ByteArrayOutputStream(),
+            Collections.emptySet(),
+            Collections.emptySet(),
+            xContent.type().toParsedMediaType(),
+            restApiVersion);
+    }
+
     /**
      * Create a new {@link XContentBuilder} using the given {@link XContentType} xContentType and some inclusive and/or exclusive filters.
      * <p>
@@ -68,7 +86,7 @@ public final class XContentBuilder implements Closeable, Flushable {
      */
     public static XContentBuilder builder(XContentType xContentType, Set<String> includes, Set<String> excludes) throws IOException {
         return new XContentBuilder(xContentType.xContent(), new ByteArrayOutputStream(), includes, excludes,
-            xContentType.toParsedMediaType());
+            xContentType.toParsedMediaType(), RestApiVersion.current());
     }
 
     private static final Map<Class<?>, Writer> WRITERS;
@@ -152,21 +170,23 @@ public final class XContentBuilder implements Closeable, Flushable {
      */
     private final OutputStream bos;
 
+    private final RestApiVersion restApiVersion;
+
+    private final ParsedMediaType responseContentType;
+
     /**
      * When this flag is set to true, some types of values are written in a format easier to read for a human.
      */
     private boolean humanReadable = false;
 
-    private RestApiVersion restApiVersion;
 
-    private ParsedMediaType responseContentType;
 
     /**
      * Constructs a new builder using the provided XContent and an OutputStream. Make sure
      * to call {@link #close()} when the builder is done with.
      */
     public XContentBuilder(XContent xContent, OutputStream bos) throws IOException {
-        this(xContent, bos, Collections.emptySet(), Collections.emptySet(), xContent.type().toParsedMediaType());
+        this(xContent, bos, Collections.emptySet(), Collections.emptySet(), xContent.type().toParsedMediaType(), RestApiVersion.current());
     }
     /**
      * Constructs a new builder using the provided XContent, an OutputStream and
@@ -175,7 +195,7 @@ public final class XContentBuilder implements Closeable, Flushable {
      * {@link #close()} when the builder is done with.
      */
     public XContentBuilder(XContentType xContentType, OutputStream bos, Set<String> includes) throws IOException {
-        this(xContentType.xContent(), bos, includes, Collections.emptySet(), xContentType.toParsedMediaType());
+        this(xContentType.xContent(), bos, includes, Collections.emptySet(), xContentType.toParsedMediaType(), RestApiVersion.current());
     }
 
     /**
@@ -191,10 +211,30 @@ public final class XContentBuilder implements Closeable, Flushable {
      */
     public XContentBuilder(XContent xContent, OutputStream os, Set<String> includes, Set<String> excludes,
                            ParsedMediaType responseContentType) throws IOException {
+        this(xContent, os, includes, excludes, responseContentType, RestApiVersion.current());
+    }
+
+    /**
+     * Creates a new builder using the provided XContent, output stream and some inclusive and/or exclusive filters. When both exclusive and
+     * inclusive filters are provided, the underlying builder will first use exclusion filters to remove fields and then will check the
+     * remaining fields against the inclusive filters.
+     * Stores RestApiVersion to help steer the use of the builder depending on the version.
+     * @see #getRestApiVersion()
+     * <p>
+     * Make sure to call {@link #close()} when the builder is done with.
+     * @param os       the output stream
+     * @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 a rest api version indicating with which version the XContent is compatible with.
+     */
+    public XContentBuilder(XContent xContent, OutputStream os, Set<String> includes, Set<String> excludes,
+                           ParsedMediaType responseContentType, RestApiVersion restApiVersion) throws IOException {
         this.bos = os;
         assert responseContentType != null : "generated response cannot be null";
         this.responseContentType = responseContentType;
         this.generator = xContent.createGenerator(bos, includes, excludes);
+        this.restApiVersion = restApiVersion;
     }
 
     public String getResponseContentTypeString() {
@@ -1006,16 +1046,6 @@ public final class XContentBuilder implements Closeable, Flushable {
         return this;
     }
 
-    /**
-     * Sets a version used for serialising a response compatible with a previous version.
-     * @param restApiVersion - indicates requested a version of API that the builder will be creating
-     */
-    public XContentBuilder withCompatibleVersion(RestApiVersion restApiVersion) {
-        assert this.restApiVersion == null : "restApiVersion has already been set";
-        Objects.requireNonNull(restApiVersion, "restApiVersion cannot be null");
-        this.restApiVersion = restApiVersion;
-        return this;
-    }
 
     /**
      * Returns a version used for serialising a response.

server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java

@@ -126,7 +126,7 @@ public abstract class AbstractRestChannel implements RestChannel {
 
         XContentBuilder builder =
             new XContentBuilder(XContentFactory.xContent(responseContentType), unclosableOutputStream,
-                includes, excludes, responseMediaType);
+                includes, excludes, responseMediaType, request.getRestApiVersion());
         if (pretty) {
             builder.prettyPrint().lfAtEnd();
         }

server/src/main/java/org/elasticsearch/rest/RestController.java

@@ -268,7 +268,7 @@ public class RestController implements HttpServerTransport.Dispatcher {
                 inFlightRequestsBreaker(circuitBreakerService).addWithoutBreaking(contentLength);
             }
             // iff we could reserve bytes for the request we need to send the response also over this channel
-            responseChannel = new ResourceHandlingHttpChannel(channel, circuitBreakerService, contentLength, restApiVersion);
+            responseChannel = new ResourceHandlingHttpChannel(channel, circuitBreakerService, contentLength);
             // TODO: Count requests double in the circuit breaker if they need copying?
             if (handler.allowsUnsafeBuffers() == false) {
                 request.ensureSafeBuffers();
@@ -492,40 +492,33 @@ public class RestController implements HttpServerTransport.Dispatcher {
         private final RestChannel delegate;
         private final CircuitBreakerService circuitBreakerService;
         private final int contentLength;
-        private final RestApiVersion restApiVersion;
         private final AtomicBoolean closed = new AtomicBoolean();
 
-        ResourceHandlingHttpChannel(RestChannel delegate, CircuitBreakerService circuitBreakerService, int contentLength,
-                                    RestApiVersion restApiVersion) {
+        ResourceHandlingHttpChannel(RestChannel delegate, CircuitBreakerService circuitBreakerService, int contentLength) {
             this.delegate = delegate;
             this.circuitBreakerService = circuitBreakerService;
             this.contentLength = contentLength;
-            this.restApiVersion = restApiVersion;
         }
 
         @Override
         public XContentBuilder newBuilder() throws IOException {
-            return delegate.newBuilder()
-                .withCompatibleVersion(restApiVersion);
+            return delegate.newBuilder();
         }
 
         @Override
         public XContentBuilder newErrorBuilder() throws IOException {
-            return delegate.newErrorBuilder()
-                .withCompatibleVersion(restApiVersion);
+            return delegate.newErrorBuilder();
         }
 
         @Override
         public XContentBuilder newBuilder(@Nullable XContentType xContentType, boolean useFiltering) throws IOException {
-            return delegate.newBuilder(xContentType, useFiltering)
-                .withCompatibleVersion(restApiVersion);
+            return delegate.newBuilder(xContentType, useFiltering);
         }
 
         @Override
         public XContentBuilder newBuilder(XContentType xContentType, XContentType responseContentType, boolean useFiltering)
             throws IOException {
-            return delegate.newBuilder(xContentType, responseContentType, useFiltering)
-                .withCompatibleVersion(restApiVersion);
+            return delegate.newBuilder(xContentType, responseContentType, useFiltering);
         }
 
         @Override

server/src/test/java/org/elasticsearch/action/DocWriteResponseTests.java

@@ -110,8 +110,7 @@ public class DocWriteResponseTests extends ESTestCase {
         ) {
             // DocWriteResponse is abstract so we have to sneak a subclass in here to test it.
         };
-        try (XContentBuilder builder = JsonXContent.contentBuilder()) {
-            builder.withCompatibleVersion(RestApiVersion.V_7);
+        try (XContentBuilder builder = XContentBuilder.builder(JsonXContent.jsonXContent, RestApiVersion.V_7)) {
             response.toXContent(builder, ToXContent.EMPTY_PARAMS);
 
             try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) {
@@ -119,8 +118,7 @@ public class DocWriteResponseTests extends ESTestCase {
             }
         }
 
-        try (XContentBuilder builder = JsonXContent.contentBuilder()) {
-            builder.withCompatibleVersion(RestApiVersion.V_8);
+        try (XContentBuilder builder = XContentBuilder.builder(JsonXContent.jsonXContent, RestApiVersion.V_8)) {
             response.toXContent(builder, ToXContent.EMPTY_PARAMS);
 
             try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) {
@@ -128,5 +126,4 @@ public class DocWriteResponseTests extends ESTestCase {
             }
         }
     }
-
 }

server/src/test/java/org/elasticsearch/index/get/GetResultTests.java

@@ -96,8 +96,7 @@ public class GetResultTests extends ESTestCase {
                 singletonList("value1"))), singletonMap("field1", new DocumentField("metafield",
                 singletonList("metavalue"))));
 
-            try (XContentBuilder builder = JsonXContent.contentBuilder()) {
-                builder.withCompatibleVersion(RestApiVersion.V_7);
+            try (XContentBuilder builder = XContentBuilder.builder(JsonXContent.jsonXContent, RestApiVersion.V_7)) {
                 getResult.toXContent(builder, ToXContent.EMPTY_PARAMS);
                 String output = Strings.toString(builder);
                 assertEquals("{\"_index\":\"index\",\"_type\":\"_doc\",\"_id\":\"id\",\"_version\":1,\"_seq_no\":0,\"_primary_term\":1," +
@@ -108,8 +107,7 @@ public class GetResultTests extends ESTestCase {
         {
             GetResult getResult = new GetResult("index", "id", UNASSIGNED_SEQ_NO, 0, 1, false, null, null, null);
 
-            try (XContentBuilder builder = JsonXContent.contentBuilder()) {
-                builder.withCompatibleVersion(RestApiVersion.V_7);
+            try (XContentBuilder builder = XContentBuilder.builder(JsonXContent.jsonXContent, RestApiVersion.V_7)) {
                 getResult.toXContent(builder, ToXContent.EMPTY_PARAMS);
                 String output = Strings.toString(builder);
                 assertEquals("{\"_index\":\"index\",\"_type\":\"_doc\",\"_id\":\"id\",\"found\":false}", output);

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/action/EqlSearchResponse.java

@@ -204,7 +204,7 @@ public class EqlSearchResponse extends ActionResponse implements ToXContentObjec
 
         @SuppressWarnings("unchecked")
         private static final ConstructingObjectParser<Event, Void> PARSER =
-                new ConstructingObjectParser<>("eql/search_response_event", true, 
+                new ConstructingObjectParser<>("eql/search_response_event", true,
                     args -> new Event((String) args[0], (String) args[1], (BytesReference) args[2], (Map<String, DocumentField>) args[3]));
 
         static {

x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/action/EqlSearchResponseTests.java

@@ -233,7 +233,7 @@ public class EqlSearchResponseTests extends AbstractBWCWireSerializingTestCase<E
                 mutatedSequences.add(new Sequence(s.joinKeys(), mutateEvents(s.events(), version)));
             }
         }
-        
+
         return new EqlSearchResponse(new EqlSearchResponse.Hits(mutatedEvents, mutatedSequences, instance.hits().totalHits()),
             instance.took(), instance.isTimeout(), instance.id(), instance.isRunning(), instance.isPartial());
     }