Skip to content

Commit

Permalink
[SDCISA-15833, swisspost/vertx-redisques#170] Introduce ExceptionFactory
Browse files Browse the repository at this point in the history
  • Loading branch information
hiddenalpha committed May 21, 2024
1 parent cd579e0 commit 7d1e970
Show file tree
Hide file tree
Showing 10 changed files with 185 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package org.swisspush.gateleen.core.exception;

import io.vertx.core.eventbus.ReplyException;
import io.vertx.core.eventbus.ReplyFailure;


/**
* Applies dependency inversion for exception instantiation.
*
* This class did arise because we had different use cases in different
* applications. One of them has the need to perform fine-grained error
* reporting. Whereas in the other application this led to performance issues.
* So now through this abstraction, both applications can choose the behavior
* they need.
*
* If dependency-injection gets applied properly, an app can even provide its
* custom implementation to fine-tune the exact behavior even further.
*/
public interface GateleenExceptionFactory {

public Exception newException(String message, Throwable cause);

public ReplyException newReplyException(ReplyFailure failureType, int failureCode, String message);


/**
* See {@link GateleenThriftyExceptionFactory}.
*/
public static GateleenExceptionFactory newGateleenThriftyExceptionFactory() {
return new GateleenThriftyExceptionFactory();
}

/**
* See {@link GateleenWastefulExceptionFactory}.
*/
public static GateleenExceptionFactory newGateleenWastefulExceptionFactory() {
return new GateleenWastefulExceptionFactory();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package org.swisspush.gateleen.core.exception;

import io.vertx.core.eventbus.ReplyFailure;

/**
* There was once a fix in vertx for this (https://github.com/eclipse-vertx/vert.x/issues/4840)
* but for whatever reason in our case we still see stack-trace recordings. Passing
* this subclass to {@link io.vertx.core.eventbus.Message#reply(Object)} seems to
* do the trick.
*/
public class GateleenNoStackReplyException extends io.vertx.core.eventbus.ReplyException {

public GateleenNoStackReplyException(ReplyFailure failureType, int failureCode, String message) {
super(failureType, failureCode, message);
}

public GateleenNoStackReplyException(ReplyFailure failureType, String message) {
this(failureType, -1, message);
}

public GateleenNoStackReplyException(ReplyFailure failureType) {
this(failureType, -1, null);
}

@Override
public Throwable fillInStackTrace() {
return this;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.swisspush.gateleen.core.exception;

/**
* Basically same as in vertx, But adding the forgotten contructors.
*/
public class GateleenNoStacktraceException extends RuntimeException {

public GateleenNoStacktraceException() {
}

public GateleenNoStacktraceException(String message) {
super(message);
}

public GateleenNoStacktraceException(String message, Throwable cause) {
super(message, cause);
}

public GateleenNoStacktraceException(Throwable cause) {
super(cause);
}

@Override
public Throwable fillInStackTrace() {
return this;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.swisspush.gateleen.core.exception;

import io.vertx.core.eventbus.ReplyException;
import io.vertx.core.eventbus.ReplyFailure;

/**
* Trades maintainability for speed. For example prefers lightweight
* exceptions without stacktrace recording. It may even decide to drop 'cause'
* or other details. If an app needs more error details it should use
* {@link GateleenWastefulExceptionFactory}. If none of those fits the apps needs, it
* can provide its own implementation.
*/
class GateleenThriftyExceptionFactory implements GateleenExceptionFactory {

GateleenThriftyExceptionFactory() {
}

public Exception newException(String message, Throwable cause) {
if (cause instanceof Exception) return (Exception) cause;
return new GateleenNoStacktraceException(message, cause);
}

@Override
public ReplyException newReplyException(ReplyFailure failureType, int failureCode, String message) {
return new GateleenNoStackReplyException(failureType, failureCode, message);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.swisspush.gateleen.core.exception;

import io.vertx.core.eventbus.ReplyException;
import io.vertx.core.eventbus.ReplyFailure;

/**
* Trades speed for maintainability. For example invests more resources like
* recording stack traces (which likely provocates more logs) to get easier
* to debug error messages and better hints of what is happening. It also
* keeps details like 'causes' and 'suppressed' exceptions. If an app needs
* more error details it should use {@link GateleenWastefulExceptionFactory}. If none
* of those fits the apps needs, it can provide its own implementation.
*/
class GateleenWastefulExceptionFactory implements GateleenExceptionFactory {

GateleenWastefulExceptionFactory() {
}

public Exception newException(String message, Throwable cause) {
return new Exception(message, cause);
}

@Override
public ReplyException newReplyException(ReplyFailure failureType, int failureCode, String message) {
return new ReplyException(failureType, failureCode, message);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import io.vertx.redis.client.Response;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.swisspush.gateleen.core.exception.GateleenExceptionFactory;
import org.swisspush.gateleen.core.lock.Lock;
import org.swisspush.gateleen.core.lock.lua.LockLuaScripts;
import org.swisspush.gateleen.core.lock.lua.ReleaseLockRedisCommand;
Expand All @@ -28,14 +29,17 @@
public class RedisBasedLock implements Lock {

private static final Logger log = LoggerFactory.getLogger(RedisBasedLock.class);
private static final String[] EMPTY_STRING_ARRAY = new String[0];

public static final String STORAGE_PREFIX = "gateleen.core-lock:";

private final LuaScriptState releaseLockLuaScriptState;
private final RedisProvider redisProvider;
private final GateleenExceptionFactory exceptionFactory;

public RedisBasedLock(RedisProvider redisProvider) {
public RedisBasedLock(RedisProvider redisProvider, GateleenExceptionFactory exceptionFactory) {
this.redisProvider = redisProvider;
this.exceptionFactory = exceptionFactory;
this.releaseLockLuaScriptState = new LuaScriptState(LockLuaScripts.LOCK_RELEASE, redisProvider, false);
}

Expand All @@ -47,31 +51,38 @@ private void redisSetWithOptions(String key, String value, boolean nx, long px,
}
redisProvider.redis().onComplete( redisEv -> {
if( redisEv.failed() ){
handler.handle(new FailedAsyncResult<>(redisEv.cause()));
Throwable ex = exceptionFactory.newException("redisProvider.redis() failed", redisEv.cause());
handler.handle(new FailedAsyncResult<>(ex));
return;
}
var redisAPI = redisEv.result();
redisAPI.send(Command.SET, RedisUtils.toPayload(key, value, options).toArray(new String[0]))
.onComplete( ev -> {
if( ev.failed() && log.isInfoEnabled() ) log.info("stacktrace", new Exception("stacktrace", ev.cause()));
handler.handle(ev);
});
String[] payload = RedisUtils.toPayload(key, value, options).toArray(EMPTY_STRING_ARRAY);
redisAPI.send(Command.SET, payload).onComplete(ev -> {
if (ev.failed()) {
Throwable ex = exceptionFactory.newException("redisAPI.send() failed", ev.cause());
handler.handle(new FailedAsyncResult<>(ex));
return;
}
handler.handle(ev);
});
});
}

@Override
public Future<Boolean> acquireLock(String lock, String token, long lockExpiryMs) {
Promise<Boolean> promise = Promise.promise();
redisSetWithOptions(buildLockKey(lock), token, true, lockExpiryMs, event -> {
String lockKey = buildLockKey(lock);
redisSetWithOptions(lockKey, token, true, lockExpiryMs, event -> {
if (event.succeeded()) {
if (event.result() != null) {
promise.complete("OK".equalsIgnoreCase(event.result().toString()));
} else {
promise.complete(false);
}
} else {
if( log.isInfoEnabled() ) log.info("stacktrace", new Exception("stacktrace", event.cause()));
promise.fail(event.cause().getMessage());
Throwable ex = exceptionFactory.newException(
"redisSetWithOptions(lockKey=\"" + lockKey + "\") failed", event.cause());
promise.fail(ex);
}
});
return promise.future();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.swisspush.gateleen.core.exception.GateleenExceptionFactory;
import redis.clients.jedis.Jedis;
import redis.clients.jedis.exceptions.JedisConnectionException;

import java.time.Duration;

import static org.awaitility.Awaitility.await;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.swisspush.gateleen.core.exception.GateleenExceptionFactory.newGateleenWastefulExceptionFactory;

/**
* Tests for the {@link RedisBasedLock} class
Expand All @@ -48,7 +50,7 @@ public static void setupLock(){
vertx = Vertx.vertx();
RedisAPI redisAPI = RedisAPI.api(new RedisClient(vertx, new NetClientOptions(), new PoolOptions(),
new RedisStandaloneConnectOptions(), TracingPolicy.IGNORE));
redisBasedLock = new RedisBasedLock(() -> Future.succeededFuture(redisAPI));
redisBasedLock = new RedisBasedLock(() -> Future.succeededFuture(redisAPI), newGateleenWastefulExceptionFactory());
}

@Before
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@
import java.util.Arrays;
import java.util.Map;

import static org.swisspush.gateleen.core.exception.GateleenExceptionFactory.newGateleenWastefulExceptionFactory;

/**
* Playground server to try Gateleen at home.
*
Expand Down Expand Up @@ -214,7 +216,7 @@ public void start() {
copyResourceHandler = new CopyResourceHandler(selfClient, SERVER_ROOT + "/v1/copy");
monitoringHandler = new MonitoringHandler(vertx, storage, PREFIX, SERVER_ROOT + "/monitoring/rpr");

Lock lock = new RedisBasedLock(redisProvider);
Lock lock = new RedisBasedLock(redisProvider, newGateleenWastefulExceptionFactory());

cacheStorage = new RedisCacheStorage(vertx, lock, redisProvider, 20 * 1000);
cacheDataFetcher = new DefaultCacheDataFetcher(new ClientRequestCreator(selfClient));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.swisspush.gateleen.queue.queuing.QueueBrowser;
import org.swisspush.gateleen.queue.queuing.QueuingHandler;
import org.swisspush.gateleen.queue.queuing.circuitbreaker.configuration.QueueCircuitBreakerConfigurationResourceManager;
import org.swisspush.gateleen.queue.queuing.splitter.NoOpQueueSplitter;
import org.swisspush.gateleen.queue.queuing.splitter.QueueSplitter;
import org.swisspush.gateleen.routing.CustomHttpResponseHandler;
import org.swisspush.gateleen.routing.Router;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
import org.swisspush.gateleen.scheduler.SchedulerResourceManager;
import org.swisspush.gateleen.user.RoleProfileHandler;
import org.swisspush.gateleen.user.UserProfileHandler;
import org.swisspush.reststorage.exception.RestStorageExceptionFactory;
import redis.clients.jedis.Jedis;

import javax.management.*;
Expand All @@ -82,6 +83,8 @@
import java.util.Map;
import java.util.Set;

import static org.swisspush.reststorage.exception.RestStorageExceptionFactory.newRestStorageWastefulExceptionFactory;

/**
* TestVerticle all Gateleen tests. <br />
*
Expand Down Expand Up @@ -170,7 +173,7 @@ public static void setupBeforeClass(TestContext context) {
RoleProfileHandler roleProfileHandler = new RoleProfileHandler(vertx, storage, SERVER_ROOT + "/roles/v1/([^/]+)/profile");
qosHandler = new QoSHandler(vertx, storage, SERVER_ROOT + "/admin/v1/qos", props, PREFIX);

Lock lock = new RedisBasedLock(redisProvider);
Lock lock = new RedisBasedLock(redisProvider, newRestStorageWastefulExceptionFactory());

QueueClient queueClient = new QueueClient(vertx, monitoringHandler);
ReducedPropagationManager reducedPropagationManager = new ReducedPropagationManager(vertx,
Expand Down

0 comments on commit 7d1e970

Please sign in to comment.