Grails 7: Accessing config key 'grails.sitemesh.default.layout' through dot notation is deprecated

[jamesfredley]

Issue description

There is an open discussion on which way to proceed.

Sample App: https://github.com/jamesfredley/grails-website-test/tree/7.0.0-SNAPSHOT

Start with: ./gradlew bootRun or ./grailsw run-app

2024-09-19T08:56:44.757-04:00  WARN 13688 --- [  restartedMain] org.grails.config.NavigableMap           : Accessing config key 'grails.sitemesh.default.layout' through dot notation is deprecated, and it will be removed in a future release. Use 'config.getProperty(key, targetClass)' instead.

[davydotcom]

@jamesfredley if I recall this config access method was deprecated mostly due to micronaut... is it still worth deprecating? I always thought it was silly we removed some of this magic traversal

[jamesfredley]

@davydotcom I wasn't around for that historical context, but it would be great to keep config key access via dot notation and get rid of these warnings.

[matrei]

I think the problem here might be faulty condition logic. This method in NavigableMap is what prints the warning:

public Object getProperty(String name) {
    if (!containsKey(name)) {
        return null
    }
    Object result = get(name)
    if (!(result instanceof NavigableMap)) {
        if (LOG.isWarnEnabled()) {
            LOG.warn("Accessing config key '{}' through dot notation is deprecated, and it will be removed in a future release. Use 'config.getProperty(key, targetClass)' instead.", name)
        }
    }
    return result
}

When Grails is starting up it passes the first condition (containsKey('grails.sitemesh.default.layout') == true) but then get('grails.sitemesh.default.layout') == null which the second condition (!(result instanceof NavigableMap) does not account for.

If we change the second condition to if(result && !(result instanceof NavigableMap)) this warning should go away.

[jamesfredley]

related to:

https://github.com/grails/grails-core/issues/10188 https://github.com/grails/grails-core/pull/10280

[codeconsole]

Dot notation currently exclusively broken in plugins on 7..0.x

gsp example Nullpointer exception - console == null

${grailsApplication.config.grails.plugin.console.layout}

XXGrailsPlugin.groovy example Nullpointer exception - console == null

    void doWithApplicationContext() {
        config.grails.assets.plugin.'console'.excludes = ['**/*']
    }

Config is org.grails.config.PropertySourcesConfig

[codeconsole]

https://github.com/grails/grails-core/blob/7.0.x/grails-core/src/main/groovy/org/grails/config/PropertySourcesConfig.java https://github.com/grails/grails-core/commits/7.0.x/grails-core/src/main/groovy/org/grails/config/PropertySourcesConfig.java

[codeconsole]

https://github.com/grails/grails-core/blob/7.0.x/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java https://github.com/grails/grails-core/commits/7.0.x/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java

[codeconsole]

https://github.com/grails/grails-core/compare/6.2.x...7.0.x

[codeconsole]

https://github.com/grails/grails-core/pull/13448

[codeconsole]

Remove deprecated type NavigableMap.NullSafeNavigator https://github.com/grails/grails-core/pull/13448/commits/1ee3954f9688761a88d375c779e42481197c56e1

[codeconsole]

Deprecated here: https://github.com/grails/grails-core/pull/11554 https://github.com/grails/grails-core/commit/bdd4bc86b7fde4da4aaec99bbb1e82eb9db7dee7

https://docs.grails.org/5.3.6/guide/single.html#whatsNew

Deprecating ‘dot’-Based Navigation The ‘dot’-based navigation to Grails config is deprecated and will be removed in the future.

We request that you update your plugins to use configuration beans @ConfigurationProperties or @Value, or access configuration settings using grailsApplication.config.getProperty(‘a.b.c’, String) instead of grailsApplication.config.a.b.c. For more information, read the documentation at Creating and Installing Plugins.

[codeconsole]

From my comments on slack:

I don't really care about losing dot notation to be honest if it incurs and performance impact The issue I have is with the loss of the setter ${grailsApplication.config['grails.plugin.console.layout']} is a viable alternative but I do not see an inherent way to set a Map as we saw in the web console plugin upgrade I think this could be just a matter of adding a setProperty method? setProperty could be inefficient, but getters should be as efficient and null safe as possible..

we just need a solution for

config.grails.assets.plugin.'console'.excludes = ['**/*']

The current workaround is

config.merge(['config.grails.assets.plugin.console.excludes': ['**/*']])

[codeconsole]

https://github.com/grails/grails-core/blob/ab1b7e9ec4312170b4e6172bab89105cd12f9c4a/grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy#L27

https://github.com/grails/grails-core/blob/7.0.x/grails-core/src/main/groovy/grails/config/Config.groovy

[codeconsole]

Config extends ConfigMap which has a setAt(Object key, Object value) but that won't be navigable it will just set the full key I think instead of a TreeMap I think the only real lacking solution here is the loss of the namespace. For instance, you can no longer config['grails'] and get everything prefixed with grails (edited) If we can modify the existing code to set to namespaces and add a setAt function that uses merge, it could be a better solution IF we are truly suffering a performance hit at the current use of getters.

So the following questions need to be answered in the current state:

1. Why does config.grails.assets.plugin.'console'.excludes = ['**/*'] still work in application.groovy? I am guessing because it is parsed differently.
2. Is there still a solution to get all configurations for a namespace? e.g. config['grails.assets'] should return all configurations in that namespace.
3. Setting should set to a namespace. Currently config['grails.assets.plugin.console.excludes'] sets grails.assets.plugin.console.excludes as the key instead of a NavigableMap. Is there a workaround setter? Does application.groovy still work correctly? Always setting to namespace will resolve question 2.

[jeffscottbrown]

if I recall this config access method was deprecated mostly due to micronaut

Do you recall what it was about Micronaut integration that lead to this method being deprecated?