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 diff --git a/src/Core/Manifest/ClassManifest.php b/src/Core/Manifest/ClassManifest.php index 9d406620d..0e2887635 100644 --- a/src/Core/Manifest/ClassManifest.php +++ b/src/Core/Manifest/ClassManifest.php @@ -650,6 +650,11 @@ class ClassManifest } $this->children[$lowerAncestor][$lowerClassName] = $className; } + + // If the class extends a core class, add class to roots + if (strpos($ancestor, 'SilverStripe\\Control') === 0) { + $this->roots[$lowerAncestor] = $ancestor; + } } else { $this->roots[$lowerClassName] = $className; } diff --git a/src/Core/Manifest/VersionProvider.php b/src/Core/Manifest/VersionProvider.php index 503ed4486..75339b4ba 100644 --- a/src/Core/Manifest/VersionProvider.php +++ b/src/Core/Manifest/VersionProvider.php @@ -44,7 +44,7 @@ class VersionProvider */ public function getVersion() { - $key = sprintf('%s-%s', $this->getComposerLockPath(), 'all'); + $key = preg_replace("/[^A-Za-z0-9]/", '_', $this->getComposerLockPath() . '_all'); $version = $this->getCachedValue($key); if ($version) { return $version; @@ -80,7 +80,7 @@ class VersionProvider */ public function getModuleVersion(string $module): string { - $key = sprintf('%s-%s', $this->getComposerLockPath(), $module); + $key = preg_replace("/[^A-Za-z0-9]/", '_', $this->getComposerLockPath() . '_' . $module); $version = $this->getCachedValue($key); if ($version) { return $version; diff --git a/src/Forms/FieldsValidator.php b/src/Forms/FieldsValidator.php new file mode 100644 index 000000000..0de96225f --- /dev/null +++ b/src/Forms/FieldsValidator.php @@ -0,0 +1,26 @@ +form->Fields(); + + foreach ($fields as $field) { + $valid = ($field->validate($this) && $valid); + } + + return $valid; + } + + public function canBeCached(): bool + { + return true; + } +} diff --git a/src/Forms/GridField/GridFieldFilterHeader.php b/src/Forms/GridField/GridFieldFilterHeader.php index 9a09c3664..187b9bd4f 100755 --- a/src/Forms/GridField/GridFieldFilterHeader.php +++ b/src/Forms/GridField/GridFieldFilterHeader.php @@ -214,7 +214,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/Forms/HTMLEditor/HTMLEditorField.php b/src/Forms/HTMLEditor/HTMLEditorField.php index 9075a5895..63ef950c2 100644 --- a/src/Forms/HTMLEditor/HTMLEditorField.php +++ b/src/Forms/HTMLEditor/HTMLEditorField.php @@ -190,4 +190,14 @@ class HTMLEditorField extends TextareaField $stateDefaults['data'] = $config->getConfigSchemaData(); return $stateDefaults; } + + /** + * Return value with all values encoded in html entities + * + * @return string Raw HTML + */ + public function ValueEntities() + { + return htmlentities($this->Value() ?? '', ENT_COMPAT, 'UTF-8', false); + } } diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 8f97eba51..948800b71 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -16,6 +16,7 @@ use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormField; use SilverStripe\Forms\FormScaffolder; use SilverStripe\Forms\CompositeValidator; +use SilverStripe\Forms\FieldsValidator; use SilverStripe\Forms\HiddenField; use SilverStripe\i18n\i18n; use SilverStripe\i18n\i18nEntityProvider; @@ -2574,7 +2575,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ public function getCMSCompositeValidator(): CompositeValidator { - $compositeValidator = CompositeValidator::create(); + $compositeValidator = CompositeValidator::create([FieldsValidator::create()]); // Support for the old method during the deprecation period if ($this->hasMethod('getCMSValidator')) { @@ -3721,6 +3722,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 @@ -3739,21 +3786,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/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php index 97ad8e6ab..8036f6039 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\Control\Session; @@ -62,7 +63,7 @@ class SessionAuthenticationHandler implements AuthenticationHandler return null; } /** @var Member $member */ - $member = Member::get()->byID($id); + $member = DataObject::get_by_id(Member::class, $id); return $member; } diff --git a/src/View/SSViewer_DataPresenter.php b/src/View/SSViewer_DataPresenter.php index b808773a3..9a65d01fa 100644 --- a/src/View/SSViewer_DataPresenter.php +++ b/src/View/SSViewer_DataPresenter.php @@ -377,7 +377,7 @@ class SSViewer_DataPresenter extends SSViewer_Scope // Check if the method to-be-called exists on the target object - if so, don't check any further // injection locations $on = $this->itemIterator ? $this->itemIterator->current() : $this->item; - if ($on !== null && (isset($on->$property) || method_exists($on, $property ?? ''))) { + if (is_object($on) && (isset($on->$property) || method_exists($on, $property ?? ''))) { return []; } diff --git a/tests/behat/src/CmsUiContext.php b/tests/behat/src/CmsUiContext.php index a0a5bd4e1..125bce5d6 100644 --- a/tests/behat/src/CmsUiContext.php +++ b/tests/behat/src/CmsUiContext.php @@ -6,6 +6,7 @@ use Behat\Behat\Context\Context; use Behat\Behat\Hook\Scope\AfterStepScope; use Behat\Mink\Element\Element; use Behat\Mink\Element\NodeElement; +use Behat\Mink\Exception\ElementNotFoundException; use Behat\Mink\Selector\Xpath\Escaper; use Behat\Mink\Session; use PHPUnit\Framework\Assert; @@ -92,26 +93,37 @@ class CmsUiContext implements Context } /** - * @Then /^I should see a "(.+)" (\w+) toast$/ + * @Then /^I should (not |)see a "(.+)" (\w+) toast$/ */ - public function iShouldSeeAToast($notice, $type) + public function iShouldSeeAToast($not, $notice, $type) { - $this->getMainContext()->assertElementContains('.toast--' . $type, $notice); + if ($not) { + try { + // If there is a toast of that type, ensure it doesn't contain the notice. + $this->getMainContext()->assertElementNotContains('.toast--' . $type, $notice); + } catch (ElementNotFoundException $e) { + // no-op - if the element doesn't exist at all, then that passes the test. + } + } else { + $this->getMainContext()->assertElementContains('.toast--' . $type, $notice); + } } /** - * @Then /^I should see a "(.+)" (\w+) toast with these actions: (.+)$/ + * @Then /^I should (not |)see a "(.+)" (\w+) toast with these actions: (.+)$/ */ - public function iShouldSeeAToastWithAction($notice, $type, $actions) + public function iShouldSeeAToastWithAction($not, $notice, $type, $actions) { - $this->iShouldSeeAToast($notice, $type); + $this->iShouldSeeAToast($not, $notice, $type); - $actions = explode(',', $actions ?? ''); - foreach ($actions as $order => $action) { - $this->getMainContext()->assertElementContains( - sprintf('.toast--%s .toast__action:nth-child(%s)', $type, $order+1), - trim($action ?? '') - ); + if (!$not) { + $actions = explode(',', $actions ?? ''); + foreach ($actions as $order => $action) { + $this->getMainContext()->assertElementContains( + sprintf('.toast--%s .toast__action:nth-child(%s)', $type, $order+1), + trim($action ?? '') + ); + } } } diff --git a/tests/php/Core/ClassInfoTest.php b/tests/php/Core/ClassInfoTest.php index a1d353483..7528c3f36 100644 --- a/tests/php/Core/ClassInfoTest.php +++ b/tests/php/Core/ClassInfoTest.php @@ -87,6 +87,14 @@ class ClassInfoTest extends SapphireTest $subclassesWithoutBase, ClassInfo::subclassesFor('silverstripe\\core\\tests\\classinfotest\\baseclass', false) ); + + // Check that core classes are present (eg: Email subclasses) + $emailClasses = ClassInfo::subclassesFor(\SilverStripe\Control\Email\Email::class); + $this->assertArrayHasKey( + 'silverstripe\\control\\tests\\email\\emailtest\\emailsubclass', + $emailClasses, + 'It contains : ' . json_encode($emailClasses) + ); } public function testClassName() diff --git a/tests/php/Forms/FieldsValidatorTest.php b/tests/php/Forms/FieldsValidatorTest.php new file mode 100644 index 000000000..8c958bcd3 --- /dev/null +++ b/tests/php/Forms/FieldsValidatorTest.php @@ -0,0 +1,79 @@ + [ + 'values' => [], + 'isValid' => true, + ], + 'empty values arent invalid' => [ + 'values' => [ + 'EmailField1' => '', + 'EmailField2' => null, + ], + 'isValid' => true, + ], + 'any invalid is invalid' => [ + 'values' => [ + 'EmailField1' => 'email@example.com', + 'EmailField2' => 'not email', + ], + 'isValid' => false, + ], + 'all invalid is invalid' => [ + 'values' => [ + 'EmailField1' => 'not email', + 'EmailField2' => 'not email', + ], + 'isValid' => false, + ], + 'all valid is valid' => [ + 'values' => [ + 'EmailField1' => 'email@example.com', + 'EmailField2' => 'email@example.com', + ], + 'isValid' => true, + ], + ]; + } + + /** + * @dataProvider provideValidation + */ + public function testValidation(array $values, bool $isValid) + { + $fieldList = new FieldList([ + $field1 = new EmailField('EmailField1'), + $field2 = new EmailField('EmailField2'), + ]); + if (array_key_exists('EmailField1', $values)) { + $field1->setValue($values['EmailField1']); + } + if (array_key_exists('EmailField2', $values)) { + $field2->setValue($values['EmailField2']); + } + $form = new Form(null, 'testForm', $fieldList, new FieldList([/* no actions */]), new FieldsValidator()); + + $result = $form->validationResult(); + $this->assertSame($isValid, $result->isValid()); + $messages = $result->getMessages(); + if ($isValid) { + $this->assertEmpty($messages); + } else { + $this->assertNotEmpty($messages); + } + } +} diff --git a/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php b/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php index 8fadaa8c6..68ff44b51 100644 --- a/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php +++ b/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php @@ -208,4 +208,25 @@ EOS $readonlyContent->getValue() ); } + + public function testValueEntities() + { + $inputText = "The company & partners"; + $field = new HTMLEditorField("Content"); + $field->setValue($inputText); + + $this->assertEquals( + "The company & partners", + $field->obj('ValueEntities')->forTemplate() + ); + + $inputText = "The company && partners"; + $field = new HTMLEditorField("Content"); + $field->setValue($inputText); + + $this->assertEquals( + "The company && partners", + $field->obj('ValueEntities')->forTemplate() + ); + } } diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php index 2fccf4405..8d65c03db 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 @@ -2681,4 +2682,48 @@ class DataObjectTest extends SapphireTest // Getter overrides it with all upper case $this->assertSame('SOME VALUE', $obj->MyTestField); } + + 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; + } } diff --git a/tests/php/Security/GroupTest.php b/tests/php/Security/GroupTest.php index 76f627074..b4b0af7c8 100644 --- a/tests/php/Security/GroupTest.php +++ b/tests/php/Security/GroupTest.php @@ -287,10 +287,11 @@ class GroupTest extends FunctionalTest $newGroup = new Group(); - $validators = $newGroup->getCMSCompositeValidator()->getValidators(); + $validators = $newGroup->getCMSCompositeValidator()->getValidatorsByType(RequiredFields::class); $this->assertCount(1, $validators); - $this->assertInstanceOf(RequiredFields::class, $validators[0]); - $this->assertTrue(in_array('Title', $validators[0]->getRequired() ?? [])); + $validator = array_shift($validators); + $this->assertInstanceOf(RequiredFields::class, $validator); + $this->assertTrue(in_array('Title', $validator->getRequired() ?? [])); $newGroup->Title = $group1->Title; $result = $newGroup->validate(); diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php index da4fc3c49..487153e02 100644 --- a/tests/php/View/SSViewerTest.php +++ b/tests/php/View/SSViewerTest.php @@ -1069,22 +1069,22 @@ after' { $this->assertEquals( $this->render('<% include SSViewerTestIncludeWithArguments %>'), - '

