From 928d989aef13e72064f31e3d6cad64f46c56bfdc Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Tue, 10 Feb 2015 20:57:16 +0100 Subject: added a document parsing test. --- test/plugins/xml/XmlParserTest.cpp | 91 +++++++++++++++++++++++++++++++++++++- 1 file changed, 89 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/test/plugins/xml/XmlParserTest.cpp b/test/plugins/xml/XmlParserTest.cpp index af1ef56..067214c 100644 --- a/test/plugins/xml/XmlParserTest.cpp +++ b/test/plugins/xml/XmlParserTest.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -303,12 +304,98 @@ TEST(XmlParser, domainParsing) checkFieldDescriptor(heading, paragraph, {text, comment}); } +static void checkStructuredEntity( + Handle s, Handle expectedParent, Handle strct, + const Variant::mapType &expectedAttributes = Variant::mapType{}, + const std::string &expectedName = "") +{ + ASSERT_FALSE(s == nullptr); + ASSERT_TRUE(s->isa(&RttiTypes::StructuredEntity)); + Rooted entity = s.cast(); + ASSERT_EQ(expectedParent, entity->getParent()); + ASSERT_EQ(strct, entity->getDescriptor()); + ASSERT_EQ(expectedAttributes, entity->getAttributes()); + ASSERT_EQ(expectedName, entity->getName()); +} + +static void checkStructuredEntity( + Handle s, Handle expectedParent, Handle doc, + const std::string &className, + const Variant::mapType &expectedAttributes = Variant::mapType{}, + const std::string &expectedName = "") +{ + auto res = doc->resolve(&RttiTypes::StructuredClass, className); + if (res.size() != 1) { + throw OusiaException("resolution error!"); + } + Handle sc = res[0].node.cast(); + checkStructuredEntity(s, expectedParent, sc, expectedAttributes, + expectedName); +} + +static void checkText(Handle p, Handle expectedParent, + Handle doc, Variant expected) +{ + checkStructuredEntity(p, expectedParent, doc, "paragraph"); + Rooted par = p.cast(); + ASSERT_EQ(1, par->getField().size()); + checkStructuredEntity(par->getField()[0], par, doc, "text"); + Rooted text = par->getField()[0].cast(); + ASSERT_EQ(1, text->getField().size()); + + Handle d = text->getField()[0]; + ASSERT_FALSE(d == nullptr); + ASSERT_TRUE(d->isa(&RttiTypes::DocumentPrimitive)); + Rooted prim = d.cast(); + ASSERT_EQ(text, prim->getParent()); + ASSERT_EQ(expected, prim->getContent()); +} + TEST(XmlParser, documentParsing) { XmlStandaloneEnvironment env(logger); - Rooted book_domain_node = + Rooted book_document_node = env.parse("simple_book.oxd", "", "", RttiSet{&RttiTypes::Document}); - //TODO: Check result + ASSERT_FALSE(book_document_node == nullptr); + ASSERT_TRUE(book_document_node->isa(&RttiTypes::Document)); + Rooted doc = book_document_node.cast(); + ASSERT_TRUE(doc->validate(logger)); + checkStructuredEntity(doc->getRoot(), doc, doc, "book"); + { + Rooted book = doc->getRoot(); + ASSERT_EQ(2, book->getField().size()); + checkText(book->getField()[0], book, doc, + "This might be some introductory text or a dedication."); + checkStructuredEntity(book->getField()[1], book, doc, "chapter", + Variant::mapType{}, "myFirstChapter"); + { + Rooted chapter = + book->getField()[1].cast(); + ASSERT_EQ(3, chapter->getField().size()); + checkText(chapter->getField()[0], chapter, doc, + "Here we might have an introduction to the chapter."); + checkStructuredEntity(chapter->getField()[1], chapter, doc, + "section", Variant::mapType{}, + "myFirstSection"); + { + Rooted section = + chapter->getField()[1].cast(); + ASSERT_EQ(1, section->getField().size()); + checkText(section->getField()[0], section, doc, + "Here we might find the actual section content."); + } + checkStructuredEntity(chapter->getField()[2], chapter, doc, + "section", Variant::mapType{}, + "mySndSection"); + { + Rooted section = + chapter->getField()[2].cast(); + ASSERT_EQ(1, section->getField().size()); + checkText(section->getField()[0], section, doc, + "Here we might find the actual section content."); + } + } + } } } -- cgit v1.2.3 From f812e01570aedd5033245a76846b5afc0063bc17 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Wed, 11 Feb 2015 17:51:50 +0100 Subject: made isSubtree (fieldType) and primitivity orthogonal concepts: PRIMITIVE is no FieldType anymore. --- src/core/model/Domain.cpp | 115 ++++++++++++++++++++------------- src/core/model/Domain.hpp | 37 +++++------ src/plugins/xml/XmlParser.cpp | 15 ++++- test/core/model/DocumentTest.cpp | 38 ++++++----- test/core/model/DomainTest.cpp | 66 ++++++++++--------- test/core/model/TestAdvanced.hpp | 4 +- test/core/model/TestDomain.hpp | 18 +++--- test/plugins/xml/XmlParserTest.cpp | 1 + testdata/xmlparser/comments_domain.oxm | 6 +- 9 files changed, 168 insertions(+), 132 deletions(-) (limited to 'test') diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp index 806b9b8..787f1ff 100644 --- a/src/core/model/Domain.cpp +++ b/src/core/model/Domain.cpp @@ -27,18 +27,16 @@ namespace ousia { /* Class FieldDescriptor */ -FieldDescriptor::FieldDescriptor(Manager &mgr, Handle parent, - Handle primitiveType, std::string name, - bool optional) +FieldDescriptor::FieldDescriptor(Manager &mgr, Handle primitiveType, + Handle parent, FieldType fieldType, + std::string name, bool optional) : Node(mgr, std::move(name), parent), children(this), - fieldType(FieldType::PRIMITIVE), + fieldType(fieldType), primitiveType(acquire(primitiveType)), - optional(optional) + optional(optional), + primitive(true) { - if (parent != nullptr) { - parent->addFieldDescriptor(this); - } } FieldDescriptor::FieldDescriptor(Manager &mgr, Handle parent, @@ -47,11 +45,9 @@ FieldDescriptor::FieldDescriptor(Manager &mgr, Handle parent, : Node(mgr, std::move(name), parent), children(this), fieldType(fieldType), - optional(optional) + optional(optional), + primitive(false) { - if (parent != nullptr) { - parent->addFieldDescriptor(this); - } } bool FieldDescriptor::doValidate(Logger &logger) const @@ -59,45 +55,63 @@ bool FieldDescriptor::doValidate(Logger &logger) const bool valid = true; // check parent type if (getParent() == nullptr) { - logger.error("This field has no parent!", *this); + logger.error(std::string("Field \"") + getName() + "\" has no parent!", + *this); valid = false; } else if (!getParent()->isa(&RttiTypes::Descriptor)) { - logger.error("The parent of this field is not a descriptor!", *this); + logger.error(std::string("The parent of Field \"") + getName() + + "\" is not a descriptor!", + *this); valid = false; } // check name - if (getName() != DEFAULT_FIELD_NAME) { + if (getName().empty()) { + if (fieldType != FieldType::TREE) { + logger.error(std::string("Field \"") + getName() + + "\" is not the main field but has an empty name!", + *this); + valid = false; + } + } else { valid = valid & validateName(logger); } + // check consistency of FieldType with the rest of the FieldDescriptor. - if (fieldType == FieldType::PRIMITIVE) { + if (primitive) { if (children.size() > 0) { - logger.error( - "This field is supposed to be primitive but has " - "registered child classes!", - *this); + logger.error(std::string("Field \"") + getName() + + "\" is supposed to be primitive but has " + "registered child classes!", + *this); valid = false; } if (primitiveType == nullptr) { - logger.error( - "This field is supposed to be primitive but has " - "no primitive type!", - *this); + logger.error(std::string("Field \"") + getName() + + "\" is supposed to be primitive but has " + "no primitive type!", + *this); valid = false; } } else { if (primitiveType != nullptr) { - logger.error( - "This field is supposed to be non-primitive but has " - "a primitive type!", - *this); + logger.error(std::string("Field \"") + getName() + + "\" is supposed to be non-primitive but has " + "a primitive type!", + *this); + valid = false; + } + // if this is not a primitive field we require at least one child. + if (children.empty()) { + logger.error(std::string("Field \"") + getName() + + "\" is non primitive but does not allow children!", + *this); valid = false; } } /* * we are not allowed to call the validation functions of each child because * this might lead to cycles. What we should do, however, is to check if - * there are no duplicates. + * there are duplicates. */ std::set names; for (Handle c : children) { @@ -140,10 +154,14 @@ bool Descriptor::doValidate(Logger &logger) const bool valid = true; // check parent type if (getParent() == nullptr) { - logger.error("This Descriptor has no parent!", *this); + logger.error( + std::string("Descriptor \"") + getName() + "\" has no parent!", + *this); valid = false; } else if (!getParent()->isa(&RttiTypes::Domain)) { - logger.error("The parent of this Descriptor is not a Domain!", *this); + logger.error(std::string("The parent of Descriptor \"") + getName() + + "\" is not a Domain!", + *this); valid = false; } // check name @@ -305,28 +323,26 @@ void Descriptor::moveFieldDescriptor(Handle fd) } } -void Descriptor::copyFieldDescriptor(Handle fd) +void Descriptor::copyFieldDescriptor(Handle fd, Logger &logger) { - if (fd->getFieldType() == FieldDescriptor::FieldType::PRIMITIVE) { - /* - * To call the "new" operation is enough here, because the - * constructor will add the newly constructed FieldDescriptor to this - * Descriptor automatically. - */ - new FieldDescriptor(getManager(), this, fd->getPrimitiveType(), - fd->getName(), fd->isOptional()); + Rooted copy; + if (fd->isPrimitive()) { + copy = Rooted{new FieldDescriptor( + getManager(), fd->getPrimitiveType(), this, fd->getFieldType(), + fd->getName(), fd->isOptional())}; } else { /* * In case of non-primitive FieldDescriptors we also want to copy the * child references. */ - Rooted copy = { + copy = Rooted{ new FieldDescriptor(getManager(), this, fd->getFieldType(), fd->getName(), fd->isOptional())}; for (auto &c : fd->getChildren()) { copy->addChild(c); } } + addFieldDescriptor(copy, logger); } bool Descriptor::removeFieldDescriptor(Handle fd) @@ -342,17 +358,24 @@ bool Descriptor::removeFieldDescriptor(Handle fd) } Rooted Descriptor::createPrimitiveFieldDescriptor( - Handle primitiveType, std::string name, bool optional) + Handle primitiveType, Logger &logger, + FieldDescriptor::FieldType fieldType, std::string name, bool optional) { - return Rooted{new FieldDescriptor( - getManager(), this, primitiveType, std::move(name), optional)}; + Rooted fd{new FieldDescriptor(getManager(), primitiveType, + this, fieldType, + std::move(name), optional)}; + addFieldDescriptor(fd, logger); + return fd; } Rooted Descriptor::createFieldDescriptor( - FieldDescriptor::FieldType fieldType, std::string name, bool optional) + Logger &logger, FieldDescriptor::FieldType fieldType, std::string name, + bool optional) { - return Rooted{new FieldDescriptor( + Rooted fd{new FieldDescriptor( getManager(), this, fieldType, std::move(name), optional)}; + addFieldDescriptor(fd, logger); + return fd; } /* Class StructuredClass */ diff --git a/src/core/model/Domain.hpp b/src/core/model/Domain.hpp index 24199b1..241c25d 100644 --- a/src/core/model/Domain.hpp +++ b/src/core/model/Domain.hpp @@ -253,32 +253,26 @@ class FieldDescriptor : public Node { public: /** * This enum class contains all possible FieldTypes, meaning either the - * main structure beneath this Descritor (TREE), supporting structure - * (SUBTREE) or a primitive terminal (PRIMITIVE). + * main structure beneath this Descriptor (TREE) or supporting structure + * (SUBTREE) * - * Note the following rules (which are also mentioned above): - * 1.) There may be only one TREE field in a Descriptor. - * 2.) Each TREE field must allow for at least one child, which in turn has - * either a TREE field or a PRIMITIVE field. - * 3.) SUBTREE fields may not allow for children with TREE fields. - * 4.) SUBTREE fields must allow for at least one child with another SUBTREE - * or PRIMITIVE field. + * Note that there may be only one TREE field in a descriptor. */ - enum class FieldType { TREE, SUBTREE, PRIMITIVE }; + enum class FieldType { TREE, SUBTREE }; private: NodeVector children; FieldType fieldType; Owned primitiveType; bool optional; + bool primitive; protected: bool doValidate(Logger &logger) const override; public: /** - * This is the constructor for primitive fields. The type is automatically - * set to "PRIMITIVE". + * This is the constructor for primitive fields. * * @param mgr is the global Manager instance. * @param parent is a handle of the Descriptor node that has this @@ -290,10 +284,10 @@ public: * filled in order for an instance of the parent * Descriptor to be valid. */ - FieldDescriptor(Manager &mgr, Handle parent, - Handle primitiveType, - std::string name = DEFAULT_FIELD_NAME, - bool optional = false); + FieldDescriptor(Manager &mgr, Handle primitiveType, + Handle parent, + FieldType fieldType = FieldType::TREE, + std::string name = "", bool optional = false); /** * This is the constructor for non-primitive fields. You have to provide @@ -312,8 +306,7 @@ public: */ FieldDescriptor(Manager &mgr, Handle parent = nullptr, FieldType fieldType = FieldType::TREE, - std::string name = DEFAULT_FIELD_NAME, - bool optional = false); + std::string name = "", bool optional = false); /** * Returns a const reference to the NodeVector of StructuredClasses whose @@ -616,8 +609,9 @@ public: * @return the newly created FieldDescriptor. */ Rooted createPrimitiveFieldDescriptor( - Handle primitiveType, std::string name = DEFAULT_FIELD_NAME, - bool optional = false); + Handle primitiveType, Logger &logger, + FieldDescriptor::FieldType fieldType = FieldDescriptor::FieldType::TREE, + std::string name = "", bool optional = false); /** * This creates a new primitive FieldDescriptor and adds it to this @@ -634,8 +628,9 @@ public: * @return the newly created FieldDescriptor. */ Rooted createFieldDescriptor( + Logger &logger, FieldDescriptor::FieldType fieldType = FieldDescriptor::FieldType::TREE, - std::string name = DEFAULT_FIELD_NAME, bool optional = false); + std::string name = "", bool optional = false); /** * This tries to construct the shortest possible path of this Descriptor diff --git a/src/plugins/xml/XmlParser.cpp b/src/plugins/xml/XmlParser.cpp index e8e97e2..22498f4 100644 --- a/src/plugins/xml/XmlParser.cpp +++ b/src/plugins/xml/XmlParser.cpp @@ -575,7 +575,7 @@ public: Rooted parent = scope().selectOrThrow(); Rooted field = parent->createFieldDescriptor( - type, args["name"].asString(), args["optional"].asBool()); + logger(), type, args["name"].asString(), args["optional"].asBool()); field->setLocation(location()); scope().push(field); @@ -624,8 +624,16 @@ public: { Rooted parent = scope().selectOrThrow(); + FieldDescriptor::FieldType fieldType; + if (args["isSubtree"].asBool()) { + fieldType = FieldDescriptor::FieldType::SUBTREE; + } else { + fieldType = FieldDescriptor::FieldType::TREE; + } + Rooted field = parent->createPrimitiveFieldDescriptor( - nullptr, args["name"].asString(), args["optional"].asBool()); + nullptr, logger(), fieldType, args["name"].asString(), + args["optional"].asBool()); field->setLocation(location()); const std::string &type = args["type"].asString(); @@ -742,7 +750,7 @@ public: if (parent != nullptr) { Rooted field = parent.cast()->createFieldDescriptor( - type, name, optional); + logger, type, name, optional); field->addChild(strct.cast()); } }); @@ -978,6 +986,7 @@ static const ParserState DomainStructPrimitive = .createdNodeType(&RttiTypes::FieldDescriptor) .elementHandler(DomainPrimitiveHandler::create) .arguments({Argument::String("name", DEFAULT_FIELD_NAME), + Argument::Bool("isSubtree", false), Argument::Bool("optional", false), Argument::String("type")}); diff --git a/test/core/model/DocumentTest.cpp b/test/core/model/DocumentTest.cpp index 3164b7e..5acf13f 100644 --- a/test/core/model/DocumentTest.cpp +++ b/test/core/model/DocumentTest.cpp @@ -67,11 +67,9 @@ TEST(Document, construct) ASSERT_EQ("text", text->getDescriptor()->getName()); ASSERT_TRUE(text->getDescriptor()->hasField()); ASSERT_EQ(1U, text->getField().size()); - ASSERT_TRUE( - text->getField()[0]->isa(typeOf())); - Variant content = text->getField()[0] - .cast() - ->getContent(); + ASSERT_TRUE(text->getField()[0]->isa(typeOf())); + Variant content = + text->getField()[0].cast()->getContent(); ASSERT_EQ("Some introductory text", content.asString()); } } @@ -101,11 +99,10 @@ TEST(Document, construct) ASSERT_EQ("text", text->getDescriptor()->getName()); ASSERT_TRUE(text->getDescriptor()->hasField()); ASSERT_EQ(1U, text->getField().size()); - ASSERT_TRUE(text->getField()[0]->isa( - typeOf())); - Variant content = text->getField()[0] - .cast() - ->getContent(); + ASSERT_TRUE( + text->getField()[0]->isa(typeOf())); + Variant content = + text->getField()[0].cast()->getContent(); ASSERT_EQ("Some actual text", content.asString()); } } @@ -149,7 +146,8 @@ TEST(Document, validate) } // now let's extend the rootClass with a default field. - Rooted rootField{new FieldDescriptor(mgr, rootClass)}; + Rooted rootField = + rootClass->createFieldDescriptor(logger); // and add a child class for it. Rooted childClass{ new StructuredClass(mgr, "child", domain, single)}; @@ -195,7 +193,8 @@ TEST(Document, validate) * Make it even more complicated: child gets a field for further child * instances now. */ - Rooted childField{new FieldDescriptor(mgr, childClass)}; + Rooted childField = + childClass->createFieldDescriptor(logger); childField->addChild(childClass); { /* @@ -211,10 +210,13 @@ TEST(Document, validate) ASSERT_FALSE(doc->validate(logger)); } /* - * Override the default field in childSubClass. + * Override the default field in childSubClass with an optional field. */ - Rooted childSubField{ - new FieldDescriptor(mgr, childSubClass)}; + Rooted childSubField = + childSubClass->createFieldDescriptor( + logger, FieldDescriptor::FieldType::TREE, "dummy", true); + // add a child pro forma to make it valid. + childSubField->addChild(childSubClass); { /* * Now a document with one instance of the Child subclass should be @@ -229,8 +231,10 @@ TEST(Document, validate) ASSERT_TRUE(doc->validate(logger)); } // add a primitive field to the subclass with integer content. - Rooted primitive_field{new FieldDescriptor( - mgr, childSubClass, sys->getIntType(), "int", false)}; + Rooted primitive_field = + childSubClass->createPrimitiveFieldDescriptor( + sys->getIntType(), logger, FieldDescriptor::FieldType::SUBTREE, + "int", false); { /* * Now a document with one instance of the Child subclass should be diff --git a/test/core/model/DomainTest.cpp b/test/core/model/DomainTest.cpp index 32ef7f0..59062f0 100644 --- a/test/core/model/DomainTest.cpp +++ b/test/core/model/DomainTest.cpp @@ -145,9 +145,10 @@ TEST(Descriptor, pathToAdvanced) * 8.) E is transparent and has target as child in the default field * (longer path) * - * So the path start_field , E , E_field should be returned. + * So the path A_second_field, C, C_field should be returned. */ Manager mgr{1}; + Logger logger; Rooted sys{new SystemTypesystem(mgr)}; // Construct the domain Rooted domain{new Domain(mgr, sys, "nasty")}; @@ -162,8 +163,8 @@ TEST(Descriptor, pathToAdvanced) Rooted B{new StructuredClass( mgr, "B", domain, Cardinality::any(), {nullptr}, true, false)}; - Rooted C{ - new StructuredClass(mgr, "C", domain, Cardinality::any(), B, true, false)}; + Rooted C{new StructuredClass( + mgr, "C", domain, Cardinality::any(), B, true, false)}; Rooted D{new StructuredClass( mgr, "D", domain, Cardinality::any(), {nullptr}, true, false)}; @@ -175,31 +176,32 @@ TEST(Descriptor, pathToAdvanced) new StructuredClass(mgr, "target", domain, Cardinality::any())}; // We create two fields for A - Rooted A_field{new FieldDescriptor(mgr, A)}; + Rooted A_field = A->createFieldDescriptor(logger); A_field->addChild(target); - Rooted A_field2{new FieldDescriptor( - mgr, A, FieldDescriptor::FieldType::SUBTREE, "second")}; + Rooted A_field2 = A->createFieldDescriptor( + logger, FieldDescriptor::FieldType::SUBTREE, "second", false); A_field2->addChild(B); // We create no field for B // One for C - Rooted C_field{new FieldDescriptor(mgr, C)}; + Rooted C_field = C->createFieldDescriptor(logger); C_field->addChild(target); // one for start - Rooted start_field{new FieldDescriptor(mgr, start)}; + Rooted start_field = start->createFieldDescriptor(logger); start_field->addChild(D); // One for D - Rooted D_field{new FieldDescriptor(mgr, D)}; + Rooted D_field = D->createFieldDescriptor(logger); D_field->addChild(E); // One for E - Rooted E_field{new FieldDescriptor(mgr, E)}; + Rooted E_field = E->createFieldDescriptor(logger); E_field->addChild(target); + ASSERT_TRUE(domain->validate(logger)); #ifdef MANAGER_GRAPHVIZ_EXPORT // dump the manager state mgr.exportGraphviz("nastyDomain.dot"); @@ -313,8 +315,8 @@ TEST(Domain, validate) ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); ASSERT_TRUE(domain->validate(logger)); // Let's add a primitive field (without a primitive type at first) - Rooted base_field{ - new FieldDescriptor(mgr, base, nullptr)}; + Rooted base_field = + base->createPrimitiveFieldDescriptor(nullptr, logger); // this should not be valid. ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); ASSERT_FALSE(domain->validate(logger)); @@ -322,13 +324,6 @@ TEST(Domain, validate) base_field->setPrimitiveType(sys->getStringType()); ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); ASSERT_TRUE(domain->validate(logger)); - // not anymore, however, if we tamper with the FieldType. - base_field->setFieldType(FieldDescriptor::FieldType::TREE); - ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); - ASSERT_FALSE(domain->validate(logger)); - base_field->setFieldType(FieldDescriptor::FieldType::PRIMITIVE); - ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); - ASSERT_TRUE(domain->validate(logger)); // add a subclass for our base class. Rooted sub{new StructuredClass(mgr, "sub", domain)}; // this should be valid in itself. @@ -338,7 +333,7 @@ TEST(Domain, validate) sub->setSuperclass(base, logger); ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); ASSERT_TRUE(domain->validate(logger)); - // and still we we remove the subclass from the base class. + // and still if we remove the subclass from the base class. base->removeSubclass(sub, logger); ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); ASSERT_TRUE(domain->validate(logger)); @@ -349,19 +344,11 @@ TEST(Domain, validate) ASSERT_TRUE(domain->validate(logger)); ASSERT_EQ(base, sub->getSuperclass()); // add a non-primitive field to the child class. - Rooted sub_field{new FieldDescriptor(mgr, sub)}; - // this should be valid - ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); - ASSERT_TRUE(domain->validate(logger)); - // .. until we set a primitive type. - sub_field->setPrimitiveType(sys->getStringType()); + Rooted sub_field = sub->createFieldDescriptor(logger); + // this should not be valid because we allow no children. ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); ASSERT_FALSE(domain->validate(logger)); - // and valid again if we unset it. - sub_field->setPrimitiveType(nullptr); - ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); - ASSERT_TRUE(domain->validate(logger)); - // we should also be able to add a child and have it still be valid. + // we should also be able to add a child and make it valid. sub_field->addChild(base); ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); ASSERT_TRUE(domain->validate(logger)); @@ -373,6 +360,23 @@ TEST(Domain, validate) sub_field->removeChild(base); ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); ASSERT_TRUE(domain->validate(logger)); + // if we set a primitive type it should be invalid + sub_field->setPrimitiveType(sys->getStringType()); + ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); + ASSERT_FALSE(domain->validate(logger)); + // and valid again if we unset it. + sub_field->setPrimitiveType(nullptr); + ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); + ASSERT_TRUE(domain->validate(logger)); + // It should be invalid if we set another TREE field. + Rooted sub_field2 = sub->createFieldDescriptor( + logger, FieldDescriptor::FieldType::TREE, "test", false); + ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); + ASSERT_FALSE(domain->validate(logger)); + // but valid again if we remove it + sub->removeFieldDescriptor(sub_field2); + ASSERT_EQ(ValidationState::UNKNOWN, domain->getValidationState()); + ASSERT_TRUE(domain->validate(logger)); } } } diff --git a/test/core/model/TestAdvanced.hpp b/test/core/model/TestAdvanced.hpp index 915d973..9c95400 100644 --- a/test/core/model/TestAdvanced.hpp +++ b/test/core/model/TestAdvanced.hpp @@ -65,8 +65,8 @@ static Rooted constructHeadingDomain(Manager &mgr, "paragraph"}; for (auto &s : secclasses) { Rooted desc = resolveDescriptor(bookDomain, s); - Rooted heading_field{new FieldDescriptor( - mgr, desc, FieldDescriptor::FieldType::SUBTREE, "heading")}; + Rooted heading_field = desc->createFieldDescriptor( + logger, FieldDescriptor::FieldType::SUBTREE, "heading", true); heading_field->addChild(heading); } return domain; diff --git a/test/core/model/TestDomain.hpp b/test/core/model/TestDomain.hpp index 5ac510c..c107a0d 100644 --- a/test/core/model/TestDomain.hpp +++ b/test/core/model/TestDomain.hpp @@ -42,7 +42,7 @@ static Rooted constructBookDomain(Manager &mgr, mgr, "book", domain, single, {nullptr}, false, true)}; // The structure field of it. - Rooted book_field{new FieldDescriptor(mgr, book)}; + Rooted book_field = book->createFieldDescriptor(logger); // From there on the "section". Rooted section{ @@ -50,7 +50,8 @@ static Rooted constructBookDomain(Manager &mgr, book_field->addChild(section); // And the field of it. - Rooted section_field{new FieldDescriptor(mgr, section)}; + Rooted section_field = + section->createFieldDescriptor(logger); // We also add the "paragraph", which is transparent. Rooted paragraph{new StructuredClass( @@ -59,8 +60,8 @@ static Rooted constructBookDomain(Manager &mgr, book_field->addChild(paragraph); // And the field of it. - Rooted paragraph_field{ - new FieldDescriptor(mgr, paragraph)}; + Rooted paragraph_field = + paragraph->createFieldDescriptor(logger); // We append "subsection" to section. Rooted subsection{ @@ -68,8 +69,8 @@ static Rooted constructBookDomain(Manager &mgr, section_field->addChild(subsection); // And the field of it. - Rooted subsection_field{ - new FieldDescriptor(mgr, subsection)}; + Rooted subsection_field = + subsection->createFieldDescriptor(logger); // and we add the paragraph to subsections fields subsection_field->addChild(paragraph); @@ -80,9 +81,8 @@ static Rooted constructBookDomain(Manager &mgr, paragraph_field->addChild(text); // ... and has a primitive field. - Rooted text_field{new FieldDescriptor( - mgr, text, domain->getTypesystems()[0]->getTypes()[0], - DEFAULT_FIELD_NAME, false)}; + Rooted text_field = + text->createPrimitiveFieldDescriptor(sys->getStringType(), logger); return domain; } diff --git a/test/plugins/xml/XmlParserTest.cpp b/test/plugins/xml/XmlParserTest.cpp index 067214c..ef95552 100644 --- a/test/plugins/xml/XmlParserTest.cpp +++ b/test/plugins/xml/XmlParserTest.cpp @@ -169,6 +169,7 @@ static void checkFieldDescriptor( ASSERT_EQ(parent, field->getParent()); ASSERT_EQ(type, field->getFieldType()); ASSERT_EQ(primitiveType, field->getPrimitiveType()); + ASSERT_EQ(primitiveType != nullptr, field->isPrimitive()); ASSERT_EQ(optional, field->isOptional()); // check the children. ASSERT_EQ(children.size(), field->getChildren().size()); diff --git a/testdata/xmlparser/comments_domain.oxm b/testdata/xmlparser/comments_domain.oxm index 2a3ad59..4d8c531 100644 --- a/testdata/xmlparser/comments_domain.oxm +++ b/testdata/xmlparser/comments_domain.oxm @@ -4,7 +4,7 @@ - + @@ -17,7 +17,7 @@ - + @@ -30,7 +30,7 @@ - + -- cgit v1.2.3 From 2f75ac166594b6bc2ea30901669304eca23174ec Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Wed, 11 Feb 2015 17:54:48 +0100 Subject: changed semantics of default field, now referring to the only TREE field. --- src/core/model/Domain.cpp | 133 +++++++++++++++----- src/core/model/Domain.hpp | 246 +++++++++++++------------------------ src/plugins/html/DemoOutput.cpp | 2 +- src/plugins/xml/XmlParser.cpp | 8 +- test/core/model/DomainTest.cpp | 2 +- test/core/model/TestAdvanced.hpp | 14 +-- test/plugins/xml/XmlParserTest.cpp | 6 +- 7 files changed, 209 insertions(+), 202 deletions(-) (limited to 'test') diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp index 787f1ff..3fb525f 100644 --- a/src/core/model/Domain.cpp +++ b/src/core/model/Domain.cpp @@ -173,20 +173,38 @@ bool Descriptor::doValidate(Logger &logger) const } // ensure that no attribute with the key "name" exists. if (attributesDescriptor == nullptr) { - logger.error("This Descriptor has no Attribute specification!"); + logger.error(std::string("Descriptor \"") + getName() + + "\" has no Attribute specification!"); valid = false; } else { if (attributesDescriptor->hasAttribute("name")) { logger.error( - "This Descriptor has an attribute \"name\" which is a reserved " - "word!"); + std::string("Descriptor \"") + getName() + + "\" has an attribute \"name\" which is a reserved word!"); valid = false; } valid = valid & attributesDescriptor->validate(logger); } + // check that only one FieldDescriptor is of type TREE. + auto fds = Descriptor::getFieldDescriptors(); + bool hasTREE = false; + for (auto fd : fds) { + if (fd->getFieldType() == FieldDescriptor::FieldType::TREE) { + if (!hasTREE) { + hasTREE = true; + } else { + logger.error( + std::string("Descriptor \"") + getName() + + "\" has multiple TREE fields, which is not permitted", + *fd); + valid = false; + break; + } + } + } // check attributes and the FieldDescriptors - return valid & continueValidationCheckDuplicates(fieldDescriptors, logger); + return valid & continueValidationCheckDuplicates(fds, logger); } std::vector> Descriptor::pathTo( @@ -259,18 +277,39 @@ bool Descriptor::continuePath(Handle target, return found; } -ssize_t Descriptor::getFieldDescriptorIndex(const std::string &name) const +static ssize_t getFieldDescriptorIndex(const NodeVector &fds, + const std::string &name) { - size_t f = 0; - for (auto &fd : getFieldDescriptors()) { - if (fd->getName() == name) { + if (fds.empty()) { + return -1; + } + + if (name == DEFAULT_FIELD_NAME) { + if (fds.back()->getFieldType() == FieldDescriptor::FieldType::TREE) { + return fds.size() - 1; + } else { + /* The last field has to be the TREE field. If the last field does + * not have the FieldType TREE no TREE-field exists at all. So we + * return -1. + */ + return -1; + } + } + + for (size_t f = 0; f < fds.size(); f++) { + if (fds[f]->getName() == name) { return f; } - f++; } return -1; } +ssize_t Descriptor::getFieldDescriptorIndex(const std::string &name) const +{ + NodeVector fds = getFieldDescriptors(); + return ousia::getFieldDescriptorIndex(fds, name); +} + ssize_t Descriptor::getFieldDescriptorIndex(Handle fd) const { size_t f = 0; @@ -286,33 +325,54 @@ ssize_t Descriptor::getFieldDescriptorIndex(Handle fd) const Rooted Descriptor::getFieldDescriptor( const std::string &name) const { - for (auto &fd : getFieldDescriptors()) { - if (fd->getName() == name) { - return fd; - } + NodeVector fds = getFieldDescriptors(); + ssize_t idx = ousia::getFieldDescriptorIndex(fds, name); + if (idx != -1) { + return fds[idx]; + } else { + return nullptr; } - return nullptr; } -void Descriptor::addFieldDescriptor(Handle fd) +void Descriptor::addAndSortFieldDescriptor(Handle fd, + Logger &logger) { // only add it if we need to. - if (fieldDescriptors.find(fd) == fieldDescriptors.end()) { + auto fds = getFieldDescriptors(); + if (fds.find(fd) == fds.end()) { invalidate(); - fieldDescriptors.push_back(fd); + // check if the previous field is a tree field already. + if (!fds.empty() && + fds.back()->getFieldType() == FieldDescriptor::FieldType::TREE && + fd->getFieldType() != FieldDescriptor::FieldType::TREE) { + // if so we add the new field before the TREE field and log a + // warning. + + logger.warning( + std::string("Field \"") + fd->getName() + + "\" was declared after main field \"" + + fds.back()->getName() + + "\". The order of fields was changed to make the " + "main field the last field.", + *fd); + fieldDescriptors.insert(fieldDescriptors.end() - 1, fd); + } else { + fieldDescriptors.push_back(fd); + } } +} + +void Descriptor::addFieldDescriptor(Handle fd, Logger &logger) +{ + addAndSortFieldDescriptor(fd, logger); if (fd->getParent() == nullptr) { fd->setParent(this); } } -void Descriptor::moveFieldDescriptor(Handle fd) +void Descriptor::moveFieldDescriptor(Handle fd, Logger &logger) { - // only add it if we need to. - if (fieldDescriptors.find(fd) == fieldDescriptors.end()) { - invalidate(); - fieldDescriptors.push_back(fd); - } + addAndSortFieldDescriptor(fd, logger); Handle par = fd->getParent(); if (par != this) { if (par != nullptr) { @@ -494,18 +554,35 @@ void StructuredClass::removeSubclass(Handle sc, Logger &logger) sc->setSuperclass(nullptr, logger); } -const void StructuredClass::gatherFieldDescriptors( +void StructuredClass::gatherFieldDescriptors( NodeVector ¤t, - std::set &overriddenFields) const + std::set &overriddenFields, bool hasTREE) const { // append all FieldDescriptors that are not overridden. for (auto &f : Descriptor::getFieldDescriptors()) { if (overriddenFields.insert(f->getName()).second) { - current.push_back(f); + bool isTREE = f->getFieldType() == FieldDescriptor::FieldType::TREE; + if (hasTREE) { + if (!isTREE) { + /* + * If we already have a tree field it has to be at the end + * of the current vector. So ensure that all new non-TREE + * fields are inserted before the TREE field such that after + * this method the TREE field is still at the end. + */ + current.insert(current.end() - 1, f); + } + } else { + if (isTREE) { + hasTREE = true; + } + current.push_back(f); + } } } + // if we have a superclass, go there. if (superclass != nullptr) { - superclass->gatherFieldDescriptors(current, overriddenFields); + superclass->gatherFieldDescriptors(current, overriddenFields, hasTREE); } } @@ -514,7 +591,7 @@ NodeVector StructuredClass::getFieldDescriptors() const // in this case we return a NodeVector of Rooted entries without owner. NodeVector vec; std::set overriddenFields; - gatherFieldDescriptors(vec, overriddenFields); + gatherFieldDescriptors(vec, overriddenFields, false); return vec; } diff --git a/src/core/model/Domain.hpp b/src/core/model/Domain.hpp index 241c25d..43661c2 100644 --- a/src/core/model/Domain.hpp +++ b/src/core/model/Domain.hpp @@ -34,74 +34,46 @@ * * \code{.xml} * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * * * \endcode * * Note that we define one field as the TREE (meaning the main or default * document structure) and one mearly as SUBTREE, relating to supporting * information. You are not allowed to define more than one field of type - * "TREE". Accordingly for each StructuredClass in the main TREE there must be - * at least one possible primitive child or one TREE field. Otherwise the - * grammar would be nonterminal. For SUBTREE fields no children may define a - * TREE field and at least one permitted child must exist, either primitive or - * as another StructuredClass. + * "TREE". * - * The translation to context free grammars is as follows: + * The translation to a context free grammar is as follows: * * \code{.txt} * BOOK := BOOK_TREE @@ -128,21 +100,14 @@ * * \code{.xml} * - * - * - * - * - * - * - * - * - * - * ... - * - * - * - * - * + * + * + * + * + * + * ... + * + * * * \endcode * @@ -161,36 +126,36 @@ * * \code{.xml} * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * * * \endcode * @@ -227,25 +192,15 @@ static const std::string DEFAULT_FIELD_NAME = "$default"; * accordingly typed content without further descending in the Structure * Hierarchy. * - * As an example consider the "paragraph" StructuredClass, which might allow + * As an example consider the "text" StructuredClass, which might allow * the actual text content. Here is the according XML: * * \code{.xml} - * - * - * - * - * - * - * - * - * + * + * + * * \endcode * - * Accordingly the primitiveType field of a FieldDescriptor may only be - * defined if the type is set to "PRIMITIVE". If the type is something else - * at least one child must be defined and the primitiveType remains in an - * undefined state. */ class FieldDescriptor : public Node { friend Descriptor; @@ -370,11 +325,11 @@ public: } /** - * Returns true if and only if the type of this field is PRIMITIVE. + * Returns if this field is primitive. * - * @return true if and only if the type of this field is PRIMITIVE. + * @return true if and only if this field is primitive. */ - bool isPrimitive() const { return fieldType == FieldType::PRIMITIVE; } + bool isPrimitive() const { return primitive; } /** * Returns the primitive type of this field, which is only allowed to be @@ -449,8 +404,10 @@ private: Owned attributesDescriptor; NodeVector fieldDescriptors; - bool continuePath(Handle target, - std::vector> &path) const; + bool continuePath(Handle target, NodeVector &path, + bool start) const; + + void addAndSortFieldDescriptor(Handle fd, Logger &logger); protected: void doResolve(ResolutionState &state) override; @@ -539,20 +496,7 @@ public: * * @param fd is a FieldDescriptor. */ - void addFieldDescriptor(Handle fd); - - /** - * Adds the given FieldDescriptors to this Descriptor. This also sets the - * parent of each given FieldDescriptor if it is not set yet. - * - * @param fds are FieldDescriptors. - */ - void addFieldDescriptors(const std::vector> &fds) - { - for (Handle fd : fds) { - addFieldDescriptor(fd); - } - } + void addFieldDescriptor(Handle fd, Logger &logger); /** * Adds the given FieldDescriptor to this Descriptor. This also sets the @@ -561,21 +505,7 @@ public: * * @param fd is a FieldDescriptor. */ - void moveFieldDescriptor(Handle fd); - - /** - * Adds the given FieldDescriptors to this Descriptor. This also sets the - * parent of each given FieldDescriptor if it is not set to this Descriptor - * already and removes it from the old parent Descriptor. - * - * @param fds are FieldDescriptors. - */ - void moveFieldDescriptors(const std::vector> &fds) - { - for (Handle fd : fds) { - moveFieldDescriptor(fd); - } - } + void moveFieldDescriptor(Handle fd, Logger &logger); /** * Copies a FieldDescriptor that belongs to another Descriptor to this @@ -583,7 +513,7 @@ public: * * @param fd some FieldDescriptor belonging to another Descriptor. */ - void copyFieldDescriptor(Handle fd); + void copyFieldDescriptor(Handle fd, Logger &logger); /** * Removes the given FieldDescriptor from this Descriptor. This also sets @@ -751,9 +681,9 @@ private: /** * Helper method for getFieldDescriptors. */ - const void gatherFieldDescriptors( - NodeVector ¤t, - std::set &overriddenFields) const; + void gatherFieldDescriptors(NodeVector ¤t, + std::set &overriddenFields, + bool hasTREE) const; protected: bool doValidate(Logger &logger) const override; diff --git a/src/plugins/html/DemoOutput.cpp b/src/plugins/html/DemoOutput.cpp index d041c1d..cb34cbe 100644 --- a/src/plugins/html/DemoOutput.cpp +++ b/src/plugins/html/DemoOutput.cpp @@ -324,7 +324,7 @@ Rooted DemoHTMLTransformer::transformParagraph( if (childDescriptorName == "text") { Handle primitive = t->getField()[0].cast(); - if (primitive.isNull()) { + if (primitive == nullptr) { throw OusiaException("Text field is not primitive!"); } current->addChild(new xml::Text( diff --git a/src/plugins/xml/XmlParser.cpp b/src/plugins/xml/XmlParser.cpp index 22498f4..d7efa4d 100644 --- a/src/plugins/xml/XmlParser.cpp +++ b/src/plugins/xml/XmlParser.cpp @@ -603,7 +603,7 @@ public: [](Handle field, Handle parent, Logger &logger) { if (field != nullptr) { parent.cast()->addFieldDescriptor( - field.cast()); + field.cast(), logger); } }); } @@ -969,7 +969,7 @@ static const ParserState DomainField = .parents({&DomainStruct, &DomainAnnotation}) .createdNodeType(&RttiTypes::FieldDescriptor) .elementHandler(DomainFieldHandler::create) - .arguments({Argument::String("name", DEFAULT_FIELD_NAME), + .arguments({Argument::String("name", ""), Argument::Bool("isSubtree", false), Argument::Bool("optional", false)}); @@ -985,7 +985,7 @@ static const ParserState DomainStructPrimitive = .parents({&DomainStruct, &DomainAnnotation}) .createdNodeType(&RttiTypes::FieldDescriptor) .elementHandler(DomainPrimitiveHandler::create) - .arguments({Argument::String("name", DEFAULT_FIELD_NAME), + .arguments({Argument::String("name", ""), Argument::Bool("isSubtree", false), Argument::Bool("optional", false), Argument::String("type")}); @@ -1008,7 +1008,7 @@ static const ParserState DomainStructParentField = .parent(&DomainStructParent) .createdNodeType(&RttiTypes::FieldDescriptor) .elementHandler(DomainParentFieldHandler::create) - .arguments({Argument::String("name", DEFAULT_FIELD_NAME), + .arguments({Argument::String("name", ""), Argument::Bool("isSubtree", false), Argument::Bool("optional", false)}); diff --git a/test/core/model/DomainTest.cpp b/test/core/model/DomainTest.cpp index 59062f0..79b62f0 100644 --- a/test/core/model/DomainTest.cpp +++ b/test/core/model/DomainTest.cpp @@ -215,7 +215,7 @@ TEST(Descriptor, pathToAdvanced) ASSERT_TRUE(path[1]->isa(&RttiTypes::StructuredClass)); ASSERT_EQ("B", path[1]->getName()); ASSERT_TRUE(path[2]->isa(&RttiTypes::FieldDescriptor)); - ASSERT_EQ("$default", path[2]->getName()); + ASSERT_EQ("", path[2]->getName()); } TEST(StructuredClass, isSubclassOf) diff --git a/test/core/model/TestAdvanced.hpp b/test/core/model/TestAdvanced.hpp index 9c95400..575860b 100644 --- a/test/core/model/TestAdvanced.hpp +++ b/test/core/model/TestAdvanced.hpp @@ -55,11 +55,11 @@ static Rooted constructHeadingDomain(Manager &mgr, Cardinality card; card.merge({0, 1}); // set up heading StructuredClass. - Rooted heading{new StructuredClass( - mgr, "heading", domain, card, {nullptr}, true)}; - // as field want to copy the field of paragraph. + Rooted heading{ + new StructuredClass(mgr, "heading", domain, card, {nullptr}, true)}; + // as field want to reference the field of paragraph. Rooted p = resolveDescriptor(bookDomain, "paragraph"); - heading->copyFieldDescriptor(p->getFieldDescriptors()[0]); + heading->addFieldDescriptor(p->getFieldDescriptor(), logger); // create a new field for headings in each section type. std::vector secclasses{"book", "section", "subsection", "paragraph"}; @@ -88,8 +88,8 @@ static Rooted constructListDomain(Manager &mgr, Rooted item{new StructuredClass( mgr, "item", domain, Cardinality::any(), {nullptr}, false)}; - // as field we want to copy the field of paragraph. - item->copyFieldDescriptor(p->getFieldDescriptors()[0]); + // as field we want to reference the field of paragraph. + item->addFieldDescriptor(p->getFieldDescriptor(), logger); // set up list StructuredClasses. std::vector listTypes{"ol", "ul"}; for (auto &listType : listTypes) { @@ -157,7 +157,7 @@ static bool addAnnotation(Logger &logger, Handle doc, Handle parent, const std::string &text, const std::string &annoClass) { - Manager& mgr = parent->getManager(); + Manager &mgr = parent->getManager(); Rooted start{new Anchor(mgr, std::to_string(annoIdx++), parent)}; if (!addText(logger, doc, parent, text)) { return false; diff --git a/test/plugins/xml/XmlParserTest.cpp b/test/plugins/xml/XmlParserTest.cpp index ef95552..7d5c697 100644 --- a/test/plugins/xml/XmlParserTest.cpp +++ b/test/plugins/xml/XmlParserTest.cpp @@ -181,7 +181,7 @@ static void checkFieldDescriptor( static void checkFieldDescriptor( Handle desc, Handle parent, NodeVector children, - const std::string &name = DEFAULT_FIELD_NAME, + const std::string &name = "", FieldDescriptor::FieldType type = FieldDescriptor::FieldType::TREE, Handle primitiveType = nullptr, bool optional = false) { @@ -193,7 +193,7 @@ static void checkFieldDescriptor( static void checkFieldDescriptor( Handle desc, NodeVector children, - const std::string &name = DEFAULT_FIELD_NAME, + const std::string &name = "", FieldDescriptor::FieldType type = FieldDescriptor::FieldType::TREE, Handle primitiveType = nullptr, bool optional = false) { @@ -244,7 +244,7 @@ TEST(XmlParser, domainParsing) checkFieldDescriptor(subsection, {paragraph}); checkFieldDescriptor(paragraph, {text}); checkFieldDescriptor( - text, {}, DEFAULT_FIELD_NAME, FieldDescriptor::FieldType::PRIMITIVE, + text, {}, "", FieldDescriptor::FieldType::TREE, env.project->getSystemTypesystem()->getStringType(), false); // check parent handling using the headings domain. -- cgit v1.2.3 From 7daed2f8431e89e2bd041a54bc1bef8c45329092 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Wed, 11 Feb 2015 17:55:04 +0100 Subject: improved pathto --- src/core/model/Domain.cpp | 30 ++++++++++++++++++------------ src/core/model/Domain.hpp | 3 +-- test/core/model/DomainTest.cpp | 7 ++++--- 3 files changed, 23 insertions(+), 17 deletions(-) (limited to 'test') diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp index 3fb525f..228348d 100644 --- a/src/core/model/Domain.cpp +++ b/src/core/model/Domain.cpp @@ -207,16 +207,15 @@ bool Descriptor::doValidate(Logger &logger) const return valid & continueValidationCheckDuplicates(fds, logger); } -std::vector> Descriptor::pathTo( - Handle target) const +NodeVector Descriptor::pathTo(Handle target) const { - std::vector> path; - continuePath(target, path); - return path; + NodeVector path; + continuePath(target, path, true); + return std::move(path); } bool Descriptor::continuePath(Handle target, - std::vector> ¤tPath) const + NodeVector ¤tPath, bool start) const { // check if we are at the target already if (this == target) { @@ -225,27 +224,34 @@ bool Descriptor::continuePath(Handle target, // a variable to determine if we already found a solution bool found = false; // the currently optimal path. - std::vector> optimum; + NodeVector optimum; // use recursive depth-first search from the top to reach the given child // get the list of effective FieldDescriptors. NodeVector fields = getFieldDescriptors(); + Rooted thisHandle{const_cast(this)}; + for (auto &fd : fields) { for (auto &c : fd->getChildren()) { // check if a child is the target node. if (c == target) { // if we have made the connection, stop the search. + if (!start) { + currentPath.push_back(thisHandle); + } currentPath.push_back(fd); return true; } // look for transparent intermediate nodes. if (c->isTransparent()) { // copy the path. - std::vector> cPath = currentPath; + NodeVector cPath = currentPath; + if (!start) { + cPath.push_back(thisHandle); + } cPath.push_back(fd); - cPath.push_back(c); // recursion. - if (c->continuePath(target, cPath) && + if (c->continuePath(target, cPath, false) && (!found || optimum.size() > cPath.size())) { // look if this path is better than the current optimum. optimum = std::move(cPath); @@ -260,8 +266,8 @@ bool Descriptor::continuePath(Handle target, // if this is a StructuredClass we also can call the subclasses. for (auto &c : tis->getSubclasses()) { // copy the path. - std::vector> cPath = currentPath; - if (c->continuePath(target, cPath) && + NodeVector cPath = currentPath; + if (c->continuePath(target, cPath, false) && (!found || optimum.size() > cPath.size())) { // look if this path is better than the current optimum. optimum = std::move(cPath); diff --git a/src/core/model/Domain.hpp b/src/core/model/Domain.hpp index 43661c2..57e5602 100644 --- a/src/core/model/Domain.hpp +++ b/src/core/model/Domain.hpp @@ -589,8 +589,7 @@ public: * no such path can be constructed. * */ - std::vector> pathTo( - Handle childDescriptor) const; + NodeVector pathTo(Handle childDescriptor) const; }; /* * TODO: We should discuss Cardinalities one more time. Is it smart to define diff --git a/test/core/model/DomainTest.cpp b/test/core/model/DomainTest.cpp index 79b62f0..2e20c3b 100644 --- a/test/core/model/DomainTest.cpp +++ b/test/core/model/DomainTest.cpp @@ -101,7 +101,7 @@ TEST(Descriptor, pathTo) Rooted book = getClass("book", domain); Rooted section = getClass("section", domain); // get the path in between. - std::vector> path = book->pathTo(section); + NodeVector path = book->pathTo(section); ASSERT_EQ(1U, path.size()); ASSERT_TRUE(path[0]->isa(&RttiTypes::FieldDescriptor)); @@ -202,18 +202,19 @@ TEST(Descriptor, pathToAdvanced) E_field->addChild(target); ASSERT_TRUE(domain->validate(logger)); + #ifdef MANAGER_GRAPHVIZ_EXPORT // dump the manager state mgr.exportGraphviz("nastyDomain.dot"); #endif // and now we should be able to find the shortest path as suggested - std::vector> path = start->pathTo(target); + NodeVector path = start->pathTo(target); ASSERT_EQ(3U, path.size()); ASSERT_TRUE(path[0]->isa(&RttiTypes::FieldDescriptor)); ASSERT_EQ("second", path[0]->getName()); ASSERT_TRUE(path[1]->isa(&RttiTypes::StructuredClass)); - ASSERT_EQ("B", path[1]->getName()); + ASSERT_EQ("C", path[1]->getName()); ASSERT_TRUE(path[2]->isa(&RttiTypes::FieldDescriptor)); ASSERT_EQ("", path[2]->getName()); } -- cgit v1.2.3 From 339683e64288a85b8d6f9355a10563417b7d2fa7 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Thu, 12 Feb 2015 00:05:08 +0100 Subject: addes special resolve case in ParserScope::resolve for default field descriptors and changed resolve mechanism in parent field refs to just asking for the FieldDescriptor with the given name. --- src/core/parser/ParserScope.cpp | 14 ++++++++++++++ src/plugins/xml/XmlParser.cpp | 15 ++++++--------- test/plugins/xml/XmlParserTest.cpp | 2 +- 3 files changed, 21 insertions(+), 10 deletions(-) (limited to 'test') diff --git a/src/core/parser/ParserScope.cpp b/src/core/parser/ParserScope.cpp index 9c0b729..5440b9c 100644 --- a/src/core/parser/ParserScope.cpp +++ b/src/core/parser/ParserScope.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include "ParserScope.hpp" @@ -37,6 +38,19 @@ Rooted ParserScopeBase::resolve(const Rtti *type, const std::vector &path, Logger &logger) { + // if the last element of the path is the default field name, we want to + // resolve the parent descriptor first. + if (type == &RttiTypes::FieldDescriptor && + path.back() == DEFAULT_FIELD_NAME) { + std::vector descPath; + descPath.insert(descPath.end(), path.begin(), path.end() - 1); + Rooted res = resolve(&RttiTypes::Descriptor, descPath, logger); + if (res == nullptr) { + return nullptr; + } + return res.cast()->getFieldDescriptor(); + } + // Go up the stack and try to resolve the for (auto it = nodes.rbegin(); it != nodes.rend(); it++) { std::vector res = (*it)->resolve(type, path); diff --git a/src/plugins/xml/XmlParser.cpp b/src/plugins/xml/XmlParser.cpp index d7efa4d..e0346ab 100644 --- a/src/plugins/xml/XmlParser.cpp +++ b/src/plugins/xml/XmlParser.cpp @@ -785,16 +785,14 @@ public: Handle strct, Logger &logger) { if (parent != nullptr) { - auto res = parent.cast()->resolve( - &RttiTypes::FieldDescriptor, name); - if (res.size() != 1) { + Rooted field = + parent.cast()->getFieldDescriptor(name); + if (field == nullptr) { logger.error( std::string("Could not find referenced field ") + name, loc); return; } - Rooted field = - res[0].node.cast(); field->addChild(strct.cast()); } }); @@ -985,10 +983,9 @@ static const ParserState DomainStructPrimitive = .parents({&DomainStruct, &DomainAnnotation}) .createdNodeType(&RttiTypes::FieldDescriptor) .elementHandler(DomainPrimitiveHandler::create) - .arguments({Argument::String("name", ""), - Argument::Bool("isSubtree", false), - Argument::Bool("optional", false), - Argument::String("type")}); + .arguments( + {Argument::String("name", ""), Argument::Bool("isSubtree", false), + Argument::Bool("optional", false), Argument::String("type")}); static const ParserState DomainStructChild = ParserStateBuilder() diff --git a/test/plugins/xml/XmlParserTest.cpp b/test/plugins/xml/XmlParserTest.cpp index 7d5c697..134e935 100644 --- a/test/plugins/xml/XmlParserTest.cpp +++ b/test/plugins/xml/XmlParserTest.cpp @@ -292,7 +292,7 @@ TEST(XmlParser, domainParsing) std::vector> descs{comment_anno, comment, reply}; for (auto &d : descs) { checkFieldDescriptor(d, {paragraph}, "content", - FieldDescriptor::FieldType::SUBTREE, nullptr, + FieldDescriptor::FieldType::TREE, nullptr, false); checkFieldDescriptor(d, {reply}, "replies", FieldDescriptor::FieldType::SUBTREE, nullptr, -- cgit v1.2.3 From ebac41111fa33790acce7be45e599f8de37e7f43 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Thu, 12 Feb 2015 11:33:01 +0100 Subject: Anchors do not have a name anymore and have a unique mapping to their AnnotationEntities. This also makes serialization much easier. --- src/core/model/Document.cpp | 112 +++++++++++++++++++++++++++++++---- src/core/model/Document.hpp | 66 ++++++++++++++++----- src/plugins/html/DemoOutput.cpp | 53 ++++++----------- src/plugins/html/DemoOutput.hpp | 16 ++--- test/core/model/DocumentTest.cpp | 4 +- test/core/model/TestAdvanced.hpp | 6 +- test/plugins/html/DemoOutputTest.cpp | 23 +++---- 7 files changed, 185 insertions(+), 95 deletions(-) (limited to 'test') diff --git a/src/core/model/Document.cpp b/src/core/model/Document.cpp index 8b31f14..b38f2c0 100644 --- a/src/core/model/Document.cpp +++ b/src/core/model/Document.cpp @@ -275,9 +275,9 @@ static int enforceGetFieldDescriptorIndex(Handle desc, { ssize_t idx = desc->getFieldDescriptorIndex(fieldName); if (idx == -1) { - throw OusiaException( - std::string("Descriptor \"") + desc->getName() + - "\" has no field with the name \"" + fieldName + "\""); + throw OusiaException(std::string("Descriptor \"") + desc->getName() + + "\" has no field with the name \"" + fieldName + + "\""); } return idx; } @@ -287,8 +287,7 @@ static int enforceGetFieldDescriptorIndex( { ssize_t idx = desc->getFieldDescriptorIndex(fieldDescriptor); if (idx == -1) { - throw OusiaException(std::string("Descriptor \"") + - desc->getName() + + throw OusiaException(std::string("Descriptor \"") + desc->getName() + "\" does not reference the given field \"" + fieldDescriptor->getName() + "\""); } @@ -419,11 +418,10 @@ Rooted DocumentEntity::createChildDocumentPrimitive( subInst->getManager(), subInst, std::move(content), fieldName)}; } -Rooted DocumentEntity::createChildAnchor(std::string name, - const std::string &fieldName) +Rooted DocumentEntity::createChildAnchor(const std::string &fieldName) { return Rooted{ - new Anchor(subInst->getManager(), std::move(name), subInst, fieldName)}; + new Anchor(subInst->getManager(), subInst, fieldName)}; } /* Class StructureNode */ @@ -494,13 +492,61 @@ bool Anchor::doValidate(Logger &logger) const { bool valid = true; // check name - if (getName().empty()) { - logger.error("An Anchor needs a name!", *this); + if (!getName().empty()) { + logger.error( + "This anchor has a name! Anchors should only be referred to by " + "reference, not by name!", + *this); valid = false; } + if (annotation == nullptr) { + // this is valid but should throw a warning. + logger.warning("This anchor is disconnected.", *this); + } return valid & StructureNode::doValidate(logger); } +void Anchor::setAnnotation(Handle anno, bool start) +{ + if (annotation == anno) { + return; + } + invalidate(); + // unset the old reference. + if (annotation != nullptr) { + if (isStart()) { + annotation->setStart(nullptr); + } else { + annotation->setEnd(nullptr); + } + } + annotation = acquire(anno); + // set the new reference. + if (anno != nullptr) { + if (start) { + anno->setStart(this); + } else { + anno->setEnd(this); + } + } +} + +bool Anchor::isStart() const +{ + if (annotation == nullptr) { + return false; + } + return annotation->getStart() == this; +} + +bool Anchor::isEnd() const +{ + if (annotation == nullptr) { + return false; + } + return annotation->getEnd() == this; +} + /* Class AnnotationEntity */ AnnotationEntity::AnnotationEntity(Manager &mgr, Handle parent, @@ -508,13 +554,13 @@ AnnotationEntity::AnnotationEntity(Manager &mgr, Handle parent, Handle start, Handle end, Variant attributes, std::string name) : Node(mgr, std::move(name), parent), - DocumentEntity(this, descriptor, attributes), - start(acquire(start)), - end(acquire(end)) + DocumentEntity(this, descriptor, attributes) { if (parent != nullptr) { parent->addAnnotation(this); } + setStart(start); + setEnd(end); } bool AnnotationEntity::doValidate(Logger &logger) const @@ -567,10 +613,50 @@ bool AnnotationEntity::doValidate(Logger &logger) const valid = false; } } + // check if the Anchors reference this AnnotationEntity correctly. + if (start != nullptr) { + if (start->getAnnotation() != this) { + logger.error( + "This annotations start anchor does not have the correct " + "annotation as parent!", + *this); + valid = false; + } + } + if (end != nullptr) { + if (end->getAnnotation() != this) { + logger.error( + "This annotations end anchor does not have the correct " + "annotation as parent!", + *this); + valid = false; + } + } + // check the validity as a DocumentEntity. return valid & DocumentEntity::doValidate(logger); } +void AnnotationEntity::setStart(Handle s) +{ + if (start == s) { + return; + } + invalidate(); + start = acquire(s); + s->setAnnotation(this, true); +} + +void AnnotationEntity::setEnd(Handle e) +{ + if (end == e) { + return; + } + invalidate(); + end = acquire(e); + e->setAnnotation(this, false); +} + /* Class Document */ void Document::doResolve(ResolutionState &state) diff --git a/src/core/model/Document.hpp b/src/core/model/Document.hpp index b41393e..bffd397 100644 --- a/src/core/model/Document.hpp +++ b/src/core/model/Document.hpp @@ -126,6 +126,7 @@ class Document; class StructureNode; class StructuredEntity; class DocumentPrimitive; +class AnnotationEntity; class Anchor; /** @@ -409,14 +410,13 @@ public: /** * Creates a new Anchor as child of this DocumentEntity. * - * @param name is the Anchor id. * @param fieldName is the name of the field, where the newly created * Anchor shall be added to this DocumentEntity. * * @return the newly created Anchor. */ Rooted createChildAnchor( - std::string name, const std::string &fieldName = DEFAULT_FIELD_NAME); + const std::string &fieldName = DEFAULT_FIELD_NAME); }; /** @@ -577,6 +577,9 @@ public: * Please refer to the AnnotationEntity documentation for more information. */ class Anchor : public StructureNode { +private: + Owned annotation; + protected: bool doValidate(Logger &logger) const override; @@ -589,15 +592,55 @@ public: * not the AnnotationEntity that references this Anchor. * Note that this Anchor will automatically register itself * as child of the given parent. - * @param name is the Anchor id. * @param fieldName is the name of the field in the parent DocumentEntity * where this Anchor shall be added. */ - Anchor(Manager &mgr, std::string name, Handle parent, + Anchor(Manager &mgr, Handle parent, const std::string &fieldName = DEFAULT_FIELD_NAME) - : StructureNode(mgr, std::move(name), parent, fieldName) + : StructureNode(mgr, "", parent, fieldName) { } + + /** + * Returns the AnnotationEntity this Anchor belongs to. + * + * @return the AnnotationEntity this Anchor belongs to. + */ + Rooted getAnnotation() const { return annotation; } + + /** + * Sets the AnnotationEntity this Anchor belongs to. If this Anchor belonged + * to an AnnotationEntity before already, this reference is removed. This + * also sets the start/end reference of the new AnnotationEntity this Anchor + * shall belong to. + * + * @param anno the new AnnotationEntity this Anchor shall belong to. + * @param start true if this Anchor should be added as start anchor, false + * if it should be added as end Anchor. + */ + void setAnnotation(Handle anno, bool start); + + /** + * Returns true if and only if this Anchor is the start Anchor of the + * AnnotationEntity it belongs to. Note that this will return false also if + * no AnnotationEntity is set yet. So isStart() == false and isEnd() == + * false is possible and occurs if and only if getAnnotation() == nullptr. + * + * @return true if and only if this Anchor is the start Anchor of the + * AnnotationEntity it belongs to. + */ + bool isStart() const; + + /** + * Returns true if and only if this Anchor is the end Anchor of the + * AnnotationEntity it belongs to. Note that this will return false also if + * no AnnotationEntity is set yet. So isStart() == false and isEnd() == + * false is possible and occurs if and only if getAnnotation() == nullptr. + * + * @return true if and only if this Anchor is the end Anchor of the + * AnnotationEntity it belongs to. + */ + bool isEnd() const; }; /** @@ -679,22 +722,13 @@ public: * * @param s is the new start Anchor for this AnnotationEntity. */ - void setStart(Handle s) - { - invalidate(); - start = acquire(s); - } - + void setStart(Handle s); /** * Sets the end Anchor of this AnnotationEntity. * * @param e is the new end Anchor for this AnnotationEntity. */ - void setEnd(Handle e) - { - invalidate(); - end = acquire(e); - } + void setEnd(Handle e); }; /** diff --git a/src/plugins/html/DemoOutput.cpp b/src/plugins/html/DemoOutput.cpp index cb34cbe..3c54763 100644 --- a/src/plugins/html/DemoOutput.cpp +++ b/src/plugins/html/DemoOutput.cpp @@ -55,23 +55,13 @@ void DemoHTMLTransformer::writeHTML(Handle doc, std::ostream &out, // So far was the "preamble". No we have to get to the document content. - // build the start and end map for annotation processing. - AnnoMap startMap; - AnnoMap endMap; - for (auto &a : doc->getAnnotations()) { - // we assume uniquely IDed annotations, which should be checked in the - // validation process. - startMap.emplace(a->getStart()->getName(), a); - endMap.emplace(a->getEnd()->getName(), a); - } - // extract the book root node. Rooted root = doc->getRoot(); if (root->getDescriptor()->getName() != "book") { throw OusiaException("The given documents root is no book node!"); } // transform the book node. - Rooted book = transformSection(body, root, startMap, endMap); + Rooted book = transformSection(body, root); // add it as child to the body node. body->addChild(book); @@ -100,8 +90,7 @@ SectionType getSectionType(const std::string &name) } Rooted DemoHTMLTransformer::transformSection( - Handle parent, Handle section, - AnnoMap &startMap, AnnoMap &endMap) + Handle parent, Handle section) { Manager &mgr = section->getManager(); // check the section type. @@ -140,8 +129,7 @@ Rooted DemoHTMLTransformer::transformSection( Rooted h{new xml::Element{mgr, sec, headingclass}}; sec->addChild(h); // extract the heading text, enveloped in a paragraph Element. - Rooted h_content = - transformParagraph(h, heading, startMap, endMap); + Rooted h_content = transformParagraph(h, heading); // We omit the paragraph Element and add the children directly to the // heading Element for (auto &n : h_content->getChildren()) { @@ -165,11 +153,11 @@ Rooted DemoHTMLTransformer::transformSection( const std::string childDescriptorName = s->getDescriptor()->getName(); Rooted child; if (childDescriptorName == "paragraph") { - child = transformParagraph(sec, s, startMap, endMap); + child = transformParagraph(sec, s); } else if (childDescriptorName == "ul" || childDescriptorName == "ol") { - child = transformList(sec, s, startMap, endMap); + child = transformList(sec, s); } else { - child = transformSection(sec, s, startMap, endMap); + child = transformSection(sec, s); } if (!child.isNull()) { sec->addChild(child); @@ -179,8 +167,7 @@ Rooted DemoHTMLTransformer::transformSection( } Rooted DemoHTMLTransformer::transformList( - Handle parent, Handle list, - AnnoMap &startMap, AnnoMap &endMap) + Handle parent, Handle list) { Manager &mgr = list->getManager(); // create the list Element, which is either ul or ol (depends on descriptor) @@ -195,8 +182,7 @@ Rooted DemoHTMLTransformer::transformList( Rooted li{new xml::Element{mgr, l, "li"}}; l->addChild(li); // extract the item text, enveloped in a paragraph Element. - Rooted li_content = - transformParagraph(li, item, startMap, endMap); + Rooted li_content = transformParagraph(li, item); // We omit the paragraph Element and add the children directly to // the list item for (auto &n : li_content->getChildren()) { @@ -229,8 +215,7 @@ static Rooted openAnnotation(Manager &mgr, AnnoStack &opened, } Rooted DemoHTMLTransformer::transformParagraph( - Handle parent, Handle par, - AnnoMap &startMap, AnnoMap &endMap) + Handle parent, Handle par) { Manager &mgr = par->getManager(); // create the p Element @@ -245,8 +230,7 @@ Rooted DemoHTMLTransformer::transformParagraph( Rooted strong{new xml::Element{mgr, p, "strong"}}; p->addChild(strong); // extract the heading text, enveloped in a paragraph Element. - Rooted h_content = - transformParagraph(strong, heading, startMap, endMap); + Rooted h_content = transformParagraph(strong, heading); // We omit the paragraph Element and add the children directly to the // heading Element for (auto &n : h_content->getChildren()) { @@ -267,17 +251,15 @@ Rooted DemoHTMLTransformer::transformParagraph( Rooted current = p; for (auto &n : par->getField()) { if (n->isa(&RttiTypes::Anchor)) { + Rooted a = n.cast(); // check if this is a start Anchor. - // here we assume, again, that the ids/names of anchors are unique. - auto it = startMap.find(n->getName()); - if (it != startMap.end()) { + if (a->isStart()) { // if we have a start anchor, we open an annotation element. - current = openAnnotation(mgr, opened, it->second, current); + current = + openAnnotation(mgr, opened, a->getAnnotation(), current); continue; - } - // check if this is an end Anchor. - auto it2 = endMap.find(n->getName()); - if (it2 != endMap.end()) { + // check if this is an end Anchor. + } else if (a->isEnd()) { /* * Now it gets somewhat interesting: We have to close all * tags that started after the one that is closed now and @@ -289,7 +271,7 @@ Rooted DemoHTMLTransformer::transformParagraph( Rooted closed = opened.top(); current = current->getParent(); opened.pop(); - while (closed->getEnd()->getName() != n->getName()) { + while (closed != a->getAnnotation()) { /* * We implicitly do close tags by climbing up the XML tree * until we are at the right element. @@ -312,6 +294,7 @@ Rooted DemoHTMLTransformer::transformParagraph( current = openAnnotation(mgr, opened, closed, current); } } + // otherwise it is a disconnected Anchor and we can ignore it. continue; } // if this is not an anchor, we can only handle text. diff --git a/src/plugins/html/DemoOutput.hpp b/src/plugins/html/DemoOutput.hpp index 67b7494..0650621 100644 --- a/src/plugins/html/DemoOutput.hpp +++ b/src/plugins/html/DemoOutput.hpp @@ -39,8 +39,6 @@ namespace ousia { namespace html { -typedef std::map> AnnoMap; - class DemoHTMLTransformer { private: /** @@ -50,23 +48,20 @@ private: * called recursively. */ Rooted transformSection(Handle parent, - Handle sec, - AnnoMap &startMap, AnnoMap &endMap); + Handle sec); /** * This transforms a list entity, namely ul and ol to an XHTML element. * For each item, the transformParagraph function is called. */ Rooted transformList(Handle parent, - Handle list, - AnnoMap &startMap, AnnoMap &endMap); + Handle list); /** * This transforms a paragraph-like entity, namely heading, item and * paragraph, to an XHTML element including the text and the anchors - * contained. For anchor handling we require the AnnoMaps. + * contained. */ Rooted transformParagraph(Handle parent, - Handle par, - AnnoMap &startMap, AnnoMap &endMap); + Handle par); public: /** @@ -89,8 +84,7 @@ public: * @param pretty is a flag that manipulates whether newlines and tabs are * used. */ - void writeHTML(Handle doc, std::ostream &out, - bool pretty = true); + void writeHTML(Handle doc, std::ostream &out, bool pretty = true); }; } } diff --git a/test/core/model/DocumentTest.cpp b/test/core/model/DocumentTest.cpp index 5acf13f..0c6eea6 100644 --- a/test/core/model/DocumentTest.cpp +++ b/test/core/model/DocumentTest.cpp @@ -269,12 +269,12 @@ TEST(Document, validate) doc->referenceDomain(domain); Rooted root = buildRootStructuredEntity(doc, logger, {"root"}); - Rooted start{new Anchor(mgr, "start", root)}; + Rooted start{new Anchor(mgr, root)}; Rooted child = buildStructuredEntity(doc, logger, root, {"childSub"}); Rooted primitive{ new DocumentPrimitive(mgr, child, {2}, "int")}; - Rooted end{new Anchor(mgr, "end", root)}; + Rooted end{new Anchor(mgr, root)}; ASSERT_EQ(ValidationState::UNKNOWN, doc->getValidationState()); ASSERT_TRUE(doc->validate(logger)); // then add an AnnotationEntity without Anchors. diff --git a/test/core/model/TestAdvanced.hpp b/test/core/model/TestAdvanced.hpp index 575860b..27f33cc 100644 --- a/test/core/model/TestAdvanced.hpp +++ b/test/core/model/TestAdvanced.hpp @@ -150,19 +150,17 @@ static bool addHeading(Logger &logger, Handle doc, return true; } -static int annoIdx = 1; - // Only works for non-overlapping annotations! static bool addAnnotation(Logger &logger, Handle doc, Handle parent, const std::string &text, const std::string &annoClass) { Manager &mgr = parent->getManager(); - Rooted start{new Anchor(mgr, std::to_string(annoIdx++), parent)}; + Rooted start{new Anchor(mgr, parent)}; if (!addText(logger, doc, parent, text)) { return false; } - Rooted end{new Anchor(mgr, std::to_string(annoIdx++), parent)}; + Rooted end{new Anchor(mgr, parent)}; Rooted anno = buildAnnotationEntity(doc, logger, {annoClass}, start, end); if (anno.isNull()) { diff --git a/test/plugins/html/DemoOutputTest.cpp b/test/plugins/html/DemoOutputTest.cpp index 5006655..2f56e40 100644 --- a/test/plugins/html/DemoOutputTest.cpp +++ b/test/plugins/html/DemoOutputTest.cpp @@ -41,14 +41,11 @@ TEST(DemoHTMLTransformer, writeHTML) Manager mgr{1}; Rooted sys{new SystemTypesystem(mgr)}; // Get the domains. - Rooted bookDom = - constructBookDomain(mgr, sys, logger); + Rooted bookDom = constructBookDomain(mgr, sys, logger); Rooted headingDom = constructHeadingDomain(mgr, sys, bookDom, logger); - Rooted listDom = - constructListDomain(mgr, sys, bookDom, logger); - Rooted emDom = - constructEmphasisDomain(mgr, sys, logger); + Rooted listDom = constructListDomain(mgr, sys, bookDom, logger); + Rooted emDom = constructEmphasisDomain(mgr, sys, logger); // Construct the document. Rooted doc = constructAdvancedDocument( mgr, logger, bookDom, headingDom, listDom, emDom); @@ -79,10 +76,8 @@ TEST(DemoHTMLTransformer, AnnotationProcessing) Manager mgr{1}; Rooted sys{new SystemTypesystem(mgr)}; // Get the domains. - Rooted bookDom = - constructBookDomain(mgr, sys, logger); - Rooted emDom = - constructEmphasisDomain(mgr, sys, logger); + Rooted bookDom = constructBookDomain(mgr, sys, logger); + Rooted emDom = constructEmphasisDomain(mgr, sys, logger); // Construct a document only containing overlapping annotations. // it has the form: blablubbla Rooted doc{new Document(mgr, "annotations.oxd")}; @@ -93,13 +88,13 @@ TEST(DemoHTMLTransformer, AnnotationProcessing) Rooted p = buildStructuredEntity(doc, logger, book, {"paragraph"}); ASSERT_TRUE(p != nullptr); - Rooted em_start{new Anchor(mgr, "1", p)}; + Rooted em_start{new Anchor(mgr, p)}; ASSERT_TRUE(addText(logger, doc, p, "bla")); - Rooted strong_start{new Anchor(mgr, "2", p)}; + Rooted strong_start{new Anchor(mgr, p)}; ASSERT_TRUE(addText(logger, doc, p, "blub")); - Rooted em_end{new Anchor(mgr, "3", p)}; + Rooted em_end{new Anchor(mgr, p)}; ASSERT_TRUE(addText(logger, doc, p, "bla")); - Rooted strong_end{new Anchor(mgr, "4", p)}; + Rooted strong_end{new Anchor(mgr, p)}; buildAnnotationEntity(doc, logger, {"emphasized"}, em_start, em_end); buildAnnotationEntity(doc, logger, {"strong"}, strong_start, strong_end); -- cgit v1.2.3 From 1a9b7e81919e4bd52cbb2db2e9e91a244734ab2c Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Thu, 12 Feb 2015 17:39:53 +0100 Subject: restructured pathTo. Also fixed some issues with that method and made it more general. --- src/core/model/Domain.cpp | 187 +++++++++++++++++++++++++++++------------ src/core/model/Domain.hpp | 39 ++++++++- src/plugins/xml/XmlParser.cpp | 6 +- test/core/model/DomainTest.cpp | 22 +++-- 4 files changed, 185 insertions(+), 69 deletions(-) (limited to 'test') diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp index 228348d..25b2528 100644 --- a/src/core/model/Domain.cpp +++ b/src/core/model/Domain.cpp @@ -16,6 +16,8 @@ along with this program. If not, see . */ +#include +#include #include #include @@ -207,80 +209,153 @@ bool Descriptor::doValidate(Logger &logger) const return valid & continueValidationCheckDuplicates(fds, logger); } -NodeVector Descriptor::pathTo(Handle target) const +struct PathState { + std::shared_ptr pred; + Node *node; + int length; + + PathState(std::shared_ptr pred, Node *node) + : pred(pred), node(node) + { + if (pred == nullptr) { + length = 1; + } else { + length = pred->length + 1; + } + } +}; + +static void constructPath(std::shared_ptr state, + NodeVector &vec) { - NodeVector path; - continuePath(target, path, true); - return std::move(path); + if (state->pred != nullptr) { + constructPath(state->pred, vec); + } + vec.push_back(state->node); } -bool Descriptor::continuePath(Handle target, - NodeVector ¤tPath, bool start) const +template +static NodeVector pathTo(const Descriptor *start, Logger &logger, + F finished) { - // check if we are at the target already - if (this == target) { - return true; + // state queue for breadth-first search. + std::queue> states; + { + // initially put every field descriptor on the queue. + NodeVector fields = start->getFieldDescriptors(); + + for (auto fd : fields) { + states.push(std::make_shared(nullptr, fd.get())); + } } - // a variable to determine if we already found a solution - bool found = false; - // the currently optimal path. - NodeVector optimum; - // use recursive depth-first search from the top to reach the given child - // get the list of effective FieldDescriptors. - NodeVector fields = getFieldDescriptors(); + // shortest path. + NodeVector shortest; + // set of visited nodes. + std::unordered_set visited; + while (!states.empty()) { + std::shared_ptr current = states.front(); + states.pop(); + // do not proceed if this node was already visited. + if (!visited.insert(current->node).second) { + continue; + } + // also do not proceed if we can't get better than the current shortest + // path anymore. + if (!shortest.empty() && current->length > shortest.size()) { + continue; + } + + bool fin = false; + if (current->node->isa(&RttiTypes::StructuredClass)) { + const StructuredClass *strct = + static_cast(current->node); + + // continue the search path via all fields. + NodeVector fields = strct->getFieldDescriptors(); + for (auto fd : fields) { + // if we found our target, break off the search in this branch. + if (finished(fd)) { + fin = true; + continue; + } + states.push(std::make_shared(current, fd.get())); + } - Rooted thisHandle{const_cast(this)}; + /* + * Furthermore we have to consider that all subclasses of this + * StructuredClass are allowed in place of this StructuredClass as + * well, so we continue the search for them as well. + */ - for (auto &fd : fields) { - for (auto &c : fd->getChildren()) { - // check if a child is the target node. - if (c == target) { - // if we have made the connection, stop the search. - if (!start) { - currentPath.push_back(thisHandle); + NodeVector subs = strct->getSubclasses(); + for (auto sub : subs) { + // if we found our target, break off the search in this branch. + if (finished(sub)) { + fin = true; + current = current->pred; + continue; + } + // We only continue our path via transparent classes. + if (sub->isTransparent()) { + states.push( + std::make_shared(current->pred, sub.get())); } - currentPath.push_back(fd); - return true; } - // look for transparent intermediate nodes. - if (c->isTransparent()) { - // copy the path. - NodeVector cPath = currentPath; - if (!start) { - cPath.push_back(thisHandle); + } else { + // otherwise this is a FieldDescriptor. + const FieldDescriptor *field = + static_cast(current->node); + // and we proceed by visiting all permitted children. + for (auto c : field->getChildren()) { + // if we found our target, break off the search in this branch. + if (finished(c)) { + fin = true; + continue; } - cPath.push_back(fd); - // recursion. - if (c->continuePath(target, cPath, false) && - (!found || optimum.size() > cPath.size())) { - // look if this path is better than the current optimum. - optimum = std::move(cPath); - found = true; + // We only allow to continue our path via transparent children. + if (c->isTransparent()) { + states.push(std::make_shared(current, c.get())); } } } - } - - if (isa(&RttiTypes::StructuredClass)) { - const StructuredClass *tis = static_cast(this); - // if this is a StructuredClass we also can call the subclasses. - for (auto &c : tis->getSubclasses()) { - // copy the path. - NodeVector cPath = currentPath; - if (c->continuePath(target, cPath, false) && - (!found || optimum.size() > cPath.size())) { - // look if this path is better than the current optimum. - optimum = std::move(cPath); - found = true; + // check if we are finished. + if (fin) { + // if so we look if we found a shorter path than the current minimum + if (shortest.empty() || current->length < shortest.size()) { + NodeVector newPath; + constructPath(current, newPath); + shortest = newPath; + } else if (current->length == shortest.size()) { + // if the length is the same the result is ambigous and we log + // an error. + NodeVector newPath; + constructPath(current, newPath); + logger.error( + std::string("Can not unambigously create a path from \"") + + start->getName() + "\"."); + logger.note("Dismissed the path:", SourceLocation{}, + MessageMode::NO_CONTEXT); + for (auto n : newPath) { + logger.note(n->getName()); + } } } } + return shortest; +} - // put the optimum in the given path reference. - currentPath = std::move(optimum); +NodeVector Descriptor::pathTo(Handle target, + Logger &logger) const +{ + return ousia::pathTo(this, logger, + [target](Handle n) { return n == target; }); +} - // return if we found something. - return found; +NodeVector Descriptor::pathTo(Handle field, + Logger &logger) const +{ + return ousia::pathTo(this, logger, + [field](Handle n) { return n == field; }); } static ssize_t getFieldDescriptorIndex(const NodeVector &fds, diff --git a/src/core/model/Domain.hpp b/src/core/model/Domain.hpp index 57e5602..ae5ba1d 100644 --- a/src/core/model/Domain.hpp +++ b/src/core/model/Domain.hpp @@ -404,9 +404,6 @@ private: Owned attributesDescriptor; NodeVector fieldDescriptors; - bool continuePath(Handle target, NodeVector &path, - bool start) const; - void addAndSortFieldDescriptor(Handle fd, Logger &logger); protected: @@ -581,6 +578,9 @@ public: * a path between book and section (via chapter), but chapter is not * transparent. Therefore that path is not allowed. * + * Implicitly this does a breadth-first search on the graph of + * StructuredClasses that are transparent. It also takes care of cycles. + * * @param childDescriptor is a supposedly valid child Descriptor of this * Descriptor. * @return either a path of FieldDescriptors and @@ -589,7 +589,38 @@ public: * no such path can be constructed. * */ - NodeVector pathTo(Handle childDescriptor) const; + NodeVector pathTo(Handle childDescriptor, + Logger &logger) const; + /** + * This tries to construct the shortest possible path of this Descriptor + * to the given FieldDescriptor. + * + * Implicitly this does a breadth-first search on the graph of + * StructuredClasses that are transparent. It also takes care of cycles. + * + * @param field is a FieldDescriptor that may be allowed as child of this + * Descriptor. + * @return either a path of FieldDescriptors and StructuredClasses + * between this Descriptor and the input FieldDescriptor or an + * empty vector if no such path can be constructed. + */ + NodeVector pathTo(Handle field, + Logger &logger) const; + /** + * This tries to construct the shortest possible path of this Descriptor + * to the given FieldDescriptor. + * + * Implicitly this does a breadth-first search on the graph of + * StructuredClasses that are transparent. It also takes care of cycles. + * + * @param fieldName is the name of a FieldDescriptor that may be allowed as + * child of this Descriptor. + * @return either a path of FieldDescriptors and StructuredClasses + * between this Descriptor and a FieldDescriptor with the + * given name or an empty vector if no such path can be + * constructed. + */ + NodeVector pathTo(const std::string &fieldName, Logger &logger) const; }; /* * TODO: We should discuss Cardinalities one more time. Is it smart to define diff --git a/src/plugins/xml/XmlParser.cpp b/src/plugins/xml/XmlParser.cpp index c51ca8c..63d9df5 100644 --- a/src/plugins/xml/XmlParser.cpp +++ b/src/plugins/xml/XmlParser.cpp @@ -131,8 +131,8 @@ public: preamble(parentNode, fieldName, parent, inField); // try to find a FieldDescriptor for the given tag if we are not in a - // field already. - // TODO: Consider fields of transparent classes + // field already. This does _not_ try to construct transparent paths + // in between. if (!inField && parent != nullptr && parent->getDescriptor()->hasField(name())) { Rooted field{new DocumentField( @@ -166,7 +166,7 @@ public: strct, args, name); } else { // calculate a path if transparent entities are needed in between. - auto path = parent->getDescriptor()->pathTo(strct); + auto path = parent->getDescriptor()->pathTo(strct, logger()); if (path.empty()) { throw LoggableException( std::string("An instance of \"") + strct->getName() + diff --git a/test/core/model/DomainTest.cpp b/test/core/model/DomainTest.cpp index 2e20c3b..0097900 100644 --- a/test/core/model/DomainTest.cpp +++ b/test/core/model/DomainTest.cpp @@ -91,7 +91,7 @@ Rooted getClass(const std::string name, Handle dom) TEST(Descriptor, pathTo) { // Start with some easy examples from the book domain. - Logger logger; + TerminalLogger logger{std::cout}; Manager mgr{1}; Rooted sys{new SystemTypesystem(mgr)}; // Get the domain. @@ -101,14 +101,14 @@ TEST(Descriptor, pathTo) Rooted book = getClass("book", domain); Rooted section = getClass("section", domain); // get the path in between. - NodeVector path = book->pathTo(section); + NodeVector path = book->pathTo(section, logger); ASSERT_EQ(1U, path.size()); ASSERT_TRUE(path[0]->isa(&RttiTypes::FieldDescriptor)); // get the text node. Rooted text = getClass("text", domain); // get the path between book and text via paragraph. - path = book->pathTo(text); + path = book->pathTo(text, logger); ASSERT_EQ(3U, path.size()); ASSERT_TRUE(path[0]->isa(&RttiTypes::FieldDescriptor)); ASSERT_TRUE(path[1]->isa(&RttiTypes::StructuredClass)); @@ -118,9 +118,19 @@ TEST(Descriptor, pathTo) // get the subsection node. Rooted subsection = getClass("subsection", domain); // try to get the path between book and subsection. - path = book->pathTo(subsection); + path = book->pathTo(subsection, logger); // this should be impossible. ASSERT_EQ(0U, path.size()); + + // try to construct the path between section and the text field. + path = section->pathTo(text->getFieldDescriptor(), logger); + ASSERT_EQ(4U, path.size()); + ASSERT_TRUE(path[0]->isa(&RttiTypes::FieldDescriptor)); + ASSERT_TRUE(path[1]->isa(&RttiTypes::StructuredClass)); + ASSERT_EQ("paragraph", path[1]->getName()); + ASSERT_TRUE(path[2]->isa(&RttiTypes::FieldDescriptor)); + ASSERT_TRUE(path[3]->isa(&RttiTypes::StructuredClass)); + ASSERT_EQ("text", path[3]->getName()); } TEST(Descriptor, pathToAdvanced) @@ -148,7 +158,7 @@ TEST(Descriptor, pathToAdvanced) * So the path A_second_field, C, C_field should be returned. */ Manager mgr{1}; - Logger logger; + TerminalLogger logger{std::cout}; Rooted sys{new SystemTypesystem(mgr)}; // Construct the domain Rooted domain{new Domain(mgr, sys, "nasty")}; @@ -209,7 +219,7 @@ TEST(Descriptor, pathToAdvanced) #endif // and now we should be able to find the shortest path as suggested - NodeVector path = start->pathTo(target); + NodeVector path = start->pathTo(target, logger); ASSERT_EQ(3U, path.size()); ASSERT_TRUE(path[0]->isa(&RttiTypes::FieldDescriptor)); ASSERT_EQ("second", path[0]->getName()); -- cgit v1.2.3 From 110fb7da850377e39b2879da44339dc936c266dc Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Thu, 12 Feb 2015 19:31:18 +0100 Subject: further revised pathTo. Now only the TREE field is used in further exploration. --- src/core/model/Domain.cpp | 50 +++++++++++++++++++++++++++--------------- src/core/model/Domain.hpp | 18 ++++++++++----- test/core/model/DomainTest.cpp | 36 ++++++++++++------------------ 3 files changed, 58 insertions(+), 46 deletions(-) (limited to 'test') diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp index 25b2528..619454c 100644 --- a/src/core/model/Domain.cpp +++ b/src/core/model/Domain.cpp @@ -212,7 +212,7 @@ bool Descriptor::doValidate(Logger &logger) const struct PathState { std::shared_ptr pred; Node *node; - int length; + size_t length; PathState(std::shared_ptr pred, Node *node) : pred(pred), node(node) @@ -234,10 +234,12 @@ static void constructPath(std::shared_ptr state, vec.push_back(state->node); } -template static NodeVector pathTo(const Descriptor *start, Logger &logger, - F finished) + Handle target, bool &success) { + success = false; + // shortest path. + NodeVector shortest; // state queue for breadth-first search. std::queue> states; { @@ -245,11 +247,16 @@ static NodeVector pathTo(const Descriptor *start, Logger &logger, NodeVector fields = start->getFieldDescriptors(); for (auto fd : fields) { - states.push(std::make_shared(nullptr, fd.get())); + if (fd == target) { + // if we have found the target directly, return without search. + success = true; + return shortest; + } + if (fd->getFieldType() == FieldDescriptor::FieldType::TREE) { + states.push(std::make_shared(nullptr, fd.get())); + } } } - // shortest path. - NodeVector shortest; // set of visited nodes. std::unordered_set visited; while (!states.empty()) { @@ -270,15 +277,18 @@ static NodeVector pathTo(const Descriptor *start, Logger &logger, const StructuredClass *strct = static_cast(current->node); - // continue the search path via all fields. + // look through all fields. NodeVector fields = strct->getFieldDescriptors(); for (auto fd : fields) { // if we found our target, break off the search in this branch. - if (finished(fd)) { + if (fd == target) { fin = true; continue; } - states.push(std::make_shared(current, fd.get())); + // only continue in the TREE field. + if (fd->getFieldType() == FieldDescriptor::FieldType::TREE) { + states.push(std::make_shared(current, fd.get())); + } } /* @@ -290,7 +300,7 @@ static NodeVector pathTo(const Descriptor *start, Logger &logger, NodeVector subs = strct->getSubclasses(); for (auto sub : subs) { // if we found our target, break off the search in this branch. - if (finished(sub)) { + if (sub == target) { fin = true; current = current->pred; continue; @@ -308,7 +318,7 @@ static NodeVector pathTo(const Descriptor *start, Logger &logger, // and we proceed by visiting all permitted children. for (auto c : field->getChildren()) { // if we found our target, break off the search in this branch. - if (finished(c)) { + if (c == target) { fin = true; continue; } @@ -320,6 +330,7 @@ static NodeVector pathTo(const Descriptor *start, Logger &logger, } // check if we are finished. if (fin) { + success = true; // if so we look if we found a shorter path than the current minimum if (shortest.empty() || current->length < shortest.size()) { NodeVector newPath; @@ -332,7 +343,7 @@ static NodeVector pathTo(const Descriptor *start, Logger &logger, constructPath(current, newPath); logger.error( std::string("Can not unambigously create a path from \"") + - start->getName() + "\"."); + start->getName() + "\" to \"" + target->getName() + "\"."); logger.note("Dismissed the path:", SourceLocation{}, MessageMode::NO_CONTEXT); for (auto n : newPath) { @@ -347,15 +358,18 @@ static NodeVector pathTo(const Descriptor *start, Logger &logger, NodeVector Descriptor::pathTo(Handle target, Logger &logger) const { - return ousia::pathTo(this, logger, - [target](Handle n) { return n == target; }); + bool success = false; + return ousia::pathTo(this, logger, target, success); } -NodeVector Descriptor::pathTo(Handle field, - Logger &logger) const +std::pair, bool> Descriptor::pathTo( + Handle field, Logger &logger) const +{ + bool success = false; + NodeVector path = ousia::pathTo(this, logger, field, success); + return std::make_pair(path, success); +} { - return ousia::pathTo(this, logger, - [field](Handle n) { return n == field; }); } static ssize_t getFieldDescriptorIndex(const NodeVector &fds, diff --git a/src/core/model/Domain.hpp b/src/core/model/Domain.hpp index abe7a52..91d635e 100644 --- a/src/core/model/Domain.hpp +++ b/src/core/model/Domain.hpp @@ -593,19 +593,25 @@ public: Logger &logger) const; /** * This tries to construct the shortest possible path of this Descriptor - * to the given FieldDescriptor. + * to the given FieldDescriptor. Note that this method has the problem that + * an empty return path does NOT strictly imply that no such path could + * be constructed: We also return an empty vector if the given + * FieldDescriptor is a direct child. Therefore we also return a bool value + * indicating that the path is valid or not. + * * * Implicitly this does a breadth-first search on the graph of * StructuredClasses that are transparent. It also takes care of cycles. * * @param field is a FieldDescriptor that may be allowed as child of this * Descriptor. - * @return either a path of FieldDescriptors and StructuredClasses - * between this Descriptor and the input FieldDescriptor or an - * empty vector if no such path can be constructed. + * @return returns a tuple containing a path of FieldDescriptors and + * StructuredClasses between this Descriptor and the input + * FieldDescriptor and a bool value indicating if the + * construction was successful. */ - NodeVector pathTo(Handle field, - Logger &logger) const; + std::pair, bool> pathTo(Handle field, + Logger &logger) const; }; /* * TODO: We should discuss Cardinalities one more time. Is it smart to define diff --git a/test/core/model/DomainTest.cpp b/test/core/model/DomainTest.cpp index 0097900..672b2d1 100644 --- a/test/core/model/DomainTest.cpp +++ b/test/core/model/DomainTest.cpp @@ -123,7 +123,9 @@ TEST(Descriptor, pathTo) ASSERT_EQ(0U, path.size()); // try to construct the path between section and the text field. - path = section->pathTo(text->getFieldDescriptor(), logger); + auto res = section->pathTo(text->getFieldDescriptor(), logger); + ASSERT_TRUE(res.second); + path = res.first; ASSERT_EQ(4U, path.size()); ASSERT_TRUE(path[0]->isa(&RttiTypes::FieldDescriptor)); ASSERT_TRUE(path[1]->isa(&RttiTypes::StructuredClass)); @@ -144,15 +146,13 @@ TEST(Descriptor, pathToAdvanced) * * To achieve that we have the following structure: * 1.) The start class inherits from A. - * 2.) A has the target as child in the default field, but the default - * field is overridden in the start class. - * 3.) A has B as child in another field. - * 4.) B is transparent and has no children (but C as subclass) - * 5.) C is a subclass of B, transparent and has + * 2.) A has B as child in the default field. + * 3.) B is transparent and has no children (but C as subclass) + * 4.) C is a subclass of B, transparent and has * the target as child (shortest path). - * 6.) start has D as child in the default field. - * 7.) D is transparent has E as child in the default field. - * 8.) E is transparent and has target as child in the default field + * 5.) A has D as child in the default field. + * 6.) D is transparent has E as child in the default field. + * 7.) E is transparent and has target as child in the default field * (longer path) * * So the path A_second_field, C, C_field should be returned. @@ -185,30 +185,22 @@ TEST(Descriptor, pathToAdvanced) Rooted target{ new StructuredClass(mgr, "target", domain, Cardinality::any())}; - // We create two fields for A + // We create a field for A Rooted A_field = A->createFieldDescriptor(logger); + A_field->addChild(B); + A_field->addChild(D); - A_field->addChild(target); - Rooted A_field2 = A->createFieldDescriptor( - logger, FieldDescriptor::FieldType::SUBTREE, "second", false); - - A_field2->addChild(B); // We create no field for B // One for C Rooted C_field = C->createFieldDescriptor(logger); - C_field->addChild(target); - // one for start - Rooted start_field = start->createFieldDescriptor(logger); - start_field->addChild(D); // One for D Rooted D_field = D->createFieldDescriptor(logger); - D_field->addChild(E); + // One for E Rooted E_field = E->createFieldDescriptor(logger); - E_field->addChild(target); ASSERT_TRUE(domain->validate(logger)); @@ -222,7 +214,7 @@ TEST(Descriptor, pathToAdvanced) NodeVector path = start->pathTo(target, logger); ASSERT_EQ(3U, path.size()); ASSERT_TRUE(path[0]->isa(&RttiTypes::FieldDescriptor)); - ASSERT_EQ("second", path[0]->getName()); + ASSERT_EQ("", path[0]->getName()); ASSERT_TRUE(path[1]->isa(&RttiTypes::StructuredClass)); ASSERT_EQ("C", path[1]->getName()); ASSERT_TRUE(path[2]->isa(&RttiTypes::FieldDescriptor)); -- cgit v1.2.3 From 89f01a0a49f4fd23034d532b37d54d3f3f612082 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Thu, 12 Feb 2015 19:31:50 +0100 Subject: added a method to retrieve all reachable default fields from a given descriptor. --- src/core/model/Domain.cpp | 99 ++++++++++++++++++++++++++++++++++++++++++ src/core/model/Domain.hpp | 12 +++++ test/core/model/DomainTest.cpp | 80 ++++++++++++++++++++++++++++++++++ 3 files changed, 191 insertions(+) (limited to 'test') diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp index 619454c..6f33ebd 100644 --- a/src/core/model/Domain.cpp +++ b/src/core/model/Domain.cpp @@ -369,7 +369,106 @@ std::pair, bool> Descriptor::pathTo( NodeVector path = ousia::pathTo(this, logger, field, success); return std::make_pair(path, success); } + +template +static NodeVector collect(const Descriptor *start, F match) { + // result + NodeVector res; + // queue for breadth-first search of graph. + std::queue> q; + { + // initially put every field descriptor on the queue. + NodeVector fields = start->getFieldDescriptors(); + + for (auto fd : fields) { + // note matches. + if (match(fd)) { + res.push_back(fd); + } + if (fd->getFieldType() == FieldDescriptor::FieldType::TREE) { + q.push(fd); + } + } + } + // set of visited nodes. + std::unordered_set visited; + while (!q.empty()) { + Rooted n = q.front(); + q.pop(); + // do not proceed if this node was already visited. + if (!visited.insert(n.get()).second) { + continue; + } + + if (n->isa(&RttiTypes::StructuredClass)) { + Rooted strct = n.cast(); + + // look through all fields. + NodeVector fields = strct->getFieldDescriptors(); + for (auto fd : fields) { + // note matches. + if (match(fd)) { + res.push_back(fd); + } + // only continue in the TREE field. + if (fd->getFieldType() == FieldDescriptor::FieldType::TREE) { + q.push(fd); + } + } + + /* + * Furthermore we have to consider that all subclasses of this + * StructuredClass are allowed in place of this StructuredClass as + * well, so we continue the search for them as well. + */ + + NodeVector subs = strct->getSubclasses(); + for (auto sub : subs) { + // note matches. + if (match(sub)) { + res.push_back(sub); + } + // We only continue our search via transparent classes. + if (sub->isTransparent()) { + q.push(sub); + } + } + } else { + // otherwise this is a FieldDescriptor. + Rooted field = n.cast(); + // and we proceed by visiting all permitted children. + for (auto c : field->getChildren()) { + // note matches. + if (match(c)) { + res.push_back(c); + } + // We only continue our search via transparent children. + if (c->isTransparent()) { + q.push(c); + } + } + } + } + return res; +} + +NodeVector Descriptor::getDefaultFields() const +{ + // TODO: In principle a cast would be nicer here, but for now we copy. + NodeVector nodes = collect(this, [](Handle n) { + if (!n->isa(&RttiTypes::FieldDescriptor)) { + return false; + } + Handle f = n.cast(); + return f->getFieldType() == FieldDescriptor::FieldType::TREE && + f->isPrimitive(); + }); + NodeVector res; + for (auto n : nodes) { + res.push_back(n.cast()); + } + return res; } static ssize_t getFieldDescriptorIndex(const NodeVector &fds, diff --git a/src/core/model/Domain.hpp b/src/core/model/Domain.hpp index 91d635e..c277812 100644 --- a/src/core/model/Domain.hpp +++ b/src/core/model/Domain.hpp @@ -612,6 +612,18 @@ public: */ std::pair, bool> pathTo(Handle field, Logger &logger) const; + + /** + * Returns a vector of all TREE fields that are allowed as structure tree + * children of an instance of this Descriptor. This also makes use of + * transparency. + * The list is sorted by the number of transparent elements that have to be + * constructed to arrive at the respective FieldDescriptor. + * + * @return a vector of all TREE fields that are allowed as structure tree + * children of an instance of this Descriptor. + */ + NodeVector getDefaultFields() const; }; /* * TODO: We should discuss Cardinalities one more time. Is it smart to define diff --git a/test/core/model/DomainTest.cpp b/test/core/model/DomainTest.cpp index 672b2d1..83f290f 100644 --- a/test/core/model/DomainTest.cpp +++ b/test/core/model/DomainTest.cpp @@ -221,6 +221,86 @@ TEST(Descriptor, pathToAdvanced) ASSERT_EQ("", path[2]->getName()); } +TEST(Descriptor, getDefaultFields) +{ + // construct a domain with lots of default fields to test. + // start with a single structure class. + Manager mgr{1}; + TerminalLogger logger{std::cout}; + Rooted sys{new SystemTypesystem(mgr)}; + // Construct the domain + Rooted domain{new Domain(mgr, sys, "nasty")}; + + Rooted A{new StructuredClass( + mgr, "A", domain, Cardinality::any(), nullptr, false, true)}; + + // in this trivial case no field should be found. + ASSERT_TRUE(A->getDefaultFields().empty()); + + // create a field. + Rooted A_prim_field = + A->createPrimitiveFieldDescriptor(sys->getStringType(), logger); + // now we should find that. + auto fields = A->getDefaultFields(); + ASSERT_EQ(1, fields.size()); + ASSERT_EQ(A_prim_field, fields[0]); + + // remove that field from A and add it to another class. + + Rooted B{new StructuredClass( + mgr, "B", domain, Cardinality::any(), nullptr, false, true)}; + + B->moveFieldDescriptor(A_prim_field, logger); + + // new we shouldn't find the field anymore. + ASSERT_TRUE(A->getDefaultFields().empty()); + + // but we should find it again if we set B as superclass of A. + A->setSuperclass(B, logger); + fields = A->getDefaultFields(); + ASSERT_EQ(1, fields.size()); + ASSERT_EQ(A_prim_field, fields[0]); + + // and we should not be able to find it if we override the field. + Rooted A_field = A->createFieldDescriptor(logger); + ASSERT_TRUE(A->getDefaultFields().empty()); + + // add a transparent child class. + + Rooted C{new StructuredClass( + mgr, "C", domain, Cardinality::any(), nullptr, true, false)}; + A_field->addChild(C); + + // add a primitive field for it. + Rooted C_field = + C->createPrimitiveFieldDescriptor(sys->getStringType(), logger); + + // now we should find that. + fields = A->getDefaultFields(); + ASSERT_EQ(1, fields.size()); + ASSERT_EQ(C_field, fields[0]); + + // add another transparent child class to A with a daughter class that has + // in turn a subclass with a primitive field. + Rooted D{new StructuredClass( + mgr, "D", domain, Cardinality::any(), nullptr, true, false)}; + A_field->addChild(D); + Rooted D_field = D->createFieldDescriptor(logger); + Rooted E{new StructuredClass( + mgr, "E", domain, Cardinality::any(), nullptr, true, false)}; + D_field->addChild(E); + Rooted F{new StructuredClass( + mgr, "E", domain, Cardinality::any(), E, true, false)}; + Rooted F_field = + F->createPrimitiveFieldDescriptor(sys->getStringType(), logger); + + // now we should find both primitive fields, but the C field first. + fields = A->getDefaultFields(); + ASSERT_EQ(2, fields.size()); + ASSERT_EQ(C_field, fields[0]); + ASSERT_EQ(F_field, fields[1]); +} + TEST(StructuredClass, isSubclassOf) { // create an inheritance hierarchy. -- cgit v1.2.3 From 3ed124aeed2cb65b05f61224115366601ee3b05f Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Thu, 12 Feb 2015 22:36:38 +0100 Subject: added Descriptor::getPermittedChildren. --- src/core/model/Domain.cpp | 13 +++++++++++++ src/core/model/Domain.hpp | 12 ++++++++++++ test/core/model/DomainTest.cpp | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+) (limited to 'test') diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp index 6f33ebd..3cc9755 100644 --- a/src/core/model/Domain.cpp +++ b/src/core/model/Domain.cpp @@ -471,6 +471,19 @@ NodeVector Descriptor::getDefaultFields() const return res; } +NodeVector Descriptor::getPermittedChildren() const +{ + // TODO: In principle a cast would be nicer here, but for now we copy. + NodeVector nodes = collect(this, [](Handle n) { + return n->isa(&RttiTypes::StructuredClass); + }); + NodeVector res; + for (auto n : nodes) { + res.push_back(n.cast()); + } + return res; +} + static ssize_t getFieldDescriptorIndex(const NodeVector &fds, const std::string &name) { diff --git a/src/core/model/Domain.hpp b/src/core/model/Domain.hpp index c277812..6bc2fba 100644 --- a/src/core/model/Domain.hpp +++ b/src/core/model/Domain.hpp @@ -624,6 +624,18 @@ public: * children of an instance of this Descriptor. */ NodeVector getDefaultFields() const; + + /** + * Returns a vector of all StructuredClasses that are allowed as children + * of an instance of this Descriptor in the structure tree. This also makes + * use of transparency. + * The list is sorted by the number of transparent elements that have to be + * constructed to arrive at the respective FieldDescriptor. + * + * @return a vector of all StructuredClasses that are allowed as children + * of an instance of this Descriptor in the structure tree. + */ + NodeVector getPermittedChildren() const; }; /* * TODO: We should discuss Cardinalities one more time. Is it smart to define diff --git a/test/core/model/DomainTest.cpp b/test/core/model/DomainTest.cpp index 83f290f..8fcbdf2 100644 --- a/test/core/model/DomainTest.cpp +++ b/test/core/model/DomainTest.cpp @@ -301,6 +301,43 @@ TEST(Descriptor, getDefaultFields) ASSERT_EQ(F_field, fields[1]); } +TEST(Descriptor, getPermittedChildren) +{ + // analyze the book domain. + TerminalLogger logger{std::cout}; + Manager mgr{1}; + Rooted sys{new SystemTypesystem(mgr)}; + // Get the domain. + Rooted domain = constructBookDomain(mgr, sys, logger); + // get the relevant classes. + Rooted book = getClass("book", domain); + Rooted section = getClass("section", domain); + Rooted paragraph = getClass("paragraph", domain); + Rooted text = getClass("text", domain); + /* + * as permitted children we expect section, paragraph and text in exactly + * that order. section should be before paragraph because of declaration + * order and text should be last because it needs a transparent paragraph + * in between. + */ + NodeVector children = book->getPermittedChildren(); + ASSERT_EQ(3, children.size()); + ASSERT_EQ(section, children[0]); + ASSERT_EQ(paragraph, children[1]); + ASSERT_EQ(text, children[2]); + + // Now we add a subclass to text. + Rooted sub{new StructuredClass( + mgr, "Subclass", domain, Cardinality::any(), text, true, false)}; + // And that should be in the result list as well now. + children = book->getPermittedChildren(); + ASSERT_EQ(4, children.size()); + ASSERT_EQ(section, children[0]); + ASSERT_EQ(paragraph, children[1]); + ASSERT_EQ(text, children[2]); + ASSERT_EQ(sub, children[3]); +} + TEST(StructuredClass, isSubclassOf) { // create an inheritance hierarchy. -- cgit v1.2.3 From 4c8cf263e280bd1e59241b7a783d61e6621a2c5e Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Fri, 13 Feb 2015 11:34:24 +0100 Subject: added VariantReader::parseBool --- src/core/common/VariantReader.cpp | 30 ++++++++++++++++++++ src/core/common/VariantReader.hpp | 13 +++++++++ test/core/common/VariantReaderTest.cpp | 50 +++++++++++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/src/core/common/VariantReader.cpp b/src/core/common/VariantReader.cpp index ef71740..d9e340f 100644 --- a/src/core/common/VariantReader.cpp +++ b/src/core/common/VariantReader.cpp @@ -485,6 +485,36 @@ std::pair VariantReader::parseUnescapedString( return std::make_pair(true, res.str()); } +std::pair VariantReader::parseBool( + CharReader &reader, Logger &logger) +{ + // first we consume all whitespaces. + reader.consumePeek(); + reader.consumeWhitespace(); + // then we try to find the words "true" or "false". + + bool val = false; + CharReaderFork readerFork = reader.fork(); + LoggerFork loggerFork = logger.fork(); + auto res = parseToken(readerFork, loggerFork, {}); + if (res.first) { + bool valid = false; + if (res.second == "true") { + val = true; + valid = true; + } else if (res.second == "false") { + val = false; + valid = true; + } + if (valid) { + readerFork.commit(); + loggerFork.commit(); + return std::make_pair(true, val); + } + } + return std::make_pair(false, val); +} + std::pair VariantReader::parseInteger( CharReader &reader, Logger &logger, const std::unordered_set &delims) { diff --git a/src/core/common/VariantReader.hpp b/src/core/common/VariantReader.hpp index 6b157d8..6a87723 100644 --- a/src/core/common/VariantReader.hpp +++ b/src/core/common/VariantReader.hpp @@ -132,6 +132,19 @@ public: CharReader &reader, Logger &logger, const std::unordered_set &delims); + /** + * Parses a bool from the given CharReader instance (the strings "true" or + * "false"). + * + * @param reader is a reference to the CharReader instance which is + * the source for the character data. The reader will be positioned after + * the bool. + * @param logger is the logger instance that should be used to log error + * messages and warnings. + */ + static std::pair parseBool(CharReader &reader, + Logger &logger); + /** * Parses an integer from the given CharReader instance until one of the * given delimiter characters is reached. diff --git a/test/core/common/VariantReaderTest.cpp b/test/core/common/VariantReaderTest.cpp index f6a699b..7a688c5 100644 --- a/test/core/common/VariantReaderTest.cpp +++ b/test/core/common/VariantReaderTest.cpp @@ -205,7 +205,7 @@ TEST(VariantReader, parseUnescapedString) // Simple case with whitespace { - CharReader reader(" hello world ; "); + CharReader reader(" hello world ; aha"); auto res = VariantReader::parseUnescapedString(reader, logger, {';'}); ASSERT_TRUE(res.first); ASSERT_EQ("hello world", res.second); @@ -228,6 +228,54 @@ TEST(VariantReader, parseUnescapedString) } } +TEST(VariantReader, parseBool) +{ + // Valid bools + { + CharReader reader(" true "); + auto res = VariantReader::parseBool(reader, logger); + ASSERT_TRUE(res.first); + ASSERT_TRUE(res.second); + } + { + CharReader reader(" false "); + auto res = VariantReader::parseBool(reader, logger); + ASSERT_TRUE(res.first); + ASSERT_FALSE(res.second); + } + { + CharReader reader(" true bla"); + auto res = VariantReader::parseBool(reader, logger); + ASSERT_TRUE(res.first); + ASSERT_TRUE(res.second); + reader.consumeWhitespace(); + char c; + ASSERT_TRUE(reader.read(c)); + ASSERT_EQ('b', c); + ASSERT_TRUE(reader.read(c)); + ASSERT_EQ('l', c); + ASSERT_TRUE(reader.read(c)); + ASSERT_EQ('a', c); + ASSERT_FALSE(reader.read(c)); + } + // invalid bools. + { + CharReader reader(" bla "); + auto res = VariantReader::parseBool(reader, logger); + ASSERT_FALSE(res.first); + } + { + CharReader reader(" TRUE "); + auto res = VariantReader::parseBool(reader, logger); + ASSERT_FALSE(res.first); + } + { + CharReader reader(" tr ue "); + auto res = VariantReader::parseBool(reader, logger); + ASSERT_FALSE(res.first); + } +} + static const std::unordered_set noDelim; TEST(VariantReader, parseInteger) -- cgit v1.2.3 From addf63c2337bc1c5e31cad5790c46ac8ae13a724 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Fri, 13 Feb 2015 13:58:21 +0100 Subject: added VariantReader::parseTyped --- src/core/common/VariantReader.cpp | 84 +++++++++++++++++++++++++++++++++- src/core/common/VariantReader.hpp | 36 ++++++++++++++- test/core/common/VariantReaderTest.cpp | 65 ++++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 4 deletions(-) (limited to 'test') diff --git a/src/core/common/VariantReader.cpp b/src/core/common/VariantReader.cpp index cac29b9..3f02226 100644 --- a/src/core/common/VariantReader.cpp +++ b/src/core/common/VariantReader.cpp @@ -484,8 +484,8 @@ std::pair VariantReader::parseUnescapedString( return std::make_pair(true, res.str()); } -std::pair VariantReader::parseBool( - CharReader &reader, Logger &logger) +std::pair VariantReader::parseBool(CharReader &reader, + Logger &logger) { // first we consume all whitespaces. reader.consumePeek(); @@ -867,5 +867,85 @@ std::pair VariantReader::parseGenericString( v.setLocation({sourceId, offs, offs + str.size()}); return std::make_pair(true, v); } + +std::pair VariantReader::parseTyped( + VariantType type, CharReader &reader, Logger &logger, + const std::unordered_set &delims) +{ + switch (type) { + case VariantType::BOOL: { + auto res = parseBool(reader, logger); + return std::make_pair(res.first, Variant{res.second}); + } + case VariantType::INT: { + auto res = parseInteger(reader, logger, delims); + if (res.second < std::numeric_limits::min() || + res.second > std::numeric_limits::max()) { + logger.error("Number exceeds type limits.", reader); + return std::make_pair(false, Variant{}); + } + return std::make_pair( + res.first, Variant{static_cast(res.second)}); + } + case VariantType::DOUBLE: { + auto res = parseDouble(reader, logger, delims); + return std::make_pair(res.first, Variant{res.second}); + } + case VariantType::STRING: { + auto res = parseString(reader, logger, delims); + return std::make_pair(res.first, Variant::fromString(res.second)); + } + case VariantType::ARRAY: { + char delim = 0; + if (delims.size() == 1) { + delim = *delims.begin(); + } + auto res = parseArray(reader, logger, delim); + return std::make_pair(res.first, Variant{res.second}); + } + + case VariantType::MAP: + case VariantType::OBJECT: { + char delim = 0; + if (delims.size() == 1) { + delim = *delims.begin(); + } + auto res = parseObject(reader, logger, delim); + return std::make_pair(res.first, Variant{res.second}); + } + case VariantType::CARDINALITY: { + auto res = parseCardinality(reader, logger); + return std::make_pair(res.first, Variant{res.second}); + } + default: + break; + } + + return std::make_pair(false, Variant{}); +} + +std::pair VariantReader::parseTyped(VariantType type, + const std::string &str, + Logger &logger, + SourceId sourceId, + size_t offs) +{ + // create a char reader and forward the method. + CharReader reader{str, sourceId, offs}; + LoggerFork loggerFork = logger.fork(); + std::pair res = + parseTyped(type, reader, loggerFork, std::unordered_set{}); + + // If all content could be parsed, commit the result. + if (reader.atEnd()) { + loggerFork.commit(); + return res; + } + + // otherwise do not. + logger.error("Not all input could be processed", + {sourceId, offs, offs + str.size()}); + return std::make_pair(false, Variant{}); +} } diff --git a/src/core/common/VariantReader.hpp b/src/core/common/VariantReader.hpp index 6a87723..1232f6e 100644 --- a/src/core/common/VariantReader.hpp +++ b/src/core/common/VariantReader.hpp @@ -182,7 +182,7 @@ public: * * @param reader is a reference to the CharReader instance which is * the source for the character data. The reader will be positioned after - * the number or at the terminating delimiting character. + * the array or at the terminating delimiting character. * @param logger is the logger instance that should be used to log error * messages and warnings. * @param delim is the terminating character. If nonzero, the parse function @@ -198,7 +198,7 @@ public: * * @param reader is a reference to the CharReader instance which is * the source for the character data. The reader will be positioned after - * the number or at the terminating delimiting character. + * the object or at the terminating delimiting character. * @param logger is the logger instance that should be used to log error * messages and warnings. * @param delim is the terminating character. If nonzero, the parse function @@ -306,6 +306,38 @@ public: static std::pair parseGenericString( const std::string &str, Logger &logger, SourceId sourceId = InvalidSourceId, size_t offs = 0); + + /** + * Tries to parse an instance of the given type from the given stream. The + * called method is one of the parse methods defined here and adheres to the + * specifications defined there. + * + * @param type is the VariantType for which an instance shall be parsed. + * @param reader is a reference to the CharReader instance which is + * the source for the character data. The reader will be positioned + * at the end of the type instance (or the delimiting character). + * @param delims is a set of characters which will terminate the typed + * instance if the according parser uses delimiting characters. + * These characters are not included in the result. May not be nullptr. + */ + static std::pair parseTyped( + VariantType type, CharReader &reader, Logger &logger, + const std::unordered_set &delims = {}); + /** + * Tries to parse an instance of the given type from the given string. The + * called method is one of the parse methods defined here and adheres to the + * specifications defined there. + * + * @param type is the VariantType for which an instance shall be parsed. + * @param str is the string from which the value should be read. + * @param sourceId is an optional descriptor of the source file from which + * the element is being read. + * @param offs is the by offset in the source file at which the string + * starts. + */ + static std::pair parseTyped( + VariantType type, const std::string &str, Logger &logger, + SourceId sourceId = InvalidSourceId, size_t offs = 0); }; } diff --git a/test/core/common/VariantReaderTest.cpp b/test/core/common/VariantReaderTest.cpp index 7a688c5..feaf06d 100644 --- a/test/core/common/VariantReaderTest.cpp +++ b/test/core/common/VariantReaderTest.cpp @@ -1182,5 +1182,70 @@ TEST(VariantReader, parseGenericComplex) ASSERT_TRUE(reader.peek(c)); ASSERT_EQ(';', c); } + +TEST(VariantReader, parseTyped) +{ + { + auto res = VariantReader::parseTyped(VariantType::BOOL, "true", logger); + ASSERT_TRUE(res.first); + ASSERT_EQ(VariantType::BOOL, res.second.getType()); + ASSERT_TRUE(res.second.asBool()); + } + { + auto res = + VariantReader::parseTyped(VariantType::INT, " 1254", logger); + ASSERT_TRUE(res.first); + ASSERT_EQ(VariantType::INT, res.second.getType()); + ASSERT_EQ(1254, res.second.asInt()); + } + { + auto res = + VariantReader::parseTyped(VariantType::DOUBLE, " 3.14", logger); + ASSERT_TRUE(res.first); + ASSERT_EQ(VariantType::DOUBLE, res.second.getType()); + ASSERT_EQ(3.14, res.second.asDouble()); + } + { + auto res = VariantReader::parseTyped(VariantType::STRING, + "\'my string\'", logger); + ASSERT_TRUE(res.first); + ASSERT_EQ(VariantType::STRING, res.second.getType()); + ASSERT_EQ("my string", res.second.asString()); + } + { + auto res = + VariantReader::parseTyped(VariantType::STRING, "my string", logger); + ASSERT_FALSE(res.first); + } + { + auto res = + VariantReader::parseTyped(VariantType::ARRAY, "[1, 4, 5]", logger); + ASSERT_TRUE(res.first); + ASSERT_EQ(VariantType::ARRAY, res.second.getType()); + Variant::arrayType actual = res.second.asArray(); + Variant::arrayType expected{{1}, {4}, {5}}; + ASSERT_EQ(expected, actual); + } + { + auto res = VariantReader::parseTyped( + VariantType::MAP, "[a=\"str\", b=true, i=4]", logger); + ASSERT_TRUE(res.first); + ASSERT_EQ(VariantType::MAP, res.second.getType()); + Variant::mapType actual = res.second.asMap(); + Variant::mapType expected{{"a", {"str"}}, {"b", {true}}, {"i", {4}}}; + ASSERT_EQ(expected, actual); + } + { + auto res = VariantReader::parseTyped(VariantType::CARDINALITY, + "{1-2, >18}", logger); + ASSERT_TRUE(res.first); + ASSERT_EQ(VariantType::CARDINALITY, res.second.getType()); + Variant::cardinalityType actual = res.second.asCardinality(); + Variant::cardinalityType expected; + expected.merge({1, 2}); + expected.merge(Variant::rangeType::typeRangeFrom(19)); + ASSERT_EQ(expected, actual); + } +} } -- cgit v1.2.3 From e36b245cfd9a8e0f69530e8143634d75cbae787b Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Fri, 13 Feb 2015 20:19:33 +0100 Subject: added a test case which lead to a truly interesting bug that cost us quite some time. --- test/core/common/VariantReaderTest.cpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'test') diff --git a/test/core/common/VariantReaderTest.cpp b/test/core/common/VariantReaderTest.cpp index feaf06d..a23af09 100644 --- a/test/core/common/VariantReaderTest.cpp +++ b/test/core/common/VariantReaderTest.cpp @@ -622,6 +622,13 @@ TEST(VariantReader, parseObject) auto res = VariantReader::parseObject(reader, logger); ASSERT_FALSE(res.first); } + + // Mutliple commas + { + CharReader reader("[r=50,,t=70, b=70,g=60]"); + auto res = VariantReader::parseObject(reader, logger); + ASSERT_FALSE(res.first); + } } TEST(VariantReader, parseCardinality) -- cgit v1.2.3