Add pagination to bugsnag and generic api controller

src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java

@@ -112,7 +112,7 @@ private static void initializeHttpEndpoints() throws IOException, InterruptedExc
          * _encoded_ URL e.g. http://localhost:3000/#/register which allows for greater flexibility.
          */
         spark.get("/register", (request, response) -> {
-            String route = HttpUtils.getRequiredQueryParamFromRequest(request, "route", false);
+            String route = HttpUtils.getQueryParamFromRequest(request, "route", false);
             if (route == null) {
                 logMessageAndHalt(request,
                     HttpStatus.BAD_REQUEST_400,

src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java

@@ -2,13 +2,14 @@
 
 import com.beerboy.ss.ApiEndpoint;
 import com.beerboy.ss.SparkSwagger;
+import com.beerboy.ss.descriptor.ParameterDescriptor;
 import com.beerboy.ss.rest.Endpoint;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.mongodb.client.model.Filters;
-import org.bson.conversions.Bson;
 import org.eclipse.jetty.http.HttpStatus;
 import org.opentripplanner.middleware.auth.Auth0Connection;
 import org.opentripplanner.middleware.auth.Auth0UserProfile;
+import org.opentripplanner.middleware.controllers.response.ResponseList;
 import org.opentripplanner.middleware.models.Model;
 import org.opentripplanner.middleware.models.OtpUser;
 import org.opentripplanner.middleware.persistence.TypedPersistence;
@@ -21,9 +22,7 @@
 import spark.Request;
 import spark.Response;
 
-import java.lang.reflect.Array;
 import java.util.Date;
-import java.util.List;
 
 import static com.beerboy.ss.descriptor.EndpointDescriptor.endpointPath;
 import static com.beerboy.ss.descriptor.MethodDescriptor.path;
@@ -38,8 +37,8 @@
  * methods. This will identify the MongoDB collection on which to operate based on the provided {@link Model} class.
  *
  * TODO: Add hooks so that validation can be performed on certain methods (e.g., validating fields on create/update,
- * checking user permissions to perform certain actions, checking whether an entity can be deleted due to references
- * that exist in other collection).
+ *  checking user permissions to perform certain actions, checking whether an entity can be deleted due to references
+ *  that exist in other collection).
  *
  * @param <T> One of the {@link Model} classes (extracted from {@link TypedPersistence})
  */
@@ -52,6 +51,19 @@ public abstract class ApiController<T extends Model> implements Endpoint {
     private static final Logger LOG = LoggerFactory.getLogger(ApiController.class);
     final TypedPersistence<T> persistence;
     private final Class<T> clazz;
+    public static final String LIMIT_PARAM = "limit";
+    public static final int DEFAULT_LIMIT = 10;
+    public static final int DEFAULT_OFFSET = 0;
+    public static final String OFFSET_PARAM = "offset";
+
+    public static final ParameterDescriptor LIMIT = ParameterDescriptor.newBuilder()
+        .withName(LIMIT_PARAM)
+        .withDefaultValue(String.valueOf(DEFAULT_LIMIT))
+        .withDescription("If specified, the maximum number of items to return.").build();
+    public static final ParameterDescriptor OFFSET = ParameterDescriptor.newBuilder()
+        .withName(OFFSET_PARAM)
+        .withDefaultValue(String.valueOf(DEFAULT_OFFSET))
+        .withDescription("If specified, the number of records to skip/offset.").build();
 
     /**
      * @param apiPrefix string prefix to use in determining the resource location
@@ -112,19 +124,19 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) {
 
         baseEndpoint
             // Get multiple entities.
-            .get(path(ROOT_ROUTE)
+            .get(
+                path(ROOT_ROUTE)
                     .withDescription("Gets a list of all '" + className + "' entities.")
+                    .withQueryParam(LIMIT)
+                    .withQueryParam(OFFSET)
                     .withProduces(JSON_ONLY)
-                    // Set the return type as the array of clazz objects.
-                    // Note: there exists a method withResponseAsCollection, but unlike what its name suggests,
-                    // it does exactly the same as .withResponseType and does not generate a return type array.
-                    // See issue https://github.com/manusant/spark-swagger/issues/12.
-                    .withResponseType(Array.newInstance(clazz, 0).getClass()),
+                    .withResponseType(ResponseList.class),
                 this::getMany, JsonUtils::toJson
             )
 
             // Get one entity.
-            .get(path(ROOT_ROUTE + ID_PATH)
+            .get(
+                path(ROOT_ROUTE + ID_PATH)
                     .withDescription("Returns the '" + className + "' entity with the specified id, or 404 if not found.")
                     .withPathParam().withName(ID_PARAM).withRequired(true).withDescription("The id of the entity to search.").and()
                     // .withResponses(...) // FIXME: not implemented (requires source change).
@@ -134,7 +146,8 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) {
             )
 
             // Create entity request
-            .post(path("")
+            .post(
+                path("")
                     .withDescription("Creates a '" + className + "' entity.")
                     .withConsumes(JSON_ONLY)
                     .withRequestType(clazz)
@@ -144,7 +157,8 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) {
             )
 
             // Update entity request
-            .put(path(ID_PATH)
+            .put(
+                path(ID_PATH)
                     .withDescription("Updates and returns the '" + className + "' entity with the specified id, or 404 if not found.")
                     .withPathParam().withName(ID_PARAM).withRequired(true).withDescription("The id of the entity to update.").and()
                     .withConsumes(JSON_ONLY)
@@ -158,7 +172,8 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) {
             )
 
             // Delete entity request
-            .delete(path(ID_PATH)
+            .delete(
+                path(ID_PATH)
                     .withDescription("Deletes the '" + className + "' entity with the specified id if it exists.")
                     .withPathParam().withName(ID_PARAM).withRequired(true).withDescription("The id of the entity to delete.").and()
                     .withProduces(JSON_ONLY)
@@ -172,33 +187,26 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) {
      */
     // FIXME Maybe better if the user check (and filtering) was done in a pre hook?
     // FIXME Will require further granularity for admin
-    private List<T> getMany(Request req, Response res) {
-
+    private ResponseList<T> getMany(Request req, Response res) {
+        int limit = HttpUtils.getQueryParamFromRequest(req, LIMIT_PARAM, 0, DEFAULT_LIMIT, 100);
+        int offset = HttpUtils.getQueryParamFromRequest(req, OFFSET_PARAM, 0, DEFAULT_OFFSET);
         Auth0UserProfile requestingUser = getUserFromRequest(req);
         if (isUserAdmin(requestingUser)) {
             // If the user is admin, the context is presumed to be the admin dashboard, so we deliver all entities for
             // management or review without restriction.
-            return persistence.getAll();
+            return persistence.getResponseList(offset, limit);
         } else if (persistence.clazz == OtpUser.class) {
             // If the required entity is of type 'OtpUser' the assumption is that a call is being made via the
             // OtpUserController. Therefore, the request should be limited to return just the entity matching the
             // requesting user.
-            return getObjectsFiltered("_id", requestingUser.otpUser.id);
+            return persistence.getResponseList(Filters.eq("_id", requestingUser.otpUser.id), offset, limit);
         } else {
             // For all other cases the assumption is that the request is being made by an Otp user and the requested
             // entities have a 'userId' parameter. Only entities that match the requesting user id are returned.
-            return getObjectsFiltered("userId", requestingUser.otpUser.id);
+            return persistence.getResponseList(Filters.eq("userId", requestingUser.otpUser.id), offset, limit);
         }
     }
 
-    /**
-     * Get a list of objects filtered by the provided field name and value.
-     */
-    private List<T> getObjectsFiltered(String fieldName, String value) {
-        Bson filter = Filters.eq(fieldName, value);
-        return persistence.getFiltered(filter);
-    }
-
     /**
      * HTTP endpoint to get one entity specified by ID. This will return an object based on the checks carried out in
      * the overridden 'canBeManagedBy' method. It is the responsibility of this method to define access to it's own

src/main/java/org/opentripplanner/middleware/controllers/api/BugsnagController.java

@@ -2,9 +2,11 @@
 
 import com.beerboy.ss.SparkSwagger;
 import com.beerboy.ss.rest.Endpoint;
-import com.mongodb.client.model.Filters;
-import org.bson.conversions.Bson;
+import com.google.common.collect.Maps;
+import com.mongodb.client.FindIterable;
+import com.mongodb.client.model.Sorts;
 import org.opentripplanner.middleware.bugsnag.EventSummary;
+import org.opentripplanner.middleware.controllers.response.ResponseList;
 import org.opentripplanner.middleware.models.BugsnagEvent;
 import org.opentripplanner.middleware.models.BugsnagProject;
 import org.opentripplanner.middleware.persistence.Persistence;
@@ -16,9 +18,13 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 
 import static com.beerboy.ss.descriptor.EndpointDescriptor.endpointPath;
 import static com.beerboy.ss.descriptor.MethodDescriptor.path;
+import static org.opentripplanner.middleware.controllers.api.ApiController.DEFAULT_LIMIT;
+import static org.opentripplanner.middleware.controllers.api.ApiController.LIMIT_PARAM;
+import static org.opentripplanner.middleware.controllers.api.ApiController.OFFSET_PARAM;
 import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY;
 
 /**
@@ -54,19 +60,24 @@ public void bind(final SparkSwagger restApi) {
     }
 
     /**
-     * Get all Bugsnag events from Mongo and replace the project id with the project name and return
+     * Get all Bugsnag events from Mongo and replace the project id with the project name and return.
      */
-    private static List<EventSummary> getEventSummary(Request request, Response response) {
-        List<EventSummary> eventSummaries = new ArrayList<>();
-        List<BugsnagEvent> events = bugsnagEvents.getAll();
-
+    private static ResponseList<EventSummary> getEventSummary(Request req, Response res) {
+        int limit = HttpUtils.getQueryParamFromRequest(req, LIMIT_PARAM, 0, DEFAULT_LIMIT, 100);
+        int offset = HttpUtils.getQueryParamFromRequest(req, OFFSET_PARAM, 0, 0);
+        // Get latest events from database.
+        FindIterable<BugsnagEvent> events = bugsnagEvents.getSortedIterableWithOffsetAndLimit(
+            Sorts.descending("receivedAt"),
+            offset,
+            limit
+        );
+        // Get Bugsnag projects by id (avoid multiple queries to Mongo for the same project).
+        Map<String, BugsnagProject> projectsById = Maps.uniqueIndex(bugsnagProjects.getAll(), p -> p.projectId);
+        // Construct event summaries from project map.
         // FIXME: Group by error/project type?
-        for (BugsnagEvent event : events) {
-            Bson filter = Filters.eq("projectId", event.projectId);
-            BugsnagProject project = bugsnagProjects.getOneFiltered(filter);
-            eventSummaries.add(new EventSummary(project, event));
-        }
-
-        return eventSummaries;
+        List<EventSummary> eventSummaries = events
+            .map(event -> new EventSummary(projectsById.get(event.projectId), event))
+            .into(new ArrayList<>());
+        return new ResponseList<>(eventSummaries, offset, limit, bugsnagEvents.getCount());
     }
 }

src/main/java/org/opentripplanner/middleware/controllers/api/TripHistoryController.java

@@ -2,25 +2,32 @@
 
 import com.beerboy.ss.SparkSwagger;
 import com.beerboy.ss.rest.Endpoint;
+import org.bson.conversions.Bson;
 import org.eclipse.jetty.http.HttpStatus;
+import org.opentripplanner.middleware.controllers.response.ResponseList;
 import org.opentripplanner.middleware.models.TripRequest;
+import org.opentripplanner.middleware.persistence.Persistence;
 import org.opentripplanner.middleware.utils.DateTimeUtils;
 import org.opentripplanner.middleware.utils.HttpUtils;
 import org.opentripplanner.middleware.utils.JsonUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import spark.Request;
 import spark.Response;
 
 import java.time.LocalDate;
 import java.time.LocalTime;
 import java.time.format.DateTimeParseException;
 import java.util.Date;
-import java.util.List;
 
 import static com.beerboy.ss.descriptor.EndpointDescriptor.endpointPath;
 import static com.beerboy.ss.descriptor.MethodDescriptor.path;
 import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthorized;
+import static org.opentripplanner.middleware.controllers.api.ApiController.DEFAULT_LIMIT;
+import static org.opentripplanner.middleware.controllers.api.ApiController.DEFAULT_OFFSET;
+import static org.opentripplanner.middleware.controllers.api.ApiController.LIMIT;
+import static org.opentripplanner.middleware.controllers.api.ApiController.LIMIT_PARAM;
+import static org.opentripplanner.middleware.controllers.api.ApiController.OFFSET;
+import static org.opentripplanner.middleware.controllers.api.ApiController.OFFSET_PARAM;
+import static org.opentripplanner.middleware.persistence.TypedPersistence.filterByUserAndDateRange;
 import static org.opentripplanner.middleware.utils.DateTimeUtils.YYYY_MM_DD;
 import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY;
 import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt;
@@ -30,14 +37,8 @@
  * To provide a response to the calling MOD UI in JSON based on the passed in parameters.
  */
 public class TripHistoryController implements Endpoint {
-
-    private static final Logger LOG = LoggerFactory.getLogger(TripHistoryController.class);
-
-    private static final String FROM_DATE_PARAM_NAME = "fromDate";
-    private static final String TO_DATE_PARAM_NAME = "toDate";
-    private static final String LIMIT_PARAM_NAME = "limit";
-    private static final int DEFAULT_LIMIT = 10;
-
+    private static final String FROM_DATE_PARAM = "fromDate";
+    private static final String TO_DATE_PARAM = "toDate";
     private final String ROOT_ROUTE;
 
     public TripHistoryController(String apiPrefix) {
@@ -60,72 +61,56 @@ public void bind(final SparkSwagger restApi) {
                     .withName("userId")
                     .withRequired(true)
                     .withDescription("The OTP user for which to retrieve trip requests.").and()
-                .withQueryParam()
-                    .withName(LIMIT_PARAM_NAME)
-                    .withDefaultValue(String.valueOf(DEFAULT_LIMIT))
-                    .withDescription("If specified, the maximum number of trip requests to return, starting from the most recent.").and()
-                .withQueryParam()
-                    .withName(FROM_DATE_PARAM_NAME)
+               .withQueryParam(LIMIT)
+               .withQueryParam(OFFSET)
+               .withQueryParam()
+                    .withName(FROM_DATE_PARAM)
                     .withPattern(YYYY_MM_DD)
                     .withDefaultValue("The current date")
                     .withDescription(String.format(
                         "If specified, the earliest date (format %s) for which trip requests are retrieved.", YYYY_MM_DD
                     )).and()
-                .withQueryParam()
-                    .withName(TO_DATE_PARAM_NAME)
+               .withQueryParam()
+               .withName(TO_DATE_PARAM)
                     .withPattern(YYYY_MM_DD)
                     .withDefaultValue("The current date")
                     .withDescription(String.format(
-                        "If specified, the latest date (format %s) for which usage logs are retrieved.", YYYY_MM_DD
+                        "If specified, the latest date (format %s) for which trip requests are retrieved.", YYYY_MM_DD
                     )).and()
-                .withProduces(JSON_ONLY)
-                // Note: unlike the name suggests, withResponseAsCollection does not generate an array
-                // as the return type for this method. (It does generate the type for that class nonetheless.)
-                .withResponseAsCollection(TripRequest.class),
+               .withProduces(JSON_ONLY)
+               // Note: unlike the name suggests, withResponseAsCollection does not generate an array
+               // as the return type for this method. (It does generate the type for that class nonetheless.)
+               .withResponseAsCollection(TripRequest.class),
            TripHistoryController::getTripRequests, JsonUtils::toJson);
     }
 
     /**
      * Return a user's trip request history based on provided parameters.
      * An authorized user (Auth0) and user id are required.
      */
-    private static List<TripRequest> getTripRequests(Request request, Response response) {
-        final String userId = HttpUtils.getRequiredQueryParamFromRequest(request, "userId", false);
+    private static ResponseList<TripRequest> getTripRequests(Request request, Response response) {
+        final String userId = HttpUtils.getQueryParamFromRequest(request, "userId", false);
         // Check that the user is authorized (otherwise a halt is thrown).
         isAuthorized(userId, request);
-
-        int limit = DEFAULT_LIMIT;
-
-        String paramLimit = null;
-        try {
-            paramLimit = HttpUtils.getRequiredQueryParamFromRequest(request, LIMIT_PARAM_NAME, true);
-            if (paramLimit != null) {
-                limit = Integer.parseInt(paramLimit);
-                if (limit <= 0) {
-                    limit = DEFAULT_LIMIT;
-                }
-            }
-        } catch (NumberFormatException e) {
-            LOG.error("Unable to parse {} value of {}. Using default limit: {}", LIMIT_PARAM_NAME,
-                paramLimit, DEFAULT_LIMIT, e);
-        }
-
-        String paramFromDate = HttpUtils.getRequiredQueryParamFromRequest(request, FROM_DATE_PARAM_NAME, true);
-        Date fromDate = getDate(request, FROM_DATE_PARAM_NAME, paramFromDate, LocalTime.MIDNIGHT);
-
-        String paramToDate = HttpUtils.getRequiredQueryParamFromRequest(request, TO_DATE_PARAM_NAME, true);
-        Date toDate = getDate(request, TO_DATE_PARAM_NAME, paramToDate, LocalTime.MAX);
-
+        // Get params from request (or use defaults).
+        int limit = HttpUtils.getQueryParamFromRequest(request, LIMIT_PARAM, 0, DEFAULT_LIMIT, 100);
+        int offset = HttpUtils.getQueryParamFromRequest(request, OFFSET_PARAM, 0, DEFAULT_OFFSET);
+        String paramFromDate = HttpUtils.getQueryParamFromRequest(request, FROM_DATE_PARAM, true);
+        Date fromDate = getDate(request, FROM_DATE_PARAM, paramFromDate, LocalTime.MIDNIGHT);
+        String paramToDate = HttpUtils.getQueryParamFromRequest(request, TO_DATE_PARAM, true);
+        Date toDate = getDate(request, TO_DATE_PARAM, paramToDate, LocalTime.MAX);
+        // Throw halt if the date params are bad.
         if (fromDate != null && toDate != null && toDate.before(fromDate)) {
             logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400,
-                String.format("%s (%s) before %s (%s)", TO_DATE_PARAM_NAME, paramToDate, FROM_DATE_PARAM_NAME,
+                String.format("%s (%s) before %s (%s)", TO_DATE_PARAM, paramToDate, FROM_DATE_PARAM,
                     paramFromDate));
         }
-        return TripRequest.requestsForUser(userId, fromDate, toDate, limit);
+        Bson filter = filterByUserAndDateRange(userId, fromDate, toDate);
+        return Persistence.tripRequests.getResponseList(filter, offset, limit);
     }
 
     /**
-     * Get date from request parameter and convert to java.util.Date at a specific time of day. The date conversion
+     * Get date from request parameter and convert to {@link Date} at a specific time of day. The date conversion
      * is based on the system time zone.
      */
     private static Date getDate(Request request, String paramName, String paramValue, LocalTime timeOfDay) {