diff --git a/Makefile b/Makefile index ed9965b7..05f23b0a 100644 --- a/Makefile +++ b/Makefile @@ -21,8 +21,8 @@ run-contract-tests: | VERSION=v2 PARAMS="-url http://localhost:8001 -host 10.0.2.2 -debug -skip-from $(SUPPRESSION_FILE) $(TEST_HARNESS_PARAMS_V2)" sh @echo "Running SDK contract test v3..." @curl $${GITHUB_TOKEN:+ -H "Authorization: Token $${GITHUB_TOKEN}"} \ - -s https://raw.githubusercontent.com/launchdarkly/sdk-test-harness/v3.1.0-alpha.6/downloader/run.sh \ - | VERSION=v3.1.0-alpha.6 PARAMS="-url http://localhost:8001 -host 10.0.2.2 -debug -stop-service-at-end -skip-from $(SUPPRESSION_FILE_FDV2) $(TEST_HARNESS_PARAMS_V3)" sh + -s https://raw.githubusercontent.com/launchdarkly/sdk-test-harness/v3.2.0-alpha.10/downloader/run.sh \ + | VERSION=v3.2.0-alpha.10 PARAMS="-url http://localhost:8001 -host 10.0.2.2 -debug -stop-service-at-end -skip-from $(SUPPRESSION_FILE_FDV2) $(TEST_HARNESS_PARAMS_V3)" sh contract-tests: build-contract-tests start-emulator start-contract-test-service run-contract-tests diff --git a/contract-tests/src/main/java/com/launchdarkly/sdktest/Representations.java b/contract-tests/src/main/java/com/launchdarkly/sdktest/Representations.java index 4b7aa650..5c94b580 100644 --- a/contract-tests/src/main/java/com/launchdarkly/sdktest/Representations.java +++ b/contract-tests/src/main/java/com/launchdarkly/sdktest/Representations.java @@ -78,6 +78,7 @@ public static class SdkConfigClientSideParams { LDContext initialContext; boolean evaluationReasons; boolean useReport; + boolean usePost; boolean includeEnvironmentAttributes; } diff --git a/contract-tests/src/main/java/com/launchdarkly/sdktest/SdkClientEntity.java b/contract-tests/src/main/java/com/launchdarkly/sdktest/SdkClientEntity.java index f9627704..2056d47e 100644 --- a/contract-tests/src/main/java/com/launchdarkly/sdktest/SdkClientEntity.java +++ b/contract-tests/src/main/java/com/launchdarkly/sdktest/SdkClientEntity.java @@ -329,7 +329,9 @@ private LDConfig buildSdkConfig(SdkConfigParams params, LDLogAdapter logAdapter, builder.evaluationReasons(params.clientSide.evaluationReasons); builder.http( - Components.httpConfiguration().useReport(params.clientSide.useReport) + Components.httpConfiguration() + .useReport(params.clientSide.useReport) + .usePost(params.clientSide.usePost) ); if (params.tags != null) { diff --git a/contract-tests/src/main/java/com/launchdarkly/sdktest/TestService.java b/contract-tests/src/main/java/com/launchdarkly/sdktest/TestService.java index 50acf5e5..246388bc 100644 --- a/contract-tests/src/main/java/com/launchdarkly/sdktest/TestService.java +++ b/contract-tests/src/main/java/com/launchdarkly/sdktest/TestService.java @@ -45,7 +45,8 @@ public class TestService extends NanoHTTPD { "client-per-context-summaries", "client-event-source-http-errors", "retry-conformance-fdv1-streaming", - "retry-conformance-fdv1-polling" + "retry-conformance-fdv1-polling", + "client-use-post" }; private static final String MIME_JSON = "application/json"; static final Gson gson = new GsonBuilder() diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java index 34b47a2f..b3413f85 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java @@ -233,7 +233,8 @@ public HttpConfiguration build(ClientContext clientContext) { connectTimeoutMillis, headers, headerTransform, - useReport + useReport, + usePost ); } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DataSystemComponents.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DataSystemComponents.java index 4d9dfea8..7e6c5ec1 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DataSystemComponents.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DataSystemComponents.java @@ -141,7 +141,7 @@ public Synchronizer build(DataSourceBuildInputs inputs) { StandardEndpoints.FDV2_STREAMING_REQUEST_BASE_PATH, requestor, initialReconnectDelayMillis, - inputs.isEvaluationReasons(), inputs.getHttp().isUseReport(), + inputs.isEvaluationReasons(), inputs.getHttp().shouldUsePost(), httpProps, inputs.getSharedExecutor(), inputs.getBaseLogger(), null); } @@ -159,10 +159,13 @@ public FDv1PollingSynchronizerBuilderImpl pollIntervalMillis(int pollIntervalMil @Override public Synchronizer build(DataSourceBuildInputs inputs) { + // The FDv1 endpoints have no POST variant. When usePost is enabled, the fetcher + // sends the context in the request body with REPORT so it stays out of the + // request URI. useReport has no effect inside the data system. FeatureFetcher fetcher = new HttpFeatureFlagFetcher( inputs.getServiceEndpoints().getPollingBaseUri(), inputs.isEvaluationReasons(), - inputs.getHttp().isUseReport(), + inputs.getHttp().shouldUsePost(), LDUtil.makeHttpProperties(inputs.getHttp()), inputs.getCacheDir(), inputs.getBaseLogger() @@ -361,8 +364,8 @@ private static FDv2Requestor makePollingRequestor(DataSourceBuildInputs inputs, return new DefaultFDv2Requestor( inputs.getEvaluationContext(), pollingBase, StandardEndpoints.FDV2_POLLING_REQUEST_GET_BASE_PATH, - StandardEndpoints.FDV2_POLLING_REQUEST_REPORT_BASE_PATH, - httpProps, inputs.getHttp().isUseReport(), + StandardEndpoints.FDV2_POLLING_REQUEST_POST_BASE_PATH, + httpProps, inputs.getHttp().shouldUsePost(), inputs.isEvaluationReasons(), null, inputs.getBaseLogger()); } } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DefaultFDv2Requestor.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DefaultFDv2Requestor.java index 2ffa79e8..31c254dd 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DefaultFDv2Requestor.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DefaultFDv2Requestor.java @@ -31,16 +31,15 @@ /** * Default OkHttp-based implementation of {@link FDv2Requestor}. *

