test: share one Postgres per module + fix API-key prefix collision - #246
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
The stored prefix is rawKey.take(12) = "sk_live_" + 4 hex chars (~65k values) and its index is non-unique by design, but findByPrefix returned a single row. Once two live keys shared a prefix, validate() threw IncorrectResultSizeDataAccessException and ApiKeyAuthFilter 401'd both keys until one was revoked — ~50% likely by ~300 active keys. Replace the port's findByPrefix with findAllByPrefix; validate() now bcrypt-matches the raw key against each non-revoked candidate. No schema change; Redis cache stays prefix-scoped. Adds a repository collision test and a service disambiguation test. Signed-off-by: ifsantana <flaubert165@gmail.com>
Consolidate ~27 per-class Postgres Testcontainers in the infrastructure module onto the existing PostgresTestContainers singleton via a new SharedPostgresTestBase (@DynamicPropertySource); slice tests roll back and committing tests stay tenant-scoped, so the shared DB is safe. App module @Serviceconnection containers become JVM singletons shared across the ~5 @SpringBootTest contexts. Adds -T 1C (.mvn/maven.config) and class-level JUnit parallelism in the container-free modules (core, application, sdk-kotlin). Caps the Hikari pool to 2 and raises Postgres max_connections so the many cached Spring contexts sharing one container don't exhaust connections. Postgres container starts across the build: ~33 -> 6. Local clean verify: 5:31 -> 4:00, all suites green. Signed-off-by: ifsantana <flaubert165@gmail.com>
ifsantana
force-pushed
the
test/shared-postgres-testcontainers
branch
from
July 16, 2026 11:39
d957de5 to
094f702
Compare
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #242, closes #245. Phase B of the pipeline cost/speed plan.
Why
./mvnw verify's dominant cost was Testcontainers churn: ~27 infrastructure test classes each started their ownPostgreSQLContainer("postgres:16")(~8–12 s boot + a full Flyway run each), and the ~5 app@SpringBootTestcontexts each booted their own Postgres+Redis pair.Changes
Testcontainers consolidation (#242)
SharedPostgresTestBasewiring the existingPostgresTestContainerssingleton via@DynamicPropertySource. The 21@DataJpaTest/service slice classes and the infra@SpringBootTestclasses now extend it instead of declaring@Container. Slice tests roll back per-test; committing tests are tenant-scoped (TenantId.generate()per class), so a shared DB is safe. Flyway migrates once instead of ~27×.FlywayMigrationTestintentionally keeps its own container (it asserts a pristine schema).TestcontainersConfiguration:@ServiceConnectionbeans now back onto JVM-singleton containers, shared across the ~5 app contexts.max_connectionsto 300 — many cached Spring contexts hold live pools against the one shared instance.Bounded parallelism (#242)
-T 1Cvia.mvn/maven.config(module graph is-T-safe; each parallel module gets its own fork JVM → own singleton container).core,application,sdk-kotlin). Not enabled in infrastructure/app (would interleave transactions on the shared DB).Prefix-collision fix (#245) — surfaced by the shared container
The stored API-key prefix is
rawKey.take(12)=sk_live_+ 4 hex chars (~65k values) with a non-unique index, butfindByPrefixreturned a single row. Once two live keys shared a prefix,validate()threwIncorrectResultSizeDataAccessExceptionandApiKeyAuthFilter401'd both keys until one was revoked — ~50% likely by ~300 active keys, and deterministic in CI once test classes shared one DB. Replaced the port'sfindByPrefixwithfindAllByPrefix;validate()bcrypt-matches the raw key against each non-revoked candidate. No schema change; Redis cache stays prefix-scoped. Added a repository collision test and a service disambiguation test.Impact
./mvnw clean verify: 5:31 → 4:00, all suites green (same wins apply in CI and for every contributor).Test plan
./mvnw clean verify— BUILD SUCCESS; 597 infra tests + all module suites green.findAllByPrefix returns every key sharing a prefix(repository),validate disambiguates by hash when several keys share a prefix(service).