Fix/quota store bootstrap#562
Conversation
📝 WalkthroughWalkthroughBoth ChangesDefault Tenant Quota Bootstrapping
Dependency Bumps
Sequence Diagram(s)sequenceDiagram
rect rgba(70, 130, 180, 0.5)
note right of CDI: MongoTenantQuotaStore startup
CDI->>MongoTenantQuotaStore: new(database, defaultTenantId, quota config props)
MongoTenantQuotaStore->>MongoDatabase: getCollection("tenant_quotas")
MongoTenantQuotaStore->>MongoDatabase: getCollection("tenant_usage")
MongoTenantQuotaStore->>MongoDatabase: createIndex(tenantId, unique) × 2
MongoTenantQuotaStore->>MongoDatabase: findOneAndUpdate(filter, $setOnInsert, upsert=true)
MongoDatabase-->>MongoTenantQuotaStore: null
end
rect rgba(60, 179, 113, 0.5)
note right of CDI: PostgresTenantQuotaStore startup
CDI->>PostgresTenantQuotaStore: new(dataSourceInstance, defaultTenantId, quota config props)
note over PostgresTenantQuotaStore: deferred until first getQuota call
PostgresTenantQuotaStore->>DataSource: getConnection()
PostgresTenantQuotaStore->>PostgreSQL: CREATE TABLE IF NOT EXISTS tenant_quotas
PostgresTenantQuotaStore->>PostgreSQL: CREATE TABLE IF NOT EXISTS tenant_usage
PostgresTenantQuotaStore->>PostgreSQL: INSERT INTO tenant_quotas ON CONFLICT DO NOTHING
PostgresTenantQuotaStore->>PostgreSQL: SELECT * FROM tenant_quotas WHERE tenantId=?
PostgreSQL-->>PostgresTenantQuotaStore: ResultSet
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency ReviewThe following issues were found:
License Issuespom.xml
OpenSSF Scorecard
Scanned Files
|
There was a problem hiding this comment.
Pull request overview
This PR updates a couple of dependencies and adds “default tenant quota” bootstrapping so MongoDB and PostgreSQL quota stores auto-create a default quota when none exists (matching the in-memory store), with accompanying unit tests.
Changes:
- Bumped
quarkus-mcp-serverandswagger-parserversions inpom.xmland documented it in the changelog. - Added default-tenant quota bootstrapping to
MongoTenantQuotaStoreandPostgresTenantQuotaStoreusing injected config defaults. - Added unit tests to validate bootstrapping behavior (create when missing; don’t overwrite when present).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/ai/labs/eddi/engine/tenancy/PostgresTenantQuotaStore.java |
Adds config-driven default quota bootstrap during lazy schema initialization. |
src/main/java/ai/labs/eddi/engine/tenancy/MongoTenantQuotaStore.java |
Adds config-driven default quota bootstrap during store construction. |
src/test/java/ai/labs/eddi/engine/tenancy/PostgresTenantQuotaStoreTest.java |
Adds bootstrap-focused tests for the Postgres store. |
src/test/java/ai/labs/eddi/engine/tenancy/MongoTenantQuotaStoreTest.java |
Adds bootstrap-focused tests for the Mongo store. |
pom.xml |
Dependency version bumps for MCP server + swagger parser. |
docs/changelog.md |
Documents dependency bumps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Bootstrap default tenant quota if none exists (parity with | ||
| // InMemoryTenantQuotaStore) | ||
| if (getQuota(defaultTenantId) == null) { | ||
| var defaultQuota = new TenantQuota(defaultTenantId, maxConvPerDay, maxAgents, maxApiCalls, maxCost, enabled); | ||
| setQuota(defaultQuota); | ||
| LOGGER.infof("Bootstrapped default tenant quota: tenantId=%s, enabled=%s, maxConv=%d, maxAgents=%d, maxApi=%d, maxCost=%.2f", | ||
| defaultTenantId, enabled, maxConvPerDay, maxAgents, maxApiCalls, maxCost); | ||
| } |
…ove schemaInitialized after bootstrap, remove unused vars
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/java/ai/labs/eddi/engine/tenancy/PostgresTenantQuotaStoreTest.java (1)
705-707: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAssert bootstrap SQL contract, not only invocation count.
executeUpdate()being called doesn’t prove the no-overwrite path is preserved. Capture prepared SQL and assert it includesON CONFLICT (tenant_id) DO NOTHING.Suggested assertion upgrade
+import org.mockito.ArgumentCaptor; @@ void bootstrapsAtomically() throws Exception { @@ // CREATE TABLE x2 + bootstrap INSERT + getQuota SELECT verify(statement, times(2)).execute(anyString()); verify(preparedStatement, atLeastOnce()).executeUpdate(); + ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class); + verify(connection, atLeastOnce()).prepareStatement(sqlCaptor.capture()); + assertTrue(sqlCaptor.getAllValues().stream() + .anyMatch(sql -> sql.contains("ON CONFLICT (tenant_id) DO NOTHING"))); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/ai/labs/eddi/engine/tenancy/PostgresTenantQuotaStoreTest.java` around lines 705 - 707, The test currently only verifies that executeUpdate() is called on the preparedStatement, but does not validate the actual SQL being executed. To properly assert that the no-overwrite path is preserved, capture the SQL string that was passed to the preparedStatement during execution and add an assertion that verifies the captured SQL contains the clause `ON CONFLICT (tenant_id) DO NOTHING`. This ensures the bootstrap operation properly handles duplicate inserts rather than just checking invocation count.src/test/java/ai/labs/eddi/engine/tenancy/MongoTenantQuotaStoreTest.java (1)
542-550: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winStrengthen bootstrap verification to assert atomic upsert semantics.
This test currently proves only that
findOneAndUpdatewas called. It won’t catch regressions whereupsert(true)is removed. CaptureFindOneAndUpdateOptionsand assertisUpsert().Suggested test tightening
+import org.mockito.ArgumentCaptor; @@ void bootstrapsAtomically() { @@ - verify(quotasCollection, atLeastOnce()).findOneAndUpdate( - any(Bson.class), any(Bson.class), any(FindOneAndUpdateOptions.class)); + ArgumentCaptor<FindOneAndUpdateOptions> optionsCaptor = + ArgumentCaptor.forClass(FindOneAndUpdateOptions.class); + verify(quotasCollection, atLeastOnce()).findOneAndUpdate( + any(Bson.class), any(Bson.class), optionsCaptor.capture()); + assertTrue(optionsCaptor.getValue().isUpsert()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/ai/labs/eddi/engine/tenancy/MongoTenantQuotaStoreTest.java` around lines 542 - 550, The test for MongoTenantQuotaStore bootstrap initialization currently only verifies that findOneAndUpdate was called, but does not validate that the upsert option is actually enabled. To strengthen this test, use an ArgumentCaptor to capture the FindOneAndUpdateOptions argument passed to the findOneAndUpdate method call, then assert that isUpsert() returns true on the captured options object. This will ensure the test catches any regressions where the upsert(true) setting is accidentally removed from the bootstrap logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/changelog.md`:
- Around line 7-19: The changelog entry for quarkus-mcp-server version bump
incorrectly describes the feature as adding Streamable HTTP transport support
when it actually added lazy SSE initialization for Streamable HTTP. Update the
description in the quarkus-mcp-server.version line to accurately state that
version 1.13.0 adds lazy SSE initialization for Streamable HTTP rather than
claiming it adds the Streamable HTTP transport itself, since that transport
already existed in prior versions.
---
Nitpick comments:
In `@src/test/java/ai/labs/eddi/engine/tenancy/MongoTenantQuotaStoreTest.java`:
- Around line 542-550: The test for MongoTenantQuotaStore bootstrap
initialization currently only verifies that findOneAndUpdate was called, but
does not validate that the upsert option is actually enabled. To strengthen this
test, use an ArgumentCaptor to capture the FindOneAndUpdateOptions argument
passed to the findOneAndUpdate method call, then assert that isUpsert() returns
true on the captured options object. This will ensure the test catches any
regressions where the upsert(true) setting is accidentally removed from the
bootstrap logic.
In `@src/test/java/ai/labs/eddi/engine/tenancy/PostgresTenantQuotaStoreTest.java`:
- Around line 705-707: The test currently only verifies that executeUpdate() is
called on the preparedStatement, but does not validate the actual SQL being
executed. To properly assert that the no-overwrite path is preserved, capture
the SQL string that was passed to the preparedStatement during execution and add
an assertion that verifies the captured SQL contains the clause `ON CONFLICT
(tenant_id) DO NOTHING`. This ensures the bootstrap operation properly handles
duplicate inserts rather than just checking invocation count.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee22b98f-41e1-4c9b-a488-f511f45bae9a
📒 Files selected for processing (6)
docs/changelog.mdpom.xmlsrc/main/java/ai/labs/eddi/engine/tenancy/MongoTenantQuotaStore.javasrc/main/java/ai/labs/eddi/engine/tenancy/PostgresTenantQuotaStore.javasrc/test/java/ai/labs/eddi/engine/tenancy/MongoTenantQuotaStoreTest.javasrc/test/java/ai/labs/eddi/engine/tenancy/PostgresTenantQuotaStoreTest.java
Summary
This pull request introduces dependency updates and improves tenant quota bootstrapping for both MongoDB and Postgres implementations. The main focus is on ensuring that a default tenant quota is created automatically if none exists, bringing parity with the in-memory implementation. It also adds comprehensive tests to verify this behavior. Additionally, two dependencies have been safely bumped to their latest stable versions.
Dependency Updates:
quarkus-mcp-server.versionfrom1.12.1to1.13.0inpom.xml, which adds Streamable HTTP transport support.swagger-parserfrom2.1.42to2.1.44inpom.xml, including patch-level bug fixes.Tenant Quota Bootstrapping Enhancements:
MongoTenantQuotaStorenow injects default quota config values and bootstraps a default tenant quota if none exists, with a new test-only constructor for flexibility. [1] [2]PostgresTenantQuotaStoresimilarly injects default quota config, bootstraps a default tenant quota if missing, and splits out internal helpers for quota access during bootstrap. [1] [2] [3] [4]Testing Improvements:
Type of Change
Checklist
./mvnw clean verify -DskipITs)Summary by CodeRabbit
Release Notes
New Features
Chores
Tests