chore: Improve paged request handling

fortify · Oct 18, 2023 · b1a3d12 · b1a3d12
1 parent 910ea6e
commit b1a3d12
Show file tree

Hide file tree

Showing 21 changed files with 216 additions and 63 deletions.
diff --git a/...mmon/src/main/java/com/fortify/cli/common/output/cli/mixin/AbstractOutputHelperMixin.java b/...mmon/src/main/java/com/fortify/cli/common/output/cli/mixin/AbstractOutputHelperMixin.java
@@ -34,12 +34,16 @@
 import com.fortify.cli.common.output.writer.output.IOutputWriter;
 import com.fortify.cli.common.output.writer.output.IOutputWriterFactory;
 import com.fortify.cli.common.output.writer.output.standard.StandardOutputConfig;
+import com.fortify.cli.common.rest.paging.INextPageRequestProducer;
 import com.fortify.cli.common.rest.paging.INextPageUrlProducer;
 import com.fortify.cli.common.rest.paging.INextPageUrlProducerSupplier;
+import com.fortify.cli.common.rest.paging.PagingHelper;
 import com.fortify.cli.common.rest.unirest.IHttpRequestUpdater;
+import com.fortify.cli.common.rest.unirest.IUnirestInstanceSupplier;
 import com.fortify.cli.common.util.JavaHelper;
 
 import kong.unirest.HttpRequest;
