server: Consolidate query parameters validation and conversion

Add QueryParameterUtil class with method to validate each endpoint's query parameters map. The method will verify the presence of required parameters and translate parameters when necessary to conform with the data provider interfaces. When a specific parameter is missing or invalid, identify it in the error message response. Remove the QueryParametersDeserializer which is no longer needed. Make the parameters field non-null in QueryParameters and assign empty map in constructor when not received. Fix TraceQueryParameters "uri" Json property name. Bug: 579456 Change-Id: I1cf727d5d1087e7f511fa2a7a62490b5a41d4807 Signed-off-by: Patrick Tasse <patrick.tasse@gmail.com> Reviewed-on: https://git.eclipse.org/r/c/tracecompass.incubator/org.eclipse.tracecompass.incubator/+/192291 Tested-by: Trace Compass Bot <tracecompass-bot@eclipse.org> Tested-by: Bernd Hufmann <bernd.hufmann@ericsson.com> Reviewed-by: Bernd Hufmann <bernd.hufmann@ericsson.com>
eclipse-tracecompass-incubator · Apr 5, 2022 · e4e6768 · e4e6768
1 parent 9a1071a
commit e4e6768
Show file tree

Hide file tree

Showing 11 changed files with 855 additions and 218 deletions.
diff --git a/...mpass/incubator/trace/server/jersey/rest/core/tests/services/DataProviderServiceTest.java b/...mpass/incubator/trace/server/jersey/rest/core/tests/services/DataProviderServiceTest.java
@@ -129,7 +129,6 @@ public void testCallStackDataProvider() {
         assertEquals("There should be a positive response for the data provider", 200, tree.getStatus());
 
         parameters = new HashMap<>();
-        parameters.put(REQUESTED_TIMES_KEY, Collections.emptyList());
         Response defaults = callstackTree.request().post(Entity.json(new QueryParameters(parameters, Collections.emptyList())));
         assertEquals("Default values should return OK code", 200, defaults.getStatus());
     }

diff --git a/...mpass/incubator/trace/server/jersey/rest/core/tests/services/QueryParametersUtilTest.java b/...mpass/incubator/trace/server/jersey/rest/core/tests/services/QueryParametersUtilTest.java
diff --git a/...compass/incubator/trace/server/jersey/rest/core/tests/services/XmlManagerServiceTest.java b/...compass/incubator/trace/server/jersey/rest/core/tests/services/XmlManagerServiceTest.java
@@ -36,7 +36,6 @@
 import org.eclipse.tracecompass.incubator.internal.trace.server.jersey.rest.core.services.XmlManagerService;
 import org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests.stubs.ExperimentModelStub;
 import org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests.utils.RestServerTest;
-import org.eclipse.tracecompass.tmf.core.dataprovider.DataProviderParameterUtils;
 import org.junit.Test;
 import org.osgi.framework.Bundle;
 
@@ -85,7 +84,6 @@ public void test() throws URISyntaxException, IOException {
 
         WebTarget xmlProviderPath = getXYTreeEndpoint(exp.getUUID().toString(), "org.eclipse.linuxtools.tmf.analysis.xml.core.tests.xy");
         Map<String, Object> parameters = new HashMap<>();
-        parameters.put(DataProviderParameterUtils.REQUESTED_TIME_KEY, Collections.emptyList());
         Response xmlTree = xmlProviderPath.request().post(Entity.json(new QueryParameters(parameters , Collections.emptyList())));
         assertEquals("The end point for the XML data provider should be available.", 200, xmlTree.getStatus());
 

diff --git a/...ecompass/incubator/internal/trace/server/jersey/rest/core/model/TraceQueryParameters.java b/...ecompass/incubator/internal/trace/server/jersey/rest/core/model/TraceQueryParameters.java
@@ -37,7 +37,7 @@ public interface TraceQueryParameters {
     /**
      * @return The URI.
      */
-    @JsonProperty("URI")
+    @JsonProperty("uri")
     @NonNull
     @Schema(description = "URI of the trace", required = true)
     String getUri();

diff --git a/...compass/incubator/internal/trace/server/jersey/rest/core/model/views/QueryParameters.java b/...compass/incubator/internal/trace/server/jersey/rest/core/model/views/QueryParameters.java
@@ -11,9 +11,11 @@
 
 package org.eclipse.tracecompass.incubator.internal.trace.server.jersey.rest.core.model.views;
 
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 
 import io.swagger.v3.oas.annotations.Hidden;
@@ -24,14 +26,15 @@
  * @author Simon Delisle
  */
 public class QueryParameters {
-    private Map<String, Object> parameters;
+    private @NonNull Map<String, Object> parameters;
     private List<Filter> filters;
 
     /**
      * Constructor for Jackson
      */
     public QueryParameters() {
         // Default constructor for Jackson
+        this.parameters = new HashMap<>();
     }
 
     /**
@@ -43,15 +46,15 @@ public QueryParameters() {
      *            List of filters
      */
     public QueryParameters(Map<String, Object> parameters, List<Filter> filters) {
-        this.parameters = parameters;
+        this.parameters = parameters != null ? parameters : new HashMap<>();
         this.filters = filters;
     }
 
     /**
      * @return Map of parameters
      */
     @Hidden
-    public @Nullable Map<String, Object> getParameters() {
+    public @NonNull Map<String, Object> getParameters() {
         return parameters;
     }
 

diff --git a/...ompass/incubator/internal/trace/server/jersey/rest/core/services/DataProviderService.java b/...ompass/incubator/internal/trace/server/jersey/rest/core/services/DataProviderService.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2017, 2021 Ericsson
+ * Copyright (c) 2017, 2022 Ericsson
  *
  * All rights reserved. This program and the accompanying materials are
  * made available under the terms of the Eclipse Public License 2.0 which
@@ -140,7 +140,6 @@
 import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerConfigXmlParser;
 import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerSet;
 import org.eclipse.tracecompass.internal.tmf.core.model.DataProviderDescriptor;
-import org.eclipse.tracecompass.internal.tmf.core.model.filters.FetchParametersUtils;
 import org.eclipse.tracecompass.tmf.analysis.xml.core.module.TmfXmlUtils;
 import org.eclipse.tracecompass.tmf.core.analysis.IAnalysisModuleHelper;
 import org.eclipse.tracecompass.tmf.core.analysis.TmfAnalysisManager;
@@ -151,8 +150,6 @@
 import org.eclipse.tracecompass.tmf.core.model.CommonStatusMessage;
 import org.eclipse.tracecompass.tmf.core.model.IOutputStyleProvider;
 import org.eclipse.tracecompass.tmf.core.model.OutputStyleModel;
-import org.eclipse.tracecompass.tmf.core.model.filters.SelectionTimeQueryFilter;
-import org.eclipse.tracecompass.tmf.core.model.filters.TimeQueryFilter;
 import org.eclipse.tracecompass.tmf.core.model.timegraph.ITimeGraphArrow;
 import org.eclipse.tracecompass.tmf.core.model.timegraph.ITimeGraphDataProvider;
 import org.eclipse.tracecompass.tmf.core.model.timegraph.ITimeGraphEntryModel;
@@ -346,16 +343,9 @@ public Response getXY(
                             "}}"), schema = @Schema(implementation = RequestedQueryParameters.class))
             }, required = true) QueryParameters queryParameters) {
 
-        if (outputId == null) {
-            return Response.status(Status.BAD_REQUEST).entity(MISSING_OUTPUTID).build();
-        }
-        Map<String, Object> params = queryParameters.getParameters();
-        if (params == null) {
-            return Response.status(Status.BAD_REQUEST).entity(MISSING_PARAMETERS).build();
-        }
-        SelectionTimeQueryFilter selectionTimeQueryFilter = FetchParametersUtils.createSelectionTimeQuery(params);
-        if (selectionTimeQueryFilter == null) {
-            return Response.status(Status.BAD_REQUEST).entity(MISSING_PARAMETERS).build();
+        Response errorResponse = validateParameters(outputId, queryParameters);
+        if (errorResponse != null) {
+            return errorResponse;
         }
         try (FlowScopeLog scope = new FlowScopeLogBuilder(LOGGER, Level.FINE, "DataProviderService#getXY") //$NON-NLS-1$
                 .setCategory(outputId).build()) {
@@ -377,6 +367,12 @@ public Response getXY(
                 return Response.status(Status.METHOD_NOT_ALLOWED).entity(NO_PROVIDER).build();
             }
 
+            Map<String, Object> params = queryParameters.getParameters();
+            String errorMessage = QueryParametersUtil.validateRequestedQueryParameters(params);
+            if (errorMessage != null) {
+                return Response.status(Status.BAD_REQUEST).entity(errorMessage).build();
+            }
+
             TmfModelResponse<@NonNull ITmfXyModel> response = provider.fetchXY(params, null);
             return Response.ok(response).build();
         }
@@ -475,15 +471,10 @@ public Response getStates(
                             "}}"), schema = @Schema(implementation = RequestedQueryParameters.class))
             }, required = true) QueryParameters queryParameters) {
 
-        Map<String, Object> params = queryParameters.getParameters();
-        Response errorResponse = validateQueryParameters(outputId, params);
+        Response errorResponse = validateParameters(outputId, queryParameters);
         if (errorResponse != null) {
             return errorResponse;
         }
-        SelectionTimeQueryFilter selectionTimeQueryFilter = FetchParametersUtils.createSelectionTimeQuery(params);
-        if (selectionTimeQueryFilter == null) {
-            return Response.status(Status.BAD_REQUEST).entity(MISSING_PARAMETERS).build();
-        }
         try (FlowScopeLog scope = new FlowScopeLogBuilder(LOGGER, Level.FINE, "DataProviderService#getStates") //$NON-NLS-1$
                 .setCategory(outputId).build()) {
             TmfExperiment experiment = ExperimentManagerService.getExperimentByUUID(expUUID);
@@ -498,6 +489,12 @@ public Response getStates(
                 return Response.status(Status.METHOD_NOT_ALLOWED).entity(NO_PROVIDER).build();
             }
 
+            Map<String, Object> params = queryParameters.getParameters();
+            String errorMessage = QueryParametersUtil.validateRequestedQueryParameters(params);
+            if (errorMessage != null) {
+                return Response.status(Status.BAD_REQUEST).entity(errorMessage).build();
+            }
+
             TmfModelResponse<TimeGraphModel> response = provider.fetchRowModel(params, null);
             return Response.ok(response).build();
         }
@@ -535,15 +532,10 @@ public Response getArrows(
                             "}}"), schema = @Schema(implementation = ArrowsQueryParameters.class))
             }, required = true) QueryParameters queryParameters) {
 
-        Map<String, Object> params = queryParameters.getParameters();
-        Response errorResponse = validateQueryParameters(outputId, params);
+        Response errorResponse = validateParameters(outputId, queryParameters);
         if (errorResponse != null) {
             return errorResponse;
         }
-        TimeQueryFilter timeQueryFilter = FetchParametersUtils.createTimeQuery(params);
-        if (timeQueryFilter == null) {
-            return Response.status(Status.BAD_REQUEST).entity(MISSING_PARAMETERS).build();
-        }
         try (FlowScopeLog scope = new FlowScopeLogBuilder(LOGGER, Level.FINE, "DataProviderService#getArrows") //$NON-NLS-1$
                 .setCategory(outputId).build()) {
             TmfExperiment experiment = ExperimentManagerService.getExperimentByUUID(expUUID);
@@ -558,6 +550,12 @@ public Response getArrows(
                 return Response.status(Status.METHOD_NOT_ALLOWED).entity(NO_PROVIDER).build();
             }
 
+            Map<String, Object> params = queryParameters.getParameters();
+            String errorMessage = QueryParametersUtil.validateArrowsQueryParameters(params);
+            if (errorMessage != null) {
+                return Response.status(Status.BAD_REQUEST).entity(errorMessage).build();
+            }
+
             TmfModelResponse<@NonNull List<@NonNull ITimeGraphArrow>> response = provider.fetchArrows(params, null);
             return Response.ok(response).build();
         }
@@ -698,8 +696,7 @@ public Response getAnnotations(
                                     "}}"), schema = @Schema(implementation = AnnotationsQueryParameters.class))
                     }, required = true) QueryParameters queryParameters) {
 
-        Map<String, Object> params = queryParameters.getParameters();
-        Response errorResponse = validateQueryParameters(outputId, params);
+        Response errorResponse = validateParameters(outputId, queryParameters);
         if (errorResponse != null) {
             return errorResponse;
         }
@@ -718,6 +715,12 @@ public Response getAnnotations(
                 return Response.status(Status.METHOD_NOT_ALLOWED).entity(NO_PROVIDER).build();
             }
 
+            Map<String, Object> params = queryParameters.getParameters();
+            String errorMessage = QueryParametersUtil.validateAnnotationsQueryParameters(params);
+            if (errorMessage != null) {
+                return Response.status(Status.BAD_REQUEST).entity(errorMessage).build();
+            }
+
             boolean isComplete = true;
             AnnotationModel model = null;
 
@@ -786,8 +789,7 @@ public Response getTimeGraphTooltip(
                             "}}"), schema = @Schema(implementation = TooltipQueryParameters.class))
             }, required = true) QueryParameters queryParameters) {
 
-        Map<String, Object> params = queryParameters.getParameters();
-        Response errorResponse = validateQueryParameters(outputId, params);
+        Response errorResponse = validateParameters(outputId, queryParameters);
         if (errorResponse != null) {
             return errorResponse;
         }
@@ -805,6 +807,12 @@ public Response getTimeGraphTooltip(
                 return Response.status(Status.METHOD_NOT_ALLOWED).entity(NO_PROVIDER).build();
             }
 
+            Map<String, Object> params = queryParameters.getParameters();
+            String errorMessage = QueryParametersUtil.validateTooltipQueryParameters(params);
+            if (errorMessage != null) {
+                return Response.status(Status.BAD_REQUEST).entity(errorMessage).build();
+            }
+
             TmfModelResponse<@NonNull Map<@NonNull String, @NonNull String>> response = provider.fetchTooltip(params, null);
             return Response.ok(response).build();
         }
