Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Commit

Permalink
rhbz988202 - simplify class and test
Browse files Browse the repository at this point in the history
  • Loading branch information
Patrick Huang committed Apr 9, 2014
1 parent 21a60ed commit 9c17aab
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 54 deletions.
Expand Up @@ -29,7 +29,7 @@
* @author Patrick Huang <a
* href="mailto:pahuang@redhat.com">pahuang@redhat.com</a>
*/
@Name("rateLimiterHolder")
@Name("rateLimitManager")
@Scope(ScopeType.APPLICATION)
@AutoCreate
@Slf4j
Expand All @@ -48,7 +48,7 @@ public class RateLimitManager implements Introspectable {
private int maxActive;

public static RateLimitManager getInstance() {
return (RateLimitManager) Component.getInstance("rateLimiterHolder");
return (RateLimitManager) Component.getInstance("rateLimitManager");
}

@Create
Expand Down
Expand Up @@ -2,10 +2,9 @@

import java.util.concurrent.TimeUnit;

import com.google.common.annotations.VisibleForTesting;
import org.jboss.resteasy.spi.HttpResponse;
import org.jboss.seam.Component;
import org.zanata.ApplicationConfiguration;
import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.extern.slf4j.Slf4j;

/**
Expand All @@ -16,17 +15,23 @@
* href="mailto:pahuang@redhat.com">pahuang@redhat.com</a>
*/
@Slf4j
@AllArgsConstructor(access = AccessLevel.PROTECTED)
public class RateLimitingProcessor {
// http://tools.ietf.org/html/rfc6585
public static final int TOO_MANY_REQUEST = 429;
private RateLimitManager rateLimitManager;

// for seam to use
public RateLimitingProcessor() {
rateLimitManager = RateLimitManager.getInstance();
}

private final LeakyBucket logLimiter = new LeakyBucket(1, 5,
TimeUnit.MINUTES);

public void
process(String apiKey, HttpResponse response, Runnable taskToRun)
throws Exception {
RateLimitManager rateLimitManager = getRateLimitManager();
RestCallLimiter rateLimiter = rateLimitManager.getLimiter(apiKey);

log.debug("check semaphore for {}", this);
Expand All @@ -45,9 +50,4 @@ public class RateLimitingProcessor {
}
}

@VisibleForTesting
RateLimitManager getRateLimitManager() {
return RateLimitManager.getInstance();
}

}
Expand Up @@ -38,6 +38,7 @@ public class RateLimitingProcessorTest {
private HttpResponse response;
@Mock
private FilterChain filterChain;
@Mock
private RateLimitManager rateLimitManager;
@Mock
private Runnable runnable;
Expand All @@ -46,55 +47,30 @@ public class RateLimitingProcessorTest {
public void beforeMethod() throws IOException {
MockitoAnnotations.initMocks(this);

// so that we can verify its interaction
rateLimitManager = spy(new RateLimitManager());
processor =
spy(new RateLimitingProcessor());
processor = new RateLimitingProcessor(rateLimitManager);
}

doReturn(rateLimitManager).when(processor).getRateLimitManager();
@Test
public void restCallLimiterReturnsFalseWillCauseErrorResponse()
throws Exception {
RestCallLimiter restCallLimiter = mock(RestCallLimiter.class);
when(restCallLimiter.tryAcquireAndRun(runnable)).thenReturn(false);
doReturn(restCallLimiter).when(rateLimitManager).getLimiter(API_KEY);

processor.process(API_KEY, response, runnable);

verify(response).sendError(eq(429), anyString());
}

@Test
public void willFirstTryAcquire() throws InterruptedException, IOException,
ServletException {
int threads = 2;
int maxConcurrent = 1;
when(rateLimitManager.getMaxConcurrent()).thenReturn(maxConcurrent);
when(rateLimitManager.getMaxActive()).thenReturn(maxConcurrent);
final CountDownLatch countDownLatch = new CountDownLatch(threads);
doAnswer(new Answer() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
countDownLatch.countDown();
return invocation.callRealMethod();
}
}).when(rateLimitManager).getLimiter(API_KEY);

// two concurrent requests which will exceed the limit
Callable<Void> callable = new Callable<Void>() {
public void restCallLimiterReturnsTrueWillNotReturnErrorResponse()
throws Exception {
RestCallLimiter restCallLimiter = mock(RestCallLimiter.class);
when(restCallLimiter.tryAcquireAndRun(runnable)).thenReturn(true);
doReturn(restCallLimiter).when(rateLimitManager).getLimiter(API_KEY);

@Override
public Void call() throws Exception {
processor.process(API_KEY, response, runnable);
return null;
}
};
List<Callable<Void>> callables = Collections.nCopies(threads, callable);
ExecutorService executorService = Executors.newFixedThreadPool(threads);
List<Future<Void>> futures = executorService.invokeAll(callables);
for (Future<Void> future : futures) {
try {
future.get();
} catch (ExecutionException e) {
throw Throwables.propagate(e.getCause());
}
}
countDownLatch.await(1, TimeUnit.SECONDS);
processor.process(API_KEY, response, runnable);

// one request will receive 429 and an error message
verify(response, atLeastOnce()).sendError(eq(429), anyString());
// one should go through
verify(runnable).run();
verifyZeroInteractions(response);

This comment has been minimized.

Copy link
@seanf

seanf Apr 9, 2014

Contributor
  • Shouldn't you keep this line? verify(runnable).run(); EDIT: just need to check that tryAcquireAndRun is called
}
}

1 comment on commit 9c17aab

@huangp
Copy link
Collaborator

@huangp huangp commented on 9c17aab Apr 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zanata-jenkins retest this please

Please sign in to comment.