CESNET / libyang-cpp

C++ bindings for the libyang library
https://gerrit.cesnet.cz/q/project:CzechLight/libyang-cpp
BSD 3-Clause "New" or "Revised" License
10 stars 3 forks source link

[Question] parseData gives unexpected notification element #14

Closed safesintesi closed 11 months ago

safesintesi commented 11 months ago

We installed some modules on sysrepo containing the following snippet:

module my-types {
  identity APP-TYPES {
    description "Application types.";
  }

  identity CLIENT {
    base APP-TYPES;
  }

  identity INTERNAL {
    base APP-TYPES;
  }

  typedef app-types {
    type identityref {
      base APP-TYPES;
    }
  }
}

module my-module{
  ...
  import my-types { prefix "my-types"; }

  notification application_new {
    container application {
      leaf app_type {
        type my-types:app-types;
      }
    }
  }
}

during some tests we tried to create a notification from a json file

std::string json(R"({
  "my-module:application_new": {
    "application": {
      "app_type": "my-types:CLIENT"
    }
  }
}
)");
    auto node = session.getContext().parseData(json, libyang::DataFormat::JSON);

This leads to a crash with the following error:

libyang[0]: Unexpected notification element "application_new". (path: Schema location "/my-module:application_new", line number 2.)
FAIL!  : TestAgentQt::TestBuildNotifString() Caught unhandled exception

Using newPath()/newPath2() to create the notification node by node works, but receiving the data as json objects would be easier to parse them.

Am I missing something?

jktjkt commented 11 months ago

These have to be parsed in a special way, via libyang's lyd_parse_op(). We have a wrapper for this (Context::parseOp and DataNode::parseOp), but so far we've only wrapped these for XML-serialized RPCs and their replies. Let me look into this.

jktjkt commented 11 months ago

Can you try this patch? https://gerrit.cesnet.cz/c/CzechLight/libyang-cpp/+/6416

safesintesi commented 11 months ago

Thank you for your fast reply.

I downloaded and applied the patch but seems it doesn't build. The problems seems to be that OperationType::NotificationRestconf called at line 188 of Context.cpp is missing form OperationType enum class in Enum.hpp at line 52.

Added this I could build the repo and use it.

I still get

libyang[0]: Invalid argument parent (lyd_parse_op_()).

but it might be a bad format of the notification by my part, I'm currently checking it.

I need a clarification though, if I want to send the notification with sysrepo on session.sendNotificaton() a DataNode is required. Is this the tree or the op field of the returned ParsedOp?

jktjkt commented 11 months ago

Oops, it's actually a series of patches, you'll need:

Nice timing, i pushed these just a minute ago :). Can you please check the doxygen comments in these patches and report back if it's all clear now? You can find some extra information and usage patterns in the test suite, but please let me know if the docs can be made any clearer.

jktjkt commented 11 months ago

Also, the error path is probably wrong, there's very likely a memory leak. But I need CESNET/libyang#2077 fixed first to actually test the exception handling. In the meanwhile, I would not run this on untrusted input :).

safesintesi commented 11 months ago

Thank you for your time, with the new patches all our tests pass now. The documentation seems perfectly clear. You just gained a new follower. :)

safesintesi commented 11 months ago

Quick note.

Given the following notifications:

{
    "ietf-restconf:notification" : {
        "test:eventTime" : "2013-12-21T00:01:00Z",
        "test:application_new": {
            "application": {
                "app_type": "my-types:CLIENT"
            }
        }
    }
}
{
    "ietf-restconf:notification" : {
        "test:application_new": {
            "application": {
                "app_type": "my-types:CLIENT"
            }
        },
        "test:eventTime" : "2013-12-21T00:01:00Z"
    }
}

the first gets parsed while the second throws an exception. Since the JSON represents the same data I would expect that they would be both parsed without problems.

Is this a bug or a feature? Is there some kind of parameter order enforcement that I'm not aware of?

jktjkt commented 11 months ago

Your example has test:eventTime instead of the eventTime (which is a sohrthand for ietf-restconf:eventTime) -- but apparently that doesn't matter. Anyway, this is not a bug in the C++ bindings, it's something form the C library. I filed this at CESNET/libyang#2080.