From 34215a582f6c965c48335c8d500338b6cbe977b8 Mon Sep 17 00:00:00 2001 From: Andreas Fankhauser <23085769+hiddenalpha@users.noreply.github.com> Date: Thu, 23 May 2024 15:36:48 +0200 Subject: [PATCH] Fix some for 'https://github.com/swisspost/gateleen/pull/577#pullrequestreview-2070233347' Related: SDCISA-15833, https://github.com/swisspost/vertx-redisques/issues/170, https://github.com/swisspost/vertx-redisques/pull/177, https://github.com/swisspost/vertx-rest-storage/pull/186, https://github.com/swisspost/gateleen/pull/577 --- .../ConfigurationResourceManager.java | 19 ++++++++----- .../exception/GateleenExceptionFactory.java | 8 +++++- .../core/http/LocalHttpClientRequest.java | 15 ++++++++--- .../lock/lua/ReleaseLockRedisCommand.java | 25 ++++++++++++----- .../gateleen/core/lua/LuaScriptState.java | 20 +++++++++----- .../core/resource/CopyResourceHandler.java | 27 ++++++++++++------- 6 files changed, 82 insertions(+), 32 deletions(-) diff --git a/gateleen-core/src/main/java/org/swisspush/gateleen/core/configuration/ConfigurationResourceManager.java b/gateleen-core/src/main/java/org/swisspush/gateleen/core/configuration/ConfigurationResourceManager.java index 5e2a97711..0583ff8b2 100644 --- a/gateleen-core/src/main/java/org/swisspush/gateleen/core/configuration/ConfigurationResourceManager.java +++ b/gateleen-core/src/main/java/org/swisspush/gateleen/core/configuration/ConfigurationResourceManager.java @@ -11,6 +11,7 @@ import io.vertx.core.json.JsonObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.swisspush.gateleen.core.exception.GateleenExceptionFactory; import org.swisspush.gateleen.core.http.RequestLoggerFactory; import org.swisspush.gateleen.core.logging.LoggableResource; import org.swisspush.gateleen.core.logging.RequestLogger; @@ -35,6 +36,7 @@ public class ConfigurationResourceManager implements LoggableResource { private final Vertx vertx; private final ResourceStorage storage; + private final GateleenExceptionFactory exceptionFactory; private Map registeredResources; private Map> observers; private final ConfigurationResourceValidator configurationResourceValidator; @@ -44,9 +46,14 @@ public class ConfigurationResourceManager implements LoggableResource { private static final String MESSAGE_REQUEST_URI = "requestUri"; private static final String MESSAGE_RESOURCE_TYPE = "type"; - public ConfigurationResourceManager(Vertx vertx, final ResourceStorage storage) { + public ConfigurationResourceManager( + Vertx vertx, + ResourceStorage storage, + GateleenExceptionFactory exceptionFactory + ) { this.vertx = vertx; this.storage = storage; + this.exceptionFactory = exceptionFactory; this.configurationResourceValidator = new ConfigurationResourceValidator(vertx); @@ -192,11 +199,11 @@ private Future> getValidatedRegisteredResource(String resourceU if (event.result().isSuccess()) { promise.complete(Optional.of(buffer)); } else { - promise.fail(new Exception("Failure during validation of resource " + promise.fail(exceptionFactory.newException("Failure during validation of resource " + resourceUri + ". Message: " + event.result().getMessage())); } } else { - promise.fail(new Exception("ReleaseLockRedisCommand request failed", event.cause())); + promise.fail(exceptionFactory.newException("ReleaseLockRedisCommand request failed", event.cause())); } }); } else { @@ -217,9 +224,9 @@ private void notifyObserversAboutRemovedResource(String requestUri) { private void notifyObserverAboutResourceChange(String requestUri, ConfigurationResourceObserver observer) { getValidatedRegisteredResource(requestUri).onComplete(event -> { - if(event.failed()){ - log.warn("TODO error handling", new Exception(event.cause())); - } else if(event.result().isPresent()){ + if (event.failed()) { + log.warn("stacktrace", exceptionFactory.newException("TODO error handling", event.cause())); + } else if (event.result().isPresent()) { if(observer != null) { observer.resourceChanged(requestUri, event.result().get()); } else { diff --git a/gateleen-core/src/main/java/org/swisspush/gateleen/core/exception/GateleenExceptionFactory.java b/gateleen-core/src/main/java/org/swisspush/gateleen/core/exception/GateleenExceptionFactory.java index 9154af415..af7db1e55 100644 --- a/gateleen-core/src/main/java/org/swisspush/gateleen/core/exception/GateleenExceptionFactory.java +++ b/gateleen-core/src/main/java/org/swisspush/gateleen/core/exception/GateleenExceptionFactory.java @@ -28,7 +28,13 @@ */ public interface GateleenExceptionFactory { - public Exception newException(String message, Throwable cause); + /** Convenience overload for {@link #newException(String, Throwable)}. */ + public default Exception newException(String msg){ return newException(msg, null); } + + /** Convenience overload for {@link #newException(String, Throwable)}. */ + public default Exception newException(Throwable cause){ return newException(null, cause); } + + public Exception newException(String msg, Throwable cause); public ReplyException newReplyException(ReplyFailure failureType, int failureCode, String message); diff --git a/gateleen-core/src/main/java/org/swisspush/gateleen/core/http/LocalHttpClientRequest.java b/gateleen-core/src/main/java/org/swisspush/gateleen/core/http/LocalHttpClientRequest.java index fb152839e..5cdc3b710 100644 --- a/gateleen-core/src/main/java/org/swisspush/gateleen/core/http/LocalHttpClientRequest.java +++ b/gateleen-core/src/main/java/org/swisspush/gateleen/core/http/LocalHttpClientRequest.java @@ -15,6 +15,7 @@ import io.vertx.ext.auth.User; import io.vertx.ext.web.*; import org.slf4j.Logger; +import org.swisspush.gateleen.core.exception.GateleenExceptionFactory; import javax.net.ssl.SSLSession; import javax.security.cert.X509Certificate; @@ -44,6 +45,7 @@ public class LocalHttpClientRequest extends BufferBridge implements FastFailHttp private HttpServerResponse serverResponse; private final HttpConnection connection; private Handler routingContextHandler; + private final GateleenExceptionFactory exceptionFactory; private boolean bound = false; private static final SocketAddress address = new SocketAddressImpl(0, "localhost"); @@ -538,11 +540,19 @@ public MultiMap queryParams(Charset charset) { } }; - public LocalHttpClientRequest(HttpMethod method, String uri, Vertx vertx, Handler routingContextHandler, HttpServerResponse response) { + public LocalHttpClientRequest( + HttpMethod method, + String uri, + Vertx vertx, + Handler routingContextHandler, + GateleenExceptionFactory exceptionFactory, + HttpServerResponse response + ) { super(vertx); this.method = method; this.uri = uri; this.routingContextHandler = routingContextHandler; + this.exceptionFactory = exceptionFactory; this.serverResponse = response; this.connection = new LocalHttpConnection(); } @@ -854,8 +864,7 @@ public boolean writeQueueFull() { @Override public HttpClientRequest drainHandler(Handler handler) { - log.warn("Happy debugging, as this impl will just ignore your drainHandler anyway", - new Exception("may this stacktrace lead you where this problem comes from")); + log.warn("stacktrace", exceptionFactory.newException("TODO impl drainHandler")); return this; } diff --git a/gateleen-core/src/main/java/org/swisspush/gateleen/core/lock/lua/ReleaseLockRedisCommand.java b/gateleen-core/src/main/java/org/swisspush/gateleen/core/lock/lua/ReleaseLockRedisCommand.java index 03f6e6281..c7c058dcf 100644 --- a/gateleen-core/src/main/java/org/swisspush/gateleen/core/lock/lua/ReleaseLockRedisCommand.java +++ b/gateleen-core/src/main/java/org/swisspush/gateleen/core/lock/lua/ReleaseLockRedisCommand.java @@ -3,6 +3,7 @@ import io.vertx.core.Promise; import io.vertx.redis.client.RedisAPI; import org.slf4j.Logger; +import org.swisspush.gateleen.core.exception.GateleenExceptionFactory; import org.swisspush.gateleen.core.lua.LuaScriptState; import org.swisspush.gateleen.core.lua.RedisCommand; import org.swisspush.gateleen.core.redis.RedisProvider; @@ -20,14 +21,23 @@ public class ReleaseLockRedisCommand implements RedisCommand { private List arguments; private Promise promise; private RedisProvider redisProvider; + private final GateleenExceptionFactory exceptionFactory; private Logger log; - public ReleaseLockRedisCommand(LuaScriptState luaScriptState, List keys, List arguments, - RedisProvider redisProvider, Logger log, final Promise promise) { + public ReleaseLockRedisCommand( + LuaScriptState luaScriptState, + List keys, + List arguments, + RedisProvider redisProvider, + GateleenExceptionFactory exceptionFactory, + Logger log, + final Promise promise + ) { this.luaScriptState = luaScriptState; this.keys = keys; this.arguments = arguments; this.redisProvider = redisProvider; + this.exceptionFactory = exceptionFactory; this.log = log; this.promise = promise; } @@ -42,7 +52,7 @@ public void exec(int executionCounter) { redisProvider.redis().onComplete( redisEv -> { if( redisEv.failed() ){ - promise.fail(new Exception("redisProvider.redis()", redisEv.cause())); + promise.fail(exceptionFactory.newException("redisProvider.redis() failed", redisEv.cause())); return; } RedisAPI redisAPI = redisEv.result(); @@ -54,16 +64,17 @@ public void exec(int executionCounter) { Throwable ex = event.cause(); String message = ex.getMessage(); if (message != null && message.startsWith("NOSCRIPT")) { - log.warn("ReleaseLockRedisCommand script couldn't be found, reload it", new Exception("stacktrace",ex)); + log.warn("ReleaseLockRedisCommand script couldn't be found, reload it", + exceptionFactory.newException(ex)); log.warn("amount the script got loaded: {}", executionCounter); if (executionCounter > 10) { - promise.fail(new Exception("amount the script got loaded is higher than 10, we abort")); + promise.fail(exceptionFactory.newException("amount the script got loaded is higher than 10, we abort")); } else { luaScriptState.loadLuaScript(new ReleaseLockRedisCommand(luaScriptState, keys, - arguments, redisProvider, log, promise), executionCounter); + arguments, redisProvider, exceptionFactory, log, promise), executionCounter); } } else { - promise.fail(new Exception("ReleaseLockRedisCommand request failed", ex)); + promise.fail(exceptionFactory.newException("ReleaseLockRedisCommand request failed", ex)); } } }); diff --git a/gateleen-core/src/main/java/org/swisspush/gateleen/core/lua/LuaScriptState.java b/gateleen-core/src/main/java/org/swisspush/gateleen/core/lua/LuaScriptState.java index 5bb5e92f1..81f000928 100644 --- a/gateleen-core/src/main/java/org/swisspush/gateleen/core/lua/LuaScriptState.java +++ b/gateleen-core/src/main/java/org/swisspush/gateleen/core/lua/LuaScriptState.java @@ -4,6 +4,7 @@ import org.apache.commons.codec.digest.DigestUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.swisspush.gateleen.core.exception.GateleenExceptionFactory; import org.swisspush.gateleen.core.redis.RedisProvider; import java.io.BufferedReader; @@ -30,12 +31,19 @@ public class LuaScriptState { private String sha; private RedisProvider redisProvider; + private final GateleenExceptionFactory exceptionFactory; private Logger log = LoggerFactory.getLogger(LuaScriptState.class); - public LuaScriptState(LuaScript luaScriptType, RedisProvider redisProvider, boolean logoutput) { + public LuaScriptState( + LuaScript luaScriptType, + RedisProvider redisProvider, + GateleenExceptionFactory exceptionFactory, + boolean logoutput + ) { this.luaScriptType = luaScriptType; this.redisProvider = redisProvider; + this.exceptionFactory = exceptionFactory; this.logoutput = logoutput; this.composeLuaScript(luaScriptType); this.loadLuaScript(new RedisCommandDoNothing(), 0); @@ -87,17 +95,17 @@ private String readLuaScriptFromClasspath(LuaScript luaScriptType) { public void loadLuaScript(final RedisCommand redisCommand, int executionCounter) { final int executionCounterIncr = ++executionCounter; // check first if the lua script already exists in the store - redisProvider.redis().onComplete( redisEv -> { + redisProvider.redis().onComplete(redisEv -> { if (redisEv.failed()) { - log.error("Redis: Error checking whether lua script exists", - new Exception("stacktrace", redisEv.cause())); + log.error("stacktrace", exceptionFactory.newException( + "redisProvider.redis() failed", redisEv.cause())); return; } RedisAPI redisAPI = redisEv.result(); redisAPI.script(Arrays.asList("exists", sha), resultArray -> { if (resultArray.failed()) { - log.error("Error checking whether lua script exists", - new Exception("stacktrace", resultArray.cause())); + log.error("stacktrace", exceptionFactory.newException( + "redisAPI.script(['exists', sha]) failed", resultArray.cause())); return; } Long exists = resultArray.result().get(0).toLong(); diff --git a/gateleen-core/src/main/java/org/swisspush/gateleen/core/resource/CopyResourceHandler.java b/gateleen-core/src/main/java/org/swisspush/gateleen/core/resource/CopyResourceHandler.java index f0913a6e7..17709e912 100644 --- a/gateleen-core/src/main/java/org/swisspush/gateleen/core/resource/CopyResourceHandler.java +++ b/gateleen-core/src/main/java/org/swisspush/gateleen/core/resource/CopyResourceHandler.java @@ -7,6 +7,7 @@ import io.vertx.core.json.JsonObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.swisspush.gateleen.core.exception.GateleenExceptionFactory; import org.swisspush.gateleen.core.http.HeaderFunction; import org.swisspush.gateleen.core.http.HeaderFunctions; import org.swisspush.gateleen.core.http.RequestLoggerFactory; @@ -25,11 +26,13 @@ public class CopyResourceHandler { private static final String SLASH = "/"; private static final int DEFAULT_TIMEOUT = 120000; - private final String copyPath; private final HttpClient selfClient; + private final GateleenExceptionFactory exceptionFactory; + private final String copyPath; - public CopyResourceHandler(HttpClient selfClient, String copyPath) { + public CopyResourceHandler(HttpClient selfClient, GateleenExceptionFactory exceptionFactory, String copyPath) { this.selfClient = selfClient; + this.exceptionFactory = exceptionFactory; this.copyPath = copyPath; } @@ -94,7 +97,8 @@ protected void performGETRequest(final HttpServerRequest request, final CopyTask selfClient.request(HttpMethod.GET, task.getSourceUri()).onComplete(event -> { if (event.failed()) { - log.warn("Failed request to {}", request.uri(), new Exception("stacktrace", event.cause())); + log.warn("stacktrace", exceptionFactory.newException( + "Failed request to " + request.uri(), event.cause())); return; } HttpClientRequest selfRequest = event.result(); @@ -105,12 +109,16 @@ protected void performGETRequest(final HttpServerRequest request, final CopyTask selfRequest.idleTimeout(DEFAULT_TIMEOUT); // add exception handler - selfRequest.exceptionHandler( ex -> log.warn("CopyResourceHandler: GET request failed: {}", request.uri(), new Exception("stacktrace", ex))); + selfRequest.exceptionHandler(ex -> { + log.warn("stacktrace", exceptionFactory.newException( + "CopyResourceHandler: GET request failed: " + request.uri(), ex)); + }); // fire selfRequest.send(asyncResult -> { - if( asyncResult.failed() ){ - log.warn("stacktrace", new Exception("stacktrace", asyncResult.cause())); + if (asyncResult.failed()) { + log.warn("stacktrace", exceptionFactory.newException( + "selfRequest.send() failed", asyncResult.cause())); return; } HttpClientResponse response = asyncResult.result(); @@ -138,7 +146,7 @@ protected void performPUTRequest(final HttpServerRequest request, final HttpClie selfClient.request(HttpMethod.PUT, task.getDestinationUri()).onComplete(event -> { if (event.failed()) { - log.warn("Failed request to {}", request.uri(), new Exception("stacktrace", event.cause())); + log.warn("Failed request to {}", request.uri(), exceptionFactory.newException(event.cause())); return; } HttpClientRequest selfRequest = event.result(); @@ -154,8 +162,9 @@ protected void performPUTRequest(final HttpServerRequest request, final HttpClie // fire selfRequest.send(asyncResult -> { - if( asyncResult.failed() ){ - log.warn("stacktrace", new Exception("stacktrace", asyncResult.cause())); + if (asyncResult.failed()) { + log.warn("stacktrace", exceptionFactory.newException( + "selfRequest.send() failed", asyncResult.cause())); return; } HttpClientResponse response = asyncResult.result();