Add support for instance-identifier
My goal was at least to make basic R/O operation work with ietf-alarms.
Then I didn't have an easy way of testing this code, so I made it work
for editing as well.
Limitations:
- You can set an instance-identifier which points to an input node of an
RPC/action, which sounds fishy, but that could be a libyang bug.
- Non-config lists works funnily, which might point to a bug somewhere
in our handling of data paths:
/> set example-schema:iid-valid /example-schema:users/userList
libyang[0]: Unexpected XPath token "]" ("]"). (path: Data location "/example-schema:iid-valid".)
libyang[0]: Invalid instance-identifier "/example-schema:users/userList[]" value - syntax error. (path: Data location "/example-schema:iid-valid".)
The following errors occured:
Message: Invalid instance-identifier "/example-schema:users/userList[]" value - syntax error.
XPath: Data location "/example-schema:iid-valid".
- XPath says that any key value must be quoted, but the CLI reuses the
existing leaf_data_ parser for list keys (even prior to this patch),
which means that a list indexed by, say, an enum looks like:
/example-schema/list[val=666]
...which is not a valid XPath. A valid XPath would look like this:
/example-schema/list[val='666']
This new feature hits the same problem; when the instance-identifier
gets parsed from the user's input and converted to a string, the usual
netconf-cli rules are followed, and the list keys are not quoted
unless they are strings. This means that these values cannot be stored
or committed because libyang correctly rejects unquoted key values in,
say, instance-identifier values.
Quite frankly I'm not even sure how come that this has not been a
problem during normal operation, but perhaps it's only because we've
always tested with string-based lists and nothing else. Sure, we have
partial unit tests for all sorts of data types, but nothing actually
checks the full stack from the parser's input all the way to a
backend.
Path completion was a challenge for me to get right. At first I wanted
to use a different, fresh ParserContext to make sure that we do not
affect the main (leaf) path's state at all. That did not work well even
when I tried to copy the "relevant" memebrs of ParserContext back to the
original parser. But it turns out that simply reusing the existing
ParserContext works perfectly well. The only caveat is the concept of
"cwd", where a leaf below a certain container would "expect" paths that
are relative to the original cwd (which means "not root"). That's just
broken. The easiest fix is to always enforce an absolute path. The
easiest way of getting there was a yet another "parser mode" for
absolute-paths-only.
Yay for the immediately-invoked-lambda, because I really wanted to use
an constexpr if in there.
Change-Id: I68302ad854eab43aa6c42dcb2693b71ca3d2bd48
Depends-on: https://gerrit.cesnet.cz/c/CzechLight/dependencies/+/6445
diff --git a/src/ast_values.cpp b/src/ast_values.cpp
index da65d7c..a7aa5fe 100644
--- a/src/ast_values.cpp
+++ b/src/ast_values.cpp
@@ -26,6 +26,21 @@
{
}
+instanceIdentifier_::instanceIdentifier_(const std::string& xpath)
+ : m_xpath(xpath)
+{
+}
+
+bool instanceIdentifier_::operator==(const instanceIdentifier_& other) const
+{
+ return this->m_xpath == other.m_xpath;
+}
+
+bool instanceIdentifier_::operator<(const instanceIdentifier_& other) const
+{
+ return this->m_xpath < other.m_xpath;
+}
+
binary_::binary_() = default;
binary_::binary_(const std::string& value)
diff --git a/src/ast_values.hpp b/src/ast_values.hpp
index 42608f7..208028b 100644
--- a/src/ast_values.hpp
+++ b/src/ast_values.hpp
@@ -54,6 +54,13 @@
std::string m_value;
};
+struct instanceIdentifier_ {
+ instanceIdentifier_(const std::string& xpath);
+ bool operator==(const instanceIdentifier_& b) const;
+ bool operator<(const instanceIdentifier_& b) const;
+ std::string m_xpath;
+};
+
enum class SpecialValue {
List,
LeafList,
@@ -74,6 +81,7 @@
empty_,
bits_,
identityRef_,
+ instanceIdentifier_,
special_,
double,
bool,
diff --git a/src/leaf_data.hpp b/src/leaf_data.hpp
index 0e0e499..0a7e465 100644
--- a/src/leaf_data.hpp
+++ b/src/leaf_data.hpp
@@ -39,6 +39,9 @@
-module >> node_identifier;
template <typename It, typename Ctx, typename RCtx, typename Attr>
+bool leaf_data_parse_data_path(It& first, It last, Ctx const& ctx, RCtx& rctx, Attr& attr);
+
+template <typename It, typename Ctx, typename RCtx, typename Attr>
struct impl_LeafData {
It& first;
It last;
@@ -95,6 +98,10 @@
{
return leaf_data_string.parse(first, last, ctx, rctx, attr);
}
+ bool operator()(const yang::InstanceIdentifier&) const
+ {
+ return leaf_data_parse_data_path(first, last, ctx, rctx, attr);
+ }
bool operator()(const yang::Empty) const
{
return x3::attr(empty_{}).parse(first, last, ctx, rctx, attr);
diff --git a/src/leaf_data_type.cpp b/src/leaf_data_type.cpp
index f97e382..aa36b8e 100644
--- a/src/leaf_data_type.cpp
+++ b/src/leaf_data_type.cpp
@@ -109,4 +109,11 @@
{
return this->m_allowedValues == other.m_allowedValues;
}
+InstanceIdentifier::InstanceIdentifier()
+{
+}
+bool InstanceIdentifier::operator==(const InstanceIdentifier&) const
+{
+ return true;
+}
}
diff --git a/src/leaf_data_type.hpp b/src/leaf_data_type.hpp
index c3c30c9..f1619ec 100644
--- a/src/leaf_data_type.hpp
+++ b/src/leaf_data_type.hpp
@@ -71,6 +71,10 @@
std::set<std::string> m_allowedValues;
};
struct LeafRef;
+struct InstanceIdentifier {
+ InstanceIdentifier();
+ bool operator==(const InstanceIdentifier& other) const;
+};
struct Union;
using LeafDataType = std::variant<
yang::String,
@@ -90,6 +94,7 @@
yang::IdentityRef,
yang::Bits,
yang::LeafRef,
+ yang::InstanceIdentifier,
yang::Union
>;
struct TypeInfo;
diff --git a/src/libyang_utils.cpp b/src/libyang_utils.cpp
index 09676d6..8ff4d65 100644
--- a/src/libyang_utils.cpp
+++ b/src/libyang_utils.cpp
@@ -40,9 +40,9 @@
return dec.number * std::pow(10, -dec.digits);
}
- leaf_data_ operator()(const libyang::InstanceIdentifier&) const
+ leaf_data_ operator()(const libyang::InstanceIdentifier& node) const
{
- throw std::runtime_error("instance-identifier is not supported");
+ return instanceIdentifier_{node.path};
}
template <typename Type>
diff --git a/src/path_parser.hpp b/src/path_parser.hpp
index 67345c7..5a4cf9e 100644
--- a/src/path_parser.hpp
+++ b/src/path_parser.hpp
@@ -256,7 +256,8 @@
enum class PathParserMode {
AnyPath,
DataPath,
- DataPathListEnd
+ DataPathListEnd,
+ DataPathAbsolute
};
template <>
@@ -274,6 +275,11 @@
using type = dataPath_;
};
+template <>
+struct ModeToAttribute<PathParserMode::DataPathAbsolute> {
+ using type = dataPath_;
+};
+
auto const trailingSlash = x3::rule<struct trailingSlash_class, x3::unused_type>{"trailingSlash"} =
x3::omit['/'];
@@ -300,10 +306,22 @@
initializePath.parse(begin, end, ctx, rctx, x3::unused);
dataPath_ attrData;
+ auto res = [](){
+ if constexpr (PARSER_MODE == PathParserMode::DataPathAbsolute) {
+ return absoluteStart;
+ } else {
+ return -absoluteStart;
+ }
+ }
// absoluteStart has to be separate from the dataPath parser,
// otherwise, if the "dataNode % '/'" parser fails, the begin iterator
// gets reverted to before the starting slash.
- auto res = (-absoluteStart).parse(begin, end, ctx, rctx, attrData.m_scope);
+ ().parse(begin, end, ctx, rctx, attrData.m_scope);
+ if (!res) {
+ x3::get<parser_context_tag>(ctx).m_suggestions.emplace(Completion{"/"});
+ return res;
+ }
+
auto dataPath = x3::attr(attrData.m_scope)
>> (dataNode<COMPLETION_MODE>{m_filterFunction} % '/' | pathEnd >> x3::attr(std::vector<dataNode_>{}))
>> -trailingSlash;
@@ -362,6 +380,7 @@
auto const anyPath = x3::rule<struct anyPath_class, AnyPath>{"anyPath"} = PathParser<PathParserMode::AnyPath, CompletionMode::Schema>{};
auto const dataPath = x3::rule<struct dataPath_class, dataPath_>{"dataPath"} = PathParser<PathParserMode::DataPath, CompletionMode::Data>{};
auto const dataPathListEnd = x3::rule<struct dataPath_class, dataPath_>{"dataPath"} = PathParser<PathParserMode::DataPathListEnd, CompletionMode::Data>{};
+auto const dataPathAbsolute = x3::rule<struct dataPath_class, dataPath_>{"dataPathAbsolute"} = PathParser<PathParserMode::DataPathAbsolute, CompletionMode::Data>{};
#if __clang__
#pragma GCC diagnostic push
@@ -465,7 +484,21 @@
auto const leafListElementPath = x3::rule<leafListElementPath_class, dataPath_>{"leafListElementPath"} =
dataPath;
-
+template <typename It, typename Ctx, typename RCtx, typename Attr>
+bool leaf_data_parse_data_path(It& first, It last, Ctx const& ctx, RCtx& rctx, Attr& attr)
+{
+ dataPath_ path;
+ auto res = dataPathAbsolute.parse(first, last, ctx, rctx, path);
+ if (res) {
+ // FIXME: list key escaping in XPath is different from netconf-cli.
+ // If the key(s) are not all strings, this will produce netconf-cli-style
+ // string like /foo:xxx[k1=blah][k2=666] instead of standard XPaths like
+ // /foo:xxx[k1='blah'][k2='666'], and these will cause troubles when passed
+ // to libyang later on.
+ attr = instanceIdentifier_{pathToDataString(path, Prefixes::WhenNeeded)};
+ }
+ return res;
+}
#if __clang__
#pragma GCC diagnostic pop
diff --git a/src/utils.cpp b/src/utils.cpp
index afb291e..fd2e297 100644
--- a/src/utils.cpp
+++ b/src/utils.cpp
@@ -127,6 +127,10 @@
{
return "an empty leaf";
}
+ std::string operator()(const yang::InstanceIdentifier&)
+ {
+ return "an instance identifier";
+ }
std::string operator()(const yang::Union& type)
{
std::ostringstream ss;
@@ -211,6 +215,11 @@
return ss.str();
}
+ std::string operator()(const instanceIdentifier_& data) const
+ {
+ return data.m_xpath;
+ }
+
template <typename T>
std::string operator()(const T& data) const
{
diff --git a/src/yang_schema.cpp b/src/yang_schema.cpp
index 8e282f1..2f046c7 100644
--- a/src/yang_schema.cpp
+++ b/src/yang_schema.cpp
@@ -246,6 +246,10 @@
resType.emplace<yang::Union>(std::move(resUnion));
break;
}
+ case libyang::LeafBaseType::InstanceIdentifier: {
+ resType.emplace<yang::InstanceIdentifier>();
+ break;
+ }
default:
using namespace std::string_literals;
throw UnsupportedYangTypeException("the type of "s +