Module B "uses" grouping defined in module A behaivor is changed

[quantang]

Hi @saintkepha,

Here is another issue related to the namespace.

I have two module files, module A and module B.

Module A:

    grouping amf-node-attributes {
        description "AMF node attributes";
        container amf-node-attributes {
            leaf hostname {
                type string;
                description "The hostname of the AMF node";
            }
        }
    }

Module B:

    augment "/nw:networks/nw:network/nw:node" {
        uses A:amf-node-attributes;
    }

Before I upgrade to the latest yang-js, I can eval the data in this way:

                        "B:amf-node-attributes": {
                            "hostname": "abc"
                        }

But now, I found I need to set the data in this way:

                        "A:amf-node-attributes": {
                            "hostname": "abc"
                        }

Maybe I am wrong, but I guess Module B "uses" the grouping, should it be under B's namespace?

Cheers.

[quantang]

I had a check on the YANG schema RFC.

The identifiers defined in the grouping are not bound to a namespace until the contents of the grouping are added to the schema tree via a "uses" statement that does not appear inside a "grouping" statement, at which point they are bound to the namespace of the current module.

So I think the original behaviour should be right. Currently, I am looking at the code, but I am not confident that I can figure out the root cause. If you have any idea, please feel free to share.

Cheers.

[sekur]

Ok, I see what the issue is. The recent change compares the schema element's parent with schema element's origin to detect whether the element is externally augmented. Since in your case, you're augmenting nodes from module A using module B back into module A, the augmented nodes thinks they're still part of the same parent/origin namespace.

I'll take this one and make the appropriate changes to the augment extension transform function.

[sekur]

I've published 0.22.12 with a fix for this issue. I don't think this would fix issue #90 but please check. We may want to further tweak the element.clone to handle nested sub-statement clone to inherit parent origin instead.

[quantang]

I pulled your latest code, but the issue seems to remain there. I will debug further to check what is happening there.

[sekur]

I've added test case to module.coffee for augment where schema B augments schema A using grouping from schema A and it seems to properly re-scope the augmented nodes to schema B namespace. Let me know what you find.

https://github.com/corenova/yang-js/blob/c9c83c1851c054b1216b2953f36a0e26d77986fb/test/extension/module.coffee#L152

[sekur]

It's likely that issue #90 is still outstanding, so you may need to set as:

"B:amf-node-attributes": {
  "B:hostname": "abc"
}

[quantang]

Yes, I will try to update my change based on your latest change.

[sekur]

I've just pushed another package 0.22.13 that fixes #90.

[quantang]

Cool. It works right now. Terrific. Thanks a lot.

One small comment. The toJSON() function always returns the fully qualified property name. Do you think whether we could make it return contextual property name by options? :P

Cheers.

[sekur]

I've actually debated on this a bit. If you augment a container with an external namespace, it stands to reason that all sub-nodes of that container should be treated as external namespaced nodes. If that is the only augment, then it would be convenient to treat all sub-nodes via short-handed contextual names.

However, if yet another module then augments into that augmented container, then technically, they can use overlapping "contextual" property name that would then cause a conflict on which property gets to use the contextual name and which one retains the full namespaced name.

So... it seems to me that being more explicit in all toJSON cases would avoid potential conflicts and be correct all the time, although it makes post processing logic to be a bit more explicit (and retain correctness even when additional module composition occurs).

[sekur]

BTW, by allowing #90 to use contextual names, we potentially introduce a breaking/conflict scenario. I'll keep it as-is for now but just keep that in mind.

[quantang]

I think we can keep it as-is, as I can have a post-process to trim the redundant namespaces if the attributes share the same namespace as their parents. If we follow this approach, the full namespace naming should be fully compatible with the contextual one.

For the "augments issue" you mentioned above. Personally, I think it should be independent of how we name the properties, but be related to how the namespace should be resolved.

If my understanding is correct, the scenario would be:

module A {
  grouping Shape {
    container shape {
      leaf borderColor;
      leaf fillColor;
    }
  }
  grouping Size {
    container size {
      leaf width;
      leaf height;
    }
  }
  container canvas {
  }
}
module B {
  import A;
  augment "/A:canvas" {
    uses A:Shape;
  }
}
module C {
  import A;
  import B;
  augment "/A:canvas/B:shape" {
    uses A:Size;
  }
  augment "/A:canvas" {
    uses A:Shape;
  }
}

My understanding is, the first A:Shape was augmented by module B, so it should be B:shape. While the A:Size was augmented by module C, so it should be C:size. As the module C also augments the A:Shape again, it should also have a C:shape on /A:canvas. Eventually we should have:

/A:canvas
  /B:shape
    /B:borderColor
    /B:fillColor
    /C:size
      /C:width
      /C:height
  /C:shape
    /C:borderColor
    /C:fillColor

Convert to the contextual property name:

/A:canvas
  /B:shape
    /borderColor
    /fillColor
    /C:size
      /width
      /height
  /C:shape
    /borderColor
    /fillColor

Please feel correct me if I am wrong. Cheers.

[sekur]

Now, let's say we have module D:

module D {
  import A;
  import B;
  import C;
  augment "/A:canvas/B:shape" {
    leaf borderColor;
  }
}

We would need to deal with non-contextual property names for the borderColor leafs:

/A:canvas
  /B:shape
    /C:borderColor
    /D:borderColor

But whether the toJSON would have produced /borderColor or the two separate FQN depends on whether module D was loaded or not. Which basically means that the post-processing logic would need to be different in how it treats the borderColor property depending on whether module D is used or not. So any previous logic that simply assumed /borderColor would no longer work because another module was loaded into the system. It seems to me that this is a Bad Thing since YANG should provide consistent behavior irrespective of new modules being loaded into a given system.

Having said that, we can definitely enable toJSON logic to spit out contextual name when it detects that there are no other nodes that share the same schema.tag value. I suppose I just don't like allowing contextual names to be returned that can change and break post-processing logic...

[sekur]

I guess what you are suggesting is that we expose an option flag to the .toJSON that would allow the user to specify they prefer contextual names when possible but otherwise default to current behavior otherwise.

I'll need to update current toJSON method to accept option object instead of current parameter list (I should've done it that was originally...) but it may break other dependency modules that was using custom toJSON params. I suppose we can consider a 0.23 branch for that.

[quantang]

For the module D, I guess it will be:

/A:canvas
  /B:shape
    /B:borderColor
    /D:borderColor

The contextual style should be:

/A:canvas
  /B:shape
    /borderColor
    /D:borderColor

No matter whether module D is used or not, the /borderColor will only be interpreted to /B:borderColor. If the attribute belongs to a different namespace to its parent, the namespace cannot be omitted.

For the toJSON method, it is not urgent and we should keep it backwards compatible. So we can leave it for a while.

Really appreciate for your actively responds and fixing these days. I start to update my project to use the latest yang-js. :P

Cheers.

[sekur]

Ah right, foobar on my example. :-)

The latest yang-js has significant perf improvement during eval, especially with larger lists. BTW, have you taken a look at kos yet? Not sure if your project involves integration/interaction between web browser and server but if so, kos can be a powerful tool to automate state sync between browser app and the server instance.

[quantang]

Not yet. I will take a look at KOS and learn something new. :) Have a good night. Thanks a lot.