Skip to content

Commit

Permalink
tests + request body caching preparations
Browse files Browse the repository at this point in the history
  • Loading branch information
Serhii Vydiuk committed Dec 17, 2024
1 parent 4cecca1 commit 8626bc4
Show file tree
Hide file tree
Showing 8 changed files with 194 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,40 @@ readme:
readmeApiKey: ${README_API_KEY}
userdata:
apiKey:
source: jsonBody
fieldName: /owl-creator/name
source: header
fieldName: X-User-Name
email:
source: jsonBody
fieldName: /owl-creator/contacts/email
source: header
fieldName: X-User-Email
label:
source: jsonBody
fieldName: owl-creator/label
source: header
fieldName: X-User-Id

#readme:
# readmeApiKey: ${README_API_KEY}
# userdata:
# apiKey:
# source: jwt
# fieldName: name
# source: jsonBody
# fieldName: /owl-creator/name
# email:
# source: jwt
# fieldName: aud
# source: jsonBody
# fieldName: /owl-creator/contacts/email
# label:
# source: jwt
# fieldName: user_id
# source: jsonBody
# fieldName: owl-creator/label

#readme:
# readmeApiKey: ${README_API_KEY}
# userdata:
# apiKey:
# source: header
# fieldName: X-User-Name
# source: jwt
# fieldName: name
# email:
# source: header
# fieldName: X-User-Email
# source: jwt
# fieldName: aud
# label:
# source: header
# fieldName: X-User-Id
# source: jwt
# fieldName: user_id



4 changes: 2 additions & 2 deletions packages/java/readme-metrics-spring-boot-starter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ Each field (`apiKey`, `email`, `label`) requires two sub-properties:
### Example Configuration (YAML)
```yaml
readme:
readmeApiKey: a11b33b2c44de78f7a
readmeApiKey: ${readmeApiKey}
userdata:
apiKey:
source: header
fieldName: X-User-Id
email:
source: jwtClaim
source: jwt
fieldName: aud
label:
source: jsonBody
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,15 @@
import jakarta.servlet.http.HttpServletResponse;
import lombok.AllArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.web.util.ContentCachingRequestWrapper;
import org.springframework.web.util.ContentCachingResponseWrapper;

import java.io.IOException;

import static org.springframework.http.HttpMethod.GET;
import static org.springframework.http.HttpMethod.OPTIONS;


/**
* A filter for collecting HTTP request and response metrics in environments using the
* jakarta.servlet API (e.g., Spring Boot 3).
*
* <p>This filter intercepts HTTP requests and responses, passing them to a {@code DataCollector}
* for processing. It enables applications to gather usage data,
* such as response codes, headers, and payloads.</p>
*
* <p>Problem Solved: Provides a unified mechanism for metric collection while maintaining
* compatibility with modern Servlet API versions.</p>
*/
@AllArgsConstructor
@Slf4j
public class DataCollectionFilter implements Filter {
Expand All @@ -31,21 +25,46 @@ public class DataCollectionFilter implements Filter {

private UserDataCollector<ServletDataPayloadAdapter> userDataCollector;


//TODO
// 1. Research possibility to collect metrics in a separate thread, as it may produce
// race condition on reading body data stream.
// 2. Problem to solve: if we collect a request/response after doFilter(r,r), it means
// the request dataStream will be red by customer's business logic and will not be available to us.
// On the other hand, if we collect a request before doFilter, the response data is not available yet.
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain) throws IOException, ServletException {
HttpServletRequest request = (HttpServletRequest) req;
HttpServletResponse response = (HttpServletResponse) resp;

try {
ServletDataPayloadAdapter payload =
new ServletDataPayloadAdapter((HttpServletRequest) request,
(HttpServletResponse) response);
UserData userData = userDataCollector.collect(payload);
if (request.getMethod().equalsIgnoreCase(OPTIONS.name())) {
chain.doFilter(req, resp);
} else if (request.getMethod().equalsIgnoreCase(GET.name())) {
ServletDataPayloadAdapter payload =
new ServletDataPayloadAdapter(request, response);

//TODO: Handle case if SDK user configured getting request user data from body, but GET req doesn't have it
UserData userData = userDataCollector.collect(payload);
//TODO: Validate user data. Collect request data only if user data is valid ?
requestDataCollector.collect(payload, userData);
chain.doFilter(req, resp);
} else {
ContentCachingRequestWrapper cacheableRequest =
new ContentCachingRequestWrapper(request);
ContentCachingResponseWrapper cacheableResponse =
new ContentCachingResponseWrapper(response);

ServletDataPayloadAdapter payload =
new ServletDataPayloadAdapter(cacheableRequest, cacheableResponse);
UserData userData = userDataCollector.collect(payload);

//TODO: Validate user data. Collect request data only if user data is valid ?
requestDataCollector.collect(payload, userData);
requestDataCollector.collect(payload, userData);
chain.doFilter(cacheableRequest, cacheableResponse);
}
} catch (Exception e){
log.error("Error occurred while processing request by readme metrics-sdk: {}", e.getMessage());
} finally {
chain.doFilter(request, response);
chain.doFilter(req, resp);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import jakarta.servlet.http.HttpServletRequest;
import lombok.extern.slf4j.Slf4j;

import java.io.IOException;
import java.util.*;
import java.util.stream.Collectors;

Expand All @@ -19,9 +20,26 @@ public class ServletDataPayloadAdapter implements DataPayloadAdapter {

//TODO Do I need a separate method to get request parameters?

@Override
public String getRequestMethod() {
return request.getMethod();
}

@Override
public String getRequestContentType() {
return request.getContentType();
}

@Override
public String getRequestBody() {
throw new UnsupportedOperationException("Not implemented yet");
try {
return request.getReader()
.lines()
.collect(Collectors.joining(System.lineSeparator()));
} catch (IOException e) {
log.error("Error when trying to get request body: {}", e.getMessage());
}
return "";
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.readme.starter.datacollection.userinfo;

import com.auth0.jwt.interfaces.DecodedJWT;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.readme.dataextraction.UserDataExtractor;
import com.readme.starter.datacollection.ServletDataPayloadAdapter;
import lombok.AllArgsConstructor;
Expand All @@ -17,6 +19,8 @@
@Slf4j
public class ServletUserDataExtractor implements UserDataExtractor<ServletDataPayloadAdapter> {

private ObjectMapper objectMapper;

//TODO: Consider possibility to extract the data from the header`s multiple value
// Is there any practical sense?
@Override
Expand All @@ -30,8 +34,21 @@ public String extractFromHeader(ServletDataPayloadAdapter payload, String fieldN
}

