Skip to content

Commit d826cb3

Browse files
authored
Remove version read/write logic in Verify Response (#30879)
Since master will always communicate with a >=6.4 node, the logic for checking if the node is 6.4 and conditionally reading and writing based on that can be removed from master. This logic will stay in 6.x as it is the bridge to the cleaner response in master. This also unmutes the failing test due to this bwc change. Closes #30807
1 parent 3960a7a commit d826cb3

File tree

4 files changed

+10
-54
lines changed

4 files changed

+10
-54
lines changed

rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.get_repository/10_basic.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ setup:
5151

5252
---
5353
"Verify created repository":
54-
- skip:
55-
version: "all"
56-
reason: AwaitsFix for https://github.com/elastic/elasticsearch/issues/30807
5754
- do:
5855
snapshot.verify_repository:
5956
repository: test_repo_get_2

server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/verify/TransportVerifyRepositoryAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public void onResponse(RepositoriesService.VerifyResponse verifyResponse) {
7373
if (verifyResponse.failed()) {
7474
listener.onFailure(new RepositoryVerificationException(request.name(), verifyResponse.failureDescription()));
7575
} else {
76-
listener.onResponse(new VerifyRepositoryResponse(clusterService.getClusterName(), verifyResponse.nodes()));
76+
listener.onResponse(new VerifyRepositoryResponse(verifyResponse.nodes()));
7777
}
7878
}
7979

server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/verify/VerifyRepositoryResponse.java

Lines changed: 9 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,20 @@
1919

2020
package org.elasticsearch.action.admin.cluster.repositories.verify;
2121

22-
import org.elasticsearch.Version;
2322
import org.elasticsearch.action.ActionResponse;
24-
import org.elasticsearch.cluster.ClusterName;
2523
import org.elasticsearch.cluster.node.DiscoveryNode;
2624
import org.elasticsearch.common.ParseField;
2725
import org.elasticsearch.common.Strings;
2826
import org.elasticsearch.common.io.stream.StreamInput;
2927
import org.elasticsearch.common.io.stream.StreamOutput;
3028
import org.elasticsearch.common.io.stream.Writeable;
31-
import org.elasticsearch.common.transport.TransportAddress;
3229
import org.elasticsearch.common.xcontent.ObjectParser;
3330
import org.elasticsearch.common.xcontent.ToXContentObject;
3431
import org.elasticsearch.common.xcontent.XContentBuilder;
3532
import org.elasticsearch.common.xcontent.XContentParser;
3633

3734
import java.io.IOException;
3835
import java.util.Arrays;
39-
import java.util.Collections;
4036
import java.util.List;
4137
import java.util.Objects;
4238
import java.util.stream.Collectors;
@@ -92,20 +88,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
9288
return builder;
9389
}
9490

95-
/**
96-
* Temporary method that allows turning a {@link NodeView} into a {@link DiscoveryNode}. This representation will never be used in
97-
* practice, because in >= 6.4 a consumer of the response will only be able to retrieve a representation of {@link NodeView}
98-
* objects.
99-
*
100-
* Effectively this will be used to hold the state of the object in 6.x so there is no need to have 2 backing objects that
101-
* represent the state of the Response. In practice these will always be read by a consumer as a NodeView, but it eases the
102-
* transition to master which will not contain any representation of a {@link DiscoveryNode}.
103-
*/
104-
DiscoveryNode convertToDiscoveryNode() {
105-
return new DiscoveryNode(name, nodeId, "", "", "", new TransportAddress(TransportAddress.META_ADDRESS, 0),
106-
Collections.emptyMap(), Collections.emptySet(), Version.CURRENT);
107-
}
108-
10991
@Override
11092
public boolean equals(Object obj) {
11193
if (obj == null) {
@@ -125,10 +107,7 @@ public int hashCode() {
125107
}
126108
}
127109

128-
private List<DiscoveryNode> nodes;
129-
130-
private ClusterName clusterName;
131-
110+
private List<NodeView> nodes;
132111

133112
private static final ObjectParser<VerifyRepositoryResponse, Void> PARSER =
134113
new ObjectParser<>(VerifyRepositoryResponse.class.getName(), VerifyRepositoryResponse::new);
@@ -139,43 +118,28 @@ public int hashCode() {
139118
VerifyRepositoryResponse() {
140119
}
141120

142-
public VerifyRepositoryResponse(ClusterName clusterName, DiscoveryNode[] nodes) {
143-
this.clusterName = clusterName;
144-
this.nodes = Arrays.asList(nodes);
121+
public VerifyRepositoryResponse(DiscoveryNode[] nodes) {
122+
this.nodes = Arrays.stream(nodes).map(dn -> new NodeView(dn.getId(), dn.getName())).collect(Collectors.toList());
145123
}
146124

147125
@Override
148126
public void readFrom(StreamInput in) throws IOException {
149127
super.readFrom(in);
150-
if (in.getVersion().onOrAfter(Version.V_6_4_0)) {
151-
this.nodes = in.readList(NodeView::new).stream().map(n -> n.convertToDiscoveryNode()).collect(Collectors.toList());
152-
} else {
153-
clusterName = new ClusterName(in);
154-
this.nodes = in.readList(DiscoveryNode::new);
155-
}
128+
this.nodes = in.readList(NodeView::new);
156129
}
157130

158131
@Override
159132
public void writeTo(StreamOutput out) throws IOException {
160133
super.writeTo(out);
161-
if (out.getVersion().onOrAfter(Version.V_6_4_0)) {
162-
out.writeList(getNodes());
163-
} else {
164-
clusterName.writeTo(out);
165-
out.writeList(nodes);
166-
}
134+
out.writeList(nodes);
167135
}
168136

169137
public List<NodeView> getNodes() {
170-
return nodes.stream().map(dn -> new NodeView(dn.getId(), dn.getName())).collect(Collectors.toList());
171-
}
172-
173-
public ClusterName getClusterName() {
174-
return clusterName;
138+
return nodes;
175139
}
176140

177141
protected void setNodes(List<NodeView> nodes) {
178-
this.nodes = nodes.stream().map(n -> n.convertToDiscoveryNode()).collect(Collectors.toList());
142+
this.nodes = nodes;
179143
}
180144

181145
@Override
@@ -184,12 +148,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
184148
{
185149
builder.startObject(NODES);
186150
{
187-
for (DiscoveryNode node : nodes) {
188-
builder.startObject(node.getId());
189-
{
190-
builder.field(NAME, node.getName());
191-
}
192-
builder.endObject();
151+
for (NodeView node : nodes) {
152+
node.toXContent(builder, params);
193153
}
194154
}
195155
builder.endObject();

server/src/test/java/org/elasticsearch/snapshots/RepositoriesIT.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
4141
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThrows;
4242
import static org.hamcrest.Matchers.containsString;
43-
import static org.hamcrest.Matchers.either;
4443
import static org.hamcrest.Matchers.equalTo;
4544
import static org.hamcrest.Matchers.notNullValue;
4645

0 commit comments

Comments
 (0)