Skip to content

Commit f6a9781

Browse files
pc9795pc9795
committed
Restructuring SettingsResponse
1 parent 8ef782a commit f6a9781

File tree

11 files changed

+123
-81
lines changed

11 files changed

+123
-81
lines changed

‎src/main/java/com/exceptionless/exceptionlessclient/exceptions/SettingsClientException.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package com.exceptionless.exceptionlessclient.exceptions;
2+
3+
public class SettingsClientException extends RuntimeException{
4+
public SettingsClientException(Throwable cause) {
5+
super(cause);
6+
}
7+
8+
public SettingsClientException(String message) {
9+
super(message);
10+
}
11+
}

‎src/main/java/com/exceptionless/exceptionlessclient/models/submission/SettingsResponse.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,15 @@
99
@Value
1010
@NonFinal
1111
public class SettingsResponse {
12-
@Builder.Default Boolean success = false;
12+
int code;
13+
String body;
1314
ServerSettings settings;
14-
Exception exception;
15-
String message;
1615

17-
public Boolean isSuccess() {
18-
return success;
16+
public boolean isSuccess() {
17+
return code >= 200 && code <= 299;
18+
}
19+
20+
public boolean isNotModified() {
21+
return code == 304;
1922
}
2023
}

‎src/main/java/com/exceptionless/exceptionlessclient/queue/DefaultEventQueue.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ private boolean shouldSuspendProcessing() {
8383
}
8484

8585
@VisibleForTesting
86-
Boolean isProcessingCurrentlySuspended(){
86+
Boolean isProcessingCurrentlySuspended() {
8787
return shouldSuspendProcessing();
8888
}
8989

@@ -113,7 +113,7 @@ private boolean shouldDiscard() {
113113

114114
@Override
115115
public void process() {
116-
synchronized (this){
116+
synchronized (this) {
117117
if (processingQueue) {
118118
LOG.trace("Currently processing queue; Returning...");
119119
return;
@@ -137,10 +137,10 @@ public void process() {
137137
processSubmissionResponse(response, storedEvents);
138138
eventPosted(response, events);
139139
} catch (SubmissionClientException e) {
140-
LOG.error("Error processing queue", e);
140+
LOG.error("Error submitting events from queue", e);
141141
suspendProcessing();
142142
} finally {
143-
synchronized (this){
143+
synchronized (this) {
144144
processingQueue = false;
145145
}
146146
}
@@ -176,7 +176,10 @@ private void processSubmissionResponse(
176176
}
177177

178178
if (response.isNotFound() || response.isBadRequest()) {
179-
LOG.error(String.format("Error while trying to submit data: %s", response.getBody()));
179+
LOG.error(
180+
String.format(
181+
"Error while trying to submit data, Code:%s, Body:%s",
182+
response.getCode(), response.getBody()));
180183
suspendProcessing(Duration.ofMinutes(4));
181184
removeEvents(storedEvents);
182185
return;
@@ -195,7 +198,9 @@ private void processSubmissionResponse(
195198
return;
196199
}
197200

198-
LOG.error(String.format("Error submitting events: %s", response.getBody()));
201+
LOG.error(
202+
String.format(
203+
"Error submitting events, Code: %s, Body: %s", response.getCode(), response.getBody()));
199204
suspendProcessing();
200205
}
201206

‎src/main/java/com/exceptionless/exceptionlessclient/settings/DefaultSettingsClient.java

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.exceptionless.exceptionlessclient.settings;
22

33
import com.exceptionless.exceptionlessclient.configuration.Configuration;
4+
import com.exceptionless.exceptionlessclient.exceptions.SettingsClientException;
45
import com.exceptionless.exceptionlessclient.models.submission.SettingsResponse;
56
import com.exceptionless.exceptionlessclient.utils.Utils;
67
import com.exceptionless.exceptionlessclient.utils.VisibleForTesting;
@@ -25,7 +26,6 @@ public DefaultSettingsClient(Configuration configuration) {
2526
.newBuilder()
2627
.connectTimeout(Duration.ofMillis(configuration.getSettingsClientTimeoutInMillis()))
2728
.build();
28-
;
2929
}
3030

3131
@VisibleForTesting
@@ -49,24 +49,20 @@ public SettingsResponse getSettings(long version) {
4949
Response response = httpClient.newCall(request).execute();
5050

5151
ResponseBody body = response.body();
52-
if (response.code() / 100 != 2) {
53-
return SettingsResponse.builder()
54-
.success(false)
55-
.message(Utils.addCodeToResponseBodyStr(response))
56-
.build();
57-
}
58-
if (body == null) {
59-
return SettingsResponse.builder()
60-
.success(false)
61-
.message("No settings returned by server!")
62-
.build();
52+
String bodyStr = body == null ? null : body.string();
53+
if (bodyStr == null) {
54+
return SettingsResponse.builder().code(response.code()).body("").build();
6355
}
6456

6557
ServerSettings serverSettings =
66-
Utils.JSON_MAPPER.readValue(body.string(), new TypeReference<ServerSettings>() {});
67-
return SettingsResponse.builder().success(true).settings(serverSettings).build();
58+
Utils.JSON_MAPPER.readValue(bodyStr, new TypeReference<ServerSettings>() {});
59+
return SettingsResponse.builder()
60+
.code(response.code())
61+
.body(bodyStr)
62+
.settings(serverSettings)
63+
.build();
6864
} catch (Exception e) {
69-
return SettingsResponse.builder().success(false).exception(e).message(e.getMessage()).build();
65+
throw new SettingsClientException(e);
7066
}
7167
}
7268
}

‎src/main/java/com/exceptionless/exceptionlessclient/settings/SettingsManager.java

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.exceptionless.exceptionlessclient.settings;
22

3+
import com.exceptionless.exceptionlessclient.exceptions.SettingsClientException;
34
import com.exceptionless.exceptionlessclient.models.storage.StorageItem;
45
import com.exceptionless.exceptionlessclient.models.submission.SettingsResponse;
56
import com.exceptionless.exceptionlessclient.storage.StorageProviderIF;
@@ -66,20 +67,39 @@ public void updateSettings() {
6667

6768
try {
6869
long currentVersion = getVersion();
69-
LOG.info(String.format("Checking for updated settings from: v%s", currentVersion));
70+
LOG.info(String.format("Checking for updated settings from v%s", currentVersion));
7071

7172
SettingsResponse response = settingsClient.getSettings(currentVersion);
72-
if (!response.isSuccess()) {
73-
LOG.warn(String.format("Unable to update settings: %s", response.getMessage()));
73+
if (shouldNotUpdate(response)) {
7474
return;
7575
}
76+
7677
ServerSettings prevValue = getSavedServerSettings();
7778
storageProvider.getSettings().save(response.getSettings());
7879
propertyChangeSupport.firePropertyChange("settings", prevValue, response.getSettings());
80+
} catch (SettingsClientException e) {
81+
LOG.error(String.format("Error retrieving settings for v%s", getVersion()), e);
7982
} finally {
8083
synchronized (this) {
8184
updatingSettings = false;
8285
}
8386
}
8487
}
88+
89+
private boolean shouldNotUpdate(SettingsResponse response) {
90+
if (response.isNotModified()) {
91+
LOG.trace("No need to update, settings are not modified");
92+
return true;
93+
}
94+
if (!response.isSuccess()) {
95+
LOG.warn(String.format("Unable to update settings: %s", response.getBody()));
96+
return true;
97+
}
98+
if (response.getSettings() == null) {
99+
LOG.warn("Not settings returned by server!");
100+
return true;
101+
}
102+
103+
return false;
104+
}
85105
}

‎src/main/java/com/exceptionless/exceptionlessclient/submission/DefaultSubmissionClient.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,10 @@ private SubmissionResponse postSubmission(String url, Object data) {
8080
updateSettingsFromHeaders(response.headers());
8181
}
8282

83+
ResponseBody body = response.body();
8384
return SubmissionResponse.builder()
8485
.code(response.code())
85-
.body(Utils.addCodeToResponseBodyStr(response))
86+
.body(body == null ? "" : body.string())
8687
.build();
8788
} catch (Exception e) {
8889
throw new SubmissionClientException(e);

‎src/main/java/com/exceptionless/exceptionlessclient/utils/Utils.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,9 @@
33
import com.fasterxml.jackson.databind.ObjectMapper;
44
import com.fasterxml.jackson.databind.SerializationFeature;
55
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
6-
import okhttp3.Response;
7-
import okhttp3.ResponseBody;
86
import org.slf4j.Logger;
97
import org.slf4j.LoggerFactory;
108

11-
import java.io.IOException;
129
import java.net.URI;
1310
import java.net.http.HttpRequest;
1411
import java.util.*;
@@ -90,13 +87,4 @@ public static boolean match(String value, String pattern) {
9087
}
9188
return false;
9289
}
93-
94-
public static String addCodeToResponseBodyStr(Response response) throws IOException {
95-
ResponseBody body = response.body();
96-
if (body == null) {
97-
return String.format("Code: %s", response.code());
98-
}
99-
100-
return String.format("Code: %s, Body: %s", response.code(), body.string());
101-
}
10290
}

‎src/test/java/com/exceptionless/exceptionlessclient/ExceptionlessClientTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public void setup() {
5555
public void itCanSetupASettingsChangeTimer() throws InterruptedException {
5656
doReturn(settingsStorage).when(storageProvider).getSettings();
5757
settingsStorage.save(ServerSettings.builder().version(3L).build());
58-
doReturn(SettingsResponse.builder().message("test-message").success(false).build())
58+
doReturn(SettingsResponse.builder().body("test-message").code(400).build())
5959
.when(settingsClient)
6060
.getSettings(anyLong());
6161

‎src/test/java/com/exceptionless/exceptionlessclient/settings/DefaultSettingsClientTest.java

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

33
import com.exceptionless.exceptionlessclient.TestFixtures;
44
import com.exceptionless.exceptionlessclient.configuration.Configuration;
5+
import com.exceptionless.exceptionlessclient.exceptions.SettingsClientException;
56
import com.exceptionless.exceptionlessclient.models.submission.SettingsResponse;
67
import okhttp3.*;
78
import org.junit.jupiter.api.BeforeEach;
@@ -14,6 +15,7 @@
1415
import java.util.Map;
1516

1617
import static org.assertj.core.api.Assertions.assertThat;
18+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1719
import static org.mockito.ArgumentMatchers.any;
1820
import static org.mockito.ArgumentMatchers.argThat;
1921
import static org.mockito.Mockito.doReturn;
@@ -74,49 +76,38 @@ public void itCanHandleASuccessfulResponse() throws IOException {
7476

7577
SettingsResponse response = settingsClient.getSettings(1);
7678

77-
assertThat(response.getMessage()).isNull();
78-
assertThat(response.getException()).isNull();
79-
assertThat(response.isSuccess()).isTrue();
79+
assertThat(response.getBody())
80+
.isEqualTo(
81+
"{\n"
82+
+ "\t\"version\":1,\n"
83+
+ "\t\"settings\":{\n"
84+
+ "\t\t\"key\":\"value\"\n"
85+
+ "\t}\n"
86+
+ "}");
87+
assertThat(response.getCode()).isEqualTo(200);
8088
assertThat(response.getSettings())
8189
.isEqualTo(ServerSettings.builder().version(1L).settings(Map.of("key", "value")).build());
8290
}
8391

84-
@Test
85-
public void itCanHandleAUnsuccessfulResponse() throws IOException {
86-
doReturn(responseBuilder.code(400).build()).when(call).execute();
87-
doReturn(call).when(httpClient).newCall(any());
88-
89-
SettingsResponse response = settingsClient.getSettings(1);
90-
91-
assertThat(response.getMessage()).isEqualTo("Code: 400, Body: test-body");
92-
assertThat(response.getException()).isNull();
93-
assertThat(response.isSuccess()).isFalse();
94-
assertThat(response.getSettings()).isNull();
95-
}
96-
9792
@Test
9893
public void itCanHandleNullSettingsReturnedByTheServer() throws IOException {
9994
doReturn(responseBuilder.body(null).build()).when(call).execute();
10095
doReturn(call).when(httpClient).newCall(any());
10196

10297
SettingsResponse response = settingsClient.getSettings(1);
10398

104-
assertThat(response.getMessage()).isEqualTo("No settings returned by server!");
105-
assertThat(response.getException()).isNull();
106-
assertThat(response.isSuccess()).isFalse();
99+
assertThat(response.getBody()).isEqualTo("");
100+
assertThat(response.getCode()).isEqualTo(200);
107101
assertThat(response.getSettings()).isNull();
108102
}
109103

110104
@Test
111-
public void itCanHandleAnyException() {
105+
public void itCanMapAnyExceptionToSettingsClientException() {
112106
RuntimeException e = new RuntimeException("test");
113107
doThrow(e).when(httpClient).newCall(any());
114108

115-
SettingsResponse response = settingsClient.getSettings(1);
116-
117-
assertThat(response.getMessage()).isEqualTo("test");
118-
assertThat(response.getException()).isEqualTo(e);
119-
assertThat(response.isSuccess()).isFalse();
120-
assertThat(response.getSettings()).isNull();
109+
assertThatThrownBy(() -> settingsClient.getSettings(1))
110+
.isInstanceOf(SettingsClientException.class)
111+
.hasMessage("java.lang.RuntimeException: test");
121112
}
122113
}

‎src/test/java/com/exceptionless/exceptionlessclient/settings/SettingsManagerTest.java

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.exceptionless.exceptionlessclient.settings;
22

3+
import com.exceptionless.exceptionlessclient.exceptions.SettingsClientException;
34
import com.exceptionless.exceptionlessclient.models.submission.SettingsResponse;
45
import com.exceptionless.exceptionlessclient.storage.InMemoryStorage;
56
import com.exceptionless.exceptionlessclient.storage.InMemoryStorageProvider;
@@ -87,7 +88,7 @@ public void itCanUpdateSettingsIfCheckedVersionIsGreaterThanCurrentVersion() {
8788

8889
ServerSettings newSettingsFromServer =
8990
ServerSettings.builder().version(4L).settings(Map.of("new-key", "new-value")).build();
90-
doReturn(SettingsResponse.builder().success(true).settings(newSettingsFromServer).build())
91+
doReturn(SettingsResponse.builder().code(200).settings(newSettingsFromServer).build())
9192
.when(settingsClient)
9293
.getSettings(3);
9394

@@ -104,10 +105,7 @@ public void itWillLetOnlyOneThreadUpdateSettingsAtATime()
104105
doAnswer(
105106
invocation -> {
106107
Thread.sleep(1000);
107-
return SettingsResponse.builder()
108-
.success(true)
109-
.settings(newSettingsFromServer)
110-
.build();
108+
return SettingsResponse.builder().code(200).settings(newSettingsFromServer).build();
111109
})
112110
.when(settingsClient)
113111
.getSettings(anyLong());
@@ -121,8 +119,8 @@ public void itWillLetOnlyOneThreadUpdateSettingsAtATime()
121119
}
122120

123121
@Test
124-
public void itWillNotSaveSettingsIfNotAbleToGetSettingsFromServer() {
125-
doReturn(SettingsResponse.builder().success(false).message("test-message").build())
122+
public void itWillNotSaveSettingsIfSettingsAreNotModified() {
123+
doReturn(SettingsResponse.builder().code(304).body("test-message").build())
126124
.when(settingsClient)
127125
.getSettings(anyLong());
128126

@@ -131,11 +129,40 @@ public void itWillNotSaveSettingsIfNotAbleToGetSettingsFromServer() {
131129
assertThat(storage.peek()).isNull();
132130
}
133131

132+
@Test
133+
public void itWillNotSaveSettingsIfUnableToGetSettingsFromServer() {
134+
doReturn(SettingsResponse.builder().code(400).body("test-message").build())
135+
.when(settingsClient)
136+
.getSettings(anyLong());
137+
138+
settingsManager.updateSettings();
139+
140+
assertThat(storage.peek()).isNull();
141+
}
142+
143+
@Test
144+
public void itWillNotSaveSettingsIfNoSettingsReturnedByTheServer() {
145+
doReturn(SettingsResponse.builder().code(200).body("test-message").build())
146+
.when(settingsClient)
147+
.getSettings(anyLong());
148+
149+
settingsManager.updateSettings();
150+
151+
assertThat(storage.peek()).isNull();
152+
}
153+
154+
@Test
155+
public void itCanHandleSettingsClientException() {
156+
doThrow(new SettingsClientException("test")).when(settingsClient).getSettings(anyLong());
157+
158+
settingsManager.updateSettings();
159+
}
160+
134161
@Test
135162
public void itCanSuccessfullyUpdateSettingsAndNotifyListeners() {
136163
ServerSettings newSettingsFromServer =
137164
ServerSettings.builder().version(4L).settings(Map.of("new-key", "new-value")).build();
138-
doReturn(SettingsResponse.builder().success(true).settings(newSettingsFromServer).build())
165+
doReturn(SettingsResponse.builder().code(200).settings(newSettingsFromServer).build())
139166
.when(settingsClient)
140167
.getSettings(anyLong());
141168

0 commit comments

Comments
 (0)