Skip to content

Commit 5f0f99a

Browse files
Fix mutateInstance() in several tests (#136414)
Relates #134166 This addresses misuse of `randomValueOtherThan()` in all the offending tests in the following packages: - elasticsearch.server - elasticsearch.x-pack.plugin.autoscaling - elasticsearch.x-pack.plugin.core
1 parent 8b35732 commit 5f0f99a

File tree

32 files changed

+294
-57
lines changed

32 files changed

+294
-57
lines changed

server/src/main/java/org/elasticsearch/action/synonyms/AbstractSynonymsPagedResultAction.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ public int hashCode() {
182182
result = 31 * result + Arrays.hashCode(resultList);
183183
return result;
184184
}
185+
186+
PagedResult<? extends Writeable> getResults() {
187+
return new PagedResult<>(totalCount, resultList);
188+
}
185189
}
186190

187191
}

server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,14 @@ public boolean equals(Object o) {
9595
if (this == o) return true;
9696
if (o == null || getClass() != o.getClass()) return false;
9797
Request request = (Request) o;
98-
return Objects.equals(synonymsSetId, request.synonymsSetId) && Objects.equals(synonymRuleId, request.synonymRuleId);
98+
return Objects.equals(synonymsSetId, request.synonymsSetId)
99+
&& Objects.equals(synonymRuleId, request.synonymRuleId)
100+
&& refresh == request.refresh;
99101
}
100102

101103
@Override
102104
public int hashCode() {
103-
return Objects.hash(synonymsSetId, synonymRuleId);
105+
return Objects.hash(synonymsSetId, synonymRuleId, refresh);
104106
}
105107
}
106108
}

server/src/main/java/org/elasticsearch/action/synonyms/GetSynonymsAction.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,5 +91,11 @@ protected Reader<SynonymRule> reader() {
9191
protected IntFunction<SynonymRule[]> arraySupplier() {
9292
return SynonymRule[]::new;
9393
}
94+
95+
@SuppressWarnings("unchecked")
96+
@Override
97+
PagedResult<SynonymRule> getResults() {
98+
return (PagedResult<SynonymRule>) super.getResults();
99+
}
94100
}
95101
}

server/src/main/java/org/elasticsearch/action/synonyms/GetSynonymsSetsAction.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ protected Reader<SynonymSetSummary> reader() {
6161
protected IntFunction<SynonymSetSummary[]> arraySupplier() {
6262
return SynonymSetSummary[]::new;
6363
}
64+
65+
@SuppressWarnings("unchecked")
66+
@Override
67+
PagedResult<SynonymSetSummary> getResults() {
68+
return (PagedResult<SynonymSetSummary>) super.getResults();
69+
}
6470
}
6571

6672
}

