From 9d3395987cec41185350ae5d8091530f8f8c1a38 Mon Sep 17 00:00:00 2001 From: Douglas Miller Date: Mon, 29 Jun 2026 19:38:03 -0500 Subject: [PATCH 1/2] feat: Add HttpAdapter interface for pluggable HTTP transport Introduce HttpAdapter, HttpResponse, and DefaultHttpAdapter in the com.recurly.v3.http package. DefaultHttpAdapter wraps OkHttp and owns gzip negotiation, connection management, and timeout configuration. BaseClient now builds its headers map directly and delegates all transport to the injected adapter, removing the OkHttp dependency from the SDK core. ClientOptions gains setHttpAdapter() as the injection point. HeaderInterceptor is removed; its User-Agent logic moves to BaseClient. Co-Authored-By: Claude --- docs/http-adapter-implementation-guide.md | 277 ++++++++++++ pom.xml | 19 +- src/main/java/com/recurly/v3/BaseClient.java | 353 ++++++++------- .../java/com/recurly/v3/ClientOptions.java | 18 +- .../v3/exception/ExceptionFactory.java | 8 +- .../recurly/v3/http/DefaultHttpAdapter.java | 98 ++++ .../recurly/v3/http/HeaderInterceptor.java | 58 --- .../java/com/recurly/v3/http/HttpAdapter.java | 85 ++++ .../com/recurly/v3/http/HttpResponse.java | 39 ++ .../java/com/recurly/v3/BaseClientTest.java | 424 ++++++++---------- .../com/recurly/v3/HeaderInterceptorTest.java | 42 -- src/test/java/com/recurly/v3/PagerTest.java | 201 ++++----- .../com/recurly/v3/fixtures/MockClient.java | 58 +-- .../http/DefaultHttpAdapterContractTest.java | 13 + .../recurly/v3/http/HttpAdapterContract.java | 373 +++++++++++++++ .../com/recurly/v3/http/HttpResponseTest.java | 59 +++ 16 files changed, 1419 insertions(+), 706 deletions(-) create mode 100644 docs/http-adapter-implementation-guide.md create mode 100644 src/main/java/com/recurly/v3/http/DefaultHttpAdapter.java delete mode 100644 src/main/java/com/recurly/v3/http/HeaderInterceptor.java create mode 100644 src/main/java/com/recurly/v3/http/HttpAdapter.java create mode 100644 src/main/java/com/recurly/v3/http/HttpResponse.java delete mode 100644 src/test/java/com/recurly/v3/HeaderInterceptorTest.java create mode 100644 src/test/java/com/recurly/v3/http/DefaultHttpAdapterContractTest.java create mode 100644 src/test/java/com/recurly/v3/http/HttpAdapterContract.java create mode 100644 src/test/java/com/recurly/v3/http/HttpResponseTest.java diff --git a/docs/http-adapter-implementation-guide.md b/docs/http-adapter-implementation-guide.md new file mode 100644 index 00000000..b49c4985 --- /dev/null +++ b/docs/http-adapter-implementation-guide.md @@ -0,0 +1,277 @@ +# Implementing a Custom `HttpAdapter` + +The `HttpAdapter` interface lets you replace the default transport with any HTTP library or +middleware you prefer. Common reasons to do this: + +- Use a company-standard HTTP client (e.g. Apache HttpClient, `java.net.http.HttpClient`) +- Route requests through a proxy +- Add observability (logging, metrics, distributed tracing) +- Inject a fake transport in tests without hitting the network + +## Registering your implementation + +```java +ClientOptions options = new ClientOptions(); +options.setHttpAdapter(new MyHttpAdapter()); +Client client = new Client(apiKey, options); +``` + +When no adapter is set, the client uses `DefaultHttpAdapter`. + +--- + +## The `execute` contract + +```java +HttpResponse execute(String method, String url, Map headers, String body) + throws IOException; +``` + +### Method + +Always one of: `GET`, `POST`, `PUT`, `DELETE`, `HEAD`. No other values are sent by the client. + +### URL + +A fully-qualified absolute URL, e.g. `https://v3.recurly.com/accounts?limit=20`. Never `null`. +Query parameters are already encoded and appended by the client. + +### Headers + +All headers the client wants to send, including: + +| Header | Value | +|-----------------|----------------------------------------------------| +| `Authorization` | `Basic ` | +| `Accept` | Recurly API version string | +| `Content-Type` | `application/json` | +| `User-Agent` | `Recurly/; java ` | + +**Forward every entry without modification.** Do not add, remove, or override headers in the +adapter. The client owns header construction; the adapter owns transport. + +### Body + +- `POST` and `PUT` requests: a UTF-8-encoded JSON string. +- `GET`, `HEAD`, `DELETE` requests: `null`. + +When `body` is `null` and the HTTP method requires a body (e.g. `DELETE` with some servers), send +an empty body (`Content-Length: 0`). + +--- + +## Building the `HttpResponse` + +```java +return new HttpResponse(statusCode, responseHeaders, responseBodyBytes); +``` + +### Status code + +Return the exact HTTP status code from the server (e.g. `200`, `404`, `500`). The client uses it +to decide success vs. error and to select the right typed exception. Do not normalise or translate +status codes. + +### Response headers + +Pass a `Map` of all response headers. `HttpResponse` normalises keys to lower-case +internally, so you do not need to do it yourself — but passing lower-case keys is fine too. + +The client reads these specific headers: + +| Header | Purpose | +|-------------------------|---------------------------------------------------| +| `content-type` | Determines JSON vs. binary (PDF) deserialisation | +| `x-request-id` | Included in error messages | +| `recurly-deprecated` | Triggers a deprecation warning log | +| `recurly-sunset-date` | Included in the deprecation warning | +| `recurly-total-records` | Returned by `getRecordCount()` (HEAD requests) | + +Include all headers from the server response, not just these. Future library versions may read +additional headers without a breaking change. + +### Response body + +- Read the body **fully** before returning. Do not return a lazy or streaming reference — `HttpResponse` + takes a `byte[]` and the underlying stream will be closed once `execute` returns. +- Pass an empty `byte[]` (never `null`) when there is no body (e.g. `204 No Content`, `HEAD`). +- The client interprets the bytes as UTF-8 text for JSON and as raw bytes for binary types (PDF). + +--- + +## Error handling + +| Situation | What to do | +|-----------------------------------|----------------------------------------------------| +| Network failure (timeout, refused)| Throw `IOException`. The client wraps it in `NetworkException`. | +| TLS / certificate error | Throw `IOException`. | +| HTTP 4xx / 5xx response | Return the `HttpResponse` — do not throw. The client maps status codes to typed exceptions. | +| `InterruptedException` (JDK `HttpClient` only) | Restore the interrupt flag, then throw `IOException`. | + +Most HTTP libraries handle thread cancellation through their own timeout/cancellation mechanism and +only throw `IOException` from their synchronous execute call. You only need the pattern below if +your underlying client declares `throws InterruptedException` — notably +`java.net.http.HttpClient.send()`: + +```java +} catch (InterruptedException e) { + Thread.currentThread().interrupt(); // restore the flag + throw new IOException("Request interrupted", e); +} +``` + +--- + +## Thread safety + +A single `HttpAdapter` instance is shared across all requests made by one `Client` instance, and +`Client` is designed to be shared across threads. Your adapter must be safe for concurrent use. + +In practice this means: +- Hold your underlying HTTP client as a **field** (not a local variable per call). +- Ensure any client configuration (timeouts, proxy settings) is immutable after construction, or + protected by synchronisation. + +--- + +## Connection pooling + +Creating a new TCP connection for every request adds 50–200 ms of latency and wastes server +resources. Use an HTTP client that maintains a connection pool and hold it as a field: + +```java +public class MyHttpAdapter implements HttpAdapter { + // Shared, thread-safe; connection pool lives here. + private final java.net.http.HttpClient client = java.net.http.HttpClient.newHttpClient(); + + @Override + public HttpResponse execute(String method, String url, + Map headers, String body) throws IOException { + // ... + } +} +``` + +--- + +## Timeouts + +The Recurly client does not enforce timeouts. Set connect, read, and write timeouts inside your +adapter and adjust to your SLA requirements. + +--- + +## Minimal example + +```java +import com.recurly.v3.http.HttpAdapter; +import com.recurly.v3.http.HttpResponse; + +import java.io.IOException; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpRequest.BodyPublishers; +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; + +public class JavaNetHttpAdapter implements HttpAdapter { + + private final HttpClient client = HttpClient.newBuilder() + .connectTimeout(Duration.ofSeconds(30)) + .build(); + + @Override + public HttpResponse execute(String method, String url, + Map headers, String body) throws IOException { + HttpRequest.Builder builder = HttpRequest.newBuilder() + .uri(URI.create(url)) + .method(method, body != null + ? BodyPublishers.ofString(body) + : BodyPublishers.noBody()); + + headers.forEach(builder::header); + + try { + java.net.http.HttpResponse resp = + client.send(builder.build(), java.net.http.HttpResponse.BodyHandlers.ofByteArray()); + + Map responseHeaders = new HashMap<>(); + resp.headers().map().forEach((k, vs) -> { + if (k != null && !vs.isEmpty()) responseHeaders.put(k, vs.get(0)); + }); + + return new HttpResponse(resp.statusCode(), responseHeaders, + resp.body() != null ? resp.body() : new byte[0]); + + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException("HTTP request interrupted", e); + } + } +} +``` + +--- + +## Verifying your implementation with `HttpAdapterContract` + +The library ships `HttpAdapterContract`, a portable JUnit 5 contract test that verifies any +`HttpAdapter` against the full behavioural requirements above. Extend it and implement +`createAdapter()`: + +```java +class MyAdapterTest extends HttpAdapterContract { + @Override + protected HttpAdapter createAdapter() { + return new MyHttpAdapter(); + } +} +``` + +**Java 11 required at test runtime.** `HttpAdapterContract` depends on WireMock 3, which requires +Java 11+. Tests are automatically skipped on Java 8 via `@DisabledOnJre(JRE.JAVA_8)`. Your +compiled adapter can still target Java 8 bytecode — only the test JVM must be Java 11+. + +Add these test-scope dependencies to your project: + +```xml + + org.wiremock + wiremock + 3.13.2 + test + + + org.junit.jupiter + junit-jupiter-engine + 5.12.2 + test + +``` + +--- + +## Test doubles + +To test code that uses the Recurly client without network calls, implement `HttpAdapter` to return +canned `HttpResponse` objects: + +```java +public class FakeHttpAdapter implements HttpAdapter { + private final Queue responses = new ArrayDeque<>(); + + public void enqueue(HttpResponse r) { responses.add(r); } + + @Override + public HttpResponse execute(String method, String url, + Map headers, String body) { + HttpResponse r = responses.poll(); + if (r == null) throw new IllegalStateException("No response queued for " + method + " " + url); + return r; + } +} +``` + +See `DefaultHttpAdapter` for the complete production reference implementation. diff --git a/pom.xml b/pom.xml index 8ffaf60a..afcad0af 100644 --- a/pom.xml +++ b/pom.xml @@ -62,6 +62,7 @@ UTF-8 UTF-8 4.12.0 + 3.13.2 3.5.5 0.8.13 @@ -152,6 +153,18 @@ + + org.apache.maven.plugins + maven-jar-plugin + + + test-jar + + test-jar + + + + org.apache.maven.plugins maven-source-plugin @@ -238,9 +251,9 @@ ${okhttp3.version} - com.squareup.okhttp3 - mockwebserver - ${okhttp3.version} + org.wiremock + wiremock + ${wiremock.version} test diff --git a/src/main/java/com/recurly/v3/BaseClient.java b/src/main/java/com/recurly/v3/BaseClient.java index 17b0a8dc..fd05fc7e 100644 --- a/src/main/java/com/recurly/v3/BaseClient.java +++ b/src/main/java/com/recurly/v3/BaseClient.java @@ -2,54 +2,54 @@ import com.google.gson.annotations.SerializedName; import com.recurly.v3.exception.ExceptionFactory; -import com.recurly.v3.http.HeaderInterceptor; -import com.recurly.v3.ClientOptions; +import com.recurly.v3.http.DefaultHttpAdapter; +import com.recurly.v3.http.HttpAdapter; +import com.recurly.v3.http.HttpResponse; import java.io.IOException; +import java.io.InputStream; import java.io.UnsupportedEncodingException; import java.lang.reflect.Field; import java.lang.reflect.Type; -import java.math.BigDecimal; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Base64; import java.util.HashMap; -import java.util.Map; +import java.util.HashSet; import java.util.List; -import java.util.Arrays; +import java.util.Map; +import java.util.Properties; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; - -import okhttp3.*; -import okhttp3.Request.Builder; -import okhttp3.logging.HttpLoggingInterceptor; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; public abstract class BaseClient { private static final List BINARY_TYPES = Arrays.asList("application/pdf"); + private static final Set VALID_METHODS = + new HashSet<>(Arrays.asList("GET", "POST", "PUT", "DELETE", "HEAD")); + private static final String USER_AGENT = buildUserAgent(); private static final JsonSerializer jsonSerializer = new JsonSerializer(); private static final FileSerializer fileSerializer = new FileSerializer(); - private final String apiKey; - private final OkHttpClient client; + + private final String authToken; + private final HttpAdapter httpAdapter; private String apiUrl; protected BaseClient(final String apiKey) { - this(apiKey, newHttpClient(validateApiKey(apiKey)), new ClientOptions()); + this(apiKey, new ClientOptions()); } protected BaseClient(final String apiKey, final ClientOptions clientOptions) { - this(apiKey, newHttpClient(validateApiKey(apiKey)), clientOptions); - } - - protected BaseClient(final String apiKey, final OkHttpClient client) { - this(apiKey, client, new ClientOptions()); - } - - protected BaseClient(final String apiKey, final OkHttpClient client, final ClientOptions clientOptions) { - this.apiKey = validateApiKey(apiKey); - this.client = client; + this.authToken = buildAuthToken(validateApiKey(apiKey)); this.apiUrl = clientOptions.getBaseUrl(); + this.httpAdapter = + clientOptions.getHttpAdapter() != null + ? clientOptions.getHttpAdapter() + : new DefaultHttpAdapter(); } private static String validateApiKey(final String apiKey) { @@ -59,21 +59,82 @@ private static String validateApiKey(final String apiKey) { return apiKey; } - private static OkHttpClient newHttpClient(final String apiKey) { - final OkHttpClient.Builder httpClientBuilder = new OkHttpClient.Builder(); - - final String authToken = Credentials.basic(apiKey, ""); - final HeaderInterceptor headerInterceptor = - new HeaderInterceptor(authToken, Client.API_VERSION); - httpClientBuilder.addInterceptor(headerInterceptor); - - if (envEnabled("RECURLY_INSECURE") && envEnabled("RECURLY_DEBUG")) { - final HttpLoggingInterceptor logging = new HttpLoggingInterceptor(); - logging.setLevel(HttpLoggingInterceptor.Level.BASIC); - httpClientBuilder.addInterceptor(logging); + private static String buildAuthToken(final String apiKey) { + return "Basic " + + Base64.getEncoder() + .encodeToString((apiKey + ":").getBytes(StandardCharsets.ISO_8859_1)); + } + + private Map buildHeaders() { + return buildHeaders(null); + } + + private Map buildHeaders(final RequestOptions options) { + final Map headers = new HashMap<>(); + headers.put("Authorization", authToken); + headers.put("Accept", "application/vnd.recurly." + Client.API_VERSION); + headers.put("Content-Type", "application/json"); + headers.put("User-Agent", USER_AGENT); + + if (options != null) { + headers.putAll(options.getHeaders()); + if (options.getIdempotencyKey() != null) { + headers.put("Idempotency-Key", options.getIdempotencyKey()); + } } - return httpClientBuilder.build(); + return headers; + } + + private String buildUrl(final String path, final HashMap queryParams) { + if (queryParams == null || queryParams.isEmpty()) { + return this.apiUrl + path; + } + + final StringBuilder sb = new StringBuilder(this.apiUrl).append(path); + boolean first = true; + + for (final Map.Entry param : queryParams.entrySet()) { + final Object value = param.getValue(); + if (value == null) continue; + + final String stringValue; + if (value instanceof String) { + stringValue = value.toString(); + } else if (value instanceof ZonedDateTime) { + stringValue = DateTimeFormatter.ISO_OFFSET_DATE_TIME.format((ZonedDateTime) value); + } else if (value instanceof Integer) { + stringValue = Integer.toString((Integer) value); + } else if (value instanceof Float) { + stringValue = Float.toString((Float) value); + } else if (value instanceof Double) { + stringValue = Double.toString((Double) value); + } else if (value instanceof Long) { + stringValue = Long.toString((Long) value); + } else if (value instanceof Enum) { + stringValue = getSerializedEnumName((Enum) value); + } else { + stringValue = value.toString(); + } + + if (stringValue == null) continue; + + try { + sb.append(first ? "?" : "&") + .append(param.getKey()) + .append("=") + .append(URLEncoder.encode(stringValue, StandardCharsets.UTF_8.toString())); + first = false; + } catch (UnsupportedEncodingException ex) { + throw new RecurlyException(ex.getCause()); + } + } + + return sb.toString(); + } + + private static boolean isSuccessful(final int statusCode) { + return statusCode >= 200 && statusCode < 300; } protected static boolean envEnabled(final String envVar) { @@ -85,26 +146,25 @@ protected void makeRequest(final String method, final String url) { } protected void makeRequest(final String method, final String url, final RequestOptions options) { - final okhttp3.Request request = buildRequest(method, url, null, null, options); - - try (final Response response = client.newCall(request).execute()) { - if (!response.isSuccessful()) { - String responseString = response.body().string(); - if (envEnabled("RECURLY_INSECURE") && envEnabled("RECURLY_DEBUG")) { - System.out.println(responseString); - } - throw jsonSerializer.deserializeError(responseString); - } + validateMethod(method); + final String fullUrl = buildUrl(url, null); + final Map headers = buildHeaders(options); - final Headers responseHeaders = response.headers(); - - if (envEnabled("RECURLY_INSECURE") && envEnabled("RECURLY_DEBUG")) { - for (int i = 0; i < responseHeaders.size(); i++) { - System.out.println(responseHeaders.name(i) + ": " + responseHeaders.value(i)); + try { + final HttpResponse response = httpAdapter.execute(method, fullUrl, headers, null); + + if (!isSuccessful(response.getStatusCode())) { + final String contentType = + response.getHeaders().getOrDefault("content-type", "application/json"); + if (contentType.contains("application/json")) { + throw jsonSerializer.deserializeError( + new String(response.getBody(), StandardCharsets.UTF_8)); + } else { + throw ExceptionFactory.getExceptionClass(response); } } - this.warnIfDeprecated(responseHeaders); + warnIfDeprecated(response.getHeaders()); } catch (IOException e) { throw new NetworkException(e); @@ -162,28 +222,34 @@ protected T makeRequest( final HashMap queryParams, final RequestOptions options, final Type resourceClass) { - final okhttp3.Request request = buildRequest(method, url, body, queryParams, options); + validateMethod(method); + final String fullUrl = buildUrl(url, queryParams); + final String bodyString = body != null ? jsonSerializer.serialize(body) : null; + final Map headers = buildHeaders(options); - try (final Response response = client.newCall(request).execute()) { + try { + final HttpResponse response = httpAdapter.execute(method, fullUrl, headers, bodyString); - final Headers responseHeaders = response.headers(); - final ResponseBody responseBody = response.body(); - MediaType contentType = responseBody.contentType(); + final int statusCode = response.getStatusCode(); + final String contentType = + response.getHeaders().getOrDefault("content-type", "application/json"); - if (!response.isSuccessful()) { - if (contentType.type().equals("application") && contentType.subtype().equals("json")) { - throw jsonSerializer.deserializeError(responseBody.string()); + if (!isSuccessful(statusCode)) { + if (contentType.contains("application/json")) { + throw jsonSerializer.deserializeError( + new String(response.getBody(), StandardCharsets.UTF_8)); } else { throw ExceptionFactory.getExceptionClass(response); } } - this.warnIfDeprecated(responseHeaders); + warnIfDeprecated(response.getHeaders()); - if (BINARY_TYPES.contains(contentType.type() + "/" + contentType.subtype())) { - return fileSerializer.deserialize(responseBody.bytes(), resourceClass); + if (BINARY_TYPES.stream().anyMatch(contentType::startsWith)) { + return fileSerializer.deserialize(response.getBody(), resourceClass); } else { - return jsonSerializer.deserialize(responseBody.string(), resourceClass); + return jsonSerializer.deserialize( + new String(response.getBody(), StandardCharsets.UTF_8), resourceClass); } } catch (IOException e) { @@ -192,121 +258,51 @@ protected T makeRequest( } public int getRecordCount(final String url, final HashMap queryParams) { - final okhttp3.Request request = buildRequest("HEAD", url, null, queryParams, null); + final String fullUrl = buildUrl(url, queryParams); + final Map headers = buildHeaders(); - try (final Response response = client.newCall(request).execute()) { - - final Headers responseHeaders = response.headers(); - final ResponseBody responseBody = response.body(); + try { + final HttpResponse response = httpAdapter.execute("HEAD", fullUrl, headers, null); - if (!response.isSuccessful()) { - throw jsonSerializer.deserializeError(responseBody.string()); + if (!isSuccessful(response.getStatusCode())) { + throw ExceptionFactory.getExceptionClass(response); } - this.warnIfDeprecated(responseHeaders); + warnIfDeprecated(response.getHeaders()); - String count = responseHeaders.get("Recurly-Total-Records"); - return Integer.parseInt(count); + final String recordCount = response.getHeaders().get("recurly-total-records"); + try { + return Integer.parseInt(recordCount); + } catch (NumberFormatException e) { + throw new RecurlyException("Invalid recurly-total-records header value: " + recordCount); + } } catch (IOException e) { throw new NetworkException(e); } } - private String getSerializedEnumName(Enum e) { + private static void validateMethod(final String method) { + if (!VALID_METHODS.contains(method)) { + throw new IllegalArgumentException(method + " is not a valid Recurly HTTP method"); + } + } + + private String getSerializedEnumName(final Enum e) { try { - Field f = e.getClass().getField(e.name()); - SerializedName a = f.getAnnotation(SerializedName.class); + final Field f = e.getClass().getField(e.name()); + final SerializedName a = f.getAnnotation(SerializedName.class); return a == null ? null : a.value(); } catch (NoSuchFieldException ignored) { return null; } } - private okhttp3.Request buildRequest( - final String method, - final String url, - final Request body, - final HashMap queryParams, - final RequestOptions options) { - final HttpUrl.Builder httpBuilder = HttpUrl.parse(this.apiUrl + url).newBuilder(); - - final RequestBody requestBody = - RequestBody.create( - jsonSerializer.serialize(body), MediaType.parse("application/json; charset=utf-8")); - - if (queryParams != null) { - for (Map.Entry param : queryParams.entrySet()) { - final Object value = param.getValue(); - final String stringValue; - - if (value == null) { - continue; - } else if (value instanceof String) { - stringValue = value.toString(); - } else if (value instanceof ZonedDateTime) { - stringValue = DateTimeFormatter.ISO_OFFSET_DATE_TIME.format((ZonedDateTime) value); - } else if (value instanceof Integer) { - stringValue = Integer.toString((Integer) value); - } else if (value instanceof Float) { - stringValue = Float.toString((Float) value); - } else if (value instanceof Double) { - stringValue = Double.toString((Double) value); - } else if (value instanceof Long) { - stringValue = Long.toString((Long) value); - } else if (value instanceof Enum) { - stringValue = getSerializedEnumName((Enum)value); - } else { - stringValue = value.toString(); - } - - httpBuilder.addQueryParameter(param.getKey(), stringValue); - } - } - - final HttpUrl requestUrl = httpBuilder.build(); - - if (envEnabled("RECURLY_INSECURE") && envEnabled("RECURLY_DEBUG")) { - System.out.println("Performing " + method + " request to " + requestUrl); - } - - final Builder requestBuilder = new okhttp3.Request.Builder().url(requestUrl); - - if (options != null) { - for (Map.Entry entry : options.getHeaders().entrySet()) { - requestBuilder.header(entry.getKey(), entry.getValue()); - } - if (options.getIdempotencyKey() != null) { - requestBuilder.header("Idempotency-Key", options.getIdempotencyKey()); - } - } - - switch (method) { - case "HEAD": - return requestBuilder.head().build(); - - case "GET": - return requestBuilder.build(); - - case "POST": - return requestBuilder.post(requestBody).build(); - - case "PUT": - return requestBuilder.put(requestBody).build(); - - case "DELETE": - return requestBuilder.delete().build(); - - default: - String message = method + " is not a valid Recurly HTTP method"; - throw new IllegalArgumentException(message); - } - } - private void validatePathParameters(final HashMap urlParams) { - Map invalidParams = urlParams.entrySet().stream() - .filter(p -> p.getValue() == null || p.getValue().trim().isEmpty()) - .collect(Collectors.toMap(e->e.getKey(),e->e.getValue())); + Map invalidParams = + urlParams.entrySet().stream() + .filter(p -> p.getValue() == null || p.getValue().trim().isEmpty()) + .collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue())); if (!invalidParams.isEmpty()) { String invalidKeys = String.join(",", invalidParams.keySet()); throw new RecurlyException(invalidKeys + " cannot be an empty value"); @@ -325,10 +321,11 @@ protected String interpolatePath(String path, final HashMap urlP while (m.find()) { final String key = m.group(1).replace("{", "").replace("}", ""); try { - final String value = URLEncoder.encode(urlParams.get(key), StandardCharsets.UTF_8.toString()); + final String value = + URLEncoder.encode(urlParams.get(key), StandardCharsets.UTF_8.toString()); path = path.replace(m.group(1), value); } catch (UnsupportedEncodingException ex) { - throw new RecurlyException(ex.getCause()); + throw new RecurlyException(ex.getCause()); } } @@ -351,19 +348,37 @@ public String getApiUrl() { return this.apiUrl; } - private void warnIfDeprecated(Headers responseHeaders) { - String deprecated = responseHeaders.get("Recurly-Deprecated"); - - if (deprecated != null && deprecated.toUpperCase() == "TRUE") { - String sunset = responseHeaders.get("Recurly-Sunset-Date"); + private void warnIfDeprecated(final Map headers) { + final String deprecated = headers.get("recurly-deprecated"); - String warning = + if (deprecated != null && "TRUE".equalsIgnoreCase(deprecated)) { + final String sunset = headers.get("recurly-sunset-date"); + System.out.println( "[recurly-client-java] WARNING: Your current API version \"" + Client.API_VERSION + "\" is deprecated and will be sunset on " - + sunset; + + sunset); + } + } - System.out.println(warning); + private static String buildUserAgent() { + final String defaultVersion = "3.?.?"; + final String defaultJvmInfo = "?"; + final Properties properties = new Properties(); + + try (final InputStream inputStream = + BaseClient.class.getResourceAsStream("/version.properties")) { + if (inputStream != null) { + properties.load(inputStream); + final String version = properties.getProperty("version", defaultVersion); + final String jvmInfo = System.getProperty("java.version", defaultJvmInfo); + return String.format("Recurly/%s; java %s", version, jvmInfo); + } + } catch (Exception e) { + System.out.println("[Recurly][WARNING] " + e.toString()); } + + System.out.println("[Recurly][WARNING] Could not set user agent header."); + return String.format("Recurly/%s; java %s", defaultVersion, defaultJvmInfo); } } diff --git a/src/main/java/com/recurly/v3/ClientOptions.java b/src/main/java/com/recurly/v3/ClientOptions.java index eacf7f7f..0b739a94 100644 --- a/src/main/java/com/recurly/v3/ClientOptions.java +++ b/src/main/java/com/recurly/v3/ClientOptions.java @@ -1,4 +1,6 @@ package com.recurly.v3; + +import com.recurly.v3.http.HttpAdapter; import java.util.HashMap; public class ClientOptions { @@ -9,18 +11,20 @@ public enum Regions { }; private static final HashMap regionsMap = new HashMap<>(); + static { - regionsMap.put(Regions.US, "https://v3.recurly.com"); - regionsMap.put(Regions.EU, "https://v3.eu.recurly.com"); + regionsMap.put(Regions.US, "https://v3.recurly.com"); + regionsMap.put(Regions.EU, "https://v3.eu.recurly.com"); } private Regions region; + private HttpAdapter httpAdapter; public ClientOptions() { this.region = Regions.US; } - public void setRegion(Regions r) { + public void setRegion(final Regions r) { this.region = r; } @@ -28,4 +32,12 @@ public void setRegion(Regions r) { public String getBaseUrl() { return regionsMap.get(this.region); } + + public void setHttpAdapter(final HttpAdapter adapter) { + this.httpAdapter = adapter; + } + + public HttpAdapter getHttpAdapter() { + return httpAdapter; + } } \ No newline at end of file diff --git a/src/main/java/com/recurly/v3/exception/ExceptionFactory.java b/src/main/java/com/recurly/v3/exception/ExceptionFactory.java index 64e8d519..bf622f66 100644 --- a/src/main/java/com/recurly/v3/exception/ExceptionFactory.java +++ b/src/main/java/com/recurly/v3/exception/ExceptionFactory.java @@ -7,8 +7,8 @@ import com.recurly.v3.ApiException; import com.recurly.v3.RecurlyException; +import com.recurly.v3.http.HttpResponse; import com.recurly.v3.resources.ErrorMayHaveTransaction; -import okhttp3.Response; public class ExceptionFactory { @@ -79,9 +79,9 @@ public static T getExceptionClass(ApiException apiE } @SuppressWarnings("unchecked") - public static T getExceptionClass(Response response) { - String requestId = response.header("X-Request-Id", "none"); - int code = response.code(); + public static T getExceptionClass(HttpResponse response) { + String requestId = response.getHeaders().getOrDefault("x-request-id", "none"); + int code = response.getStatusCode(); String message = "Unexpected " + code + " Error. Recurly Request Id: " + requestId; switch (code) { case 500: diff --git a/src/main/java/com/recurly/v3/http/DefaultHttpAdapter.java b/src/main/java/com/recurly/v3/http/DefaultHttpAdapter.java new file mode 100644 index 00000000..65f3d8bd --- /dev/null +++ b/src/main/java/com/recurly/v3/http/DefaultHttpAdapter.java @@ -0,0 +1,98 @@ +package com.recurly.v3.http; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import okhttp3.Headers; +import okhttp3.MediaType; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.RequestBody; +import okhttp3.Response; +import okhttp3.ResponseBody; +import okhttp3.logging.HttpLoggingInterceptor; + +public class DefaultHttpAdapter implements HttpAdapter { + private static final int DEFAULT_TIMEOUT_MS = 60_000; + + private final OkHttpClient httpClient; + + public DefaultHttpAdapter() { + this(DEFAULT_TIMEOUT_MS); + } + + public DefaultHttpAdapter(final int timeoutMs) { + final OkHttpClient.Builder builder = + new OkHttpClient.Builder() + .connectTimeout(timeoutMs, TimeUnit.MILLISECONDS) + .readTimeout(timeoutMs, TimeUnit.MILLISECONDS) + .writeTimeout(timeoutMs, TimeUnit.MILLISECONDS); + + if (envEnabled("RECURLY_INSECURE") && envEnabled("RECURLY_DEBUG")) { + final HttpLoggingInterceptor logging = new HttpLoggingInterceptor(); + logging.setLevel(HttpLoggingInterceptor.Level.BASIC); + builder.addInterceptor(logging); + } + + this.httpClient = builder.build(); + } + + @Override + public HttpResponse execute( + final String method, + final String url, + final Map headers, + final String body) + throws IOException { + final Request.Builder requestBuilder = new Request.Builder().url(url); + + for (final Map.Entry header : headers.entrySet()) { + requestBuilder.header(header.getKey(), header.getValue()); + } + + final RequestBody requestBody = + body != null + ? RequestBody.create(body, MediaType.parse("application/json; charset=utf-8")) + : RequestBody.create(new byte[0]); + + switch (method) { + case "HEAD": + requestBuilder.head(); + break; + case "GET": + requestBuilder.get(); + break; + case "POST": + requestBuilder.post(requestBody); + break; + case "PUT": + requestBuilder.put(requestBody); + break; + case "DELETE": + requestBuilder.delete(); + break; + default: + throw new IllegalArgumentException(method + " is not a valid Recurly HTTP method"); + } + + try (final Response response = httpClient.newCall(requestBuilder.build()).execute()) { + final int statusCode = response.code(); + + final Map responseHeaders = new HashMap<>(); + final Headers okHeaders = response.headers(); + for (int i = 0; i < okHeaders.size(); i++) { + responseHeaders.put(okHeaders.name(i).toLowerCase(), okHeaders.value(i)); + } + + final ResponseBody responseBody = response.body(); + final byte[] responseBodyBytes = responseBody != null ? responseBody.bytes() : new byte[0]; + + return new HttpResponse(statusCode, responseHeaders, responseBodyBytes); + } + } + + private static boolean envEnabled(final String envVar) { + return "true".equals(System.getenv(envVar)); + } +} diff --git a/src/main/java/com/recurly/v3/http/HeaderInterceptor.java b/src/main/java/com/recurly/v3/http/HeaderInterceptor.java deleted file mode 100644 index 79f542db..00000000 --- a/src/main/java/com/recurly/v3/http/HeaderInterceptor.java +++ /dev/null @@ -1,58 +0,0 @@ -package com.recurly.v3.http; - -import java.io.IOException; -import java.io.InputStream; -import java.util.Properties; -import okhttp3.Interceptor; -import okhttp3.Request; -import okhttp3.Response; - -public class HeaderInterceptor implements Interceptor { - - private final String apiVersion; - private final String authToken; - private static String USER_AGENT = buildUserAgent(); - - public HeaderInterceptor(final String authToken, final String apiVersion) { - this.apiVersion = apiVersion; - this.authToken = authToken; - } - - public Response intercept(final Chain chain) throws IOException { - Request original = chain.request(); - - Request.Builder builder = - original - .newBuilder() - .header("Authorization", authToken) - .header("Accept", "application/vnd.recurly." + apiVersion) - .header("Content-Type", "application/json") - .header("User-Agent", USER_AGENT); - - Request request = builder.build(); - return chain.proceed(request); - } - - private static String buildUserAgent() { - final String defaultVersion = "3.?.?"; - final String defaultJvmInfo = "?"; - final Properties properties = new Properties(); - - try { - final InputStream inputStream = - HeaderInterceptor.class.getResourceAsStream("/version.properties"); - if (inputStream != null) { - properties.load(inputStream); - final String version = properties.getProperty("version", defaultVersion); - final String jvmInfo = System.getProperty("java.version", defaultJvmInfo); - return String.format("Recurly/%s; java %s", version, jvmInfo); - } - } catch (Exception e) { - // TODO rethrow exception in strict-mode - System.out.println("[Recurly][WARNING] " + e.getStackTrace().toString()); - } - - System.out.println("[Recurly][WARNING] Could not set user agent header."); - return String.format("Recurly/%s; java %s", defaultVersion, defaultJvmInfo); - } -} diff --git a/src/main/java/com/recurly/v3/http/HttpAdapter.java b/src/main/java/com/recurly/v3/http/HttpAdapter.java new file mode 100644 index 00000000..16703f26 --- /dev/null +++ b/src/main/java/com/recurly/v3/http/HttpAdapter.java @@ -0,0 +1,85 @@ +package com.recurly.v3.http; + +import java.io.IOException; +import java.util.Map; + +/** + * Pluggable HTTP transport layer for the Recurly Java client. + * + *

