groovy

ConfigSlurper().parse() - property not located correctly in returned tree.

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.7.5, 1.8-beta-2
  • Fix Version/s: None
  • Component/s: groovy-jdk
  • Labels:
    None
  • Environment:
    windows 7; java version "1.6.0_21; gdk I.7.5 but also occurs for gdk 1.8.0 beta 2.
  • Testcase included:
    yes
  • Number of attachments :
    1

Description

package tasktrack;

import groovy.util.GroovyTestCase;
class ConfigSurlperTest extends GroovyTestCase {

	public void testParseScript() {
		def configFile = '''
		log4j {
			rootLogger="debug,stdout"
			appender {		
				stdout("org.apache.log4j.ConsoleAppender") { // See Note 2
					layout("org.apache.log4j.PatternLayout") {		  
						ConversionPattern="%5p [%t] (%F:%L) - %m%n"
					}
				}
			}		
		}
		'''
		
		def cfg = new ConfigSlurper().parse(configFile)
		assertEquals "debug,stdout", cfg.log4j.rootLogger  //Pass
		assertEquals "org.apache.log4j.ConsoleAppender", cfg.log4j.appender.stdout //Pass
		assertEquals "org.apache.log4j.PatternLayout", cfg.log4j.appender.stdout.layout // Fails - See Note 1
	}
}

Note 1
======

Result in the following exception

groovy.lang.MissingPropertyException: No such property: layout for class: java.lang.String
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.unwrap(ScriptBytecodeAdapter.java:49)
at org.codehaus.groovy.runtime.callsite.GetEffectivePojoPropertySite.getProperty(GetEffectivePojoPropertySite.java:63)
...

Note 2
======

The third assert works when the value, org.apache.log4j.ConsoleAppender, is not declared.
That is:

...
appender {
stdout {
layout("org.apache.log4j.PatternLayout")

{ ConversionPattern="%5p [%t] (%F:%L) - %m%n" }

}
}
...

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Roshan Dawrani added a comment -

Added code tags

Show
Roshan Dawrani added a comment - Added code tags
Roshan Dawrani made changes -
Field Original Value New Value
Description package tasktrack;

import groovy.util.GroovyTestCase;
class ConfigSurlperTest extends GroovyTestCase {

public void testParseScript() {
def configFile = '''
log4j {
rootLogger="debug,stdout"
appender {
stdout("org.apache.log4j.ConsoleAppender") { // See Note 2
layout("org.apache.log4j.PatternLayout") {
ConversionPattern="%5p [%t] (%F:%L) - %m%n"
}
}
}
}
'''

def cfg = new ConfigSlurper().parse(configFile)
assertEquals "debug,stdout", cfg.log4j.rootLogger //Pass
assertEquals "org.apache.log4j.ConsoleAppender", cfg.log4j.appender.stdout //Pass
assertEquals "org.apache.log4j.PatternLayout", cfg.log4j.appender.stdout.layout // Fails - See Note 1
}
}


Note 1
======

Result in the following exception

groovy.lang.MissingPropertyException: No such property: layout for class: java.lang.String
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.unwrap(ScriptBytecodeAdapter.java:49)
at org.codehaus.groovy.runtime.callsite.GetEffectivePojoPropertySite.getProperty(GetEffectivePojoPropertySite.java:63)
...

Note 2
======

The third assert works when the value, org.apache.log4j.ConsoleAppender, is not declared.
That is:

...
appender {
stdout {
layout("org.apache.log4j.PatternLayout") {
ConversionPattern="%5p [%t] (%F:%L) - %m%n"
}
}
}
... 		{code}
package tasktrack;

import groovy.util.GroovyTestCase;
class ConfigSurlperTest extends GroovyTestCase {

public void testParseScript() {
def configFile = '''
log4j {
rootLogger="debug,stdout"
appender {
stdout("org.apache.log4j.ConsoleAppender") { // See Note 2
layout("org.apache.log4j.PatternLayout") {
ConversionPattern="%5p [%t] (%F:%L) - %m%n"
}
}
}
}
'''

def cfg = new ConfigSlurper().parse(configFile)
assertEquals "debug,stdout", cfg.log4j.rootLogger //Pass
assertEquals "org.apache.log4j.ConsoleAppender", cfg.log4j.appender.stdout //Pass
assertEquals "org.apache.log4j.PatternLayout", cfg.log4j.appender.stdout.layout // Fails - See Note 1
}
}
{code}

