From b190d600b9e0ba06726ad6a2f0f76eed2ab8084b Mon Sep 17 00:00:00 2001 From: dmgcodevil Date: Fri, 27 Mar 2015 22:11:47 +0300 Subject: [PATCH 1/9] #689: BatchHystrixCommand has no sence and doesn't collapse requests --- .../javanica/annotation/HystrixCollapser.java | 4 + .../aop/aspectj/HystrixCommandAspect.java | 109 +++++++-- .../javanica/command/BatchHystrixCommand.java | 23 +- .../contrib/javanica/command/MetaHolder.java | 4 + .../test/spring/collapser/CollapserTest.java | 212 ++++-------------- .../collapser/CollapserPropertiesTest.java | 12 +- 6 files changed, 168 insertions(+), 196 deletions(-) diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java index aee478488..b58578ca4 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java @@ -41,6 +41,9 @@ */ String collapserKey() default ""; + // todo: add javadoc + String commandKey(); + /** * Defines what scope the collapsing should occur within. *

@@ -64,6 +67,7 @@ * * @return true if processing of a fallback is allowed */ + @Deprecated boolean fallbackEnabled() default false; } 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 dcf56784f..55d9952ef 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 @@ -31,7 +31,9 @@ import org.aspectj.lang.annotation.Pointcut; import java.lang.reflect.Method; +import java.util.List; +import static com.netflix.hystrix.contrib.javanica.utils.AopUtils.getDeclaredMethod; import static com.netflix.hystrix.contrib.javanica.utils.AopUtils.getMethodFromTarget; /** @@ -44,38 +46,109 @@ public class HystrixCommandAspect { public void hystrixCommandAnnotationPointcut() { } - @Around("hystrixCommandAnnotationPointcut()") + @Pointcut("@annotation(com.netflix.hystrix.contrib.javanica.annotation.HystrixCollapser)") + public void hystrixCollapserAnnotationPointcut() { + } + + @Around("hystrixCommandAnnotationPointcut() || hystrixCollapserAnnotationPointcut()") public Object methodsAnnotatedWithHystrixCommand(final ProceedingJoinPoint joinPoint) throws Throwable { - Method method = getMethodFromTarget(joinPoint); - Object obj = joinPoint.getTarget(); - Object[] args = joinPoint.getArgs(); - Validate.notNull(method, "failed to get method from joinPoint: %s", joinPoint); - HystrixCommand hystrixCommand = method.getAnnotation(HystrixCommand.class); - HystrixCollapser hystrixCollapser = method.getAnnotation(HystrixCollapser.class); - ExecutionType executionType = ExecutionType.getExecutionType(method.getReturnType()); - MetaHolder metaHolder = MetaHolder.builder() - .args(args).method(method).obj(obj).proxyObj(joinPoint.getThis()) - .executionType(executionType) - .hystrixCommand(hystrixCommand).hystrixCollapser(hystrixCollapser) - .defaultCommandKey(method.getName()) - .defaultCollapserKey(method.getName()) - .defaultGroupKey(obj.getClass().getSimpleName()).build(); + MetaHolderCreator metaHolderCreator = new MetaHolderCreator(joinPoint); + MetaHolder metaHolder = metaHolderCreator.create(); HystrixExecutable executable; - if (hystrixCollapser != null) { + ExecutionType executionType = metaHolderCreator.isCollapser() ? + metaHolderCreator.collapserExecutionType : metaHolderCreator.commandExecutionType; + if (metaHolderCreator.isCollapser()) { executable = new CommandCollapser(metaHolder); } else { executable = GenericHystrixCommandFactory.getInstance().create(metaHolder, null); } Object result; try { + result = CommandExecutor.execute(executable, executionType); } catch (HystrixBadRequestException e) { throw e.getCause(); - } catch (Throwable throwable){ - throw throwable; + } catch (Exception ex) { + throw ex; } return result; } + + private static abstract class MetaHolderFactory { + + } + + private static class MetaHolderCreator { + + private final Method method; + private final Object obj; + private final Object[] args; + private ExecutionType commandExecutionType; + private ExecutionType collapserExecutionType; + + private final Object proxy; + private MetaHolder.Builder builder; + private HystrixCollapser hystrixCollapser; + private HystrixCommand hystrixCommand; + + private MetaHolderCreator(final ProceedingJoinPoint joinPoint) { + this.method = getMethodFromTarget(joinPoint); + Validate.notNull(method, "failed to get method from joinPoint: %s", joinPoint); + this.obj = joinPoint.getTarget(); + this.args = joinPoint.getArgs(); + this.proxy = joinPoint.getThis(); + this.builder = metaHolderBuilder(); + + if (method.isAnnotationPresent(HystrixCommand.class) && method.isAnnotationPresent(HystrixCollapser.class)) { + throw new IllegalStateException("method cannot be annotated with HystrixCommand and HystrixCollapser annotations at the same time"); + } + if (method.isAnnotationPresent(HystrixCommand.class)) { + withCommand(method); + this.commandExecutionType = ExecutionType.getExecutionType(method.getReturnType()); + builder.executionType(commandExecutionType); + } else { + withCollapser(method); + collapserExecutionType = ExecutionType.getExecutionType(method.getReturnType()); + Method batchCommandMethod = getDeclaredMethod(obj.getClass(), hystrixCollapser.commandKey(), List.class); + if (batchCommandMethod == null) { + throw new IllegalStateException("no such method: " + hystrixCollapser.commandKey()); + } + withCommand(batchCommandMethod); + this.commandExecutionType = ExecutionType.getExecutionType(batchCommandMethod.getReturnType()); + builder.method(batchCommandMethod); + builder.executionType(commandExecutionType); + } + + } + + private MetaHolder.Builder metaHolderBuilder() { + return MetaHolder.builder() + .args(args).method(method).obj(obj).proxyObj(proxy) + .defaultGroupKey(obj.getClass().getSimpleName()); + } + + private void withCommand(Method commandMethod) { + hystrixCommand = commandMethod.getAnnotation(HystrixCommand.class); + builder.defaultCommandKey(commandMethod.getName()); + builder.hystrixCommand(hystrixCommand); + } + + + private void withCollapser(Method collapserMethod) { + hystrixCollapser = collapserMethod.getAnnotation(HystrixCollapser.class); + builder.defaultCollapserKey(collapserMethod.getName()); + builder.hystrixCollapser(hystrixCollapser); + } + + public boolean isCollapser() { + return hystrixCollapser != null; + } + + public MetaHolder create() { + return builder.build(); + } + } + } diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java index 56393d514..e5bba0dbd 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java @@ -25,6 +25,7 @@ import org.slf4j.LoggerFactory; import javax.annotation.concurrent.ThreadSafe; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; @@ -63,14 +64,11 @@ public void setFallbackEnabled(boolean fallbackEnabled) { @Override protected List> run() throws Exception { List> response = Lists.newArrayList(); - for (HystrixCollapser.CollapsedRequest request : getCollapsedRequests()) { - final Object[] args = (Object[]) request.getArgument(); - try { - response.add(Optional.of(fallbackEnabled ? processWithFallback(args) : process(args))); - } catch (Exception ex) { - request.setException(ex); - response.add(Optional.absent()); - } + List requests = collect(getCollapsedRequests()); + Object[] args = {requests}; + List result = (List) process(args); + for (Object res : result) { + response.add(Optional.of(res)); } return response; } @@ -134,4 +132,13 @@ protected List getFallback() { eliminate the break of all requests and the collapser as a whole. }*/ + List collect(Collection> requests) { + List commandArgs = new ArrayList(); + for (HystrixCollapser.CollapsedRequest request : requests) { + final Object[] args = (Object[]) request.getArgument(); + commandArgs.add(args[0]); + } + return commandArgs; + } + } diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MetaHolder.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MetaHolder.java index 5098f509c..7d0381ac8 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MetaHolder.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MetaHolder.java @@ -114,6 +114,10 @@ public Class[] getParameterTypes() { return method.getParameterTypes(); } + public boolean isCollapser(){ + return hystrixCollapser!=null; + } + public static final class Builder { private HystrixCollapser hystrixCollapser; diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java index 14212887e..e97789042 100644 --- a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java +++ b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java @@ -18,6 +18,8 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -41,23 +43,24 @@ public class CollapserTest { public void testCollapserAsync() throws ExecutionException, InterruptedException { HystrixRequestContext context = HystrixRequestContext.initializeContext(); try { - Future f1 = userService.getUserAsync("1", "name: "); - Future f2 = userService.getUserAsync("2", "name: "); - Future f3 = userService.getUserAsync("3", "name: "); - Future f4 = userService.getUserAsync("4", "name: "); + 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"); assertEquals("name: 1", f1.get().getName()); assertEquals("name: 2", f2.get().getName()); assertEquals("name: 3", f3.get().getName()); assertEquals("name: 4", f4.get().getName()); - + assertEquals("name: 5", f5.get().getName()); // assert that the batch command 'GetUserCommand' was in fact // executed and that it executed only once assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size()); HystrixInvokableInfo command = HystrixRequestLog.getCurrentRequest() .getAllExecutedCommands().iterator().next(); // assert the command is the one we're expecting - assertEquals("GetUserCommand", command.getCommandKey().name()); + assertEquals("getUserByIds", command.getCommandKey().name()); // confirm that it was a COLLAPSED command execution assertTrue(command.getExecutionEvents().contains(HystrixEventType.COLLAPSED)); // and that it was successful @@ -67,173 +70,54 @@ public void testCollapserAsync() throws ExecutionException, InterruptedException } } - /** - * This test covers situation when fallback isn't enabled in collapser. - */ - @Test(expected = ExecutionException.class) - public void testCollapserAsyncNotFound() throws ExecutionException, InterruptedException { - HystrixRequestContext context = HystrixRequestContext.initializeContext(); - try { - Future f1 = userService.getUserAsync("1", "name: "); - Future f2 = userService.getUserAsync("2", "name: "); - Future f3 = userService.getUserAsync("not found", "name"); // not found, exception here - Future f4 = userService.getUserAsync("4", "name: "); // will not be processed - Future f5 = userService.getUserAsync("5", "name: "); // will not be processed - System.out.println(f1.get().getName()); // this line will be executed - System.out.println(f2.get().getName()); // this line will be executed - System.out.println(f3.get().getName()); // this line will not be executed - System.out.println(f4.get().getName()); // this line will not be executed - System.out.println(f5.get().getName()); // this line will not be executed - - } finally { - context.shutdown(); - } - } - - /** - * This test covers situation when fallback is enabled in collapser. - */ - @Test - public void testCollapserAsyncNotFoundWithFallbackEnabled() throws ExecutionException, InterruptedException { - HystrixRequestContext context = HystrixRequestContext.initializeContext(); - try { - Future f1 = userService.getUserAsyncWithFallback("1", "name: "); - Future f2 = userService.getUserAsyncWithFallback("2", "name: "); - Future f3 = userService.getUserAsyncWithFallback("not found", "name"); // not found, exception here - Future f4 = userService.getUserAsyncWithFallback("4", "name: "); - Future f5 = userService.getUserAsyncWithFallback("5", "name: "); - - - assertEquals("name: 1", f1.get().getName()); - assertEquals("name: 2", f2.get().getName()); - assertEquals(UserService.DEFAULT_USER, f3.get()); // default value from fallback - assertEquals("name: 4", f4.get().getName()); - assertEquals("name: 5", f5.get().getName()); - - assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size()); - com.netflix.hystrix.HystrixInvokableInfo getUserCommand = getHystrixCommandByKey("getUserAsyncWithFallback"); - - // confirm that it was a COLLAPSED command execution - assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.COLLAPSED)); - assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.SUCCESS)); - - } finally { - context.shutdown(); - } - } - - /** - * This test covers situation when fallback is enabled in collapser. - */ - @Test - public void testCollapserAsyncNotFoundWithFallbackCommandEnabled() throws ExecutionException, InterruptedException { - HystrixRequestContext context = HystrixRequestContext.initializeContext(); - try { - Future f1 = userService.getUserAsyncWithFallbackCommand("1", "name: "); - Future f2 = userService.getUserAsyncWithFallbackCommand("2", "name: "); - Future f3 = userService.getUserAsyncWithFallbackCommand("not found", "name"); // not found, exception here - Future f4 = userService.getUserAsyncWithFallbackCommand("4", "name: "); - Future f5 = userService.getUserAsyncWithFallbackCommand("5", "name: "); - - - assertEquals("name: 1", f1.get().getName()); - assertEquals("name: 2", f2.get().getName()); - assertEquals(UserService.DEFAULT_USER, f3.get()); // default value from fallback - assertEquals("name: 4", f4.get().getName()); - assertEquals("name: 5", f5.get().getName()); - assertEquals(2, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size()); - com.netflix.hystrix.HystrixInvokableInfo getUserCommand = getHystrixCommandByKey("getUserAsyncWithFallbackCommand"); - - // confirm that it was a COLLAPSED command execution - assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.COLLAPSED)); - assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.SUCCESS)); - - } finally { - context.shutdown(); - } - } - - - @Test - public void testCollapserSync() throws ExecutionException, InterruptedException { - HystrixRequestContext context = HystrixRequestContext.initializeContext(); - try { - User u1 = userService.getUserSync("1", "name: "); - User u2 = userService.getUserSync("2", "name: "); - User u3 = userService.getUserSync("3", "name: "); - User u4 = userService.getUserSync("4", "name: "); - - assertEquals("name: 1", u1.getName()); - assertEquals("name: 2", u2.getName()); - assertEquals("name: 3", u3.getName()); - assertEquals("name: 4", u4.getName()); - - HystrixInvokableInfo command = HystrixRequestLog.getCurrentRequest() - .getAllExecutedCommands().iterator().next(); - assertEquals("GetUserCommand", 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)); - } finally { - context.shutdown(); - } - } - - @Test - public void testCollapserSyncWithFallbackCommand() throws ExecutionException, InterruptedException { - HystrixRequestContext context = HystrixRequestContext.initializeContext(); - try { - User u1 = userService.getUserSync("5", "name: "); - assertEquals("name: 5", u1.getName()); - HystrixInvokableInfo command = HystrixRequestLog.getCurrentRequest() - .getAllExecutedCommands().iterator().next(); - assertEquals("GetUserCommand", command.getCommandKey().name()); - // confirm that it was a COLLAPSED command execution - assertTrue(command.getExecutionEvents().contains(HystrixEventType.COLLAPSED)); - } finally { - context.shutdown(); - } - } public static class UserService { public static final User DEFAULT_USER = new User("def", "def"); - @HystrixCommand(commandKey = "GetUserCommand", fallbackMethod = "fallback") - @HystrixCollapser(collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) - public Future getUserAsync(final String id, final String name) { - emulateNotFound(id); - return new AsyncResult() { - @Override - public User invoke() { - return new User(id, name + id); // it should be network call - } - }; - } - - @HystrixCommand(fallbackMethod = "fallback") - @HystrixCollapser(fallbackEnabled = true, collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) - public Future getUserAsyncWithFallback(final String id, final String name) { - return getUserAsync(id, name); +// @HystrixCommand(commandKey = "GetUserCommand", fallbackMethod = "fallback") +// @HystrixCollapser(collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) +// public Future getUserAsync(final String id, final String name) { +// emulateNotFound(id); +// return new AsyncResult() { +// @Override +// public User invoke() { +// return new User(id, name + id); // it should be network call +// } +// }; +// } +// +// @HystrixCommand(fallbackMethod = "fallback") +// @HystrixCollapser(fallbackEnabled = true, collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) +// public Future getUserAsyncWithFallback(final String id, final String name) { +// return getUserAsync(id, name); +// } +// +// /** +// * Fallback for this command is also Hystrix command {@link #fallbackCommand(String, String)}. +// */ +// @HystrixCommand(fallbackMethod = "fallbackCommand") +// @HystrixCollapser(fallbackEnabled = true, collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) +// public Future getUserAsyncWithFallbackCommand(final String id, final String name) { +// return getUserAsync(id, name); +// } + + + @HystrixCollapser(commandKey = "getUserByIds", collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) + public Future getUserById(String id) { + return null; } - /** - * Fallback for this command is also Hystrix command {@link #fallbackCommand(String, String)}. - */ - @HystrixCommand(fallbackMethod = "fallbackCommand") - @HystrixCollapser(fallbackEnabled = true, collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) - public Future getUserAsyncWithFallbackCommand(final String id, final String name) { - return getUserAsync(id, name); - } - - @HystrixCommand(commandKey = "GetUserCommand", fallbackMethod = "fallback") - @HystrixCollapser(fallbackEnabled = true, collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) - public User getUserSync(String id, String name) { - emulateNotFound(id); - return new User(id, name + id); // it should be network call + @HystrixCommand(commandProperties = {@HystrixProperty(name = "execution.isolation.thread.timeoutInMilliseconds", value = "100000")}) + // + public List getUserByIds(List ids) { + List users = new ArrayList(); + for (String id : ids) { + users.add(new User(id, "name: " + id)); + } + return users; } /** diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/configuration/collapser/CollapserPropertiesTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/configuration/collapser/CollapserPropertiesTest.java index 0d2f9613c..242ca11a3 100644 --- a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/configuration/collapser/CollapserPropertiesTest.java +++ b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/configuration/collapser/CollapserPropertiesTest.java @@ -23,7 +23,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -@RunWith(SpringJUnit4ClassRunner.class) +//@RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(classes = {AopCglibConfig.class, CollapserPropertiesTest.CollapserPropertiesTestConfig.class}) public class CollapserPropertiesTest { @@ -31,7 +31,7 @@ public class CollapserPropertiesTest { @Autowired private UserService userService; - @Test + // @Test public void testCollapser() throws ExecutionException, InterruptedException { HystrixRequestContext context = HystrixRequestContext.initializeContext(); try { @@ -62,10 +62,10 @@ public void testCollapser() throws ExecutionException, InterruptedException { public static class UserService { @HystrixCommand - @HystrixCollapser(collapserKey = "GetUserCollapser", collapserProperties = { - @HystrixProperty(name = "maxRequestsInBatch", value = "1"), - @HystrixProperty(name = "timerDelayInMilliseconds", value = "200") - }) +// @HystrixCollapser(collapserKey = "GetUserCollapser", collapserProperties = { +// @HystrixProperty(name = "maxRequestsInBatch", value = "1"), +// @HystrixProperty(name = "timerDelayInMilliseconds", value = "200") +// }) public User getUser(String id, String name) { return new User(id, name + id); } From 45df1580291c16cdb97e87bcbb8fcf5157f55256 Mon Sep 17 00:00:00 2001 From: dmgcodevil Date: Fri, 27 Mar 2015 23:20:18 +0300 Subject: [PATCH 2/9] #689: removed unnecessary functionality --- .../aop/aspectj/HystrixCommandAspect.java | 138 +++++++++--------- .../javanica/collapser/CommandCollapser.java | 13 +- .../javanica/command/BatchHystrixCommand.java | 71 ++------- .../contrib/javanica/command/MetaHolder.java | 12 ++ .../test/spring/collapser/CollapserTest.java | 25 +++- 5 files changed, 118 insertions(+), 141 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 55d9952ef..70462cc85 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 @@ -15,6 +15,7 @@ */ package com.netflix.hystrix.contrib.javanica.aop.aspectj; +import com.google.common.collect.ImmutableMap; import com.netflix.hystrix.HystrixExecutable; import com.netflix.hystrix.contrib.javanica.annotation.HystrixCollapser; import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand; @@ -32,6 +33,7 @@ import java.lang.reflect.Method; import java.util.List; +import java.util.Map; import static com.netflix.hystrix.contrib.javanica.utils.AopUtils.getDeclaredMethod; import static com.netflix.hystrix.contrib.javanica.utils.AopUtils.getMethodFromTarget; @@ -42,7 +44,17 @@ @Aspect public class HystrixCommandAspect { + private static final Map META_HOLDER_FACTORY_MAP; + + static { + META_HOLDER_FACTORY_MAP = ImmutableMap.builder() + .put(HystrixPointcutType.COMMAND, new CommandMetaHolderFactory()) + .put(HystrixPointcutType.COLLAPSER, new CollapserMetaHolderFactory()) + .build(); + } + @Pointcut("@annotation(com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand)") + public void hystrixCommandAnnotationPointcut() { } @@ -52,103 +64,97 @@ public void hystrixCollapserAnnotationPointcut() { @Around("hystrixCommandAnnotationPointcut() || hystrixCollapserAnnotationPointcut()") public Object methodsAnnotatedWithHystrixCommand(final ProceedingJoinPoint joinPoint) throws Throwable { - - MetaHolderCreator metaHolderCreator = new MetaHolderCreator(joinPoint); - MetaHolder metaHolder = metaHolderCreator.create(); + Method method = getMethodFromTarget(joinPoint); + Validate.notNull(method, "failed to get method from joinPoint: %s", joinPoint); + if (method.isAnnotationPresent(HystrixCommand.class) && method.isAnnotationPresent(HystrixCollapser.class)) { + throw new IllegalStateException("method cannot be annotated with HystrixCommand and HystrixCollapser " + + "annotations at the same time"); + } + MetaHolderFactory metaHolderFactory = META_HOLDER_FACTORY_MAP.get(HystrixPointcutType.of(method)); + MetaHolder metaHolder = metaHolderFactory.create(joinPoint); HystrixExecutable executable; - ExecutionType executionType = metaHolderCreator.isCollapser() ? - metaHolderCreator.collapserExecutionType : metaHolderCreator.commandExecutionType; - if (metaHolderCreator.isCollapser()) { + ExecutionType executionType = metaHolder.isCollapser() ? + metaHolder.getCollapserExecutionType() : metaHolder.getExecutionType(); + if (metaHolder.isCollapser()) { executable = new CommandCollapser(metaHolder); } else { executable = GenericHystrixCommandFactory.getInstance().create(metaHolder, null); } Object result; try { - result = CommandExecutor.execute(executable, executionType); } catch (HystrixBadRequestException e) { throw e.getCause(); - } catch (Exception ex) { - throw ex; } return result; } + /** + * A factory to create MetaHolder depending on {@link HystrixPointcutType}. + */ private static abstract class MetaHolderFactory { - - } - - private static class MetaHolderCreator { - - private final Method method; - private final Object obj; - private final Object[] args; - private ExecutionType commandExecutionType; - private ExecutionType collapserExecutionType; - - private final Object proxy; - private MetaHolder.Builder builder; - private HystrixCollapser hystrixCollapser; - private HystrixCommand hystrixCommand; - - private MetaHolderCreator(final ProceedingJoinPoint joinPoint) { - this.method = getMethodFromTarget(joinPoint); - Validate.notNull(method, "failed to get method from joinPoint: %s", joinPoint); - this.obj = joinPoint.getTarget(); - this.args = joinPoint.getArgs(); - this.proxy = joinPoint.getThis(); - this.builder = metaHolderBuilder(); - - if (method.isAnnotationPresent(HystrixCommand.class) && method.isAnnotationPresent(HystrixCollapser.class)) { - throw new IllegalStateException("method cannot be annotated with HystrixCommand and HystrixCollapser annotations at the same time"); - } - if (method.isAnnotationPresent(HystrixCommand.class)) { - withCommand(method); - this.commandExecutionType = ExecutionType.getExecutionType(method.getReturnType()); - builder.executionType(commandExecutionType); - } else { - withCollapser(method); - collapserExecutionType = ExecutionType.getExecutionType(method.getReturnType()); - Method batchCommandMethod = getDeclaredMethod(obj.getClass(), hystrixCollapser.commandKey(), List.class); - if (batchCommandMethod == null) { - throw new IllegalStateException("no such method: " + hystrixCollapser.commandKey()); - } - withCommand(batchCommandMethod); - this.commandExecutionType = ExecutionType.getExecutionType(batchCommandMethod.getReturnType()); - builder.method(batchCommandMethod); - builder.executionType(commandExecutionType); - } - + public MetaHolder create(final ProceedingJoinPoint joinPoint) { + Method method = getMethodFromTarget(joinPoint); + Object obj = joinPoint.getTarget(); + Object[] args = joinPoint.getArgs(); + Object proxy = joinPoint.getThis(); + return create(proxy, method, obj, args); } - private MetaHolder.Builder metaHolderBuilder() { + public abstract MetaHolder create(Object proxy, Method method, Object obj, Object[] args); + + MetaHolder.Builder metaHolderBuilder(Object proxy, Method method, Object obj, Object[] args) { return MetaHolder.builder() .args(args).method(method).obj(obj).proxyObj(proxy) .defaultGroupKey(obj.getClass().getSimpleName()); } + } - private void withCommand(Method commandMethod) { - hystrixCommand = commandMethod.getAnnotation(HystrixCommand.class); - builder.defaultCommandKey(commandMethod.getName()); - builder.hystrixCommand(hystrixCommand); - } + private static class CollapserMetaHolderFactory extends MetaHolderFactory { + @Override + public MetaHolder create(Object proxy, Method collapserMethod, Object obj, Object[] args) { + HystrixCollapser hystrixCollapser = collapserMethod.getAnnotation(HystrixCollapser.class); + Method batchCommandMethod = getDeclaredMethod(obj.getClass(), hystrixCollapser.commandKey(), List.class); + if (batchCommandMethod == null) { + throw new IllegalStateException("required batch method for collapser is absent: " + + "(java.util.List) " + obj.getClass().getCanonicalName() + "." + + hystrixCollapser.commandKey() + "(java.util.List)"); + } + HystrixCommand hystrixCommand = batchCommandMethod.getAnnotation(HystrixCommand.class); - private void withCollapser(Method collapserMethod) { - hystrixCollapser = collapserMethod.getAnnotation(HystrixCollapser.class); - builder.defaultCollapserKey(collapserMethod.getName()); + MetaHolder.Builder builder = metaHolderBuilder(proxy, batchCommandMethod, obj, args); builder.hystrixCollapser(hystrixCollapser); - } + builder.defaultCollapserKey(collapserMethod.getName()); + builder.collapserExecutionType(ExecutionType.getExecutionType(collapserMethod.getReturnType())); - public boolean isCollapser() { - return hystrixCollapser != null; + builder.defaultCommandKey(batchCommandMethod.getName()); + builder.hystrixCommand(hystrixCommand); + builder.executionType(ExecutionType.getExecutionType(batchCommandMethod.getReturnType())); + return builder.build(); } + } - public MetaHolder create() { + private static class CommandMetaHolderFactory extends MetaHolderFactory { + @Override + public MetaHolder create(Object proxy, Method method, Object obj, Object[] args) { + HystrixCommand hystrixCommand = method.getAnnotation(HystrixCommand.class); + MetaHolder.Builder builder = metaHolderBuilder(proxy, method, obj, args); + builder.defaultCommandKey(method.getName()); + builder.hystrixCommand(hystrixCommand); + builder.executionType(ExecutionType.getExecutionType(method.getReturnType())); return builder.build(); } } + private static enum HystrixPointcutType { + COMMAND, + COLLAPSER; + + static HystrixPointcutType of(Method method) { + return method.isAnnotationPresent(HystrixCommand.class) ? COMMAND : COLLAPSER; + } + } + } diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java index c51b949a7..02fac554e 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java @@ -34,7 +34,7 @@ * Collapses multiple requests into a single {@link HystrixCommand} execution based * on a time window and optionally a max batch size. */ -public class CommandCollapser extends HystrixCollapser>, Object, Object> { +public class CommandCollapser extends HystrixCollapser, Object, Object> { private MetaHolder metaHolder; @@ -70,7 +70,7 @@ public Object getRequestArgument() { * Creates batch command. */ @Override - protected HystrixCommand>> createCommand( + protected HystrixCommand> createCommand( Collection> collapsedRequests) { BatchHystrixCommand command = BatchHystrixCommandFactory.getInstance().create(metaHolder, collapsedRequests); command.setFallbackEnabled(metaHolder.getHystrixCollapser().fallbackEnabled()); @@ -81,17 +81,14 @@ protected HystrixCommand>> createCommand( * {@inheritDoc} */ @Override - protected void mapResponseToRequests(List> batchResponse, + protected void mapResponseToRequests(List batchResponse, Collection> collapsedRequests) { if (batchResponse.size() < collapsedRequests.size()) { throw new RuntimeException(createMessage(collapsedRequests, batchResponse)); } int count = 0; for (CollapsedRequest request : collapsedRequests) { - Optional response = batchResponse.get(count++); - if (response.isPresent()) { // allows prevent IllegalStateException - request.setResponse(response.get()); - } + request.setResponse(batchResponse.get(count++)); } } @@ -120,7 +117,7 @@ public Setter build() { } private String createMessage(Collection> requests, - List> response) { + List response) { return ERROR_MSG + arrayFormat(ERROR_MSF_TEMPLATE, new Object[]{getCollapserKey().name(), requests.size(), response.size()}).getMessage(); } diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java index e5bba0dbd..c32350e20 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java @@ -16,11 +16,8 @@ package com.netflix.hystrix.contrib.javanica.command; -import com.google.common.base.Optional; -import com.google.common.collect.Lists; import com.netflix.hystrix.HystrixCollapser; import com.netflix.hystrix.contrib.javanica.exception.FallbackInvocationException; -import com.netflix.hystrix.exception.HystrixBadRequestException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -28,49 +25,28 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.Map; /** * This command is used in collapser. */ @ThreadSafe -public class BatchHystrixCommand extends AbstractHystrixCommand>> { +public class BatchHystrixCommand extends AbstractHystrixCommand> { private static final Logger LOGGER = LoggerFactory.getLogger(GenericCommand.class); - /** - * If some error occurs during the processing in run() then if {@link #fallbackEnabled} is true then the {@link #processWithFallback} - * will be invoked. If {@link #getFallbackAction()} doesn't process fallback logic as Hystrix command then - * command fallback will be processed in the single thread with BatchHystrixCommand, - * because the {@link #processWithFallback} is called from run(); - */ - private boolean fallbackEnabled; + protected BatchHystrixCommand(HystrixCommandBuilder builder) { super(builder); } - public boolean isFallbackEnabled() { - return fallbackEnabled; - } - - public void setFallbackEnabled(boolean fallbackEnabled) { - this.fallbackEnabled = fallbackEnabled; - } - /** * {@inheritDoc} */ @Override - protected List> run() throws Exception { - List> response = Lists.newArrayList(); - List requests = collect(getCollapsedRequests()); - Object[] args = {requests}; - List result = (List) process(args); - for (Object res : result) { - response.add(Optional.of(res)); - } - return response; + protected List run() throws Exception { + Object[] args = toArgs(getCollapsedRequests()); + return (List) process(args); } private Object process(final Object[] args) throws Exception { @@ -82,31 +58,14 @@ Object execute() { }); } - private Object processWithFallback(final Object[] args) throws Exception { - Object result; - try { - result = process(args); - } catch (Exception ex) { - if (ex instanceof HystrixBadRequestException) { - throw ex; - } else { - if (getFallbackAction() != null) { - result = processFallback(args); - } else { - // if command doesn't have fallback then - // call super.getFallback() that throws exception by default. - result = super.getFallback(); - } - } - } - return result; - } - private Object processFallback(final Object[] args) { + @Override + protected List getFallback() { if (getFallbackAction() != null) { final CommandAction commandAction = getFallbackAction(); + final Object[] args = toArgs(getCollapsedRequests()); try { - return process(new Action() { + return (List) process(new Action() { @Override Object execute() { return commandAction.executeWithArgs(ExecutionType.SYNCHRONOUS, args); @@ -122,15 +81,9 @@ Object execute() { } } - -/* @Override - protected List getFallback() { - Calling this method for Batch commands makes no sense in general because if the processing of a request fails - then other requests will not be processed within this collapser and setException method will be automatically - called on requests instances. Generally this is an all or nothing affair. For example, a timeout or rejection of - the HystrixCommand would cause failure on all of the batched calls. Thus existence of fallback method does not - eliminate the break of all requests and the collapser as a whole. - }*/ + Object[] toArgs(Collection> requests) { + return new Object[]{collect(getCollapsedRequests())}; + } List collect(Collection> requests) { List commandArgs = new ArrayList(); diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MetaHolder.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MetaHolder.java index 7d0381ac8..cd33d1718 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MetaHolder.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MetaHolder.java @@ -42,6 +42,7 @@ public class MetaHolder { private final String defaultCommandKey; private final String defaultCollapserKey; private final ExecutionType executionType; + private final ExecutionType collapserExecutionType; private MetaHolder(Builder builder) { this.hystrixCommand = builder.hystrixCommand; @@ -56,6 +57,7 @@ private MetaHolder(Builder builder) { this.defaultCollapserKey = builder.defaultCollapserKey; this.hystrixCollapser = builder.hystrixCollapser; this.executionType = builder.executionType; + this.collapserExecutionType = builder.collapserExecutionType; } public static Builder builder() { @@ -94,6 +96,10 @@ public ExecutionType getExecutionType() { return executionType; } + public ExecutionType getCollapserExecutionType() { + return collapserExecutionType; + } + public Object[] getArgs() { return args != null ? Arrays.copyOf(args, args.length) : new Object[]{}; } @@ -132,6 +138,7 @@ public static final class Builder { private String defaultCommandKey; private String defaultCollapserKey; private ExecutionType executionType; + private ExecutionType collapserExecutionType; public Builder hystrixCollapser(HystrixCollapser hystrixCollapser) { this.hystrixCollapser = hystrixCollapser; @@ -178,6 +185,11 @@ public Builder executionType(ExecutionType executionType) { return this; } + public Builder collapserExecutionType(ExecutionType collapserExecutionType) { + this.collapserExecutionType = collapserExecutionType; + return this; + } + public Builder defaultGroupKey(String defGroupKey) { this.defaultGroupKey = defGroupKey; return this; diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java index f61fba3f7..18b1136b8 100644 --- a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java +++ b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java @@ -125,14 +125,18 @@ public Future getUserById(String id) { return null; } - @HystrixCommand(commandProperties = {@HystrixProperty(name = "execution.isolation.thread.timeoutInMilliseconds", value = "100000")}) + @HystrixCommand( + fallbackMethod = "getUserByIdsFallback", + commandProperties = + {@HystrixProperty(name = "execution.isolation.thread.timeoutInMilliseconds", value = "100000")}) // public List getUserByIds(List ids) { - List users = new ArrayList(); - for (String id : ids) { - users.add(new User(id, "name: " + id)); - } - return users; + throw new RuntimeException(""); +// List users = new ArrayList(); +// for (String id : ids) { +// users.add(new User(id, "name: " + id)); +// } +// return users; } /** @@ -154,8 +158,13 @@ private User fallback(String id, String name) { } @HystrixCommand - private User fallbackCommand(String id, String name) { - return DEFAULT_USER; + private List getUserByIdsFallback(List ids) { + + List users = new ArrayList(); + for (String id : ids) { + users.add(new User(id, "name: " + id)); + } + return users; } } From 533661cad121cae845e7cc8d8fb1d3f4ce852722 Mon Sep 17 00:00:00 2001 From: dmgcodevil Date: Sat, 28 Mar 2015 00:24:10 +0300 Subject: [PATCH 3/9] #689: fixed tests --- .../javanica/annotation/HystrixCollapser.java | 52 ++++++-- .../aop/aspectj/HystrixCommandAspect.java | 13 +- .../javanica/collapser/CommandCollapser.java | 5 +- .../javanica/command/BatchHystrixCommand.java | 10 +- .../test/spring/collapser/CollapserTest.java | 117 +++++++++--------- .../collapser/CollapserPropertiesTest.java | 44 ++++--- 6 files changed, 135 insertions(+), 106 deletions(-) diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java index b58578ca4..3e8b952db 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java @@ -26,6 +26,34 @@ /** * This annotation is used to collapse some commands into a single backend dependency call. * This annotation should be used together with {@link HystrixCommand} annotation. + *

+ * Example: + * 

+ *     @HystrixCollapser(batchMethod = "getUserByIds"){
+ *          public Future getUserById(String id) {
+ *          return null;
+ * }
+ *  @HystrixCommand
+ *      public List getUserByIds(List ids) {
+ *          List users = new ArrayList();
+ *          for (String id : ids) {
+ *              users.add(new User(id, "name: " + id));
+ *          }
+ *      return users;
+ * }
+ *
+ * + * A method annotated with {@link HystrixCollapser} annotation can return any + * value with compatible type, it does not affect the result of collapser execution, + * collapser method can even return {@code null} or another stab. + * Pay attention that if a collapser method returns parametrized Future then generic type must be equal to generic type of List, + * for instance: + * 
+ *     Future - return type of collapser method
+ *     List - return type of batch command method
+ *
+ *

+ * Note: batch command method must be annotated with {@link HystrixCommand} annotation. */ @Target({ElementType.METHOD}) @Retention(RetentionPolicy.RUNTIME) @@ -41,8 +69,18 @@ */ String collapserKey() default ""; - // todo: add javadoc - String commandKey(); + /** + * Method name of batch command. + *

+ * Method must have the following signature: + * 

+     *     java.util.List method(java.util.List)
+     *
+ * NOTE: batch method can have only one argument. + * + * @return method name of batch command + */ + String batchMethod(); /** * Defines what scope the collapsing should occur within. @@ -60,14 +98,4 @@ */ HystrixProperty[] collapserProperties() default {}; - /** - * Used to enabled fallback logic for a batch hystrix command. - *

- * default => the false - * - * @return true if processing of a fallback is allowed - */ - @Deprecated - boolean fallbackEnabled() default false; - } 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 70462cc85..89d616384 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 @@ -89,7 +89,6 @@ public Object methodsAnnotatedWithHystrixCommand(final ProceedingJoinPoint joinP return result; } - /** * A factory to create MetaHolder depending on {@link HystrixPointcutType}. */ @@ -116,14 +115,18 @@ private static class CollapserMetaHolderFactory extends MetaHolderFactory { @Override public MetaHolder create(Object proxy, Method collapserMethod, Object obj, Object[] args) { HystrixCollapser hystrixCollapser = collapserMethod.getAnnotation(HystrixCollapser.class); - Method batchCommandMethod = getDeclaredMethod(obj.getClass(), hystrixCollapser.commandKey(), List.class); - if (batchCommandMethod == null) { + Method batchCommandMethod = getDeclaredMethod(obj.getClass(), hystrixCollapser.batchMethod(), List.class); + if (batchCommandMethod == null || !batchCommandMethod.getReturnType().equals(List.class)) { throw new IllegalStateException("required batch method for collapser is absent: " + "(java.util.List) " + obj.getClass().getCanonicalName() + "." + - hystrixCollapser.commandKey() + "(java.util.List)"); + hystrixCollapser.batchMethod() + "(java.util.List)"); } HystrixCommand hystrixCommand = batchCommandMethod.getAnnotation(HystrixCommand.class); - + if (hystrixCommand == null) { + throw new IllegalStateException("batch method must be annotated with HystrixCommand annotation"); + } + // method of batch hystrix command must be passed to metaholder because basically collapser doesn't have any actions + // that should be invoked upon intercepted method, its required only for underlying batch command MetaHolder.Builder builder = metaHolderBuilder(proxy, batchCommandMethod, obj, args); builder.hystrixCollapser(hystrixCollapser); builder.defaultCollapserKey(collapserMethod.getName()); diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java index 02fac554e..bd8039877 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java @@ -15,7 +15,6 @@ */ package com.netflix.hystrix.contrib.javanica.collapser; -import com.google.common.base.Optional; import com.netflix.hystrix.HystrixCollapser; import com.netflix.hystrix.HystrixCollapserKey; import com.netflix.hystrix.HystrixCommand; @@ -73,7 +72,6 @@ public Object getRequestArgument() { protected HystrixCommand> createCommand( Collection> collapsedRequests) { BatchHystrixCommand command = BatchHystrixCommandFactory.getInstance().create(metaHolder, collapsedRequests); - command.setFallbackEnabled(metaHolder.getHystrixCollapser().fallbackEnabled()); return command; } @@ -118,7 +116,8 @@ public Setter build() { private String createMessage(Collection> requests, List response) { - return ERROR_MSG + arrayFormat(ERROR_MSF_TEMPLATE, new Object[]{getCollapserKey().name(), requests.size(), response.size()}).getMessage(); + return ERROR_MSG + arrayFormat(ERROR_MSF_TEMPLATE, new Object[]{getCollapserKey().name(), + requests.size(), response.size()}).getMessage(); } } diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java index c32350e20..25e050cae 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java @@ -34,8 +34,6 @@ public class BatchHystrixCommand extends AbstractHystrixCommand> { private static final Logger LOGGER = LoggerFactory.getLogger(GenericCommand.class); - - protected BatchHystrixCommand(HystrixCommandBuilder builder) { super(builder); } @@ -44,6 +42,7 @@ protected BatchHystrixCommand(HystrixCommandBuilder builder) { * {@inheritDoc} */ @Override + @SuppressWarnings("unchecked") protected List run() throws Exception { Object[] args = toArgs(getCollapsedRequests()); return (List) process(args); @@ -60,6 +59,7 @@ Object execute() { @Override + @SuppressWarnings("unchecked") protected List getFallback() { if (getFallbackAction() != null) { final CommandAction commandAction = getFallbackAction(); @@ -81,11 +81,11 @@ Object execute() { } } - Object[] toArgs(Collection> requests) { - return new Object[]{collect(getCollapsedRequests())}; + private Object[] toArgs(Collection> requests) { + return new Object[]{collect(requests)}; } - List collect(Collection> requests) { + private List collect(Collection> requests) { List commandArgs = new ArrayList(); for (HystrixCollapser.CollapsedRequest request : requests) { final Object[] args = (Object[]) request.getArgument(); diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java index 18b1136b8..85aab3100 100644 --- a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java +++ b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/collapser/CollapserTest.java @@ -21,7 +21,6 @@ import com.netflix.hystrix.contrib.javanica.annotation.HystrixCollapser; import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand; import com.netflix.hystrix.contrib.javanica.annotation.HystrixProperty; -import com.netflix.hystrix.contrib.javanica.command.AsyncResult; import com.netflix.hystrix.contrib.javanica.test.spring.conf.AopCglibConfig; import com.netflix.hystrix.contrib.javanica.test.spring.domain.User; import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext; @@ -55,21 +54,21 @@ public class CollapserTest { private UserService userService; @Test - public void testCollapserAsync() throws ExecutionException, InterruptedException { + public void testGetUserById() throws ExecutionException, InterruptedException { HystrixRequestContext context = HystrixRequestContext.initializeContext(); try { - Future f1= userService.getUserById("1"); + 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"); + Future f4 = userService.getUserById("4"); + Future f5 = userService.getUserById("5"); assertEquals("name: 1", f1.get().getName()); assertEquals("name: 2", f2.get().getName()); assertEquals("name: 3", f3.get().getName()); assertEquals("name: 4", f4.get().getName()); assertEquals("name: 5", f5.get().getName()); - // assert that the batch command 'GetUserCommand' was in fact + // assert that the batch command 'getUserByIds' was in fact // executed and that it executed only once assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size()); HystrixInvokableInfo command = HystrixRequestLog.getCurrentRequest() @@ -85,81 +84,77 @@ public void testCollapserAsync() throws ExecutionException, InterruptedException } } + @Test + public void testGetUserByIdWithFallback() throws ExecutionException, InterruptedException { + HystrixRequestContext context = HystrixRequestContext.initializeContext(); + try { + Future f1 = userService.getUserByIdWithFallback("1"); + Future f2 = userService.getUserByIdWithFallback("2"); + Future f3 = userService.getUserByIdWithFallback("3"); + Future f4 = userService.getUserByIdWithFallback("4"); + Future f5 = userService.getUserByIdWithFallback("5"); + assertEquals("name: 1", f1.get().getName()); + assertEquals("name: 2", f2.get().getName()); + assertEquals("name: 3", f3.get().getName()); + assertEquals("name: 4", f4.get().getName()); + assertEquals("name: 5", f5.get().getName()); + // two command should be executed: "getUserByIdWithFallback" and "getUserByIdsWithFallback" + assertEquals(2, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size()); + HystrixInvokableInfo getUserByIdsWithFallback = getHystrixCommandByKey("getUserByIdsWithFallback"); + com.netflix.hystrix.HystrixInvokableInfo getUserByIdsFallback = getHystrixCommandByKey("getUserByIdsFallback"); + // confirm that command has failed + assertTrue(getUserByIdsWithFallback.getExecutionEvents().contains(HystrixEventType.FAILURE)); + assertTrue(getUserByIdsWithFallback.getExecutionEvents().contains(HystrixEventType.FALLBACK_SUCCESS)); + // and that fallback was successful + assertTrue(getUserByIdsFallback.getExecutionEvents().contains(HystrixEventType.SUCCESS)); + } finally { + context.shutdown(); + } + } public static class UserService { public static final User DEFAULT_USER = new User("def", "def"); -// @HystrixCommand(commandKey = "GetUserCommand", fallbackMethod = "fallback") -// @HystrixCollapser(collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) -// public Future getUserAsync(final String id, final String name) { -// emulateNotFound(id); -// return new AsyncResult() { -// @Override -// public User invoke() { -// return new User(id, name + id); // it should be network call -// } -// }; -// } -// -// @HystrixCommand(fallbackMethod = "fallback") -// @HystrixCollapser(fallbackEnabled = true, collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) -// public Future getUserAsyncWithFallback(final String id, final String name) { -// return getUserAsync(id, name); -// } -// -// /** -// * Fallback for this command is also Hystrix command {@link #fallbackCommand(String, String)}. -// */ -// @HystrixCommand(fallbackMethod = "fallbackCommand") -// @HystrixCollapser(fallbackEnabled = true, collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) -// public Future getUserAsyncWithFallbackCommand(final String id, final String name) { -// return getUserAsync(id, name); -// } - - - @HystrixCollapser(commandKey = "getUserByIds", collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) + + @HystrixCollapser(batchMethod = "getUserByIds", + collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) public Future getUserById(String id) { return null; } - @HystrixCommand( - fallbackMethod = "getUserByIdsFallback", - commandProperties = - {@HystrixProperty(name = "execution.isolation.thread.timeoutInMilliseconds", value = "100000")}) - // - public List getUserByIds(List ids) { - throw new RuntimeException(""); -// List users = new ArrayList(); -// for (String id : ids) { -// users.add(new User(id, "name: " + id)); -// } -// return users; + @HystrixCollapser(batchMethod = "getUserByIdsWithFallback", + collapserProperties = {@HystrixProperty(name = "timerDelayInMilliseconds", value = "200")}) + public Future getUserByIdWithFallback(String id) { + return null; } - /** - * Emulates situation when a server returns NOT_FOUND if user id = 5. - * - * @param id user id - */ - private void emulateNotFound(String id) throws RuntimeException { - if ("not found".equals(id)) { - throw new RuntimeException("not found"); + + @HystrixCommand(commandProperties = { + @HystrixProperty(name = "execution.isolation.thread.timeoutInMilliseconds", value = "10000")// for debug + }) + public List getUserByIds(List ids) { + List users = new ArrayList(); + for (String id : ids) { + users.add(new User(id, "name: " + id)); } + return users; } - /** - * Makes network call to a 'backup' server. - */ - private User fallback(String id, String name) { - return DEFAULT_USER; + @HystrixCommand(fallbackMethod = "getUserByIdsFallback", + commandProperties = { + @HystrixProperty(name = "execution.isolation.thread.timeoutInMilliseconds", value = "10000")// for debug + }) + + public List getUserByIdsWithFallback(List ids) { + throw new RuntimeException("not found"); } + @HystrixCommand private List getUserByIdsFallback(List ids) { - List users = new ArrayList(); for (String id : ids) { users.add(new User(id, "name: " + id)); diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/configuration/collapser/CollapserPropertiesTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/configuration/collapser/CollapserPropertiesTest.java index 8faee4384..de98069e4 100644 --- a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/configuration/collapser/CollapserPropertiesTest.java +++ b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/configuration/collapser/CollapserPropertiesTest.java @@ -24,7 +24,6 @@ import com.netflix.hystrix.contrib.javanica.test.spring.conf.AopCglibConfig; import com.netflix.hystrix.contrib.javanica.test.spring.domain.User; import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext; -import com.netflix.hystrix.util.HystrixRollingNumberEvent; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -33,12 +32,14 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.ExecutionException; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -//@RunWith(SpringJUnit4ClassRunner.class) +@RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(classes = {AopCglibConfig.class, CollapserPropertiesTest.CollapserPropertiesTestConfig.class}) public class CollapserPropertiesTest { @@ -46,25 +47,23 @@ public class CollapserPropertiesTest { @Autowired private UserService userService; - // @Test + @Test public void testCollapser() throws ExecutionException, InterruptedException { HystrixRequestContext context = HystrixRequestContext.initializeContext(); try { - User u1 = userService.getUser("1", "name: "); - User u2 = userService.getUser("2", "name: "); - User u3 = userService.getUser("3", "name: "); - User u4 = userService.getUser("4", "name: "); + User u1 = userService.getUser("1"); + User u2 = userService.getUser("2"); + User u3 = userService.getUser("3"); + User u4 = userService.getUser("4"); assertEquals("name: 1", u1.getName()); assertEquals("name: 2", u2.getName()); assertEquals("name: 3", u3.getName()); assertEquals("name: 4", u4.getName()); - + assertEquals(4, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size()); HystrixInvokableInfo command = HystrixRequestLog.getCurrentRequest() .getAllExecutedCommands().iterator().next(); - assertEquals("getUser", command.getCommandKey().name()); - //When a command is fronted by an HystrixCollapser then this marks how many requests are collapsed into the single command execution. - assertEquals(4, command.getMetrics().getCumulativeCount(HystrixRollingNumberEvent.COLLAPSED)); + assertEquals("getUsers", command.getCommandKey().name()); // confirm that it was a COLLAPSED command execution assertTrue(command.getExecutionEvents().contains(HystrixEventType.COLLAPSED)); // and that it was successful @@ -76,18 +75,23 @@ public void testCollapser() throws ExecutionException, InterruptedException { public static class UserService { - @HystrixCommand -// @HystrixCollapser(collapserKey = "GetUserCollapser", collapserProperties = { -// @HystrixProperty(name = "maxRequestsInBatch", value = "1"), -// @HystrixProperty(name = "timerDelayInMilliseconds", value = "200") -// }) - public User getUser(String id, String name) { - return new User(id, name + id); + @HystrixCollapser( + batchMethod = "getUsers", + collapserKey = "GetUserCollapser", collapserProperties = { + @HystrixProperty(name = "maxRequestsInBatch", value = "1"), + @HystrixProperty(name = "timerDelayInMilliseconds", value = "200") + }) + public User getUser(String id) { + return null; } @HystrixCommand - public User getUserDefProperties(String id, String name) { - return new User(id, name + id); + public List getUsers(List ids) { + List users = new ArrayList(); + for (String id : ids) { + users.add(new User(id, "name: " + id)); + } + return users; } } From bf9da0e1b954468e059ccca11971a52c0e7ce480 Mon Sep 17 00:00:00 2001 From: dmgcodevil Date: Sat, 28 Mar 2015 09:23:35 +0300 Subject: [PATCH 4/9] #733: added tests for async generic method --- .../javanica/annotation/HystrixCollapser.java | 2 +- .../test/spring/command/CommandTest.java | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java index 3e8b952db..21b4da2a7 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/annotation/HystrixCollapser.java @@ -45,7 +45,7 @@ * * A method annotated with {@link HystrixCollapser} annotation can return any * value with compatible type, it does not affect the result of collapser execution, - * collapser method can even return {@code null} or another stab. + * collapser method can even return {@code null} or another stub. * Pay attention that if a collapser method returns parametrized Future then generic type must be equal to generic type of List, * for instance: * 
diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/command/CommandTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/command/CommandTest.java
index 2497cb0a2..750e95040 100644
--- a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/command/CommandTest.java
+++ b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/command/CommandTest.java
@@ -102,6 +102,15 @@ public void should_work_with_parameterized_method() throws Exception {
         assertTrue(getCommand().getExecutionEvents().contains(HystrixEventType.SUCCESS));
     }
 
+    @Test
+    public void should_work_with_parameterized_asyncMethod() throws Exception {
+        assertEquals(Integer.valueOf(1), userService.echoAsync(1).get());
+
+        assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size());
+        assertTrue(getCommand().getExecutionEvents().contains(HystrixEventType.SUCCESS));
+    }
+
+
     private void assertGetUserSnycCommandExecuted(User u1) {
         assertEquals("name: 1", u1.getName());
         assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size());
@@ -138,6 +147,16 @@ public  T echo(T value) {
             return value;
         }
 
+        @HystrixCommand
+        public  Future echoAsync(final T value) {
+            return new AsyncResult() {
+                @Override
+                public T invoke() {
+                    return value;
+                }
+            };
+        }
+
     }
 
     public static class AdvancedUserService extends UserService {

From ba90b6c30a6970a589c2312b45f0f69939dc3287 Mon Sep 17 00:00:00 2001
From: dmgcodevil 
Date: Sat, 28 Mar 2015 14:50:48 +0300
Subject: [PATCH 5/9] #689: code style

---
 .../hystrix/contrib/javanica/collapser/CommandCollapser.java   | 3 +--
 .../hystrix/contrib/javanica/command/BatchHystrixCommand.java  | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java
index bd8039877..9bca04be3 100644
--- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java
+++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java
@@ -71,8 +71,7 @@ public Object getRequestArgument() {
     @Override
     protected HystrixCommand> createCommand(
             Collection> collapsedRequests) {
-        BatchHystrixCommand command = BatchHystrixCommandFactory.getInstance().create(metaHolder, collapsedRequests);
-        return command;
+        return BatchHystrixCommandFactory.getInstance().create(metaHolder, collapsedRequests);
     }
 
     /**
diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java
index 25e050cae..67884f089 100644
--- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java
+++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java
@@ -86,7 +86,7 @@ private Object[] toArgs(Collection collect(Collection> requests) {
-        List commandArgs = new ArrayList();
+        List commandArgs = new ArrayList<>();
         for (HystrixCollapser.CollapsedRequest request : requests) {
             final Object[] args = (Object[]) request.getArgument();
             commandArgs.add(args[0]);

From e44494f6b38845fca4833dcfc79a3d819bcc21a3 Mon Sep 17 00:00:00 2001
From: Roman Pleshkov 
Date: Sat, 28 Mar 2015 16:02:15 +0300
Subject: [PATCH 6/9] Update README.md

#689: updated wiki: collapser section
---
 hystrix-contrib/hystrix-javanica/README.md | 150 ++++++++++++++-------
 1 file changed, 101 insertions(+), 49 deletions(-)

diff --git a/hystrix-contrib/hystrix-javanica/README.md b/hystrix-contrib/hystrix-javanica/README.md
index d5bea981b..8cb6fe022 100644
--- a/hystrix-contrib/hystrix-javanica/README.md
+++ b/hystrix-contrib/hystrix-javanica/README.md
@@ -380,73 +380,125 @@ ThreadPoolProperties can be set using @HystrixCommand's 'threadPoolProperties' l
 
 Suppose you have some command which calls should be collapsed in one backend call. For this goal you can use ```@HystrixCollapser``` annotation.
 
-**Asynchronous call**
+Example:
 ```java
+        @HystrixCollapser(batchMethod = "getUserByIds")
+        public Future getUserById(String id) {
+            return null;
+        }
+        
         @HystrixCommand
-        @HystrixCollapser
-        public Future getUserAsync(final String id, final String name) {
-            return new AsyncResult() {
-                @Override
-                public User invoke() {
-                    return new User(id, name + id); // there should be a network call
-                }
-            };
+        public List getUserByIds(List ids) {
+            List users = new ArrayList();
+            for (String id : ids) {
+                users.add(new User(id, "name: " + id));
+            }
+            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");
+```
+A method annotated with {@link HystrixCollapser} annotation can return any value with compatible type, it does not affect the result of collapser execution, collapser method can even return {@code null} or another stub.
+There are several rules applied for methods signatures.
+1. Collapser method must have one argument of any type, desired a wrapper of a primitive type like Integer, Long, String and etc. 
+2. A batch method must have one argument with type java.util.List parameterized with corresponding type, that's if a type of collapser argument is Integer then type of batch method argument must be List.
+3. Return type of batch method must be java.util.List parameterized with corresponding type, that's if a return type of collapser method is ```User``` then a return type of batch command must be List
+
+**Convention for batch method behavior**
+1. The size of response collection must be equal to the size of request collection. 
+```java
+  @HystrixCommand
+  public List getUserByIds(List ids); // batch method
+  
+  List ids = List("1", "2", "3");
+  getUserByIds(ids).size() == ids.size();
+  
 ```
+2. Order of elements in response collection must be same to order of elements in request collection.
 
-**Synchronous call**
-_Note_ : Request collapsing on a single thread makes no sense, because if a single thread is invoking ```execute()``` or ```queue().get()``` synchronously it will block and wait for a response and thus not submit any further requests that will be collapsed inside the window. Doing requests on a separate threads (such as user requests with collapsing scope set to GLOBAL) would make sense:
+```
+ @HystrixCommand
+  public List getUserByIds(List ids); // batch method
+  
+  List ids = List("1", "2", "3");
+  List users = getUserByIds(ids);
+  System.out.println(users);
+  // output
+  User: id=1
+  User: id=2
+  User: id=3
+```
+
+Why order of elements of request and response collections is important. The reason of this is in reducing logic, basically request elements are mapped one-to-one to response elements. Thus if order of elements of request collection is different then the result of execution can be unpredictable.
+
+**Deduplication batch command request parameters**.
+In some cases your batch method can depend on behavior of third-party service or library that skips duplicates in a request. It can be a rest service that expects unique values. In this case the size of elements in request collection can be different from size of elements in response collection. It violates one of the behavior principle. To fix it you need to manually map request to response collection, for example:
 
 ```java
-        @HystrixCommand
-        @HystrixCollapser(scope = GLOBAL)
-        public User getUserSync(String id, String name) {
-            return new User(id, name + id); // there should be a network call
-        }
+// hava 8
+@HystrixCommand
+List batchMethod(List ids){
+// ids = [1, 2, 2, 3]
+List users = restClient.getUsersByIds(ids);
+// users = [User{id='1', name='user1'}, User{id='2', name='user2'}, User{id='3', name='user3'}]
+List response = ids.stream().map(it -> users.stream()
+                .filter(u -> u.getId().equals(it)).findFirst().get())
+                .collect(Collectors.toList());
+// response = [User{id='1', name='user1'}, User{id='2', name='user2'}, User{id='2', name='user2'}, User{id='3', name='user3'}]
+return response;
 ```
 
+Same case if you want to remove duplicate elements from request collection before a service call.
+Example:
+```java
+// hava 8
+@HystrixCommand
+List batchMethod(List ids){
+// ids = [1, 2, 2, 3]
+List uniqueIds = ids.stream().distinct().collect(Collectors.toList());
+// uniqueIds = [1, 2, 3]
+List users = restClient.getUsersByIds(uniqueIds);
+// users = [User{id='1', name='user1'}, User{id='2', name='user2'}, User{id='3', name='user3'}]
+List response = ids.stream().map(it -> users.stream()
+                .filter(u -> u.getId().equals(it)).findFirst().get())
+                .collect(Collectors.toList());
+// response = [User{id='1', name='user1'}, User{id='2', name='user2'}, User{id='2', name='user2'}, User{id='3', name='user3'}]
+return response;
+```
 To set collapser [properties](https://github.com/Netflix/Hystrix/wiki/Configuration#Collapser) use `@HystrixCollapser#collapserProperties`
 
 Read more about Hystrix request collapsing [here] (https://github.com/Netflix/Hystrix/wiki/How-it-Works#wiki-RequestCollapsing)
 
 **Collapser error processing**
+Bath command can have a fallback method.
+Example:
 
-All commands are collapsed are instances of ```BatchHystrixCommand```. In BatchHystrixCommand the ```getFallback()``` isn't implemented, even if a collapsed command has fallback ``` @HystrixCommand(fallbackMethod = "fallbackCommand")``` then it never be processed in ```getFallback()```, below this moment is consecrated in detail.
-
-If the processing of a request fails then other requests will not be processed within this collapser and ```setException``` method will be automatically called on requests instances. Generally this is an all or nothing affair. For example, a timeout or rejection of the HystrixCommand would cause failure on all of the batched calls. Thus existence of fallback method does not eliminate the break of all requests and the collapser as a whole. Thus there are two scenarios that you can change using ```@HystrixCollapser#fallbackEnabled``` annotation property.
-
-When one of the requests has failed:
-
-**Scenario #1** (```@HystrixCollapser(fallbackEnabled = false)```):
-
-All requests that were processed successfully will be returned to an user and will be available from Future, other requests including one that failed will not. The fallback of collapsed command will not be executed in a case of any errors.
-Short example:
 ```java
+        @HystrixCollapser(batchMethod = "getUserByIdsWithFallback")
+        public Future getUserByIdWithFallback(String id) {
+            return null;
+        }
+        
+        @HystrixCommand(fallbackMethod = "getUserByIdsFallback")
+        public List getUserByIdsWithFallback(List ids) {
+            throw new RuntimeException("not found");
+        }
 
- @HystrixCommand(fallbackMethod = "fallback")
-        @HystrixCollapser(fallbackEnabled = false)
-        public Future getUserAsync(final String id, final String name) {
-            // some logic here
-            };
-            
-        // .......
-
-
-            Future f1 = userService.getUserAsync("1", "name: ");
-            Future f2 = userService.getUserAsync("2", "name: ");
-            Future f3 = userService.getUserAsync("not found", "name"); // not found, exception here
-            Future f4 = userService.getUserAsync("4", "name: "); // will not be processed
-            Future f5 = userService.getUserAsync("5", "name: "); // will not be processed
-            System.out.println(f1.get().getName()); // this line will be executed
-            System.out.println(f2.get().getName()); // this line will be executed
-            System.out.println(f3.get().getName()); // this line will not be executed
-            System.out.println(f4.get().getName()); // this line will not be executed
-            System.out.println(f5.get().getName()); // this line will not be executed
-```
-
-**Scenario #2** (```@HystrixCollapser(fallbackEnabled = true)```):
 
-All requests that were processed successfully or failed will be returned to an user and will be available from a Future instance. For failed requests the collapsed command's fallback method will be executed, of course if the name of fallback method was defined in ```@HystrixCommand```. The fallback of collapsed command will be used to process any exceptions during batch processing. If the fallback method hasn't ```@HystrixCommand``` annotation then this method will be processed in the a single thread with the BatchHystrixCommand in method _run()_, otherwise fallback will be process as Hystrix command in separate thread if fallback method was annotated with ```@HystrixCommand```. This is important point that you need to pay attention if you want to use fallback for your collapsed command. Thus fallback method will be processed in ```BatchHystrixCommand#run()``` not in ```BatchHystrixCommand#getFallback()```, but fallback can be process as Hystrix command therefore set the ```fallbackEnabled``` as true can be useful in some situations to avoid crash of whole collapser and continue to process requests, but I recommend to use ```@HystrixCollapser(fallbackEnabled = false)``` and give preference to the first scenario as this is the correct behavior in most cases.
+        @HystrixCommand
+        private List getUserByIdsFallback(List ids) {
+            List users = new ArrayList();
+            for (String id : ids) {
+                users.add(new User(id, "name: " + id));
+            }
+            return users;
+        }
+```
 
 
 #Development Status and Future

From eecfdc059a0378b637fe3b6adae39ef86b121965 Mon Sep 17 00:00:00 2001
From: Roman Pleshkov 
Date: Sat, 28 Mar 2015 19:51:00 +0300
Subject: [PATCH 7/9] Update README.md

---
 hystrix-contrib/hystrix-javanica/README.md | 85 ++++++++++++----------
 1 file changed, 46 insertions(+), 39 deletions(-)

diff --git a/hystrix-contrib/hystrix-javanica/README.md b/hystrix-contrib/hystrix-javanica/README.md
index 8cb6fe022..222a23638 100644
--- a/hystrix-contrib/hystrix-javanica/README.md
+++ b/hystrix-contrib/hystrix-javanica/README.md
@@ -382,44 +382,46 @@ Suppose you have some command which calls should be collapsed in one backend cal
 
 Example:
 ```java
-        @HystrixCollapser(batchMethod = "getUserByIds")
-        public Future getUserById(String id) {
-            return null;
-        }
+    @HystrixCollapser(batchMethod = "getUserByIds")
+    public Future getUserById(String id) {
+        return null;
+    }
         
-        @HystrixCommand
-        public List getUserByIds(List ids) {
-            List users = new ArrayList();
-            for (String id : ids) {
-                users.add(new User(id, "name: " + id));
-            }
-            return users;
+    @HystrixCommand
+    public List getUserByIds(List ids) {
+        List users = new ArrayList();
+        for (String id : ids) {
+            users.add(new User(id, "name: " + id));
         }
+        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");
+    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");
 ```
-A method annotated with {@link HystrixCollapser} annotation can return any value with compatible type, it does not affect the result of collapser execution, collapser method can even return {@code null} or another stub.
+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.
+
 1. Collapser method must have one argument of any type, desired a wrapper of a primitive type like Integer, Long, String and etc. 
 2. A batch method must have one argument with type java.util.List parameterized with corresponding type, that's if a type of collapser argument is Integer then type of batch method argument must be List.
-3. Return type of batch method must be java.util.List parameterized with corresponding type, that's if a return type of collapser method is ```User``` then a return type of batch command must be List
+3. Return type of batch method must be java.util.List parameterized with corresponding type, that's if a return type of collapser method is ```User``` then a return type of batch command must be List.
 
 **Convention for batch method behavior**
-1. The size of response collection must be equal to the size of request collection. 
+
+The size of response collection must be equal to the size of request collection.
+
 ```java
   @HystrixCommand
   public List getUserByIds(List ids); // batch method
   
   List ids = List("1", "2", "3");
   getUserByIds(ids).size() == ids.size();
-  
 ```
-2. Order of elements in response collection must be same to order of elements in request collection.
+Order of elements in response collection must be same as in request collection.
 
 ```
  @HystrixCommand
@@ -434,10 +436,13 @@ There are several rules applied for methods signatures.
   User: id=3
 ```
 
-Why order of elements of request and response collections is important. The reason of this is in reducing logic, basically request elements are mapped one-to-one to response elements. Thus if order of elements of request collection is different then the result of execution can be unpredictable.
+**Why order of elements of request and response collections is important?**
+
+The reason of this is in reducing logic, basically request elements are mapped one-to-one to response elements. Thus if order of elements of request collection is different then the result of execution can be unpredictable.
 
 **Deduplication batch command request parameters**.
-In some cases your batch method can depend on behavior of third-party service or library that skips duplicates in a request. It can be a rest service that expects unique values. In this case the size of elements in request collection can be different from size of elements in response collection. It violates one of the behavior principle. To fix it you need to manually map request to response collection, for example:
+
+In some cases your batch method can depend on behavior of third-party service or library that skips duplicates in a request. It can be a rest service that expects unique values and ignores duplicates. In this case the size of elements in request collection can be different from size of elements in response collection. It violates one of the behavior principle. To fix it you need manually map request to response, for example:
 
 ```java
 // hava 8
@@ -479,29 +484,31 @@ Bath command can have a fallback method.
 Example:
 
 ```java
-        @HystrixCollapser(batchMethod = "getUserByIdsWithFallback")
-        public Future getUserByIdWithFallback(String id) {
-            return null;
-        }
+    @HystrixCollapser(batchMethod = "getUserByIdsWithFallback")
+    public Future getUserByIdWithFallback(String id) {
+        return null;
+    }
         
-        @HystrixCommand(fallbackMethod = "getUserByIdsFallback")
-        public List getUserByIdsWithFallback(List ids) {
-            throw new RuntimeException("not found");
-        }
+    @HystrixCommand(fallbackMethod = "getUserByIdsFallback")
+    public List getUserByIdsWithFallback(List ids) {
+        throw new RuntimeException("not found");
+    }
 
 
-        @HystrixCommand
-        private List getUserByIdsFallback(List ids) {
-            List users = new ArrayList();
-            for (String id : ids) {
-                users.add(new User(id, "name: " + id));
-            }
-            return users;
+    @HystrixCommand
+    private List getUserByIdsFallback(List ids) {
+        List users = new ArrayList();
+        for (String id : ids) {
+            users.add(new User(id, "name: " + id));
         }
+        return users;
+    }
 ```
 
 
 #Development Status and Future
 Please create an issue if you need a feature or you detected some bugs. Thanks
 
-**Note**: Javaniva 1.4.+ is more stable than 1.3.+ All fixes initially are added in 1.4.+ and after if it's not much efforts then merged to 1.3.  **It's recommended to use Javaniva 1.4.+** 
+**Note**: Javaniva 1.4.+ is updated more frequently than 1.3.+ hence 1.4+ is more stable. 
+
+**It's recommended to use Javaniva 1.4.+** 

From 3f1b9078e07dc06ddf4c3a961570b8f6b88bbd24 Mon Sep 17 00:00:00 2001
From: Roman Pleshkov 
Date: Sat, 28 Mar 2015 20:05:09 +0300
Subject: [PATCH 8/9] Update README.md

#689: fix wiki format
---
 hystrix-contrib/hystrix-javanica/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hystrix-contrib/hystrix-javanica/README.md b/hystrix-contrib/hystrix-javanica/README.md
index 222a23638..c6d5b10ae 100644
--- a/hystrix-contrib/hystrix-javanica/README.md
+++ b/hystrix-contrib/hystrix-javanica/README.md
@@ -407,8 +407,8 @@ A method annotated with ```@HystrixCollapser``` annotation can return any value
 There are several rules applied for methods signatures.
 
 1. Collapser method must have one argument of any type, desired a wrapper of a primitive type like Integer, Long, String and etc. 
-2. A batch method must have one argument with type java.util.List parameterized with corresponding type, that's if a type of collapser argument is Integer then type of batch method argument must be List.
-3. Return type of batch method must be java.util.List parameterized with corresponding type, that's if a return type of collapser method is ```User``` then a return type of batch command must be List.
+2. A batch method must have one argument with type java.util.List parameterized with corresponding type, that's if a type of collapser argument is ```Integer``` then type of batch method argument must be ```List```.
+3. Return type of batch method must be java.util.List parameterized with corresponding type, that's if a return type of collapser method is ```User``` then a return type of batch command must be ```List```.
 
 **Convention for batch method behavior**
 

From a614fdf9e2f3ad4e178db8253cd6a79be10bb417 Mon Sep 17 00:00:00 2001
From: dmgcodevil 
Date: Sun, 29 Mar 2015 17:35:32 +0300
Subject: [PATCH 9/9] #689: revert to java 6

---
 .../hystrix/contrib/javanica/command/BatchHystrixCommand.java   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java
index 67884f089..25e050cae 100644
--- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java
+++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java
@@ -86,7 +86,7 @@ private Object[] toArgs(Collection collect(Collection> requests) {
-        List commandArgs = new ArrayList<>();
+        List commandArgs = new ArrayList();
         for (HystrixCollapser.CollapsedRequest request : requests) {
             final Object[] args = (Object[]) request.getArgument();
             commandArgs.add(args[0]);