API Support named join alias for many_many through list

Add tests for sorting on joined alias

ORM/DataObject.php

@@ -1935,35 +1935,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * format, or RecordClass.FieldClass(args) format if $includeClass is true
 	 */
 	public function db($fieldName = null, $includeClass = false) {
-		$classes = ClassInfo::ancestry($this, true);
-
-		// If we're looking for a specific field, we want to hit subclasses first as they may override field types
-		if($fieldName) {
-			$classes = array_reverse($classes);
-		}
-
-		$db = array();
-		foreach($classes as $class) {
-			// Merge fields with new fields and composite fields
-			$fields = self::database_fields($class);
-			$compositeFields = self::composite_fields($class, false);
-			$db = array_merge($db, $fields, $compositeFields);
-
-			// Check for search field
-			if($fieldName && isset($db[$fieldName])) {
-				// Return found field
-				if(!$includeClass) {
-					return $db[$fieldName];
-				}
-				return $class . "." . $db[$fieldName];
-			}
-		}
-
-		// At end of search complete
-		if($fieldName) {
-			return null;
+		if ($fieldName) {
+			return static::getSchema()->fieldSpecification(static::class, $fieldName, $includeClass);
 		} else {
-			return $db;
+			return static::getSchema()->fieldSpecifications(static::class);
 		}
 	}
 
@@ -2071,7 +2046,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	/**
 	 * Return information about a specific many_many component. Returns a numeric array.
 	 * The first item in the array will be the class name of the relation: either
-	 * RELATION_MANY_MANY or RELATION_MANY_MANY_THROUGH constant value.
+	 * ManyManyList or ManyManyThroughList.
 	 *
 	 * Standard many_many return type is:
 	 *
@@ -3870,10 +3845,19 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * Set joining object
 	 *
 	 * @param DataObject $object
+	 * @param string $alias Alias
 	 * @return $this
 	 */
-	public function setJoin(DataObject $object) {
+	public function setJoin(DataObject $object, $alias = null) {
 		$this->joinRecord = $object;
+		if ($alias) {
+			if ($this->db($alias)) {
+				throw new InvalidArgumentException(
+					"Joined record $alias cannot also be a db field"
+				);
+			}
+			$this->record[$alias] = $object;
+		}
 		return $this;
 	}
 

ORM/DataObjectSchema.php

@@ -137,6 +137,54 @@ class DataObjectSchema {
 		return $this->tableName($this->baseDataClass($class));
 	}
 
+	/**
+	 * Get all DB field specifications for a class, including ancestors and composite fields.
+	 *
+	 * @param string $class
+	 * @return array
+	 */
+	public function fieldSpecifications($class) {
+		$classes = ClassInfo::ancestry($class, true);
+		$db = [];
+		foreach($classes as $tableClass) {
+			// Merge fields with new fields and composite fields
+			$fields = $this->databaseFields($tableClass);
+			$compositeFields = $this->compositeFields($tableClass, false);
+			$db = array_merge($db, $fields, $compositeFields);
+		}
+		return $db;
+	}
+
+
+	/**
+	 * Get specifications for a single class field
+	 *
+	 * @param string $class
+	 * @param string $fieldName
+	 * @param bool $includeClass If returning a single column, prefix the column with the class name
+	 * in RecordClass.Column(spec) format
+	 * @return string|null Field will be a string in FieldClass(args) format, or
+	 * RecordClass.FieldClass(args) format if $includeClass is true. Will be null if no field is found.
+	 */
+	public function fieldSpecification($class, $fieldName, $includeClass = false) {
+		$classes = array_reverse(ClassInfo::ancestry($class, true));
+		foreach($classes as $tableClass) {
+			// Merge fields with new fields and composite fields
+			$fields = $this->databaseFields($tableClass);
+			$compositeFields = $this->compositeFields($tableClass, false);
+			$db = array_merge($fields, $compositeFields);
+
+			// Check for search field
+			if(isset($db[$fieldName])) {
+				$prefix = $includeClass ? "{$tableClass}." : "";
+				return $prefix . $db[$fieldName];
+			}
+		}
+
+		// At end of search complete
+		return null;
+	}
+
 	/**
 	 * Find the class for the given table
 	 *
@@ -689,6 +737,16 @@ class DataObjectSchema {
 			);
 		}
 
+		// Validate the join class isn't also the name of a field or relation on either side
+		// of the relation
+		$field = $this->fieldSpecification($relationClass, $joinClass);
+		if ($field) {
+			throw new InvalidArgumentException(
+				"many_many through relation {$parentClass}.{$component} {$key} class {$relationClass} "
+				. " cannot have a db field of the same name of the join class {$joinClass}"
+			);
+		}
+
 		// Validate bad types on parent relation
 		if ($key === 'from' && $relationClass !== $parentClass) {
 			throw new InvalidArgumentException(

ORM/ManyManyList.php

@@ -228,7 +228,7 @@ class ManyManyList extends RelationList {
 		// Validate foreignID
 		$foreignIDs = $this->getForeignID();
 		if(empty($foreignIDs)) {
-			throw new BadMethodCallException("ManyManyList::add() can't be called until a foreign ID is set", E_USER_WARNING);
+			throw new BadMethodCallException("ManyManyList::add() can't be called until a foreign ID is set");
 		}
 
 		// Apply this item to each given foreign ID record
@@ -317,7 +317,7 @@ class ManyManyList extends RelationList {
 		if($filter = $this->foreignIDWriteFilter($this->getForeignID())) {
 			$query->setWhere($filter);
 		} else {
-			user_error("Can't call ManyManyList::remove() until a foreign ID is set", E_USER_WARNING);
+			user_error("Can't call ManyManyList::remove() until a foreign ID is set");
 		}
 
 		$query->addWhere(array(
@@ -380,7 +380,7 @@ class ManyManyList extends RelationList {
 		}
 
 		if(!is_numeric($itemID)) {
-			user_error('ComponentSet::getExtraData() passed a non-numeric child ID', E_USER_ERROR);
+			throw new InvalidArgumentException('ManyManyList::getExtraData() passed a non-numeric child ID');
 		}
 
 		$cleanExtraFields = array();
@@ -392,7 +392,7 @@ class ManyManyList extends RelationList {
 		if($filter) {
 			$query->setWhere($filter);
 		} else {
-			user_error("Can't call ManyManyList::getExtraData() until a foreign ID is set", E_USER_WARNING);
+			throw new BadMethodCallException("Can't call ManyManyList::getExtraData() until a foreign ID is set");
 		}
 		$query->addWhere(array(
 			"\"{$this->joinTable}\".\"{$this->localKey}\"" => $itemID

ORM/ManyManyThroughList.php

@@ -50,7 +50,8 @@ class ManyManyThroughList extends RelationList
 	public function createDataObject($row) {
 		// Add joined record
 		$joinRow = [];
-		$prefix = ManyManyThroughQueryManipulator::JOIN_TABLE_ALIAS . '_';
+		$joinAlias = $this->manipulator->getJoinAlias();
+		$prefix = $joinAlias . '_';
 		foreach ($row as $key => $value) {
 			if (strpos($key, $prefix) === 0) {
 				$joinKey = substr($key, strlen($prefix));
@@ -67,7 +68,7 @@ class ManyManyThroughList extends RelationList
 			$joinClass = $this->manipulator->getJoinClass();
 			$joinQueryParams = $this->manipulator->extractInheritableQueryParameters($this->dataQuery);
 			$joinRecord = Injector::inst()->create($joinClass, $joinRow, false, $this->model, $joinQueryParams);
-			$record->setJoin($joinRecord);
+			$record->setJoin($joinRecord, $joinAlias);
 		}
 
 		return $record;
@@ -151,7 +152,7 @@ class ManyManyThroughList extends RelationList
 		// Validate foreignID
 		$foreignIDs = $this->getForeignID();
 		if (empty($foreignIDs)) {
-			throw new BadMethodCallException("ManyManyList::add() can't be called until a foreign ID is set", E_USER_WARNING);
+			throw new BadMethodCallException("ManyManyList::add() can't be called until a foreign ID is set");
 		}
 
 		// Apply this item to each given foreign ID record

ORM/ManyManyThroughQueryManipulator.php

@@ -5,7 +5,6 @@ namespace SilverStripe\ORM;
 
 use SilverStripe\Core\Injector\Injectable;
 use SilverStripe\Core\Injector\Injector;
-use SilverStripe\Dev\Debug;
 use SilverStripe\ORM\Queries\SQLSelect;
 
 /**
@@ -13,10 +12,6 @@ use SilverStripe\ORM\Queries\SQLSelect;
  */
 class ManyManyThroughQueryManipulator implements DataQueryManipulator
 {
-	/**
-	 * Alias to use for sql join table
-	 */
-	const JOIN_TABLE_ALIAS = 'Join';
 
 	use Injectable;
 
@@ -152,6 +147,15 @@ class ManyManyThroughQueryManipulator implements DataQueryManipulator
 		return $inst->getInheritableQueryParams();
 	}
 
+	/**
+	 * Get name of join table alias for use in queries.
+	 *
+	 * @return string
+	 */
+	public function getJoinAlias() {
+		return $this->getJoinClass();
+	}
+
 	/**
 	 * Invoked prior to getFinalisedQuery()
 	 *
@@ -166,7 +170,7 @@ class ManyManyThroughQueryManipulator implements DataQueryManipulator
 		$joinTableSQLSelect = $hasManyRelation->dataQuery()->query();
 		$joinTableSQL = $joinTableSQLSelect->sql($joinTableParameters);
 		$joinTableColumns = array_keys($joinTableSQLSelect->getSelect()); // Get aliases (keys) only
-		$joinTableAlias = static::JOIN_TABLE_ALIAS;
+		$joinTableAlias = $this->getJoinAlias();
 
 		// Get fields to join on
 		$localKey = $this->getLocalKey();

docs/en/02_Developer_Guides/00_Model/02_Relations.md

@@ -281,6 +281,9 @@ This is declared via array syntax, with the following keys on the many_many:
  - `through` Class name of the mapping table
  - `from` Name of the has_one relationship pointing back at the object declaring many_many
  - `to` Name of the has_one relationship pointing to the object declaring belongs_many_many.
+ 
+Note: The `through` class must not also be the name of any field or relation on the parent
+or child record.
 
 The syntax for `belongs_many_many` is unchanged.
 
@@ -314,30 +317,35 @@ The syntax for `belongs_many_many` is unchanged.
 	  ];
 	}
 
-In order to filter on the join table during queries, you can use the "Join" table alias
+In order to filter on the join table during queries, you can use the class name of the joining table
 for any sql conditions.
 
 
 	:::php
 	$team = Team::get()->byId(1);
-	$supporters = $team->Supporters()->where(['"Join"."Ranking"' => 1]);
+	$supporters = $team->Supporters()->where(['"TeamSupporter"."Ranking"' => 1]);
 
 
-Note: ->filter() currently does not support joined fields natively.
-  
+Note: ->filter() currently does not support joined fields natively due to the fact that the
+query for the join table is isolated from the outer query controlled by DataList.
+
 
 ### Using many_many in templates
 
 The relationship can also be navigated in [templates](../templates).
-The joined record can be accessed via getJoin() (many_many through only)
+The joined record can be accessed via `Join` or `TeamSupporter` property (many_many through only)
 	
 	:::ss
 	<% with $Supporter %>
 		<% loop $Supports %>
-			Supports $Title <% if $Join %>(rank $Join.Ranking)<% end_if %>
+			Supports $Title <% if $TeamSupporter %>(rank $TeamSupporter.Ranking)<% end_if %>
 		<% end_if %>
 	<% end_with %>
 
+
+You can also use `$Join` in place of the join class alias (`$TeamSupporter`), if your template
+is class-agnostic and doesn't know the type of the join table. 
+
 ## belongs_many_many
 
 The belongs_many_many represents the other side of the relationship on the target data class.

tests/model/ManyManyThroughListTest.php

@@ -4,7 +4,6 @@ use SilverStripe\Dev\Debug;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Dev\TestOnly;
 use SilverStripe\ORM\DataObject;
-use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\ORM\ManyManyThroughList;
 use SilverStripe\ORM\Versioning\Versioned;
 
@@ -21,6 +20,16 @@ class ManyManyThroughListTest extends SapphireTest
 		ManyManyThroughListTest_VersionedObject::class,
 	];
 
+	public function setUp() {
+		parent::setUp();
+		DataObject::reset();
+	}
+
+	public function tearDown() {
+		DataObject::reset();
+		parent::tearDown();
+	}
+
 	public function testSelectJoin() {
 		/** @var ManyManyThroughListTest_Object $parent */
 		$parent = $this->objFromFixture(ManyManyThroughListTest_Object::class, 'parent1');
@@ -36,19 +45,53 @@ class ManyManyThroughListTest extends SapphireTest
 		$this->assertNotNull($item1);
 		$this->assertNotNull($item1->getJoin());
 		$this->assertEquals('join 1', $item1->getJoin()->Title);
+		$this->assertInstanceOf(
+			ManyManyThroughListTest_JoinObject::class,
+			$item1->ManyManyThroughListTest_JoinObject
+		);
+		$this->assertEquals('join 1', $item1->ManyManyThroughListTest_JoinObject->Title);
 
 		// Check filters on list work
 		$item2 = $parent->Items()->filter('Title', 'item 2')->first();
 		$this->assertNotNull($item2);
 		$this->assertNotNull($item2->getJoin());
 		$this->assertEquals('join 2', $item2->getJoin()->Title);
+		$this->assertEquals('join 2', $item2->ManyManyThroughListTest_JoinObject->Title);
 
 		// To filter on join table need to use some raw sql
-		$item2 = $parent->Items()->where(['"Join"."Title"' => 'join 2'])->first();
+		$item2 = $parent->Items()->where(['"ManyManyThroughListTest_JoinObject"."Title"' => 'join 2'])->first();
 		$this->assertNotNull($item2);
 		$this->assertEquals('item 2', $item2->Title);
 		$this->assertNotNull($item2->getJoin());
 		$this->assertEquals('join 2', $item2->getJoin()->Title);
+		$this->assertEquals('join 2', $item2->ManyManyThroughListTest_JoinObject->Title);
+
+		// Test sorting on join table
+		$items = $parent->Items()->sort('"ManyManyThroughListTest_JoinObject"."Sort"');
+		$this->assertDOSEquals(
+			[
+				['Title' => 'item 2'],
+				['Title' => 'item 1'],
+			],
+			$items
+		);
+
+		$items = $parent->Items()->sort('"ManyManyThroughListTest_JoinObject"."Sort" ASC');
+		$this->assertDOSEquals(
+			[
+				['Title' => 'item 1'],
+				['Title' => 'item 2'],
+			],
+			$items
+		);
+		$items = $parent->Items()->sort('"ManyManyThroughListTest_JoinObject"."Title" DESC');
+		$this->assertDOSEquals(
+			[
+				['Title' => 'item 2'],
+				['Title' => 'item 1'],
+			],
+			$items
+		);
 	}
 
 	public function testAdd() {
@@ -63,8 +106,15 @@ class ManyManyThroughListTest extends SapphireTest
 		$newItem = $parent->Items()->filter(['Title' => 'my new item'])->first();
 		$this->assertNotNull($newItem);
 		$this->assertEquals('my new item', $newItem->Title);
-		$this->assertNotNull($newItem->getJoin());
-		$this->assertEquals('new join record', $newItem->getJoin()->Title);
+		$this->assertInstanceOf(
+			ManyManyThroughListTest_JoinObject::class,
+			$newItem->getJoin()
+		);
+		$this->assertInstanceOf(
+			ManyManyThroughListTest_JoinObject::class,
+			$newItem->ManyManyThroughListTest_JoinObject
+		);
+		$this->assertEquals('new join record', $newItem->ManyManyThroughListTest_JoinObject->Title);
 	}
 
 	public function testRemove() {
@@ -146,6 +196,19 @@ class ManyManyThroughListTest extends SapphireTest
 			$liveOwnedObjects
 		);
 	}
