Improved typesystem, allowing online creation of structs

3 files changed, 181 insertions, 63 deletions

diff --git a/src/core/model/Typesystem.cpp b/src/core/model/Typesystem.cpp
index c2f8a9c..726de3e 100644
--- a/src/core/model/Typesystem.cpp
+++ b/src/core/model/Typesystem.cpp
@@ -147,6 +147,18 @@ EnumType::Ordinal EnumType::valueOf(const std::string &name) const
 	throw LoggableException(std::string("Unknown enum constant: ") + name);
 }
 
+/* Class Attribute */
+
+bool Attribute::doValidate(Logger &logger) const
+{
+	if (!Utils::isIdentifier(getName())) {
+		logger.error("Attribute name \"" + getName() +
+		             "\" is not a valid identifier.");
+		return false;
+	}
+	return true;
+}
+
 /* Class StructType */
 
 bool StructType::resolveIndexKey(const std::string &key, size_t &idx) const
@@ -289,66 +301,115 @@ bool StructType::buildFromArrayOrMap(Variant &data, Logger &logger,
 	    data.getTypeName());
 }
 
+void StructType::initialize(Logger &logger)
+{
+	// Copy the location in the attributes list at which the attributes started
+	size_t oldAttributeStart = attributeStart;
+	NodeVector<Attribute> oldAttributes{std::move(attributes)};
+
+	// Clear the attributes and attributeNames containers
+	attributes.clear();
+	attributeNames.clear();
+
+	// Assemble a new attributes list, add the attributes of the parent
+	// structure first
+	if (parentStructure != nullptr) {
+		attributes.assign(parentStructure->attributes);
+		attributeNames = parentStructure->attributeNames;
+	}
+	attributeStart = attributes.size();
+
+	// Add the own attributes from the old attribute list
+	for (size_t i = oldAttributeStart; i < oldAttributes.size(); i++) {
+		addAttribute(oldAttributes[i], logger, true);
+	}
+}
+
 bool StructType::doBuild(Variant &data, Logger &logger) const
 {
 	return buildFromArrayOrMap(data, logger, false);
 }
 
+bool StructType::doValidate(Logger &logger) const
+{
+	// Check whether all attributes are valid and unique
+	std::unordered_set<std::string> names;
+	bool res = true;
+	for (Handle<Attribute> a : attributes) {
+		res = a->validate(logger) && res;
+		const std::string &name = a->getName();
+		if (!names.emplace(name).second) {
+			logger.error(
+			    std::string("Attribute with name \"") + name +
+			    std::string("\" defined multiple times in structure \"") +
+			    Utils::join(path(), ".") + std::string("\""));
+			res = false;
+		}
+	}
+
+	return res;
+}
+
 Rooted<StructType> StructType::createValidated(
     Manager &mgr, std::string name, Handle<Typesystem> system,
     Handle<StructType> parentStructure, NodeVector<Attribute> attributes,
     Logger &logger)
 {
-	// Check the attributes for validity and uniqueness
-	std::map<std::string, size_t> attributeNames;
-	NodeVector<Attribute> collectedAttributes;
-
-	// Copy the attributes from the parent structure
-	if (parentStructure != nullptr) {
-		attributeNames = parentStructure->attributeNames;
-		collectedAttributes = parentStructure->attributes;
-	}
+	Rooted<StructType> structType{new StructType(mgr, name, system)};
+	structType->setParentStructure(parentStructure, logger);
+	structType->addAttributes(attributes, logger);
+	return structType;
+}
 
-	// Check the attributes for validity and uniqueness
-	for (size_t idx = 0; idx < attributes.size(); idx++) {
-		// Check for valid attribute names
-		const std::string &attrName = attributes[idx]->getName();
-		if (!Utils::isIdentifier(attrName)) {
-			logger.error(std::string("Invalid attribute name \"") + attrName +
-			             std::string("\""));
-		}
+Rooted<StructType> StructType::getParentStructure() const
+{
+	return parentStructure;
+}
 
-		// Check for uniqueness
-		auto res = attributeNames.emplace(attrName, idx);
-		if (!res.second) {
-			logger.error(std::string("Attribute with name \"") + attrName +
-			             std::string("\" defined multiple times"));
-			if (parentStructure != nullptr &&
-			    parentStructure->indexOf(attrName) >= 0) {
-				logger.note(std::string("Attribute \"") + attrName +
-				            std::string("\" was defined in parent class \"") +
-				            parentStructure->getName() + std::string("\""));
-			}
-		}
+void StructType::setParentStructure(Handle<StructType> parentStructure,
+                                    Logger &logger)
+{
+	invalidate();
+	this->parentStructure = acquire(parentStructure);
+	initialize(logger);
+}
 
-		// Store the attribute in the complete attribute list
-		collectedAttributes.push_back(attributes[idx]);
+void StructType::addAttribute(Handle<Attribute> attribute, Logger &logger,
+                              bool fromInitialize)
+{
+	// Make sure an attribute with the given name does not already exist
+	const std::string &attrName = attribute->getName();
+	attributes.push_back(attribute);
+	if (!hasAttribute(attrName)) {
+		attributeNames[attrName] = attributes.size() - 1;
+		return;
 	}
 
-	// Call the private constructor
-	return new StructType(mgr, name, system, parentStructure,
-	                      collectedAttributes, attributeNames);
+	// Check whether the attribute was defined in the parent structure, adapt
+	// error message accordingly
+	if (parentStructure != nullptr && parentStructure->hasAttribute(attrName)) {
+		logger.error("Field with name \"" + attrName +
+		             "\" hides field defined by parent structure \"" +
+		             parentStructure->getName() + "\".");
+	} else {
+		logger.error("Field with name \"" + attrName + "\" already exists.");
+	}
+	markInvalid();
 }
 
-Rooted<StructType> StructType::getParentStructure() const
+void StructType::addAttribute(Handle<Attribute> attribute, Logger &logger)
 {
-	return parentStructure;
+	invalidate();
+	addAttribute(attribute, logger, false);
 }
 
-void StructType::setParentStructure(Handle<StructType> parentStructure)
+void StructType::addAttributes(const NodeVector<Attribute> &attributes,
+                               Logger &logger)
 {
 	invalidate();
-	this->parentStructure = acquire(parentStructure);
+	for (Handle<Attribute> a : attributes) {
+		addAttribute(a, logger, false);
+	}
 }
 
 Variant StructType::create() const
@@ -390,6 +451,11 @@ ssize_t StructType::indexOf(const std::string &name) const
 	return -1;
 }
 
+bool StructType::hasAttribute(const std::string &name) const
+{
+	return indexOf(name) >= 0;
+}
+
 /* Class ArrayType */
 
 bool ArrayType::doBuild(Variant &data, Logger &logger) const
diff --git a/src/core/model/Typesystem.hpp b/src/core/model/Typesystem.hpp
index 42c2591..7bc8950 100644
--- a/src/core/model/Typesystem.hpp
+++ b/src/core/model/Typesystem.hpp
@@ -373,6 +373,15 @@ private:
 	 */
 	const Owned<Type> type;
 
+protected:
+	/**
+	 * Returns true if the name of the Attribute is a valid identifier.
+	 *
+	 * @param logger is the logger instance to which validation errors are
+	 * logged.
+	 */
+	bool doValidate(Logger &logger) const override;
+
 public:
 	/**
 	 * Default value of the attribute.
@@ -441,7 +450,14 @@ private:
 	Owned<StructType> parentStructure;
 
 	/**
-	 * Vector containing references to all attribute descriptors.
+	 * Contains the index at which the attributes declared by this StructType
+	 * start.
+	 */
+	size_t attributeStart;
+
+	/**
+	 * Vector containing references to all attribute descriptors, including the
+	 * attributes of the parent structure.
 	 */
 	NodeVector<Attribute> attributes;
 
@@ -528,6 +544,22 @@ private:
 	 */
 	bool buildFromArrayOrMap(Variant &data, Logger &logger, bool trim) const;
 
+	/**
+	 * Rebuilds the internal index and attribute list depending on the parent
+	 * structure.
+	 */
+	void initialize(Logger &logger);
+
+	/**
+	 * Function used internally to add and index attributes while logging
+	 * exceptions.
+	 *
+	 * @param attribute is the attribute that should be added.
+	 * @param logger is the logger instance to which
+	 */
+	void addAttribute(Handle<Attribute> attribute, Logger &logger,
+	                  bool fromInitialize);
+
 protected:
 	/**
 	 * Converts the given variant to the representation of the structure type.
@@ -549,9 +581,8 @@ protected:
 	 *
 	 * @param logger is a reference to the logger to which error messages should
 	 * be logged.
-	 * @param visited is used internally for recursion avoidance.
 	 */
-	bool doValidate(Logger &logger, std::set<ManagedUid> &visited) const override;
+	bool doValidate(Logger &logger) const override;
 
 public:
 	/**
@@ -564,10 +595,17 @@ public:
 	 * @param system is a reference to the parent Typesystem instance.
 	 */
 	StructType(Manager &mgr, std::string name, Handle<Typesystem> system)
-	    : Type(mgr, std::move(name), system, false), attributes(this)
+	    : Type(mgr, std::move(name), system, false),
+	      attributeStart(0),
+	      attributes(this)
 	{
 	}
 
+	static Rooted<StructType> createValidated(
+	    Manager &mgr, std::string name, Handle<Typesystem> system,
+	    Handle<StructType> parentStructure, NodeVector<Attribute> attributes,
+	    Logger &logger);
+
 	/**
 	 * Returns a handle pointing at the parent type.
 	 *
@@ -581,7 +619,7 @@ public:
 	 *
 	 * @param parentStructure is the new parent structure.
 	 */
-	void setParentStructure(Handle<StructType> parentStructure);
+	void setParentStructure(Handle<StructType> parentStructure, Logger &logger);
 
 	/**
 	 * Adds an attribute. Throws an exception if the name of the attribute is
@@ -590,14 +628,14 @@ public:
 	 * @param attribute is the attribute descriptor that should be added to the
 	 * internal attribute list.
 	 */
