From a9274fc94fae491c3a32c0cc41d5c005b0663e62 Mon Sep 17 00:00:00 2001 From: Chris Penny Date: Wed, 22 Dec 2021 10:47:33 +1300 Subject: [PATCH] Improve support for base and extended classes --- src/RelationshipGraph/Graph.php | 13 +- src/Services/CacheProcessingService.php | 3 +- tests/Mocks/Models/BaseCaredHasManyModel.php | 26 ++++ tests/Mocks/Models/BaseCaredHasOneModel.php | 26 ++++ .../Models/ExtendedCaredHasManyModel.php | 12 ++ .../Mocks/Models/ExtendedCaredHasOneModel.php | 12 ++ tests/Mocks/Pages/ExtendedCaresPage.php | 19 +++ tests/Mocks/Pages/ExtendedTouchedPage.php | 1 + tests/Mocks/Pages/ExtendedTouchesPage.php | 1 + tests/Scenarios/ExtendedCaresTest.php | 119 +++++++++++++++++- tests/Scenarios/ExtendedCaresTest.yml | 26 ++++ 11 files changed, 252 insertions(+), 6 deletions(-) create mode 100644 tests/Mocks/Models/BaseCaredHasManyModel.php create mode 100644 tests/Mocks/Models/BaseCaredHasOneModel.php create mode 100644 tests/Mocks/Models/ExtendedCaredHasManyModel.php create mode 100644 tests/Mocks/Models/ExtendedCaredHasOneModel.php diff --git a/src/RelationshipGraph/Graph.php b/src/RelationshipGraph/Graph.php index 370135c..11ba7cc 100644 --- a/src/RelationshipGraph/Graph.php +++ b/src/RelationshipGraph/Graph.php @@ -11,6 +11,7 @@ use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\DataObject; +use SilverStripe\View\ViewableData; use Terraformers\KeysForCache\Models\CacheKey; class Graph implements Flushable @@ -44,10 +45,18 @@ public static function flush() // phpcs:ignore SlevomatCodingStandard.TypeHints public function getEdgesFrom(string $from): array { + $ancestry = ClassInfo::ancestry($from); + // Base classes that show up in every ancestry array + $disallowList = [ + DataObject::class, + ViewableData::class, + ]; + return array_filter( $this->getEdges(), - static function (Edge $e) use ($from) { - return $e->getFromClassName() === $from; + static function (Edge $e) use ($ancestry, $disallowList) { + return in_array($e->getFromClassName(), $ancestry, true) + && !in_array($e->getFromClassName(), $disallowList, true); } ); } diff --git a/src/Services/CacheProcessingService.php b/src/Services/CacheProcessingService.php index c712939..caf3e6c 100644 --- a/src/Services/CacheProcessingService.php +++ b/src/Services/CacheProcessingService.php @@ -9,6 +9,7 @@ use SilverStripe\ORM\Queries\SQLDelete; use Terraformers\KeysForCache\DataTransferObjects\EdgeUpdateDto; use Terraformers\KeysForCache\Models\CacheKey; +use Terraformers\KeysForCache\RelationshipGraph\Edge; use Terraformers\KeysForCache\RelationshipGraph\Graph; abstract class CacheProcessingService @@ -127,7 +128,7 @@ private function createEdges(DataObject $instance): array } return array_map( - static function ($e) use ($instance) { + static function (Edge $e) use ($instance) { return new EdgeUpdateDto($e, $instance); }, $applicableEdges diff --git a/tests/Mocks/Models/BaseCaredHasManyModel.php b/tests/Mocks/Models/BaseCaredHasManyModel.php new file mode 100644 index 0000000..95c903a --- /dev/null +++ b/tests/Mocks/Models/BaseCaredHasManyModel.php @@ -0,0 +1,26 @@ + 'Varchar', + ]; + + private static array $has_one = [ + 'ExtendedCaresPage' => ExtendedCaresPage::class, + ]; + + private static array $extensions = [ + Versioned::class, + ]; + + private static string $table_name = 'BaseCaredHasManyModel'; + + private static bool $has_cache_key = false; +} diff --git a/tests/Mocks/Models/BaseCaredHasOneModel.php b/tests/Mocks/Models/BaseCaredHasOneModel.php new file mode 100644 index 0000000..e6f66e2 --- /dev/null +++ b/tests/Mocks/Models/BaseCaredHasOneModel.php @@ -0,0 +1,26 @@ + 'Varchar', + ]; + + private static array $has_many = [ + 'ExtendedCaresPages' => ExtendedCaresPage::class, + ]; + + private static array $extensions = [ + Versioned::class, + ]; + + private static string $table_name = 'BaseCaredHasOneModel'; + + private static bool $has_cache_key = false; +} diff --git a/tests/Mocks/Models/ExtendedCaredHasManyModel.php b/tests/Mocks/Models/ExtendedCaredHasManyModel.php new file mode 100644 index 0000000..79cb4f8 --- /dev/null +++ b/tests/Mocks/Models/ExtendedCaredHasManyModel.php @@ -0,0 +1,12 @@ + 'Varchar', + ]; + + private static string $table_name = 'ExtendedCaredHasManyModel'; +} diff --git a/tests/Mocks/Models/ExtendedCaredHasOneModel.php b/tests/Mocks/Models/ExtendedCaredHasOneModel.php new file mode 100644 index 0000000..6bcacbe --- /dev/null +++ b/tests/Mocks/Models/ExtendedCaredHasOneModel.php @@ -0,0 +1,12 @@ + 'Varchar', + ]; + + private static string $table_name = 'ExtendedCaredHasOneModel'; +} diff --git a/tests/Mocks/Pages/ExtendedCaresPage.php b/tests/Mocks/Pages/ExtendedCaresPage.php index 11acbee..bb4266f 100644 --- a/tests/Mocks/Pages/ExtendedCaresPage.php +++ b/tests/Mocks/Pages/ExtendedCaresPage.php @@ -2,6 +2,25 @@ namespace Terraformers\KeysForCache\Tests\Mocks\Pages; +use Terraformers\KeysForCache\Tests\Mocks\Models\BaseCaredHasManyModel; +use Terraformers\KeysForCache\Tests\Mocks\Models\BaseCaredHasOneModel; + class ExtendedCaresPage extends CaresPage { + private static array $has_one = [ + 'BaseCaredHasOneModel' => BaseCaredHasOneModel::class, + ]; + + private static array $has_many = [ + 'BaseCaredHasManyModels' => BaseCaredHasManyModel::class, + ]; + + private static array $cares = [ + 'BaseCaredHasOneModel', + 'BaseCaredHasManyModels', + 'ExtendedCaredHasOneModel', + 'ExtendedCaredHasManyModels', + ]; + + private static string $table_name = 'ExtendedCaresPage'; } diff --git a/tests/Mocks/Pages/ExtendedTouchedPage.php b/tests/Mocks/Pages/ExtendedTouchedPage.php index 0b3ac79..1bf36c1 100644 --- a/tests/Mocks/Pages/ExtendedTouchedPage.php +++ b/tests/Mocks/Pages/ExtendedTouchedPage.php @@ -4,4 +4,5 @@ class ExtendedTouchedPage extends TouchedPage { + private static string $table_name = 'ExtendedTouchedPage'; } diff --git a/tests/Mocks/Pages/ExtendedTouchesPage.php b/tests/Mocks/Pages/ExtendedTouchesPage.php index a031d39..26f1e19 100644 --- a/tests/Mocks/Pages/ExtendedTouchesPage.php +++ b/tests/Mocks/Pages/ExtendedTouchesPage.php @@ -4,4 +4,5 @@ class ExtendedTouchesPage extends TouchesPage { + private static string $table_name = 'ExtendedTouchesPage'; } diff --git a/tests/Scenarios/ExtendedCaresTest.php b/tests/Scenarios/ExtendedCaresTest.php index d4ce98e..efc382b 100644 --- a/tests/Scenarios/ExtendedCaresTest.php +++ b/tests/Scenarios/ExtendedCaresTest.php @@ -6,11 +6,15 @@ use SilverStripe\Dev\SapphireTest; use Terraformers\KeysForCache\RelationshipGraph\Graph; use Terraformers\KeysForCache\Services\ProcessedUpdatesService; +use Terraformers\KeysForCache\Tests\Mocks\Models\BaseCaredHasManyModel; +use Terraformers\KeysForCache\Tests\Mocks\Models\BaseCaredHasOneModel; use Terraformers\KeysForCache\Tests\Mocks\Models\CaredBelongsToModel; use Terraformers\KeysForCache\Tests\Mocks\Models\CaredHasManyModel; use Terraformers\KeysForCache\Tests\Mocks\Models\CaredHasOneModel; use Terraformers\KeysForCache\Tests\Mocks\Models\CaredManyManyModel; use Terraformers\KeysForCache\Tests\Mocks\Models\CaredThroughModel; +use Terraformers\KeysForCache\Tests\Mocks\Models\ExtendedCaredHasManyModel; +use Terraformers\KeysForCache\Tests\Mocks\Models\ExtendedCaredHasOneModel; use Terraformers\KeysForCache\Tests\Mocks\Pages\CaresPage; use Terraformers\KeysForCache\Tests\Mocks\Pages\ExtendedCaresPage; use Terraformers\KeysForCache\Tests\Mocks\Relations\CaresPageCaredThroughModel; @@ -24,6 +28,8 @@ class ExtendedCaresTest extends SapphireTest * @var array */ protected static $extra_dataobjects = [ + BaseCaredHasOneModel::class, + BaseCaredHasManyModel::class, CaresPage::class, CaresPageCaredThroughModel::class, CaredBelongsToModel::class, @@ -32,6 +38,8 @@ class ExtendedCaresTest extends SapphireTest CaredManyManyModel::class, CaredThroughModel::class, ExtendedCaresPage::class, + ExtendedCaredHasOneModel::class, + ExtendedCaredHasManyModel::class, ]; public function testCaresPureHasOne(): void @@ -118,9 +126,6 @@ public function testCaresHasOne(): void $this->assertNotEquals($originalKey, $newKey); } - /** - * This test is currently failing, and is a scenario we expect to support - */ public function testCaresHasMany(): void { // Updates are processed as part of scaffold, so we need to flush before we kick off @@ -144,6 +149,114 @@ public function testCaresHasMany(): void $this->assertNotEquals($originalKey, $newKey); } + /** + * Testing that Base relationships work when the explicit class is used in the relationship + */ + public function testBaseCaredHasOne(): void + { + // Updates are processed as part of scaffold, so we need to flush before we kick off + ProcessedUpdatesService::singleton()->flush(); + + $page = $this->objFromFixture(ExtendedCaresPage::class, 'page2'); + $model = $this->objFromFixture(BaseCaredHasOneModel::class, 'model1'); + + $originalKey = $page->getCacheKey(); + + $this->assertNotNull($originalKey); + $this->assertNotEmpty($originalKey); + + $model->forceChange(); + $model->write(); + + $newKey = $page->getCacheKey(); + + $this->assertNotNull($newKey); + $this->assertNotEmpty($originalKey); + $this->assertNotEquals($originalKey, $newKey); + } + + /** + * Testing that Base relationships work when the explicit class is used in the relationship + */ + public function testBaseCaredHasMany(): void + { + // Updates are processed as part of scaffold, so we need to flush before we kick off + ProcessedUpdatesService::singleton()->flush(); + + $page = $this->objFromFixture(ExtendedCaresPage::class, 'page2'); + $model = $this->objFromFixture(BaseCaredHasManyModel::class, 'model1'); + + $originalKey = $page->getCacheKey(); + + $this->assertNotNull($originalKey); + $this->assertNotEmpty($originalKey); + + $model->forceChange(); + $model->write(); + + $newKey = $page->getCacheKey(); + + $this->assertNotNull($newKey); + $this->assertNotEmpty($originalKey); + $this->assertNotEquals($originalKey, $newKey); + } + + /** + * Now testing that a relationship to a Base class still works when the related object is an extended class + */ + public function testExtendedCaredHasOne(): void + { + // Updates are processed as part of scaffold, so we need to flush before we kick off + ProcessedUpdatesService::singleton()->flush(); + + $page = $this->objFromFixture(ExtendedCaresPage::class, 'page3'); + // This model is defined on an ExtendedCaresPage relationship that is for BaseCaredHasOneModel. This is a valid + // class extension that should also trigger an update + $model = $this->objFromFixture(ExtendedCaredHasOneModel::class, 'model1'); + + $originalKey = $page->getCacheKey(); + + $this->assertNotNull($originalKey); + $this->assertNotEmpty($originalKey); + + $model->forceChange(); + $model->write(); + + $newKey = $page->getCacheKey(); + + $this->assertNotNull($newKey); + $this->assertNotEmpty($originalKey); + $this->assertNotEquals($originalKey, $newKey); + } + + /** + * Now testing that a relationship to a Base class still works when the related object is an extended class + */ + public function testExtendedCaredHasMany(): void + { + // Updates are processed as part of scaffold, so we need to flush before we kick off + ProcessedUpdatesService::singleton()->flush(); + + $page = $this->objFromFixture(ExtendedCaresPage::class, 'page3'); + // This model is defined on an ExtendedCaresPage relationship that is for BaseCaredHasManyModel. This is a valid + // class extension that should also trigger an update + $model = $this->objFromFixture(ExtendedCaredHasManyModel::class, 'model1'); + + $originalKey = $page->getCacheKey(); + + $this->assertNotNull($originalKey); + $this->assertNotEmpty($originalKey); + + $model->forceChange(); + $model->write(); + + $newKey = $page->getCacheKey(); + + $this->assertNotNull($newKey); + $this->assertNotEmpty($originalKey); + $this->assertNotEquals($originalKey, $newKey); + } + protected function tearDown(): void { Injector::inst()->get(Graph::CACHE_KEY)->clear(); diff --git a/tests/Scenarios/ExtendedCaresTest.yml b/tests/Scenarios/ExtendedCaresTest.yml index f40071a..58f9a65 100644 --- a/tests/Scenarios/ExtendedCaresTest.yml +++ b/tests/Scenarios/ExtendedCaresTest.yml @@ -10,6 +10,22 @@ Terraformers\KeysForCache\Tests\Mocks\Models\CaredHasManyModel: model1: Title: Has Many Model 1 +Terraformers\KeysForCache\Tests\Mocks\Models\BaseCaredHasManyModel: + model1: + Title: Base Has Many Model 1 + +Terraformers\KeysForCache\Tests\Mocks\Models\BaseCaredHasOneModel: + model1: + Title: Base Has One Model 1 + +Terraformers\KeysForCache\Tests\Mocks\Models\ExtendedCaredHasManyModel: + model1: + Title: Extended Has Many Model 1 + +Terraformers\KeysForCache\Tests\Mocks\Models\ExtendedCaredHasOneModel: + model1: + Title: Extended Has One Model 1 + Terraformers\KeysForCache\Tests\Mocks\Pages\ExtendedCaresPage: page1: Title: Page 1 @@ -17,3 +33,13 @@ Terraformers\KeysForCache\Tests\Mocks\Pages\ExtendedCaresPage: CaredHasOneModel: =>Terraformers\KeysForCache\Tests\Mocks\Models\CaredHasOneModel.model1 CaredHasManyModels: - =>Terraformers\KeysForCache\Tests\Mocks\Models\CaredHasManyModel.model1 + page2: + Title: Page 2 + BaseCaredHasOneModel: =>Terraformers\KeysForCache\Tests\Mocks\Models\BaseCaredHasOneModel.model1 + BaseCaredHasManyModels: + - =>Terraformers\KeysForCache\Tests\Mocks\Models\BaseCaredHasManyModel.model1 + page3: + Title: Page 3 + BaseCaredHasOneModel: =>Terraformers\KeysForCache\Tests\Mocks\Models\ExtendedCaredHasOneModel.model1 + BaseCaredHasManyModels: + - =>Terraformers\KeysForCache\Tests\Mocks\Models\ExtendedCaredHasManyModel.model1