Refactoring to avoid code duplication

cakephp · May 26, 2013 · 326ad60 · 326ad60
1 parent 399fb0a
commit 326ad60
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 134 deletions.
diff --git a/lib/Cake/ORM/Association.php b/lib/Cake/ORM/Association.php
@@ -339,10 +339,56 @@ protected  function _options(array $options) {
  * Alters a Query object to include the associated target table data in the final
  * result
  *
+ * The options array accept the following keys:
+ *
+ * - includeFields: Whether to include target model fields in the result or not
+ * - foreignKey: The name of the field to use as foreign key, if false none
+ *   will be used
+ * - conditions: array with a list of conditions to filter the join with
+ * - fields: a list of fields in the target table to include in the result
+ * - type: The type of join to be used (e.g. INNER)
+ *
  * @param Query $query the query to be altered to include the target table data
  * @param array $options Any extra options or overrides to be taken in account
  * @return void
  */
-	public abstract function attachTo(Query $query, array $options = []);
+	public function attachTo(Query $query, array $options = []) {
+		$target = $this->target();
+		$source = $this->source();
+		$options += [
+			'includeFields' => true,
+			'foreignKey' => $this->foreignKey(),
+			'conditions' => [],
+			'type' => $this->joinType(),
+			'table' => $target->table()
+		];
+		$options['conditions'] = array_merge($this->conditions(), $options['conditions']);
+
+		if (!empty($options['foreignKey'])) {
+			$options['conditions'][] = $this->_joinCondition($options);
+		}
+
+		$joinOptions = ['table' => 1, 'conditions' => 1, 'type' => 1];
+		$query->join([$target->alias() => array_intersect_key($options, $joinOptions)]);
 
+		if (empty($options['fields'])) {
+			$f = isset($options['fields']) ? $options['fields'] : null;
+			if ($options['includeFields'] && ($f === null || $f !== false)) {
+				$options['fields'] = array_keys($target->schema());
+			}
+		}
+
+		if (!empty($options['fields'])) {
+			$query->select($query->aliasFields($options['fields'], $target->alias()));
+		}
+	}
+
+/**
+ * Returns a single or multiple conditions to be appended to the generated join
+ * clause for getting the results on the target table.
+ *
+ * @param array $options list of options passed to attachTo method
+ * @return string|array
+ */
+	protected abstract function _joinCondition(array $options);
 }
diff --git a/lib/Cake/ORM/Association/BelongsTo.php b/lib/Cake/ORM/Association/BelongsTo.php
@@ -52,55 +52,19 @@ public function foreignKey($key = null) {
 	}
 
 /**
- * Alters a Query object to include the associated target table data in the final
- * result
+ * Returns a single or multiple conditions to be appended to the generated join
+ * clause for getting the results on the target table.
  *
- * The options array accept the following keys:
- *
- * - includeFields: Whether to include target model fields in the result or not
- * - foreignKey: The name of the field to use as foreign key, if false none
- *   will be used
- * - conditions: array with a list of conditions to filter the join with
- * - joinType: The type of join to be used (e.g. INNER)
- *
- * @param Query $query the query to be altered to include the target table data
- * @param array $options Any extra options or overrides to be taken in account
- * @return void
+ * @param array $options list of options passed to attachTo method
+ * @return string|array
  */
-	public function attachTo(Query $query, array $options = []) {
-		$target = $this->target();
-		$source = $this->source();
-		$options += [
-			'includeFields' => true,
-			'foreignKey' => $this->foreignKey(),
-			'conditions' => [],
-			'type' => $this->joinType(),
-			'table' => $target->table()
-		];
-		$options['conditions'] = array_merge($this->conditions(), $options['conditions']);
-
-		if (!empty($options['foreignKey'])) {
-			$options['conditions'][] =  sprintf('%s.%s = %s.%s',
-				$target->alias(),
-				$target->primaryKey(),
-				$source->alias(),
+	protected function _joinCondition(array $options) {
+		return sprintf('%s.%s = %s.%s',
+				$this->_targetTable->alias(),
+				$this->_targetTable->primaryKey(),
+				$this->_sourceTable->alias(),
 				$options['foreignKey']
 			);
-		}
-
-		$joinOptions = ['table' => 1, 'conditions' => 1, 'type' => 1];
-		$query->join([$target->alias() => array_intersect_key($options, $joinOptions)]);
-
-		if (empty($options['fields'])) {
-			$f = isset($options['fields']) ? $options['fields'] : null;
-			if ($options['includeFields'] && ($f === null || $f !== false)) {
-				$options['fields'] = array_keys($target->schema());
-			}
-		}
-
-		if (!empty($options['fields'])) {
-			$query->select($query->aliasFields($options['fields'], $target->alias()));
-		}
 	}
 
 }
diff --git a/lib/Cake/ORM/Association/ExternalAssociationTrait.php b/lib/Cake/ORM/Association/ExternalAssociationTrait.php
@@ -73,47 +73,6 @@ function sort($sort = null) {
 		return $this->_sort;
 	}
 
-/**
- * Not implemented
- *
- * @return boolean false
- */
-	public function attachTo(Query $query, array $options = []) {
-		$target = $this->target();
-		$source = $this->source();
-		$options += [
-			'includeFields' => true,
-			'foreignKey' => $this->foreignKey(),
-			'conditions' => [],
-			'type' => $this->joinType(),
-			'table' => $target->table()
-		];
-		$options['conditions'] = array_merge($this->conditions(), $options['conditions']);
-
-		if (!empty($options['foreignKey'])) {
-			$options['conditions'][] =  sprintf('%s.%s = %s.%s',
-				$source->alias(),
-				$source->primaryKey(),
-				$target->alias(),
-				$options['foreignKey']
-			);
-		}
-
-		$joinOptions = ['table' => 1, 'conditions' => 1, 'type' => 1];
-		$query->join([$target->alias() => array_intersect_key($options, $joinOptions)]);
-
-		if (empty($options['fields'])) {
-			$f = isset($options['fields']) ? $options['fields'] : null;
-			if ($options['includeFields'] && ($f === null || $f !== false)) {
-				$options['fields'] = array_keys($target->schema());
-			}
-		}
-
-		if (!empty($options['fields'])) {
-			$query->select($query->aliasFields($options['fields'], $target->alias()));
-		}
-	}
-
 /**
  * Returns true if the eager loading process will require a set of parent table's
  * primary keys in order to use them as a filter in the finder query.
@@ -134,6 +93,15 @@ public function requiresKeys($options = []) {
  */
 	public abstract function eagerLoader(array $options);
 
+	protected function _joinCondition(array $options) {
+		return sprintf('%s.%s = %s.%s',
+				$this->_sourceTable->alias(),
+				$this->_sourceTable->primaryKey(),
+				$this->_targetTable->alias(),
+				$options['foreignKey']
+			);
+	}
+
 /**
  * Returns a callable to be used for each row in a query result set
  * for injecting the eager loaded rows

diff --git a/lib/Cake/ORM/Association/HasOne.php b/lib/Cake/ORM/Association/HasOne.php
@@ -59,56 +59,20 @@ public function foreignKey($key = null) {
 	}
 
 /**
- * Alters a Query object to include the associated target table data in the final
- * result
+ * Returns a single or multiple conditions to be appended to the generated join
+ * clause for getting the results on the target table.
  *
- * The options array accept the following keys:
- *
- * - includeFields: Whether to include target model fields in the result or not
- * - foreignKey: The name of the field to use as foreign key, if false none
- *   will be used
- * - conditions: array with a list of conditions to filter the join with
- * - fields: a list of fields in the target table to include in the result
- * - type: The type of join to be used (e.g. INNER)
- *
- * @param Query $query the query to be altered to include the target table data
- * @param array $options Any extra options or overrides to be taken in account
- * @return void
+ * @param array $options list of options passed to attachTo method
+ * @return string|array
  */
-	public function attachTo(Query $query, array $options = []) {
-		$target = $this->target();
-		$source = $this->source();
-		$options += [
-			'includeFields' => true,
-			'foreignKey' => $this->foreignKey(),
-			'conditions' => [],
-			'type' => $this->joinType(),
-			'table' => $target->table()
-		];
-		$options['conditions'] = array_merge($this->conditions(), $options['conditions']);
-
-		if (!empty($options['foreignKey'])) {
-			$options['conditions'][] =  sprintf('%s.%s = %s.%s',
-				$source->alias(),
-				$source->primaryKey(),
-				$target->alias(),
+	protected function _joinCondition(array $options) {
+		return sprintf('%s.%s = %s.%s',
+				$this->_sourceTable->alias(),
+				$this->_sourceTable->primaryKey(),
+				$this->_targetTable->alias(),
 				$options['foreignKey']
 			);
-		}
-
-		$joinOptions = ['table' => 1, 'conditions' => 1, 'type' => 1];
-		$query->join([$target->alias() => array_intersect_key($options, $joinOptions)]);
-
-		if (empty($options['fields'])) {
-			$f = isset($options['fields']) ? $options['fields'] : null;
-			if ($options['includeFields'] && ($f === null || $f !== false)) {
-				$options['fields'] = array_keys($target->schema());
-			}
-		}
-
-		if (!empty($options['fields'])) {
-			$query->select($query->aliasFields($options['fields'], $target->alias()));
-		}
 	}
 
+
 }
diff --git a/lib/Cake/Test/TestCase/ORM/AssociationTest.php b/lib/Cake/Test/TestCase/ORM/AssociationTest.php
@@ -50,7 +50,7 @@ public function setUp() {
 		];
 		$this->association = $this->getMock(
 			'\Cake\ORM\Association',
-			['_options', 'attachTo'],
+			['_options', 'attachTo', '_joinCondition'],
 			['Foo', $config]
 		);
 	}