Skip to content

Commit aceb634

Browse files
ywangdywangd
committed
review feedback
1 parent e3bf4af commit aceb634

File tree

12 files changed

+65
-51
lines changed

12 files changed

+65
-51
lines changed

‎modules/data-streams/src/test/java/org/elasticsearch/datastreams/lifecycle/action/TransportGetDataStreamLifecycleStatsActionTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public class TransportGetDataStreamLifecycleStatsActionTests extends ESTestCase
5252
mock(ThreadPool.class),
5353
mock(ActionFilters.class),
5454
dataStreamLifecycleService,
55-
TestProjectResolvers.singleProjectOnly()
55+
TestProjectResolvers.alwaysThrow()
5656
);
5757
private Long lastRunDuration;
5858
private Long timeBetweenStarts;

‎server/src/test/java/org/elasticsearch/action/ActionModuleTests.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() {
134134
List.of(),
135135
RestExtension.allowAll(),
136136
new IncrementalBulkService(null, null),
137-
TestProjectResolvers.singleProjectOnly()
137+
TestProjectResolvers.alwaysThrow()
138138
);
139139
actionModule.initRestHandlers(null, null);
140140
// At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail
@@ -201,7 +201,7 @@ public String getName() {
201201
List.of(),
202202
RestExtension.allowAll(),
203203
new IncrementalBulkService(null, null),
204-
TestProjectResolvers.singleProjectOnly()
204+
TestProjectResolvers.alwaysThrow()
205205
);
206206
Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null, null));
207207
assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/_nodes] for method: GET"));
@@ -261,7 +261,7 @@ public List<RestHandler> getRestHandlers(
261261
List.of(),
262262
RestExtension.allowAll(),
263263
new IncrementalBulkService(null, null),
264-
TestProjectResolvers.singleProjectOnly()
264+
TestProjectResolvers.alwaysThrow()
265265
);
266266
actionModule.initRestHandlers(null, null);
267267
// At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail
@@ -314,7 +314,7 @@ public void test3rdPartyHandlerIsNotInstalled() {
314314
List.of(),
315315
RestExtension.allowAll(),
316316
new IncrementalBulkService(null, null),
317-
TestProjectResolvers.singleProjectOnly()
317+
TestProjectResolvers.alwaysThrow()
318318
)
319319
);
320320
assertThat(
@@ -358,7 +358,7 @@ public void test3rdPartyRestControllerIsNotInstalled() {
358358
List.of(),
359359
RestExtension.allowAll(),
360360
new IncrementalBulkService(null, null),
361-
TestProjectResolvers.singleProjectOnly()
361+
TestProjectResolvers.alwaysThrow()
362362
)
363363
);
364364
assertThat(

‎server/src/test/java/org/elasticsearch/action/admin/indices/template/reservedstate/ReservedComposableIndexTemplateActionTests.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,7 @@ public void testHandlerCorrectness() {
694694
threadPool,
695695
null,
696696
mock(ActionFilters.class),
697-
TestProjectResolvers.singleProjectOnly()
697+
TestProjectResolvers.alwaysThrow()
698698
);
699699
assertEquals(ReservedComposableIndexTemplateAction.NAME, putIndexAction.reservedStateHandlerName().get());
700700
assertThat(
@@ -707,7 +707,7 @@ public void testHandlerCorrectness() {
707707
threadPool,
708708
null,
709709
mock(ActionFilters.class),
710-
TestProjectResolvers.singleProjectOnly()
710+
TestProjectResolvers.alwaysThrow()
711711
);
712712
assertEquals(ReservedComposableIndexTemplateAction.NAME, delIndexAction.reservedStateHandlerName().get());
713713
assertThat(
@@ -722,7 +722,7 @@ public void testHandlerCorrectness() {
722722
null,
723723
mock(ActionFilters.class),
724724
indexScopedSettings,
725-
TestProjectResolvers.singleProjectOnly()
725+
TestProjectResolvers.alwaysThrow()
726726
);
727727
assertEquals(ReservedComposableIndexTemplateAction.NAME, putComponentAction.reservedStateHandlerName().get());
728728
assertThat(
@@ -736,7 +736,7 @@ public void testHandlerCorrectness() {
736736
threadPool,
737737
null,
738738
mock(ActionFilters.class),
739-
TestProjectResolvers.singleProjectOnly()
739+
TestProjectResolvers.alwaysThrow()
740740
);
741741
assertEquals(ReservedComposableIndexTemplateAction.NAME, delComponentAction.reservedStateHandlerName().get());
742742
assertThat(
@@ -959,7 +959,7 @@ public void testTemplatesWithReservedPrefix() throws Exception {
959959
threadPool,
960960
null,
961961
mock(ActionFilters.class),
962-
TestProjectResolvers.singleProjectOnly()
962+
TestProjectResolvers.alwaysThrow()
963963
);
964964

965965
// Try fake REST modification request with validate_template, this will fail

‎server/src/test/java/org/elasticsearch/cluster/ClusterModuleTests.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ public Collection<AllocationDecider> createAllocationDeciders(Settings settings,
166166
null,
167167
threadPool,
168168
EmptySystemIndices.INSTANCE,
169-
TestProjectResolvers.singleProjectOnly(),
169+
TestProjectResolvers.alwaysThrow(),
170170
WriteLoadForecaster.DEFAULT,
171171
TelemetryProvider.NOOP
172172
)
@@ -185,7 +185,7 @@ public Collection<AllocationDecider> createAllocationDeciders(Settings settings,
185185
null,
186186
threadPool,
187187
EmptySystemIndices.INSTANCE,
188-
TestProjectResolvers.singleProjectOnly(),
188+
TestProjectResolvers.alwaysThrow(),
189189
WriteLoadForecaster.DEFAULT,
190190
TelemetryProvider.NOOP
191191
);
@@ -203,7 +203,7 @@ public Map<String, Supplier<ShardsAllocator>> getShardsAllocators(Settings setti
203203
null,
204204
threadPool,
205205
EmptySystemIndices.INSTANCE,
206-
TestProjectResolvers.singleProjectOnly(),
206+
TestProjectResolvers.alwaysThrow(),
207207
WriteLoadForecaster.DEFAULT,
208208
TelemetryProvider.NOOP
209209
);
@@ -239,7 +239,7 @@ public void testUnknownShardsAllocator() {
239239
null,
240240
threadPool,
241241
EmptySystemIndices.INSTANCE,
242-
TestProjectResolvers.singleProjectOnly(),
242+
TestProjectResolvers.alwaysThrow(),
243243
WriteLoadForecaster.DEFAULT,
244244
TelemetryProvider.NOOP
245245
)
@@ -303,7 +303,7 @@ public void testRejectsReservedExistingShardsAllocatorName() {
303303
null,
304304
threadPool,
305305
EmptySystemIndices.INSTANCE,
306-
TestProjectResolvers.singleProjectOnly(),
306+
TestProjectResolvers.alwaysThrow(),
307307
WriteLoadForecaster.DEFAULT,
308308
TelemetryProvider.NOOP
309309
);
@@ -319,7 +319,7 @@ public void testRejectsDuplicateExistingShardsAllocatorName() {
319319
null,
320320
threadPool,
321321
EmptySystemIndices.INSTANCE,
322-
TestProjectResolvers.singleProjectOnly(),
322+
TestProjectResolvers.alwaysThrow(),
323323
WriteLoadForecaster.DEFAULT,
324324
TelemetryProvider.NOOP
325325
);

‎server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1184,7 +1184,7 @@ public Collection<RestHeaderDefinition> getRestHeaders() {
11841184
List.of(),
11851185
RestExtension.allowAll(),
11861186
new IncrementalBulkService(null, null),
1187-
TestProjectResolvers.singleProjectOnly()
1187+
TestProjectResolvers.alwaysThrow()
11881188
);
11891189
}
11901190

‎server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ public void testIngestPlugin() {
160160
client,
161161
null,
162162
FailureStoreMetrics.NOOP,
163-
TestProjectResolvers.singleProjectOnly()
163+
TestProjectResolvers.alwaysThrow()
164164
);
165165
Map<String, Processor.Factory> factories = ingestService.getProcessorFactories();
166166
assertTrue(factories.containsKey("foo"));
@@ -181,7 +181,7 @@ public void testIngestPluginDuplicate() {
181181
client,
182182
null,
183183
FailureStoreMetrics.NOOP,
184-
TestProjectResolvers.singleProjectOnly()
184+
TestProjectResolvers.alwaysThrow()
185185
)
186186
);
187187
assertTrue(e.getMessage(), e.getMessage().contains("already registered"));
@@ -199,7 +199,7 @@ public void testExecuteIndexPipelineDoesNotExist() {
199199
client,
200200
null,
201201
FailureStoreMetrics.NOOP,
202-
TestProjectResolvers.singleProjectOnly()
202+
TestProjectResolvers.alwaysThrow()
203203
);
204204
final IndexRequest indexRequest = new IndexRequest("_index").id("_id")
205205
.source(Map.of())
@@ -2440,7 +2440,7 @@ public Map<String, Processor.Factory> getProcessors(Processor.Parameters paramet
24402440
client,
24412441
null,
24422442
FailureStoreMetrics.NOOP,
2443-
TestProjectResolvers.singleProjectOnly()
2443+
TestProjectResolvers.alwaysThrow()
24442444
);
24452445
ingestService.addIngestClusterStateListener(ingestClusterStateListener);
24462446

@@ -2929,7 +2929,7 @@ private void testUpdatingPipeline(String pipelineString) throws Exception {
29292929
client,
29302930
null,
29312931
FailureStoreMetrics.NOOP,
2932-
TestProjectResolvers.singleProjectOnly()
2932+
TestProjectResolvers.alwaysThrow()
29332933
);
29342934
ingestService.applyClusterState(new ClusterChangedEvent("", clusterState, clusterState));
29352935

@@ -3263,7 +3263,7 @@ public Map<String, Processor.Factory> getProcessors(final Processor.Parameters p
32633263
client,
32643264
null,
32653265
FailureStoreMetrics.NOOP,
3266-
TestProjectResolvers.singleProjectOnly()
3266+
TestProjectResolvers.alwaysThrow()
32673267
);
32683268
if (randomBoolean()) {
32693269
/*

‎server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2403,7 +2403,7 @@ public RecyclerBytesStreamOutput newNetworkBytesStream() {
24032403
client,
24042404
null,
24052405
FailureStoreMetrics.NOOP,
2406-
TestProjectResolvers.singleProjectOnly()
2406+
TestProjectResolvers.alwaysThrow()
24072407
),
24082408
client,
24092409
actionFilters,
@@ -2541,7 +2541,7 @@ public RecyclerBytesStreamOutput newNetworkBytesStream() {
25412541
threadPool,
25422542
allocationService,
25432543
actionFilters,
2544-
TestProjectResolvers.singleProjectOnly()
2544+
TestProjectResolvers.alwaysThrow()
25452545
)
25462546
);
25472547
actions.put(

‎test/framework/src/main/java/org/elasticsearch/cluster/project/TestProjectResolvers.java‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import org.elasticsearch.common.util.concurrent.ThreadContext;
1717
import org.elasticsearch.core.CheckedRunnable;
1818
import org.elasticsearch.tasks.Task;
19-
import org.elasticsearch.test.ESTestCase;
2019

2120
import java.util.Collection;
2221
import java.util.Objects;
@@ -58,8 +57,7 @@ public boolean supportsMultipleProjects() {
5857

5958
/**
6059
* This method returns a ProjectResolver that is unable to provide the project-id unless explicitly specified
61-
* with the executeOnProject method. This is mostly useful in places where we just need a placeholder to satisfy
62-
* the constructor signature.
60+
* with the executeOnProject method.
6361
*/
6462
public static ProjectResolver mustExecuteFirst() {
6563
return new ProjectResolver() {
@@ -101,6 +99,31 @@ public boolean supportsMultipleProjects() {
10199
};
102100
}
103101

102+
private static final ProjectResolver ALWAYS_THROW = new ProjectResolver() {
103+
@Override
104+
public <E extends Exception> void executeOnProject(ProjectId projectId, CheckedRunnable<E> body) throws E {
105+
throw new UnsupportedOperationException("Method on the dummy ProjectResolver is not meant to be invoked");
106+
}
107+
108+
@Override
109+
public ProjectId getProjectId() {
110+
throw new UnsupportedOperationException("Method on the dummy ProjectResolver is not meant to be invoked");
111+
}
112+
113+
@Override
114+
public boolean supportsMultipleProjects() {
115+
throw new UnsupportedOperationException("Method on the dummy ProjectResolver is not meant to be invoked");
116+
}
117+
};
118+
119+
/**
120+
* This method returns a ProjectResolver always throw for all methods. This is mostly useful in places where
121+
* we just need a placeholder to satisfy the constructor signature.
122+
*/
123+
public static ProjectResolver alwaysThrow() {
124+
return ALWAYS_THROW;
125+
}
126+
104127
/**
105128
* This method returns a ProjectResolver that gives back the specified project-id when its getProjectId method is called.
106129
* The ProjectResolver can work with cluster state containing multiple projects and its supportsMultipleProjects returns true.
@@ -110,14 +133,10 @@ public static ProjectResolver singleProject(ProjectId projectId) {
110133
}
111134

112135
/**
113-
* This method returns a ProjectResolver that returns a random unique project-id.
136+
* This method returns a ProjectResolver that returns the given ProjectId.
114137
* It also assumes it is the only project in the cluster state and throws if that is not the case.
115138
* In addition, the ProjectResolvers returns false for supportsMultipleProjects.
116139
*/
117-
public static ProjectResolver singleProjectOnly() {
118-
return singleProjectOnly(ESTestCase.randomUniqueProjectId());
119-
}
120-
121140
public static ProjectResolver singleProjectOnly(ProjectId projectId) {
122141
return singleProject(projectId, true);
123142
}

‎test/framework/src/test/java/org/elasticsearch/cluster/project/TestProjectResolversTests.java‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import org.elasticsearch.test.ESTestCase;
1818

1919
import static org.hamcrest.Matchers.contains;
20-
import static org.hamcrest.Matchers.containsString;
2120
import static org.hamcrest.Matchers.equalTo;
2221
import static org.hamcrest.Matchers.greaterThan;
2322
import static org.hamcrest.Matchers.hasSize;
@@ -46,19 +45,15 @@ public void testSingleProject() {
4645
assertThat(projectResolver.getProjectMetadata(state), notNullValue());
4746
}
4847

49-
public void testSingleProjectOnly() {
50-
final ProjectResolver projectResolver = TestProjectResolvers.singleProjectOnly();
51-
assertThat(projectResolver.supportsMultipleProjects(), is(false));
52-
final var projectId = projectResolver.getProjectId();
53-
54-
ClusterState state = buildClusterState(projectId, 0);
55-
assertThat(projectResolver.getProjectMetadata(state), notNullValue());
56-
57-
final IllegalStateException e = expectThrows(
58-
IllegalStateException.class,
59-
() -> projectResolver.getProjectMetadata(buildClusterState(projectId, randomIntBetween(1, 10)))
48+
public void testAlwaysThrowProjectResolver() {
49+
final ProjectResolver projectResolver = TestProjectResolvers.alwaysThrow();
50+
expectThrows(UnsupportedOperationException.class, projectResolver::getProjectId);
51+
expectThrows(UnsupportedOperationException.class, projectResolver::supportsMultipleProjects);
52+
expectThrows(UnsupportedOperationException.class, () -> projectResolver.executeOnProject(randomProjectIdOrDefault(), () -> {}));
53+
expectThrows(
54+
UnsupportedOperationException.class,
55+
() -> projectResolver.getProjectMetadata(buildClusterState(randomProjectIdOrDefault(), randomIntBetween(0, 10)))
6056
);
61-
assertThat(e.getMessage(), containsString("Cluster has multiple projects"));
6257
}
6358

6459
public void testDefaultProjectOnly() {

‎x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/action/InternalExecutePolicyActionTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public void instantiateTransportAction() {
3535
transportService,
3636
mock(ActionFilters.class),
3737
null,
38-
TestProjectResolvers.singleProjectOnly(),
38+
TestProjectResolvers.alwaysThrow(),
3939
null
4040
);
4141
}

0 commit comments

Comments
 (0)