From d76cf800ea9a61ff3860636707558802c00da401 Mon Sep 17 00:00:00 2001 From: Andreas Stöckel Date: Wed, 18 Feb 2015 10:46:13 +0100 Subject: Implemented automatic validation of RootNode instances in ParserScope --- src/core/parser/ParserScope.cpp | 12 ++++++++---- src/core/parser/ParserScope.hpp | 9 +++++++-- src/core/parser/stack/DocumentHandler.cpp | 14 +++++++------- src/core/parser/stack/DomainHandler.cpp | 16 ++++++++-------- src/core/parser/stack/TypesystemHandler.cpp | 6 +++--- 5 files changed, 33 insertions(+), 24 deletions(-) (limited to 'src/core') diff --git a/src/core/parser/ParserScope.cpp b/src/core/parser/ParserScope.cpp index b0a4945..dabb03c 100644 --- a/src/core/parser/ParserScope.cpp +++ b/src/core/parser/ParserScope.cpp @@ -216,7 +216,7 @@ void ParserScope::push(Handle node) nodes.push_back(node); } -void ParserScope::pop() +void ParserScope::pop(Logger &logger) { // Make sure pop is not called without an element on the stack const size_t currentDepth = nodes.size(); @@ -235,10 +235,14 @@ void ParserScope::pop() flags.resize(newLen); // Whenever a RootNode is popped from the stack, we have to perform deferred - // resolution -- however, postpone issuing error messages - if (nodes.back()->isa(&RttiTypes::RootNode)) { - Logger logger; + // resolution and validate the subtree + Rooted node = nodes.back(); + if (node->isa(&RttiTypes::RootNode)) { + // Perform pending resolutions -- do not issue errors now performDeferredResolution(logger, true); + + // Perform validation of the subtree. + node->validate(logger); } // Remove the element from the stack diff --git a/src/core/parser/ParserScope.hpp b/src/core/parser/ParserScope.hpp index 24af6b8..fa78c17 100644 --- a/src/core/parser/ParserScope.hpp +++ b/src/core/parser/ParserScope.hpp @@ -423,9 +423,14 @@ public: void push(Handle node); /** - * Removes the last pushed node from the scope. + * Removes the last pushed node from the scope. If the node that is popped + * from the internal stack is a RootNode, pending resolutions are performed + * and the RootNode is validated. + * + * @param logger is the Logger instance to which error messages should be + * logged. */ - void pop(); + void pop(Logger &logger); /** * Returns the top-level nodes. These are the nodes that are pushed onto the diff --git a/src/core/parser/stack/DocumentHandler.cpp b/src/core/parser/stack/DocumentHandler.cpp index 35146b1..e959b9a 100644 --- a/src/core/parser/stack/DocumentHandler.cpp +++ b/src/core/parser/stack/DocumentHandler.cpp @@ -47,7 +47,7 @@ bool DocumentHandler::start(Variant::mapType &args) return true; } -void DocumentHandler::end() { scope().pop(); } +void DocumentHandler::end() { scope().pop(logger()); } /* DocumentChildHandler */ @@ -200,9 +200,9 @@ bool DocumentChildHandler::start(Variant::mapType &args) // if we have transparent elements above us in the structure // tree we try to unwind them before we give up. // pop the implicit field. - scope().pop(); + scope().pop(logger()); // pop the implicit element. - scope().pop(); + scope().pop(logger()); continue; } throw LoggableException( @@ -237,7 +237,7 @@ void DocumentChildHandler::end() return; } // pop the "main" element. - scope().pop(); + scope().pop(logger()); } bool DocumentChildHandler::fieldStart(bool &isDefault, size_t fieldIdx) @@ -279,15 +279,15 @@ void DocumentChildHandler::fieldEnd() assert(scope().getLeaf()->isa(&RttiTypes::DocumentField)); // pop the field from the stack. - scope().pop(); + scope().pop(logger()); // pop all remaining transparent elements. while (scope().getLeaf()->isa(&RttiTypes::StructuredEntity) && scope().getLeaf().cast()->isTransparent()) { // pop the transparent element. - scope().pop(); + scope().pop(logger()); // pop the transparent field. - scope().pop(); + scope().pop(logger()); } } diff --git a/src/core/parser/stack/DomainHandler.cpp b/src/core/parser/stack/DomainHandler.cpp index ddec1ee..aa18faa 100644 --- a/src/core/parser/stack/DomainHandler.cpp +++ b/src/core/parser/stack/DomainHandler.cpp @@ -53,7 +53,7 @@ bool DomainHandler::start(Variant::mapType &args) return true; } -void DomainHandler::end() { scope().pop(); } +void DomainHandler::end() { scope().pop(logger()); } /* DomainStructHandler */ @@ -85,7 +85,7 @@ bool DomainStructHandler::start(Variant::mapType &args) return true; } -void DomainStructHandler::end() { scope().pop(); } +void DomainStructHandler::end() { scope().pop(logger()); } /* DomainAnnotationHandler */ bool DomainAnnotationHandler::start(Variant::mapType &args) @@ -102,7 +102,7 @@ bool DomainAnnotationHandler::start(Variant::mapType &args) return true; } -void DomainAnnotationHandler::end() { scope().pop(); } +void DomainAnnotationHandler::end() { scope().pop(logger()); } /* DomainAttributesHandler */ @@ -118,7 +118,7 @@ bool DomainAttributesHandler::start(Variant::mapType &args) return true; } -void DomainAttributesHandler::end() { scope().pop(); } +void DomainAttributesHandler::end() { scope().pop(logger()); } /* DomainFieldHandler */ @@ -148,7 +148,7 @@ bool DomainFieldHandler::start(Variant::mapType &args) return true; } -void DomainFieldHandler::end() { scope().pop(); } +void DomainFieldHandler::end() { scope().pop(logger()); } /* DomainFieldRefHandler */ @@ -218,7 +218,7 @@ bool DomainPrimitiveHandler::start(Variant::mapType &args) return true; } -void DomainPrimitiveHandler::end() { scope().pop(); } +void DomainPrimitiveHandler::end() { scope().pop(logger()); } /* DomainChildHandler */ @@ -251,7 +251,7 @@ bool DomainParentHandler::start(Variant::mapType &args) return true; } -void DomainParentHandler::end() { scope().pop(); } +void DomainParentHandler::end() { scope().pop(logger()); } /* DomainParentFieldHandler */ @@ -414,4 +414,4 @@ namespace RttiTypes { const Rtti DomainParent = RttiBuilder( "DomainParent").parent(&Node); } -} \ No newline at end of file +} diff --git a/src/core/parser/stack/TypesystemHandler.cpp b/src/core/parser/stack/TypesystemHandler.cpp index 8fd9525..727c601 100644 --- a/src/core/parser/stack/TypesystemHandler.cpp +++ b/src/core/parser/stack/TypesystemHandler.cpp @@ -51,7 +51,7 @@ bool TypesystemHandler::start(Variant::mapType &args) return true; } -void TypesystemHandler::end() { scope().pop(); } +void TypesystemHandler::end() { scope().pop(logger()); } /* TypesystemEnumHandler */ @@ -70,7 +70,7 @@ bool TypesystemEnumHandler::start(Variant::mapType &args) return true; } -void TypesystemEnumHandler::end() { scope().pop(); } +void TypesystemEnumHandler::end() { scope().pop(logger()); } /* TypesystemEnumEntryHandler */ @@ -112,7 +112,7 @@ bool TypesystemStructHandler::start(Variant::mapType &args) return true; } -void TypesystemStructHandler::end() { scope().pop(); } +void TypesystemStructHandler::end() { scope().pop(logger()); } /* TypesystemStructFieldHandler */ -- cgit v1.2.3 From 2de08643afdb4771ef8d1f6dd836ded20db244cf Mon Sep 17 00:00:00 2001 From: Andreas Stöckel Date: Wed, 18 Feb 2015 11:32:23 +0100 Subject: Fix for issue #85 -- only allowing explicit fields if no structure elements or data have been given beforehand. Added unit tests. --- src/core/parser/ParserScope.hpp | 9 ++++- src/core/parser/stack/DocumentHandler.cpp | 47 ++++++++++++++++++++---- test/formats/osml/OsmlParserTest.cpp | 27 ++++++++++++++ testdata/osmlparser/explicit_fields.osml | 16 ++++++++ testdata/osmlparser/invalid_explicit_fields.osml | 16 ++++++++ 5 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 testdata/osmlparser/explicit_fields.osml create mode 100644 testdata/osmlparser/invalid_explicit_fields.osml (limited to 'src/core') diff --git a/src/core/parser/ParserScope.hpp b/src/core/parser/ParserScope.hpp index fa78c17..e27c81e 100644 --- a/src/core/parser/ParserScope.hpp +++ b/src/core/parser/ParserScope.hpp @@ -286,7 +286,13 @@ enum class ParserFlag { * Set to the boolean value "true" if the head section of a file has passed. * This happens once the first non-import tag is reached. */ - POST_HEAD + POST_HEAD, + + /** + * Set to the boolean value "true" if explicit fields may no longer be + * defined inside a structure element. + */ + POST_EXPLICIT_FIELDS }; /** @@ -797,6 +803,7 @@ public: bool resolveFieldDescriptor(const std::string &name, Handle owner, Logger &logger, ResolutionResultCallback resultCallback); + /** * Tries to resolve all currently deferred resolution steps. The list of * pending deferred resolutions is cleared after this function has run. diff --git a/src/core/parser/stack/DocumentHandler.cpp b/src/core/parser/stack/DocumentHandler.cpp index e959b9a..1df3cb3 100644 --- a/src/core/parser/stack/DocumentHandler.cpp +++ b/src/core/parser/stack/DocumentHandler.cpp @@ -98,6 +98,9 @@ void DocumentChildHandler::createPath(const NodeVector &path, manager(), scope().getLeaf(), parent->getDescriptor()->getFieldDescriptorIndex(), true)}; scope().push(field); + + // Generally allow explicit fields in the new field + scope().setFlag(ParserFlag::POST_EXPLICIT_FIELDS, false); } void DocumentChildHandler::createPath(const size_t &firstFieldIdx, @@ -113,6 +116,9 @@ void DocumentChildHandler::createPath(const size_t &firstFieldIdx, parent = static_cast(transparent.get()); createPath(path, parent, 2); + + // Generally allow explicit fields in the new field + scope().setFlag(ParserFlag::POST_EXPLICIT_FIELDS, false); } bool DocumentChildHandler::start(Variant::mapType &args) @@ -170,12 +176,25 @@ bool DocumentChildHandler::start(Variant::mapType &args) ssize_t newFieldIdx = parent->getDescriptor()->getFieldDescriptorIndex(name()); if (newFieldIdx != -1) { - Rooted field{new DocumentField( - manager(), parentNode, newFieldIdx, false)}; - field->setLocation(location()); - scope().push(field); - isExplicitField = true; - return true; + // Check whether explicit fields are allowed here, if not + if (scope().getFlag(ParserFlag::POST_EXPLICIT_FIELDS)) { + logger().note( + std::string( + "Data or structure commands have already been " + "given, command \"") + + name() + std::string( + "\" is not interpreted explicit " + "field. Move explicit field " + "references to the beginning."), + location()); + } else { + Rooted field{new DocumentField( + manager(), parentNode, newFieldIdx, false)}; + field->setLocation(location()); + scope().push(field); + isExplicitField = true; + return true; + } } } @@ -218,11 +237,17 @@ bool DocumentChildHandler::start(Variant::mapType &args) parent->getDescriptor()->getFieldDescriptorIndex(); } // create the entity for the new element at last. - //TODO: REMOVE + // TODO: REMOVE strct_name = strct->getName(); entity = parent->createChildStructuredEntity(strct, lastFieldIdx, args, nameAttr); } + + // We're past the region in which explicit fields can be defined in the + // parent structure element + scope().setFlag(ParserFlag::POST_EXPLICIT_FIELDS, true); + + // Bush the entity onto the stack entity->setLocation(location()); scope().push(entity); return true; @@ -271,6 +296,10 @@ bool DocumentChildHandler::fieldStart(bool &isDefault, size_t fieldIdx) new DocumentField(manager(), parentNode, fieldIdx, false)}; field->setLocation(location()); scope().push(field); + + // Generally allow explicit fields in the new field + scope().setFlag(ParserFlag::POST_EXPLICIT_FIELDS, false); + return true; } @@ -334,6 +363,10 @@ bool DocumentChildHandler::convertData(Handle field, bool DocumentChildHandler::data(Variant &data) { + // We're past the region in which explicit fields can be defined in the + // parent structure element + scope().setFlag(ParserFlag::POST_EXPLICIT_FIELDS, true); + Rooted parentField = scope().getLeaf(); assert(parentField->isa(&RttiTypes::DocumentField)); diff --git a/test/formats/osml/OsmlParserTest.cpp b/test/formats/osml/OsmlParserTest.cpp index 4cda20a..5127b32 100644 --- a/test/formats/osml/OsmlParserTest.cpp +++ b/test/formats/osml/OsmlParserTest.cpp @@ -172,5 +172,32 @@ TEST(OsmlParser, structWithNoField) ASSERT_TRUE(node->isa(&RttiTypes::Document)); } +TEST(OsmlParser, invalidExplicitFields) +{ + OsmlStandaloneEnvironment env(logger); + logger.reset(); + + ASSERT_FALSE(logger.hasError()); + Rooted node = env.parse("invalid_explicit_fields.osml", "", "", + RttiSet{&RttiTypes::Node}); + ASSERT_TRUE(logger.hasError()); + + ASSERT_TRUE(node != nullptr); + ASSERT_TRUE(node->isa(&RttiTypes::Document)); +} + +TEST(OsmlParser, explicitFields) +{ + OsmlStandaloneEnvironment env(logger); + logger.reset(); + + Rooted node = env.parse("explicit_fields.osml", "", "", + RttiSet{&RttiTypes::Node}); + ASSERT_FALSE(logger.hasError()); + + ASSERT_TRUE(node != nullptr); + ASSERT_TRUE(node->isa(&RttiTypes::Document)); +} + } diff --git a/testdata/osmlparser/explicit_fields.osml b/testdata/osmlparser/explicit_fields.osml new file mode 100644 index 0000000..a9ba1a3 --- /dev/null +++ b/testdata/osmlparser/explicit_fields.osml @@ -0,0 +1,16 @@ +\document + +\domain#test + \struct#a[isRoot=true] + \primitive#b[type=string,isSubtree=true] + \primitive#c[type=string,isSubtree=true] + \primitive#d[type=string,isSubtree=false] + + +\a{! + \b{test} + \c{test} + test + test + test +} diff --git a/testdata/osmlparser/invalid_explicit_fields.osml b/testdata/osmlparser/invalid_explicit_fields.osml new file mode 100644 index 0000000..9986204 --- /dev/null +++ b/testdata/osmlparser/invalid_explicit_fields.osml @@ -0,0 +1,16 @@ +\document + +\domain#test + \struct#a[isRoot=true] + \primitive#b[type=string,isSubtree=true] + \primitive#c[type=string,isSubtree=true] + \primitive#d[type=string,isSubtree=false] + + +\a{! + \b{test} + test + \c{test} + test + test +} -- cgit v1.2.3 From 1765901ff35db93ba79ce67473ffb1abcf7165d6 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Wed, 18 Feb 2015 11:42:30 +0100 Subject: detected and counteracted cycles in gatherFieldDescriptors and gatherSubclasses. Also added unit tests for those cyclic cases. --- src/core/model/Domain.cpp | 37 ++++++--- src/core/model/Domain.hpp | 1 + test/core/model/DomainTest.cpp | 185 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 213 insertions(+), 10 deletions(-) (limited to 'src/core') diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp index ca8f889..8255401 100644 --- a/src/core/model/Domain.cpp +++ b/src/core/model/Domain.cpp @@ -324,21 +324,31 @@ bool FieldDescriptor::doValidate(Logger &logger) const return valid; } -static void gatherSubclasses(NodeVector &res, - Handle strct) +static void gatherSubclasses( + std::unordered_set& visited, + NodeVector &res, Handle strct) { + // this check is to prevent cycles. + if (!visited.insert(strct.get()).second) { + return; + } for (auto sub : strct->getSubclasses()) { + // this check is to prevent cycles. + if(visited.count(sub.get())){ + continue; + } res.push_back(sub); - gatherSubclasses(res, sub); + gatherSubclasses(visited, res, sub); } } NodeVector FieldDescriptor::getChildrenWithSubclasses() const { + std::unordered_set visited; NodeVector res; for (auto c : children) { res.push_back(c); - gatherSubclasses(res, c); + gatherSubclasses(visited, res, c); } return res; } @@ -566,8 +576,9 @@ bool Descriptor::addAndSortFieldDescriptor(Handle fd, if (fds.find(fd) == fds.end()) { invalidate(); // check if the previous field is a tree field already. - if (!fds.empty() && !fieldDescriptors.empty() && - fds.back()->getFieldType() == FieldDescriptor::FieldType::TREE && + if (!fieldDescriptors.empty() && + fieldDescriptors.back()->getFieldType() == + FieldDescriptor::FieldType::TREE && fd->getFieldType() != FieldDescriptor::FieldType::TREE) { // if so we add the new field before the TREE field. fieldDescriptors.insert(fieldDescriptors.end() - 1, fd); @@ -776,8 +787,13 @@ void StructuredClass::removeSubclass(Handle sc, Logger &logger) Rooted StructuredClass::gatherFieldDescriptors( NodeVector ¤t, + std::unordered_set &visited, std::set &overriddenFields, bool hasTREE) const { + // this check is to prevent cycles of inheritance to mess up this function. + if (!visited.insert(this).second) { + return nullptr; + } Rooted mainField; NodeVector tmp; // first gather the non-overridden fields. @@ -798,8 +814,8 @@ Rooted StructuredClass::gatherFieldDescriptors( if (superclass != nullptr) { Rooted super_main_field = - superclass->gatherFieldDescriptors(current, overriddenFields, - hasTREE); + superclass->gatherFieldDescriptors(current, visited, + overriddenFields, hasTREE); if (!hasTREE) { mainField = super_main_field; } @@ -814,9 +830,10 @@ NodeVector StructuredClass::getFieldDescriptors() const { // in this case we return a NodeVector of Rooted entries without owner. NodeVector vec; + std::unordered_set visited; std::set overriddenFields; Rooted mainField = - gatherFieldDescriptors(vec, overriddenFields, false); + gatherFieldDescriptors(vec, visited, overriddenFields, false); if (mainField != nullptr) { vec.push_back(mainField); } @@ -961,4 +978,4 @@ const Rtti Domain = RttiBuilder("Domain") .parent(&RootNode) .composedOf({&StructuredClass, &AnnotationClass}); } -} +} \ No newline at end of file diff --git a/src/core/model/Domain.hpp b/src/core/model/Domain.hpp index 476a38c..b3fc6c2 100644 --- a/src/core/model/Domain.hpp +++ b/src/core/model/Domain.hpp @@ -808,6 +808,7 @@ private: */ Rooted gatherFieldDescriptors( NodeVector ¤t, + std::unordered_set &visited, std::set &overriddenFields, bool hasTREE) const; protected: diff --git a/test/core/model/DomainTest.cpp b/test/core/model/DomainTest.cpp index d68648e..6bbf26d 100644 --- a/test/core/model/DomainTest.cpp +++ b/test/core/model/DomainTest.cpp @@ -81,6 +81,115 @@ TEST(Domain, testDomainResolving) assert_path(res[0], &RttiTypes::StructuredClass, {"book", "paragraph"}); } +// i use this wrapper due to the strange behaviour of GTEST. +static void assertFalse(bool b){ + ASSERT_FALSE(b); +} + +static Rooted createUnsortedPrimitiveField( + Handle strct, Handle type, Logger &logger, bool tree, + std::string name) +{ + FieldDescriptor::FieldType fieldType = FieldDescriptor::FieldType::SUBTREE; + if (tree) { + fieldType = FieldDescriptor::FieldType::TREE; + } + + auto res = strct->createPrimitiveFieldDescriptor(type, logger, fieldType, + std::move(name)); + assertFalse(res.second); + return res.first; +} + +TEST(StructuredClass, getFieldDescriptors) +{ + /* + * We construct a case with the three levels: + * 1.) A has the SUBTREE fields a and b as well as a TREE field. + * 2.) B is a subclass of A and has the SUBTREE fields b and c as well as + * a TREE field. + * 3.) C is a subclass of B and has the SUBTREE field a. + * As a result we expect C to have none of As fields, the TREE field of B, + * and the SUBTREE fields a (of C) , b and c (of B). + */ + TerminalLogger logger{std::cout}; + Manager mgr{1}; + Rooted sys{new SystemTypesystem(mgr)}; + Rooted domain{new Domain(mgr, sys, "myDomain")}; + + Rooted A{new StructuredClass( + mgr, "A", domain, Cardinality::any(), nullptr, false, true)}; + Rooted A_a = createUnsortedPrimitiveField( + A, sys->getStringType(), logger, false, "a"); + Rooted A_b = createUnsortedPrimitiveField( + A, sys->getStringType(), logger, false, "b"); + Rooted A_main = createUnsortedPrimitiveField( + A, sys->getStringType(), logger, true, "somename"); + + Rooted B{new StructuredClass( + mgr, "B", domain, Cardinality::any(), A, false, true)}; + Rooted B_b = createUnsortedPrimitiveField( + B, sys->getStringType(), logger, false, "b"); + Rooted B_c = createUnsortedPrimitiveField( + B, sys->getStringType(), logger, false, "c"); + Rooted B_main = createUnsortedPrimitiveField( + B, sys->getStringType(), logger, true, "othername"); + + Rooted C{new StructuredClass( + mgr, "C", domain, Cardinality::any(), B, false, true)}; + Rooted C_a = createUnsortedPrimitiveField( + C, sys->getStringType(), logger, false, "a"); + + ASSERT_TRUE(domain->validate(logger)); + + // check all FieldDescriptors + { + NodeVector fds = A->getFieldDescriptors(); + ASSERT_EQ(3, fds.size()); + ASSERT_EQ(A_a, fds[0]); + ASSERT_EQ(A_b, fds[1]); + ASSERT_EQ(A_main, fds[2]); + } + { + NodeVector fds = B->getFieldDescriptors(); + ASSERT_EQ(4, fds.size()); + ASSERT_EQ(A_a, fds[0]); + ASSERT_EQ(B_b, fds[1]); + ASSERT_EQ(B_c, fds[2]); + ASSERT_EQ(B_main, fds[3]); + } + { + NodeVector fds = C->getFieldDescriptors(); + ASSERT_EQ(4, fds.size()); + ASSERT_EQ(B_b, fds[0]); + ASSERT_EQ(B_c, fds[1]); + // superclass fields come before subclass fields (except for the TREE + // field, which is always last). + ASSERT_EQ(C_a, fds[2]); + ASSERT_EQ(B_main, fds[3]); + } +} + + +TEST(StructuredClass, getFieldDescriptorsCycles) +{ + Logger logger; + Manager mgr{1}; + Rooted sys{new SystemTypesystem(mgr)}; + Rooted domain{new Domain(mgr, sys, "myDomain")}; + + Rooted A{new StructuredClass( + mgr, "A", domain, Cardinality::any(), nullptr, false, true)}; + A->addSubclass(A, logger); + Rooted A_a = createUnsortedPrimitiveField( + A, sys->getStringType(), logger, false, "a"); + ASSERT_FALSE(domain->validate(logger)); + // if we call getFieldDescriptors that should still return a valid result. + NodeVector fds = A->getFieldDescriptors(); + ASSERT_EQ(1, fds.size()); + ASSERT_EQ(A_a, fds[0]); +} + Rooted getClass(const std::string name, Handle dom) { std::vector res = @@ -221,6 +330,34 @@ TEST(Descriptor, pathToAdvanced) ASSERT_EQ("", path[2]->getName()); } +TEST(Descriptor, pathToCycles) +{ + // build a domain with a cycle. + Manager mgr{1}; + Logger logger; + Rooted sys{new SystemTypesystem(mgr)}; + // Construct the domain + Rooted domain{new Domain(mgr, sys, "cycles")}; + Rooted A{new StructuredClass( + mgr, "A", domain, Cardinality::any(), {nullptr}, true, true)}; + A->addSubclass(A, logger); + ASSERT_FALSE(domain->validate(logger)); + Rooted B{new StructuredClass( + mgr, "B", domain, Cardinality::any(), {nullptr}, false, true)}; + Rooted A_field = A->createFieldDescriptor(logger).first; + A_field->addChild(B); + /* + * Now try to create the path from A to B. A direct path is possible but + * in the worst case this could also try to find shorter paths via an + * endless repition of A instances. + * As we cut the search tree at paths that are longer than our current + * optimum this should not happen, though. + */ + NodeVector path = A->pathTo(B, logger); + ASSERT_EQ(1, path.size()); + ASSERT_EQ(A_field, path[0]); +} + TEST(Descriptor, getDefaultFields) { // construct a domain with lots of default fields to test. @@ -301,6 +438,29 @@ TEST(Descriptor, getDefaultFields) ASSERT_EQ(F_field, fields[1]); } +TEST(Descriptor, getDefaultFieldsCycles) +{ + // build a domain with a cycle. + Manager mgr{1}; + Logger logger; + Rooted sys{new SystemTypesystem(mgr)}; + // Construct the domain + Rooted domain{new Domain(mgr, sys, "cycles")}; + Rooted A{new StructuredClass( + mgr, "A", domain, Cardinality::any(), {nullptr}, true, true)}; + A->addSubclass(A, logger); + ASSERT_FALSE(domain->validate(logger)); + Rooted A_field = + A->createPrimitiveFieldDescriptor(sys->getStringType(), logger).first; + /* + * Now try to get the default fields of A. This should not lead to cycles + * if we correctly note all already visited nodes. + */ + NodeVector defaultFields = A->getDefaultFields(); + ASSERT_EQ(1, defaultFields.size()); + ASSERT_EQ(A_field, defaultFields[0]); +} + TEST(Descriptor, getPermittedChildren) { // analyze the book domain. @@ -338,6 +498,31 @@ TEST(Descriptor, getPermittedChildren) ASSERT_EQ(sub, children[3]); } +TEST(Descriptor, getPermittedChildrenCycles) +{ + // build a domain with a cycle. + Manager mgr{1}; + Logger logger; + Rooted sys{new SystemTypesystem(mgr)}; + // Construct the domain + Rooted domain{new Domain(mgr, sys, "cycles")}; + Rooted A{new StructuredClass( + mgr, "A", domain, Cardinality::any(), {nullptr}, true, true)}; + A->addSubclass(A, logger); + ASSERT_FALSE(domain->validate(logger)); + Rooted A_field = A->createFieldDescriptor(logger).first; + // we make the cycle worse by adding A as child of itself. + A_field->addChild(A); + /* + * Now try to get the permitted children of A. This should not lead to + * cycles + * if we correctly note all already visited nodes. + */ + NodeVector children = A->getPermittedChildren(); + ASSERT_EQ(1, children.size()); + ASSERT_EQ(A, children[0]); +} + TEST(StructuredClass, isSubclassOf) { // create an inheritance hierarchy. -- cgit v1.2.3 From e4aa28fc9bd606a0a2aa9be75a3bef6ad3b6fed7 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Wed, 18 Feb 2015 14:17:44 +0100 Subject: fixed a bug with doubled root nodes. --- src/core/parser/stack/DocumentHandler.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src/core') diff --git a/src/core/parser/stack/DocumentHandler.cpp b/src/core/parser/stack/DocumentHandler.cpp index 1df3cb3..98b84c7 100644 --- a/src/core/parser/stack/DocumentHandler.cpp +++ b/src/core/parser/stack/DocumentHandler.cpp @@ -142,6 +142,14 @@ bool DocumentChildHandler::start(Variant::mapType &args) Rooted entity; // handle the root note specifically. if (parentNode->isa(&RttiTypes::Document)) { + // if we already have a root node, stop. + if (parentNode.cast()->getRoot() != nullptr) { + logger().warning( + "This document already has a root node. The additional " + "node is ignored.", + location()); + return false; + } Rooted strct = scope().resolve( Utils::split(name(), ':'), logger()); if (strct == nullptr) { -- cgit v1.2.3 From b958e4c4788f1acb28398f640e0e5e80a45b3e12 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Wed, 18 Feb 2015 14:17:55 +0100 Subject: fixed a bug with empty fields. --- src/core/parser/stack/DocumentHandler.cpp | 5 ++++- src/core/parser/stack/Stack.cpp | 20 ++++++++------------ test/core/parser/stack/StackTest.cpp | 20 +++++++++----------- 3 files changed, 21 insertions(+), 24 deletions(-) (limited to 'src/core') diff --git a/src/core/parser/stack/DocumentHandler.cpp b/src/core/parser/stack/DocumentHandler.cpp index 98b84c7..bb04bd3 100644 --- a/src/core/parser/stack/DocumentHandler.cpp +++ b/src/core/parser/stack/DocumentHandler.cpp @@ -292,6 +292,9 @@ bool DocumentChildHandler::fieldStart(bool &isDefault, size_t fieldIdx) parent->getDescriptor()->getFieldDescriptors(); if (isDefault) { + if(fields.empty()){ + return false; + } fieldIdx = fields.size() - 1; } else { if (fieldIdx >= fields.size()) { @@ -468,4 +471,4 @@ namespace RttiTypes { const Rtti DocumentField = RttiBuilder( "DocumentField").parent(&Node); } -} +} \ No newline at end of file diff --git a/src/core/parser/stack/Stack.cpp b/src/core/parser/stack/Stack.cpp index 08f86e5..5b67248 100644 --- a/src/core/parser/stack/Stack.cpp +++ b/src/core/parser/stack/Stack.cpp @@ -16,8 +16,6 @@ along with this program. If not, see . */ -#include - #include #include #include @@ -256,7 +254,9 @@ void Stack::endCurrentHandler() // Make sure the fieldEnd handler is called if the element still // is in a field if (info.inField) { - info.handler->fieldEnd(); + if (info.inValidField) { + info.handler->fieldEnd(); + } info.fieldEnd(); } @@ -300,8 +300,6 @@ bool Stack::ensureHandlerIsInField() // Try to start a new default field, abort if this did not work bool isDefault = true; if (!info.handler->fieldStart(isDefault, info.fieldIdx)) { - info.handler->fieldEnd(); - endCurrentHandler(); return false; } @@ -505,10 +503,9 @@ void Stack::fieldStart(bool isDefault) // (the default field always is the last field) -- mark the command as // invalid if (info.hadDefaultField) { - logger().error( - std::string("Got field start, but command \"") + - currentCommandName() + - std::string("\" does not have any more fields")); + logger().error(std::string("Got field start, but command \"") + + currentCommandName() + + std::string("\" does not have any more fields")); } // Copy the isDefault flag to a local variable, the fieldStart method will @@ -559,7 +556,7 @@ void Stack::fieldEnd() // Only continue if the current handler stack is in a valid state, do not // call the fieldEnd function if something went wrong before - if (handlersValid() && !info.hadDefaultField) { + if (handlersValid() && !info.hadDefaultField && info.inValidField) { try { info.handler->fieldEnd(); } @@ -587,5 +584,4 @@ void Stack::token(Variant token) // TODO } } -} - +} \ No newline at end of file diff --git a/test/core/parser/stack/StackTest.cpp b/test/core/parser/stack/StackTest.cpp index e25f487..a93f14a 100644 --- a/test/core/parser/stack/StackTest.cpp +++ b/test/core/parser/stack/StackTest.cpp @@ -449,10 +449,10 @@ TEST(Stack, noImplicitDefaultFieldOnIncompatibleCommand) tracker.fieldStartResult = false; s.command("b", {}); - tracker.expect(2, 1, 1, 1, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc + tracker.expect(2, 1, 1, 0, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc ASSERT_EQ("b", s.currentCommandName()); } - tracker.expect(2, 2, 1, 1, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc + tracker.expect(2, 2, 1, 0, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc ASSERT_FALSE(logger.hasError()); } @@ -563,9 +563,9 @@ TEST(Stack, invalidDefaultField) tracker.fieldStartResult = false; s.fieldStart(true); s.fieldEnd(); - tracker.expect(1, 0, 1, 1, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc + tracker.expect(1, 0, 1, 0, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc } - tracker.expect(1, 1, 1, 1, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc + tracker.expect(1, 1, 1, 0, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc ASSERT_FALSE(logger.hasError()); } @@ -583,9 +583,9 @@ TEST(Stack, errorInvalidDefaultFieldData) s.data("test"); ASSERT_TRUE(logger.hasError()); s.fieldEnd(); - tracker.expect(1, 0, 1, 1, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc + tracker.expect(1, 0, 1, 0, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc } - tracker.expect(1, 1, 1, 1, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc + tracker.expect(1, 1, 1, 0, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc } TEST(Stack, errorInvalidFieldData) @@ -602,9 +602,9 @@ TEST(Stack, errorInvalidFieldData) ASSERT_TRUE(logger.hasError()); s.data("test"); s.fieldEnd(); - tracker.expect(1, 0, 1, 1, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc + tracker.expect(1, 0, 1, 0, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc } - tracker.expect(1, 1, 1, 1, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc + tracker.expect(1, 1, 1, 0, 0, 0, 0); // sc, ec, fsc, fse, asc, aec, dc } TEST(Stack, errorFieldStartNoCommand) @@ -743,7 +743,5 @@ TEST(Stack, fieldAfterDefaultField) tracker.expect(2, 2, 3, 3, 0, 0, 2); // sc, ec, fsc, fse, asc, aec, dc ASSERT_FALSE(logger.hasError()); } - -} } - +} \ No newline at end of file -- cgit v1.2.3 From dfca16e5fb6bd14462fef41c8c771e2f758c92d3 Mon Sep 17 00:00:00 2001 From: Andreas Stöckel Date: Wed, 18 Feb 2015 15:23:20 +0100 Subject: Registering inline typesystems in document --- src/core/parser/stack/TypesystemHandler.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'src/core') diff --git a/src/core/parser/stack/TypesystemHandler.cpp b/src/core/parser/stack/TypesystemHandler.cpp index 727c601..de8ee49 100644 --- a/src/core/parser/stack/TypesystemHandler.cpp +++ b/src/core/parser/stack/TypesystemHandler.cpp @@ -16,11 +16,13 @@ along with this program. If not, see . */ -#include +#include #include +#include #include #include +#include "DocumentHandler.hpp" #include "DomainHandler.hpp" #include "State.hpp" #include "TypesystemHandler.hpp" @@ -38,10 +40,16 @@ bool TypesystemHandler::start(Variant::mapType &args) typesystem->setLocation(location()); // If the typesystem is defined inside a domain, add a reference to the - // typesystem to the domain + // typesystem to the domain -- do the same with a document, if no domain + // is found Rooted domain = scope().select(); if (domain != nullptr) { domain->reference(typesystem); + } else { + Rooted document = scope().select(); + if (document != nullptr) { + document->reference(typesystem); + } } // Push the typesystem onto the scope, set the POST_HEAD flag to true @@ -182,7 +190,7 @@ bool TypesystemConstantHandler::start(Variant::mapType &args) namespace States { const State Typesystem = StateBuilder() - .parents({&None, &Domain}) + .parents({&None, &Domain, &Document}) .createdNodeType(&RttiTypes::Typesystem) .elementHandler(TypesystemHandler::create) .arguments({Argument::String("name", "")}); -- cgit v1.2.3 From 1f459bc05a45ae25c0e68c9b28b14444f09a53d6 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Wed, 18 Feb 2015 16:33:18 +0100 Subject: added writeOusia methods to VariantWriter. --- src/core/common/VariantWriter.cpp | 31 ++++++++++++++++++++++--------- src/core/common/VariantWriter.hpp | 22 +++++++++++++++++++++- 2 files changed, 43 insertions(+), 10 deletions(-) (limited to 'src/core') diff --git a/src/core/common/VariantWriter.cpp b/src/core/common/VariantWriter.cpp index 427ac5d..b1bafe4 100644 --- a/src/core/common/VariantWriter.cpp +++ b/src/core/common/VariantWriter.cpp @@ -104,8 +104,9 @@ static void writeLinebreak(std::ostream &stream, bool pretty) * @param pretty if true, the resulting value is properly indented. * @param level is the current indentation level. */ -static void writeJsonInternal(const Variant &var, std::ostream &stream, - bool pretty, int level) +template +static void writeInternal(const Variant &var, std::ostream &stream, bool pretty, + int level) { switch (var.getType()) { case VariantType::NULLPTR: @@ -127,7 +128,8 @@ static void writeJsonInternal(const Variant &var, std::ostream &stream, const Variant::arrayType &arr = var.asArray(); for (size_t i = 0; i < arr.size(); i++) { writeIndentation(stream, pretty, level + 1); - writeJsonInternal(arr[i], stream, pretty, level + 1); + writeInternal( + arr[i], stream, pretty, level + 1); if (i + 1 != arr.size()) { stream << ","; } @@ -139,21 +141,22 @@ static void writeJsonInternal(const Variant &var, std::ostream &stream, } case VariantType::MAP: { writeIndentation(stream, pretty, level); - stream << "{"; + stream << ObjectStart; writeLinebreak(stream, pretty); const Variant::mapType &map = var.asMap(); for (auto it = map.cbegin(); it != map.cend();) { writeIndentation(stream, pretty, level + 1); writeJsonString(it->first, stream); - stream << (pretty ? ": " : ":"); - writeJsonInternal(it->second, stream, pretty, level + 1); + stream << Equals << (pretty ? " " : ""); + writeInternal( + it->second, stream, pretty, level + 1); if ((++it) != map.cend()) { stream << ","; } writeLinebreak(stream, pretty); } writeIndentation(stream, pretty, level); - stream << "}"; + stream << ObjectEnd; return; } } @@ -162,7 +165,7 @@ static void writeJsonInternal(const Variant &var, std::ostream &stream, void VariantWriter::writeJson(const Variant &var, std::ostream &stream, bool pretty) { - writeJsonInternal(var, stream, pretty, 0); + writeInternal<'{', '}', ':'>(var, stream, pretty, 0); } std::string VariantWriter::writeJsonToString(const Variant &var, bool pretty) @@ -172,6 +175,16 @@ std::string VariantWriter::writeJsonToString(const Variant &var, bool pretty) return ss.str(); } - +void VariantWriter::writeOusia(const Variant &var, std::ostream &stream, + bool pretty) +{ + writeInternal<'[', ']', '='>(var, stream, pretty, 0); } +std::string VariantWriter::writeOusiaToString(const Variant &var, bool pretty) +{ + std::stringstream ss; + writeOusia(var, ss, pretty); + return ss.str(); +} +} \ No newline at end of file diff --git a/src/core/common/VariantWriter.hpp b/src/core/common/VariantWriter.hpp index 7fe32fb..12f4bba 100644 --- a/src/core/common/VariantWriter.hpp +++ b/src/core/common/VariantWriter.hpp @@ -59,8 +59,28 @@ public: * @param var is the variant that should be serialized. * @param pretty if true, the resulting value is properly indented. */ - static std::string writeJsonToString(const Variant &var, bool pretty = true); + static std::string writeJsonToString(const Variant &var, + bool pretty = true); + /** + * Dumps the Variant as re-readable ousia data. Note that the resulting + * data is invalid if the Variant consists of function or object references. + * + * @param var is the variant that should be serialized. + * @param stream is the stream the result should be written to. + * @param pretty if true, the resulting value is properly indented. + */ + static void writeOusia(const Variant &var, std::ostream &stream, + bool pretty = true); + + /** + * Dumps the Variant as re-readable ousia data to a string. + * + * @param var is the variant that should be serialized. + * @param pretty if true, the resulting value is properly indented. + */ + static std::string writeOusiaToString(const Variant &var, + bool pretty = true); }; } -- cgit v1.2.3 From 792357cb81ffa8b2f4fe5c16b1e359d8e51a53ae Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Wed, 18 Feb 2015 16:33:35 +0100 Subject: changed cardinality toString conversion to be reparseable as cardinality. --- src/core/common/VariantConverter.cpp | 4 ++-- test/core/common/VariantConverterTest.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src/core') diff --git a/src/core/common/VariantConverter.cpp b/src/core/common/VariantConverter.cpp index a9ca467..b43d04e 100644 --- a/src/core/common/VariantConverter.cpp +++ b/src/core/common/VariantConverter.cpp @@ -262,7 +262,7 @@ bool VariantConverter::toString(Variant &var, Logger &logger, Mode mode) // Print cardinality syntax Variant::cardinalityType card = var.asCardinality(); std::stringstream ss; - ss << "" << std::to_string(r.start - 1); } } - ss << "}>"; + ss << "}"; var = ss.str().c_str(); return true; } diff --git a/test/core/common/VariantConverterTest.cpp b/test/core/common/VariantConverterTest.cpp index 9654a7b..2860777 100644 --- a/test/core/common/VariantConverterTest.cpp +++ b/test/core/common/VariantConverterTest.cpp @@ -244,7 +244,7 @@ TEST(VariantConverter, toString) VariantConverter::Mode::ALL, logger); assertStringConversion(M, "{\"b\":true,\"d\":2.7,\"i\":6,\"s\":\"test\"}", true, VariantConverter::Mode::ALL, logger); - assertStringConversion(C, "6}>", true, + assertStringConversion(C, "{2-4, >6}", true, VariantConverter::Mode::ALL, logger); } -- cgit v1.2.3