[Extensions] Adding versioning support for registering Rest actions (#…

…7302) Add cross version support for register Rest Actions for extensions Signed-off-by: Sarat Vemulapalli <vemulapallisarat@gmail.com>
opensearch-project · May 2, 2023 · 7c3c411 · 7c3c411
1 parent 1151308
commit 7c3c411
Show file tree

Hide file tree

Showing 8 changed files with 111 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -88,6 +88,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - [Extensions] Moving Extensions APIs to protobuf serialization. ([#6960](https://github.com/opensearch-project/OpenSearch/pull/6960))
 - [Extensions] Add IdentityPlugin into core to support Extension identities ([#7246](https://github.com/opensearch-project/OpenSearch/pull/7246))
 - Add descending order search optimization through reverse segment read. ([#7244](https://github.com/opensearch-project/OpenSearch/pull/7244))
+- [Extensions] Moving RestActions APIs to protobuf serialization. ([#7302](https://github.com/opensearch-project/OpenSearch/pull/7302))
 
 ### Dependencies
 - Bump `jackson` from 2.14.2 to 2.15.0 ([#7286](https://github.com/opensearch-project/OpenSearch/pull/7286)

diff --git a/server/build.gradle b/server/build.gradle
@@ -353,6 +353,10 @@ tasks.named("missingJavadoc").configure {
    */
   dependsOn("generateProto")
   javadocMissingIgnore = [
+    "org.opensearch.extensions.proto.ExtensionIdentityProto",
+    "org.opensearch.extensions.proto.ExtensionIdentityProto.ExtensionIdentityOrBuilder",
+    "org.opensearch.extensions.proto.RegisterRestActionsProto",
+    "org.opensearch.extensions.proto.RegisterRestActionsProto.RegisterRestActionsOrBuilder",
     "org.opensearch.extensions.proto.ExtensionRequestProto",
     "org.opensearch.extensions.proto.ExtensionRequestProto.ExtensionRequestOrBuilder",
     "org.opensearch.extensions.proto"

diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionRequest.java b/server/src/main/java/org/opensearch/extensions/ExtensionRequest.java
@@ -13,6 +13,7 @@
 import org.opensearch.common.Nullable;
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.common.io.stream.StreamOutput;
+import org.opensearch.extensions.proto.ExtensionIdentityProto;
 import org.opensearch.extensions.proto.ExtensionRequestProto;
 import org.opensearch.transport.TransportRequest;
 
@@ -35,7 +36,7 @@ public ExtensionRequest(ExtensionRequestProto.RequestType requestType) {
     public ExtensionRequest(ExtensionRequestProto.RequestType requestType, @Nullable String uniqueId) {
         ExtensionRequestProto.ExtensionRequest.Builder builder = ExtensionRequestProto.ExtensionRequest.newBuilder();
         if (uniqueId != null) {
-            builder.setUniqueId(uniqueId);
+            builder.setIdentity(ExtensionIdentityProto.ExtensionIdentity.newBuilder().setUniqueId(uniqueId).build());
         }
         this.request = builder.setRequestType(requestType).build();
     }
@@ -56,24 +57,28 @@ public ExtensionRequestProto.RequestType getRequestType() {
     }
 
     public String getUniqueId() {
-        return request.getUniqueId();
+        return request.getIdentity().getUniqueId();
     }
 
     public String toString() {
         return "ExtensionRequest{" + request.toString() + '}';
     }
 
+    public ExtensionIdentityProto.ExtensionIdentity getExtensionIdentity() {
+        return request.getIdentity();
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         ExtensionRequest that = (ExtensionRequest) o;
         return Objects.equals(request.getRequestType(), that.request.getRequestType())
-            && Objects.equals(request.getUniqueId(), that.request.getUniqueId());
+            && Objects.equals(request.getIdentity().getUniqueId(), that.request.getIdentity().getUniqueId());
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(request.getRequestType(), request.getUniqueId());
+        return Objects.hash(request.getRequestType(), request.getIdentity().getUniqueId());
     }
 }
diff --git a/server/src/main/java/org/opensearch/extensions/rest/RegisterRestActionsRequest.java b/server/src/main/java/org/opensearch/extensions/rest/RegisterRestActionsRequest.java
@@ -10,10 +10,11 @@
 
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.common.io.stream.StreamOutput;
+import org.opensearch.extensions.proto.ExtensionIdentityProto.ExtensionIdentity;
+import org.opensearch.extensions.proto.RegisterRestActionsProto.RegisterRestActions;
 import org.opensearch.transport.TransportRequest;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 
@@ -23,51 +24,48 @@
  * @opensearch.internal
  */
 public class RegisterRestActionsRequest extends TransportRequest {
-    private String uniqueId;
-    private List<String> restActions;
-    private List<String> deprecatedRestActions;
+    private final RegisterRestActions request;
 
     public RegisterRestActionsRequest(String uniqueId, List<String> restActions, List<String> deprecatedRestActions) {
-        this.uniqueId = uniqueId;
-        this.restActions = new ArrayList<>(restActions);
-        this.deprecatedRestActions = new ArrayList<>(deprecatedRestActions);
+        ExtensionIdentity identity = ExtensionIdentity.newBuilder().setUniqueId(uniqueId).build();
+        this.request = RegisterRestActions.newBuilder()
+            .setIdentity(identity)
+            .addAllRestActions(restActions)
+            .addAllDeprecatedRestActions(deprecatedRestActions)
+            .build();
     }
 
     public RegisterRestActionsRequest(StreamInput in) throws IOException {
         super(in);
-        uniqueId = in.readString();
-        restActions = in.readStringList();
-        deprecatedRestActions = in.readStringList();
+        request = RegisterRestActions.parseFrom(in.readByteArray());
     }
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
-        out.writeString(uniqueId);
-        out.writeStringCollection(restActions);
-        out.writeStringCollection(deprecatedRestActions);
+        out.writeByteArray(request.toByteArray());
     }
 
     public String getUniqueId() {
-        return uniqueId;
+        return request.getIdentity().getUniqueId();
     }
 
     public List<String> getRestActions() {
-        return List.copyOf(restActions);
+        return List.copyOf(request.getRestActionsList());
     }
 
     public List<String> getDeprecatedRestActions() {
-        return List.copyOf(deprecatedRestActions);
+        return List.copyOf(request.getDeprecatedRestActionsList());
     }
 
     @Override
     public String toString() {
-        return "RestActionsRequest{uniqueId="
-            + uniqueId
+        return "RestActionsRequest{Identity="
+            + request.getIdentity()
             + ", restActions="
-            + restActions
+            + request.getRestActionsList()
             + ", deprecatedRestActions="
-            + deprecatedRestActions
+            + request.getDeprecatedRestActionsList()
             + "}";
     }
 
@@ -76,13 +74,13 @@ public boolean equals(Object obj) {
         if (this == obj) return true;
         if (obj == null || getClass() != obj.getClass()) return false;
         RegisterRestActionsRequest that = (RegisterRestActionsRequest) obj;
-        return Objects.equals(uniqueId, that.uniqueId)
-            && Objects.equals(restActions, that.restActions)
-            && Objects.equals(deprecatedRestActions, that.deprecatedRestActions);
+        return Objects.equals(request.getIdentity().getUniqueId(), that.request.getIdentity().getUniqueId())
+            && Objects.equals(request.getRestActionsList(), that.request.getRestActionsList())
+            && Objects.equals(request.getDeprecatedRestActionsList(), that.request.getDeprecatedRestActionsList());
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(uniqueId, restActions, deprecatedRestActions);
+        return Objects.hash(request.getIdentity(), request.getRestActionsList(), request.getDeprecatedRestActionsList());
     }
 }
diff --git a/server/src/main/proto/extensions/ExtensionIdentityProto.proto b/server/src/main/proto/extensions/ExtensionIdentityProto.proto
@@ -0,0 +1,19 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+syntax = "proto3";
+package org.opensearch.extensions.proto;
+
+option java_outer_classname = "ExtensionIdentityProto";
+
+message ExtensionIdentity {
+  string uniqueId = 1;
+}
diff --git a/server/src/main/proto/extensions/ExtensionRequestProto.proto b/server/src/main/proto/extensions/ExtensionRequestProto.proto
@@ -12,6 +12,7 @@
 syntax = "proto3";
 package org.opensearch.extensions.proto;
 
+import "extensions/ExtensionIdentityProto.proto";
 option java_outer_classname = "ExtensionRequestProto";
 
 enum RequestType {
@@ -27,6 +28,6 @@ enum RequestType {
 }
 
 message ExtensionRequest {
-  RequestType requestType = 1;
-  string uniqueId = 2;
+  ExtensionIdentity identity = 1;
+  RequestType requestType = 2;
 }
diff --git a/server/src/main/proto/extensions/RegisterRestActionsProto.proto b/server/src/main/proto/extensions/RegisterRestActionsProto.proto
@@ -0,0 +1,22 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+syntax = "proto3";
+package org.opensearch.extensions.proto;
+
+import "extensions/ExtensionIdentityProto.proto";
+option java_outer_classname = "RegisterRestActionsProto";
+
+message RegisterRestActions {
+  ExtensionIdentity identity = 1;
+  repeated string restActions = 2;
+  repeated string deprecatedRestActions = 3;
+}
diff --git a/server/src/test/java/org/opensearch/extensions/rest/RegisterRestActionsTests.java b/server/src/test/java/org/opensearch/extensions/rest/RegisterRestActionsTests.java
@@ -47,4 +47,35 @@ public void testRegisterRestActionsRequest() throws Exception {
             }
         }
     }
+
+    public void testNoIdentityRestActionsRequest() {
+        String uniqueId = null;
+        List<String> expected = List.of("GET /foo", "PUT /bar", "POST /baz");
+        List<String> expectedDeprecated = List.of("GET /deprecated/foo", "It's deprecated");
+        // Expect exception as Extension Identity(uniqueId) is null
+        expectThrows(NullPointerException.class, () -> new RegisterRestActionsRequest(uniqueId, expected, expectedDeprecated));
+    }
+
+    public void testNoRestActionsRequest() {
+        String uniqueId = "extension-1234";
+        List<String> expected = null;
+        List<String> expectedDeprecated = null;
+        // Expect exception as Extension Identity(uniqueId) is null
+        expectThrows(NullPointerException.class, () -> new RegisterRestActionsRequest(uniqueId, expected, expectedDeprecated));
+    }
+
+    public void testEmptyRestActionsRequest() {
+        String uniqueId = "extension-1234";
+        List<String> expected = List.of();
+        List<String> expectedDeprecated = List.of();
+        RegisterRestActionsRequest request = new RegisterRestActionsRequest(uniqueId, expected, expectedDeprecated);
+
+        assertEquals(uniqueId, request.getUniqueId());
+        assertEquals(List.of(), request.getRestActions());
+        assertEquals(List.of(), request.getDeprecatedRestActions());
+        assertEquals(
+            "RestActionsRequest{Identity=uniqueId: \"extension-1234\"\n" + ", restActions=[], deprecatedRestActions=[]}",
+            request.toString()
+        );
+    }
 }