Skip to content

Commit f958af4

Browse files
jktjktjktjkt
committed
API/ABI change: opaque node naming
Our C++ API would ignore the "module name or XML prefix", which turns out to be *the* relevant part when it comes to opaque node naming. The prefix is, instead, just that string that might have been inherited from the parent node when parsing the serialized data; it's an optional thingy which, if not set explicitly, is implicitly inherited. Adapt the API for this, and since this *will* break the build, let's bump the package version. Change-Id: I199afe5fa7a571034b744531c63b93b9c656563a
1 parent f050e7e commit f958af4

File tree

6 files changed

+82
-25
lines changed

6 files changed

+82
-25
lines changed

‎CMakeLists.txt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,15 @@ add_custom_target(libyang-cpp-version-cmake
2121
cmake/ProjectGitVersionRunner.cmake
2222
)
2323
include(cmake/ProjectGitVersion.cmake)
24-
set(LIBYANG_CPP_PKG_VERSION "3")
24+
set(LIBYANG_CPP_PKG_VERSION "4")
2525
prepare_git_version(LIBYANG_CPP_VERSION ${LIBYANG_CPP_PKG_VERSION})
2626

2727
find_package(Doxygen)
2828
option(WITH_DOCS "Create and install internal documentation (needs Doxygen)" ${DOXYGEN_FOUND})
2929
option(BUILD_SHARED_LIBS "By default, shared libs are enabled. Turn off for a static build." ON)
3030

3131
find_package(PkgConfig REQUIRED)
32-
# FIXME: it's actually 3.7.12, but that hasn't been released yet
33-
pkg_check_modules(LIBYANG REQUIRED libyang>=3.7.11 IMPORTED_TARGET)
32+
pkg_check_modules(LIBYANG REQUIRED libyang>=3.10.1 IMPORTED_TARGET)
3433

3534
# FIXME from gcc 14.1 on we should be able to use the calendar/time from libstdc++ and thus remove the date dependency
3635
find_package(date)

