[DoctrineBridge] Optimize fetching of entities to use WHERE IN and fi…

…x other inefficencies.
symfony · Dec 19, 2011 · b919d92 · b919d92
1 parent 517eebc
commit b919d92
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 41 deletions.
diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php
@@ -107,6 +107,9 @@ public function __construct(ObjectManager $manager, $class, $property = null, En
         // displaying entities as strings
         if ($property) {
             $this->propertyPath = new PropertyPath($property);
+        } else if (!method_exists($this->class, '__toString')) {
+            // Otherwise expect a __toString() method in the entity
+            throw new FormException('Entities passed to the choice field must have a "__toString()" method defined (or you can also override the "property" option).');
         }
 
         if (!is_array($choices) && !$choices instanceof \Closure && !is_null($choices)) {
@@ -202,11 +205,6 @@ private function loadEntities($entities, $group = null)
                 // If the property option was given, use it
                 $value = $this->propertyPath->getValue($entity);
             } else {
-                // Otherwise expect a __toString() method in the entity
-                if (!method_exists($entity, '__toString')) {
-                    throw new FormException('Entities passed to the choice field must have a "__toString()" method defined (or you can also override the "property" option).');
-                }
-
                 $value = (string) $entity;
             }
 
@@ -261,7 +259,7 @@ public function getEntities()
     }
 
     /**
-     * Returns the entity for the given key.
+     * Returns the entities for the given keys.
      *
      * If the underlying entities have composite identifiers, the choices
      * are initialized. The key is expected to be the index in the choices
@@ -270,35 +268,42 @@ public function getEntities()
      * If they have single identifiers, they are either fetched from the
      * internal entity cache (if filled) or loaded from the database.
      *
-     * @param  string $key The choice key (for entities with composite
-     *                     identifiers) or entity ID (for entities with single
-     *                     identifiers)
-     *
-     * @return object      The matching entity
+     * @param  array $keys  The choice key (for entities with composite
+     *                      identifiers) or entity ID (for entities with single
+     *                      identifiers)
+     * @return object[]     The matching entity
      */
-    public function getEntity($key)
+    public function getEntitiesByKeys($keys)
     {
         if (!$this->loaded) {
             $this->load();
         }
 
-        try {
-            if (count($this->identifier) > 1) {
-                // $key is a collection index
-                $entities = $this->getEntities();
-
-                return isset($entities[$key]) ? $entities[$key] : null;
-            } elseif ($this->entities) {
-                return isset($this->entities[$key]) ? $this->entities[$key] : null;
-            } else if ($entityLoader = $this->entityLoader) {
-                $entities = $entityLoader->getEntitiesByKeys($this->identifier, array($key));
-                return (isset($entities[0])) ? $entities[0] : false;
-            }
+        $found = array();
 
-            return $this->em->find($this->class, $key);
-        } catch (NoResultException $e) {
-            return null;
+        if (count($this->identifier) > 1) {
+            // $key is a collection index
+            $entities = $this->getEntities();
+            foreach ($keys as $key) {
+                if (isset($entities[$key])) {
+                    $found[] = $entities[$key];
+                }
+            }
+        } else if ($this->entities) {
+            foreach ($keys as $key) {
+                if (isset($this->entities[$key])) {
+                    $found[] = $this->entities[$key];
+                }
+            }
+        } else if ($entityLoader = $this->entityLoader) {
+            $found = $entityLoader->getEntitiesByKeys($this->identifier, $keys);
+        } else if ($keys) {
+            $identifier = current($this->identifier);
+            $found = $this->em->getRepository($this->class)
+                            ->findBy(array($identifier => $keys));
         }
+
+        return $found;
     }
 
     /**

diff --git a/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php b/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php
@@ -84,19 +84,13 @@ public function reverseTransform($keys)
             throw new UnexpectedTypeException($keys, 'array');
         }
 
-        $notFound = array();
-
-        // optimize this into a SELECT WHERE IN query
-        foreach ($keys as $key) {
-            if ($entity = $this->choiceList->getEntity($key)) {
-                $collection->add($entity);
-            } else {
-                $notFound[] = $key;
-            }
+        $entities = $this->choiceList->getEntitiesByKeys($keys);
+        if (count($keys) != count($entities)) {
+            throw new TransformationFailedException('Not all entities matching the keys were found.');
         }
-
-        if (count($notFound) > 0) {
-            throw new TransformationFailedException(sprintf('The entities with keys "%s" could not be found', implode('", "', $notFound)));
+        
+        foreach ($entities as $entity) {
+            $collection->add($entity);
         }
 
         return $collection;

diff --git a/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntityToIdTransformer.php b/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntityToIdTransformer.php
@@ -74,10 +74,10 @@ public function reverseTransform($key)
             throw new UnexpectedTypeException($key, 'numeric');
         }
 
-        if (!($entity = $this->choiceList->getEntity($key))) {
+        if (!($entities = $this->choiceList->getEntitiesByKeys(array($key)))) {
             throw new TransformationFailedException(sprintf('The entity with key "%s" could not be found', $key));
         }
 
-        return $entity;
+        return $entities[0];
     }
 }
diff --git a/tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php b/tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php
@@ -19,4 +19,9 @@ public function __construct($id, $name) {
         $this->id = $id;
         $this->name = $name;
     }
+
+    public function __toString()
+    {
+        return (string)$this->id;
+    }
 }