Note 1
======

Result in the following exception

groovy.lang.MissingPropertyException: No such property: layout for class: java.lang.String
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.unwrap(ScriptBytecodeAdapter.java:49)
at org.codehaus.groovy.runtime.callsite.GetEffectivePojoPropertySite.getProperty(GetEffectivePojoPropertySite.java:63)
...

Note 2
======

The third assert works when the value, org.apache.log4j.ConsoleAppender, is not declared.
That is:

...
appender {
stdout {
layout("org.apache.log4j.PatternLayout") {
ConversionPattern="%5p [%t] (%F:%L) - %m%n"
}
}
}
...
Hide
Permalink
Roshan Dawrani added a comment - 
prop("value") {
    ...
    ....
}

The above syntax seems to be quite ill-supported by ConfigSlurper.

It stores it like [prop="value"], i.e, the value is a string and no nested-map, so retrieving any property below this level using dot notation just does not work.

It will be better if it rejects the cases it cannot handle.

Show
Roshan Dawrani added a comment - prop( "value" ) { ... .... } The above syntax seems to be quite ill-supported by ConfigSlurper. It stores it like [prop="value"] , i.e, the value is a string and no nested-map, so retrieving any property below this level using dot notation just does not work. It will be better if it rejects the cases it cannot handle.
Hide
Permalink
Roshan Dawrani added a comment -

Since both the versions below are not supported (i.e, no fear of breaking any exiting working code that uses such structure), what about rejecting such usage and limit it to what it can properly handle?

prop1("value1") {
    prop2("value2") {}
}
println prop1.prop2

and

prop1("value1") {
    prop2="value2"
}
println prop1.prop2
Show
Roshan Dawrani added a comment - Since both the versions below are not supported (i.e, no fear of breaking any exiting working code that uses such structure), what about rejecting such usage and limit it to what it can properly handle? prop1( "value1" ) { prop2( "value2" ) {} } println prop1.prop2 and prop1( "value1" ) { prop2= "value2" } println prop1.prop2
Hide
Permalink
Tom Weekes added a comment -

I can understand the reason for rejection but it is only short term.
The ConfigSlurper should allow configuration to be declared in a manner that is natural and easy maintained in the future. As such it makes sense to be able to assign values to non-leave nodes just like XML allows (through the user of attributes).

The following style of configuration is very cumbersome.
appender."stdout"="org.apache.log4j.ConsoleAppender"
appender."stdout.layout"="org.apache.log4j.PatternLayout"

Thanks for the interest in the issue.

Show
Tom Weekes added a comment - I can understand the reason for rejection but it is only short term. The ConfigSlurper should allow configuration to be declared in a manner that is natural and easy maintained in the future. As such it makes sense to be able to assign values to non-leave nodes just like XML allows (through the user of attributes). The following style of configuration is very cumbersome. appender."stdout"="org.apache.log4j.ConsoleAppender" appender."stdout.layout"="org.apache.log4j.PatternLayout" Thanks for the interest in the issue.
Hide
Permalink
Roshan Dawrani added a comment -

Yes that's true. Being able to assign values to non-leaf nodes will be useful but I was thinking that until we are in half-baked-support-for-it situation, may be we should reject it in a more informative way.

We can leave the issue open for its proper implementation.

For now, it can properly inform the users about its limits and later try to expand its scope properly rather than allowing something at syntax level and then not handling its storage internally.

Show
Roshan Dawrani added a comment - Yes that's true. Being able to assign values to non-leaf nodes will be useful but I was thinking that until we are in half-baked-support-for-it situation, may be we should reject it in a more informative way. We can leave the issue open for its proper implementation. For now, it can properly inform the users about its limits and later try to expand its scope properly rather than allowing something at syntax level and then not handling its storage internally.

People

  • Assignee:
    Unassigned
    Reporter:
    Tom Weekes
Vote (0)
Watch (0)

Dates

  • Created:
    Updated: