Adding AbstractListAction and renaming pagination related classes

Signed-off-by: Harsh Garg <gkharsh@amazon.com>
gargharsh3134 · Sep 20, 2024 · 9aebdce · 9aebdce
1 parent 3bedc82
commit 9aebdce
Show file tree

Hide file tree

Showing 33 changed files with 425 additions and 402 deletions.
diff --git a/...mples/rest-handler/src/main/java/org/opensearch/example/resthandler/ExampleCatAction.java b/...mples/rest-handler/src/main/java/org/opensearch/example/resthandler/ExampleCatAction.java
@@ -83,7 +83,7 @@ protected RestChannelConsumer doCatRequest(final RestRequest request, final Node
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append(documentation());
     }
 

diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java
@@ -459,6 +459,7 @@
 import org.opensearch.rest.action.ingest.RestGetPipelineAction;
 import org.opensearch.rest.action.ingest.RestPutPipelineAction;
 import org.opensearch.rest.action.ingest.RestSimulatePipelineAction;
+import org.opensearch.rest.action.list.AbstractListAction;
 import org.opensearch.rest.action.list.RestIndicesListAction;
 import org.opensearch.rest.action.list.RestListAction;
 import org.opensearch.rest.action.search.RestClearScrollAction;
@@ -801,11 +802,11 @@ private ActionFilters setupActionFilters(List<ActionPlugin> actionPlugins) {
 
     public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
         List<AbstractCatAction> catActions = new ArrayList<>();
-        List<AbstractCatAction> listActions = new ArrayList<>();
+        List<AbstractListAction> listActions = new ArrayList<>();
         Consumer<RestHandler> registerHandler = handler -> {
             if (handler instanceof AbstractCatAction) {
-                if (((AbstractCatAction) handler).isActionPaginated()) {
-                    listActions.add((AbstractCatAction) handler);
+                if (handler instanceof AbstractListAction && ((AbstractListAction) handler).isActionPaginated()) {
+                    listActions.add((AbstractListAction) handler);
                 } else {
                     catActions.add((AbstractCatAction) handler);
                 }

diff --git a/server/src/main/java/org/opensearch/common/Table.java b/server/src/main/java/org/opensearch/common/Table.java
@@ -34,7 +34,7 @@
 
 import org.opensearch.common.time.DateFormatter;
 import org.opensearch.core.common.Strings;
-import org.opensearch.rest.pagination.PaginatedQueryResponse;
+import org.opensearch.rest.pagination.PageToken;
 
 import java.time.Instant;
 import java.time.ZoneOffset;
@@ -63,14 +63,14 @@ public class Table {
     /**
      * paginatedQueryResponse if null will imply the Table response is not paginated.
      */
-    private PaginatedQueryResponse paginatedQueryResponse;
+    private PageToken pageToken;
     public static final String EPOCH = "epoch";
     public static final String TIMESTAMP = "timestamp";
 
     public Table() {}
 
-    public Table(@Nullable PaginatedQueryResponse paginatedQueryResponse) {
-        this.paginatedQueryResponse = paginatedQueryResponse;
+    public Table(@Nullable PageToken pageToken) {
+        this.pageToken = pageToken;
     }
 
     public Table startHeaders() {
@@ -241,8 +241,8 @@ public Map<String, String> getAliasMap() {
         return headerAliasMap;
     }
 
-    public PaginatedQueryResponse getPaginatedQueryResponse() {
-        return paginatedQueryResponse;
+    public PageToken getPageToken() {
+        return pageToken;
     }
 
     /**

diff --git a/server/src/main/java/org/opensearch/rest/RestRequest.java b/server/src/main/java/org/opensearch/rest/RestRequest.java
@@ -51,7 +51,7 @@
 import org.opensearch.core.xcontent.XContentParser;
 import org.opensearch.http.HttpChannel;
 import org.opensearch.http.HttpRequest;
-import org.opensearch.rest.pagination.PaginatedQueryRequest;
+import org.opensearch.rest.pagination.PageParams;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -68,9 +68,9 @@
 
 import static org.opensearch.common.unit.TimeValue.parseTimeValue;
 import static org.opensearch.core.common.unit.ByteSizeValue.parseBytesSizeValue;
-import static org.opensearch.rest.pagination.PaginatedQueryRequest.PAGINATED_QUERY_PARAM_NEXT_TOKEN_KEY;
-import static org.opensearch.rest.pagination.PaginatedQueryRequest.PAGINATED_QUERY_PARAM_SIZE_KEY;
-import static org.opensearch.rest.pagination.PaginatedQueryRequest.PAGINATED_QUERY_PARAM_SORT_KEY;
+import static org.opensearch.rest.pagination.PageParams.PARAM_NEXT_TOKEN;
+import static org.opensearch.rest.pagination.PageParams.PARAM_SIZE;
+import static org.opensearch.rest.pagination.PageParams.PARAM_SORT;
 
 /**
  * REST Request
@@ -595,12 +595,8 @@ public static MediaType parseContentType(List<String> header) {
         throw new IllegalArgumentException("empty Content-Type header");
     }
 
-    public PaginatedQueryRequest parsePaginatedQueryParams(String defaultSortOrder, int defaultPageSize) {
-        return new PaginatedQueryRequest(
-            param(PAGINATED_QUERY_PARAM_NEXT_TOKEN_KEY),
-            param(PAGINATED_QUERY_PARAM_SORT_KEY, defaultSortOrder),
-            paramAsInt(PAGINATED_QUERY_PARAM_SIZE_KEY, defaultPageSize)
-        );
+    public PageParams parsePaginatedQueryParams(String defaultSortOrder, int defaultPageSize) {
+        return new PageParams(param(PARAM_NEXT_TOKEN), param(PARAM_SORT, defaultSortOrder), paramAsInt(PARAM_SIZE, defaultPageSize));
     }
 
     /**

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/AbstractCatAction.java b/server/src/main/java/org/opensearch/rest/action/cat/AbstractCatAction.java
@@ -40,13 +40,11 @@
 import org.opensearch.rest.BaseRestHandler;
 import org.opensearch.rest.BytesRestResponse;
 import org.opensearch.rest.RestRequest;
-import org.opensearch.rest.pagination.PaginatedQueryRequest;
 
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.Objects;
 import java.util.Set;
 
 import static org.opensearch.rest.action.cat.RestTable.buildHelpWidths;
@@ -59,11 +57,9 @@
  */
 public abstract class AbstractCatAction extends BaseRestHandler {
 
-    protected PaginatedQueryRequest paginatedQueryRequest;
-
     protected abstract RestChannelConsumer doCatRequest(RestRequest request, NodeClient client);
 
-    public abstract void documentation(StringBuilder sb);
+    protected abstract void documentation(StringBuilder sb);
 
     protected abstract Table getTableWithHeader(RestRequest request);
 
@@ -89,10 +85,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
                 channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, bytesOutput.bytes()));
             };
         } else {
-            if (isActionPaginated()) {
-                this.paginatedQueryRequest = validateAndGetPaginationMetadata(request);
-                assert Objects.nonNull(paginatedQueryRequest) : "paginatedQueryRequest can not be null for paginated queries";
-            }
             return doCatRequest(request, client);
         }
     }
@@ -106,23 +98,4 @@ protected Set<String> responseParams() {
         return RESPONSE_PARAMS;
     }
 
-    /**
-     *
-     * @return boolean denoting whether the RestAction will output paginated responses or not.
-     * Is kept false by default, every paginated action to override and return true.
-     */
-    public boolean isActionPaginated() {
-        return false;
-    }
-
-    /**
-     *
-     * @return Metadata that can be extracted out from the rest request. Each paginated action to override and provide
-     * its own implementation. Query params supported by the action specific to pagination along with the respective validations,
-     * should be added here.
-     */
-    protected PaginatedQueryRequest validateAndGetPaginationMetadata(RestRequest restRequest) {
-        return null;
-    }
-
 }
diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestAliasAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestAliasAction.java
@@ -89,7 +89,7 @@ public RestResponse buildResponse(GetAliasesResponse response) throws Exception
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/aliases\n");
         sb.append("/_cat/aliases/{alias}\n");
     }

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestAllocationAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestAllocationAction.java
@@ -78,7 +78,7 @@ public String getName() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/allocation\n");
     }
 

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestCatRecoveryAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestCatRecoveryAction.java
@@ -77,7 +77,7 @@ public String getName() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/recovery\n");
         sb.append("/_cat/recovery/{index}\n");
     }

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestCatSegmentReplicationAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestCatSegmentReplicationAction.java
@@ -58,7 +58,7 @@ public String getName() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/segment_replication\n");
         sb.append("/_cat/segment_replication/{index}\n");
     }

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestClusterManagerAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestClusterManagerAction.java
@@ -69,7 +69,7 @@ public String getName() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/cluster_manager\n");
     }
 

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestCountAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestCountAction.java
@@ -71,7 +71,7 @@ public String getName() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/count\n");
         sb.append("/_cat/count/{index}\n");
     }

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestFielddataAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestFielddataAction.java
@@ -82,7 +82,7 @@ public RestResponse buildResponse(NodesStatsResponse nodeStatses) throws Excepti
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/fielddata\n");
         sb.append("/_cat/fielddata/{fields}\n");
     }

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestHealthAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestHealthAction.java
@@ -69,7 +69,7 @@ public boolean allowSystemIndexAccessByDefault() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/health\n");
     }
 

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java
@@ -61,8 +61,9 @@
 import org.opensearch.rest.RestRequest;
 import org.opensearch.rest.RestResponse;
 import org.opensearch.rest.action.RestResponseListener;