‎include/libyang-cpp/Context.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ class LIBYANG_CPP_EXPORT Context {
115115
CreatedNodes newPath2(const std::string& path, libyang::JSON json, const std::optional<CreationOptions> options = std::nullopt) const;
116116
CreatedNodes newPath2(const std::string& path, libyang::XML xml, const std::optional<CreationOptions> options = std::nullopt) const;
117117
std::optional<DataNode> newExtPath(const ExtensionInstance& ext, const std::string& path, const std::optional<std::string>& value, const std::optional<CreationOptions> options = std::nullopt) const;
118-
std::optional<DataNode> newOpaqueJSON(const std::string& moduleName, const std::string& name, const std::optional<libyang::JSON>& value) const;
119-
std::optional<DataNode> newOpaqueXML(const std::string& moduleName, const std::string& name, const std::optional<libyang::XML>& value) const;
118+
std::optional<DataNode> newOpaqueJSON(const OpaqueName& name, const std::optional<libyang::JSON>& value) const;
119+
std::optional<DataNode> newOpaqueXML(const OpaqueName& name, const std::optional<libyang::XML>& value) const;
120120
SchemaNode findPath(const std::string& dataPath, const InputOutputNodes inputOutputNodes = InputOutputNodes::Input) const;
121121
Set<SchemaNode> findXPath(const std::string& path) const;
122122

‎include/libyang-cpp/DataNode.hpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,15 +225,22 @@ class LIBYANG_CPP_EXPORT DataNodeTerm : public DataNode {
225225
};
226226

227227
/**
228-
* @brief Contains a (possibly module-qualified) name of an opaque node.
228+
* @brief Contains a name of an opaque node.
229229
*
230-
* This is generic container of a prefix/module string and a name string.
230+
* An opaque node always has a name, and a module (or XML namespace) to which this node belongs.
231+
* Sometimes, it also has a prefix.
232+
*
233+
* If the prefix is set *and* the underlying node is an opaque JSON one, then the prefix must be the same as the "module or namespace" name.
234+
* If the underlying node is an opaque XML one, then the XML prefix might be something completely different, and in that case the real fun begins.
235+
* Review the libayng C manual, this is something that the C++ wrapper doesn't really have under control.
231236
*
232237
* Wraps `ly_opaq_name`.
233238
*/
234239
struct LIBYANG_CPP_EXPORT OpaqueName {
240+
std::string moduleOrNamespace;
235241
std::optional<std::string> prefix;
236242
std::string name;
243+
std::string pretty() const;
237244
};
238245

239246
/**

‎src/Context.cpp

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -378,17 +378,25 @@ std::optional<DataNode> Context::newExtPath(const ExtensionInstance& ext, const
378378
*
379379
* Wraps `lyd_new_opaq`.
380380
*
381-
* @param moduleName Node module name, used as a prefix as well
382381
* @param name Name of the created node
383382
* @param value JSON data blob, if any
384383
* @return Returns the newly created node (if created)
385384
*/
386-
std::optional<DataNode> Context::newOpaqueJSON(const std::string& moduleName, const std::string& name, const std::optional<libyang::JSON>& value) const
385+
std::optional<DataNode> Context::newOpaqueJSON(const OpaqueName& name, const std::optional<libyang::JSON>& value) const
387386
{
387+
if (name.prefix && *name.prefix != name.moduleOrNamespace) {
388+
throw Error{"invalid opaque JSON node: prefix \"" + *name.prefix + "\" doesn't match module name \"" + name.moduleOrNamespace + "\""};
389+
}
388390
lyd_node* out;
389-
auto err = lyd_new_opaq(nullptr, m_ctx.get(), name.c_str(), value ? value->content.c_str() : nullptr, nullptr, moduleName.c_str(), &out);
391+
auto err = lyd_new_opaq(nullptr,
392+
m_ctx.get(),
393+
name.name.c_str(),
394+
value ? value->content.c_str() : nullptr,
395+
name.prefix ? name.prefix->c_str() : nullptr,
396+
name.moduleOrNamespace.c_str(),
397+
&out);
390398

391-
throwIfError(err, "Couldn't create an opaque JSON node '"s + moduleName + ':' + name + "'");
399+
throwIfError(err, "Couldn't create an opaque JSON node " + name.pretty());
392400

393401
if (out) {
394402
return DataNode{out, std::make_shared<internal_refcount>(m_ctx)};
@@ -403,17 +411,23 @@ std::optional<DataNode> Context::newOpaqueJSON(const std::string& moduleName, co
403411
*
404412
* Wraps `lyd_new_opaq2`.
405413
*
406-
* @param xmlNamespace Node module namespace
407414
* @param name Name of the created node
408415
* @param value XML data blob, if any
409416
* @return Returns the newly created node (if created)
410417
*/
411-
std::optional<DataNode> Context::newOpaqueXML(const std::string& xmlNamespace, const std::string& name, const std::optional<libyang::XML>& value) const
418+
std::optional<DataNode> Context::newOpaqueXML(const OpaqueName& name, const std::optional<libyang::XML>& value) const
412419
{
420+
// the XML node naming is "complex", we cannot really check the XML namespace for sanity here
413421
lyd_node* out;
414-
auto err = lyd_new_opaq2(nullptr, m_ctx.get(), name.c_str(), value ? value->content.c_str() : nullptr, nullptr, xmlNamespace.c_str(), &out);
415-
416-
throwIfError(err, "Couldn't create an opaque XML node '"s + name +"' from namespace '" + xmlNamespace + "'");
422+
auto err = lyd_new_opaq2(nullptr,
423+
m_ctx.get(),
424+
name.name.c_str(),
425+
value ? value->content.c_str() : nullptr,
426+
name.prefix ? name.prefix->c_str() : nullptr,
427+
name.moduleOrNamespace.c_str(),
428+
&out);
429+
430+
throwIfError(err, "Couldn't create an opaque XML node " + name.pretty());
417431

418432
if (out) {
419433
return DataNode{out, std::make_shared<internal_refcount>(m_ctx)};

‎src/DataNode.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,16 +1112,29 @@ OpaqueName DataNodeOpaque::name() const
11121112
{
11131113
auto opaq = reinterpret_cast<lyd_node_opaq*>(m_node);
11141114
return OpaqueName{
1115-
.prefix = opaq->name.prefix ? std::optional(opaq->name.prefix) : std::nullopt,
1116-
.name = opaq->name.name
1117-
};
1115+
.moduleOrNamespace = opaq->name.module_name,
1116+
.prefix = opaq->name.prefix ? std::optional{opaq->name.prefix} : std::nullopt,
1117+
.name = opaq->name.name};
11181118
}
11191119

11201120
std::string DataNodeOpaque::value() const
11211121
{
11221122
return reinterpret_cast<lyd_node_opaq*>(m_node)->value;
11231123
}
11241124

1125+
std::string OpaqueName::pretty() const
1126+
{
1127+
if (prefix) {
1128+
if (*prefix == moduleOrNamespace) {
1129+
return *prefix + ':' + name;
1130+
} else {
1131+
return "{" + moduleOrNamespace + "}, " + *prefix + ':' + name;
1132+
}
1133+
} else {
1134+
return "{" + moduleOrNamespace + "}, " + name;
1135+
}
1136+
}
1137+
11251138
/**
11261139
* Wraps a raw non-null lyd_node pointer.
11271140
* @param node The pointer to be wrapped. Must not be null.

‎tests/data_node.cpp

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1969,9 +1969,11 @@ TEST_CASE("Data Node manipulation")
19691969

19701970
DOCTEST_SUBCASE("opaque nodes for sysrepo ops data discard")
19711971
{
1972-
auto discard1 = ctx.newOpaqueJSON("sysrepo", "discard-items", libyang::JSON{"/example-schema:a"});
1972+
// the "short" form with no prefix
1973+
auto discard1 = ctx.newOpaqueJSON(libyang::OpaqueName{"sysrepo", std::nullopt, "discard-items"}, libyang::JSON{"/example-schema:a"});
19731974
REQUIRE(!!discard1);
1974-
auto discard2 = ctx.newOpaqueJSON("sysrepo", "discard-items", libyang::JSON{"/example-schema:b"});
1975+
// let's use a prefix form here
1976+
auto discard2 = ctx.newOpaqueJSON(libyang::OpaqueName{"sysrepo", "sysrepo", "discard-items"}, libyang::JSON{"/example-schema:b"});
19751977
REQUIRE(!!discard2);
19761978
discard1->insertSibling(*discard2);
19771979
REQUIRE(*discard1->printStr(libyang::DataFormat::JSON, libyang::PrintFlags::WithSiblings)
@@ -2001,16 +2003,38 @@ TEST_CASE("Data Node manipulation")
20012003
auto data = ctx.newPath2("/example-schema:myRpc/outputLeaf", "AHOJ", libyang::CreationOptions::Output).createdNode;
20022004
REQUIRE(data);
20032005
data->newPath("/example-schema:myRpc/another", "yay", libyang::CreationOptions::Output);
2006+
std::string prettyName;
20042007

2005-
DOCTEST_SUBCASE("JSON") {
2006-
out = ctx.newOpaqueJSON(data->schema().module().name(), "output", std::nullopt);
2008+
DOCTEST_SUBCASE("JSON no prefix") {
2009+
out = ctx.newOpaqueJSON({data->schema().module().name(), std::nullopt, "output"}, std::nullopt);
2010+
prettyName = "{example-schema}, output";
20072011
}
20082012

2009-
DOCTEST_SUBCASE("XML") {
2010-
out = ctx.newOpaqueXML(data->schema().module().ns(), "output", std::nullopt);
2013+
DOCTEST_SUBCASE("JSON with prefix") {
2014+
out = ctx.newOpaqueJSON({data->schema().module().name(), data->schema().module().name(), "output"}, std::nullopt);
2015+
prettyName = "example-schema:output";
2016+
2017+
// wrong prefix is detected
2018+
REQUIRE_THROWS_WITH_AS(ctx.newOpaqueJSON({data->schema().module().name(), "xxx", "output"}, std::nullopt),
2019+
R"(invalid opaque JSON node: prefix "xxx" doesn't match module name "example-schema")",
2020+
libyang::Error);
2021+
}
2022+
2023+
DOCTEST_SUBCASE("XML no prefix") {
2024+
out = ctx.newOpaqueXML({data->schema().module().ns(), std::nullopt, "output"}, std::nullopt);
2025+
prettyName = "{http://example.com/coze}, output";
2026+
}
2027+
2028+
DOCTEST_SUBCASE("XML with prefix") {
2029+
out = ctx.newOpaqueXML({data->schema().module().ns(),
2030+
data->schema().module().name() /* prefix is a module name, not the XML NS*/,
2031+
"output"},
2032+
std::nullopt);
2033+
prettyName = "{http://example.com/coze}, example-schema:output";
20112034
}
20122035

20132036
REQUIRE(out);
2037+
REQUIRE(prettyName == out->asOpaque().name().pretty());
20142038
data->unlinkWithSiblings();
20152039
out->insertChild(*data);
20162040

0 commit comments

Comments
 (0)