test: share one Hydra container across the functional suite (#181)

* test: share one Hydra container across the functional suite

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).

* test: unify the class lifecycle and remove static state

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.

* 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.

Author: ardetrick

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<OAuth2Client> 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<OAuth2Client> listOAuth2Clients() {
    */
   public Optional<OAuth2LoginRequest> 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

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,17 +53,21 @@
   ForwardingController.class,
 })
 @TestPropertySource(properties = {"debug=true"})
+@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;
 
   OryHydraContainer dockerComposeEnvironment;
 
-  OAuth2Client oAuth2Client;
+  String clientId;
+  String clientSecret;
+  String redirectUri;
 
   @Autowired HydraAdminClient.Properties properties;
 
@@ -76,25 +76,26 @@ public class OryHydraReferenceApplicationFunctionalTests {
   @Captor
   ArgumentCaptor<String> 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.
+  //
+  // 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
-  static void beforeAll() {
+  void startTestEnvironment() {
     playwright = Playwright.create();
     browser = playwright.chromium().launch();
-  }
-
-  @AfterAll
-  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.
+    // 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).
     dockerComposeEnvironment =
         OryHydraContainer.builder()
             .urlsLogin("http://localhost:" + springBootAppPort + "/login")
@@ -104,54 +105,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();
+    // 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();
   }
 
-  @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 stopTestEnvironment() {
+    playwright.close();
     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 +182,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 +275,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 +297,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 +452,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).
+  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;
   }