-	void addAttribute(Handle<Attribute> attribute);
+	void addAttribute(Handle<Attribute> attribute, Logger &logger);
 
 	/**
 	 * Adds a complete list of attributes to the typesystem.
 	 *
 	 * @param attributes is the list with typesystems that should be added.
 	 */
-	void addAttributes(const NodeVector<Attribute> &attributes);
+	void addAttributes(const NodeVector<Attribute> &attributes, Logger &logger);
 
 	/**
 	 * Creates a Variant containing a valid representation of a data instance of
@@ -630,11 +668,12 @@ public:
 	bool cast(Variant &data, Logger &logger) const;
 
 	/**
-	 * Returns a reference at the list containing all attributes.
+	 * Returns a reference at the list containing all attributes, including the
+	 * attributes of the parent structure.
 	 *
 	 * @return a const reference pointing at the attribute list.
 	 */
-	const NodeVector<Attribute> &getAttributes() const { return attributes; }
+	const NodeVector<Attribute> &getAttributes() const { return attributes; };
 
 	/**
 	 * Returns the index of the given attribute in a data array representing
@@ -645,6 +684,15 @@ public:
 	 * @return the index or -1 if the attribute does not exist.
 	 */
 	ssize_t indexOf(const std::string &name) const;
+
+	/**
+	 * Returns true if an attribute with the given name exists.
+	 *
+	 * @param name is the name of the attribute for which the index is
+	 * requested.
+	 * @return true if the requested attribute name exists, false otherwise.
+	 */
+	bool hasAttribute(const std::string &name) const;
 };
 
 /**
@@ -833,14 +881,18 @@ public:
 
 	/**
 	 * Adds the given types to the type list.
+	 *
+	 * @param ts is the list of types that should be added to the typesystem.
 	 */
-	void addTypes(const std::vector<Handle<Type>> &ts)
+	void addTypes(const NodeVector<Type> &ts)
 	{
 		types.insert(types.end(), ts.begin(), ts.end());
 	}
 
 	/**
 	 * Adds the given constant to the constant list.
+	 *
+	 * @param constant is the constant that should be added to the typesystem.
 	 */
 	void addConstant(Handle<Constant> constant)
 	{
@@ -849,8 +901,11 @@ public:
 
 	/**
 	 * Adds the given constants to the constant list.
+	 *
+	 * @param cs is the list of constants that should be added to the
+	 * typesystem.
 	 */
-	void addConstants(const std::vector<Handle<Constant>> &cs)
+	void addConstants(const NodeVector<Constant> &cs)
 	{
 		constants.insert(constants.end(), cs.begin(), cs.end());
 	}
@@ -868,8 +923,6 @@ public:
 	 * @return NodeVector containing all registered constants.
 	 */
 	const NodeVector<Constant> &getConstants() const { return constants; }
-
-	static Rooted<Typesystem> createSystemTypesystem(Manager &mgr);
 };
 
 /**
diff --git a/test/core/model/TypesystemTest.cpp b/test/core/model/TypesystemTest.cpp
index c280716..5871092 100644
--- a/test/core/model/TypesystemTest.cpp
+++ b/test/core/model/TypesystemTest.cpp
@@ -578,23 +578,21 @@ TEST(StructType, createValidated)
 			    new Attribute{mgr, "c", intType, 3},
 			    new Attribute{mgr, "a", intType}},
 			logger)};
+		ASSERT_TRUE(structType->validate(logger));
 		ASSERT_EQ(Severity::DEBUG, logger.getMaxEncounteredSeverity());
 	}
 
 	{
 		logger.reset();
-		try {
-			Rooted<StructType> structType{StructType::createValidated(
-				mgr, "struct", nullptr, nullptr,
-				NodeVector<Attribute>{
-					new Attribute{mgr, "d", stringType, "attr1default"},
-					new Attribute{mgr, "b", stringType},
-					new Attribute{mgr, "a", intType, 3},
-					new Attribute{mgr, "a", intType}},
-				logger)};
-		} catch (LoggableException ex) {
-			logger.log(ex);
-		}
+		Rooted<StructType> structType{StructType::createValidated(
+			mgr, "struct", nullptr, nullptr,
+			NodeVector<Attribute>{
+				new Attribute{mgr, "d", stringType, "attr1default"},
+				new Attribute{mgr, "b", stringType},
+				new Attribute{mgr, "a", intType, 3},
+				new Attribute{mgr, "a", intType}},
+			logger)};
+		ASSERT_FALSE(structType->validate(logger));
 		ASSERT_EQ(Severity::ERROR, logger.getMaxEncounteredSeverity());
 	}
 
@@ -608,6 +606,7 @@ TEST(StructType, createValidated)
 			    new Attribute{mgr, "a", intType, 3},
 			    new Attribute{mgr, "a a", intType}},
 			logger)};
+		ASSERT_FALSE(structType->validate(logger));
 		ASSERT_EQ(Severity::ERROR, logger.getMaxEncounteredSeverity());
 	}
 }