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 --- test/core/parser/ParserScopeTest.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'test/core') diff --git a/test/core/parser/ParserScopeTest.cpp b/test/core/parser/ParserScopeTest.cpp index 7f89f2c..2ead924 100644 --- a/test/core/parser/ParserScopeTest.cpp +++ b/test/core/parser/ParserScopeTest.cpp @@ -18,6 +18,7 @@ #include +#include #include #include #include @@ -26,6 +27,7 @@ namespace ousia { TEST(ParserScope, flags) { + Logger logger; Manager mgr; ParserScope scope; @@ -42,9 +44,9 @@ TEST(ParserScope, flags) ASSERT_TRUE(scope.getFlag(ParserFlag::POST_HEAD)); scope.setFlag(ParserFlag::POST_HEAD, false); ASSERT_FALSE(scope.getFlag(ParserFlag::POST_HEAD)); - scope.pop(); + scope.pop(logger); ASSERT_TRUE(scope.getFlag(ParserFlag::POST_HEAD)); - scope.pop(); + scope.pop(logger); ASSERT_FALSE(scope.getFlag(ParserFlag::POST_HEAD)); scope.setFlag(ParserFlag::POST_HEAD, true); ASSERT_TRUE(scope.getFlag(ParserFlag::POST_HEAD)); -- 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 'test/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 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 'test/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 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 'test/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