Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public class TransportGetDataStreamLifecycleStatsActionTests extends ESTestCase
mock(ThreadPool.class),
mock(ActionFilters.class),
dataStreamLifecycleService,
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
private Long lastRunDuration;
private Long timeBetweenStarts;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() {
List.of(),
RestExtension.allowAll(),
new IncrementalBulkService(null, null),
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
actionModule.initRestHandlers(null, null);
// 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
Expand Down Expand Up @@ -201,7 +201,7 @@ public String getName() {
List.of(),
RestExtension.allowAll(),
new IncrementalBulkService(null, null),
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null, null));
assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/_nodes] for method: GET"));
Expand Down Expand Up @@ -261,7 +261,7 @@ public List<RestHandler> getRestHandlers(
List.of(),
RestExtension.allowAll(),
new IncrementalBulkService(null, null),
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
actionModule.initRestHandlers(null, null);
// 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
Expand Down Expand Up @@ -314,7 +314,7 @@ public void test3rdPartyHandlerIsNotInstalled() {
List.of(),
RestExtension.allowAll(),
new IncrementalBulkService(null, null),
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
)
);
assertThat(
Expand Down Expand Up @@ -358,7 +358,7 @@ public void test3rdPartyRestControllerIsNotInstalled() {
List.of(),
RestExtension.allowAll(),
new IncrementalBulkService(null, null),
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
)
);
assertThat(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ public void testHandlerCorrectness() {
threadPool,
null,
mock(ActionFilters.class),
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
assertEquals(ReservedComposableIndexTemplateAction.NAME, putIndexAction.reservedStateHandlerName().get());
assertThat(
Expand All @@ -707,7 +707,7 @@ public void testHandlerCorrectness() {
threadPool,
null,
mock(ActionFilters.class),
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
assertEquals(ReservedComposableIndexTemplateAction.NAME, delIndexAction.reservedStateHandlerName().get());
assertThat(
Expand All @@ -722,7 +722,7 @@ public void testHandlerCorrectness() {
null,
mock(ActionFilters.class),
indexScopedSettings,
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
assertEquals(ReservedComposableIndexTemplateAction.NAME, putComponentAction.reservedStateHandlerName().get());
assertThat(
Expand All @@ -736,7 +736,7 @@ public void testHandlerCorrectness() {
threadPool,
null,
mock(ActionFilters.class),
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
assertEquals(ReservedComposableIndexTemplateAction.NAME, delComponentAction.reservedStateHandlerName().get());
assertThat(
Expand Down Expand Up @@ -959,7 +959,7 @@ public void testTemplatesWithReservedPrefix() throws Exception {
threadPool,
null,
mock(ActionFilters.class),
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);

// Try fake REST modification request with validate_template, this will fail
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public Collection<AllocationDecider> createAllocationDeciders(Settings settings,
null,
threadPool,
EmptySystemIndices.INSTANCE,
TestProjectResolvers.singleProjectOnly(),
TestProjectResolvers.alwaysThrow(),
WriteLoadForecaster.DEFAULT,
TelemetryProvider.NOOP
)
Expand All @@ -185,7 +185,7 @@ public Collection<AllocationDecider> createAllocationDeciders(Settings settings,
null,
threadPool,
EmptySystemIndices.INSTANCE,
TestProjectResolvers.singleProjectOnly(),
TestProjectResolvers.alwaysThrow(),
WriteLoadForecaster.DEFAULT,
TelemetryProvider.NOOP
);
Expand All @@ -203,7 +203,7 @@ public Map<String, Supplier<ShardsAllocator>> getShardsAllocators(Settings setti
null,
threadPool,
EmptySystemIndices.INSTANCE,
TestProjectResolvers.singleProjectOnly(),
TestProjectResolvers.alwaysThrow(),
WriteLoadForecaster.DEFAULT,
TelemetryProvider.NOOP
);
Expand Down Expand Up @@ -239,7 +239,7 @@ public void testUnknownShardsAllocator() {
null,
threadPool,
EmptySystemIndices.INSTANCE,
TestProjectResolvers.singleProjectOnly(),
TestProjectResolvers.alwaysThrow(),
WriteLoadForecaster.DEFAULT,
TelemetryProvider.NOOP
)
Expand Down Expand Up @@ -303,7 +303,7 @@ public void testRejectsReservedExistingShardsAllocatorName() {
null,
threadPool,
EmptySystemIndices.INSTANCE,
TestProjectResolvers.singleProjectOnly(),
TestProjectResolvers.alwaysThrow(),
WriteLoadForecaster.DEFAULT,
TelemetryProvider.NOOP
);
Expand All @@ -319,7 +319,7 @@ public void testRejectsDuplicateExistingShardsAllocatorName() {
null,
threadPool,
EmptySystemIndices.INSTANCE,
TestProjectResolvers.singleProjectOnly(),
TestProjectResolvers.alwaysThrow(),
WriteLoadForecaster.DEFAULT,
TelemetryProvider.NOOP
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,7 @@ public Collection<RestHeaderDefinition> getRestHeaders() {
List.of(),
RestExtension.allowAll(),
new IncrementalBulkService(null, null),
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void testIngestPlugin() {
client,
null,
FailureStoreMetrics.NOOP,
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
Map<String, Processor.Factory> factories = ingestService.getProcessorFactories();
assertTrue(factories.containsKey("foo"));
Expand All @@ -181,7 +181,7 @@ public void testIngestPluginDuplicate() {
client,
null,
FailureStoreMetrics.NOOP,
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
)
);
assertTrue(e.getMessage(), e.getMessage().contains("already registered"));
Expand All @@ -199,7 +199,7 @@ public void testExecuteIndexPipelineDoesNotExist() {
client,
null,
FailureStoreMetrics.NOOP,
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
final IndexRequest indexRequest = new IndexRequest("_index").id("_id")
.source(Map.of())
Expand Down Expand Up @@ -2440,7 +2440,7 @@ public Map<String, Processor.Factory> getProcessors(Processor.Parameters paramet
client,
null,
FailureStoreMetrics.NOOP,
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
ingestService.addIngestClusterStateListener(ingestClusterStateListener);

Expand Down Expand Up @@ -2929,7 +2929,7 @@ private void testUpdatingPipeline(String pipelineString) throws Exception {
client,
null,
FailureStoreMetrics.NOOP,
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
ingestService.applyClusterState(new ClusterChangedEvent("", clusterState, clusterState));

Expand Down Expand Up @@ -3263,7 +3263,7 @@ public Map<String, Processor.Factory> getProcessors(final Processor.Parameters p
client,
null,
FailureStoreMetrics.NOOP,
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
);
if (randomBoolean()) {
/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2403,7 +2403,7 @@ public RecyclerBytesStreamOutput newNetworkBytesStream() {
client,
null,
FailureStoreMetrics.NOOP,
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
),
client,
actionFilters,
Expand Down Expand Up @@ -2541,7 +2541,7 @@ public RecyclerBytesStreamOutput newNetworkBytesStream() {
threadPool,
allocationService,
actionFilters,
TestProjectResolvers.singleProjectOnly()
TestProjectResolvers.alwaysThrow()
)
);
actions.put(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,9 @@ public boolean supportsMultipleProjects() {

/**
* This method returns a ProjectResolver that is unable to provide the project-id unless explicitly specified
* with the executeOnProject method. This is mostly useful in places where we just need a placeholder to satisfy
* the constructor signature.
* with the executeOnProject method.
*/
public static ProjectResolver singleProjectOnly() {
public static ProjectResolver mustExecuteFirst() {
Comment on lines -63 to +62
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is used in quite a few places in tests. Majority of them (if not all) really just need a projectResolver as a placehoder. The choice of using this variant is probably based on the method name and has no need for its execute behaviour. I renamed this method and added a new singleProjectOnly which does only what the name suggests. Hopefully this should satisfy all existing usages. But if not, we can tweak the places where it needs more.

return new ProjectResolver() {

private ProjectId enforceProjectId = null;
Expand Down Expand Up @@ -100,14 +99,48 @@ public boolean supportsMultipleProjects() {
};
}

private static final ProjectResolver ALWAYS_THROW = new ProjectResolver() {
@Override
public <E extends Exception> void executeOnProject(ProjectId projectId, CheckedRunnable<E> body) throws E {
throw new UnsupportedOperationException("Method on the dummy ProjectResolver is not meant to be invoked");
}

@Override
public ProjectId getProjectId() {
throw new UnsupportedOperationException("Method on the dummy ProjectResolver is not meant to be invoked");
}

@Override
public boolean supportsMultipleProjects() {
throw new UnsupportedOperationException("Method on the dummy ProjectResolver is not meant to be invoked");
}
};

/**
* This method returns a ProjectResolver that always throw for all methods. This is mostly useful in places where
* we just need a placeholder to satisfy the constructor signature.
*/
public static ProjectResolver alwaysThrow() {
return ALWAYS_THROW;
}

/**
* This method returns a ProjectResolver that gives back the specified project-id when its getProjectId method is called.
* It also assumes it is the only project in the cluster state and throws if that is not the case.
* The ProjectResolver can work with cluster state containing multiple projects and its supportsMultipleProjects returns true.
*/
public static ProjectResolver singleProject(ProjectId projectId) {
return singleProject(projectId, false);
}

/**
* This method returns a ProjectResolver that returns the given ProjectId.
* It also assumes it is the only project in the cluster state and throws if that is not the case.
* In addition, the ProjectResolvers returns false for supportsMultipleProjects.
*/
public static ProjectResolver singleProjectOnly(ProjectId projectId) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a bit of Javadoc explaining this method as well?

return singleProject(projectId, true);
}

private static ProjectResolver singleProject(ProjectId projectId, boolean only) {
Objects.requireNonNull(projectId);
return new ProjectResolver() {
Expand Down Expand Up @@ -144,7 +177,7 @@ public <E extends Exception> void executeOnProject(ProjectId otherProjectId, Che

@Override
public boolean supportsMultipleProjects() {
return true;
return only == false;
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;

public class TestProjectResolversTests extends ESTestCase {
Expand All @@ -37,17 +38,38 @@ public void testAllProjects() {
public void testSingleProject() {
final ProjectId projectId = randomUniqueProjectId();
final ProjectResolver projectResolver = TestProjectResolvers.singleProject(projectId);
assertThat(projectResolver.supportsMultipleProjects(), is(true));
assertThat(projectResolver.getProjectId(), equalTo(projectId));

ClusterState state = buildClusterState(projectId, randomIntBetween(0, 10));
assertThat(projectResolver.getProjectMetadata(state), notNullValue());
}

public void testSingleProjectOnly_getProjectIdAndMetadata() {
public void testAlwaysThrowProjectResolver() {
final ProjectResolver projectResolver = TestProjectResolvers.alwaysThrow();
expectThrows(UnsupportedOperationException.class, projectResolver::getProjectId);
expectThrows(UnsupportedOperationException.class, projectResolver::supportsMultipleProjects);
expectThrows(UnsupportedOperationException.class, () -> projectResolver.executeOnProject(randomProjectIdOrDefault(), () -> {}));
expectThrows(
UnsupportedOperationException.class,
() -> projectResolver.getProjectMetadata(buildClusterState(randomProjectIdOrDefault(), randomIntBetween(0, 10)))
);
}

public void testDefaultProjectOnly() {
final ProjectResolver projectResolver = TestProjectResolvers.DEFAULT_PROJECT_ONLY;
assertThat(projectResolver.supportsMultipleProjects(), is(false));
assertThat(projectResolver.getProjectId(), equalTo(ProjectId.DEFAULT));

ClusterState state = buildClusterState(ProjectId.DEFAULT, 0);
assertThat(projectResolver.getProjectMetadata(state), notNullValue());
}

public void testMustExecuteFirst_getProjectIdAndMetadata() {
final ProjectId projectId = randomUniqueProjectId();
final ClusterState state = buildClusterState(projectId);

final ProjectResolver projectResolver = TestProjectResolvers.singleProjectOnly();
final ProjectResolver projectResolver = TestProjectResolvers.mustExecuteFirst();
expectThrows(UnsupportedOperationException.class, projectResolver::getProjectId);
expectThrows(UnsupportedOperationException.class, () -> projectResolver.getProjectMetadata(state));

Expand All @@ -57,9 +79,9 @@ public void testSingleProjectOnly_getProjectIdAndMetadata() {
});
}

public void testSingleProjectOnly_getProjectIds() {
public void testMustExecuteFirst_getProjectIds() {
{
final ProjectResolver projectResolver = TestProjectResolvers.singleProjectOnly();
final ProjectResolver projectResolver = TestProjectResolvers.mustExecuteFirst();
final ProjectId projectId = randomUniqueProjectId();
ClusterState state = buildClusterState(projectId);
assertThat(state.metadata().projects().values(), hasSize(1));
Expand All @@ -71,7 +93,7 @@ public void testSingleProjectOnly_getProjectIds() {
});
}
{
final ProjectResolver projectResolver = TestProjectResolvers.singleProjectOnly();
final ProjectResolver projectResolver = TestProjectResolvers.mustExecuteFirst();
final ProjectId projectId = randomUniqueProjectId();
ClusterState state = buildClusterState(projectId, randomIntBetween(1, 10));
assertThat(state.metadata().projects().values().size(), greaterThan(1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public void testStartStopDuringClusterChanges() {
return null;
}).when(client).execute(same(DeleteByQueryAction.INSTANCE), any(DeleteByQueryRequest.class), any(ActionListener.class));

final ProjectResolver projectResolver = Mockito.spy(TestProjectResolvers.singleProjectOnly());
final ProjectResolver projectResolver = Mockito.spy(TestProjectResolvers.mustExecuteFirst());

final AsyncTaskMaintenanceService service = new AsyncTaskMaintenanceService(
clusterService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void instantiateTransportAction() {
transportService,
mock(ActionFilters.class),
null,
TestProjectResolvers.singleProjectOnly(),
TestProjectResolvers.alwaysThrow(),
null
);
}
Expand Down
Loading