Same name sibling for sequencers

[tamer_sk]

Hi,

I am currently making use of the java sequencer. I noticed if I have overloaded methodes that have the same parameter names but different types will cause an error when trying to sequence. Here is the error I get when trying to sequence (org/modeshape/common/text/TokenStream.java)

10:28:58,221 ERROR [org.modeshape.sequencer.javafile.JavaFileSequencer] (modeshape-sequencer-8-thread-2) Error sequencing file: javax.jcr.ItemExistsException: A node definition that allows same name siblings could not be found for the node "/TokenStream.java/TokenStream.java/org/modeshape/common/text/TokenStream/class:methods/consume(expected)[2]" in workspace "default"

  at org.modeshape.jcr.AbstractJcrNode.validateChildNodeDefinition(AbstractJcrNode.java:1207) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

  at org.modeshape.jcr.AbstractJcrNode.addChildNode(AbstractJcrNode.java:1045) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

  at org.modeshape.jcr.AbstractJcrNode.addNode(AbstractJcrNode.java:1001) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

  at org.modeshape.jcr.AbstractJcrNode.addNode(AbstractJcrNode.java:931) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

  at org.modeshape.jcr.AbstractJcrNode.addNode(AbstractJcrNode.java:107) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

  at org.modeshape.sequencer.javafile.ClassSourceFileRecorder.writeMethods(ClassSourceFileRecorder.java:221)

  at org.modeshape.sequencer.javafile.ClassSourceFileRecorder.writeClassMetadata(ClassSourceFileRecorder.java:80)

  at org.modeshape.sequencer.javafile.ClassSourceFileRecorder.record(ClassSourceFileRecorder.java:48)

  at org.modeshape.sequencer.javafile.JavaFileSequencer.execute(JavaFileSequencer.java:67)

  at org.modeshape.jcr.SequencingRunner.run(SequencingRunner.java:224) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

  at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:895) [rt.jar:1.6.0_45]

  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:918) [rt.jar:1.6.0_45]

  at java.lang.Thread.run(Thread.java:662) [rt.jar:1.6.0_45]

I checked the defenition for node type [class:methods] and it is

[class:methods]

+ * (class:method) = class:method

I am guessing this is not allowing same name sibling nodes. I ended up overriding it as in the following:

[class:methods]

+ * (class:method) = class:method sns

With the modification, I get the expected results. My questions

1- Is [class:methods] meant to not allow sns or is it a bug?

2- If it is not meant to allow sns, would overriding be the proper solution or is there a better solution?

Thanks,

Tamer

[rhauch]

Yes, adding "sns" should fix the problem. Can you log an issue and we'll fix the node definition in the codebase. In the meantime, please continue to use your modified definition. (That's all that we'll do on our side.)

Thanks!

[tamer_sk]

Hi Randall,

Thanks you very much as usual for the quick response. I created the following issue

https://issues.jboss.org/browse/MODE-1937

Thanks,

Tamer

[rhauch]

Actually, I'm having a difficult time replicating this problem. Each of the nodes that represents a constructor has a node name that contains the method name, open parenthesis, the comma-separated parameter types, and a close parenthesis.

For example, given two constructors like the following:

    public MyClass(int p1, double p2, Object o) {
    }

    public MyClass(int p1, double p2, float p3, Object o) {
    }

the resulting node names should be (in order):

• MyClass(int,double,Object)
• MyClass(int,double,float,Object)

Notice how no two constructor nodes will ever have the same names (since a class cannot have two constructors with the same signature).

Unfortunately, the method names do not follow this same pattern. Instead of including the parameter *types*, they instead include the parameter *names*. Thus, you can have methods like this:

    public void doSomething(double p1) {
    }

    public void doSomething(float p1) {
    }

and the resulting node names would be (in order):

• doSomething(p1)
• doSomething(p1)

These are obviously invalid with the unmodified CND.

So there are two ways to fix this problem:

1. Change the CND so that the method nodes can have the same names, but leave the current inconsistency in the method/constructor naming patterns, or
2. Leave the CND as is (without adding a 'sns' attribute), but correct the inconsistency so that methods and constructors use the same naming pattern that uses the parameter types rather than parameter names. This option changes the method naming pattern to reflect the method signature.

