Skip to content

Commit

Permalink
Merge pull request square#255 from square/jw/plank
Browse files Browse the repository at this point in the history
Add two levels of logging.
  • Loading branch information
dnkoutso committed Jul 9, 2013
2 parents 5d4e4c4 + 8d5e022 commit 9a2f308
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 47 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
Change Log
==========

Version 1.2.0 *(In Development)*
--------------------------------

* Change `setDebug` to `setLogLevel` on `RestAdapter` and `RestAdapter.Builder` and provide
two levels of logging via `LogLevel`.
* Query parameters can now be added in a request interceptor.


Version 1.1.1 *(2013-06-25)*
----------------------------

Expand Down
58 changes: 43 additions & 15 deletions retrofit/src/main/java/retrofit/RestAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,16 @@ public interface Log {
void log(String message);
}

/** Controls the level of logging. */
public enum LogLevel {
/** No logging. */
NONE,
/** Log only the request method and URL and the response status code and execution time. */
BASIC,
/** Log the headers, body, and metadata for both requests and responses. */
FULL
}

private final Server server;
private final Client.Provider clientProvider;
private final Executor httpExecutor;
Expand All @@ -122,11 +132,11 @@ public interface Log {
private final Profiler profiler;
private final ErrorHandler errorHandler;
private final Log log;
private volatile boolean debug;
private volatile LogLevel logLevel;

private RestAdapter(Server server, Client.Provider clientProvider, Executor httpExecutor,
Executor callbackExecutor, RequestInterceptor requestInterceptor, Converter converter,
Profiler profiler, ErrorHandler errorHandler, Log log, boolean debug) {
Profiler profiler, ErrorHandler errorHandler, Log log, LogLevel logLevel) {
this.server = server;
this.clientProvider = clientProvider;
this.httpExecutor = httpExecutor;
Expand All @@ -136,12 +146,15 @@ private RestAdapter(Server server, Client.Provider clientProvider, Executor http
this.profiler = profiler;
this.errorHandler = errorHandler;
this.log = log;
this.debug = debug;
this.logLevel = logLevel;
}

/** Toggle debug logging on or off. */
public void setDebug(boolean debug) {
this.debug = debug;
/** Change the level of logging. */
public void setLogLevel(LogLevel loglevel) {
if (logLevel == null) {
throw new NullPointerException("Log level may not be null.");
}
this.logLevel = loglevel;
}

/** Create an implementation of the API defined by the specified {@code service} interface. */
Expand Down Expand Up @@ -228,8 +241,10 @@ private Object invokeRequest(RestMethodInfo methodDetails, Object[] args) {
Thread.currentThread().setName(THREAD_PREFIX + url.substring(serverUrl.length()));
}

if (debug) {
if (logLevel == LogLevel.FULL) {
request = logAndReplaceRequest(request);
} else if (logLevel == LogLevel.BASIC) {
logRequestLine(request);
}

Object profilerObject = null;
Expand All @@ -248,8 +263,10 @@ private Object invokeRequest(RestMethodInfo methodDetails, Object[] args) {
profiler.afterCall(requestInfo, elapsedTime, statusCode, profilerObject);
}

if (debug) {
if (logLevel == LogLevel.FULL) {
response = logAndReplaceResponse(url, response, elapsedTime);
} else if (logLevel == LogLevel.BASIC) {
logResponseLine(url, response, elapsedTime);
}

Type type = methodDetails.responseObjectType;
Expand Down Expand Up @@ -300,9 +317,17 @@ private Object invokeRequest(RestMethodInfo methodDetails, Object[] args) {
}
}

private void logRequestLine(Request request) {
log.log(String.format("---> HTTP %s %s", request.getMethod(), request.getUrl()));
}

private void logResponseLine(String url, Response response, long elapsedTime) {
log.log(String.format("<--- HTTP %s %s (%sms)", response.getStatus(), url, elapsedTime));
}

/** Log request headers and body. Consumes request body and returns identical replacement. */
private Request logAndReplaceRequest(Request request) throws IOException {
log.log(String.format("---> HTTP %s %s", request.getMethod(), request.getUrl()));
logRequestLine(request);

for (Header header : request.getHeaders()) {
log.log(header.getName() + ": " + header.getValue());
Expand Down Expand Up @@ -338,7 +363,7 @@ private Request logAndReplaceRequest(Request request) throws IOException {
/** Log response headers and body. Consumes response body and returns identical replacement. */
private Response logAndReplaceResponse(String url, Response response, long elapsedTime)
throws IOException {
log.log(String.format("<--- HTTP %s %s (%sms)", response.getStatus(), url, elapsedTime));
logResponseLine(url, response, elapsedTime);

for (Header header : response.getHeaders()) {
log.log(header.getName() + ": " + header.getValue());
Expand Down Expand Up @@ -414,7 +439,7 @@ public static class Builder {
private Profiler profiler;
private ErrorHandler errorHandler;
private Log log;
private boolean debug;
private LogLevel logLevel = LogLevel.NONE;

/** API server base URL. */
public Builder setServer(String endpoint) {
Expand Down Expand Up @@ -522,9 +547,12 @@ public Builder setLog(Log log) {
return this;
}

/** Enable debug logging. */
public Builder setDebug(boolean debug) {
this.debug = debug;
/** Change the level of logging. */
public Builder setLogLevel(LogLevel logLevel) {
if (logLevel == null) {
throw new NullPointerException("Log level may not be null.");
}
this.logLevel = logLevel;
return this;
}

Expand All @@ -535,7 +563,7 @@ public RestAdapter build() {
}
ensureSaneDefaults();
return new RestAdapter(server, clientProvider, httpExecutor, callbackExecutor,
requestInterceptor, converter, profiler, errorHandler, log, debug);
requestInterceptor, converter, profiler, errorHandler, log, logLevel);
}

private void ensureSaneDefaults() {
Expand Down
92 changes: 60 additions & 32 deletions retrofit/src/test/java/retrofit/RestAdapterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
import java.util.concurrent.Executor;
import org.junit.Before;
import org.junit.Test;
import retrofit.converter.ConversionException;
import retrofit.http.GET;
import retrofit.client.Client;
import retrofit.client.Header;
import retrofit.client.Request;
import retrofit.client.Response;
import retrofit.converter.ConversionException;
import retrofit.http.GET;
import retrofit.mime.TypedInput;
import retrofit.mime.TypedString;

Expand All @@ -32,10 +32,27 @@
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
import static retrofit.Profiler.RequestInformation;
import static retrofit.RestAdapter.LogLevel.BASIC;
import static retrofit.RestAdapter.LogLevel.FULL;
import static retrofit.Utils.SynchronousExecutor;

public class RestAdapterTest {
private static List<Header> NO_HEADERS = Collections.emptyList();
private static final List<Header> NO_HEADERS = Collections.emptyList();

/** Not all servers play nice and add content-type headers to responses. */
private static final TypedInput NO_MIME_BODY = new TypedInput() {
@Override public String mimeType() {
return null;
}

@Override public long length() {
return 2;
}

@Override public InputStream in() throws IOException {
return new ByteArrayInputStream("{}".getBytes("UTF-8"));
}
};

private interface Example {
@GET("/") Object something();
Expand Down Expand Up @@ -85,7 +102,34 @@ private interface Example {
verify(mockProfiler).afterCall(any(RequestInformation.class), anyInt(), eq(200), same(data));
}

@Test public void logSuccessfulRequestResponseOnDebugWhenResponseBodyPresent() throws Exception {
@Test public void logRequestResponseBasic() throws Exception {
final List<String> logMessages = new ArrayList<String>();
RestAdapter.Log log = new RestAdapter.Log() {
public void log(String message) {
logMessages.add(message);
}
};

Example example = new RestAdapter.Builder() //
.setClient(mockClient)
.setExecutors(mockRequestExecutor, mockCallbackExecutor)
.setServer("http://example.com")
.setProfiler(mockProfiler)
.setLog(log)
.setLogLevel(BASIC)
.build()
.create(Example.class);

when(mockClient.execute(any(Request.class))) //
.thenReturn(new Response(200, "OK", NO_HEADERS, new TypedString("{}")));

example.something();
assertThat(logMessages).hasSize(2);
assertThat(logMessages.get(0)).isEqualTo("---> HTTP GET http://example.com/");
assertThat(logMessages.get(1)).matches("<--- HTTP 200 http://example.com/ \\([0-9]+ms\\)");
}

@Test public void logSuccessfulRequestResponseFullWhenResponseBodyPresent() throws Exception {
final List<String> logMessages = new ArrayList<String>();
RestAdapter.Log log = new RestAdapter.Log() {
public void log(String message) {
Expand All @@ -99,7 +143,7 @@ public void log(String message) {
.setServer("http://example.com")
.setProfiler(mockProfiler)
.setLog(log)
.setDebug(true)
.setLogLevel(FULL)
.build()
.create(Example.class);

Expand All @@ -115,7 +159,7 @@ public void log(String message) {
assertThat(logMessages.get(4)).isEqualTo("<--- END HTTP (2-byte body)");
}

@Test public void logSuccessfulRequestResponseOnDebugWhenResponseBodyAbsent() throws Exception {
@Test public void logSuccessfulRequestResponseFullWhenResponseBodyAbsent() throws Exception {
final List<String> logMessages = new ArrayList<String>();
RestAdapter.Log log = new RestAdapter.Log() {
public void log(String message) {
Expand All @@ -129,7 +173,7 @@ public void log(String message) {
.setServer("http://example.com")
.setProfiler(mockProfiler)
.setLog(log)
.setDebug(true)
.setLogLevel(FULL)
.build()
.create(Example.class);

Expand All @@ -144,30 +188,14 @@ public void log(String message) {
assertThat(logMessages.get(3)).isEqualTo("<--- END HTTP (0-byte body)");
}

/** Not all servers play nice and add content-type headers to responses. */
TypedInput inputMissingMimeType = new TypedInput() {

@Override public String mimeType() {
return null;
}

@Override public long length() {
return 2;
}

@Override public InputStream in() throws IOException {
return new ByteArrayInputStream("{}".getBytes());
}
};

@Test public void successfulRequestResponseWhenMimeTypeMissing() throws Exception {
when(mockClient.execute(any(Request.class))) //
.thenReturn(new Response(200, "OK", NO_HEADERS, inputMissingMimeType));
.thenReturn(new Response(200, "OK", NO_HEADERS, NO_MIME_BODY));

example.something();
}

@Test public void logSuccessfulRequestResponseOnDebugWhenMimeTypeMissing() throws Exception {
@Test public void logSuccessfulRequestResponseFullWhenMimeTypeMissing() throws Exception {
final List<String> logMessages = new ArrayList<String>();
RestAdapter.Log log = new RestAdapter.Log() {
public void log(String message) {
Expand All @@ -181,12 +209,12 @@ public void log(String message) {
.setServer("http://example.com")
.setProfiler(mockProfiler)
.setLog(log)
.setDebug(true)
.setLogLevel(FULL)
.build()
.create(Example.class);

when(mockClient.execute(any(Request.class))) //
.thenReturn(new Response(200, "OK", NO_HEADERS, inputMissingMimeType));
.thenReturn(new Response(200, "OK", NO_HEADERS, NO_MIME_BODY));

example.something();
assertThat(logMessages).hasSize(5);
Expand Down Expand Up @@ -245,7 +273,7 @@ public void log(String message) {
}
}

@Test public void logErrorRequestResponseOnDebugWhenMimeTypeMissing() throws Exception {
@Test public void logErrorRequestResponseFullWhenMimeTypeMissing() throws Exception {
final List<String> logMessages = new ArrayList<String>();
RestAdapter.Log log = new RestAdapter.Log() {
public void log(String message) {
Expand All @@ -259,12 +287,12 @@ public void log(String message) {
.setServer("http://example.com")
.setProfiler(mockProfiler)
.setLog(log)
.setDebug(true)
.setLogLevel(FULL)
.build()
.create(Example.class);

Response responseMissingMimeType = //
new Response(403, "Forbidden", NO_HEADERS, inputMissingMimeType);
new Response(403, "Forbidden", NO_HEADERS, NO_MIME_BODY);

when(mockClient.execute(any(Request.class))).thenReturn(responseMissingMimeType);

Expand All @@ -283,7 +311,7 @@ public void log(String message) {
assertThat(logMessages.get(4)).isEqualTo("<--- END HTTP (2-byte body)");
}

@Test public void logErrorRequestResponseOnDebugWhenResponseBodyAbsent() throws Exception {
@Test public void logErrorRequestResponseFullWhenResponseBodyAbsent() throws Exception {
final List<String> logMessages = new ArrayList<String>();
RestAdapter.Log log = new RestAdapter.Log() {
public void log(String message) {
Expand All @@ -297,7 +325,7 @@ public void log(String message) {
.setServer("http://example.com")
.setProfiler(mockProfiler)
.setLog(log)
.setDebug(true)
.setLogLevel(FULL)
.build()
.create(Example.class);

Expand Down

0 comments on commit 9a2f308

Please sign in to comment.