Add caching support for IdentityProviderStorageProvider.getForLogin operations

[zaibkhan]

No description provided.

[codoki-pr-intelligence]

Codoki PR Review

Summary: Add login IDP caching, ensure invalidations, tighten filtering
What’s good: The new getForLogin cache mirrors existing list-query patterns, includes per-mode keying with per-organization search keys, and integrates invalidation on create/update/remove via a shared login predicate.
Review Status: ✅ Safe to merge

Issues (Medium)

Severity	Issue	Why it matters
Medium	Correctness — Login cache misses org-level invalidation checks	…/idp/InfinispanIdentityProviderStorageProvider.java Consider also invalidating when the specific organization is invalidated. Otherwise, an ORG_ONLY/ALL query with a removed/updated org could serve stale results until a later IDP change triggers cache invalidation.
Medium	Testing — Test cleanup uses incorrect hardcoded alias	…/cache/OrganizationCacheTest.java The cleanup references a hardcoded alias string, so created IDPs are not removed. Capture the actual alias per iteration to avoid leaking test data and side effects.

Showing up to 2 medium issue(s). See inline suggestions for more details.

Key Feedback (click to expand)

• Needs improvement: Consider also invalidating the login cache when the organization itself is invalidated to avoid serving stale results for ORG_ONLY/ALL queries tied to a removed or updated org.
• Testing: The new test provides strong coverage for cache population and invalidation across create/update/link flows; however, it doesn't exercise org-level invalidation alone (e.g., removing an org and verifying getForLogin respects that). Also, the cleanup handler in the test uses a hardcoded alias string, likely leaving created IDPs undeleted.
• Documentation: Add a brief Javadoc for cacheKeyForLogin describing that per-organization results are stored as search-key entries under the same mode cache key (searchKey is empty for null organizationId). This will aid maintainability.
• Compatibility: No API-breaking changes detected. The LoginFilter predicate addition tightens semantics but aligns with existing default getForLogin filtering for public org brokers.
• Performance: Invalidating all mode caches when a single org-linked IDP changes is conservative but acceptable; if cache footprint becomes an issue, consider per-org invalidation granularity later.
• Security: No security-sensitive changes identified; filters still exclude hidden/link-only and non-public org brokers.
• Open questions: Does the OrganizationProvider link/unlink path always trigger invalidations that reach the login caches (beyond standard org cache keys)? If not, should getForLogin proactively check org invalidation like getByOrganization does?

Confidence: 4/5 — Looks good; minor fixes (2 medium)

Sequence Diagram

sequenceDiagram
    participant Caller
    participant InfinispanIDP as InfinispanIdentityProviderStorageProvider
    participant Delegate as idpDelegate
    participant Cache as RealmCacheManager
    Caller->>InfinispanIDP: getForLogin(mode, organizationId)
    alt cache invalid
        InfinispanIDP->>Delegate: getForLogin(mode, organizationId)
        Delegate-->>InfinispanIDP: Stream<IdentityProviderModel>
        InfinispanIDP-->>Caller: Stream (mapped)
    else cache present
        InfinispanIDP->>Cache: get(cacheKey)
        alt query null
            InfinispanIDP->>Delegate: getForLogin(mode, organizationId)
            Delegate-->>InfinispanIDP: Stream ids
            InfinispanIDP->>Cache: addRevisioned(query)
        else searchKey missing
            InfinispanIDP->>Cache: invalidateObject(cacheKey)
            InfinispanIDP->>Delegate: getForLogin(mode, organizationId)
            Delegate-->>InfinispanIDP: Stream ids
            InfinispanIDP->>Cache: addRevisioned(query)
        end
        InfinispanIDP-->>Caller: Stream from cached ids
    end

Loading

React with 👍 or 👎 if you found this review useful.

[codoki-pr-intelligence]

🔷 Medium: Consider also invalidating when the specific organization is invalidated. Otherwise, an ORG_ONLY/ALL query with a removed/updated org could serve stale results until a later IDP change triggers cache invalidation.

suggestion
if (isInvalid(cacheKey) || (organizationId != null && isInvalid(organizationId))) {
return idpDelegate.getForLogin(mode, organizationId).map(this::createOrganizationAwareIdentityProviderModel);
}

[codoki-pr-intelligence]

🔷 Medium: The cleanup references a hardcoded alias string, so created IDPs are not removed. Capture the actual alias per iteration to avoid leaking test data and side effects.

suggestion
String createdAlias = idpRep.getAlias();
getCleanup().addCleanup(() -> testRealm().identityProviders().get(createdAlias).remove());