I think it's not useful best and very problematic at worst to have node names for methods that include the parameter names, since (as in the example above) these node names are duplicated and ambiguous, making it difficult for client applications to actually use the node structures. For example, most Java IDEs will show an outline that includes the *signature* of the method. Therefore, I think we should choose option 2 and correct the name pattern for method nodes.

This means, however, that changing the node type to add the 'sns' attribute (as I recommended yesterday) would fix the problem but not in the same way that we'll fix it in 3.3. What do you think?

[tamer_sk]

Hi Randall,

I do get the same error if have constructors

public MyClass(int i){

}

public MyClass(String i){

}

Without sns, it tries creating:

• MyClass(i)
• MyClass(i)[2]

It is still using the pramater name not type for constructors. Here is the error I got when testing the conustructors

08:50:18,104 ERROR [org.modeshape.sequencer.javafile.JavaFileSequencer] (modeshape-sequencer-8-thread-1) Error sequencing file: javax.jcr.ItemExistsException: A node definition that allows same name siblings could not be found for the node "/MemberListProducer.java/MemberListProducer.java/com/modeshaperest/resteasytesting/data/MemberListProducer/class:constructors/MemberListProducer(i)[2]" in workspace "default"

          at org.modeshape.jcr.AbstractJcrNode.validateChildNodeDefinition(AbstractJcrNode.java:1207) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

          at org.modeshape.jcr.AbstractJcrNode.addChildNode(AbstractJcrNode.java:1045) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

          at org.modeshape.jcr.AbstractJcrNode.addNode(AbstractJcrNode.java:1001) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

          at org.modeshape.jcr.AbstractJcrNode.addNode(AbstractJcrNode.java:931) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

          at org.modeshape.jcr.AbstractJcrNode.addNode(AbstractJcrNode.java:107) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

          at org.modeshape.sequencer.javafile.ClassSourceFileRecorder.writeMethods(ClassSourceFileRecorder.java:221)

          at org.modeshape.sequencer.javafile.ClassSourceFileRecorder.writeClassMetadata(ClassSourceFileRecorder.java:77)

          at org.modeshape.sequencer.javafile.ClassSourceFileRecorder.record(ClassSourceFileRecorder.java:48)

          at org.modeshape.sequencer.javafile.JavaFileSequencer.execute(JavaFileSequencer.java:67)

          at org.modeshape.jcr.SequencingRunner.run(SequencingRunner.java:224) [modeshape-jcr-3.2.0.Final.jar:3.2.0.Final]

          at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:895) [rt.jar:1.6.0_45]

          at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:918) [rt.jar:1.6.0_45]

          at java.lang.Thread.run(Thread.java:662) [rt.jar:1.6.0_45]

My constructors' in this case are

MemberListProducer(int i) and MemberListProducer(String i). It is trying to create MemberListProducer(i) and MemberListProducer(i)[2] which fails without sns. 

Note: I agree with you still on option 2 as I think it will add clarity and makes it easir for looking up the types. We would be able to tell from the node name what types of pramaters we have rather than digging deeper in the nodes to figure it out. The only drawback with this approch is with the current node structure, we would not be able to tell the pramater names at all unless it is added as part of the [class:method] type. It currently has the [class:parameters] which again stores the types. What do you think?

Hope this helps

Thanks,

Tamer

[rhauch]

You're correct. My earlier post mis-identified the format for the constructors; they, too, were using the parameter names in the node name. The pull-request corrects both.

Note: I agree with you still on option 2 as I think it will add clarity and makes it easir for looking up the types. We would be able to tell from the node name what types of pramaters we have rather than digging deeper in the nodes to figure it out. The only drawback with this approch is with the current node structure, we would not be able to tell the pramater names at all unless it is added as part of the [class:method] type. It currently has the [class:parameters] which again stores the types. What do you think?

Good catch. I think the best way to fix this is to introduce child nodes under the method for the parameters, where they can be given names, types, and even modifiers (e.g., final) and annotations (e.g., JAX-RS annotations. (I'd probably leave the [class:parameters] property in the CND with a comment that it is no longer used, just so that anyone with existing content doesn't get validation errors.)

What do you think?

[tamer_sk]

I definitely agree. Thank you very much for your time on this

Thanks,

Tamer

[rhauch]

I created a pull-request with my proposed changes. See the comments on MODE-1937 for a few additional details.