Element toXMLString() fails to output namespace correctly #21

liggetm · 2013-07-17T13:16:25Z

Guys - I've come across an issue where a valid netconf XML configuration is output (via toXMLString) incorrectly.

The yang model contains a simple leaf-list augmentation to ietf-system but the augmentation is defined in a different namespace. The toXMLString output is producing XML with the next element (in this case dns) shown as the wrong namespace (as well as repeating this incorrect namespace value once for every instance of the augmentation).

Yang Augmentation:

    augment "/ietf-sys:system/ietf-sys:ntp" {
         /*
          * Alternate (simpler) NTP model.  This replaces the standard one in
          * the ietf model and follows the same style as the DNS servers.
          */
         leaf-list server {
            type inet:ip-address;
            ordered-by user;
            description
               "Addresses of the NTP servers to use for system clock
                synchronization.

                Implementations MAY limit the number of entries in this
                leaf list.";
         }
    }

Sample netconf XML input sent to YangXMLParser (output from toXMLString expected to be equivalent):

    <system xmlns="urn:ietf:params:xml:ns:yang:ietf-system">
    <ntp>
      <server xmlns="http://domain.com/ns/test">1.1.1.1</server>
      <server xmlns="http://domain.com/ns/test">2.2.2.2</server>
      <server xmlns="http://domain.com/ns/test">3.3.3.3</server>
    </ntp>
    <dns>
      <server>172.16.1.1</server>
    </dns>
    <radius>
      <server>
        <address>172.16.1.2</address>
        <shared-secret>password</shared-secret>
      </server>
    </radius>
    </system>

Actual Element.toXMLString() output:

<system xmlns="urn:ietf:params:xml:ns:yang:ietf-system">
  <ntp>
    <unknown:server>1.1.1.1</unknown:server>
    <unknown:server>2.2.2.2</unknown:server>
    <unknown:server>3.3.3.3</unknown:server>
  </ntp>
  <dns xmlns="http://domain.com/ns/test" xmlns="http://domain.com/ns/test" xmlns="http://domain.com/ns/test">
    <server>172.16.1.1</server>
  </dns>
  <radius>
    <server>
      <address>172.16.1.2</address>
      <shared-secret>password</shared-secret>
    </server>
  </radius>
</system>

Analysis of the elements before printing using toXMLString() - this shows the namespace of each of the children appear to be correct, leading me to the assumption its a defect in toXMLString():

2013-07-16 12:46:08 >>> Element name system --- Element namespace urn:ietf:params:xml:ns:yang:ietf-system
2013-07-16 12:46:08 >>> Element is of class type: gen.ietfSystem.System 

2013-07-16 12:46:08 >>>>>> Element name ntp --- Element namespace urn:ietf:params:xml:ns:yang:ietf-system
2013-07-16 12:46:08 >>>>>> Element is of class type: gen.ietfSystem.system.Ntp 

2013-07-16 12:46:08 >>>>>>>>> Element name server --- Element namespace http://domain.com/ns/test
2013-07-16 12:46:08 >>>>>>>>> Element is of class type: com.tailf.jnc.Leaf 

2013-07-16 12:46:08 >>>>>>>>> Element name server --- Element namespace http://domain.com/ns/test
2013-07-16 12:46:08 >>>>>>>>> Element is of class type: com.tailf.jnc.Leaf 

2013-07-16 12:46:08 >>>>>>>>> Element name server --- Element namespace http://domain.com/ns/test
2013-07-16 12:46:08 >>>>>>>>> Element is of class type: com.tailf.jnc.Leaf 

2013-07-16 12:46:08 >>>>>> Element name dns --- Element namespace urn:ietf:params:xml:ns:yang:ietf-system
2013-07-16 12:46:08 >>>>>> Element is of class type: gen.ietfSystem.system.Dns 

2013-07-16 12:46:08 >>>>>>>>> Element name server --- Element namespace urn:ietf:params:xml:ns:yang:ietf-system
2013-07-16 12:46:08 >>>>>>>>> Element is of class type: com.tailf.jnc.Leaf 

2013-07-16 12:46:08 >>>>>> Element name radius --- Element namespace urn:ietf:params:xml:ns:yang:ietf-system
2013-07-16 12:46:08 >>>>>> Element is of class type: gen.ietfSystem.system.Radius 

2013-07-16 12:46:08 >>>>>>>>> Element name server --- Element namespace urn:ietf:params:xml:ns:yang:ietf-system
2013-07-16 12:46:08 >>>>>>>>> Element is of class type: gen.ietfSystem.system.radius.Server

Please come back to me if you need the source, I've written a simple test harness I'm happy to share. Thanks, Mark

The text was updated successfully, but these errors were encountered:

Emil-Tail-f · 2013-07-17T14:15:29Z

Both me and Daniel are on vacation now, so this is not likely to be dealt with within the next two weeks. If you wish to speed up things you can make a pull request with a failing test verifying this bug.

liggetm · 2013-07-31T15:21:03Z

I had a look through jnc.py and can see that _constructor_template is responsible for setting the prefix in the class constructor, but it only does this when is_top_level is true. I'm looking at putting in a hack that sets the prefix if it's an augmented module - just have to figure out how :)

alrighttheresham · 2013-08-01T18:44:53Z

Hi Emil, has you or Daniel had a chance to look at this?

XDanz · 2013-08-01T19:31:30Z

On My vacation Will be back on monday.

Skickat från min iPhone

1 aug 2013 kl. 20:44 skrev alrighttheresham notifications@github.com:

Hi Emil, has you or Daniel had a chance to look at this?

—
Reply to this email directly or view it on GitHub.

alrighttheresham · 2013-08-01T19:32:40Z

Thanks Daniel.

Emil-Tail-f · 2013-08-08T07:47:47Z

The Element class resolves the prefix (xmlns attribute of a child node with prefixes == null) by searching for a parent node that has a prefix with the same namespace. Augmented leaves have no such parent and there is no matching prefix in the defaultPrefixes PrefixMap either, so the prefix defaults to "unknown" in the qualifiedName method. There is of course no such namespace, so the parser will fail.

My proposed solution is to use the setDefaultPrefix method on the augmented leaves before parsing, which adds a default prefix with the augmented namespace. In your test:

final Leaf leafWithNamespace = new Leaf("http://testNamespace", "leafWithNamespace");
leafWithNamespace.setDefaultPrefix();
leafWithNamespace.setValue("leafValue");
...

Then the output of toXMLString is like expectedXml (except for whitespace differences). I'll merge a modified version of the pull request shortly, with a passing test. If this doesn't solve your problem, please write a new failing test where the setDefaultPrefix doesn't do the trick.

This adds a default prefix with the augmented namespace, allowing the parser to succeed in finding the proper prefix. Tied to Issue #21

Emil-Tail-f · 2013-08-08T08:16:36Z

I'm currently looking at fixing the _constructor_template method in jnc.py so that setDefaultPrefix and setPrefix methods are called for augmented leaves as well in generated classes.

alrighttheresham · 2013-08-08T08:23:31Z

Thanks Emil, appreciate your efforts to turn this around quickly.

Damian.

On 8 Aug 2013, at 09:16, Emil Wall notifications@github.com wrote:

I'm currently looking at fixing the _constructor_template method in jnc.py so that setDefaultPrefix and setPrefix methods are called for augmented leaves as well in generated classes.

—
Reply to this email directly or view it on GitHub.

Emil-Tail-f · 2013-08-08T09:49:56Z

I have an idea that you could check if a statement is augmented either by checking if its corresponding module statement is in the augmented_modules dict, or if its parent has another namespace associated with it. Then you could have an is_augmented property that you set in the init method of MethodGenerator and use that in the _constructor_template method to determine whether or not to set the prefix.

However, when I constructed an example to see what happened when a leaf-list is augmented into a container, I was unable to find the need for such a change. The classes were generated and I was able to fetch a model-aware configuration from a confd server. Therefore I have not commited any changes to jnc.py. Please modify the test example (see tests/12-test-augment, fixed by commit 95c27be) to reproduce your problems if there are any.

Note that there is a name collision with System, so the generated class can't be imported.

Emil-Tail-f · 2013-08-08T10:37:51Z

I changed the augmenting module so it augments the container with a list instead of a leaf-list. Then it makes more sense to set the prefix in the constructor. Perhaps the default prefix should not change for augmented nodes, I'm not entirely sure how this is supposed to work to be honest.

Please check and see if you agree with these changes.

Emil-Tail-f · 2013-08-08T10:47:52Z

The code could be written as

if self.is_top_level or self.is_augmented:
    constructor.add_line('setDefaultPrefix();')
if self.is_top_level:
    setPrefix = ['setPrefix(', self.root, '.PREFIX);']
    constructor.add_line(''.join(setPrefix))

if we only want the default prefix to change, or

