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; } }