Refactor helper methods in DataStreamTestHelper

[nielsbauman]

For most of the usages of these methods, it made more sense to return a ProjectMetadata instead of a ClusterState.
We also don't need to specify a specific project ID; generating a random one inside the helper method saves some boilerplate code.

[elasticsearchmachine]

Pinging @elastic/es-data-management (Team:Data Management)

[PeteGillinElastic]

LGTM, though I'd consider introducing a new helper method as per my comments, if that works the way I imagine it would.

[PeteGillinElastic]

We don't have a DataStreamTestHelper.getProjectWithDataStreams that works here, like we had for the ...WithDataStream (singular) above?

[nielsbauman]

We currently do not. I was struggling with this exact question, because I'm not a huge fan of this approach either. My reason for not adding a getProjectWithDataStreams method is that there are a bunch of different overloads of getClusterStateWithDataStreams already, and adding a project version for every one (not right now, but perhaps over time) feels like it's getting out of hand. I looked at some of the usages but I concluded that they all need a ClusterState because they need to convert it to a ProjectState.

However, I just realized I could maybe do some refactoring of those usages instead to make use of my ESTestCase#projectStateFromProject. Let me experiment with that.

[PeteGillinElastic]

Same question as above, would a dedicated helper method to get the ProjectMetadata directly be worthwhile?

[PeteGillinElastic]

I can see a number of other places below that could (it seems) also use such a helper, I'm not going to comment on them all.

[nielsbauman]

I have good news and I have bad news. The bad news is that the PR grew quite a bit, but the good news is that the PR looks much better now (or well, the code after merging this PR will look much better). Here's a summary of those changes:

1. I changed the helper methods in DataStreamTestHelper to return a ProjectMetadata instead of a ClusterState.
2. Additionally, those methods now specify a random project ID instead of taking it as a parameter, as (almost) all usages just want a random project ID.
3. Along the way, I updated some tests to use a random project ID instead of the default one, as the production code was already project-aware. I tried to run these tests with a unique project ID to verify that that doesn't break the test, but there's a chance I forgot to double-check some tests. Worst case, a test failure gets opened that tells us the code is not project aware yet.
4. These changes allowed me to remove some redundant cluster state conversion in some tests.
5. Finally, I was able to remove a bunch more Metadata#getProject() usages in tests :)

Splitting this PR up into multiple PRs would have been difficult. Fortunately, the changes are pretty independent of one another (except for the DataStreamTestHelper of course). Let me know what you think!

[PeteGillinElastic]

Hi, sorry for dropping the ball on this review. You should have nudged me earlier!

Anyway, this LGTM, apart from one small question.

[PeteGillinElastic]

I'm slightly confused by this comment. This overload isn't the one used in the @FixForMultiProject-annotated line in ProactiveStorageDeciderServiceTests. That's using an overload which takes the project ID. So I would have expected that the change when that test no longer requires the default project would be to remove (or perhaps make private) the overload which that test currently uses and won't any more?

[nielsbauman]

Yeah, I understand your confusion. When ProactiveStorageDeciderServiceTests is updated, I essentially want to do this:

diff --git a/test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java b/test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java
index ee96bb4d5c3..0293ba37101 100644
--- a/test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java
+++ b/test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java
@@ -456,7 +456,6 @@ public final class DataStreamTestHelper {
         return getProjectWithDataStreams(dataStreams, indexNames, currentTime, settings, replicas, false, false);
     }
 
-    @FixForMultiProject() // Remove this intermediate method when ProactiveStorageDeciderServiceTests no longer needs the default project ID
     public static ProjectMetadata getProjectWithDataStreams(
         List<Tuple<String, Integer>> dataStreams,
         List<String> indexNames,
@@ -467,19 +466,6 @@ public final class DataStreamTestHelper {
         Boolean storeFailures
     ) {
         final var projectId = randomProjectIdOrDefault();
-        return getProjectWithDataStreams(projectId, dataStreams, indexNames, currentTime, settings, replicas, replicated, storeFailures);
-    }
-
-    public static ProjectMetadata getProjectWithDataStreams(
-        ProjectId projectId,
-        List<Tuple<String, Integer>> dataStreams,
-        List<String> indexNames,
-        long currentTime,
-        Settings settings,
-        int replicas,
-        boolean replicated,
-        Boolean storeFailures
-    ) {
         ProjectMetadata.Builder builder = ProjectMetadata.builder(projectId);
         builder.put(
             "template_1",

It's a bit ambiguous what "remove this intermediate method" means. I've moved the annotation and adjusted the comment slightly. This should be clearer. Thanks for bringing it up.

[Copilot]

Pull Request Overview

This PR replaces deprecated getClusterStateWithDataStreams usage in tests with getProjectWithDataStreams, introduces helper methods (emptyProject, projectStateFromProject), and updates imports and API calls across multiple test files to work with ProjectMetadata and ProjectState.

• Refactored tests to use DataStreamTestHelper.getProjectWithDataStreams and emptyProject() instead of Metadata.EMPTY_METADATA.getProject() or getClusterStateWithDataStreams.
• Updated cluster state builders to use ClusterName.DEFAULT and putProjectMetadata.
• Added ESTestCase.emptyProject and overloads of projectStateFromProject.

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java	Swapped metadata builder for ProjectMetadata and lookup changes.
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizedIndicesTests.java	Replaced Metadata.EMPTY_METADATA.getProject() with emptyProject()
x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProviderTests.java	Updated provider calls to use emptyProject and project.
x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/storage/ProactiveStorageDeciderServiceTests.java	Large refactor to use ProjectMetadata, added @FixForMultiProject
test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java	Added emptyProject() and overload of projectStateFromProject
test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java	Deprecated old methods, added getProjectWithDataStreams overloads
server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java	Updated alias tests to use ProjectMetadata
server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexServiceTests.java	Refactored delete index tests to use project-based APIs
server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsServiceTests.java	Updated data streams service tests to use getProjectWithDataStreams
server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java	Swapped ClusterState calls for ProjectMetadata
server/src/test/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolverTests.java	Moved resolver setup after project metadata builder
server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamLifecycleWithRetentionWarningsTests.java	Removed unused ClusterState import
server/src/test/java/org/elasticsearch/cluster/ClusterStateSerializationTests.java	Changed serialization to use ProjectMetadata
server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java	Updated mapping request tests to use ProjectMetadata
server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java	Refactored transport action tests to use ProjectMetadata.builder
modules/data-streams/src/test/java/org/elasticsearch/datastreams/action/TransportGetDataStreamsActionTests.java	Swapped metadata calls to getProjectWithDataStreams
modules/data-streams/src/test/java/org/elasticsearch/datastreams/action/TransportDeleteDataStreamActionTests.java	Updated delete data stream tests to use projectStateFromProject
modules/data-streams/src/test/java/org/elasticsearch/datastreams/UpdateTimeSeriesRangeServiceTests.java	Adjusted cluster state creation to use putProjectMetadata
modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java	Replaced Metadata.EMPTY_METADATA.getProject() with emptyProject()

Comments suppressed due to low confidence (2)

test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java:445

• [nitpick] Public helper methods should have Javadoc. Please add documentation explaining that this overload uses a random project ID via randomProjectIdOrDefault().

    public static ProjectMetadata getProjectWithDataStreams(List<Tuple<String, Integer>> dataStreams, List<String> indexNames) {

test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java:474

• [nitpick] The use of a boxed Boolean storeFailures parameter can introduce null-safety issues. Consider using a primitive boolean and default logic, or provide an explicit overload if null is meaningful.

    public static ProjectMetadata getProjectWithDataStreams(