Use sorted set to store targets for views

Signed-off-by: Peter Nied <peternied@hotmail.com>

Author: peternied

server/src/internalClusterTest/java/org/opensearch/action/admin/indices/view/ViewIT.java

@@ -41,6 +41,20 @@ public void testCreateView() throws Exception {
         MatcherAssert.assertThat(ex.getMessage(), is("View [" + viewName + "] already exists"));
     }
 
+    public void testCreateViewTargetsSet() throws Exception {
+        final String viewName = randomAlphaOfLength(8);
+        final String indexPattern = "a" + randomAlphaOfLength(8);
+        final String indexPattern2 = "b" + randomAlphaOfLength(8);
+        final List<String> targetPatterns = List.of(indexPattern2, indexPattern, indexPattern);
+
+        logger.info("Testing createView with targets that will be reordered and deduplicated");
+        final View view = createView(viewName, targetPatterns).getView();
+        MatcherAssert.assertThat(view.getName(), is(viewName));
+        MatcherAssert.assertThat(view.getTargets().size(), is(2));
+        MatcherAssert.assertThat(view.getTargets().first().getIndexPattern(), is(indexPattern));
+        MatcherAssert.assertThat(view.getTargets().last().getIndexPattern(), is(indexPattern2));
+    }
+
     public void testGetView() throws Exception {
         final String viewName = randomAlphaOfLength(8);
         createView(viewName, randomAlphaOfLength(8));

server/src/internalClusterTest/java/org/opensearch/action/admin/indices/view/ViewTestBase.java

@@ -34,10 +34,14 @@ protected int createIndexWithDocs(final String indexName) throws Exception {
     }
 
     protected GetViewAction.Response createView(final String name, final String indexPattern) throws Exception {
+        return createView(name, List.of(indexPattern));
+    }
+
+    protected GetViewAction.Response createView(final String name, final List<String> targets) throws Exception {
         final CreateViewAction.Request request = new CreateViewAction.Request(
             name,
             null,
-            List.of(new CreateViewAction.Request.Target(indexPattern))
+            targets.stream().map(CreateViewAction.Request.Target::new).collect(Collectors.toList())
         );
         return client().admin().indices().createView(request).actionGet();
     }

server/src/main/java/org/opensearch/action/admin/indices/view/CreateViewAction.java

@@ -124,7 +124,6 @@ public ActionRequestValidationException validate() {
             if (CollectionUtils.isEmpty(targets)) {
                 validationException = ValidateActions.addValidationError("targets cannot be empty", validationException);
             } else {
-                System.out.println("targets.size()" + targets.size());
                 if (targets.size() > MAX_TARGET_COUNT) {
                     validationException = ValidateActions.addValidationError(
                         "view cannot have more than " + MAX_TARGET_COUNT + " targets",

server/src/main/java/org/opensearch/action/admin/indices/view/ViewService.java

@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.TreeSet;
 import java.util.function.LongSupplier;
 import java.util.stream.Collectors;
 
@@ -51,7 +52,7 @@ public void createView(final CreateViewAction.Request request, final ActionListe
             .stream()
             .map(target -> new View.Target(target.getIndexPattern()))
             .collect(Collectors.toList());
-        final View view = new View(request.getName(), request.getDescription(), currentTime, currentTime, targets);
+        final View view = new View(request.getName(), request.getDescription(), currentTime, currentTime, new TreeSet<>(targets));
 
         createOrUpdateView(Operation.CreateView, view, listener);
     }
@@ -64,7 +65,13 @@ public void updateView(final CreateViewAction.Request request, final ActionListe
             .stream()
             .map(target -> new View.Target(target.getIndexPattern()))
             .collect(Collectors.toList());
-        final View updatedView = new View(request.getName(), request.getDescription(), originalView.getCreatedAt(), currentTime, targets);
+        final View updatedView = new View(
+            request.getName(),
+            request.getDescription(),
+            originalView.getCreatedAt(),
+            currentTime,
+            new TreeSet<>(targets)
+        );
 
         createOrUpdateView(Operation.UpdateView, updatedView, listener);
     }

server/src/main/java/org/opensearch/cluster/metadata/View.java

@@ -23,6 +23,10 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
 
 /** View of data in OpenSearch indices */
 @ExperimentalApi
@@ -32,18 +36,18 @@ public class View extends AbstractDiffable<View> implements ToXContentObject {
     private final String description;
     private final long createdAt;
     private final long modifiedAt;
-    private final List<Target> targets;
+    private final SortedSet<Target> targets;
 
-    public View(final String name, final String description, final Long createdAt, final Long modifiedAt, final List<Target> targets) {
+    public View(final String name, final String description, final Long createdAt, final Long modifiedAt, final Set<Target> targets) {
         this.name = Objects.requireNonNull(name, "Name must be provided");
         this.description = description;
         this.createdAt = createdAt != null ? createdAt : -1;
         this.modifiedAt = modifiedAt != null ? modifiedAt : -1;
-        this.targets = Objects.requireNonNull(targets, "Targets are required on a view");
+        this.targets = new TreeSet<>(Objects.requireNonNull(targets, "Targets are required on a view"));
     }
 
     public View(final StreamInput in) throws IOException {
-        this(in.readString(), in.readOptionalString(), in.readZLong(), in.readZLong(), in.readList(Target::new));
+        this(in.readString(), in.readOptionalString(), in.readZLong(), in.readZLong(), new TreeSet<>(in.readList(Target::new)));
     }
 
     public String getName() {
@@ -62,8 +66,8 @@ public long getModifiedAt() {
         return modifiedAt;
     }
 
-    public List<Target> getTargets() {
-        return targets;
+    public SortedSet<Target> getTargets() {
+        return new TreeSet<>(targets);
     }
 
     public static Diff<View> readDiffFrom(final StreamInput in) throws IOException {
@@ -89,7 +93,7 @@ public int hashCode() {
 
     /** The source of data used to project the view */
     @ExperimentalApi
-    public static class Target implements Writeable, ToXContentObject {
+    public static class Target implements Writeable, ToXContentObject, Comparable<Target> {
 
         private final String indexPattern;
 
@@ -144,6 +148,14 @@ public static Target fromXContent(final XContentParser parser) throws IOExceptio
         public void writeTo(final StreamOutput out) throws IOException {
             out.writeString(indexPattern);
         }
+
+        @Override
+        public int compareTo(final Target o) {
+            if (this == o) return 0;
+
+            final Target other = (Target) o;
+            return this.indexPattern.compareTo(other.indexPattern);
+        }
     }
 
     public static final ParseField NAME_FIELD = new ParseField("name");
@@ -155,7 +167,7 @@ public void writeTo(final StreamOutput out) throws IOException {
     @SuppressWarnings("unchecked")
     public static final ConstructingObjectParser<View, Void> PARSER = new ConstructingObjectParser<>(
         "view",
-        args -> new View((String) args[0], (String) args[1], (Long) args[2], (Long) args[3], (List<Target>) args[4])
+        args -> new View((String) args[0], (String) args[1], (Long) args[2], (Long) args[3], new TreeSet<>((List<Target>) args[4]))
     );
 
     static {
@@ -188,6 +200,6 @@ public void writeTo(final StreamOutput out) throws IOException {
         out.writeOptionalString(description);
         out.writeZLong(createdAt);
         out.writeZLong(modifiedAt);
-        out.writeList(targets);
+        out.writeList(targets.stream().collect(Collectors.toList()));
     }
 }

server/src/test/java/org/opensearch/action/admin/indices/view/ViewServiceTest.java

@@ -24,6 +24,7 @@
 
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.LongSupplier;
 
@@ -50,7 +51,7 @@ public class ViewServiceTest {
         "description " + randomAlphaOfLength(20),
         -1L,
         -1L,
-        List.of(typicalTarget)
+        Set.of(typicalTarget)
     );
 
     private ClusterService clusterService;