Correctly denormalize parent-child relationships in import, when only children not parent fields are given

This fixes issue #1272

src/Serializer/StructuralElementDenormalizer.php

@@ -42,6 +42,8 @@ class StructuralElementDenormalizer implements DenormalizerInterface, Denormaliz
 
     private const ALREADY_CALLED = 'STRUCTURAL_DENORMALIZER_ALREADY_CALLED';
 
+    private const PARENT_ELEMENT = 'STRUCTURAL_DENORMALIZER_PARENT_ELEMENT';
+
     private array $object_cache = [];
 
     public function __construct(
@@ -89,32 +91,55 @@ class StructuralElementDenormalizer implements DenormalizerInterface, Denormaliz
 
         $context[self::ALREADY_CALLED][] = $data;
 
+        //In the first step, denormalize without children
+        $context_without_children = $context;
+        $context_without_children['groups'] = array_filter(
+            $context_without_children['groups'] ?? [],
+            static fn($group) => $group !== 'include_children',
+        );
+        //Also unset any parent element, to avoid infinite loops. We will set the parent element in the next step, when we denormalize the children
+        unset($context_without_children[self::PARENT_ELEMENT]);
+        /** @var AbstractStructuralDBElement $entity */
+        $entity = $this->denormalizer->denormalize($data, $type, $format, $context_without_children);
 
-        /** @var AbstractStructuralDBElement $deserialized_entity */
+        //Assign the parent element to the denormalized entity, so it can be used in the denormalization of the children (e.g. for path generation)
-        $deserialized_entity = $this->denormalizer->denormalize($data, $type, $format, $context);
+        if (isset($context[self::PARENT_ELEMENT]) && $context[self::PARENT_ELEMENT] instanceof $entity && $entity->getID() === null) {
+            $entity->setParent($context[self::PARENT_ELEMENT]);
+        }
 
         //Check if we already have the entity in the database (via path)
         /** @var StructuralDBElementRepository<T> $repo */
         $repo = $this->entityManager->getRepository($type);
+        $deserialized_entity = $entity;
 
         $path = $deserialized_entity->getFullPath(AbstractStructuralDBElement::PATH_DELIMITER_ARROW);
         $db_elements = $repo->getEntityByPath($path, AbstractStructuralDBElement::PATH_DELIMITER_ARROW);
         if ($db_elements !== []) {
             //We already have the entity in the database, so we can return it
-            return end($db_elements);
+            $entity = end($db_elements);
         }
 
 
         //Check if we have created the entity in this request before (so we don't create multiple entities for the same path)
         //Entities get saved in the cache by type and path
         //We use a different cache for this then the objects created by a string value (saved in repo). However, that should not be a problem
-        //unless the user data has mixed structure between json data and a string path
+        //unless the user data has mixed structure between JSON data and a string path
         if (isset($this->object_cache[$type][$path])) {
-            return $this->object_cache[$type][$path];
+            $entity = $this->object_cache[$type][$path];
+        } else {
+            //Save the entity in the cache
+            $this->object_cache[$type][$path] = $deserialized_entity;
         }
 
-        //Save the entity in the cache
+        //In the next step we can denormalize the children, and add our children to the entity.
-        $this->object_cache[$type][$path] = $deserialized_entity;
+        if (in_array('include_children', $context['groups'], true) && isset($data['children']) && is_array($data['children'])) {
+            foreach ($data['children'] as $child_data) {
+                $child_entity = $this->denormalize($child_data, $type, $format, array_merge($context, [self::PARENT_ELEMENT => $entity]));
+                if ($child_entity !== null && !$entity->getChildren()->contains($child_entity)) {
+                    $entity->addChild($child_entity);
+                }
+            }
+        }
 
         //We don't have the entity in the database, so we have to persist it
         $this->entityManager->persist($deserialized_entity);

src/Services/ImportExportSystem/EntityImporter.php

@@ -219,11 +219,6 @@ class EntityImporter
             $entities = [$entities];
         }
 
-        //The serializer has only set the children attributes. We also have to change the parent value (the real value in DB)
-        if ($entities[0] instanceof AbstractStructuralDBElement) {
-            $this->correctParentEntites($entities, null);
-        }
-
         //Set the parent of the imported elements to the given options
         foreach ($entities as $entity) {
             if ($entity instanceof AbstractStructuralDBElement) {
@@ -297,6 +292,14 @@ class EntityImporter
         return $resolver;
     }
 
+    private function persistRecursively(AbstractStructuralDBElement $entity): void
+    {
+        $this->em->persist($entity);
+        foreach ($entity->getChildren() as $child) {
+            $this->persistRecursively($child);
+        }
+    }
+
     /**
      * This method deserializes the given file and writes the entities to the database (and flush the db).
      * The imported elements will be checked (validated) before written to database.
@@ -322,7 +325,7 @@ class EntityImporter
 
         //Iterate over each $entity write it to DB (the invalid entities were already filtered out).
         foreach ($entities as $entity) {
-            $this->em->persist($entity);
+            $this->persistRecursively($entity);
         }
 
         //Save changes to database, when no error happened, or we should continue on error.
@@ -484,21 +487,4 @@ class EntityImporter
             throw $e;
         }
     }
-
-
-    /**
-     * This functions corrects the parent setting based on the children value of the parent.
-     *
-     * @param iterable                         $entities the list of entities that should be fixed
-     * @param  AbstractStructuralDBElement|null  $parent   the parent, to which the entity should be set
-     */
-    protected function correctParentEntites(iterable $entities, ?AbstractStructuralDBElement $parent = null): void
-    {
-        foreach ($entities as $entity) {
-            /** @var AbstractStructuralDBElement $entity */
-            $entity->setParent($parent);
-            //Do the same for the children of entity
-            $this->correctParentEntites($entity->getChildren(), $entity);
-        }
-    }
 }

tests/Serializer/StructuralElementDenormalizerTest.php

@@ -85,4 +85,41 @@ final class StructuralElementDenormalizerTest extends WebTestCase
         $result2 = $this->service->denormalize($data, Category::class, 'json', ['groups' => ['import']]);
         $this->assertSame($result, $result2);
     }
+
+    public function testDenormalizeViaChildren(): void
+    {
+        $data = ['name' => 'Node',
+            'children' => [
+                ['name' => 'A', 'children' => [['name' => '1'], ['name' => '2']]],
+                ['name' => 'B', 'children' => [['name' => '1'], ['name' => '2']]],
+                ['name' => 'C', 'children' => [['name' => '1'], ['name' => '2'], ['name' => '3']]],
+            ]
+        ];
+
+        $result = $this->service->denormalize($data, Category::class, 'json', ['groups' => ['import', 'include_children']]);
+        $this->assertInstanceOf(Category::class, $result);
+
+        $this->assertCount(3, $result->getChildren());
+        $this->assertSame('A', $result->getChildren()[0]->getName());
+        $this->assertSame('B', $result->getChildren()[1]->getName());
+        $this->assertSame('C', $result->getChildren()[2]->getName());
+        //Parents should be set correctly
+        $this->assertSame($result, $result->getChildren()[0]->getParent());
+        $this->assertSame($result, $result->getChildren()[1]->getParent());
+        $this->assertSame($result, $result->getChildren()[2]->getParent());
+
+        $this->assertCount(2, $result->getChildren()[0]->getChildren());
+        $this->assertSame('1', $result->getChildren()[0]->getChildren()[0]->getName());
+        $this->assertSame('2', $result->getChildren()[0]->getChildren()[1]->getName());
+        //Parents should be set correctly
+        $this->assertSame($result->getChildren()[0], $result->getChildren()[0]->getChildren()[0]->getParent());
+        $this->assertSame($result->getChildren()[0], $result->getChildren()[0]->getChildren()[1]->getParent());
+
+        $this->assertCount(2, $result->getChildren()[1]->getChildren());
+        $this->assertSame('1', $result->getChildren()[1]->getChildren()[0]->getName());
+        $this->assertSame('2', $result->getChildren()[1]->getChildren()[1]->getName());
+        //Must be different instances than the children of A, because we create new elements for the same path, if we don't have them in the DB
+        $this->assertNotSame($result->getChildren()[0]->getChildren()[0], $result->getChildren()[1]->getChildren()[0]);
+
+    }
 }