+import kong.unirest.UnirestInstance;
 import picocli.CommandLine.Mixin;
 
 public abstract class AbstractOutputHelperMixin implements IOutputHelper {
@@ -64,8 +68,12 @@ public IProductHelper getProductHelper() {
     @Override
     public final void write(HttpRequest<?> baseRequest) {
         HttpRequest<?> request = updateRequest(baseRequest);
-        INextPageUrlProducer nextPageUrlProducer = getNextPageUrlProducer(request);
-        createOutputWriter().write(request, nextPageUrlProducer);
+        INextPageRequestProducer nextPageRequestProducer = getNextPageRequestProducer();
+        if ( nextPageRequestProducer!=null ) {
+            createOutputWriter().write(request, nextPageRequestProducer);
+        } else {
+            createOutputWriter().write(request, getNextPageUrlProducer());
+        }
     }
 
     /**
@@ -108,16 +116,33 @@ protected final HttpRequest<?> updateRequest(HttpRequest<?> request) {
         return request;
     }
 
+    protected final INextPageRequestProducer getNextPageRequestProducer() {
+        return PagingHelper.asNextPageRequestProducer(getUnirestInstance(), getNextPageUrlProducer());
+    }
+
     /**
      * This method returns a next page url producer retrieved from either the command
      * being invoked, or the configured {@link IProductHelper}, in this order, if
      * they implement the {@link INextPageUrlProducerSupplier} interface.
      * @param request
      * @return
      */
-    protected final INextPageUrlProducer getNextPageUrlProducer(HttpRequest<?> request) {
+    protected final INextPageUrlProducer getNextPageUrlProducer() {
         return Stream.of(commandHelper.getCommand(), getProductHelper())
-                .map(obj->getNextPageUrlProducerFromObject(obj, request))
+                .map(AbstractOutputHelperMixin::getNextPageUrlProducerFromObject)
+                .filter(Objects::nonNull)
+                .findFirst().orElse(null);
+    }
+
+    /**
+     * This method returns a UnirestInstance retrieved from the command being invoked, 
+     * if it implements the {@link IUnirestInstanceSupplier} interface.
+     * @param request
+     * @return
+     */
+    protected final UnirestInstance getUnirestInstance() {
+        return Stream.of(commandHelper.getCommand())
+                .map(AbstractOutputHelperMixin::getUnirestInstanceFromObject)
                 .filter(Objects::nonNull)
                 .findFirst().orElse(null);
     }
@@ -177,15 +202,25 @@ private static final Function<IHttpRequestUpdater, HttpRequest<?>> httpRequestUp
     }
 
     /**
-     * Utility method used by {@link #getNextPageUrlProducer(HttpRequest)}, returning a
+     * Utility method used by {@link #getNextPageUrlProducer()}, returning a
      * next page producer retrieved from the given object if that object implements 
      * {@link INextPageUrlProducerSupplier}, or null otherwise.
      * @param obj
-     * @param request
      * @return
      */
-    private static final INextPageUrlProducer getNextPageUrlProducerFromObject(Object obj, final HttpRequest<?> request) {
-        return apply(obj, INextPageUrlProducerSupplier.class, supplier->supplier.getNextPageUrlProducer(request));
+    private static final INextPageUrlProducer getNextPageUrlProducerFromObject(Object obj) {
+        return apply(obj, INextPageUrlProducerSupplier.class, supplier->supplier.getNextPageUrlProducer());
+    }
+
+    /**
+     * Utility method used by {@link #getUnirestInstance()}, returning a UnirestInstance
+     * retrieved from the given object if that object implements {@link IUnirestInstanceSupplier}, 
+     * or null otherwise.
+     * @param obj
+     * @return
+     */
+    private static final UnirestInstance getUnirestInstanceFromObject(Object obj) {
+        return apply(obj, IUnirestInstanceSupplier.class, supplier->supplier.getUnirestInstance());
     }
 
     /**

diff --git a/.../fcli-common/src/main/java/com/fortify/cli/common/output/writer/output/IOutputWriter.java b/.../fcli-common/src/main/java/com/fortify/cli/common/output/writer/output/IOutputWriter.java
@@ -13,6 +13,7 @@
 package com.fortify.cli.common.output.writer.output;
 
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fortify.cli.common.rest.paging.INextPageRequestProducer;
 import com.fortify.cli.common.rest.paging.INextPageUrlProducer;
 
 import kong.unirest.HttpRequest;
@@ -24,6 +25,8 @@ public interface IOutputWriter {
 
     void write(HttpRequest<?> httpRequest);
 
+    void write(HttpRequest<?> request, INextPageRequestProducer nextPageRequestProducer);
+
     void write(HttpRequest<?> httpRequest, INextPageUrlProducer nextPageUrlProducer);
 
     void write(HttpResponse<JsonNode> httpResponse);

diff --git a/.../main/java/com/fortify/cli/common/output/writer/output/standard/StandardOutputWriter.java b/.../main/java/com/fortify/cli/common/output/writer/output/standard/StandardOutputWriter.java
@@ -37,6 +37,7 @@
 import com.fortify.cli.common.output.writer.record.IRecordWriterFactory;
 import com.fortify.cli.common.output.writer.record.RecordWriterConfig;
 import com.fortify.cli.common.output.writer.record.RecordWriterConfig.RecordWriterConfigBuilder;
+import com.fortify.cli.common.rest.paging.INextPageRequestProducer;
 import com.fortify.cli.common.rest.paging.INextPageUrlProducer;
 import com.fortify.cli.common.rest.paging.PagingHelper;
 import com.fortify.cli.common.rest.unirest.IfFailureHandler;
@@ -86,7 +87,9 @@ public void write(JsonNode jsonNode) {
      */
     @Override
     public void write(HttpRequest<?> httpRequest) {
-        write(httpRequest, null);
+        try ( IRecordWriter recordWriter = new OutputAndVariableRecordWriter() ) {
+            writeRecords(recordWriter, httpRequest);
+        }
     }
 
     /**
@@ -105,6 +108,22 @@ public void write(HttpRequest<?> httpRequest, INextPageUrlProducer nextPageUrlPr
         }
     }
 
+    /**
+     * Write the output of the given, potentially paged {@link HttpRequest}, to the 
+     * configured output(s), invoking the given {@link INextPageRequestProducer} to retrieve 
+     * all pages
+     */
+    @Override
+    public void write(HttpRequest<?> httpRequest, INextPageRequestProducer nextPageRequestProducer) {
+        try ( IRecordWriter recordWriter = new OutputAndVariableRecordWriter() ) {
+            if ( nextPageRequestProducer==null ) {
+                writeRecords(recordWriter, httpRequest);
+            } else {
+                writeRecords(recordWriter, httpRequest, nextPageRequestProducer);
+            }
+        }
+    }
+
     /** 
      * Write the given {@link HttpResponse} to the configured output(s)
      */
@@ -140,6 +159,22 @@ private final void writeRecords(IRecordWriter recordWriter, HttpRequest<?> httpR
             .ifSuccess(r->writeRecords(recordWriter, r))
             .ifFailure(IfFailureHandler::handle); // Just in case no error interceptor was registered for this request
     }
+
+    /**
+     * Write records returned by the given, potentially paged {@link HttpRequest}
+     * to the given {@link IRecordWriter}, invoking the given {@link INextPageRequestProducer} 
+     * to retrieve all pages
+     * @param recordWriter
+     * @param httpRequest
+     * @param nextPageRequestProducer
+     */
+    private final void writeRecords(IRecordWriter recordWriter, HttpRequest<?> httpRequest, INextPageRequestProducer nextPageRequestProducer) {
+        while ( httpRequest!=null ) {
+            HttpResponse<JsonNode> response = httpRequest.asObject(JsonNode.class);
+            writeRecords(recordWriter, response); 
+            httpRequest = nextPageRequestProducer.getNextPageRequest(httpRequest, response);
+        }
+    }
 
     /**
      * Write records provided by the given {@link HttpResponse} to the given

diff --git a/...cli-common/src/main/java/com/fortify/cli/common/rest/cli/cmd/AbstractRestCallCommand.java b/...cli-common/src/main/java/com/fortify/cli/common/rest/cli/cmd/AbstractRestCallCommand.java
@@ -108,17 +108,17 @@ public final JsonNode transformRecord(JsonNode input) {
     }
 
     @Override
-    public final INextPageUrlProducer getNextPageUrlProducer(HttpRequest<?> originalRequest) {
+    public final INextPageUrlProducer getNextPageUrlProducer() {
         INextPageUrlProducer result = null;
         if ( !noPaging ) {
-            result = _getNextPageUrlProducer(originalRequest);
+            result = _getNextPageUrlProducer();
         }
         return result;
     }
 
     protected abstract JsonNode _transformRecord(JsonNode input);
     protected abstract JsonNode _transformInput(JsonNode input);
-    protected abstract INextPageUrlProducer _getNextPageUrlProducer(HttpRequest<?> originalRequest);
+    protected abstract INextPageUrlProducer _getNextPageUrlProducer();
 
     @SneakyThrows
     protected final HttpRequest<?> prepareRequest(UnirestInstance unirest) {

diff --git a/...cli-common/src/main/java/com/fortify/cli/common/rest/paging/INextPageRequestProducer.java b/...cli-common/src/main/java/com/fortify/cli/common/rest/paging/INextPageRequestProducer.java
@@ -0,0 +1,22 @@
+/*******************************************************************************
+ * Copyright 2021, 2023 Open Text.
+ *
+ * The only warranties for products and services of Open Text 
+ * and its affiliates and licensors ("Open Text") are as may 
+ * be set forth in the express warranty statements accompanying 
+ * such products and services. Nothing herein should be construed 
+ * as constituting an additional warranty. Open Text shall not be 
+ * liable for technical or editorial errors or omissions contained 
+ * herein. The information contained herein is subject to change 
+ * without notice.
+ *******************************************************************************/
+package com.fortify.cli.common.rest.paging;
+
+import com.fasterxml.jackson.databind.JsonNode;
+
+import kong.unirest.HttpRequest;
+import kong.unirest.HttpResponse;
+
+public interface INextPageRequestProducer {
+    HttpRequest<?> getNextPageRequest(HttpRequest<?> previousRequest, HttpResponse<? extends JsonNode> jsonResponse);
+}
diff --git a/...on/src/main/java/com/fortify/cli/common/rest/paging/INextPageRequestProducerSupplier.java b/...on/src/main/java/com/fortify/cli/common/rest/paging/INextPageRequestProducerSupplier.java
@@ -0,0 +1,17 @@
+/*******************************************************************************
+ * Copyright 2021, 2023 Open Text.
+ *
+ * The only warranties for products and services of Open Text 
+ * and its affiliates and licensors ("Open Text") are as may 
+ * be set forth in the express warranty statements accompanying 
+ * such products and services. Nothing herein should be construed 
+ * as constituting an additional warranty. Open Text shall not be 
+ * liable for technical or editorial errors or omissions contained 
+ * herein. The information contained herein is subject to change 
+ * without notice.
+ *******************************************************************************/
+package com.fortify.cli.common.rest.paging;
+
+public interface INextPageRequestProducerSupplier {
+    INextPageRequestProducer getNextPageRequestProducer();
+}
diff --git a/...re/fcli-common/src/main/java/com/fortify/cli/common/rest/paging/INextPageUrlProducer.java b/...re/fcli-common/src/main/java/com/fortify/cli/common/rest/paging/INextPageUrlProducer.java
@@ -14,8 +14,9 @@
 
 import com.fasterxml.jackson.databind.JsonNode;
 
+import kong.unirest.HttpRequest;
 import kong.unirest.HttpResponse;
 
 public interface INextPageUrlProducer {
-    String getNextPageUrl(HttpResponse<? extends JsonNode> jsonResponse);
+    String getNextPageUrl(HttpRequest<?> previousRequest, HttpResponse<? extends JsonNode> jsonResponse);
 }
diff --git a/...common/src/main/java/com/fortify/cli/common/rest/paging/INextPageUrlProducerSupplier.java b/...common/src/main/java/com/fortify/cli/common/rest/paging/INextPageUrlProducerSupplier.java
@@ -12,8 +12,6 @@
  *******************************************************************************/
 package com.fortify.cli.common.rest.paging;
 
-import kong.unirest.HttpRequest;
-
 public interface INextPageUrlProducerSupplier {
-    INextPageUrlProducer getNextPageUrlProducer(HttpRequest<?> originalRequest);
+    INextPageUrlProducer getNextPageUrlProducer();
 }
diff --git a/...rc/main/java/com/fortify/cli/common/rest/paging/LinkHeaderNextPageUrlProducerFactory.java b/...rc/main/java/com/fortify/cli/common/rest/paging/LinkHeaderNextPageUrlProducerFactory.java
@@ -19,8 +19,8 @@ public class LinkHeaderNextPageUrlProducerFactory {
     private static final Pattern linkHeaderPattern = Pattern.compile("<([^>]*)>; *rel=\"([^\"]*)\"");
 
     public static final INextPageUrlProducer nextPageUrlProducer(String headerName, String relName) {
-        return r -> {
-            String linkHeader = r.getHeaders().getFirst(headerName);
+        return (req,resp) -> {
+            String linkHeader = resp.getHeaders().getFirst(headerName);
             Optional<String> nextLink = linkHeaderPattern.matcher(linkHeader).results()
                 .filter(r1->relName.equals(r1.group(2)))
                 .findFirst()

diff --git a/fcli-core/fcli-common/src/main/java/com/fortify/cli/common/rest/paging/PagingHelper.java b/fcli-core/fcli-common/src/main/java/com/fortify/cli/common/rest/paging/PagingHelper.java
@@ -14,16 +14,76 @@
 
 import com.fasterxml.jackson.databind.JsonNode;
 
+import kong.unirest.Header;
 import kong.unirest.HttpRequest;
+import kong.unirest.HttpResponse;
 import kong.unirest.PagedList;
+import kong.unirest.UnirestInstance;
+import lombok.RequiredArgsConstructor;
 
 public class PagingHelper {
+    /**
+     * Return a Unirest {@link PagedList} based on the given base request and {@link INextPageUrlProducer}.
+     * Note that Unirest first collects all responses in memory before returning the {@link PagedList}. To 
+     * (potentially) reduce memory usage and allow for results to be processed immediately after each page 
+     * has been loaded (for example for streaming output), it may be better to handle paging manually using
+     * {@link INextPageRequestProducer}. An {@link INextPageUrlProducer} instance can be converted to an
+     * {@link INextPageRequestProducer} instance using the {@link #asNextPageRequestProducer(UnirestInstance, INextPageUrlProducer)}
+     * method. 
+     * @param request
+     * @param nextPageUrlProducer
+     * @return
+     */
+    @SuppressWarnings("unchecked") // TODO Can we get rid of these warnings in a better way?
+    public static final <R extends JsonNode> PagedList<R> pagedRequest(HttpRequest<?> request, INextPageUrlProducer nextPageUrlProducer, Class<R> returnType) {
+        return request.asPaged(r->r.asObject(returnType), response->nextPageUrlProducer.getNextPageUrl(request, response));
+    }
+
+    /**
+     * Same as {@link #pagedRequest(HttpRequest, INextPageUrlProducer, Class)} (same considerations apply),
+     * but with fixed {@link JsonNode}-based return type. 
+     * @param request
+     * @param nextPageUrlProducer
+     * @return
+     */
     public static final PagedList<JsonNode> pagedRequest(HttpRequest<?> request, INextPageUrlProducer nextPageUrlProducer) {
         return pagedRequest(request, nextPageUrlProducer, JsonNode.class);
     }
 
-    @SuppressWarnings("unchecked") // TODO Can we get rid of these warnings in a better way?
-    public static final <R extends JsonNode> PagedList<R> pagedRequest(HttpRequest<?> request, INextPageUrlProducer nextPageUrlProducer, Class<R> returnType) {
-        return request.asPaged(r->r.asObject(returnType), nextPageUrlProducer::getNextPageUrl);
+    /**
+     * Return an {@link INextPageRequestProducer} instance based on the given {@link INextPageUrlProducer},
+     * using the given {@link UnirestInstance} to produce requests for loading next pages.    
+     * @param unirest
+     * @param nextPageUrlProducer
+     * @return
+     */
+    public static final INextPageRequestProducer asNextPageRequestProducer(UnirestInstance unirest, INextPageUrlProducer nextPageUrlProducer) {
+        return unirest==null || nextPageUrlProducer==null ? null : new NextPageRequestProducer(unirest, nextPageUrlProducer);
+    }
+
+    @RequiredArgsConstructor
+    private static final class NextPageRequestProducer implements INextPageRequestProducer {
+        private final UnirestInstance unirest;
+        private final INextPageUrlProducer nextPageUrlProducer;
+
+        @Override
+        public HttpRequest<?> getNextPageRequest(HttpRequest<?> request, HttpResponse<? extends JsonNode> jsonResponse) {
+            var nextPageUrl = nextPageUrlProducer.getNextPageUrl(request, jsonResponse);
+            // TODO Any more request attributes to be copied from original request?
+            return nextPageUrl==null ? null : nextPageRequest(request, nextPageUrl); 
+        }
+
+        private HttpRequest<?> nextPageRequest(HttpRequest<?> originalRequest, String nextPageUrl) {
+            HttpRequest<?> result = unirest.request(originalRequest.getHttpMethod().name(), nextPageUrl)
+                    .socketTimeout(originalRequest.getSocketTimeout())
+                    .connectTimeout(originalRequest.getConnectTimeout())
+                    .proxy(originalRequest.getProxy());
+            for (Header header : originalRequest.getHeaders().all() ) {
+                result.headerReplace(header.getName(), header.getValue());
+            }
+            return result;
+        }
+
+
     }
 }
diff --git a/...src/main/java/com/fortify/cli/fod/_common/output/mixin/FoDProductHelperStandardMixin.java b/...src/main/java/com/fortify/cli/fod/_common/output/mixin/FoDProductHelperStandardMixin.java
@@ -19,16 +19,14 @@
 import com.fortify.cli.fod._common.rest.helper.FoDInputTransformer;
 import com.fortify.cli.fod._common.rest.helper.FoDPagingHelper;
 
-import kong.unirest.HttpRequest;
-
 // IMPORTANT: When updating/adding any methods in this class, FoDRestCallCommand
 // also likely needs to be updated
 public class FoDProductHelperStandardMixin extends FoDProductHelperBasicMixin 
     implements IInputTransformer, INextPageUrlProducerSupplier 
 {
     @Override
-    public INextPageUrlProducer getNextPageUrlProducer(HttpRequest<?> originalRequest) {
-        return FoDPagingHelper.nextPageUrlProducer(originalRequest);
+    public INextPageUrlProducer getNextPageUrlProducer() {
+        return FoDPagingHelper.nextPageUrlProducer();
     }
 
     @Override

diff --git a/...-core/fcli-fod/src/main/java/com/fortify/cli/fod/_common/rest/helper/FoDPagingHelper.java b/...-core/fcli-fod/src/main/java/com/fortify/cli/fod/_common/rest/helper/FoDPagingHelper.java
@@ -22,21 +22,18 @@
 
 public class FoDPagingHelper {
     public static final PagedList<JsonNode> pagedRequest(HttpRequest<?> request) {
-        return PagingHelper.pagedRequest(request, nextPageUrlProducer(request));
+        return PagingHelper.pagedRequest(request, nextPageUrlProducer());
     }
-    public static final INextPageUrlProducer nextPageUrlProducer(HttpRequest<?> originalRequest) {
-        return nextPageUrlProducer(originalRequest.getUrl());
-    }
-    public static final INextPageUrlProducer nextPageUrlProducer(String uri) {
-        return r -> {
-            JsonNode body = r.getBody();
+    public static final INextPageUrlProducer nextPageUrlProducer() {
+        return (req,resp) -> {
+            JsonNode body = resp.getBody();
             if ( body.has("offset") && body.has("totalCount") && body.has("limit") ) {
                 int offset = body.get("offset").asInt();
                 int totalCount = body.get("totalCount").asInt();
                 int limit = body.get("limit").asInt();
                 int newOffset = offset + limit;
                 if (newOffset < totalCount) {
-                    return URIHelper.addOrReplaceParam(uri, "offset", newOffset);
+                    return URIHelper.addOrReplaceParam(req.getUrl(), "offset", newOffset);
                 }
                 return null;
             }