Mercurial > hg > thermostat-ng > web-gateway

changeset 237:9a08bbf30b04

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Refactor how next/prev links are crafted. Reviewed-by: jerboaa Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-July/024309.html
author Chris Lessard <clessard@redhat.com>
date Wed, 30 Aug 2017 12:00:11 -0400
parents 082ffd8bc95a
children 1093c542f930
files common/mongodb/src/main/java/com/redhat/thermostat/gateway/common/mongodb/response/MongoMetaDataGenerator.java common/mongodb/src/main/java/com/redhat/thermostat/gateway/common/mongodb/response/MongoMetaDataResponseBuilder.java common/mongodb/src/main/java/com/redhat/thermostat/gateway/common/mongodb/servlet/MongoHttpHandlerHelper.java common/mongodb/src/test/java/com/redhat/thermostat/gateway/common/mongodb/response/MongoMetaDataGeneratorTest.java
diffstat 4 files changed, 64 insertions(+), 19 deletions(-) [+]
line wrap: on
line diff
--- a/common/mongodb/src/main/java/com/redhat/thermostat/gateway/common/mongodb/response/MongoMetaDataGenerator.java	Tue Aug 29 14:25:19 2017 +0200
+++ b/common/mongodb/src/main/java/com/redhat/thermostat/gateway/common/mongodb/response/MongoMetaDataGenerator.java	Wed Aug 30 12:00:11 2017 -0400
@@ -36,7 +36,7 @@
 
 package com.redhat.thermostat.gateway.common.mongodb.response;
 
-import javax.servlet.http.HttpServletRequest;
+import java.util.LinkedHashMap;
 
 import com.redhat.thermostat.gateway.common.mongodb.servlet.RequestParameters;
 import com.redhat.thermostat.gateway.common.mongodb.executor.MongoDataResultContainer;
@@ -50,11 +50,11 @@
     private final String includes;
     private final String excludes;
     private final MongoDataResultContainer execResult;
