adds AsyncSemaphore IT

Author: MarketDataDev03

src/integrationTest/java/com/marketdata/sdk/internal/http/AsyncSemaphoreIT.java

@@ -0,0 +1,60 @@
+package com.marketdata.sdk.internal.http;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.marketdata.sdk.MarketDataClient;
+import com.marketdata.sdk.markets.MarketStatus;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+/**
+ * Concurrency integration test against the live Market Data API. Verifies that the {@link
+ * AsyncSemaphore} + {@link HttpTransport} pipeline correctly handles fan-out beyond the pool size:
+ * the requests over the limit must traverse the semaphore's slow path (queue the waiter, complete
+ * it later via {@code release}) without deadlocking or losing a permit.
+ *
+ * <p>Costs {@code CONCURRENCY_LIMIT + 5 = 55} requests against the live {@code /markets/status/}
+ * endpoint per run. With a typical RTT of ~100 ms and pool size 50, the test wall time is well
+ * under a second.
+ *
+ * <p>Gated by {@code MARKETDATA_RUN_INTEGRATION_TESTS=true} like the rest of this source set.
+ */
+class AsyncSemaphoreIT {
+
+  /**
+   * If a permit ever leaked or the slow-path queue stopped being drained, {@code allOf.join()}
+   * would block forever. The 30 s timeout fails the test fast instead of leaving CI hung.
+   */
+  @Test
+  @Timeout(value = 30, unit = TimeUnit.SECONDS)
+  void concurrentFanOutBeyondPoolLimitCompletesWithoutDeadlock() {
+    try (var client = new MarketDataClient(null, null, null, false)) {
+      int n = HttpTransport.CONCURRENCY_LIMIT + 5;
+      List<CompletableFuture<MarketStatus>> futures = new ArrayList<>(n);
+
+      // Fire all N requests as fast as the loop runs. With pool=50, the first 50 take the
+      // fast path (already-completed acquire future) and dispatch immediately; requests
+      // 51..55 take the slow path and enqueue waiters that complete only when one of the
+      // first 50 releases.
+      for (int i = 0; i < n; i++) {
+        futures.add(client.markets().statusAsync());
+      }
+
+      // allOf.join() throws on any underlying failure; we let it propagate so a 429 / network
+      // hiccup surfaces as a real test failure rather than silently masking the issue.
+      CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).join();
+
+      // Every response must be a valid MarketStatus. Empty results would suggest a hidden
+      // failure (auth issue, rate limit) that wasn't observable from allOf alone.
+      for (CompletableFuture<MarketStatus> f : futures) {
+        MarketStatus status = f.join();
+        assertThat(status.days()).isNotEmpty();
+        assertThat(status.days().get(0).date()).isNotNull();
+      }
+    }
+  }
+}

src/test/java/com/marketdata/sdk/MarketDataClientTest.java

