Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge pull request #244 from dmgcodevil/master

Fix for issue #241 (leaner error propagation in hystrix javanica)
  • Loading branch information...
commit eea4a052fe5970de362eeee754466a52c97f13d6 2 parents c8333a6 + 59bbe9f
@benjchristensen benjchristensen authored
View
1  hystrix-contrib/hystrix-javanica/build.gradle
@@ -19,6 +19,7 @@ dependencies {
testCompile 'org.springframework:spring-aop:4.0.0.RELEASE'
testCompile 'org.springframework:spring-test:4.0.0.RELEASE'
testCompile 'cglib:cglib:3.1'
+ testCompile 'org.mockito:mockito-all:1.9.5'
}
eclipse {
View
23 ...ystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java
@@ -15,10 +15,15 @@
*/
package com.netflix.hystrix.contrib.javanica.aop.aspectj;
+import com.netflix.hystrix.HystrixExecutable;
import com.netflix.hystrix.contrib.javanica.annotation.HystrixCollapser;
import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand;
import com.netflix.hystrix.contrib.javanica.collapser.CommandCollapser;
-import com.netflix.hystrix.contrib.javanica.command.*;
+import com.netflix.hystrix.contrib.javanica.command.CommandExecutor;
+import com.netflix.hystrix.contrib.javanica.command.ExecutionType;
+import com.netflix.hystrix.contrib.javanica.command.GenericHystrixCommandFactory;
+import com.netflix.hystrix.contrib.javanica.command.MetaHolder;
+import com.netflix.hystrix.exception.HystrixBadRequestException;
import org.apache.commons.lang3.Validate;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
@@ -57,13 +62,21 @@ public Object methodsAnnotatedWithHystrixCommand(final ProceedingJoinPoint joinP
.defaultCommandKey(method.getName())
.defaultCollapserKey(method.getName())
.defaultGroupKey(obj.getClass().getSimpleName()).build();
+ HystrixExecutable executable;
if (hystrixCollapser != null) {
- CommandCollapser commandCollapser = new CommandCollapser(metaHolder);
- return CommandExecutor.execute(commandCollapser, executionType);
+ executable = new CommandCollapser(metaHolder);
} else {
- GenericCommand genericCommand = GenericHystrixCommandFactory.getInstance().create(metaHolder, null);
- return CommandExecutor.execute(genericCommand, executionType);
+ 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;
+ }
+ return result;
}
}
View
83 ...-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/error/ErrorPropagationTest.java
@@ -6,11 +6,11 @@
import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand;
import com.netflix.hystrix.contrib.javanica.test.spring.conf.AopCglibConfig;
import com.netflix.hystrix.contrib.javanica.test.spring.domain.User;
-import com.netflix.hystrix.exception.HystrixBadRequestException;
import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext;
-import org.apache.commons.lang3.Validate;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.MockitoAnnotations;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Configurable;
import org.springframework.context.annotation.Bean;
@@ -19,7 +19,9 @@
import static com.netflix.hystrix.contrib.javanica.CommonUtils.getHystrixCommandByKey;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
/**
* Test covers "Error Propagation" functionality.
@@ -29,43 +31,78 @@
@ContextConfiguration(classes = {AopCglibConfig.class, ErrorPropagationTest.ErrorPropagationTestConfig.class})
public class ErrorPropagationTest {
+ private static final String COMMAND_KEY = "getUserById";
@Autowired
private UserService userService;
- @Test(expected = HystrixBadRequestException.class)
- public void testGetUser() {
+ @MockitoAnnotations.Mock
+ private FailoverService failoverService;
+
+ @Before
+ public void setUp() throws Exception {
+ MockitoAnnotations.initMocks(this);
+ userService.setFailoverService(failoverService);
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testGetUserByEmptyId() {
HystrixRequestContext context = HystrixRequestContext.initializeContext();
try {
- userService.getUser("", "");
- assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size());
- com.netflix.hystrix.HystrixCommand getUserCommand = getHystrixCommandByKey("getUser");
- assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.FAILURE));
+ userService.getUserById("");
} finally {
+ assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size());
+ com.netflix.hystrix.HystrixCommand getUserCommand = getHystrixCommandByKey(COMMAND_KEY);
+ // will not affect metrics
+ assertFalse(getUserCommand.getExecutionEvents().contains(HystrixEventType.FAILURE));
+ // and will not trigger fallback logic
+ verify(failoverService, never()).getDefUser();
context.shutdown();
}
}
+ @Test(expected = NullPointerException.class)
+ public void testGetUserByNullId() {
+ HystrixRequestContext context = HystrixRequestContext.initializeContext();
+ try {
+ userService.getUserById(null);
+ } finally {
+ assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size());
+ com.netflix.hystrix.HystrixCommand getUserCommand = getHystrixCommandByKey(COMMAND_KEY);
+ // will not affect metrics
+ assertFalse(getUserCommand.getExecutionEvents().contains(HystrixEventType.FAILURE));
+ // and will not trigger fallback logic
+ verify(failoverService, never()).getDefUser();
+ context.shutdown();
+ }
+ }
public static class UserService {
- @HystrixCommand(cacheKeyMethod = "getUserIdCacheKey",
- ignoreExceptions = {NullPointerException.class, IllegalArgumentException.class})
- public User getUser(String id, String name) {
- validate(id, name);
- return new User(id, name + id); // it should be network call
+ private FailoverService failoverService;
+
+ public void setFailoverService(FailoverService failoverService) {
+ this.failoverService = failoverService;
}
- @HystrixCommand
- private String getUserIdCacheKey(String id, String name) {
- return id + name;
+ @HystrixCommand(commandKey = COMMAND_KEY, ignoreExceptions = {NullPointerException.class, IllegalArgumentException.class},
+ fallbackMethod = "fallback")
+ public User getUserById(String id) {
+ validate(id);
+ return new User(id, "name" + id); // it should be network call
}
- private void validate(String id, String name) throws NullPointerException, IllegalArgumentException {
- Validate.notBlank(id);
- Validate.notBlank(name);
+ private User fallback(String id) {
+ return failoverService.getDefUser();
}
+ private void validate(String val) throws NullPointerException, IllegalArgumentException {
+ if (val == null) {
+ throw new NullPointerException("parameter cannot be null");
+ } else if (val.length() == 0) {
+ throw new IllegalArgumentException("parameter cannot be empty");
+ }
+ }
}
@Configurable
@@ -77,4 +114,10 @@ public UserService userService() {
}
}
+ private class FailoverService {
+ public User getDefUser() {
+ return new User("def", "def");
+ }
+ }
+
}
Please sign in to comment.
Something went wrong with that request. Please try again.