From 30deb9b853f78340c938f3b369b7d0389823efb1 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Thu, 22 Dec 2022 10:51:31 -0600 Subject: [PATCH 01/29] CARE-no-cglib: checkpoint --- README.md | 3 +- rdbi-core/pom.xml | 66 +++++------ .../dbi/rdbi/BBMethodContextInterceptor.java | 18 +++ .../rdbi/JedisWrapperMethodInterceptor.java | 34 ++++-- .../com/lithium/dbi/rdbi/ProxyFactory.java | 104 ++++++++++-------- rdbi-parent/pom.xml | 12 +- 6 files changed, 141 insertions(+), 96 deletions(-) create mode 100644 rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java diff --git a/README.md b/README.md index d8e9bb3..ed844bd 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,8 @@ The main cleanup we made with Jedis is, if the jedis client came from a pool, it ## Now onto Lua and Coolness: -Jedis provides a basic way of loading a Lua script into Redis and eval the script by its sha1 hash. rDBI provides this functionality via fluent queries, based off of [jDBI's fluent queries](http://jdbi.org/fluent_queries/). The application developer does not have to think about preloading the scripts on startup of the app or creating enums and storing sha1 in hashmaps. rDBI will cache the lua scripts internally and load them on demand while keeping it all thread-safe. +Jedis provides a basic way of loading a Lua script into Redis and eval the script by its sha1 hash. rDBI provides this functionality via fluent queries, based off of [jDBI's fluent queries](http://jdbi.org/fluent_queries/). +The application developer does not have to think about preloading the scripts on startup of the app or creating enums and storing sha1 in hashmaps. rDBI will cache the lua scripts internally and load them on demand while keeping it all thread-safe. private static interface TestDAO { @RedisQuery( diff --git a/rdbi-core/pom.xml b/rdbi-core/pom.xml index 2b0754d..2d2f9a7 100644 --- a/rdbi-core/pom.xml +++ b/rdbi-core/pom.xml @@ -13,42 +13,42 @@ rdbi-core rDBI-core - - - - - org.apache.maven.plugins - maven-shade-plugin - - - shade-proxying-implementation - package - - shade - - - - - cglib:cglib-nodep - - - - - net.sf.cglib - com.lithium.dbi.rdbi.shaded.net.sf.cglib - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - cglib - cglib-nodep + net.bytebuddy + byte-buddy redis.clients diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java new file mode 100644 index 0000000..640c848 --- /dev/null +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java @@ -0,0 +1,18 @@ +package com.lithium.dbi.rdbi; + +import java.util.Arrays; + +public class BBMethodContextInterceptor { + private final MethodContext methodContext; + + public BBMethodContextInterceptor(MethodContext methodContext) { + + this.methodContext = methodContext; + } + + public Object intercept(Object[] args) { + // pull in code from methodContextInterceptor + return Arrays.toString(args); + + } +} diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java index 0fad37a..9112fca 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java @@ -5,22 +5,29 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Scope; -import net.sf.cglib.proxy.Enhancer; -import net.sf.cglib.proxy.Factory; -import net.sf.cglib.proxy.MethodInterceptor; -import net.sf.cglib.proxy.MethodProxy; +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.implementation.bind.annotation.AllArguments; +import net.bytebuddy.implementation.bind.annotation.Origin; +import net.bytebuddy.implementation.bind.annotation.RuntimeType; +import net.bytebuddy.implementation.bind.annotation.SuperCall; +import net.bytebuddy.matcher.ElementMatchers; import redis.clients.jedis.Jedis; import redis.clients.jedis.exceptions.JedisException; import java.lang.reflect.Method; +import java.util.concurrent.Callable; -class JedisWrapperMethodInterceptor implements MethodInterceptor { +class JedisWrapperMethodInterceptor { private final Jedis jedis; private final Tracer tracer; private final Attributes commonAttributes; static Factory newFactory() { + new ByteBuddy() + .subclass(JedisWrapperDoNotUse.class) + .method(ElementMatchers.isMethod()) + .intercept() Enhancer e = new Enhancer(); e.setClassLoader(Jedis.class.getClassLoader()); e.setSuperclass(JedisWrapperDoNotUse.class); @@ -38,19 +45,22 @@ private JedisWrapperMethodInterceptor(Jedis jedis, Tracer tracer) { commonAttributes = Attributes.of( AttributeKey.stringKey("db.type"), "redis", AttributeKey.stringKey("component"), "rdbi" - ); + ); } - @Override - public Object intercept(Object o, Method method, Object[] args, MethodProxy methodProxy) throws Throwable { + @RuntimeType + public Object intercept( + @AllArguments Object[] args, + @Origin Method method, + @SuperCall Callable callable) { Span s = tracer.spanBuilder(method.getName()) - .setAllAttributes(commonAttributes) - .startSpan(); + .setAllAttributes(commonAttributes) + .startSpan(); if (args.length > 0 && args[0] instanceof String) { s.setAttribute("redis.key", (String) args[0]); } - try (Scope scope = s.makeCurrent()) { - return methodProxy.invoke(jedis, args); + try (Scope ignored = s.makeCurrent()) { + return method.invoke(jedis, args); } catch (JedisException e) { s.recordException(e); throw e; diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java index 669092c..d37f2bb 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java @@ -1,15 +1,14 @@ package com.lithium.dbi.rdbi; import io.opentelemetry.api.trace.Tracer; -import net.sf.cglib.proxy.Callback; -import net.sf.cglib.proxy.CallbackFilter; -import net.sf.cglib.proxy.Enhancer; -import net.sf.cglib.proxy.Factory; -import net.sf.cglib.proxy.MethodInterceptor; +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.dynamic.DynamicType; +import net.bytebuddy.implementation.MethodDelegation; +import net.bytebuddy.matcher.ElementMatchers; import redis.clients.jedis.Jedis; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -17,14 +16,11 @@ class ProxyFactory { - private static final MethodInterceptor NO_OP = new MethodNoOpInterceptor(); - private static final CallbackFilter FINALIZE_FILTER = new FinalizeFilter(); - - final ConcurrentMap, Factory> factoryCache; + final ConcurrentMap, DynamicType.Loaded> factoryCache; final ConcurrentMap, Map> methodContextCache; - private final Factory jedisInterceptorFactory; + // private final Factory jedisInterceptorFactory; ProxyFactory() { factoryCache = new ConcurrentHashMap<>(); @@ -39,35 +35,50 @@ JedisWrapperDoNotUse attachJedis(final Jedis jedis, Tracer tracer) { @SuppressWarnings("unchecked") T createInstance(final Jedis jedis, final Class t) { - Factory factory; - if (factoryCache.containsKey(t)) { - return (T) factoryCache.get(t).newInstance(new Callback[]{NO_OP,new MethodContextInterceptor(jedis, methodContextCache.get(t))}); - } else { + DynamicType.Loaded loaded = getLoadedType(jedis, t); + try { + return loaded.getLoaded().getDeclaredConstructor().newInstance(); + } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { + throw new RuntimeException(e); + } + } + private DynamicType.Loaded getLoadedType(Jedis jedis, Class t) { +// if (!factoryCache.containsKey(t)) { +// try { +// DynamicType.Loaded loaded = buildMethodContext(t, jedis) +// .make() +// .load(t.getClassLoader()); +// +// factoryCache.put(t, loaded); +// } catch (InstantiationException | IllegalAccessException e) { +// throw new RuntimeException(e); +// } +// } + return (DynamicType.Loaded) factoryCache.computeIfAbsent(t, (key) -> { + DynamicType.Loaded loaded = null; try { - buildMethodContext(t, jedis); + loaded = buildMethodContext(t, jedis) + .make() + .load(t.getClassLoader()); } catch (InstantiationException | IllegalAccessException e) { throw new RuntimeException(e); } - Enhancer e = new Enhancer(); - e.setSuperclass(t); - e.setCallbacks(new Callback[]{NO_OP,NO_OP}); //this will be overriden anyway, we set 2 so that it valid for FINALIZE_FILTER - e.setCallbackFilter(FINALIZE_FILTER); - - factory = (Factory) e.create(); - factoryCache.putIfAbsent(t, factory); - return (T) factory.newInstance(new Callback[]{NO_OP, new MethodContextInterceptor(jedis, methodContextCache.get(t))}); - } + return loaded; + }); } - private void buildMethodContext(Class t, Jedis jedis) throws IllegalAccessException, InstantiationException { + private DynamicType.Builder buildMethodContext(Class t, Jedis jedis) throws IllegalAccessException, InstantiationException { - if (methodContextCache.containsKey(t)) { - return; - } +// if (methodContextCache.containsKey(t)) { +// return; +// } - Map contexts = new HashMap<>(); + DynamicType.Builder builder = new ByteBuddy() + .subclass(t); + + // Map contexts = new HashMap<>(); for (Method method : t.getDeclaredMethods()) { @@ -85,15 +96,19 @@ private void buildMethodContext(Class t, Jedis jedis) throws IllegalAcces } Mapper methodMapper = method.getAnnotation(Mapper.class); - ResultMapper mapper = null; + ResultMapper mapper = null; if (methodMapper != null) { mapper = methodMapper.value().newInstance(); } - contexts.put(method, new MethodContext(sha1, mapper, luaContext)); + builder.method(ElementMatchers.is(method)) + .intercept(MethodDelegation.to(new BBMethodContextInterceptor( new MethodContext(sha1, mapper, luaContext)))); + + // contexts.put(method, new MethodContext(sha1, mapper, luaContext)); } - methodContextCache.putIfAbsent(t, contexts); + // methodContextCache.putIfAbsent(t, contexts); + return builder; } /** @@ -106,15 +121,16 @@ private boolean isRawMethod(Method method) { || (method.getParameterTypes()[0] == List.class); } - private static class FinalizeFilter implements CallbackFilter { - @Override - public int accept(Method method) { - if (method.getName().equals("finalize") && - method.getParameterTypes().length == 0 && - method.getReturnType() == Void.TYPE) { - return 0; //the NO_OP method interceptor - } - return 1; //the everything else method interceptor - } - } + // i don't think we need this because we don't have to lookup finalize, we just don't override it +// private static class FinalizeFilter implements CallbackFilter { +// @Override +// public int accept(Method method) { +// if (method.getName().equals("finalize") && +// method.getParameterTypes().length == 0 && +// method.getReturnType() == Void.TYPE) { +// return 0; //the NO_OP method interceptor +// } +// return 1; //the everything else method interceptor +// } +// } } diff --git a/rdbi-parent/pom.xml b/rdbi-parent/pom.xml index 22cf2b0..2b11163 100644 --- a/rdbi-parent/pom.xml +++ b/rdbi-parent/pom.xml @@ -56,11 +56,6 @@ guava ${guava.version} - - cglib - cglib-nodep - 3.2.4 - redis.clients jedis @@ -112,11 +107,16 @@ opentelemetry-api 1.4.1 + + net.bytebuddy + byte-buddy + LATEST + org.testng testng - 7.4.0 + 7.7.0 test From 68611cda689dd8af57bf5da3cef7a285fb9c3866 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 27 Dec 2022 14:45:34 -0600 Subject: [PATCH 02/29] CARE-no-cglib: checkpoint --- .../dbi/rdbi/BBMethodContextInterceptor.java | 87 ++++++++- .../java/com/lithium/dbi/rdbi/Handle.java | 2 +- .../dbi/rdbi/JedisWrapperDoNotUse.java | 2 +- .../rdbi/JedisWrapperMethodInterceptor.java | 38 ++-- .../dbi/rdbi/MethodContextInterceptor.java | 170 +++++++++--------- .../dbi/rdbi/MethodNoOpInterceptor.java | 26 +-- .../com/lithium/dbi/rdbi/ProxyFactory.java | 84 ++++----- rdbi-core/src/main/java/module-info.java | 10 ++ .../com/lithium/dbi/rdbi/RDBIGeneralTest.java | 13 +- .../com/lithium/dbi/rdbi/RDBIOpenRawTest.java | 2 +- .../java/com/lithium/dbi/rdbi/RDBITest.java | 34 ++-- .../lithium/dbi/rdbi/RDBIWithHandleTest.java | 2 +- rdbi-parent/pom.xml | 4 +- 13 files changed, 293 insertions(+), 181 deletions(-) create mode 100644 rdbi-core/src/main/java/module-info.java diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java index 640c848..146a8d9 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java @@ -1,18 +1,91 @@ package com.lithium.dbi.rdbi; -import java.util.Arrays; +import net.bytebuddy.implementation.MethodDelegation; +import net.bytebuddy.implementation.bind.annotation.AllArguments; +import net.bytebuddy.implementation.bind.annotation.Origin; +import net.bytebuddy.implementation.bind.annotation.RuntimeType; +import redis.clients.jedis.Jedis; +import redis.clients.jedis.exceptions.JedisDataException; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; public class BBMethodContextInterceptor { - private final MethodContext methodContext; - public BBMethodContextInterceptor(MethodContext methodContext) { + private final Jedis jedis; + private final Map contexts; + + + public BBMethodContextInterceptor(Jedis jedis, Map contexts) { + this.jedis = jedis; + this.contexts = contexts; + } + + @RuntimeType + public Object intercept(@AllArguments Object[] args, + @Origin Method method) { + + MethodContext context = contexts.get(method); + + Object ret = context.hasDynamicLists() ? callEvalDynamicList(context, args) + : callEval(context, args); + // problem here is we are getting a LONG but expected an int + // did this work on older jdks? doesn't seem to be a cglib vs bytebuddy thing + // java.lang.ClassCastException: class java.lang.Long cannot be cast to class java.lang.Integer (java.lang.Long and java.lang.Integer are in module java.base of loader 'bootstrap') + if (ret == null) { + return null; + } + + if (context.getMapper() != null) { + return context.getMapper().map(ret); + } else { + return ret; + } + } + + + @SuppressWarnings("unchecked") + private Object callEval(MethodContext context, Object[] objects) { + + List keys = objects.length > 0 ? (List) objects[0] : null; + List argv = objects.length > 1 ? (List) objects[1] : null; - this.methodContext = methodContext; + return evalShaHandleReloadScript(context, keys, argv); } - public Object intercept(Object[] args) { - // pull in code from methodContextInterceptor - return Arrays.toString(args); + private Object callEvalDynamicList(MethodContext context, Object[] objects) { + + List keys = new ArrayList<>(); + List argv = new ArrayList<>(); + + for (int i = 0; i < objects.length; i++) { + if (context.getLuaContext().isKey(i)) { + keys.add(objects[i].toString()); + } else { + argv.add(objects[i].toString()); + } + } + + return evalShaHandleReloadScript(context, keys, argv); + } + private Object evalShaHandleReloadScript(MethodContext context, List keys, List argv) { + try { + return jedis.evalsha(context.getSha1(), keys, argv); + } catch (JedisDataException e) { + if (e.getMessage() != null && e.getMessage().contains("NOSCRIPT")) { + //If it throws again, we can back-off or we can just let it throw again. In this case, I think we should + //let it throw because most likely will be trying the same thing again and hopefully it will succeed later. + final String newSha = jedis.scriptLoad(context.getLuaContext().getRenderedLuaString()); + if (!newSha.equals(context.getSha1())) { + throw new IllegalStateException("sha should match but they did not"); + } + return jedis.evalsha(context.getSha1(), keys, argv); + } else { + throw e; + } + } } } diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/Handle.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/Handle.java index 0a012d8..ce7638f 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/Handle.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/Handle.java @@ -11,7 +11,7 @@ public class Handle implements Closeable { private final Jedis jedis; private final Tracer tracer; - private JedisWrapperDoNotUse jedisWrapper; + private Jedis jedisWrapper; private final ProxyFactory proxyFactory; diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperDoNotUse.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperDoNotUse.java index 686b315..81e4d06 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperDoNotUse.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperDoNotUse.java @@ -4,7 +4,7 @@ abstract class JedisWrapperDoNotUse extends Jedis { - JedisWrapperDoNotUse() { + public JedisWrapperDoNotUse() { super(); } diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java index 9112fca..c55ddaa 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java @@ -6,6 +6,8 @@ import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Scope; import net.bytebuddy.ByteBuddy; +import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; +import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.implementation.bind.annotation.AllArguments; import net.bytebuddy.implementation.bind.annotation.Origin; import net.bytebuddy.implementation.bind.annotation.RuntimeType; @@ -14,6 +16,8 @@ import redis.clients.jedis.Jedis; import redis.clients.jedis.exceptions.JedisException; +import java.lang.invoke.MethodHandles; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.concurrent.Callable; @@ -23,20 +27,23 @@ class JedisWrapperMethodInterceptor { private final Tracer tracer; private final Attributes commonAttributes; - static Factory newFactory() { - new ByteBuddy() - .subclass(JedisWrapperDoNotUse.class) - .method(ElementMatchers.isMethod()) - .intercept() - Enhancer e = new Enhancer(); - e.setClassLoader(Jedis.class.getClassLoader()); - e.setSuperclass(JedisWrapperDoNotUse.class); - e.setCallback(new MethodNoOpInterceptor()); - return (Factory) e.create(); - } + static Jedis newInstance(final Jedis realJedis, final Tracer tracer) { - static JedisWrapperDoNotUse newInstance(final Factory factory, final Jedis realJedis, final Tracer tracer) { - return (JedisWrapperDoNotUse) factory.newInstance(new JedisWrapperMethodInterceptor(realJedis, tracer)); + try { + // todo cache some of this + // https://github.com/raphw/byte-buddy/issues/663 + return new ByteBuddy() + .subclass(JedisWrapperDoNotUse.class) + .method(ElementMatchers.isMethod()) + .intercept(MethodDelegation.to(new JedisWrapperMethodInterceptor(realJedis, tracer), "intercept")) + .make() + .load(Jedis.class.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup)) + .getLoaded() + .getDeclaredConstructor() + .newInstance(); + } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { + throw new RuntimeException(e); + } } private JedisWrapperMethodInterceptor(Jedis jedis, Tracer tracer) { @@ -60,10 +67,13 @@ public Object intercept( s.setAttribute("redis.key", (String) args[0]); } try (Scope ignored = s.makeCurrent()) { - return method.invoke(jedis, args); + return callable.call(); } catch (JedisException e) { s.recordException(e); throw e; + } catch (Exception e) { + s.recordException(e); + throw new RuntimeException(e); } finally { s.end(); } diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodContextInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodContextInterceptor.java index 9ac3853..66bc1f3 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodContextInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodContextInterceptor.java @@ -1,85 +1,85 @@ -package com.lithium.dbi.rdbi; - -import net.sf.cglib.proxy.MethodInterceptor; -import net.sf.cglib.proxy.MethodProxy; -import redis.clients.jedis.Jedis; -import redis.clients.jedis.exceptions.JedisDataException; - -import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - -class MethodContextInterceptor implements MethodInterceptor { - - private final Jedis jedis; - private final Map contexts; - - public MethodContextInterceptor(Jedis jedis, Map contexts) { - this.jedis = jedis; - this.contexts = contexts; - } - - @Override - @SuppressWarnings("unchecked") - public Object intercept(Object o, Method method, Object[] objects, MethodProxy methodProxy) throws Throwable { - - MethodContext context = contexts.get(method); - - Object ret = context.hasDynamicLists() ? callEvalDynamicList(context, objects) - : callEval(context, objects); - - if (ret == null) { - return null; - } - - if (contexts.get(method).getMapper() != null) { - return contexts.get(method).getMapper().map(ret); - } else { - return ret; - } - } - - @SuppressWarnings("unchecked") - private Object callEval(MethodContext context, Object[] objects) { - - List keys = objects.length > 0 ? (List) objects[0] : null; - List argv = objects.length > 1 ? (List) objects[1] : null; - - return evalShaHandleReloadScript(context, keys, argv); - } - - private Object callEvalDynamicList(MethodContext context, Object[] objects) { - - List keys = new ArrayList<>(); - List argv = new ArrayList<>(); - - for (int i = 0; i < objects.length; i++) { - if (context.getLuaContext().isKey(i)) { - keys.add(objects[i].toString()); - } else { - argv.add(objects[i].toString()); - } - } - - return evalShaHandleReloadScript(context, keys, argv); - } - - private Object evalShaHandleReloadScript(MethodContext context, List keys, List argv) { - try { - return jedis.evalsha(context.getSha1(), keys, argv); - } catch (JedisDataException e) { - if (e.getMessage() != null && e.getMessage().contains("NOSCRIPT")) { - //If it throws again, we can back-off or we can just let it throw again. In this case, I think we should - //let it throw because most likely will be trying the same thing again and hopefully it will succeed later. - final String newSha = jedis.scriptLoad(context.getLuaContext().getRenderedLuaString()); - if (!newSha.equals(context.getSha1())) { - throw new IllegalStateException("sha should match but they did not"); - } - return jedis.evalsha(context.getSha1(), keys, argv); - } else { - throw e; - } - } - } -} +//package com.lithium.dbi.rdbi; +// +//import net.sf.cglib.proxy.MethodInterceptor; +//import net.sf.cglib.proxy.MethodProxy; +//import redis.clients.jedis.Jedis; +//import redis.clients.jedis.exceptions.JedisDataException; +// +//import java.lang.reflect.Method; +//import java.util.ArrayList; +//import java.util.List; +//import java.util.Map; +// +//class MethodContextInterceptor implements MethodInterceptor { +// +// private final Jedis jedis; +// private final Map contexts; +// +// public MethodContextInterceptor(Jedis jedis, Map contexts) { +// this.jedis = jedis; +// this.contexts = contexts; +// } +// +// @Override +// @SuppressWarnings("unchecked") +// public Object intercept(Object o, Method method, Object[] objects, MethodProxy methodProxy) throws Throwable { +// +// MethodContext context = contexts.get(method); +// +// Object ret = context.hasDynamicLists() ? callEvalDynamicList(context, objects) +// : callEval(context, objects); +// +// if (ret == null) { +// return null; +// } +// +// if (contexts.get(method).getMapper() != null) { +// return contexts.get(method).getMapper().map(ret); +// } else { +// return ret; +// } +// } +// +// @SuppressWarnings("unchecked") +// private Object callEval(MethodContext context, Object[] objects) { +// +// List keys = objects.length > 0 ? (List) objects[0] : null; +// List argv = objects.length > 1 ? (List) objects[1] : null; +// +// return evalShaHandleReloadScript(context, keys, argv); +// } +// +// private Object callEvalDynamicList(MethodContext context, Object[] objects) { +// +// List keys = new ArrayList<>(); +// List argv = new ArrayList<>(); +// +// for (int i = 0; i < objects.length; i++) { +// if (context.getLuaContext().isKey(i)) { +// keys.add(objects[i].toString()); +// } else { +// argv.add(objects[i].toString()); +// } +// } +// +// return evalShaHandleReloadScript(context, keys, argv); +// } +// +// private Object evalShaHandleReloadScript(MethodContext context, List keys, List argv) { +// try { +// return jedis.evalsha(context.getSha1(), keys, argv); +// } catch (JedisDataException e) { +// if (e.getMessage() != null && e.getMessage().contains("NOSCRIPT")) { +// //If it throws again, we can back-off or we can just let it throw again. In this case, I think we should +// //let it throw because most likely will be trying the same thing again and hopefully it will succeed later. +// final String newSha = jedis.scriptLoad(context.getLuaContext().getRenderedLuaString()); +// if (!newSha.equals(context.getSha1())) { +// throw new IllegalStateException("sha should match but they did not"); +// } +// return jedis.evalsha(context.getSha1(), keys, argv); +// } else { +// throw e; +// } +// } +// } +//} diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodNoOpInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodNoOpInterceptor.java index 220846f..7aeb760 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodNoOpInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodNoOpInterceptor.java @@ -1,13 +1,13 @@ -package com.lithium.dbi.rdbi; - -import net.sf.cglib.proxy.MethodInterceptor; -import net.sf.cglib.proxy.MethodProxy; - -import java.lang.reflect.Method; - -public class MethodNoOpInterceptor implements MethodInterceptor { - @Override - public Object intercept(Object o, Method method, Object[] objects, MethodProxy methodProxy) throws Throwable { - return null; - } -} +//package com.lithium.dbi.rdbi; +// +//import net.sf.cglib.proxy.MethodInterceptor; +//import net.sf.cglib.proxy.MethodProxy; +// +//import java.lang.reflect.Method; +// +//public class MethodNoOpInterceptor implements MethodInterceptor { +// @Override +// public Object intercept(Object o, Method method, Object[] objects, MethodProxy methodProxy) throws Throwable { +// return null; +// } +//} diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java index d37f2bb..4dc7842 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java @@ -3,12 +3,15 @@ import io.opentelemetry.api.trace.Tracer; import net.bytebuddy.ByteBuddy; import net.bytebuddy.dynamic.DynamicType; +import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.matcher.ElementMatchers; import redis.clients.jedis.Jedis; +import java.lang.invoke.MethodHandles; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -20,16 +23,15 @@ class ProxyFactory { final ConcurrentMap, Map> methodContextCache; - // private final Factory jedisInterceptorFactory; + // private final Factory jedisInterceptorFactory; ProxyFactory() { factoryCache = new ConcurrentHashMap<>(); - methodContextCache = new ConcurrentHashMap<>(); - jedisInterceptorFactory = JedisWrapperMethodInterceptor.newFactory(); + methodContextCache = new ConcurrentHashMap<>(); } - JedisWrapperDoNotUse attachJedis(final Jedis jedis, Tracer tracer) { - return JedisWrapperMethodInterceptor.newInstance(jedisInterceptorFactory, jedis, tracer); + Jedis attachJedis(final Jedis jedis, Tracer tracer) { + return JedisWrapperMethodInterceptor.newInstance(jedis, tracer); } @SuppressWarnings("unchecked") @@ -38,29 +40,31 @@ T createInstance(final Jedis jedis, final Class t) { DynamicType.Loaded loaded = getLoadedType(jedis, t); try { return loaded.getLoaded().getDeclaredConstructor().newInstance(); - } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { + } catch (InstantiationException | IllegalAccessException | InvocationTargetException | + NoSuchMethodException e) { throw new RuntimeException(e); } } private DynamicType.Loaded getLoadedType(Jedis jedis, Class t) { -// if (!factoryCache.containsKey(t)) { -// try { -// DynamicType.Loaded loaded = buildMethodContext(t, jedis) -// .make() -// .load(t.getClassLoader()); -// -// factoryCache.put(t, loaded); -// } catch (InstantiationException | IllegalAccessException e) { -// throw new RuntimeException(e); -// } -// } + // if (!factoryCache.containsKey(t)) { + // try { + // DynamicType.Loaded loaded = buildMethodContext(t, jedis) + // .make() + // .load(t.getClassLoader()); + // + // factoryCache.put(t, loaded); + // } catch (InstantiationException | IllegalAccessException e) { + // throw new RuntimeException(e); + // } + // } + // TODO use a TypeCache instead of our own return (DynamicType.Loaded) factoryCache.computeIfAbsent(t, (key) -> { DynamicType.Loaded loaded = null; try { loaded = buildMethodContext(t, jedis) .make() - .load(t.getClassLoader()); + .load(t.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup)); } catch (InstantiationException | IllegalAccessException e) { throw new RuntimeException(e); } @@ -71,17 +75,17 @@ private DynamicType.Loaded getLoadedType(Jedis jedis, Class t) { private DynamicType.Builder buildMethodContext(Class t, Jedis jedis) throws IllegalAccessException, InstantiationException { -// if (methodContextCache.containsKey(t)) { -// return; -// } + // if (methodContextCache.containsKey(t)) { + // return; + // } DynamicType.Builder builder = new ByteBuddy() .subclass(t); - // Map contexts = new HashMap<>(); + Map contexts = new HashMap<>(); - for (Method method : t.getDeclaredMethods()) { + for (Method method : t.getDeclaredMethods()) { Query query = method.getAnnotation(Query.class); String queryStr = query.value(); @@ -101,18 +105,18 @@ private DynamicType.Builder buildMethodContext(Class t, Jedis jedis) t mapper = methodMapper.value().newInstance(); } - builder.method(ElementMatchers.is(method)) - .intercept(MethodDelegation.to(new BBMethodContextInterceptor( new MethodContext(sha1, mapper, luaContext)))); - - // contexts.put(method, new MethodContext(sha1, mapper, luaContext)); + contexts.put(method, new MethodContext(sha1, mapper, luaContext)); + // don't think we need this + // methodContextCache.putIfAbsent(t, contexts); } + return builder.method(ElementMatchers.any()) + .intercept(MethodDelegation.to(new BBMethodContextInterceptor(jedis, contexts))); - // methodContextCache.putIfAbsent(t, contexts); - return builder; } /** * If the method does not have @Bind or @BindKey it is assumed to be a call without script bindings + * * @param method the function to check on * @return true if the method is considered not to have any bindings needed */ @@ -122,15 +126,15 @@ private boolean isRawMethod(Method method) { } // i don't think we need this because we don't have to lookup finalize, we just don't override it -// private static class FinalizeFilter implements CallbackFilter { -// @Override -// public int accept(Method method) { -// if (method.getName().equals("finalize") && -// method.getParameterTypes().length == 0 && -// method.getReturnType() == Void.TYPE) { -// return 0; //the NO_OP method interceptor -// } -// return 1; //the everything else method interceptor -// } -// } + // private static class FinalizeFilter implements CallbackFilter { + // @Override + // public int accept(Method method) { + // if (method.getName().equals("finalize") && + // method.getParameterTypes().length == 0 && + // method.getReturnType() == Void.TYPE) { + // return 0; //the NO_OP method interceptor + // } + // return 1; //the everything else method interceptor + // } + // } } diff --git a/rdbi-core/src/main/java/module-info.java b/rdbi-core/src/main/java/module-info.java new file mode 100644 index 0000000..6e2d657 --- /dev/null +++ b/rdbi-core/src/main/java/module-info.java @@ -0,0 +1,10 @@ +module rdbi.core { + exports com.lithium.dbi.rdbi; + requires net.bytebuddy; + requires stringtemplate; + requires io.opentelemetry.api; + requires io.opentelemetry.context; + requires redis.clients.jedis; + requires jsr305; + requires org.slf4j; +} diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIGeneralTest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIGeneralTest.java index 9309f53..4371345 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIGeneralTest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIGeneralTest.java @@ -3,6 +3,8 @@ import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPool; +import static org.testng.Assert.assertEquals; + /** * User: phutwo * Date: 9/8/13 @@ -13,12 +15,15 @@ public class RDBIGeneralTest { public static void main(String[] args) { RDBI rdbi = new RDBI(new JedisPool("localhost", 6379)); - Handle handle = rdbi.open(); + try (Handle handle = rdbi.open()) { + Jedis jedis = handle.jedis(); + jedis.set("hey", "now"); + } - try { + try (Handle handle = rdbi.open()) { Jedis jedis = handle.jedis(); - } finally { - handle.close(); + String hey = jedis.get("hey"); + assertEquals(hey, "now"); } } } diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenRawTest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenRawTest.java index bb0a05a..38671a9 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenRawTest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenRawTest.java @@ -9,7 +9,7 @@ public class RDBIOpenRawTest { @Test public void testNormalOpen() { - RDBI rdbi = new RDBI(RDBITest.getJedisPool()); + RDBI rdbi = new RDBI(RDBITest.getMockJedisPool()); Handle jedis = rdbi.open(); diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java index 0032d47..139ad34 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java @@ -45,7 +45,13 @@ interface DynamicDAO { @Query( "redis.call('SET', $a$, $b$); return 0;" ) - int testExec(@BindKey("a") String a, @BindArg("b") String b); + long testExec(@BindKey("a") String a, @BindArg("b") String b); + +// @Query( +// "redis.call('SET', $a$, $b$); return 0;" +// ) +// int testExec2(@BindKey("a") String a, @BindArg("b") String b); + } static class BasicObjectUnderTest { @@ -66,7 +72,7 @@ public BasicObjectUnderTest map(Integer result) { return new BasicObjectUnderTest(result); } } - static interface TestDAOWithResultSetMapper { + interface TestDAOWithResultSetMapper { @Query( "redis.call('SET', KEYS[1], ARGV[1]);" + @@ -112,7 +118,7 @@ public void testExceptionThrownInNormalGet() { @SuppressWarnings("unchecked") public void testBasicAttachRun() { - RDBI rdbi = new RDBI(getJedisPool()); + RDBI rdbi = new RDBI(getMockJedisPool()); Handle handle1 = rdbi.open(); try { @@ -142,7 +148,7 @@ public void testBasicAttachRun() { @Test public void testAttachWithResultSetMapper() { - RDBI rdbi = new RDBI(getJedisPool()); + RDBI rdbi = new RDBI(getMockJedisPool()); Handle handle = rdbi.open(); try { @@ -156,7 +162,7 @@ public void testAttachWithResultSetMapper() { @Test public void testMethodWithNoInput() { - RDBI rdbi = new RDBI(getJedisPool()); + RDBI rdbi = new RDBI(getMockJedisPool()); Handle handle = rdbi.open(); try { @@ -167,21 +173,25 @@ public void testMethodWithNoInput() { } } + // good test to make sure shit works @Test public void testDynamicDAO() { - RDBI rdbi = new RDBI(getJedisPool()); - Handle handle = rdbi.open(); +// RDBI rdbi = new RDBI(getMockJedisPool()); + RDBI rdbi = new RDBI(new JedisPool("localhost", 6379)); - try { + try (Handle handle = rdbi.open()) { handle.attach(DynamicDAO.class).testExec("a", "b"); - } finally { - handle.close(); + } + + try (Handle handle = rdbi.open()) { + String val = handle.jedis().get("a"); + assertEquals(val, "b"); } } @Test public void testCacheHitDAO() { - RDBI rdbi = new RDBI(getJedisPool()); + RDBI rdbi = new RDBI(getMockJedisPool()); Handle handle = rdbi.open(); try { @@ -195,7 +205,7 @@ public void testCacheHitDAO() { } @SuppressWarnings("unchecked") - static JedisPool getJedisPool() { + static JedisPool getMockJedisPool() { Jedis jedis = mock(Jedis.class); when(jedis.scriptLoad(anyString())).thenReturn("my-sha1-hash"); when(jedis.evalsha(anyString(), anyList(), anyList())).thenReturn(0); diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java index 086172c..611c34d 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java @@ -22,7 +22,7 @@ static interface TestDAO { @Test public void testBasicWithHandle() { - RDBI rdbi = new RDBI(RDBITest.getJedisPool()); + RDBI rdbi = new RDBI(RDBITest.getMockJedisPool()); rdbi.withHandle(new Callback() { @Override diff --git a/rdbi-parent/pom.xml b/rdbi-parent/pom.xml index 2b11163..bf3fba2 100644 --- a/rdbi-parent/pom.xml +++ b/rdbi-parent/pom.xml @@ -215,8 +215,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.8 - 1.8 + 11 + 11 UTF-8 true From fb2d58f2eda0e34e5f8b80f9b0001bafa8922798 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Wed, 28 Dec 2022 09:42:57 -0600 Subject: [PATCH 03/29] CARE-no-cglib: checkpoint --- .../dbi/rdbi/BBMethodContextInterceptor.java | 33 +++++++-- .../rdbi/JedisWrapperMethodInterceptor.java | 31 +++++---- .../com/lithium/dbi/rdbi/ProxyFactory.java | 69 +++++-------------- rdbi-core/src/main/java/module-info.java | 10 --- .../com/lithium/dbi/rdbi/RDBIOpenTest.java | 9 ++- .../java/com/lithium/dbi/rdbi/RDBITest.java | 15 ++-- .../lithium/dbi/rdbi/RDBIWithHandleTest.java | 36 ++++------ 7 files changed, 86 insertions(+), 117 deletions(-) delete mode 100644 rdbi-core/src/main/java/module-info.java diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java index 146a8d9..e440137 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java @@ -31,15 +31,36 @@ public Object intercept(@AllArguments Object[] args, Object ret = context.hasDynamicLists() ? callEvalDynamicList(context, args) : callEval(context, args); - // problem here is we are getting a LONG but expected an int - // did this work on older jdks? doesn't seem to be a cglib vs bytebuddy thing - // java.lang.ClassCastException: class java.lang.Long cannot be cast to class java.lang.Integer (java.lang.Long and java.lang.Integer are in module java.base of loader 'bootstrap') if (ret == null) { return null; } + if (context.getMapper() != null) { - return context.getMapper().map(ret); + // here we need to adjust for the expected input to the mapper + Class parameterType; + try { + parameterType = context.getMapper().getClass().getMethod("map", Integer.class).getParameterTypes()[0]; + } catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } + return context.getMapper().map(adjust(parameterType, ret)); + } else { + return adjust(method.getReturnType(), ret); + } + } + + private Object adjust(Class c, Object ret) { + // problem here is we are getting a LONG but expected an int + // did this work on older jdks? doesn't seem to be a cglib vs bytebuddy thing + // java.lang.ClassCastException: class java.lang.Long cannot be cast to class java.lang.Integer (java.lang.Long and java.lang.Integer are in module java.base of loader 'bootstrap') + // i am not sure why this wasn't required before, and there's probably a utility out there + // to do it better the primary thing to handle is explicit cast to int where jedis returns a number + // as a long + if (c == null) { + return ret; + } else if ((c.equals(Integer.TYPE) || c.isAssignableFrom(Integer.class)) && ret instanceof Long) { + return ((Long) ret).intValue(); } else { return ret; } @@ -49,8 +70,8 @@ public Object intercept(@AllArguments Object[] args, @SuppressWarnings("unchecked") private Object callEval(MethodContext context, Object[] objects) { - List keys = objects.length > 0 ? (List) objects[0] : null; - List argv = objects.length > 1 ? (List) objects[1] : null; + List keys = objects.length > 0 ? (List) objects[0] : new ArrayList<>(); + List argv = objects.length > 1 ? (List) objects[1] : new ArrayList<>(); return evalShaHandleReloadScript(context, keys, argv); } diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java index c55ddaa..43bedf0 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java @@ -6,6 +6,7 @@ import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Scope; import net.bytebuddy.ByteBuddy; +import net.bytebuddy.TypeCache; import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.implementation.bind.annotation.AllArguments; @@ -21,32 +22,31 @@ import java.lang.reflect.Method; import java.util.concurrent.Callable; -class JedisWrapperMethodInterceptor { +public class JedisWrapperMethodInterceptor { private final Jedis jedis; private final Tracer tracer; private final Attributes commonAttributes; + private static final TypeCache> cache = new TypeCache<>(); static Jedis newInstance(final Jedis realJedis, final Tracer tracer) { try { - // todo cache some of this - // https://github.com/raphw/byte-buddy/issues/663 - return new ByteBuddy() - .subclass(JedisWrapperDoNotUse.class) - .method(ElementMatchers.isMethod()) - .intercept(MethodDelegation.to(new JedisWrapperMethodInterceptor(realJedis, tracer), "intercept")) - .make() - .load(Jedis.class.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup)) - .getLoaded() - .getDeclaredConstructor() - .newInstance(); - } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { + return (Jedis) cache.findOrInsert(Jedis.class.getClassLoader(), Jedis.class, () -> new ByteBuddy() + .subclass(JedisWrapperDoNotUse.class) + .method(ElementMatchers.isMethod()) + .intercept(MethodDelegation.to(new JedisWrapperMethodInterceptor(realJedis, tracer), "intercept")) + .make() + .load(Jedis.class.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup)) + .getLoaded()).getDeclaredConstructor() + .newInstance(); + } catch (InstantiationException | IllegalAccessException | InvocationTargetException | + NoSuchMethodException e) { throw new RuntimeException(e); } } - private JedisWrapperMethodInterceptor(Jedis jedis, Tracer tracer) { + public JedisWrapperMethodInterceptor(Jedis jedis, Tracer tracer) { this.jedis = jedis; this.tracer = tracer; commonAttributes = Attributes.of( @@ -67,7 +67,8 @@ public Object intercept( s.setAttribute("redis.key", (String) args[0]); } try (Scope ignored = s.makeCurrent()) { - return callable.call(); + return method.invoke(jedis, args); +// return callable.call(); } catch (JedisException e) { s.recordException(e); throw e; diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java index 4dc7842..7eb406e 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java @@ -2,6 +2,7 @@ import io.opentelemetry.api.trace.Tracer; import net.bytebuddy.ByteBuddy; +import net.bytebuddy.TypeCache; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; import net.bytebuddy.implementation.MethodDelegation; @@ -14,21 +15,10 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; class ProxyFactory { - final ConcurrentMap, DynamicType.Loaded> factoryCache; - - final ConcurrentMap, Map> methodContextCache; - - // private final Factory jedisInterceptorFactory; - - ProxyFactory() { - factoryCache = new ConcurrentHashMap<>(); - methodContextCache = new ConcurrentHashMap<>(); - } + private final TypeCache> cache = new TypeCache<>(); Jedis attachJedis(final Jedis jedis, Tracer tracer) { return JedisWrapperMethodInterceptor.newInstance(jedis, tracer); @@ -36,55 +26,27 @@ Jedis attachJedis(final Jedis jedis, Tracer tracer) { @SuppressWarnings("unchecked") T createInstance(final Jedis jedis, final Class t) { - - DynamicType.Loaded loaded = getLoadedType(jedis, t); try { - return loaded.getLoaded().getDeclaredConstructor().newInstance(); + return (T) cache.findOrInsert(t.getClassLoader(), t, () -> + buildClass(t, jedis) + ).getDeclaredConstructor().newInstance(); } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { throw new RuntimeException(e); } } - private DynamicType.Loaded getLoadedType(Jedis jedis, Class t) { - // if (!factoryCache.containsKey(t)) { - // try { - // DynamicType.Loaded loaded = buildMethodContext(t, jedis) - // .make() - // .load(t.getClassLoader()); - // - // factoryCache.put(t, loaded); - // } catch (InstantiationException | IllegalAccessException e) { - // throw new RuntimeException(e); - // } - // } - // TODO use a TypeCache instead of our own - return (DynamicType.Loaded) factoryCache.computeIfAbsent(t, (key) -> { - DynamicType.Loaded loaded = null; - try { - loaded = buildMethodContext(t, jedis) - .make() - .load(t.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup)); - } catch (InstantiationException | IllegalAccessException e) { - throw new RuntimeException(e); - } - - return loaded; - }); + boolean isCached(Class t) { + return cache.find(t.getClassLoader(), t) != null; } - private DynamicType.Builder buildMethodContext(Class t, Jedis jedis) throws IllegalAccessException, InstantiationException { - // if (methodContextCache.containsKey(t)) { - // return; - // } + private Class buildClass(Class t, Jedis jedis) throws IllegalAccessException, InstantiationException { DynamicType.Builder builder = new ByteBuddy() .subclass(t); Map contexts = new HashMap<>(); - - for (Method method : t.getDeclaredMethods()) { Query query = method.getAnnotation(Query.class); String queryStr = query.value(); @@ -102,15 +64,20 @@ private DynamicType.Builder buildMethodContext(Class t, Jedis jedis) t Mapper methodMapper = method.getAnnotation(Mapper.class); ResultMapper mapper = null; if (methodMapper != null) { - mapper = methodMapper.value().newInstance(); + try { + mapper = methodMapper.value().getDeclaredConstructor().newInstance(); + } catch (InvocationTargetException | NoSuchMethodException e) { + throw new RuntimeException(e); + } } - contexts.put(method, new MethodContext(sha1, mapper, luaContext)); - // don't think we need this - // methodContextCache.putIfAbsent(t, contexts); } + return builder.method(ElementMatchers.any()) - .intercept(MethodDelegation.to(new BBMethodContextInterceptor(jedis, contexts))); + .intercept(MethodDelegation.to(new BBMethodContextInterceptor(jedis, contexts))) + .make() + .load(t.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup)) + .getLoaded(); } diff --git a/rdbi-core/src/main/java/module-info.java b/rdbi-core/src/main/java/module-info.java deleted file mode 100644 index 6e2d657..0000000 --- a/rdbi-core/src/main/java/module-info.java +++ /dev/null @@ -1,10 +0,0 @@ -module rdbi.core { - exports com.lithium.dbi.rdbi; - requires net.bytebuddy; - requires stringtemplate; - requires io.opentelemetry.api; - requires io.opentelemetry.context; - requires redis.clients.jedis; - requires jsr305; - requires org.slf4j; -} diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenTest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenTest.java index d6f543e..97de5cc 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenTest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenTest.java @@ -2,22 +2,21 @@ import org.testng.annotations.Test; -import redis.clients.jedis.exceptions.JedisException; public class RDBIOpenTest { - static interface TestDAO { + interface TestDAO { @Query("doesn't matter") - public int doesntMatter(); + int doesntMatter(); } - @Test(expectedExceptions = JedisException.class) + @Test(expectedExceptions = IllegalArgumentException.class) public void testOpenThrow() { RDBI rdbi = new RDBI(RDBITest.getBadJedisPool()); rdbi.open().attach(TestDAO.class); } - @Test(expectedExceptions = Exception.class) + @Test(expectedExceptions = IllegalArgumentException.class) public void testOpenThrowRuntimeException() { RDBI rdbi = new RDBI(RDBITest.getBadJedisPoolWithRuntimeException()); rdbi.open().attach(TestDAO.class); diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java index 139ad34..0986571 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java @@ -15,6 +15,7 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; @@ -45,7 +46,7 @@ interface DynamicDAO { @Query( "redis.call('SET', $a$, $b$); return 0;" ) - long testExec(@BindKey("a") String a, @BindArg("b") String b); + int testExec(@BindKey("a") String a, @BindArg("b") String b); // @Query( // "redis.call('SET', $a$, $b$); return 0;" @@ -89,11 +90,11 @@ public void testExceptionThrownInRDBIAttach() { Handle handle = rdbi.open(); try { handle.attach(TestCopyDAO.class); + // real problem fail("Should have thrown exception for loadScript error"); } catch (RuntimeException e) { //expected - assertFalse(rdbi.proxyFactory.factoryCache.containsKey(TestCopyDAO.class)); - assertFalse(rdbi.proxyFactory.methodContextCache.containsKey(TestCopyDAO.class)); + assertFalse(rdbi.proxyFactory.isCached(TestCopyDAO.class)); } finally { handle.close(); } @@ -108,7 +109,7 @@ public void testExceptionThrownInNormalGet() { handle.jedis().get("hello"); fail("Should have thrown exception on get"); } catch (Exception e) { - //expected + //expected // hmm i don't think this is right } finally { handle.close(); } @@ -134,9 +135,7 @@ public void testBasicAttachRun() { handle2.close(); } - assertTrue(rdbi.proxyFactory.factoryCache.containsKey(TestDAO.class)); - assertTrue(rdbi.proxyFactory.methodContextCache.containsKey(TestDAO.class)); - + assertTrue(rdbi.proxyFactory.isCached(TestDAO.class)); Handle handle3 = rdbi.open(); try { @@ -198,7 +197,7 @@ public void testCacheHitDAO() { for (int i = 0; i < 2; i++) { handle.attach(DynamicDAO.class).testExec("a", "b"); } - assertTrue(rdbi.proxyFactory.factoryCache.containsKey(DynamicDAO.class)); + assertTrue(rdbi.proxyFactory.isCached(DynamicDAO.class)); } finally { handle.close(); } diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java index 611c34d..ebfaf5a 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java @@ -8,11 +8,12 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNull; import static org.testng.Assert.fail; public class RDBIWithHandleTest { - static interface TestDAO { + interface TestDAO { @Query( "redis.call('SET', KEYS[1], ARGV[1]);" + "return 0;" @@ -24,12 +25,9 @@ static interface TestDAO { public void testBasicWithHandle() { RDBI rdbi = new RDBI(RDBITest.getMockJedisPool()); - rdbi.withHandle(new Callback() { - @Override - public Object run(Handle handle) { - assertEquals(handle.attach(TestDAO.class).testExec(Collections.singletonList("hello"), Collections.singletonList("world")), 0); - return null; - } + rdbi.withHandle(handle -> { + assertEquals(handle.attach(TestDAO.class).testExec(Collections.singletonList("hello"), Collections.singletonList("world")), 0); + return null; }); } @@ -38,16 +36,12 @@ void testBasicWithHandleFailure() { RDBI rdbi = new RDBI(RDBITest.getBadJedisPool()); try { - rdbi.withHandle(new Callback() { - @Override - public Object run(Handle handle) { - handle.attach(TestDAO.class); - return null; - } + rdbi.withHandle(handle -> { + handle.attach(TestDAO.class); + return null; }); } catch (RuntimeException e) { - assertFalse(rdbi.proxyFactory.methodContextCache.containsKey(TestDAO.class)); - assertFalse(rdbi.proxyFactory.factoryCache.containsKey(TestDAO.class)); + assertFalse(rdbi.proxyFactory.isCached(TestDAO.class)); } } @@ -56,13 +50,11 @@ void testBasicWithRuntimeException() { RDBI rdbi = new RDBI(RDBITest.getBadJedisPool()); - rdbi.withHandle(new Callback() { - @Override - public Object run(Handle handle) { - handle.jedis().get("hello"); - fail("Should have thrown exception on get"); - return null; - } + rdbi.withHandle(handle -> { + handle.jedis().get("hello"); + // probably same issue as other exception tests + fail("Should have thrown exception on get"); + return null; }); } } From 7f3c17cbda647538c6a1c79c0c821c5d7c0d4345 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Wed, 28 Dec 2022 10:16:18 -0600 Subject: [PATCH 04/29] CARE-no-cglib: checkpoint --- .../java/com/lithium/dbi/rdbi/RDBITest.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java index 0986571..f6368b3 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java @@ -203,6 +203,41 @@ public void testCacheHitDAO() { } } + @Test + public void testSeparation() { + RDBI realRdbi = new RDBI(new JedisPool("localhost", 6379)); + RDBI mockRdbi = new RDBI(getMockJedisPool()); + RDBI badRdbi = new RDBI(getBadJedisPool()); + + + Jedis realJedis = realRdbi.open().jedis(); + Jedis mockJedis = mockRdbi.open().jedis(); + Jedis badJedis = badRdbi.open().jedis(); + + + realRdbi.withHandle(h -> { + Jedis jedis = h.jedis(); + jedis.set("hi", "there"); + return 0; + }); + + + mockRdbi.withHandle(h -> { + Jedis jedis = h.jedis(); + jedis.set("hi", "there"); + return 0; + }); + + + badRdbi.withHandle(h -> { + Jedis jedis = h.jedis(); + jedis.set("hi", "there"); + return 0; + }); + + + } + @SuppressWarnings("unchecked") static JedisPool getMockJedisPool() { Jedis jedis = mock(Jedis.class); From a1686e11f95521234bd81618df55646549bb46c4 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Wed, 28 Dec 2022 14:06:47 -0600 Subject: [PATCH 05/29] CARE-no-cglib: most things working now --- .../dbi/rdbi/BBMethodContextInterceptor.java | 2 +- .../rdbi/JedisWrapperMethodInterceptor.java | 64 +++++++++++++++---- .../com/lithium/dbi/rdbi/ProxyFactory.java | 32 +++++++--- .../java/com/lithium/dbi/rdbi/RDBITest.java | 49 ++------------ .../lithium/dbi/rdbi/RDBIWithHandleTest.java | 2 +- 5 files changed, 84 insertions(+), 65 deletions(-) diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java index e440137..a013b2a 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java @@ -42,7 +42,7 @@ public Object intercept(@AllArguments Object[] args, try { parameterType = context.getMapper().getClass().getMethod("map", Integer.class).getParameterTypes()[0]; } catch (NoSuchMethodException e) { - throw new RuntimeException(e); + parameterType = null; } return context.getMapper().map(adjust(parameterType, ret)); } else { diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java index 43bedf0..c6a878c 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java @@ -20,6 +20,7 @@ import java.lang.invoke.MethodHandles; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Objects; import java.util.concurrent.Callable; public class JedisWrapperMethodInterceptor { @@ -27,18 +28,45 @@ public class JedisWrapperMethodInterceptor { private final Jedis jedis; private final Tracer tracer; private final Attributes commonAttributes; - private static final TypeCache> cache = new TypeCache<>(); + private static final TypeCache cache = new TypeCache<>(); + + + static class Key { + private final Class klass; + private final long hashCode; + + Key(Object toCache) { + this(toCache.getClass(), System.identityHashCode(toCache)); + } + + Key(Class klass, long hashCode){ + this.klass = klass; + this.hashCode = hashCode; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + Key key = (Key) o; + + if (hashCode != key.hashCode) return false; + return Objects.equals(klass, key.klass); + } + + @Override + public int hashCode() { + return Objects.hash(klass, hashCode); + } + } static Jedis newInstance(final Jedis realJedis, final Tracer tracer) { try { - return (Jedis) cache.findOrInsert(Jedis.class.getClassLoader(), Jedis.class, () -> new ByteBuddy() - .subclass(JedisWrapperDoNotUse.class) - .method(ElementMatchers.isMethod()) - .intercept(MethodDelegation.to(new JedisWrapperMethodInterceptor(realJedis, tracer), "intercept")) - .make() - .load(Jedis.class.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup)) - .getLoaded()).getDeclaredConstructor() + return (Jedis) cache.findOrInsert(Jedis.class.getClassLoader(), new Key(realJedis), () -> + newLoadedClass(realJedis, tracer)) + .getDeclaredConstructor() .newInstance(); } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { @@ -46,6 +74,16 @@ static Jedis newInstance(final Jedis realJedis, final Tracer tracer) { } } + private static Class newLoadedClass(Jedis realJedis, Tracer tracer) { + return new ByteBuddy() + .subclass(JedisWrapperDoNotUse.class) + .method(ElementMatchers.isMethod()) + .intercept(MethodDelegation.to(new JedisWrapperMethodInterceptor(realJedis, tracer), "intercept")) + .make() + .load(Jedis.class.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup)) + .getLoaded(); + } + public JedisWrapperMethodInterceptor(Jedis jedis, Tracer tracer) { this.jedis = jedis; this.tracer = tracer; @@ -68,12 +106,16 @@ public Object intercept( } try (Scope ignored = s.makeCurrent()) { return method.invoke(jedis, args); -// return callable.call(); } catch (JedisException e) { s.recordException(e); throw e; - } catch (Exception e) { - s.recordException(e); + } catch (InvocationTargetException e) { + if (e.getCause() instanceof RuntimeException) { + throw (RuntimeException) e.getCause(); + } else { + throw new RuntimeException(e.getCause()); + } + } catch (IllegalAccessException e) { throw new RuntimeException(e); } finally { s.end(); diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java index 7eb406e..ad4b741 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java @@ -2,9 +2,11 @@ import io.opentelemetry.api.trace.Tracer; import net.bytebuddy.ByteBuddy; +import net.bytebuddy.NamingStrategy; import net.bytebuddy.TypeCache; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; +import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.matcher.ElementMatchers; import redis.clients.jedis.Jedis; @@ -12,9 +14,11 @@ import java.lang.invoke.MethodHandles; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; class ProxyFactory { @@ -27,9 +31,11 @@ Jedis attachJedis(final Jedis jedis, Tracer tracer) { @SuppressWarnings("unchecked") T createInstance(final Jedis jedis, final Class t) { try { + // return buildClass(t, jedis) return (T) cache.findOrInsert(t.getClassLoader(), t, () -> buildClass(t, jedis) - ).getDeclaredConstructor().newInstance(); + ) + .getDeclaredConstructor().newInstance(); } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { throw new RuntimeException(e); @@ -43,9 +49,21 @@ boolean isCached(Class t) { private Class buildClass(Class t, Jedis jedis) throws IllegalAccessException, InstantiationException { - DynamicType.Builder builder = new ByteBuddy() - .subclass(t); + BBMethodContextInterceptor interceptor = new BBMethodContextInterceptor(jedis, getMethodMethodContextMap(t, jedis)); + DynamicType.Unloaded make = new ByteBuddy() + .subclass(t, ConstructorStrategy.Default.DEFAULT_CONSTRUCTOR) + .method(ElementMatchers.any()) + .intercept(MethodDelegation.to(interceptor)) + .make(); + return make + // .load(t.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup, true)) // this works for the DAO tests in rdbi core but nothing else + .load(getClass().getClassLoader(), ClassLoadingStrategy.Default.WRAPPER) // this works for everything BUT the DAO tests in rdbi core + .getLoaded(); + + } + + private Map getMethodMethodContextMap(Class t, Jedis jedis) throws InstantiationException, IllegalAccessException { Map contexts = new HashMap<>(); for (Method method : t.getDeclaredMethods()) { Query query = method.getAnnotation(Query.class); @@ -72,13 +90,7 @@ private Class buildClass(Class t, Jedis jedis) throws Illega } contexts.put(method, new MethodContext(sha1, mapper, luaContext)); } - - return builder.method(ElementMatchers.any()) - .intercept(MethodDelegation.to(new BBMethodContextInterceptor(jedis, contexts))) - .make() - .load(t.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup)) - .getLoaded(); - + return Collections.unmodifiableMap(contexts); } /** diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java index f6368b3..17c10d4 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java @@ -21,7 +21,7 @@ public class RDBITest { - interface TestDAO { + public interface TestDAO { @Query( "redis.call('SET', KEYS[1], ARGV[1]);" + "return 0;" @@ -29,7 +29,7 @@ interface TestDAO { int testExec(List keys, List args); } - interface TestCopyDAO { + public interface TestCopyDAO { @Query( "redis.call('SET', KEYS[1], ARGV[1]);" + "return 0;" @@ -37,12 +37,12 @@ interface TestCopyDAO { int testExec2(List keys, List args); } - interface NoInputDAO { + public interface NoInputDAO { @Query("return 0;") int noInputMethod(); } - interface DynamicDAO { + public interface DynamicDAO { @Query( "redis.call('SET', $a$, $b$); return 0;" ) @@ -55,7 +55,7 @@ interface DynamicDAO { } - static class BasicObjectUnderTest { + public static class BasicObjectUnderTest { private final String input; @@ -67,13 +67,13 @@ public String getInput() { return input; } } - static class BasicResultMapper implements ResultMapper { + public static class BasicResultMapper implements ResultMapper { @Override public BasicObjectUnderTest map(Integer result) { return new BasicObjectUnderTest(result); } } - interface TestDAOWithResultSetMapper { + public interface TestDAOWithResultSetMapper { @Query( "redis.call('SET', KEYS[1], ARGV[1]);" + @@ -203,41 +203,6 @@ public void testCacheHitDAO() { } } - @Test - public void testSeparation() { - RDBI realRdbi = new RDBI(new JedisPool("localhost", 6379)); - RDBI mockRdbi = new RDBI(getMockJedisPool()); - RDBI badRdbi = new RDBI(getBadJedisPool()); - - - Jedis realJedis = realRdbi.open().jedis(); - Jedis mockJedis = mockRdbi.open().jedis(); - Jedis badJedis = badRdbi.open().jedis(); - - - realRdbi.withHandle(h -> { - Jedis jedis = h.jedis(); - jedis.set("hi", "there"); - return 0; - }); - - - mockRdbi.withHandle(h -> { - Jedis jedis = h.jedis(); - jedis.set("hi", "there"); - return 0; - }); - - - badRdbi.withHandle(h -> { - Jedis jedis = h.jedis(); - jedis.set("hi", "there"); - return 0; - }); - - - } - @SuppressWarnings("unchecked") static JedisPool getMockJedisPool() { Jedis jedis = mock(Jedis.class); diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java index ebfaf5a..854685d 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java @@ -13,7 +13,7 @@ public class RDBIWithHandleTest { - interface TestDAO { + public interface TestDAO { @Query( "redis.call('SET', KEYS[1], ARGV[1]);" + "return 0;" From 9f51c3f41bf9341912d6a741b0a90a910dc0588a Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Wed, 28 Dec 2022 14:28:39 -0600 Subject: [PATCH 06/29] CARE-no-cglib: ?? --- Jenkinsfile-continuous | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkinsfile-continuous b/Jenkinsfile-continuous index 2bbc34b..ff632a9 100644 --- a/Jenkinsfile-continuous +++ b/Jenkinsfile-continuous @@ -1,7 +1,7 @@ #!groovy pipeline { - agent { label 'java8 && small && dev' } + agent { label 'java11 && small && dev' } tools { maven 'Maven 3 (latest)' jdk 'Latest' From 22fb57d4d4bb7d4cf2162b70bf5a85d960da94e6 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Wed, 28 Dec 2022 14:38:49 -0600 Subject: [PATCH 07/29] CARE-no-cglib: bump failsafe plugin to rm deps that don't work on later jvms --- rdbi-parent/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rdbi-parent/pom.xml b/rdbi-parent/pom.xml index bf3fba2..b9079c3 100644 --- a/rdbi-parent/pom.xml +++ b/rdbi-parent/pom.xml @@ -164,7 +164,7 @@ org.apache.maven.plugins maven-failsafe-plugin - 2.20 + 2.22.2 From f17e450d74582da8d7dbd31b0bfcb9b8bafc801f Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Wed, 28 Dec 2022 14:45:15 -0600 Subject: [PATCH 08/29] CARE-no-cglib: yep --- .../dbi/rdbi/recipes/cache/RedisCacheInvalidateTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisCacheInvalidateTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisCacheInvalidateTest.java index 4c96e98..9ec06c1 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisCacheInvalidateTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisCacheInvalidateTest.java @@ -28,7 +28,7 @@ public class RedisCacheInvalidateTest { private static AtomicInteger sharedMutableState; @BeforeClass - public static void setUp() throws Exception { + public void setUp() { jedisPool = new JedisPool("localhost", 6379); final RDBI rdbi = new RDBI(jedisPool); @@ -84,7 +84,7 @@ public String encode(Integer value) { } @AfterClass - public static void tearDown() throws Exception { + public void tearDown() { jedisPool.close(); } From 1dba8c6455b96617e8921e5665611f1724964dc7 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Wed, 28 Dec 2022 15:55:03 -0600 Subject: [PATCH 09/29] CARE-no-cglib: checkpoint --- .../lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java index 86404ea..062f8f4 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java @@ -52,7 +52,7 @@ public void testPublishChannelLuaPerformanceTest() throws InterruptedException { thread1.start(); thread2.start(); - long timeToFinish = 1500; + long timeToFinish = 5500; thread1.join(timeToFinish); thread2.join(timeToFinish); From 6891a4c4da7627bc106f6ea3a62828ca0ccfd738 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Wed, 28 Dec 2022 16:20:05 -0600 Subject: [PATCH 10/29] CARE-no-cglib: checkpoint --- .../recipes/channel/ChannelPublisherTest.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java index 062f8f4..4cb6426 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java @@ -28,13 +28,14 @@ public void testPublishChannelLuaPerformanceTest() throws InterruptedException { final ChannelPublisher channelPublisher = new ChannelPublisher(new RDBI(new JedisPool("localhost", 6379))); channelPublisher.resetChannels(channel); - final List value =ImmutableList.of("value1"); + final List value = ImmutableList.of("value1"); final AtomicBoolean thread1Finished = new AtomicBoolean(false); final AtomicBoolean thread2Finished = new AtomicBoolean(false); + Thread thread1 = new Thread(() -> { - for ( int i = 0; i < 1000; i++) { - channelPublisher.publish(channel, value ); + for (int i = 0; i < 1000; i++) { + channelPublisher.publish(channel, value); if (Thread.interrupted()) { return; @@ -43,8 +44,8 @@ public void testPublishChannelLuaPerformanceTest() throws InterruptedException { thread1Finished.set(true); }); Thread thread2 = new Thread(() -> { - for ( int i = 0; i < 1000; i++) { - channelPublisher.publish(channel, value ); + for (int i = 0; i < 1000; i++) { + channelPublisher.publish(channel, value); } thread2Finished.set(true); }); @@ -157,15 +158,15 @@ public void testPublishChannelPerformanceTest() throws InterruptedException { final ChannelPublisher channelPublisher = new ChannelPublisher(rdbi); channelPublisher.resetChannels(channel); - final List value =ImmutableList.of("value1"); + final List value = ImmutableList.of("value1"); final AtomicBoolean thread1Finished = new AtomicBoolean(false); final AtomicBoolean thread2Finished = new AtomicBoolean(false); Thread thread1 = new Thread(new Runnable() { @Override public void run() { - for ( int i = 0; i < 1000; i++) { - channelPublisher.publish(channel, value ); + for (int i = 0; i < 1000; i++) { + channelPublisher.publish(channel, value); if (Thread.interrupted()) { return; @@ -177,8 +178,8 @@ public void run() { Thread thread2 = new Thread(new Runnable() { @Override public void run() { - for ( int i = 0; i < 1000; i++) { - channelPublisher.publish(channel, value ); + for (int i = 0; i < 1000; i++) { + channelPublisher.publish(channel, value); } thread2Finished.set(true); } From 27519c1b1f101f6b78b0d6de8bd6b2025a5fbabb Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Wed, 28 Dec 2022 17:13:23 -0600 Subject: [PATCH 11/29] CARE-no-cglib: use instance creation to create a stateful delegate w/out rebuilding the class --- .../rdbi/JedisWrapperMethodInterceptor.java | 56 +++++-------------- 1 file changed, 13 insertions(+), 43 deletions(-) diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java index c6a878c..2f5b4ea 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java @@ -7,78 +7,49 @@ import io.opentelemetry.context.Scope; import net.bytebuddy.ByteBuddy; import net.bytebuddy.TypeCache; +import net.bytebuddy.description.modifier.Visibility; import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.implementation.bind.annotation.AllArguments; import net.bytebuddy.implementation.bind.annotation.Origin; import net.bytebuddy.implementation.bind.annotation.RuntimeType; -import net.bytebuddy.implementation.bind.annotation.SuperCall; import net.bytebuddy.matcher.ElementMatchers; import redis.clients.jedis.Jedis; import redis.clients.jedis.exceptions.JedisException; import java.lang.invoke.MethodHandles; +import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.Objects; -import java.util.concurrent.Callable; public class JedisWrapperMethodInterceptor { private final Jedis jedis; private final Tracer tracer; private final Attributes commonAttributes; - private static final TypeCache cache = new TypeCache<>(); - - - static class Key { - private final Class klass; - private final long hashCode; - - Key(Object toCache) { - this(toCache.getClass(), System.identityHashCode(toCache)); - } - - Key(Class klass, long hashCode){ - this.klass = klass; - this.hashCode = hashCode; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - Key key = (Key) o; - - if (hashCode != key.hashCode) return false; - return Objects.equals(klass, key.klass); - } - - @Override - public int hashCode() { - return Objects.hash(klass, hashCode); - } - } + private static final TypeCache> cache = new TypeCache<>(); static Jedis newInstance(final Jedis realJedis, final Tracer tracer) { try { - return (Jedis) cache.findOrInsert(Jedis.class.getClassLoader(), new Key(realJedis), () -> - newLoadedClass(realJedis, tracer)) + Object proxy = cache.findOrInsert(Jedis.class.getClassLoader(), Jedis.class, JedisWrapperMethodInterceptor::newLoadedClass) .getDeclaredConstructor() .newInstance(); - } catch (InstantiationException | IllegalAccessException | InvocationTargetException | - NoSuchMethodException e) { + final Field field = proxy.getClass().getDeclaredField("handler"); + field.set(proxy, new JedisWrapperMethodInterceptor(realJedis, tracer)); + return (Jedis) proxy; + } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException | + NoSuchFieldException e) { throw new RuntimeException(e); } } - private static Class newLoadedClass(Jedis realJedis, Tracer tracer) { + private static Class newLoadedClass() { return new ByteBuddy() .subclass(JedisWrapperDoNotUse.class) + .defineField("handler", JedisWrapperMethodInterceptor.class, Visibility.PUBLIC) .method(ElementMatchers.isMethod()) - .intercept(MethodDelegation.to(new JedisWrapperMethodInterceptor(realJedis, tracer), "intercept")) + .intercept(MethodDelegation.toField("handler")) .make() .load(Jedis.class.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup)) .getLoaded(); @@ -96,8 +67,7 @@ public JedisWrapperMethodInterceptor(Jedis jedis, Tracer tracer) { @RuntimeType public Object intercept( @AllArguments Object[] args, - @Origin Method method, - @SuperCall Callable callable) { + @Origin Method method) { Span s = tracer.spanBuilder(method.getName()) .setAllAttributes(commonAttributes) .startSpan(); From 3759ba4aff77972fcebb58587b37e887321c9836 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Wed, 28 Dec 2022 17:16:05 -0600 Subject: [PATCH 12/29] CARE-no-cglib: put this back --- .../lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java index 4cb6426..804eb1c 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java @@ -53,7 +53,7 @@ public void testPublishChannelLuaPerformanceTest() throws InterruptedException { thread1.start(); thread2.start(); - long timeToFinish = 5500; + long timeToFinish = 1500; thread1.join(timeToFinish); thread2.join(timeToFinish); From 9401bf040aa0de31fe5e302a46a385c4b71e779a Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Thu, 29 Dec 2022 11:20:06 -0600 Subject: [PATCH 13/29] CARE-no-cglib: make it so we can know how actually bad we are --- .../recipes/channel/ChannelPublisherTest.java | 127 ++++++++---------- 1 file changed, 57 insertions(+), 70 deletions(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java index 804eb1c..650d6d8 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java @@ -8,9 +8,17 @@ import redis.clients.jedis.JedisPool; import java.time.Instant; +import java.util.Arrays; import java.util.List; import java.util.Set; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertNotNull; @@ -149,6 +157,31 @@ public void getDepthTest() { } } + static void timedThreads(long timeLimit, TimeUnit unit, Runnable... runnables) { + long timeOut = timeLimit * 10; + long start = System.currentTimeMillis(); + + + List> futures = Arrays.stream(runnables) + .map(CompletableFuture::runAsync) + .collect(Collectors.toList()); + + CompletableFuture all = CompletableFuture.allOf(futures.toArray(new CompletableFuture[futures.size()])); + + try { + all.get(timeOut, unit); + } catch (TimeoutException e) { + fail(String.format("did not complete within time limit %d %s, or timeout %d %s", timeLimit, unit, timeOut, unit)); + } catch (ExecutionException | InterruptedException e) { + throw new RuntimeException(e); + } + + long elapsed = System.currentTimeMillis() - start; + if (elapsed > timeLimit) { + fail(String.format("Did not finish in time. Expected to finish in %d %s, but finished in %d", timeLimit, unit, elapsed)); + } + } + @Test public void testPublishChannelPerformanceTest() throws InterruptedException { @@ -159,80 +192,34 @@ public void testPublishChannelPerformanceTest() throws InterruptedException { channelPublisher.resetChannels(channel); final List value = ImmutableList.of("value1"); - final AtomicBoolean thread1Finished = new AtomicBoolean(false); - final AtomicBoolean thread2Finished = new AtomicBoolean(false); - - Thread thread1 = new Thread(new Runnable() { - @Override - public void run() { - for (int i = 0; i < 1000; i++) { - channelPublisher.publish(channel, value); - - if (Thread.interrupted()) { - return; - } - } - thread1Finished.set(true); - } - }); - Thread thread2 = new Thread(new Runnable() { - @Override - public void run() { - for (int i = 0; i < 1000; i++) { - channelPublisher.publish(channel, value); - } - thread2Finished.set(true); - } - }); - - thread1.start(); - thread2.start(); - long timeToFinish = 1500; - thread1.join(timeToFinish); - thread2.join(timeToFinish); - - if (!thread1Finished.get() || !thread2Finished.get()) { - fail("Did not finish in time"); - } + timedThreads(1500, TimeUnit.MILLISECONDS, + () -> { + for (int i = 0; i < 1000; i++) { + channelPublisher.publish(channel, value); + } + }, + () -> { + for (int i = 0; i < 1000; i++) { + channelPublisher.publish(channel, value); + } + } + ); final ChannelReceiver receiver = new ChannelLuaReceiver(rdbi); - final AtomicBoolean thread3Finished = new AtomicBoolean(false); - final AtomicBoolean thread4Finished = new AtomicBoolean(false); - - Thread thread3 = new Thread(new Runnable() { - @Override - public void run() { - for (int i = 0; i < 100; i++) { - receiver.get(channel.iterator().next(), 900L); - } - thread3Finished.set(true); - } - }); - - Thread thread4 = new Thread(new Runnable() { - @Override - public void run() { - for (int i = 0; i < 100; i++) { - receiver.get(channel.iterator().next(), 900L); - } - thread4Finished.set(true); - } - }); - - Instant before = Instant.now(); - thread3.start(); - thread4.start(); - - thread3.join(timeToFinish); - thread4.join(timeToFinish); - Instant after = Instant.now(); - - if (!thread3Finished.get() || !thread3Finished.get()) { - fail("Did not finish in time"); - } + timedThreads(1500, TimeUnit.MILLISECONDS, + () -> { + for (int i = 0; i < 100; i++) { + receiver.get(channel.iterator().next(), 900L); + } + }, + () -> { + for (int i = 0; i < 100; i++) { + receiver.get(channel.iterator().next(), 900L); + } + } + ); - System.out.println("final time " + (after.toEpochMilli() - before.toEpochMilli())); } } From ab23d30e7f531c359319de87f4052ea4a49feff1 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Thu, 29 Dec 2022 11:46:31 -0600 Subject: [PATCH 14/29] CARE-no-cglib: more info in timing tests --- .../TokenBucketRateLimiterTest.java | 2 +- .../recipes/channel/ChannelPublisherTest.java | 102 +++--------------- .../presence/PresenceRepositoryTest.java | 61 +++++------ .../scheduler/ExclusiveJobSchedulerTest.java | 4 +- .../scheduler/MultiChannelSchedulerTest.java | 2 +- .../PriorityBasedJobSchedulerTest.java | 4 +- .../StateDedupedJobSchedulerTest.java | 6 +- .../scheduler/TimeBasedJobSchedulerTest.java | 4 +- .../dbi/rdbi/{ => testutil}/TestClock.java | 2 +- .../lithium/dbi/rdbi/testutil/TubeUtils.java | 10 -- .../com/lithium/dbi/rdbi/testutil/Utils.java | 45 ++++++++ 11 files changed, 97 insertions(+), 145 deletions(-) rename rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/{ => testutil}/TestClock.java (90%) delete mode 100644 rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/TubeUtils.java create mode 100644 rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/Utils.java diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/ratelimiter/TokenBucketRateLimiterTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/ratelimiter/TokenBucketRateLimiterTest.java index 2b7f7ce..fceae9c 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/ratelimiter/TokenBucketRateLimiterTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/ratelimiter/TokenBucketRateLimiterTest.java @@ -3,7 +3,7 @@ import com.lithium.dbi.rdbi.Callback; import com.lithium.dbi.rdbi.Handle; import com.lithium.dbi.rdbi.RDBI; -import com.lithium.dbi.rdbi.TestClock; +import com.lithium.dbi.rdbi.testutil.TestClock; import org.testng.annotations.Test; import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPool; diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java index 650d6d8..6aa1bc5 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelPublisherTest.java @@ -7,19 +7,12 @@ import org.testng.annotations.Test; import redis.clients.jedis.JedisPool; -import java.time.Instant; -import java.util.Arrays; import java.util.List; import java.util.Set; -import java.util.concurrent.Callable; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Collectors; +import java.util.stream.IntStream; +import static com.lithium.dbi.rdbi.testutil.Utils.assertTiming; import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertNotNull; import static org.testng.AssertJUnit.assertNull; @@ -37,37 +30,11 @@ public void testPublishChannelLuaPerformanceTest() throws InterruptedException { channelPublisher.resetChannels(channel); final List value = ImmutableList.of("value1"); - final AtomicBoolean thread1Finished = new AtomicBoolean(false); - final AtomicBoolean thread2Finished = new AtomicBoolean(false); - - - Thread thread1 = new Thread(() -> { - for (int i = 0; i < 1000; i++) { - channelPublisher.publish(channel, value); - - if (Thread.interrupted()) { - return; - } - } - thread1Finished.set(true); - }); - Thread thread2 = new Thread(() -> { - for (int i = 0; i < 1000; i++) { - channelPublisher.publish(channel, value); - } - thread2Finished.set(true); - }); - - thread1.start(); - thread2.start(); - - long timeToFinish = 1500; - thread1.join(timeToFinish); - thread2.join(timeToFinish); - - if (!thread1Finished.get() && !thread2Finished.get()) { - fail("Did not finish in time"); - } + + assertTiming(1500, TimeUnit.MILLISECONDS, + () -> IntStream.range(0, 1000).forEach(i -> channelPublisher.publish(channel, value)), + () -> IntStream.range(0, 1000).forEach(i -> channelPublisher.publish(channel, value)) + ); } @Test @@ -157,33 +124,8 @@ public void getDepthTest() { } } - static void timedThreads(long timeLimit, TimeUnit unit, Runnable... runnables) { - long timeOut = timeLimit * 10; - long start = System.currentTimeMillis(); - - - List> futures = Arrays.stream(runnables) - .map(CompletableFuture::runAsync) - .collect(Collectors.toList()); - - CompletableFuture all = CompletableFuture.allOf(futures.toArray(new CompletableFuture[futures.size()])); - - try { - all.get(timeOut, unit); - } catch (TimeoutException e) { - fail(String.format("did not complete within time limit %d %s, or timeout %d %s", timeLimit, unit, timeOut, unit)); - } catch (ExecutionException | InterruptedException e) { - throw new RuntimeException(e); - } - - long elapsed = System.currentTimeMillis() - start; - if (elapsed > timeLimit) { - fail(String.format("Did not finish in time. Expected to finish in %d %s, but finished in %d", timeLimit, unit, elapsed)); - } - } - @Test - public void testPublishChannelPerformanceTest() throws InterruptedException { + public void testPublishChannelPerformanceTest() { final Set channel = ImmutableSet.of("channel1", "channel2", "channel3", "channel4", "channel5"); @@ -193,32 +135,16 @@ public void testPublishChannelPerformanceTest() throws InterruptedException { final List value = ImmutableList.of("value1"); - timedThreads(1500, TimeUnit.MILLISECONDS, - () -> { - for (int i = 0; i < 1000; i++) { - channelPublisher.publish(channel, value); - } - }, - () -> { - for (int i = 0; i < 1000; i++) { - channelPublisher.publish(channel, value); - } - } + assertTiming(1500, TimeUnit.MILLISECONDS, + () -> IntStream.range(0, 1000).forEach(i -> channelPublisher.publish(channel, value)), + () -> IntStream.range(0, 1000).forEach(i -> channelPublisher.publish(channel, value)) ); final ChannelReceiver receiver = new ChannelLuaReceiver(rdbi); - timedThreads(1500, TimeUnit.MILLISECONDS, - () -> { - for (int i = 0; i < 100; i++) { - receiver.get(channel.iterator().next(), 900L); - } - }, - () -> { - for (int i = 0; i < 100; i++) { - receiver.get(channel.iterator().next(), 900L); - } - } + assertTiming(1500, TimeUnit.MILLISECONDS, + () -> IntStream.range(0, 1000).forEach(i -> channelPublisher.publish(channel, value)), + () -> IntStream.range(0, 1000).forEach(i -> channelPublisher.publish(channel, value)) ); } diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java index 7efd13b..b605e32 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java @@ -10,7 +10,10 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.concurrent.TimeUnit; +import java.util.stream.IntStream; +import static com.lithium.dbi.rdbi.testutil.Utils.assertTiming; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; @@ -19,7 +22,7 @@ public class PresenceRepositoryTest { @Test - public void addTest () throws InterruptedException { + public void addTest() throws InterruptedException { final PresenceRepository presenceRepository = new PresenceRepository(new RDBI(new JedisPool("localhost", 6379)), "myprefix"); @@ -44,41 +47,29 @@ public void basicPerformanceTest() throws InterruptedException { final PresenceRepository presenceRepository = new PresenceRepository(new RDBI(new JedisPool("localhost", 6379)), "myprefix"); presenceRepository.nukeForTest("mytube"); - Instant before = Instant.now(); - for ( int i = 0; i < 10000; i++ ) { - presenceRepository.addHeartbeat("mytube", "id" + i, 10 * 1000L); - } - Instant after = Instant.now(); - System.out.println("Time for 10,000 heartbeats " + Long.toString(after.toEpochMilli() - before.toEpochMilli())); - - assertTrue(after.toEpochMilli() - before.toEpochMilli() < 2000L); - - Instant before2 = Instant.now(); - for ( int i = 0; i < 10000; i++ ) { - assertFalse(presenceRepository.expired("mytube", "id" + i)); - } - Instant after2 = Instant.now(); - System.out.println("Time for 10,000 expired " + Long.toString(after2.toEpochMilli() - before2.toEpochMilli())); - - assertTrue(after2.toEpochMilli() - before2.toEpochMilli() < 2000L); - - Thread.sleep(10 * 1000L); - - Instant before3 = Instant.now(); - for ( int i = 0; i < 5000; i++ ) { - assertTrue(presenceRepository.remove("mytube", "id" + i)); - } - Instant after3 = Instant.now(); - System.out.println("Time for 5000 removes " + Long.toString(after3.toEpochMilli() - before3.toEpochMilli())); - - assertTrue(after3.toEpochMilli() - before3.toEpochMilli() < 1000L); - - Instant before4 = Instant.now(); - presenceRepository.cull("mytube"); - Instant after4 = Instant.now(); - System.out.println("Time for 5000 cull " + Long.toString(after4.toEpochMilli() - before4.toEpochMilli())); + assertTiming(2000, TimeUnit.MILLISECONDS, + () -> IntStream + .range(0, 10_000) + .forEach(i -> presenceRepository.addHeartbeat("mytube", "id" + i, 10 * 1000L)) + ); + + assertTiming(2000, TimeUnit.MILLISECONDS, + () -> IntStream + .range(0, 10_000) + .forEach(i -> assertFalse(presenceRepository.expired("mytube", "id" + i))) + ); + Thread.sleep(2_000L); + + assertTiming(2000, TimeUnit.MILLISECONDS, + () -> IntStream + .range(0, 5_000) + .forEach(i -> assertTrue(presenceRepository.remove("mytube", "id" + i))) + ); + + assertTiming(500L, TimeUnit.MILLISECONDS, + () -> presenceRepository.cull("mytube") + ); - assertTrue(after4.toEpochMilli() - before4.toEpochMilli() < 500L); } @Test diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/ExclusiveJobSchedulerTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/ExclusiveJobSchedulerTest.java index 8a775cc..5443fe2 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/ExclusiveJobSchedulerTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/ExclusiveJobSchedulerTest.java @@ -4,7 +4,7 @@ import com.lithium.dbi.rdbi.Callback; import com.lithium.dbi.rdbi.Handle; import com.lithium.dbi.rdbi.RDBI; -import com.lithium.dbi.rdbi.testutil.TubeUtils; +import com.lithium.dbi.rdbi.testutil.Utils; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -30,7 +30,7 @@ public class ExclusiveJobSchedulerTest { @BeforeMethod public void setup(){ - tubeName = TubeUtils.uniqueTubeName(); + tubeName = Utils.uniqueTubeName(); scheduledJobSystem = new ExclusiveJobScheduler(rdbi, "myprefix:"); } diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/MultiChannelSchedulerTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/MultiChannelSchedulerTest.java index 337f460..df7715b 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/MultiChannelSchedulerTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/MultiChannelSchedulerTest.java @@ -2,7 +2,7 @@ import com.lithium.dbi.rdbi.Handle; import com.lithium.dbi.rdbi.RDBI; -import com.lithium.dbi.rdbi.TestClock; +import com.lithium.dbi.rdbi.testutil.TestClock; import org.testng.annotations.AfterMethod; import org.testng.annotations.Test; import redis.clients.jedis.JedisPool; diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/PriorityBasedJobSchedulerTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/PriorityBasedJobSchedulerTest.java index 1ff1be0..b2d500f 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/PriorityBasedJobSchedulerTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/PriorityBasedJobSchedulerTest.java @@ -2,7 +2,7 @@ import com.lithium.dbi.rdbi.Callback; import com.lithium.dbi.rdbi.RDBI; -import com.lithium.dbi.rdbi.testutil.TubeUtils; +import com.lithium.dbi.rdbi.testutil.Utils; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -25,7 +25,7 @@ public class PriorityBasedJobSchedulerTest { @BeforeMethod public void setup(){ - tubeName = TubeUtils.uniqueTubeName(); + tubeName = Utils.uniqueTubeName(); scheduledJobSystem = new PriorityBasedJobScheduler(rdbi, "myprefix:"); } diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/StateDedupedJobSchedulerTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/StateDedupedJobSchedulerTest.java index d7814ed..f4d4d5c 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/StateDedupedJobSchedulerTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/StateDedupedJobSchedulerTest.java @@ -1,8 +1,8 @@ package com.lithium.dbi.rdbi.recipes.scheduler; import com.lithium.dbi.rdbi.RDBI; -import com.lithium.dbi.rdbi.TestClock; -import com.lithium.dbi.rdbi.testutil.TubeUtils; +import com.lithium.dbi.rdbi.testutil.TestClock; +import com.lithium.dbi.rdbi.testutil.Utils; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -27,7 +27,7 @@ public class StateDedupedJobSchedulerTest { @BeforeMethod public void setup() { - tubeName = TubeUtils.uniqueTubeName(); + tubeName = Utils.uniqueTubeName(); scheduledJobSystem = new StateDedupedJobScheduler(rdbi, "myprefix:"); } diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/TimeBasedJobSchedulerTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/TimeBasedJobSchedulerTest.java index 11597dd..10c7b03 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/TimeBasedJobSchedulerTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/scheduler/TimeBasedJobSchedulerTest.java @@ -3,7 +3,7 @@ import com.google.common.collect.ImmutableSet; import com.lithium.dbi.rdbi.Callback; import com.lithium.dbi.rdbi.RDBI; -import com.lithium.dbi.rdbi.testutil.TubeUtils; +import com.lithium.dbi.rdbi.testutil.Utils; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -33,7 +33,7 @@ public class TimeBasedJobSchedulerTest { @BeforeMethod public void setup(){ - tubeName = TubeUtils.uniqueTubeName(); + tubeName = Utils.uniqueTubeName(); scheduledJobSystem = new TimeBasedJobScheduler(rdbi, "myprefix:"); } diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/TestClock.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/TestClock.java similarity index 90% rename from rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/TestClock.java rename to rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/TestClock.java index 736a30f..72e2fbd 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/TestClock.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/TestClock.java @@ -1,4 +1,4 @@ -package com.lithium.dbi.rdbi; +package com.lithium.dbi.rdbi.testutil; import java.util.function.LongSupplier; diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/TubeUtils.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/TubeUtils.java deleted file mode 100644 index 7702605..0000000 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/TubeUtils.java +++ /dev/null @@ -1,10 +0,0 @@ -package com.lithium.dbi.rdbi.testutil; - -import java.util.UUID; - -public class TubeUtils { - - public static String uniqueTubeName() { - return "test_tube_" + UUID.randomUUID().toString(); - } -} diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/Utils.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/Utils.java new file mode 100644 index 0000000..b0b1367 --- /dev/null +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/testutil/Utils.java @@ -0,0 +1,45 @@ +package com.lithium.dbi.rdbi.testutil; + +import java.util.Arrays; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.stream.Collectors; + +import static org.testng.AssertJUnit.fail; + +public class Utils { + + public static String uniqueTubeName() { + return "test_tube_" + UUID.randomUUID().toString(); + } + + public static void assertTiming(long timeLimit, TimeUnit unit, Runnable... runnables) { + long timeOut = timeLimit * 10; + long start = System.currentTimeMillis(); + + + List> futures = Arrays.stream(runnables) + .map(CompletableFuture::runAsync) + .collect(Collectors.toList()); + + CompletableFuture all = CompletableFuture.allOf(futures.toArray(new CompletableFuture[futures.size()])); + + try { + all.get(timeOut, unit); + } catch (TimeoutException e) { + fail(String.format("did not complete within time limit %d %s, or timeout %d %s", timeLimit, unit, timeOut, unit)); + } catch (ExecutionException | InterruptedException e) { + throw new RuntimeException(e); + } + + long elapsed = System.currentTimeMillis() - start; + if (elapsed > timeLimit) { + fail(String.format("Did not finish in time. Expected to finish in %d %s, but finished in %d", timeLimit, unit, elapsed)); + } + } + +} From 121f4077f1a317f57e1a531dfb3f2fd24f5802ac Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Thu, 29 Dec 2022 12:47:10 -0600 Subject: [PATCH 15/29] CARE-no-cglib: working some more ideas --- .../rdbi/JedisWrapperMethodInterceptor.java | 7 ++--- .../com/lithium/dbi/rdbi/ProxyFactory.java | 27 +++++++++++-------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java index 2f5b4ea..c7c0231 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/JedisWrapperMethodInterceptor.java @@ -6,7 +6,6 @@ import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Scope; import net.bytebuddy.ByteBuddy; -import net.bytebuddy.TypeCache; import net.bytebuddy.description.modifier.Visibility; import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; import net.bytebuddy.implementation.MethodDelegation; @@ -27,13 +26,11 @@ public class JedisWrapperMethodInterceptor { private final Jedis jedis; private final Tracer tracer; private final Attributes commonAttributes; - private static final TypeCache> cache = new TypeCache<>(); + private static final Class clazz = newLoadedClass(); static Jedis newInstance(final Jedis realJedis, final Tracer tracer) { - try { - Object proxy = cache.findOrInsert(Jedis.class.getClassLoader(), Jedis.class, JedisWrapperMethodInterceptor::newLoadedClass) - .getDeclaredConstructor() + Object proxy = clazz.getDeclaredConstructor() .newInstance(); final Field field = proxy.getClass().getDeclaredField("handler"); field.set(proxy, new JedisWrapperMethodInterceptor(realJedis, tracer)); diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java index ad4b741..2fabd76 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java @@ -2,8 +2,6 @@ import io.opentelemetry.api.trace.Tracer; import net.bytebuddy.ByteBuddy; -import net.bytebuddy.NamingStrategy; -import net.bytebuddy.TypeCache; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; @@ -11,30 +9,27 @@ import net.bytebuddy.matcher.ElementMatchers; import redis.clients.jedis.Jedis; -import java.lang.invoke.MethodHandles; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; class ProxyFactory { - private final TypeCache> cache = new TypeCache<>(); - + //private final TypeCache> cache = new TypeCache<>(); + final Map, Class> cache = new ConcurrentHashMap<>(); Jedis attachJedis(final Jedis jedis, Tracer tracer) { return JedisWrapperMethodInterceptor.newInstance(jedis, tracer); } @SuppressWarnings("unchecked") T createInstance(final Jedis jedis, final Class t) { + // TODO: implement delegate/field pattern here too? try { - // return buildClass(t, jedis) - return (T) cache.findOrInsert(t.getClassLoader(), t, () -> - buildClass(t, jedis) - ) + return (T) get(t, jedis) .getDeclaredConstructor().newInstance(); } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { @@ -43,10 +38,20 @@ T createInstance(final Jedis jedis, final Class t) { } boolean isCached(Class t) { - return cache.find(t.getClassLoader(), t) != null; + return cache.get(t) != null; } + private Class get(Class t, Jedis jedis) throws IllegalAccessException, InstantiationException { + Class cached = cache.get(t); + if (cached != null){ + return cached; + } else { + Class newClass = buildClass(t, jedis); + cache.putIfAbsent(t, newClass); + return cache.get(t); + } + } private Class buildClass(Class t, Jedis jedis) throws IllegalAccessException, InstantiationException { BBMethodContextInterceptor interceptor = new BBMethodContextInterceptor(jedis, getMethodMethodContextMap(t, jedis)); From 83731b1b81145e320cea43e41a707254bba6a2c8 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Thu, 29 Dec 2022 12:55:31 -0600 Subject: [PATCH 16/29] CARE-no-cglib: few more --- .../src/test/java/com/lithium/dbi/rdbi/RDBIOpenTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenTest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenTest.java index 97de5cc..a10d4df 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenTest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIOpenTest.java @@ -2,6 +2,7 @@ import org.testng.annotations.Test; +import redis.clients.jedis.exceptions.JedisException; public class RDBIOpenTest { @@ -10,13 +11,13 @@ interface TestDAO { int doesntMatter(); } - @Test(expectedExceptions = IllegalArgumentException.class) + @Test(expectedExceptions = JedisException.class) public void testOpenThrow() { RDBI rdbi = new RDBI(RDBITest.getBadJedisPool()); rdbi.open().attach(TestDAO.class); } - @Test(expectedExceptions = IllegalArgumentException.class) + @Test(expectedExceptions = RuntimeException.class) public void testOpenThrowRuntimeException() { RDBI rdbi = new RDBI(RDBITest.getBadJedisPoolWithRuntimeException()); rdbi.open().attach(TestDAO.class); From 1644badb41a73919a855a485238b8de779727536 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Thu, 29 Dec 2022 15:22:21 -0600 Subject: [PATCH 17/29] CARE-no-cglib: try this --- .../com/lithium/dbi/rdbi/ProxyFactory.java | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java index 2fabd76..97b7994 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java @@ -2,6 +2,7 @@ import io.opentelemetry.api.trace.Tracer; import net.bytebuddy.ByteBuddy; +import net.bytebuddy.description.modifier.Visibility; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; @@ -9,6 +10,7 @@ import net.bytebuddy.matcher.ElementMatchers; import redis.clients.jedis.Jedis; +import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Collections; @@ -19,20 +21,23 @@ class ProxyFactory { - //private final TypeCache> cache = new TypeCache<>(); final Map, Class> cache = new ConcurrentHashMap<>(); + final Map, Map> methodContextCache = new ConcurrentHashMap<>(); + Jedis attachJedis(final Jedis jedis, Tracer tracer) { return JedisWrapperMethodInterceptor.newInstance(jedis, tracer); } @SuppressWarnings("unchecked") T createInstance(final Jedis jedis, final Class t) { - // TODO: implement delegate/field pattern here too? try { - return (T) get(t, jedis) - .getDeclaredConstructor().newInstance(); - } catch (InstantiationException | IllegalAccessException | InvocationTargetException | - NoSuchMethodException e) { + BBMethodContextInterceptor interceptor = new BBMethodContextInterceptor(jedis, getMethodMethodContextMap(t, jedis)); + Object instance = get(t).getDeclaredConstructor().newInstance(); + final Field field = instance.getClass().getDeclaredField("handler"); + field.set(instance, interceptor); + return (T) instance; + } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException | + NoSuchFieldException e) { throw new RuntimeException(e); } } @@ -42,33 +47,37 @@ boolean isCached(Class t) { } - private Class get(Class t, Jedis jedis) throws IllegalAccessException, InstantiationException { + private Class get(Class t) throws IllegalAccessException, InstantiationException { Class cached = cache.get(t); - if (cached != null){ + if (cached != null) { return cached; } else { - Class newClass = buildClass(t, jedis); + Class newClass = buildClass(t); cache.putIfAbsent(t, newClass); return cache.get(t); } } - private Class buildClass(Class t, Jedis jedis) throws IllegalAccessException, InstantiationException { - BBMethodContextInterceptor interceptor = new BBMethodContextInterceptor(jedis, getMethodMethodContextMap(t, jedis)); - DynamicType.Unloaded make = new ByteBuddy() + private Class buildClass(Class t) throws IllegalAccessException, InstantiationException { + + + return new ByteBuddy() .subclass(t, ConstructorStrategy.Default.DEFAULT_CONSTRUCTOR) + .defineField("handler", BBMethodContextInterceptor.class, Visibility.PUBLIC) .method(ElementMatchers.any()) - .intercept(MethodDelegation.to(interceptor)) - .make(); - - return make - // .load(t.getClassLoader(), ClassLoadingStrategy.UsingLookup.withFallback(MethodHandles::lookup, true)) // this works for the DAO tests in rdbi core but nothing else - .load(getClass().getClassLoader(), ClassLoadingStrategy.Default.WRAPPER) // this works for everything BUT the DAO tests in rdbi core + .intercept(MethodDelegation.toField("handler")) + .make() + .load(getClass().getClassLoader(), ClassLoadingStrategy.Default.WRAPPER) // this works for everything BUT the DAO tests in rdbi core .getLoaded(); } private Map getMethodMethodContextMap(Class t, Jedis jedis) throws InstantiationException, IllegalAccessException { + Map cached = methodContextCache.get(t); + if (cached != null) { + return cached; + } + Map contexts = new HashMap<>(); for (Method method : t.getDeclaredMethods()) { Query query = method.getAnnotation(Query.class); @@ -95,7 +104,8 @@ private Map getMethodMethodContextMap(Class t, Jed } contexts.put(method, new MethodContext(sha1, mapper, luaContext)); } - return Collections.unmodifiableMap(contexts); + methodContextCache.putIfAbsent(t, Collections.unmodifiableMap(contexts)); + return methodContextCache.get(t); } /** From 260b0e6144b9aeb50aee9ce16b4406b9fbbb0bb9 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Thu, 29 Dec 2022 16:57:50 -0600 Subject: [PATCH 18/29] CARE-no-cglib: try this --- .../channel/ChannelLuaReceiverTest.java | 68 ++++++------------- 1 file changed, 20 insertions(+), 48 deletions(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelLuaReceiverTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelLuaReceiverTest.java index 705c0f8..3806a61 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelLuaReceiverTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/channel/ChannelLuaReceiverTest.java @@ -14,9 +14,12 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import java.util.stream.IntStream; +import static com.lithium.dbi.rdbi.testutil.Utils.assertTiming; import static java.util.stream.Collectors.toList; import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertTrue; @@ -162,7 +165,7 @@ public void testEmptyChannelPublishAndReceive() throws Exception { } @Test - public void testMultiThreadedMultiChannelPublishAndReceive() throws InterruptedException { + public void testMultiThreadedMultiChannelPublishAndReceive() { final Set channelSet = ImmutableSet.of("channel1", "channel2", "channel3", "channel4", "channel5"); final int messageAmount = 50; @@ -171,55 +174,24 @@ public void testMultiThreadedMultiChannelPublishAndReceive() throws InterruptedE final ChannelPublisher channelPublisher = new ChannelPublisher(rdbi); channelPublisher.resetChannels(channelSet); - final AtomicBoolean thread1Finished = new AtomicBoolean(false); - final AtomicBoolean thread2Finished = new AtomicBoolean(false); - Map uuidMap = new HashMap<>(); - Thread thread1 = new Thread(new Runnable() { - @Override - public void run() { - for (int i = 0; i < messageAmount; i++) { - String stringVal = "value" + UUID.randomUUID(); - uuidMap.put(stringVal, 0); - final List value = ImmutableList.of(stringVal); - channelPublisher.publish(channelSet, value); - - if (Thread.interrupted()) { - return; - } - } - thread1Finished.set(true); - } - }); - - Thread thread2 = new Thread(new Runnable() { - @Override - public void run() { - for (int i = 0; i < messageAmount; i++) { - String stringVal = "value" + UUID.randomUUID(); - uuidMap.put(stringVal, 0); - final List value = ImmutableList.of(stringVal); - channelPublisher.publish(channelSet, value); - - if (Thread.interrupted()) { - return; - } - } - thread2Finished.set(true); - } - }); - - thread1.start(); - thread2.start(); - - long timeToFinish = 1500; - thread1.join(timeToFinish); - thread2.join(timeToFinish); - - if (!thread1Finished.get() && !thread2Finished.get()) { - fail("Did not finish in time"); - } + assertTiming(1500L, TimeUnit.MILLISECONDS, + () -> IntStream.range(0, messageAmount) + .forEach(i -> { + String stringVal = "value" + UUID.randomUUID(); + uuidMap.put(stringVal, 0); + final List value = ImmutableList.of(stringVal); + channelPublisher.publish(channelSet, value); + }), + () -> IntStream.range(0, messageAmount) + .forEach(i -> { + String stringVal = "value" + UUID.randomUUID(); + uuidMap.put(stringVal, 0); + final List value = ImmutableList.of(stringVal); + channelPublisher.publish(channelSet, value); + }) + ); final List channels = ImmutableList.of("channel1", "channel2", "channel3", "channel4", "channel5"); From 0e829a87132eef42823129513d60ea46eca29dad Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 10:29:14 -0600 Subject: [PATCH 19/29] CARE-no-cglib: get more info on this assertion --- .../dbi/rdbi/recipes/presence/PresenceRepositoryTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java index b605e32..2a42e6d 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java @@ -100,7 +100,8 @@ public void getPresentTest() throws InterruptedException { break; } } - assertTrue(presenceRepository.getPresent(mytube, Optional.empty()).isEmpty()); + List tubeContents = presenceRepository.getPresent(mytube, Optional.empty()); + assertTrue(tubeContents.isEmpty(), String.format("tube contents should be empty, but are %s", tubeContents)); // test with limit will not return full set for (int i = 0; i < 100; ++i) { From ba40416586bb07651a692b212b7eeac7ebf02bf5 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 10:43:08 -0600 Subject: [PATCH 20/29] CARE-no-cglib: get more info on this assertion --- .../dbi/rdbi/recipes/presence/PresenceRepositoryTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java index 2a42e6d..c979697 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java @@ -84,6 +84,7 @@ public void getPresentTest() throws InterruptedException { // put something in and verify we can get it back out final String uuid = UUID.randomUUID().toString(); presenceRepository.addHeartbeat(mytube, uuid, Duration.ofSeconds(1).toMillis()); + final long insertionTimeApprox = Instant.now().toEpochMilli(); final List presentSet = presenceRepository.getPresent(mytube, Optional.empty()); assertEquals(uuid, presentSet.iterator().next(), "Expected to have one heartbeat with uuid: " + uuid); @@ -100,8 +101,10 @@ public void getPresentTest() throws InterruptedException { break; } } + final long expirationCheckApprox = Instant.now().toEpochMilli(); List tubeContents = presenceRepository.getPresent(mytube, Optional.empty()); - assertTrue(tubeContents.isEmpty(), String.format("tube contents should be empty, but are %s", tubeContents)); + assertTrue(tubeContents.isEmpty(), String.format("tube contents should be empty, but are %s. inserted around %d, checked at %d." + + " should have expire at 1s", tubeContents, insertionTimeApprox, expirationCheckApprox)); // test with limit will not return full set for (int i = 0; i < 100; ++i) { From 51658f8912cc8bbb1c4c8ad2fd892e83e1517fac Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 10:54:17 -0600 Subject: [PATCH 21/29] CARE-no-cglib: the timing on these seems trickier now --- rdbi-recipes/pom.xml | 5 ++++ .../locking/MultiReadSingleWriteLockTest.java | 24 +++++++++++-------- .../presence/PresenceRepositoryTest.java | 3 ++- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/rdbi-recipes/pom.xml b/rdbi-recipes/pom.xml index 80df030..f9364af 100644 --- a/rdbi-recipes/pom.xml +++ b/rdbi-recipes/pom.xml @@ -90,6 +90,11 @@ assertj-core test + + org.awaitility + awaitility + test + diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java index 12c7a00..22ad7cd 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java @@ -16,6 +16,7 @@ import java.util.UUID; import java.util.concurrent.TimeUnit; +import static org.awaitility.Awaitility.await; import static org.testng.Assert.assertNull; import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertTrue; @@ -82,15 +83,18 @@ public void testAcquireWriteLock_writeLockExpiration() throws Exception { lock.acquireWriteLock(newLockOwnerId); assertEquals(newLockOwnerId, handle.jedis().get(writeLockKey)); - // wait for new owner to expire and check that no one owns lock - final Instant beyondExpiration = Instant.now().plus(Duration.ofMillis(500)); - while (true) { - Thread.sleep(100); - if (Instant.now().isAfter(beyondExpiration)) { - break; - } - } - assertNull(handle.jedis().get(writeLockKey)); +// // wait for new owner to expire and check that no one owns lock +// final Instant beyondExpiration = Instant.now().plus(Duration.ofMillis(500)); +// while (true) { +// Thread.sleep(100); +// if (Instant.now().isAfter(beyondExpiration)) { +// break; +// } +// } + await() + .atLeast(Duration.ofMillis(500)) + .atMost(Duration.ofSeconds(1)) + .untilAsserted(() -> assertNull(handle.jedis().get(writeLockKey))); } } @@ -211,7 +215,7 @@ public void testAcquireReadLock_blockedByWrite() throws Exception { assertTrue(Instant.now().isAfter(expiration)); } - @Test (timeOut = 5000L) + @Test(timeOut = 5000L) public void testReacquireReadLock() throws Exception { final MultiReadSingleWriteLock lock = new MultiReadSingleWriteLock(rdbi, writeLockKey, diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java index c979697..e324560 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java @@ -75,7 +75,8 @@ public void basicPerformanceTest() throws InterruptedException { @Test public void getPresentTest() throws InterruptedException { final String mytube = "getPresentTest"; - final PresenceRepository presenceRepository = new PresenceRepository(new RDBI(new JedisPool("localhost", 6379)), "myprefix"); + RDBI rdbi = new RDBI(new JedisPool("localhost", 6379)); + final PresenceRepository presenceRepository = new PresenceRepository(rdbi, "myprefix"); presenceRepository.nukeForTest(mytube); // assert set is empty at start From a93293d14aa199a2c30c84ce53ad6761ae500856 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 10:54:46 -0600 Subject: [PATCH 22/29] CARE-no-cglib: cleanup --- .../recipes/locking/MultiReadSingleWriteLockTest.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java index 22ad7cd..e8c7112 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java @@ -12,7 +12,6 @@ import java.time.Duration; import java.time.Instant; import java.util.List; -import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; @@ -82,15 +81,6 @@ public void testAcquireWriteLock_writeLockExpiration() throws Exception { final String newLockOwnerId = UUID.randomUUID().toString(); lock.acquireWriteLock(newLockOwnerId); assertEquals(newLockOwnerId, handle.jedis().get(writeLockKey)); - -// // wait for new owner to expire and check that no one owns lock -// final Instant beyondExpiration = Instant.now().plus(Duration.ofMillis(500)); -// while (true) { -// Thread.sleep(100); -// if (Instant.now().isAfter(beyondExpiration)) { -// break; -// } -// } await() .atLeast(Duration.ofMillis(500)) .atMost(Duration.ofSeconds(1)) From 6a448a97bc6f317524c2b4c604a9c132f81048bb Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 10:57:53 -0600 Subject: [PATCH 23/29] CARE-no-cglib: the timing on these seems trickier now --- .../presence/PresenceRepositoryTest.java | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java index e324560..c3eb41e 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/presence/PresenceRepositoryTest.java @@ -14,6 +14,7 @@ import java.util.stream.IntStream; import static com.lithium.dbi.rdbi.testutil.Utils.assertTiming; +import static org.awaitility.Awaitility.await; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; @@ -67,13 +68,13 @@ public void basicPerformanceTest() throws InterruptedException { ); assertTiming(500L, TimeUnit.MILLISECONDS, - () -> presenceRepository.cull("mytube") + () -> presenceRepository.cull("mytube") ); } @Test - public void getPresentTest() throws InterruptedException { + public void getPresentTest() { final String mytube = "getPresentTest"; RDBI rdbi = new RDBI(new JedisPool("localhost", 6379)); final PresenceRepository presenceRepository = new PresenceRepository(rdbi, "myprefix"); @@ -95,18 +96,17 @@ public void getPresentTest() throws InterruptedException { assertEquals(stillpresentSet.iterator().next(), uuid, "Expected to still have one heartbeat with uuid: " + uuid); // wait a second and verify previous heartbeat is expired - final Instant beforeSleep = Instant.now(); - while (true) { - Thread.sleep(Duration.ofSeconds(1).toMillis()); - if (Duration.between(beforeSleep, Instant.now()).compareTo(Duration.ofSeconds(1)) > 0) { - break; - } - } - final long expirationCheckApprox = Instant.now().toEpochMilli(); - List tubeContents = presenceRepository.getPresent(mytube, Optional.empty()); - assertTrue(tubeContents.isEmpty(), String.format("tube contents should be empty, but are %s. inserted around %d, checked at %d." + - " should have expire at 1s", tubeContents, insertionTimeApprox, expirationCheckApprox)); - + await() + .atLeast(Duration.ofSeconds(1)) + .atMost(Duration.ofMillis(1250L)) + .untilAsserted( + () -> { + final long expirationCheckApprox = Instant.now().toEpochMilli(); + List tubeContents = presenceRepository.getPresent(mytube, Optional.empty()); + assertTrue(tubeContents.isEmpty(), String.format("tube contents should be empty, but are %s. inserted around %d, checked at %d." + + " should have expire at 1s", tubeContents, insertionTimeApprox, expirationCheckApprox)); + + }); // test with limit will not return full set for (int i = 0; i < 100; ++i) { presenceRepository.addHeartbeat(mytube, UUID.randomUUID().toString(), Duration.ofMinutes(1).toMillis()); @@ -136,14 +136,10 @@ public void getExpiredTest() throws InterruptedException { assertEquals(stillpresentSet.iterator().next(), uuid, "Expected to still have one heartbeat with uuid: " + uuid); // wait a second and verify previous heartbeat is expired - final Instant beforeSleep = Instant.now(); - while (true) { - Thread.sleep(Duration.ofSeconds(1).toMillis()); - if (Duration.between(beforeSleep, Instant.now()).compareTo(Duration.ofSeconds(1)) > 0) { - break; - } - } - assertFalse(presenceRepository.getExpired(mytube, Optional.empty()).isEmpty()); + await() + .atLeast(Duration.ofSeconds(1)) + .atMost(Duration.ofMillis(1250L)) + .untilAsserted(() -> assertFalse(presenceRepository.getExpired(mytube, Optional.empty()).isEmpty())); // test with limit will not return full set for (int i = 0; i < 100; ++i) { From 88135e7ea660f3ce03410b15d9f9f1b67bf377b2 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 11:03:04 -0600 Subject: [PATCH 24/29] CARE-no-cglib: the timing on these seems trickier now --- .../locking/MultiReadSingleWriteLockTest.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java index e8c7112..93a3f42 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/locking/MultiReadSingleWriteLockTest.java @@ -155,14 +155,12 @@ public void testAcquireReadLock_readLockExpiration() throws Exception { assertTrue(owners.contains(lockOwnerId)); // wait for expiration and verify the lock has expired - final Instant beyondExpiration = Instant.now().plus(Duration.ofMillis(500)); - while (true) { - Thread.sleep(100); - if (Instant.now().isAfter(beyondExpiration)) { - break; - } - } - assertTrue(handle.jedis().zrangeByScore(readLockKey, Long.toString(Instant.now().toEpochMilli()), "+inf", 0, 1).isEmpty()); + await() + .atLeast(Duration.ofMillis(500)) + .atMost(Duration.ofSeconds(1)) + .untilAsserted(() -> { + assertTrue(handle.jedis().zrangeByScore(readLockKey, Long.toString(Instant.now().toEpochMilli()), "+inf", 0, 1).isEmpty()); + }); } } From 5bef6beeff9bc3f488f07fc7ccf2ec2527c2c15a Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 11:46:18 -0600 Subject: [PATCH 25/29] CARE-no-cglib: correct some of these assertions (expected vs actual), and temporarly run this test many times --- .../dbi/rdbi/recipes/cache/RedisHashCacheTest.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java index 1badede..8bd06dc 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java @@ -340,7 +340,7 @@ public TestContainer apply(@Nullable String s) { assertEquals(0, loadFailure.get()); } - @Test + @Test(invocationCount = 100) public void testRemove() throws ExecutionException { final String key1 = "key1"; @@ -379,19 +379,20 @@ public void testRemove() throws ExecutionException { cache.invalidateAll(); // cache contains expected keys - assertEquals(originalValueForKey1, cache.get(key1)); - assertEquals(originalValueForKey2, cache.get(key2)); + assertEquals(cache.get(key1), originalValueForKey1); + assertEquals(cache.get(key2), originalValueForKey2); // manipulate the data source for key1 final TestContainer newValueForKey1 = new TestContainer(key1, UUID.randomUUID(), "test-remove-new-value-for-key-1"); dataSource.put(key1, newValueForKey1); - assertEquals(originalValueForKey1, cache.get(key1)); + // wait.. why did this pass??? + assertEquals(cache.get(key1), originalValueForKey1); // invalidate the key ... should reload on next request cache.invalidate(key1); assertTrue(cache.getMissing().contains(key1)); - assertEquals(newValueForKey1, cache.get(key1)); - assertEquals(originalValueForKey2, cache.get(key2)); + assertEquals(cache.get(key1), newValueForKey1); + assertEquals(cache.get(key2), originalValueForKey2); assertTrue(cache.getMissing().isEmpty()); // manipulate the data source for key1 From 77f35cef32a7738b2744db5f6635c43f9883b4f7 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 12:59:41 -0600 Subject: [PATCH 26/29] CARE-no-cglib: put this back --- .../com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java index 8bd06dc..82c3cc5 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java @@ -340,7 +340,7 @@ public TestContainer apply(@Nullable String s) { assertEquals(0, loadFailure.get()); } - @Test(invocationCount = 100) + @Test public void testRemove() throws ExecutionException { final String key1 = "key1"; From 2a77e0a8e5ad44a4a327a6feef541ab76346af57 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 13:38:49 -0600 Subject: [PATCH 27/29] CARE-no-cglib: some cleanup --- rdbi-core/pom.xml | 32 --- .../dbi/rdbi/BBMethodContextInterceptor.java | 112 ---------- .../dbi/rdbi/MethodContextInterceptor.java | 196 ++++++++++-------- .../dbi/rdbi/MethodNoOpInterceptor.java | 13 -- .../com/lithium/dbi/rdbi/ProxyFactory.java | 20 +- .../java/com/lithium/dbi/rdbi/RDBITest.java | 16 +- .../lithium/dbi/rdbi/RDBIWithHandleTest.java | 4 +- .../recipes/cache/RedisHashCacheTest.java | 1 - 8 files changed, 117 insertions(+), 277 deletions(-) delete mode 100644 rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java delete mode 100644 rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodNoOpInterceptor.java diff --git a/rdbi-core/pom.xml b/rdbi-core/pom.xml index 2d2f9a7..386464b 100644 --- a/rdbi-core/pom.xml +++ b/rdbi-core/pom.xml @@ -13,38 +13,6 @@ rdbi-core rDBI-core - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - net.bytebuddy diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java deleted file mode 100644 index a013b2a..0000000 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/BBMethodContextInterceptor.java +++ /dev/null @@ -1,112 +0,0 @@ -package com.lithium.dbi.rdbi; - -import net.bytebuddy.implementation.MethodDelegation; -import net.bytebuddy.implementation.bind.annotation.AllArguments; -import net.bytebuddy.implementation.bind.annotation.Origin; -import net.bytebuddy.implementation.bind.annotation.RuntimeType; -import redis.clients.jedis.Jedis; -import redis.clients.jedis.exceptions.JedisDataException; - -import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - -public class BBMethodContextInterceptor { - - private final Jedis jedis; - private final Map contexts; - - - public BBMethodContextInterceptor(Jedis jedis, Map contexts) { - this.jedis = jedis; - this.contexts = contexts; - } - - @RuntimeType - public Object intercept(@AllArguments Object[] args, - @Origin Method method) { - - MethodContext context = contexts.get(method); - - Object ret = context.hasDynamicLists() ? callEvalDynamicList(context, args) - : callEval(context, args); - if (ret == null) { - return null; - } - - - if (context.getMapper() != null) { - // here we need to adjust for the expected input to the mapper - Class parameterType; - try { - parameterType = context.getMapper().getClass().getMethod("map", Integer.class).getParameterTypes()[0]; - } catch (NoSuchMethodException e) { - parameterType = null; - } - return context.getMapper().map(adjust(parameterType, ret)); - } else { - return adjust(method.getReturnType(), ret); - } - } - - private Object adjust(Class c, Object ret) { - // problem here is we are getting a LONG but expected an int - // did this work on older jdks? doesn't seem to be a cglib vs bytebuddy thing - // java.lang.ClassCastException: class java.lang.Long cannot be cast to class java.lang.Integer (java.lang.Long and java.lang.Integer are in module java.base of loader 'bootstrap') - // i am not sure why this wasn't required before, and there's probably a utility out there - // to do it better the primary thing to handle is explicit cast to int where jedis returns a number - // as a long - if (c == null) { - return ret; - } else if ((c.equals(Integer.TYPE) || c.isAssignableFrom(Integer.class)) && ret instanceof Long) { - return ((Long) ret).intValue(); - } else { - return ret; - } - } - - - @SuppressWarnings("unchecked") - private Object callEval(MethodContext context, Object[] objects) { - - List keys = objects.length > 0 ? (List) objects[0] : new ArrayList<>(); - List argv = objects.length > 1 ? (List) objects[1] : new ArrayList<>(); - - return evalShaHandleReloadScript(context, keys, argv); - } - - private Object callEvalDynamicList(MethodContext context, Object[] objects) { - - List keys = new ArrayList<>(); - List argv = new ArrayList<>(); - - for (int i = 0; i < objects.length; i++) { - if (context.getLuaContext().isKey(i)) { - keys.add(objects[i].toString()); - } else { - argv.add(objects[i].toString()); - } - } - - return evalShaHandleReloadScript(context, keys, argv); - } - - private Object evalShaHandleReloadScript(MethodContext context, List keys, List argv) { - try { - return jedis.evalsha(context.getSha1(), keys, argv); - } catch (JedisDataException e) { - if (e.getMessage() != null && e.getMessage().contains("NOSCRIPT")) { - //If it throws again, we can back-off or we can just let it throw again. In this case, I think we should - //let it throw because most likely will be trying the same thing again and hopefully it will succeed later. - final String newSha = jedis.scriptLoad(context.getLuaContext().getRenderedLuaString()); - if (!newSha.equals(context.getSha1())) { - throw new IllegalStateException("sha should match but they did not"); - } - return jedis.evalsha(context.getSha1(), keys, argv); - } else { - throw e; - } - } - } -} diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodContextInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodContextInterceptor.java index 66bc1f3..f5eef62 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodContextInterceptor.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodContextInterceptor.java @@ -1,85 +1,111 @@ -//package com.lithium.dbi.rdbi; -// -//import net.sf.cglib.proxy.MethodInterceptor; -//import net.sf.cglib.proxy.MethodProxy; -//import redis.clients.jedis.Jedis; -//import redis.clients.jedis.exceptions.JedisDataException; -// -//import java.lang.reflect.Method; -//import java.util.ArrayList; -//import java.util.List; -//import java.util.Map; -// -//class MethodContextInterceptor implements MethodInterceptor { -// -// private final Jedis jedis; -// private final Map contexts; -// -// public MethodContextInterceptor(Jedis jedis, Map contexts) { -// this.jedis = jedis; -// this.contexts = contexts; -// } -// -// @Override -// @SuppressWarnings("unchecked") -// public Object intercept(Object o, Method method, Object[] objects, MethodProxy methodProxy) throws Throwable { -// -// MethodContext context = contexts.get(method); -// -// Object ret = context.hasDynamicLists() ? callEvalDynamicList(context, objects) -// : callEval(context, objects); -// -// if (ret == null) { -// return null; -// } -// -// if (contexts.get(method).getMapper() != null) { -// return contexts.get(method).getMapper().map(ret); -// } else { -// return ret; -// } -// } -// -// @SuppressWarnings("unchecked") -// private Object callEval(MethodContext context, Object[] objects) { -// -// List keys = objects.length > 0 ? (List) objects[0] : null; -// List argv = objects.length > 1 ? (List) objects[1] : null; -// -// return evalShaHandleReloadScript(context, keys, argv); -// } -// -// private Object callEvalDynamicList(MethodContext context, Object[] objects) { -// -// List keys = new ArrayList<>(); -// List argv = new ArrayList<>(); -// -// for (int i = 0; i < objects.length; i++) { -// if (context.getLuaContext().isKey(i)) { -// keys.add(objects[i].toString()); -// } else { -// argv.add(objects[i].toString()); -// } -// } -// -// return evalShaHandleReloadScript(context, keys, argv); -// } -// -// private Object evalShaHandleReloadScript(MethodContext context, List keys, List argv) { -// try { -// return jedis.evalsha(context.getSha1(), keys, argv); -// } catch (JedisDataException e) { -// if (e.getMessage() != null && e.getMessage().contains("NOSCRIPT")) { -// //If it throws again, we can back-off or we can just let it throw again. In this case, I think we should -// //let it throw because most likely will be trying the same thing again and hopefully it will succeed later. -// final String newSha = jedis.scriptLoad(context.getLuaContext().getRenderedLuaString()); -// if (!newSha.equals(context.getSha1())) { -// throw new IllegalStateException("sha should match but they did not"); -// } -// return jedis.evalsha(context.getSha1(), keys, argv); -// } else { -// throw e; -// } -// } -// } -//} +package com.lithium.dbi.rdbi; + +import net.bytebuddy.implementation.bind.annotation.AllArguments; +import net.bytebuddy.implementation.bind.annotation.Origin; +import net.bytebuddy.implementation.bind.annotation.RuntimeType; +import redis.clients.jedis.Jedis; +import redis.clients.jedis.exceptions.JedisDataException; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +public class MethodContextInterceptor { + + private final Jedis jedis; + private final Map contexts; + + + public MethodContextInterceptor(Jedis jedis, Map contexts) { + this.jedis = jedis; + this.contexts = contexts; + } + + @RuntimeType + public Object intercept(@AllArguments Object[] args, + @Origin Method method) { + + MethodContext context = contexts.get(method); + + Object ret = context.hasDynamicLists() ? callEvalDynamicList(context, args) + : callEval(context, args); + if (ret == null) { + return null; + } + + + if (context.getMapper() != null) { + // here we need to adjust for the expected input to the mapper + Class parameterType; + try { + parameterType = context.getMapper().getClass().getMethod("map", Integer.class).getParameterTypes()[0]; + } catch (NoSuchMethodException e) { + parameterType = null; + } + return context.getMapper().map(adjust(parameterType, ret)); + } else { + return adjust(method.getReturnType(), ret); + } + } + + private Object adjust(Class c, Object ret) { + // problem here is we are getting a LONG but expected an int + // did this work on older jdks? doesn't seem to be a cglib vs bytebuddy thing + // java.lang.ClassCastException: class java.lang.Long cannot be cast to class java.lang.Integer (java.lang.Long and java.lang.Integer are in module java.base of loader 'bootstrap') + // i am not sure why this wasn't required before, and there's probably a utility out there + // to do it better the primary thing to handle is explicit cast to int where jedis returns a number + // as a long + if (c == null) { + return ret; + } else if ((c.equals(Integer.TYPE) || c.isAssignableFrom(Integer.class)) && ret instanceof Long) { + return ((Long) ret).intValue(); + } else { + return ret; + } + } + + + @SuppressWarnings("unchecked") + private Object callEval(MethodContext context, Object[] objects) { + + List keys = objects.length > 0 ? (List) objects[0] : new ArrayList<>(); + List argv = objects.length > 1 ? (List) objects[1] : new ArrayList<>(); + + return evalShaHandleReloadScript(context, keys, argv); + } + + private Object callEvalDynamicList(MethodContext context, Object[] objects) { + + List keys = new ArrayList<>(); + List argv = new ArrayList<>(); + + for (int i = 0; i < objects.length; i++) { + if (context.getLuaContext().isKey(i)) { + keys.add(objects[i].toString()); + } else { + argv.add(objects[i].toString()); + } + } + + return evalShaHandleReloadScript(context, keys, argv); + } + + private Object evalShaHandleReloadScript(MethodContext context, List keys, List argv) { + try { + return jedis.evalsha(context.getSha1(), keys, argv); + } catch (JedisDataException e) { + if (e.getMessage() != null && e.getMessage().contains("NOSCRIPT")) { + //If it throws again, we can back-off or we can just let it throw again. In this case, I think we should + //let it throw because most likely will be trying the same thing again and hopefully it will succeed later. + final String newSha = jedis.scriptLoad(context.getLuaContext().getRenderedLuaString()); + if (!newSha.equals(context.getSha1())) { + throw new IllegalStateException("sha should match but they did not"); + } + return jedis.evalsha(context.getSha1(), keys, argv); + } else { + throw e; + } + } + } +} diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodNoOpInterceptor.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodNoOpInterceptor.java deleted file mode 100644 index 7aeb760..0000000 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/MethodNoOpInterceptor.java +++ /dev/null @@ -1,13 +0,0 @@ -//package com.lithium.dbi.rdbi; -// -//import net.sf.cglib.proxy.MethodInterceptor; -//import net.sf.cglib.proxy.MethodProxy; -// -//import java.lang.reflect.Method; -// -//public class MethodNoOpInterceptor implements MethodInterceptor { -// @Override -// public Object intercept(Object o, Method method, Object[] objects, MethodProxy methodProxy) throws Throwable { -// return null; -// } -//} diff --git a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java index 97b7994..763c8a5 100644 --- a/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java +++ b/rdbi-core/src/main/java/com/lithium/dbi/rdbi/ProxyFactory.java @@ -3,7 +3,6 @@ import io.opentelemetry.api.trace.Tracer; import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.modifier.Visibility; -import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; import net.bytebuddy.implementation.MethodDelegation; @@ -31,7 +30,7 @@ Jedis attachJedis(final Jedis jedis, Tracer tracer) { @SuppressWarnings("unchecked") T createInstance(final Jedis jedis, final Class t) { try { - BBMethodContextInterceptor interceptor = new BBMethodContextInterceptor(jedis, getMethodMethodContextMap(t, jedis)); + MethodContextInterceptor interceptor = new MethodContextInterceptor(jedis, getMethodMethodContextMap(t, jedis)); Object instance = get(t).getDeclaredConstructor().newInstance(); final Field field = instance.getClass().getDeclaredField("handler"); field.set(instance, interceptor); @@ -63,11 +62,11 @@ private Class buildClass(Class t) throws IllegalAccessExcept return new ByteBuddy() .subclass(t, ConstructorStrategy.Default.DEFAULT_CONSTRUCTOR) - .defineField("handler", BBMethodContextInterceptor.class, Visibility.PUBLIC) + .defineField("handler", MethodContextInterceptor.class, Visibility.PUBLIC) .method(ElementMatchers.any()) .intercept(MethodDelegation.toField("handler")) .make() - .load(getClass().getClassLoader(), ClassLoadingStrategy.Default.WRAPPER) // this works for everything BUT the DAO tests in rdbi core + .load(getClass().getClassLoader(), ClassLoadingStrategy.Default.WRAPPER) .getLoaded(); } @@ -118,17 +117,4 @@ private boolean isRawMethod(Method method) { return (method.getParameterTypes().length == 0) || (method.getParameterTypes()[0] == List.class); } - - // i don't think we need this because we don't have to lookup finalize, we just don't override it - // private static class FinalizeFilter implements CallbackFilter { - // @Override - // public int accept(Method method) { - // if (method.getName().equals("finalize") && - // method.getParameterTypes().length == 0 && - // method.getReturnType() == Void.TYPE) { - // return 0; //the NO_OP method interceptor - // } - // return 1; //the everything else method interceptor - // } - // } } diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java index 17c10d4..39f1e60 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBITest.java @@ -47,12 +47,6 @@ public interface DynamicDAO { "redis.call('SET', $a$, $b$); return 0;" ) int testExec(@BindKey("a") String a, @BindArg("b") String b); - -// @Query( -// "redis.call('SET', $a$, $b$); return 0;" -// ) -// int testExec2(@BindKey("a") String a, @BindArg("b") String b); - } public static class BasicObjectUnderTest { @@ -90,7 +84,6 @@ public void testExceptionThrownInRDBIAttach() { Handle handle = rdbi.open(); try { handle.attach(TestCopyDAO.class); - // real problem fail("Should have thrown exception for loadScript error"); } catch (RuntimeException e) { //expected @@ -172,20 +165,13 @@ public void testMethodWithNoInput() { } } - // good test to make sure shit works @Test public void testDynamicDAO() { -// RDBI rdbi = new RDBI(getMockJedisPool()); - RDBI rdbi = new RDBI(new JedisPool("localhost", 6379)); + RDBI rdbi = new RDBI(getMockJedisPool()); try (Handle handle = rdbi.open()) { handle.attach(DynamicDAO.class).testExec("a", "b"); } - - try (Handle handle = rdbi.open()) { - String val = handle.jedis().get("a"); - assertEquals(val, "b"); - } } @Test diff --git a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java index 854685d..f194b4b 100644 --- a/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java +++ b/rdbi-core/src/test/java/com/lithium/dbi/rdbi/RDBIWithHandleTest.java @@ -11,7 +11,8 @@ import static org.testng.Assert.assertNull; import static org.testng.Assert.fail; -public class RDBIWithHandleTest { +public class +RDBIWithHandleTest { public interface TestDAO { @Query( @@ -52,7 +53,6 @@ void testBasicWithRuntimeException() { rdbi.withHandle(handle -> { handle.jedis().get("hello"); - // probably same issue as other exception tests fail("Should have thrown exception on get"); return null; }); diff --git a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java index 82c3cc5..91dfef1 100644 --- a/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java +++ b/rdbi-recipes/src/test/java/com/lithium/dbi/rdbi/recipes/cache/RedisHashCacheTest.java @@ -385,7 +385,6 @@ public void testRemove() throws ExecutionException { // manipulate the data source for key1 final TestContainer newValueForKey1 = new TestContainer(key1, UUID.randomUUID(), "test-remove-new-value-for-key-1"); dataSource.put(key1, newValueForKey1); - // wait.. why did this pass??? assertEquals(cache.get(key1), originalValueForKey1); // invalidate the key ... should reload on next request From ab5984ac7da00999b3c0e1b4563d2201d218e9b2 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 13:48:07 -0600 Subject: [PATCH 28/29] CARE-no-cglib: works on java8? --- Jenkinsfile-continuous | 2 +- rdbi-parent/pom.xml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Jenkinsfile-continuous b/Jenkinsfile-continuous index ff632a9..2bbc34b 100644 --- a/Jenkinsfile-continuous +++ b/Jenkinsfile-continuous @@ -1,7 +1,7 @@ #!groovy pipeline { - agent { label 'java11 && small && dev' } + agent { label 'java8 && small && dev' } tools { maven 'Maven 3 (latest)' jdk 'Latest' diff --git a/rdbi-parent/pom.xml b/rdbi-parent/pom.xml index b9079c3..60e7b5d 100644 --- a/rdbi-parent/pom.xml +++ b/rdbi-parent/pom.xml @@ -215,8 +215,8 @@ org.apache.maven.plugins maven-compiler-plugin - 11 - 11 + 1.8 + 1.8 UTF-8 true From 11b4fdc60db3a5f8f4e4e0d1ca010859620676c2 Mon Sep 17 00:00:00 2001 From: Jesse Hodges Date: Tue, 3 Jan 2023 13:54:11 -0600 Subject: [PATCH 29/29] CARE-no-cglib: some nits --- Jenkinsfile-continuous | 2 +- rdbi-parent/pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Jenkinsfile-continuous b/Jenkinsfile-continuous index 2bbc34b..acfa3fe 100644 --- a/Jenkinsfile-continuous +++ b/Jenkinsfile-continuous @@ -19,7 +19,7 @@ pipeline { stage('Build') { steps { - sh 'mvn clean verify -P code-coverage' + sh 'mvn clean verify' } } } diff --git a/rdbi-parent/pom.xml b/rdbi-parent/pom.xml index 60e7b5d..98e897e 100644 --- a/rdbi-parent/pom.xml +++ b/rdbi-parent/pom.xml @@ -116,7 +116,7 @@ org.testng testng - 7.7.0 + 7.5 test