src/BaseContext.cs (1)

126-130: Optional: restrict mutability of the property.

No external re-assignment needed; only per‑entry updates. Prefer get‑only to prevent accidental wholesale replacement.

-        public ConcurrentDictionary<string, string> AdminSDHolderHash { get; set; }
+        public ConcurrentDictionary<string, string> AdminSDHolderHash { get; }

Initialize in the constructor (already shown in the other diff).

src/Producers/LdapProducer.cs (3)

97-107: Lower log level for sensitive-ish hash and reduce noise.

The computed hash needn’t be Info; Debug/Trace is sufficient.

-                                Context.Logger.LogInformation(
-                                    "AdminSDHolder ACL hash {Hash} calculated for {Domain}.", authoritativeSd,
-                                    domain.Name);
+                                Context.Logger.LogDebug(
+                                    "AdminSDHolder ACL hash {Hash} calculated for {Domain}.", authoritativeSd,
+                                    domain.Name);

---

110-116: Log “collected” only on actual success.

Currently logs Trace even if the SD wasn’t retrieved; gate it after the hash is stored.

-                            Context.Logger.LogTrace("AdminSDHolder data collected for {Domain}", domain.Name);
+                            if (Context.AdminSDHolderHash.ContainsKey(domain.Name))
+                            {
+                                Context.Logger.LogTrace("AdminSDHolder data collected for {Domain}", domain.Name);
+                            }

---

97-99: Optional: reuse ACLProcessor instance.

Instantiate once per producer (or per domain iteration) instead of per AdminSDHolder hit. Minor allocation win.

Example (outside the shown hunk):

// class field
private readonly ACLProcessor _aclProcessor;

// in constructor
_aclProcessor = new ACLProcessor(Context.LDAPUtils);

// here, replace new ACLProcessor(...) with _aclProcessor

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/Producers/LdapProducer.cs (1)

103-106: Race fix confirmed: thread-safe update of AdminSDHolderHash.

Using ConcurrentDictionary.AddOrUpdate resolves the earlier concurrent write/read race on AdminSDHolderHash. Nice.

Run to confirm the dictionary type and case-insensitive comparer in Context/BaseContext/IContext:

#!/bin/bash
set -euo pipefail
rg -n -C2 'AdminSDHolderHash' --hidden
rg -nP 'ConcurrentDictionary\s*<\s*string\s*,\s*string\s*>' --hidden
rg -nP 'new\s+ConcurrentDictionary<\s*string\s*,\s*string\s*>\s*\(\s*StringComparer\.OrdinalIgnoreCase' --hidden

src/Producers/LdapProducer.cs (3)

58-125: AdminSDHolder prefetch: add guardrails and tighten logging.

Solid flow and ACL gating. Recommend:

• Wrap hash compute/store in try/catch to avoid aborting domain on unexpected parsing errors.
• Guard against null/empty hashes before storing/logging.
• Log Trace only when SD processed; warn if SD missing.
• Consider downgrading the hash log to Debug to reduce noise.

Apply this diff within this block:

-                            // Create an instance of ACLProcessor - _aclProcessor from ObjectProcessors isn't in this context
-                             var aclProcessor = new ACLProcessor(Context.LDAPUtils);
-
-                            // Calculate the authoritative SD based on a hash of the implicit ACLs & AclProtected
-                            var authoritativeSd = aclProcessor.CalculateImplicitACLHash(sd);
-
-                            // Store the hashes in the Context for later use
-                            Context.AdminSDHolderHash.AddOrUpdate(
-                                domain.Name,
-                                _ => authoritativeSd,
-                                (_, _) => authoritativeSd);
-
-                            Context.Logger.LogInformation(
-                                "AdminSDHolder ACL hash {Hash} calculated for {Domain}.", authoritativeSd,
-                                domain.Name);
+                            try
+                            {
+                                var aclProcessor = new ACLProcessor(Context.LDAPUtils);
+                                var authoritativeSd = aclProcessor.CalculateImplicitACLHash(sd);
+                                if (!string.IsNullOrEmpty(authoritativeSd))
+                                {
+                                    Context.AdminSDHolderHash.AddOrUpdate(
+                                        domain.Name,
+                                        _ => authoritativeSd,
+                                        (_, _) => authoritativeSd);
+                                    Context.Logger.LogDebug(
+                                        "AdminSDHolder ACL hash {Hash} calculated for {Domain}.",
+                                        authoritativeSd, domain.Name);
+                                    Context.Logger.LogTrace("AdminSDHolder data collected for {Domain}", domain.Name);
+                                }
+                                else
+                                {
+                                    Context.Logger.LogWarning(
+                                        "Empty AdminSDHolder ACL hash for {Domain}; skipping store.", domain.Name);
+                                }
+                            }
+                            catch (Exception ex)
+                            {
+                                Context.Logger.LogWarning(ex,
+                                    "Failed to compute/store AdminSDHolder hash for {Domain}", domain.Name);
+                            }
                         }
-
-                        Context.Logger.LogTrace("AdminSDHolder data collected for {Domain}", domain.Name);
+                        else
+                        {
+                            Context.Logger.LogWarning("AdminSDHolder SD missing for {Domain}", domain.Name);
+                        }

---

148-155: Use culture-invariant lowercasing for DN checks.

Avoid culture-sensitive casing for protocol strings. Prefer ToLowerInvariant().

-    var lower = distinguishedName.ToLower();
+    var lower = distinguishedName.ToLowerInvariant();

---

166-166: Typo in flag name (non-blocking).

ParititonLdapQueries appears misspelled. If not intentional, consider introducing a correctly spelled alias (PartitionLdapQueries) and deprecating the old name to avoid breaking configs/CLIs.

Can you confirm if this name is public/serialized? If so, we can add a non-breaking alias.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro