From 583123bea14bbf740b0080abfcb3731fbf012b68 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Wed, 8 Mar 2023 11:39:30 -0800 Subject: [PATCH 1/6] Make test proxy use default http client if available. --- .../com/azure/core/test/InterceptorManager.java | 14 ++++++++++++-- .../main/java/com/azure/core/test/TestBase.java | 3 ++- .../core/test/http/TestProxyPlaybackClient.java | 5 +++-- .../core/test/policy/TestProxyRecordPolicy.java | 6 ++++-- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index ad70217ae3e79..a7ca16fcdb0d8 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -71,6 +71,7 @@ public class InterceptorManager implements AutoCloseable { private TestProxyRecordPolicy testProxyRecordPolicy; private TestProxyPlaybackClient testProxyPlaybackClient; private final Queue proxyVariableQueue = new LinkedList<>(); + private HttpClient httpClient; /** * Creates a new InterceptorManager that either replays test-session records or saves them. @@ -301,7 +302,7 @@ public HttpPipelinePolicy getRecordPolicy(List> recordi public HttpClient getPlaybackClient() { if (testProxyEnabled) { if (testProxyPlaybackClient == null) { - testProxyPlaybackClient = new TestProxyPlaybackClient(); + testProxyPlaybackClient = new TestProxyPlaybackClient(httpClient); proxyVariableQueue.addAll(testProxyPlaybackClient.startPlayback(playbackRecordName)); } return testProxyPlaybackClient; @@ -346,7 +347,7 @@ private RecordedData readDataFromFile() { private HttpPipelinePolicy getProxyRecordingPolicy() { if (testProxyRecordPolicy == null) { - testProxyRecordPolicy = new TestProxyRecordPolicy(); + testProxyRecordPolicy = new TestProxyRecordPolicy(httpClient); testProxyRecordPolicy.startRecording(playbackRecordName); } return testProxyRecordPolicy; @@ -433,4 +434,13 @@ public void addMatchers(List testProxyMatchers) { throw new RuntimeException("Playback must have been started before adding matchers."); } } + + /** + * Sets the httpClient to be used for this test. + * @param httpClient The {@link HttpClient} implementation to use. + */ + void setHttpClient(HttpClient httpClient) { + + this.httpClient = httpClient; + } } diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java index 606702e4e53df..8adbbfe8653b3 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java @@ -155,6 +155,7 @@ public void setupTest(TestInfo testInfo) { } if (isTestProxyEnabled()) { + interceptorManager.setHttpClient(getHttpClients().findFirst().orElse(null)); // The supplier/consumer are used to retrieve/store variables over the wire. testResourceNamer = new TestResourceNamer(testContextManager, interceptorManager.getProxyVariableConsumer(), @@ -238,7 +239,7 @@ public static Stream getHttpClients() { * In LIVE or RECORD mode load all HttpClient instances and let the test run determine which HttpClient * implementation it will use. */ - if (testMode == TestMode.PLAYBACK) { + if (testMode == TestMode.PLAYBACK && !enableTestProxy) { return Stream.of(new HttpClient[] { null }); } diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java index c6e0563ee9526..f495c7543be6e 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java @@ -36,7 +36,7 @@ */ public class TestProxyPlaybackClient implements HttpClient { - private final HttpURLConnectionHttpClient client = new HttpURLConnectionHttpClient(); + private final HttpClient client; private String xRecordingId; private static final SerializerAdapter SERIALIZER = new JacksonAdapter(); @@ -48,7 +48,8 @@ public class TestProxyPlaybackClient implements HttpClient { /** * Create an instance of {@link TestProxyPlaybackClient} with a list of custom sanitizers. */ - public TestProxyPlaybackClient() { + public TestProxyPlaybackClient(HttpClient httpClient) { + this.client = (httpClient == null ? new HttpURLConnectionHttpClient() : httpClient); this.sanitizers.addAll(DEFAULT_SANITIZERS); } diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java index 069404b9dea28..54e5ac5878b54 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java @@ -3,6 +3,7 @@ package com.azure.core.test.policy; +import com.azure.core.http.HttpClient; import com.azure.core.http.HttpMethod; import com.azure.core.http.HttpPipelineCallContext; import com.azure.core.http.HttpPipelineNextPolicy; @@ -36,7 +37,7 @@ */ public class TestProxyRecordPolicy implements HttpPipelinePolicy { private static final SerializerAdapter SERIALIZER = new JacksonAdapter(); - private final HttpURLConnectionHttpClient client = new HttpURLConnectionHttpClient(); + private final HttpClient client; private String xRecordingId; private final List sanitizers = new ArrayList<>(); private static final List DEFAULT_SANITIZERS = loadSanitizers(); @@ -44,7 +45,8 @@ public class TestProxyRecordPolicy implements HttpPipelinePolicy { /** * Create an instance of {@link TestProxyRecordPolicy} with a list of custom sanitizers. */ - public TestProxyRecordPolicy() { + public TestProxyRecordPolicy(HttpClient httpClient) { + this.client = (httpClient == null ? new HttpURLConnectionHttpClient() : httpClient); this.sanitizers.addAll(DEFAULT_SANITIZERS); } From 96850e451fa0d50583fd5bb550996ea3ba6bb1a2 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Wed, 8 Mar 2023 17:48:28 -0800 Subject: [PATCH 2/6] Prevent getting playback/record objects in LIVE mode. --- .../com/azure/core/test/InterceptorManager.java | 6 ++++++ .../main/java/com/azure/core/test/TestBase.java | 2 ++ .../java/com/azure/core/test/TestProxyTests.java | 14 ++++++++++++++ 3 files changed, 22 insertions(+) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index a7ca16fcdb0d8..11dae8c525ef3 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -301,6 +301,9 @@ public HttpPipelinePolicy getRecordPolicy(List> recordi */ public HttpClient getPlaybackClient() { if (testProxyEnabled) { + if (isLiveMode()) { + throw new RuntimeException("A playback client cannot be requested in LIVE mode."); + } if (testProxyPlaybackClient == null) { testProxyPlaybackClient = new TestProxyPlaybackClient(httpClient); proxyVariableQueue.addAll(testProxyPlaybackClient.startPlayback(playbackRecordName)); @@ -347,6 +350,9 @@ private RecordedData readDataFromFile() { private HttpPipelinePolicy getProxyRecordingPolicy() { if (testProxyRecordPolicy == null) { + if (isLiveMode()) { + throw new RuntimeException("A recording policy cannot be requested in LIVE mode."); + } testProxyRecordPolicy = new TestProxyRecordPolicy(httpClient); testProxyRecordPolicy.startRecording(playbackRecordName); } diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java index 8adbbfe8653b3..968aff40eda35 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java @@ -142,6 +142,8 @@ public void setupTest(TestInfo testInfo) { localTestMode = TestMode.RECORD; } else if (testInfo.getTags().contains("Playback")) { localTestMode = TestMode.PLAYBACK; + } else if (testInfo.getTags().contains("Live")) { + localTestMode = TestMode.LIVE; } this.testContextManager = new TestContextManager(testInfo.getTestMethod().get(), localTestMode, isTestProxyEnabled()); testContextManager.setTestIteration(testIterationContext.getTestIteration()); diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java index 1a60153d571d6..fbe8628c4534f 100644 --- a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java @@ -197,6 +197,20 @@ public void testPlayback() { assertEquals(200, response.getStatusCode()); } + @Test + @Tag("Live") + public void testCannotGetPlaybackClient() { + RuntimeException thrown = assertThrows(RuntimeException.class, () -> interceptorManager.getPlaybackClient()); + assertEquals("A playback client cannot be requested in LIVE mode.", thrown.getMessage()); + } + + @Test + @Tag("Live") + public void testCannotGetRecordPolicy() { + RuntimeException thrown = assertThrows(RuntimeException.class, () -> interceptorManager.getRecordPolicy()); + assertEquals("A recording policy cannot be requested in LIVE mode.", thrown.getMessage()); + } + @Test @Tag("Record") public void testRecordWithRedaction() { From ff9206ba588a92d50e6b2986cfb56764c93e5cba Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Wed, 8 Mar 2023 18:09:44 -0800 Subject: [PATCH 3/6] checkstyle fixes --- .../src/main/java/com/azure/core/test/InterceptorManager.java | 4 ++++ .../com/azure/core/test/http/TestProxyPlaybackClient.java | 2 ++ .../com/azure/core/test/policy/TestProxyRecordPolicy.java | 2 ++ 3 files changed, 8 insertions(+) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index 11dae8c525ef3..4fa243baff046 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -271,6 +271,8 @@ public Consumer getProxyVariableConsumer() { * {@link InterceptorManager}. * * @return HttpPipelinePolicy to record network calls. + * + * @throws RuntimeException A playback client was requested when the test proxy is enabled and test mode is LIVE. */ public HttpPipelinePolicy getRecordPolicy() { if (testProxyEnabled) { @@ -298,6 +300,8 @@ public HttpPipelinePolicy getRecordPolicy(List> recordi * Gets a new HTTP client that plays back test session records managed by {@link InterceptorManager}. * * @return An HTTP client that plays back network calls from its recorded data. + * + * @throws RuntimeException A playback client was requested when the test proxy is enabled and test mode is LIVE. */ public HttpClient getPlaybackClient() { if (testProxyEnabled) { diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java index f495c7543be6e..d210c2bf3b3cc 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java @@ -47,6 +47,8 @@ public class TestProxyPlaybackClient implements HttpClient { /** * Create an instance of {@link TestProxyPlaybackClient} with a list of custom sanitizers. + * + * @param httpClient The {@link HttpClient} to use. If none is passed {@link HttpURLConnectionHttpClient} is the default. */ public TestProxyPlaybackClient(HttpClient httpClient) { this.client = (httpClient == null ? new HttpURLConnectionHttpClient() : httpClient); diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java index 54e5ac5878b54..a571fed4b94db 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java @@ -44,6 +44,8 @@ public class TestProxyRecordPolicy implements HttpPipelinePolicy { /** * Create an instance of {@link TestProxyRecordPolicy} with a list of custom sanitizers. + * + * @param httpClient The {@link HttpClient} to use. If none is passed {@link HttpURLConnectionHttpClient} is the default. */ public TestProxyRecordPolicy(HttpClient httpClient) { this.client = (httpClient == null ? new HttpURLConnectionHttpClient() : httpClient); From fbc94799d79e233194fbe6779f7a3badb4575b60 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Thu, 9 Mar 2023 09:40:30 -0800 Subject: [PATCH 4/6] PR feedback --- .../com/azure/core/test/InterceptorManager.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index 4fa243baff046..b033b65ce6492 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -233,6 +233,14 @@ public boolean isLiveMode() { return testMode == TestMode.LIVE; } + /** + * Gets whether this InterceptorManager is in record mode. + * + * @return true if the InterceptorManager is in record mode and false otherwise. + */ + public boolean isRecordMode() { + return testMode == TestMode.RECORD; + } /** * Gets the recorded data InterceptorManager is keeping track of. @@ -305,8 +313,8 @@ public HttpPipelinePolicy getRecordPolicy(List> recordi */ public HttpClient getPlaybackClient() { if (testProxyEnabled) { - if (isLiveMode()) { - throw new RuntimeException("A playback client cannot be requested in LIVE mode."); + if (!isPlaybackMode()) { + throw new IllegalStateException("A playback client can only be requested in PLAYBACK mode."); } if (testProxyPlaybackClient == null) { testProxyPlaybackClient = new TestProxyPlaybackClient(httpClient); @@ -354,8 +362,8 @@ private RecordedData readDataFromFile() { private HttpPipelinePolicy getProxyRecordingPolicy() { if (testProxyRecordPolicy == null) { - if (isLiveMode()) { - throw new RuntimeException("A recording policy cannot be requested in LIVE mode."); + if (!isRecordMode()) { + throw new IllegalStateException("A recording policy can only be requested in RECORD mode."); } testProxyRecordPolicy = new TestProxyRecordPolicy(httpClient); testProxyRecordPolicy.startRecording(playbackRecordName); From f7adb91dbfed9f42113fbd9b4eca3403b5c4f3d0 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Thu, 9 Mar 2023 09:58:56 -0800 Subject: [PATCH 5/6] fix javadoc --- .../main/java/com/azure/core/test/InterceptorManager.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index b033b65ce6492..fc51bd2667218 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -280,7 +280,7 @@ public Consumer getProxyVariableConsumer() { * * @return HttpPipelinePolicy to record network calls. * - * @throws RuntimeException A playback client was requested when the test proxy is enabled and test mode is LIVE. + * @throws IllegalStateException A recording policy was requested when the test proxy is enabled and test mode is not RECORD. */ public HttpPipelinePolicy getRecordPolicy() { if (testProxyEnabled) { @@ -296,6 +296,8 @@ public HttpPipelinePolicy getRecordPolicy() { * @param recordingRedactors The custom redactor functions that are applied in addition to the default redactor * functions defined in {@link RecordingRedactor}. * @return {@link HttpPipelinePolicy} to record network calls. + * + * @throws IllegalStateException A recording policy was requested when the test proxy is enabled and test mode is not RECORD. */ public HttpPipelinePolicy getRecordPolicy(List> recordingRedactors) { if (testProxyEnabled) { @@ -309,7 +311,7 @@ public HttpPipelinePolicy getRecordPolicy(List> recordi * * @return An HTTP client that plays back network calls from its recorded data. * - * @throws RuntimeException A playback client was requested when the test proxy is enabled and test mode is LIVE. + * @throws IllegalStateException A playback client was requested when the test proxy is enabled and test mode is LIVE. */ public HttpClient getPlaybackClient() { if (testProxyEnabled) { From e128343e47f12a1546e2daa4c5b59684587edce1 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Thu, 9 Mar 2023 10:01:39 -0800 Subject: [PATCH 6/6] fix tests --- .../src/test/java/com/azure/core/test/TestProxyTests.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java index fbe8628c4534f..e47ffc3329f48 100644 --- a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java @@ -200,15 +200,15 @@ public void testPlayback() { @Test @Tag("Live") public void testCannotGetPlaybackClient() { - RuntimeException thrown = assertThrows(RuntimeException.class, () -> interceptorManager.getPlaybackClient()); - assertEquals("A playback client cannot be requested in LIVE mode.", thrown.getMessage()); + RuntimeException thrown = assertThrows(IllegalStateException.class, () -> interceptorManager.getPlaybackClient()); + assertEquals("A playback client can only be requested in PLAYBACK mode.", thrown.getMessage()); } @Test @Tag("Live") public void testCannotGetRecordPolicy() { - RuntimeException thrown = assertThrows(RuntimeException.class, () -> interceptorManager.getRecordPolicy()); - assertEquals("A recording policy cannot be requested in LIVE mode.", thrown.getMessage()); + RuntimeException thrown = assertThrows(IllegalStateException.class, () -> interceptorManager.getRecordPolicy()); + assertEquals("A recording policy can only be requested in RECORD mode.", thrown.getMessage()); } @Test