test: Add more tests and doc comments

Author: Abhi347

src/main/java/com/spotify/github/http/BaseHttpResponse.java

@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.Map;
 
+/** BaseHttpResponse is the base implementation of HttpResponse. */
 public abstract class BaseHttpResponse implements HttpResponse {
   private static final int HTTP_OK = 200;
   private static final int HTTP_BAD_REQUEST = 400;
@@ -43,30 +44,66 @@ public BaseHttpResponse(
     this.headers = headers;
   }
 
+  /**
+   * Returns the request that generated this response.
+   *
+   * @return HttpRequest the request that generated this response
+   */
   @Override
   public HttpRequest request() {
     return request;
   }
 
+  /**
+   * Returns the HTTP status code of the response.
+   *
+   * @return the status code of the response
+   */
+  @Override
   public int statusCode() {
     return statusCode;
   }
 
+  /**
+   * Returns the HTTP status message of the response.
+   *
+   * @return the status message of the response
+   */
   @Override
   public String statusMessage() {
     return statusMessage;
   }
 
+  /**
+   * Returns the headers of the response.
+   *
+   * @return the headers of the response as a Map of strings
+   */
   @Override
   public Map<String, List<String>> headers() {
     return this.headers;
   }
 
+  /**
+   * Returns the values of the header with the given name. If the header is not present, this method
+   * returns null.
+   *
+   * @param headerName the name of the header
+   * @return the values of the header with the given name as a List of strings, or null if the
+   *     header is not present
+   */
   @Override
   public List<String> headers(final String headerName) {
     return this.headers.get(headerName);
   }
 
+  /**
+   * Returns the first value of the header with the given name. If the header is not present, this
+   * method returns null.
+   *
+   * @param headerName the name of the header
+   * @return the first value of the header with the given name, or null if the header is not present
+   */
   @Override
   public String header(final String headerName) {
     List<String> headerValues = this.headers(headerName);
@@ -80,6 +117,11 @@ public String header(final String headerName) {
     }
   }
 
+  /**
+   * Was the request successful?
+   *
+   * @return true if the status code is in the range [200, 400)
+   */
   @Override
   public boolean isSuccessful() {
     return this.statusCode() >= HTTP_OK && this.statusCode() < HTTP_BAD_REQUEST;

src/main/java/com/spotify/github/http/okhttp/OkHttpHttpClient.java

@@ -39,6 +39,11 @@
 import okhttp3.*;
 import org.jetbrains.annotations.NotNull;
 
+/**
+ * OkHttpHttpClient is the implementation of HttpClient using OkHttp. This also serves as an example
+ * of how to create a custom HttpClient. This HttpClient is also capable of tracing the requests
+ * using OpenCensus or OpenTelemetry.
+ */
 public class OkHttpHttpClient implements HttpClient {
   private final OkHttpClient client;
   private Tracer tracer;
@@ -56,9 +61,15 @@ public OkHttpHttpClient(final OkHttpClient client, final Tracer tracer) {
     this.callFactory = createTracedClient();
   }
 
+  /**
+   * Send a request and return a future with the response.
+   *
+   * @param httpRequest the request to send
+   * @return a future with the response
+   */
   @Override
   public CompletableFuture<HttpResponse> send(final HttpRequest httpRequest) {
-    Request request = buildRequest(httpRequest);
+    Request request = buildOkHttpRequest(httpRequest);
     CompletableFuture<HttpResponse> future = new CompletableFuture<>();
     try (Span span = tracer.span(httpRequest)) {
       if (this.callFactory == null) {
@@ -91,7 +102,13 @@ public void setTracer(final Tracer tracer) {
     this.callFactory = createTracedClient();
   }
 
-  private Request buildRequest(final HttpRequest request) {
+  /**
+   * Build an OkHttp Request from an HttpRequest.
+   *
+   * @param request the HttpRequest
+   * @return the OkHttp Request
+   */
+  private Request buildOkHttpRequest(final HttpRequest request) {
     Request.Builder requestBuilder = new Request.Builder().url(request.url());
     request
         .headers()
@@ -109,6 +126,12 @@ private Request buildRequest(final HttpRequest request) {
     return requestBuilder.build();
   }
 
+  /**
+   * Build an HttpRequest from an OkHttp Request.
+   *
+   * @param request the OkHttp Request
+   * @return the HttpRequest
+   */
   private HttpRequest buildHttpRequest(final Request request) {
     return ImmutableHttpRequest.builder()
         .url(request.url().toString())
@@ -118,6 +141,11 @@ private HttpRequest buildHttpRequest(final Request request) {
         .build();
   }
 
+  /**
+   * Create a traced client based on the tracer.
+   *
+   * @return the traced client
+   */
   private Call.Factory createTracedClient() {
     if (this.tracer == null || this.tracer instanceof NoopTracer) {
       return createTracedClientNoopTracer();
@@ -131,6 +159,11 @@ private Call.Factory createTracedClient() {
     return createTracedClientNoopTracer();
   }
 
+  /**
+   * Create a traced client with a NoopTracer.
+   *
+   * @return the traced client
+   */
   private Call.Factory createTracedClientNoopTracer() {
     return new Call.Factory() {
       @NotNull
@@ -141,12 +174,23 @@ public Call newCall(@NotNull final Request request) {
     };
   }
 
+  /**
+   * Create a traced client with OpenTelemetry.
+   *
+   * @return the traced client
+   */
   private Call.Factory createTracedClientOpenTelemetry() {
+    // OkHttpTelemetry is a helper class that provides a Call.Factory that can be used to trace
     return OkHttpTelemetry.builder(((OpenTelemetryTracer) this.tracer).getOpenTelemetry())
         .build()
         .newCallFactory(client);
   }
 
+  /**
+   * Create a traced client with OpenCensus.
+   *
+   * @return the traced client
+   */
   private Call.Factory createTracedClientOpenCensus() {
     return new Call.Factory() {
       @NotNull
@@ -159,6 +203,7 @@ public Call newCall(@NotNull final Request request) {
                 .span(buildHttpRequest(request))
                 .addTag(TraceHelper.TraceTags.HTTP_URL, request.url().toString());
         OkHttpClient.Builder okBuilder = client.newBuilder();
+        // Add a network interceptor to trace the request
         okBuilder
             .networkInterceptors()
             .add(

src/main/java/com/spotify/github/http/okhttp/OkHttpHttpResponse.java

@@ -32,9 +32,10 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+/** OkHttpHttpResponse is the implementation of HttpResponse using OkHttp. */
 public class OkHttpHttpResponse extends BaseHttpResponse {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
-  
+
   private final Response response;
   private InputStream body;
   private String bodyString;
@@ -52,6 +53,7 @@ public InputStream body() {
     return body;
   }
 
+  @Override
   public String bodyString() {
     if (bodyString == null) {
       if (response != null) {
@@ -75,6 +77,12 @@ public void close() {
     }
   }
 
+  /**
+   * Get the response body as a string.
+   *
+   * @param response the response
+   * @return the response body as a string
+   */
   private static String responseBodyUnchecked(final Response response) {
     if (response.body() == null) {
       return null;

src/test/java/com/spotify/github/http/okhttp/OkHttpHttpClientTest.java

@@ -0,0 +1,122 @@
+package com.spotify.github.http.okhttp;
+
+import com.spotify.github.http.HttpRequest;
+import com.spotify.github.http.HttpResponse;
+import com.spotify.github.http.ImmutableHttpRequest;
+import com.spotify.github.tracing.Tracer;
+import okhttp3.*;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.*;
+
+class OkHttpHttpClientTest {
+  private static OkHttpClient okHttpClient;
+  private static OkHttpHttpClient httpClient;
+  private static Tracer tracer;
+
+  @BeforeAll
+  static void setUp() {
+    okHttpClient = mock(OkHttpClient.class);
+    tracer = mock(Tracer.class);
+    httpClient = new OkHttpHttpClient(okHttpClient, tracer);
+  }
+
+  @AfterEach
+  void tearDown() {
+    reset(okHttpClient, tracer);
+  }
+
+  @Test
+  void sendSuccessfully() throws IOException {
+    // Given
+    final Call call = mock(Call.class);
+    final ArgumentCaptor<Callback> capture = ArgumentCaptor.forClass(Callback.class);
+    doNothing().when(call).enqueue(capture.capture());
+    final Response response =
+        new okhttp3.Response.Builder()
+            .code(200)
+            .body(ResponseBody.create(MediaType.get("application/json"), "{\"foo\":\"bar\"}"))
+            .message("foo")
+            .protocol(Protocol.HTTP_1_1)
+            .request(new Request.Builder().url("https://example.com").build())
+            .build();
+
+    HttpRequest httpRequest = ImmutableHttpRequest.builder().url("https://example.com").build();
+    when(okHttpClient.newCall(any())).thenReturn(call);
+
+    // When
+    CompletableFuture<HttpResponse> futureResponse = httpClient.send(httpRequest);
+    capture.getValue().onResponse(call, response);
+    HttpResponse httpResponse = futureResponse.join();
+
+    // Then
+    assertNotNull(httpResponse);
+    assertEquals("{\"foo\":\"bar\"}", httpResponse.bodyString());
+    assertEquals(200, httpResponse.statusCode());
+    assertEquals("foo", httpResponse.statusMessage());
+    assertTrue(httpResponse.isSuccessful());
+    verify(tracer, times(1)).span(any(HttpRequest.class));
+  }
+
+  @Test
+  void sendWithException() {
+    // Given
+    final Call call = mock(Call.class);
+    final ArgumentCaptor<Callback> capture = ArgumentCaptor.forClass(Callback.class);
+    doNothing().when(call).enqueue(capture.capture());
+    final IOException exception = new IOException("Network error");
+
+    HttpRequest httpRequest = ImmutableHttpRequest.builder().url("https://example.com").build();
+    when(okHttpClient.newCall(any())).thenReturn(call);
+
+    // When
+    CompletableFuture<HttpResponse> futureResponse = httpClient.send(httpRequest);
+    capture.getValue().onFailure(call, exception);
+
+    // Then
+    assertThrows(CompletionException.class, futureResponse::join);
+    verify(tracer, times(1)).span(any(HttpRequest.class));
+  }
+
+  @Test
+  void sendWithClientError() throws IOException {
+    // Given
+    final Call call = mock(Call.class);
+    final ArgumentCaptor<Callback> capture = ArgumentCaptor.forClass(Callback.class);
+    doNothing().when(call).enqueue(capture.capture());
+    final Response response =
+        new okhttp3.Response.Builder()
+            .code(404)
+            .body(
+                ResponseBody.create(MediaType.get("application/json"), "{\"error\":\"Not Found\"}"))
+            .message("Not Found")
+            .protocol(Protocol.HTTP_1_1)
+            .request(new Request.Builder().url("https://example.com").build())
+            .build();
+
+    HttpRequest httpRequest = ImmutableHttpRequest.builder().url("https://example.com").build();
+    when(okHttpClient.newCall(any())).thenReturn(call);
+
+    // When
+    CompletableFuture<HttpResponse> futureResponse = httpClient.send(httpRequest);
+    capture.getValue().onResponse(call, response);
+    HttpResponse httpResponse = futureResponse.join();
+
+    // Then
+    assertNotNull(httpResponse);
+    assertEquals("{\"error\":\"Not Found\"}", httpResponse.bodyString());
+    assertEquals(404, httpResponse.statusCode());
+    assertEquals("Not Found", httpResponse.statusMessage());
+    assertFalse(httpResponse.isSuccessful());
+    verify(tracer, times(1)).span(any(HttpRequest.class));
+  }
+}