made isSubtree (fieldType) and primitivity orthogonal concepts: PRIMITIVE is no FieldType anymore.

9 files changed, 168 insertions, 132 deletions

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<Descriptor> parent,
-                                 Handle<Type> primitiveType, std::string name,
-                                 bool optional)
+FieldDescriptor::FieldDescriptor(Manager &mgr, Handle<Type> primitiveType,
+                                 Handle<Descriptor> 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<Descriptor> parent,
@@ -47,11 +45,9 @@ FieldDescriptor::FieldDescriptor(Manager &mgr, Handle<Descriptor> 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<std::string> names;
 	for (Handle<StructuredClass> 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<FieldDescriptor> fd)
 	}
 }
 
-void Descriptor::copyFieldDescriptor(Handle<FieldDescriptor> fd)
+void Descriptor::copyFieldDescriptor(Handle<FieldDescriptor> 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<FieldDescriptor> copy;
+	if (fd->isPrimitive()) {
+		copy = Rooted<FieldDescriptor>{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<FieldDescriptor> copy = {
+		copy = Rooted<FieldDescriptor>{
 		    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<FieldDescriptor> fd)
@@ -342,17 +358,24 @@ bool Descriptor::removeFieldDescriptor(Handle<FieldDescriptor> fd)
 }
 
 Rooted<FieldDescriptor> Descriptor::createPrimitiveFieldDescriptor(
-    Handle<Type> primitiveType, std::string name, bool optional)
+    Handle<Type> primitiveType, Logger &logger,
+    FieldDescriptor::FieldType fieldType, std::string name, bool optional)
 {
-	return Rooted<FieldDescriptor>{new FieldDescriptor(
-	    getManager(), this, primitiveType, std::move(name), optional)};
+	Rooted<FieldDescriptor> fd{new FieldDescriptor(getManager(), primitiveType,
+	                                               this, fieldType,
+	                                               std::move(name), optional)};
+	addFieldDescriptor(fd, logger);
+	return fd;
 }
 
 Rooted<FieldDescriptor> Descriptor::createFieldDescriptor(
-    FieldDescriptor::FieldType fieldType, std::string name, bool optional)
+    Logger &logger, FieldDescriptor::FieldType fieldType, std::string name,
+    bool optional)
 {
-	return Rooted<FieldDescriptor>{new FieldDescriptor(
+	Rooted<FieldDescriptor> 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<StructuredClass> children;
 	FieldType fieldType;
 	Owned<Type> 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<Descriptor> parent,
-	                Handle<Type> primitiveType,
-	                std::string name = DEFAULT_FIELD_NAME,
-	                bool optional = false);
+	FieldDescriptor(Manager &mgr, Handle<Type> primitiveType,
+	                Handle<Descriptor> 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<Descriptor> 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<FieldDescriptor> createPrimitiveFieldDescriptor(
-	    Handle<Type> primitiveType, std::string name = DEFAULT_FIELD_NAME,
-	    bool optional = false);
+	    Handle<Type> 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<FieldDescriptor> 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<Descriptor> parent = scope().selectOrThrow<Descriptor>();
 
 		Rooted<FieldDescriptor> 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<Descriptor> parent = scope().selectOrThrow<Descriptor>();
 
+		FieldDescriptor::FieldType fieldType;
+		if (args["isSubtree"].asBool()) {
+			fieldType = FieldDescriptor::FieldType::SUBTREE;
+		} else {
+			fieldType = FieldDescriptor::FieldType::TREE;
+		}
+
 		Rooted<FieldDescriptor> 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<FieldDescriptor> field =
 				        parent.cast<Descriptor>()->createFieldDescriptor(
-				            type, name, optional);
+				            logger, type, name, optional);
 				    field->addChild(strct.cast<StructuredClass>());
 			    }
 			});
@@ -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<DocumentPrimitive>()));
-			Variant content = text->getField()[0]
-			                      .cast<DocumentPrimitive>()
-			                      ->getContent();
+			ASSERT_TRUE(text->getField()[0]->isa(typeOf<DocumentPrimitive>()));
+			Variant content =
+			    text->getField()[0].cast<DocumentPrimitive>()->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<DocumentPrimitive>()));
-				Variant content = text->getField()[0]
-				                      .cast<DocumentPrimitive>()
-				                      ->getContent();
+				ASSERT_TRUE(
+				    text->getField()[0]->isa(typeOf<DocumentPrimitive>()));
+				Variant content =
+				    text->getField()[0].cast<DocumentPrimitive>()->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<FieldDescriptor> rootField{new FieldDescriptor(mgr, rootClass)};
+	Rooted<FieldDescriptor> rootField =
+	    rootClass->createFieldDescriptor(logger);
 	// and add a child class for it.
 	Rooted<StructuredClass> 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<FieldDescriptor> childField{new FieldDescriptor(mgr, childClass)};
+	Rooted<FieldDescriptor> 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<FieldDescriptor> childSubField{
-	    new FieldDescriptor(mgr, childSubClass)};
+	Rooted<FieldDescriptor> 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<FieldDescriptor> primitive_field{new FieldDescriptor(
-	    mgr, childSubClass, sys->getIntType(), "int", false)};
+	Rooted<FieldDescriptor> 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<SystemTypesystem> sys{new SystemTypesystem(mgr)};
 	// Construct the domain
 	Rooted<Domain> domain{new Domain(mgr, sys, "nasty")};
@@ -162,8 +163,8 @@ TEST(Descriptor, pathToAdvanced)
 	Rooted<StructuredClass> B{new StructuredClass(
 	    mgr, "B", domain, Cardinality::any(), {nullptr}, true, false)};
 
-	Rooted<StructuredClass> C{
-	    new StructuredClass(mgr, "C", domain, Cardinality::any(), B, true, false)};
+	Rooted<StructuredClass> C{new StructuredClass(
+	    mgr, "C", domain, Cardinality::any(), B, true, false)};
 
 	Rooted<StructuredClass> 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<FieldDescriptor> A_field{new FieldDescriptor(mgr, A)};
+	Rooted<FieldDescriptor> A_field = A->createFieldDescriptor(logger);
 
 	A_field->addChild(target);
-	Rooted<FieldDescriptor> A_field2{new FieldDescriptor(
-	    mgr, A, FieldDescriptor::FieldType::SUBTREE, "second")};
+	Rooted<FieldDescriptor> A_field2 = A->createFieldDescriptor(
+	    logger, FieldDescriptor::FieldType::SUBTREE, "second", false);
 
 	A_field2->addChild(B);
 	// We create no field for B
 	// One for C
-	Rooted<FieldDescriptor> C_field{new FieldDescriptor(mgr, C)};
+	Rooted<FieldDescriptor> C_field = C->createFieldDescriptor(logger);
 
 	C_field->addChild(target);
 	// one for start
-	Rooted<FieldDescriptor> start_field{new FieldDescriptor(mgr, start)};
+	Rooted<FieldDescriptor> start_field = start->createFieldDescriptor(logger);
 
 	start_field->addChild(D);
 	// One for D
-	Rooted<FieldDescriptor> D_field{new FieldDescriptor(mgr, D)};
+	Rooted<FieldDescriptor> D_field = D->createFieldDescriptor(logger);
 
 	D_field->addChild(E);
 	// One for E
-	Rooted<FieldDescriptor> E_field{new FieldDescriptor(mgr, E)};
+	Rooted<FieldDescriptor> 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<FieldDescriptor> base_field{
-		    new FieldDescriptor(mgr, base, nullptr)};
+		Rooted<FieldDescriptor> 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<StructuredClass> 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<FieldDescriptor> 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<FieldDescriptor> 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<FieldDescriptor> 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<Domain> constructHeadingDomain(Manager &mgr,
 	                                    "paragraph"};
 	for (auto &s : secclasses) {
 		Rooted<StructuredClass> desc = resolveDescriptor(bookDomain, s);
-		Rooted<FieldDescriptor> heading_field{new FieldDescriptor(
-		    mgr, desc, FieldDescriptor::FieldType::SUBTREE, "heading")};
+		Rooted<FieldDescriptor> 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<Domain> constructBookDomain(Manager &mgr,
 	    mgr, "book", domain, single, {nullptr}, false, true)};
 
 	// The structure field of it.
-	Rooted<FieldDescriptor> book_field{new FieldDescriptor(mgr, book)};
+	Rooted<FieldDescriptor> book_field = book->createFieldDescriptor(logger);
 
 	// From there on the "section".
 	Rooted<StructuredClass> section{
@@ -50,7 +50,8 @@ static Rooted<Domain> constructBookDomain(Manager &mgr,
 	book_field->addChild(section);
 
 	// And the field of it.
-	Rooted<FieldDescriptor> section_field{new FieldDescriptor(mgr, section)};
+	Rooted<FieldDescriptor> section_field =
+	    section->createFieldDescriptor(logger);
 
 	// We also add the "paragraph", which is transparent.
 	Rooted<StructuredClass> paragraph{new StructuredClass(
@@ -59,8 +60,8 @@ static Rooted<Domain> constructBookDomain(Manager &mgr,
 	book_field->addChild(paragraph);
 
 	// And the field of it.
-	Rooted<FieldDescriptor> paragraph_field{
-	    new FieldDescriptor(mgr, paragraph)};
+	Rooted<FieldDescriptor> paragraph_field =
+	    paragraph->createFieldDescriptor(logger);
 
 	// We append "subsection" to section.
 	Rooted<StructuredClass> subsection{
@@ -68,8 +69,8 @@ static Rooted<Domain> constructBookDomain(Manager &mgr,
 	section_field->addChild(subsection);
 
 	// And the field of it.
-	Rooted<FieldDescriptor> subsection_field{
-	    new FieldDescriptor(mgr, subsection)};
+	Rooted<FieldDescriptor> subsection_field =
+	    subsection->createFieldDescriptor(logger);
 
 	// and we add the paragraph to subsections fields
 	subsection_field->addChild(paragraph);
@@ -80,9 +81,8 @@ static Rooted<Domain> constructBookDomain(Manager &mgr,
 	paragraph_field->addChild(text);
 
 	// ... and has a primitive field.
-	Rooted<FieldDescriptor> text_field{new FieldDescriptor(
-	    mgr, text, domain->getTypesystems()[0]->getTypes()[0],
-	    DEFAULT_FIELD_NAME, false)};
+	Rooted<FieldDescriptor> 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 @@
 
 	<!-- an annotation comment -->
 	<annotation name="comment">
-		<field name="content" isSubtree="true">
+		<field name="content">
 			<childRef ref="book.paragraph"/>
 		</field>
 		<field name="replies" isSubtree="true">
@@ -17,7 +17,7 @@
 		<!-- Is there a chance to prevent users from having to redefine these
 		     two fields in comment and reply? Could we use a fieldRef here?
 		     Or would that be circular? -->
-		<field name="content" isSubtree="true">
+		<field name="content">
 			<childRef ref="book.paragraph"/>
 		</field>
 		<field name="replies" isSubtree="true">
@@ -30,7 +30,7 @@
 	<!-- note that replies are organized in a tree fashion: One can also reply
 	     to a reply -->
 	<struct name="reply">
-		<field name="content" isSubtree="true">
+		<field name="content">
 			<childRef ref="book.paragraph"/>
 		</field>
 		<field name="replies" isSubtree="true">