From 2e73b5eeca692f015d63539322dccf62e25cf72a Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Thu, 22 Jun 2023 11:04:26 +0200 Subject: [PATCH 1/4] Use cached query Fixes https://github.com/silverstripe/silverstripe-framework/issues/10833 --- .../MemberAuthenticator/SessionAuthenticationHandler.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php index c2fbd399c..ef5008ee6 100644 --- a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php @@ -4,6 +4,7 @@ namespace SilverStripe\Security\MemberAuthenticator; use SilverStripe\Control\Controller; use SilverStripe\Control\Cookie; +use SilverStripe\ORM\DataObject; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\Security\AuthenticationHandler; @@ -59,7 +60,7 @@ class SessionAuthenticationHandler implements AuthenticationHandler return null; } /** @var Member $member */ - $member = Member::get()->byID($id); + $member = DataObject::get_by_id("SilverStripe\\Security\\Member", $id); return $member; } From 9391e696bbb4e12f58c52f3335b15b0e157cc2c1 Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Fri, 23 Jun 2023 09:35:34 +0200 Subject: [PATCH 2/4] use Member::class --- .../MemberAuthenticator/SessionAuthenticationHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php index ef5008ee6..a192f7a71 100644 --- a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php @@ -60,7 +60,7 @@ class SessionAuthenticationHandler implements AuthenticationHandler return null; } /** @var Member $member */ - $member = DataObject::get_by_id("SilverStripe\\Security\\Member", $id); + $member = DataObject::get_by_id(Member::class, $id); return $member; } From d24095aba8de6e3328859953f00e1c6a4ac00db3 Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Mon, 24 Jul 2023 16:10:47 +1200 Subject: [PATCH 3/4] FIX Image in summaryfields breaks search --- src/Forms/GridField/GridFieldFilterHeader.php | 2 +- src/ORM/DataObject.php | 63 ++++++++++++++----- tests/php/ORM/DataObjectTest.php | 45 +++++++++++++ tests/php/ORM/DataObjectTest/Player.php | 5 ++ tests/php/ORM/Search/SearchContextTest.php | 11 +++- .../SearchContextTest/NoSearchableFields.php | 23 +++++++ 6 files changed, 133 insertions(+), 16 deletions(-) diff --git a/src/Forms/GridField/GridFieldFilterHeader.php b/src/Forms/GridField/GridFieldFilterHeader.php index a0fff8038..e16e870a2 100755 --- a/src/Forms/GridField/GridFieldFilterHeader.php +++ b/src/Forms/GridField/GridFieldFilterHeader.php @@ -269,7 +269,7 @@ class GridFieldFilterHeader extends AbstractGridFieldComponent implements GridFi sort($searchableFields); sort($summaryFields); // searchable_fields has been explictily defined i.e. searchableFields() is not falling back to summary_fields - if ($searchableFields !== $summaryFields) { + if (!empty($searchableFields) && ($searchableFields !== $summaryFields)) { return true; } // we have fallen back to summary_fields, check they are filterable diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index cdb804206..f17c5d487 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -3724,6 +3724,52 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $this->extend('onAfterBuild'); } + private function getDatabaseBackedField(string $fieldPath): ?string + { + $component = $this; + $fieldParts = []; + $parts = explode('.', $fieldPath ?? ''); + + foreach ($parts as $nextPart) { + if (!$component) { + return null; + } + $fieldParts[] = $nextPart; + + if ($component instanceof Relation || $component instanceof DataList) { + if ($component->hasMethod($nextPart)) { + // If the next part is a method, we don't have a database-backed field. + return null; + } + // The next part could either be a field, or another relation + $singleton = DataObject::singleton($component->dataClass()); + if ($singleton->dbObject($nextPart) instanceof DBField) { + // If the next part is a DBField, we've found the database-backed field. + break; + } + $component = $component->relation($nextPart); + array_shift($parts); + } elseif ($component instanceof DataObject && ($component->dbObject($nextPart) instanceof DBField)) { + // If the next part is a DBField, we've found the database-backed field. + break; + } elseif ($component instanceof DataObject && $component->getRelationType($nextPart) !== null) { + // If it's a last part or only one elemnt of a relation, we don't have a database-backed field. + if (count($parts) === 1) { + return null; + } + $component = $component->$nextPart(); + array_shift($parts); + } elseif (ClassInfo::hasMethod($component, $nextPart)) { + // If the next part is a method, we don't have a database-backed field. + return null; + } else { + return null; + } + } + + return implode('.', $fieldParts) ?: null; + } + /** * Get the default searchable fields for this object, as defined in the * $searchable_fields list. If searchable fields are not defined on the @@ -3742,21 +3788,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $summaryFields = array_keys($this->summaryFields() ?? []); $fields = []; - // remove the custom getters as the search should not include them - $schema = static::getSchema(); if ($summaryFields) { - foreach ($summaryFields as $key => $name) { - $spec = $name; - - // Extract field name in case this is a method called on a field (e.g. "Date.Nice") - if (($fieldPos = strpos($name ?? '', '.')) !== false) { - $name = substr($name ?? '', 0, $fieldPos); - } - - if ($schema->fieldSpec($this, $name)) { - $fields[] = $name; - } elseif ($this->relObject($spec)) { - $fields[] = $spec; + foreach ($summaryFields as $name) { + if ($field = $this->getDatabaseBackedField($name)) { + $fields[] = $field; } } } diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php index 9ec9e1629..ed6c6a199 100644 --- a/tests/php/ORM/DataObjectTest.php +++ b/tests/php/ORM/DataObjectTest.php @@ -25,6 +25,7 @@ use SilverStripe\ORM\Tests\DataObjectTest\TreeNode; use SilverStripe\ORM\ValidationException; use SilverStripe\Security\Member; use SilverStripe\View\ViewableData; +use ReflectionMethod; use stdClass; class DataObjectTest extends SapphireTest @@ -2671,4 +2672,48 @@ class DataObjectTest extends SapphireTest $vals = ['25.25', '50.00', '75.00', '100.50']; $this->assertSame(array_combine($vals ?? [], $vals ?? []), $obj->dbObject('MyEnumWithDots')->enumValues()); } + + public function provideTestGetDatabaseBackedField() + { + return [ + ['Captain.IsRetired', 'Captain.IsRetired'], + ['Captain.ShirtNumber', 'Captain.ShirtNumber'], + ['Captain.FavouriteTeam', null], + ['Captain.FavouriteTeam.Fans', null], + ['Captain.FavouriteTeam.Fans.Count', null], + ['Captain.FavouriteTeam.Title', 'Captain.FavouriteTeam.Title'], + ['Captain.FavouriteTeam.Title.Plain', 'Captain.FavouriteTeam.Title'], + ['Captain.FavouriteTeam.ReturnsNull', null], + ['Captain.FavouriteTeam.MethodDoesNotExist', null], + ['Captain.ReturnsNull', null], + ['Founder.FavouriteTeam.Captain.ShirtNumber', 'Founder.FavouriteTeam.Captain.ShirtNumber'], + ['Founder.FavouriteTeam.Captain.Fans', null], + ['Founder.FavouriteTeam.Captain.Fans.Name.Plain', 'Founder.FavouriteTeam.Captain.Fans.Name'], + ['Founder.FavouriteTeam.Captain.ReturnsNull', null], + ['HasOneRelationship.FavouriteTeam.MyTitle', null], + ['SubTeams.Comments.Name.Plain', 'SubTeams.Comments.Name'], + ['Title', 'Title'], + ['Title.Plain', 'Title'], + ['DatabaseField', 'DatabaseField'], + ['DatabaseField.MethodDoesNotExist', 'DatabaseField'], + ['ReturnsNull', null], + ['DynamicField', null], + ['SubTeams.ParentTeam.Fans', null], + ['SubTeams.ParentTeam.Founder.FoundingTeams', null], + ]; + } + + /** + * @dataProvider provideTestGetDatabaseBackedField + */ + public function testGetDatabaseBackedField(string $fieldPath, $expected) + { + $dataObjectClass = new DataObject(); + $method = new ReflectionMethod($dataObjectClass, 'getDatabaseBackedField'); + $method->setAccessible(true); + $class = new Team([]); + + $databaseBackedField = $method->invokeArgs($class, [$fieldPath]); + $this->assertSame($expected, $databaseBackedField); + } } diff --git a/tests/php/ORM/DataObjectTest/Player.php b/tests/php/ORM/DataObjectTest/Player.php index 0dc5a795f..4aa2f69c7 100644 --- a/tests/php/ORM/DataObjectTest/Player.php +++ b/tests/php/ORM/DataObjectTest/Player.php @@ -37,4 +37,9 @@ class Player extends Member implements TestOnly 'IsRetired', 'ShirtNumber' ]; + + public function ReturnsNull() + { + return null; + } } diff --git a/tests/php/ORM/Search/SearchContextTest.php b/tests/php/ORM/Search/SearchContextTest.php index 8c80a4492..7236709cb 100644 --- a/tests/php/ORM/Search/SearchContextTest.php +++ b/tests/php/ORM/Search/SearchContextTest.php @@ -58,13 +58,22 @@ class SearchContextTest extends SapphireTest $obj = new SearchContextTest\NoSearchableFields(); $summaryFields = $obj->summaryFields(); $expected = []; - foreach ($summaryFields as $field => $label) { + + $expectedSearchableFields = [ + 'Name', + 'Customer.FirstName', + 'HairColor', + 'EyeColor', + ]; + + foreach ($expectedSearchableFields as $field) { $expected[$field] = [ 'title' => $obj->fieldLabel($field), 'filter' => 'PartialMatchFilter', ]; } $this->assertEquals($expected, $obj->searchableFields()); + $this->assertNotEquals($summaryFields, $obj->searchableFields()); } public function testSummaryIncludesDefaultFieldsIfNotDefined() diff --git a/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php b/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php index 39ead8537..515300895 100644 --- a/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php +++ b/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php @@ -4,6 +4,7 @@ namespace SilverStripe\ORM\Tests\Search\SearchContextTest; use SilverStripe\Dev\TestOnly; use SilverStripe\ORM\DataObject; +use SilverStripe\Assets\Image; class NoSearchableFields extends DataObject implements TestOnly { @@ -18,12 +19,34 @@ class NoSearchableFields extends DataObject implements TestOnly private static $has_one = [ 'Customer' => Customer::class, + 'Image' => Image::class, ]; private static $summary_fields = [ 'Name' => 'Custom Label', + 'Customer' => 'Customer', 'Customer.FirstName' => 'Customer', + 'Image.CMSThumbnail' => 'Image', + 'Image.BackLinks' => 'Backlinks', + 'Image.BackLinks.Count' => 'Backlinks', 'HairColor', 'EyeColor', + 'ReturnsNull', + 'DynamicField' ]; + + public function MyName() + { + return 'Class ' . $this->Name; + } + + public function getDynamicField() + { + return 'dynamicfield'; + } + + public function ReturnsNull() + { + return null; + } } From 2a56cc3ea4f75579299cd8039eca33e29e6ee047 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Mon, 7 Aug 2023 11:57:23 +1200 Subject: [PATCH 4/4] MNT Run ORM tests in parallel to other tests (#10901) --- phpunit.xml.dist | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index dbb77b368..f435f9a45 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -8,8 +8,15 @@ Requires PHPUnit ^9 tests/php - + + tests/php + + tests/php/ORM + + + + tests/php/ORM vendor/silverstripe/cms/tests