diff --git a/src/Handler/Elemental/ArchiveElementHandler.php b/src/Handler/Elemental/ArchiveElementHandler.php index 187beef..0610c0d 100644 --- a/src/Handler/Elemental/ArchiveElementHandler.php +++ b/src/Handler/Elemental/ArchiveElementHandler.php @@ -42,7 +42,7 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot return null; } - $archivedBlock = SnapshotPublishable::get_at_last_snapshot(BaseElement::class, $blockID); + $archivedBlock = SnapshotPublishable::singleton()->getAtLastSnapshotByClassAndId(BaseElement::class, $blockID); if (!$archivedBlock) { return null; diff --git a/src/Handler/Elemental/ModifyElementHandler.php b/src/Handler/Elemental/ModifyElementHandler.php index 5d01ee1..4274c39 100644 --- a/src/Handler/Elemental/ModifyElementHandler.php +++ b/src/Handler/Elemental/ModifyElementHandler.php @@ -50,7 +50,7 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot } foreach ($snapshot->Items() as $item) { - if (!static::hashSnapshotCompare($item->getItem(), $block)) { + if (!$this->hashSnapshotCompare($item->getItem(), $block)) { continue; } diff --git a/src/Handler/Form/UnpublishHandler.php b/src/Handler/Form/UnpublishHandler.php index 4aa5898..d8099bc 100644 --- a/src/Handler/Form/UnpublishHandler.php +++ b/src/Handler/Form/UnpublishHandler.php @@ -26,7 +26,7 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot } foreach ($snapshot->Items() as $item) { - if (!static::hashSnapshotCompare($item->getItem(), $record)) { + if (!$this->hashSnapshotCompare($item->getItem(), $record)) { continue; } diff --git a/src/Snapshot.php b/src/Snapshot.php index ba147e5..e6088f0 100644 --- a/src/Snapshot.php +++ b/src/Snapshot.php @@ -207,7 +207,7 @@ public function getIsLiveSnapshot(): bool return false; } - $liveVersionNumber = SnapshotPublishable::get_published_version_number( + $liveVersionNumber = SnapshotPublishable::singleton()->getPublishedVersionNumber( $originVersion->baseClass(), $originVersion->ID ); @@ -217,7 +217,7 @@ public function getIsLiveSnapshot(): bool ->setFrom("\"$table\"") ->addWhere([ "\"$table\".\"Version\" = ?" => $liveVersionNumber, - "\"$table\".\"ObjectHash\" = ?" => static::hashObjectForSnapshot($originVersion), + "\"$table\".\"ObjectHash\" = ?" => $this->hashObjectForSnapshot($originVersion), ]) ->execute() ->value(); @@ -283,7 +283,7 @@ public function onBeforeWrite(): void { parent::onBeforeWrite(); - $this->OriginHash = static::hashForSnapshot($this->OriginClass, $this->OriginID); + $this->OriginHash = $this->hashForSnapshot($this->OriginClass, $this->OriginID); } /** diff --git a/src/SnapshotHasher.php b/src/SnapshotHasher.php index 1fc5657..87abc34 100644 --- a/src/SnapshotHasher.php +++ b/src/SnapshotHasher.php @@ -16,7 +16,7 @@ trait SnapshotHasher * @param int|null $id * @return string */ - public static function hashForSnapshot(?string $class, ?int $id): string + public function hashForSnapshot(?string $class, ?int $id): string { return md5(sprintf('%s:%s', $class, $id)); } @@ -27,9 +27,9 @@ public static function hashForSnapshot(?string $class, ?int $id): string * @param DataObject $obj * @return string */ - public static function hashObjectForSnapshot(DataObject $obj): string + public function hashObjectForSnapshot(DataObject $obj): string { - return static::hashForSnapshot($obj->baseClass(), $obj->ID); + return $this->hashForSnapshot($obj->baseClass(), $obj->ID); } /** @@ -37,8 +37,8 @@ public static function hashObjectForSnapshot(DataObject $obj): string * @param DataObject $obj2 * @return bool */ - public static function hashSnapshotCompare(DataObject $obj1, DataObject $obj2): bool + public function hashSnapshotCompare(DataObject $obj1, DataObject $obj2): bool { - return static::hashObjectForSnapshot($obj1) === static::hashObjectForSnapshot($obj2); + return $this->hashObjectForSnapshot($obj1) === $this->hashObjectForSnapshot($obj2); } } diff --git a/src/SnapshotItem.php b/src/SnapshotItem.php index 81a6590..184b6b6 100644 --- a/src/SnapshotItem.php +++ b/src/SnapshotItem.php @@ -166,7 +166,7 @@ public function onBeforeWrite() { parent::onBeforeWrite(); - $this->ObjectHash = static::hashForSnapshot($this->ObjectClass, $this->ObjectID); + $this->ObjectHash = $this->hashForSnapshot($this->ObjectClass, $this->ObjectID); } /** @@ -217,7 +217,7 @@ public function hydrateFromDataObject(DataObject $object): self } else { // Track publish state for non-versioned owners, they're always in a published state. $exists = SnapshotItem::get()->filter([ - 'ObjectHash' => static::hashObjectForSnapshot($object) + 'ObjectHash' => $this->hashObjectForSnapshot($object) ]); $this->WasCreated = !$exists->exists(); $this->WasPublished = true; diff --git a/src/SnapshotPublishable.php b/src/SnapshotPublishable.php index f5aa5a6..931853f 100644 --- a/src/SnapshotPublishable.php +++ b/src/SnapshotPublishable.php @@ -4,7 +4,9 @@ use Exception; use InvalidArgumentException; +use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Core\Resettable; use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; @@ -18,9 +20,10 @@ * * @property DataObject|SnapshotPublishable|Versioned $owner */ -class SnapshotPublishable extends RecursivePublishable +class SnapshotPublishable extends RecursivePublishable implements Resettable { + use Injectable; use SnapshotHasher; /** @@ -32,7 +35,17 @@ class SnapshotPublishable extends RecursivePublishable /** * @var array */ - private static $relationDiffs = []; + private $relationDiffs = []; + + public function flushCachedData(): void + { + $this->relationDiffs = []; + } + + public static function reset() + { + static::singleton()->flushCachedData(); + } /** * A more resilient wrapper for the Versioned function that holds up against un-staged versioned @@ -42,7 +55,7 @@ class SnapshotPublishable extends RecursivePublishable * @param int $id * @return int|null */ - public static function get_published_version_number(string $class, int $id): ?int + public function getPublishedVersionNumber(string $class, int $id): ?int { $inst = DataObject::singleton($class); @@ -68,7 +81,7 @@ public static function get_published_version_number(string $class, int $id): ?in * @param string|int $snapshot A snapshot ID or a Y-m-d h:i:s date formatted string * @return DataObject|null */ - public static function get_at_snapshot(string $class, int $id, $snapshot): ?DataObject + public function getAtSnapshotByClassAndId(string $class, int $id, $snapshot): ?DataObject { $baseClass = DataObject::getSchema()->baseDataClass($class); @@ -94,16 +107,16 @@ public static function get_at_snapshot(string $class, int $id, $snapshot): ?Data return $list->byID($id); } - public static function get_at_last_snapshot(string $class, int $id): ?DataObject + public function getAtLastSnapshotByClassAndId(string $class, int $id): ?DataObject { /** @var SnapshotItem $lastItem */ - $lastItem = static::get_last_snapshot_item($class, $id); + $lastItem = $this->getLastSnapshotItemByClassAndId($class, $id); if (!$lastItem) { return null; } - return static::get_at_snapshot($class, $id, $lastItem->SnapshotID); + return $this->getAtSnapshotByClassAndId($class, $id, $lastItem->SnapshotID); } /** @@ -111,17 +124,17 @@ public static function get_at_last_snapshot(string $class, int $id): ?DataObject * @param int $id * @return DataObject|null */ - public static function get_last_snapshot_item(string $class, int $id): ?DataObject + public function getLastSnapshotItemByClassAndId(string $class, int $id): ?DataObject { return SnapshotItem::get() ->sort('Created', 'DESC') - ->find('ObjectHash', static::hashForSnapshot($class, $id)); + ->find('ObjectHash', $this->hashForSnapshot($class, $id)); } /** * @return DataList */ - public static function getSnapshots(): DataList + public function getSnapshots(): DataList { $snapshotTable = DataObject::getSchema()->tableName(Snapshot::class); $itemTable = DataObject::getSchema()->tableName(SnapshotItem::class); @@ -138,7 +151,7 @@ public function getRelevantSnapshots(): DataList $itemTable = DataObject::getSchema()->tableName(SnapshotItem::class); $snapshots = $this->owner->getSnapshots() ->where([ - ["\"$itemTable\".\"ObjectHash\" = ?" => static::hashObjectForSnapshot($this->owner)] + ["\"$itemTable\".\"ObjectHash\" = ?" => $this->hashObjectForSnapshot($this->owner)] ]); $this->owner->extend('updateRelevantSnapshots', $snapshots); @@ -177,7 +190,7 @@ public function getSnapshotsSinceVersion($sinceVersion): DataList ), 0)", $itemTable ) => [ - static::hashObjectForSnapshot($this->owner), + $this->hashObjectForSnapshot($this->owner), $sinceVersion, ], ], @@ -194,7 +207,7 @@ public function getSnapshotsSinceLastPublish(): DataList { $class = $this->owner->baseClass(); $id = $this->owner->ID; - $publishedVersion = static::get_published_version_number($class, $id); + $publishedVersion = $this->getPublishedVersionNumber($class, $id); return $this->owner->getSnapshotsSinceVersion($publishedVersion); } @@ -212,7 +225,7 @@ protected function getSnapshotsBetweenVersionsFilters(int $min, ?int $max = null { $itemTable = DataObject::getSchema()->tableName(SnapshotItem::class); - $hash = static::hashObjectForSnapshot($this->owner); + $hash = $this->hashObjectForSnapshot($this->owner); $minShot = SQLSelect::create( "MIN(\"$itemTable\".\"SnapshotID\")", "\"$itemTable\"", @@ -270,7 +283,7 @@ public function hasOwnedModifications(): bool $class = $this->owner->baseClass(); $id = $this->owner->ID; - $minVersion = static::get_published_version_number($class, $id); + $minVersion = $this->getPublishedVersionNumber($class, $id); if (is_null($minVersion)) { return false; //Draft page. @@ -319,7 +332,7 @@ public function getPublishableObjects(): ArrayList $id = $row['ObjectID']; /** @var DataObject|SnapshotPublishable $obj */ $obj = DataObject::get_by_id($class, $id); - $map[static::hashObjectForSnapshot($obj)] = $obj; + $map[$this->hashObjectForSnapshot($obj)] = $obj; } return ArrayList::create(array_values($map)); @@ -352,7 +365,7 @@ public function getRelationTracking(): array */ public function getAtSnapshot($snapshot): ?DataObject { - return static::get_at_snapshot($this->owner->baseClass(), $this->owner->ID, $snapshot); + return $this->getAtSnapshotByClassAndId($this->owner->baseClass(), $this->owner->ID, $snapshot); } /** @@ -366,7 +379,7 @@ public function getAtLastSnapshot(): ?DataObject return null; } - return static::get_at_snapshot($this->owner->baseClass(), $this->owner->ID, $lastItem->SnapshotID); + return $this->getAtSnapshotByClassAndId($this->owner->baseClass(), $this->owner->ID, $lastItem->SnapshotID); } /** @@ -377,7 +390,7 @@ public function onAfterRevertToLive(): void { $snapshots = $this->getSnapshotsSinceVersion($this->owner->Version) ->filter([ - 'OriginHash' => static::hashObjectForSnapshot($this->owner), + 'OriginHash' => $this->hashObjectForSnapshot($this->owner), ]); $snapshots->removeAll(); @@ -390,7 +403,7 @@ public function getPreviousSnapshotItem(): ?DataObject { return SnapshotItem::get() ->sort('Created', 'DESC') - ->find('ObjectHash', static::hashObjectForSnapshot($this->owner)); + ->find('ObjectHash', $this->hashObjectForSnapshot($this->owner)); } /** @@ -403,7 +416,7 @@ public function atPreviousSnapshot(callable $callback) // timestamp prior to now, because the Version may be unchanged. $lastSnapshot = SnapshotItem::get() ->filter([ - 'ObjectHash' => static::hashObjectForSnapshot($this->owner), + 'ObjectHash' => $this->hashObjectForSnapshot($this->owner), ]) ->max('LastEdited'); @@ -440,12 +453,27 @@ public function isModifiedSinceLastSnapshot(): bool } /** + * @param bool $cache * @return RelationDiffer[] * @throws Exception * TODO Memoise / cache / enable cache once it's confirmed that this feature is needed */ - public function getRelationDiffs(): array + public function getRelationDiffs(bool $cache = true): array { + $cacheKey = $this->owner->isInDB() + ? sprintf( + '%s-%s', + $this->owner->getUniqueKey(), + $this->hashObjectForSnapshot($this->owner) + ) + : ''; + // TODO in-memory cache disabled until we can confirm that we need it + $cacheKey = ''; + + if ($cache && $cacheKey && array_key_exists($cacheKey, $this->relationDiffs)) { + return $this->relationDiffs[$cacheKey]; + } + $diffs = []; $previousTracking = $this->owner->atPreviousSnapshot(function ($date) { if (!$date) { @@ -471,7 +499,9 @@ public function getRelationDiffs(): array $diffs[] = RelationDiffer::create($class, $type, $prevMap, $currentMap); } - static::$relationDiffs[static::hashObjectForSnapshot($this->owner)] = $diffs; + if ($cacheKey) { + $this->relationDiffs[$cacheKey] = $diffs; + } return $diffs; } @@ -624,7 +654,7 @@ public function reconcileOwnershipChanges(?DataObject $previous = null): void $currentOwner = $spec['current']; $currentOwners = array_merge([$currentOwner], $currentOwner->findOwners()->toArray()); - $previousHashes = array_map([static::class, 'hashObjectForSnapshot'], $previousOwners); + $previousHashes = array_map([$this, 'hashObjectForSnapshot'], $previousOwners); // Get the earliest snapshot where the previous owner was published. $cutoff = $previousOwner->getSnapshotsSinceLastPublish() @@ -684,7 +714,7 @@ public function getIntermediaryObjects(): array $extraObjects = []; foreach ($intermediaryObjects as $extra) { - $extraObjects[SnapshotHasher::hashObjectForSnapshot($extra)] = $extra; + $extraObjects[$this->hashObjectForSnapshot($extra)] = $extra; } return $extraObjects; @@ -732,7 +762,7 @@ public function getActivityFeed(?int $minVersion = null, ?int $maxVersion = null if (is_null($minVersion)) { $class = $this->owner->baseClass(); $id = $this->owner->ID; - $minVersion = static::get_published_version_number($class, $id); + $minVersion = $this->getPublishedVersionNumber($class, $id); if (is_null($minVersion)) { $minVersion = 1; diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index 6b0f054..9fddd09 100644 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -82,6 +82,7 @@ protected function setUp(): void /** * @throws ValidationException * @throws Exception + * @group ttt */ public function testFundamentals(): void { @@ -1489,11 +1490,11 @@ private function assertActivityContains(ArrayList $activity, array $objs = []): } $expectedHash = $obj->isInDB() - ? SnapshotPublishable::hashObjectForSnapshot($obj) - : SnapshotPublishable::hashForSnapshot($obj->ClassName, $obj->OldID); + ? SnapshotPublishable::singleton()->hashObjectForSnapshot($obj) + : SnapshotPublishable::singleton()->hashForSnapshot($obj->ClassName, $obj->OldID); $this->assertEquals( $expectedHash, - SnapshotPublishable::hashObjectForSnapshot($entry->Subject) + SnapshotPublishable::singleton()->hashObjectForSnapshot($entry->Subject) ); $this->assertEquals($action, $entry->Action); } @@ -1512,11 +1513,11 @@ private function assertPublishableObjectsContains(ArrayList $items, array $objs $obj= $objs[$i]; $expectedHash = $obj->isInDB() - ? SnapshotPublishable::hashObjectForSnapshot($obj) - : SnapshotPublishable::hashForSnapshot($obj->ClassName, $obj->OldID); + ? SnapshotPublishable::singleton()->hashObjectForSnapshot($obj) + : SnapshotPublishable::singleton()->hashForSnapshot($obj->ClassName, $obj->OldID); $this->assertEquals( $expectedHash, - SnapshotPublishable::hashObjectForSnapshot($dataObject) + SnapshotPublishable::singleton()->hashObjectForSnapshot($dataObject) ); } } diff --git a/tests/SnapshotItemTest.php b/tests/SnapshotItemTest.php index e57eb65..c33e5ba 100644 --- a/tests/SnapshotItemTest.php +++ b/tests/SnapshotItemTest.php @@ -4,8 +4,8 @@ use Exception; use SilverStripe\ORM\ValidationException; -use SilverStripe\Snapshots\SnapshotHasher; use SilverStripe\Snapshots\SnapshotItem; +use SilverStripe\Snapshots\SnapshotPublishable; use SilverStripe\Snapshots\Tests\SnapshotTest\Block; use SilverStripe\Versioned\Versioned; @@ -49,7 +49,7 @@ public function testHydration(): void $this->assertEquals(Block::class, $item->ObjectClass); $this->assertEquals($block->ID, $item->ObjectID); $this->assertEquals($block->Version, $item->Version); - $this->assertEquals(SnapshotHasher::hashObjectForSnapshot($block), $item->ObjectHash); + $this->assertEquals(SnapshotPublishable::singleton()->hashObjectForSnapshot($block), $item->ObjectHash); $this->assertTrue($item->WasDraft); $this->assertFalse($item->WasDeleted); diff --git a/tests/SnapshotPublishableTest.php b/tests/SnapshotPublishableTest.php index 7303c37..6b9c7ec 100644 --- a/tests/SnapshotPublishableTest.php +++ b/tests/SnapshotPublishableTest.php @@ -25,7 +25,8 @@ public function testGetAtSnapshot(): void $a1 = $state['a1']; $firstSnapshot = Snapshot::get()->sort('Created ASC')->first(); - $result = SnapshotPublishable::get_at_snapshot(BlockPage::class, $a1->ID, $firstSnapshot->Created); + $result = SnapshotPublishable::singleton() + ->getAtSnapshotByClassAndId(BlockPage::class, $a1->ID, $firstSnapshot->Created); $param = $result->getSourceQueryParam('Versioned.date'); $this->assertNotNull($param); @@ -44,7 +45,7 @@ public function testGetAtLastSnapshot(): void $a1->Title = 'changed'; $a1->write(); - $result = SnapshotPublishable::get_at_last_snapshot(BlockPage::class, $a1->ID); + $result = SnapshotPublishable::singleton()->getAtLastSnapshotByClassAndId(BlockPage::class, $a1->ID); $this->assertNotNull($result); $this->assertEquals('A1 Block Page', $result->Title); } @@ -61,7 +62,7 @@ public function testGetLastSnapshotItem(): void $this->snapshot($a1); /** @var DataObject|Versioned $result */ - $result = SnapshotPublishable::get_last_snapshot_item(BlockPage::class, $a1->ID); + $result = SnapshotPublishable::singleton()->getLastSnapshotItemByClassAndId(BlockPage::class, $a1->ID); $this->assertNotNull($result); $this->assertEquals($a1->Version, $result->Version); } @@ -72,7 +73,7 @@ public function testGetLastSnapshotItem(): void public function testGetSnapshots(): void { $state = $this->buildState(); - $snapshots = SnapshotPublishable::getSnapshots(); + $snapshots = SnapshotPublishable::singleton()->getSnapshots(); $this->assertOrigins($snapshots, $state); } diff --git a/tests/SnapshotTestAbstract.php b/tests/SnapshotTestAbstract.php index 2aa20b1..7246a2d 100644 --- a/tests/SnapshotTestAbstract.php +++ b/tests/SnapshotTestAbstract.php @@ -14,7 +14,6 @@ use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Snapshot; use SilverStripe\Snapshots\SnapshotEvent; -use SilverStripe\Snapshots\SnapshotHasher; use SilverStripe\Snapshots\SnapshotItem; use SilverStripe\Snapshots\SnapshotPublishable; use SilverStripe\Snapshots\Tests\SnapshotTest\BaseJoin; @@ -203,7 +202,7 @@ protected function assertItems(SS_List $result, array $objects, string $column = $this->assertCount(count($objects), $hashes); foreach ($objects as $o) { - $hash = SnapshotHasher::hashObjectForSnapshot($o); + $hash = SnapshotPublishable::singleton()->hashObjectForSnapshot($o); $this->assertTrue(in_array($hash, $hashes)); } } @@ -220,16 +219,16 @@ protected function assertOrigins(SS_List $result, array $objects): void protected function assertHashCompare(DataObject $obj1, DataObject $obj2): void { - $this->assertTrue(SnapshotHasher::hashSnapshotCompare($obj1, $obj2)); + $this->assertTrue(SnapshotPublishable::singleton()->hashSnapshotCompare($obj1, $obj2)); } protected function assertHashCompareList(array $objs1, array $objs2): void { $hashes1 = array_map(static function ($o) { - return SnapshotHasher::hashObjectForSnapshot($o); + return SnapshotPublishable::singleton()->hashObjectForSnapshot($o); }, $objs1); $hashes2 = array_map(static function ($o) { - return SnapshotHasher::hashObjectForSnapshot($o); + return SnapshotPublishable::singleton()->hashObjectForSnapshot($o); }, $objs2); sort($hashes1); sort($hashes2);