feat(tmmongo): add WithSettingsCheckInterval for tenant config revalidation (#376)

[jeffersonrodrigues92]

Summary

Closes #376. Ports the WithSettingsCheckInterval revalidation logic from tmpostgres.Manager to tmmongo.Manager.

Changes

• WithSettingsCheckInterval(d time.Duration) option added to tmmongo
• Per-tenant lazy revalidation: spawns goroutine on cache hit after interval passes
• Detects TenantSuspendedError → evicts cached MongoDB connection
• Panic recovery in goroutine (same pattern as postgres)
• Close() waits for in-flight revalidation goroutines
• 11 new tests covering all scenarios

Test plan

• go build ./... passes
• go test ./commons/tenant-manager/mongo/... -count=1 -short — all pass
• Integration test: purge tenant → MongoDB connection evicted after interval

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9d593ddf-1d3a-4e5a-92ff-b071def462e9

📥 Commits

Reviewing files that changed from the base of the PR and between 69cc4e4 and 728fe42.

📒 Files selected for processing (2)

• commons/tenant-manager/mongo/manager.go
• commons/tenant-manager/mongo/manager_test.go

---

Walkthrough

The change adds periodic, per-tenant asynchronous settings revalidation for cached MongoDB connections in the tenant manager. A new option WithSettingsCheckInterval configures the revalidation interval. On a cached-connection ping hit, GetConnection updates an LRU timestamp, checks lastSettingsCheck, and—if due—updates it and spawns a goroutine running revalidatePoolSettings. That routine requests tenant settings (with timeout), evicts suspended tenants, logs or ignores other failures, and applies settings. revalidateWG tracks revalidations so Close() waits for them before shutdown. Tests cover timing, eviction, errors, and cleanup.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Manager
    participant Cache
    participant TMgrAPI as Tenant Manager API
    participant Revalidator

    Caller->>Manager: GetConnection(tenantID)
    Manager->>Cache: Lookup cached connection
    alt Cache hit
        Manager->>Manager: Ping health check
        alt Ping OK
            Manager->>Manager: Record LRU timestamp
            Manager->>Manager: Check lastSettingsCheck vs interval
            alt Revalidation due
                Manager->>Manager: Update lastSettingsCheck
                Manager->>Revalidator: Spawn revalidatePoolSettings(tenantID)
                Manager-->>Caller: Return cached connection
                Revalidator->>TMgrAPI: HTTP GET tenant config (with timeout)
                alt HTTP 403 (suspended)
                    Revalidator->>Cache: Evict connection
                    Revalidator->>Manager: Remove lastAccessed & lastSettingsCheck
                else HTTP success
                    Revalidator->>Manager: ApplyConnectionSettings (no-op for Mongo)
                else HTTP error
                    Revalidator->>Manager: Log warning
                end
            else Not due
                Manager-->>Caller: Return cached connection
            end
        else Ping failed
            Manager-->>Caller: Create new connection
        end
    else Cache miss
        Manager-->>Caller: Create new connection
    end

Loading

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding WithSettingsCheckInterval option to tmmongo for tenant config revalidation.
Description check	✅ Passed	The description covers the key changes, implementation details, and test plan; however, it does not follow the provided template structure with explicit checklist items.
Linked Issues check	✅ Passed	All coding requirements from issue #376 are addressed: WithSettingsCheckInterval option added, per-tenant revalidation implemented, TenantSuspendedError detection with cache eviction, panic recovery, Close() waits for goroutines, and 11 tests cover scenarios.
Out of Scope Changes check	✅ Passed	All changes are scoped to implementing WithSettingsCheckInterval for tmmongo.Manager; no out-of-scope modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@commons/tenant-manager/mongo/manager_test.go`:
- Around line 1200-1332: Tests currently bypass Manager.GetConnection by calling
revalidatePoolSettings directly or re-implementing its logic; change the tests
to call manager.GetConnection(ctx, "tenant-...") so the actual cache-hit branch,
mutation of manager.lastSettingsCheck and the goroutine launch are exercised.
Specifically, pre-populate manager.connections["tenant-..."] with a connection
whose DB is non-nil (or a lightweight mock) and set lastAccessed and
lastSettingsCheck so GetConnection returns the cached connection and then
triggers revalidation based on manager.settingsCheckInterval; for the enabled
case set WithSettingsCheckInterval to a small >0 duration and assert the
tenant-manager HTTP callCount increases and lastSettingsCheck was updated, and
for the disabled (zero/negative) cases call GetConnection and assert no HTTP
calls occur and manager.settingsCheckInterval is zero (verify negative is
clamped). Use manager.GetConnection, manager.settingsCheckInterval,
manager.connections, manager.lastSettingsCheck, manager.lastAccessed and avoid
calling manager.revalidatePoolSettings directly.

In `@commons/tenant-manager/mongo/manager.go`:
- Around line 354-360: The eviction path uses context.Background() when
core.IsTenantSuspendedError(err) triggers, which can hang if
Disconnect/CloseConnection blocks; change the call to CloseConnection to use a
bounded context (e.g., create a context with timeout using
settingsRevalidationTimeout via context.WithTimeout) and pass that ctx to
p.CloseConnection (and ensure Disconnect/Close honor ctx), cancel the ctx after
the call so the goroutine cannot block indefinitely; reference
IsTenantSuspendedError, CloseConnection, Disconnect and Close in the change.
- Around line 294-307: The race occurs because shouldRevalidate is computed
under p.mu but p.revalidateWG.Go(...) is called after unlocking, allowing
Close() to clear state before the goroutine is registered; fix by registering
the work while still holding the mutex: move the p.revalidateWG.Go(...) (or at
least a call to revalidateWG.Add/Go-equivalent) so it happens before
p.mu.Unlock(), update p.lastSettingsCheck[tenantID] as currently done, and only
then release p.mu and call p.revalidatePoolSettings(tenantID) from within the
goroutine; reference p.mu, shouldRevalidate, p.revalidateWG.Go,
p.lastSettingsCheck, revalidatePoolSettings, and Close() to locate the code to
change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 44f6a88a-54c5-4be9-bbf2-aa1996106c1f

📥 Commits

Reviewing files that changed from the base of the PR and between 977411b and 69cc4e4.

📒 Files selected for processing (2)

• commons/tenant-manager/mongo/manager.go
• commons/tenant-manager/mongo/manager_test.go