Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent ConfigOverrides copy when there haven't been declared any overrides #1580

Closed
TuomasKiviaho opened this issue Mar 29, 2017 · 2 comments
Milestone

Comments

@TuomasKiviaho
Copy link

#1347 did not implement copy properly probably because the previous commits had already forgotten their parts as well. Perhaps the ConfigOverrides workaround below explains it better.

@@ -70,12 +70,12 @@
 
     public ConfigOverrides copy()
     {
-        if (_overrides == null) {
-            return new ConfigOverrides();
-        }
-        Map<Class<?>, MutableConfigOverride> newOverrides = _newMap();
-        for (Map.Entry<Class<?>, MutableConfigOverride> entry : _overrides.entrySet()) {
-            newOverrides.put(entry.getKey(), entry.getValue().copy());
+        Map<Class<?>, MutableConfigOverride> newOverrides = null;
+        if (_overrides != null) {
+            newOverrides = _newMap();
+            for (Map.Entry<Class<?>, MutableConfigOverride> entry : _overrides.entrySet()) {
+                newOverrides.put(entry.getKey(), entry.getValue().copy());
+            }
         }
         return new ConfigOverrides(newOverrides,
                 _defaultInclusion, _defaultSetterInfo, _visibilityChecker);

@cowtowncoder
Copy link
Member

Ah. Yes, you are right this would drop global defaults. Thank you for reporting!

@cowtowncoder cowtowncoder added 2.8 and removed 2.8 labels Mar 29, 2017
@cowtowncoder
Copy link
Member

Ah. Seems to affect 2.9 only, since 2.8 didn't hold global defaults.

@cowtowncoder cowtowncoder added this to the 2.9.0.pr3 milestone Mar 29, 2017
@cowtowncoder cowtowncoder removed the 2.9 label Mar 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants