groovy
  1. groovy
  2. GROOVY-3219

NamespaceBuilder does not create child nodes

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.7
    • Fix Version/s: 1.6-rc-2, 1.5.8, 1.7-beta-1
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      0

      Description

      NamespaceBuilder does not create child nodes

      	@Test
      	void testNamespaceBuilder() {
      
      		def n = new Namespace('http://foo/bar')
      		
      		def builder = new NamespaceBuilder(new NodeBuilder()).namespace(n.uri)
      		Node node = builder.crap(id:"3") {
      			bucket(name:"foo")
      		}
      		// ** this prints out only the "crap" element, not its child
      		new XmlNodePrinter().print(node)
      
      		// *** fails
      		assert node[n.bucket][0].@name == 'foo'
      
      	}
      

      If I'm not missing something here then the bug is in NamespaceBuidlerSupport (it doesn't do anything with the BuilderSupport instance that you pass into the constructor).

        Activity

        Hide
        Dave Syer added a comment -

        This worked for me building vanilla XML nodes (maybe it deosn't work for some other use case that I'm not aware of - otherwise why was NamespaceBuilder* implemented the way it was in Groovy?):

        import groovy.xml.QName
        import groovy.util.NodeBuilder
        
        class NamespaceBuilder extends NodeBuilder {
        
            private final String uri;
            private final String prefix;
        
            public NamespaceBuilder(String uri) {
                this(uri, "");
            }
        
            public NamespaceBuilder(String uri, String prefix) {
                super();
                this.uri = uri;
                this.prefix = prefix;
            }
        
            protected Object getName(String methodName) {
                 return new QName(uri, methodName, prefix);
            }
        
        }
        
        Show
        Dave Syer added a comment - This worked for me building vanilla XML nodes (maybe it deosn't work for some other use case that I'm not aware of - otherwise why was NamespaceBuilder* implemented the way it was in Groovy?): import groovy.xml.QName import groovy.util.NodeBuilder class NamespaceBuilder extends NodeBuilder { private final String uri; private final String prefix; public NamespaceBuilder( String uri) { this (uri, ""); } public NamespaceBuilder( String uri, String prefix) { super (); this .uri = uri; this .prefix = prefix; } protected Object getName( String methodName) { return new QName(uri, methodName, prefix); } }
        Hide
        Paul King added a comment - - edited

        I think NamespaceBuilder was intended to work for more than just groovy.util.Node objects, e.g. W3C Nodes/Elements, Ant, MBean nodes etc. Though I don't think it actually covers all of the cases it would need to make that intention a reality. I think a little bit of redesign/refactoring would be a good thing. In your simple proposal above, you only handle the case of a single namespace. It would need to handle a map of namespaces.

        Show
        Paul King added a comment - - edited I think NamespaceBuilder was intended to work for more than just groovy.util.Node objects, e.g. W3C Nodes/Elements, Ant, MBean nodes etc. Though I don't think it actually covers all of the cases it would need to make that intention a reality. I think a little bit of redesign/refactoring would be a good thing. In your simple proposal above, you only handle the case of a single namespace. It would need to handle a map of namespaces.
        Hide
        Paul King added a comment -

        Initial fix in trunk. Seems OK for MarkupBuilder, AntBuilder, DOMBuilder, NodeBuilder but if you could test it, that would be great.

        Show
        Paul King added a comment - Initial fix in trunk. Seems OK for MarkupBuilder, AntBuilder, DOMBuilder, NodeBuilder but if you could test it, that would be great.
        Hide
        Dave Syer added a comment -

        I copied the code for NamespaceBuilderSupport only and tried it (did I need more?). Failed my simple tests. The nodes that are built are not addressable still (child nodes are not added):

        	@Test
        	void testDefaultPrefix() {
        		def Namespace n = new Namespace("http://foo.com/bar")
        		def builder = new NamespaceBuilderSupport(new NodeBuilder(), n.uri)
        		Node node = builder.foo {
        			bar(name:"foo")
        		}
        		// Fails because there is no child node "bar"
        		assert node[n.bar][0].@name == "foo"
        		
        		def writer = new StringWriter()
        		new XmlNodePrinter(new PrintWriter(writer)).print(node)
        		
        		assert writer.toString().contains("<bar name=\"foo\"/>")
        	}
        	
        	@Test
        	void testNonDefaultPrefix() {
        		def Namespace n = new Namespace("http://foo.com/bar", "f")
        		def builder = new NamespaceBuilderSupport(new NodeBuilder(), n.uri, n.prefix)
        		Node node = builder.foo {
        			bar(name:"foo")
        		}
        		assert node[n.bar][0].@name == "foo"
        
        		def writer = new StringWriter()
        		new XmlNodePrinter(new PrintWriter(writer)).print(node)
        		println writer
        		
        		assert writer.toString().contains("<f:bar name=\"foo\"/>")
        	}
        
        Show
        Dave Syer added a comment - I copied the code for NamespaceBuilderSupport only and tried it (did I need more?). Failed my simple tests. The nodes that are built are not addressable still (child nodes are not added): @Test void testDefaultPrefix() { def Namespace n = new Namespace( "http: //foo.com/bar" ) def builder = new NamespaceBuilderSupport( new NodeBuilder(), n.uri) Node node = builder.foo { bar(name: "foo" ) } // Fails because there is no child node "bar" assert node[n.bar][0].@name == "foo" def writer = new StringWriter() new XmlNodePrinter( new PrintWriter(writer)).print(node) assert writer.toString().contains( "<bar name=\" foo\ "/>" ) } @Test void testNonDefaultPrefix() { def Namespace n = new Namespace( "http: //foo.com/bar" , "f" ) def builder = new NamespaceBuilderSupport( new NodeBuilder(), n.uri, n.prefix) Node node = builder.foo { bar(name: "foo" ) } assert node[n.bar][0].@name == "foo" def writer = new StringWriter() new XmlNodePrinter( new PrintWriter(writer)).print(node) println writer assert writer.toString().contains( "<f:bar name=\" foo\ "/>" ) }
        Hide
        Paul King added a comment - - edited

        There was also a minor change to BuilderSupport that you would need.

        Can you try with 1.6-RC-1? For me with 1.6-RC-1 both your tests worked fine.

        Show
        Paul King added a comment - - edited There was also a minor change to BuilderSupport that you would need. Can you try with 1.6-RC-1? For me with 1.6-RC-1 both your tests worked fine.
        Hide
        Dave Syer added a comment -

        They both fail for me still (null pointer exception on the line where I try to reference the child node).

        Show
        Dave Syer added a comment - They both fail for me still (null pointer exception on the line where I try to reference the child node).
        Hide
        Paul King added a comment -

        Was that with 1.6-RC-1?

        Show
        Paul King added a comment - Was that with 1.6-RC-1?
        Hide
        Dave Syer added a comment -

        Yes, sorry. I thought that's what you wanted me to try. I used

        <dependency>
        	<groupId>org.codehaus.groovy</groupId>
        	<artifactId>groovy-all</artifactId>
        	<version>1.6-RC-1</version>
        </dependency>
        
        Show
        Dave Syer added a comment - Yes, sorry. I thought that's what you wanted me to try. I used <dependency> <groupId>org.codehaus.groovy</groupId> <artifactId>groovy-all</artifactId> <version>1.6-RC-1</version> </dependency>
        Hide
        Dave Syer added a comment -

        Wait, no... there was another groovy on my classpath (groovy-all-minimal-1.5.6). Why is there no groovy-all-minimal for 1.6?

        Show
        Dave Syer added a comment - Wait, no... there was another groovy on my classpath (groovy-all-minimal-1.5.6). Why is there no groovy-all-minimal for 1.6?
        Hide
        Dave Syer added a comment -

        (So it works for me in 1.6, as promised.)

        Show
        Dave Syer added a comment - (So it works for me in 1.6, as promised.)
        Hide
        Paul King added a comment -

        Great. I will do whatever merge is required for the 1_5_x branch and then set this issue to resolved.

        Why is there no groovy-all-minimal for 1.6?

        We phased that out. Old versions of Maven didn't handle the 'optional' Maven dependency element very well so the minimal artifact was a hack to help people out because it would never download any of the optional artifacts (because they were removed).

        Show
        Paul King added a comment - Great. I will do whatever merge is required for the 1_5_x branch and then set this issue to resolved. Why is there no groovy-all-minimal for 1.6? We phased that out. Old versions of Maven didn't handle the 'optional' Maven dependency element very well so the minimal artifact was a hack to help people out because it would never download any of the optional artifacts (because they were removed).
        Hide
        Paul King added a comment -

        merged to 1_5_x branch as well now

        Show
        Paul King added a comment - merged to 1_5_x branch as well now

          People

          • Assignee:
            Paul King
            Reporter:
            Dave Syer
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: