Skip to content

Commit 555aab5

Browse files
sanzmaurosanzmauro
committed
feedback applied
1 parent 40c124d commit 555aab5

File tree

14 files changed

+39
-51
lines changed

14 files changed

+39
-51
lines changed

‎client/src/main/java/io/split/client/SplitFactoryImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ private static CloseableHttpClient buildHttpClient(String apiToken, SplitClientC
102102

103103
private static CloseableHttpClient buildSSEdHttpClient(SplitClientConfig config) {
104104
RequestConfig requestConfig = RequestConfig.custom()
105-
.setConnectTimeout(Timeout.of(SSE_CONNECT_TIMEOUT, TimeUnit.MILLISECONDS))
105+
.setConnectTimeout(Timeout.ofMilliseconds(SSE_CONNECT_TIMEOUT))
106106
.build();
107107

108108
SSLConnectionSocketFactory sslSocketFactory = SSLConnectionSocketFactoryBuilder.create()

‎client/src/main/java/io/split/engine/common/PushManagerImp.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public synchronized void stop() {
101101
_eventSourceClient.stop();
102102
stopWorkers();
103103
if (_nextTokenRefreshTask != null) {
104-
_log.warn("Cancel nextTokenRefreshTask");
104+
_log.debug("Cancel nextTokenRefreshTask");
105105
_nextTokenRefreshTask.cancel(false);
106106
}
107107
}
@@ -120,7 +120,7 @@ private boolean startSse(String token, String channels) {
120120
_log.debug("SSE Handler starting ...");
121121
return _eventSourceClient.start(channels, token);
122122
} catch (Exception e) {
123-
_log.error("Exception in SSE Handler start: " + e.getMessage());
123+
_log.debug("Exception in SSE Handler start: " + e.getMessage());
124124
return false;
125125
}
126126
}

‎client/src/main/java/io/split/engine/sse/AuthApiClientImp.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,14 @@ public AuthenticationResponse Authenticate() {
4646
return getSuccessResponse(jsonContent);
4747
}
4848

49-
_log.warn(String.format("Problem to connect to : %s. Response status: %s", _target, statusCode));
49+
_log.debug(String.format("Problem to connect to : %s. Response status: %s", _target, statusCode));
5050
if (statusCode >= HttpStatus.SC_BAD_REQUEST && statusCode < HttpStatus.SC_INTERNAL_SERVER_ERROR) {
5151
return new AuthenticationResponse(false,false);
5252
}
5353

5454
return new AuthenticationResponse(false,true);
5555
} catch (Exception ex) {
56-
_log.error(ex.getMessage());
56+
_log.debug(ex.getMessage());
5757
return new AuthenticationResponse(false,true);
5858
}
5959
}

‎client/src/main/java/io/split/engine/sse/EventSourceClientImp.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,15 @@ public boolean start(String channelList, String token) {
6565
try {
6666
return _sseClient.open(buildUri(channelList, token));
6767
} catch (URISyntaxException e) {
68-
_log.error("Error building Streaming URI: " + e.getMessage());
68+
_log.debug("Error building Streaming URI: " + e.getMessage());
6969
return false;
7070
}
7171
}
7272

