From 008b72ffb9f3eb2cdd3ed8bf0117e65a4319945c Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 1 Aug 2018 12:31:58 -0600 Subject: [PATCH 1/2] Remove RolloverIndexTestHelper This removes the `RolloverIndexTestHelper` class in favor of making a couple of getters publically accessible as well as custom building a response object using JSON parsing. Relates to #29823 --- .../admin/indices/rollover/Condition.java | 4 ++ .../indices/rollover/RolloverRequest.java | 2 +- .../rollover/RolloverIndexTestHelper.java | 39 ----------------- .../indexlifecycle/RolloverStepTests.java | 43 ++++++++++++++++--- 4 files changed, 42 insertions(+), 46 deletions(-) delete mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIndexTestHelper.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/Condition.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/Condition.java index 6efebde18f577..4a65427f34e17 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/Condition.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/Condition.java @@ -71,6 +71,10 @@ public final String toString() { return "[" + name + ": " + value + "]"; } + public T value() { + return value; + } + /** * Holder for index stats used to evaluate conditions */ diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java index 48c9d46066034..1475e4bd42088 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java @@ -196,7 +196,7 @@ public boolean isDryRun() { return dryRun; } - Map> getConditions() { + public Map> getConditions() { return conditions; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIndexTestHelper.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIndexTestHelper.java deleted file mode 100644 index abe069137a312..0000000000000 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIndexTestHelper.java +++ /dev/null @@ -1,39 +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; - * you may not use this file except in compliance with the Elastic License. - * - */ -package org.elasticsearch.action.admin.indices.rollover; - -import java.util.Collections; -import java.util.Set; -import java.util.stream.Collectors; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; - -public final class RolloverIndexTestHelper { - - // NORELEASE this isn't nice but it's currently the only way to inspect the - // settings in an update settings request. Need to see if we can make the - // getter public in ES - public static void assertRolloverIndexRequest(RolloverRequest request, String alias, Set> expectedConditions) { - assertNotNull(request); - assertEquals(1, request.indices().length); - assertEquals(alias, request.indices()[0]); - assertEquals(alias, request.getAlias()); - assertEquals(expectedConditions.size(), request.getConditions().size()); - Set expectedConditionValues = expectedConditions.stream().map(condition -> condition.value).collect(Collectors.toSet()); - Set actualConditionValues = request.getConditions().values().stream() - .map(condition -> condition.value).collect(Collectors.toSet()); - assertEquals(expectedConditionValues, actualConditionValues); - } - - // NORELEASE this isn't nice but it's currently the only way to create an - // UpdateSettingsResponse. Need to see if we can make the constructor public - // in ES - public static RolloverResponse createMockResponse(RolloverRequest request, boolean rolledOver) { - return new RolloverResponse(null, null, Collections.emptyMap(), request.isDryRun(), rolledOver, true, true); - } -} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStepTests.java index 7eb340b57220b..7b069db5830ed 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStepTests.java @@ -12,7 +12,6 @@ import org.elasticsearch.action.admin.indices.rollover.MaxAgeCondition; import org.elasticsearch.action.admin.indices.rollover.MaxDocsCondition; import org.elasticsearch.action.admin.indices.rollover.MaxSizeCondition; -import org.elasticsearch.action.admin.indices.rollover.RolloverIndexTestHelper; import org.elasticsearch.action.admin.indices.rollover.RolloverRequest; import org.elasticsearch.action.admin.indices.rollover.RolloverResponse; import org.elasticsearch.client.AdminClient; @@ -22,6 +21,9 @@ import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.xpack.core.indexlifecycle.AsyncActionStep.Listener; import org.elasticsearch.xpack.core.indexlifecycle.Step.StepKey; import org.junit.Before; @@ -29,10 +31,13 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import java.io.IOException; import java.util.HashSet; import java.util.Locale; import java.util.Set; +import java.util.stream.Collectors; +import static org.elasticsearch.common.xcontent.DeprecationHandler.THROW_UNSUPPORTED_OPERATION; import static org.hamcrest.Matchers.equalTo; public class RolloverStepTests extends AbstractStepTestCase { @@ -97,6 +102,32 @@ public RolloverStep copyInstance(RolloverStep instance) { instance.getMaxSize(), instance.getMaxAge(), instance.getMaxDocs()); } + private static void assertRolloverIndexRequest(RolloverRequest request, String alias, Set> expectedConditions) { + assertNotNull(request); + assertEquals(1, request.indices().length); + assertEquals(alias, request.indices()[0]); + assertEquals(alias, request.getAlias()); + assertEquals(expectedConditions.size(), request.getConditions().size()); + Set expectedConditionValues = expectedConditions.stream().map(Condition::value).collect(Collectors.toSet()); + Set actualConditionValues = request.getConditions().values().stream() + .map(Condition::value).collect(Collectors.toSet()); + assertEquals(expectedConditionValues, actualConditionValues); + } + + private static RolloverResponse createMockResponse(RolloverRequest request, boolean rolledOver) throws IOException { + String json = "{\"new_index\": \"foo\"," + + "\"old_index\": \"bar\"," + + "\"dry_run\": " + request.isDryRun() + "," + + "\"rolled_over\":" + rolledOver + "," + + "\"conditions\": {}," + + "\"acknowledged\": true," + + "\"shards_acknowledged\": true}"; + try (XContentParser parser = XContentType.JSON.xContent() + .createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, json)) { + return RolloverResponse.fromXContent(parser); + } + } + public void testPerformAction() throws Exception { String alias = randomAlphaOfLength(5); IndexMetaData indexMetaData = IndexMetaData.builder(randomAlphaOfLength(10)) @@ -127,8 +158,8 @@ public Void answer(InvocationOnMock invocation) throws Throwable { if (step.getMaxDocs() != null) { expectedConditions.add(new MaxDocsCondition(step.getMaxDocs())); } - RolloverIndexTestHelper.assertRolloverIndexRequest(request, alias, expectedConditions); - listener.onResponse(RolloverIndexTestHelper.createMockResponse(request, true)); + assertRolloverIndexRequest(request, alias, expectedConditions); + listener.onResponse(createMockResponse(request, true)); return null; } @@ -184,8 +215,8 @@ public Void answer(InvocationOnMock invocation) throws Throwable { if (step.getMaxDocs() != null) { expectedConditions.add(new MaxDocsCondition(step.getMaxDocs())); } - RolloverIndexTestHelper.assertRolloverIndexRequest(request, alias, expectedConditions); - listener.onResponse(RolloverIndexTestHelper.createMockResponse(request, false)); + assertRolloverIndexRequest(request, alias, expectedConditions); + listener.onResponse(createMockResponse(request, false)); return null; } @@ -242,7 +273,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable { if (step.getMaxDocs() != null) { expectedConditions.add(new MaxDocsCondition(step.getMaxDocs())); } - RolloverIndexTestHelper.assertRolloverIndexRequest(request, alias, expectedConditions); + assertRolloverIndexRequest(request, alias, expectedConditions); listener.onFailure(exception); return null; } From e10218c28e73559179a84669d0e38e91deda1152 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 2 Aug 2018 11:54:39 -0600 Subject: [PATCH 2/2] Make RolloverResponse constructor public, remove JSON parsing --- .../indices/rollover/RolloverResponse.java | 4 +-- .../indexlifecycle/RolloverStepTests.java | 30 ++++--------------- 2 files changed, 8 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java index 2d699591192f1..4fb5b6a19f117 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java @@ -68,8 +68,8 @@ public final class RolloverResponse extends ShardsAcknowledgedResponse implement RolloverResponse() { } - RolloverResponse(String oldIndex, String newIndex, Map conditionResults, - boolean dryRun, boolean rolledOver, boolean acknowledged, boolean shardsAcknowledged) { + public RolloverResponse(String oldIndex, String newIndex, Map conditionResults, + boolean dryRun, boolean rolledOver, boolean acknowledged, boolean shardsAcknowledged) { super(acknowledged, shardsAcknowledged); this.oldIndex = oldIndex; this.newIndex = newIndex; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStepTests.java index 7b069db5830ed..c2445c57db24b 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStepTests.java @@ -21,9 +21,6 @@ import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.xpack.core.indexlifecycle.AsyncActionStep.Listener; import org.elasticsearch.xpack.core.indexlifecycle.Step.StepKey; import org.junit.Before; @@ -31,13 +28,12 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import java.io.IOException; +import java.util.Collections; import java.util.HashSet; import java.util.Locale; import java.util.Set; import java.util.stream.Collectors; -import static org.elasticsearch.common.xcontent.DeprecationHandler.THROW_UNSUPPORTED_OPERATION; import static org.hamcrest.Matchers.equalTo; public class RolloverStepTests extends AbstractStepTestCase { @@ -114,21 +110,7 @@ private static void assertRolloverIndexRequest(RolloverRequest request, String a assertEquals(expectedConditionValues, actualConditionValues); } - private static RolloverResponse createMockResponse(RolloverRequest request, boolean rolledOver) throws IOException { - String json = "{\"new_index\": \"foo\"," + - "\"old_index\": \"bar\"," + - "\"dry_run\": " + request.isDryRun() + "," + - "\"rolled_over\":" + rolledOver + "," + - "\"conditions\": {}," + - "\"acknowledged\": true," + - "\"shards_acknowledged\": true}"; - try (XContentParser parser = XContentType.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, json)) { - return RolloverResponse.fromXContent(parser); - } - } - - public void testPerformAction() throws Exception { + public void testPerformAction() { String alias = randomAlphaOfLength(5); IndexMetaData indexMetaData = IndexMetaData.builder(randomAlphaOfLength(10)) .settings(settings(Version.CURRENT).put(RolloverAction.LIFECYCLE_ROLLOVER_ALIAS, alias)) @@ -159,7 +141,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable { expectedConditions.add(new MaxDocsCondition(step.getMaxDocs())); } assertRolloverIndexRequest(request, alias, expectedConditions); - listener.onResponse(createMockResponse(request, true)); + listener.onResponse(new RolloverResponse(null, null, Collections.emptyMap(), request.isDryRun(), true, true, true)); return null; } @@ -186,7 +168,7 @@ public void onFailure(Exception e) { Mockito.verify(indicesClient, Mockito.only()).rolloverIndex(Mockito.any(), Mockito.any()); } - public void testPerformActionNotComplete() throws Exception { + public void testPerformActionNotComplete() { String alias = randomAlphaOfLength(5); IndexMetaData indexMetaData = IndexMetaData.builder(randomAlphaOfLength(10)) .settings(settings(Version.CURRENT).put(RolloverAction.LIFECYCLE_ROLLOVER_ALIAS, alias)) @@ -216,7 +198,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable { expectedConditions.add(new MaxDocsCondition(step.getMaxDocs())); } assertRolloverIndexRequest(request, alias, expectedConditions); - listener.onResponse(createMockResponse(request, false)); + listener.onResponse(new RolloverResponse(null, null, Collections.emptyMap(), request.isDryRun(), false, true, true)); return null; } @@ -243,7 +225,7 @@ public void onFailure(Exception e) { Mockito.verify(indicesClient, Mockito.only()).rolloverIndex(Mockito.any(), Mockito.any()); } - public void testPerformActionFailure() throws Exception { + public void testPerformActionFailure() { String alias = randomAlphaOfLength(5); IndexMetaData indexMetaData = IndexMetaData.builder(randomAlphaOfLength(10)) .settings(settings(Version.CURRENT).put(RolloverAction.LIFECYCLE_ROLLOVER_ALIAS, alias))