Merge pull request #274 from hexus/5.6-fix-belongs-to-many-eager-loads

Fixed incomplete BelongsToMany eager loads (#273)
analogueorm · Mar 12, 2019 · b12cdea · b12cdea
2 parents 6d0e5e3 + 841d2ae
commit b12cdea
Show file tree

Hide file tree

Showing 14 changed files with 220 additions and 45 deletions.
diff --git a/src/Commands/Store.php b/src/Commands/Store.php
@@ -150,7 +150,7 @@ protected function postStoreProcess()
 
         // Now we can sync the related collections
         //
-        // TODO (note) : not sute this check is needed, as we can assume
+        // TODO (note) : not sure this check is needed, as we can assume
         // the aggregate exists in the Post Store Process
         if ($this->aggregate->exists()) {
             $this->aggregate->syncRelationships($foreignRelationships);

diff --git a/src/Relationships/BelongsToMany.php b/src/Relationships/BelongsToMany.php
@@ -205,7 +205,7 @@ public function get($columns = ['*']) : Collection
 
         $select = $this->getSelectColumns($columns);
 
-        $entities = $this->query->addSelect($select)->get()->all();
+        $entities = $this->query->addSelect($select)->disableCache()->get()->all();
 
         $entities = $this->hydratePivotRelation($entities);
 
@@ -217,28 +217,29 @@ public function get($columns = ['*']) : Collection
      *
      * @param array $entities
      *
-     * @return void
+     * @return array
      */
     protected function hydratePivotRelation(array $entities)
     {
         // TODO (note) We should definitely get rid of the pivot in a next
         // release, as this is not quite relevant in a datamapper context.
-        $host = $this;
-
-        return array_map(function ($entity) use ($host) {
+        return array_map(function ($entity) {
             $entityWrapper = $this->factory->make($entity);
 
-            $pivot = $this->newExistingPivot($this->cleanPivotAttributes($entityWrapper));
+            $pivotAttributes = $this->cleanPivotAttributes($entityWrapper);
+            $pivot = $this->newExistingPivot($pivotAttributes);
             $entityWrapper->setEntityAttribute('pivot', $pivot);
 
-            return $entityWrapper->getObject();
+            $object = $entityWrapper->unwrap();
+
+            return $object;
         }, $entities);
     }
 
     /**
      * Get the pivot attributes from a model.
      *
-     * @param  $entity
+     * @param InternallyMappable $entity
      *
      * @return array
      */
@@ -442,7 +443,9 @@ public function match(array $results, $relation)
         // children back to their parent using the dictionary and the keys on the
         // the parent models. Then we will return the hydrated models back out.
         return array_map(function ($result) use ($dictionary, $keyName, $cache, $relation, $host) {
-            if (isset($dictionary[$key = $result[$keyName]])) {
+            $key = $result[$keyName];
+
+            if (isset($dictionary[$key])) {
                 $collection = $host->relatedMap->newCollection($dictionary[$key]);
 
                 $result[$relation] = $collection;
@@ -475,7 +478,10 @@ protected function buildDictionary(EntityCollection $results)
 
         foreach ($results as $entity) {
             $wrapper = $this->factory->make($entity);
-            $dictionary[$wrapper->getEntityAttribute('pivot')->$foreign][] = $entity;
+
+            $foreignKey = $wrapper->getEntityAttribute('pivot')->$foreign;
+
+            $dictionary[$foreignKey][] = $entity;
         }
 
         return $dictionary;
@@ -614,8 +620,6 @@ protected function detachExcept(array $entities = [])
         $query->where($this->foreignKey, '=', $this->parent->getEntityAttribute($parentKey));
 
         $query->delete();
-
-        $query = $this->newPivotQuery();
     }
 
     /**

diff --git a/src/Relationships/EmbeddedRelationship.php b/src/Relationships/EmbeddedRelationship.php
@@ -217,7 +217,7 @@ abstract public function match(array $results) : array;
      */
     protected function buildEmbeddedObject(array $attributes)
     {
-        $resultBuilder = new ResultBuilder($this->getRelatedMapper());
+        $resultBuilder = new ResultBuilder($this->getRelatedMapper(), true);
 
         // TODO : find a way to support eager load within an embedded
         // object.

diff --git a/src/Relationships/HasManyThrough.php b/src/Relationships/HasManyThrough.php
@@ -6,6 +6,7 @@
 use Analogue\ORM\System\Mapper;
 use Analogue\ORM\System\Query;
 use Illuminate\Database\Query\Expression;
+use Illuminate\Support\Collection;
 
 class HasManyThrough extends Relationship
 {
@@ -217,7 +218,7 @@ public function getResults($relation)
      *
      * @return EntityCollection
      */
-    public function get($columns = ['*'])
+    public function get($columns = ['*']): Collection
     {
         // First we'll add the proper select columns onto the query so it is run with
         // the proper columns. Then, we will get the results and hydrate out pivot

diff --git a/src/System/Builders/EntityBuilder.php b/src/System/Builders/EntityBuilder.php
@@ -2,6 +2,7 @@
 
 namespace Analogue\ORM\System\Builders;
 
+use Analogue\ORM\System\InternallyMappable;
 use Analogue\ORM\System\Mapper;
 use Analogue\ORM\System\Wrappers\Factory;
 
@@ -43,13 +44,19 @@ class EntityBuilder
      */
     protected $factory;
 
+    /**
+     * @var bool
+     */
+    protected $useCache;
+
     /**
      * EntityBuilder constructor.
      *
      * @param Mapper $mapper
      * @param array  $eagerLoads
+     * @param bool   $useCache
      */
-    public function __construct(Mapper $mapper, array $eagerLoads)
+    public function __construct(Mapper $mapper, array $eagerLoads, bool $useCache = false)
     {
         $this->mapper = $mapper;
 
@@ -58,20 +65,22 @@ public function __construct(Mapper $mapper, array $eagerLoads)
         $this->eagerLoads = $eagerLoads;
 
         $this->factory = new Factory();
+
+        $this->useCache = $useCache;
     }
 
     /**
      * Convert an array of attributes into an entity, or retrieve entity instance from cache.
      *
      * @param array $attributes
      *
-     * @return array
+     * @return mixed
      */
     public function build(array $attributes)
     {
         // If the object we are building is a value object,
         // we won't be using the instance cache.
-        if ($this->entityMap->getKeyName() === null) {
+        if (!$this->useCache || $this->entityMap->getKeyName() === null) {
             return $this->buildEntity($attributes);
         }
 

diff --git a/src/System/Builders/ResultBuilder.php b/src/System/Builders/ResultBuilder.php
@@ -37,21 +37,30 @@ class ResultBuilder implements ResultBuilderInterface
      */
     protected $builders = [];
 
+    /**
+     * Whether to use result and entity caching.
+     *
+     * @var bool
+     */
+    protected $useCache;
+
     /**
      * ResultBuilder constructor.
      *
-     * @param Mapper $mapper
+     * @param Mapper $mapper   The mapper used to build entities with.
+     * @param bool   $useCache [optional] Whether to use result and entity caching. Defaults to false.
      */
-    public function __construct(Mapper $mapper)
+    public function __construct(Mapper $mapper, bool $useCache = false)
     {
         $this->mapper = $mapper;
         $this->entityMap = $mapper->getEntityMap();
+        $this->useCache = $useCache;
     }
 
     /**
      * Convert a result set into an array of entities.
      *
-     * @param array $results
+     * @param array $results    The results to convert into entities.
      * @param array $eagerLoads name of the relation(s) to be eager loaded on the Entities
      *
      * @return array
@@ -82,9 +91,10 @@ public function build(array $results, array $eagerLoads)
     protected function cacheResults(array $results)
     {
         $mapper = $this->mapper;
+
         // When hydrating EmbeddedValue object, they'll likely won't
         // have a primary key set.
-        if (!is_null($mapper->getEntityMap()->getKeyName())) {
+        if ($mapper->getEntityMap()->getKeyName() !== null) {
             $mapper->getEntityCache()->add($results);
         }
     }
@@ -313,11 +323,7 @@ protected function isNested(string $name, string $relation): bool
      */
     protected function buildResultSet(array $results): array
     {
-        $keyName = $this->entityMap->getKeyName();
-
-        return $keyName
-            ? $this->buildKeyedResultSet($results, $keyName)
-            : $this->buildUnkeyedResultSet($results);
+        return $this->buildUnkeyedResultSet($results);
     }
 
     /**
@@ -329,7 +335,7 @@ protected function buildResultSet(array $results): array
      */
     protected function buildUnkeyedResultSet(array $results) : array
     {
-        $builder = new EntityBuilder($this->mapper, array_keys($this->eagerLoads));
+        $builder = new EntityBuilder($this->mapper, array_keys($this->eagerLoads), $this->useCache);
 
         return array_map(function ($item) use ($builder) {
             return $builder->build($item);
@@ -346,7 +352,7 @@ protected function buildUnkeyedResultSet(array $results) : array
      */
     protected function buildKeyedResultSet(array $results, string $primaryKey) : array
     {
-        $builder = new EntityBuilder($this->mapper, array_keys($this->eagerLoads));
+        $builder = new EntityBuilder($this->mapper, array_keys($this->eagerLoads), $this->useCache);
 
         $keys = array_map(function ($item) use ($primaryKey) {
             return $item[$primaryKey];

diff --git a/src/System/Builders/ResultBuilderFactory.php b/src/System/Builders/ResultBuilderFactory.php
@@ -6,13 +6,13 @@
 
 class ResultBuilderFactory
 {
-    public function make(Mapper $mapper) : ResultBuilderInterface
+    public function make(Mapper $mapper, bool $skipCache = false) : ResultBuilderInterface
     {
         switch ($mapper->getEntityMap()->getInheritanceType()) {
             case 'single_table':
                 return new PolymorphicResultBuilder($mapper);
             default:
-                return new ResultBuilder($mapper);
+                return new ResultBuilder($mapper, !$skipCache);
         }
     }
 }
diff --git a/src/System/Builders/ResultBuilderInterface.php b/src/System/Builders/ResultBuilderInterface.php
@@ -4,5 +4,13 @@
 
 interface ResultBuilderInterface
 {
+    /**
+     * Convert a result set into an array of entities.
+     *
+     * @param array $results    The results to convert into entities.
+     * @param array $eagerLoads Relationships to eagerly load for these results.
+     *
+     * @return mixed
+     */
     public function build(array $results, array $eagerLoads);
 }
diff --git a/src/System/Mapper.php b/src/System/Mapper.php
@@ -126,12 +126,13 @@ public function __construct(EntityMap $entityMap, DBAdapter $adapter, Dispatcher
      *
      * @param array|Collection $results
      * @param array            $eagerLoads
+     * @param bool             $useCache
      *
      * @return Collection
      */
-    public function map($results, array $eagerLoads = []) : Collection
+    public function map($results, array $eagerLoads = [], $useCache = false) : Collection
     {
-        $builder = $this->newResultBuilder();
+        $builder = $this->newResultBuilder(!$useCache);
 
         if ($results instanceof Collection) {
             // Get underlying collection array
@@ -159,13 +160,15 @@ public function map($results, array $eagerLoads = []) : Collection
     /**
      * Return result builder used by this mapper.
      *
-     * @return ResultBuilder
+     * @param bool $skipCache
+     *
+     * @return ResultBuilderInterface
      */
-    protected function newResultBuilder() : ResultBuilderInterface
+    protected function newResultBuilder(bool $skipCache = false) : ResultBuilderInterface
     {
         $factory = new ResultBuilderFactory();
 
-        return $factory->make($this);
+        return $factory->make($this, $skipCache);
     }
 
     /**

diff --git a/src/System/Query.php b/src/System/Query.php
@@ -98,6 +98,13 @@ class Query
         'raw',
     ];
 
+    /**
+     * Whether to use the mapper's entity caching.
+     *
+     * @var bool
+     */
+    protected $useCache = true;
+
     /**
      * Create a new Analogue Query Builder instance.
      *
@@ -503,6 +510,32 @@ protected function getHasRelationQuery($relation, $entity)
         });
     }
 
+    /**
+     * Disable loading results from the entity instance cache.
+     *
+     * Loaded entities will still be stored in the cache.
+     *
+     * @return \Analogue\ORM\System\Query
+     */
+    public function disableCache()
+    {
+        $this->useCache = false;
+
+        return $this;
+    }
+
+    /**
+     * Enable loading results from the entity instance cache.
+     *
+     * @return \Analogue\ORM\System\Query
+     */
+    public function enableCache()
+    {
+        $this->useCache = true;
+
+        return $this;
+    }
+
     /**
      * Get the table for the current query object.
      *
@@ -577,7 +610,7 @@ public function getEntities($columns = ['*'])
         $results = $this->query->get($columns);
 
         // Pass result set to the mapper and return the EntityCollection
-        return $this->mapper->map($results, $this->getEagerLoads());
+        return $this->mapper->map($results, $this->getEagerLoads(), $this->useCache);
     }
 
     /**

diff --git a/src/System/Wrappers/Factory.php b/src/System/Wrappers/Factory.php
@@ -14,15 +14,16 @@ class Factory
      *
      * @throws \Analogue\ORM\Exceptions\MappingException
      *
-     * @return Wrapper
+     * @return ObjectWrapper
      */
     public function make($object)
     {
         $manager = Manager::getInstance();
 
-        // Instantiate hydrator. We'll need to optimize this and allow pre-generation
-        // of these hydrator, and get it, ideally, from the entityMap or the Mapper class,
-        // so it's only instantiated once
+        // Instantiate hydrator
+        // TODO: We'll need to optimize this and allow pre-generation
+        //       of these hydrators, and get it, ideally, from the entityMap or
+        //       the Mapper class, so it's only instantiated once
         $config = new Configuration(get_class($object));
 
         $hydratorClass = $config->createFactory()->getHydratorClass();