-import org.opensearch.rest.pagination.IndexBasedPaginationStrategy;
-import org.opensearch.rest.pagination.PaginatedQueryResponse;
+import org.opensearch.rest.action.list.AbstractListAction;
+import org.opensearch.rest.pagination.IndexPaginationStrategy;
+import org.opensearch.rest.pagination.PageToken;
 
 import java.time.Instant;
 import java.time.ZoneOffset;
@@ -89,7 +90,7 @@
  *
  * @opensearch.api
  */
-public class RestIndicesAction extends AbstractCatAction {
+public class RestIndicesAction extends AbstractListAction {
 
     private static final DateFormatter STRICT_DATE_TIME_FORMATTER = DateFormatter.forPattern("strict_date_time");
     private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestIndicesAction.class);
@@ -114,7 +115,7 @@ public boolean allowSystemIndexAccessByDefault() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/indices\n");
         sb.append("/_cat/indices/{index}\n");
     }
@@ -177,14 +178,14 @@ public void onResponse(final GetSettingsResponse getSettingsResponse) {
                             new ActionListener<ClusterStateResponse>() {
                                 @Override
                                 public void onResponse(ClusterStateResponse clusterStateResponse) {
-                                    IndexBasedPaginationStrategy paginationStrategy = getPaginationStrategy(clusterStateResponse);
+                                    IndexPaginationStrategy paginationStrategy = getPaginationStrategy(clusterStateResponse);
                                     final String[] indicesToBeQueried = getIndicesToBeQueried(indices, paginationStrategy);
                                     final GroupedActionListener<ActionResponse> groupedListener = createGroupedListener(
                                         request,
                                         4,
                                         listener,
                                         indicesToBeQueried,
-                                        getPaginatedQueryResponse(paginationStrategy)
+                                        getPageToken(paginationStrategy)
                                     );
                                     groupedListener.onResponse(getSettingsResponse);
                                     groupedListener.onResponse(clusterStateResponse);
@@ -309,7 +310,7 @@ private GroupedActionListener<ActionResponse> createGroupedListener(
         final int size,
         final ActionListener<Table> listener,
         final String[] indicesToBeQueried,
-        final PaginatedQueryResponse paginatedQueryResponse
+        final PageToken pageToken
     ) {
         return new GroupedActionListener<>(new ActionListener<Collection<ActionResponse>>() {
             @Override
@@ -340,7 +341,7 @@ public void onResponse(final Collection<ActionResponse> responses) {
                         indicesStats,
                         indicesStates,
                         indicesToBeQueried,
-                        paginatedQueryResponse
+                        pageToken
                     );
                     listener.onResponse(responseTable);
                 } catch (Exception e) {
@@ -373,8 +374,8 @@ protected Table getTableWithHeader(final RestRequest request) {
         return getTableWithHeader(request, null);
     }
 
-    protected Table getTableWithHeader(final RestRequest request, final PaginatedQueryResponse paginatedQueryResponse) {
-        Table table = new Table(paginatedQueryResponse);
+    protected Table getTableWithHeader(final RestRequest request, final PageToken pageToken) {
+        Table table = new Table(pageToken);
         table.startHeaders();
         table.addCell("health", "alias:h;desc:current health status");
         table.addCell("status", "alias:s;desc:open/close status");
@@ -745,10 +746,10 @@ protected Table buildTable(
         final Map<String, IndexStats> indicesStats,
         final Map<String, IndexMetadata> indicesMetadatas,
         final String[] indicesToBeQueried,
-        final PaginatedQueryResponse paginatedQueryResponse
+        final PageToken pageToken
     ) {
         final String healthParam = request.param("health");
-        final Table table = getTableWithHeader(request, paginatedQueryResponse);
+        final Table table = getTableWithHeader(request, pageToken);
 
         indicesSettings.forEach((indexName, settings) -> {
             if (indicesMetadatas.containsKey(indexName) == false) {
@@ -1039,15 +1040,15 @@ private static <A extends ActionResponse> A extractResponse(final Collection<? e
         return (A) responses.stream().filter(c::isInstance).findFirst().get();
     }
 
-    protected IndexBasedPaginationStrategy getPaginationStrategy(ClusterStateResponse clusterStateResponse) {
+    protected IndexPaginationStrategy getPaginationStrategy(ClusterStateResponse clusterStateResponse) {
         return null;
     }
 
-    protected PaginatedQueryResponse getPaginatedQueryResponse(IndexBasedPaginationStrategy paginationStrategy) {
+    protected PageToken getPageToken(IndexPaginationStrategy paginationStrategy) {
         return null;
     }
 
-    protected String[] getIndicesToBeQueried(String[] indices, IndexBasedPaginationStrategy paginationStrategy) {
+    protected String[] getIndicesToBeQueried(String[] indices, IndexPaginationStrategy paginationStrategy) {
         return indices;
     }
 

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodeAttrsAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodeAttrsAction.java
@@ -75,7 +75,7 @@ public String getName() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/nodeattrs\n");
     }
 

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java
@@ -103,7 +103,7 @@ public String getName() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/nodes\n");
     }
 

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestPendingClusterTasksAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestPendingClusterTasksAction.java
@@ -67,7 +67,7 @@ public String getName() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/pending_tasks\n");
     }
 

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestPitSegmentsAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestPitSegmentsAction.java
@@ -88,7 +88,7 @@ public RestResponse buildResponse(final IndicesSegmentResponse indicesSegmentRes
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/pit_segments\n");
         sb.append("/_cat/pit_segments/{pit_id}\n");
     }

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestPluginsAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestPluginsAction.java
@@ -74,7 +74,7 @@ public String getName() {
     }
 
     @Override
-    public void documentation(StringBuilder sb) {
+    protected void documentation(StringBuilder sb) {
         sb.append("/_cat/plugins\n");
     }