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.

Sign up for GitHub

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

fix #13655 - restore dot notation navigation #13869

Merged
merged 2 commits into from
Dec 11, 2024
Merged

fix #13655 - restore dot notation navigation #13869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b92fc87
fix #13655 - restore dot notation navigation
jdaugherty Nov 21, 2024
682e7db
Merge branch '7.0.x' into feature/7.0.0/13655-dot-notation
jdaugherty Dec 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion grails-bootstrap/src/main/groovy/org/grails/config/CodeGenConfig.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ class CodeGenConfig implements Cloneable, ConfigMap {
}

protected <T> T convertToType(Object value, Class<T> requiredType) {
if(value == null) {
if(value == null || value instanceof NavigableMap.NullSafeNavigator) {
return null
}
else if(requiredType.isInstance(value)) {
Expand Down
197 changes: 183 additions & 14 deletions grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package org.grails.config
import groovy.transform.CompileDynamic
import groovy.transform.CompileStatic
import groovy.transform.EqualsAndHashCode
import org.codehaus.groovy.runtime.DefaultGroovyMethods
import org.slf4j.Logger
import org.slf4j.LoggerFactory

Expand Down Expand Up @@ -47,14 +48,14 @@ class NavigableMap implements Map<String, Object>, Cloneable {
final Map<String, Object> delegateMap
final String dottedPath

public NavigableMap() {
NavigableMap() {
rootConfig = this
path = []
dottedPath = ""
delegateMap = new LinkedHashMap<>()
}

public NavigableMap(NavigableMap rootConfig, List<String> path) {
NavigableMap(NavigableMap rootConfig, List<String> path) {
super()
this.rootConfig = rootConfig
this.path = path
Expand Down Expand Up @@ -144,7 +145,7 @@ class NavigableMap implements Map<String, Object>, Cloneable {
delegateMap.entrySet()
}

public void merge(Map sourceMap, boolean parseFlatKeys=false) {
void merge(Map sourceMap, boolean parseFlatKeys=false) {
mergeMaps(this, "", this, sourceMap, parseFlatKeys)
}

Expand Down Expand Up @@ -341,17 +342,17 @@ class NavigableMap implements Map<String, Object>, Cloneable {
targetMap.put(sourceKey, newValue)
}

public Object getAt(Object key) {
Object getAt(Object key) {
getProperty(String.valueOf(key))
}
public void setAt(Object key, Object value) {

void setAt(Object key, Object value) {
setProperty(String.valueOf(key), value)
}

public Object getProperty(String name) {
Object getProperty(String name) {
if (!containsKey(name)) {
return null
return new NullSafeNavigator(this, [name].asImmutable())
}
Object result = get(name)
if (!(result instanceof NavigableMap)) {
Expand All @@ -361,12 +362,12 @@ class NavigableMap implements Map<String, Object>, Cloneable {
}
return result
}
public void setProperty(String name, Object value) {

void setProperty(String name, Object value) {
mergeMapEntry(rootConfig, dottedPath, this, name, value, false, true)
}
public Object navigate(String... path) {

Object navigate(String... path) {
return navigateMap(this, path)
}

Expand All @@ -392,8 +393,8 @@ class NavigableMap implements Map<String, Object>, Cloneable {
}
}
}
public NavigableMap navigateSubMap(List<String> path, boolean createMissing) {

NavigableMap navigateSubMap(List<String> path, boolean createMissing) {
NavigableMap rootMap = this
NavigableMap currentMap = this
StringBuilder accumulatedPath = new StringBuilder()
Expand Down Expand Up @@ -488,4 +489,172 @@ class NavigableMap implements Map<String, Object>, Cloneable {
boolean equals(Object obj) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paulk-asert This class is tagged with both @EqualsAndHashCode and it then implements it's own equals / hashcode. Do you know if the groovy transformation will have any effect?

return delegateMap.equals(obj)
}

/**
* @deprecated This class should be avoided due to known performance reasons. Use {@code config.getProperty(String key, Class<T> targetType)} instead of dot based navigation.
*/
@Deprecated
@CompileStatic
static class NullSafeNavigator implements Map<String, Object>{
final NavigableMap parent
final List<String> path

NullSafeNavigator(NavigableMap parent, List<String> path) {
this.parent = parent
this.path = path
if (LOG.isWarnEnabled()) {
LOG.warn("Accessing config key '{}' through dot notation has known performance issues, consider using 'config.getProperty(key, targetClass)' instead.", path)
}
}

Object getAt(Object key) {
getProperty(String.valueOf(key))
}

void setAt(Object key, Object value) {
setProperty(String.valueOf(key), value)
}

@Override
int size() {
NavigableMap parentMap = parent.navigateSubMap(path, false)
if(parentMap != null) {
return parentMap.size()
}
return 0
}

@Override
boolean isEmpty() {
NavigableMap parentMap = parent.navigateSubMap(path, false)
if(parentMap != null) {
return parentMap.isEmpty()
}
return true
}

boolean containsKey(Object key) {
NavigableMap parentMap = parent.navigateSubMap(path, false)
if(parentMap == null) return false
else {
return parentMap.containsKey(key)
}
}

@Override
boolean containsValue(Object value) {
NavigableMap parentMap = parent.navigateSubMap(path, false)
if(parentMap != null) {
return parentMap.containsValue(value)
}
return false
}

@Override
Object get(Object key) {
return getAt(key)
}

@Override
Object put(String key, Object value) {
throw new UnsupportedOperationException("Configuration cannot be modified");
}

@Override
Object remove(Object key) {
throw new UnsupportedOperationException("Configuration cannot be modified");
}

@Override
void putAll(Map<? extends String, ?> m) {
throw new UnsupportedOperationException("Configuration cannot be modified");
}

@Override
void clear() {
throw new UnsupportedOperationException("Configuration cannot be modified");
}

@Override
Set<String> keySet() {
NavigableMap parentMap = parent.navigateSubMap(path, false)
if(parentMap != null) {
return parentMap.keySet()
}
return Collections.emptySet()
}

@Override
Collection<Object> values() {
NavigableMap parentMap = parent.navigateSubMap(path, false)
if(parentMap != null) {
return parentMap.values()
}
return Collections.emptySet()
}

@Override
Set<Entry<String, Object>> entrySet() {
NavigableMap parentMap = parent.navigateSubMap(path, false)
if(parentMap != null) {
return parentMap.entrySet()
}
return Collections.emptySet()
}

Object getProperty(String name) {
NavigableMap parentMap = parent.navigateSubMap(path, false)
if(parentMap == null) {
return new NullSafeNavigator(parent, ((path + [name]) as List<String>).asImmutable())
} else {
return parentMap.get(name)
}
}

void setProperty(String name, Object value) {
NavigableMap parentMap = parent.navigateSubMap(path, true)
parentMap.setProperty(name, value)
}

boolean asBoolean() {
false
}

Object invokeMethod(String name, Object args) {
throw new NullPointerException("Cannot invoke method " + name + "() on NullSafeNavigator");
}

boolean equals(Object to) {
return to == null || DefaultGroovyMethods.is(this, to)
}

Iterator iterator() {
return Collections.EMPTY_LIST.iterator()
}

Object plus(String s) {
return toString() + s
}

Object plus(Object o) {
throw new NullPointerException("Cannot invoke method plus on NullSafeNavigator")
}

boolean is(Object other) {
return other == null || DefaultGroovyMethods.is(this, other)
}

Object asType(Class c) {
if(c==Boolean || c==boolean) return false
return null
}

String toString() {
return null
}

// public int hashCode() {
// throw new NullPointerException("Cannot invoke method hashCode() on NullSafeNavigator");
// }
}
}
78 changes: 73 additions & 5 deletions grails-bootstrap/src/test/groovy/grails/config/ConfigMapSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,26 @@ test.another = true
configMap.getProperty('bar.two') == 'good4'

}
def "should support flattening keys"() {
def "should support flattening keys - map syntax"() {
given:
NavigableMap configMap = new NavigableMap()
when:
configMap.a = [b: [c: 1, d: 2]]
then:
configMap.toFlatConfig() == ['a.b.c': 1, 'a.b.d': 2]
}
def "should support flattening keys - dot syntax"() {
given:
NavigableMap configMap = new NavigableMap()
when:
configMap.a.b.c = 1
configMap.a.b.d = 2
then:
configMap.toFlatConfig() == ['a.b.c': 1, 'a.b.d': 2]
}

@Issue('#9146')
def "should support hashCode()"() {
def "should support hashCode() - map syntax"() {
given:
NavigableMap configMap = new NavigableMap()
when:
Expand All @@ -77,7 +86,18 @@ test.another = true
configMap.hashCode() == configMap.hashCode()
}
def "should support flattening list values"() {
@Issue('#9146')
def "should support hashCode() - dot syntax"() {
given:
NavigableMap configMap = new NavigableMap()
when:
configMap.a.b.c = 1
configMap.a.b.d = 2
then:"hasCode() doesn't cause a Stack Overflow error"
configMap.hashCode() == configMap.hashCode()
}
def "should support flattening list values - map syntax"() {
given:
NavigableMap configMap = new NavigableMap()
when:
Expand All @@ -90,8 +110,23 @@ test.another = true
'a.b.c[2]': 3,
'a.b.d': 2]
}
def "should support flattening list values - dot syntax"() {
given:
NavigableMap configMap = new NavigableMap()
when:
configMap.a.b.c = [1, 2, 3]
configMap.a.b.d = 2
then:
configMap.toFlatConfig() ==
['a.b.c': [1, 2, 3],
'a.b.c[0]': 1,
'a.b.c[1]': 2,
'a.b.c[2]': 3,
'a.b.d': 2]
}
def "should support flattening to properties"() {
def "should support flattening to properties - map syntax"() {
given:
NavigableMap configMap = new NavigableMap()
when:
Expand All @@ -104,8 +139,23 @@ test.another = true
'a.b.c[2]': '3',
'a.b.d': '2']
}
def "should support flattening to properties - dot syntax"() {
given:
NavigableMap configMap = new NavigableMap()
when:
configMap.a.b.c = [1, 2, 3]
configMap.a.b.d = 2
then:
configMap.toProperties() ==
['a.b.c': '1,2,3',
'a.b.c[0]': '1',
'a.b.c[1]': '2',
'a.b.c[2]': '3',
'a.b.d': '2']
}
def "should support cloning"() {
def "should support cloning - map syntax"() {
given:
NavigableMap configMap = new NavigableMap()
configMap.a = [b: [c: [1, 2, 3], d: 2]]
Expand All @@ -121,4 +171,22 @@ test.another = true
!cloned.is(configMap)
cloned == configMap
}
def "should support cloning - dot syntax"() {
given:
NavigableMap configMap = new NavigableMap()
configMap.a.b.c = [1, 2, 3]
configMap.a.b.d = 2
when:
NavigableMap cloned = configMap.clone()
then:
cloned.toFlatConfig() ==
['a.b.c': [1, 2, 3],
'a.b.c[0]': 1,
'a.b.c[1]': 2,
'a.b.c[2]': 3,
'a.b.d': 2]
!cloned.is(configMap)
cloned == configMap
}
}
Loading
Loading