Fix Eloquent eager loading matching.

taylorotwell · taylorotwell · commit 94948cf675b9 · 2012-04-23T16:08:57.000-05:00
diff --git a/laravel/database/eloquent/query.php b/laravel/database/eloquent/query.php
@@ -66,12 +66,11 @@ public function first($columns = array('*'))
 	 * Get all of the model results for the query.
 	 *
 	 * @param  array  $columns
-	 * @param  bool   $keyed
 	 * @return array
 	 */
-	public function get($columns = array('*'), $keyed = true)
+	public function get($columns = array('*'))
 	{
-		return $this->hydrate($this->model, $this->table->get($columns), $keyed);
+		return $this->hydrate($this->model, $this->table->get($columns));
 	}
 
 	/**
@@ -100,10 +99,9 @@ public function paginate($per_page = null, $columns = array('*'))
 	 *
 	 * @param  Model  $model
 	 * @param  array  $results
-	 * @param  bool   $keyed
 	 * @return array
 	 */
-	public function hydrate($model, $results, $keyed = true)
+	public function hydrate($model, $results)
 	{
 		$class = get_class($model);
 
@@ -128,17 +126,7 @@ public function hydrate($model, $results, $keyed = true)
 
 			$new->original = $new->attributes;
 
-			// Typically, the resulting models are keyed by their primary key, but it
-			// may be useful to not do this in some circumstances such as when we
-			// are eager loading a *-to-* relationships which has duplicates.
-			if ($keyed)
-			{
-				$models[$result[$this->model->key()]] = $new;
-			}
-			else
-			{
-				$models[] = $new;
-			}
+			$models[] = $new;
 		}
 
 		if (count($results) > 0)
@@ -199,17 +187,7 @@ protected function load(&$results, $relationship, $constraints)
 
 		$query->initialize($results, $relationship);
 
-		// If we're eager loading a many-to-many relationship we will disable
-		// the primary key indexing on the hydration since there could be
-		// roles shared across users and we don't want to overwrite.
-		if ( ! $query instanceof Has_Many_And_Belongs_To)
-		{
-			$query->match($relationship, $results, $query->get());
-		}
-		else
-		{
-			$query->match($relationship, $results, $query->get(array('*'), false));
-		}
+		$query->match($relationship, $results, $query->get());
 	}
 
 	/**
diff --git a/laravel/database/eloquent/relationships/belongs_to.php b/laravel/database/eloquent/relationships/belongs_to.php
@@ -87,9 +87,14 @@ public function match($relationship, &$children, $parents)
 
 		foreach ($children as &$child)
 		{
-			if (array_key_exists($child->$foreign, $parents))
+			$parent = array_first($parents, function($k, $v) use ($child, $foreign)
 			{
-				$child->relationships[$relationship] = $parents[$child->$foreign];
+				return $v->get_key() == $child->$foreign;
+			});
+
+			if ( ! is_null($parent))
+			{
+				$child->relationships[$relationship] = $parent;
 			}
 		}
 	}
diff --git a/laravel/database/eloquent/relationships/has_many.php b/laravel/database/eloquent/relationships/has_many.php
@@ -91,9 +91,14 @@ public function match($relationship, &$parents, $children)
 	{
 		$foreign = $this->foreign_key();
 
-		foreach ($children as $key => $child)
+		foreach ($parents as &$parent)
 		{
-			$parents[$child->$foreign]->relationships[$relationship][$child->get_key()] = $child;
+			$matching = array_filter($children, function($v) use ($parent, $foreign)
+			{
+				return $v->$foreign == $parent->get_key();
+			});
+
+			$parent->relationships[$relationship] = $matching;
 		}
 	}
 
diff --git a/laravel/database/eloquent/relationships/has_many_and_belongs_to.php b/laravel/database/eloquent/relationships/has_many_and_belongs_to.php
@@ -311,7 +311,7 @@ public function initialize(&$parents, $relationship)
 	 */
 	public function eagerly_constrain($results)
 	{
-		$this->table->where_in($this->joining.'.'.$this->foreign_key(), array_keys($results));
+		$this->table->where_in($this->joining.'.'.$this->foreign_key(), $this->keys($results));
 	}
 
 	/**
@@ -325,14 +325,14 @@ public function match($relationship, &$parents, $children)
 	{
 		$foreign = $this->foreign_key();
 
-		// For each child we'll just get the parent that connects to the child and set the
-		// child model on the relationship array using the keys. Once we're done looping
-		// through the children all of the proper relations will be set.
-		foreach ($children as $key => $child)
+		foreach ($parents as &$parent)
 		{
-			$parent =& $parents[$child->pivot->$foreign];
+			$matching = array_filter($children, function($v) use ($parent, $foreign)
+			{
+				return $v->pivot->$foreign == $parent->get_key();
+			});
 
-			$parent->relationships[$relationship][$child->{$child->key()}] = $child;
+			$parent->relationships[$relationship] = $matching;
 		}
 	}
 
diff --git a/laravel/database/eloquent/relationships/has_one.php b/laravel/database/eloquent/relationships/has_one.php
@@ -38,9 +38,14 @@ public function match($relationship, &$parents, $children)
 	{
 		$foreign = $this->foreign_key();
 
-		foreach ($children as $key => $child)
+		foreach ($parents as &$parent)
 		{
-			$parents[$child->$foreign]->relationships[$relationship] = $child;
+			$matching = array_first($children, function($k, $v) use ($parent, $foreign)
+			{
+				return $v->$foreign == $parent->get_key();
+			});
+
+			$parent->relationships[$relationship] = $matching;
 		}
 	}
 
diff --git a/laravel/database/eloquent/relationships/has_one_or_many.php b/laravel/database/eloquent/relationships/has_one_or_many.php
@@ -53,7 +53,7 @@ protected function constrain()
 	 */
 	public function eagerly_constrain($results)
 	{
-		$this->table->where_in($this->foreign_key(), array_keys($results));
+		$this->table->where_in($this->foreign_key(), $this->keys($results));
 	}
 
 }
diff --git a/laravel/database/eloquent/relationships/relationship.php b/laravel/database/eloquent/relationships/relationship.php
@@ -101,4 +101,22 @@ public function foreign_key()
 		return static::foreign($this->base, $this->foreign);
 	}
 
+	/**
+	 * Gather all the primary keys from a result set.
+	 *
+	 * @param  array  $results
+	 * @return array
+	 */
+	public function keys($results)
+	{
+		$keys = array();
+
+		foreach ($results as $result)
+		{
+			$keys[] = $result->get_key();
+		}
+
+		return array_unique($keys);
+	}
+
 }
diff --git a/laravel/error.php b/laravel/error.php
@@ -13,7 +13,7 @@ public static function exception($exception, $trace = true)
 	{
 		static::log($exception);
 
-		ob_get_level() and ob_end_clean();
+		//ob_get_level() and ob_end_clean();
 
 		// If detailed errors are enabled, we'll just format the exception into
 		// a simple error message and display it on the screen. We don't use a

Original file line number	Diff line number	Diff line change
`@@ -87,9 +87,14 @@ public function match($relationship, &$children, $parents)`
`87`	`87`
`88`	`88`	`foreach ($children as &$child)`
`89`	`89`	`{`
`90`		`- if (array_key_exists($child->$foreign, $parents))`
	`90`	`+ $parent = array_first($parents, function($k, $v) use ($child, $foreign)`
`91`	`91`	`{`
`92`		`- $child->relationships[$relationship] = $parents[$child->$foreign];`
	`92`	`+ return $v->get_key() == $child->$foreign;`
	`93`	`+ });`
	`94`	`+`
	`95`	`+ if ( ! is_null($parent))`
	`96`	`+ {`
	`97`	`+ $child->relationships[$relationship] = $parent;`
`93`	`98`	`}`
`94`	`99`	`}`
`95`	`100`	`}`
Original file line number	Diff line number	Diff line change
`@@ -91,9 +91,14 @@ public function match($relationship, &$parents, $children)`
`91`	`91`	`{`
`92`	`92`	`$foreign = $this->foreign_key();`
`93`	`93`
`94`		`- foreach ($children as $key => $child)`
	`94`	`+ foreach ($parents as &$parent)`
`95`	`95`	`{`
`96`		`- $parents[$child->$foreign]->relationships[$relationship][$child->get_key()] = $child;`
	`96`	`+ $matching = array_filter($children, function($v) use ($parent, $foreign)`
	`97`	`+ {`
	`98`	`+ return $v->$foreign == $parent->get_key();`
	`99`	`+ });`
	`100`	`+`
	`101`	`+ $parent->relationships[$relationship] = $matching;`
`97`	`102`	`}`
`98`	`103`	`}`
`99`	`104`
Original file line number	Diff line number	Diff line change
`@@ -311,7 +311,7 @@ public function initialize(&$parents, $relationship)`
`311`	`311`	`*/`
`312`	`312`	`public function eagerly_constrain($results)`
`313`	`313`	`{`
`314`		`- $this->table->where_in($this->joining.'.'.$this->foreign_key(), array_keys($results));`
	`314`	`+ $this->table->where_in($this->joining.'.'.$this->foreign_key(), $this->keys($results));`
`315`	`315`	`}`
`316`	`316`
`317`	`317`	`/**`
`@@ -325,14 +325,14 @@ public function match($relationship, &$parents, $children)`
`325`	`325`	`{`
`326`	`326`	`$foreign = $this->foreign_key();`
`327`	`327`
`328`		`- // For each child we'll just get the parent that connects to the child and set the`
`329`		`- // child model on the relationship array using the keys. Once we're done looping`
`330`		`- // through the children all of the proper relations will be set.`
`331`		`- foreach ($children as $key => $child)`
	`328`	`+ foreach ($parents as &$parent)`
`332`	`329`	`{`
`333`		`- $parent =& $parents[$child->pivot->$foreign];`
	`330`	`+ $matching = array_filter($children, function($v) use ($parent, $foreign)`
	`331`	`+ {`
	`332`	`+ return $v->pivot->$foreign == $parent->get_key();`
	`333`	`+ });`
`334`	`334`
`335`		`- $parent->relationships[$relationship][$child->{$child->key()}] = $child;`
	`335`	`+ $parent->relationships[$relationship] = $matching;`
`336`	`336`	`}`
`337`	`337`	`}`
`338`	`338`
Original file line number	Diff line number	Diff line change
`@@ -38,9 +38,14 @@ public function match($relationship, &$parents, $children)`
`38`	`38`	`{`
`39`	`39`	`$foreign = $this->foreign_key();`
`40`	`40`
`41`		`- foreach ($children as $key => $child)`
	`41`	`+ foreach ($parents as &$parent)`
`42`	`42`	`{`
`43`		`- $parents[$child->$foreign]->relationships[$relationship] = $child;`
	`43`	`+ $matching = array_first($children, function($k, $v) use ($parent, $foreign)`
	`44`	`+ {`
	`45`	`+ return $v->$foreign == $parent->get_key();`
	`46`	`+ });`
	`47`	`+`
	`48`	`+ $parent->relationships[$relationship] = $matching;`
`44`	`49`	`}`
`45`	`50`	`}`
`46`	`51`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ protected function constrain()`
`53`	`53`	`*/`
`54`	`54`	`public function eagerly_constrain($results)`
`55`	`55`	`{`
`56`		`- $this->table->where_in($this->foreign_key(), array_keys($results));`
	`56`	`+ $this->table->where_in($this->foreign_key(), $this->keys($results));`
`57`	`57`	`}`
`58`	`58`
`59`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ public static function exception($exception, $trace = true)`
`13`	`13`	`{`
`14`	`14`	`static::log($exception);`
`15`	`15`
`16`		`- ob_get_level() and ob_end_clean();`
	`16`	`+ //ob_get_level() and ob_end_clean();`
`17`	`17`
`18`	`18`	`// If detailed errors are enabled, we'll just format the exception into`
`19`	`19`	`// a simple error message and display it on the screen. We don't use a`