Merge pull request #519 from hiddenalpha/RandomImprovements-20230628

Cleanup random findings
swisspost · Oct 5, 2023 · 289da09 · 289da09
2 parents 16c28a5 + 1572a59
commit 289da09
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 67 deletions.
diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml
@@ -12,6 +12,7 @@ on:
 jobs:
   build_maven:
     runs-on: ubuntu-latest
+    timeout-minutes: 30
 
     steps:
       - uses: actions/checkout@v3

diff --git a/gateleen-core/src/main/java/org/swisspush/gateleen/core/http/AbstractHttpClient.java b/gateleen-core/src/main/java/org/swisspush/gateleen/core/http/AbstractHttpClient.java
@@ -4,6 +4,7 @@
 import io.vertx.core.Future;
 import io.vertx.core.Handler;
 import io.vertx.core.MultiMap;
+import io.vertx.core.Promise;
 import io.vertx.core.Vertx;
 import io.vertx.core.http.*;
 
@@ -60,12 +61,14 @@ public Function<HttpClientResponse, Future<RequestOptions>> redirectHandler() {
 
     @Override
     public void close(Handler<AsyncResult<Void>> handler) {
-        throw new UnsupportedOperationException();
+        close().onComplete(handler);
     }
 
     @Override
     public Future<Void> close() {
-        throw new UnsupportedOperationException();
+        Promise<Void> promise = Promise.promise();
+        vertx.runOnContext(v -> promise.complete());
+        return promise.future();
     }
 
     @Override
@@ -90,6 +93,7 @@ public Future<HttpClientRequest> request(HttpMethod httpMethod, int i, String s,
 
     @Override
     public void request(HttpMethod httpMethod, String s, String s1, Handler<AsyncResult<HttpClientRequest>> handler) {
+        throw new UnsupportedOperationException();
     }
 
     @Override
@@ -161,4 +165,4 @@ public Future<WebSocket> webSocketAbs(String s, MultiMap multiMap, WebsocketVers
     public boolean isMetricsEnabled() {
         throw new UnsupportedOperationException();
     }
-}
+}
diff --git a/gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java b/gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java
@@ -5,16 +5,24 @@
 import io.vertx.core.MultiMap;
 import io.vertx.core.Vertx;
 import io.vertx.core.buffer.Buffer;
-import io.vertx.core.http.*;
+import io.vertx.core.http.HttpClient;
+import io.vertx.core.http.HttpClientRequest;
+import io.vertx.core.http.HttpClientResponse;
+import io.vertx.core.http.HttpMethod;
+import io.vertx.core.http.HttpServerRequest;
 import io.vertx.core.json.JsonArray;
 import io.vertx.core.json.JsonObject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.swisspush.gateleen.core.http.RequestLoggerFactory;
 import org.swisspush.gateleen.core.storage.ResourceStorage;
-import org.swisspush.gateleen.core.util.*;
+import org.swisspush.gateleen.core.util.ExpansionDeltaUtil;
 import org.swisspush.gateleen.core.util.ExpansionDeltaUtil.CollectionResourceContainer;
 import org.swisspush.gateleen.core.util.ExpansionDeltaUtil.SlashHandling;
+import org.swisspush.gateleen.core.util.HttpServerRequestUtil;
+import org.swisspush.gateleen.core.util.ResourceCollectionException;
+import org.swisspush.gateleen.core.util.ResponseStatusCodeLogUtil;
+import org.swisspush.gateleen.core.util.StatusCode;
 import org.swisspush.gateleen.routing.Rule;
 import org.swisspush.gateleen.routing.RuleFeaturesProvider;
 import org.swisspush.gateleen.routing.RuleProvider;
@@ -25,6 +33,7 @@
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import static org.swisspush.gateleen.core.util.StatusCode.INTERNAL_SERVER_ERROR;
 import static org.swisspush.gateleen.routing.RuleFeatures.Feature.EXPAND_ON_BACKEND;
 import static org.swisspush.gateleen.routing.RuleFeatures.Feature.STORAGE_EXPAND;
 import static org.swisspush.gateleen.routing.RuleProvider.RuleChangesObserver;
@@ -491,7 +500,9 @@ private void removeZipParameter(final HttpServerRequest request) {
 
     private void makeStorageExpandRequest(final String targetUri, final List subResourceNames, final HttpServerRequest req, final DeltaHandler<ResourceNode> handler) {
         Logger log = RequestLoggerFactory.getLogger(ExpansionHandler.class, req);
-        httpClient.request(HttpMethod.POST, targetUri + "?storageExpand=true").onComplete(asyncResult -> {
+        HttpMethod reqMethod = HttpMethod.POST;
+        String reqUri = targetUri + "?storageExpand=true";
+        httpClient.request(reqMethod, reqUri).onComplete(asyncResult -> {
             if (asyncResult.failed()) {
                 log.warn("Failed request to {}: {}", targetUri + "?storageExpand=true", asyncResult.cause());
                 return;
@@ -512,16 +523,23 @@ private void makeStorageExpandRequest(final String targetUri, final List subReso
             cReq.write(payload);
 
             cReq.send(event -> {
+                if (event.failed()) {
+                    Throwable ex = event.cause();
+                    log.debug("{} {}", reqMethod, reqUri, ex);
+                    var exWrappr = new ResourceCollectionException(ex.getMessage(), INTERNAL_SERVER_ERROR);
+                    handler.handle(new ResourceNode(SERIOUS_EXCEPTION, exWrappr));
+                }
                 HttpClientResponse cRes = event.result();
                 cRes.bodyHandler(data -> {
                     if (StatusCode.NOT_FOUND.getStatusCode() == cRes.statusCode()) {
-                        log.debug("requested resource could not be found: {}", targetUri);
+                        log.debug("NotFound: {}", targetUri);
                         handler.handle(new ResourceNode(SERIOUS_EXCEPTION, new ResourceCollectionException(cRes.statusMessage(), StatusCode.NOT_FOUND)));
                     } else if (StatusCode.INTERNAL_SERVER_ERROR.getStatusCode() == cRes.statusCode()) {
-                        log.error("error in request resource : {} message : {}", targetUri, data.toString());
-                        handler.handle(new ResourceNode(SERIOUS_EXCEPTION, new ResourceCollectionException(data.toString(), StatusCode.INTERNAL_SERVER_ERROR)));
+                        String fullResponseBody = data.toString();
+                        log.error("{}: {}: {}", INTERNAL_SERVER_ERROR, targetUri, fullResponseBody);
+                        handler.handle(new ResourceNode(SERIOUS_EXCEPTION, new ResourceCollectionException(fullResponseBody, StatusCode.INTERNAL_SERVER_ERROR)));
                     } else if (StatusCode.METHOD_NOT_ALLOWED.getStatusCode() == cRes.statusCode()) {
-                        log.error("POST requests (storageExpand) not allowed for uri: {}", targetUri);
+                        log.error("storageExpand not allowed for: {}", targetUri);
                         handler.handle(new ResourceNode(SERIOUS_EXCEPTION, new ResourceCollectionException(cRes.statusMessage(), StatusCode.METHOD_NOT_ALLOWED)));
                     } else {
                         String eTag = geteTag(cRes.headers());

diff --git a/gateleen-hook/src/main/java/org/swisspush/gateleen/hook/HookHandler.java b/gateleen-hook/src/main/java/org/swisspush/gateleen/hook/HookHandler.java
@@ -47,6 +47,8 @@
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
+import static io.vertx.core.http.HttpMethod.DELETE;
+import static io.vertx.core.http.HttpMethod.PUT;
 import static org.swisspush.gateleen.core.util.HttpRequestHeader.CONTENT_LENGTH;
 
 /**
@@ -502,24 +504,28 @@ public boolean handle(final RoutingContext ctx) {
         /*
          * 1) Un- / Register Listener / Routes
          */
-        if (isHookListenerRegistration(request)) {
-            handleListenerRegistration(request);
-            return true;
-        }
-
-        if (isHookListenerUnregistration(request)) {
-            handleListenerUnregistration(request);
-            return true;
-        }
-
-        if (isHookRouteRegistration(request)) {
-            handleRouteRegistration(request);
-            return true;
+        var requestMethod = request.method();
+        if (requestMethod == PUT) {
+            var requestUri = request.uri();
+            if (requestUri.contains(HOOKS_LISTENERS_URI_PART)) {
+                handleListenerRegistration(request);
+                return true;
+            }
+            if (requestUri.contains(HOOKS_ROUTE_URI_PART)) {
+                handleRouteRegistration(request);
+                return true;
+            }
         }
-
-        if (isHookRouteUnregistration(request)) {
-            handleRouteUnregistration(request);
-            return true;
+        if (requestMethod == DELETE) {
+            var requestUri = request.uri();
+            if (requestUri.contains(HOOKS_LISTENERS_URI_PART)) {
+                handleListenerUnregistration(request);
+                return true;
+            }
+            if (requestUri.contains(HOOKS_ROUTE_URI_PART)) {
+                handleRouteUnregistration(request);
+                return true;
+            }
         }
 
         /*
@@ -1654,46 +1660,6 @@ private String getListenerUrlSegment(String requestUrl) {
         return requestUrl.substring(pos + HOOKS_LISTENERS_URI_PART.length());
     }
 
-    /**
-     * Checks if the given request is a listener unregistration instruction.
-     *
-     * @param request request
-     * @return boolean
-     */
-    private boolean isHookListenerUnregistration(HttpServerRequest request) {
-        return request.uri().contains(HOOKS_LISTENERS_URI_PART) && HttpMethod.DELETE == request.method();
-    }
-
-    /**
-     * Checks if the given request is a listener registration instruction.
-     *
-     * @param request request
-     * @return boolean
-     */
-    private boolean isHookListenerRegistration(HttpServerRequest request) {
-        return request.uri().contains(HOOKS_LISTENERS_URI_PART) && HttpMethod.PUT == request.method();
-    }
-
-    /**
-     * Checks if the given request is a route registration instruction.
-     *
-     * @param request request
-     * @return boolean
-     */
-    private boolean isHookRouteRegistration(HttpServerRequest request) {
-        return request.uri().contains(HOOKS_ROUTE_URI_PART) && HttpMethod.PUT == request.method();
-    }
-
-    /**
-     * Checks if the given request is a route registration instruction.
-     *
-     * @param request request
-     * @return boolean
-     */
-    private boolean isHookRouteUnregistration(HttpServerRequest request) {
-        return request.uri().contains(HOOKS_ROUTE_URI_PART) && HttpMethod.DELETE == request.method();
-    }
-
     /**
      * @param request Request to extract the value from. This instance gets manipulated
      *                internally during call.