Reuse successful LDAP base searches - #79
Merged
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5b8ed060-ed02-4402-b5e1-af407bcb2f61
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Successful deletions must invalidate cached existence to prevent stale results.
Get a fresh assessment by requesting another Copilot review.
Review tier: Balanced (auto)
Findings: 1
Note
Copilot is running an experiment and ran this review at Balanced.
Open findings (1)
What changed in this PR
Caches successfully searched LDAP bases to avoid redundant existence queries during calculation.
Changes:
- Adds synchronized tracking of known-existing DNs.
- Reuses successful searches in
exists?. - Adds success/failure cache tests and corrects connection comments.
| File | Description |
|---|---|
lib/entitlements/service/ldap.rb |
Implements synchronized LDAP base caching. |
spec/unit/entitlements/service/ldap_spec.rb |
Tests successful and failed search caching. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Clear both known-existence and read caches after a successful LDAP delete so later checks re-read the directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3f79283a-9bd3-46dd-8e33-676d145619c5
Prepare the LDAP cache optimization for publication on merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3f79283a-9bd3-46dd-8e33-676d145619c5
trespilhas
approved these changes
Sep 14, 2026
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.
In benchmarking, I discovered that we have a number of situations where we check to see if a DN exists during the same runtime and in many cases do so repeatedly.
This change keeps an in memory cache of the DNs already checked so that we don't need to do 2-3 lookups of the same DN with
exists?against live LDAP.In the benchmarks I ran, this eliminated as much as 50% of the LDAP queries.