- * Builds GET or REPORT requests to the FDv2 polling endpoint. If the current selector is + * Builds GET or POST requests to the FDv2 polling endpoint. If the current selector is * non-empty, its state is sent as the {@code basis} query parameter. ETag tracking is used * to detect 304 Not Modified responses; when the server returns 304 the application layer * treats it as {@code ChangeSetType.None} (no flags changed), so no OkHttp disk cache is - * needed or used. + * needed or used. ETag tracking runs only for GET requests. *

* The OkHttpClient is closed by {@link #close()}. */ final class DefaultFDv2Requestor implements FDv2Requestor { - private static final String METHOD_REPORT = "REPORT"; private static final String BASIS_PARAM = "basis"; private static final String FILTER_PARAM = "filter"; private static final String WITH_REASONS_PARAM = "withReasons"; @@ -50,10 +49,10 @@ final class DefaultFDv2Requestor implements FDv2Requestor { private final OkHttpClient httpClient; private final URI pollingUri; private final okhttp3.Headers headers; - private final boolean useReport; + private final boolean usePost; private final boolean evaluationReasons; @Nullable - private final RequestBody reportBody; + private final RequestBody postBody; @Nullable private final String payloadFilter; private final LDLogger logger; @@ -68,9 +67,9 @@ final class DefaultFDv2Requestor implements FDv2Requestor { * @param evaluationContext the context to evaluate flags for * @param baseUri polling base URI from service endpoints * @param getRequestPath path for GET requests (context appended as a path segment) - * @param reportRequestPath path for REPORT requests (context sent in the request body) + * @param postRequestPath path for POST requests (context sent in the request body) * @param httpProperties SDK HTTP configuration (timeouts, proxy, TLS, user-agent, etc.) - * @param useReport if true, send context in the request body via REPORT; otherwise + * @param usePost if true, send context in the request body via POST; otherwise * append the base64-encoded context to the GET path * @param evaluationReasons if true, append {@code withReasons=true} to the query string * @param payloadFilter optional payload filter key; sent as {@code filter} query param @@ -80,13 +79,13 @@ final class DefaultFDv2Requestor implements FDv2Requestor { @NonNull LDContext evaluationContext, @NonNull URI baseUri, @NonNull String getRequestPath, - @NonNull String reportRequestPath, + @NonNull String postRequestPath, @NonNull HttpProperties httpProperties, - boolean useReport, + boolean usePost, boolean evaluationReasons, @Nullable String payloadFilter, @NonNull LDLogger logger) { - this.useReport = useReport; + this.usePost = usePost; this.evaluationReasons = evaluationReasons; this.payloadFilter = payloadFilter; this.logger = logger; @@ -94,14 +93,14 @@ final class DefaultFDv2Requestor implements FDv2Requestor { // Precompute the base polling URI for the chosen request method. For GET, the // base64-encoded context is a fixed path segment (context never changes after - // construction). For REPORT, the context goes in the request body so no path + // construction). For POST, the context goes in the request body so no path // segment is needed. URI basePollingUri = HttpHelpers.concatenateUriPath(baseUri, - useReport ? reportRequestPath : getRequestPath); - this.pollingUri = useReport + usePost ? postRequestPath : getRequestPath); + this.pollingUri = usePost ? basePollingUri : HttpHelpers.concatenateUriPath(basePollingUri, LDUtil.urlSafeBase64(evaluationContext)); - this.reportBody = useReport + this.postBody = usePost ? RequestBody.create(JsonSerialization.serialize(evaluationContext), JSON) : null; @@ -139,8 +138,8 @@ public Future poll(@NonNull Selector selector) { .url(requestUri.toURL()) .headers(headers); - if (useReport) { - reqBuilder.method(METHOD_REPORT, reportBody); + if (usePost) { + reqBuilder.post(postBody); } else { synchronized (etagLock) { if (!requestUri.equals(lastRequestUri)) { @@ -207,7 +206,7 @@ private void handleResponse( return; } - if (!useReport) { + if (!usePost) { synchronized (etagLock) { cachedEtag = response.header(ETAG_HEADER); } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2DataSourceBuilder.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2DataSourceBuilder.java index 769eaa31..2fdfc103 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2DataSourceBuilder.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2DataSourceBuilder.java @@ -39,6 +39,10 @@ class FDv2DataSourceBuilder implements ComponentConfigurer, Closeabl private boolean includeInitializers = true; // start with initializers private ScheduledExecutorService sharedExecutor; + // The useReport warning logs once per builder; build runs again on every + // connection mode switch. + private boolean useReportWarningLogged; + FDv2DataSourceBuilder() { this(DataSystemComponents.makeDefaultModeTable(), ConnectionMode.STREAMING, ModeResolutionTable.MOBILE); } @@ -98,6 +102,13 @@ ModeDefinition getModeDefinition(@NonNull ConnectionMode mode) { @Override public DataSource build(ClientContext clientContext) { + if (clientContext.getHttp().isUseReport() && !useReportWarningLogged) { + useReportWarningLogged = true; + clientContext.getBaseLogger().warn( + "The useReport option has no effect when the data system is configured;" + + " requests use GET unless usePost is enabled."); + } + if (sharedExecutor == null) { // Pool size 4: supports the FDv2DataSource main loop (1), an active synchronizer // such as the streaming event loop (1), FDv2DataSource condition timers for diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizer.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizer.java index ad6daf6d..c44e0f71 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizer.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizer.java @@ -53,7 +53,7 @@ *

*/ final class FDv2StreamingSynchronizer implements Synchronizer { - private static final String METHOD_REPORT = "REPORT"; + private static final String METHOD_POST = "POST"; private static final String PING = "ping"; private static final long READ_TIMEOUT_MS = 300_000; // 5 minutes private static final long MAX_RECONNECT_TIME_MS = 300_000; // 5 minutes @@ -61,7 +61,7 @@ final class FDv2StreamingSynchronizer implements Synchronizer { private final HttpProperties httpProperties; private final URI streamBaseUri; private final String streamRequestPath; - private final boolean useReport; + private final boolean usePost; private final LDContext evaluationContext; private final SelectorSource selectorSource; @Nullable @@ -92,7 +92,7 @@ final class FDv2StreamingSynchronizer implements Synchronizer { * @param requestor optional requestor for handling ping events via poll; may be null * @param initialReconnectDelayMillis delay before reconnecting after an error, in milliseconds * @param evaluationReasons true to request evaluation reasons in the stream - * @param useReport true to use HTTP REPORT for the request body + * @param usePost true to send the context in the request body via HTTP POST * @param httpProperties HTTP configuration for the stream request * @param executor executor used to run the streaming loop on a background * thread; should use background-priority threads @@ -107,7 +107,7 @@ final class FDv2StreamingSynchronizer implements Synchronizer { @Nullable FDv2Requestor requestor, int initialReconnectDelayMillis, boolean evaluationReasons, - boolean useReport, + boolean usePost, @NonNull HttpProperties httpProperties, @NonNull Executor executor, @NonNull LDLogger logger, @@ -120,7 +120,7 @@ final class FDv2StreamingSynchronizer implements Synchronizer { this.requestor = requestor; this.initialReconnectDelayMillis = initialReconnectDelayMillis; this.evaluationReasons = evaluationReasons; - this.useReport = useReport; + this.usePost = usePost; this.httpProperties = httpProperties; this.executor = executor; this.logger = logger; @@ -188,9 +188,9 @@ private void startStream() { return reqBuilder.build(); }); - if (useReport) { + if (usePost) { connectStrategy = connectStrategy.methodAndBody( - METHOD_REPORT, + METHOD_POST, RequestBody.create(JsonSerialization.serialize(evaluationContext), JSON)); } @@ -244,7 +244,7 @@ private void startStream() { private URI getStreamUri() { URI uri = HttpHelpers.concatenateUriPath(streamBaseUri, streamRequestPath); - if (!useReport) { + if (!usePost) { uri = HttpHelpers.concatenateUriPath(uri, LDUtil.urlSafeBase64(evaluationContext)); } if (evaluationReasons) { diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StandardEndpoints.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StandardEndpoints.java index 06c51e32..0c216aa3 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StandardEndpoints.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StandardEndpoints.java @@ -18,7 +18,7 @@ private StandardEndpoints() {} static final String DIAGNOSTIC_EVENTS_REQUEST_PATH = "/mobile/events/diagnostic"; static final String FDV2_POLLING_REQUEST_GET_BASE_PATH = "/sdk/poll/eval"; - static final String FDV2_POLLING_REQUEST_REPORT_BASE_PATH = "/sdk/poll/eval"; + static final String FDV2_POLLING_REQUEST_POST_BASE_PATH = "/sdk/poll/eval"; static final String FDV2_STREAMING_REQUEST_BASE_PATH = "/sdk/stream/eval"; /** diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/HttpConfigurationBuilder.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/HttpConfigurationBuilder.java index d59f1680..26531d8f 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/HttpConfigurationBuilder.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/HttpConfigurationBuilder.java @@ -46,6 +46,11 @@ public abstract class HttpConfigurationBuilder implements ComponentConfigurer + * By default, polling and streaming connections are made with the GET method, with the + * evaluation context data encoded into the request URI. When this option is enabled, the + * SDK sends the context data in the request body instead. This keeps the context out of + * URL-based request logs and CDN logs. + *

+ * This option only takes effect when the data system is configured with + * {@link com.launchdarkly.sdk.android.LDConfig.Builder#dataSystem(DataSystemBuilder)}. + * + * @param usePost true to enable the POST method + * @return the builder + */ + public HttpConfigurationBuilder usePost(boolean usePost) { + this.usePost = usePost; + return this; + } + /** * For use by wrapper libraries to set an identifying name for the wrapper being used. This will be included in a * header during requests to the LaunchDarkly servers to allow recording metrics on the usage of diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/HttpConfiguration.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/HttpConfiguration.java index a0257cc2..c9e53d15 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/HttpConfiguration.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/HttpConfiguration.java @@ -25,6 +25,7 @@ public final class HttpConfiguration { private final Map defaultHeaders; private final LDHeaderUpdater headerTransform; private final boolean useReport; + private final boolean usePost; /** * Creates an instance. @@ -33,18 +34,38 @@ public final class HttpConfiguration { * @param defaultHeaders see {@link #getDefaultHeaders()} * @param headerTransform see {@link #getHeaderTransform()} * @param useReport see {@link #isUseReport()} + * @param usePost see {@link #shouldUsePost()} */ public HttpConfiguration( int connectTimeoutMillis, Map defaultHeaders, LDHeaderUpdater headerTransform, - boolean useReport + boolean useReport, + boolean usePost ) { super(); this.connectTimeoutMillis = connectTimeoutMillis; this.defaultHeaders = defaultHeaders == null ? emptyMap() : new HashMap<>(defaultHeaders); this.headerTransform = headerTransform; this.useReport = useReport; + this.usePost = usePost; + } + + /** + * Creates an instance with {@code usePost} disabled. + * + * @param connectTimeoutMillis see {@link #getConnectTimeoutMillis()} + * @param defaultHeaders see {@link #getDefaultHeaders()} + * @param headerTransform see {@link #getHeaderTransform()} + * @param useReport see {@link #isUseReport()} + */ + public HttpConfiguration( + int connectTimeoutMillis, + Map defaultHeaders, + LDHeaderUpdater headerTransform, + boolean useReport + ) { + this(connectTimeoutMillis, defaultHeaders, headerTransform, useReport, false); } /** @@ -84,4 +105,13 @@ public LDHeaderUpdater getHeaderTransform() { public boolean isUseReport() { return useReport; } + + /** + * The setting for whether to use the HTTP POST method for FDv2 flag requests. + * + * @return true to use HTTP POST + */ + public boolean shouldUsePost() { + return usePost; + } } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DataSystemComponentsTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DataSystemComponentsTest.java new file mode 100644 index 00000000..ed30edac --- /dev/null +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DataSystemComponentsTest.java @@ -0,0 +1,181 @@ +package com.launchdarkly.sdk.android; + +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.sdk.LDContext; +import com.launchdarkly.sdk.android.interfaces.ServiceEndpoints; +import com.launchdarkly.sdk.android.subsystems.FDv2SourceResult; +import com.launchdarkly.sdk.android.subsystems.HttpConfiguration; +import com.launchdarkly.sdk.android.subsystems.Synchronizer; +import com.launchdarkly.sdk.fdv2.Selector; +import com.launchdarkly.testhelpers.httptest.Handlers; +import com.launchdarkly.testhelpers.httptest.HttpServer; +import com.launchdarkly.testhelpers.httptest.RequestInfo; + +import org.junit.After; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.rules.Timeout; + +import java.util.Collections; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +/** + * Tests the request wiring of the data source builder implementations in + * {@link DataSystemComponents}: which HTTP method, path, and body each FDv2 + * component and the FDv1 fallback synchronizer use for a given HTTP configuration. + */ +public class DataSystemComponentsTest { + + @Rule + public Timeout globalTimeout = Timeout.seconds(10); + + @Rule + public TemporaryFolder cacheDir = new TemporaryFolder(); + + private final ScheduledExecutorService executor = Executors.newScheduledThreadPool(2); + + private static final LDContext CONTEXT = LDContext.create("test-context-key"); + private static final LDLogger LOGGER = LDLogger.none(); + + // A valid FDv2 polling response that produces a changeset when processed + private static final String VALID_POLL_RESPONSE_JSON = + "{\"events\":[" + + "{\"event\":\"server-intent\",\"data\":{\"payloads\":[{\"id\":\"p1\",\"target\":100,\"intentCode\":\"xfer-full\",\"reason\":\"\"}]}}," + + "{\"event\":\"payload-transferred\",\"data\":{\"state\":\"(p:p1:100)\",\"version\":100}}" + + "]}"; + + // A valid FDv1 polling response (flag key to flag object map) + private static final String VALID_FDV1_JSON = + "{\"flag1\":{\"key\":\"flag1\",\"version\":1,\"value\":true}}"; + + @After + public void tearDown() { + executor.shutdownNow(); + } + + private static HttpConfiguration httpConfig(boolean useReport, boolean usePost) { + return new HttpConfiguration(10_000, Collections.emptyMap(), null, useReport, usePost); + } + + private DataSourceBuildInputsInternal makeInputs(HttpServer server, HttpConfiguration http) { + ServiceEndpoints endpoints = new ServiceEndpoints( + server.getUri(), server.getUri(), server.getUri()); + return new DataSourceBuildInputsInternal( + CONTEXT, endpoints, http, false, + () -> Selector.EMPTY, executor, cacheDir.getRoot(), LOGGER, null); + } + + @Test + public void pollingSynchronizerSendsPostWhenUsePostEnabled() throws Exception { + try (HttpServer server = HttpServer.start(Handlers.bodyJson(VALID_POLL_RESPONSE_JSON))) { + Synchronizer sync = new DataSystemComponents.PollingSynchronizerBuilderImpl() + .build(makeInputs(server, httpConfig(false, true))); + try { + sync.next().get(5, TimeUnit.SECONDS); + + RequestInfo req = server.getRecorder().requireRequest(); + assertEquals("POST", req.getMethod()); + assertEquals("/sdk/poll/eval", req.getPath()); + assertNotNull("body should contain the serialized context", req.getBody()); + assertTrue("body should contain the context key", + req.getBody().contains("test-context-key")); + } finally { + sync.close(); + } + } + } + + @Test + public void pollingSynchronizerSendsGetWhenOnlyUseReportEnabled() throws Exception { + // useReport has no effect on FDv2 components; the request stays GET + try (HttpServer server = HttpServer.start(Handlers.bodyJson(VALID_POLL_RESPONSE_JSON))) { + Synchronizer sync = new DataSystemComponents.PollingSynchronizerBuilderImpl() + .build(makeInputs(server, httpConfig(true, false))); + try { + sync.next().get(5, TimeUnit.SECONDS); + + RequestInfo req = server.getRecorder().requireRequest(); + assertEquals("GET", req.getMethod()); + assertTrue("path should use the GET endpoint with a context segment", + req.getPath().startsWith("/sdk/poll/eval/")); + assertTrue("path should contain the base64 context", + req.getPath().contains(LDUtil.urlSafeBase64(CONTEXT))); + } finally { + sync.close(); + } + } + } + + @Test + public void streamingSynchronizerSendsPostWhenUsePostEnabled() throws Exception { + String serverIntent = "event: server-intent\ndata: {\"payloads\":[{\"id\":\"p1\",\"target\":100,\"intentCode\":\"xfer-full\",\"reason\":\"\"}]}"; + String payloadTransferred = "event: payload-transferred\ndata: {\"state\":\"(p:p1:100)\",\"version\":100}"; + + try (HttpServer server = HttpServer.start(Handlers.all( + Handlers.SSE.start(), + Handlers.SSE.event(serverIntent), + Handlers.SSE.event(payloadTransferred), + Handlers.SSE.leaveOpen()))) { + Synchronizer sync = new DataSystemComponents.StreamingSynchronizerBuilderImpl() + .build(makeInputs(server, httpConfig(false, true))); + try { + FDv2SourceResult result = sync.next().get(5, TimeUnit.SECONDS); + assertNotNull(result); + + RequestInfo req = server.getRecorder().requireRequest(); + assertEquals("POST", req.getMethod()); + assertEquals("/sdk/stream/eval", req.getPath()); + assertNotNull("body should contain the serialized context", req.getBody()); + assertTrue("body should contain the context key", + req.getBody().contains("test-context-key")); + } finally { + sync.close(); + } + } + } + + @Test + public void fdv1FallbackSynchronizerSendsReportWhenUsePostEnabled() throws Exception { + try (HttpServer server = HttpServer.start(Handlers.bodyJson(VALID_FDV1_JSON))) { + Synchronizer sync = new DataSystemComponents.FDv1PollingSynchronizerBuilderImpl() + .build(makeInputs(server, httpConfig(false, true))); + try { + sync.next().get(5, TimeUnit.SECONDS); + + RequestInfo req = server.getRecorder().requireRequest(); + assertEquals("REPORT", req.getMethod()); + assertEquals("/msdk/evalx/context", req.getPath()); + assertNotNull("body should contain the serialized context", req.getBody()); + assertTrue("body should contain the context key", + req.getBody().contains("test-context-key")); + } finally { + sync.close(); + } + } + } + + @Test + public void fdv1FallbackSynchronizerSendsGetByDefault() throws Exception { + try (HttpServer server = HttpServer.start(Handlers.bodyJson(VALID_FDV1_JSON))) { + Synchronizer sync = new DataSystemComponents.FDv1PollingSynchronizerBuilderImpl() + .build(makeInputs(server, httpConfig(false, false))); + try { + sync.next().get(5, TimeUnit.SECONDS); + + RequestInfo req = server.getRecorder().requireRequest(); + assertEquals("GET", req.getMethod()); + assertTrue("path should use the FDv1 GET endpoint with a context segment", + req.getPath().startsWith("/msdk/evalx/contexts/")); + } finally { + sync.close(); + } + } + } +} diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DefaultFDv2RequestorTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DefaultFDv2RequestorTest.java index a724ea87..1d649ec1 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DefaultFDv2RequestorTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DefaultFDv2RequestorTest.java @@ -37,7 +37,7 @@ public class DefaultFDv2RequestorTest { private static final LDContext CONTEXT = LDContext.create("test-context-key"); // Bogus paths used by tests — deliberately not the real production paths. private static final String GET_REQUEST_PATH = "/fake-poll-get"; - private static final String REPORT_REQUEST_PATH = "/fake-poll-report"; + private static final String POST_REQUEST_PATH = "/fake-poll-post"; private static final LDLogger LOGGER = LDLogger.none(); private static HttpProperties httpProperties() { @@ -49,14 +49,14 @@ private static HttpProperties httpProperties() { null, null); } - private static DefaultFDv2Requestor makeRequestor(HttpServer server, boolean useReport, boolean evaluationReasons, String payloadFilter) { + private static DefaultFDv2Requestor makeRequestor(HttpServer server, boolean usePost, boolean evaluationReasons, String payloadFilter) { return new DefaultFDv2Requestor( CONTEXT, server.getUri(), GET_REQUEST_PATH, - REPORT_REQUEST_PATH, + POST_REQUEST_PATH, httpProperties(), - useReport, + usePost, evaluationReasons, payloadFilter, LOGGER); @@ -358,7 +358,7 @@ public void baseUriCanHaveContextPath() throws Exception { CONTEXT, uri, GET_REQUEST_PATH, - REPORT_REQUEST_PATH, + POST_REQUEST_PATH, httpProperties(), false, false, @@ -506,16 +506,16 @@ public void evaluationReasonsAddedToRequest() throws Exception { } @Test - public void useReportMethod() throws Exception { + public void usePostMethod() throws Exception { try (HttpServer server = HttpServer.start(Handlers.bodyJson(EMPTY_EVENTS_JSON))) { try (DefaultFDv2Requestor requestor = makeRequestor(server, true, false, null)) { requestor.poll(Selector.EMPTY).get(5, TimeUnit.SECONDS); RequestInfo req = server.getRecorder().requireRequest(); - assertEquals("REPORT", req.getMethod()); - // Context should be in the request body, not the URL path - assertTrue("path should use REPORT path without context segment", - req.getPath().startsWith(REPORT_REQUEST_PATH)); + assertEquals("POST", req.getMethod()); + // Context goes in the request body, not the URL path + assertTrue("path should use POST path without context segment", + req.getPath().startsWith(POST_REQUEST_PATH)); assertFalse("path should not contain GET context segment", req.getPath().startsWith(GET_REQUEST_PATH)); assertNotNull("body should contain serialized context", req.getBody()); @@ -526,23 +526,23 @@ public void useReportMethod() throws Exception { } @Test - public void useReportMethodDoesNotUseEtag() throws Exception { - // ETag caching is disabled for REPORT requests since the context is in the body + public void usePostMethodDoesNotUseEtag() throws Exception { + // ETag caching only runs for GET requests, where the context is in the URL Handler etagResp = Handlers.all( Handlers.header("ETag", "some-etag"), Handlers.bodyJson(EMPTY_EVENTS_JSON)); try (HttpServer server = HttpServer.start(etagResp)) { try (DefaultFDv2Requestor requestor = makeRequestor(server, true, false, null)) { - // First REPORT request — server returns an ETag, but it should not be cached + // First POST request; the server returns an ETag, which is not cached requestor.poll(Selector.EMPTY).get(5, TimeUnit.SECONDS); RequestInfo req1 = server.getRecorder().requireRequest(); - assertNull("REPORT request should not send If-None-Match", req1.getHeader("If-None-Match")); + assertNull("POST request should not send If-None-Match", req1.getHeader("If-None-Match")); - // Second REPORT request — still no If-None-Match because ETag was never stored + // Second POST request; still no If-None-Match because the ETag was never stored requestor.poll(Selector.EMPTY).get(5, TimeUnit.SECONDS); RequestInfo req2 = server.getRecorder().requireRequest(); - assertNull("REPORT request should never send If-None-Match", req2.getHeader("If-None-Match")); + assertNull("POST request should never send If-None-Match", req2.getHeader("If-None-Match")); } } } @@ -551,7 +551,7 @@ public void useReportMethodDoesNotUseEtag() throws Exception { public void http400LogsProguardWarning() throws Exception { try (HttpServer server = HttpServer.start(Handlers.status(400))) { try (DefaultFDv2Requestor requestor = new DefaultFDv2Requestor( - CONTEXT, server.getUri(), GET_REQUEST_PATH, REPORT_REQUEST_PATH, + CONTEXT, server.getUri(), GET_REQUEST_PATH, POST_REQUEST_PATH, httpProperties(), false, false, null, logging.logger)) { Future future = requestor.poll(Selector.EMPTY); @@ -569,7 +569,7 @@ public void http400LogsProguardWarning() throws Exception { public void networkFailureThrowsException() throws Exception { try (DefaultFDv2Requestor requestor = new DefaultFDv2Requestor( CONTEXT, URI.create("http://localhost:1"), - GET_REQUEST_PATH, REPORT_REQUEST_PATH, + GET_REQUEST_PATH, POST_REQUEST_PATH, httpProperties(), false, false, null, LOGGER)) { requestor.poll(Selector.EMPTY).get(5, TimeUnit.SECONDS); } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceBuilderTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceBuilderTest.java index a2044ab7..1993edde 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceBuilderTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceBuilderTest.java @@ -36,9 +36,12 @@ public class FDv2DataSourceBuilderTest { public LogCaptureRule logging = new LogCaptureRule(); private ClientContext makeClientContext() { + return makeClientContext(new HttpConfiguration(10_000, Collections.emptyMap(), null, false)); + } + + private ClientContext makeClientContext(HttpConfiguration http) { LDConfig config = new LDConfig.Builder(AutoEnvAttributes.Disabled).build(); MockComponents.MockDataSourceUpdateSink sink = new MockComponents.MockDataSourceUpdateSink(); - HttpConfiguration http = new HttpConfiguration(10_000, Collections.emptyMap(), null, false); ClientContext base = new ClientContext( "mobile-key", ENV_REPORTER, @@ -342,4 +345,34 @@ public void setActiveMode_notInTable_throws() { assertTrue(e.getMessage().contains("not found in mode table")); } } + + // ---- useReport warning when the data system is configured ---- + + private long useReportWarnCount() { + return logging.logCapture.getMessageStrings().stream() + .filter(m -> m.contains("WARN") && m.contains("useReport")) + .count(); + } + + @Test + public void build_withUseReport_logsWarningOnce() { + HttpConfiguration http = new HttpConfiguration( + 10_000, Collections.emptyMap(), null, true, false); + FDv2DataSourceBuilder builder = new FDv2DataSourceBuilder(); + // build runs once per connection mode switch; the warning logs only once + builder.build(makeClientContext(http)); + builder.build(makeClientContext(http)); + builder.close(); + + assertEquals(1, useReportWarnCount()); + } + + @Test + public void build_withoutUseReport_logsNoWarning() { + FDv2DataSourceBuilder builder = new FDv2DataSourceBuilder(); + builder.build(makeClientContext()); + builder.close(); + + assertEquals(0, useReportWarnCount()); + } } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizerTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizerTest.java index 9aa3891d..17937c13 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizerTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizerTest.java @@ -92,10 +92,10 @@ private FDv2StreamingSynchronizer makeSynchronizer( private FDv2StreamingSynchronizer makeSynchronizer( URI streamBaseUri, - boolean evaluationReasons, boolean useReport) { + boolean evaluationReasons, boolean usePost) { return new FDv2StreamingSynchronizer( CONTEXT, EMPTY_SELECTOR_SOURCE, streamBaseUri, STREAM_PATH, - null, 100, evaluationReasons, useReport, + null, 100, evaluationReasons, usePost, httpProperties(), executor, LOGGER, null); } @@ -1038,7 +1038,7 @@ public void evaluationReasonsAddedToStreamUrl() throws Exception { Handlers.SSE.leaveOpen()))) { FDv2StreamingSynchronizer sync = makeSynchronizer(server.getUri(), - true /* evaluationReasons */, false); + true /* evaluationReasons */, false /* usePost */); sync.next().get(5, TimeUnit.SECONDS); @@ -1051,7 +1051,7 @@ public void evaluationReasonsAddedToStreamUrl() throws Exception { } @Test - public void useReportMethodForStreamRequest() throws Exception { + public void usePostMethodForStreamRequest() throws Exception { String serverIntent = makeEvent("server-intent", "{\"payloads\":[{\"id\":\"payload-1\",\"target\":100,\"intentCode\":\"xfer-full\",\"reason\":\"payload-missing\"}]}"); String payloadTransferred = makeEvent("payload-transferred", "{\"state\":\"(p:payload-1:100)\",\"version\":100}"); @@ -1062,16 +1062,16 @@ public void useReportMethodForStreamRequest() throws Exception { Handlers.SSE.leaveOpen()))) { FDv2StreamingSynchronizer sync = makeSynchronizer(server.getUri(), - false, true /* useReport */); + false, true /* usePost */); Future resultFuture = sync.next(); FDv2SourceResult result = resultFuture.get(5, TimeUnit.SECONDS); assertEquals(SourceResultType.CHANGE_SET, result.getResultType()); RequestInfo request = server.getRecorder().requireRequest(); - assertEquals("REPORT", request.getMethod()); - // With REPORT the context goes in the body, not the URL path - assertFalse("path should not contain base64 context segment when using REPORT", + assertEquals("POST", request.getMethod()); + // With POST the context goes in the body, not the URL path + assertFalse("path should not contain base64 context segment when using POST", request.getPath().contains(LDUtil.urlSafeBase64(CONTEXT))); assertNotNull("body should contain serialized context", request.getBody()); assertTrue("body should contain context key", diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/HttpConfigurationBuilderTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/HttpConfigurationBuilderTest.java index d9b20495..a4c64dd4 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/HttpConfigurationBuilderTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/HttpConfigurationBuilderTest.java @@ -11,6 +11,8 @@ import static com.launchdarkly.sdk.android.integrations.HttpConfigurationBuilder.DEFAULT_CONNECT_TIMEOUT_MILLIS; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; public class HttpConfigurationBuilderTest { private static final String MOBILE_KEY = "mobile-key"; @@ -49,6 +51,20 @@ public void testConnectTimeout() { assertEquals(999, hc.getConnectTimeoutMillis()); } + @Test + public void testUsePostDefaultsToFalse() { + HttpConfiguration hc = Components.httpConfiguration().build(BASIC_CONTEXT); + assertFalse(hc.shouldUsePost()); + } + + @Test + public void testUsePost() { + HttpConfiguration hc = Components.httpConfiguration() + .usePost(true) + .build(BASIC_CONTEXT); + assertTrue(hc.shouldUsePost()); + } + @Test public void testWrapperNameOnly() { HttpConfiguration hc = Components.httpConfiguration() diff --git a/testharness-suppressions-fdv2.txt b/testharness-suppressions-fdv2.txt index 5e53af75..ceb41bc5 100644 --- a/testharness-suppressions-fdv2.txt +++ b/testharness-suppressions-fdv2.txt @@ -1 +1,4 @@ -streaming/fdv2/fallback to FDv1 handling \ No newline at end of file +streaming/fdv2/fallback to FDv1 handling +events/feature events/single-kind anonymous context redacts all attributes +events/feature events/multi-kind with anonymous context redacts attributes appropriately +events/context properties/single-kind, allAttributesPrivate, slash-prefixed attribute name