+
+	/**
+	 * Test validation
+	 */
+	public function testValidateModelValidatesJoinType() {
+		DataObject::reset();
+		ManyManyThroughListTest_Item::config()->update('db', [
+			'ManyManyThroughListTest_JoinObject' => 'Text'
+		]);
+		$this->setExpectedException(InvalidArgumentException::class);
+		$object = new ManyManyThroughListTest_Object();
+		$object->manyManyComponent('Items');
+	}
 }
 
 /**
@@ -177,7 +240,8 @@ class ManyManyThroughListTest_Object extends DataObject implements TestOnly
 class ManyManyThroughListTest_JoinObject extends DataObject implements TestOnly
 {
 	private static $db = [
-		'Title' => 'Varchar'
+		'Title' => 'Varchar',
+		'Sort' => 'Int',
 	];
 
 	private static $has_one = [

tests/model/ManyManyThroughListTest.yml

@@ -9,10 +9,12 @@ ManyManyThroughListTest_Item:
 ManyManyThroughListTest_JoinObject:
   join1:
     Title: 'join 1'
+    Sort: 4
     Parent: =>ManyManyThroughListTest_Object.parent1
     Child: =>ManyManyThroughListTest_Item.child1
   join2:
     Title: 'join 2'
+    Sort: 2
     Parent: =>ManyManyThroughListTest_Object.parent1
     Child: =>ManyManyThroughListTest_Item.child2
 ManyManyThroughListTest_VersionedObject: