Fixed error handling of structural data import

This was the reason for the exception in #632

src/Controller/AdminPages/BaseAdminController.php

@@ -60,6 +60,8 @@ use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
 use Symfony\Component\Serializer\Exception\UnexpectedValueException;
+use Symfony\Component\Validator\ConstraintViolationInterface;
+use Symfony\Component\Validator\ConstraintViolationListInterface;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
 use function Symfony\Component\Translation\t;
@@ -322,8 +324,8 @@ abstract class BaseAdminController extends AbstractController
             try {
                 $errors = $importer->importFileAndPersistToDB($file, $options);
 
-                foreach ($errors as $name => $error) {
+                foreach ($errors as $name => ['violations' => $violations]) {
-                    foreach ($error as $violation) {
+                    foreach ($violations as $violation) {
                         $this->addFlash('error', $name.': '.$violation->getMessage());
                     }
                 }
@@ -333,6 +335,7 @@ abstract class BaseAdminController extends AbstractController
             }
         }
 
+        ret:
         //Mass creation form
         $mass_creation_form = $this->createForm(MassCreationForm::class, ['entity_class' => $this->entity_class]);
         $mass_creation_form->handleRequest($request);
@@ -345,11 +348,14 @@ abstract class BaseAdminController extends AbstractController
             $results = $importer->massCreation($data['lines'], $this->entity_class, $data['parent'] ?? null, $errors);
 
             //Show errors to user:
-            foreach ($errors as $error) {
+            foreach ($errors as ['entity' => $new_entity, 'violations' => $violations]) {
-                if ($error['entity'] instanceof AbstractStructuralDBElement) {
+                /** @var ConstraintViolationInterface $violation */
-                    $this->addFlash('error', $error['entity']->getFullPath().':'.$error['violations']);
+                foreach ($violations as $violation) {
+                    if ($new_entity instanceof AbstractStructuralDBElement) {
+                        $this->addFlash('error', $new_entity->getFullPath().':'.$violation->getMessage());
                     } else { //When we don't have a structural element, we can only show the name
-                    $this->addFlash('error', $error['entity']->getName().':'.$error['violations']);
+                        $this->addFlash('error', $new_entity->getName().':'.$violation->getMessage());
+                    }
                 }
             }
 
@@ -364,7 +370,6 @@ abstract class BaseAdminController extends AbstractController
             }
         }
 
-        ret:
         return $this->render($this->twig_template, [
             'entity' => $new_entity,
             'form' => $form,

src/Serializer/StructuralElementDenormalizer.php

@@ -40,6 +40,8 @@ class StructuralElementDenormalizer implements DenormalizerInterface, Denormaliz
 
     use DenormalizerAwareTrait;
 
+    private const ALREADY_CALLED = 'STRUCTURAL_DENORMALIZER_ALREADY_CALLED';
+
     private array $object_cache = [];
 
     public function __construct(
@@ -54,6 +56,13 @@ class StructuralElementDenormalizer implements DenormalizerInterface, Denormaliz
             return false;
         }
 
+        //If we already handled this object, skip it
+        if (isset($context[self::ALREADY_CALLED])
+            && is_array($context[self::ALREADY_CALLED])
+            && in_array($data, $context[self::ALREADY_CALLED], true)) {
+            return false;
+        }
+
         return is_array($data)
             && is_subclass_of($type, AbstractStructuralDBElement::class)
             //Only denormalize if we are doing a file import operation
@@ -65,6 +74,13 @@ class StructuralElementDenormalizer implements DenormalizerInterface, Denormaliz
         //Do not use API Platform's denormalizer
         $context[SkippableItemNormalizer::DISABLE_ITEM_NORMALIZER] = true;
 
+        if (!isset($context[self::ALREADY_CALLED])) {
+            $context[self::ALREADY_CALLED] = [];
+        }
+
+        $context[self::ALREADY_CALLED][] = $data;
+
+
         /** @var AbstractStructuralDBElement $deserialized_entity */
         $deserialized_entity = $this->denormalizer->denormalize($data, $type, $format, $context);
 

src/Services/ImportExportSystem/EntityImporter.php

@@ -29,6 +29,7 @@ use App\Entity\Parts\Part;
 use App\Repository\StructuralDBElementRepository;
 use App\Serializer\APIPlatform\SkippableItemNormalizer;
 use Symfony\Component\Validator\ConstraintViolationList;
+use Symfony\Component\Validator\ConstraintViolationListInterface;
 use function count;
 use Doctrine\ORM\EntityManagerInterface;
 use InvalidArgumentException;
@@ -57,6 +58,7 @@ class EntityImporter
      * @phpstan-param class-string<T> $class_name
      * @param AbstractStructuralDBElement|null $parent     the element which will be used as parent element for new elements
      * @param array                            $errors     an associative array containing all validation errors
+     * @param-out  array<string, array{'entity': object, 'violations': ConstraintViolationListInterface}>  $errors
      *
      * @return AbstractNamedDBElement[] An array containing all valid imported entities (with the type $class_name)
      * @return T[]
@@ -152,6 +154,7 @@ class EntityImporter
      * @param  string  $data The serialized data which should be imported
      * @param  array  $options The options for the import process
      * @param  array  $errors An array which will be filled with the validation errors, if any occurs during import
+     * @param-out array<string, array{'entity': object, 'violations': ConstraintViolationListInterface}> $errors
      * @return array An array containing all valid imported entities
      */
     public function importString(string $data, array $options = [], array &$errors = []): array
@@ -218,6 +221,10 @@ class EntityImporter
             if (count($tmp) > 0) { //Log validation errors to global log.
                 $name = $entity instanceof AbstractStructuralDBElement ? $entity->getFullPath() : $entity->getName();
 
+                if (trim($name) === '') {
+                    $name = 'Row ' . (string) $key;
+                }
+
                 $errors[$name] = [
                     'violations' => $tmp,
                     'entity' => $entity,
@@ -264,7 +271,7 @@ class EntityImporter
      * @param array  $options    options for the import process
      * @param AbstractNamedDBElement[]  $entities  The imported entities are returned in this array
      *
-     * @return array<string, ConstraintViolationList> An associative array containing an ConstraintViolationList and the entity name as key are returned,
+     * @return array<string, array{'entity': object, 'violations': ConstraintViolationListInterface}> An associative array containing an ConstraintViolationList and the entity name as key are returned,
      *               if an error happened during validation. When everything was successfully, the array should be empty.
      */
     public function importFileAndPersistToDB(File $file, array $options = [], array &$entities = []): array
@@ -296,6 +303,7 @@ class EntityImporter
      *
      * @param File   $file       the file that should be used for importing
      * @param array  $options    options for the import process
+     * @param-out  array<string, array{'entity': object, 'violations': ConstraintViolationListInterface}>  $errors
      *
      * @return array an array containing the deserialized elements
      */

tests/Services/ImportExportSystem/EntityImporterTest.php

@@ -27,11 +27,13 @@ use App\Entity\Attachments\AttachmentType;
 use App\Entity\LabelSystem\LabelProfile;
 use App\Entity\Parts\Category;
 use App\Entity\Parts\Part;
+use App\Entity\ProjectSystem\Project;
 use App\Entity\UserSystem\User;
 use App\Services\ImportExportSystem\EntityImporter;
 use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
 use Symfony\Component\Validator\ConstraintViolation;
+use Symfony\Component\Validator\ConstraintViolationListInterface;
 
 /**
  * @group DB
@@ -190,6 +192,63 @@ EOT;
         $this->assertSame($expected, $this->service->determineFormat($extension));
     }
 
+    public function testImportStringProjects(): void
+    {
+        $input = <<<EOT
+        name;comment
+        Test 1;Test 1 notes
+        Test 2;Test 2 notes
+        EOT;
+
+        $errors = [];
+
+        $results = $this->service->importString($input, [
+            'class' => Project::class,
+            'format' => 'csv',
+            'csv_delimiter' => ';',
+        ], $errors);
+
+        $this->assertCount(2, $results);
+
+        //No errors must be present
+        $this->assertEmpty($errors);
+
+
+        $this->assertContainsOnlyInstancesOf(Project::class, $results);
+
+        $this->assertSame('Test 1', $results[0]->getName());
+        $this->assertSame('Test 1 notes', $results[0]->getComment());
+    }
+
+    public function testImportStringProjectWithErrors(): void
+    {
+        $input = <<<EOT
+        name;comment
+        ;Test 1 notes
+        Test 2;Test 2 notes
+        EOT;
+
+        $errors = [];
+
+        $results = $this->service->importString($input, [
+            'class' => Project::class,
+            'format' => 'csv',
+            'csv_delimiter' => ';',
+        ], $errors);
+
+        $this->assertCount(1, $results);
+        $this->assertCount(1, $errors);
+
+        //Validate shape of error output
+
+        $this->assertArrayHasKey('Row 0', $errors);
+        $this->assertArrayHasKey('entity', $errors['Row 0']);
+        $this->assertArrayHasKey('violations', $errors['Row 0']);
+
+        $this->assertInstanceOf(ConstraintViolationListInterface::class, $errors['Row 0']['violations']);
+        $this->assertInstanceOf(Project::class, $errors['Row 0']['entity']);
+    }
+
     public function testImportStringParts(): void
     {
         $input = <<<EOT