@Override
public String extractFromBody(ServletDataPayloadAdapter payload, String fieldName) {
return "Field value from body";
public String extractFromBody(ServletDataPayloadAdapter payload, String fieldPath) {
if (payload.getRequestContentType().equalsIgnoreCase("application/json")) {
String requestBody = payload.getRequestBody();
try {
JsonNode currentNode = objectMapper.readTree(requestBody);
if (!fieldPath.startsWith("/")) {
fieldPath = "/" + fieldPath;
}
return currentNode.at(fieldPath).asText();
} catch (Exception e) {
log.error("Error when reading the user data from JSON body: {}", e.getMessage());
}
}
log.error("The provided body content type {} is not supported to get user data.", payload.getRequestContentType());
return "";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.StringReader;
import java.util.Collections;
import java.util.Enumeration;
import java.util.List;
Expand All @@ -31,6 +34,9 @@ void setUp() {
adapter = new ServletDataPayloadAdapter(requestMock, responseMock);
}


// --------------------------- REQUEST --------------------------------

@Test
void getRequestHeaders_HappyPath_ReturnsAllHeaders() {
String usernameHeader = "X-User-Name".toLowerCase();
Expand All @@ -51,12 +57,44 @@ void getRequestHeaders_HappyPath_ReturnsAllHeaders() {
@Test
void getRequestHeaders_NoHeaders_ReturnsEmptyMap() {
when(requestMock.getHeaderNames()).thenReturn(Collections.emptyEnumeration());

Map<String, String> headers = adapter.getRequestHeaders();

assertTrue(headers.isEmpty());
}

@Test
void getRequestMethod_HappyPath_ReturnsCorrectMethod() {
when(requestMock.getMethod()).thenReturn("POST");
String method = adapter.getRequestMethod();

assertEquals("POST", method);
}

@Test
void getRequestContentType_HappyPath_ReturnsContentType() {
when(requestMock.getContentType()).thenReturn("application/json");
String contentType = adapter.getRequestContentType();

assertEquals("application/json", contentType);
}

@Test
void getRequestBody_HappyPath_ReturnsRequestBody() throws IOException {
String requestBody = "{\"bird\": \"Owl\"}";
BufferedReader bufferedReader = new BufferedReader(new StringReader(requestBody));
when(requestMock.getReader()).thenReturn(bufferedReader);
String result = adapter.getRequestBody();

assertEquals(requestBody, result);
}

@Test
void getRequestBody_WhenIOExceptionOccurs_ReturnsEmptyString() throws IOException {
when(requestMock.getReader()).thenThrow(new IOException("Failed to read request"));
String result = adapter.getRequestBody();

assertEquals("", result);
}

// --------------------------- RESPONSE --------------------------------
@Test
Expand All @@ -75,11 +113,24 @@ void getResponseHeaders_HappyPath_ReturnsAllHeaders() {
assertEquals("[email protected]", headers.get(userIdHeader));
}

// TODO implement this, once it fails
@Test
void getResponseBody_ThrowsUnsupportedOperationException() {
UnsupportedOperationException exception = assertThrows(
UnsupportedOperationException.class,
adapter::getResponseBody
);

assertEquals("Not implemented yet", exception.getMessage());
}

@Test
void getResponseHeaders_NoHeaders_ReturnsEmptyMap() {
when(responseMock.getHeaderNames()).thenReturn(Collections.emptyList());

Map<String, String> headers = adapter.getResponseHeaders();

assertTrue(headers.isEmpty());
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ void collect_HappyCase() {
@Test
void collect_MissingApiKeyConfiguration() {
when(userDataProperties.getApiKey()).thenReturn(null);

UserData result = userDataCollector.collect(payload);

assertThat(result).isNotNull();
Expand All @@ -70,7 +69,6 @@ void collect_MissingApiKeyConfiguration() {
void collect_EmptyHeaderValue() {
FieldMapping apiKeyMapping = new FieldMapping(UserDataSource.HEADER.getValue(), "x-api-key");
when(userDataProperties.getApiKey()).thenReturn(apiKeyMapping);

when(extractionService.extractFromHeader(payload, "x-api-key")).thenReturn("");

UserData result = userDataCollector.collect(payload);
Expand Down
Loading

0 comments on commit 8626bc4

Please sign in to comment.