Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix for issue #241 (leaner error propagation in hystrix javanica) #244

Merged
merged 1 commit into from
Apr 15, 2014
Merged

Fix for issue #241 (leaner error propagation in hystrix javanica) #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hystrix-contrib/hystrix-javanica/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
23 changes: 18 additions & 5 deletions .../src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

}
83 changes: 63 additions & 20 deletions ...est/java/com/netflix/hystrix/contrib/javanica/test/spring/error/ErrorPropagationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -77,4 +114,10 @@ public UserService userService() {
}
}

private class FailoverService {
public User getDefUser() {
return new User("def", "def");
}
}

}