[out:Arg1]

[out:Arg2]

' + '

[out:Arg1]

[out:Arg2]

[out:Arg2.Count]

' ); $this->assertEquals( $this->render('<% include SSViewerTestIncludeWithArguments Arg1=A %>'), - '

A

[out:Arg2]

' + '

A

[out:Arg2]

[out:Arg2.Count]

' ); $this->assertEquals( $this->render('<% include SSViewerTestIncludeWithArguments Arg1=A, Arg2=B %>'), - '

A

B

' + '

A

B

' ); $this->assertEquals( $this->render('<% include SSViewerTestIncludeWithArguments Arg1=A Bare String, Arg2=B Bare String %>'), - '

A Bare String

B Bare String

' + '

A Bare String

B Bare String

' ); $this->assertEquals( @@ -1092,7 +1092,7 @@ after' '<% include SSViewerTestIncludeWithArguments Arg1="A", Arg2=$B %>', new ArrayData(['B' => 'Bar']) ), - '

A

Bar

' + '

A

Bar

' ); $this->assertEquals( @@ -1100,7 +1100,22 @@ after' '<% include SSViewerTestIncludeWithArguments Arg1="A" %>', new ArrayData(['Arg1' => 'Foo', 'Arg2' => 'Bar']) ), - '

A

Bar

' + '

A

Bar

' + ); + + $this->assertEquals( + $this->render('<% include SSViewerTestIncludeWithArguments Arg1="A", Arg2=0 %>'), + '

A

0

' + ); + + $this->assertEquals( + $this->render('<% include SSViewerTestIncludeWithArguments Arg1="A", Arg2=false %>'), + '

A

' + ); + + $this->assertEquals( + $this->render('<% include SSViewerTestIncludeWithArguments Arg1="A", Arg2=null %>'), + '

A

' ); $this->assertEquals( diff --git a/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestIncludeWithArguments.ss b/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestIncludeWithArguments.ss index 385eb7abf..73004e1b7 100644 --- a/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestIncludeWithArguments.ss +++ b/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestIncludeWithArguments.ss @@ -1 +1 @@ -

$Arg1

$Arg2

+

$Arg1

$Arg2

{$Arg2.Count}