From 9056fe430d0679593ce140a27c658215ba84415a Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Wed, 11 Dec 2024 15:54:03 -0500 Subject: [PATCH 1/8] Convert some ILM classes to records (#118466) --- .../xpack/core/ilm/ClusterStateWaitStep.java | 18 +-- .../ClusterStateWaitUntilThresholdStep.java | 4 +- .../xpack/core/ilm/SegmentCountStep.java | 30 +---- .../core/ilm/WaitForFollowShardTasksStep.java | 106 ++++-------------- .../core/ilm/step/info/AllocationInfo.java | 53 +-------- .../ilm/step/info/SingleMessageFieldInfo.java | 31 +---- .../core/ilm/AllocationRoutedStepTests.java | 12 +- .../CheckNoDataStreamWriteIndexStepTests.java | 14 +-- .../core/ilm/CheckShrinkReadyStepTests.java | 14 +-- .../ilm/CheckTargetShardsCountStepTests.java | 8 +- ...usterStateWaitUntilThresholdStepTests.java | 30 ++--- .../ilm/DataTierMigrationRoutedStepTests.java | 32 +++--- .../core/ilm/SegmentCountStepInfoTests.java | 2 +- .../ilm/ShrunkShardsAllocatedStepTests.java | 12 +- .../core/ilm/ShrunkenIndexCheckStepTests.java | 12 +- .../core/ilm/WaitForActiveShardsTests.java | 16 +-- .../core/ilm/WaitForDataTierStepTests.java | 6 +- .../WaitForFollowShardTasksStepInfoTests.java | 2 +- .../ilm/WaitForFollowShardTasksStepTests.java | 8 +- .../core/ilm/WaitForIndexColorStepTests.java | 48 ++++---- .../ilm/WaitForIndexingCompleteStepTests.java | 18 +-- .../info/AllocationRoutedStepInfoTests.java | 12 +- ...adataMigrateToDataTiersRoutingService.java | 62 ++-------- .../xpack/ilm/ExecuteStepsUpdateTask.java | 4 +- .../TransportMigrateToDataTiersAction.java | 24 ++-- ...MigrateToDataTiersRoutingServiceTests.java | 42 +++---- 26 files changed, 199 insertions(+), 421 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitStep.java index d1dbfede63c60..4ed83fa170ead 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitStep.java @@ -33,22 +33,6 @@ public boolean isCompletable() { return true; } - public static class Result { - private final boolean complete; - private final ToXContentObject informationContext; - - public Result(boolean complete, ToXContentObject informationContext) { - this.complete = complete; - this.informationContext = informationContext; - } - - public boolean isComplete() { - return complete; - } - - public ToXContentObject getInformationContext() { - return informationContext; - } - } + public record Result(boolean complete, ToXContentObject informationContext) {} } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitUntilThresholdStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitUntilThresholdStep.java index 5e30baa6b9669..c7fa1ea611a0f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitUntilThresholdStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitUntilThresholdStep.java @@ -62,7 +62,7 @@ public Result isConditionMet(Index index, ClusterState clusterState) { Result stepResult = stepToExecute.isConditionMet(index, clusterState); - if (stepResult.isComplete() == false) { + if (stepResult.complete() == false) { // checking the threshold after we execute the step to make sure we execute the wrapped step at least once (because time is a // wonderful thing) TimeValue retryThreshold = LifecycleSettings.LIFECYCLE_STEP_WAIT_TIME_THRESHOLD_SETTING.get(idxMeta.getSettings()); @@ -77,7 +77,7 @@ public Result isConditionMet(Index index, ClusterState clusterState) { getKey().name(), getKey().action(), idxMeta.getIndex().getName(), - Strings.toString(stepResult.getInformationContext()), + Strings.toString(stepResult.informationContext()), nextKeyOnThresholdBreach ); logger.debug(message); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SegmentCountStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SegmentCountStep.java index 800ea603ede8c..ad8f450fb0849 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SegmentCountStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SegmentCountStep.java @@ -114,9 +114,7 @@ public boolean equals(Object obj) { return super.equals(obj) && Objects.equals(maxNumSegments, other.maxNumSegments); } - public static class Info implements ToXContentObject { - - private final long numberShardsLeftToMerge; + public record Info(long numberShardsLeftToMerge) implements ToXContentObject { static final ParseField SHARDS_TO_MERGE = new ParseField("shards_left_to_merge"); static final ParseField MESSAGE = new ParseField("message"); @@ -124,19 +122,12 @@ public static class Info implements ToXContentObject { "segment_count_step_info", a -> new Info((long) a[0]) ); + static { PARSER.declareLong(ConstructingObjectParser.constructorArg(), SHARDS_TO_MERGE); PARSER.declareString((i, s) -> {}, MESSAGE); } - public Info(long numberShardsLeftToMerge) { - this.numberShardsLeftToMerge = numberShardsLeftToMerge; - } - - public long getNumberShardsLeftToMerge() { - return numberShardsLeftToMerge; - } - @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); @@ -150,23 +141,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - @Override - public int hashCode() { - return Objects.hash(numberShardsLeftToMerge); - } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { - return false; - } - Info other = (Info) obj; - return Objects.equals(numberShardsLeftToMerge, other.numberShardsLeftToMerge); - } - @Override public String toString() { return Strings.toString(this); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStep.java index f1fbdde1e9a5d..224319722297c 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStep.java @@ -22,7 +22,6 @@ import java.io.IOException; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.stream.Collectors; import static org.elasticsearch.xpack.core.ilm.UnfollowAction.CCR_METADATA_KEY; @@ -70,9 +69,9 @@ static void handleResponse(FollowStatsAction.StatsResponses responses, Listener if (conditionMet) { listener.onResponse(true, null); } else { - List shardFollowTaskInfos = unSyncedShardFollowStatuses.stream() + List shardFollowTaskInfos = unSyncedShardFollowStatuses.stream() .map( - status -> new Info.ShardFollowTaskInfo( + status -> new ShardFollowTaskInfo( status.followerIndex(), status.getShardId(), status.leaderGlobalCheckpoint(), @@ -84,21 +83,11 @@ static void handleResponse(FollowStatsAction.StatsResponses responses, Listener } } - static final class Info implements ToXContentObject { + record Info(List shardFollowTaskInfos) implements ToXContentObject { static final ParseField SHARD_FOLLOW_TASKS = new ParseField("shard_follow_tasks"); static final ParseField MESSAGE = new ParseField("message"); - private final List shardFollowTaskInfos; - - Info(List shardFollowTaskInfos) { - this.shardFollowTaskInfos = shardFollowTaskInfos; - } - - List getShardFollowTaskInfos() { - return shardFollowTaskInfos; - } - @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); @@ -114,85 +103,30 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Info info = (Info) o; - return Objects.equals(shardFollowTaskInfos, info.shardFollowTaskInfos); - } - - @Override - public int hashCode() { - return Objects.hash(shardFollowTaskInfos); - } - @Override public String toString() { return Strings.toString(this); } + } - static final class ShardFollowTaskInfo implements ToXContentObject { - - static final ParseField FOLLOWER_INDEX_FIELD = new ParseField("follower_index"); - static final ParseField SHARD_ID_FIELD = new ParseField("shard_id"); - static final ParseField LEADER_GLOBAL_CHECKPOINT_FIELD = new ParseField("leader_global_checkpoint"); - static final ParseField FOLLOWER_GLOBAL_CHECKPOINT_FIELD = new ParseField("follower_global_checkpoint"); - - private final String followerIndex; - private final int shardId; - private final long leaderGlobalCheckpoint; - private final long followerGlobalCheckpoint; - - ShardFollowTaskInfo(String followerIndex, int shardId, long leaderGlobalCheckpoint, long followerGlobalCheckpoint) { - this.followerIndex = followerIndex; - this.shardId = shardId; - this.leaderGlobalCheckpoint = leaderGlobalCheckpoint; - this.followerGlobalCheckpoint = followerGlobalCheckpoint; - } - - String getFollowerIndex() { - return followerIndex; - } - - int getShardId() { - return shardId; - } - - long getLeaderGlobalCheckpoint() { - return leaderGlobalCheckpoint; - } - - long getFollowerGlobalCheckpoint() { - return followerGlobalCheckpoint; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - builder.field(FOLLOWER_INDEX_FIELD.getPreferredName(), followerIndex); - builder.field(SHARD_ID_FIELD.getPreferredName(), shardId); - builder.field(LEADER_GLOBAL_CHECKPOINT_FIELD.getPreferredName(), leaderGlobalCheckpoint); - builder.field(FOLLOWER_GLOBAL_CHECKPOINT_FIELD.getPreferredName(), followerGlobalCheckpoint); - builder.endObject(); - return builder; - } + record ShardFollowTaskInfo(String followerIndex, int shardId, long leaderGlobalCheckpoint, long followerGlobalCheckpoint) + implements + ToXContentObject { - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ShardFollowTaskInfo that = (ShardFollowTaskInfo) o; - return shardId == that.shardId - && leaderGlobalCheckpoint == that.leaderGlobalCheckpoint - && followerGlobalCheckpoint == that.followerGlobalCheckpoint - && Objects.equals(followerIndex, that.followerIndex); - } + static final ParseField FOLLOWER_INDEX_FIELD = new ParseField("follower_index"); + static final ParseField SHARD_ID_FIELD = new ParseField("shard_id"); + static final ParseField LEADER_GLOBAL_CHECKPOINT_FIELD = new ParseField("leader_global_checkpoint"); + static final ParseField FOLLOWER_GLOBAL_CHECKPOINT_FIELD = new ParseField("follower_global_checkpoint"); - @Override - public int hashCode() { - return Objects.hash(followerIndex, shardId, leaderGlobalCheckpoint, followerGlobalCheckpoint); - } + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field(FOLLOWER_INDEX_FIELD.getPreferredName(), followerIndex); + builder.field(SHARD_ID_FIELD.getPreferredName(), shardId); + builder.field(LEADER_GLOBAL_CHECKPOINT_FIELD.getPreferredName(), leaderGlobalCheckpoint); + builder.field(FOLLOWER_GLOBAL_CHECKPOINT_FIELD.getPreferredName(), followerGlobalCheckpoint); + builder.endObject(); + return builder; } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/step/info/AllocationInfo.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/step/info/AllocationInfo.java index 5732f5e72a42f..9f280bd344083 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/step/info/AllocationInfo.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/step/info/AllocationInfo.java @@ -14,19 +14,15 @@ import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; -import java.util.Objects; /** * Represents the state of an index's shards allocation, including a user friendly message describing the current state. * It allows to transfer the allocation information to {@link org.elasticsearch.xcontent.XContent} using * {@link #toXContent(XContentBuilder, Params)} */ -public class AllocationInfo implements ToXContentObject { - - private final long numberOfReplicas; - private final long numberShardsLeftToAllocate; - private final boolean allShardsActive; - private final String message; +public record AllocationInfo(long numberOfReplicas, long numberShardsLeftToAllocate, boolean allShardsActive, String message) + implements + ToXContentObject { static final ParseField NUMBER_OF_REPLICAS = new ParseField("number_of_replicas"); static final ParseField SHARDS_TO_ALLOCATE = new ParseField("shards_left_to_allocate"); @@ -44,13 +40,6 @@ public class AllocationInfo implements ToXContentObject { PARSER.declareString(ConstructingObjectParser.constructorArg(), MESSAGE); } - public AllocationInfo(long numberOfReplicas, long numberShardsLeftToAllocate, boolean allShardsActive, String message) { - this.numberOfReplicas = numberOfReplicas; - this.numberShardsLeftToAllocate = numberShardsLeftToAllocate; - this.allShardsActive = allShardsActive; - this.message = message; - } - /** * Builds the AllocationInfo representing a cluster state with a routing table that does not have enough shards active for a * particular index. @@ -72,22 +61,6 @@ public static AllocationInfo allShardsActiveAllocationInfo(long numReplicas, lon ); } - public long getNumberOfReplicas() { - return numberOfReplicas; - } - - public long getNumberShardsLeftToAllocate() { - return numberShardsLeftToAllocate; - } - - public boolean allShardsActive() { - return allShardsActive; - } - - public String getMessage() { - return message; - } - @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); @@ -99,26 +72,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - @Override - public int hashCode() { - return Objects.hash(numberOfReplicas, numberShardsLeftToAllocate, allShardsActive); - } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { - return false; - } - AllocationInfo other = (AllocationInfo) obj; - return Objects.equals(numberOfReplicas, other.numberOfReplicas) - && Objects.equals(numberShardsLeftToAllocate, other.numberShardsLeftToAllocate) - && Objects.equals(message, other.message) - && Objects.equals(allShardsActive, other.allShardsActive); - } - @Override public String toString() { return Strings.toString(this); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/step/info/SingleMessageFieldInfo.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/step/info/SingleMessageFieldInfo.java index 8d7eb8c3d303b..bd23e21d46489 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/step/info/SingleMessageFieldInfo.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/step/info/SingleMessageFieldInfo.java @@ -12,20 +12,13 @@ import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; -import java.util.Objects; /** * A simple object that allows a `message` field to be transferred to `XContent`. */ -public class SingleMessageFieldInfo implements ToXContentObject { +public record SingleMessageFieldInfo(String message) implements ToXContentObject { - private final String message; - - static final ParseField MESSAGE = new ParseField("message"); - - public SingleMessageFieldInfo(String message) { - this.message = message; - } + private static final ParseField MESSAGE = new ParseField("message"); @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { @@ -35,24 +28,4 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - public String getMessage() { - return message; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - SingleMessageFieldInfo that = (SingleMessageFieldInfo) o; - return Objects.equals(message, that.message); - } - - @Override - public int hashCode() { - return Objects.hash(message); - } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/AllocationRoutedStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/AllocationRoutedStepTests.java index 415014623f340..afad708ddbe2c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/AllocationRoutedStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/AllocationRoutedStepTests.java @@ -181,8 +181,8 @@ public void testClusterExcludeFiltersConditionMetOnlyOneCopyAllocated() { Result actualResult = step.isConditionMet(index, clusterState); Result expectedResult = new ClusterStateWaitStep.Result(false, allShardsActiveAllocationInfo(1, 1)); - assertEquals(expectedResult.isComplete(), actualResult.isComplete()); - assertEquals(expectedResult.getInformationContext(), actualResult.getInformationContext()); + assertEquals(expectedResult.complete(), actualResult.complete()); + assertEquals(expectedResult.informationContext(), actualResult.informationContext()); } public void testExcludeConditionMetOnlyOneCopyAllocated() { @@ -495,8 +495,8 @@ public void testExecuteIndexMissing() throws Exception { AllocationRoutedStep step = createRandomInstance(); Result actualResult = step.isConditionMet(index, clusterState); - assertFalse(actualResult.isComplete()); - assertNull(actualResult.getInformationContext()); + assertFalse(actualResult.complete()); + assertNull(actualResult.informationContext()); } private void assertAllocateStatus( @@ -537,7 +537,7 @@ private void assertAllocateStatus( .routingTable(RoutingTable.builder().add(indexRoutingTable).build()) .build(); Result actualResult = step.isConditionMet(index, clusterState); - assertEquals(expectedResult.isComplete(), actualResult.isComplete()); - assertEquals(expectedResult.getInformationContext(), actualResult.getInformationContext()); + assertEquals(expectedResult.complete(), actualResult.complete()); + assertEquals(expectedResult.informationContext(), actualResult.informationContext()); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckNoDataStreamWriteIndexStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckNoDataStreamWriteIndexStepTests.java index af9aa0982d61d..54c6ceb814af8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckNoDataStreamWriteIndexStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckNoDataStreamWriteIndexStepTests.java @@ -59,8 +59,8 @@ public void testStepCompleteIfIndexIsNotPartOfDataStream() { .build(); ClusterStateWaitStep.Result result = createRandomInstance().isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), is(nullValue())); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), is(nullValue())); } public void testStepIncompleteIfIndexIsTheDataStreamWriteIndex() { @@ -94,10 +94,10 @@ public void testStepIncompleteIfIndexIsTheDataStreamWriteIndex() { IndexMetadata indexToOperateOn = useFailureStore ? failureIndexMetadata : indexMetadata; String expectedIndexName = indexToOperateOn.getIndex().getName(); ClusterStateWaitStep.Result result = createRandomInstance().isConditionMet(indexToOperateOn.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); - SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.getInformationContext(); + assertThat(result.complete(), is(false)); + SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.informationContext(); assertThat( - info.getMessage(), + info.message(), is( "index [" + expectedIndexName @@ -161,7 +161,7 @@ public void testStepCompleteIfPartOfDataStreamButNotWriteIndex() { boolean useFailureStore = randomBoolean(); IndexMetadata indexToOperateOn = useFailureStore ? failureIndexMetadata : indexMetadata; ClusterStateWaitStep.Result result = createRandomInstance().isConditionMet(indexToOperateOn.getIndex(), clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), is(nullValue())); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), is(nullValue())); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckShrinkReadyStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckShrinkReadyStepTests.java index 371f7def67c52..8dcd8fc7ddd55 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckShrinkReadyStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckShrinkReadyStepTests.java @@ -417,8 +417,8 @@ public void testExecuteIndexMissing() throws Exception { CheckShrinkReadyStep step = createRandomInstance(); ClusterStateWaitStep.Result actualResult = step.isConditionMet(index, clusterState); - assertFalse(actualResult.isComplete()); - assertNull(actualResult.getInformationContext()); + assertFalse(actualResult.complete()); + assertNull(actualResult.informationContext()); } public void testStepCompletableIfAllShardsActive() { @@ -495,7 +495,7 @@ public void testStepCompletableIfAllShardsActive() { .build(); assertTrue(step.isCompletable()); ClusterStateWaitStep.Result actualResult = step.isConditionMet(index, clusterState); - assertTrue(actualResult.isComplete()); + assertTrue(actualResult.complete()); assertTrue(step.isCompletable()); } } @@ -574,9 +574,9 @@ public void testStepBecomesUncompletable() { .build(); assertTrue(step.isCompletable()); ClusterStateWaitStep.Result actualResult = step.isConditionMet(index, clusterState); - assertFalse(actualResult.isComplete()); + assertFalse(actualResult.complete()); assertThat( - Strings.toString(actualResult.getInformationContext()), + Strings.toString(actualResult.informationContext()), containsString("node with id [node1] is currently marked as shutting down") ); assertFalse(step.isCompletable()); @@ -625,8 +625,8 @@ private void assertAllocateStatus( .routingTable(RoutingTable.builder().add(indexRoutingTable).build()) .build(); ClusterStateWaitStep.Result actualResult = step.isConditionMet(index, clusterState); - assertEquals(expectedResult.isComplete(), actualResult.isComplete()); - assertEquals(expectedResult.getInformationContext(), actualResult.getInformationContext()); + assertEquals(expectedResult.complete(), actualResult.complete()); + assertEquals(expectedResult.informationContext(), actualResult.informationContext()); } public static UnassignedInfo randomUnassignedInfo(String message) { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckTargetShardsCountStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckTargetShardsCountStepTests.java index 8eb8d0f395aba..23d24fbd28730 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckTargetShardsCountStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckTargetShardsCountStepTests.java @@ -56,7 +56,7 @@ public void testStepCompleteIfTargetShardsCountIsValid() { CheckTargetShardsCountStep checkTargetShardsCountStep = new CheckTargetShardsCountStep(randomStepKey(), randomStepKey(), 2); ClusterStateWaitStep.Result result = checkTargetShardsCountStep.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(true)); + assertThat(result.complete(), is(true)); } public void testStepIncompleteIfTargetShardsCountNotValid() { @@ -75,10 +75,10 @@ public void testStepIncompleteIfTargetShardsCountNotValid() { CheckTargetShardsCountStep checkTargetShardsCountStep = new CheckTargetShardsCountStep(randomStepKey(), randomStepKey(), 3); ClusterStateWaitStep.Result result = checkTargetShardsCountStep.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); - SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.getInformationContext(); + assertThat(result.complete(), is(false)); + SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.informationContext(); assertThat( - info.getMessage(), + info.message(), is( "lifecycle action of policy [" + policyName diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitUntilThresholdStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitUntilThresholdStepTests.java index ea583b51c4c28..f24f0f86de7db 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitUntilThresholdStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ClusterStateWaitUntilThresholdStepTests.java @@ -70,8 +70,8 @@ public void testIndexIsMissingReturnsIncompleteResult() { new Index("testName", UUID.randomUUID().toString()), ClusterState.EMPTY_STATE ); - assertThat(result.isComplete(), is(false)); - assertThat(result.getInformationContext(), nullValue()); + assertThat(result.complete(), is(false)); + assertThat(result.informationContext(), nullValue()); } public void testIsConditionMetForUnderlyingStep() { @@ -95,8 +95,8 @@ public void testIsConditionMetForUnderlyingStep() { ClusterStateWaitUntilThresholdStep underTest = new ClusterStateWaitUntilThresholdStep(stepToExecute, randomStepKey()); ClusterStateWaitStep.Result result = underTest.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), nullValue()); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), nullValue()); } { @@ -120,10 +120,10 @@ public void testIsConditionMetForUnderlyingStep() { ClusterStateWaitUntilThresholdStep underTest = new ClusterStateWaitUntilThresholdStep(stepToExecute, randomStepKey()); ClusterStateWaitStep.Result result = underTest.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); - assertThat(result.getInformationContext(), notNullValue()); + assertThat(result.complete(), is(false)); + assertThat(result.informationContext(), notNullValue()); WaitForIndexingCompleteStep.IndexingNotCompleteInfo info = (WaitForIndexingCompleteStep.IndexingNotCompleteInfo) result - .getInformationContext(); + .informationContext(); assertThat( info.getMessage(), equalTo( @@ -154,8 +154,8 @@ public void testIsConditionMetForUnderlyingStep() { ClusterStateWaitUntilThresholdStep underTest = new ClusterStateWaitUntilThresholdStep(stepToExecute, nextKeyOnThresholdBreach); ClusterStateWaitStep.Result result = underTest.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), nullValue()); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), nullValue()); assertThat(underTest.getNextStepKey(), is(not(nextKeyOnThresholdBreach))); assertThat(underTest.getNextStepKey(), is(stepToExecute.getNextStepKey())); } @@ -184,11 +184,11 @@ public void testIsConditionMetForUnderlyingStep() { ClusterStateWaitUntilThresholdStep underTest = new ClusterStateWaitUntilThresholdStep(stepToExecute, nextKeyOnThresholdBreach); ClusterStateWaitStep.Result result = underTest.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), notNullValue()); - SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.getInformationContext(); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), notNullValue()); + SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.informationContext(); assertThat( - info.getMessage(), + info.message(), equalTo( "[" + currentStepKey.name() @@ -267,7 +267,7 @@ public boolean isRetryable() { new StepKey("phase", "action", "breached") ); - assertFalse(step.isConditionMet(indexMetadata.getIndex(), clusterState).isComplete()); + assertFalse(step.isConditionMet(indexMetadata.getIndex(), clusterState).complete()); assertThat(step.getNextStepKey().name(), equalTo("next-key")); @@ -290,7 +290,7 @@ public boolean isRetryable() { }, new StepKey("phase", "action", "breached") ); - assertTrue(step.isConditionMet(indexMetadata.getIndex(), clusterState).isComplete()); + assertTrue(step.isConditionMet(indexMetadata.getIndex(), clusterState).complete()); assertThat(step.getNextStepKey().name(), equalTo("breached")); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java index 95c1f5c4aa96b..51d1464ed5525 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java @@ -89,8 +89,8 @@ public void testExecuteWithUnassignedShard() { Result expectedResult = new Result(false, waitingForActiveShardsAllocationInfo(1)); Result actualResult = step.isConditionMet(index, clusterState); - assertThat(actualResult.isComplete(), is(false)); - assertThat(actualResult.getInformationContext(), is(expectedResult.getInformationContext())); + assertThat(actualResult.complete(), is(false)); + assertThat(actualResult.informationContext(), is(expectedResult.informationContext())); } public void testExecuteWithPendingShards() { @@ -129,8 +129,8 @@ public void testExecuteWithPendingShards() { ); Result actualResult = step.isConditionMet(index, clusterState); - assertThat(actualResult.isComplete(), is(false)); - assertThat(actualResult.getInformationContext(), is(expectedResult.getInformationContext())); + assertThat(actualResult.complete(), is(false)); + assertThat(actualResult.informationContext(), is(expectedResult.informationContext())); } public void testExecuteWithPendingShardsAndTargetRoleNotPresentInCluster() { @@ -163,8 +163,8 @@ public void testExecuteWithPendingShardsAndTargetRoleNotPresentInCluster() { ); Result actualResult = step.isConditionMet(index, clusterState); - assertThat(actualResult.isComplete(), is(false)); - assertThat(actualResult.getInformationContext(), is(expectedResult.getInformationContext())); + assertThat(actualResult.complete(), is(false)); + assertThat(actualResult.informationContext(), is(expectedResult.informationContext())); } public void testExecuteIndexMissing() { @@ -174,8 +174,8 @@ public void testExecuteIndexMissing() { DataTierMigrationRoutedStep step = createRandomInstance(); Result actualResult = step.isConditionMet(index, clusterState); - assertThat(actualResult.isComplete(), is(false)); - assertThat(actualResult.getInformationContext(), is(nullValue())); + assertThat(actualResult.complete(), is(false)); + assertThat(actualResult.informationContext(), is(nullValue())); } public void testExecuteIsComplete() { @@ -199,8 +199,8 @@ public void testExecuteIsComplete() { .build(); DataTierMigrationRoutedStep step = createRandomInstance(); Result result = step.isConditionMet(index, clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), is(nullValue())); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), is(nullValue())); } public void testExecuteWithGenericDataNodes() { @@ -220,8 +220,8 @@ public void testExecuteWithGenericDataNodes() { .build(); DataTierMigrationRoutedStep step = createRandomInstance(); Result result = step.isConditionMet(index, clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), is(nullValue())); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), is(nullValue())); } public void testExecuteForIndexWithoutTierRoutingInformationWaitsForReplicasToBeActive() { @@ -245,8 +245,8 @@ public void testExecuteForIndexWithoutTierRoutingInformationWaitsForReplicasToBe Result expectedResult = new Result(false, waitingForActiveShardsAllocationInfo(1)); Result result = step.isConditionMet(index, clusterState); - assertThat(result.isComplete(), is(false)); - assertThat(result.getInformationContext(), is(expectedResult.getInformationContext())); + assertThat(result.complete(), is(false)); + assertThat(result.informationContext(), is(expectedResult.informationContext())); } { @@ -266,8 +266,8 @@ public void testExecuteForIndexWithoutTierRoutingInformationWaitsForReplicasToBe DataTierMigrationRoutedStep step = createRandomInstance(); Result result = step.isConditionMet(index, clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), is(nullValue())); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), is(nullValue())); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SegmentCountStepInfoTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SegmentCountStepInfoTests.java index 7aeeba557ee54..9e0c7c7c6b167 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SegmentCountStepInfoTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SegmentCountStepInfoTests.java @@ -38,7 +38,7 @@ public final void testEqualsAndHashcode() { } protected final Info copyInstance(Info instance) throws IOException { - return new Info(instance.getNumberShardsLeftToMerge()); + return new Info(instance.numberShardsLeftToMerge()); } protected Info mutateInstance(Info instance) throws IOException { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrunkShardsAllocatedStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrunkShardsAllocatedStepTests.java index 59eff971c1643..592d259f07069 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrunkShardsAllocatedStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrunkShardsAllocatedStepTests.java @@ -94,8 +94,8 @@ public void testConditionMet() { .build(); Result result = step.isConditionMet(originalIndexMetadata.getIndex(), clusterState); - assertTrue(result.isComplete()); - assertNull(result.getInformationContext()); + assertTrue(result.complete()); + assertNull(result.informationContext()); } public void testConditionNotMetBecauseOfActive() { @@ -137,8 +137,8 @@ public void testConditionNotMetBecauseOfActive() { .build(); Result result = step.isConditionMet(originalIndexMetadata.getIndex(), clusterState); - assertFalse(result.isComplete()); - assertEquals(new ShrunkShardsAllocatedStep.Info(true, shrinkNumberOfShards, false), result.getInformationContext()); + assertFalse(result.complete()); + assertEquals(new ShrunkShardsAllocatedStep.Info(true, shrinkNumberOfShards, false), result.informationContext()); } public void testConditionNotMetBecauseOfShrunkIndexDoesntExistYet() { @@ -166,7 +166,7 @@ public void testConditionNotMetBecauseOfShrunkIndexDoesntExistYet() { .build(); Result result = step.isConditionMet(originalIndexMetadata.getIndex(), clusterState); - assertFalse(result.isComplete()); - assertEquals(new ShrunkShardsAllocatedStep.Info(false, -1, false), result.getInformationContext()); + assertFalse(result.complete()); + assertEquals(new ShrunkShardsAllocatedStep.Info(false, -1, false), result.informationContext()); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrunkenIndexCheckStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrunkenIndexCheckStepTests.java index 523404a00a0c5..4eb49df7f89c5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrunkenIndexCheckStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrunkenIndexCheckStepTests.java @@ -59,8 +59,8 @@ public void testConditionMet() { ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metadata(metadata).build(); Result result = step.isConditionMet(indexMetadata.getIndex(), clusterState); - assertTrue(result.isComplete()); - assertNull(result.getInformationContext()); + assertTrue(result.complete()); + assertNull(result.informationContext()); } public void testConditionNotMetBecauseNotSameShrunkenIndex() { @@ -77,8 +77,8 @@ public void testConditionNotMetBecauseNotSameShrunkenIndex() { .build(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metadata(metadata).build(); Result result = step.isConditionMet(shrinkIndexMetadata.getIndex(), clusterState); - assertFalse(result.isComplete()); - assertEquals(new ShrunkenIndexCheckStep.Info(sourceIndex), result.getInformationContext()); + assertFalse(result.complete()); + assertEquals(new ShrunkenIndexCheckStep.Info(sourceIndex), result.informationContext()); } public void testConditionNotMetBecauseSourceIndexExists() { @@ -101,8 +101,8 @@ public void testConditionNotMetBecauseSourceIndexExists() { .build(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metadata(metadata).build(); Result result = step.isConditionMet(shrinkIndexMetadata.getIndex(), clusterState); - assertFalse(result.isComplete()); - assertEquals(new ShrunkenIndexCheckStep.Info(sourceIndex), result.getInformationContext()); + assertFalse(result.complete()); + assertEquals(new ShrunkenIndexCheckStep.Info(sourceIndex), result.informationContext()); } public void testIllegalState() { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForActiveShardsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForActiveShardsTests.java index e12bae3b92f80..328698254dc76 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForActiveShardsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForActiveShardsTests.java @@ -125,7 +125,7 @@ public void testResultEvaluatedOnWriteIndexAliasWhenExists() { assertThat( "the rolled index has both the primary and the replica shards started so the condition should be met", - createRandomInstance().isConditionMet(originalIndex.getIndex(), clusterState).isComplete(), + createRandomInstance().isConditionMet(originalIndex.getIndex(), clusterState).complete(), is(true) ); } @@ -163,7 +163,7 @@ public void testResultEvaluatedOnOnlyIndexTheAliasPointsToIfWriteIndexIsNull() { assertThat( "the index the alias is pointing to has both the primary and the replica shards started so the condition should be" + " met", - createRandomInstance().isConditionMet(originalIndex.getIndex(), clusterState).isComplete(), + createRandomInstance().isConditionMet(originalIndex.getIndex(), clusterState).complete(), is(true) ); } @@ -244,13 +244,13 @@ public void testResultEvaluatedOnDataStream() throws IOException { boolean useFailureStore = randomBoolean(); IndexMetadata indexToOperateOn = useFailureStore ? failureOriginalIndexMeta : originalIndexMeta; ClusterStateWaitStep.Result result = waitForActiveShardsStep.isConditionMet(indexToOperateOn.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); + assertThat(result.complete(), is(false)); XContentBuilder expected = new WaitForActiveShardsStep.ActiveShardsInfo(2, "3", false).toXContent( JsonXContent.contentBuilder(), ToXContent.EMPTY_PARAMS ); - String actualResultAsString = Strings.toString(result.getInformationContext()); + String actualResultAsString = Strings.toString(result.informationContext()); assertThat(actualResultAsString, is(Strings.toString(expected))); assertThat(actualResultAsString, containsString("waiting for [3] shards to become active, but only [2] are active")); } @@ -288,13 +288,13 @@ public void testResultReportsMeaningfulMessage() throws IOException { .build(); ClusterStateWaitStep.Result result = createRandomInstance().isConditionMet(originalIndex.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); + assertThat(result.complete(), is(false)); XContentBuilder expected = new WaitForActiveShardsStep.ActiveShardsInfo(2, "3", false).toXContent( JsonXContent.contentBuilder(), ToXContent.EMPTY_PARAMS ); - String actualResultAsString = Strings.toString(result.getInformationContext()); + String actualResultAsString = Strings.toString(result.informationContext()); assertThat(actualResultAsString, is(Strings.toString(expected))); assertThat(actualResultAsString, containsString("waiting for [3] shards to become active, but only [2] are active")); } @@ -316,9 +316,9 @@ public void testResultReportsErrorMessage() { WaitForActiveShardsStep step = createRandomInstance(); ClusterStateWaitStep.Result result = step.isConditionMet(new Index("index-000000", UUID.randomUUID().toString()), clusterState); - assertThat(result.isComplete(), is(false)); + assertThat(result.complete(), is(false)); - String actualResultAsString = Strings.toString(result.getInformationContext()); + String actualResultAsString = Strings.toString(result.informationContext()); assertThat( actualResultAsString, containsString( diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForDataTierStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForDataTierStepTests.java index 3247c02cd9bac..00012575ea5de 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForDataTierStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForDataTierStepTests.java @@ -79,11 +79,11 @@ public void testConditionMet() { private void verify(WaitForDataTierStep step, ClusterState state, boolean complete, String message) { ClusterStateWaitStep.Result result = step.isConditionMet(null, state); - assertThat(result.isComplete(), is(complete)); + assertThat(result.complete(), is(complete)); if (message != null) { - assertThat(Strings.toString(result.getInformationContext()), containsString(message)); + assertThat(Strings.toString(result.informationContext()), containsString(message)); } else { - assertThat(result.getInformationContext(), is(nullValue())); + assertThat(result.informationContext(), is(nullValue())); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStepInfoTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStepInfoTests.java index 62c12e272ef59..0e5323d51f155 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStepInfoTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStepInfoTests.java @@ -10,7 +10,7 @@ import org.elasticsearch.xcontent.ConstructingObjectParser; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xpack.core.ilm.WaitForFollowShardTasksStep.Info; -import org.elasticsearch.xpack.core.ilm.WaitForFollowShardTasksStep.Info.ShardFollowTaskInfo; +import org.elasticsearch.xpack.core.ilm.WaitForFollowShardTasksStep.ShardFollowTaskInfo; import java.io.IOException; import java.util.ArrayList; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStepTests.java index 162f0ec3361b4..4ac5511a247c9 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStepTests.java @@ -131,10 +131,10 @@ public void onFailure(Exception e) { assertThat(informationContextHolder[0], notNullValue()); assertThat(exceptionHolder[0], nullValue()); WaitForFollowShardTasksStep.Info info = (WaitForFollowShardTasksStep.Info) informationContextHolder[0]; - assertThat(info.getShardFollowTaskInfos().size(), equalTo(1)); - assertThat(info.getShardFollowTaskInfos().get(0).getShardId(), equalTo(1)); - assertThat(info.getShardFollowTaskInfos().get(0).getLeaderGlobalCheckpoint(), equalTo(8L)); - assertThat(info.getShardFollowTaskInfos().get(0).getFollowerGlobalCheckpoint(), equalTo(3L)); + assertThat(info.shardFollowTaskInfos().size(), equalTo(1)); + assertThat(info.shardFollowTaskInfos().get(0).shardId(), equalTo(1)); + assertThat(info.shardFollowTaskInfos().get(0).leaderGlobalCheckpoint(), equalTo(8L)); + assertThat(info.shardFollowTaskInfos().get(0).followerGlobalCheckpoint(), equalTo(3L)); } public void testConditionNotMetNotAFollowerIndex() { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForIndexColorStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForIndexColorStepTests.java index 0ae7b02c7400a..1414788f3ff98 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForIndexColorStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForIndexColorStepTests.java @@ -93,8 +93,8 @@ public void testConditionMetForGreen() { WaitForIndexColorStep step = new WaitForIndexColorStep(randomStepKey(), randomStepKey(), ClusterHealthStatus.GREEN); ClusterStateWaitStep.Result result = step.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), nullValue()); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), nullValue()); } public void testConditionNotMetForGreen() { @@ -119,10 +119,10 @@ public void testConditionNotMetForGreen() { WaitForIndexColorStep step = new WaitForIndexColorStep(randomStepKey(), randomStepKey(), ClusterHealthStatus.GREEN); ClusterStateWaitStep.Result result = step.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); - SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.getInformationContext(); + assertThat(result.complete(), is(false)); + SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.informationContext(); assertThat(info, notNullValue()); - assertThat(info.getMessage(), equalTo("index is not green; not all shards are active")); + assertThat(info.message(), equalTo("index is not green; not all shards are active")); } public void testConditionNotMetNoIndexRoutingTable() { @@ -139,10 +139,10 @@ public void testConditionNotMetNoIndexRoutingTable() { WaitForIndexColorStep step = new WaitForIndexColorStep(randomStepKey(), randomStepKey(), ClusterHealthStatus.YELLOW); ClusterStateWaitStep.Result result = step.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); - SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.getInformationContext(); + assertThat(result.complete(), is(false)); + SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.informationContext(); assertThat(info, notNullValue()); - assertThat(info.getMessage(), equalTo("index is red; no indexRoutingTable")); + assertThat(info.message(), equalTo("index is red; no indexRoutingTable")); } public void testConditionMetForYellow() { @@ -167,8 +167,8 @@ public void testConditionMetForYellow() { WaitForIndexColorStep step = new WaitForIndexColorStep(randomStepKey(), randomStepKey(), ClusterHealthStatus.YELLOW); ClusterStateWaitStep.Result result = step.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), nullValue()); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), nullValue()); } public void testConditionNotMetForYellow() { @@ -193,10 +193,10 @@ public void testConditionNotMetForYellow() { WaitForIndexColorStep step = new WaitForIndexColorStep(randomStepKey(), randomStepKey(), ClusterHealthStatus.YELLOW); ClusterStateWaitStep.Result result = step.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); - SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.getInformationContext(); + assertThat(result.complete(), is(false)); + SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.informationContext(); assertThat(info, notNullValue()); - assertThat(info.getMessage(), equalTo("index is red; not all primary shards are active")); + assertThat(info.message(), equalTo("index is red; not all primary shards are active")); } public void testConditionNotMetNoIndexRoutingTableForYellow() { @@ -213,10 +213,10 @@ public void testConditionNotMetNoIndexRoutingTableForYellow() { WaitForIndexColorStep step = new WaitForIndexColorStep(randomStepKey(), randomStepKey(), ClusterHealthStatus.YELLOW); ClusterStateWaitStep.Result result = step.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); - SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.getInformationContext(); + assertThat(result.complete(), is(false)); + SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.informationContext(); assertThat(info, notNullValue()); - assertThat(info.getMessage(), equalTo("index is red; no indexRoutingTable")); + assertThat(info.message(), equalTo("index is red; no indexRoutingTable")); } public void testStepReturnsFalseIfTargetIndexIsMissing() { @@ -243,11 +243,11 @@ public void testStepReturnsFalseIfTargetIndexIsMissing() { WaitForIndexColorStep step = new WaitForIndexColorStep(randomStepKey(), randomStepKey(), ClusterHealthStatus.GREEN, indexPrefix); ClusterStateWaitStep.Result result = step.isConditionMet(originalIndex.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); - SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.getInformationContext(); + assertThat(result.complete(), is(false)); + SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.informationContext(); String targetIndex = indexPrefix + originalIndex.getIndex().getName(); assertThat( - info.getMessage(), + info.message(), is( "[" + step.getKey().action() @@ -303,9 +303,9 @@ public void testStepWaitsForTargetIndexHealthWhenPrefixConfigured() { WaitForIndexColorStep step = new WaitForIndexColorStep(randomStepKey(), randomStepKey(), ClusterHealthStatus.GREEN); ClusterStateWaitStep.Result result = step.isConditionMet(originalIndex.getIndex(), clusterTargetInitializing); - assertThat(result.isComplete(), is(false)); - SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.getInformationContext(); - assertThat(info.getMessage(), is("index is not green; not all shards are active")); + assertThat(result.complete(), is(false)); + SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.informationContext(); + assertThat(info.message(), is("index is not green; not all shards are active")); } { @@ -326,8 +326,8 @@ public void testStepWaitsForTargetIndexHealthWhenPrefixConfigured() { WaitForIndexColorStep step = new WaitForIndexColorStep(randomStepKey(), randomStepKey(), ClusterHealthStatus.GREEN); ClusterStateWaitStep.Result result = step.isConditionMet(originalIndex.getIndex(), clusterTargetInitializing); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), nullValue()); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), nullValue()); } } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForIndexingCompleteStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForIndexingCompleteStepTests.java index ad5e4c9533c99..2f91393b451d7 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForIndexingCompleteStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForIndexingCompleteStepTests.java @@ -65,8 +65,8 @@ public void testConditionMet() { WaitForIndexingCompleteStep step = createRandomInstance(); ClusterStateWaitStep.Result result = step.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), nullValue()); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), nullValue()); } public void testConditionMetNotAFollowerIndex() { @@ -82,8 +82,8 @@ public void testConditionMetNotAFollowerIndex() { WaitForIndexingCompleteStep step = createRandomInstance(); ClusterStateWaitStep.Result result = step.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(true)); - assertThat(result.getInformationContext(), nullValue()); + assertThat(result.complete(), is(true)); + assertThat(result.informationContext(), nullValue()); } public void testConditionNotMet() { @@ -104,10 +104,10 @@ public void testConditionNotMet() { WaitForIndexingCompleteStep step = createRandomInstance(); ClusterStateWaitStep.Result result = step.isConditionMet(indexMetadata.getIndex(), clusterState); - assertThat(result.isComplete(), is(false)); - assertThat(result.getInformationContext(), notNullValue()); + assertThat(result.complete(), is(false)); + assertThat(result.informationContext(), notNullValue()); WaitForIndexingCompleteStep.IndexingNotCompleteInfo info = (WaitForIndexingCompleteStep.IndexingNotCompleteInfo) result - .getInformationContext(); + .informationContext(); assertThat( info.getMessage(), equalTo( @@ -122,7 +122,7 @@ public void testIndexDeleted() { WaitForIndexingCompleteStep step = createRandomInstance(); ClusterStateWaitStep.Result result = step.isConditionMet(new Index("this-index-doesnt-exist", "uuid"), clusterState); - assertThat(result.isComplete(), is(false)); - assertThat(result.getInformationContext(), nullValue()); + assertThat(result.complete(), is(false)); + assertThat(result.informationContext(), nullValue()); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/step/info/AllocationRoutedStepInfoTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/step/info/AllocationRoutedStepInfoTests.java index 67214868293ea..0e6903ba6cf44 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/step/info/AllocationRoutedStepInfoTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/step/info/AllocationRoutedStepInfoTests.java @@ -38,18 +38,18 @@ public final void testEqualsAndHashcode() { protected final AllocationInfo copyInstance(AllocationInfo instance) { return new AllocationInfo( - instance.getNumberOfReplicas(), - instance.getNumberShardsLeftToAllocate(), + instance.numberOfReplicas(), + instance.numberShardsLeftToAllocate(), instance.allShardsActive(), - instance.getMessage() + instance.message() ); } protected AllocationInfo mutateInstance(AllocationInfo instance) throws IOException { - long actualReplicas = instance.getNumberOfReplicas(); - long shardsToAllocate = instance.getNumberShardsLeftToAllocate(); + long actualReplicas = instance.numberOfReplicas(); + long shardsToAllocate = instance.numberShardsLeftToAllocate(); boolean allShardsActive = instance.allShardsActive(); - var message = instance.getMessage(); + var message = instance.message(); switch (between(0, 2)) { case 0 -> shardsToAllocate += between(1, 20); case 1 -> allShardsActive = allShardsActive == false; diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/cluster/metadata/MetadataMigrateToDataTiersRoutingService.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/cluster/metadata/MetadataMigrateToDataTiersRoutingService.java index e06c7bc2708ca..9efe46402428c 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/cluster/metadata/MetadataMigrateToDataTiersRoutingService.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/cluster/metadata/MetadataMigrateToDataTiersRoutingService.java @@ -43,7 +43,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.SortedMap; import java.util.TreeMap; @@ -811,13 +810,12 @@ static String convertAttributeValueToTierPreference(String nodeAttributeValue) { * Represents the elasticsearch abstractions that were, in some way, migrated such that the system is managing indices lifecycles and * allocations using data tiers. */ - public static final class MigratedEntities { - @Nullable - public final String removedIndexTemplateName; - public final List migratedIndices; - public final List migratedPolicies; - public final MigratedTemplates migratedTemplates; - + public record MigratedEntities( + @Nullable String removedIndexTemplateName, + List migratedIndices, + List migratedPolicies, + MigratedTemplates migratedTemplates + ) { public MigratedEntities( @Nullable String removedIndexTemplateName, List migratedIndices, @@ -830,36 +828,17 @@ public MigratedEntities( this.migratedTemplates = migratedTemplates; } - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - MigratedEntities that = (MigratedEntities) o; - return Objects.equals(removedIndexTemplateName, that.removedIndexTemplateName) - && Objects.equals(migratedIndices, that.migratedIndices) - && Objects.equals(migratedPolicies, that.migratedPolicies) - && Objects.equals(migratedTemplates, that.migratedTemplates); - } - - @Override - public int hashCode() { - return Objects.hash(removedIndexTemplateName, migratedIndices, migratedPolicies, migratedTemplates); - } } /** * Represents the legacy, composable, and component templates that were migrated away from shard allocation settings based on custom * node attributes. */ - public static final class MigratedTemplates { - public final List migratedLegacyTemplates; - public final List migratedComposableTemplates; - public final List migratedComponentTemplates; - + public record MigratedTemplates( + List migratedLegacyTemplates, + List migratedComposableTemplates, + List migratedComponentTemplates + ) { public MigratedTemplates( List migratedLegacyTemplates, List migratedComposableTemplates, @@ -869,24 +848,5 @@ public MigratedTemplates( this.migratedComposableTemplates = Collections.unmodifiableList(migratedComposableTemplates); this.migratedComponentTemplates = Collections.unmodifiableList(migratedComponentTemplates); } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - MigratedTemplates that = (MigratedTemplates) o; - return Objects.equals(migratedLegacyTemplates, that.migratedLegacyTemplates) - && Objects.equals(migratedComposableTemplates, that.migratedComposableTemplates) - && Objects.equals(migratedComponentTemplates, that.migratedComponentTemplates); - } - - @Override - public int hashCode() { - return Objects.hash(migratedLegacyTemplates, migratedComposableTemplates, migratedComponentTemplates); - } } } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/ExecuteStepsUpdateTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/ExecuteStepsUpdateTask.java index 77b143f93576b..8c08194b11e05 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/ExecuteStepsUpdateTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/ExecuteStepsUpdateTask.java @@ -159,7 +159,7 @@ public ClusterState doExecute(final ClusterState currentState) throws IOExceptio // to be met (eg. {@link LifecycleSettings#LIFECYCLE_STEP_WAIT_TIME_THRESHOLD_SETTING}, so it's important we // re-evaluate what the next step is after we evaluate the condition nextStepKey = currentStep.getNextStepKey(); - if (result.isComplete()) { + if (result.complete()) { logger.trace( "[{}] cluster state step condition met successfully ({}) [{}], moving to next step {}", index.getName(), @@ -180,7 +180,7 @@ public ClusterState doExecute(final ClusterState currentState) throws IOExceptio ); } } else { - final ToXContentObject stepInfo = result.getInformationContext(); + final ToXContentObject stepInfo = result.informationContext(); if (logger.isTraceEnabled()) { logger.trace( "[{}] condition not met ({}) [{}], returning existing state (info: {})", diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportMigrateToDataTiersAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportMigrateToDataTiersAction.java index 494f0ee444236..ef7554beed9e9 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportMigrateToDataTiersAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportMigrateToDataTiersAction.java @@ -100,12 +100,12 @@ protected void masterOperation( ).v2(); listener.onResponse( new MigrateToDataTiersResponse( - entities.removedIndexTemplateName, - entities.migratedPolicies, - entities.migratedIndices, - entities.migratedTemplates.migratedLegacyTemplates, - entities.migratedTemplates.migratedComposableTemplates, - entities.migratedTemplates.migratedComponentTemplates, + entities.removedIndexTemplateName(), + entities.migratedPolicies(), + entities.migratedIndices(), + entities.migratedTemplates().migratedLegacyTemplates(), + entities.migratedTemplates().migratedComposableTemplates(), + entities.migratedTemplates().migratedComponentTemplates(), true ) ); @@ -161,12 +161,12 @@ public void onFailure(Exception e) { MigratedEntities entities = migratedEntities.get(); listener.onResponse( new MigrateToDataTiersResponse( - entities.removedIndexTemplateName, - entities.migratedPolicies, - entities.migratedIndices, - entities.migratedTemplates.migratedLegacyTemplates, - entities.migratedTemplates.migratedComposableTemplates, - entities.migratedTemplates.migratedComponentTemplates, + entities.removedIndexTemplateName(), + entities.migratedPolicies(), + entities.migratedIndices(), + entities.migratedTemplates().migratedLegacyTemplates(), + entities.migratedTemplates().migratedComposableTemplates(), + entities.migratedTemplates().migratedComponentTemplates(), false ) ); diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/cluster/metadata/MetadataMigrateToDataTiersRoutingServiceTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/cluster/metadata/MetadataMigrateToDataTiersRoutingServiceTests.java index 51df651ea4a4c..570c2f5231acf 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/cluster/metadata/MetadataMigrateToDataTiersRoutingServiceTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/cluster/metadata/MetadataMigrateToDataTiersRoutingServiceTests.java @@ -1080,11 +1080,11 @@ public void testMigrateToDataTiersRouting() { ); MigratedEntities migratedEntities = migratedEntitiesTuple.v2(); - assertThat(migratedEntities.removedIndexTemplateName, is("catch-all")); - assertThat(migratedEntities.migratedPolicies.size(), is(1)); - assertThat(migratedEntities.migratedPolicies.get(0), is(lifecycleName)); - assertThat(migratedEntities.migratedIndices.size(), is(2)); - assertThat(migratedEntities.migratedIndices, hasItems("indexWithWarmDataAttribute", "indexWithUnknownDataAttribute")); + assertThat(migratedEntities.removedIndexTemplateName(), is("catch-all")); + assertThat(migratedEntities.migratedPolicies().size(), is(1)); + assertThat(migratedEntities.migratedPolicies().get(0), is(lifecycleName)); + assertThat(migratedEntities.migratedIndices().size(), is(2)); + assertThat(migratedEntities.migratedIndices(), hasItems("indexWithWarmDataAttribute", "indexWithUnknownDataAttribute")); ClusterState newState = migratedEntitiesTuple.v1(); assertThat(newState.metadata().getTemplates().size(), is(1)); @@ -1105,11 +1105,11 @@ public void testMigrateToDataTiersRouting() { ); MigratedEntities migratedEntities = migratedEntitiesTuple.v2(); - assertThat(migratedEntities.removedIndexTemplateName, nullValue()); - assertThat(migratedEntities.migratedPolicies.size(), is(1)); - assertThat(migratedEntities.migratedPolicies.get(0), is(lifecycleName)); - assertThat(migratedEntities.migratedIndices.size(), is(2)); - assertThat(migratedEntities.migratedIndices, hasItems("indexWithWarmDataAttribute", "indexWithUnknownDataAttribute")); + assertThat(migratedEntities.removedIndexTemplateName(), nullValue()); + assertThat(migratedEntities.migratedPolicies().size(), is(1)); + assertThat(migratedEntities.migratedPolicies().get(0), is(lifecycleName)); + assertThat(migratedEntities.migratedIndices().size(), is(2)); + assertThat(migratedEntities.migratedIndices(), hasItems("indexWithWarmDataAttribute", "indexWithUnknownDataAttribute")); ClusterState newState = migratedEntitiesTuple.v1(); assertThat(newState.metadata().getTemplates().size(), is(2)); @@ -1130,10 +1130,10 @@ public void testMigrateToDataTiersRouting() { ); MigratedEntities migratedEntities = migratedEntitiesTuple.v2(); - assertThat(migratedEntities.migratedPolicies.size(), is(1)); - assertThat(migratedEntities.migratedPolicies.get(0), is(lifecycleName)); - assertThat(migratedEntities.migratedIndices.size(), is(2)); - assertThat(migratedEntities.migratedIndices, hasItems("indexWithWarmDataAttribute", "indexWithUnknownDataAttribute")); + assertThat(migratedEntities.migratedPolicies().size(), is(1)); + assertThat(migratedEntities.migratedPolicies().get(0), is(lifecycleName)); + assertThat(migratedEntities.migratedIndices().size(), is(2)); + assertThat(migratedEntities.migratedIndices(), hasItems("indexWithWarmDataAttribute", "indexWithUnknownDataAttribute")); IndexMetadata migratedIndex; migratedIndex = migratedEntitiesTuple.v1().metadata().index("indexWithWarmDataAttribute"); @@ -1185,9 +1185,9 @@ public void testMigrateToDataTiersRoutingRequiresILMStopped() { null, false ); - assertThat(migratedState.v2().migratedIndices, empty()); - assertThat(migratedState.v2().migratedPolicies, empty()); - assertThat(migratedState.v2().removedIndexTemplateName, nullValue()); + assertThat(migratedState.v2().migratedIndices(), empty()); + assertThat(migratedState.v2().migratedPolicies(), empty()); + assertThat(migratedState.v2().removedIndexTemplateName(), nullValue()); } } @@ -1232,7 +1232,7 @@ public void testMigrationDoesNotRemoveComposableTemplates() { null, false ); - assertThat(migratedEntitiesTuple.v2().removedIndexTemplateName, nullValue()); + assertThat(migratedEntitiesTuple.v2().removedIndexTemplateName(), nullValue()); // the composable template still exists, however it was migrated to not use the custom require.data routing setting assertThat(migratedEntitiesTuple.v1().metadata().templatesV2().get(composableTemplateName), is(notNullValue())); } @@ -1676,9 +1676,9 @@ public void testMigrateIndexAndComponentTemplates() { Metadata.Builder mb = Metadata.builder(clusterState.metadata()); MetadataMigrateToDataTiersRoutingService.MigratedTemplates migratedTemplates = MetadataMigrateToDataTiersRoutingService .migrateIndexAndComponentTemplates(mb, clusterState, nodeAttrName); - assertThat(migratedTemplates.migratedLegacyTemplates, is(List.of("template-with-require-routing"))); - assertThat(migratedTemplates.migratedComposableTemplates, is(List.of("composable-template-with-require-routing"))); - assertThat(migratedTemplates.migratedComponentTemplates, is(List.of("component-with-require-and-include-routing"))); + assertThat(migratedTemplates.migratedLegacyTemplates(), is(List.of("template-with-require-routing"))); + assertThat(migratedTemplates.migratedComposableTemplates(), is(List.of("composable-template-with-require-routing"))); + assertThat(migratedTemplates.migratedComponentTemplates(), is(List.of("component-with-require-and-include-routing"))); } private String getWarmPhaseDef() { From f4dc7168eb790e4c4b0540b2ea5b7b9072840ffe Mon Sep 17 00:00:00 2001 From: Jonathan Buttner <56361221+jonathan-buttner@users.noreply.github.com> Date: Wed, 11 Dec 2024 16:03:36 -0500 Subject: [PATCH 2/8] [ML] Checking for presence of error object when validating response (#118375) * Refactoring error handling logic * Refactoring base response handler to remove duplication * Update docs/changelog/118375.yaml * Addressing feedback --------- Co-authored-by: Max Hniebergall <137079448+maxhniebergall@users.noreply.github.com> --- docs/changelog/118375.yaml | 5 + .../AlibabaCloudSearchResponseHandler.java | 14 +-- .../anthropic/AnthropicResponseHandler.java | 13 +- .../cohere/CohereResponseHandler.java | 14 +-- ...lasticInferenceServiceResponseHandler.java | 12 +- .../GoogleAiStudioResponseHandler.java | 13 +- .../GoogleVertexAiResponseHandler.java | 11 +- .../http/retry/BaseResponseHandler.java | 44 ++++++- .../external/http/retry/ErrorMessage.java | 12 -- .../external/http/retry/ErrorResponse.java | 37 ++++++ .../HuggingFaceResponseHandler.java | 14 +-- .../ibmwatsonx/IbmWatsonxResponseHandler.java | 13 +- .../openai/OpenAiResponseHandler.java | 19 +-- ...eMistralOpenAiExternalResponseHandler.java | 4 +- .../response/ErrorMessageResponseEntity.java | 39 +++--- ...AlibabaCloudSearchErrorResponseEntity.java | 19 +-- .../cohere/CohereErrorResponseEntity.java | 19 +-- ...icInferenceServiceErrorResponseEntity.java | 20 +-- .../GoogleAiStudioErrorResponseEntity.java | 24 ++-- .../GoogleVertexAiErrorResponseEntity.java | 24 ++-- .../HuggingFaceErrorResponseEntity.java | 20 +-- .../IbmWatsonxErrorResponseEntity.java | 22 ++-- .../openai/OpenAiErrorResponseEntity.java | 69 ---------- .../http/retry/BaseResponseHandlerTests.java | 118 ++++++++++++++++++ .../ErrorMessageResponseEntityTests.java | 33 ++++- ...baCloudSearchErrorResponseEntityTests.java | 2 +- .../CohereErrorResponseEntityTests.java | 8 +- ...erenceServiceErrorResponseEntityTests.java | 12 +- ...oogleAiStudioErrorResponseEntityTests.java | 9 +- ...oogleVertexAiErrorResponseEntityTests.java | 10 +- .../HuggingFaceErrorResponseEntityTests.java | 12 +- .../OpenAiErrorResponseEntityTests.java | 66 ---------- 32 files changed, 348 insertions(+), 403 deletions(-) create mode 100644 docs/changelog/118375.yaml delete mode 100644 x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/ErrorMessage.java create mode 100644 x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/ErrorResponse.java delete mode 100644 x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/openai/OpenAiErrorResponseEntity.java delete mode 100644 x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/openai/OpenAiErrorResponseEntityTests.java diff --git a/docs/changelog/118375.yaml b/docs/changelog/118375.yaml new file mode 100644 index 0000000000000..bad2751aeaa50 --- /dev/null +++ b/docs/changelog/118375.yaml @@ -0,0 +1,5 @@ +pr: 118375 +summary: Check for presence of error object when validating streaming responses from integrations in the inference API +area: Machine Learning +type: enhancement +issues: [] diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/alibabacloudsearch/AlibabaCloudSearchResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/alibabacloudsearch/AlibabaCloudSearchResponseHandler.java index ecfa988b5035e..30b371f3172ea 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/alibabacloudsearch/AlibabaCloudSearchResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/alibabacloudsearch/AlibabaCloudSearchResponseHandler.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.inference.external.alibabacloudsearch; -import org.apache.logging.log4j.Logger; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xpack.inference.external.http.HttpResult; import org.elasticsearch.xpack.inference.external.http.retry.BaseResponseHandler; @@ -15,9 +14,6 @@ import org.elasticsearch.xpack.inference.external.http.retry.RetryException; import org.elasticsearch.xpack.inference.external.request.Request; import org.elasticsearch.xpack.inference.external.response.alibabacloudsearch.AlibabaCloudSearchErrorResponseEntity; -import org.elasticsearch.xpack.inference.logging.ThrottlerManager; - -import static org.elasticsearch.xpack.inference.external.http.HttpUtils.checkForEmptyBody; /** * Defines how to handle various errors returned from the AlibabaCloudSearch integration. @@ -28,13 +24,6 @@ public AlibabaCloudSearchResponseHandler(String requestType, ResponseParser pars super(requestType, parseFunction, AlibabaCloudSearchErrorResponseEntity::fromResponse); } - @Override - public void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) - throws RetryException { - checkForFailureStatusCode(request, result); - checkForEmptyBody(throttlerManager, logger, request, result); - } - /** * Validates the status code throws an RetryException if not in the range [200, 300). * @@ -42,7 +31,8 @@ public void validateResponse(ThrottlerManager throttlerManager, Logger logger, R * @param result The http response and body * @throws RetryException Throws if status code is {@code >= 300 or < 200 } */ - void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { + @Override + protected void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { int statusCode = result.response().getStatusLine().getStatusCode(); if (RestStatus.isSuccessful(statusCode)) { return; diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/anthropic/AnthropicResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/anthropic/AnthropicResponseHandler.java index 373045f879afa..d9a78a56af0d6 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/anthropic/AnthropicResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/anthropic/AnthropicResponseHandler.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.inference.external.anthropic; -import org.apache.logging.log4j.Logger; import org.elasticsearch.common.Strings; import org.elasticsearch.inference.InferenceServiceResults; import org.elasticsearch.xpack.core.inference.results.StreamingChatCompletionResults; @@ -19,11 +18,9 @@ import org.elasticsearch.xpack.inference.external.response.ErrorMessageResponseEntity; import org.elasticsearch.xpack.inference.external.response.streaming.ServerSentEventParser; import org.elasticsearch.xpack.inference.external.response.streaming.ServerSentEventProcessor; -import org.elasticsearch.xpack.inference.logging.ThrottlerManager; import java.util.concurrent.Flow; -import static org.elasticsearch.xpack.inference.external.http.HttpUtils.checkForEmptyBody; import static org.elasticsearch.xpack.inference.external.http.retry.ResponseHandlerUtils.getFirstHeaderOrUnknown; public class AnthropicResponseHandler extends BaseResponseHandler { @@ -54,13 +51,6 @@ public AnthropicResponseHandler(String requestType, ResponseParser parseFunction this.canHandleStreamingResponses = canHandleStreamingResponses; } - @Override - public void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) - throws RetryException { - checkForFailureStatusCode(request, result); - checkForEmptyBody(throttlerManager, logger, request, result); - } - @Override public boolean canHandleStreamingResponses() { return canHandleStreamingResponses; @@ -83,7 +73,8 @@ public InferenceServiceResults parseResult(Request request, Flow.Publisher= 300 or < 200 } */ - void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { + @Override + protected void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { if (result.isSuccessfulResponse()) { return; } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/cohere/CohereResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/cohere/CohereResponseHandler.java index ac2e1747f8057..e3a74785caa4b 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/cohere/CohereResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/cohere/CohereResponseHandler.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.inference.external.cohere; -import org.apache.logging.log4j.Logger; import org.elasticsearch.inference.InferenceServiceResults; import org.elasticsearch.xpack.core.inference.results.StreamingChatCompletionResults; import org.elasticsearch.xpack.inference.external.http.HttpResult; @@ -17,12 +16,9 @@ import org.elasticsearch.xpack.inference.external.request.Request; import org.elasticsearch.xpack.inference.external.response.cohere.CohereErrorResponseEntity; import org.elasticsearch.xpack.inference.external.response.streaming.NewlineDelimitedByteProcessor; -import org.elasticsearch.xpack.inference.logging.ThrottlerManager; import java.util.concurrent.Flow; -import static org.elasticsearch.xpack.inference.external.http.HttpUtils.checkForEmptyBody; - /** * Defines how to handle various errors returned from the Cohere integration. * @@ -45,13 +41,6 @@ public CohereResponseHandler(String requestType, ResponseParser parseFunction, b this.canHandleStreamingResponse = canHandleStreamingResponse; } - @Override - public void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) - throws RetryException { - checkForFailureStatusCode(request, result); - checkForEmptyBody(throttlerManager, logger, request, result); - } - @Override public boolean canHandleStreamingResponses() { return canHandleStreamingResponse; @@ -73,7 +62,8 @@ public InferenceServiceResults parseResult(Request request, Flow.Publisher= 300 or < 200 } */ - void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { + @Override + protected void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { if (result.isSuccessfulResponse()) { return; } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/elastic/ElasticInferenceServiceResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/elastic/ElasticInferenceServiceResponseHandler.java index 2b79afb3b56fd..b11b4a743fb27 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/elastic/ElasticInferenceServiceResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/elastic/ElasticInferenceServiceResponseHandler.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.inference.external.elastic; -import org.apache.logging.log4j.Logger; import org.elasticsearch.xpack.inference.external.http.HttpResult; import org.elasticsearch.xpack.inference.external.http.retry.BaseResponseHandler; import org.elasticsearch.xpack.inference.external.http.retry.ContentTooLargeException; @@ -15,9 +14,6 @@ import org.elasticsearch.xpack.inference.external.http.retry.RetryException; import org.elasticsearch.xpack.inference.external.request.Request; import org.elasticsearch.xpack.inference.external.response.elastic.ElasticInferenceServiceErrorResponseEntity; -import org.elasticsearch.xpack.inference.logging.ThrottlerManager; - -import static org.elasticsearch.xpack.inference.external.http.HttpUtils.checkForEmptyBody; public class ElasticInferenceServiceResponseHandler extends BaseResponseHandler { @@ -26,13 +22,7 @@ public ElasticInferenceServiceResponseHandler(String requestType, ResponseParser } @Override - public void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) - throws RetryException { - checkForFailureStatusCode(request, result); - checkForEmptyBody(throttlerManager, logger, request, result); - } - - void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { + protected void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { if (result.isSuccessfulResponse()) { return; } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/googleaistudio/GoogleAiStudioResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/googleaistudio/GoogleAiStudioResponseHandler.java index 0241dcd6142a6..d61e82cb83b45 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/googleaistudio/GoogleAiStudioResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/googleaistudio/GoogleAiStudioResponseHandler.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.inference.external.googleaistudio; -import org.apache.logging.log4j.Logger; import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.inference.InferenceServiceResults; import org.elasticsearch.xcontent.XContentParser; @@ -20,13 +19,11 @@ import org.elasticsearch.xpack.inference.external.response.googleaistudio.GoogleAiStudioErrorResponseEntity; import org.elasticsearch.xpack.inference.external.response.streaming.ServerSentEventParser; import org.elasticsearch.xpack.inference.external.response.streaming.ServerSentEventProcessor; -import org.elasticsearch.xpack.inference.logging.ThrottlerManager; import java.io.IOException; import java.util.concurrent.Flow; import static org.elasticsearch.core.Strings.format; -import static org.elasticsearch.xpack.inference.external.http.HttpUtils.checkForEmptyBody; public class GoogleAiStudioResponseHandler extends BaseResponseHandler { @@ -52,13 +49,6 @@ public GoogleAiStudioResponseHandler( this.content = content; } - @Override - public void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) - throws RetryException { - checkForFailureStatusCode(request, result); - checkForEmptyBody(throttlerManager, logger, request, result); - } - /** * Validates the status code and throws a RetryException if not in the range [200, 300). * @@ -67,7 +57,8 @@ public void validateResponse(ThrottlerManager throttlerManager, Logger logger, R * @param result The http response and body * @throws RetryException Throws if status code is {@code >= 300 or < 200 } */ - void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { + @Override + protected void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { if (result.isSuccessfulResponse()) { return; } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/googlevertexai/GoogleVertexAiResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/googlevertexai/GoogleVertexAiResponseHandler.java index 6b1aef9856d33..6924a5e4cb336 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/googlevertexai/GoogleVertexAiResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/googlevertexai/GoogleVertexAiResponseHandler.java @@ -7,17 +7,14 @@ package org.elasticsearch.xpack.inference.external.googlevertexai; -import org.apache.logging.log4j.Logger; import org.elasticsearch.xpack.inference.external.http.HttpResult; import org.elasticsearch.xpack.inference.external.http.retry.BaseResponseHandler; import org.elasticsearch.xpack.inference.external.http.retry.ResponseParser; import org.elasticsearch.xpack.inference.external.http.retry.RetryException; import org.elasticsearch.xpack.inference.external.request.Request; import org.elasticsearch.xpack.inference.external.response.googlevertexai.GoogleVertexAiErrorResponseEntity; -import org.elasticsearch.xpack.inference.logging.ThrottlerManager; import static org.elasticsearch.core.Strings.format; -import static org.elasticsearch.xpack.inference.external.http.HttpUtils.checkForEmptyBody; public class GoogleVertexAiResponseHandler extends BaseResponseHandler { @@ -28,13 +25,7 @@ public GoogleVertexAiResponseHandler(String requestType, ResponseParser parseFun } @Override - public void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) - throws RetryException { - checkForFailureStatusCode(request, result); - checkForEmptyBody(throttlerManager, logger, request, result); - } - - void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { + protected void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { if (result.isSuccessfulResponse()) { return; } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/BaseResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/BaseResponseHandler.java index c9cbe169ec03d..1b0dd893ada6f 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/BaseResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/BaseResponseHandler.java @@ -7,16 +7,20 @@ package org.elasticsearch.xpack.inference.external.http.retry; +import org.apache.logging.log4j.Logger; import org.elasticsearch.ElasticsearchStatusException; +import org.elasticsearch.common.Strings; import org.elasticsearch.inference.InferenceServiceResults; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xpack.inference.external.http.HttpResult; import org.elasticsearch.xpack.inference.external.request.Request; +import org.elasticsearch.xpack.inference.logging.ThrottlerManager; import java.util.Objects; import java.util.function.Function; import static org.elasticsearch.core.Strings.format; +import static org.elasticsearch.xpack.inference.external.http.HttpUtils.checkForEmptyBody; public abstract class BaseResponseHandler implements ResponseHandler { @@ -27,14 +31,15 @@ public abstract class BaseResponseHandler implements ResponseHandler { public static final String REDIRECTION = "Unhandled redirection"; public static final String CONTENT_TOO_LARGE = "Received a content too large status code"; public static final String UNSUCCESSFUL = "Received an unsuccessful status code"; + public static final String SERVER_ERROR_OBJECT = "Received an error response"; public static final String BAD_REQUEST = "Received a bad request status code"; public static final String METHOD_NOT_ALLOWED = "Received a method not allowed status code"; protected final String requestType; private final ResponseParser parseFunction; - private final Function errorParseFunction; + private final Function errorParseFunction; - public BaseResponseHandler(String requestType, ResponseParser parseFunction, Function errorParseFunction) { + public BaseResponseHandler(String requestType, ResponseParser parseFunction, Function errorParseFunction) { this.requestType = Objects.requireNonNull(requestType); this.parseFunction = Objects.requireNonNull(parseFunction); this.errorParseFunction = Objects.requireNonNull(errorParseFunction); @@ -54,11 +59,42 @@ public String getRequestType() { return requestType; } + @Override + public void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) { + checkForFailureStatusCode(request, result); + checkForEmptyBody(throttlerManager, logger, request, result); + + // When the response is streamed the status code could be 200 but the error object will be set + // so we need to check for that specifically + checkForErrorObject(request, result); + } + + protected abstract void checkForFailureStatusCode(Request request, HttpResult result); + + private void checkForErrorObject(Request request, HttpResult result) { + var errorEntity = errorParseFunction.apply(result); + + if (errorEntity.errorStructureFound()) { + // We don't really know what happened because the status code was 200 so we'll return a failure and let the + // client retry if necessary + // If we did want to retry here, we'll need to determine if this was a streaming request, if it was + // we shouldn't retry because that would replay the entire streaming request and the client would get + // duplicate chunks back + throw new RetryException(false, buildError(SERVER_ERROR_OBJECT, request, result, errorEntity)); + } + } + protected Exception buildError(String message, Request request, HttpResult result) { var errorEntityMsg = errorParseFunction.apply(result); + return buildError(message, request, result, errorEntityMsg); + } + + protected Exception buildError(String message, Request request, HttpResult result, ErrorResponse errorResponse) { var responseStatusCode = result.response().getStatusLine().getStatusCode(); - if (errorEntityMsg == null) { + if (errorResponse == null + || errorResponse.errorStructureFound() == false + || Strings.isNullOrEmpty(errorResponse.getErrorMessage())) { return new ElasticsearchStatusException( format( "%s for request from inference entity id [%s] status [%s]", @@ -76,7 +112,7 @@ protected Exception buildError(String message, Request request, HttpResult resul message, request.getInferenceEntityId(), responseStatusCode, - errorEntityMsg.getErrorMessage() + errorResponse.getErrorMessage() ), toRestStatus(responseStatusCode) ); diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/ErrorMessage.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/ErrorMessage.java deleted file mode 100644 index a4be7f15827fb..0000000000000 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/ErrorMessage.java +++ /dev/null @@ -1,12 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.inference.external.http.retry; - -public interface ErrorMessage { - String getErrorMessage(); -} diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/ErrorResponse.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/ErrorResponse.java new file mode 100644 index 0000000000000..be9669c331371 --- /dev/null +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/ErrorResponse.java @@ -0,0 +1,37 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.inference.external.http.retry; + +import java.util.Objects; + +public class ErrorResponse { + + // Denotes an error object that was not found + public static final ErrorResponse UNDEFINED_ERROR = new ErrorResponse(false); + + private final String errorMessage; + private final boolean errorStructureFound; + + public ErrorResponse(String errorMessage) { + this.errorMessage = Objects.requireNonNull(errorMessage); + this.errorStructureFound = true; + } + + private ErrorResponse(boolean errorStructureFound) { + this.errorMessage = ""; + this.errorStructureFound = errorStructureFound; + } + + public String getErrorMessage() { + return errorMessage; + } + + public boolean errorStructureFound() { + return errorStructureFound; + } +} diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/huggingface/HuggingFaceResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/huggingface/HuggingFaceResponseHandler.java index f6fd9afabe28d..5b6b0de4f2428 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/huggingface/HuggingFaceResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/huggingface/HuggingFaceResponseHandler.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.inference.external.huggingface; -import org.apache.logging.log4j.Logger; import org.elasticsearch.xpack.inference.external.http.HttpResult; import org.elasticsearch.xpack.inference.external.http.retry.BaseResponseHandler; import org.elasticsearch.xpack.inference.external.http.retry.ContentTooLargeException; @@ -15,9 +14,6 @@ import org.elasticsearch.xpack.inference.external.http.retry.RetryException; import org.elasticsearch.xpack.inference.external.request.Request; import org.elasticsearch.xpack.inference.external.response.huggingface.HuggingFaceErrorResponseEntity; -import org.elasticsearch.xpack.inference.logging.ThrottlerManager; - -import static org.elasticsearch.xpack.inference.external.http.HttpUtils.checkForEmptyBody; public class HuggingFaceResponseHandler extends BaseResponseHandler { @@ -25,13 +21,6 @@ public HuggingFaceResponseHandler(String requestType, ResponseParser parseFuncti super(requestType, parseFunction, HuggingFaceErrorResponseEntity::fromResponse); } - @Override - public void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) - throws RetryException { - checkForFailureStatusCode(request, result); - checkForEmptyBody(throttlerManager, logger, request, result); - } - /** * Validates the status code and throws a RetryException if it is not in the range [200, 300). * @@ -40,7 +29,8 @@ public void validateResponse(ThrottlerManager throttlerManager, Logger logger, R * @param result the http response and body * @throws RetryException thrown if status code is {@code >= 300 or < 200} */ - void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { + @Override + protected void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { if (result.isSuccessfulResponse()) { return; } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/ibmwatsonx/IbmWatsonxResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/ibmwatsonx/IbmWatsonxResponseHandler.java index cb686ddb654db..6d1d3fb2a4f91 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/ibmwatsonx/IbmWatsonxResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/ibmwatsonx/IbmWatsonxResponseHandler.java @@ -7,17 +7,14 @@ package org.elasticsearch.xpack.inference.external.ibmwatsonx; -import org.apache.logging.log4j.Logger; import org.elasticsearch.xpack.inference.external.http.HttpResult; import org.elasticsearch.xpack.inference.external.http.retry.BaseResponseHandler; import org.elasticsearch.xpack.inference.external.http.retry.ResponseParser; import org.elasticsearch.xpack.inference.external.http.retry.RetryException; import org.elasticsearch.xpack.inference.external.request.Request; import org.elasticsearch.xpack.inference.external.response.ibmwatsonx.IbmWatsonxErrorResponseEntity; -import org.elasticsearch.xpack.inference.logging.ThrottlerManager; import static org.elasticsearch.core.Strings.format; -import static org.elasticsearch.xpack.inference.external.http.HttpUtils.checkForEmptyBody; public class IbmWatsonxResponseHandler extends BaseResponseHandler { @@ -25,13 +22,6 @@ public IbmWatsonxResponseHandler(String requestType, ResponseParser parseFunctio super(requestType, parseFunction, IbmWatsonxErrorResponseEntity::fromResponse); } - @Override - public void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) - throws RetryException { - checkForFailureStatusCode(request, result); - checkForEmptyBody(throttlerManager, logger, request, result); - } - /** * Validates the status code and throws a RetryException if it is not in the range [200, 300). * @@ -41,7 +31,8 @@ public void validateResponse(ThrottlerManager throttlerManager, Logger logger, R * @param result the http response and body * @throws RetryException thrown if status code is {@code >= 300 or < 200} */ - void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { + @Override + protected void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { if (result.isSuccessfulResponse()) { return; } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/openai/OpenAiResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/openai/OpenAiResponseHandler.java index 6404236d51184..cf867fb1a0ab0 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/openai/OpenAiResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/openai/OpenAiResponseHandler.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.inference.external.openai; -import org.apache.logging.log4j.Logger; import org.elasticsearch.common.Strings; import org.elasticsearch.inference.InferenceServiceResults; import org.elasticsearch.xpack.core.inference.results.StreamingChatCompletionResults; @@ -17,14 +16,12 @@ import org.elasticsearch.xpack.inference.external.http.retry.ResponseParser; import org.elasticsearch.xpack.inference.external.http.retry.RetryException; import org.elasticsearch.xpack.inference.external.request.Request; -import org.elasticsearch.xpack.inference.external.response.openai.OpenAiErrorResponseEntity; +import org.elasticsearch.xpack.inference.external.response.ErrorMessageResponseEntity; import org.elasticsearch.xpack.inference.external.response.streaming.ServerSentEventParser; import org.elasticsearch.xpack.inference.external.response.streaming.ServerSentEventProcessor; -import org.elasticsearch.xpack.inference.logging.ThrottlerManager; import java.util.concurrent.Flow; -import static org.elasticsearch.xpack.inference.external.http.HttpUtils.checkForEmptyBody; import static org.elasticsearch.xpack.inference.external.http.retry.ResponseHandlerUtils.getFirstHeaderOrUnknown; public class OpenAiResponseHandler extends BaseResponseHandler { @@ -47,17 +44,10 @@ public class OpenAiResponseHandler extends BaseResponseHandler { private final boolean canHandleStreamingResponses; public OpenAiResponseHandler(String requestType, ResponseParser parseFunction, boolean canHandleStreamingResponses) { - super(requestType, parseFunction, OpenAiErrorResponseEntity::fromResponse); + super(requestType, parseFunction, ErrorMessageResponseEntity::fromResponse); this.canHandleStreamingResponses = canHandleStreamingResponses; } - @Override - public void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) - throws RetryException { - checkForFailureStatusCode(request, result); - checkForEmptyBody(throttlerManager, logger, request, result); - } - /** * Validates the status code throws an RetryException if not in the range [200, 300). * @@ -66,7 +56,8 @@ public void validateResponse(ThrottlerManager throttlerManager, Logger logger, R * @param result The http response and body * @throws RetryException Throws if status code is {@code >= 300 or < 200 } */ - void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { + @Override + protected void checkForFailureStatusCode(Request request, HttpResult result) throws RetryException { if (result.isSuccessfulResponse()) { return; } @@ -104,7 +95,7 @@ private static boolean isContentTooLarge(HttpResult result) { } if (statusCode == 400) { - var errorEntity = OpenAiErrorResponseEntity.fromResponse(result); + var errorEntity = ErrorMessageResponseEntity.fromResponse(result); return errorEntity != null && errorEntity.getErrorMessage().contains(CONTENT_TOO_LARGE_MESSAGE); } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/AzureMistralOpenAiExternalResponseHandler.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/AzureMistralOpenAiExternalResponseHandler.java index 7764a5b6586e2..de627a758c7c3 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/AzureMistralOpenAiExternalResponseHandler.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/AzureMistralOpenAiExternalResponseHandler.java @@ -14,7 +14,7 @@ import org.elasticsearch.xpack.inference.external.http.HttpResult; import org.elasticsearch.xpack.inference.external.http.retry.BaseResponseHandler; import org.elasticsearch.xpack.inference.external.http.retry.ContentTooLargeException; -import org.elasticsearch.xpack.inference.external.http.retry.ErrorMessage; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; import org.elasticsearch.xpack.inference.external.http.retry.ResponseParser; import org.elasticsearch.xpack.inference.external.http.retry.RetryException; import org.elasticsearch.xpack.inference.external.openai.OpenAiStreamingProcessor; @@ -54,7 +54,7 @@ public class AzureMistralOpenAiExternalResponseHandler extends BaseResponseHandl public AzureMistralOpenAiExternalResponseHandler( String requestType, ResponseParser parseFunction, - Function errorParseFunction, + Function errorParseFunction, boolean canHandleStreamingResponses ) { super(requestType, parseFunction, errorParseFunction); diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/ErrorMessageResponseEntity.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/ErrorMessageResponseEntity.java index dbf2b37955b22..489844f4d14de 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/ErrorMessageResponseEntity.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/ErrorMessageResponseEntity.java @@ -12,9 +12,10 @@ import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.inference.external.http.HttpResult; -import org.elasticsearch.xpack.inference.external.http.retry.ErrorMessage; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; import java.util.Map; +import java.util.Objects; /** * A pattern is emerging in how external providers provide error responses. @@ -31,27 +32,14 @@ * This currently covers error handling for Azure AI Studio, however this pattern * can be used to simplify and refactor handling for Azure OpenAI and OpenAI responses. */ -public class ErrorMessageResponseEntity implements ErrorMessage { - protected String errorMessage; +public class ErrorMessageResponseEntity extends ErrorResponse { public ErrorMessageResponseEntity(String errorMessage) { - this.errorMessage = errorMessage; + super(errorMessage); } - @Override - public String getErrorMessage() { - return errorMessage; - } - - /** - * Standard error response parser. This can be overridden for those subclasses that - * might have a different format - * - * @param response the HttpResult - * @return the error response - */ @SuppressWarnings("unchecked") - public static ErrorMessage fromResponse(HttpResult response) { + public static ErrorResponse fromResponse(HttpResult response, String defaultMessage) { try ( XContentParser jsonParser = XContentFactory.xContent(XContentType.JSON) .createParser(XContentParserConfiguration.EMPTY, response.body()) @@ -61,14 +49,23 @@ public static ErrorMessage fromResponse(HttpResult response) { var error = (Map) responseMap.get("error"); if (error != null) { var message = (String) error.get("message"); - if (message != null) { - return new ErrorMessageResponseEntity(message); - } + return new ErrorMessageResponseEntity(Objects.requireNonNullElse(message, defaultMessage)); } } catch (Exception e) { // swallow the error } - return null; + return ErrorResponse.UNDEFINED_ERROR; + } + + /** + * Standard error response parser. This can be overridden for those subclasses that + * might have a different format + * + * @param response the HttpResult + * @return the error response + */ + public static ErrorResponse fromResponse(HttpResult response) { + return fromResponse(response, ""); } } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/alibabacloudsearch/AlibabaCloudSearchErrorResponseEntity.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/alibabacloudsearch/AlibabaCloudSearchErrorResponseEntity.java index 77a0c6ecc7cc8..fe69176db2a1d 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/alibabacloudsearch/AlibabaCloudSearchErrorResponseEntity.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/alibabacloudsearch/AlibabaCloudSearchErrorResponseEntity.java @@ -14,20 +14,13 @@ import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.inference.external.http.HttpResult; -import org.elasticsearch.xpack.inference.external.http.retry.ErrorMessage; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; -public class AlibabaCloudSearchErrorResponseEntity implements ErrorMessage { +public class AlibabaCloudSearchErrorResponseEntity extends ErrorResponse { private static final Logger logger = LogManager.getLogger(AlibabaCloudSearchErrorResponseEntity.class); - private final String errorMessage; - private AlibabaCloudSearchErrorResponseEntity(String errorMessage) { - this.errorMessage = errorMessage; - } - - @Override - public String getErrorMessage() { - return errorMessage; + super(errorMessage); } /** @@ -44,9 +37,9 @@ public String getErrorMessage() { * * @param response The error response * @return An error entity if the response is JSON with the above structure - * or null if the response does not contain the message field + * or {@link ErrorResponse#UNDEFINED_ERROR} if the message field wasn't found */ - public static AlibabaCloudSearchErrorResponseEntity fromResponse(HttpResult response) { + public static ErrorResponse fromResponse(HttpResult response) { try ( XContentParser jsonParser = XContentFactory.xContent(XContentType.JSON) .createParser(XContentParserConfiguration.EMPTY, response.body()) @@ -64,6 +57,6 @@ public static AlibabaCloudSearchErrorResponseEntity fromResponse(HttpResult resp // swallow the error } - return null; + return ErrorResponse.UNDEFINED_ERROR; } } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/cohere/CohereErrorResponseEntity.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/cohere/CohereErrorResponseEntity.java index 7d1731105e2f5..28f361b12fa2b 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/cohere/CohereErrorResponseEntity.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/cohere/CohereErrorResponseEntity.java @@ -12,19 +12,12 @@ import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.inference.external.http.HttpResult; -import org.elasticsearch.xpack.inference.external.http.retry.ErrorMessage; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; -public class CohereErrorResponseEntity implements ErrorMessage { - - private final String errorMessage; +public class CohereErrorResponseEntity extends ErrorResponse { private CohereErrorResponseEntity(String errorMessage) { - this.errorMessage = errorMessage; - } - - @Override - public String getErrorMessage() { - return errorMessage; + super(errorMessage); } /** @@ -38,9 +31,9 @@ public String getErrorMessage() { * * @param response The error response * @return An error entity if the response is JSON with the above structure - * or null if the response does not contain the message field + * or {@link ErrorResponse#UNDEFINED_ERROR} if the message field wasn't found */ - public static CohereErrorResponseEntity fromResponse(HttpResult response) { + public static ErrorResponse fromResponse(HttpResult response) { try ( XContentParser jsonParser = XContentFactory.xContent(XContentType.JSON) .createParser(XContentParserConfiguration.EMPTY, response.body()) @@ -54,6 +47,6 @@ public static CohereErrorResponseEntity fromResponse(HttpResult response) { // swallow the error } - return null; + return ErrorResponse.UNDEFINED_ERROR; } } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/elastic/ElasticInferenceServiceErrorResponseEntity.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/elastic/ElasticInferenceServiceErrorResponseEntity.java index c860821c81bbf..696be7b2acdd2 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/elastic/ElasticInferenceServiceErrorResponseEntity.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/elastic/ElasticInferenceServiceErrorResponseEntity.java @@ -9,27 +9,19 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.core.Nullable; import org.elasticsearch.xcontent.XContentFactory; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.inference.external.http.HttpResult; -import org.elasticsearch.xpack.inference.external.http.retry.ErrorMessage; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; -public class ElasticInferenceServiceErrorResponseEntity implements ErrorMessage { - - private final String errorMessage; +public class ElasticInferenceServiceErrorResponseEntity extends ErrorResponse { private static final Logger logger = LogManager.getLogger(ElasticInferenceServiceErrorResponseEntity.class); private ElasticInferenceServiceErrorResponseEntity(String errorMessage) { - this.errorMessage = errorMessage; - } - - @Override - public String getErrorMessage() { - return errorMessage; + super(errorMessage); } /** @@ -43,9 +35,9 @@ public String getErrorMessage() { * * @param response The error response * @return An error entity if the response is JSON with the above structure - * or null if the response does not contain the error field + * or {@link ErrorResponse#UNDEFINED_ERROR} if the error field wasn't found */ - public static @Nullable ElasticInferenceServiceErrorResponseEntity fromResponse(HttpResult response) { + public static ErrorResponse fromResponse(HttpResult response) { try ( XContentParser jsonParser = XContentFactory.xContent(XContentType.JSON) .createParser(XContentParserConfiguration.EMPTY, response.body()) @@ -59,6 +51,6 @@ public String getErrorMessage() { logger.debug("Failed to parse error response", e); } - return null; + return ErrorResponse.UNDEFINED_ERROR; } } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/googleaistudio/GoogleAiStudioErrorResponseEntity.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/googleaistudio/GoogleAiStudioErrorResponseEntity.java index f57f672e10b16..fa8751a2fb99b 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/googleaistudio/GoogleAiStudioErrorResponseEntity.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/googleaistudio/GoogleAiStudioErrorResponseEntity.java @@ -12,21 +12,15 @@ import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.inference.external.http.HttpResult; -import org.elasticsearch.xpack.inference.external.http.retry.ErrorMessage; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; import java.util.Map; +import java.util.Objects; -public class GoogleAiStudioErrorResponseEntity implements ErrorMessage { - - private final String errorMessage; +public class GoogleAiStudioErrorResponseEntity extends ErrorResponse { private GoogleAiStudioErrorResponseEntity(String errorMessage) { - this.errorMessage = errorMessage; - } - - @Override - public String getErrorMessage() { - return errorMessage; + super(errorMessage); } /** @@ -52,11 +46,11 @@ public String getErrorMessage() { * * @param response The error response * @return An error entity if the response is JSON with the above structure - * or null if the response does not contain the `error.message` field + * or {@link ErrorResponse#UNDEFINED_ERROR} if the error field wasn't found */ @SuppressWarnings("unchecked") - public static GoogleAiStudioErrorResponseEntity fromResponse(HttpResult response) { + public static ErrorResponse fromResponse(HttpResult response) { try ( XContentParser jsonParser = XContentFactory.xContent(XContentType.JSON) .createParser(XContentParserConfiguration.EMPTY, response.body()) @@ -65,14 +59,12 @@ public static GoogleAiStudioErrorResponseEntity fromResponse(HttpResult response var error = (Map) responseMap.get("error"); if (error != null) { var message = (String) error.get("message"); - if (message != null) { - return new GoogleAiStudioErrorResponseEntity(message); - } + return new GoogleAiStudioErrorResponseEntity(Objects.requireNonNullElse(message, "")); } } catch (Exception e) { // swallow the error } - return null; + return ErrorResponse.UNDEFINED_ERROR; } } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/googlevertexai/GoogleVertexAiErrorResponseEntity.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/googlevertexai/GoogleVertexAiErrorResponseEntity.java index bf14d751db868..99e6f45f7e988 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/googlevertexai/GoogleVertexAiErrorResponseEntity.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/googlevertexai/GoogleVertexAiErrorResponseEntity.java @@ -12,21 +12,15 @@ import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.inference.external.http.HttpResult; -import org.elasticsearch.xpack.inference.external.http.retry.ErrorMessage; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; import java.util.Map; +import java.util.Objects; -public class GoogleVertexAiErrorResponseEntity implements ErrorMessage { - - private final String errorMessage; +public class GoogleVertexAiErrorResponseEntity extends ErrorResponse { private GoogleVertexAiErrorResponseEntity(String errorMessage) { - this.errorMessage = errorMessage; - } - - @Override - public String getErrorMessage() { - return errorMessage; + super(errorMessage); } /** @@ -54,10 +48,10 @@ public String getErrorMessage() { * * @param response The error response * @return An error entity if the response is JSON with the above structure - * or null if the response does not contain the `error.message` field + * or {@link ErrorResponse#UNDEFINED_ERROR} if the error field wasn't found */ @SuppressWarnings("unchecked") - public static GoogleVertexAiErrorResponseEntity fromResponse(HttpResult response) { + public static ErrorResponse fromResponse(HttpResult response) { try ( XContentParser jsonParser = XContentFactory.xContent(XContentType.JSON) .createParser(XContentParserConfiguration.EMPTY, response.body()) @@ -66,14 +60,12 @@ public static GoogleVertexAiErrorResponseEntity fromResponse(HttpResult response var error = (Map) responseMap.get("error"); if (error != null) { var message = (String) error.get("message"); - if (message != null) { - return new GoogleVertexAiErrorResponseEntity(message); - } + return new GoogleVertexAiErrorResponseEntity(Objects.requireNonNullElse(message, "")); } } catch (Exception e) { // swallow the error } - return null; + return ErrorResponse.UNDEFINED_ERROR; } } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceErrorResponseEntity.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceErrorResponseEntity.java index faeb7c6ac4fa9..af9057c334b12 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceErrorResponseEntity.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceErrorResponseEntity.java @@ -12,9 +12,14 @@ import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.inference.external.http.HttpResult; -import org.elasticsearch.xpack.inference.external.http.retry.ErrorMessage; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; + +public class HuggingFaceErrorResponseEntity extends ErrorResponse { + + public HuggingFaceErrorResponseEntity(String message) { + super(message); + } -public record HuggingFaceErrorResponseEntity(String message) implements ErrorMessage { /** * An example error response for invalid auth would look like * @@ -26,9 +31,9 @@ public record HuggingFaceErrorResponseEntity(String message) implements ErrorMes * * @param response The error response * @return An error entity if the response is JSON with the above structure - * or null if the response does not contain the error field + * or {@link ErrorResponse#UNDEFINED_ERROR} if the error field wasn't found */ - public static HuggingFaceErrorResponseEntity fromResponse(HttpResult response) { + public static ErrorResponse fromResponse(HttpResult response) { try ( XContentParser jsonParser = XContentFactory.xContent(XContentType.JSON) .createParser(XContentParserConfiguration.EMPTY, response.body()) @@ -42,11 +47,6 @@ public static HuggingFaceErrorResponseEntity fromResponse(HttpResult response) { // swallow the error } - return null; - } - - @Override - public String getErrorMessage() { - return message; + return ErrorResponse.UNDEFINED_ERROR; } } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/ibmwatsonx/IbmWatsonxErrorResponseEntity.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/ibmwatsonx/IbmWatsonxErrorResponseEntity.java index 582e3d009be50..9c805b22ff018 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/ibmwatsonx/IbmWatsonxErrorResponseEntity.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/ibmwatsonx/IbmWatsonxErrorResponseEntity.java @@ -12,25 +12,19 @@ import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.inference.external.http.HttpResult; -import org.elasticsearch.xpack.inference.external.http.retry.ErrorMessage; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; import java.util.Map; +import java.util.Objects; -public class IbmWatsonxErrorResponseEntity implements ErrorMessage { - - private final String errorMessage; +public class IbmWatsonxErrorResponseEntity extends ErrorResponse { private IbmWatsonxErrorResponseEntity(String errorMessage) { - this.errorMessage = errorMessage; - } - - @Override - public String getErrorMessage() { - return errorMessage; + super(errorMessage); } @SuppressWarnings("unchecked") - public static IbmWatsonxErrorResponseEntity fromResponse(HttpResult response) { + public static ErrorResponse fromResponse(HttpResult response) { try ( XContentParser jsonParser = XContentFactory.xContent(XContentType.JSON) .createParser(XContentParserConfiguration.EMPTY, response.body()) @@ -39,14 +33,12 @@ public static IbmWatsonxErrorResponseEntity fromResponse(HttpResult response) { var error = (Map) responseMap.get("error"); if (error != null) { var message = (String) error.get("message"); - if (message != null) { - return new IbmWatsonxErrorResponseEntity(message); - } + return new IbmWatsonxErrorResponseEntity(Objects.requireNonNullElse(message, "")); } } catch (Exception e) { // swallow the error } - return null; + return ErrorResponse.UNDEFINED_ERROR; } } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/openai/OpenAiErrorResponseEntity.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/openai/OpenAiErrorResponseEntity.java deleted file mode 100644 index a364be29ada33..0000000000000 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/response/openai/OpenAiErrorResponseEntity.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.inference.external.response.openai; - -import org.elasticsearch.xcontent.XContentFactory; -import org.elasticsearch.xcontent.XContentParser; -import org.elasticsearch.xcontent.XContentParserConfiguration; -import org.elasticsearch.xcontent.XContentType; -import org.elasticsearch.xpack.inference.external.http.HttpResult; -import org.elasticsearch.xpack.inference.external.http.retry.ErrorMessage; - -import java.util.Map; - -public class OpenAiErrorResponseEntity implements ErrorMessage { - - private final String errorMessage; - - private OpenAiErrorResponseEntity(String errorMessage) { - this.errorMessage = errorMessage; - } - - public String getErrorMessage() { - return errorMessage; - } - - /** - * An example error response for invalid auth would look like - * - * { - * "error": { - * "message": "You didn't provide an API key...", - * "type": "invalid_request_error", - * "param": null, - * "code": null - * } - * } - * - * - * - * @param response The error response - * @return An error entity if the response is JSON with the above structure - * or null if the response does not contain the error.message field - */ - @SuppressWarnings("unchecked") - public static OpenAiErrorResponseEntity fromResponse(HttpResult response) { - try ( - XContentParser jsonParser = XContentFactory.xContent(XContentType.JSON) - .createParser(XContentParserConfiguration.EMPTY, response.body()) - ) { - var responseMap = jsonParser.map(); - var error = (Map) responseMap.get("error"); - if (error != null) { - var message = (String) error.get("message"); - if (message != null) { - return new OpenAiErrorResponseEntity(message); - } - } - } catch (Exception e) { - // swallow the error - } - - return null; - } -} diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/retry/BaseResponseHandlerTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/retry/BaseResponseHandlerTests.java index b7095979b0fa5..444a187261fff 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/retry/BaseResponseHandlerTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/retry/BaseResponseHandlerTests.java @@ -7,11 +7,22 @@ package org.elasticsearch.xpack.inference.external.http.retry; +import org.apache.http.HttpResponse; +import org.apache.http.StatusLine; +import org.apache.logging.log4j.Logger; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.inference.external.http.HttpResult; +import org.elasticsearch.xpack.inference.external.request.Request; +import org.elasticsearch.xpack.inference.external.response.ErrorMessageResponseEntity; +import org.elasticsearch.xpack.inference.logging.ThrottlerManager; + +import java.nio.charset.StandardCharsets; import static org.elasticsearch.xpack.inference.external.http.retry.BaseResponseHandler.toRestStatus; import static org.hamcrest.core.Is.is; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class BaseResponseHandlerTests extends ESTestCase { public void testToRestStatus_ReturnsBadRequest_WhenStatusIs500() { @@ -29,4 +40,111 @@ public void testToRestStatus_ReturnsStatusCodeValue_WhenStatusIs200() { public void testToRestStatus_ReturnsBadRequest_WhenStatusIsUnknown() { assertThat(toRestStatus(1000), is(RestStatus.BAD_REQUEST)); } + + public void testValidateResponse_DoesNotThrowAnExceptionWhenStatus200_AndNoErrorObject() { + var handler = getBaseResponseHandler(); + + String responseJson = """ + { + "field": "hello" + } + """; + + var response = mock200Response(); + + var request = mock(Request.class); + when(request.getInferenceEntityId()).thenReturn("abc"); + + handler.validateResponse( + mock(ThrottlerManager.class), + mock(Logger.class), + request, + new HttpResult(response, responseJson.getBytes(StandardCharsets.UTF_8)) + ); + } + + public void testValidateResponse_ThrowsErrorWhenMalformedErrorObjectExists() { + var handler = getBaseResponseHandler(); + + String responseJson = """ + { + "error": { + "type": "not_found_error" + } + } + """; + + var response = mock200Response(); + + var request = mock(Request.class); + when(request.getInferenceEntityId()).thenReturn("abc"); + + var exception = expectThrows( + RetryException.class, + () -> handler.validateResponse( + mock(ThrottlerManager.class), + mock(Logger.class), + request, + new HttpResult(response, responseJson.getBytes(StandardCharsets.UTF_8)) + ) + ); + + assertFalse(exception.shouldRetry()); + assertThat( + exception.getCause().getMessage(), + is("Received an error response for request from inference entity id [abc] status [200]") + ); + } + + public void testValidateResponse_ThrowsErrorWhenWellFormedErrorObjectExists() { + var handler = getBaseResponseHandler(); + + String responseJson = """ + { + "error": { + "type": "not_found_error", + "message": "a message" + } + } + """; + + var response = mock200Response(); + + var request = mock(Request.class); + when(request.getInferenceEntityId()).thenReturn("abc"); + + var exception = expectThrows( + RetryException.class, + () -> handler.validateResponse( + mock(ThrottlerManager.class), + mock(Logger.class), + request, + new HttpResult(response, responseJson.getBytes(StandardCharsets.UTF_8)) + ) + ); + + assertFalse(exception.shouldRetry()); + assertThat( + exception.getCause().getMessage(), + is("Received an error response for request from inference entity id [abc] status [200]. Error message: [a message]") + ); + } + + private static HttpResponse mock200Response() { + int statusCode = 200; + var statusLine = mock(StatusLine.class); + when(statusLine.getStatusCode()).thenReturn(statusCode); + + var response = mock(HttpResponse.class); + when(response.getStatusLine()).thenReturn(statusLine); + + return response; + } + + private static BaseResponseHandler getBaseResponseHandler() { + return new BaseResponseHandler("abc", (Request request, HttpResult result) -> null, ErrorMessageResponseEntity::fromResponse) { + @Override + protected void checkForFailureStatusCode(Request request, HttpResult result) {} + }; + } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/ErrorMessageResponseEntityTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/ErrorMessageResponseEntityTests.java index d57d1537f6c30..9ad9b9f3ca0a5 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/ErrorMessageResponseEntityTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/ErrorMessageResponseEntityTests.java @@ -11,10 +11,12 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.inference.external.http.HttpResult; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; import java.nio.charset.StandardCharsets; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; import static org.mockito.Mockito.mock; public class ErrorMessageResponseEntityTests extends ESTestCase { @@ -33,11 +35,29 @@ public void testErrorResponse_ExtractsError() { assertThat(error.getErrorMessage(), is("test_error_message")); } + public void testFromResponse_WithOtherFieldsPresent() { + String responseJson = """ + { + "error": { + "message": "You didn't provide an API key", + "type": "invalid_request_error", + "param": null, + "code": null + } + } + """; + + ErrorResponse errorMessage = ErrorMessageResponseEntity.fromResponse( + new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) + ); + assertEquals("You didn't provide an API key", errorMessage.getErrorMessage()); + } + public void testFromResponse_noMessage() { String responseJson = """ { "error": { - "type": "not_found_error", + "type": "not_found_error" } } """; @@ -45,21 +65,22 @@ public void testFromResponse_noMessage() { var errorMessage = ErrorMessageResponseEntity.fromResponse( new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ); - assertNull(errorMessage); + assertThat(errorMessage.getErrorMessage(), is("")); + assertTrue(errorMessage.errorStructureFound()); } - public void testErrorResponse_ReturnsNullIfNoError() { + public void testErrorResponse_ReturnsUndefinedObjectIfNoError() { var result = getMockResult(""" {"noerror":true}"""); var error = ErrorMessageResponseEntity.fromResponse(result); - assertNull(error); + assertThat(error, sameInstance(ErrorResponse.UNDEFINED_ERROR)); } - public void testErrorResponse_ReturnsNullIfNotJson() { + public void testErrorResponse_ReturnsUndefinedObjectIfNotJson() { var result = getMockResult("not a json string"); var error = ErrorMessageResponseEntity.fromResponse(result); - assertNull(error); + assertThat(error, sameInstance(ErrorResponse.UNDEFINED_ERROR)); } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/alibabacloudsearch/AlibabaCloudSearchErrorResponseEntityTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/alibabacloudsearch/AlibabaCloudSearchErrorResponseEntityTests.java index a03349c66b6d5..1dd570537f12e 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/alibabacloudsearch/AlibabaCloudSearchErrorResponseEntityTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/alibabacloudsearch/AlibabaCloudSearchErrorResponseEntityTests.java @@ -26,7 +26,7 @@ public void testFromResponse() { } """; - AlibabaCloudSearchErrorResponseEntity errorMessage = AlibabaCloudSearchErrorResponseEntity.fromResponse( + var errorMessage = AlibabaCloudSearchErrorResponseEntity.fromResponse( new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ); assertNotNull(errorMessage); diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/cohere/CohereErrorResponseEntityTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/cohere/CohereErrorResponseEntityTests.java index a2b1c26b2b3d5..d770adb0b5a65 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/cohere/CohereErrorResponseEntityTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/cohere/CohereErrorResponseEntityTests.java @@ -10,7 +10,9 @@ import org.apache.http.HttpResponse; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.inference.external.http.HttpResult; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; import java.nio.charset.StandardCharsets; @@ -25,7 +27,7 @@ public void testFromResponse() { } """; - CohereErrorResponseEntity errorMessage = CohereErrorResponseEntity.fromResponse( + var errorMessage = CohereErrorResponseEntity.fromResponse( new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ); assertNotNull(errorMessage); @@ -42,9 +44,9 @@ public void testFromResponse_noMessage() { } """; - CohereErrorResponseEntity errorMessage = CohereErrorResponseEntity.fromResponse( + var errorMessage = CohereErrorResponseEntity.fromResponse( new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ); - assertNull(errorMessage); + assertThat(errorMessage, Matchers.sameInstance(ErrorResponse.UNDEFINED_ERROR)); } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/elastic/ElasticInferenceServiceErrorResponseEntityTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/elastic/ElasticInferenceServiceErrorResponseEntityTests.java index 4da0518084828..d99e1c1ce5afc 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/elastic/ElasticInferenceServiceErrorResponseEntityTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/elastic/ElasticInferenceServiceErrorResponseEntityTests.java @@ -10,6 +10,8 @@ import org.apache.http.HttpResponse; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.inference.external.http.HttpResult; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; +import org.hamcrest.Matchers; import java.nio.charset.StandardCharsets; @@ -25,7 +27,7 @@ public void testFromResponse() { } """; - ElasticInferenceServiceErrorResponseEntity errorResponseEntity = ElasticInferenceServiceErrorResponseEntity.fromResponse( + var errorResponseEntity = ElasticInferenceServiceErrorResponseEntity.fromResponse( new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ); @@ -40,11 +42,11 @@ public void testFromResponse_NoErrorMessagePresent() { } """; - ElasticInferenceServiceErrorResponseEntity errorResponseEntity = ElasticInferenceServiceErrorResponseEntity.fromResponse( + var errorResponseEntity = ElasticInferenceServiceErrorResponseEntity.fromResponse( new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ); - assertNull(errorResponseEntity); + assertThat(errorResponseEntity, Matchers.sameInstance(ErrorResponse.UNDEFINED_ERROR)); } public void testFromResponse_InvalidJson() { @@ -52,10 +54,10 @@ public void testFromResponse_InvalidJson() { { """; - ElasticInferenceServiceErrorResponseEntity errorResponseEntity = ElasticInferenceServiceErrorResponseEntity.fromResponse( + var errorResponseEntity = ElasticInferenceServiceErrorResponseEntity.fromResponse( new HttpResult(mock(HttpResponse.class), invalidResponseJson.getBytes(StandardCharsets.UTF_8)) ); - assertNull(errorResponseEntity); + assertThat(errorResponseEntity, Matchers.sameInstance(ErrorResponse.UNDEFINED_ERROR)); } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/googleaistudio/GoogleAiStudioErrorResponseEntityTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/googleaistudio/GoogleAiStudioErrorResponseEntityTests.java index 61448f2e35bdf..e2ccbb4167ef6 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/googleaistudio/GoogleAiStudioErrorResponseEntityTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/googleaistudio/GoogleAiStudioErrorResponseEntityTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.xpack.inference.external.http.HttpResult; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; import static org.mockito.Mockito.mock; public class GoogleAiStudioErrorResponseEntityTests extends ESTestCase { @@ -48,7 +49,7 @@ public void testErrorResponse_ExtractsError() { assertThat(error.getErrorMessage(), is("error message")); } - public void testErrorResponse_ReturnsNullIfNoError() { + public void testErrorResponse_ReturnsUndefinedObjectIfNoError() { var result = getMockResult(""" { "foo": "bar" @@ -56,13 +57,13 @@ public void testErrorResponse_ReturnsNullIfNoError() { """); var error = GoogleAiStudioErrorResponseEntity.fromResponse(result); - assertNull(error); + assertThat(error, sameInstance(GoogleAiStudioErrorResponseEntity.UNDEFINED_ERROR)); } - public void testErrorResponse_ReturnsNullIfNotJson() { + public void testErrorResponse_ReturnsUndefinedIfNotJson() { var result = getMockResult("error message"); var error = GoogleAiStudioErrorResponseEntity.fromResponse(result); - assertNull(error); + assertThat(error, sameInstance(GoogleAiStudioErrorResponseEntity.UNDEFINED_ERROR)); } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/googlevertexai/GoogleVertexAiErrorResponseEntityTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/googlevertexai/GoogleVertexAiErrorResponseEntityTests.java index e2c9ebed2c164..23bb2f9829e62 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/googlevertexai/GoogleVertexAiErrorResponseEntityTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/googlevertexai/GoogleVertexAiErrorResponseEntityTests.java @@ -11,8 +11,10 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.inference.external.http.HttpResult; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; import static org.mockito.Mockito.mock; public class GoogleVertexAiErrorResponseEntityTests extends ESTestCase { @@ -48,7 +50,7 @@ public void testErrorResponse_ExtractsError() { assertThat(error.getErrorMessage(), is("error message")); } - public void testErrorResponse_ReturnsNullIfNoError() { + public void testErrorResponse_ReturnsUndefinedObjectIfNoError() { var result = getMockResult(""" { "foo": "bar" @@ -56,14 +58,14 @@ public void testErrorResponse_ReturnsNullIfNoError() { """); var error = GoogleVertexAiErrorResponseEntity.fromResponse(result); - assertNull(error); + assertThat(error, sameInstance(ErrorResponse.UNDEFINED_ERROR)); } - public void testErrorResponse_ReturnsNullIfNotJson() { + public void testErrorResponse_ReturnsUndefinedObjectIfNotJson() { var result = getMockResult("error message"); var error = GoogleVertexAiErrorResponseEntity.fromResponse(result); - assertNull(error); + assertThat(error, sameInstance(ErrorResponse.UNDEFINED_ERROR)); } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceErrorResponseEntityTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceErrorResponseEntityTests.java index ed381de844731..06b15d57f5a89 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceErrorResponseEntityTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceErrorResponseEntityTests.java @@ -10,6 +10,8 @@ import org.apache.http.HttpResponse; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.inference.external.http.HttpResult; +import org.elasticsearch.xpack.inference.external.http.retry.ErrorResponse; +import org.hamcrest.Matchers; import java.nio.charset.StandardCharsets; @@ -23,7 +25,7 @@ public void testFromResponse() { } """; - HuggingFaceErrorResponseEntity errorMessage = HuggingFaceErrorResponseEntity.fromResponse( + var errorMessage = HuggingFaceErrorResponseEntity.fromResponse( new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ); assertNotNull(errorMessage); @@ -39,10 +41,10 @@ public void testFromResponse_noMessage() { } """; - HuggingFaceErrorResponseEntity errorMessage = HuggingFaceErrorResponseEntity.fromResponse( + var errorMessage = HuggingFaceErrorResponseEntity.fromResponse( new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ); - assertNull(errorMessage); + assertThat(errorMessage, Matchers.sameInstance(ErrorResponse.UNDEFINED_ERROR)); } public void testFromResponse_noError() { @@ -54,9 +56,9 @@ public void testFromResponse_noError() { } """; - HuggingFaceErrorResponseEntity errorMessage = HuggingFaceErrorResponseEntity.fromResponse( + var errorMessage = HuggingFaceErrorResponseEntity.fromResponse( new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ); - assertNull(errorMessage); + assertThat(errorMessage, Matchers.sameInstance(ErrorResponse.UNDEFINED_ERROR)); } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/openai/OpenAiErrorResponseEntityTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/openai/OpenAiErrorResponseEntityTests.java deleted file mode 100644 index 4dc6c4190f92c..0000000000000 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/openai/OpenAiErrorResponseEntityTests.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.inference.external.response.openai; - -import org.apache.http.HttpResponse; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.inference.external.http.HttpResult; - -import java.nio.charset.StandardCharsets; - -import static org.mockito.Mockito.mock; - -public class OpenAiErrorResponseEntityTests extends ESTestCase { - public void testFromResponse() { - String responseJson = """ - { - "error": { - "message": "You didn't provide an API key", - "type": "invalid_request_error", - "param": null, - "code": null - } - } - """; - - OpenAiErrorResponseEntity errorMessage = OpenAiErrorResponseEntity.fromResponse( - new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) - ); - assertEquals("You didn't provide an API key", errorMessage.getErrorMessage()); - } - - public void testFromResponse_noMessage() { - String responseJson = """ - { - "error": { - "type": "invalid_request_error" - } - } - """; - - OpenAiErrorResponseEntity errorMessage = OpenAiErrorResponseEntity.fromResponse( - new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) - ); - assertNull(errorMessage); - } - - public void testFromResponse_noError() { - String responseJson = """ - { - "something": { - "not": "relevant" - } - } - """; - - OpenAiErrorResponseEntity errorMessage = OpenAiErrorResponseEntity.fromResponse( - new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) - ); - assertNull(errorMessage); - } -} From b85e6491a91d11548b39540e87a3dcf536186d60 Mon Sep 17 00:00:00 2001 From: Patrick Doyle <810052+prdoyle@users.noreply.github.com> Date: Wed, 11 Dec 2024 16:29:54 -0500 Subject: [PATCH 3/8] Relax assertion - remove unnecessary times(1) (#118497) --- muted-tests.yml | 3 --- .../service/FileSettingsServiceTests.java | 11 +++++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index 613d3a3655ccf..50ad8c27675b4 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -314,9 +314,6 @@ tests: issue: https://github.com/elastic/elasticsearch/issues/118220 - class: org.elasticsearch.xpack.esql.action.EsqlActionBreakerIT issue: https://github.com/elastic/elasticsearch/issues/118238 -- class: org.elasticsearch.reservedstate.service.FileSettingsServiceTests - method: testInvalidJSON - issue: https://github.com/elastic/elasticsearch/issues/116521 # Examples: # diff --git a/server/src/test/java/org/elasticsearch/reservedstate/service/FileSettingsServiceTests.java b/server/src/test/java/org/elasticsearch/reservedstate/service/FileSettingsServiceTests.java index c19cf7c31bc68..b1cf40d7f22ec 100644 --- a/server/src/test/java/org/elasticsearch/reservedstate/service/FileSettingsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/reservedstate/service/FileSettingsServiceTests.java @@ -43,6 +43,7 @@ import org.elasticsearch.xcontent.XContentParser; import org.junit.After; import org.junit.Before; +import org.mockito.Mockito; import org.mockito.stubbing.Answer; import java.io.IOException; @@ -315,7 +316,13 @@ public void testInvalidJSON() throws Exception { writeTestFile(fileSettingsService.watchedFile(), "test_invalid_JSON"); awaitOrBust(fileChangeBarrier); - verify(fileSettingsService, times(1)).onProcessFileChangesException( + // These checks use atLeast(1) because the initial JSON is also invalid, + // and so we sometimes get two calls to these error-reporting methods + // depending on timing. Rather than trace down the root cause and fix + // it, we tolerate this for now because, hey, invalid JSON is invalid JSON + // and this is still testing what we want to test. + + verify(fileSettingsService, Mockito.atLeast(1)).onProcessFileChangesException( argThat(e -> unwrapException(e) instanceof XContentParseException) ); @@ -324,7 +331,7 @@ public void testInvalidJSON() throws Exception { // of the watcher thread itself, which occurs asynchronously when clusterChanged is first called. assertEquals(YELLOW, healthIndicatorService.calculate(false, null).status()); - verify(healthIndicatorService).failureOccurred(contains(XContentParseException.class.getName())); + verify(healthIndicatorService, Mockito.atLeast(1)).failureOccurred(contains(XContentParseException.class.getName())); } /** From c6b42bb5026d1641703a3eb7f34fae331f663785 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Thu, 12 Dec 2024 10:46:39 +1100 Subject: [PATCH 4/8] Mute org.elasticsearch.packaging.test.DockerTests test011SecurityEnabledStatus #118517 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 50ad8c27675b4..573bd035b872b 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -314,6 +314,9 @@ tests: issue: https://github.com/elastic/elasticsearch/issues/118220 - class: org.elasticsearch.xpack.esql.action.EsqlActionBreakerIT issue: https://github.com/elastic/elasticsearch/issues/118238 +- class: org.elasticsearch.packaging.test.DockerTests + method: test011SecurityEnabledStatus + issue: https://github.com/elastic/elasticsearch/issues/118517 # Examples: # From a5607829958f15d92eddbfab59c2309ca0b0f5ea Mon Sep 17 00:00:00 2001 From: John Verwolf Date: Wed, 11 Dec 2024 17:43:17 -0800 Subject: [PATCH 5/8] Remove any references to V_7 (#118103) This PR removes any references to org.elasticsearch.core.RestApiVersion#V_7. --- docs/changelog/118103.yaml | 11 +++++++++++ .../org/elasticsearch/core/RestApiVersion.java | 10 +--------- .../action/ingest/SimulatePipelineRequest.java | 2 +- .../common/xcontent/ChunkedToXContent.java | 1 - .../org/elasticsearch/ingest/IngestDocument.java | 3 +++ .../action/bulk/BulkRequestParserTests.java | 2 +- .../SimulatePipelineRequestParsingTests.java | 2 +- .../java/org/elasticsearch/license/License.java | 3 +-- .../xpack/core/ml/MachineLearningField.java | 3 --- .../xpack/core/ml/action/CloseJobAction.java | 13 +------------ .../core/ml/action/GetDatafeedsStatsAction.java | 2 ++ .../core/ml/action/GetOverallBucketsAction.java | 13 +------------ .../xpack/core/ml/action/StopDatafeedAction.java | 13 +------------ .../xpack/ml/rest/cat/RestCatDatafeedsAction.java | 14 +------------- .../xpack/ml/rest/cat/RestCatJobsAction.java | 14 +------------- .../rest/datafeeds/RestGetDatafeedStatsAction.java | 14 +------------- .../ml/rest/datafeeds/RestGetDatafeedsAction.java | 14 +------------- .../ml/rest/datafeeds/RestStopDatafeedAction.java | 14 +------------- .../xpack/ml/rest/job/RestCloseJobAction.java | 14 +------------- .../xpack/ml/rest/job/RestGetJobStatsAction.java | 14 +------------- .../xpack/ml/rest/job/RestGetJobsAction.java | 14 +------------- .../rest/results/RestGetOverallBucketsAction.java | 14 +------------- 22 files changed, 33 insertions(+), 171 deletions(-) create mode 100644 docs/changelog/118103.yaml diff --git a/docs/changelog/118103.yaml b/docs/changelog/118103.yaml new file mode 100644 index 0000000000000..c0bb8f56d6931 --- /dev/null +++ b/docs/changelog/118103.yaml @@ -0,0 +1,11 @@ +pr: 118103 +summary: "Remove any references to org.elasticsearch.core.RestApiVersion#V_7" +area: Infra/Core +type: breaking +issues: [] +breaking: + title: "Remove any references to org.elasticsearch.core.RestApiVersion#V_7" + area: REST API + details: "This PR removes all references to V_7 in the Rest API. V7 features marked for deprecation have been removed." + impact: "This change is breaking for any external plugins/clients that rely on the V_7 enum or deprecated version 7 functionality" + notable: false diff --git a/libs/core/src/main/java/org/elasticsearch/core/RestApiVersion.java b/libs/core/src/main/java/org/elasticsearch/core/RestApiVersion.java index 672090d195c5a..085887fdcdb0c 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/RestApiVersion.java +++ b/libs/core/src/main/java/org/elasticsearch/core/RestApiVersion.java @@ -20,10 +20,7 @@ public enum RestApiVersion { V_9(9), - V_8(8), - - @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) // remove all references to V_7 then delete this annotation - V_7(7); + V_8(8); public final byte major; @@ -54,7 +51,6 @@ public static Predicate equalTo(RestApiVersion restApiVersion) { return switch (restApiVersion) { case V_9 -> r -> r.major == V_9.major; case V_8 -> r -> r.major == V_8.major; - case V_7 -> r -> r.major == V_7.major; }; } @@ -62,15 +58,11 @@ public static Predicate onOrAfter(RestApiVersion restApiVersion) return switch (restApiVersion) { case V_9 -> r -> r.major >= V_9.major; case V_8 -> r -> r.major >= V_8.major; - case V_7 -> r -> r.major >= V_7.major; }; } public static RestApiVersion forMajor(int major) { switch (major) { - case 7 -> { - return V_7; - } case 8 -> { return V_8; } diff --git a/server/src/main/java/org/elasticsearch/action/ingest/SimulatePipelineRequest.java b/server/src/main/java/org/elasticsearch/action/ingest/SimulatePipelineRequest.java index d6a2d81fdb7d3..02027b1f633d2 100644 --- a/server/src/main/java/org/elasticsearch/action/ingest/SimulatePipelineRequest.java +++ b/server/src/main/java/org/elasticsearch/action/ingest/SimulatePipelineRequest.java @@ -177,7 +177,7 @@ private static List parseDocs(Map config, RestAp String index = ConfigurationUtils.readStringOrIntProperty(null, null, dataMap, Metadata.INDEX.getFieldName(), "_index"); String id = ConfigurationUtils.readStringOrIntProperty(null, null, dataMap, Metadata.ID.getFieldName(), "_id"); String routing = ConfigurationUtils.readOptionalStringOrIntProperty(null, null, dataMap, Metadata.ROUTING.getFieldName()); - if (restApiVersion != RestApiVersion.V_8 && dataMap.containsKey(Metadata.TYPE.getFieldName())) { + if (dataMap.containsKey(Metadata.TYPE.getFieldName())) { deprecationLogger.compatibleCritical( "simulate_pipeline_with_types", "[types removal] specifying _type in pipeline simulation requests is deprecated" diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/ChunkedToXContent.java b/server/src/main/java/org/elasticsearch/common/xcontent/ChunkedToXContent.java index db0b5b4357c7d..1970983654b88 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/ChunkedToXContent.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/ChunkedToXContent.java @@ -49,7 +49,6 @@ static ChunkedToXContentBuilder builder(ToXContent.Params params) { */ default Iterator toXContentChunked(RestApiVersion restApiVersion, ToXContent.Params params) { return switch (restApiVersion) { - case V_7 -> throw new AssertionError("v7 API not supported"); case V_8 -> toXContentChunkedV8(params); case V_9 -> toXContentChunked(params); }; diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index 280c7684a8553..0614e9e92edf2 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -13,6 +13,7 @@ import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.set.Sets; +import org.elasticsearch.core.UpdateForV10; import org.elasticsearch.index.VersionType; import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.index.mapper.IndexFieldMapper; @@ -957,6 +958,8 @@ void resetTerminate() { terminate = false; } + // Unconditionally deprecate the _type field once V7 BWC support is removed + @UpdateForV10(owner = UpdateForV10.Owner.DATA_MANAGEMENT) public enum Metadata { INDEX(IndexFieldMapper.NAME), TYPE("_type"), diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java index 5785d076693e7..9d944d43f4c36 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java @@ -30,7 +30,7 @@ public class BulkRequestParserTests extends ESTestCase { @UpdateForV10(owner = UpdateForV10.Owner.DATA_MANAGEMENT) // Replace with just RestApiVersion.values() when V8 no longer exists public static final List REST_API_VERSIONS_POST_V8 = Stream.of(RestApiVersion.values()) - .filter(v -> v.compareTo(RestApiVersion.V_8) > 0) + .filter(v -> v.matches(RestApiVersion.onOrAfter(RestApiVersion.V_9))) .toList(); public void testParserCannotBeReusedAfterFailure() { diff --git a/server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java b/server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java index b8be05fbfb72d..391c258b6f098 100644 --- a/server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java +++ b/server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java @@ -342,7 +342,7 @@ public void testIngestPipelineWithDocumentsWithType() throws Exception { requestContent, false, ingestService, - RestApiVersion.V_7 + RestApiVersion.V_8 ); assertThat(actualRequest.verbose(), equalTo(false)); assertThat(actualRequest.documents().size(), equalTo(numDocs)); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/License.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/License.java index f280dcf9b3edf..19790e61b6102 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/License.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/License.java @@ -15,7 +15,6 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.features.NodeFeature; import org.elasticsearch.xcontent.ToXContentObject; @@ -142,7 +141,7 @@ static boolean isEnterprise(String typeName) { */ public static final String LICENSE_VERSION_MODE = "license_version"; /** - * Set for {@link RestApiVersion#V_7} requests only + * Set for RestApiVersion#V_7 requests only * XContent param name to map the "enterprise" license type to "platinum" * for backwards compatibility with older clients */ diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MachineLearningField.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MachineLearningField.java index 6c49cadb8d189..3a37f94e6b2d4 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MachineLearningField.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MachineLearningField.java @@ -22,9 +22,6 @@ public final class MachineLearningField { - public static final String DEPRECATED_ALLOW_NO_JOBS_PARAM = "allow_no_jobs"; - public static final String DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM = "allow_no_datafeeds"; - public static final Setting AUTODETECT_PROCESS = Setting.boolSetting( "xpack.ml.autodetect_process", true, diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/CloseJobAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/CloseJobAction.java index bddae0417e467..5963cff9746b6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/CloseJobAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/CloseJobAction.java @@ -12,9 +12,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.tasks.Task; import org.elasticsearch.xcontent.ObjectParser; import org.elasticsearch.xcontent.ParseField; @@ -27,10 +25,6 @@ import java.io.IOException; import java.util.Objects; -import static org.elasticsearch.core.RestApiVersion.equalTo; -import static org.elasticsearch.core.RestApiVersion.onOrAfter; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_JOBS_PARAM; - public class CloseJobAction extends ActionType { public static final CloseJobAction INSTANCE = new CloseJobAction(); @@ -45,11 +39,7 @@ public static class Request extends BaseTasksRequest implements ToXCont public static final ParseField TIMEOUT = new ParseField("timeout"); public static final ParseField FORCE = new ParseField("force"); - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate forRestApiVersion - public static final ParseField ALLOW_NO_MATCH = new ParseField("allow_no_match").forRestApiVersion(onOrAfter(RestApiVersion.V_8)); - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 - public static final ParseField ALLOW_NO_MATCH_V7 = new ParseField("allow_no_match", DEPRECATED_ALLOW_NO_JOBS_PARAM) - .forRestApiVersion(equalTo(RestApiVersion.V_7)); + public static final ParseField ALLOW_NO_MATCH = new ParseField("allow_no_match"); public static final ObjectParser PARSER = new ObjectParser<>(NAME, Request::new); static { @@ -60,7 +50,6 @@ public static class Request extends BaseTasksRequest implements ToXCont ); PARSER.declareBoolean(Request::setForce, FORCE); PARSER.declareBoolean(Request::setAllowNoMatch, ALLOW_NO_MATCH); - PARSER.declareBoolean(Request::setAllowNoMatch, ALLOW_NO_MATCH_V7); } public static Request parseRequest(String jobId, XContentParser parser) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java index fafb9afa99f85..eabd6ef5939e3 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.persistent.PersistentTasksCustomMetadata; import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.Task; @@ -62,6 +63,7 @@ private GetDatafeedsStatsAction() { // serialized to older nodes where the transport action was a MasterNodeReadAction. // TODO: Make this a simple request in a future version where there is no possibility // of this request being serialized to another node. + @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) public static class Request extends MasterNodeReadRequest { public static final String ALLOW_NO_MATCH = "allow_no_match"; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetOverallBucketsAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetOverallBucketsAction.java index 47bc6df5f6536..f9e91194fe31b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetOverallBucketsAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetOverallBucketsAction.java @@ -13,9 +13,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.time.DateMathParser; -import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.index.mapper.DateFieldMapper; import org.elasticsearch.xcontent.ObjectParser; import org.elasticsearch.xcontent.ParseField; @@ -33,10 +31,6 @@ import java.util.Objects; import java.util.function.LongSupplier; -import static org.elasticsearch.core.RestApiVersion.equalTo; -import static org.elasticsearch.core.RestApiVersion.onOrAfter; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_JOBS_PARAM; - /** *

* This action returns summarized bucket results over multiple jobs. @@ -68,11 +62,7 @@ public static class Request extends ActionRequest implements ToXContentObject { public static final ParseField EXCLUDE_INTERIM = new ParseField("exclude_interim"); public static final ParseField START = new ParseField("start"); public static final ParseField END = new ParseField("end"); - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate forRestApiVersion - public static final ParseField ALLOW_NO_MATCH = new ParseField("allow_no_match").forRestApiVersion(onOrAfter(RestApiVersion.V_8)); - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 - public static final ParseField ALLOW_NO_MATCH_V7 = new ParseField("allow_no_match", DEPRECATED_ALLOW_NO_JOBS_PARAM) - .forRestApiVersion(equalTo(RestApiVersion.V_7)); + public static final ParseField ALLOW_NO_MATCH = new ParseField("allow_no_match"); private static final ObjectParser PARSER = new ObjectParser<>(NAME, Request::new); @@ -88,7 +78,6 @@ public static class Request extends ActionRequest implements ToXContentObject { ); PARSER.declareString((request, endTime) -> request.setEnd(parseDateOrThrow(endTime, END, System::currentTimeMillis)), END); PARSER.declareBoolean(Request::setAllowNoMatch, ALLOW_NO_MATCH); - PARSER.declareBoolean(Request::setAllowNoMatch, ALLOW_NO_MATCH_V7); } static long parseDateOrThrow(String date, ParseField paramName, LongSupplier now) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/StopDatafeedAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/StopDatafeedAction.java index bd4aac7ccad89..1278d1a57ec55 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/StopDatafeedAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/StopDatafeedAction.java @@ -13,9 +13,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.tasks.Task; import org.elasticsearch.xcontent.ObjectParser; import org.elasticsearch.xcontent.ParseField; @@ -29,10 +27,6 @@ import java.io.IOException; import java.util.Objects; -import static org.elasticsearch.core.RestApiVersion.equalTo; -import static org.elasticsearch.core.RestApiVersion.onOrAfter; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM; - public class StopDatafeedAction extends ActionType { public static final StopDatafeedAction INSTANCE = new StopDatafeedAction(); @@ -47,11 +41,7 @@ public static class Request extends BaseTasksRequest implements ToXCont public static final ParseField TIMEOUT = new ParseField("timeout"); public static final ParseField FORCE = new ParseField("force"); - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate forRestApiVersion - public static final ParseField ALLOW_NO_MATCH = new ParseField("allow_no_match").forRestApiVersion(onOrAfter(RestApiVersion.V_8)); - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 - public static final ParseField ALLOW_NO_MATCH_V7 = new ParseField("allow_no_match", DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM) - .forRestApiVersion(equalTo(RestApiVersion.V_7)); + public static final ParseField ALLOW_NO_MATCH = new ParseField("allow_no_match"); public static final ObjectParser PARSER = new ObjectParser<>(NAME, Request::new); static { @@ -62,7 +52,6 @@ public static class Request extends BaseTasksRequest implements ToXCont ); PARSER.declareBoolean(Request::setForce, FORCE); PARSER.declareBoolean(Request::setAllowNoMatch, ALLOW_NO_MATCH); - PARSER.declareBoolean(Request::setAllowNoMatch, ALLOW_NO_MATCH_V7); } public static Request parseRequest(String datafeedId, XContentParser parser) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/cat/RestCatDatafeedsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/cat/RestCatDatafeedsAction.java index 205bb4f68a62c..417e9bca0a497 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/cat/RestCatDatafeedsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/cat/RestCatDatafeedsAction.java @@ -10,9 +10,7 @@ import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.Strings; import org.elasticsearch.common.Table; -import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; import org.elasticsearch.rest.Scope; @@ -30,8 +28,6 @@ import java.util.List; import static org.elasticsearch.rest.RestRequest.Method.GET; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM; -import static org.elasticsearch.xpack.ml.rest.RestCompatibilityChecker.checkAndSetDeprecatedParam; @ServerlessScope(Scope.PUBLIC) public class RestCatDatafeedsAction extends AbstractCatAction { @@ -52,16 +48,8 @@ protected RestChannelConsumer doCatRequest(RestRequest restRequest, NodeClient c if (Strings.isNullOrEmpty(datafeedId)) { datafeedId = GetDatafeedsStatsAction.ALL; } - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 Request request = new Request(datafeedId); - checkAndSetDeprecatedParam( - DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM, - Request.ALLOW_NO_MATCH, - RestApiVersion.V_7, - restRequest, - (r, s) -> r.paramAsBoolean(s, request.allowNoMatch()), - request::setAllowNoMatch - ); + request.setAllowNoMatch(restRequest.paramAsBoolean(Request.ALLOW_NO_MATCH, request.allowNoMatch())); return channel -> client.execute(GetDatafeedsStatsAction.INSTANCE, request, new RestResponseListener<>(channel) { @Override public RestResponse buildResponse(Response getDatafeedsStatsRespons) throws Exception { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/cat/RestCatJobsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/cat/RestCatJobsAction.java index b27819bceee44..d8088c9510b6a 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/cat/RestCatJobsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/cat/RestCatJobsAction.java @@ -12,9 +12,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.Table; import org.elasticsearch.common.unit.ByteSizeValue; -import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; import org.elasticsearch.rest.Scope; @@ -35,8 +33,6 @@ import java.util.List; import static org.elasticsearch.rest.RestRequest.Method.GET; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_JOBS_PARAM; -import static org.elasticsearch.xpack.ml.rest.RestCompatibilityChecker.checkAndSetDeprecatedParam; @ServerlessScope(Scope.PUBLIC) public class RestCatJobsAction extends AbstractCatAction { @@ -57,16 +53,8 @@ protected RestChannelConsumer doCatRequest(RestRequest restRequest, NodeClient c if (Strings.isNullOrEmpty(jobId)) { jobId = Metadata.ALL; } - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 Request request = new Request(jobId); - checkAndSetDeprecatedParam( - DEPRECATED_ALLOW_NO_JOBS_PARAM, - Request.ALLOW_NO_MATCH, - RestApiVersion.V_7, - restRequest, - (r, s) -> r.paramAsBoolean(s, request.allowNoMatch()), - request::setAllowNoMatch - ); + request.setAllowNoMatch(restRequest.paramAsBoolean(Request.ALLOW_NO_MATCH, request.allowNoMatch())); return channel -> client.execute(GetJobsStatsAction.INSTANCE, request, new RestResponseListener<>(channel) { @Override public RestResponse buildResponse(Response getJobStatsResponse) throws Exception { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestGetDatafeedStatsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestGetDatafeedStatsAction.java index 8c85c055fca3b..a4879eca46d39 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestGetDatafeedStatsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestGetDatafeedStatsAction.java @@ -8,8 +8,6 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.common.Strings; -import org.elasticsearch.core.RestApiVersion; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.Scope; @@ -24,10 +22,8 @@ import java.util.List; import static org.elasticsearch.rest.RestRequest.Method.GET; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM; import static org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig.ID; import static org.elasticsearch.xpack.ml.MachineLearning.BASE_PATH; -import static org.elasticsearch.xpack.ml.rest.RestCompatibilityChecker.checkAndSetDeprecatedParam; @ServerlessScope(Scope.PUBLIC) public class RestGetDatafeedStatsAction extends BaseRestHandler { @@ -48,16 +44,8 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient if (Strings.isNullOrEmpty(datafeedId)) { datafeedId = GetDatafeedsStatsAction.ALL; } - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 Request request = new Request(datafeedId); - checkAndSetDeprecatedParam( - DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM, - Request.ALLOW_NO_MATCH, - RestApiVersion.V_7, - restRequest, - (r, s) -> r.paramAsBoolean(s, request.allowNoMatch()), - request::setAllowNoMatch - ); + request.setAllowNoMatch(restRequest.paramAsBoolean(Request.ALLOW_NO_MATCH, request.allowNoMatch())); return channel -> new RestCancellableNodeClient(client, restRequest.getHttpChannel()).execute( GetDatafeedsStatsAction.INSTANCE, request, diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestGetDatafeedsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestGetDatafeedsAction.java index fd0681f68a3a5..6955b81fdbb4c 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestGetDatafeedsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestGetDatafeedsAction.java @@ -7,8 +7,6 @@ package org.elasticsearch.xpack.ml.rest.datafeeds; import org.elasticsearch.client.internal.node.NodeClient; -import org.elasticsearch.core.RestApiVersion; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.Scope; @@ -26,11 +24,9 @@ import java.util.Set; import static org.elasticsearch.rest.RestRequest.Method.GET; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM; import static org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig.ID; import static org.elasticsearch.xpack.core.ml.utils.ToXContentParams.EXCLUDE_GENERATED; import static org.elasticsearch.xpack.ml.MachineLearning.BASE_PATH; -import static org.elasticsearch.xpack.ml.rest.RestCompatibilityChecker.checkAndSetDeprecatedParam; @ServerlessScope(Scope.PUBLIC) public class RestGetDatafeedsAction extends BaseRestHandler { @@ -51,16 +47,8 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient if (datafeedId == null) { datafeedId = GetDatafeedsAction.ALL; } - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 Request request = new Request(datafeedId); - checkAndSetDeprecatedParam( - DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM, - GetDatafeedsStatsAction.Request.ALLOW_NO_MATCH, - RestApiVersion.V_7, - restRequest, - (r, s) -> r.paramAsBoolean(s, request.allowNoMatch()), - request::setAllowNoMatch - ); + request.setAllowNoMatch(restRequest.paramAsBoolean(GetDatafeedsStatsAction.Request.ALLOW_NO_MATCH, request.allowNoMatch())); return channel -> new RestCancellableNodeClient(client, restRequest.getHttpChannel()).execute( GetDatafeedsAction.INSTANCE, request, diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestStopDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestStopDatafeedAction.java index 8235e2785cc37..dcc213a571469 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestStopDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestStopDatafeedAction.java @@ -7,9 +7,7 @@ package org.elasticsearch.xpack.ml.rest.datafeeds; import org.elasticsearch.client.internal.node.NodeClient; -import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; @@ -28,10 +26,8 @@ import java.util.List; import static org.elasticsearch.rest.RestRequest.Method.POST; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM; import static org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig.ID; import static org.elasticsearch.xpack.ml.MachineLearning.BASE_PATH; -import static org.elasticsearch.xpack.ml.rest.RestCompatibilityChecker.checkAndSetDeprecatedParam; @ServerlessScope(Scope.PUBLIC) public class RestStopDatafeedAction extends BaseRestHandler { @@ -49,7 +45,6 @@ public String getName() { @Override protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { String datafeedId = restRequest.param(DatafeedConfig.ID.getPreferredName()); - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 Request request; if (restRequest.hasContentOrSourceParam()) { XContentParser parser = restRequest.contentOrSourceParamParser(); @@ -63,14 +58,7 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient if (restRequest.hasParam(Request.FORCE.getPreferredName())) { request.setForce(restRequest.paramAsBoolean(Request.FORCE.getPreferredName(), request.isForce())); } - checkAndSetDeprecatedParam( - DEPRECATED_ALLOW_NO_DATAFEEDS_PARAM, - Request.ALLOW_NO_MATCH.getPreferredName(), - RestApiVersion.V_7, - restRequest, - (r, s) -> r.paramAsBoolean(s, request.allowNoMatch()), - request::setAllowNoMatch - ); + request.setAllowNoMatch(restRequest.paramAsBoolean(Request.ALLOW_NO_MATCH.getPreferredName(), request.allowNoMatch())); } return channel -> client.execute(StopDatafeedAction.INSTANCE, request, new RestBuilderListener(channel) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestCloseJobAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestCloseJobAction.java index f98a2f5a933ae..56afb7d65dfe3 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestCloseJobAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestCloseJobAction.java @@ -7,9 +7,7 @@ package org.elasticsearch.xpack.ml.rest.job; import org.elasticsearch.client.internal.node.NodeClient; -import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.Scope; @@ -23,10 +21,8 @@ import java.util.List; import static org.elasticsearch.rest.RestRequest.Method.POST; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_JOBS_PARAM; import static org.elasticsearch.xpack.core.ml.job.config.Job.ID; import static org.elasticsearch.xpack.ml.MachineLearning.BASE_PATH; -import static org.elasticsearch.xpack.ml.rest.RestCompatibilityChecker.checkAndSetDeprecatedParam; @ServerlessScope(Scope.PUBLIC) public class RestCloseJobAction extends BaseRestHandler { @@ -43,7 +39,6 @@ public String getName() { @Override protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 Request request; if (restRequest.hasContentOrSourceParam()) { request = Request.parseRequest(restRequest.param(Job.ID.getPreferredName()), restRequest.contentParser()); @@ -57,14 +52,7 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient if (restRequest.hasParam(Request.FORCE.getPreferredName())) { request.setForce(restRequest.paramAsBoolean(Request.FORCE.getPreferredName(), request.isForce())); } - checkAndSetDeprecatedParam( - DEPRECATED_ALLOW_NO_JOBS_PARAM, - Request.ALLOW_NO_MATCH.getPreferredName(), - RestApiVersion.V_7, - restRequest, - (r, s) -> r.paramAsBoolean(s, request.allowNoMatch()), - request::setAllowNoMatch - ); + request.setAllowNoMatch(restRequest.paramAsBoolean(Request.ALLOW_NO_MATCH.getPreferredName(), request.allowNoMatch())); } return channel -> client.execute(CloseJobAction.INSTANCE, request, new RestToXContentListener<>(channel)); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestGetJobStatsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestGetJobStatsAction.java index 2899faabdc40f..79951d7fad621 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestGetJobStatsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestGetJobStatsAction.java @@ -9,8 +9,6 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.Strings; -import org.elasticsearch.core.RestApiVersion; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.Scope; @@ -25,10 +23,8 @@ import java.util.List; import static org.elasticsearch.rest.RestRequest.Method.GET; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_JOBS_PARAM; import static org.elasticsearch.xpack.core.ml.job.config.Job.ID; import static org.elasticsearch.xpack.ml.MachineLearning.BASE_PATH; -import static org.elasticsearch.xpack.ml.rest.RestCompatibilityChecker.checkAndSetDeprecatedParam; @ServerlessScope(Scope.PUBLIC) public class RestGetJobStatsAction extends BaseRestHandler { @@ -52,16 +48,8 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient if (Strings.isNullOrEmpty(jobId)) { jobId = Metadata.ALL; } - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 Request request = new Request(jobId); - checkAndSetDeprecatedParam( - DEPRECATED_ALLOW_NO_JOBS_PARAM, - Request.ALLOW_NO_MATCH, - RestApiVersion.V_7, - restRequest, - (r, s) -> r.paramAsBoolean(s, request.allowNoMatch()), - request::setAllowNoMatch - ); + request.setAllowNoMatch(restRequest.paramAsBoolean(Request.ALLOW_NO_MATCH, request.allowNoMatch())); return channel -> new RestCancellableNodeClient(client, restRequest.getHttpChannel()).execute( GetJobsStatsAction.INSTANCE, request, diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestGetJobsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestGetJobsAction.java index ae8d234d1d8bd..ea63a38ab01a8 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestGetJobsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestGetJobsAction.java @@ -9,8 +9,6 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.Strings; -import org.elasticsearch.core.RestApiVersion; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.Scope; @@ -27,11 +25,9 @@ import java.util.Set; import static org.elasticsearch.rest.RestRequest.Method.GET; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_JOBS_PARAM; import static org.elasticsearch.xpack.core.ml.job.config.Job.ID; import static org.elasticsearch.xpack.core.ml.utils.ToXContentParams.EXCLUDE_GENERATED; import static org.elasticsearch.xpack.ml.MachineLearning.BASE_PATH; -import static org.elasticsearch.xpack.ml.rest.RestCompatibilityChecker.checkAndSetDeprecatedParam; @ServerlessScope(Scope.PUBLIC) public class RestGetJobsAction extends BaseRestHandler { @@ -52,16 +48,8 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient if (Strings.isNullOrEmpty(jobId)) { jobId = Metadata.ALL; } - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 Request request = new Request(jobId); - checkAndSetDeprecatedParam( - DEPRECATED_ALLOW_NO_JOBS_PARAM, - Request.ALLOW_NO_MATCH, - RestApiVersion.V_7, - restRequest, - (r, s) -> r.paramAsBoolean(s, request.allowNoMatch()), - request::setAllowNoMatch - ); + request.setAllowNoMatch(restRequest.paramAsBoolean(Request.ALLOW_NO_MATCH, request.allowNoMatch())); return channel -> new RestCancellableNodeClient(client, restRequest.getHttpChannel()).execute( GetJobsAction.INSTANCE, request, diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetOverallBucketsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetOverallBucketsAction.java index 2700e01cb9f6b..c74f49efe0aed 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetOverallBucketsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetOverallBucketsAction.java @@ -7,8 +7,6 @@ package org.elasticsearch.xpack.ml.rest.results; import org.elasticsearch.client.internal.node.NodeClient; -import org.elasticsearch.core.RestApiVersion; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.Scope; @@ -24,10 +22,8 @@ import static org.elasticsearch.rest.RestRequest.Method.GET; import static org.elasticsearch.rest.RestRequest.Method.POST; -import static org.elasticsearch.xpack.core.ml.MachineLearningField.DEPRECATED_ALLOW_NO_JOBS_PARAM; import static org.elasticsearch.xpack.core.ml.job.config.Job.ID; import static org.elasticsearch.xpack.ml.MachineLearning.BASE_PATH; -import static org.elasticsearch.xpack.ml.rest.RestCompatibilityChecker.checkAndSetDeprecatedParam; @ServerlessScope(Scope.PUBLIC) public class RestGetOverallBucketsAction extends BaseRestHandler { @@ -48,7 +44,6 @@ public String getName() { @Override protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { String jobId = restRequest.param(Job.ID.getPreferredName()); - @UpdateForV9(owner = UpdateForV9.Owner.MACHINE_LEARNING) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7 final Request request; if (restRequest.hasContentOrSourceParam()) { XContentParser parser = restRequest.contentOrSourceParamParser(); @@ -67,14 +62,7 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient if (restRequest.hasParam(Request.END.getPreferredName())) { request.setEnd(restRequest.param(Request.END.getPreferredName())); } - checkAndSetDeprecatedParam( - DEPRECATED_ALLOW_NO_JOBS_PARAM, - Request.ALLOW_NO_MATCH.getPreferredName(), - RestApiVersion.V_7, - restRequest, - (r, s) -> r.paramAsBoolean(s, request.allowNoMatch()), - request::setAllowNoMatch - ); + request.setAllowNoMatch(restRequest.paramAsBoolean(Request.ALLOW_NO_MATCH.getPreferredName(), request.allowNoMatch())); } return channel -> client.execute(GetOverallBucketsAction.INSTANCE, request, new RestToXContentListener<>(channel)); From 7573312f2ad955c0af47b365860b6aed8c705460 Mon Sep 17 00:00:00 2001 From: Tommaso Teofili Date: Thu, 12 Dec 2024 07:50:18 +0100 Subject: [PATCH 6/8] `_score` should not be a reserved attribute in ES|QL (#118435) --- docs/changelog/118435.yaml | 6 ++++ muted-tests.yml | 6 ++++ .../src/main/resources/scoring.csv-spec | 30 +++++++++++++++++++ .../xpack/esql/analysis/Verifier.java | 9 ------ .../xpack/esql/analysis/VerifierTests.java | 25 ---------------- 5 files changed, 42 insertions(+), 34 deletions(-) create mode 100644 docs/changelog/118435.yaml diff --git a/docs/changelog/118435.yaml b/docs/changelog/118435.yaml new file mode 100644 index 0000000000000..8bccbeb54698d --- /dev/null +++ b/docs/changelog/118435.yaml @@ -0,0 +1,6 @@ +pr: 118435 +summary: '`_score` should not be a reserved attribute in ES|QL' +area: ES|QL +type: enhancement +issues: + - 118460 diff --git a/muted-tests.yml b/muted-tests.yml index 573bd035b872b..5c99a2d5a5efd 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -180,6 +180,12 @@ tests: - class: "org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT" method: "test {scoring.*}" issue: https://github.com/elastic/elasticsearch/issues/117641 +- class: "org.elasticsearch.xpack.esql.qa.mixed.MultilusterEsqlSpecIT" + method: "test {scoring.*}" + issue: https://github.com/elastic/elasticsearch/issues/118460 +- class: "org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT" + method: "test {scoring.*}" + issue: https://github.com/elastic/elasticsearch/issues/118460 - class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT method: test {scoring.QstrWithFieldAndScoringSortedEval} issue: https://github.com/elastic/elasticsearch/issues/117751 diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/scoring.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/scoring.csv-spec index d4c7b8c59fdbc..cb38204a71ab0 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/scoring.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/scoring.csv-spec @@ -283,3 +283,33 @@ book_no:keyword | c_score:double 7350 | 2.0 7140 | 3.0 ; + +QstrScoreManipulation +required_capability: metadata_score +required_capability: qstr_function + +from books metadata _score +| where qstr("title:rings") +| eval _score = _score + 1 +| keep book_no, title, _score +| limit 2; + +book_no:keyword | title:text | _score:double +4023 | A Tolkien Compass: Including J. R. R. Tolkien's Guide to the Names in The Lord of the Rings | 2.6404519081115723 +2714 | Return of the King Being the Third Part of The Lord of the Rings | 2.9239964485168457 +; + +QstrScoreOverride +required_capability: metadata_score +required_capability: qstr_function + +from books metadata _score +| where qstr("title:rings") +| eval _score = "foobar" +| keep book_no, title, _score +| limit 2; + +book_no:keyword | title:text | _score:keyword +4023 | A Tolkien Compass: Including J. R. R. Tolkien's Guide to the Names in The Lord of the Rings | foobar +2714 | Return of the King Being the Third Part of The Lord of the Rings | foobar +; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java index a0728c9a91088..c805adf5d5a57 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java @@ -18,7 +18,6 @@ import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.expression.Expressions; import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; -import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute; import org.elasticsearch.xpack.esql.core.expression.NamedExpression; import org.elasticsearch.xpack.esql.core.expression.TypeResolutions; import org.elasticsearch.xpack.esql.core.expression.function.Function; @@ -208,7 +207,6 @@ else if (p instanceof Lookup lookup) { checkJoin(p, failures); }); checkRemoteEnrich(plan, failures); - checkMetadataScoreNameReserved(plan, failures); if (failures.isEmpty()) { checkLicense(plan, licenseState, failures); @@ -222,13 +220,6 @@ else if (p instanceof Lookup lookup) { return failures; } - private static void checkMetadataScoreNameReserved(LogicalPlan p, Set failures) { - // _score can only be set as metadata attribute - if (p.inputSet().stream().anyMatch(a -> MetadataAttribute.SCORE.equals(a.name()) && (a instanceof MetadataAttribute) == false)) { - failures.add(fail(p, "`" + MetadataAttribute.SCORE + "` is a reserved METADATA attribute")); - } - } - private void checkSort(LogicalPlan p, Set failures) { if (p instanceof OrderBy ob) { ob.order().forEach(o -> { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java index d58d233168e2b..84dcdbadef9f0 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java @@ -12,7 +12,6 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.EsqlCapabilities; -import org.elasticsearch.xpack.esql.core.expression.Attribute; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.core.type.EsField; import org.elasticsearch.xpack.esql.core.type.InvalidMappedField; @@ -22,7 +21,6 @@ import org.elasticsearch.xpack.esql.parser.EsqlParser; import org.elasticsearch.xpack.esql.parser.QueryParam; import org.elasticsearch.xpack.esql.parser.QueryParams; -import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -1805,29 +1803,6 @@ public void testToDatePeriodToTimeDurationWithInvalidType() { ); } - public void testNonMetadataScore() { - assumeTrue("'METADATA _score' is disabled", EsqlCapabilities.Cap.METADATA_SCORE.isEnabled()); - assertEquals("1:12: `_score` is a reserved METADATA attribute", error("from foo | eval _score = 10")); - - assertEquals( - "1:48: `_score` is a reserved METADATA attribute", - error("from foo metadata _score | where qstr(\"bar\") | eval _score = _score + 1") - ); - } - - public void testScoreRenaming() { - assumeTrue("'METADATA _score' is disabled", EsqlCapabilities.Cap.METADATA_SCORE.isEnabled()); - assertEquals("1:33: `_score` is a reserved METADATA attribute", error("from foo METADATA _id, _score | rename _id as _score")); - - assertTrue(passes("from foo metadata _score | rename _score as foo").stream().anyMatch(a -> a.name().equals("foo"))); - } - - private List passes(String query) { - LogicalPlan logicalPlan = defaultAnalyzer.analyze(parser.createStatement(query)); - assertTrue(logicalPlan.resolved()); - return logicalPlan.output(); - } - public void testIntervalAsString() { // DateTrunc for (String interval : List.of("1 minu", "1 dy", "1.5 minutes", "0.5 days", "minutes 1", "day 5")) { From ff8e5e964e545984063dc21b28e6c8da0f5acbcb Mon Sep 17 00:00:00 2001 From: Iraklis Psaroudakis Date: Thu, 12 Dec 2024 09:41:14 +0200 Subject: [PATCH 7/8] Improve InputStreamIndexInput testSkipBytes (#118485) Relates ES-10234 --- .../store/InputStreamIndexInputTests.java | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/lucene/store/InputStreamIndexInputTests.java b/server/src/test/java/org/elasticsearch/common/lucene/store/InputStreamIndexInputTests.java index 4bea6f50c7c4b..b982bd7b95aad 100644 --- a/server/src/test/java/org/elasticsearch/common/lucene/store/InputStreamIndexInputTests.java +++ b/server/src/test/java/org/elasticsearch/common/lucene/store/InputStreamIndexInputTests.java @@ -11,6 +11,7 @@ import org.apache.lucene.store.ByteBuffersDirectory; import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FilterIndexInput; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.IndexOutput; @@ -267,17 +268,47 @@ public void testSkipBytes() throws Exception { skipBytesExpected ); - IndexInput input = dir.openInput("test", IOContext.DEFAULT); - InputStreamIndexInput is = new InputStreamIndexInput(input, limit); + var countingInput = new CountingReadBytesIndexInput("test", dir.openInput("test", IOContext.DEFAULT)); + InputStreamIndexInput is = new InputStreamIndexInput(countingInput, limit); is.readNBytes(initialReadBytes); assertThat(is.skip(skipBytes), equalTo((long) skipBytesExpected)); + long expectedActualInitialBytesRead = Math.min(Math.min(initialReadBytes, limit), bytes); + assertThat(countingInput.getBytesRead(), equalTo(expectedActualInitialBytesRead)); int remainingBytes = Math.min(bytes, limit) - seekExpected; for (int i = seekExpected; i < seekExpected + remainingBytes; i++) { assertThat(is.read(), equalTo(i)); } + assertThat(countingInput.getBytesRead(), equalTo(expectedActualInitialBytesRead + remainingBytes)); } + protected static class CountingReadBytesIndexInput extends FilterIndexInput { + private long bytesRead = 0; + + public CountingReadBytesIndexInput(String resourceDescription, IndexInput in) { + super(resourceDescription, in); + } + + @Override + public byte readByte() throws IOException { + long filePointerBefore = getFilePointer(); + byte b = super.readByte(); + bytesRead += getFilePointer() - filePointerBefore; + return b; + } + + @Override + public void readBytes(byte[] b, int offset, int len) throws IOException { + long filePointerBefore = getFilePointer(); + super.readBytes(b, offset, len); + bytesRead += getFilePointer() - filePointerBefore; + } + + public long getBytesRead() { + return bytesRead; + } + }; + public void testReadZeroShouldReturnZero() throws IOException { try (Directory dir = new ByteBuffersDirectory()) { try (IndexOutput output = dir.createOutput("test", IOContext.DEFAULT)) { From 80a1a6f7afef990a7243a2e415866c6746113e28 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Thu, 12 Dec 2024 08:59:52 +0100 Subject: [PATCH 8/8] ESQL: Add CCS tests for FLS and DLS against data streams (#118423) CCS test coverage for https://github.com/elastic/elasticsearch/pull/118378 --- .../security/qa/multi-cluster/build.gradle | 1 + ...teClusterSecurityDataStreamEsqlRcs1IT.java | 402 ++++++++++++++++++ ...teClusterSecurityDataStreamEsqlRcs2IT.java | 126 ++++++ .../src/javaRestTest/resources/roles.yml | 99 +++++ 4 files changed, 628 insertions(+) create mode 100644 x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityDataStreamEsqlRcs1IT.java create mode 100644 x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityDataStreamEsqlRcs2IT.java diff --git a/x-pack/plugin/security/qa/multi-cluster/build.gradle b/x-pack/plugin/security/qa/multi-cluster/build.gradle index 5b682cfdccade..f4eee4ef46c02 100644 --- a/x-pack/plugin/security/qa/multi-cluster/build.gradle +++ b/x-pack/plugin/security/qa/multi-cluster/build.gradle @@ -24,6 +24,7 @@ dependencies { clusterModules project(':x-pack:plugin:enrich') clusterModules project(':x-pack:plugin:autoscaling') clusterModules project(':x-pack:plugin:ml') + clusterModules project(xpackModule('ilm')) clusterModules(project(":modules:ingest-common")) } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityDataStreamEsqlRcs1IT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityDataStreamEsqlRcs1IT.java new file mode 100644 index 0000000000000..57eb583912c49 --- /dev/null +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityDataStreamEsqlRcs1IT.java @@ -0,0 +1,402 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.remotecluster; + +import org.elasticsearch.Build; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.RequestOptions; +import org.elasticsearch.client.Response; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.CheckedFunction; +import org.elasticsearch.core.Strings; +import org.elasticsearch.test.MapMatcher; +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.util.resource.Resource; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.json.JsonXContent; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.io.IOException; +import java.util.List; +import java.util.Locale; + +import static org.elasticsearch.test.ListMatcher.matchesList; +import static org.elasticsearch.test.MapMatcher.assertMap; +import static org.elasticsearch.test.MapMatcher.matchesMap; + +// TODO consolidate me with RemoteClusterSecurityDataStreamEsqlRcs2IT +public class RemoteClusterSecurityDataStreamEsqlRcs1IT extends AbstractRemoteClusterSecurityTestCase { + static { + fulfillingCluster = ElasticsearchCluster.local() + .name("fulfilling-cluster") + .module("x-pack-autoscaling") + .module("x-pack-esql") + .module("x-pack-enrich") + .module("x-pack-ml") + .module("x-pack-ilm") + .module("ingest-common") + .apply(commonClusterConfig) + .setting("xpack.ml.enabled", "false") + .setting("xpack.security.authc.token.enabled", "true") + .rolesFile(Resource.fromClasspath("roles.yml")) + .build(); + + queryCluster = ElasticsearchCluster.local() + .name("query-cluster") + .module("x-pack-autoscaling") + .module("x-pack-esql") + .module("x-pack-enrich") + .module("x-pack-ml") + .module("x-pack-ilm") + .module("ingest-common") + .apply(commonClusterConfig) + .setting("xpack.ml.enabled", "false") + .setting("xpack.security.authc.token.enabled", "true") + .rolesFile(Resource.fromClasspath("roles.yml")) + .user("logs_foo_all", "x-pack-test-password", "logs_foo_all", false) + .user("logs_foo_16_only", "x-pack-test-password", "logs_foo_16_only", false) + .user("logs_foo_after_2021", "x-pack-test-password", "logs_foo_after_2021", false) + .user("logs_foo_after_2021_pattern", "x-pack-test-password", "logs_foo_after_2021_pattern", false) + .user("logs_foo_after_2021_alias", "x-pack-test-password", "logs_foo_after_2021_alias", false) + .build(); + } + + @ClassRule + public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); + + public void testDataStreamsWithDlsAndFls() throws Exception { + configureRemoteCluster(REMOTE_CLUSTER_ALIAS, fulfillingCluster, true, randomBoolean(), randomBoolean()); + createDataStreamOnFulfillingCluster(); + setupAdditionalUsersAndRoles(); + + doTestDataStreamsWithFlsAndDls(); + } + + private void setupAdditionalUsersAndRoles() throws IOException { + createUserAndRoleOnQueryCluster("fls_user_logs_pattern", "fls_user_logs_pattern", """ + { + "indices": [ + { + "names": ["logs-*"], + "privileges": ["read"], + "field_security": { + "grant": ["@timestamp", "data_stream.namespace"] + } + } + ] + }"""); + createUserAndRoleOnFulfillingCluster("fls_user_logs_pattern", "fls_user_logs_pattern", """ + { + "indices": [ + { + "names": ["logs-*"], + "privileges": ["read"], + "field_security": { + "grant": ["@timestamp", "data_stream.namespace"] + } + } + ] + }"""); + } + + static void createUserAndRoleOnQueryCluster(String username, String roleName, String roleJson) throws IOException { + final var putRoleRequest = new Request("PUT", "/_security/role/" + roleName); + putRoleRequest.setJsonEntity(roleJson); + assertOK(adminClient().performRequest(putRoleRequest)); + + final var putUserRequest = new Request("PUT", "/_security/user/" + username); + putUserRequest.setJsonEntity(Strings.format(""" + { + "password": "%s", + "roles" : ["%s"] + }""", PASS, roleName)); + assertOK(adminClient().performRequest(putUserRequest)); + } + + static void createUserAndRoleOnFulfillingCluster(String username, String roleName, String roleJson) throws IOException { + final var putRoleRequest = new Request("PUT", "/_security/role/" + roleName); + putRoleRequest.setJsonEntity(roleJson); + assertOK(performRequestAgainstFulfillingCluster(putRoleRequest)); + + final var putUserRequest = new Request("PUT", "/_security/user/" + username); + putUserRequest.setJsonEntity(Strings.format(""" + { + "password": "%s", + "roles" : ["%s"] + }""", PASS, roleName)); + assertOK(performRequestAgainstFulfillingCluster(putUserRequest)); + } + + static Response runESQLCommandAgainstQueryCluster(String user, String command) throws IOException { + if (command.toLowerCase(Locale.ROOT).contains("limit") == false) { + // add a (high) limit to avoid warnings on default limit + command += " | limit 10000000"; + } + XContentBuilder json = JsonXContent.contentBuilder(); + json.startObject(); + json.field("query", command); + addRandomPragmas(json); + json.endObject(); + Request request = new Request("POST", "_query"); + request.setJsonEntity(org.elasticsearch.common.Strings.toString(json)); + request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("es-security-runas-user", user)); + request.addParameter("error_trace", "true"); + Response response = adminClient().performRequest(request); + assertOK(response); + return response; + } + + static void addRandomPragmas(XContentBuilder builder) throws IOException { + if (Build.current().isSnapshot()) { + Settings pragmas = randomPragmas(); + if (pragmas != Settings.EMPTY) { + builder.startObject("pragma"); + builder.value(pragmas); + builder.endObject(); + } + } + } + + static Settings randomPragmas() { + Settings.Builder settings = Settings.builder(); + if (randomBoolean()) { + settings.put("page_size", between(1, 5)); + } + if (randomBoolean()) { + settings.put("exchange_buffer_size", between(1, 2)); + } + if (randomBoolean()) { + settings.put("data_partitioning", randomFrom("shard", "segment", "doc")); + } + if (randomBoolean()) { + settings.put("enrich_max_workers", between(1, 5)); + } + if (randomBoolean()) { + settings.put("node_level_reduction", randomBoolean()); + } + return settings.build(); + } + + static void createDataStreamOnFulfillingCluster() throws Exception { + createDataStreamPolicy(AbstractRemoteClusterSecurityTestCase::performRequestAgainstFulfillingCluster); + createDataStreamComponentTemplate(AbstractRemoteClusterSecurityTestCase::performRequestAgainstFulfillingCluster); + createDataStreamIndexTemplate(AbstractRemoteClusterSecurityTestCase::performRequestAgainstFulfillingCluster); + createDataStreamDocuments(AbstractRemoteClusterSecurityTestCase::performRequestAgainstFulfillingCluster); + createDataStreamAlias(AbstractRemoteClusterSecurityTestCase::performRequestAgainstFulfillingCluster); + } + + private static void createDataStreamPolicy(CheckedFunction requestConsumer) throws Exception { + Request request = new Request("PUT", "_ilm/policy/my-lifecycle-policy"); + request.setJsonEntity(""" + { + "policy": { + "phases": { + "hot": { + "actions": { + "rollover": { + "max_primary_shard_size": "50gb" + } + } + }, + "delete": { + "min_age": "735d", + "actions": { + "delete": {} + } + } + } + } + }"""); + + requestConsumer.apply(request); + } + + private static void createDataStreamComponentTemplate(CheckedFunction requestConsumer) throws Exception { + Request request = new Request("PUT", "_component_template/my-template"); + request.setJsonEntity(""" + { + "template": { + "settings": { + "index.lifecycle.name": "my-lifecycle-policy" + }, + "mappings": { + "properties": { + "@timestamp": { + "type": "date", + "format": "date_optional_time||epoch_millis" + }, + "data_stream": { + "properties": { + "namespace": {"type": "keyword"}, + "environment": {"type": "keyword"} + } + } + } + } + } + }"""); + requestConsumer.apply(request); + } + + private static void createDataStreamIndexTemplate(CheckedFunction requestConsumer) throws Exception { + Request request = new Request("PUT", "_index_template/my-index-template"); + request.setJsonEntity(""" + { + "index_patterns": ["logs-*"], + "data_stream": {}, + "composed_of": ["my-template"], + "priority": 500 + }"""); + requestConsumer.apply(request); + } + + private static void createDataStreamDocuments(CheckedFunction requestConsumer) throws Exception { + Request request = new Request("POST", "logs-foo/_bulk"); + request.addParameter("refresh", ""); + request.setJsonEntity(""" + { "create" : {} } + { "@timestamp": "2099-05-06T16:21:15.000Z", "data_stream": {"namespace": "16", "environment": "dev"} } + { "create" : {} } + { "@timestamp": "2001-05-06T16:21:15.000Z", "data_stream": {"namespace": "17", "environment": "prod"} } + """); + assertMap(entityAsMap(requestConsumer.apply(request)), matchesMap().extraOk().entry("errors", false)); + } + + private static void createDataStreamAlias(CheckedFunction requestConsumer) throws Exception { + Request request = new Request("PUT", "_alias"); + request.setJsonEntity(""" + { + "actions": [ + { + "add": { + "index": "logs-foo", + "alias": "alias-foo" + } + } + ] + }"""); + assertMap(entityAsMap(requestConsumer.apply(request)), matchesMap().extraOk().entry("errors", false)); + } + + static void doTestDataStreamsWithFlsAndDls() throws IOException { + // DLS + MapMatcher twoResults = matchesMap().extraOk().entry("values", matchesList().item(matchesList().item(2))); + MapMatcher oneResult = matchesMap().extraOk().entry("values", matchesList().item(matchesList().item(1))); + assertMap( + entityAsMap(runESQLCommandAgainstQueryCluster("logs_foo_all", "FROM my_remote_cluster:logs-foo | STATS COUNT(*)")), + twoResults + ); + assertMap( + entityAsMap(runESQLCommandAgainstQueryCluster("logs_foo_16_only", "FROM my_remote_cluster:logs-foo | STATS COUNT(*)")), + oneResult + ); + assertMap( + entityAsMap(runESQLCommandAgainstQueryCluster("logs_foo_after_2021", "FROM my_remote_cluster:logs-foo | STATS COUNT(*)")), + oneResult + ); + assertMap( + entityAsMap( + runESQLCommandAgainstQueryCluster("logs_foo_after_2021_pattern", "FROM my_remote_cluster:logs-foo | STATS COUNT(*)") + ), + oneResult + ); + assertMap( + entityAsMap(runESQLCommandAgainstQueryCluster("logs_foo_all", "FROM my_remote_cluster:logs-* | STATS COUNT(*)")), + twoResults + ); + assertMap( + entityAsMap(runESQLCommandAgainstQueryCluster("logs_foo_16_only", "FROM my_remote_cluster:logs-* | STATS COUNT(*)")), + oneResult + ); + assertMap( + entityAsMap(runESQLCommandAgainstQueryCluster("logs_foo_after_2021", "FROM my_remote_cluster:logs-* | STATS COUNT(*)")), + oneResult + ); + assertMap( + entityAsMap(runESQLCommandAgainstQueryCluster("logs_foo_after_2021_pattern", "FROM my_remote_cluster:logs-* | STATS COUNT(*)")), + oneResult + ); + + assertMap( + entityAsMap( + runESQLCommandAgainstQueryCluster("logs_foo_after_2021_alias", "FROM my_remote_cluster:alias-foo | STATS COUNT(*)") + ), + oneResult + ); + assertMap( + entityAsMap(runESQLCommandAgainstQueryCluster("logs_foo_after_2021_alias", "FROM my_remote_cluster:alias-* | STATS COUNT(*)")), + oneResult + ); + + // FLS + // logs_foo_all does not have FLS restrictions so should be able to access all fields + assertMap( + entityAsMap( + runESQLCommandAgainstQueryCluster("logs_foo_all", "FROM my_remote_cluster:logs-foo | SORT data_stream.namespace | LIMIT 1") + ), + matchesMap().extraOk() + .entry( + "columns", + List.of( + matchesMap().entry("name", "@timestamp").entry("type", "date"), + matchesMap().entry("name", "data_stream.environment").entry("type", "keyword"), + matchesMap().entry("name", "data_stream.namespace").entry("type", "keyword") + ) + ) + ); + assertMap( + entityAsMap( + runESQLCommandAgainstQueryCluster("logs_foo_all", "FROM my_remote_cluster:logs-* | SORT data_stream.namespace | LIMIT 1") + ), + matchesMap().extraOk() + .entry( + "columns", + List.of( + matchesMap().entry("name", "@timestamp").entry("type", "date"), + matchesMap().entry("name", "data_stream.environment").entry("type", "keyword"), + matchesMap().entry("name", "data_stream.namespace").entry("type", "keyword") + ) + ) + ); + + assertMap( + entityAsMap( + runESQLCommandAgainstQueryCluster( + "fls_user_logs_pattern", + "FROM my_remote_cluster:logs-foo | SORT data_stream.namespace | LIMIT 1" + ) + ), + matchesMap().extraOk() + .entry( + "columns", + List.of( + matchesMap().entry("name", "@timestamp").entry("type", "date"), + matchesMap().entry("name", "data_stream.namespace").entry("type", "keyword") + ) + ) + ); + assertMap( + entityAsMap( + runESQLCommandAgainstQueryCluster( + "fls_user_logs_pattern", + "FROM my_remote_cluster:logs-* | SORT data_stream.namespace | LIMIT 1" + ) + ), + matchesMap().extraOk() + .entry( + "columns", + List.of( + matchesMap().entry("name", "@timestamp").entry("type", "date"), + matchesMap().entry("name", "data_stream.namespace").entry("type", "keyword") + ) + ) + ); + } +} diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityDataStreamEsqlRcs2IT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityDataStreamEsqlRcs2IT.java new file mode 100644 index 0000000000000..c5cf704177020 --- /dev/null +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityDataStreamEsqlRcs2IT.java @@ -0,0 +1,126 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.remotecluster; + +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.util.resource.Resource; +import org.elasticsearch.test.junit.RunnableTestRuleAdapter; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.io.IOException; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static org.elasticsearch.xpack.remotecluster.RemoteClusterSecurityDataStreamEsqlRcs1IT.createDataStreamOnFulfillingCluster; +import static org.elasticsearch.xpack.remotecluster.RemoteClusterSecurityDataStreamEsqlRcs1IT.createUserAndRoleOnQueryCluster; +import static org.elasticsearch.xpack.remotecluster.RemoteClusterSecurityDataStreamEsqlRcs1IT.doTestDataStreamsWithFlsAndDls; + +// TODO consolidate me with RemoteClusterSecurityDataStreamEsqlRcs1IT +public class RemoteClusterSecurityDataStreamEsqlRcs2IT extends AbstractRemoteClusterSecurityTestCase { + private static final AtomicReference> API_KEY_MAP_REF = new AtomicReference<>(); + private static final AtomicBoolean SSL_ENABLED_REF = new AtomicBoolean(); + private static final AtomicBoolean NODE1_RCS_SERVER_ENABLED = new AtomicBoolean(); + private static final AtomicBoolean NODE2_RCS_SERVER_ENABLED = new AtomicBoolean(); + + static { + fulfillingCluster = ElasticsearchCluster.local() + .name("fulfilling-cluster") + .nodes(3) + .module("x-pack-autoscaling") + .module("x-pack-esql") + .module("x-pack-enrich") + .module("x-pack-ml") + .module("x-pack-ilm") + .module("ingest-common") + .apply(commonClusterConfig) + .setting("remote_cluster.port", "0") + .setting("xpack.ml.enabled", "false") + .setting("xpack.security.remote_cluster_server.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get())) + .setting("xpack.security.remote_cluster_server.ssl.key", "remote-cluster.key") + .setting("xpack.security.remote_cluster_server.ssl.certificate", "remote-cluster.crt") + .setting("xpack.security.authc.token.enabled", "true") + .keystore("xpack.security.remote_cluster_server.ssl.secure_key_passphrase", "remote-cluster-password") + .node(0, spec -> spec.setting("remote_cluster_server.enabled", "true")) + .node(1, spec -> spec.setting("remote_cluster_server.enabled", () -> String.valueOf(NODE1_RCS_SERVER_ENABLED.get()))) + .node(2, spec -> spec.setting("remote_cluster_server.enabled", () -> String.valueOf(NODE2_RCS_SERVER_ENABLED.get()))) + .build(); + + queryCluster = ElasticsearchCluster.local() + .name("query-cluster") + .module("x-pack-autoscaling") + .module("x-pack-esql") + .module("x-pack-enrich") + .module("x-pack-ml") + .module("x-pack-ilm") + .module("ingest-common") + .apply(commonClusterConfig) + .setting("xpack.ml.enabled", "false") + .setting("xpack.security.remote_cluster_client.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get())) + .setting("xpack.security.remote_cluster_client.ssl.certificate_authorities", "remote-cluster-ca.crt") + .setting("xpack.security.authc.token.enabled", "true") + .keystore("cluster.remote.my_remote_cluster.credentials", () -> { + if (API_KEY_MAP_REF.get() == null) { + final Map apiKeyMap = createCrossClusterAccessApiKey(""" + { + "search": [ + { + "names": ["logs-*", "alias-*"] + } + ] + }"""); + API_KEY_MAP_REF.set(apiKeyMap); + } + return (String) API_KEY_MAP_REF.get().get("encoded"); + }) + .rolesFile(Resource.fromClasspath("roles.yml")) + .user("logs_foo_all", "x-pack-test-password", "logs_foo_all", false) + .user("logs_foo_16_only", "x-pack-test-password", "logs_foo_16_only", false) + .user("logs_foo_after_2021", "x-pack-test-password", "logs_foo_after_2021", false) + .user("logs_foo_after_2021_pattern", "x-pack-test-password", "logs_foo_after_2021_pattern", false) + .user("logs_foo_after_2021_alias", "x-pack-test-password", "logs_foo_after_2021_alias", false) + .build(); + } + + @ClassRule + // Use a RuleChain to ensure that fulfilling cluster is started before query cluster + // `SSL_ENABLED_REF` is used to control the SSL-enabled setting on the test clusters + // We set it here, since randomization methods are not available in the static initialize context above + public static TestRule clusterRule = RuleChain.outerRule(new RunnableTestRuleAdapter(() -> { + SSL_ENABLED_REF.set(usually()); + NODE1_RCS_SERVER_ENABLED.set(randomBoolean()); + NODE2_RCS_SERVER_ENABLED.set(randomBoolean()); + })).around(fulfillingCluster).around(queryCluster); + + public void testDataStreamsWithDlsAndFls() throws Exception { + configureRemoteCluster(); + createDataStreamOnFulfillingCluster(); + setupAdditionalUsersAndRoles(); + + doTestDataStreamsWithFlsAndDls(); + } + + private void setupAdditionalUsersAndRoles() throws IOException { + createUserAndRoleOnQueryCluster("fls_user_logs_pattern", "fls_user_logs_pattern", """ + { + "indices": [{"names": [""], "privileges": ["read"]}], + "remote_indices": [ + { + "names": ["logs-*"], + "privileges": ["read"], + "field_security": { + "grant": ["@timestamp", "data_stream.namespace"] + }, + "clusters": ["*"] + } + ] + }"""); + } +} diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/resources/roles.yml b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/resources/roles.yml index b61daa068ed1a..c09f9dc620a4c 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/resources/roles.yml +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/resources/roles.yml @@ -41,3 +41,102 @@ ccr_user_role: manage_role: cluster: [ 'manage' ] + +logs_foo_all: + cluster: [] + indices: + - names: [ 'logs-foo' ] + privileges: [ 'read' ] + remote_indices: + - names: [ 'logs-foo' ] + clusters: [ '*' ] + privileges: [ 'read' ] + +logs_foo_16_only: + cluster: [] + indices: + - names: [ 'logs-foo' ] + privileges: [ 'read' ] + query: | + { + "term": { + "data_stream.namespace": "16" + } + } + remote_indices: + - names: [ 'logs-foo' ] + clusters: [ '*' ] + privileges: [ 'read' ] + query: | + { + "term": { + "data_stream.namespace": "16" + } + } + +logs_foo_after_2021: + cluster: [] + indices: + - names: [ 'logs-foo' ] + privileges: [ 'read' ] + query: | + { + "range": { + "@timestamp": {"gte": "2021-01-01T00:00:00"} + } + } + remote_indices: + - names: [ 'logs-foo' ] + clusters: [ '*' ] + privileges: [ 'read' ] + query: | + { + "range": { + "@timestamp": {"gte": "2021-01-01T00:00:00"} + } + } + +logs_foo_after_2021_pattern: + cluster: [] + indices: + - names: [ 'logs-*' ] + privileges: [ 'read' ] + query: | + { + "range": { + "@timestamp": {"gte": "2021-01-01T00:00:00"} + } + } + remote_indices: + - names: [ 'logs-*' ] + clusters: [ '*' ] + privileges: [ 'read' ] + query: | + { + "range": { + "@timestamp": {"gte": "2021-01-01T00:00:00"} + } + } + +logs_foo_after_2021_alias: + cluster: [] + indices: + - names: [ 'alias-foo' ] + privileges: [ 'read' ] + query: | + { + "range": { + "@timestamp": {"gte": "2021-01-01T00:00:00"} + } + } + remote_indices: + - names: [ 'alias-foo' ] + clusters: [ '*' ] + privileges: [ 'read' ] + query: | + { + "range": { + "@timestamp": {"gte": "2021-01-01T00:00:00"} + } + } +