The client ships with {@link DefaultHttpAdapter}. Implement this interface when you need to + * supply a different HTTP library, add middleware (logging, metrics, proxy routing), or inject a + * fake transport in tests. + * + *

Registration + * + * 

{@code
+ * ClientOptions options = new ClientOptions();
+ * options.setHttpAdapter(new MyHttpAdapter());
+ * Client client = new Client(apiKey, options);
+ * }
+ * + *

Thread safety
+ * A single {@code HttpAdapter} instance is shared across all requests made by a {@code Client}. + * Implementations must be safe for concurrent use from multiple threads. + * + *

Connection pooling
+ * Implementations should reuse connections across calls (e.g. via an {@code HttpClient} instance + * held as a field) to avoid the overhead of establishing a new TCP connection on every request. + * + *

Timeouts
+ * The client imposes no timeout of its own. Set connect, read, and write timeouts inside the + * implementation. + * + *

See {@code docs/http-adapter-implementation-guide.md} for a full contract reference and + * worked examples. + */ +public interface HttpAdapter { + + /** + * Executes a single HTTP request and returns the complete response. + * + *

Parameters + * + *

    + *
  • {@code method} — always one of {@code GET}, {@code POST}, {@code PUT}, {@code DELETE}, + * {@code HEAD}. + *
  • {@code url} — fully-qualified URL including scheme, host, path, and any query string. + * Never {@code null}. + *
  • {@code headers} — all request headers the client wants sent (Authorization, + * Accept, Content-Type, User-Agent, etc.). Forward every entry without modification; + * do not add, remove, or override headers in the adapter. + *
  • {@code body} — UTF-8 JSON string for {@code POST} and {@code PUT} requests; {@code null} + * for {@code GET}, {@code HEAD}, and {@code DELETE}. + *
+ * + *

Return value
+ * Return an {@link HttpResponse} containing: + * + *

    + *
  • The HTTP status code exactly as received. + *
  • All response headers. {@link HttpResponse} normalises header names to lower-case + * internally, so case in the map passed to the constructor does not matter. The client + * reads {@code content-type}, {@code x-request-id}, {@code recurly-deprecated}, + * {@code recurly-sunset-date}, and {@code recurly-total-records}. + *
  • The complete response body as a byte array. Read the body fully before returning; do not + * return a lazy or streaming reference. Pass an empty {@code byte[]} (never {@code null}) + * when there is no body. + *
+ * + *

Error handling
+ * Throw {@link IOException} for any network-level failure (connection refused, timeout, TLS + * error, etc.). The client wraps it in a {@code NetworkException}. Do not throw for + * HTTP-level errors (4xx/5xx) — return the response and let the client map those to typed + * exceptions. + * + * @param method HTTP method ({@code GET}, {@code POST}, {@code PUT}, {@code DELETE}, + * {@code HEAD}) + * @param url absolute URL to request + * @param headers request headers to send; must be forwarded unmodified + * @param body request body as a JSON string, or {@code null} if there is no body + * @return the complete HTTP response + * @throws IOException on network or I/O failure + */ + HttpResponse execute(String method, String url, Map headers, String body) + throws IOException; +} diff --git a/src/main/java/com/recurly/v3/http/HttpResponse.java b/src/main/java/com/recurly/v3/http/HttpResponse.java new file mode 100644 index 00000000..8ba8f2e4 --- /dev/null +++ b/src/main/java/com/recurly/v3/http/HttpResponse.java @@ -0,0 +1,39 @@ +package com.recurly.v3.http; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +public class HttpResponse { + private final int statusCode; + private final Map headers; + private final byte[] body; + + /** + * Constructs an immutable HTTP response snapshot. The {@code body} array is defensively copied; + * callers may not observe mutations to the original array through this object. + */ + public HttpResponse(final int statusCode, final Map headers, final byte[] body) { + Objects.requireNonNull(headers, "headers must not be null"); + Objects.requireNonNull(body, "body must not be null"); + this.statusCode = statusCode; + final Map normalized = new HashMap<>(); + headers.forEach((k, v) -> normalized.put(k.toLowerCase(), v)); + this.headers = Collections.unmodifiableMap(normalized); + this.body = body.clone(); + } + + public int getStatusCode() { + return statusCode; + } + + public Map getHeaders() { + return headers; + } + + /** Returns a copy of the response body. Mutations to the returned array do not affect this object. */ + public byte[] getBody() { + return body.clone(); + } +} diff --git a/src/test/java/com/recurly/v3/BaseClientTest.java b/src/test/java/com/recurly/v3/BaseClientTest.java index bd15991e..19e40dbf 100644 --- a/src/test/java/com/recurly/v3/BaseClientTest.java +++ b/src/test/java/com/recurly/v3/BaseClientTest.java @@ -7,155 +7,109 @@ import com.recurly.v3.exception.TransactionException; import com.recurly.v3.exception.ValidationException; import com.recurly.v3.fixtures.FixtureConstants; -import com.recurly.v3.ApiException; import com.recurly.v3.fixtures.MockClient; import com.recurly.v3.fixtures.MockQueryParams; import com.recurly.v3.fixtures.MyRequest; import com.recurly.v3.fixtures.MyResource; import com.recurly.v3.RequestOptions; -import okhttp3.Call; -import okhttp3.Headers; -import okhttp3.HttpUrl; -import okhttp3.MediaType; -import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.Response; - +import com.recurly.v3.http.HttpAdapter; +import com.recurly.v3.http.HttpResponse; import org.apache.commons.io.IOUtils; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; -import org.junit.Assert; import org.junit.jupiter.api.Test; -import org.mockito.stubbing.Answer; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; import java.io.IOException; import java.io.InputStream; -import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; -import org.mockito.MockedStatic; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.mockStatic; -import static org.mockito.Mockito.when; -import static org.mockito.Mockito.eq; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.*; @SuppressWarnings("unchecked") public class BaseClientTest { - @Test - public void testMakeRequestWithResource() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { - Request request = i.getArgument(0); - HttpUrl url = request.url(); - assertEquals("GET", request.method()); - assertEquals("/resources/code-aaron", url.url().getPath()); - return mCall; - }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(200, "OK", getResponseJson())); + private static HttpResponse jsonResponse(final int statusCode, final String body) { + final Map headers = new HashMap<>(); + headers.put("content-type", "application/json; charset=utf-8"); + return new HttpResponse(statusCode, headers, body.getBytes(StandardCharsets.UTF_8)); + } - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); + private static HttpResponse htmlResponse(final int statusCode, final String body) { + final Map headers = new HashMap<>(); + headers.put("content-type", "text/html; charset=UTF-8"); + return new HttpResponse(statusCode, headers, body.getBytes(StandardCharsets.UTF_8)); + } - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - final MyResource resource = client.getResource("code-aaron"); + private static HttpResponse headResponse(final int statusCode, final String recordCount) { + final Map headers = new HashMap<>(); + headers.put("recurly-total-records", recordCount); + return new HttpResponse(statusCode, headers, new byte[0]); + } + private static MockClient mockClientWith(final HttpAdapter adapter) { + final ClientOptions options = new ClientOptions(); + options.setHttpAdapter(adapter); + return new MockClient("apiKey", options); + } + + @Test + public void testMakeRequestWithResource() throws IOException { + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(eq("GET"), contains("/resources/code-aaron"), any(), isNull())) + .thenReturn(jsonResponse(200, getResponseJson())); + + final MyResource resource = mockClientWith(mockAdapter).getResource("code-aaron"); assertEquals(MyResource.class, resource.getClass()); } @Test public void testMakeRequestWithBody() throws IOException { - final Call mCall = mock(Call.class); - AtomicBoolean postCalled = new AtomicBoolean(false); - AtomicBoolean putCalled = new AtomicBoolean(false); - Answer answer = (i) -> { - Request request = i.getArgument(0); - HttpUrl url = request.url(); - switch (request.method()) { - case "POST": - assertEquals("/resources", url.url().getPath()); - postCalled.set(true); - break; - case "PUT": - assertEquals("/resources/someId", url.url().getPath()); - putCalled.set(true); - break; - default: - // Any other request method is a failure - Assert.fail(); - } - return mCall; - }; - when(mCall.execute()) - .thenReturn(MockClient.buildResponse(200, "OK", getResponseJson())) - .thenReturn(MockClient.buildResponse(200, "OK", getResponseJson())); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(200, getResponseJson())); - final MockClient client = new MockClient("apiKey", mockOkHttpClient); + final MockClient client = mockClientWith(mockAdapter); final MyRequest newResource = new MyRequest(); newResource.setMyString("aaron"); final MyResource resource = client.createResource(newResource); + verify(mockAdapter).execute(eq("POST"), contains("/resources"), any(), notNull()); assertEquals(MyResource.class, resource.getClass()); assertEquals("aaron", resource.getMyString()); - assertTrue(postCalled.get()); final MyResource anotherResource = client.updateResource("someId", newResource); + verify(mockAdapter).execute(eq("PUT"), contains("/resources/someId"), any(), notNull()); assertEquals(MyResource.class, anotherResource.getClass()); assertEquals("aaron", anotherResource.getMyString()); - assertTrue(putCalled.get()); } @Test public void testMakeRequestWithoutResource() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { - Request request = i.getArgument(0); - HttpUrl url = request.url(); - assertEquals("DELETE", request.method()); - assertEquals("/resources/resource-id", url.url().getPath()); - return mCall; - }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(200, "OK", "")); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(eq("DELETE"), contains("/resources/resource-id"), any(), isNull())) + .thenReturn(jsonResponse(200, "")); - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - client.removeResource("resource-id"); + mockClientWith(mockAdapter).removeResource("resource-id"); + verify(mockAdapter).execute(eq("DELETE"), contains("/resources/resource-id"), any(), isNull()); } @Test public void testMakeRequestWithQueryParams() throws IOException { - ZonedDateTime dateTime = ZonedDateTime.now(); + final ZonedDateTime dateTime = ZonedDateTime.now(); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(200, getResponseListJson())); - final Call mCall = mock(Call.class); - Answer answer = (i) -> { - Request request = i.getArgument(0); - HttpUrl url = request.url(); - assertEquals("Aaron", url.queryParameter("my_string")); - assertEquals(DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(dateTime), url.queryParameter("my_date_time")); - assertEquals("1", url.queryParameter("my_integer")); - assertEquals("2.3", url.queryParameter("my_float")); - assertEquals("4.5", url.queryParameter("my_double")); - assertEquals("6", url.queryParameter("my_long")); - assertEquals("twenty-three", url.queryParameter("my_enum")); - assertEquals(null, url.queryParameter("my_random")); - assertEquals("[]", url.queryParameter("unsupported")); - return mCall; - }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(200, "OK", getResponseListJson())); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); final MockQueryParams qp = new MockQueryParams(); qp.setMyString("Aaron"); qp.setMyDateTime(dateTime); @@ -166,186 +120,128 @@ public void testMakeRequestWithQueryParams() throws IOException { qp.setMyEnum(FixtureConstants.ConstantType.TWENTY_THREE); qp.setMyRandom(null); qp.setUnsupported(new ArrayList<>()); - final Pager pager = client.listResources(qp); - pager.getNextPage(); + + final ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(String.class); + mockClientWith(mockAdapter).listResources(qp).getNextPage(); + + verify(mockAdapter).execute(eq("GET"), urlCaptor.capture(), any(), isNull()); + final String url = urlCaptor.getValue(); + + assertTrue(url.contains("my_string=Aaron")); + assertTrue(url.contains("my_date_time=" + DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(dateTime).replace(":", "%3A").replace("+", "%2B"))); + assertTrue(url.contains("my_integer=1")); + assertTrue(url.contains("my_float=2.3")); + assertTrue(url.contains("my_double=4.5")); + assertTrue(url.contains("my_long=6")); + assertTrue(url.contains("my_enum=twenty-three")); + assertFalse(url.contains("my_random")); + assertTrue(url.contains("unsupported=%5B%5D")); } @Test public void testNonJsonError0() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - Headers headers = new Headers.Builder().build(); - MediaType contentType = MediaType.get("text/html; charset=UTF-8"); - when(mCall.execute()).thenReturn(MockClient.buildResponse(0, "Not A Real Status", "badness", headers, contentType)); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(htmlResponse(0, "badness")); - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - - assertThrows( - ApiException.class, - () -> { - client.getResource("code-aaron"); - }); + assertThrows(ApiException.class, () -> mockClientWith(mockAdapter).getResource("code-aaron")); } @Test public void testNonJsonError500() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - Headers headers = new Headers.Builder().build(); - MediaType contentType = MediaType.get("text/html; charset=UTF-8"); - when(mCall.execute()).thenReturn(MockClient.buildResponse(500, "Internal Server Error", "badness", headers, contentType)); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(htmlResponse(500, "badness")); assertThrows( InternalServerException.class, - () -> { - client.getResource("code-aaron"); - }); + () -> mockClientWith(mockAdapter).getResource("code-aaron")); } @Test public void testInvalidApiKey() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(404, "Not Found", getErrorJson("invalid_api_key"))); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(404, getErrorJson("invalid_api_key"))); - // This test is important because it ensures that application/json response errors are based on the json - // body's error type and not the status code based error assertThrows( InvalidApiKeyException.class, - () -> { - client.getResource("code-aaron"); - }); + () -> mockClientWith(mockAdapter).getResource("code-aaron")); } @Test public void testNotFoundError() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(404, "Not Found", getErrorJson("not_found"))); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(404, getErrorJson("not_found"))); assertThrows( NotFoundException.class, - () -> { - client.getResource("code-aaron"); - }); + () -> mockClientWith(mockAdapter).getResource("code-aaron")); } @Test public void testUnknownError() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - final Response response = MockClient.buildResponse(999, "Unknown", getErrorJson("unknown")); - when(mCall.execute()).thenReturn(response); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(999, getErrorJson("unknown"))); - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); + assertThrows(ApiException.class, () -> mockClientWith(mockAdapter).getResource("code-aaron")); - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - // asserts that generic api exception is thrown for unknown error - assertThrows( - ApiException.class, - () -> { - client.getResource("code-aaron"); - }); - final RecurlyException exception = ExceptionFactory.getExceptionClass(response); + final Map headers = new HashMap<>(); + headers.put("content-type", "application/json"); + final HttpResponse httpResponse = + new HttpResponse(999, headers, getErrorJson("unknown").getBytes(StandardCharsets.UTF_8)); + final RecurlyException exception = ExceptionFactory.getExceptionClass(httpResponse); assertTrue(exception.toString().contains("ApiException")); } @Test public void testValidationError() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(422, "Unprocessable Entity", getErrorResponse("validation"))); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(422, getErrorResponse("validation"))); assertThrows( ValidationException.class, - () -> { - client.removeResource("code-aaron"); - }); + () -> mockClientWith(mockAdapter).removeResource("code-aaron")); } @Test public void testTransactionError() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(422, "Unprocessable Entity", getErrorResponse("transaction"))); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(422, getErrorResponse("transaction"))); - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - - TransactionException t = assertThrows( + final TransactionException t = + assertThrows( TransactionException.class, - () -> { - client.removeResource("code-aaron"); - }); + () -> mockClientWith(mockAdapter).removeResource("code-aaron")); assertEquals("mbca9aaao6xr", t.getError().getTransactionError().getTransactionId()); } @Test public void testNetworkError() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - when(mCall.execute()).thenThrow(new IOException()); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())).thenThrow(new IOException()); - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - assertThrows( - NetworkException.class, - () -> { - client.getResource("code-aaron"); - }); + assertThrows(NetworkException.class, () -> mockClientWith(mockAdapter).getResource("code-aaron")); } @Test public void testNetworkErrorWithoutResource() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - when(mCall.execute()).thenThrow(new IOException()); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())).thenThrow(new IOException()); - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); assertThrows( NetworkException.class, - () -> { - client.removeResource("code-aaron"); - }); + () -> mockClientWith(mockAdapter).removeResource("code-aaron")); } @Test - public void testBadMethodError() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - when(mCall.execute()).thenThrow(new IOException()); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - - assertThrows( - IllegalArgumentException.class, - () -> { - client.badRequestMethod(); - }); + public void testBadMethodError() { + final MockClient client = new MockClient("apiKey"); + assertThrows(IllegalArgumentException.class, () -> client.badRequestMethod()); } @Test @@ -362,14 +258,13 @@ public void testSetApiUrl() { } @Test - public void testCantSetApiUrlWithoutRecurlyInsecure() throws Exception { + public void testCantSetApiUrlWithoutRecurlyInsecure() { try (MockedStatic theMock = mockStatic(BaseClient.class)) { theMock.when(() -> BaseClient.envEnabled(eq("RECURLY_INSECURE"))).thenReturn(false); final MockClient client = new MockClient("apiKey"); final String originalUrl = client.getApiUrl(); - final String newApiUrl = "https://my.base.url/"; - client._setApiUrl(newApiUrl); + client._setApiUrl("https://my.base.url/"); assertEquals(originalUrl, client.getApiUrl()); } @@ -377,25 +272,21 @@ public void testCantSetApiUrlWithoutRecurlyInsecure() throws Exception { @Test public void testWithoutClientOptions() { - // The default region should be ClientOptions.Regions.US - final MockClient client = new MockClient("apiKey"); - assertEquals("https://v3.recurly.com", client.getApiUrl()); + assertEquals("https://v3.recurly.com", new MockClient("apiKey").getApiUrl()); } @Test public void testUsingRegionUSClientOptions() { final ClientOptions options = new ClientOptions(); options.setRegion(ClientOptions.Regions.US); - final MockClient client = new MockClient("apiKey", options); - assertEquals("https://v3.recurly.com", client.getApiUrl()); + assertEquals("https://v3.recurly.com", new MockClient("apiKey", options).getApiUrl()); } @Test public void testUsingRegionEUClientOptions() { final ClientOptions options = new ClientOptions(); options.setRegion(ClientOptions.Regions.EU); - final MockClient client = new MockClient("apiKey", options); - assertEquals("https://v3.eu.recurly.com", client.getApiUrl()); + assertEquals("https://v3.eu.recurly.com", new MockClient("apiKey", options).getApiUrl()); } @Test @@ -458,49 +349,84 @@ public void testNoIdempotencyKeyHeader() throws IOException { @Test public void testInterpolatePathWithoutParams() { - final MockClient client = new MockClient("apiKey"); - final String path = "/accounts"; - final String interpolatedPath = client.interpolatePath(path); - - assertEquals("/accounts", interpolatedPath); + assertEquals("/accounts", new MockClient("apiKey").interpolatePath("/accounts")); } @Test public void testInterpolatePathWithParams() { - final MockClient client = new MockClient("apiKey"); - final String path = "/accounts/{account_id}/notes/{account_note_id}"; - final HashMap urlParams = new HashMap(); + final HashMap urlParams = new HashMap<>(); urlParams.put("account_id", "accountId/"); urlParams.put("account_note_id", "noteId,"); - final String interpolatedPath = client.interpolatePath(path, urlParams); + assertEquals( + "/accounts/accountId%2F/notes/noteId%2C", + new MockClient("apiKey") + .interpolatePath("/accounts/{account_id}/notes/{account_note_id}", urlParams)); + } + + @Test + public void testGetRecordCountNonSuccessThrowsRecurlyException() throws IOException { + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(eq("HEAD"), any(), any(), isNull())) + .thenReturn(new HttpResponse(404, new HashMap<>(), new byte[0])); - assertEquals("/accounts/accountId%2F/notes/noteId%2C", interpolatedPath); + assertThrows( + RecurlyException.class, + () -> mockClientWith(mockAdapter).getRecordCount("/resources", null)); + } + + @Test + public void testGetRecordCountMissingHeader() throws IOException { + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + final Map headers = new HashMap<>(); + when(mockAdapter.execute(eq("HEAD"), any(), any(), isNull())) + .thenReturn(new HttpResponse(200, headers, new byte[0])); + + assertThrows( + RecurlyException.class, + () -> mockClientWith(mockAdapter).getRecordCount("/resources", null)); + } + + @Test + public void testHttpResponseNullHeadersThrows() { + assertThrows(NullPointerException.class, () -> new HttpResponse(200, null, new byte[0])); + } + + @Test + public void testMixedCaseHeadersAreNormalized() throws IOException { + // Without normalization BaseClient looks up "content-type" but finds nothing (key is + // "Content-Type"), falls back to "application/json", and tries to JSON-parse an HTML body. + // With normalization it correctly routes to ExceptionFactory → InternalServerException. + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + final Map headers = new HashMap<>(); + headers.put("Content-Type", "text/html; charset=UTF-8"); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(new HttpResponse(500, headers, "error".getBytes(StandardCharsets.UTF_8))); + + assertThrows( + InternalServerException.class, + () -> mockClientWith(mockAdapter).getResource("code-aaron")); } @Test public void testInterpolatePathValidations() { - final MockClient client = new MockClient("apiKey"); - final String path = "/accounts/{account_id}/notes/{account_note_id}"; - final HashMap urlParams = new HashMap(); + final HashMap urlParams = new HashMap<>(); urlParams.put("account_id", ""); urlParams.put("account_note_id", ""); - assertThrows( RecurlyException.class, - () -> { - client.interpolatePath(path, urlParams); - }); + () -> + new MockClient("apiKey") + .interpolatePath( + "/accounts/{account_id}/notes/{account_note_id}", urlParams)); } @Test public void testInterpolatePathMatching() { - final MockClient client = new MockClient("apiKey"); - final String path = "/url_path/{url_path}"; - final HashMap urlParams = new HashMap(); + final HashMap urlParams = new HashMap<>(); urlParams.put("url_path", "replacement"); - - final String interpolatedPath = client.interpolatePath(path, urlParams); - assertEquals("/url_path/replacement", interpolatedPath); + assertEquals( + "/url_path/replacement", + new MockClient("apiKey").interpolatePath("/url_path/{url_path}", urlParams)); } private static String getResponseJson() { @@ -521,7 +447,7 @@ private static String getResponseListJson() { + "}"; } - private static String getErrorJson(String exception) { + private static String getErrorJson(final String exception) { return "" + "{\n" + " \"error\": {\n" @@ -536,7 +462,7 @@ private static String getErrorJson(String exception) { + "}"; } - private static String getErrorResponse(String exception) { + private static String getErrorResponse(final String exception) { InputStream resource = null; if ("validation".equals(exception)) { diff --git a/src/test/java/com/recurly/v3/HeaderInterceptorTest.java b/src/test/java/com/recurly/v3/HeaderInterceptorTest.java deleted file mode 100644 index e4ff015e..00000000 --- a/src/test/java/com/recurly/v3/HeaderInterceptorTest.java +++ /dev/null @@ -1,42 +0,0 @@ -package com.recurly.v3; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -import com.recurly.v3.http.HeaderInterceptor; -import java.io.IOException; -import okhttp3.*; -import okhttp3.Request; -import okhttp3.mockwebserver.MockResponse; -import okhttp3.mockwebserver.MockWebServer; -import okhttp3.mockwebserver.RecordedRequest; -import org.junit.jupiter.api.Test; - -public class HeaderInterceptorTest { - @Test - public void testHttpHeaders() throws IOException, InterruptedException { - MockWebServer mockWebServer = new MockWebServer(); - mockWebServer.start(); - mockWebServer.enqueue(new MockResponse()); - - OkHttpClient okHttpClient = - new OkHttpClient() - .newBuilder() - .addInterceptor(new HeaderInterceptor("apikey", "version")) - .build(); - - okHttpClient.newCall(new Request.Builder().url(mockWebServer.url("/")).build()).execute(); - - RecordedRequest request = mockWebServer.takeRequest(); - assertEquals("apikey", request.getHeader("Authorization")); - assertEquals("application/vnd.recurly.version", request.getHeader("Accept")); - assertEquals("application/json", request.getHeader("Content-Type")); - - // TODO this regex will change on GA - // BETA semver sequence is forced until then - final String agentFormat = - "Recurly/\\d+\\.\\d+\\.\\d+(-SNAPSHOT)?;\\s+java\\s+\\d+.*"; - assertEquals(request.getHeader("User-Agent").matches(agentFormat), true); - - mockWebServer.shutdown(); - } -} diff --git a/src/test/java/com/recurly/v3/PagerTest.java b/src/test/java/com/recurly/v3/PagerTest.java index 8b3a68e6..5485e2f9 100644 --- a/src/test/java/com/recurly/v3/PagerTest.java +++ b/src/test/java/com/recurly/v3/PagerTest.java @@ -5,38 +5,41 @@ import com.recurly.v3.fixtures.MockClient; import com.recurly.v3.fixtures.MyResource; +import com.recurly.v3.http.HttpAdapter; +import com.recurly.v3.http.HttpResponse; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; import java.util.NoSuchElementException; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import okhttp3.*; -import okhttp3.Request; import org.junit.jupiter.api.Test; -import org.mockito.stubbing.Answer; public class PagerTest { + + private static HttpResponse jsonResponse(final int statusCode, final String body) { + final Map headers = new HashMap<>(); + headers.put("content-type", "application/json; charset=utf-8"); + return new HttpResponse(statusCode, headers, body.getBytes(StandardCharsets.UTF_8)); + } + + private static MockClient mockClientWith(final HttpAdapter adapter) { + final ClientOptions options = new ClientOptions(); + options.setHttpAdapter(adapter); + return new MockClient("apiKey", options); + } + @Test public void testForEach() throws IOException { - final Call mCall = mock(Call.class); - AtomicBoolean firstCalled = new AtomicBoolean(false); - Answer answer = (i) -> { - Request request = i.getArgument(0); - HttpUrl url = request.url(); - if (firstCalled.get()) { - assertEquals("/next", url.url().getPath()); - } - firstCalled.set(true); - return mCall; - }; - when(mCall.execute()) - .thenReturn(MockClient.buildResponse(200, "OK", getResourceFirstPageJson("/next"))) - .thenReturn(MockClient.buildResponse(200, "OK", getResourceSecondPageJson())); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - Pager pager = client.listResources(null); - AtomicInteger count = new AtomicInteger(0); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(200, getResourceFirstPageJson("/next"))) + .thenReturn(jsonResponse(200, getResourceSecondPageJson())); + + final MockClient client = mockClientWith(mockAdapter); + final Pager pager = client.listResources(null); + final AtomicInteger count = new AtomicInteger(0); + pager.forEach( resource -> { if (count.get() < 3) { @@ -50,58 +53,40 @@ public void testForEach() throws IOException { @Test public void testEachItem() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(200, "OK", getResourceSecondPageJson())); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(200, getResourceSecondPageJson())); - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - Pager pager = client.listResources(null); + final MockClient client = mockClientWith(mockAdapter); + final Pager pager = client.listResources(null); pager.eachItem(resource -> assertNotNull(resource.getMyString())); } @Test public void testEmptyList() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { return mCall; }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(200, "OK", getEmptyListJson())); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(200, getEmptyListJson())); - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - Pager pager = client.listResources(null); + final MockClient client = mockClientWith(mockAdapter); + final Pager pager = client.listResources(null); assertEquals(0, pager.getData().size()); for (MyResource myResource : pager) { - myResource.getMyString(); // This should not throw NullPointerException + myResource.getMyString(); } - pager.forEach( - myResource -> - myResource.getMyString()); // This should not throw NullPointerException either + pager.forEach(myResource -> myResource.getMyString()); } @Test public void testForLoop() throws IOException { - final Call mCall = mock(Call.class); - AtomicBoolean firstCalled = new AtomicBoolean(false); - Answer answer = (i) -> { - Request request = i.getArgument(0); - HttpUrl url = request.url(); - if (firstCalled.get()) { - assertEquals("/next", url.url().getPath()); - } - firstCalled.set(true); - return mCall; - }; - when(mCall.execute()) - .thenReturn(MockClient.buildResponse(200, "OK", getResourceFirstPageJson("/next"))) - .thenReturn(MockClient.buildResponse(200, "OK", getResourceSecondPageJson())); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(200, getResourceFirstPageJson("/next"))) + .thenReturn(jsonResponse(200, getResourceSecondPageJson())); - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - Pager pager = client.listResources(null); + final MockClient client = mockClientWith(mockAdapter); + final Pager pager = client.listResources(null); int count = 0; for (MyResource res : pager) { if (count < 3) { @@ -116,67 +101,50 @@ public void testForLoop() throws IOException { @Test public void testNullNextPage() { - Pager pager = new Pager<>(null, null, null, null); - + final Pager pager = new Pager<>(null, null, null, null); assertThrows(NoSuchElementException.class, () -> pager.getNextPage()); } @Test public void testCount() throws IOException { - final Call mCall = mock(Call.class); - Headers headers = new Headers.Builder().set("Recurly-Total-Records", "1337").build(); - Answer answer = (i) -> { - Request request = i.getArgument(0); - assertEquals("HEAD", request.method()); - return mCall; - }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(200, "OK", getResourceFirstItemJson(), headers)); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - Pager pager = client.listResources(null); - int count = pager.getCount(); - assertEquals(1337, count); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + final Map headers = new HashMap<>(); + headers.put("recurly-total-records", "1337"); + when(mockAdapter.execute(eq("HEAD"), any(), any(), any())) + .thenReturn(new HttpResponse(200, headers, new byte[0])); + + final MockClient client = mockClientWith(mockAdapter); + final Pager pager = client.listResources(null); + assertEquals(1337, pager.getCount()); } @Test public void testFirst() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { - Request request = i.getArgument(0); - HttpUrl url = request.url(); - assertEquals("1", url.queryParameter("limit")); - return mCall; - }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(200, "OK", getResourceFirstItemJson())); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); - Pager pager = client.listResources(null); - MyResource resource = pager.getFirst(); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(jsonResponse(200, getResourceFirstItemJson())); + + final MockClient client = mockClientWith(mockAdapter); + final Pager pager = client.listResources(null); + final MyResource resource = pager.getFirst(); + + verify(mockAdapter).execute(eq("GET"), contains("limit=1"), any(), any()); assertEquals("Resource First Item", resource.getMyString()); } - private String getResourceFirstPageJson(String next) { + private String getResourceFirstPageJson(final String next) { return "" + "{" + "\"object\":\"list\"," + "\"has_more\":true," - + "\"next\":\"" + next + "\"," + + "\"next\":\"" + + next + + "\"," + "\"data\": [" - + "{" - + "\"my_string\":\"Resource Page 1\"" - + "}," - + "{" - + "\"my_string\":\"Resource Page 1\"" - + "}," - + "{" - + "\"my_string\":\"Resource Page 1\"" - + "}" - + "]" - + "}"; + + "{\"my_string\":\"Resource Page 1\"}," + + "{\"my_string\":\"Resource Page 1\"}," + + "{\"my_string\":\"Resource Page 1\"}" + + "]}"; } private String getResourceSecondPageJson() { @@ -186,14 +154,9 @@ private String getResourceSecondPageJson() { + "\"has_more\":false," + "\"next\":null," + "\"data\": [" - + "{" - + "\"my_string\":\"Resource Page 2\"" - + "}," - + "{" - + "\"my_string\":\"Resource Page 2\"" - + "}" - + "]" - + "}"; + + "{\"my_string\":\"Resource Page 2\"}," + + "{\"my_string\":\"Resource Page 2\"}" + + "]}"; } private String getEmptyListJson() { @@ -201,14 +164,10 @@ private String getEmptyListJson() { } private String getResourceFirstItemJson() { - return "{" + - "\"object\": \"list\"," + - "\"has_more\": false," + - "\"next\": null," + - "\"data\": [" + - " {" + - " \"my_string\":\"Resource First Item\"" + - " }" + - "]}"; + return "{" + + "\"object\": \"list\"," + + "\"has_more\": false," + + "\"next\": null," + + "\"data\": [{\"my_string\":\"Resource First Item\"}]}"; } } diff --git a/src/test/java/com/recurly/v3/fixtures/MockClient.java b/src/test/java/com/recurly/v3/fixtures/MockClient.java index 179297fb..fd7ea93f 100644 --- a/src/test/java/com/recurly/v3/fixtures/MockClient.java +++ b/src/test/java/com/recurly/v3/fixtures/MockClient.java @@ -2,51 +2,27 @@ import com.google.gson.reflect.TypeToken; import com.recurly.v3.BaseClient; -import com.recurly.v3.Pager; import com.recurly.v3.ClientOptions; +import com.recurly.v3.Pager; import com.recurly.v3.RequestOptions; import com.recurly.v3.fixtures.MockQueryParams; -import org.mockito.stubbing.Answer; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; - import java.lang.reflect.Type; import java.util.HashMap; -import okhttp3.Headers; -import okhttp3.MediaType; -import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.Response; -import okhttp3.ResponseBody; - public class MockClient extends BaseClient { public MockClient(final String apiKey) { super(apiKey); } - public MockClient(final String apiKey, final OkHttpClient client) { - super(apiKey, client, new ClientOptions()); - } - public MockClient(final String apiKey, final ClientOptions clientOptions) { super(apiKey, clientOptions); } - public MockClient(final String apiKey, final OkHttpClient client, final ClientOptions clientOptions) { - super(apiKey, client, clientOptions); - } - - public String apiUrl; - public MyResource getResource(String resourceId) { final String url = "/resources/{resource_id}"; final HashMap urlParams = new HashMap(); urlParams.put("resource_id", resourceId); - final HashMap queryParams = new HashMap(); final String path = this.interpolatePath(url, urlParams); Type returnType = MyResource.class; return this.makeRequest("GET", path, returnType); @@ -91,7 +67,6 @@ public void removeResource(String resourceId) { final String url = "/resources/{resource_id}"; final HashMap urlParams = new HashMap(); urlParams.put("resource_id", resourceId); - final HashMap queryParams = new HashMap(); final String path = this.interpolatePath(url, urlParams); this.makeRequest("DELETE", path); } @@ -99,35 +74,4 @@ public void removeResource(String resourceId) { public void badRequestMethod() { this.makeRequest("BOGUS", "/accounts"); } - - public static final Response buildResponse(Integer code, String message, String response) { - Headers headers = new Headers.Builder().build(); - return buildResponse(code, message, response, headers); - } - - public static final Response buildResponse(Integer code, String message, String response, Headers headers) { - MediaType contentType = MediaType.get("application/json; charset=utf-8"); - return buildResponse(code, message, response, headers, contentType); - } - - public static final Response buildResponse(Integer code, String message, String response, Headers headers, MediaType contentType) { - final Request mRequest = new Request.Builder().url("https://v3.recurly.com").build(); - - final Response mResponse = - new Response.Builder() - .request(mRequest) - .protocol(okhttp3.Protocol.HTTP_1_1) - .code(code) // status code - .message(message) - .body(ResponseBody.create(contentType, response)) - .headers(headers) - .build(); - return mResponse; - } - - public static OkHttpClient getMockOkHttpClient(Answer answer) { - final OkHttpClient mockOkHttpClient = mock(OkHttpClient.class); - doAnswer(answer).when(mockOkHttpClient).newCall(any()); - return mockOkHttpClient; - } } diff --git a/src/test/java/com/recurly/v3/http/DefaultHttpAdapterContractTest.java b/src/test/java/com/recurly/v3/http/DefaultHttpAdapterContractTest.java new file mode 100644 index 00000000..2c601267 --- /dev/null +++ b/src/test/java/com/recurly/v3/http/DefaultHttpAdapterContractTest.java @@ -0,0 +1,13 @@ +package com.recurly.v3.http; + +/** + * Verifies that {@link DefaultHttpAdapter} satisfies the {@link HttpAdapterContract}. + * Serves as a live example of how to wire up the contract test for a custom implementation. + */ +class DefaultHttpAdapterContractTest extends HttpAdapterContract { + + @Override + protected HttpAdapter createAdapter() { + return new DefaultHttpAdapter(); + } +} diff --git a/src/test/java/com/recurly/v3/http/HttpAdapterContract.java b/src/test/java/com/recurly/v3/http/HttpAdapterContract.java new file mode 100644 index 00000000..856c575e --- /dev/null +++ b/src/test/java/com/recurly/v3/http/HttpAdapterContract.java @@ -0,0 +1,373 @@ +package com.recurly.v3.http; + +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.stubbing.ServeEvent; +import com.github.tomakehurst.wiremock.verification.LoggedRequest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnJre; +import org.junit.jupiter.api.condition.JRE; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.junit.jupiter.api.Assertions.*; + +/** + * Portable contract test for {@link HttpAdapter} implementations. + * + *

Extend this class and implement {@link #createAdapter()} to verify that your adapter + * satisfies the full behavioral contract required by the Recurly Java client. + * + * 

{@code
+ * class MyAdapterTest extends HttpAdapterContract {
+ *     @Override
+ *     protected HttpAdapter createAdapter() {
+ *         return new MyHttpAdapter();
+ *     }
+ * }
+ * }
+ * + *

Each test starts a local {@link WireMockServer} on a random port, so no real network + * access is needed. Requires Java 11 or later at test runtime (WireMock 3 constraint); + * tests are automatically skipped on Java 8. + * + *

You will need the following test-scope dependencies in your project: + * + * 

{@code
+ * 
+ *     org.wiremock
+ *     wiremock
+ *     3.13.2
+ *     test
+ * 
+ * 
+ *     org.junit.jupiter
+ *     junit-jupiter-engine
+ *     5.12.2
+ *     test
+ * 
+ * }
+ */ +@DisabledOnJre(JRE.JAVA_8) +public abstract class HttpAdapterContract { + + private WireMockServer server; + protected HttpAdapter adapter; + + /** + * Return the {@link HttpAdapter} implementation under test. Called once per test method; the + * returned instance is assigned to {@link #adapter} before the test runs. + */ + protected abstract HttpAdapter createAdapter(); + + @BeforeEach + void setUp() { + server = new WireMockServer(wireMockConfig().dynamicPort()); + server.start(); + adapter = createAdapter(); + } + + @AfterEach + void tearDown() { + server.stop(); + } + + // --------------------------------------------------------------------------- + // HTTP methods + // --------------------------------------------------------------------------- + + @Test + void getRequest_sendsCorrectMethodAndNoBody() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts")).willReturn(ok().withBody("{}"))); + + adapter.execute("GET", url("/accounts"), noHeaders(), null); + + LoggedRequest req = singleRequest(); + assertEquals("GET", req.getMethod().getName()); + assertEquals(0, req.getBody().length, "GET must not send a body"); + } + + @Test + void postRequest_sendsBodyAndCorrectMethod() throws Exception { + String body = "{\"code\":\"silver\"}"; + server.stubFor(any(urlPathEqualTo("/subscriptions")) + .willReturn(aResponse().withStatus(201).withBody("{}"))); + + adapter.execute("POST", url("/subscriptions"), jsonHeaders(), body); + + LoggedRequest req = singleRequest(); + assertEquals("POST", req.getMethod().getName()); + assertEquals(body, req.getBodyAsString(), "POST body must be forwarded verbatim"); + } + + @Test + void putRequest_sendsBodyAndCorrectMethod() throws Exception { + String body = "{\"first_name\":\"Jane\"}"; + server.stubFor(any(urlPathEqualTo("/accounts/abc123")).willReturn(ok().withBody("{}"))); + + adapter.execute("PUT", url("/accounts/abc123"), jsonHeaders(), body); + + LoggedRequest req = singleRequest(); + assertEquals("PUT", req.getMethod().getName()); + assertEquals(body, req.getBodyAsString(), "PUT body must be forwarded verbatim"); + } + + @Test + void postRequest_nullBody_sendsContentLengthZero() throws Exception { + server.stubFor(any(urlPathEqualTo("/subscriptions")).willReturn(aResponse().withStatus(201).withBody("{}"))); + + adapter.execute("POST", url("/subscriptions"), noHeaders(), null); + + LoggedRequest req = singleRequest(); + assertEquals("POST", req.getMethod().getName()); + assertEquals(0, req.getBody().length, "POST with null body must not send body bytes"); + assertEquals("0", req.getHeader("Content-Length"), + "POST with null body must send Content-Length: 0"); + } + + @Test + void putRequest_nullBody_sendsContentLengthZero() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts/abc123")).willReturn(ok().withBody("{}"))); + + adapter.execute("PUT", url("/accounts/abc123"), noHeaders(), null); + + LoggedRequest req = singleRequest(); + assertEquals("PUT", req.getMethod().getName()); + assertEquals(0, req.getBody().length, "PUT with null body must not send body bytes"); + assertEquals("0", req.getHeader("Content-Length"), + "PUT with null body must send Content-Length: 0"); + } + @Test + void deleteRequest_sendsCorrectMethodAndNoBody() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts/abc123")) + .willReturn(aResponse().withStatus(204))); + + adapter.execute("DELETE", url("/accounts/abc123"), noHeaders(), null); + + LoggedRequest req = singleRequest(); + assertEquals("DELETE", req.getMethod().getName()); + assertEquals(0, req.getBody().length, "DELETE must not send a body"); + } + + @Test + void headRequest_sendsCorrectMethodAndNoBody() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts")) + .willReturn(ok().withHeader("recurly-total-records", "42"))); + + HttpResponse response = adapter.execute("HEAD", url("/accounts"), noHeaders(), null); + + LoggedRequest req = singleRequest(); + assertEquals("HEAD", req.getMethod().getName()); + assertEquals(0, req.getBody().length, "HEAD must not send a body"); + assertNotNull(response.getBody(), "HEAD response body must be a non-null byte array"); + } + + // --------------------------------------------------------------------------- + // Request headers + // --------------------------------------------------------------------------- + + @Test + void requestHeaders_forwardedUnmodified() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts")).willReturn(ok().withBody("{}"))); + + Map headers = new HashMap<>(); + headers.put("Authorization", "Basic dXNlcjpwYXNz"); + headers.put("Accept", "application/json"); + headers.put("X-Api-Version", "2021-02-25"); + adapter.execute("GET", url("/accounts"), headers, null); + + LoggedRequest req = singleRequest(); + assertEquals("Basic dXNlcjpwYXNz", req.getHeader("Authorization"), + "Authorization header must be forwarded unmodified"); + assertEquals("application/json", req.getHeader("Accept"), + "Accept header must be forwarded unmodified"); + assertEquals("2021-02-25", req.getHeader("X-Api-Version"), + "Custom headers must be forwarded unmodified"); + } + + // --------------------------------------------------------------------------- + // Response: status code, headers, body + // --------------------------------------------------------------------------- + + @Test + void responseStatusCode_matchesServerResponse() throws Exception { + server.stubFor(any(urlPathEqualTo("/missing")) + .willReturn(aResponse().withStatus(404).withBody("{}"))); + + HttpResponse response = adapter.execute("GET", url("/missing"), noHeaders(), null); + + assertEquals(404, response.getStatusCode(), + "Status code must match what the server returned"); + } + + @Test + void responseHeaders_returnedInResponse() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts")) + .willReturn(ok().withBody("{}") + .withHeader("X-Request-Id", "req-xyz") + .withHeader("Recurly-Total-Records", "99"))); + + HttpResponse response = adapter.execute("GET", url("/accounts"), noHeaders(), null); + + // HttpResponse normalises header names to lower-case + Map h = response.getHeaders(); + assertNotNull(h); + assertTrue(h.containsKey("x-request-id"), + "x-request-id response header must be present (case-insensitive lookup)"); + assertEquals("req-xyz", h.get("x-request-id")); + } + + @Test + void responseBody_returnedAsBytes() throws Exception { + String json = "{\"object\":\"account\",\"code\":\"abc\"}"; + server.stubFor(any(urlPathEqualTo("/accounts/abc")).willReturn(ok().withBody(json))); + + HttpResponse response = adapter.execute("GET", url("/accounts/abc"), noHeaders(), null); + + assertNotNull(response.getBody(), "body must never be null"); + assertEquals(json, new String(response.getBody(), StandardCharsets.UTF_8)); + } + + @Test + void emptyResponseBody_returnsEmptyByteArray_notNull() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts/abc")) + .willReturn(aResponse().withStatus(204))); + + HttpResponse response = adapter.execute("DELETE", url("/accounts/abc"), noHeaders(), null); + + assertNotNull(response.getBody(), + "body must be a non-null byte[] even when the response has no body"); + } + + @Test + void largeResponseBody_readFully() throws Exception { + // 256 KB — guards against implementations that return a lazy or truncated stream + StringBuilder sb = new StringBuilder(256 * 1024); + for (int i = 0; i < 256 * 1024; i++) { + sb.append('x'); + } + String large = sb.toString(); + server.stubFor(any(urlPathEqualTo("/data")).willReturn(ok().withBody(large))); + + HttpResponse response = adapter.execute("GET", url("/data"), noHeaders(), null); + + assertEquals(large.length(), response.getBody().length, + "Adapter must read the response body fully before returning"); + } + + // --------------------------------------------------------------------------- + // URL forwarding + // --------------------------------------------------------------------------- + + @Test + void urlWithQueryString_forwardedUnmodified() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts")).willReturn(ok().withBody("{}"))); + + adapter.execute("GET", url("/accounts?limit=20&sort=created_at"), noHeaders(), null); + + LoggedRequest req = singleRequest(); + String fullUrl = req.getUrl(); + assertNotNull(fullUrl); + assertTrue(fullUrl.contains("limit=20"), "Query parameter 'limit' must be forwarded"); + assertTrue(fullUrl.contains("sort=created_at"), "Query parameter 'sort' must be forwarded"); + } + + // --------------------------------------------------------------------------- + // Error handling + // --------------------------------------------------------------------------- + + @Test + void httpErrors_returnedAsResponses_notThrown() throws Exception { + // 4xx and 5xx must NOT cause an exception — return the response and let the client handle it + server.stubFor(any(urlPathEqualTo("/subscriptions")) + .willReturn(aResponse().withStatus(422).withBody("{\"error\":{}}"))); + + HttpResponse response = adapter.execute("POST", url("/subscriptions"), jsonHeaders(), "{}"); + + assertEquals(422, response.getStatusCode(), + "HTTP error status codes must be returned as HttpResponse, not thrown as exceptions"); + } + + @Test + void networkFailure_throwsIOException() { + WireMockServer dead = new WireMockServer(wireMockConfig().dynamicPort()); + dead.start(); + int port = dead.port(); + dead.stop(); + + assertThrows(IOException.class, + () -> adapter.execute("GET", "http://localhost:" + port + "/test", noHeaders(), null), + "Network-level failures must propagate as IOException"); + } + + // --------------------------------------------------------------------------- + // Thread safety + // --------------------------------------------------------------------------- + + @Test + void concurrentRequests_completeSafely() throws Exception { + int threadCount = 20; + server.stubFor(any(anyUrl()).willReturn(ok().withBody("{}"))); + + ExecutorService pool = Executors.newFixedThreadPool(threadCount); + CountDownLatch allReady = new CountDownLatch(threadCount); + CountDownLatch startGun = new CountDownLatch(1); + AtomicInteger errors = new AtomicInteger(0); + + for (int i = 0; i < threadCount; i++) { + pool.submit(() -> { + allReady.countDown(); + try { + startGun.await(); + adapter.execute("GET", url("/accounts"), noHeaders(), null); + } catch (Exception e) { + errors.incrementAndGet(); + } + }); + } + + allReady.await(5, TimeUnit.SECONDS); + startGun.countDown(); + pool.shutdown(); + assertTrue(pool.awaitTermination(15, TimeUnit.SECONDS), + "All concurrent requests must complete within 15 seconds"); + assertEquals(0, errors.get(), + "No exceptions should occur during concurrent use of the adapter"); + } + + // --------------------------------------------------------------------------- + // Helpers + // --------------------------------------------------------------------------- + + private String url(final String path) { + return "http://localhost:" + server.port() + path; + } + + private static Map noHeaders() { + return new HashMap<>(); + } + + private static Map jsonHeaders() { + Map headers = new HashMap<>(); + headers.put("Content-Type", "application/json; charset=utf-8"); + return headers; + } + + private LoggedRequest singleRequest() { + List events = server.getAllServeEvents(); + assertFalse(events.isEmpty(), "No request was received by the mock server"); + return events.get(0).getRequest(); + } +} diff --git a/src/test/java/com/recurly/v3/http/HttpResponseTest.java b/src/test/java/com/recurly/v3/http/HttpResponseTest.java new file mode 100644 index 00000000..418424c6 --- /dev/null +++ b/src/test/java/com/recurly/v3/http/HttpResponseTest.java @@ -0,0 +1,59 @@ +package com.recurly.v3.http; + +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.*; + +public class HttpResponseTest { + + private static final Map EMPTY_HEADERS = Collections.emptyMap(); + private static final byte[] EMPTY_BODY = new byte[0]; + + @Test + public void nullHeadersThrows() { + assertThrows(NullPointerException.class, () -> + new HttpResponse(200, null, EMPTY_BODY)); + } + + @Test + public void nullBodyThrows() { + assertThrows(NullPointerException.class, () -> + new HttpResponse(200, EMPTY_HEADERS, null)); + } + + @Test + public void validConstructionSucceeds() { + final HttpResponse response = new HttpResponse(200, EMPTY_HEADERS, EMPTY_BODY); + assertEquals(200, response.getStatusCode()); + assertNotNull(response.getHeaders()); + assertNotNull(response.getBody()); + } + + @Test + public void headersAreNormalizedToLowercase() { + final Map headers = new HashMap<>(); + headers.put("Content-Type", "application/json"); + headers.put("X-Custom-Header", "value"); + + final HttpResponse response = new HttpResponse(200, headers, EMPTY_BODY); + + assertEquals("application/json", response.getHeaders().get("content-type")); + assertEquals("value", response.getHeaders().get("x-custom-header")); + assertNull(response.getHeaders().get("Content-Type")); + } + + @Test + public void headersAreImmutable() { + final Map headers = new HashMap<>(); + headers.put("content-type", "application/json"); + + final HttpResponse response = new HttpResponse(200, headers, EMPTY_BODY); + + assertThrows(UnsupportedOperationException.class, () -> + response.getHeaders().put("x-injected", "value")); + } +} From 9ba64272cbc3f48e5f160dfb8a0ec9513a51297b Mon Sep 17 00:00:00 2001 From: Douglas Miller Date: Tue, 30 Jun 2026 13:22:38 -0500 Subject: [PATCH 2/2] fix: Switch DefaultHttpAdapter from OkHttp to HttpURLConnection Replace OkHttp transport with java.net.HttpURLConnection; remove okhttp and logging-interceptor compile dependencies. Preserve debug logging behind RECURLY_INSECURE + RECURLY_DEBUG using System.out.println. Fix 411 errors on POST/PUT with null body by sending Content-Length: 0, matching OkHttp prior behavior. Add contract tests for this case. Update the implementation guide to use OkHttp as the example adapter instead of java.net.http.HttpClient. Co-Authored-By: Claude feat: Add gzip response decompression to DefaultHttpAdapter DefaultHttpAdapter now sets Accept-Encoding: gzip on outgoing requests (unless the caller already set one) and transparently decompresses gzip-encoded response and error bodies via GZIPInputStream, matching the behavior OkHttp provided automatically before the HttpURLConnection migration. content-encoding and content-length are stripped from the returned headers once decompressed since they no longer describe the decompressed body. Co-Authored-By: Claude fix: Exclude WireMock-dependent tests from testCompile on JDK 8 WireMock 3.x ships Java 11 class files, which javac on a JDK 8 toolchain cannot read from the classpath regardless of source/target level. @DisabledOnJre(JRE.JAVA_8) only skips execution, not compilation, so the Java 8 CI job was failing to build. Co-Authored-By: Claude Sonnet 5 fix: Update RequestOptions header tests for HttpAdapter migration Rebase onto v3-v2021-02-25 merged in idempotency-key/custom-header tests written against the old OkHttp-based BaseClient. Rewrite them against the HttpAdapter mock so they compile and assert against the headers map passed to httpAdapter.execute. Co-Authored-By: Claude Sonnet 5 --- README.md | 8 + docs/examples/JdkHttpClientAdapter.java | 71 ++++++ docs/examples/OkHttpAdapter.java | 69 ++++++ docs/http-adapter-implementation-guide.md | 93 +++---- pom.xml | 50 ++-- src/main/java/com/recurly/v3/BaseClient.java | 68 +++--- src/main/java/com/recurly/v3/Client.java | 1 - .../java/com/recurly/v3/ClientOptions.java | 47 ++++ .../java/com/recurly/v3/RequestOptions.java | 3 +- .../recurly/v3/http/DefaultHttpAdapter.java | 180 +++++++++----- .../java/com/recurly/v3/http/HttpAdapter.java | 19 +- .../com/recurly/v3/http/HttpResponse.java | 4 + .../com/recurly/v3/internal/InternalApi.java | 21 ++ .../java/com/recurly/v3/internal/Utils.java | 12 + .../java/com/recurly/v3/BaseClientTest.java | 229 +++++++++++++----- .../v3/ClientOptionsLegacyApiTest.java | 37 +++ .../com/recurly/v3/ClientOptionsTest.java | 39 +++ src/test/java/com/recurly/v3/PagerTest.java | 15 +- .../com/recurly/v3/RequestOptionsTest.java | 40 +++ .../recurly/v3/fixtures/HttpTestFixtures.java | 24 ++ .../v3/http/DefaultHttpAdapterGzipTest.java | 154 ++++++++++++ .../http/DefaultHttpAdapterTimeoutTest.java | 50 ++++ .../recurly/v3/http/HttpAdapterContract.java | 13 +- .../recurly/v3/http/WireMockTestSupport.java | 24 ++ .../com/recurly/v3/internal/UtilsTest.java | 13 + 25 files changed, 1025 insertions(+), 259 deletions(-) create mode 100644 docs/examples/JdkHttpClientAdapter.java create mode 100644 docs/examples/OkHttpAdapter.java create mode 100644 src/main/java/com/recurly/v3/internal/InternalApi.java create mode 100644 src/main/java/com/recurly/v3/internal/Utils.java create mode 100644 src/test/java/com/recurly/v3/ClientOptionsLegacyApiTest.java create mode 100644 src/test/java/com/recurly/v3/ClientOptionsTest.java create mode 100644 src/test/java/com/recurly/v3/RequestOptionsTest.java create mode 100644 src/test/java/com/recurly/v3/fixtures/HttpTestFixtures.java create mode 100644 src/test/java/com/recurly/v3/http/DefaultHttpAdapterGzipTest.java create mode 100644 src/test/java/com/recurly/v3/http/DefaultHttpAdapterTimeoutTest.java create mode 100644 src/test/java/com/recurly/v3/http/WireMockTestSupport.java create mode 100644 src/test/java/com/recurly/v3/internal/UtilsTest.java diff --git a/README.md b/README.md index 9d0858ba..5b80d5fe 100644 --- a/README.md +++ b/README.md @@ -280,6 +280,14 @@ final RequestOptions options = RequestOptions.builder() .header("X-Custom-Header", "value"); ``` +### Custom HTTP Transport + +By default, the client uses `DefaultHttpAdapter` to make HTTP requests. If you need to route +requests through a proxy, use a specific HTTP library, or add observability, you can supply your +own implementation of the `HttpAdapter` interface via `ClientOptions`. See the +[HttpAdapter implementation guide](docs/http-adapter-implementation-guide.md) for details, which +includes a full example implementation using OkHttp (the library's original default transport). + ## Support Looking for help? Please contact [support@recurly.com](mailto:support@recurly.com) or visit diff --git a/docs/examples/JdkHttpClientAdapter.java b/docs/examples/JdkHttpClientAdapter.java new file mode 100644 index 00000000..bd97f0bc --- /dev/null +++ b/docs/examples/JdkHttpClientAdapter.java @@ -0,0 +1,71 @@ +// Example HttpAdapter implementation backed by java.net.http.HttpClient (JDK 11+). +// +// This file is documentation only — it is not compiled as part of this library, which +// targets Java 8. Copy it into your own project if you're on Java 11+ and want to use +// the JDK's built-in HTTP client instead of adding a third-party dependency. + +import com.recurly.v3.http.HttpAdapter; +import com.recurly.v3.http.HttpResponse; + +import java.io.IOException; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse.BodyHandlers; +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; + +public class JdkHttpClientAdapter implements HttpAdapter { + + private final HttpClient client = HttpClient.newBuilder() + .connectTimeout(Duration.ofSeconds(60)) + .build(); + + @Override + public HttpResponse execute(String method, String url, + Map headers, String body) throws IOException { + HttpRequest.Builder builder = HttpRequest.newBuilder() + .uri(URI.create(url)) + .timeout(Duration.ofSeconds(60)); + + for (Map.Entry header : headers.entrySet()) { + builder.header(header.getKey(), header.getValue()); + } + + HttpRequest.BodyPublisher bodyPublisher = body != null + ? HttpRequest.BodyPublishers.ofString(body) + : HttpRequest.BodyPublishers.noBody(); + + switch (method) { + case "HEAD": builder.method("HEAD", HttpRequest.BodyPublishers.noBody()); break; + case "GET": builder.GET(); break; + case "POST": builder.POST(bodyPublisher); break; + case "PUT": builder.PUT(bodyPublisher); break; + case "DELETE": builder.DELETE(); break; + default: + throw new IllegalArgumentException(method + " is not a valid Recurly HTTP method"); + } + + java.net.http.HttpResponse response; + try { + response = client.send(builder.build(), BodyHandlers.ofByteArray()); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException("Request interrupted", e); + } + + int statusCode = response.statusCode(); + + Map responseHeaders = new HashMap<>(); + response.headers().map().forEach((name, values) -> { + if (!values.isEmpty()) { + responseHeaders.put(name, values.get(0)); + } + }); + + byte[] responseBodyBytes = "HEAD".equals(method) ? new byte[0] : response.body(); + + return new HttpResponse(statusCode, responseHeaders, responseBodyBytes); + } +} diff --git a/docs/examples/OkHttpAdapter.java b/docs/examples/OkHttpAdapter.java new file mode 100644 index 00000000..85cb1dbc --- /dev/null +++ b/docs/examples/OkHttpAdapter.java @@ -0,0 +1,69 @@ +// Example HttpAdapter implementation backed by OkHttp. +// +// This file is documentation only — it is not compiled as part of this library +// and OkHttp is NOT a dependency of this project. Copy it into your own +// project and add the OkHttp dependency shown in +// docs/http-adapter-implementation-guide.md if you want to use it. + +import com.recurly.v3.http.HttpAdapter; +import com.recurly.v3.http.HttpResponse; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import okhttp3.Headers; +import okhttp3.MediaType; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.RequestBody; +import okhttp3.Response; +import okhttp3.ResponseBody; + +public class OkHttpAdapter implements HttpAdapter { + + private final OkHttpClient client = new OkHttpClient.Builder() + .connectTimeout(60, TimeUnit.SECONDS) + .readTimeout(60, TimeUnit.SECONDS) + .writeTimeout(60, TimeUnit.SECONDS) + .build(); + + @Override + public HttpResponse execute(String method, String url, + Map headers, String body) throws IOException { + Request.Builder builder = new Request.Builder().url(url); + + for (Map.Entry header : headers.entrySet()) { + builder.header(header.getKey(), header.getValue()); + } + + RequestBody requestBody = body != null + ? RequestBody.create(body, MediaType.parse("application/json; charset=utf-8")) + : RequestBody.create(new byte[0], MediaType.parse("application/json; charset=utf-8")); + + switch (method) { + case "HEAD": builder.head(); break; + case "GET": builder.get(); break; + case "POST": builder.post(requestBody); break; + case "PUT": builder.put(requestBody); break; + case "DELETE": builder.delete(); break; + default: + throw new IllegalArgumentException(method + " is not a valid Recurly HTTP method"); + } + + try (Response response = client.newCall(builder.build()).execute()) { + int statusCode = response.code(); + + Map responseHeaders = new HashMap<>(); + Headers okHeaders = response.headers(); + for (int i = 0; i < okHeaders.size(); i++) { + responseHeaders.put(okHeaders.name(i).toLowerCase(), okHeaders.value(i)); + } + + ResponseBody responseBody = response.body(); + byte[] responseBodyBytes = responseBody != null ? responseBody.bytes() : new byte[0]; + + return new HttpResponse(statusCode, responseHeaders, responseBodyBytes); + } + } +} diff --git a/docs/http-adapter-implementation-guide.md b/docs/http-adapter-implementation-guide.md index b49c4985..1937c51f 100644 --- a/docs/http-adapter-implementation-guide.md +++ b/docs/http-adapter-implementation-guide.md @@ -11,8 +11,7 @@ middleware you prefer. Common reasons to do this: ## Registering your implementation ```java -ClientOptions options = new ClientOptions(); -options.setHttpAdapter(new MyHttpAdapter()); +ClientOptions options = ClientOptions.builder().httpAdapter(new MyHttpAdapter()).build(); Client client = new Client(apiKey, options); ``` @@ -47,16 +46,16 @@ All headers the client wants to send, including: | `Content-Type` | `application/json` | | `User-Agent` | `Recurly/; java ` | -**Forward every entry without modification.** Do not add, remove, or override headers in the -adapter. The client owns header construction; the adapter owns transport. +**Forward every client-supplied entry unmodified.** Do not remove or override a header the client +set. The client owns header construction; the adapter owns transport. You may add your own +transport-layer headers (e.g. `Accept-Encoding`) as long as they don't conflict with a header the +client already set — `DefaultHttpAdapter` does this to negotiate gzip. ### Body -- `POST` and `PUT` requests: a UTF-8-encoded JSON string. -- `GET`, `HEAD`, `DELETE` requests: `null`. - -When `body` is `null` and the HTTP method requires a body (e.g. `DELETE` with some servers), send -an empty body (`Content-Length: 0`). +- `POST` and `PUT` requests: typically a UTF-8-encoded JSON string, but may be `null` (e.g. no + request object was passed) — send the request with `Content-Length: 0` in that case. +- `GET`, `HEAD`, `DELETE` requests: always `null`. --- @@ -77,6 +76,9 @@ status codes. Pass a `Map` of all response headers. `HttpResponse` normalises keys to lower-case internally, so you do not need to do it yourself — but passing lower-case keys is fine too. +Only one value per header name is supported. When a server sends multiple values for a single +header name (e.g. `Set-Cookie`), pass just the first value. + The client reads these specific headers: | Header | Purpose | @@ -159,60 +161,39 @@ public class MyHttpAdapter implements HttpAdapter { The Recurly client does not enforce timeouts. Set connect, read, and write timeouts inside your adapter and adjust to your SLA requirements. ---- +`DefaultHttpAdapter` sets its connect and read timeouts from its `timeoutMs` constructor argument +(10 seconds by default). `java.net.HttpURLConnection` has no write-timeout API, so a stalled +request-body upload is not bounded by `timeoutMs` and can block until the underlying OS-level TCP +timeout is reached. If your SLA requires a bounded write phase, implement a custom `HttpAdapter` +(e.g. using OkHttp, which supports `writeTimeout` directly) instead of relying on the default. -## Minimal example +--- -```java -import com.recurly.v3.http.HttpAdapter; -import com.recurly.v3.http.HttpResponse; +## Full example implementations -import java.io.IOException; -import java.net.URI; -import java.net.http.HttpClient; -import java.net.http.HttpRequest; -import java.net.http.HttpRequest.BodyPublishers; -import java.time.Duration; -import java.util.HashMap; -import java.util.Map; +The examples below are documentation only — they are not compiled as part of this library, so +their dependencies (where applicable) are not dependencies of this project. Copy the one you want +into your own project. -public class JavaNetHttpAdapter implements HttpAdapter { +### OkHttp - private final HttpClient client = HttpClient.newBuilder() - .connectTimeout(Duration.ofSeconds(30)) - .build(); +[`docs/examples/OkHttpAdapter.java`](examples/OkHttpAdapter.java) — useful if your SLA requires a +bounded write-timeout (see [Timeouts](#timeouts) above). Add the OkHttp dependency to use it: - @Override - public HttpResponse execute(String method, String url, - Map headers, String body) throws IOException { - HttpRequest.Builder builder = HttpRequest.newBuilder() - .uri(URI.create(url)) - .method(method, body != null - ? BodyPublishers.ofString(body) - : BodyPublishers.noBody()); - - headers.forEach(builder::header); - - try { - java.net.http.HttpResponse resp = - client.send(builder.build(), java.net.http.HttpResponse.BodyHandlers.ofByteArray()); - - Map responseHeaders = new HashMap<>(); - resp.headers().map().forEach((k, vs) -> { - if (k != null && !vs.isEmpty()) responseHeaders.put(k, vs.get(0)); - }); - - return new HttpResponse(resp.statusCode(), responseHeaders, - resp.body() != null ? resp.body() : new byte[0]); - - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IOException("HTTP request interrupted", e); - } - } -} +```xml + + com.squareup.okhttp3 + okhttp + 4.12.0 + ``` +### `java.net.http.HttpClient` (JDK 11+) + +[`docs/examples/JdkHttpClientAdapter.java`](examples/JdkHttpClientAdapter.java) — no third-party +dependency required, but needs Java 11+ at compile and run time (this library itself targets Java +8, which is why this example isn't part of `src/main`). + --- ## Verifying your implementation with `HttpAdapterContract` @@ -274,4 +255,4 @@ public class FakeHttpAdapter implements HttpAdapter { } ``` -See `DefaultHttpAdapter` for the complete production reference implementation. +See `DefaultHttpAdapter` for the `HttpURLConnection`-based reference implementation. diff --git a/pom.xml b/pom.xml index afcad0af..55c67231 100644 --- a/pom.xml +++ b/pom.xml @@ -58,10 +58,9 @@ - 1.8 + 8 UTF-8 UTF-8 - 4.12.0 3.13.2 3.5.5 0.8.13 @@ -87,8 +86,7 @@ maven-compiler-plugin 3.15.0 - ${java.version} - ${java.version} + ${java.version} @@ -156,6 +154,7 @@ org.apache.maven.plugins maven-jar-plugin + 3.5.0 test-jar @@ -234,22 +233,45 @@ + + + + jdk8-exclude-wiremock-tests + + 1.8 + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + **/http/HttpAdapterContract.java + **/http/DefaultHttpAdapterContractTest.java + **/http/DefaultHttpAdapterGzipTest.java + **/http/DefaultHttpAdapterTimeoutTest.java + **/http/WireMockTestSupport.java + + + + + + + + com.google.code.gson gson 2.13.1 - - com.squareup.okhttp3 - okhttp - ${okhttp3.version} - - - com.squareup.okhttp3 - logging-interceptor - ${okhttp3.version} - org.wiremock wiremock diff --git a/src/main/java/com/recurly/v3/BaseClient.java b/src/main/java/com/recurly/v3/BaseClient.java index fd05fc7e..1aed69e1 100644 --- a/src/main/java/com/recurly/v3/BaseClient.java +++ b/src/main/java/com/recurly/v3/BaseClient.java @@ -2,9 +2,9 @@ import com.google.gson.annotations.SerializedName; import com.recurly.v3.exception.ExceptionFactory; -import com.recurly.v3.http.DefaultHttpAdapter; import com.recurly.v3.http.HttpAdapter; import com.recurly.v3.http.HttpResponse; +import com.recurly.v3.internal.Utils; import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; @@ -40,16 +40,13 @@ public abstract class BaseClient { private String apiUrl; protected BaseClient(final String apiKey) { - this(apiKey, new ClientOptions()); + this(apiKey, ClientOptions.builder().build()); } protected BaseClient(final String apiKey, final ClientOptions clientOptions) { this.authToken = buildAuthToken(validateApiKey(apiKey)); this.apiUrl = clientOptions.getBaseUrl(); - this.httpAdapter = - clientOptions.getHttpAdapter() != null - ? clientOptions.getHttpAdapter() - : new DefaultHttpAdapter(); + this.httpAdapter = clientOptions.getHttpAdapter(); } private static String validateApiKey(final String apiKey) { @@ -71,18 +68,19 @@ private Map buildHeaders() { private Map buildHeaders(final RequestOptions options) { final Map headers = new HashMap<>(); - headers.put("Authorization", authToken); - headers.put("Accept", "application/vnd.recurly." + Client.API_VERSION); - headers.put("Content-Type", "application/json"); - headers.put("User-Agent", USER_AGENT); if (options != null) { headers.putAll(options.getHeaders()); if (options.getIdempotencyKey() != null) { - headers.put("Idempotency-Key", options.getIdempotencyKey()); + headers.put("idempotency-key", options.getIdempotencyKey()); } } + headers.put("authorization", authToken); + headers.put("accept", "application/vnd.recurly." + Client.API_VERSION); + headers.put("content-type", "application/json"); + headers.put("user-agent", USER_AGENT); + return headers; } @@ -92,7 +90,7 @@ private String buildUrl(final String path, final HashMap queryPa } final StringBuilder sb = new StringBuilder(this.apiUrl).append(path); - boolean first = true; + boolean first = !path.contains("?"); for (final Map.Entry param : queryParams.entrySet()) { final Object value = param.getValue(); @@ -124,10 +122,10 @@ private String buildUrl(final String path, final HashMap queryPa .append(param.getKey()) .append("=") .append(URLEncoder.encode(stringValue, StandardCharsets.UTF_8.toString())); - first = false; } catch (UnsupportedEncodingException ex) { throw new RecurlyException(ex.getCause()); } + first = false; } return sb.toString(); @@ -137,9 +135,7 @@ private static boolean isSuccessful(final int statusCode) { return statusCode >= 200 && statusCode < 300; } - protected static boolean envEnabled(final String envVar) { - return "true".equals(System.getenv(envVar)); - } + protected void makeRequest(final String method, final String url) { makeRequest(method, url, (RequestOptions) null); @@ -154,14 +150,7 @@ protected void makeRequest(final String method, final String url, final RequestO final HttpResponse response = httpAdapter.execute(method, fullUrl, headers, null); if (!isSuccessful(response.getStatusCode())) { - final String contentType = - response.getHeaders().getOrDefault("content-type", "application/json"); - if (contentType.contains("application/json")) { - throw jsonSerializer.deserializeError( - new String(response.getBody(), StandardCharsets.UTF_8)); - } else { - throw ExceptionFactory.getExceptionClass(response); - } + throwForErrorResponse(response); } warnIfDeprecated(response.getHeaders()); @@ -231,16 +220,10 @@ protected T makeRequest( final HttpResponse response = httpAdapter.execute(method, fullUrl, headers, bodyString); final int statusCode = response.getStatusCode(); - final String contentType = - response.getHeaders().getOrDefault("content-type", "application/json"); + final String contentType = getContentType(response); if (!isSuccessful(statusCode)) { - if (contentType.contains("application/json")) { - throw jsonSerializer.deserializeError( - new String(response.getBody(), StandardCharsets.UTF_8)); - } else { - throw ExceptionFactory.getExceptionClass(response); - } + throwForErrorResponse(response); } warnIfDeprecated(response.getHeaders()); @@ -257,6 +240,20 @@ protected T makeRequest( } } + private static String getContentType(final HttpResponse response) { + return response.getHeaders().getOrDefault("content-type", "application/json"); + } + + private void throwForErrorResponse(final HttpResponse response) { + final byte[] body = response.getBody(); + final String contentType = getContentType(response); + if (body.length > 0 && contentType.startsWith("application/json")) { + throw jsonSerializer.deserializeError(new String(body, StandardCharsets.UTF_8)); + } else { + throw ExceptionFactory.getExceptionClass(response); + } + } + public int getRecordCount(final String url, final HashMap queryParams) { final String fullUrl = buildUrl(url, queryParams); final Map headers = buildHeaders(); @@ -265,7 +262,7 @@ public int getRecordCount(final String url, final HashMap queryP final HttpResponse response = httpAdapter.execute("HEAD", fullUrl, headers, null); if (!isSuccessful(response.getStatusCode())) { - throw ExceptionFactory.getExceptionClass(response); + throwForErrorResponse(response); } warnIfDeprecated(response.getHeaders()); @@ -321,8 +318,7 @@ protected String interpolatePath(String path, final HashMap urlP while (m.find()) { final String key = m.group(1).replace("{", "").replace("}", ""); try { - final String value = - URLEncoder.encode(urlParams.get(key), StandardCharsets.UTF_8.toString()); + final String value = URLEncoder.encode(urlParams.get(key), StandardCharsets.UTF_8.toString()); path = path.replace(m.group(1), value); } catch (UnsupportedEncodingException ex) { throw new RecurlyException(ex.getCause()); @@ -336,7 +332,7 @@ public void _setApiUrl(final String uri) { System.out.println( "[SECURITY WARNING] _setApiUrl is for testing only and not supported in production."); - if (envEnabled("RECURLY_INSECURE")) { + if (Utils.envEnabled("RECURLY_INSECURE")) { this.apiUrl = uri; } else { System.out.println( diff --git a/src/main/java/com/recurly/v3/Client.java b/src/main/java/com/recurly/v3/Client.java index 99d3292c..a2257d55 100644 --- a/src/main/java/com/recurly/v3/Client.java +++ b/src/main/java/com/recurly/v3/Client.java @@ -10,7 +10,6 @@ import com.recurly.v3.requests.*; import com.recurly.v3.resources.*; import com.recurly.v3.queryparams.*; -import okhttp3.OkHttpClient; import java.time.ZonedDateTime; import java.lang.reflect.Type; diff --git a/src/main/java/com/recurly/v3/ClientOptions.java b/src/main/java/com/recurly/v3/ClientOptions.java index 0b739a94..897d52d7 100644 --- a/src/main/java/com/recurly/v3/ClientOptions.java +++ b/src/main/java/com/recurly/v3/ClientOptions.java @@ -1,5 +1,6 @@ package com.recurly.v3; +import com.recurly.v3.http.DefaultHttpAdapter; import com.recurly.v3.http.HttpAdapter; import java.util.HashMap; @@ -20,10 +21,30 @@ public enum Regions { private Regions region; private HttpAdapter httpAdapter; + /** + * @deprecated use {@link #builder()} instead. This constructor and the mutable setters will be + * removed in the next major version. + */ + @Deprecated public ClientOptions() { this.region = Regions.US; + this.httpAdapter = new DefaultHttpAdapter(); } + private ClientOptions(final Builder builder) { + this.region = builder.region != null ? builder.region : Regions.US; + this.httpAdapter = builder.httpAdapter != null ? builder.httpAdapter : new DefaultHttpAdapter(); + } + + public static Builder builder() { + return new Builder(); + } + + /** + * @deprecated use {@link #builder()} instead. This setter will be removed in the next major + * version. + */ + @Deprecated public void setRegion(final Regions r) { this.region = r; } @@ -33,6 +54,11 @@ public String getBaseUrl() { return regionsMap.get(this.region); } + /** + * @deprecated use {@link #builder()} instead. This setter will be removed in the next major + * version. + */ + @Deprecated public void setHttpAdapter(final HttpAdapter adapter) { this.httpAdapter = adapter; } @@ -40,4 +66,25 @@ public void setHttpAdapter(final HttpAdapter adapter) { public HttpAdapter getHttpAdapter() { return httpAdapter; } + + public static class Builder { + private Regions region; + private HttpAdapter httpAdapter; + + private Builder() {} + + public Builder region(final Regions region) { + this.region = region; + return this; + } + + public Builder httpAdapter(final HttpAdapter httpAdapter) { + this.httpAdapter = httpAdapter; + return this; + } + + public ClientOptions build() { + return new ClientOptions(this); + } + } } \ No newline at end of file diff --git a/src/main/java/com/recurly/v3/RequestOptions.java b/src/main/java/com/recurly/v3/RequestOptions.java index ca8b7fbd..2e7f871f 100644 --- a/src/main/java/com/recurly/v3/RequestOptions.java +++ b/src/main/java/com/recurly/v3/RequestOptions.java @@ -2,6 +2,7 @@ import java.util.Collections; import java.util.HashMap; +import java.util.Locale; import java.util.Map; public class RequestOptions { @@ -37,7 +38,7 @@ public Builder idempotencyKey(final String idempotencyKey) { } public Builder header(final String name, final String value) { - this.headers.put(name, value); + this.headers.put(name == null ? null : name.toLowerCase(Locale.ROOT), value); return this; } diff --git a/src/main/java/com/recurly/v3/http/DefaultHttpAdapter.java b/src/main/java/com/recurly/v3/http/DefaultHttpAdapter.java index 65f3d8bd..09b47a63 100644 --- a/src/main/java/com/recurly/v3/http/DefaultHttpAdapter.java +++ b/src/main/java/com/recurly/v3/http/DefaultHttpAdapter.java @@ -1,41 +1,37 @@ package com.recurly.v3.http; +import com.recurly.v3.internal.Utils; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.HttpURLConnection; +import java.net.URL; +import java.nio.charset.StandardCharsets; import java.util.HashMap; +import java.util.List; import java.util.Map; -import java.util.concurrent.TimeUnit; -import okhttp3.Headers; -import okhttp3.MediaType; -import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.RequestBody; -import okhttp3.Response; -import okhttp3.ResponseBody; -import okhttp3.logging.HttpLoggingInterceptor; - +import java.util.zip.GZIPInputStream; + +/** + * Default {@link HttpAdapter} backed by {@link HttpURLConnection}. + * + *

{@code timeoutMs} bounds the connect and read phases only. {@link HttpURLConnection} has no + * write-timeout API, so a stalled request-body upload is not bounded by {@code timeoutMs} and can + * block until the underlying OS-level TCP timeout is reached. Implement a custom {@link + * HttpAdapter} (e.g. using OkHttp) if your SLA requires a bounded write phase. + */ public class DefaultHttpAdapter implements HttpAdapter { - private static final int DEFAULT_TIMEOUT_MS = 60_000; + private static final int DEFAULT_TIMEOUT_MS = 10_000; - private final OkHttpClient httpClient; + private final int timeoutMs; public DefaultHttpAdapter() { this(DEFAULT_TIMEOUT_MS); } public DefaultHttpAdapter(final int timeoutMs) { - final OkHttpClient.Builder builder = - new OkHttpClient.Builder() - .connectTimeout(timeoutMs, TimeUnit.MILLISECONDS) - .readTimeout(timeoutMs, TimeUnit.MILLISECONDS) - .writeTimeout(timeoutMs, TimeUnit.MILLISECONDS); - - if (envEnabled("RECURLY_INSECURE") && envEnabled("RECURLY_DEBUG")) { - final HttpLoggingInterceptor logging = new HttpLoggingInterceptor(); - logging.setLevel(HttpLoggingInterceptor.Level.BASIC); - builder.addInterceptor(logging); - } - - this.httpClient = builder.build(); + this.timeoutMs = timeoutMs; } @Override @@ -45,54 +41,112 @@ public HttpResponse execute( final Map headers, final String body) throws IOException { - final Request.Builder requestBuilder = new Request.Builder().url(url); - - for (final Map.Entry header : headers.entrySet()) { - requestBuilder.header(header.getKey(), header.getValue()); + final boolean debug = Utils.envEnabled("RECURLY_INSECURE") && Utils.envEnabled("RECURLY_DEBUG"); + if (debug) { + System.out.println("--> " + method + " " + url); } + final long startMs = System.currentTimeMillis(); + + HttpURLConnection connection = null; + boolean success = false; + try { + connection = (HttpURLConnection) new URL(url).openConnection(); + connection.setRequestMethod(method); + connection.setConnectTimeout(timeoutMs); + connection.setReadTimeout(timeoutMs); + connection.setInstanceFollowRedirects(true); + + boolean callerSetAcceptEncoding = false; + for (final Map.Entry header : headers.entrySet()) { + connection.setRequestProperty(header.getKey(), header.getValue()); + if ("Accept-Encoding".equalsIgnoreCase(header.getKey())) { + callerSetAcceptEncoding = true; + } + } + if (!callerSetAcceptEncoding) { + connection.setRequestProperty("Accept-Encoding", "gzip"); + } - final RequestBody requestBody = - body != null - ? RequestBody.create(body, MediaType.parse("application/json; charset=utf-8")) - : RequestBody.create(new byte[0]); - - switch (method) { - case "HEAD": - requestBuilder.head(); - break; - case "GET": - requestBuilder.get(); - break; - case "POST": - requestBuilder.post(requestBody); - break; - case "PUT": - requestBuilder.put(requestBody); - break; - case "DELETE": - requestBuilder.delete(); - break; - default: - throw new IllegalArgumentException(method + " is not a valid Recurly HTTP method"); - } + if (body != null || "POST".equals(method) || "PUT".equals(method)) { + connection.setDoOutput(true); + final byte[] bodyBytes = body != null ? body.getBytes(StandardCharsets.UTF_8) : new byte[0]; + try (final OutputStream out = connection.getOutputStream()) { + out.write(bodyBytes); + } + } - try (final Response response = httpClient.newCall(requestBuilder.build()).execute()) { - final int statusCode = response.code(); + final int statusCode = connection.getResponseCode(); final Map responseHeaders = new HashMap<>(); - final Headers okHeaders = response.headers(); - for (int i = 0; i < okHeaders.size(); i++) { - responseHeaders.put(okHeaders.name(i).toLowerCase(), okHeaders.value(i)); + for (final Map.Entry> entry : connection.getHeaderFields().entrySet()) { + final String key = entry.getKey(); + if (key != null && !entry.getValue().isEmpty()) { + responseHeaders.put(key, entry.getValue().get(0)); + } + } + + final InputStream inputStream; + if ("HEAD".equals(method)) { + inputStream = null; + } else { + inputStream = statusCode >= 400 ? connection.getErrorStream() : connection.getInputStream(); + } + + final String contentEncoding = findHeaderIgnoreCase(responseHeaders, "Content-Encoding"); + final boolean gzipEncoded = contentEncoding != null && "gzip".equalsIgnoreCase(contentEncoding.trim()); + + final byte[] responseBodyBytes; + if (inputStream == null) { + responseBodyBytes = new byte[0]; + } else { + try (final InputStream rawStream = inputStream; + final InputStream is = gzipEncoded ? new GZIPInputStream(rawStream) : rawStream) { + responseBodyBytes = readAllBytes(is); + } } - final ResponseBody responseBody = response.body(); - final byte[] responseBodyBytes = responseBody != null ? responseBody.bytes() : new byte[0]; + if (gzipEncoded) { + removeHeaderIgnoreCase(responseHeaders, "Content-Encoding"); + removeHeaderIgnoreCase(responseHeaders, "Content-Length"); + } + + if (debug) { + System.out.println( + "<-- " + statusCode + " " + url + " (" + (System.currentTimeMillis() - startMs) + "ms)"); + } + + final HttpResponse response = new HttpResponse(statusCode, responseHeaders, responseBodyBytes); + success = true; + return response; + } finally { + if (!success && connection != null) { + connection.disconnect(); + } + } + } - return new HttpResponse(statusCode, responseHeaders, responseBodyBytes); + private static String findHeaderIgnoreCase(final Map headers, final String name) { + for (final Map.Entry entry : headers.entrySet()) { + if (entry.getKey().equalsIgnoreCase(name)) { + return entry.getValue(); + } } + return null; + } + + private static void removeHeaderIgnoreCase(final Map headers, final String name) { + headers.keySet().removeIf(key -> key.equalsIgnoreCase(name)); } - private static boolean envEnabled(final String envVar) { - return "true".equals(System.getenv(envVar)); + private static byte[] readAllBytes(final InputStream inputStream) throws IOException { + final ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + final byte[] chunk = new byte[8192]; + int n; + while ((n = inputStream.read(chunk)) != -1) { + buffer.write(chunk, 0, n); + } + return buffer.toByteArray(); } + + } diff --git a/src/main/java/com/recurly/v3/http/HttpAdapter.java b/src/main/java/com/recurly/v3/http/HttpAdapter.java index 16703f26..ca34ce5b 100644 --- a/src/main/java/com/recurly/v3/http/HttpAdapter.java +++ b/src/main/java/com/recurly/v3/http/HttpAdapter.java @@ -13,8 +13,7 @@ *

Registration * * 

{@code
- * ClientOptions options = new ClientOptions();
- * options.setHttpAdapter(new MyHttpAdapter());
+ * ClientOptions options = ClientOptions.builder().httpAdapter(new MyHttpAdapter()).build();
  * Client client = new Client(apiKey, options);
  * }
* @@ -46,10 +45,15 @@ public interface HttpAdapter { *
  • {@code url} — fully-qualified URL including scheme, host, path, and any query string. * Never {@code null}. *
  • {@code headers} — all request headers the client wants sent (Authorization, - * Accept, Content-Type, User-Agent, etc.). Forward every entry without modification; - * do not add, remove, or override headers in the adapter. - *
  • {@code body} — UTF-8 JSON string for {@code POST} and {@code PUT} requests; {@code null} - * for {@code GET}, {@code HEAD}, and {@code DELETE}. + * Accept, Content-Type, User-Agent, etc.). Forward every client-supplied entry + * unmodified — do not remove or override them. Implementations may add their own + * transport-layer headers (e.g. {@code Accept-Encoding}) as long as they do not conflict + * with a header the client already set. + *
  • {@code body} — UTF-8 JSON string when a request payload is present; {@code null} when + * there is none. {@code GET}, {@code HEAD}, and {@code DELETE} never carry a body. + * {@code POST} and {@code PUT} typically carry a body but may receive {@code null} (e.g. + * no request object) — implementations must still send the request with + * {@code Content-Length: 0}. * * *

    Return value
    @@ -75,7 +79,8 @@ public interface HttpAdapter { * @param method HTTP method ({@code GET}, {@code POST}, {@code PUT}, {@code DELETE}, * {@code HEAD}) * @param url absolute URL to request - * @param headers request headers to send; must be forwarded unmodified + * @param headers request headers to send; must be forwarded unmodified (adapters may add their + * own non-conflicting transport-layer headers) * @param body request body as a JSON string, or {@code null} if there is no body * @return the complete HTTP response * @throws IOException on network or I/O failure diff --git a/src/main/java/com/recurly/v3/http/HttpResponse.java b/src/main/java/com/recurly/v3/http/HttpResponse.java index 8ba8f2e4..e2717e16 100644 --- a/src/main/java/com/recurly/v3/http/HttpResponse.java +++ b/src/main/java/com/recurly/v3/http/HttpResponse.java @@ -13,6 +13,10 @@ public class HttpResponse { /** * Constructs an immutable HTTP response snapshot. The {@code body} array is defensively copied; * callers may not observe mutations to the original array through this object. + * + * @param headers response headers, keyed by name (case-insensitive). Only one value per name is + * supported; if a server sends multiple values for the same header name, pass the first + * value. */ public HttpResponse(final int statusCode, final Map headers, final byte[] body) { Objects.requireNonNull(headers, "headers must not be null"); diff --git a/src/main/java/com/recurly/v3/internal/InternalApi.java b/src/main/java/com/recurly/v3/internal/InternalApi.java new file mode 100644 index 00000000..f414a30c --- /dev/null +++ b/src/main/java/com/recurly/v3/internal/InternalApi.java @@ -0,0 +1,21 @@ +package com.recurly.v3.internal; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks a type or member as internal to the Recurly client library. Elements annotated with {@code + * InternalApi} are not part of the public API: they may change incompatibly or be removed at any + * time and must not be relied upon by consumers of this library. + * + *

    The library's Java 8 baseline prevents the compiler from hiding cross-package internals (that + * would require the Java Platform Module System), so this annotation documents intent that the + * language cannot enforce and makes it discoverable to tooling. + */ +@Documented +@Retention(RetentionPolicy.CLASS) +@Target({ElementType.TYPE, ElementType.METHOD}) +public @interface InternalApi {} diff --git a/src/main/java/com/recurly/v3/internal/Utils.java b/src/main/java/com/recurly/v3/internal/Utils.java new file mode 100644 index 00000000..01ac6522 --- /dev/null +++ b/src/main/java/com/recurly/v3/internal/Utils.java @@ -0,0 +1,12 @@ +package com.recurly.v3.internal; + +/** Internal helpers shared across the client library. Not part of the public API. */ +@InternalApi +public final class Utils { + + private Utils() {} + + public static boolean envEnabled(final String envVar) { + return "true".equals(System.getenv(envVar)); + } +} diff --git a/src/test/java/com/recurly/v3/BaseClientTest.java b/src/test/java/com/recurly/v3/BaseClientTest.java index 19e40dbf..fbbede2f 100644 --- a/src/test/java/com/recurly/v3/BaseClientTest.java +++ b/src/test/java/com/recurly/v3/BaseClientTest.java @@ -7,12 +7,14 @@ import com.recurly.v3.exception.TransactionException; import com.recurly.v3.exception.ValidationException; import com.recurly.v3.fixtures.FixtureConstants; +import com.recurly.v3.fixtures.HttpTestFixtures; import com.recurly.v3.fixtures.MockClient; import com.recurly.v3.fixtures.MockQueryParams; import com.recurly.v3.fixtures.MyRequest; import com.recurly.v3.fixtures.MyResource; import com.recurly.v3.RequestOptions; import com.recurly.v3.http.HttpAdapter; +import com.recurly.v3.internal.Utils; import com.recurly.v3.http.HttpResponse; import org.apache.commons.io.IOUtils; import java.time.ZonedDateTime; @@ -23,6 +25,7 @@ import java.io.IOException; import java.io.InputStream; +import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; @@ -33,35 +36,19 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static com.recurly.v3.fixtures.HttpTestFixtures.jsonResponse; +import static com.recurly.v3.fixtures.HttpTestFixtures.mockClientWith; import static org.mockito.Mockito.*; @SuppressWarnings("unchecked") public class BaseClientTest { - private static HttpResponse jsonResponse(final int statusCode, final String body) { - final Map headers = new HashMap<>(); - headers.put("content-type", "application/json; charset=utf-8"); - return new HttpResponse(statusCode, headers, body.getBytes(StandardCharsets.UTF_8)); - } - private static HttpResponse htmlResponse(final int statusCode, final String body) { final Map headers = new HashMap<>(); headers.put("content-type", "text/html; charset=UTF-8"); return new HttpResponse(statusCode, headers, body.getBytes(StandardCharsets.UTF_8)); } - private static HttpResponse headResponse(final int statusCode, final String recordCount) { - final Map headers = new HashMap<>(); - headers.put("recurly-total-records", recordCount); - return new HttpResponse(statusCode, headers, new byte[0]); - } - - private static MockClient mockClientWith(final HttpAdapter adapter) { - final ClientOptions options = new ClientOptions(); - options.setHttpAdapter(adapter); - return new MockClient("apiKey", options); - } - @Test public void testMakeRequestWithResource() throws IOException { final HttpAdapter mockAdapter = mock(HttpAdapter.class); @@ -103,6 +90,75 @@ public void testMakeRequestWithoutResource() throws IOException { verify(mockAdapter).execute(eq("DELETE"), contains("/resources/resource-id"), any(), isNull()); } + @Test + public void testBuildHeadersSendsCorrectAuthAcceptAndUserAgent() throws IOException { + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())).thenReturn(jsonResponse(200, "{}")); + final ArgumentCaptor> headersCaptor = ArgumentCaptor.forClass(Map.class); + + mockClientWith(mockAdapter).getResource("resource-id"); + + verify(mockAdapter).execute(any(), any(), headersCaptor.capture(), any()); + final Map headers = headersCaptor.getValue(); + + assertEquals( + "Basic " + java.util.Base64.getEncoder().encodeToString("apiKey:".getBytes(StandardCharsets.ISO_8859_1)), + headers.get("authorization")); + assertEquals("application/vnd.recurly." + Client.API_VERSION, headers.get("accept")); + assertTrue( + headers.get("user-agent").matches("Recurly/\\d+\\.\\d+\\.\\d+(-SNAPSHOT)?;\\s+java\\s+\\d+.*"), + "User-Agent header should match the expected format, was: " + headers.get("user-agent")); + } + + @Test + public void testWarnIfDeprecatedPrintsWarningWhenHeaderPresent() throws IOException { + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + final Map responseHeaders = new HashMap<>(); + responseHeaders.put("content-type", "application/json; charset=utf-8"); + responseHeaders.put("recurly-deprecated", "true"); + responseHeaders.put("recurly-sunset-date", "2026-01-01"); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(new HttpResponse(200, responseHeaders, "{}".getBytes(StandardCharsets.UTF_8))); + + final java.io.PrintStream originalOut = System.out; + final java.io.ByteArrayOutputStream captured = new java.io.ByteArrayOutputStream(); + System.setOut(new java.io.PrintStream(captured)); + try { + mockClientWith(mockAdapter).getResource("resource-id"); + } finally { + System.setOut(originalOut); + } + + final String output = captured.toString(StandardCharsets.UTF_8.name()); + assertTrue(output.contains("WARNING"), "Expected a deprecation warning, got: " + output); + assertTrue(output.contains("2026-01-01"), "Expected the sunset date in the warning, got: " + output); + } + + + @Test + public void testWarnIfDeprecatedPrintsWarningWhenHeaderPresentOnVoidResponse() throws IOException { + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + final Map responseHeaders = new HashMap<>(); + responseHeaders.put("content-type", "application/json; charset=utf-8"); + responseHeaders.put("recurly-deprecated", "true"); + responseHeaders.put("recurly-sunset-date", "2026-01-01"); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(new HttpResponse(200, responseHeaders, "{}".getBytes(StandardCharsets.UTF_8))); + + final java.io.PrintStream originalOut = System.out; + final java.io.ByteArrayOutputStream captured = new java.io.ByteArrayOutputStream(); + System.setOut(new java.io.PrintStream(captured)); + try { + mockClientWith(mockAdapter).removeResource("resource-id"); + } finally { + System.setOut(originalOut); + } + + final String output = captured.toString(StandardCharsets.UTF_8.name()); + assertTrue(output.contains("WARNING"), "Expected a deprecation warning, got: " + output); + assertTrue(output.contains("2026-01-01"), "Expected the sunset date in the warning, got: " + output); + } + @Test public void testMakeRequestWithQueryParams() throws IOException { final ZonedDateTime dateTime = ZonedDateTime.now(); @@ -128,7 +184,11 @@ public void testMakeRequestWithQueryParams() throws IOException { final String url = urlCaptor.getValue(); assertTrue(url.contains("my_string=Aaron")); - assertTrue(url.contains("my_date_time=" + DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(dateTime).replace(":", "%3A").replace("+", "%2B"))); + assertTrue( + url.contains( + "my_date_time=" + + URLEncoder.encode( + DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(dateTime), "UTF-8"))); assertTrue(url.contains("my_integer=1")); assertTrue(url.contains("my_float=2.3")); assertTrue(url.contains("my_double=4.5")); @@ -158,6 +218,17 @@ public void testNonJsonError500() throws IOException { () -> mockClientWith(mockAdapter).getResource("code-aaron")); } + @Test + public void testNonJsonErrorViaRemoveResource() throws IOException { + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())) + .thenReturn(htmlResponse(500, "badness")); + + assertThrows( + InternalServerException.class, + () -> mockClientWith(mockAdapter).removeResource("code-aaron")); + } + @Test public void testInvalidApiKey() throws IOException { final HttpAdapter mockAdapter = mock(HttpAdapter.class); @@ -246,8 +317,8 @@ public void testBadMethodError() { @Test public void testSetApiUrl() { - try (MockedStatic theMock = mockStatic(BaseClient.class)) { - theMock.when(() -> BaseClient.envEnabled(eq("RECURLY_INSECURE"))).thenReturn(true); + try (MockedStatic theMock = mockStatic(Utils.class)) { + theMock.when(() -> Utils.envEnabled(eq("RECURLY_INSECURE"))).thenReturn(true); final MockClient client = new MockClient("apiKey"); final String newApiUrl = "https://my.base.url/"; @@ -259,8 +330,8 @@ public void testSetApiUrl() { @Test public void testCantSetApiUrlWithoutRecurlyInsecure() { - try (MockedStatic theMock = mockStatic(BaseClient.class)) { - theMock.when(() -> BaseClient.envEnabled(eq("RECURLY_INSECURE"))).thenReturn(false); + try (MockedStatic theMock = mockStatic(Utils.class)) { + theMock.when(() -> Utils.envEnabled(eq("RECURLY_INSECURE"))).thenReturn(false); final MockClient client = new MockClient("apiKey"); final String originalUrl = client.getApiUrl(); @@ -277,74 +348,90 @@ public void testWithoutClientOptions() { @Test public void testUsingRegionUSClientOptions() { - final ClientOptions options = new ClientOptions(); - options.setRegion(ClientOptions.Regions.US); + final ClientOptions options = ClientOptions.builder().region(ClientOptions.Regions.US).build(); assertEquals("https://v3.recurly.com", new MockClient("apiKey", options).getApiUrl()); } @Test public void testUsingRegionEUClientOptions() { - final ClientOptions options = new ClientOptions(); - options.setRegion(ClientOptions.Regions.EU); + final ClientOptions options = ClientOptions.builder().region(ClientOptions.Regions.EU).build(); assertEquals("https://v3.eu.recurly.com", new MockClient("apiKey", options).getApiUrl()); } @Test public void testIdempotencyKeyHeader() throws IOException { - final Call mCall = mock(Call.class); - final String idempotencyKey = "test-idempotency-key-123"; - Answer answer = (i) -> { - Request request = i.getArgument(0); - assertEquals(idempotencyKey, request.header("Idempotency-Key")); - return mCall; - }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(200, "OK", getResponseJson())); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())).thenReturn(jsonResponse(200, getResponseJson())); + final ArgumentCaptor> headersCaptor = ArgumentCaptor.forClass(Map.class); - final MockClient client = new MockClient("apiKey", mockOkHttpClient); + final String idempotencyKey = "test-idempotency-key-123"; + final MockClient client = mockClientWith(mockAdapter); final MyRequest body = new MyRequest(); final RequestOptions options = RequestOptions.builder().idempotencyKey(idempotencyKey).build(); client.createResource(body, options); + + verify(mockAdapter).execute(any(), any(), headersCaptor.capture(), any()); + assertEquals(idempotencyKey, headersCaptor.getValue().get("idempotency-key")); } @Test public void testRawHeaders() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { - Request request = i.getArgument(0); - assertEquals("bar", request.header("X-Custom-Foo")); - assertEquals("baz", request.header("X-Custom-Qux")); - return mCall; - }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(200, "OK", getResponseJson())); - - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); - - final MockClient client = new MockClient("apiKey", mockOkHttpClient); + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())).thenReturn(jsonResponse(200, getResponseJson())); + final ArgumentCaptor> headersCaptor = ArgumentCaptor.forClass(Map.class); + + final MockClient client = mockClientWith(mockAdapter); final MyRequest body = new MyRequest(); final RequestOptions options = RequestOptions.builder() .header("X-Custom-Foo", "bar") .header("X-Custom-Qux", "baz") .build(); client.createResource(body, options); + + verify(mockAdapter).execute(any(), any(), headersCaptor.capture(), any()); + final Map headers = headersCaptor.getValue(); + assertEquals("bar", headers.get("x-custom-foo")); + assertEquals("baz", headers.get("x-custom-qux")); } @Test - public void testNoIdempotencyKeyHeader() throws IOException { - final Call mCall = mock(Call.class); - Answer answer = (i) -> { - Request request = i.getArgument(0); - assertEquals(null, request.header("Idempotency-Key")); - return mCall; - }; - when(mCall.execute()).thenReturn(MockClient.buildResponse(200, "OK", getResponseJson())); + public void testRequestOptionsCannotOverrideBuiltInHeaders() throws IOException { + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())).thenReturn(jsonResponse(200, getResponseJson())); + final ArgumentCaptor> headersCaptor = ArgumentCaptor.forClass(Map.class); + + final MockClient client = mockClientWith(mockAdapter); + final MyRequest body = new MyRequest(); + // Caller supplies the reserved headers in two different casings: Title-case + // ("Authorization") and lower-case ("content-type"). Both must be overridden by the + // client's built-in values, and neither casing may survive as a stray duplicate. + final RequestOptions options = RequestOptions.builder() + .header("Authorization", "Bearer evil") + .header("content-type", "text/plain") + .build(); + client.createResource(body, options); - OkHttpClient mockOkHttpClient = MockClient.getMockOkHttpClient(answer); + verify(mockAdapter).execute(any(), any(), headersCaptor.capture(), any()); + final Map headers = headersCaptor.getValue(); + assertFalse(headers.get("authorization").contains("evil")); + assertEquals("application/json", headers.get("content-type")); + // No Title-cased duplicate lingers, so the effective header is deterministic. + assertEquals(null, headers.get("Authorization")); + assertEquals(null, headers.get("Content-Type")); + } + + @Test + public void testNoIdempotencyKeyHeader() throws IOException { + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + when(mockAdapter.execute(any(), any(), any(), any())).thenReturn(jsonResponse(200, getResponseJson())); + final ArgumentCaptor> headersCaptor = ArgumentCaptor.forClass(Map.class); - final MockClient client = new MockClient("apiKey", mockOkHttpClient); + final MockClient client = mockClientWith(mockAdapter); final MyRequest body = new MyRequest(); client.createResource(body); + + verify(mockAdapter).execute(any(), any(), headersCaptor.capture(), any()); + assertEquals(null, headersCaptor.getValue().get("idempotency-key")); } @Test @@ -386,6 +473,28 @@ public void testGetRecordCountMissingHeader() throws IOException { () -> mockClientWith(mockAdapter).getRecordCount("/resources", null)); } + @Test + public void testGetRecordCountMergesQueryParamsIntoPathWithExistingQueryString() throws IOException { + final HttpAdapter mockAdapter = mock(HttpAdapter.class); + final Map headers = new HashMap<>(); + headers.put("recurly-total-records", "5"); + when(mockAdapter.execute(eq("HEAD"), any(), any(), isNull())) + .thenReturn(new HttpResponse(200, headers, new byte[0])); + + final HashMap queryParams = new HashMap<>(); + queryParams.put("limit", 20); + + final ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(String.class); + mockClientWith(mockAdapter).getRecordCount("/resources?cursor=xyz", queryParams); + + verify(mockAdapter).execute(eq("HEAD"), urlCaptor.capture(), any(), isNull()); + final String url = urlCaptor.getValue(); + + assertEquals(1, url.length() - url.replace("?", "").length()); + assertTrue(url.contains("cursor=xyz")); + assertTrue(url.contains("limit=20")); + } + @Test public void testHttpResponseNullHeadersThrows() { assertThrows(NullPointerException.class, () -> new HttpResponse(200, null, new byte[0])); diff --git a/src/test/java/com/recurly/v3/ClientOptionsLegacyApiTest.java b/src/test/java/com/recurly/v3/ClientOptionsLegacyApiTest.java new file mode 100644 index 00000000..590b66e0 --- /dev/null +++ b/src/test/java/com/recurly/v3/ClientOptionsLegacyApiTest.java @@ -0,0 +1,37 @@ +package com.recurly.v3; + +/** + * Regression coverage for ClientOptions' deprecated no-arg constructor and setters. This whole + * file can be deleted once the deprecated API is removed in the next major version. + */ +import com.recurly.v3.http.HttpAdapter; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; + +@SuppressWarnings("deprecation") +public class ClientOptionsLegacyApiTest { + + @Test + public void testNoArgConstructorDefaultsToUSRegion() { + final ClientOptions options = new ClientOptions(); + assertEquals("https://v3.recurly.com", options.getBaseUrl()); + } + + @Test + public void testSetRegionEU() { + final ClientOptions options = new ClientOptions(); + options.setRegion(ClientOptions.Regions.EU); + assertEquals("https://v3.eu.recurly.com", options.getBaseUrl()); + } + + @Test + public void testSetHttpAdapter() { + final HttpAdapter adapter = mock(HttpAdapter.class); + final ClientOptions options = new ClientOptions(); + options.setHttpAdapter(adapter); + assertSame(adapter, options.getHttpAdapter()); + } +} diff --git a/src/test/java/com/recurly/v3/ClientOptionsTest.java b/src/test/java/com/recurly/v3/ClientOptionsTest.java new file mode 100644 index 00000000..8d5b44f9 --- /dev/null +++ b/src/test/java/com/recurly/v3/ClientOptionsTest.java @@ -0,0 +1,39 @@ +package com.recurly.v3; + +import com.recurly.v3.http.HttpAdapter; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; + +public class ClientOptionsTest { + + @Test + public void testBuilderDefaultsToUSRegion() { + final ClientOptions options = ClientOptions.builder().build(); + assertEquals("https://v3.recurly.com", options.getBaseUrl()); + } + + @Test + public void testBuilderWithEURegion() { + final ClientOptions options = ClientOptions.builder().region(ClientOptions.Regions.EU).build(); + assertEquals("https://v3.eu.recurly.com", options.getBaseUrl()); + } + + @Test + public void testBuilderWithHttpAdapter() { + final HttpAdapter adapter = mock(HttpAdapter.class); + final ClientOptions options = ClientOptions.builder().httpAdapter(adapter).build(); + assertSame(adapter, options.getHttpAdapter()); + } + + @Test + public void testBuilderChaining() { + final HttpAdapter adapter = mock(HttpAdapter.class); + final ClientOptions options = + ClientOptions.builder().region(ClientOptions.Regions.EU).httpAdapter(adapter).build(); + assertEquals("https://v3.eu.recurly.com", options.getBaseUrl()); + assertSame(adapter, options.getHttpAdapter()); + } +} diff --git a/src/test/java/com/recurly/v3/PagerTest.java b/src/test/java/com/recurly/v3/PagerTest.java index 5485e2f9..e0f7ef05 100644 --- a/src/test/java/com/recurly/v3/PagerTest.java +++ b/src/test/java/com/recurly/v3/PagerTest.java @@ -1,5 +1,7 @@ package com.recurly.v3; +import static com.recurly.v3.fixtures.HttpTestFixtures.jsonResponse; +import static com.recurly.v3.fixtures.HttpTestFixtures.mockClientWith; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; @@ -8,7 +10,6 @@ import com.recurly.v3.http.HttpAdapter; import com.recurly.v3.http.HttpResponse; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; import java.util.NoSuchElementException; @@ -17,18 +18,6 @@ public class PagerTest { - private static HttpResponse jsonResponse(final int statusCode, final String body) { - final Map headers = new HashMap<>(); - headers.put("content-type", "application/json; charset=utf-8"); - return new HttpResponse(statusCode, headers, body.getBytes(StandardCharsets.UTF_8)); - } - - private static MockClient mockClientWith(final HttpAdapter adapter) { - final ClientOptions options = new ClientOptions(); - options.setHttpAdapter(adapter); - return new MockClient("apiKey", options); - } - @Test public void testForEach() throws IOException { final HttpAdapter mockAdapter = mock(HttpAdapter.class); diff --git a/src/test/java/com/recurly/v3/RequestOptionsTest.java b/src/test/java/com/recurly/v3/RequestOptionsTest.java new file mode 100644 index 00000000..04cf485d --- /dev/null +++ b/src/test/java/com/recurly/v3/RequestOptionsTest.java @@ -0,0 +1,40 @@ +package com.recurly.v3; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.util.Map; +import org.junit.jupiter.api.Test; + +public class RequestOptionsTest { + + @Test + public void headerNormalizesReservedHeaderNameToLowerCase() { + final RequestOptions options = + RequestOptions.builder().header("Authorization", "Bearer token").build(); + + final Map headers = options.getHeaders(); + assertEquals("Bearer token", headers.get("authorization")); + assertNull(headers.get("Authorization"), "header name must be normalized to lower case"); + } + + @Test + public void headerNormalizesCustomHeaderNameToLowerCase() { + final RequestOptions options = + RequestOptions.builder().header("X-Custom-Header", "value").build(); + + final Map headers = options.getHeaders(); + assertEquals("value", headers.get("x-custom-header")); + assertNull(headers.get("X-Custom-Header")); + } + + @Test + public void headerWithDifferentCasingCollapsesToSingleEntry() { + final RequestOptions options = + RequestOptions.builder().header("X-Dup", "first").header("x-dup", "second").build(); + + final Map headers = options.getHeaders(); + assertEquals(1, headers.size()); + assertEquals("second", headers.get("x-dup")); + } +} diff --git a/src/test/java/com/recurly/v3/fixtures/HttpTestFixtures.java b/src/test/java/com/recurly/v3/fixtures/HttpTestFixtures.java new file mode 100644 index 00000000..625d81b6 --- /dev/null +++ b/src/test/java/com/recurly/v3/fixtures/HttpTestFixtures.java @@ -0,0 +1,24 @@ +package com.recurly.v3.fixtures; + +import com.recurly.v3.ClientOptions; +import com.recurly.v3.http.HttpAdapter; +import com.recurly.v3.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; + +public final class HttpTestFixtures { + + private HttpTestFixtures() {} + + public static HttpResponse jsonResponse(final int statusCode, final String body) { + final Map headers = new HashMap<>(); + headers.put("content-type", "application/json; charset=utf-8"); + return new HttpResponse(statusCode, headers, body.getBytes(StandardCharsets.UTF_8)); + } + + public static MockClient mockClientWith(final HttpAdapter adapter) { + final ClientOptions options = ClientOptions.builder().httpAdapter(adapter).build(); + return new MockClient("apiKey", options); + } +} diff --git a/src/test/java/com/recurly/v3/http/DefaultHttpAdapterGzipTest.java b/src/test/java/com/recurly/v3/http/DefaultHttpAdapterGzipTest.java new file mode 100644 index 00000000..a46a1e94 --- /dev/null +++ b/src/test/java/com/recurly/v3/http/DefaultHttpAdapterGzipTest.java @@ -0,0 +1,154 @@ +package com.recurly.v3.http; + +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.verification.LoggedRequest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnJre; +import org.junit.jupiter.api.condition.JRE; + +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; +import java.util.zip.GZIPOutputStream; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.junit.jupiter.api.Assertions.*; + +/** + * Gzip-specific behavior of {@link DefaultHttpAdapter}. Not part of {@link HttpAdapterContract} + * because transparent gzip is an implementation detail of the HttpURLConnection-based default + * adapter, not a requirement of the {@link HttpAdapter} interface itself. + */ +@DisabledOnJre(JRE.JAVA_8) +class DefaultHttpAdapterGzipTest { + + private WireMockServer server; + private HttpAdapter adapter; + + @BeforeEach + void setUp() { + // Disable WireMock's own auto-gzip of stub responses so tests control compression explicitly. + server = new WireMockServer(wireMockConfig().dynamicPort().gzipDisabled(true)); + server.start(); + adapter = new DefaultHttpAdapter(); + } + + @AfterEach + void tearDown() { + server.stop(); + } + + @Test + void acceptEncodingGzip_addedWhenNotPresent() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts")).willReturn(ok().withBody("{}"))); + + adapter.execute("GET", url("/accounts"), new HashMap<>(), null); + + LoggedRequest req = singleRequest(); + assertEquals("gzip", req.getHeader("Accept-Encoding"), + "Accept-Encoding: gzip must be added when the caller did not specify one"); + } + + @Test + void acceptEncodingGzip_notOverridden_whenCallerSetsOwn() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts")).willReturn(ok().withBody("{}"))); + + Map headers = new HashMap<>(); + headers.put("Accept-Encoding", "identity"); + adapter.execute("GET", url("/accounts"), headers, null); + + LoggedRequest req = singleRequest(); + assertEquals("identity", req.getHeader("Accept-Encoding"), + "Caller-supplied Accept-Encoding must not be overridden"); + } + + @Test + void gzipResponse_decompressedTransparently() throws Exception { + byte[] gzipped = gzip("{\"hello\":\"world\"}"); + server.stubFor(any(urlPathEqualTo("/accounts")) + .willReturn(aResponse().withStatus(200) + .withHeader("Content-Encoding", "gzip") + .withBody(gzipped))); + + HttpResponse response = adapter.execute("GET", url("/accounts"), new HashMap<>(), null); + + assertEquals("{\"hello\":\"world\"}", new String(response.getBody(), StandardCharsets.UTF_8), + "Gzip-encoded response body must be transparently decompressed"); + } + + @Test + void gzipResponse_stripsContentEncodingAndContentLengthHeaders() throws Exception { + byte[] gzipped = gzip("{\"hello\":\"world\"}"); + server.stubFor(any(urlPathEqualTo("/accounts")) + .willReturn(aResponse().withStatus(200) + .withHeader("Content-Encoding", "gzip") + .withBody(gzipped))); + + HttpResponse response = adapter.execute("GET", url("/accounts"), new HashMap<>(), null); + + Map headers = response.getHeaders(); + assertFalse(headers.containsKey("content-encoding"), + "content-encoding must be stripped once the body has been decompressed"); + assertFalse(headers.containsKey("content-length"), + "content-length (of the compressed body) must be stripped once decompressed"); + } + + @Test + void gzipErrorResponse_decompressedTransparently() throws Exception { + byte[] gzipped = gzip("{\"error\":\"boom\"}"); + server.stubFor(any(urlPathEqualTo("/accounts")) + .willReturn(aResponse().withStatus(500) + .withHeader("Content-Encoding", "gzip") + .withBody(gzipped))); + + HttpResponse response = adapter.execute("GET", url("/accounts"), new HashMap<>(), null); + + assertEquals(500, response.getStatusCode()); + assertEquals("{\"error\":\"boom\"}", new String(response.getBody(), StandardCharsets.UTF_8), + "Gzip-encoded error response body must be transparently decompressed"); + } + + @Test + void malformedGzipBody_throwsInsteadOfLeaking() { + server.stubFor(any(urlPathEqualTo("/accounts")) + .willReturn(aResponse().withStatus(200) + .withHeader("Content-Encoding", "gzip") + .withBody("not actually gzip"))); + + assertThrows(java.io.IOException.class, + () -> adapter.execute("GET", url("/accounts"), new HashMap<>(), null), + "A Content-Encoding: gzip response with a non-gzip body must fail loudly " + + "instead of leaking the underlying connection"); + } + + @Test + void nonGzipResponse_bodyAndHeadersUntouched() throws Exception { + server.stubFor(any(urlPathEqualTo("/accounts")) + .willReturn(ok().withBody("{\"hello\":\"world\"}"))); + + HttpResponse response = adapter.execute("GET", url("/accounts"), new HashMap<>(), null); + + assertEquals("{\"hello\":\"world\"}", new String(response.getBody(), StandardCharsets.UTF_8)); + assertFalse(response.getHeaders().containsKey("content-encoding")); + } + + private String url(final String path) { + return WireMockTestSupport.url(server, path); + } + + private LoggedRequest singleRequest() { + return WireMockTestSupport.singleRequest(server); + } + + private static byte[] gzip(final String content) throws Exception { + final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (final GZIPOutputStream gzos = new GZIPOutputStream(baos)) { + gzos.write(content.getBytes(StandardCharsets.UTF_8)); + } + return baos.toByteArray(); + } +} diff --git a/src/test/java/com/recurly/v3/http/DefaultHttpAdapterTimeoutTest.java b/src/test/java/com/recurly/v3/http/DefaultHttpAdapterTimeoutTest.java new file mode 100644 index 00000000..66d7f48d --- /dev/null +++ b/src/test/java/com/recurly/v3/http/DefaultHttpAdapterTimeoutTest.java @@ -0,0 +1,50 @@ +package com.recurly.v3.http; + +import com.github.tomakehurst.wiremock.WireMockServer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnJre; +import org.junit.jupiter.api.condition.JRE; + +import java.io.IOException; +import java.util.HashMap; + +import static com.github.tomakehurst.wiremock.client.WireMock.any; +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Verifies that {@link DefaultHttpAdapter#DefaultHttpAdapter(int)} actually bounds the read phase + * with the supplied timeout, rather than always falling back to the default. + */ +@DisabledOnJre(JRE.JAVA_8) +class DefaultHttpAdapterTimeoutTest { + + private WireMockServer server; + + @BeforeEach + void setUp() { + server = new WireMockServer(wireMockConfig().dynamicPort()); + server.start(); + } + + @AfterEach + void tearDown() { + server.stop(); + } + + @Test + void customTimeout_boundsReadPhase() { + server.stubFor(any(urlPathEqualTo("/accounts")) + .willReturn(aResponse().withStatus(200).withBody("{}").withFixedDelay(500))); + + final HttpAdapter adapter = new DefaultHttpAdapter(100); + + assertThrows(IOException.class, + () -> adapter.execute("GET", WireMockTestSupport.url(server, "/accounts"), new HashMap<>(), null), + "A response slower than the configured timeout must fail with an IOException"); + } +} diff --git a/src/test/java/com/recurly/v3/http/HttpAdapterContract.java b/src/test/java/com/recurly/v3/http/HttpAdapterContract.java index 856c575e..8396c6f2 100644 --- a/src/test/java/com/recurly/v3/http/HttpAdapterContract.java +++ b/src/test/java/com/recurly/v3/http/HttpAdapterContract.java @@ -1,7 +1,6 @@ package com.recurly.v3.http; import com.github.tomakehurst.wiremock.WireMockServer; -import com.github.tomakehurst.wiremock.stubbing.ServeEvent; import com.github.tomakehurst.wiremock.verification.LoggedRequest; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -12,7 +11,6 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; @@ -73,14 +71,14 @@ public abstract class HttpAdapterContract { protected abstract HttpAdapter createAdapter(); @BeforeEach - void setUp() { + final void setUp() { server = new WireMockServer(wireMockConfig().dynamicPort()); server.start(); adapter = createAdapter(); } @AfterEach - void tearDown() { + final void tearDown() { server.stop(); } @@ -172,6 +170,7 @@ void headRequest_sendsCorrectMethodAndNoBody() throws Exception { assertEquals("HEAD", req.getMethod().getName()); assertEquals(0, req.getBody().length, "HEAD must not send a body"); assertNotNull(response.getBody(), "HEAD response body must be a non-null byte array"); + assertEquals(0, response.getBody().length, "HEAD response body must be empty"); } // --------------------------------------------------------------------------- @@ -352,7 +351,7 @@ void concurrentRequests_completeSafely() throws Exception { // --------------------------------------------------------------------------- private String url(final String path) { - return "http://localhost:" + server.port() + path; + return WireMockTestSupport.url(server, path); } private static Map noHeaders() { @@ -366,8 +365,6 @@ private static Map jsonHeaders() { } private LoggedRequest singleRequest() { - List events = server.getAllServeEvents(); - assertFalse(events.isEmpty(), "No request was received by the mock server"); - return events.get(0).getRequest(); + return WireMockTestSupport.singleRequest(server); } } diff --git a/src/test/java/com/recurly/v3/http/WireMockTestSupport.java b/src/test/java/com/recurly/v3/http/WireMockTestSupport.java new file mode 100644 index 00000000..61b51ac9 --- /dev/null +++ b/src/test/java/com/recurly/v3/http/WireMockTestSupport.java @@ -0,0 +1,24 @@ +package com.recurly.v3.http; + +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.stubbing.ServeEvent; +import com.github.tomakehurst.wiremock.verification.LoggedRequest; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertFalse; + +final class WireMockTestSupport { + + private WireMockTestSupport() {} + + static String url(final WireMockServer server, final String path) { + return "http://localhost:" + server.port() + path; + } + + static LoggedRequest singleRequest(final WireMockServer server) { + final List events = server.getAllServeEvents(); + assertFalse(events.isEmpty(), "No request was received by the mock server"); + return events.get(0).getRequest(); + } +} diff --git a/src/test/java/com/recurly/v3/internal/UtilsTest.java b/src/test/java/com/recurly/v3/internal/UtilsTest.java new file mode 100644 index 00000000..aaf884c2 --- /dev/null +++ b/src/test/java/com/recurly/v3/internal/UtilsTest.java @@ -0,0 +1,13 @@ +package com.recurly.v3.internal; + +import static org.junit.jupiter.api.Assertions.assertFalse; + +import org.junit.jupiter.api.Test; + +public class UtilsTest { + + @Test + public void envEnabled_falseWhenVariableIsUnset() { + assertFalse(Utils.envEnabled("RECURLY_JAVA_CLIENT_UTILS_TEST_UNSET_VAR")); + } +}