From ae7185a1f9ce2bcec7da854f6b25c496cb99bdd3 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Fri, 18 Jan 2019 14:16:49 -0600 Subject: [PATCH 1/4] Update verify repository to allow unknown fields The subparser in verify repository allows for unknown fields. This commit sets the value to true for the parser and modifies the test such that it accurately tests it. Relates #36938 --- .../VerifyRepositoryResponseTests.java | 84 +++++++++++++++++++ .../verify/VerifyRepositoryResponse.java | 24 +++--- 2 files changed, 96 insertions(+), 12 deletions(-) create mode 100644 client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java new file mode 100644 index 0000000000000..2f895ebab2daa --- /dev/null +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java @@ -0,0 +1,84 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.client.watcher; + +import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryResponse; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.XContentTestUtils; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import static org.elasticsearch.test.AbstractXContentTestCase.xContentTester; + +public class VerifyRepositoryResponseTests extends ESTestCase { + + public void testFromXContent() throws IOException { + xContentTester(this::createParser, + VerifyRepositoryResponseTests::createTestInstance, + VerifyRepositoryResponseTests::toXContent, + VerifyRepositoryResponse::fromXContent) + .supportsUnknownFields(false) + .shuffleFieldsExceptions(new String[] {"nodes"}) // do not mix up the order of nodes, it will cause the tests to fail + .assertToXContentEquivalence(false) + .test(); + } + + private static VerifyRepositoryResponse createTestInstance() { + List nodes = new ArrayList<>(); + for (int i = 0; i < randomIntBetween(0, 2); i++) { + nodes.add(new VerifyRepositoryResponse.NodeView(randomAlphaOfLength(5), randomAlphaOfLength(5))); + } + + return new VerifyRepositoryResponse(nodes); + } + + private static XContentBuilder toXContent(VerifyRepositoryResponse.NodeView node, XContentBuilder builder) throws IOException { + // Use a temp builder to create the object we want to add random fields to + XContentBuilder tempBuilder = JsonXContent.contentBuilder().startObject().field("name", node.getName()).endObject(); + BytesReference newBytes = XContentTestUtils.insertRandomFields(XContentType.JSON, BytesReference.bytes(tempBuilder), + null, random()); + + // add the temp object into the actual builder + builder.rawField(node.getNodeId(), newBytes.streamInput(), XContentType.JSON); + + return builder; + } + + private static XContentBuilder toXContent(VerifyRepositoryResponse response, XContentBuilder builder) throws IOException { + builder.startObject(); + { + builder.startObject("nodes"); + { + for (VerifyRepositoryResponse.NodeView node : response.getNodes()) { + toXContent(node, builder); + } + } + builder.endObject(); + } + builder.endObject(); + return builder; + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/verify/VerifyRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/verify/VerifyRepositoryResponse.java index 41835d3e11255..d72136852631f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/verify/VerifyRepositoryResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/verify/VerifyRepositoryResponse.java @@ -48,7 +48,7 @@ public class VerifyRepositoryResponse extends ActionResponse implements ToXConte public static class NodeView implements Writeable, ToXContentObject { private static final ObjectParser.NamedObjectParser PARSER; static { - ObjectParser internalParser = new ObjectParser<>(NODES); + ObjectParser internalParser = new ObjectParser<>(NODES, true, null); internalParser.declareString(NodeView::setName, new ParseField(NAME)); PARSER = (p, v, name) -> internalParser.parse(p, new NodeView(name), null); } @@ -110,7 +110,7 @@ public int hashCode() { private List nodes; private static final ObjectParser PARSER = - new ObjectParser<>(VerifyRepositoryResponse.class.getName(), VerifyRepositoryResponse::new); + new ObjectParser<>(VerifyRepositoryResponse.class.getName(), true, VerifyRepositoryResponse::new); static { PARSER.declareNamedObjects(VerifyRepositoryResponse::setNodes, NodeView.PARSER, new ParseField("nodes")); } @@ -122,6 +122,10 @@ public VerifyRepositoryResponse(DiscoveryNode[] nodes) { this.nodes = Arrays.stream(nodes).map(dn -> new NodeView(dn.getId(), dn.getName())).collect(Collectors.toList()); } + public VerifyRepositoryResponse(List nodes) { + this.nodes = nodes; + } + @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); @@ -168,19 +172,15 @@ public String toString() { } @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { - return false; - } - VerifyRepositoryResponse other = (VerifyRepositoryResponse) obj; - return nodes.equals(other.nodes); + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + VerifyRepositoryResponse that = (VerifyRepositoryResponse) o; + return Objects.equals(nodes, that.nodes); } @Override public int hashCode() { - return nodes.hashCode(); + return Objects.hash(nodes); } } From f788bc838c514a2dc0c60b927216159d07521964 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Fri, 25 Jan 2019 11:32:38 -0600 Subject: [PATCH 2/4] Fixing the test --- .../watcher/VerifyRepositoryResponseTests.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java index 2f895ebab2daa..261b365a35129 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java @@ -40,8 +40,9 @@ public void testFromXContent() throws IOException { VerifyRepositoryResponseTests::createTestInstance, VerifyRepositoryResponseTests::toXContent, VerifyRepositoryResponse::fromXContent) - .supportsUnknownFields(false) + .supportsUnknownFields(true) .shuffleFieldsExceptions(new String[] {"nodes"}) // do not mix up the order of nodes, it will cause the tests to fail + .randomFieldsExcludeFilter((f) -> f.equals("nodes")) // everything in nodes needs to be a particular parseable object .assertToXContentEquivalence(false) .test(); } @@ -56,17 +57,15 @@ private static VerifyRepositoryResponse createTestInstance() { } private static XContentBuilder toXContent(VerifyRepositoryResponse.NodeView node, XContentBuilder builder) throws IOException { - // Use a temp builder to create the object we want to add random fields to - XContentBuilder tempBuilder = JsonXContent.contentBuilder().startObject().field("name", node.getName()).endObject(); - BytesReference newBytes = XContentTestUtils.insertRandomFields(XContentType.JSON, BytesReference.bytes(tempBuilder), - null, random()); - - // add the temp object into the actual builder - builder.rawField(node.getNodeId(), newBytes.streamInput(), XContentType.JSON); - + builder.startObject(node.getNodeId()); + { + builder.field("name", node.getName()); + } + builder.endObject(); return builder; } + private static XContentBuilder toXContent(VerifyRepositoryResponse response, XContentBuilder builder) throws IOException { builder.startObject(); { From e9f4762385d3abc8914647a5a823c43f821bfee6 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Fri, 25 Jan 2019 12:56:47 -0600 Subject: [PATCH 3/4] precommit --- .../client/watcher/VerifyRepositoryResponseTests.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java index 261b365a35129..223b83fa2b7fd 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java @@ -20,12 +20,8 @@ package org.elasticsearch.client.watcher; import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryResponse; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.XContentTestUtils; import java.io.IOException; import java.util.ArrayList; @@ -65,7 +61,6 @@ private static XContentBuilder toXContent(VerifyRepositoryResponse.NodeView node return builder; } - private static XContentBuilder toXContent(VerifyRepositoryResponse response, XContentBuilder builder) throws IOException { builder.startObject(); { From 14a17acf9507ea248a13f39bb5d76b5190898e52 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Wed, 30 Jan 2019 09:08:13 -0600 Subject: [PATCH 4/4] realized there is a toxcontent on the server side object, so using that --- .../VerifyRepositoryResponseTests.java | 23 ++----------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java index 223b83fa2b7fd..72193dc55e9e1 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/watcher/VerifyRepositoryResponseTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.client.watcher; import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryResponse; +import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.test.ESTestCase; @@ -52,27 +53,7 @@ private static VerifyRepositoryResponse createTestInstance() { return new VerifyRepositoryResponse(nodes); } - private static XContentBuilder toXContent(VerifyRepositoryResponse.NodeView node, XContentBuilder builder) throws IOException { - builder.startObject(node.getNodeId()); - { - builder.field("name", node.getName()); - } - builder.endObject(); - return builder; - } - private static XContentBuilder toXContent(VerifyRepositoryResponse response, XContentBuilder builder) throws IOException { - builder.startObject(); - { - builder.startObject("nodes"); - { - for (VerifyRepositoryResponse.NodeView node : response.getNodes()) { - toXContent(node, builder); - } - } - builder.endObject(); - } - builder.endObject(); - return builder; + return response.toXContent(builder, ToXContent.EMPTY_PARAMS); } }