perf: return Collections.emptyMap() from asUnmodifiableMap() when attributes is empty#1974
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
ChangesNull/empty guard in
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/dev/openfeature/sdk/AbstractStructure.java`:
- Around line 32-34: The early return of Collections.emptyMap() when attributes
is empty but non-null breaks consistency with the live-view semantics of
asUnmodifiableMap() that is applied in the non-empty branch. Instead of
returning Collections.emptyMap() in the condition checking if attributes is null
or empty, remove the isEmpty() check from the conditional at line 32 so that
both empty and non-empty attributes maps are processed through the same
asUnmodifiableMap() logic used at line 35, ensuring consistent behavior
regardless of map size.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cfc6f27-ef9b-48e4-a981-9a7b8dda9b5c
📒 Files selected for processing (2)
src/main/java/dev/openfeature/sdk/AbstractStructure.javasrc/test/java/dev/openfeature/sdk/MutableStructureTest.java
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1974 +/- ##
============================================
+ Coverage 92.11% 93.11% +0.99%
- Complexity 660 665 +5
============================================
Files 59 59
Lines 1624 1626 +2
Branches 182 182
============================================
+ Hits 1496 1514 +18
+ Misses 80 66 -14
+ Partials 48 46 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ributes is empty Signed-off-by: Tobias Ibounig <tobias.ibounig@dynatrace.com>
c812b44 to
9202413
Compare
|



This PR
AbstractStructure.asUnmodifiableMap()returns a shared staticEMPTY_UNMODIFIABLE_MAPconstant whenattributesis empty, instead of allocating a newUnmodifiableMapwrapper on every callRelated Issues
None
Notes
Collections.unmodifiableMap(m)always allocates a new wrapper object, even whenmis empty. For empty structures the result is always semantically identical, so the wrapper can be replaced with a single static instance. The constant is typed asMap<String, Value>and backed byCollections.unmodifiableMap(Collections.emptyMap()), so callers see no behavioral difference.This path is not exercised by the current benchmark workload.
Follow-up Tasks
benchmark.txtafter all PRs are appliedSummary by CodeRabbit
Release Notes
Bug Fixes
Tests