7373
@Override
7474
public void stop() {
7575
if (!_sseClient.isOpen()) {
76-
_log.warn("Event Source Client is closed.");
76+
_log.debug("Event Source Client is closed.");
7777
return;
7878
}
7979
_sseClient.close();

‎client/src/main/java/io/split/engine/sse/client/SSEClient.java‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ private enum ConnectionState {
4343
private final static String SOCKET_CLOSED_MESSAGE = "Socket closed";
4444
private final static String KEEP_ALIVE_PAYLOAD = ":keepalive\n";
4545
private final static long CONNECT_TIMEOUT = 30000;
46-
private final static long SOCKET_TIMEOUT = 70000;
4746
private static final Logger _log = LoggerFactory.getLogger(SSEClient.class);
4847

4948
private final CloseableHttpClient _client;
@@ -62,7 +61,7 @@ public SSEClient(Function<RawEvent, Void> eventCallback,
6261

6362
public synchronized boolean open(URI uri) {
6463
if (isOpen()) {
65-
_log.warn("SSEClient already open.");
64+
_log.debug("SSEClient already open.");
6665
return false;
6766
}
6867

@@ -78,7 +77,7 @@ public synchronized boolean open(URI uri) {
7877
};
7978
} catch (InterruptedException e) {
8079
Thread.currentThread().interrupt();
81-
_log.warn(e.getMessage());
80+
_log.debug(e.getMessage());
8281
return false;
8382
}
8483
return isOpen();
@@ -94,7 +93,7 @@ public synchronized void close() {
9493
try {
9594
_ongoingResponse.get().close();
9695
} catch (IOException e) {
97-
_log.warn(String.format("Error closing SSEClient: %s", e.getMessage()));
96+
_log.debug(String.format("Error closing SSEClient: %s", e.getMessage()));
9897
}
9998
}
10099
}
@@ -116,7 +115,7 @@ private void connectAndLoop(URI uri, CountDownLatch signal) {
116115
try {
117116
handleMessage(readMessageAsString(reader));
118117
} catch (SocketException exc) {
119-
_log.warn(exc.getMessage());
118+
_log.debug(exc.getMessage());
120119
if (SOCKET_CLOSED_MESSAGE.equals(exc.getMessage())) { // Connection closed by us
121120
_statusCallback.apply(StatusMessage.FORCED_STOP);
122121
return;
@@ -125,23 +124,23 @@ private void connectAndLoop(URI uri, CountDownLatch signal) {
125124
_statusCallback.apply(StatusMessage.RETRYABLE_ERROR);
126125
return;
127126
} catch (IOException exc) { // Other type of connection error
128-
_log.warn(exc.getMessage());
127+
_log.debug(exc.getMessage());
129128
_statusCallback.apply(StatusMessage.RETRYABLE_ERROR);
130129
return;
131130
}
132131
}
133132
} catch (Exception e) { // Any other error non related to the connection disables streaming altogether
134-
_log.error(e.getMessage(), e);
133+
_log.warn(e.getMessage(), e);
135134
_statusCallback.apply(StatusMessage.NONRETRYABLE_ERROR);
136135
} finally {
137136
try {
138137
_ongoingResponse.get().close();
139138
} catch (IOException e) {
140-
_log.warn(e.getMessage());
139+
_log.debug(e.getMessage());
141140
}
142141

143142
_state.set(ConnectionState.CLOSED);
144-
_log.warn("SSEClient finished.");
143+
_log.debug("SSEClient finished.");
145144
}
146145
}
147146

@@ -156,7 +155,7 @@ private boolean establishConnection(URI uri, CountDownLatch signal) {
156155
_state.set(ConnectionState.OPEN);
157156
_statusCallback.apply(StatusMessage.CONNECTED);
158157
} catch (IOException exc) {
159-
_log.warn(String.format("Error establishConnection: %s", exc));
158+
_log.debug(String.format("Error establishConnection: %s", exc));
160159
return false;
161160
} finally {
162161
signal.countDown();

‎client/src/main/java/io/split/engine/sse/workers/SplitsWorkerImp.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package io.split.engine.sse.workers;
22

33
import io.split.engine.common.Synchronizer;
4-
import io.split.engine.experiments.SplitFetcher;
54

65
import static com.google.common.base.Preconditions.checkNotNull;
76

@@ -19,7 +18,7 @@ public void killSplit(long changeNumber, String splitName, String defaultTreatme
1918
_synchronizer.localKillSplit(splitName, defaultTreatment, changeNumber);
2019
_log.debug(String.format("Kill split: %s, changeNumber: %s, defaultTreatment: %s", splitName, changeNumber, defaultTreatment));
2120
} catch (Exception ex) {
22-
_log.error(String.format("Exception on SplitWorker killSplit: %s", ex.getMessage()));
21+
_log.warn(String.format("Exception on SplitWorker killSplit: %s", ex.getMessage()));
2322
}
2423
}
2524

‎client/src/main/java/io/split/engine/sse/workers/Worker.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public void start() {
2727
_thread = new Thread( this);
2828
_thread.start();
2929
} else {
30-
_log.warn(String.format("%s Worker already running.", _workerName));
30+
_log.debug(String.format("%s Worker already running.", _workerName));
3131
return;
3232
}
3333
}
@@ -37,7 +37,7 @@ public void stop() {
3737
_thread.interrupt();
3838
_log.debug(String.format("%s Worked stopped.", _workerName));
3939
} else {
40-
_log.warn(String.format("%s Worker not running.", _workerName));
40+
_log.debug(String.format("%s Worker not running.", _workerName));
4141
}
4242
}
4343

