From 37a002577e7f4949e0634dca7e828b5333d2ccc5 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Sun, 25 Jan 2015 13:19:48 +0100 Subject: Bugfixes in VariantReader for cardinality reading and extensive cardinality reading tests. --- src/core/common/VariantReader.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'src/core') diff --git a/src/core/common/VariantReader.cpp b/src/core/common/VariantReader.cpp index 7320973..c0368f1 100644 --- a/src/core/common/VariantReader.cpp +++ b/src/core/common/VariantReader.cpp @@ -559,6 +559,7 @@ std::pair VariantReader::parseCardinality( switch (c) { case '}': case ',': + card.merge({start}); reader.resetPeek(); break; case '-': { @@ -569,6 +570,7 @@ std::pair VariantReader::parseCardinality( error(reader, logger, ERR_UNEXPECTED_END, Variant::cardinalityType{}); } + reader.resetPeek(); Number n2; if (!n2.parse(reader, logger, cardDelims) || !n2.isInt() || n2.intValue() < 0) { @@ -576,8 +578,16 @@ std::pair VariantReader::parseCardinality( "Invalid number for cardinality!", Variant::cardinalityType{}); } - unsigned int end = (unsigned int)n2.intValue(); + if (end <= start) { + return error(reader, logger, + std::string("The start of the range (") + + std::to_string(start) + + ") was bigger (or equal) to the end " + "of the range (" + + std::to_string(end) + ")!", + Variant::cardinalityType{}); + } card.merge({start, end}); break; } -- cgit v1.2.3 From eab6577b066319aab7ebaf514e6bb7aab9590624 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Sun, 25 Jan 2015 15:50:52 +0100 Subject: removed cardinality type definition from Domain.hpp and made everything compatible with the Variant cardinality. --- src/core/model/Document.cpp | 8 ++++++-- src/core/model/Domain.cpp | 17 +++++++++++------ src/core/model/Domain.hpp | 15 +++++++-------- test/core/model/DocumentTest.cpp | 2 +- test/core/model/DomainTest.cpp | 8 ++++---- test/core/model/TestAdvanced.hpp | 6 +++--- test/core/model/TestDomain.hpp | 6 +++--- 7 files changed, 35 insertions(+), 27 deletions(-) (limited to 'src/core') diff --git a/src/core/model/Document.cpp b/src/core/model/Document.cpp index 61c384d..42192a2 100644 --- a/src/core/model/Document.cpp +++ b/src/core/model/Document.cpp @@ -147,6 +147,10 @@ bool DocumentEntity::doValidate(Logger &logger) const // a constructor we can not check anything else. return false; } + // if we have an invalid descriptor we can not proceed either. + if(!descriptor->validate(logger)){ + return false; + } // check the attribute primitive content. bool valid; if (descriptor->getAttributesDescriptor() == nullptr) { @@ -218,7 +222,7 @@ bool DocumentEntity::doValidate(Logger &logger) const * cardinality. */ for (auto &ac : fieldDescs[f]->getChildren()) { - const size_t min = ac->getCardinality().min(); + const size_t min = ac->getCardinality().asCardinality().min(); if (min > 0) { logger.error( std::string("Field \"") + fieldDescs[f]->getName() + @@ -305,7 +309,7 @@ bool DocumentEntity::doValidate(Logger &logger) const if (n != nums.end()) { num = n->second; } - if (!ac->getCardinality().contains(num)) { + if (!ac->getCardinality().asCardinality().contains(num)) { logger.error( std::string("Field \"") + fieldDescs[f]->getName() + "\" had " + std::to_string(num) + " elements of class \"" + diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp index c7afd22..635fc50 100644 --- a/src/core/model/Domain.cpp +++ b/src/core/model/Domain.cpp @@ -306,13 +306,12 @@ Rooted Descriptor::createFieldDescriptor( /* Class StructuredClass */ StructuredClass::StructuredClass(Manager &mgr, std::string name, - Handle domain, - const Cardinality &cardinality, + Handle domain, Variant cardinality, Handle attributesDescriptor, Handle superclass, bool transparent, bool root) : Descriptor(mgr, std::move(name), domain, attributesDescriptor), - cardinality(cardinality), + cardinality(std::move(cardinality)), superclass(acquire(superclass)), subclasses(this), transparent(transparent), @@ -338,6 +337,11 @@ bool StructuredClass::doValidate(Logger &logger) const valid = false; } } + // check the cardinality. + if(!cardinality.isCardinality()){ + logger.error(cardinality.toString() + " is not a cardinality!"); + valid = false; + } // check the validity of this superclass. if (superclass != nullptr) { valid = valid & superclass->validate(logger); @@ -496,13 +500,14 @@ bool Domain::removeStructuredClass(Handle s) } Rooted Domain::createStructuredClass( - std::string name, const Cardinality &cardinality, + std::string name, Variant cardinality, Handle attributesDescriptor, Handle superclass, bool transparent, bool root) { return Rooted{new StructuredClass( - getManager(), std::move(name), this, cardinality, attributesDescriptor, - superclass, std::move(transparent), std::move(root))}; + getManager(), std::move(name), this, std::move(cardinality), + attributesDescriptor, superclass, std::move(transparent), + std::move(root))}; } void Domain::addAnnotationClass(Handle a) diff --git a/src/core/model/Domain.hpp b/src/core/model/Domain.hpp index e40a9f3..ef228b1 100644 --- a/src/core/model/Domain.hpp +++ b/src/core/model/Domain.hpp @@ -618,20 +618,19 @@ public: * cardinalities independent of context? Should we not have at least have the * possibility to define it context-dependently? */ -typedef RangeSet Cardinality; /** * This is the default cardinality. */ -static Cardinality createAny() +static Variant::cardinalityType createAny() { - Cardinality any; + Variant::cardinalityType any; any.merge(Range::typeRangeFrom(0)); return std::move(any); } -static const Cardinality AnyCardinality = createAny(); +static const Variant AnyCardinality = createAny(); /** * A StructuredClass specifies nodes in the StructureTree of a document that @@ -710,7 +709,7 @@ class StructuredClass : public Descriptor { friend Domain; private: - const Cardinality cardinality; + const Variant cardinality; Owned superclass; NodeVector subclasses; bool transparent; @@ -758,7 +757,7 @@ public: */ StructuredClass(Manager &mgr, std::string name = "", Handle domain = nullptr, - const Cardinality &cardinality = AnyCardinality, + Variant cardinality = AnyCardinality, Handle attributesDescriptor = nullptr, Handle superclass = nullptr, bool transparent = false, bool root = false); @@ -768,7 +767,7 @@ public: * * @return the Cardinality of this StructuredClass (as a RangeSet). */ - const Cardinality &getCardinality() const { return cardinality; } + const Variant &getCardinality() const { return cardinality; } /** * Returns the superclass of this StructuredClass. This is not the same as @@ -1016,7 +1015,7 @@ public: * @return the newly created StructuredClass. */ Rooted createStructuredClass( - std::string name, const Cardinality &cardinality = AnyCardinality, + std::string name, Variant cardinality = AnyCardinality, Handle attributesDescriptor = nullptr, Handle superclass = nullptr, bool transparent = false, bool root = false); diff --git a/test/core/model/DocumentTest.cpp b/test/core/model/DocumentTest.cpp index e07deb8..1d8457c 100644 --- a/test/core/model/DocumentTest.cpp +++ b/test/core/model/DocumentTest.cpp @@ -117,7 +117,7 @@ TEST(Document, validate) Manager mgr{1}; Rooted sys{new SystemTypesystem(mgr)}; Rooted domain{new Domain(mgr, sys, "trivial")}; - Cardinality single; + Variant::cardinalityType single; single.merge({1}); // Set up the "root" StructuredClass. Rooted rootClass{new StructuredClass( diff --git a/test/core/model/DomainTest.cpp b/test/core/model/DomainTest.cpp index c00b122..54fd86a 100644 --- a/test/core/model/DomainTest.cpp +++ b/test/core/model/DomainTest.cpp @@ -152,8 +152,8 @@ TEST(Descriptor, pathToAdvanced) Rooted sys{new SystemTypesystem(mgr)}; // Construct the domain Rooted domain{new Domain(mgr, sys, "nasty")}; - Cardinality any; - any.merge(Range::typeRangeFrom(0)); + Variant::cardinalityType any; + any.merge(Range::typeRange()); // Let's create the classes that we need first Rooted A{new StructuredClass( @@ -225,8 +225,8 @@ TEST(StructuredClass, isSubclassOf) Manager mgr{1}; Rooted sys{new SystemTypesystem(mgr)}; Rooted domain{new Domain(mgr, sys, "inheritance")}; - Cardinality any; - any.merge(Range::typeRangeFrom(0)); + Variant::cardinalityType any; + any.merge(Range::typeRange()); Rooted A{new StructuredClass( mgr, "A", domain, any, {nullptr}, {nullptr}, false, true)}; // first branch diff --git a/test/core/model/TestAdvanced.hpp b/test/core/model/TestAdvanced.hpp index f8585d1..6f8ca33 100644 --- a/test/core/model/TestAdvanced.hpp +++ b/test/core/model/TestAdvanced.hpp @@ -53,7 +53,7 @@ static Rooted constructHeadingDomain(Manager &mgr, // set up domain node. Rooted domain{new Domain(mgr, sys, "headings")}; // set up cardinality (every section may have at most one heading). - Cardinality card; + Variant::cardinalityType card; card.merge({0, 1}); // set up heading StructuredClass. Rooted heading{new StructuredClass( @@ -84,8 +84,8 @@ static Rooted constructListDomain(Manager &mgr, // set up domain node. Rooted domain{new Domain(mgr, sys, "list")}; // set up cardinality - Cardinality any; - any.merge(Range::typeRangeFrom(0)); + Variant::cardinalityType any; + any.merge(Range::typeRange()); // get book.paragraph Rooted p = resolveDescriptor(bookDomain, "paragraph"); // set up item StructuredClass; diff --git a/test/core/model/TestDomain.hpp b/test/core/model/TestDomain.hpp index f6b8805..ec63216 100644 --- a/test/core/model/TestDomain.hpp +++ b/test/core/model/TestDomain.hpp @@ -36,10 +36,10 @@ static Rooted constructBookDomain(Manager &mgr, // Start with the Domain itself. Rooted domain{new Domain(mgr, sys, "book")}; // Set up the cardinalities we'll need. - Cardinality single; + Variant::cardinalityType single; single.merge({1}); - Cardinality any; - any.merge(Range::typeRangeFrom(0)); + Variant::cardinalityType any; + any.merge(Range::typeRange()); // Set up the "book" node. Rooted book{new StructuredClass( -- cgit v1.2.3 From 93a8af2431529d145919c5898971c503d6c73e93 Mon Sep 17 00:00:00 2001 From: Benjamin Paassen Date: Sun, 25 Jan 2015 18:28:56 +0100 Subject: added node information to error logging in Document and Domain validation. --- src/core/model/Document.cpp | 102 +++++++++++++++++++++++++------------------- src/core/model/Domain.cpp | 24 +++++------ 2 files changed, 71 insertions(+), 55 deletions(-) (limited to 'src/core') diff --git a/src/core/model/Document.cpp b/src/core/model/Document.cpp index 42192a2..f22ccd6 100644 --- a/src/core/model/Document.cpp +++ b/src/core/model/Document.cpp @@ -142,13 +142,13 @@ bool DocumentEntity::doValidate(Logger &logger) const { // if we have no descriptor, this is invalid. if (descriptor == nullptr) { - logger.error("This entity has no descriptor!"); + logger.error("This entity has no descriptor!", *subInst); // in this case we have to stop the validation process, because without // a constructor we can not check anything else. return false; } // if we have an invalid descriptor we can not proceed either. - if(!descriptor->validate(logger)){ + if (!descriptor->validate(logger)) { return false; } // check the attribute primitive content. @@ -181,8 +181,9 @@ bool DocumentEntity::doValidate(Logger &logger) const case 0: if (!fieldDescs[f]->isOptional()) { logger.error(std::string("Primitive Field \"") + - fieldDescs[f]->getName() + - "\" had no content!"); + fieldDescs[f]->getName() + + "\" had no content!", + *subInst); valid = false; } continue; @@ -190,16 +191,18 @@ bool DocumentEntity::doValidate(Logger &logger) const break; default: logger.error(std::string("Primitive Field \"") + - fieldDescs[f]->getName() + - "\" had more than one child!"); + fieldDescs[f]->getName() + + "\" had more than one child!", + *subInst); valid = false; continue; } // if we are here we know that exactly one child exists. if (!fields[f][0]->isa(RttiTypes::DocumentPrimitive)) { logger.error(std::string("Primitive Field \"") + - fieldDescs[f]->getName() + - "\" has non primitive content!"); + fieldDescs[f]->getName() + + "\" has non primitive content!", + *subInst); valid = false; } else { Handle primitive = @@ -224,12 +227,14 @@ bool DocumentEntity::doValidate(Logger &logger) const for (auto &ac : fieldDescs[f]->getChildren()) { const size_t min = ac->getCardinality().asCardinality().min(); if (min > 0) { - logger.error( - std::string("Field \"") + fieldDescs[f]->getName() + - "\" was empty but needs at least " + - std::to_string(min) + " elements of class \"" + - ac->getName() + "\" according to the definition of \"" + - descriptor->getName() + "\""); + logger.error(std::string("Field \"") + + fieldDescs[f]->getName() + + "\" was empty but needs at least " + + std::to_string(min) + + " elements of class \"" + ac->getName() + + "\" according to the definition of \"" + + descriptor->getName() + "\"", + *subInst); valid = false; } } @@ -249,8 +254,9 @@ bool DocumentEntity::doValidate(Logger &logger) const // check if the parent reference is correct. if (rc->getParent() != subInst) { logger.error(std::string("A child of field \"") + - fieldDescs[f]->getName() + - "\" has the wrong parent reference!"); + fieldDescs[f]->getName() + + "\" has the wrong parent reference!", + *rc); valid = false; } if (rc->isa(RttiTypes::Anchor)) { @@ -259,8 +265,9 @@ bool DocumentEntity::doValidate(Logger &logger) const } if (rc->isa(RttiTypes::DocumentPrimitive)) { logger.error(std::string("Non-primitive Field \"") + - fieldDescs[f]->getName() + - "\" had primitive content!"); + fieldDescs[f]->getName() + + "\" had primitive content!", + *rc); valid = false; continue; } @@ -285,11 +292,13 @@ bool DocumentEntity::doValidate(Logger &logger) const } } if (!allowed) { - logger.error(std::string("An instance of \"") + - c->getDescriptor()->getName() + - "\" is not allowed as child of an instance of \"" + - descriptor->getName() + "\" in field \"" + - fieldDescs[f]->getName() + "\""); + logger.error( + std::string("An instance of \"") + + c->getDescriptor()->getName() + + "\" is not allowed as child of an instance of \"" + + descriptor->getName() + "\" in field \"" + + fieldDescs[f]->getName() + "\"", + *rc); valid = false; continue; } @@ -310,12 +319,14 @@ bool DocumentEntity::doValidate(Logger &logger) const num = n->second; } if (!ac->getCardinality().asCardinality().contains(num)) { - logger.error( - std::string("Field \"") + fieldDescs[f]->getName() + - "\" had " + std::to_string(num) + " elements of class \"" + - ac->getName() + - "\", which is invalid according to the definition of \"" + - descriptor->getName() + "\""); + logger.error(std::string("Field \"") + + fieldDescs[f]->getName() + "\" had " + + std::to_string(num) + " elements of class \"" + + ac->getName() + + "\", which is invalid according to the " + "definition of \"" + + descriptor->getName() + "\"", + *subInst); valid = false; continue; } @@ -466,13 +477,13 @@ bool StructureNode::doValidate(Logger &logger) const } // check the parent. if (getParent() == nullptr) { - logger.error("The parent is not set!"); + logger.error("The parent is not set!", *this); valid = false; } if (!getParent()->isa(RttiTypes::StructuredEntity) && !getParent()->isa(RttiTypes::AnnotationEntity) && !getParent()->isa(RttiTypes::Document)) { - logger.error("The parent does not have a valid type!"); + logger.error("The parent does not have a valid type!", *this); valid = false; } return valid; @@ -524,7 +535,7 @@ bool Anchor::doValidate(Logger &logger) const bool valid = true; // check name if (getName().empty()) { - logger.error("An Anchor needs a name!"); + logger.error("An Anchor needs a name!", *this); valid = false; } return valid & StructureNode::doValidate(logger); @@ -555,10 +566,10 @@ bool AnnotationEntity::doValidate(Logger &logger) const } // check if this AnnotationEntity is correctly registered at its document. if (getParent() == nullptr) { - logger.error("The parent is not set!"); + logger.error("The parent is not set!", *this); valid = false; } else if (!getParent()->isa(RttiTypes::Document)) { - logger.error("The parent is not a document!"); + logger.error("The parent is not a document!", *this); valid = false; } else { Handle doc = getParent().cast(); @@ -570,26 +581,29 @@ bool AnnotationEntity::doValidate(Logger &logger) const } } if (!found) { - logger.error("This annotation was not registered at the document."); + logger.error("This annotation was not registered at the document.", + *this); valid = false; } // check if the Anchors are part of the right document. if (start == nullptr) { - logger.error("This annotation has no start Anchor!"); + logger.error("This annotation has no start Anchor!", *this); valid = false; } else if (!doc->hasChild(start)) { logger.error( "This annotations start anchor was not part of the same " - "document!"); + "document!", + *this); valid = false; } if (end == nullptr) { - logger.error("This annotation has no end Anchor!"); + logger.error("This annotation has no end Anchor!", *this); valid = false; } else if (!doc->hasChild(end)) { logger.error( "This annotations end anchor was not part of the same " - "document!"); + "document!", + *this); valid = false; } } @@ -613,7 +627,7 @@ bool Document::doValidate(Logger &logger) const // An empty document is always invalid. TODO: Is this a smart choice? bool valid = true; if (root == nullptr) { - logger.error("This document is empty (it has no root)!"); + logger.error("This document is empty (it has no root)!", *this); valid = false; } else { // check if the root is allowed to be a root. @@ -621,14 +635,16 @@ bool Document::doValidate(Logger &logger) const .cast() ->hasRootPermission()) { logger.error(std::string("A node of type \"") + - root->getDescriptor()->getName() + - "\" is not allowed to be the Document root!"); + root->getDescriptor()->getName() + + "\" is not allowed to be the Document root!", + *root); valid = false; } // check if it has this document as parent. if (root->getParent() != this) { logger.error( - "The document root does not have the document as parent!"); + "The document root does not have the document as parent!", + *root); valid = false; } // then call validate on the root diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp index 635fc50..3c4b64c 100644 --- a/src/core/model/Domain.cpp +++ b/src/core/model/Domain.cpp @@ -60,10 +60,10 @@ bool FieldDescriptor::doValidate(Logger &logger) const bool valid = true; // check parent type if (getParent() == nullptr) { - logger.error("This field has no parent!"); + logger.error("This field has no parent!", *this); valid = false; } else if (!getParent()->isa(RttiTypes::Descriptor)) { - logger.error("The parent of this field is not a descriptor!"); + logger.error("The parent of this field is not a descriptor!", *this); valid = false; } // check name @@ -75,20 +75,20 @@ bool FieldDescriptor::doValidate(Logger &logger) const if (children.size() > 0) { logger.error( "This field is supposed to be primitive but has " - "registered child classes!"); + "registered child classes!", *this); valid = false; } if (primitiveType == nullptr) { logger.error( "This field is supposed to be primitive but has " - "no primitive type!"); + "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!"); + "a primitive type!", *this); valid = false; } } @@ -102,7 +102,7 @@ bool FieldDescriptor::doValidate(Logger &logger) const if (!names.insert(c->getName()).second) { logger.error(std::string("Field \"") + getName() + "\" had multiple children with the name \"" + - c->getName() + "\""); + c->getName() + "\"", *this); valid = false; } } @@ -139,15 +139,15 @@ bool Descriptor::doValidate(Logger &logger) const bool valid = true; // check parent type if (getParent() == nullptr) { - logger.error("This Descriptor has no parent!"); + logger.error("This Descriptor has no parent!", *this); valid = false; } else if (!getParent()->isa(RttiTypes::Domain)) { - logger.error("The parent of this Descriptor is not a Domain!"); + logger.error("The parent of this Descriptor is not a Domain!", *this); valid = false; } // check name if (getName().empty()) { - logger.error("The name of this Descriptor is empty!"); + logger.error("The name of this Descriptor is empty!", *this); valid = false; } else { valid = valid & validateName(logger); @@ -160,7 +160,7 @@ bool Descriptor::doValidate(Logger &logger) const "field \"" + fd->getName() + "\" as child but the field does not " - "have the Descriptor as parent."); + "have the Descriptor as parent.", *this); valid = false; } } @@ -333,13 +333,13 @@ bool StructuredClass::doValidate(Logger &logger) const if (sub->getSuperclass() != this) { logger.error(std::string("Struct \"") + sub->getName() + "\" is registered as subclass of \"" + getName() + - "\" but does not have it as superclass!"); + "\" but does not have it as superclass!", *this); valid = false; } } // check the cardinality. if(!cardinality.isCardinality()){ - logger.error(cardinality.toString() + " is not a cardinality!"); + logger.error(cardinality.toString() + " is not a cardinality!", *this); valid = false; } // check the validity of this superclass. -- cgit v1.2.3