From d25a45c485161ad51e03c30b69776c85fc307999 Mon Sep 17 00:00:00 2001 From: ardetrick <1452182+ardetrick@users.noreply.github.com> Date: Sun, 5 Jul 2026 14:38:29 -0500 Subject: [PATCH 1/3] test: share one Hydra container across the functional suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starts Hydra once per class (@TestInstance(PER_CLASS) makes the injected @LocalServerPort available to a non-static @BeforeAll) and registers a unique OAuth2 client per test via the container's upserting createOrReplaceClient — Hydra keys remembered consent by subject + client, so per-test clients preserve the isolation the remember-me tests rely on. Functional suite drops from ~40-50s to ~7s. The refactor exposed three latent defects that the per-test containers and a global-singleton side channel had been masking, all fixed here: HydraAdminClient captured its base path at construction (now resolved lazily per call, with a dedicated ApiClient instead of the SDK's mutable global default); the test pointed that base path at Hydra's public port — the admin API only ever worked because the deleted SDK helper had re-pointed the global client at the admin port; and the test's public proxy read the same property for a different endpoint (it now has its own explicit public base URI). --- .../hydra/HydraAdminClient.java | 27 ++-- ...raReferenceApplicationFunctionalTests.java | 120 +++++++----------- 2 files changed, 65 insertions(+), 82 deletions(-) diff --git a/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/HydraAdminClient.java b/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/HydraAdminClient.java index 36204c4..5f74d18 100644 --- a/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/HydraAdminClient.java +++ b/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/HydraAdminClient.java @@ -11,7 +11,6 @@ import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.stereotype.Service; import sh.ory.hydra.ApiException; -import sh.ory.hydra.Configuration; import sh.ory.hydra.api.OAuth2Api; import sh.ory.hydra.model.*; @@ -20,16 +19,23 @@ @FieldDefaults(level = AccessLevel.PRIVATE, makeFinal = true) public class HydraAdminClient { - @NonNull OAuth2Api oAuth2Api; + @NonNull Properties properties; HydraAdminClient(@NonNull final HydraAdminClient.Properties properties) { - val apiClient = Configuration.getDefaultApiClient().setBasePath(properties.getBasePath()); - oAuth2Api = new OAuth2Api(apiClient); + this.properties = properties; + } + + // The base path is resolved on every call rather than captured at construction: the bean is + // built when the Spring context boots, but the configured base path may legitimately change + // afterwards (integration tests only learn Hydra's randomized port once it is running). A + // dedicated ApiClient instance also avoids mutating the SDK's global default client. + private OAuth2Api oAuth2Api() { + return new OAuth2Api(new sh.ory.hydra.ApiClient().setBasePath(properties.getBasePath())); } public List listOAuth2Clients() { try { - return oAuth2Api.listOAuth2Clients(1000L, null, null, null); + return oAuth2Api().listOAuth2Clients(1000L, null, null, null); } catch (ApiException e) { throw new RuntimeException(e); } @@ -41,7 +47,7 @@ public List listOAuth2Clients() { */ public Optional getLoginRequest(@NonNull String loginChallenge) { try { - return Optional.of(oAuth2Api.getOAuth2LoginRequest(loginChallenge)); + return Optional.of(oAuth2Api().getOAuth2LoginRequest(loginChallenge)); } catch (ApiException e) { return switch (e.getCode()) { case 410 -> Optional.empty(); // requestWasHandledResponse @@ -59,7 +65,7 @@ public OAuth2RedirectTo acceptLoginRequest( // ... acceptLoginRequest.subject(loginEmail); try { - return oAuth2Api.acceptOAuth2LoginRequest(loginChallenge, acceptLoginRequest); + return oAuth2Api().acceptOAuth2LoginRequest(loginChallenge, acceptLoginRequest); } catch (ApiException e) { switch (e.getCode()) { case 400, 401, 404, 500 -> @@ -71,7 +77,7 @@ public OAuth2RedirectTo acceptLoginRequest( public OAuth2ConsentRequest getConsentRequest(@NonNull String consentChallenge) { try { - return oAuth2Api.getOAuth2ConsentRequest(consentChallenge); + return oAuth2Api().getOAuth2ConsentRequest(consentChallenge); } catch (ApiException e) { switch (e.getCode()) { case 400, 404 -> throw new RuntimeException("code: " + e.getCode(), e); // jsonError @@ -84,8 +90,9 @@ public OAuth2RedirectTo acceptConsentRequest(@NonNull AcceptConsentRequest accep val acceptOAuth2ConsentRequest = OryHydraRequestMapper.map(acceptConsentRequest); try { - return oAuth2Api.acceptOAuth2ConsentRequest( - acceptConsentRequest.consentChallenge(), acceptOAuth2ConsentRequest); + return oAuth2Api() + .acceptOAuth2ConsentRequest( + acceptConsentRequest.consentChallenge(), acceptOAuth2ConsentRequest); } catch (ApiException e) { switch (e.getCode()) { case 404, 500 -> throw new RuntimeException("code: " + e.getCode(), e); // jsonError diff --git a/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java b/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java index daa7601..5bd8978 100644 --- a/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java +++ b/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java @@ -45,10 +45,6 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.servlet.view.RedirectView; -import sh.ory.hydra.ApiException; -import sh.ory.hydra.Configuration; -import sh.ory.hydra.api.OAuth2Api; -import sh.ory.hydra.model.OAuth2Client; @Slf4j @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @@ -57,6 +53,7 @@ ForwardingController.class, }) @TestPropertySource(properties = {"debug=true"}) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) public class OryHydraReferenceApplicationFunctionalTests { // Shared between all tests in this class. @@ -67,7 +64,9 @@ public class OryHydraReferenceApplicationFunctionalTests { OryHydraContainer dockerComposeEnvironment; - OAuth2Client oAuth2Client; + String clientId; + String clientSecret; + String redirectUri; @Autowired HydraAdminClient.Properties properties; @@ -87,14 +86,12 @@ static void afterAll() { playwright.close(); } - @BeforeEach - public void beforeEachTest() throws ApiException { - // Start Ory Hydra, passing it the port of the reference application to configure urls. - // An alternative approach would be to start the container once and re-use it across all tests. - // However, @BeforeAllTests requires the method be static but @LocalServerPort is unavailable - // statically. - // Creating the containers for each test increases test execution time but keeps all tests - // isolated. + @BeforeAll + void startHydra() { + // One Hydra container serves every test in this class — containers are the expensive + // resource. Test isolation is preserved by registering a unique OAuth2 client per test + // instead (see registerTestClient). @TestInstance(PER_CLASS) lets this run as a non-static + // @BeforeAll, which is what makes the injected @LocalServerPort available for Hydra's urls. dockerComposeEnvironment = OryHydraContainer.builder() .urlsLogin("http://localhost:" + springBootAppPort + "/login") @@ -104,54 +101,37 @@ public void beforeEachTest() throws ApiException { .build(); dockerComposeEnvironment.start(); - // A "cheat" to break a circular dependency where the reference application needs to know the - // URI of Ory Hydra - // and Ory Hydra needs to know the URI of the reference application. In a production application - // these two URIs - // should be static and well known. However, in the context of these tests the ports of both the - // reference - // application and Ory Hydra are randomized and are unknown until after the application is - // already running - // (this follows testing best practices where hard coding ports should be avoided in case the - // host machine is - // already using that port). There may be a cleaner approach out there (perhaps using Docker - // Networking?) but - // in the meantime this is a low cost and sufficient work around. - properties.setBasePath(dockerComposeEnvironment.publicBaseUriString()); - - oAuth2Client = createOAuthClient(); + // A "cheat" to break a circular dependency: the reference application needs to know the URI + // of Ory Hydra and Ory Hydra needs to know the URI of the reference application, but both + // ports are randomized and unknown until each is already running. See ForwardingController. + // Two consumers, two different Hydra endpoints: the app's HydraAdminClient speaks the admin + // API, while the test's public proxy forwards browsers to the public authorize endpoint. + properties.setBasePath(dockerComposeEnvironment.adminBaseUriString()); + ForwardingController.hydraPublicBaseUri = dockerComposeEnvironment.publicBaseUriString(); } - @AfterEach - public void afterEachTest() { - // Test containers must be stopped to avoid a port conflict error when starting them up again - // for the next test. + @AfterAll + void stopHydra() { dockerComposeEnvironment.stop(); } - private OAuth2Client createOAuthClient() throws ApiException { - val oAuth2Client = new OAuth2Client(); - oAuth2Client.clientName("test-client"); - oAuth2Client.redirectUris( - List.of("http://localhost:" + springBootAppPort + CLIENT_CALL_BACK_PATH)); - oAuth2Client.grantTypes(List.of("authorization_code", "refresh_token")); - oAuth2Client.responseTypes(List.of("code", "id_token")); - oAuth2Client.clientSecret("client-secret"); - oAuth2Client.scope(String.join(" ", "offline_access", "openid", "offline", "profile")); - - // Initialize API - val oauth2Api = - new OAuth2Api( - Configuration.getDefaultApiClient() - .setBasePath(dockerComposeEnvironment.adminBaseUriString())); - - // Create client - val oauth2Client = oauth2Api.createOAuth2Client(oAuth2Client); - - // Basic assertions on created client - assertThat(oauth2Api.getOAuth2Client(oauth2Client.getClientId())).isNotNull(); - - return oauth2Client; + @BeforeEach + public void registerTestClient() { + // A unique client per test keeps tests isolated on the shared container: Hydra remembers + // consent per subject + client, so a shared client would leak remembered consent between + // tests. Registration is a cheap upserting admin API call via the container. + clientId = "test-client-" + UUID.randomUUID(); + clientSecret = "client-secret"; + redirectUri = "http://localhost:" + springBootAppPort + CLIENT_CALL_BACK_PATH; + dockerComposeEnvironment.createOrReplaceClient( + client -> + client + .clientId(clientId) + .clientSecret(clientSecret) + .redirectUris(redirectUri) + .grantTypes("authorization_code", "refresh_token") + .responseTypes("code", "id_token") + .scope("offline_access", "openid", "offline", "profile")); } /** @@ -198,9 +178,8 @@ private URI getUriToInitiateFlow() { try { return new URIBuilder(dockerComposeEnvironment.publicBaseUriString() + "/oauth2/auth") .addParameter("response_type", "code") - .addParameter("client_id", oAuth2Client.getClientId()) - .addParameter( - "redirect_uri", Objects.requireNonNull(oAuth2Client.getRedirectUris()).get(0)) + .addParameter("client_id", clientId) + .addParameter("redirect_uri", redirectUri) .addParameter("scope", "offline_access openid offline profile") .addParameter("state", "12345678901234567890") .build(); @@ -292,14 +271,10 @@ public void completeFlowWithPartialScopeSelection() { private CodeExchangeResponse exchangeCode(String code) { val encodedParams = Map.of( - "client_id", - Objects.requireNonNull(oAuth2Client.getClientId()), - "code", - code, - "grant_type", - Objects.requireNonNull(Objects.requireNonNull(oAuth2Client.getGrantTypes()).get(0)), - "redirect_uri", - Objects.requireNonNull(oAuth2Client.getRedirectUris()).get(0)) + "client_id", clientId, + "code", code, + "grant_type", "authorization_code", + "redirect_uri", redirectUri) .entrySet() .stream() .map( @@ -318,9 +293,7 @@ private CodeExchangeResponse exchangeCode(String code) { "authorization", "Basic " + Base64.getEncoder() - .encodeToString( - (oAuth2Client.getClientId() + ":" + oAuth2Client.getClientSecret()) - .getBytes())) + .encodeToString((clientId + ":" + clientSecret).getBytes())) .POST(HttpRequest.BodyPublishers.ofString(encodedParams)) .build(); @@ -475,11 +448,14 @@ record CodeExchangeResponse( @RequestMapping("/integration-test-public-proxy") class ForwardingController { - @Autowired HydraAdminClient.Properties properties; + // Set by the test once Hydra's mapped public port is known; the proxy forwards browsers to + // Hydra's public authorize endpoint (the admin base in HydraAdminClient.Properties is a + // different endpoint for a different consumer). + static String hydraPublicBaseUri; @GetMapping("oauth2/auth") public RedirectView oauth2Auth() { - val redirectView = new RedirectView(properties.getBasePath() + "/oauth2/auth"); + val redirectView = new RedirectView(hydraPublicBaseUri + "/oauth2/auth"); redirectView.setPropagateQueryParams(true); return redirectView; } From 6365398b403c9aeca420e4c158d8d94b6d048fe4 Mon Sep 17 00:00:00 2001 From: ardetrick <1452182+ardetrick@users.noreply.github.com> Date: Sun, 5 Jul 2026 15:07:49 -0500 Subject: [PATCH 2/3] test: unify the class lifecycle and remove static state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under @TestInstance(PER_CLASS) the single test instance lives for the whole class, so Playwright joins Hydra as plain instance state in one documented non-static @BeforeAll (which is what can read the injected @LocalServerPort), and the test proxy's public base URI becomes a field on the autowired bean — set beside properties.setBasePath, the same late-binding pattern — instead of a mutable static. --- ...raReferenceApplicationFunctionalTests.java | 42 +++++++++---------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java b/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java index 5bd8978..c1f2f5e 100644 --- a/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java +++ b/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java @@ -56,9 +56,10 @@ @TestInstance(TestInstance.Lifecycle.PER_CLASS) public class OryHydraReferenceApplicationFunctionalTests { - // Shared between all tests in this class. - private static Playwright playwright; - private static Browser browser; + // Shared between all tests in this class — plain instance fields: under + // @TestInstance(PER_CLASS) the single test instance lives for the whole class. + private Playwright playwright; + private Browser browser; @LocalServerPort int springBootAppPort; @@ -75,23 +76,20 @@ public class OryHydraReferenceApplicationFunctionalTests { @Captor ArgumentCaptor queryStringConsumerArgumentCaptor = ArgumentCaptor.forClass(String.class); + @Autowired ForwardingController forwardingController; + + // Lifecycle: the Spring context (and its random-port Tomcat) boots before JUnit creates the + // test instance; @TestInstance(PER_CLASS) reuses that single instance for the whole class, + // which is what allows @BeforeAll to be non-static — and therefore able to read the injected + // @LocalServerPort — while still running exactly once before all tests. @BeforeAll - static void beforeAll() { + void startTestEnvironment() { playwright = Playwright.create(); browser = playwright.chromium().launch(); - } - - @AfterAll - static void afterAll() { - playwright.close(); - } - @BeforeAll - void startHydra() { // One Hydra container serves every test in this class — containers are the expensive // resource. Test isolation is preserved by registering a unique OAuth2 client per test - // instead (see registerTestClient). @TestInstance(PER_CLASS) lets this run as a non-static - // @BeforeAll, which is what makes the injected @LocalServerPort available for Hydra's urls. + // instead (see registerTestClient). dockerComposeEnvironment = OryHydraContainer.builder() .urlsLogin("http://localhost:" + springBootAppPort + "/login") @@ -101,17 +99,17 @@ void startHydra() { .build(); dockerComposeEnvironment.start(); - // A "cheat" to break a circular dependency: the reference application needs to know the URI - // of Ory Hydra and Ory Hydra needs to know the URI of the reference application, but both - // ports are randomized and unknown until each is already running. See ForwardingController. - // Two consumers, two different Hydra endpoints: the app's HydraAdminClient speaks the admin - // API, while the test's public proxy forwards browsers to the public authorize endpoint. + // Late-bind the two Hydra endpoints now that the mapped ports exist (the circular + // port dependency: the app and Hydra each need the other's randomized URI). Two consumers, + // two different endpoints: the app's HydraAdminClient speaks the admin API, while the + // test's public proxy forwards browsers to the public authorize endpoint. properties.setBasePath(dockerComposeEnvironment.adminBaseUriString()); - ForwardingController.hydraPublicBaseUri = dockerComposeEnvironment.publicBaseUriString(); + forwardingController.hydraPublicBaseUri = dockerComposeEnvironment.publicBaseUriString(); } @AfterAll - void stopHydra() { + void stopTestEnvironment() { + playwright.close(); dockerComposeEnvironment.stop(); } @@ -451,7 +449,7 @@ class ForwardingController { // Set by the test once Hydra's mapped public port is known; the proxy forwards browsers to // Hydra's public authorize endpoint (the admin base in HydraAdminClient.Properties is a // different endpoint for a different consumer). - static String hydraPublicBaseUri; + String hydraPublicBaseUri; @GetMapping("oauth2/auth") public RedirectView oauth2Auth() { From f55c66065b7ea34705ca031066a31cfcb6124cf1 Mon Sep 17 00:00:00 2001 From: ardetrick <1452182+ardetrick@users.noreply.github.com> Date: Sun, 5 Jul 2026 15:09:35 -0500 Subject: [PATCH 3/3] test: document why the container lifecycle is manual The idiomatic @Testcontainers/@Container static-field pattern constructs containers at class-load time, but this container's configuration needs the injected @LocalServerPort, which only exists after the Spring context boots. Recording the reasoning where the deviation lives so it is not refactored into breakage. --- .../OryHydraReferenceApplicationFunctionalTests.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java b/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java index c1f2f5e..9f39097 100644 --- a/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java +++ b/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java @@ -82,6 +82,12 @@ public class OryHydraReferenceApplicationFunctionalTests { // test instance; @TestInstance(PER_CLASS) reuses that single instance for the whole class, // which is what allows @BeforeAll to be non-static — and therefore able to read the injected // @LocalServerPort — while still running exactly once before all tests. + // + // Deliberately NOT the idiomatic @Testcontainers/@Container-on-a-static-field pattern: that + // constructs the container at class-load time, but this container's configuration needs the + // injected @LocalServerPort, which only exists after the context boots. The alternatives all + // cost more than they save (a DEFINED_PORT app breaks parallel safety; an instance @Container + // restarts per test regardless of @TestInstance). @BeforeAll void startTestEnvironment() { playwright = Playwright.create();