Improved validation system

4 files changed, 128 insertions, 84 deletions

diff --git a/src/core/model/Document.cpp b/src/core/model/Document.cpp
index 92791a4..0de750f 100644
--- a/src/core/model/Document.cpp
+++ b/src/core/model/Document.cpp
@@ -92,8 +92,7 @@ int DocumentEntity::getFieldDescriptorIndex(
 	}
 }
 
-bool DocumentEntity::doValidate(Logger &logger,
-                                std::set<ManagedUid> &visited) const
+bool DocumentEntity::validate(Logger &logger) const
 {
 	// TODO: check the validated form of Attributes
 	// iterate over every field
@@ -205,14 +204,7 @@ bool DocumentEntity::doValidate(Logger &logger,
 	// go into recursion.
 	for (auto &f : fields) {
 		for (auto &n : f) {
-			if (!visited.insert(n->getUid()).second) {
-				logger.error("The given document contains a cycle!");
-				return false;
-			}
-			if (n->isValidated()) {
-				continue;
-			}
-			if (!n->validate(logger, visited)) {
+			if (!n->validate(logger)) {
 				return false;
 			}
 		}
@@ -247,15 +239,14 @@ StructuredEntity::StructuredEntity(Manager &mgr, Handle<Document> doc,
 	doc->setRoot(this);
 }
 
-bool StructuredEntity::doValidate(Logger &logger,
-                                  std::set<ManagedUid> &visited) const
+bool StructuredEntity::doValidate(Logger &logger) const
 {
 	// check if the parent is set.
 	if (getParent() == nullptr) {
 		return false;
 	}
 	// check the validity as a DocumentEntity.
-	return DocumentEntity::doValidate(logger, visited);
+	return DocumentEntity::validate(logger);
 }
 
 /* Class AnnotationEntity */
@@ -272,8 +263,7 @@ AnnotationEntity::AnnotationEntity(Manager &mgr, Handle<Document> parent,
 	parent->annotations.push_back(this);
 }
 
-bool AnnotationEntity::doValidate(Logger &logger,
-                                  std::set<ManagedUid> &visited) const
+bool AnnotationEntity::doValidate(Logger &logger) const
 {
 	// check if this AnnotationEntity is correctly registered at its document.
 	if (getParent() == nullptr || !getParent()->isa(RttiTypes::Document)) {
@@ -293,7 +283,7 @@ bool AnnotationEntity::doValidate(Logger &logger,
 	}
 
 	// check the validity as a DocumentEntity.
-	if (!DocumentEntity::doValidate(logger, visited)) {
+	if (!DocumentEntity::validate(logger)) {
 		return false;
 	}
 	// TODO: then check if the anchors are in the correct document.
diff --git a/src/core/model/Document.hpp b/src/core/model/Document.hpp
index b73e07d..5af3ce2 100644
--- a/src/core/model/Document.hpp
+++ b/src/core/model/Document.hpp
@@ -161,7 +161,7 @@ protected:
 		fields[getFieldDescriptorIndex(fieldName, true)].push_back(s);
 	}
 
-	bool doValidate(Logger &logger, std::set<ManagedUid> &visited) const;
+	bool validate(Logger &logger) const;
 
 public:
 	/**
@@ -369,8 +369,7 @@ public:
  */
 class StructuredEntity : public StructureNode, public DocumentEntity {
 protected:
-	bool doValidate(Logger &logger,
-	                std::set<ManagedUid> &visited) const override;
+	bool doValidate(Logger &logger) const override;
 
 public:
 	/**
@@ -514,8 +513,7 @@ private:
 	Owned<Anchor> start;
 	Owned<Anchor> end;
 protected:
-	bool doValidate(Logger &logger,
-	                std::set<ManagedUid> &visited) const override;
+	bool doValidate(Logger &logger) const override;
 
 public:
 	/**
diff --git a/src/core/model/Node.cpp b/src/core/model/Node.cpp
index 17ee9b3..da83598 100644
--- a/src/core/model/Node.cpp
+++ b/src/core/model/Node.cpp
@@ -20,6 +20,7 @@
 #include <unordered_set>
 
 #include <core/common/Exceptions.hpp>
+#include <core/common/Logger.hpp>
 #include <core/common/Rtti.hpp>
 #include <core/common/TypedRttiBuilder.hpp>
 
@@ -352,6 +353,63 @@ std::vector<ResolutionResult> Node::resolve(const std::string &name,
 	return resolve(std::vector<std::string>{name}, type);
 }
 
+bool Node::doValidate(Logger &logger) const { return true; }
+
+void Node::invalidate()
+{
+	// Only perform the invalidation if necessary
+	if (validationState != ValidationState::UNKNOWN) {
+		validationState = ValidationState::UNKNOWN;
+		if (parent != nullptr) {
+			parent->invalidate();
+		}
+	}
+}
+
+void Node::markInvalid()
+{
+	// Do not override the validationState if we're currently in the validation
+	// procedure, try to mark the parent node as invalid
+	if (validationState != ValidationState::VALIDATING &&
+	    validationState != ValidationState::INVALID) {
+		validationState = ValidationState::INVALID;
+		if (parent != nullptr) {
+			parent->markInvalid();
+		}
+	}
+}
+
+bool Node::validate(Logger &logger) const
+{
+	switch (validationState) {
+		case ValidationState::UNKNOWN:
+			validationState = ValidationState::VALIDATING;
+			try {
+				if (doValidate(logger)) {
+					validationState = ValidationState::VALID;
+					return true;
+				}
+			}
+			catch (OusiaException ex) {
+				// Make sure the validation state does not stay in the
+				// "VALIDATING" state
+				validationState = ValidationState::INVALID;
+				throw;
+			}
+			return false;
+		case ValidationState::VALID:
+			return true;
+		case ValidationState::INVALID:
+			return false;
+		case ValidationState::VALIDATING:
+			// We've run into recursion -- a circular structure cannot be
+			// properly validated, so return false
+			logger.error("The given document is cyclic.");
+			return false;
+	}
+	return false;
+}
+
 /* RTTI type registrations */
 namespace RttiTypes {
 const Rtti<ousia::Node> Node =
@@ -363,9 +421,10 @@ const Rtti<ousia::Node> Node =
                            {[](const Variant &value, ousia::Node *obj) {
 	                           obj->setName(value.asString());
 	                       }}})
-        .property("parent", {Node, {[](const ousia::Node *obj) {
-	                            return Variant::fromObject(obj->getParent());
-	                        }}});
+        .property("parent", {Node,
+                             {[](const ousia::Node *obj) {
+	                             return Variant::fromObject(obj->getParent());
+	                         }}});
 }
 }
 
diff --git a/src/core/model/Node.hpp b/src/core/model/Node.hpp
index 3763395..0523c69 100644
--- a/src/core/model/Node.hpp
+++ b/src/core/model/Node.hpp
@@ -28,6 +28,7 @@
 #ifndef _OUSIA_NODE_HPP_
 #define _OUSIA_NODE_HPP_
 
+#include <cstdint>
 #include <map>
 #include <set>
 #include <string>
@@ -47,6 +48,36 @@ template <class T>
 class Rtti;
 
 /**
+ * Describes the validity of a node structure.
+ */
+enum class ValidationState : uint8_t {
+	/**
+     * The validity is set to UNKNOWN if the Node has not yet been validated or
+     * the validation state has been reset (because the node was changed).
+     */
+	UNKNOWN,
+
+	/**
+     * The validity is set to validating if the Node has not yet been validated,
+     * but its validation is currently running. This flag is used to prevent
+     * recursion.
+     */
+	VALIDATING,
+
+	/**
+     * The validity is set to VALID if the Node has been validaten and is known
+     * to be valid.
+     */
+	VALID,
+
+	/**
+     * The validity is set to INVALID if the Node has been validated and is
+     * known to be invalid.
+     */
+	INVALID
+};
+
+/**
  * Structure describing a single result obtained from the resolution function.
  */
 struct ResolutionResult {
@@ -105,11 +136,12 @@ private:
 	 * Reference to a parent node instace.
 	 */
 	Owned<Node> parent;
+
 	/**
-	 * A "dirty" flag that signifies if this Node has been validated already
+	 * A "dirty" flag that signifies if this Node has been already validated
 	 * or not.
 	 */
-	mutable bool validated = false;
+	mutable ValidationState validationState;
 
 	/**
 	 * Private version of the "path" function used to construct the path. Calls
@@ -272,9 +304,15 @@ protected:
 
 	/**
 	 * This method should be called if the internal state of this Node is
-	 * changed such that a new validation run has to be made.
+	 * changed such that a new validation run has to be made. Also informs the
+	 * parent node about the invalidation.
 	 */
-	void invalidate() const { validated = false; }
+	void invalidate();
+
+	/**
+	 * This method should be called if a Node finds itself in an invalid state.
+	 */
+	void markInvalid();
 
 	/**
 	 * The convention for this function is as follows:
@@ -284,44 +322,12 @@ protected:
 	 *     If some child returns false this method should return false as well.
 	 * 4.) If all children could be validated this method should return true.
 	 *
-	 * Note that this implementation does not have to touch the visited set.
-	 * You can use it for validation purposes, however (e.g. to detect cycles
-	 * in the graph).
-	 *
 	 * The default and trivial behaviour of this function is to return true.
 	 *
-	 * @param logger  is a logger for error messages if false is returned.
-	 * @param visited is a set containing the ManagedUids of all children
-	 *                that already have been visited.
-	 * @return        true if this is a valid node and false if it is not.
+	 * @param logger is a logger for error messages if false is returned.
+	 * @return true if this is a valid node and false if it is not.
 	 */
-	virtual bool doValidate(Logger &logger, std::set<ManagedUid> &visited) const
-	{
-		return true;
-	}
-	
-	/**
-	 * A wrapper for doValidate that takes care of not visiting children twice,
-	 * and handling the validated flag.
-	 */
-	bool validate(Logger &logger, std::set<ManagedUid> &visited) const {
-		if (!visited.insert(getUid()).second) {
-			// if we have visited this Node already within this run, return
-			// the validated flag.
-			return validated;
-		}
-		if(validated){
-			// if this is validated, return true.
-			return true;
-		}
-		if(!doValidate(logger, visited)){
-			// if this can not be validated, return false.
-			return false;
-		}
-		// if it could be validated, set the validated flag.
-		validated = true;
-		return true;
-	}
+	virtual bool doValidate(Logger &logger) const;
 
 public:
 	/**
@@ -330,7 +336,9 @@ public:
 	 * @param mgr is a reference to the Manager instace the node belongs to.
 	 */
 	Node(Manager &mgr, Handle<Node> parent = nullptr)
-	    : Managed(mgr), parent(acquire(parent))
+	    : Managed(mgr),
+	      parent(acquire(parent)),
+	      validationState(ValidationState::UNKNOWN)
 	{
 	}
 
@@ -342,7 +350,10 @@ public:
 	 * @param parent is a handle pointing at the parent node.
 	 */
 	Node(Manager &mgr, std::string name, Handle<Node> parent = nullptr)
-	    : Managed(mgr), name(name), parent(acquire(parent))
+	    : Managed(mgr),
+	      name(name),
+	      parent(acquire(parent)),
+	      validationState(ValidationState::UNKNOWN)
 	{
 	}
 
@@ -423,29 +434,15 @@ public:
 	                                      const RttiType &type);
 
 	/**
-	 * Returns true if this node has been validated. Note that a 'false' return
-	 * value does _not_ imply that this Node is invalid. It merely says that
-	 * validity of this node is uncertain. The opposite is true, however: If
-	 * this node is invalid, the validated flag will be false.
-	 */
-	bool isValidated() const { return validated; }
-
-	/**
 	 * Checks whether this node is valid and returns true if it is and false
 	 * if it is not. If the node is invalid further information will be appended
-	 * to the logger. If this is valid this will also set the "validated" flag
-	 * to "true".
+	 * to the logger.
 	 *
-	 * @param logger    is a logger where errors will be logged if this
-	 *                  Node is invalid.
-	 *
-	 * @return          true if this Node is valid.
+	 * @param logger is a logger where errors will be logged if this Node is
+	 * invalid.
+	 * @return true if this Node is valid.
 	 */
-	bool validate(Logger &logger)
-	{
-		std::set<ManagedUid> visited;
-		return validate(logger, visited);
-	}
+	bool validate(Logger &logger) const;
 };
 
 /**