Skip to content

Commit

Permalink
Pull request #239: RUM-2317 Adding SessionIdentifier
Browse files Browse the repository at this point in the history
Merge in OP/openkit-java from feature/RUM-2317-openkit-java-implement-si-parameter to main

* commit 'e35054892eca884ba905da66f06bf1fa15f3b41f':
  RUM-2317 Changed to anyInt()
  RUM-2317 Removing lines and imports
  RUM-2317 Removing sessionIdentifier call
  RUM-2317 Changing approach to beacon request only
  RUM-2317 Adding SessionIdentifier

GitOrigin-RevId: 4157c7f70438dbc16f1026a278080c6efd432577
  • Loading branch information
TheHighriser authored and openkitdt committed Nov 10, 2022
1 parent 62fb422 commit 0b85122
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class HTTPClientConfiguration {
private final SSLTrustManager sslTrustManager;
private final HttpRequestInterceptor httpRequestInterceptor;
private final HttpResponseInterceptor httpResponseInterceptor;
private final long deviceID;

private HTTPClientConfiguration(Builder builder) {
this.baseURL = builder.baseURL;
Expand All @@ -39,6 +40,7 @@ private HTTPClientConfiguration(Builder builder) {
this.sslTrustManager = builder.sslTrustManager;
this.httpRequestInterceptor = builder.httpRequestInterceptor;
this.httpResponseInterceptor = builder.httpResponseInterceptor;
this.deviceID = builder.deviceID;
}

/**
Expand All @@ -65,7 +67,8 @@ public static Builder modifyWith(OpenKitConfiguration openKitConfig) {
.withSSLTrustManager(openKitConfig.getSSLTrustManager())
.withServerID(openKitConfig.getDefaultServerID())
.withHttpRequestInterceptor(openKitConfig.getHttpRequestInterceptor())
.withHttpResponseInterceptor(openKitConfig.getHttpResponseInterceptor());
.withHttpResponseInterceptor(openKitConfig.getHttpResponseInterceptor())
.withDeviceID(openKitConfig.getDeviceID());
}

/**
Expand All @@ -81,7 +84,8 @@ public static Builder modifyWith(HTTPClientConfiguration httpClientConfig) {
.withSSLTrustManager(httpClientConfig.getSSLTrustManager())
.withServerID(httpClientConfig.getServerID())
.withHttpRequestInterceptor(httpClientConfig.getHttpRequestInterceptor())
.withHttpResponseInterceptor(httpClientConfig.getHttpResponseInterceptor());
.withHttpResponseInterceptor(httpClientConfig.getHttpResponseInterceptor())
.withDeviceID(httpClientConfig.getDeviceID());
}

/**
Expand Down Expand Up @@ -132,6 +136,11 @@ public HttpResponseInterceptor getHttpResponseInterceptor() {
return httpResponseInterceptor;
}

/**
* Returns the unique device identifier
*/
public long getDeviceID() { return deviceID; }

/**
* Builder class for building {@link HTTPClientConfiguration}.
*/
Expand All @@ -143,6 +152,7 @@ public static final class Builder {
private SSLTrustManager sslTrustManager = null;
private HttpRequestInterceptor httpRequestInterceptor = null;
private HttpResponseInterceptor httpResponseInterceptor = null;
private long deviceID;


public Builder withBaseURL(String baseURL) {
Expand Down Expand Up @@ -175,6 +185,11 @@ public Builder withHttpResponseInterceptor(HttpResponseInterceptor httpResponseI
return this;
}

public Builder withDeviceID(long deviceID) {
this.deviceID = deviceID;
return this;
}

public HTTPClientConfiguration build() {
return new HTTPClientConfiguration(this);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/dynatrace/openkit/protocol/Beacon.java
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ public StatusResponse send(HTTPClientProvider provider, AdditionalQueryParameter
}

// send the request
response = httpClient.sendBeaconRequest(clientIPAddress, encodedBeacon, additionalParameters);
response = httpClient.sendBeaconRequest(clientIPAddress, encodedBeacon, additionalParameters, getSessionNumber());
if (response == null || response.isErroneousResponse()) {
// error happened - but don't know what exactly
// reset the previously retrieved chunk (restore it in internal cache) & retry another time
Expand Down
15 changes: 14 additions & 1 deletion src/main/java/com/dynatrace/openkit/protocol/HTTPClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public String getRequestName() {
private static final String QUERY_KEY_RESPONSE_TYPE = "resp";
private static final String QUERY_KEY_CONFIG_TIMESTAMP = "cts";
private static final String QUERY_KEY_NEW_SESSION = "ns";
private static final String QUERY_KEY_SESSION_IDENTIFIER = "si";

// additional reserved characters for URL encoding
private static final char[] QUERY_RESERVED_CHARACTERS = {'_'};
Expand All @@ -100,6 +101,7 @@ public String getRequestName() {
private final String newSessionURL;

private final int serverID;
private final long deviceID;

private final SSLTrustManager sslTrustManager;

Expand All @@ -118,6 +120,7 @@ public HTTPClient(Logger logger, HTTPClientConfiguration configuration) {
sslTrustManager = configuration.getSSLTrustManager();
httpRequestInterceptor = configuration.getHttpRequestInterceptor();
httpResponseInterceptor = configuration.getHttpResponseInterceptor();
deviceID = configuration.getDeviceID();
}

// *** public methods ***
Expand All @@ -143,8 +146,11 @@ public StatusResponse sendNewSessionRequest(AdditionalQueryParameters additional
public StatusResponse sendBeaconRequest(
String clientIPAddress,
byte[] data,
AdditionalQueryParameters additionalParameters) {
AdditionalQueryParameters additionalParameters,
int sessionNumber) {
String url = appendAdditionalQueryParameters(monitorURL, additionalParameters);
url = appendSessionIdentifierParameter(url, String.valueOf(sessionNumber), String.valueOf(deviceID));

StatusResponse response = sendRequest(RequestType.BEACON, url, clientIPAddress, data, "POST");
return response == null
? StatusResponse.createErrorResponse(logger, Integer.MAX_VALUE)
Expand Down Expand Up @@ -371,6 +377,13 @@ private static String appendAdditionalQueryParameters(String baseUrl, Additional
return builder.toString();
}

private static String appendSessionIdentifierParameter(String baseUrl, String sessionNumber, String deviceID) {
StringBuilder builder = new StringBuilder(baseUrl);
appendQueryParam(builder, QUERY_KEY_SESSION_IDENTIFIER, deviceID + "_" + sessionNumber);

return builder.toString();
}

// helper method for appending query parameters
private static void appendQueryParam(StringBuilder urlBuilder, String key, String value) {
urlBuilder.append('&');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.sameInstance;
import static org.junit.Assert.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.isA;
import static org.mockito.Matchers.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
Expand Down Expand Up @@ -80,7 +79,7 @@ public void setUp() {
200,
Collections.<String, List<String>>emptyMap()
);
when(httpClient.sendBeaconRequest(isA(String.class), any(byte[].class), any(AdditionalQueryParameters.class)))
when(httpClient.sendBeaconRequest(isA(String.class), any(byte[].class), any(AdditionalQueryParameters.class), anyInt()))
.thenReturn(statusResponse);

httpClientProvider = mock(HTTPClientProvider.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,22 @@ public void instanceFromOpenKitConfigTakesOverHttpResponseInterceptor() {
assertThat(obtained, is(equalTo(httpResponseInterceptor)));
}

@Test
public void instanceFromOpenKitConfigTakesOverDeviceId() {
// given
OpenKitConfiguration openKitConfig = mock(OpenKitConfiguration.class);
when(openKitConfig.getDeviceID()).thenReturn(42l);

HTTPClientConfiguration target = HTTPClientConfiguration.from(openKitConfig);

// when
long obtained = target.getDeviceID();

// then
verify(openKitConfig, times(1)).getDeviceID();
assertThat(obtained, is(equalTo(42l)));
}

@Test
public void builderFromHttpClientConfigTakesOverBaseUrl() {
// given
Expand Down Expand Up @@ -224,6 +240,23 @@ public void builderFromHttpClientConfigTakesHttpResponseInterceptor() {
assertThat(target.getHttpResponseInterceptor(), is(sameInstance(httpResponseInterceptor)));
}

@Test
public void builderFromHttpClientConfigTakesDeviceId() {
// given
HTTPClientConfiguration httpConfig = mock(HTTPClientConfiguration.class);
when(httpConfig.getDeviceID()).thenReturn(42l);

// when
HTTPClientConfiguration target = HTTPClientConfiguration.modifyWith(httpConfig).build();

// when
long obtained = target.getDeviceID();

// then
verify(httpConfig, times(1)).getDeviceID();
assertThat(obtained, is(equalTo(42l)));
}

@Test
public void emptyBuilderCreatesEmptyInstance() {
// given
Expand All @@ -237,6 +270,7 @@ public void emptyBuilderCreatesEmptyInstance() {
assertThat(obtained.getApplicationID(), is(nullValue()));
assertThat(obtained.getSSLTrustManager(), is(nullValue()));
assertThat(obtained.getServerID(), is(equalTo(-1)));
assertThat(obtained.getDeviceID(), is(equalTo(0l)));
}

@Test
Expand Down Expand Up @@ -318,4 +352,17 @@ public void builderWithHttpResponseInterceptorPropagatesToInstance() {
// then
assertThat(obtained.getHttpResponseInterceptor(), is(sameInstance(httpResponseInterceptor)));
}

@Test
public void builderWithDeviceIdPropagatesToInstance() {
// given
HTTPClientConfiguration.Builder target = new HTTPClientConfiguration.Builder()
.withDeviceID(42l);

// when
HTTPClientConfiguration obtained = target.build();

// then
assertThat(obtained.getDeviceID(), is(equalTo(42l)));
}
}
16 changes: 8 additions & 8 deletions src/test/java/com/dynatrace/openkit/protocol/BeaconTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public void testCreateInstanceWithInvalidIpAddress() {
// then
ArgumentCaptor<String> ipCaptor = ArgumentCaptor.forClass(String.class);
verify(httpClient, times(1))
.sendBeaconRequest(ipCaptor.capture(), any(byte[].class), eq(mockAdditionalParameters));
.sendBeaconRequest(ipCaptor.capture(), any(byte[].class), eq(mockAdditionalParameters), anyInt());

String capturedIp = ipCaptor.getValue();
assertThat(capturedIp, is(nullValue()));
Expand Down Expand Up @@ -248,7 +248,7 @@ public void testCreateInstanceWithNullIpAddress() {
// then
ArgumentCaptor<String> ipCaptor = ArgumentCaptor.forClass(String.class);
verify(httpClient, times(1))
.sendBeaconRequest(ipCaptor.capture(), any(byte[].class), eq(mockAdditionalParameters));
.sendBeaconRequest(ipCaptor.capture(), any(byte[].class), eq(mockAdditionalParameters), anyInt());

String capturedIp = ipCaptor.getValue();
assertThat(capturedIp, is(nullValue()));
Expand Down Expand Up @@ -3726,7 +3726,7 @@ public void sendValidData() {
responseCode,
Collections.<String, List<String>>emptyMap()
);
when(httpClient.sendBeaconRequest(any(String.class), any(byte[].class), any(AdditionalQueryParameters.class)))
when(httpClient.sendBeaconRequest(any(String.class), any(byte[].class), any(AdditionalQueryParameters.class), anyInt()))
.thenReturn(successResponse);
when(httpClientProvider.createClient(any(HTTPClientConfiguration.class))).thenReturn(httpClient);

Expand All @@ -3737,7 +3737,7 @@ public void sendValidData() {
// then
assertThat(response, notNullValue());
assertThat(response.getResponseCode(), is(responseCode));
verify(httpClient, times(1)).sendBeaconRequest(eq(ipAddress), any(byte[].class), eq(mockAdditionalParameters));
verify(httpClient, times(1)).sendBeaconRequest(eq(ipAddress), any(byte[].class), eq(mockAdditionalParameters), anyInt());
}

@Test
Expand All @@ -3753,7 +3753,7 @@ public void sendDataAndFakeErrorResponse() {
HTTPClient httpClient = mock(HTTPClient.class);
int responseCode = 418;
StatusResponse errorResponse = StatusResponse.createErrorResponse(mockLogger, responseCode);
when(httpClient.sendBeaconRequest(any(String.class), any(byte[].class), any(AdditionalQueryParameters.class)))
when(httpClient.sendBeaconRequest(any(String.class), any(byte[].class), any(AdditionalQueryParameters.class), anyInt()))
.thenReturn(errorResponse);
when(httpClientProvider.createClient(any(HTTPClientConfiguration.class))).thenReturn(httpClient);

Expand All @@ -3764,7 +3764,7 @@ public void sendDataAndFakeErrorResponse() {
// then
assertThat(response, notNullValue());
assertThat(response.getResponseCode(), is(responseCode));
verify(httpClient, times(1)).sendBeaconRequest(eq(ipAddress), any(byte[].class), eq(mockAdditionalParameters));
verify(httpClient, times(1)).sendBeaconRequest(eq(ipAddress), any(byte[].class), eq(mockAdditionalParameters), anyInt());
}

@Test
Expand Down Expand Up @@ -3831,7 +3831,7 @@ public void sendCanHandleMultipleChunks() {
responseCode,
Collections.<String, List<String>>emptyMap()
);
when(httpClient.sendBeaconRequest(any(String.class), any(byte[].class), any(AdditionalQueryParameters.class)))
when(httpClient.sendBeaconRequest(any(String.class), any(byte[].class), any(AdditionalQueryParameters.class), anyInt()))
.thenReturn(firstResponse, secondResponse);
when(httpClientProvider.createClient(any(HTTPClientConfiguration.class))).thenReturn(httpClient);

Expand All @@ -3844,7 +3844,7 @@ public void sendCanHandleMultipleChunks() {
assertThat(response, is(notNullValue()));
assertThat(response, is(sameInstance(secondResponse)));

verify(httpClient, times(2)).sendBeaconRequest(any(String.class), any(byte[].class), eq(mockAdditionalParameters));
verify(httpClient, times(2)).sendBeaconRequest(any(String.class), any(byte[].class), eq(mockAdditionalParameters), anyInt());

verify(mockBeaconCache, times(1)).prepareDataForSending(any(BeaconKey.class));
verify(mockBeaconCache, times(3)).hasDataForSending(any(BeaconKey.class));
Expand Down
31 changes: 28 additions & 3 deletions src/test/java/com/dynatrace/openkit/protocol/HTTPClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ public void sendBeaconRequestDoesNotReturnNull() throws UnsupportedEncodingExcep
doReturn(null).when(target).sendRequest(Mockito.any(RequestType.class), anyString(), anyString(), Mockito.any(byte[].class), anyString());

// when
StatusResponse obtained = target.sendBeaconRequest("127.0.0.1", "".getBytes(CHARSET), mockAdditionalParameters);
StatusResponse obtained = target.sendBeaconRequest("127.0.0.1", "".getBytes(CHARSET), mockAdditionalParameters, 1);

// then
assertThat(obtained, is(notNullValue()));
Expand All @@ -656,6 +656,29 @@ public void sendStatusRequestDoesNotAppendIfAdditionalQueryParametersAreNull() {
assertThat(urlCaptor.getValue(), is(expectedUrl.toString()));
}

@Test
public void sendStatusRequestDoesNotAppendSessionIdentifierIfNull() {
// given
long timestamp = 1234;
AdditionalQueryParameters additionalQueryParameters = mock(AdditionalQueryParameters.class);
when(additionalQueryParameters.getConfigurationTimestamp()).thenReturn(timestamp);

ArgumentCaptor<String> urlCaptor = ArgumentCaptor.forClass(String.class);

HTTPClient target = spy(new HTTPClient(logger, configuration));
doReturn(null).when(target).sendRequest(any(RequestType.class), anyString(), anyString(), any(byte[].class), anyString());

// when
target.sendStatusRequest(additionalQueryParameters);

// then
verify(target, times(1)).sendRequest(any(RequestType.class), urlCaptor.capture(), anyString(), any(byte[].class), anyString());

StringBuilder expectedUrl = initializeBaseUrl();
appendUrlParameter(expectedUrl, "cts", String.valueOf(timestamp));
assertThat(urlCaptor.getValue(), is(expectedUrl.toString()));
}

@Test
public void sendStatusRequestAppendsAdditionalQueryParameters() {
// given
Expand Down Expand Up @@ -731,12 +754,13 @@ public void sendBeaconRequestDoesNotAppendIfAdditionalQueryParametersAreNull() {
doReturn(null).when(target).sendRequest(any(RequestType.class), anyString(), anyString(), any(byte[].class), anyString());

// when
target.sendBeaconRequest(null, null, null);
target.sendBeaconRequest(null, null, null, 1);

// then
verify(target, times(1)).sendRequest(any(RequestType.class), urlCaptor.capture(), anyString(), any(byte[].class), anyString());

StringBuilder expectedUrl = initializeBaseUrl();
appendUrlParameter(expectedUrl, "si", "0%5F1");
assertThat(urlCaptor.getValue(), is(expectedUrl.toString()));
}

Expand All @@ -753,13 +777,14 @@ public void sendBeaconRequestAppendsAdditionalQueryParameters() {
doReturn(null).when(target).sendRequest(any(RequestType.class), anyString(), anyString(), any(byte[].class), anyString());

// when
target.sendBeaconRequest(null, null, additionalQueryParameters);
target.sendBeaconRequest(null, null, additionalQueryParameters, 1);

// then
verify(target, times(1)).sendRequest(any(RequestType.class), urlCaptor.capture(), anyString(), any(byte[].class), anyString());

StringBuilder expectedUrl = initializeBaseUrl();
appendUrlParameter(expectedUrl, "cts", String.valueOf(timestamp));
appendUrlParameter(expectedUrl, "si", "0%5F1");
assertThat(urlCaptor.getValue(), is(expectedUrl.toString()));
}

Expand Down

0 comments on commit 0b85122

Please sign in to comment.