if self.is_top_level:
    constructor.add_line('setDefaultPrefix();')
if self.is_top_level or self.is_augmented:
    setPrefix = ['setPrefix(', self.root, '.PREFIX);']
    constructor.add_line(''.join(setPrefix))

if we don't want it to change.

liggetm · 2013-08-09T17:07:02Z

Hi Emil - sorry for the delay. I modified the yang XML source to ensure the augmented module was a list instead of a leaf-list. I regenerated our jar and it all looks good, including some unit tests I'd ignored in the build. I've passed the updated yang to the team that deals with that to see if it causes them any issue otherwise I think we can close this issue.

liggetm · 2013-08-12T14:46:51Z

Hi Emil - I've spoken to the yang team and they've told me that moving from a leaf-list to a list is not going to be possible. The reason given, is that, confd responds with different behavior for each list and it's non-trivial to update. Obviously we don't want to manage different models, so is it possible that we can do something with the .is_augmented method discussed above?

liggetm · 2013-08-16T14:59:35Z

Hi Emil - I'm starting to come under pressure because of this issue. Using the workaround above for the leaf-list augmentation got me passed the Ntp server issue however we've another instance of this issue, again in an augmentation, only this time in a leaf value. The leaf is an augmentation to ietf-sys:system and is of type boolean.

leaf auto-prov {
    type boolean;
    tailf:cli-full-command;
    description
        "If set to true, the system will auto-provision equipment
        modules when inserted into the chassis.";
    default true;
}

The toXMLString (we use to store in a database) is giving us:

<unknown:auto-prov>false</unknown:auto-prov>

Any suggestions welcome. Thanks.

XDanz · 2013-08-16T15:22:08Z

Hi !
I will be try to assist you and Emil on this issue and will be working on
this full time.
But i am quite new to this library so it will take some time hope i could
help here.

/Daniel

2013/8/16 liggetm notifications@github.com

Hi Emil - I'm starting to come under pressure because of this issue. Using
the workaround above for the leaf-list augmentation got me passed the Ntp
server issue however we've another instance of this issue, again in an
augmentation, only this time in a leaf value. The leaf is an augmentation
to ietf-sys:system and is of type boolean.

leaf auto-prov {
type boolean;
tailf:cli-full-command;
description
"If set to true, the system will auto-provision equipment
modules when inserted into the chassis.";
default true;
}

The toXMLString (we use to store in a database) is giving us:

unknown:auto-provfalse/unknown:auto-prov

Any suggestions welcome. Thanks.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/21#issuecomment-22771560
.

Emil-Tail-f added a commit that referenced this issue Aug 8, 2013

Use the setDefaultPrefix method on the augmented leaves before parsing

8324817

This adds a default prefix with the augmented namespace, allowing the parser to succeed in finding the proper prefix. Tied to Issue #21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Element toXMLString() fails to output namespace correctly #21

Element toXMLString() fails to output namespace correctly #21

liggetm commented Jul 17, 2013

Emil-Tail-f commented Jul 17, 2013

liggetm commented Jul 31, 2013

alrighttheresham commented Aug 1, 2013

XDanz commented Aug 1, 2013

alrighttheresham commented Aug 1, 2013

Emil-Tail-f commented Aug 8, 2013

Emil-Tail-f commented Aug 8, 2013

alrighttheresham commented Aug 8, 2013

Emil-Tail-f commented Aug 8, 2013

Emil-Tail-f commented Aug 8, 2013

Emil-Tail-f commented Aug 8, 2013

liggetm commented Aug 9, 2013

liggetm commented Aug 12, 2013

liggetm commented Aug 16, 2013

XDanz commented Aug 16, 2013

Element toXMLString() fails to output namespace correctly #21

Element toXMLString() fails to output namespace correctly #21

Comments

liggetm commented Jul 17, 2013

Emil-Tail-f commented Jul 17, 2013

liggetm commented Jul 31, 2013

alrighttheresham commented Aug 1, 2013

XDanz commented Aug 1, 2013

alrighttheresham commented Aug 1, 2013

Emil-Tail-f commented Aug 8, 2013

Emil-Tail-f commented Aug 8, 2013

alrighttheresham commented Aug 8, 2013

Emil-Tail-f commented Aug 8, 2013

Emil-Tail-f commented Aug 8, 2013

Emil-Tail-f commented Aug 8, 2013

liggetm commented Aug 9, 2013

liggetm commented Aug 12, 2013

liggetm commented Aug 16, 2013

XDanz commented Aug 16, 2013