server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverConfigurationTests.java

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,16 @@ protected Writeable.Reader<RolloverConfiguration> instanceReader() {
4949

5050
@Override
5151
protected RolloverConfiguration createTestInstance() {
52-
return randomRolloverConditions();
52+
return randomRolloverConfiguration();
5353
}
5454

55-
public static RolloverConfiguration randomRolloverConditions() {
55+
public static RolloverConfiguration randomRolloverConfiguration() {
56+
RolloverConditions.Builder concreteConditionsBuilder = randomRolloverConditionsWithoutMaxAgeBuilder();
57+
Set<String> automaticConditions = randomAutomaticConditions(concreteConditionsBuilder);
58+
return new RolloverConfiguration(concreteConditionsBuilder.build(), automaticConditions);
59+
}
60+
61+
private static RolloverConditions.Builder randomRolloverConditionsWithoutMaxAgeBuilder() {
5662
ByteSizeValue maxSize = randomBoolean() ? randomByteSizeValue() : null;
5763
ByteSizeValue maxPrimaryShardSize = randomBoolean() ? randomByteSizeValue() : null;
5864
Long maxDocs = randomBoolean() ? randomNonNegativeLong() : null;
@@ -63,7 +69,7 @@ public static RolloverConfiguration randomRolloverConditions() {
6369
TimeValue minAge = randomBoolean() ? randomPositiveTimeValue() : null;
6470
Long minPrimaryShardDocs = randomBoolean() ? randomNonNegativeLong() : null;
6571

66-
RolloverConditions.Builder concreteConditionsBuilder = RolloverConditions.newBuilder()
72+
return RolloverConditions.newBuilder()
6773
.addMaxIndexSizeCondition(maxSize)
6874
.addMaxPrimaryShardSizeCondition(maxPrimaryShardSize)
6975
.addMaxIndexDocsCondition(maxDocs)
@@ -73,20 +79,31 @@ public static RolloverConfiguration randomRolloverConditions() {
7379
.addMinIndexAgeCondition(minAge)
7480
.addMinIndexDocsCondition(minDocs)
7581
.addMinPrimaryShardDocsCondition(minPrimaryShardDocs);
82+
}
83+
84+
private static Set<String> randomAutomaticConditions(RolloverConditions.Builder conditionsBuilder) {
7685
Set<String> automaticConditions = new HashSet<>();
7786
if (randomBoolean()) {
7887
if (randomBoolean()) {
79-
concreteConditionsBuilder.addMaxIndexAgeCondition(TimeValue.timeValueMillis(randomMillisUpToYear9999()));
88+
conditionsBuilder.addMaxIndexAgeCondition(TimeValue.timeValueMillis(randomMillisUpToYear9999()));
8089
} else {
8190
automaticConditions.add(MaxAgeCondition.NAME);
8291
}
8392
}
84-
return new RolloverConfiguration(concreteConditionsBuilder.build(), automaticConditions);
93+
return automaticConditions;
8594
}
8695

8796
@Override
8897
protected RolloverConfiguration mutateInstance(RolloverConfiguration instance) {
89-
return randomValueOtherThan(instance, RolloverConfigurationTests::randomRolloverConditions);
98+
RolloverConditions originalConditions = instance.getConcreteConditions();
99+
RolloverConditions newConditions = randomValueOtherThan(
100+
originalConditions,
101+
() -> RolloverConfigurationTests.randomRolloverConditionsWithoutMaxAgeBuilder().build()
102+
);
103+
// There is no point mutating automatic conditions. If they change, then the concrete conditions
104+
// should also change to produce a valid configuration Consequently, the mutated instance is guaranteed to be
105+
// different from the original instance and the test purpose would be voided.
106+
return new RolloverConfiguration(newConditions, instance.getAutomaticConditions());
90107
}
91108

92109
public void testConstructorValidation() {
@@ -277,7 +294,7 @@ public void testAutoMaxAgeCalculation() {
277294

278295
public void testToXContent() throws IOException {
279296
try (XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint()) {
280-
RolloverConfiguration rolloverConfiguration = randomRolloverConditions();
297+
RolloverConfiguration rolloverConfiguration = randomRolloverConfiguration();
281298
rolloverConfiguration.toXContent(builder, EMPTY_PARAMS);
282299
Map<String, Object> xContentMap = XContentHelper.convertToMap(BytesReference.bytes(builder), false, builder.contentType()).v2();
283300
RolloverConditions concreteConditions = rolloverConfiguration.getConcreteConditions();

server/src/test/java/org/elasticsearch/action/admin/indices/template/get/GetComponentTemplateResponseTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public void testXContentSerializationWithRolloverAndEffectiveRetention() throws
6262
null,
6363
null
6464
);
65-
var rolloverConfiguration = RolloverConfigurationTests.randomRolloverConditions();
65+
var rolloverConfiguration = RolloverConfigurationTests.randomRolloverConfiguration();
6666
var response = new GetComponentTemplateAction.Response(Map.of(randomAlphaOfLength(10), template), rolloverConfiguration);
6767

6868
try (XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent())) {

server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutComposableIndexTemplateRequestTests.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,18 @@ protected TransportPutComposableIndexTemplateAction.Request createTestInstance()
4545

4646
@Override
4747
protected TransportPutComposableIndexTemplateAction.Request mutateInstance(TransportPutComposableIndexTemplateAction.Request instance) {
48-
return randomValueOtherThan(instance, this::createTestInstance);
48+
String name = instance.name();
49+
String cause = instance.cause();
50+
boolean create = instance.create();
51+
ComposableIndexTemplate indexTemplate = instance.indexTemplate();
52+
switch (between(0, 3)) {
53+
case 0 -> name = randomValueOtherThan(name, () -> randomAlphaOfLength(4));
54+
case 1 -> cause = randomValueOtherThan(cause, () -> randomAlphaOfLength(4));
55+
case 2 -> create = create == false;
56+
case 3 -> indexTemplate = ComposableIndexTemplateTests.randomInstance();
57+
default -> throw new AssertionError("Illegal randomisation branch");
58+
}
59+
return new TransportPutComposableIndexTemplateAction.Request(name).cause(cause).create(create).indexTemplate(indexTemplate);
4960
}
5061

5162
public void testPutGlobalTemplatesCannotHaveHiddenIndexSetting() {

server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ protected DeleteSynonymRuleAction.Request createTestInstance() {
2828

2929
@Override
3030
protected DeleteSynonymRuleAction.Request mutateInstance(DeleteSynonymRuleAction.Request instance) throws IOException {
31-
return randomValueOtherThan(instance, this::createTestInstance);
31+
String synonymsSetId = instance.synonymsSetId();
32+
String synonymRuleId = instance.synonymRuleId();
33+
boolean refresh = instance.refresh();
34+
switch (between(0, 2)) {
35+
case 0 -> synonymsSetId = randomValueOtherThan(synonymsSetId, () -> randomIdentifier());
36+
case 1 -> synonymRuleId = randomValueOtherThan(synonymRuleId, () -> randomIdentifier());
37+
case 2 -> refresh = refresh == false;
38+
default -> throw new AssertionError("Illegal randomisation branch");
39+
}
40+
return new DeleteSynonymRuleAction.Request(synonymsSetId, synonymRuleId, refresh);
3241
}
3342
}

server/src/test/java/org/elasticsearch/action/synonyms/GetSynonymRuleActionRequestSerializingTests.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ protected GetSynonymRuleAction.Request createTestInstance() {
2828

2929
@Override
3030
protected GetSynonymRuleAction.Request mutateInstance(GetSynonymRuleAction.Request instance) throws IOException {
31-
return randomValueOtherThan(instance, this::createTestInstance);
31+
String synonymsSetId = instance.synonymsSetId();
32+
String synonymRuleId = instance.synonymRuleId();
33+
switch (between(0, 1)) {
34+
case 0 -> synonymsSetId = randomValueOtherThan(synonymsSetId, () -> randomIdentifier());
35+
case 1 -> synonymRuleId = randomValueOtherThan(synonymRuleId, () -> randomIdentifier());
36+
default -> throw new AssertionError("Illegal randomisation branch");
37+
}
38+
return new GetSynonymRuleAction.Request(synonymsSetId, synonymRuleId);
3239
}
3340
}

server/src/test/java/org/elasticsearch/action/synonyms/GetSynonymsActionRequestSerializingTests.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@ protected GetSynonymsAction.Request createTestInstance() {
3232

3333
@Override
3434
protected GetSynonymsAction.Request mutateInstance(GetSynonymsAction.Request instance) throws IOException {
35-
return randomValueOtherThan(instance, this::createTestInstance);
35+
String synonymsSetId = instance.synonymsSetId();
36+
int from = instance.from();
37+
int size = instance.size();
38+
switch (between(0, 2)) {
39+
case 0 -> synonymsSetId = randomValueOtherThan(synonymsSetId, () -> randomIdentifier());
40+
case 1 -> from = randomValueOtherThan(from, () -> randomIntBetween(0, Integer.MAX_VALUE));
41+
case 2 -> size = randomValueOtherThan(size, () -> randomIntBetween(0, Integer.MAX_VALUE));
42+
default -> throw new AssertionError("Illegal randomisation branch");
43+
}
44+
return new GetSynonymsAction.Request(synonymsSetId, from, size);
3645
}
3746
}

0 commit comments

Comments
 (0)