Skip to content

Commit 4129d07

Browse files
rubengeesrubengees
committed
Implement LoggingConstraints and increase test coverage
1 parent d6342a6 commit 4129d07

6 files changed

Lines changed: 123 additions & 15 deletions

File tree

‎gradle/versions.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ ext {
4343

4444
junit : "junit:junit:$junitVersion",
4545
assertj : "org.assertj:assertj-core:$assertjVersion",
46-
mockito : "org.mockito:mockito-android:$mockitoVersion",
46+
mockito : "org.mockito:mockito-core:$mockitoVersion",
4747
okhttpTls : "com.squareup.okhttp3:okhttp-tls:$okHttpVersion",
4848
mockwebserver : "com.squareup.okhttp3:mockwebserver:$okHttpVersion",
4949
privateConstructor: "com.pushtorefresh.java-private-constructor-checker:checker:$privateConstructorVersion"

‎library/src/main/java/me/proxer/library/api/LoggingInterceptor.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package me.proxer.library.api;
22

33
import lombok.AllArgsConstructor;
4+
import me.proxer.library.api.ProxerApi.Builder.LoggingConstraints;
45
import me.proxer.library.api.ProxerApi.Builder.LoggingStrategy;
56
import me.proxer.library.util.ProxerUrls;
67
import me.proxer.library.util.ProxerUtils;
@@ -24,23 +25,30 @@ class LoggingInterceptor implements Interceptor {
2425
private final CustomLogger customLogger;
2526

2627
private final LoggingStrategy loggingStrategy;
28+
private final LoggingConstraints loggingConstraints;
2729
private final String loggingTag;
2830

2931
@Override
3032
public Response intercept(final Chain chain) throws IOException {
3133
if (loggingStrategy == LoggingStrategy.ALL || ProxerUrls.hasProxerHost(chain.request().url())) {
3234
final Request requestCopy = chain.request().newBuilder().build();
33-
final String headerMessage = buildHeaderMessage(requestCopy);
34-
final String bodyMessage = buildBodyMessage(requestCopy);
35+
final String resultMessage;
3536

36-
final String message = "Requesting " + requestCopy.url() + " with method " + requestCopy.method()
37-
+ (bodyMessage.isEmpty() ? " and " : ", ") + headerMessage
38-
+ (!headerMessage.contains("\n") && bodyMessage.isEmpty() ? "." : bodyMessage);
37+
if (loggingConstraints == LoggingConstraints.URL_ONLY) {
38+
resultMessage = "Requesting " + requestCopy.url() + " with method " + requestCopy.method() + ".";
39+
} else {
40+
final String headerMessage = buildHeaderMessage(requestCopy);
41+
final String bodyMessage = buildBodyMessage(requestCopy);
42+
43+
resultMessage = "Requesting " + requestCopy.url() + " with method " + requestCopy.method()
44+
+ (bodyMessage.isEmpty() ? " and " : ", ") + headerMessage
45+
+ (!headerMessage.contains("\n") && bodyMessage.isEmpty() ? "." : bodyMessage);
46+
}
3947

4048
if (customLogger != null) {
41-
customLogger.log(message);
49+
customLogger.log(resultMessage);
4250
} else {
43-
Logger.getLogger(loggingTag).info(message);
51+
Logger.getLogger(loggingTag).info(resultMessage);
4452
}
4553
}
4654

‎library/src/main/java/me/proxer/library/api/ProxerApi.java‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,13 @@ public static final class Builder {
209209
@Setter
210210
private LoggingStrategy loggingStrategy = LoggingStrategy.NONE;
211211

212+
/**
213+
* Sets the logging constraints. These allow for logging only parts of the request.
214+
* By default, every part is logged.
215+
*/
216+
@Setter
217+
private LoggingConstraints loggingConstraints = LoggingConstraints.NONE;
218+
212219
/**
213220
* Sets the tag to use for logs.
214221
*/
@@ -284,7 +291,8 @@ private void initClient() {
284291
}
285292

286293
if (loggingStrategy != LoggingStrategy.NONE) {
287-
builder.addInterceptor(new LoggingInterceptor(customLogger, loggingStrategy, loggingTag));
294+
builder.addInterceptor(new LoggingInterceptor(customLogger, loggingStrategy,
295+
loggingConstraints, loggingTag));
288296
}
289297

290298
final List<Interceptor> existingInterceptors = builder.interceptors();
@@ -345,5 +353,18 @@ public enum LoggingStrategy {
345353
*/
346354
ALL
347355
}
356+
357+
public enum LoggingConstraints {
358+
359+
/**
360+
* Only the requested url shall be logged.
361+
*/
362+
URL_ONLY,
363+
364+
/**
365+
* Everything including url, body and headers shall be logged.
366+
*/
367+
NONE
368+
}
348369
}
349370
}

‎library/src/test/java/me/proxer/library/api/LoggingInterceptorTest.java‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import me.proxer.library.BuildConfig;
44
import me.proxer.library.ProxerTest;
5+
import me.proxer.library.api.ProxerApi.Builder.LoggingConstraints;
56
import me.proxer.library.api.ProxerApi.Builder.LoggingStrategy;
67
import okhttp3.Request;
78
import okhttp3.mockwebserver.MockResponse;
@@ -11,6 +12,7 @@
1112

1213
import java.io.ByteArrayOutputStream;
1314
import java.io.IOException;
15+
import java.nio.charset.StandardCharsets;
1416
import java.security.GeneralSecurityException;
1517
import java.util.concurrent.CountDownLatch;
1618
import java.util.logging.Formatter;
@@ -63,7 +65,7 @@ public void testLog() throws IOException, ProxerException {
6365
loggerHandler.flush();
6466
loggerStream.flush();
6567

66-
assertThat(loggerStream.toString("UTF-8")).isEqualTo("Requesting https://"
68+
assertThat(loggerStream.toString(StandardCharsets.UTF_8)).isEqualTo("Requesting https://"
6769
+ server.getHostName() + ":" + server.getPort()
6870
+ "/api/v1/notifications/news with method GET and these headers:\n"
6971
+ "proxer-api-key: mockKey\n"
@@ -83,7 +85,7 @@ public void testLogWithBody() throws IOException, ProxerException {
8385
loggerHandler.flush();
8486
loggerStream.flush();
8587

86-
assertThat(loggerStream.toString("UTF-8")).isEqualTo("Requesting https://"
88+
assertThat(loggerStream.toString(StandardCharsets.UTF_8)).isEqualTo("Requesting https://"
8789
+ server.getHostName() + ":" + server.getPort()
8890
+ "/api/v1/user/login with method POST, these headers:\n"
8991
+ "proxer-api-key: mockKey\n"
@@ -104,7 +106,7 @@ public void testLogWithEmptyBody() throws IOException, ProxerException {
104106
loggerHandler.flush();
105107
loggerStream.flush();
106108

107-
assertThat(loggerStream.toString("UTF-8")).isEqualTo("Requesting https://"
109+
assertThat(loggerStream.toString(StandardCharsets.UTF_8)).isEqualTo("Requesting https://"
108110
+ server.getHostName() + ":" + server.getPort()
109111
+ "/api/v1/user/logout with method POST, these headers:\n"
110112
+ "proxer-api-key: mockKey\n"
@@ -124,7 +126,7 @@ public void testLogAllOtherHost() throws IOException {
124126
loggerHandler.flush();
125127
loggerStream.flush();
126128

127-
assertThat(loggerStream.toString("UTF-8")).isEqualTo("Requesting http://"
129+
assertThat(loggerStream.toString(StandardCharsets.UTF_8)).isEqualTo("Requesting http://"
128130
+ server.getHostName() + ":" + server.getPort()
129131
+ "/test with method GET and no headers.");
130132
}
@@ -142,7 +144,7 @@ public void testLogApiOnly() throws IOException {
142144
loggerHandler.flush();
143145
loggerStream.flush();
144146

145-
assertThat(loggerStream.toString("UTF-8")).isEmpty();
147+
assertThat(loggerStream.toString(StandardCharsets.UTF_8)).isEmpty();
146148
}
147149

148150
@Test
@@ -158,7 +160,28 @@ public void testLogNone() throws IOException {
158160
loggerHandler.flush();
159161
loggerStream.flush();
160162

161-
assertThat(loggerStream.toString("UTF-8")).isEmpty();
163+
assertThat(loggerStream.toString(StandardCharsets.UTF_8)).isEmpty();
164+
}
165+
166+
@Test
167+
public void testLogRequestOnly() throws IOException {
168+
startHttpOnlyServer();
169+
170+
api = constructApi()
171+
.loggingStrategy(LoggingStrategy.ALL)
172+
.loggingConstraints(LoggingConstraints.URL_ONLY)
173+
.build();
174+
175+
server.enqueue(new MockResponse());
176+
177+
api.client().newCall(new Request.Builder().url("http://example.com/test").build()).execute();
178+
179+
loggerHandler.flush();
180+
loggerStream.flush();
181+
182+
assertThat(loggerStream.toString(StandardCharsets.UTF_8)).isEqualTo("Requesting http://"
183+
+ server.getHostName() + ":" + server.getPort()
184+
+ "/test with method GET.");
162185
}
163186

164187
@Test

‎library/src/test/java/me/proxer/library/api/ProxerCallTest.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@
88
import okhttp3.mockwebserver.SocketPolicy;
99
import org.junit.Test;
1010
import retrofit2.Call;
11+
import retrofit2.Response;
1112

1213
import java.io.IOException;
14+
import java.util.Collections;
1315
import java.util.List;
1416
import java.util.concurrent.CountDownLatch;
1517

@@ -75,6 +77,59 @@ public void testServerError() throws Exception {
7577
"Exception should have the correct message");
7678
}
7779

80+
@SuppressWarnings("unchecked")
81+
@Test
82+
public void testUnknownError() throws Exception {
83+
final Call<ProxerResponse<?>> internalCall = mock(Call.class);
84+
final ProxerCall<?> call = new ProxerCall(internalCall);
85+
86+
when(internalCall.execute()).thenThrow(IllegalStateException.class);
87+
88+
assertThatExceptionOfType(ProxerException.class)
89+
.isThrownBy(call::execute)
90+
.matches(exception -> exception.getErrorType() == ErrorType.UNKNOWN,
91+
"Exception should have the UNKNOWN ErrorType")
92+
.withCauseExactlyInstanceOf(IllegalStateException.class);
93+
}
94+
95+
@SuppressWarnings("unchecked")
96+
@Test
97+
public void testSafeExecute() throws IOException, ProxerException {
98+
final Call<ProxerResponse<List<NewsArticle>>> internalCall = mock(Call.class);
99+
final ProxerCall<List<NewsArticle>> call = new ProxerCall<>(internalCall);
100+
final Response<ProxerResponse<List<NewsArticle>>> internalResponse = mock(Response.class);
101+
final ProxerResponse<List<NewsArticle>> response = mock(ProxerResponse.class);
102+
103+
when(internalResponse.isSuccessful()).thenReturn(true);
104+
when(response.isSuccessful()).thenReturn(true);
105+
when(internalResponse.body()).thenReturn(response);
106+
when(response.getData()).thenReturn(Collections.emptyList());
107+
when(internalCall.execute()).thenReturn(internalResponse);
108+
109+
assertThat(call.safeExecute()).isNotNull();
110+
}
111+
112+
@SuppressWarnings("unchecked")
113+
@Test
114+
public void testSafeExecuteNull() throws IOException {
115+
final Call<ProxerResponse<List<NewsArticle>>> internalCall = mock(Call.class);
116+
final ProxerCall<List<NewsArticle>> call = new ProxerCall<>(internalCall);
117+
final Response<ProxerResponse<List<NewsArticle>>> internalResponse = mock(Response.class);
118+
final ProxerResponse<List<NewsArticle>> response = mock(ProxerResponse.class);
119+
120+
when(internalResponse.isSuccessful()).thenReturn(true);
121+
when(response.isSuccessful()).thenReturn(true);
122+
when(internalResponse.body()).thenReturn(response);
123+
when(response.getData()).thenReturn(null);
124+
when(internalCall.execute()).thenReturn(internalResponse);
125+
126+
assertThatExceptionOfType(ProxerException.class)
127+
.isThrownBy(call::safeExecute)
128+
.matches(exception -> exception.getErrorType() == ErrorType.UNKNOWN,
129+
"Exception should have the UNKNOWN ErrorType")
130+
.withCauseExactlyInstanceOf(NullPointerException.class);
131+
}
132+
78133
@Test(timeout = 1000L)
79134
public void testEnqueue() throws IOException, InterruptedException {
80135
final CountDownLatch lock = new CountDownLatch(1);

‎library/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
mock-maker-inline

0 commit comments

Comments
 (0)