groovy
  1. groovy
  2. GROOVY-6304 Configslurper Issues
  3. GROOVY-4493

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

    Details

    • Type: Sub-task Sub-task
    • 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

        Hide
        Roshan Dawrani added a comment -

        Added code tags

        Show
        Roshan Dawrani added a comment - Added code tags
        Hide
        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
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: