[REFACTOR] Avoid double lookups in keyed repo checks; trim names; clarify opened check in Nitrite.java

[AKakkar28]

• Avoid double Map lookups and null chaining in keyed-repository existence checks by using a single get() and null guard.
• Trim collection names in validateCollectionName before emptiness and reserved-token checks.
• Keep closed-store guard centralized in checkOpened() for all callers.

@anidotnet I have refactored and tested all my changes as well. They passed.
Can you please review and merge?
Thanks

Summary by CodeRabbit

• Bug Fixes

  • Collection name validation now trims whitespace before checks, preventing unexpected rejections and improving detection of reserved names.

• Refactor

  • Streamlined repository existence checks to eliminate redundant lookups, improving performance when working with keyed repositories, especially at scale.
  • Internal logic improvements only; no changes to public APIs or configuration. Behavior remains backward compatible.

[coderabbitai]

Walkthrough

Refactors hasRepository overloads to avoid duplicate map lookups and null-pointer risk, by caching and null-checking the keyed repository set. Updates validateCollectionName to trim input before validation, using the normalized value for emptiness and reserved-name checks. No public API signatures changed; logic is confined to Nitrite.java.

Changes

Cohort / File(s)	Summary
Keyed repository lookup optimization nitrite/src/main/java/org/dizitart/no2/Nitrite.java	hasRepository(Class<T>, String) and hasRepository(EntityDecorator<T>, String) now cache listKeyedRepositories() result, retrieve the Set once via map.get(key), null-check it, and test containment against that set to avoid double lookups and NPEs.
Collection name normalization/validation nitrite/src/main/java/org/dizitart/no2/Nitrite.java	validateCollectionName(String) trims the input to a normalized value, validates non-null first, then uses the trimmed value for empty/reserved-name checks.

Sequence Diagram(s)

sequenceDiagram
  participant C as Caller
  participant N as Nitrite
  participant M as KeyedRepo Map

  C->>N: hasRepository(Class/Decorator, key)
  N->>N: map = listKeyedRepositories()
  N->>M: set = map.get(key)
  alt set is null
    N-->>C: return false
  else set contains entityName
    N-->>C: return true
  else not contained
    N-->>C: return false
  end

Loading

sequenceDiagram
  participant C as Caller
  participant N as Nitrite

  C->>N: validateCollectionName(name)
  N->>N: requireNonNull(name)
  N->>N: trimmed = name.trim()
  alt trimmed is empty or reserved
    N-->>C: throw IllegalArgumentException
  else valid
    N-->>C: proceed
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• [FIX] Small safety/cleanup in Nitrite interface (map lookups, closed check, name trim) #1161 — Adjusts keyed-repository checks to avoid double lookups/NPE and trims names in validation, matching the issue’s scope.

Poem

A whisk, a hop, a tidy tweak,
Maps checked once—no double peek.
Names now trimmed, so neat and clean,
Fewer nits in Nitrite’s sheen.
Thump-thump! goes my happy feet—
Refactors small, results so sweet. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title accurately and concisely summarizes the primary refactors — avoiding double lookups in keyed-repo checks, trimming collection names, and clarifying the opened-store guard — and includes a clear [REFACTOR] prefix so reviewers can quickly understand the intent.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

nitrite/src/main/java/org/dizitart/no2/Nitrite.java (3)

315-320: LGTM on single lookup; add explicit null-key guard to avoid possible NPE.

This reduces lookups and reads well. To be defensive if listKeyedRepositories() ever returns a ConcurrentHashMap (or another Map impl that NPEs on get(null)), add a fast null-key guard.

     checkOpened();
-    Map<String, Set<String>> keyed = listKeyedRepositories();
-    Set<String> entities = keyed.get(key);
+    if (key == null) return false;
+    Map<String, Set<String>> keyed = listKeyedRepositories();
+    Set<String> entities = keyed.get(key);
     if (entities == null) return false;
     String entityName = ObjectUtils.getEntityName(type);
     return entities.contains(entityName);

---

348-352: Guard nulls for consistency (decorator and key).

Same perf/readability win here. Add a null check for entityDecorator (aligns with other validation style) and a fast-return for null key to prevent potential Map NPE.

     checkOpened();
-    Map<String, Set<String>> keyed = listKeyedRepositories();
-    Set<String> entities = keyed.get(key);
-    if (entities == null) return false;
-    return entities.contains(entityDecorator.getEntityName());
+    notNull(entityDecorator, "entityDecorator cannot be null");
+    if (key == null) return false;
+    Map<String, Set<String>> keyed = listKeyedRepositories();
+    Set<String> entities = keyed.get(key);
+    if (entities == null) return false;
+    return entities.contains(entityDecorator.getEntityName());

---

362-365: Normalize collection names in lookup helpers (optional refactor)

Trimming before validation is good — also trim/normalize inputs at lookup sites to avoid mismatches (e.g., default hasCollection uses listCollectionNames().contains(name)). See nitrite/src/main/java/org/dizitart/no2/Nitrite.java:285.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9806e58 and 6ffaba0.

📒 Files selected for processing (1)

• nitrite/src/main/java/org/dizitart/no2/Nitrite.java (3 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Codacy Static Code Analysis

[AKakkar28]

@anidotnet Thanks for reviewing and approving.
How do I merge and close this PR? Do I have permissions for the same?

[anidotnet]

I have merged that