@@ -903,8 +911,7 @@ public Response getLines(
                                     "}}"), schema = @Schema(implementation = LinesQueryParameters.class))
                     }, required = true) QueryParameters queryParameters) {
 
-        Map<String, Object> params = queryParameters.getParameters();
-        Response errorResponse = validateQueryParameters(outputId, params);
+        Response errorResponse = validateParameters(outputId, queryParameters);
         if (errorResponse != null) {
             return errorResponse;
         }
@@ -920,6 +927,12 @@ public Response getLines(
                 return Response.status(Status.METHOD_NOT_ALLOWED).entity(NO_PROVIDER).build();
             }
 
+            Map<String, Object> params = queryParameters.getParameters();
+            String errorMessage = QueryParametersUtil.validateLinesQueryParameters(params);
+            if (errorMessage != null) {
+                return Response.status(Status.BAD_REQUEST).entity(errorMessage).build();
+            }
+
             TmfModelResponse<?> response = provider.fetchLines(params, null);
             if (response.getStatus() == ITmfResponse.Status.FAILED) {
                 return Response.status(Status.BAD_REQUEST).entity(response.getStatusMessage()).build();
@@ -990,16 +1003,14 @@ public Response getLines(
     }
 
     private Response getTree(UUID expUUID, String outputId, QueryParameters queryParameters) {
-        if (outputId == null) {
-            return Response.status(Status.BAD_REQUEST).entity(MISSING_OUTPUTID).build();
+        Response errorResponse = validateParameters(outputId, queryParameters);
+        if (errorResponse != null) {
+            return errorResponse;
         }
         Map<String, Object> params = queryParameters.getParameters();
-        if (params == null) {
-            return Response.status(Status.BAD_REQUEST).entity(MISSING_PARAMETERS).build();
-        }
-        List<Long> timeRequested = DataProviderParameterUtils.extractTimeRequested(params);
-        if (timeRequested != null && timeRequested.size() == 1) {
-            return Response.status(Status.BAD_REQUEST).entity(INVALID_PARAMETERS).build();
+        String errorMessage = QueryParametersUtil.validateTreeQueryParameters(params);
+        if (errorMessage != null) {
+            return Response.status(Status.BAD_REQUEST).entity(errorMessage).build();
         }
         try (FlowScopeLog scope = new FlowScopeLogBuilder(LOGGER, Level.FINE, "DataProviderService#getTree") //$NON-NLS-1$
                 .setCategory(outputId).build()) {
@@ -1020,6 +1031,7 @@ private Response getTree(UUID expUUID, String outputId, QueryParameters queryPar
                 // The analysis cannot be run on this trace
                 return Response.status(Status.METHOD_NOT_ALLOWED).entity(NO_PROVIDER).build();
             }
+            List<Long> timeRequested = DataProviderParameterUtils.extractTimeRequested(params);
             if (timeRequested == null || timeRequested.isEmpty()) {
                 // Make a shallow copy to be able to modify the map
                 params = new HashMap<>(params);
@@ -1062,11 +1074,11 @@ public Response getStyles(
                     @Content(examples = @ExampleObject("{\"parameters\":{}}"), schema = @Schema(implementation = OptionalQueryParameters.class))
             }, required = true) QueryParameters queryParameters) {
 
-        Map<String, Object> params = queryParameters.getParameters();
-        Response errorResponse = validateQueryParameters(outputId, params);
+        Response errorResponse = validateParameters(outputId, queryParameters);
         if (errorResponse != null) {
             return errorResponse;
         }
+        Map<String, Object> params = queryParameters.getParameters();
         try (FlowScopeLog scope = new FlowScopeLogBuilder(LOGGER, Level.FINE, "DataProviderService#getStyles") //$NON-NLS-1$
                 .setCategory(outputId).build()) {
             TmfExperiment experiment = ExperimentManagerService.getExperimentByUUID(expUUID);
@@ -1093,11 +1105,11 @@ public Response getStyles(
         }
     }
 
-    private static Response validateQueryParameters(String outputId, Map<String, Object> params) {
+    private static Response validateParameters(String outputId, QueryParameters queryParameters) {
         if (outputId == null) {
             return Response.status(Status.BAD_REQUEST).entity(MISSING_OUTPUTID).build();
         }
-        if (params == null) {
+        if (queryParameters == null) {
             return Response.status(Status.BAD_REQUEST).entity(MISSING_PARAMETERS).build();
         }
         return null;

diff --git a/...s/incubator/internal/trace/server/jersey/rest/core/services/ExperimentManagerService.java b/...s/incubator/internal/trace/server/jersey/rest/core/services/ExperimentManagerService.java
@@ -268,20 +268,21 @@ public Response deleteExperiment(@Parameter(description = EXP_UUID) @PathParam("
     public Response postExperiment(@RequestBody(content = {
             @Content(schema = @Schema(implementation = ExperimentQueryParameters.class))
     }, required = true) QueryParameters queryParameters) {
-        Map<String, Object> parameters = queryParameters.getParameters();
-        if (parameters == null) {
+
+        if (queryParameters == null) {
             return Response.status(Status.BAD_REQUEST).entity(MISSING_PARAMETERS).build();
         }
-        Object nameObj = parameters.get("name"); //$NON-NLS-1$
-        Object tracesObj = parameters.get("traces"); //$NON-NLS-1$
-        if (!(nameObj instanceof String) || !(tracesObj instanceof List<?>)) {
-            return Response.status(Status.BAD_REQUEST).build();
+        Map<String, Object> parameters = queryParameters.getParameters();
+        String errorMessage = QueryParametersUtil.validateExperimentQueryParameters(parameters);
+        if (errorMessage != null) {
+            return Response.status(Status.BAD_REQUEST).entity(errorMessage).build();
         }
-        String name = (String) nameObj;
+        String name = Objects.requireNonNull((String) parameters.get("name")); //$NON-NLS-1$
+        List<String> tracesObj = Objects.requireNonNull((List<String>) parameters.get("traces")); //$NON-NLS-1$
         List<UUID> traceUUIDs = new ArrayList<>();
 
         List<IResource> traceResources = new ArrayList<>();
-        for (Object uuidObj : (List<?>) tracesObj) {
+        for (Object uuidObj : tracesObj) {
             if (!(uuidObj instanceof String)) {
                 return Response.status(Status.BAD_REQUEST).build();
             }