diff --git a/ORM/DataObject.php b/ORM/DataObject.php index 90ae1ce8a5f..7f533d0d9f3 100644 --- a/ORM/DataObject.php +++ b/ORM/DataObject.php @@ -1935,35 +1935,10 @@ public function belongsToComponent($component, $classOnly = true) { * 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 @@ public function manyMany() { /** * 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 @@ public function getJoin() { * 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; } diff --git a/ORM/DataObjectSchema.php b/ORM/DataObjectSchema.php index 0431143bd68..5c758e3c40e 100644 --- a/ORM/DataObjectSchema.php +++ b/ORM/DataObjectSchema.php @@ -137,6 +137,54 @@ public function baseDataTable($class) { 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 @@ protected function checkManyManyFieldClass($parentClass, $component, $joinClass, ); } + // 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( diff --git a/ORM/ManyManyList.php b/ORM/ManyManyList.php index a2558ee7631..048d036b788 100644 --- a/ORM/ManyManyList.php +++ b/ORM/ManyManyList.php @@ -228,7 +228,7 @@ public function add($item, $extraFields = array()) { // 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 @@ public function removeByID($itemID) { 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 @@ public function getExtraData($componentName, $itemID) { } 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 @@ public function getExtraData($componentName, $itemID) { 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 diff --git a/ORM/ManyManyThroughList.php b/ORM/ManyManyThroughList.php index c9a036ba8f6..32b662c1cf7 100644 --- a/ORM/ManyManyThroughList.php +++ b/ORM/ManyManyThroughList.php @@ -50,7 +50,8 @@ protected function foreignIDFilter($id = null) { 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 @@ public function createDataObject($row) { $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 @@ public function add($item, $extraFields = []) { // 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 diff --git a/ORM/ManyManyThroughQueryManipulator.php b/ORM/ManyManyThroughQueryManipulator.php index 6b93a5619d0..073000350d3 100644 --- a/ORM/ManyManyThroughQueryManipulator.php +++ b/ORM/ManyManyThroughQueryManipulator.php @@ -5,7 +5,6 @@ use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; -use SilverStripe\Dev\Debug; use SilverStripe\ORM\Queries\SQLSelect; /** @@ -13,10 +12,6 @@ */ class ManyManyThroughQueryManipulator implements DataQueryManipulator { - /** - * Alias to use for sql join table - */ - const JOIN_TABLE_ALIAS = 'Join'; use Injectable; @@ -152,6 +147,15 @@ public function extractInheritableQueryParameters(DataQuery $query) { 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 @@ public function beforeGetFinalisedQuery(DataQuery $dataQuery, $queriedColumns = $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(); diff --git a/docs/en/02_Developer_Guides/00_Model/02_Relations.md b/docs/en/02_Developer_Guides/00_Model/02_Relations.md index 5a1ad108920..54299d5804e 100644 --- a/docs/en/02_Developer_Guides/00_Model/02_Relations.md +++ b/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. diff --git a/tests/model/ManyManyThroughListTest.php b/tests/model/ManyManyThroughListTest.php index 0127a474a5d..fb97d336267 100644 --- a/tests/model/ManyManyThroughListTest.php +++ b/tests/model/ManyManyThroughListTest.php @@ -4,7 +4,6 @@ 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 @@ public function testSelectJoin() { $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 @@ public function testAdd() { $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 @@ public function testPublishing() { $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 = [ diff --git a/tests/model/ManyManyThroughListTest.yml b/tests/model/ManyManyThroughListTest.yml index 4a2bef9985b..c446a2451f6 100644 --- a/tests/model/ManyManyThroughListTest.yml +++ b/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: