Replace multiple DB transactions with a single one

src/Form/DataTransformer/ModelToIdPropertyTransformer.php

@@ -17,6 +17,7 @@
 use Doctrine\Common\Collections\Collection;
 use Doctrine\Common\Util\ClassUtils;
 use Sonata\AdminBundle\Model\ModelManagerInterface;
+use Sonata\AdminBundle\Util\TraversableToCollection;
 use Symfony\Component\Form\DataTransformerInterface;
 
 /**
@@ -89,12 +90,9 @@ public function __construct(
      */
     public function reverseTransform($value)
     {
-        /** @phpstan-var ArrayCollection<array-key, T> $collection */
-        $collection = new ArrayCollection();
-
         if (empty($value)) {
             if ($this->multiple) {
-                return $collection;
+                return new ArrayCollection();
             }
 
             return null;
@@ -110,13 +108,19 @@ public function reverseTransform($value)
 
         foreach ($value as $key => $id) {
             if ('_labels' === $key) {
+                unset($value[$key]);
+
                 continue;
             }
 
-            $collection->add($this->modelManager->find($this->className, $id));
+            $value[$key] = (string) $id;
         }
 
-        return $collection;
+        $query = $this->modelManager->createQuery($this->className);
+        $this->modelManager->addIdentifiersToQuery($this->className, $query, $value);
+        $result = $this->modelManager->executeQuery($query);
+
+        return TraversableToCollection::transform($result);
     }
 
     /**

src/Form/DataTransformer/ModelsToArrayTransformer.php

@@ -18,6 +18,7 @@
 use Doctrine\Common\Util\ClassUtils;
 use Sonata\AdminBundle\Form\ChoiceList\ModelChoiceLoader;
 use Sonata\AdminBundle\Model\ModelManagerInterface;
+use Sonata\AdminBundle\Util\TraversableToCollection;
 use Sonata\Doctrine\Adapter\AdapterInterface;
 use Symfony\Component\Form\ChoiceList\LazyChoiceList;
 use Symfony\Component\Form\DataTransformerInterface;
@@ -55,8 +56,6 @@ class ModelsToArrayTransformer implements DataTransformerInterface
     protected $choiceList;
 
     /**
-     * ModelsToArrayTransformer constructor.
-     *
      * @param LazyChoiceList|ModelChoiceLoader $choiceList
      * @param ModelManagerInterface            $modelManager
      * @param string                           $class
@@ -168,23 +167,22 @@ public function reverseTransform($value)
             throw new UnexpectedTypeException($value, 'array');
         }
 
+        $value = array_map('strval', $value);
+
+        $query = $this->modelManager->createQuery($this->class);
+        $this->modelManager->addIdentifiersToQuery($this->class, $query, $value);
+        $result = $this->modelManager->executeQuery($query);
+
         /** @phpstan-var ArrayCollection<array-key, T> $collection */
-        $collection = new ArrayCollection();
-        $notFound = [];
-
-        // optimize this into a SELECT WHERE IN query
-        foreach ($value as $key) {
-            if ($model = $this->modelManager->find($this->class, $key)) {
-                $collection->add($model);
-            } else {
-                $notFound[] = $key;
-            }
-        }
+        $collection = TraversableToCollection::transform($result);
+
+        $diffCount = \count($value) - $collection->count();
 
-        if (\count($notFound) > 0) {
+        if (0 !== $diffCount) {
             throw new TransformationFailedException(sprintf(
-                'The entities with keys "%s" could not be found',
-                implode('", "', $notFound)
+                '%u keys could not be found in the provided values: "%s".',
+                $diffCount,
+                implode('", "', $value)
             ));
         }
 

src/Util/TraversableToCollection.php

@@ -0,0 +1,55 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Sonata Project package.
+ *
+ * (c) Thomas Rabaix <thomas.rabaix@sonata-project.org>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Sonata\AdminBundle\Util;
+
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\Common\Collections\Collection;
+
+/**
+ * @author Javier Spagnoletti <phansys@gmail.com>
+ */
+final class TraversableToCollection
+{
+    /**
+     * @param mixed $value
+     *
+     * @throws \TypeError
+     *
+     * @return Collection<int|string, mixed>
+     *
+     * @phpstan-return Collection<array-key, mixed>
+     */
+    public static function transform($value): Collection
+    {
+        if ($value instanceof Collection) {
+            return $value;
+        }
+
+        if ($value instanceof \Traversable) {
+            return new ArrayCollection(iterator_to_array($value));
+        }
+
+        if (\is_array($value)) {
+            return new ArrayCollection($value);
+        }
+
+        throw new \TypeError(sprintf(
+            'Argument 1 passed to "%s()" must be of type "%s" or "%s", %s given.',
+            __METHOD__,
+            \Traversable::class,
+            'array',
+            \is_object($value) ? 'instance of "'.\get_class($value).'"' : '"'.\gettype($value).'"'
+        ));
+    }
+}

tests/Form/DataTransformer/ModelToIdPropertyTransformerTest.php

@@ -14,7 +14,9 @@
 namespace Sonata\AdminBundle\Tests\Form\DataTransformer;
 
 use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\Common\Collections\Collection;
 use PHPUnit\Framework\TestCase;
+use Sonata\AdminBundle\Datagrid\ProxyQueryInterface;
 use Sonata\AdminBundle\Form\DataTransformer\ModelToIdPropertyTransformer;
 use Sonata\AdminBundle\Model\ModelManagerInterface;
 use Sonata\AdminBundle\Tests\Fixtures\Entity\Foo;
@@ -58,40 +60,43 @@ public function testReverseTransform(): void
      */
     public function testReverseTransformMultiple(array $expected, $params, Foo $entity1, Foo $entity2, Foo $entity3): void
     {
-        $transformer = new ModelToIdPropertyTransformer($this->modelManager, Foo::class, 'bar', true);
-
-        $this->modelManager
-            ->method('find')
-            ->willReturnCallback(static function (string $className, int $value) use ($entity1, $entity2, $entity3) {
-                if (Foo::class !== $className) {
-                    return;
+        $modelManager = $this->createMock(ModelManagerInterface::class);
+        $transformer = new ModelToIdPropertyTransformer($modelManager, Foo::class, 'bar', true);
+        $proxyQuery = $this->createMock(ProxyQueryInterface::class);
+        $modelManager
+            ->expects($this->exactly($params ? 1 : 0))
+            ->method('createQuery')
+            ->with($this->equalTo(Foo::class))
+            ->willReturn($proxyQuery);
+        $modelManager
+            ->expects($this->exactly($params ? 1 : 0))
+            ->method('executeQuery')
+            ->with($this->equalTo($proxyQuery))
+            ->willReturnCallback(static function (ProxyQueryInterface $query) use ($params, $entity1, $entity2, $entity3): array {
+                $collection = [];
+
+                if (\in_array(123, $params, true)) {
+                    $collection[] = $entity1;
                 }
 
-                if (123 === $value) {
-                    return $entity1;
+                if (\in_array(456, $params, true)) {
+                    $collection[] = $entity2;
                 }
 
-                if (456 === $value) {
-                    return $entity2;
+                if (\in_array(789, $params, true)) {
+                    $collection[] = $entity3;
                 }
 
-                if (789 === $value) {
-                    return $entity3;
-                }
+                return $collection;
             });
 
-        $collection = new ArrayCollection();
-        $this->modelManager
-            ->method('getModelCollectionInstance')
-            ->with($this->equalTo(Foo::class))
-            ->willReturn($collection);
-
         $result = $transformer->reverseTransform($params);
-        $this->assertInstanceOf(ArrayCollection::class, $result);
+        $this->assertInstanceOf(Collection::class, $result);
+        $this->assertCount(\count($expected), $result);
         $this->assertSame($expected, $result->getValues());
     }
 
-    public function getReverseTransformMultipleTests()
+    public function getReverseTransformMultipleTests(): iterable
     {
         $entity1 = new Foo();
         $entity1->setBaz(123);
@@ -105,12 +110,10 @@ public function getReverseTransformMultipleTests()
         $entity3->setBaz(789);
         $entity3->setBar('example3');
 
-        return [
-            [[], null, $entity1, $entity2, $entity3],
-            [[], false, $entity1, $entity2, $entity3],
-            [[$entity1], [123, '_labels' => ['example']], $entity1, $entity2, $entity3],
-            [[$entity1, $entity2, $entity3], [123, 456, 789, '_labels' => ['example', 'example2', 'example3']], $entity1, $entity2, $entity3],
-        ];
+        yield [[], null, $entity1, $entity2, $entity3];
+        yield [[], false, $entity1, $entity2, $entity3];
+        yield [[$entity1], [123, '_labels' => ['example']], $entity1, $entity2, $entity3];
+        yield [[$entity1, $entity2, $entity3], [123, 456, 789, '_labels' => ['example', 'example2', 'example3']], $entity1, $entity2, $entity3];
     }
 
     /**

tests/Form/DataTransformer/ModelsToArrayTransformerTest.php

@@ -13,11 +13,15 @@
 
 namespace Sonata\AdminBundle\Tests\Form\DataTransformer;
 
+use Doctrine\Common\Collections\Collection;
 use PHPUnit\Framework\TestCase;
+use Sonata\AdminBundle\Datagrid\ProxyQueryInterface;
 use Sonata\AdminBundle\Form\ChoiceList\ModelChoiceLoader;
 use Sonata\AdminBundle\Form\DataTransformer\ModelsToArrayTransformer;
 use Sonata\AdminBundle\Model\ModelManagerInterface;
 use Sonata\AdminBundle\Tests\Fixtures\Entity\Foo;
+use Symfony\Component\Form\Exception\TransformationFailedException;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
 
 class ModelsToArrayTransformerTest extends TestCase
 {
@@ -44,4 +48,87 @@ public function testLegacyConstructor(): void
 
         $this->assertInstanceOf(ModelsToArrayTransformer::class, $transformer);
     }
+
+    /**
+     * @dataProvider reverseTransformProvider
+     */
+    public function testReverseTransform(?array $value): void
+    {
+        $modelManager = $this->createStub(ModelManagerInterface::class);
+
+        if (null !== $value) {
+            $proxyQuery = $this->createStub(ProxyQueryInterface::class);
+            $modelManager
+                ->method('createQuery')
+                ->with($this->equalTo(Foo::class))
+                ->willReturn($proxyQuery);
+            $modelManager
+                ->method('executeQuery')
+                ->with($this->equalTo($proxyQuery))
+                ->willReturn($value);
+        }
+
+        $transformer = new ModelsToArrayTransformer(
+            $modelManager,
+            Foo::class
+        );
+
+        $result = $transformer->reverseTransform($value);
+
+        if (null === $value) {
+            $this->assertNull($result);
+        } else {
+            $this->assertInstanceOf(Collection::class, $result);
+            $this->assertCount(\count($value), $result);
+        }
+    }
+
+    public function reverseTransformProvider(): iterable
+    {
+        yield [['a']];
+        yield [['a', 'b', 3]];
+        yield [null];
+    }
+
+    public function testReverseTransformUnexpectedType(): void
+    {
+        $value = 'unexpected';
+        $modelManager = $this->createStub(ModelManagerInterface::class);
+
+        $transformer = new ModelsToArrayTransformer(
+            $modelManager,
+            Foo::class
+        );
+
+        $this->expectException(UnexpectedTypeException::class);
+        $this->expectExceptionMessage('Expected argument of type "array", "string" given');
+
+        $transformer->reverseTransform($value);
+    }
+
+    public function testReverseTransformFailed(): void
+    {
+        $value = ['a', 'b'];
+        $reverseTransformCollection = ['a'];
+        $modelManager = $this->createStub(ModelManagerInterface::class);
+        $proxyQuery = $this->createStub(ProxyQueryInterface::class);
+        $modelManager
+            ->method('createQuery')
+            ->with($this->equalTo(Foo::class))
+            ->willReturn($proxyQuery);
+        $modelManager
+            ->method('executeQuery')
+            ->with($this->equalTo($proxyQuery))
+            ->willReturn($reverseTransformCollection);
+
+        $transformer = new ModelsToArrayTransformer(
+            $modelManager,
+            Foo::class
+        );
+
+        $this->expectException(TransformationFailedException::class);
+        $this->expectExceptionMessage('1 keys could not be found in the provided values: "a", "b".');
+
+        $transformer->reverseTransform($value);
+    }
 }