Skip to content

Commit

Permalink
Remove singleton dependency from request tracker unit tests. (#340)
Browse files Browse the repository at this point in the history 
* Remove singleton dependency from request tracker unit tests.
* Tidy up feature code.
  • Loading branch information
@mikkokar
mikkokar authored Nov 13, 2018
1 parent b744a77 commit aa825f8
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 94 deletions.
52 changes: 24 additions & 28 deletions components/proxy/src/main/java/com/hotels/styx/admin/handlers/CurrentRequestsHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,34 +15,32 @@
*/
package com.hotels.styx.admin.handlers;

import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8;
import static com.hotels.styx.api.HttpHeaderNames.CONTENT_TYPE;
import static com.hotels.styx.api.HttpResponseStatus.OK;
import static java.lang.management.ManagementFactory.getThreadMXBean;
import static java.lang.Thread.State.BLOCKED;
import static java.lang.System.currentTimeMillis;
import static java.lang.String.format;
import com.hotels.styx.api.HttpResponse;
import com.hotels.styx.api.LiveHttpRequest;
import com.hotels.styx.api.LiveHttpResponse;
import com.hotels.styx.common.http.handler.BaseHttpHandler;
import com.hotels.styx.server.track.CurrentRequestTracker;

import java.lang.management.LockInfo;
import java.lang.management.MonitorInfo;
import java.lang.management.ThreadInfo;
import java.lang.management.ThreadMXBean;
import java.nio.charset.Charset;

import com.hotels.styx.api.HttpResponse;
import com.hotels.styx.api.LiveHttpRequest;
import com.hotels.styx.api.LiveHttpResponse;
import com.hotels.styx.common.http.handler.BaseHttpHandler;
import com.hotels.styx.server.track.CurrentRequestTracker;
import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8;
import static com.hotels.styx.api.HttpHeaderNames.CONTENT_TYPE;
import static com.hotels.styx.api.HttpResponseStatus.OK;
import static java.lang.String.format;
import static java.lang.System.currentTimeMillis;
import static java.lang.Thread.State.BLOCKED;
import static java.lang.management.ManagementFactory.getThreadMXBean;
import static java.nio.charset.StandardCharsets.UTF_8;

/**
* Admin handler that will help in tracking only the current HTTP requests to Styx.
*/
public class CurrentRequestsHandler extends BaseHttpHandler {

private static final Charset UTF_8 = Charset.forName("UTF-8");
private final ThreadMXBean threadMXBean;
private CurrentRequestTracker tracker;
private final CurrentRequestTracker tracker;

public CurrentRequestsHandler(CurrentRequestTracker tracker) {
this.threadMXBean = getThreadMXBean();
Expand All @@ -52,7 +50,7 @@ public CurrentRequestsHandler(CurrentRequestTracker tracker) {
@Override
public LiveHttpResponse doHandle(LiveHttpRequest request) {
boolean withStackTrace = request.queryParam("withStackTrace")
.map(it -> "true".equals(it))
.map("true"::equals)
.orElse(false);

return HttpResponse
Expand All @@ -66,7 +64,8 @@ public LiveHttpResponse doHandle(LiveHttpRequest request) {

private String getCurrentRequestContent(boolean withStackTrace) {
StringBuilder sb = new StringBuilder();
tracker.getCurrentRequests().forEach(req -> {

tracker.currentRequests().forEach(req -> {
sb.append("[\n");
sb.append(req.getRequest().replaceAll(",", "\n"));
sb.append("\n\n");
Expand All @@ -93,10 +92,9 @@ private String getCurrentRequestContent(boolean withStackTrace) {
}

private String getThreadInfo(long threadId) {

StringBuilder sb = new StringBuilder();

final ThreadInfo t = threadMXBean.getThreadInfo(threadId, Integer.MAX_VALUE);
ThreadInfo t = threadMXBean.getThreadInfo(threadId, Integer.MAX_VALUE);
sb.append(format("\"%s\" id=%d state=%s", t.getThreadName(), t.getThreadId(), t.getThreadState()));
sb.append(getThreadState(t));

Expand All @@ -122,10 +120,9 @@ private String getThreadInfo(long threadId) {
}

private String getThreadState(ThreadInfo t) {

StringBuilder sb = new StringBuilder();

final LockInfo lock = t.getLockInfo();
LockInfo lock = t.getLockInfo();
if (lock != null && t.getThreadState() != BLOCKED) {
sb.append(format("%n - waiting on <0x%08x> (a %s)", lock.getIdentityHashCode(), lock.getClassName()));
sb.append(format("%n - locked <0x%08x> (a %s)", lock.getIdentityHashCode(), lock.getClassName()));
Expand All @@ -137,10 +134,9 @@ private String getThreadState(ThreadInfo t) {
}

private String getThreadLockedSynchronizer(ThreadInfo t) {

StringBuilder sb = new StringBuilder();

final LockInfo[] locks = t.getLockedSynchronizers();
LockInfo[] locks = t.getLockedSynchronizers();
if (locks.length > 0) {
sb.append(format(" Locked synchronizers: count = %d%n", locks.length));
for (LockInfo l : locks) {
Expand All @@ -153,16 +149,16 @@ private String getThreadLockedSynchronizer(ThreadInfo t) {
}

private String getThreadElements(ThreadInfo t) {
final StackTraceElement[] elements = t.getStackTrace();
final MonitorInfo[] monitors = t.getLockedMonitors();
StackTraceElement[] elements = t.getStackTrace();
MonitorInfo[] monitors = t.getLockedMonitors();

StringBuilder sb = new StringBuilder();

for (int i = 0; i < elements.length; i++) {
final StackTraceElement element = elements[i];
StackTraceElement element = elements[i];
sb.append(format(" at %s%n", element));
for (int j = 1; j < monitors.length; j++) {
final MonitorInfo monitor = monitors[j];
MonitorInfo monitor = monitors[j];
if (monitor.getLockedStackDepth() == i) {
sb.append(format(" - locked %s%n", monitor));
}
Expand Down
33 changes: 19 additions & 14 deletions ...onents/proxy/src/test/java/com/hotels/styx/admin/handlers/CurrentRequestsHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.hotels.styx.api.LiveHttpRequest.get;
import static java.nio.charset.StandardCharsets.UTF_8;

import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import com.hotels.styx.api.Buffer;
Expand All @@ -33,42 +34,46 @@
public class CurrentRequestsHandlerTest {

LiveHttpRequest req1 = get("/requestId1").build();
LiveHttpRequest req2 = get("/requestId2").build();

CurrentRequestTracker tracker = new CurrentRequestTracker();
private CurrentRequestsHandler handler;

@BeforeMethod
public void setUp() {
tracker = new CurrentRequestTracker();
handler = new CurrentRequestsHandler(tracker);
}

@Test
public void testStackTrace() {
CurrentRequestTracker.INSTANCE.clear();
Thread.currentThread().setName("Test-Thread");
CurrentRequestTracker.INSTANCE.trackRequest(req1);
LiveHttpResponse response = (new CurrentRequestsHandler(CurrentRequestTracker.INSTANCE)).doHandle(req1);
tracker.trackRequest(req1);
LiveHttpResponse response = handler.doHandle(req1);
assertThat(Flux.from(response.body()).map(this::decodeUtf8String).blockFirst().contains("Test-Thread"), is(true));
}

@Test
public void testStackTraceForSentRequest() {
CurrentRequestTracker.INSTANCE.clear();
Thread.currentThread().setName("Test-Thread-1");
CurrentRequestTracker.INSTANCE.trackRequest(req1);
CurrentRequestTracker.INSTANCE.markRequestAsSent(req1);
LiveHttpResponse response = (new CurrentRequestsHandler(CurrentRequestTracker.INSTANCE)).doHandle(req1);
tracker.trackRequest(req1);
tracker.markRequestAsSent(req1);
LiveHttpResponse response = handler.doHandle(req1);
assertThat(Flux.from(response.body()).map(this::decodeUtf8String).blockFirst().contains("Request state: Waiting response from origin."), is(true));
}

@Test
public void testWithStackTrace() {
CurrentRequestTracker.INSTANCE.clear();
Thread.currentThread().setName("Test-Thread");
CurrentRequestTracker.INSTANCE.trackRequest(req1);
LiveHttpResponse response = (new CurrentRequestsHandler(CurrentRequestTracker.INSTANCE)).doHandle(get("/req?withStackTrace=true").build());
tracker.trackRequest(req1);
LiveHttpResponse response = handler.doHandle(get("/req?withStackTrace=true").build());
assertThat(Flux.from(response.body()).map(this::decodeUtf8String).blockFirst().contains("id=" + Thread.currentThread().getId()), is(true));
}

@Test
public void testWithoutStackTrace() {
CurrentRequestTracker.INSTANCE.clear();
Thread.currentThread().setName("Test-Thread");
CurrentRequestTracker.INSTANCE.trackRequest(req1);
LiveHttpResponse response = (new CurrentRequestsHandler(CurrentRequestTracker.INSTANCE)).doHandle(req1);
tracker.trackRequest(req1);
LiveHttpResponse response = handler.doHandle(req1);
assertThat(Flux.from(response.body()).map(this::decodeUtf8String).blockFirst().contains("id=" + Thread.currentThread().getId()), is(false));
}

Expand Down
21 changes: 11 additions & 10 deletions components/server/src/main/java/com/hotels/styx/server/track/CurrentRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,21 @@
import static java.lang.System.currentTimeMillis;

import java.util.function.Supplier;

import com.hotels.styx.api.LiveHttpRequest;

/**
* Bean that represent the current request.
*/
public class CurrentRequest {

private final String request;
private final long startingTimeMillies;
private volatile Thread currentThread;
private final Supplier<String> stateSupplier;
private boolean requestSent;

public CurrentRequest(LiveHttpRequest request, Supplier<String> stateSupplier) {
private volatile boolean requestSent;
private volatile Thread currentThread;

CurrentRequest(LiveHttpRequest request, Supplier<String> stateSupplier) {
this.startingTimeMillies = currentTimeMillis();
this.currentThread = Thread.currentThread();
this.request = request.toString();
Expand All @@ -42,10 +43,6 @@ public Thread getCurrentThread() {
return currentThread;
}

public void setCurrentThread(Thread currentThread) {
this.currentThread = currentThread;
}

public String getRequest() {
return request;
}
Expand All @@ -62,7 +59,11 @@ public boolean isRequestSent() {
return requestSent;
}

public void setRequestSent(boolean requestSent) {
this.requestSent = requestSent;
void setCurrentThread(Thread currentThread) {
this.currentThread = currentThread;
}

void requestSent() {
this.requestSent = true;
}
}
19 changes: 5 additions & 14 deletions components/server/src/main/java/com/hotels/styx/server/track/CurrentRequestTracker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,19 @@
*/
package com.hotels.styx.server.track;

import com.hotels.styx.api.LiveHttpRequest;

import java.util.Collection;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Supplier;

import com.hotels.styx.api.LiveHttpRequest;

/**
* Manger class to manage the current requests.
*/
public class CurrentRequestTracker implements RequestTracker {

private static ConcurrentHashMap<Object, CurrentRequest> currentRequests = new ConcurrentHashMap<>();

public static final CurrentRequestTracker INSTANCE = new CurrentRequestTracker();

private CurrentRequestTracker() {
}
private final ConcurrentHashMap<Object, CurrentRequest> currentRequests = new ConcurrentHashMap<>();

public void trackRequest(LiveHttpRequest request, Supplier<String> state) {
if (currentRequests.containsKey(request.id())) {
Expand All @@ -50,21 +46,16 @@ public void trackRequest(LiveHttpRequest request) {
}

public void markRequestAsSent(LiveHttpRequest request) {

if (currentRequests.containsKey(request.id())) {
currentRequests.get(request.id()).setRequestSent(true);
currentRequests.get(request.id()).requestSent();
}
}

public void endTrack(LiveHttpRequest request) {
currentRequests.remove(request.id());
}

public Collection<CurrentRequest> getCurrentRequests() {
public Collection<CurrentRequest> currentRequests() {
return currentRequests.values();
}

public void clear() {
currentRequests.clear();
}
}
58 changes: 30 additions & 28 deletions components/server/src/test/java/com/hotels/styx/server/track/CurrentRequestTrackerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,61 +29,63 @@ public class CurrentRequestTrackerTest {
LiveHttpRequest req1 = get("/requestId1").build();
LiveHttpRequest req2 = get("/requestId2").build();

CurrentRequestTracker tracker = new CurrentRequestTracker();

@BeforeMethod
public void setUp() {
CurrentRequestTracker.INSTANCE.clear();
tracker = new CurrentRequestTracker();
}

@Test
public void testTrackRequest() {
CurrentRequestTracker.INSTANCE.trackRequest(req1);
assertThat(CurrentRequestTracker.INSTANCE.getCurrentRequests().iterator().next().getRequest(), is(req1.toString()));
tracker.trackRequest(req1);
assertThat(tracker.currentRequests().iterator().next().getRequest(), is(req1.toString()));
}

@Test
public void testChangeWorkingThread() {
Thread.currentThread().setName("thread-1");
CurrentRequestTracker.INSTANCE.trackRequest(req1);
assertThat("thread-1", is(CurrentRequestTracker.INSTANCE.getCurrentRequests().iterator().next().getCurrentThread().getName()));
tracker.trackRequest(req1);
assertThat("thread-1", is(tracker.currentRequests().iterator().next().getCurrentThread().getName()));
Thread.currentThread().setName("thread-2");
CurrentRequestTracker.INSTANCE.trackRequest(req1);
assertThat("thread-2", is(CurrentRequestTracker.INSTANCE.getCurrentRequests().iterator().next().getCurrentThread().getName()));
tracker.trackRequest(req1);
assertThat("thread-2", is(tracker.currentRequests().iterator().next().getCurrentThread().getName()));
}

@Test
public void testTrackingSameReqMultipleTimesWillNotGenerateMultipleEntries() {
assertThat(CurrentRequestTracker.INSTANCE.getCurrentRequests().size(), is(0));
CurrentRequestTracker.INSTANCE.trackRequest(req1);
CurrentRequestTracker.INSTANCE.trackRequest(req1);
CurrentRequestTracker.INSTANCE.trackRequest(req1);
CurrentRequestTracker.INSTANCE.trackRequest(req1);
assertThat(CurrentRequestTracker.INSTANCE.getCurrentRequests().size(), is(1));
assertThat(CurrentRequestTracker.INSTANCE.getCurrentRequests().iterator().next().getRequest(), is(req1.toString()));
assertThat(tracker.currentRequests().size(), is(0));
tracker.trackRequest(req1);
tracker.trackRequest(req1);
tracker.trackRequest(req1);
tracker.trackRequest(req1);
assertThat(tracker.currentRequests().size(), is(1));
assertThat(tracker.currentRequests().iterator().next().getRequest(), is(req1.toString()));
}

@Test
public void testEndTrack() {
CurrentRequestTracker.INSTANCE.trackRequest(req1);
assertThat(CurrentRequestTracker.INSTANCE.getCurrentRequests().size(), is(1));
assertThat(CurrentRequestTracker.INSTANCE.getCurrentRequests().iterator().next().getRequest(), is(req1.toString()));
CurrentRequestTracker.INSTANCE.endTrack(req1);
assertThat(CurrentRequestTracker.INSTANCE.getCurrentRequests().size(), is(0));
tracker.trackRequest(req1);
assertThat(tracker.currentRequests().size(), is(1));
assertThat(tracker.currentRequests().iterator().next().getRequest(), is(req1.toString()));
tracker.endTrack(req1);
assertThat(tracker.currentRequests().size(), is(0));
}

@Test
public void testEndTrackWillEffectOneRequest() {
CurrentRequestTracker.INSTANCE.trackRequest(req1);
CurrentRequestTracker.INSTANCE.trackRequest(req2);
assertThat(CurrentRequestTracker.INSTANCE.getCurrentRequests().size(), is(2));
CurrentRequestTracker.INSTANCE.endTrack(req1);
assertThat(CurrentRequestTracker.INSTANCE.getCurrentRequests().size(), is(1));
tracker.trackRequest(req1);
tracker.trackRequest(req2);
assertThat(tracker.currentRequests().size(), is(2));
tracker.endTrack(req1);
assertThat(tracker.currentRequests().size(), is(1));
}

@Test
public void testEndTrackWillEffectTheCorrectRequest() {
CurrentRequestTracker.INSTANCE.trackRequest(req1);
CurrentRequestTracker.INSTANCE.trackRequest(req2);
CurrentRequestTracker.INSTANCE.endTrack(req1);
assertThat(CurrentRequestTracker.INSTANCE.getCurrentRequests().iterator().next().getRequest(), is(req2.toString()));
tracker.trackRequest(req1);
tracker.trackRequest(req2);
tracker.endTrack(req1);
assertThat(tracker.currentRequests().iterator().next().getRequest(), is(req2.toString()));
}
}

0 comments on commit aa825f8

Please sign in to comment.