From 94ff54dce96913705d461833827563f2bd667cda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zsolt=20M=C3=A9sz=C3=A1rovics?= Date: Wed, 10 Aug 2016 18:08:04 +0200 Subject: [PATCH 1/4] implemented basic Javanica collapsible Observer support --- .../aop/aspectj/HystrixCommandAspect.java | 63 ++++++++++++------- .../command/AbstractHystrixCommand.java | 9 ++- .../command/HystrixCommandBuilder.java | 10 +-- .../command/HystrixCommandBuilderFactory.java | 2 +- .../command/HystrixCommandFactory.java | 1 - .../common/collapser/BasicCollapserTest.java | 40 +++++++++++- .../src/test/resources/log4j.properties | 5 +- 7 files changed, 92 insertions(+), 38 deletions(-) diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java index f6794723a..f552b5d3d 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java @@ -16,7 +16,6 @@ package com.netflix.hystrix.contrib.javanica.aop.aspectj; import com.google.common.base.Optional; -import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import com.netflix.hystrix.HystrixInvokable; import com.netflix.hystrix.contrib.javanica.annotation.DefaultProperties; @@ -168,33 +167,38 @@ public MetaHolder create(Object proxy, Method collapserMethod, Object obj, Objec } Method batchCommandMethod = getDeclaredMethod(obj.getClass(), hystrixCollapser.batchMethod(), List.class); - if (batchCommandMethod == null || !batchCommandMethod.getReturnType().equals(List.class)) { + + if (batchCommandMethod == null) + throw new IllegalStateException("batch method is absent: " + hystrixCollapser.batchMethod()); + + Class batchReturnType = batchCommandMethod.getReturnType(); + Class collapserReturnType = collapserMethod.getReturnType(); + boolean observable = collapserReturnType.equals(Observable.class); + + if (!batchReturnType.equals(List.class)) throw new IllegalStateException("required batch method for collapser is absent: " + "(java.util.List) " + obj.getClass().getCanonicalName() + "." + hystrixCollapser.batchMethod() + "(java.util.List)"); - } if (!collapserMethod.getParameterTypes()[0] - .equals(getGenericParameter(batchCommandMethod.getGenericParameterTypes()[0]))) { - throw new IllegalStateException("required batch method for collapser is absent, wrong generic type: expected" + .equals(getFirstGenericParameter(batchCommandMethod.getGenericParameterTypes()[0]))) { + throw new IllegalStateException("required batch method for collapser is absent, wrong generic type: expected " + obj.getClass().getCanonicalName() + "." + hystrixCollapser.batchMethod() + "(java.util.List<" + collapserMethod.getParameterTypes()[0] + ">), but it's " + - getGenericParameter(batchCommandMethod.getGenericParameterTypes()[0])); + getFirstGenericParameter(batchCommandMethod.getGenericParameterTypes()[0])); } - Class collapserMethodReturnType; - if (Future.class.isAssignableFrom(collapserMethod.getReturnType())) { - collapserMethodReturnType = getGenericParameter(collapserMethod.getGenericReturnType()); - } else { - collapserMethodReturnType = collapserMethod.getReturnType(); - } + final Class collapserMethodReturnType = getFirstGenericParameter( + collapserMethod.getGenericReturnType(), + Future.class.isAssignableFrom(collapserReturnType) || Observable.class.isAssignableFrom(collapserReturnType) ? 1 : 0); + Class batchCommandActualReturnType = getFirstGenericParameter(batchCommandMethod.getGenericReturnType()); if (!collapserMethodReturnType - .equals(getGenericParameter(batchCommandMethod.getGenericReturnType()))) { + .equals(batchCommandActualReturnType)) { throw new IllegalStateException("Return type of batch method must be java.util.List parametrized with corresponding type: expected " + "(java.util.List<" + collapserMethodReturnType + ">)" + obj.getClass().getCanonicalName() + "." + hystrixCollapser.batchMethod() + "(java.util.List<" + collapserMethod.getParameterTypes()[0] + ">), but it's " + - getGenericParameter(batchCommandMethod.getGenericReturnType())); + batchCommandActualReturnType); } HystrixCommand hystrixCommand = batchCommandMethod.getAnnotation(HystrixCommand.class); @@ -212,11 +216,12 @@ public MetaHolder create(Object proxy, Method collapserMethod, Object obj, Objec builder.hystrixCollapser(hystrixCollapser); builder.defaultCollapserKey(collapserMethod.getName()); - builder.collapserExecutionType(ExecutionType.getExecutionType(collapserMethod.getReturnType())); + builder.collapserExecutionType(ExecutionType.getExecutionType(collapserReturnType)); builder.defaultCommandKey(batchCommandMethod.getName()); builder.hystrixCommand(hystrixCommand); - builder.executionType(ExecutionType.getExecutionType(batchCommandMethod.getReturnType())); + builder.executionType(ExecutionType.getExecutionType(batchReturnType)); + builder.observable(observable); FallbackMethod fallbackMethod = MethodProvider.getInstance().getFallbackMethod(obj.getClass(), batchCommandMethod); if (fallbackMethod.isPresent()) { fallbackMethod.validateReturnType(batchCommandMethod); @@ -260,14 +265,26 @@ private static Method getAjcMethodFromTarget(JoinPoint joinPoint) { } - private static Class getGenericParameter(Type type) { - Type tType = ((ParameterizedType) type).getActualTypeArguments()[0]; - String className = tType.toString().split(" ")[1]; - try { - return Class.forName(className); - } catch (ClassNotFoundException e) { - throw Throwables.propagate(e); + private static Class getFirstGenericParameter(Type type) { + return getFirstGenericParameter(type, 1); + } + + private static Class getFirstGenericParameter(final Type type, final int nestedDepth) { + int cDepth = 0; + Type tType = type; + + for (int cDept = 0; cDept < nestedDepth; cDept++) { + if (!(tType instanceof ParameterizedType)) + throw new IllegalStateException(String.format("Sub type at nesting level %d of %s is expected to be generic", cDepth, type)); + tType = ((ParameterizedType) tType).getActualTypeArguments()[0]; } + + if (tType instanceof ParameterizedType) + return (Class) ((ParameterizedType) tType).getRawType(); + else if (tType instanceof Class) + return (Class) tType; + + throw new UnsupportedOperationException("Unsupported type " + tType); } private static MetaHolder.Builder setDefaultProperties(MetaHolder.Builder builder, Class declaringClass, final ProceedingJoinPoint joinPoint) { diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommand.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommand.java index 6568cf778..9c1e7f435 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommand.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommand.java @@ -16,7 +16,6 @@ package com.netflix.hystrix.contrib.javanica.command; -import com.google.common.base.Throwables; import com.netflix.hystrix.HystrixCollapser; import com.netflix.hystrix.contrib.javanica.cache.CacheInvocationContext; import com.netflix.hystrix.contrib.javanica.cache.HystrixCacheKeyGenerator; @@ -140,8 +139,8 @@ boolean isIgnorable(Throwable throwable) { * @param action the action * @return result of command action execution */ - Object process(Action action) throws Exception { - Object result; + ReturnType process(Action action) throws Exception { + ReturnType result; try { result = action.execute(); flushCache(); @@ -188,14 +187,14 @@ protected void flushCache() { /** * Common action. */ - abstract class Action { + abstract class Action { /** * Each implementation of this method should wrap any exceptions in CommandActionExecutionException. * * @return execution result * @throws CommandActionExecutionException */ - abstract Object execute() throws CommandActionExecutionException; + abstract ReturnType execute() throws CommandActionExecutionException; } diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandBuilder.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandBuilder.java index c862bed5d..2b91c69b4 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandBuilder.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandBuilder.java @@ -52,8 +52,8 @@ public HystrixCommandBuilder(Builder builder) { this.executionType = builder.executionType; } - public static Builder builder() { - return new Builder(); + public static Builder builder() { + return new Builder(); } public GenericSetterBuilder getSetterBuilder() { @@ -85,12 +85,12 @@ public ExecutionType getExecutionType() { } - public static class Builder { + public static class Builder { private GenericSetterBuilder setterBuilder; private CommandActions commandActions; private CacheInvocationContext cacheResultInvocationContext; private CacheInvocationContext cacheRemoveInvocationContext; - private Collection> collapsedRequests = Collections.emptyList(); + private Collection> collapsedRequests = Collections.emptyList(); private List> ignoreExceptions = Collections.emptyList(); private ExecutionType executionType = ExecutionType.SYNCHRONOUS; @@ -144,7 +144,7 @@ public Builder cacheRemoveInvocationContext(CacheInvocationContext * @param pCollapsedRequests the collapsed requests * @return this {@link HystrixCommandBuilder.Builder} */ - public Builder collapsedRequests(Collection> pCollapsedRequests) { + public Builder collapsedRequests(Collection> pCollapsedRequests) { this.collapsedRequests = pCollapsedRequests; return this; } diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandBuilderFactory.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandBuilderFactory.java index c334fe73b..c49c0f4da 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandBuilderFactory.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandBuilderFactory.java @@ -51,7 +51,7 @@ public HystrixCommandBuilder create(MetaHolder metaHolder) { return create(metaHolder, Collections.>emptyList()); } - public HystrixCommandBuilder create(MetaHolder metaHolder, Collection> collapsedRequests) { + public HystrixCommandBuilder create(MetaHolder metaHolder, Collection> collapsedRequests) { validateMetaHolder(metaHolder); return HystrixCommandBuilder.builder() diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandFactory.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandFactory.java index 216642403..1dd12f3c0 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandFactory.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/HystrixCommandFactory.java @@ -15,7 +15,6 @@ */ package com.netflix.hystrix.contrib.javanica.command; -import com.netflix.hystrix.HystrixExecutable; import com.netflix.hystrix.HystrixInvokable; import com.netflix.hystrix.contrib.javanica.collapser.CommandCollapser; diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/collapser/BasicCollapserTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/collapser/BasicCollapserTest.java index 7a15839a7..9e70e1d9b 100644 --- a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/collapser/BasicCollapserTest.java +++ b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/collapser/BasicCollapserTest.java @@ -15,6 +15,7 @@ */ package com.netflix.hystrix.contrib.javanica.test.common.collapser; +import com.google.common.collect.Sets; import com.netflix.hystrix.HystrixEventType; import com.netflix.hystrix.HystrixInvokableInfo; import com.netflix.hystrix.HystrixRequestLog; @@ -25,9 +26,13 @@ import com.netflix.hystrix.contrib.javanica.test.common.domain.User; import org.junit.Before; import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import rx.Observable; import java.util.ArrayList; import java.util.List; +import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -77,6 +82,33 @@ public void testGetUserById() throws ExecutionException, InterruptedException { assertTrue(command.getExecutionEvents().contains(HystrixEventType.SUCCESS)); } + @Test + public void testReactive() throws Exception { + + final Observable u1 = userService.getUserByIdReactive("1"); + final Observable u2 = userService.getUserByIdReactive("2"); + final Observable u3 = userService.getUserByIdReactive("3"); + final Observable u4 = userService.getUserByIdReactive("4"); + final Observable u5 = userService.getUserByIdReactive("5"); + + final Iterable users = Observable.merge(u1, u2, u3, u4, u5).toBlocking().toIterable(); + + Set expectedIds = Sets.newHashSet("1", "2", "3", "4", "5"); + for (User cUser : users) { + assertEquals(expectedIds.remove(cUser.getId()), true); + } + assertEquals(expectedIds.isEmpty(), true); + assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size()); + HystrixInvokableInfo command = HystrixRequestLog.getCurrentRequest() + .getAllExecutedCommands().iterator().next(); + // assert the command is the one we're expecting + assertEquals("getUserByIds", command.getCommandKey().name()); + // confirm that it was a COLLAPSED command execution + assertTrue(command.getExecutionEvents().contains(HystrixEventType.COLLAPSED)); + // and that it was successful + assertTrue(command.getExecutionEvents().contains(HystrixEventType.SUCCESS)); + } + @Test public void testGetUserByIdWithFallback() throws ExecutionException, InterruptedException { Future f1 = userService.getUserByIdWithFallback("1"); @@ -158,6 +190,7 @@ public void testGetUserByIdWrongCollapserNoArgs() { public static class UserService { + public static final Logger log = LoggerFactory.getLogger(UserService.class); public static final User DEFAULT_USER = new User("def", "def"); @@ -173,6 +206,11 @@ public Future getUserByIdWithFallback(String id) { return null; } + @HystrixCollapser(batchMethod = "getUserByIds", + collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) + public Observable getUserByIdReactive(String id) { + return null; + } @HystrixCollapser(batchMethod = "getUserByIdsThrowsException", collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) @@ -226,6 +264,7 @@ public List getUserByIds(List ids) { for (String id : ids) { users.add(new User(id, "name: " + id)); } + log.debug("executing on thread id: {}", Thread.currentThread().getId()); return users; } @@ -233,7 +272,6 @@ public List getUserByIds(List ids) { commandProperties = { @HystrixProperty(name = "execution.isolation.thread.timeoutInMilliseconds", value = "10000")// for debug }) - public List getUserByIdsWithFallback(List ids) { throw new RuntimeException("not found"); } diff --git a/hystrix-contrib/hystrix-javanica/src/test/resources/log4j.properties b/hystrix-contrib/hystrix-javanica/src/test/resources/log4j.properties index bc1caf1b1..a1465fae2 100644 --- a/hystrix-contrib/hystrix-javanica/src/test/resources/log4j.properties +++ b/hystrix-contrib/hystrix-javanica/src/test/resources/log4j.properties @@ -19,8 +19,9 @@ log4j.rootLogger = ERROR, CONSOLE # Define the console appender log4j.appender.CONSOLE=org.apache.log4j.ConsoleAppender -log4j.appender.CONSOLE.File=${log}/log.out # Define the layout for console appender log4j.appender.CONSOLE.layout=org.apache.log4j.PatternLayout -log4j.appender.CONSOLE.layout.conversionPattern=%m%n \ No newline at end of file +log4j.appender.CONSOLE.layout.conversionPattern=%m%n + +log4j.logger.com.netflix.hystrix.contrib.javanica=DEBUG From cbecc005a244a42f363e710d396c6c094ffcc206 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zsolt=20M=C3=A9sz=C3=A1rovics?= Date: Fri, 2 Sep 2016 13:47:29 +0200 Subject: [PATCH 2/4] review items: unnecessary checks; numeric constant instead of loop var --- .../contrib/javanica/aop/aspectj/HystrixCommandAspect.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java index f552b5d3d..577c3ef54 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java @@ -175,11 +175,6 @@ public MetaHolder create(Object proxy, Method collapserMethod, Object obj, Objec Class collapserReturnType = collapserMethod.getReturnType(); boolean observable = collapserReturnType.equals(Observable.class); - if (!batchReturnType.equals(List.class)) - throw new IllegalStateException("required batch method for collapser is absent: " - + "(java.util.List) " + obj.getClass().getCanonicalName() + "." + - hystrixCollapser.batchMethod() + "(java.util.List)"); - if (!collapserMethod.getParameterTypes()[0] .equals(getFirstGenericParameter(batchCommandMethod.getGenericParameterTypes()[0]))) { throw new IllegalStateException("required batch method for collapser is absent, wrong generic type: expected " @@ -276,7 +271,7 @@ private static Class getFirstGenericParameter(final Type type, final int nest for (int cDept = 0; cDept < nestedDepth; cDept++) { if (!(tType instanceof ParameterizedType)) throw new IllegalStateException(String.format("Sub type at nesting level %d of %s is expected to be generic", cDepth, type)); - tType = ((ParameterizedType) tType).getActualTypeArguments()[0]; + tType = ((ParameterizedType) tType).getActualTypeArguments()[cDept]; } if (tType instanceof ParameterizedType) From 695ee06542c6dedbd1c700b497f3b1b5f50fdefe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zsolt=20M=C3=A9sz=C3=A1rovics?= Date: Fri, 2 Sep 2016 14:07:02 +0200 Subject: [PATCH 3/4] review: removed unnecessary generics --- .../contrib/javanica/command/AbstractHystrixCommand.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommand.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommand.java index 9c1e7f435..63ea6d322 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommand.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommand.java @@ -139,8 +139,8 @@ boolean isIgnorable(Throwable throwable) { * @param action the action * @return result of command action execution */ - ReturnType process(Action action) throws Exception { - ReturnType result; + Object process(Action action) throws Exception { + Object result; try { result = action.execute(); flushCache(); @@ -187,14 +187,14 @@ protected void flushCache() { /** * Common action. */ - abstract class Action { + abstract class Action { /** * Each implementation of this method should wrap any exceptions in CommandActionExecutionException. * * @return execution result * @throws CommandActionExecutionException */ - abstract ReturnType execute() throws CommandActionExecutionException; + abstract Object execute() throws CommandActionExecutionException; } From e9cf47bec1aba4c062085e93b547d721ebea9db4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zsolt=20M=C3=A9sz=C3=A1rovics?= Date: Fri, 2 Sep 2016 14:07:35 +0200 Subject: [PATCH 4/4] added reactive collapser example --- hystrix-contrib/hystrix-javanica/README.md | 32 +++++++++++++++++----- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/hystrix-contrib/hystrix-javanica/README.md b/hystrix-contrib/hystrix-javanica/README.md index 49a730dfd..a09147f3f 100644 --- a/hystrix-contrib/hystrix-javanica/README.md +++ b/hystrix-contrib/hystrix-javanica/README.md @@ -580,10 +580,18 @@ Suppose you have some command which calls should be collapsed in one backend cal Example: ```java + + /** Asynchronous Execution */ @HystrixCollapser(batchMethod = "getUserByIds") - public Future getUserById(String id) { + public Future getUserByIdAsync(String id) { return null; } + + /** Reactive Execution */ + @HystrixCollapser(batchMethod = "getUserByIds") + public Observable getUserByIdReact(String id) { + return null; + } @HystrixCommand public List getUserByIds(List ids) { @@ -593,13 +601,23 @@ Example: } return users; } - - Future f1 = userService.getUserById("1"); - Future f2 = userService.getUserById("2"); - Future f3 = userService.getUserById("3"); - Future f4 = userService.getUserById("4"); - Future f5 = userService.getUserById("5"); + // Async + Future f1 = userService.getUserByIdAsync("1"); + Future f2 = userService.getUserByIdAsync("2"); + Future f3 = userService.getUserByIdAsync("3"); + Future f4 = userService.getUserByIdAsync("4"); + Future f5 = userService.getUserByIdAsync("5"); + + // Reactive + Observable u1 = getUserByIdReact("1"); + Observable u2 = getUserByIdReact("2"); + Observable u3 = getUserByIdReact("3"); + Observable u4 = getUserByIdReact("4"); + Observable u5 = getUserByIdReact("5"); + + // Materialize reactive commands + Iterable users = Observables.merge(u1, u2, u3, u4, u5).toBlocking().toIterable(); ``` A method annotated with ```@HystrixCollapser``` annotation can return any value with compatible type, it does not affect the result of collapser execution, collapser method can even return ```null``` or another stub. There are several rules applied for methods signatures.