-    private final HttpServletRequest requestInfo;
+    private final LinkedHashMap<String, String> requestParamArguments;
     private final String baseURL;
 
     public MongoMetaDataGenerator(Integer limit, Integer offset, String sort, String queries, String includes, String excludes,
-                                  HttpServletRequest requestInfo, MongoDataResultContainer execResult) {
+                                  LinkedHashMap<String, String> requestParamArguments, MongoDataResultContainer execResult, String baseUrl) {
         this.limit = limit;
         this.offset = offset;
         this.sort = sort;
@@ -62,8 +62,8 @@
         this.includes = includes;
         this.excludes = excludes;
         this.execResult = execResult;
-        this.requestInfo = requestInfo;
-        this.baseURL = requestInfo.getRequestURL().toString();
+        this.requestParamArguments = requestParamArguments;
+        this.baseURL = baseUrl;
     }
 
     public void setDocAndPayloadCount(MongoMetaDataResponseBuilder.MetaBuilder response) {
@@ -77,8 +77,7 @@
     public void setPrev(MongoMetaDataResponseBuilder.MetaBuilder response) {
         if (!Integer.valueOf(0).equals(offset)) {
             StringBuilder prev = new StringBuilder();
-            String[] arguments = requestInfo.getQueryString().split("&");
-            prev.append(baseURL).append("?").append(response.getQueryArgumentsNoOffsetLimit(arguments));
+            prev.append(baseURL).append("?").append(response.getQueryArgumentsNoOffsetLimit(requestParamArguments));
 
             if (limit > 1) {
                 int newLim = (offset >= limit) ? limit : offset;
@@ -100,8 +99,7 @@
                 int nextLimit = (remaining > limit) ? limit : remaining;
                 next.append(baseURL).append('?' + RequestParameters.OFFSET + '=').append(offset + limit).append('&' + RequestParameters.LIMIT + '=').append(nextLimit).append("&");
 
-                String[] arguments = requestInfo.getQueryString().split("&");
-                next.append(response.getQueryArgumentsNoOffsetLimit(arguments));
+                next.append(response.getQueryArgumentsNoOffsetLimit(requestParamArguments));
 
                 response.payloadCount(nextLimit).next(next.toString());
             }
--- a/common/mongodb/src/main/java/com/redhat/thermostat/gateway/common/mongodb/response/MongoMetaDataResponseBuilder.java	Tue Aug 29 14:25:19 2017 +0200
+++ b/common/mongodb/src/main/java/com/redhat/thermostat/gateway/common/mongodb/response/MongoMetaDataResponseBuilder.java	Wed Aug 30 12:00:11 2017 -0400
@@ -36,6 +36,8 @@
 
 package com.redhat.thermostat.gateway.common.mongodb.response;
 
+import java.util.LinkedHashMap;
+
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.redhat.thermostat.gateway.common.mongodb.servlet.RequestParameters;
@@ -120,12 +122,13 @@
             return new MongoMetaDataResponseBuilder(this);
         }
 
-        public String getQueryArgumentsNoOffsetLimit(String[] URLQueryPath) {
+        public String getQueryArgumentsNoOffsetLimit(LinkedHashMap<String, String> requestParamArguments) {
             StringBuilder queryString = new StringBuilder();
             String sep = "";
-            for (String arg : URLQueryPath) {
-                if (!(arg.contains(RequestParameters.LIMIT) || arg.contains(RequestParameters.OFFSET))) {
-                    queryString.append(sep).append(arg);
+            for (String param : requestParamArguments.keySet()) {
+                String val = requestParamArguments.get(param);
+                if (!(RequestParameters.LIMIT.equals(param) || RequestParameters.OFFSET.equals(param) || (val == null))) {
+                    queryString.append(sep).append(param).append("=").append(val);
                     sep = "&";
                 }
             }
--- a/common/mongodb/src/main/java/com/redhat/thermostat/gateway/common/mongodb/servlet/MongoHttpHandlerHelper.java	Tue Aug 29 14:25:19 2017 +0200
+++ b/common/mongodb/src/main/java/com/redhat/thermostat/gateway/common/mongodb/servlet/MongoHttpHandlerHelper.java	Wed Aug 30 12:00:11 2017 -0400
@@ -55,6 +55,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.core.Response;
 import java.io.IOException;
+import java.util.LinkedHashMap;
 
 import static com.redhat.thermostat.gateway.common.util.ServiceException.CANNOT_QUERY_REALMS_PROPERTY;
 import static com.redhat.thermostat.gateway.common.util.ServiceException.DATABASE_UNAVAILABLE;
@@ -84,14 +85,22 @@
      */
 
     public Response handleGetWithSystemID(HttpServletRequest httpServletRequest, ServletContext context, String systemId, int limit, int offset, String sort, String queries, String includes, String excludes, String returnMetadata) {
-        return handleGet(httpServletRequest, context, limit, offset, sort, andSystemIdQuery(queries, systemId), includes, excludes, returnMetadata);
+        return handleGet(httpServletRequest, context, limit, offset, sort, andSystemIdQuery(queries, systemId), includes, excludes, returnMetadata, queries);
     }
 
     public Response handleGetWithJvmID(HttpServletRequest httpServletRequest, ServletContext context, String systemId, String jvmId, int limit, int offset, String sort, String queries, String includes, String excludes, String returnMetadata) {
-        return handleGet(httpServletRequest, context, limit, offset, sort, andSystemIdJvmIdQuery(queries, systemId, jvmId), includes, excludes, returnMetadata);
+        return handleGet(httpServletRequest, context, limit, offset, sort, andSystemIdJvmIdQuery(queries, systemId, jvmId), includes, excludes, returnMetadata, queries);
     }
 
     public Response handleGet(HttpServletRequest httpServletRequest, ServletContext context, int limit, int offset, String sort, String queries, String includes, String excludes, String returnMetadata) {
+        return handleGet(httpServletRequest, context, limit, offset, sort, queries, includes, excludes, returnMetadata, "");
+    }
+
+    /*
+     * originalQueries contains only query info from the client's original request argument. queries contains this info,
+     * as well as added JVM/SYS ids built by andSystemIdJvmIdQuery(...). 
+     */
+    public Response handleGet(HttpServletRequest httpServletRequest, ServletContext context, int limit, int offset, String sort, String queries, String includes, String excludes, String returnMetadata, String originalQueries) {
         try {
             boolean metadata = Boolean.valueOf(returnMetadata);
             RealmAuthorizer realmAuthorizer = (RealmAuthorizer) httpServletRequest.getAttribute(RealmAuthorizer.class.getName());
@@ -104,10 +113,23 @@
 
                 MongoResponseBuilder.Builder response = new MongoResponseBuilder.Builder();
                 response.addQueryDocuments(execResult.getQueryDataResult());
+
                 if (metadata) {
+
+                    // Test suites expect a consistent order of next and prev links, hence LinkedHashMap
+                    LinkedHashMap<String, String> paramArgs = new LinkedHashMap<>();
+                    paramArgs.put(RequestParameters.SORT, sort);
+                    paramArgs.put(RequestParameters.QUERY, originalQueries);
+                    paramArgs.put(RequestParameters.INCLUDE, includes);
+                    paramArgs.put(RequestParameters.EXCLUDE, excludes);
+                    paramArgs.put(RequestParameters.METADATA, returnMetadata);
+                    paramArgs.put(RequestParameters.LIMIT, String.valueOf(limit));
+                    paramArgs.put(RequestParameters.OFFSET, String.valueOf(offset));
+
+                    String baseUrl = httpServletRequest.getRequestURL().toString();
                     MongoMetaDataResponseBuilder.MetaBuilder metaDataResponse = new MongoMetaDataResponseBuilder.MetaBuilder();
                     MongoMetaDataGenerator metaDataGenerator = new MongoMetaDataGenerator(limit, offset, sort, queries,
-                            includes, excludes, httpServletRequest, execResult);
+                            includes, excludes, paramArgs, execResult, baseUrl);
 
                     metaDataGenerator.setDocAndPayloadCount(metaDataResponse);
                     metaDataGenerator.setPrev(metaDataResponse);
--- a/common/mongodb/src/test/java/com/redhat/thermostat/gateway/common/mongodb/response/MongoMetaDataGeneratorTest.java	Tue Aug 29 14:25:19 2017 +0200
+++ b/common/mongodb/src/test/java/com/redhat/thermostat/gateway/common/mongodb/response/MongoMetaDataGeneratorTest.java	Wed Aug 30 12:00:11 2017 -0400
@@ -37,9 +37,12 @@
 package com.redhat.thermostat.gateway.common.mongodb.response;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.util.LinkedHashMap;
+
 import javax.servlet.http.HttpServletRequest;
 
 import com.redhat.thermostat.gateway.common.mongodb.servlet.RequestParameters;
@@ -52,18 +55,21 @@
 
     private MongoMetaDataGenerator fullGenerator;
     private MongoMetaDataResponseBuilder.MetaBuilder response;
+    private final String baseUrl = "127.0.0.1:8080/base/";
 
     @Before
     public void setup() {
         HttpServletRequest requestInfo = mock(HttpServletRequest.class);
-        when(requestInfo.getRequestURL()).thenReturn(new StringBuffer("127.0.0.1:8080/base/"));
+        when(requestInfo.getRequestURL()).thenReturn(new StringBuffer(baseUrl));
         when(requestInfo.getQueryString()).thenReturn(RequestParameters.LIMIT + "=2&" + RequestParameters.OFFSET + "=2&" + RequestParameters.METADATA + "=true");
 
         MongoDataResultContainer container = new MongoDataResultContainer();
         container.setRemainingNumQueryDocuments(1);
         container.setGetReqCount(4);
+        LinkedHashMap<String, String> paramArgs = new LinkedHashMap<>();
+        paramArgs.put(RequestParameters.METADATA, "true");
 
-        fullGenerator = new MongoMetaDataGenerator(2, 2, "", "test1==b", "", "", requestInfo, container);
+        fullGenerator = new MongoMetaDataGenerator(2, 2, "", "test1==b", "", "", paramArgs, container, baseUrl);
         response = new MongoMetaDataResponseBuilder.MetaBuilder();
     }
 
@@ -92,10 +98,26 @@
         fullGenerator.setNext(response);
         String output = response.build().toString();
 
-        // {"payloadCount":1,"next":"127.0.0.1:8080/base/?o=4&l=1&m=true"}//{"payloadCount":1,"next":"127.0.0.1:8080/base/?o=4&l=1&m=true"}
+        // {"payloadCount":1,"next":"127.0.0.1:8080/base/?o=4&l=1&m=true"}
         String expected = "{\"payloadCount\":1,\"next\":\"127.0.0.1:8080/base/?offset\\u003d4\\u0026limit\\u003d1\\u0026" + RequestParameters.METADATA + "\\u003dtrue\"}";
 
         assertEquals(expected, output);
     }
 
+    @Test
+    public void testGetQueryArgumentsNoOffsetLimitWithEmptyValue() {
+        LinkedHashMap<String, String> paramArgs = new LinkedHashMap<>();
+
+        paramArgs.put(RequestParameters.METADATA, "true");
+        paramArgs.put("foo", "bar");
+        paramArgs.put("baz", null);
+
+        String output = response.getQueryArgumentsNoOffsetLimit(paramArgs);
+
+        // We expect "baz" to be dropped entirely after the call; this cannot be avoided unless the response
+        // handler helpers are refactored
+        String expected = "metadata=true&foo=bar";
+
+        assertEquals(expected, output);
+    }
 }