@@ -48,14 +48,14 @@ public void addToQueue(T element) {
4848
}
4949
try {
5050
if (!_running.get()) {
51-
_log.warn(String.format("%s Worker not running. Can't add items.", _workerName));
51+
_log.debug(String.format("%s Worker not running. Can't add items.", _workerName));
5252
return;
5353
}
5454

5555
_queue.add(element);
5656
_log.debug(String.format("Added to %s queue: %s", _workerName, element.toString()));
5757
} catch (Exception ex) {
58-
_log.error(String.format("Exception on %s Worker addToQueue: %s", _workerName, ex.getMessage()));
58+
_log.debug(String.format("Exception on %s Worker addToQueue: %s", _workerName, ex.getMessage()));
5959
}
6060
}
6161

‎client/src/test/java/io/split/TestHelper.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@
1111
import java.lang.reflect.Method;
1212

1313
public class TestHelper {
14-
public CloseableHttpClient mockHttpClient(String jsonName, int httpStatus) throws IOException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
14+
public static CloseableHttpClient mockHttpClient(String jsonName, int httpStatus) throws IOException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
1515
HttpEntity entityMock = Mockito.mock(HttpEntity.class);
16-
Mockito.when(entityMock.getContent()).thenReturn(getClass().getClassLoader().getResourceAsStream(jsonName));
16+
Mockito.when(entityMock.getContent()).thenReturn(TestHelper.class.getClassLoader().getResourceAsStream(jsonName));
1717

1818
ClassicHttpResponse httpResponseMock = Mockito.mock(ClassicHttpResponse.class);
1919
Mockito.when(httpResponseMock.getEntity()).thenReturn(entityMock);
@@ -25,7 +25,7 @@ public CloseableHttpClient mockHttpClient(String jsonName, int httpStatus) throw
2525
return httpClientMock;
2626
}
2727

28-
private CloseableHttpResponse classicResponseToCloseableMock(ClassicHttpResponse mocked) throws InvocationTargetException, IllegalAccessException, NoSuchMethodException {
28+
private static CloseableHttpResponse classicResponseToCloseableMock(ClassicHttpResponse mocked) throws InvocationTargetException, IllegalAccessException, NoSuchMethodException {
2929
Method adaptMethod = CloseableHttpResponse.class.getDeclaredMethod("adapt", ClassicHttpResponse.class);
3030
adaptMethod.setAccessible(true);
3131
return (CloseableHttpResponse) adaptMethod.invoke(null, mocked);

‎client/src/test/java/io/split/client/HttpSegmentChangeFetcherTest.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
import java.net.URISyntaxException;
1717

1818
public class HttpSegmentChangeFetcherTest {
19-
private final TestHelper _testHelper = new TestHelper();
20-
2119
@Test
2220
public void testDefaultURL() throws URISyntaxException {
2321
URI rootTarget = URI.create("https://api.split.io");
@@ -58,7 +56,7 @@ public void testCustomURLAppendingPathNoBackslash() throws URISyntaxException {
5856
public void testFetcherWithSpecialCharacters() throws URISyntaxException, IOException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
5957
URI rootTarget = URI.create("https://api.split.io/api/segmentChanges");
6058

61-
CloseableHttpClient httpClientMock = _testHelper.mockHttpClient("segment-change-special-chatacters.json", HttpStatus.SC_OK);
59+
CloseableHttpClient httpClientMock = TestHelper.mockHttpClient("segment-change-special-chatacters.json", HttpStatus.SC_OK);
6260

6361
Metrics.NoopMetrics metrics = new Metrics.NoopMetrics();
6462
HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(httpClientMock, rootTarget, metrics);

‎client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
import java.util.Map;
1919

2020
public class HttpSplitChangeFetcherTest {
21-
private final TestHelper _testHelper = new TestHelper();
22-
2321
@Test
2422
public void testDefaultURL() throws URISyntaxException {
2523
URI rootTarget = URI.create("https://api.split.io");
@@ -60,7 +58,7 @@ public void testCustomURLAppendingPathNoBackslash() throws URISyntaxException {
6058
public void testFetcherWithSpecialCharacters() throws URISyntaxException, InvocationTargetException, NoSuchMethodException, IllegalAccessException, IOException {
6159
URI rootTarget = URI.create("https://api.split.io");
6260

63-
CloseableHttpClient httpClientMock = _testHelper.mockHttpClient("split-change-special-characters.json", HttpStatus.SC_OK);
61+
CloseableHttpClient httpClientMock = TestHelper.mockHttpClient("split-change-special-characters.json", HttpStatus.SC_OK);
6462

6563
Metrics.NoopMetrics metrics = new Metrics.NoopMetrics();
6664
HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(httpClientMock, rootTarget, metrics);

0 commit comments

Comments
 (0)