@@ -3,6 +3,13 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import com.marketdata.sdk.internal.Configuration;
+import com.marketdata.sdk.internal.EnvVars;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import java.util.logging.Logger;
 import org.junit.jupiter.api.Test;
 
 class MarketDataClientTest {
@@ -18,17 +25,68 @@ void buildsWithExplicitToken() {
 
   @Test
   void demoModeWhenNoTokenAvailable() {
-    // No apiKey passed to the constructor. Demo mode iff the env/dotenv
-    // cascade also yields nothing — true on any CI environment that
-    // doesn't export MARKETDATA_TOKEN. This assertion is conditional
-    // so the test stays valid in both cases.
+    // Demo mode iff the full cascade (env var → .env → null) yields nothing. Deriving the
+    // expectation from the same Configuration helper the constructor uses keeps the test
+    // valid both on CI (no token anywhere → demoMode) and locally (.env-supplied token →
+    // not demoMode); a plain `System.getenv` check would miss the .env source and break
+    // locally.
     try (var client = new MarketDataClient()) {
-      String envToken = System.getenv("MARKETDATA_TOKEN");
-      boolean expectDemo = envToken == null || envToken.isBlank();
+      boolean expectDemo = Configuration.loadFromProcess().resolve(null, EnvVars.TOKEN) == null;
       assertThat(client.isDemoMode()).isEqualTo(expectDemo);
     }
   }
 
+  @Test
+  void fineLevelLoggingEmitsRedactedToken() {
+    // The constructor logs the redacted token at FINE only. With the default logger
+    // configuration (INFO), `LOG.isLoggable(FINE)` returns false and the line is dead from
+    // JaCoCo's perspective. This test installs a capturing handler at FINE and asserts the
+    // redacted token shows up — the unredacted token must not.
+    Logger logger = Logger.getLogger(MarketDataClient.class.getName());
+    Level previousLevel = logger.getLevel();
+    boolean previousUseParent = logger.getUseParentHandlers();
+    CapturingHandler capture = new CapturingHandler();
+    logger.addHandler(capture);
+    logger.setLevel(Level.FINE);
+    logger.setUseParentHandlers(false);
+
+    try (var client = new MarketDataClient("supersecret-token-VALUE-YKT0", null, null, false)) {
+      assertThat(client.isDemoMode()).isFalse();
+    } finally {
+      logger.removeHandler(capture);
+      logger.setLevel(previousLevel);
+      logger.setUseParentHandlers(previousUseParent);
+    }
+
+    assertThat(capture.records)
+        .anySatisfy(
+            r -> {
+              assertThat(r.getLevel()).isEqualTo(Level.FINE);
+              assertThat(r.getMessage()).contains("Token");
+            });
+    // Whatever was logged at FINE, the raw token must never appear in any record.
+    for (LogRecord r : capture.records) {
+      assertThat(r.getMessage() == null ? "" : r.getMessage())
+          .doesNotContain("supersecret-token-VALUE-YKT0");
+    }
+  }
+
+  /** Minimal {@link Handler} that buffers everything in memory for assertions. */
+  private static final class CapturingHandler extends Handler {
+    final List<LogRecord> records = new ArrayList<>();
+
+    @Override
+    public void publish(LogRecord record) {
+      records.add(record);
+    }
+
+    @Override
+    public void flush() {}
+
+    @Override
+    public void close() {}
+  }
+
   @Test
   void noArgConstructorAppliesProductionDefaults() {
     // The no-arg constructor must be equivalent to `new MarketDataClient(null, null, null,

src/test/java/com/marketdata/sdk/exception/MarketDataExceptionTest.java

@@ -98,6 +98,25 @@ void everySubtypeExposesBothConstructors() {
     }
   }
 
+  @Test
+  void supportInfoFormatsNullContextAsNotApplicable() {
+    // When the exception is built from ErrorContext.empty() (e.g. client-side validation
+    // errors that fire before any HTTP request), getSupportInfo() must render each null
+    // field as "(n/a)" instead of literal "null". Covers the null-branches of the three
+    // ternaries in MarketDataException.getSupportInfo.
+    var error = new BadRequestError("symbol must not be blank", ErrorContext.empty());
+
+    String supportInfo = error.getSupportInfo();
+
+    assertThat(supportInfo)
+        .contains("BadRequestError")
+        .contains("symbol must not be blank")
+        .contains("Status code: (n/a)")
+        .contains("Request ID:  (n/a)")
+        .contains("Request URL: (n/a)")
+        .doesNotContain("null");
+  }
+
   @Test
   void supportInfoNeverContainsSensitiveData() {
     // The exception itself never receives the token; we just

src/test/java/com/marketdata/sdk/internal/ConfigurationTest.java

@@ -130,6 +130,29 @@ void missingDotEnvReturnsEmpty(@TempDir Path tmp) {
     assertThat(Configuration.readDotEnvFile(tmp.resolve(".env"))).isEmpty();
   }
 
+  @Test
+  void mismatchedQuotesArePreservedVerbatim(@TempDir Path tmp) throws IOException {
+    // stripQuotes only strips when the first AND last characters match (both " or both ').
+    // Lines with mixed or unbalanced quotes must keep the value as-is. Covers the right-hand
+    // false branches of the `||` in (first == '"' && last == '"') || (first == '\'' && last ==
+    // '\'').
+    Path dotenv = tmp.resolve(".env");
+    Files.writeString(
+        dotenv,
+        """
+        UNCLOSED_DOUBLE="abc
+        UNCLOSED_SINGLE='abc
+        MIXED_QUOTES="abc'
+        """);
+
+    Map<String, String> parsed = Configuration.readDotEnvFile(dotenv);
+
+    assertThat(parsed)
+        .containsEntry("UNCLOSED_DOUBLE", "\"abc")
+        .containsEntry("UNCLOSED_SINGLE", "'abc")
+        .containsEntry("MIXED_QUOTES", "\"abc'");
+  }
+
   @Test
   void dotEnvParsingIntegratesWithCascade(@TempDir Path tmp) throws IOException {
     Path dotenv = tmp.resolve(".env");

src/test/java/com/marketdata/sdk/internal/http/HttpStatusMapperTest.java

@@ -0,0 +1,83 @@
+package com.marketdata.sdk.internal.http;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.marketdata.sdk.exception.AuthenticationError;
+import com.marketdata.sdk.exception.BadRequestError;
+import com.marketdata.sdk.exception.MarketDataException;
+import com.marketdata.sdk.exception.RateLimitError;
+import com.marketdata.sdk.exception.ServerError;
+import org.junit.jupiter.api.Test;
+
+class HttpStatusMapperTest {
+
+  private static final String URL = "https://api.marketdata.app/v1/test/";
+  private static final String RAY = "ray-1";
+
+  // ---------- switch coverage: each case + default ----------
+
+  @Test
+  void status400MapsToBadRequest() {
+    MarketDataException e = HttpStatusMapper.toException(400, URL, RAY);
+    assertThat(e).isInstanceOf(BadRequestError.class);
+    assertThat(e.getStatusCode()).isEqualTo(400);
+    assertThat(e.getMessage()).contains("400");
+  }
+
+  @Test
+  void status422AlsoMapsToBadRequest() {
+    // Same case-arm as 400; without exercising 422 explicitly, half the multi-label arm is
+    // unrecorded by JaCoCo.
+    MarketDataException e = HttpStatusMapper.toException(422, URL, RAY);
+    assertThat(e).isInstanceOf(BadRequestError.class);
+    assertThat(e.getStatusCode()).isEqualTo(422);
+    assertThat(e.getMessage()).contains("422");
+  }
+
+  @Test
+  void status401MapsToAuthenticationError() {
+    MarketDataException e = HttpStatusMapper.toException(401, URL, RAY);
+    assertThat(e).isInstanceOf(AuthenticationError.class);
+    assertThat(e.getStatusCode()).isEqualTo(401);
+  }
+
+  @Test
+  void status429MapsToRateLimitError() {
+    MarketDataException e = HttpStatusMapper.toException(429, URL, RAY);
+    assertThat(e).isInstanceOf(RateLimitError.class);
+    assertThat(e.getStatusCode()).isEqualTo(429);
+  }
+
+  @Test
+  void everyOtherStatusFallsThroughToServerError() {
+    // Any status not explicitly handled (402, 500, 502, 503, 504, weird ones) maps to
+    // ServerError. Covers the `default ->` arm.
+    for (int code : new int[] {402, 500, 502, 503, 504, 599}) {
+      MarketDataException e = HttpStatusMapper.toException(code, URL, RAY);
+      assertThat(e).as("status %d", code).isInstanceOf(ServerError.class);
+      assertThat(e.getStatusCode()).isEqualTo(code);
+    }
+  }
+
+  // ---------- emptyToNull: null vs blank vs valid ----------
+
+  @Test
+  void nullRequestIdIsPropagatedAsNull() {
+    MarketDataException e = HttpStatusMapper.toException(500, URL, null);
+    assertThat(e.getRequestId()).isNull();
+  }
+
+  @Test
+  void blankRequestIdIsTreatedAsNull() {
+    // emptyToNull's `s == null || s.isBlank()` short-circuits — without an explicit blank
+    // input, the right-hand isBlank() branch is never evaluated.
+    MarketDataException e = HttpStatusMapper.toException(500, URL, "   ");
+    assertThat(e.getRequestId()).isNull();
+  }
+
+  @Test
+  void validRequestIdIsPreserved() {
+    MarketDataException e = HttpStatusMapper.toException(500, URL, "ray-abc");
+    assertThat(e.getRequestId()).isEqualTo("ray-abc");
+  }
+}

src/test/java/com/marketdata/sdk/internal/http/HttpTransportE2ETest.java

@@ -0,0 +1,102 @@
+package com.marketdata.sdk.internal.http;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.sun.net.httpserver.HttpExchange;
+import com.sun.net.httpserver.HttpHandler;
+import com.sun.net.httpserver.HttpServer;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.net.URI;
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.atomic.AtomicReference;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * End-to-end tests for {@link HttpTransport} that exercise URI shapes and status codes the public
+ * resource façades don't naturally hit (status 203, trailing-slash paths). Uses the JDK's built-in
+ * {@link HttpServer} to avoid any extra mocking dependencies.
+ */
+class HttpTransportE2ETest {
+
+  private HttpServer server;
+  private final AtomicReference<URI> capturedUri = new AtomicReference<>();
+  private RouteHandler handler;
+
+  /** Minimal record matching {@code {"value": "..."}} so we can verify a successful decode. */
+  record Echo(@JsonProperty("value") String value) {}
+
+  @BeforeEach
+  void startServer() throws IOException {
+    server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
+    handler = new RouteHandler();
+    server.createContext("/", handler);
+    server.start();
+  }
+
+  @AfterEach
+  void stopServer() {
+    server.stop(0);
+  }
+
+  private HttpTransport newTransport() {
+    int port = server.getAddress().getPort();
+    return new HttpTransport("http://127.0.0.1:" + port, "v1", "test/0.0", null);
+  }
+
+  /**
+   * Status 203 (Non-Authoritative Information) is treated identically to 200 by the transport —
+   * decoding the body and returning the result. The check {@code status == 200 || status == 203 ||
+   * status == 404} in {@code processResponse} is the only place 203 appears, and without an
+   * explicit test the 203 leg is dead from JaCoCo's perspective.
+   */
+  @Test
+  void status203IsTreatedAsSuccess() {
+    handler.setResponse(203, "{\"value\":\"ok\"}");
+
+    Echo result = newTransport().executeSync(RequestSpec.get("ping").build(), Echo.class);
+
+    assertThat(result.value()).isEqualTo("ok");
+  }
+
+  /**
+   * When the {@link RequestSpec#path()} already ends with a slash, the transport must not append
+   * another one. Covers the {@code endsWith("/")} → true branch in {@code buildUri}.
+   */
+  @Test
+  void pathEndingInSlashIsNotDoubled() {
+    handler.setResponse(200, "{\"value\":\"ok\"}");
+
+    Echo result = newTransport().executeSync(RequestSpec.get("ping/").build(), Echo.class);
+
+    assertThat(result.value()).isEqualTo("ok");
+    assertThat(capturedUri.get().getPath()).isEqualTo("/v1/ping/");
+    assertThat(capturedUri.get().getPath()).doesNotContain("//");
+  }
+
+  // ---------- in-process server plumbing ----------
+
+  private final class RouteHandler implements HttpHandler {
+    private int statusCode = 200;
+    private String body = "{}";
+
+    void setResponse(int code, String body) {
+      this.statusCode = code;
+      this.body = body;
+    }
+
+    @Override
+    public void handle(HttpExchange exchange) throws IOException {
+      capturedUri.set(exchange.getRequestURI());
+
+      byte[] bodyBytes = body.getBytes(StandardCharsets.UTF_8);
+      exchange.getResponseHeaders().add("Content-Type", "application/json");
+      exchange.sendResponseHeaders(statusCode, bodyBytes.length);
+      exchange.getResponseBody().write(bodyBytes);
+      exchange.getResponseBody().close();
+    }
+  }
+}