From c9f8cc970b6394f93323fb42b34bacb7c012af90 Mon Sep 17 00:00:00 2001 From: Mojmir Fendek Date: Fri, 18 Mar 2022 15:33:57 +1300 Subject: [PATCH 1/2] NEW: Code cleanup (tests, linting). --- .scrutinizer.yml | 11 + .travis.yml | 52 +-- composer.json | 15 +- phpcs.xml.dist | 38 +- src/ActivityEntry.php | 19 +- src/CsvBulkLoader/SaveListener.php | 11 +- src/Elemental/SaveListener.php | 11 +- src/Handler/CMSMain/Handler.php | 7 +- src/Handler/CsvBulkLoader/Handler.php | 5 +- .../Elemental/ArchiveElementHandler.php | 11 +- src/Handler/Elemental/CMSActionsHandler.php | 7 +- .../Elemental/CreateElementHandler.php | 9 +- .../Elemental/ModifyElementHandler.php | 18 +- src/Handler/Elemental/PageSaveHandler.php | 33 +- src/Handler/Elemental/SortElementsHandler.php | 26 +- src/Handler/Form/Handler.php | 17 +- src/Handler/Form/PublishHandler.php | 14 +- src/Handler/Form/SaveHandler.php | 7 +- src/Handler/Form/UnpublishHandler.php | 15 +- src/Handler/GraphQL/Middleware/Handler.php | 2 +- .../GraphQL/Middleware/RollbackHandler.php | 16 +- src/Handler/GraphQL/Mutation/Handler.php | 2 +- src/Handler/GridField/Action/Handler.php | 6 +- .../GridField/Action/ReorderHandler.php | 19 +- src/Handler/GridField/Alteration/Handler.php | 15 +- src/Handler/HandlerAbstract.php | 17 +- src/ImplicitModification.php | 26 +- src/InvalidSnapshotException.php | 2 +- src/Migration/Job.php | 35 +- src/Migration/MigrationService.php | 28 +- src/Migration/Task.php | 19 +- src/PageContextProvider.php | 18 +- src/RelationDiffer.php | 17 +- src/Snapshot.php | 69 +++- src/SnapshotChangeSetItem.php | 22 +- src/SnapshotHasher.php | 7 +- src/SnapshotItem.php | 2 + src/SnapshotPublishable.php | 263 ++++++------ src/SnapshotSiteTree.php | 41 +- src/Thirdparty/EmbargoExpiryExtension.php | 12 + src/Thirdparty/UsedOnTableExtension.php | 2 + src/Workflow/WorkflowExtension.php | 6 +- tests/Handler/CMSMain/HandlerTest.php | 19 +- .../Elemental/ArchiveElementHandlerTest.php | 38 +- .../Elemental/CMSActionsHandlerTest.php | 33 +- .../Elemental/CreateElementHandlerTest.php | 31 +- .../Elemental/ModifyElementHandlerTest.php | 51 ++- .../Handler/Elemental/PageSaveHandlerTest.php | 58 +-- .../Elemental/SortElementsHandlerTest.php | 40 +- .../GraphQL/FakePageContextProvider.php | 4 +- .../GraphQL/Middleware/HandlerTest.php | 21 +- .../Middleware/RollbackHandlerTest.php | 15 +- .../Handler/GraphQL/Mutation/HandlerTest.php | 2 +- .../Handler/GridField/Action/HandlerTest.php | 13 +- .../GridField/Action/ReorderHandlerTest.php | 6 +- .../GridField/Alteration/HandlerTest.php | 13 +- tests/IntegrationTest.php | 384 ++++++++++++------ tests/PageContextProviderTest.php | 16 +- tests/RelationDifferTest.php | 43 +- tests/SnapshotItemTest.php | 18 +- tests/SnapshotPublishableTest.php | 115 ++++-- tests/SnapshotTest.php | 157 ++++--- tests/SnapshotTest/BaseJoin.php | 3 + tests/SnapshotTest/Block.php | 29 +- tests/SnapshotTest/BlockPage.php | 25 +- tests/SnapshotTest/Gallery.php | 32 +- tests/SnapshotTest/GalleryImage.php | 16 +- tests/SnapshotTest/GalleryImageJoin.php | 9 + tests/SnapshotTestAbstract.php | 112 +++-- 69 files changed, 1511 insertions(+), 764 deletions(-) create mode 100644 .scrutinizer.yml diff --git a/.scrutinizer.yml b/.scrutinizer.yml new file mode 100644 index 0000000..2c6e7e5 --- /dev/null +++ b/.scrutinizer.yml @@ -0,0 +1,11 @@ +checks: + php: true + +build: + nodes: + analysis: + tests: + override: [php-scrutinizer-run] + +filter: + paths: ["src/*", "tests/*"] diff --git a/.travis.yml b/.travis.yml index c33b4a3..5fe844e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,51 +1,9 @@ -language: php - -dist: trusty +version: ~> 1.0 env: global: - - COMPOSER_ROOT_VERSION=dev-master - -matrix: - fast_finish: true - include: - - php: '7.2' - env: DB=MYSQL PHPUNIT_TEST=1 - - - php: '7.3' - env: DB=MYSQL PHPUNIT_TEST=1 - - - php: '7.4' - env: DB=MYSQL PHPUNIT_TEST=1 - - - php: *min-version - env: DB=MYSQL PHPUNIT_COVERAGE_TEST=1 PHPCS_TEST=1 - -# - php: *min-version -# env: DB=PGSQL PHPUNIT_TEST=1 - - - php: *min-version - env: DB=MYSQL PDO=1 PHPUNIT_TEST=1 - -before_script: -# Init PHP - - printf "\n" | pecl install imagick - - phpenv rehash - - phpenv config-rm xdebug.ini - - echo 'memory_limit = 2048M' >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini - -# Install composer - - composer validate - - composer require silverstripe/recipe-core:4.5.x-dev silverstripe/recipe-cms:4.5.x-dev dnadesign/silverstripe-elemental:4.3.x-dev --prefer-dist --no-update -# silverstripe/recipe-cms requires versioned 1.4.x-dev - - composer require silverstripe/versioned:"1.5.x-dev" --prefer-dist --no-update - - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.1.x-dev --prefer-dist --no-update; fi - - composer install --prefer-dist - -script: - - if [[ $PHPUNIT_TEST ]]; then vendor/bin/phpunit; fi - - if [[ $PHPCS_TEST ]]; then composer run-script lint; fi - - if [[ $PHPUNIT_COVERAGE_TEST ]]; then phpdbg -qrr vendor/bin/phpunit --coverage-clover=coverage.xml; fi + - COMPOSER_ROOT_VERSION="4.x-dev" + - REQUIRE_RECIPE="4.x-dev" -after_success: - - if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml; fi +import: + - silverstripe/silverstripe-travis-shared:config/provision/standard-jobs-fixed.yml diff --git a/composer.json b/composer.json index 3f3dd0a..7449624 100755 --- a/composer.json +++ b/composer.json @@ -19,18 +19,23 @@ "homepage": "http://silverstripe.org" } ], + "extra": { + "branch-alias": { + "dev-master": "2.x-dev", + "dev-1": "1.x-dev" + } + }, "require": { - "php": ">=7.1.0", - "silverstripe/framework": "^4.5.1", + "php": "^7.4 || ^8", + "silverstripe/framework": "^4.7", "silverstripe/versioned": "^1", "silverstripe/vendor-plugin": "^1", "silverstripe/cms-events": "dev-master" }, "require-dev": { - "phpunit/phpunit": "^5.7", + "silverstripe/recipe-testing": "^1 || ^2", "silverstripe/versioned": "^1", - "squizlabs/php_codesniffer": "^3", - "sminnee/phpunit-mock-objects": "^3.4.9" + "dnadesign/silverstripe-elemental": "^4" }, "conflict": { "dnadesign/elemental": "< 3.2.1", diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 7503b36..5779613 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -1,24 +1,26 @@ - CodeSniffer ruleset for SilverStripe coding conventions. + CodeSniffer ruleset for SilverStripe coding conventions. + + src + tests - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + - diff --git a/src/ActivityEntry.php b/src/ActivityEntry.php index 0027f2e..eaab54a 100644 --- a/src/ActivityEntry.php +++ b/src/ActivityEntry.php @@ -2,10 +2,14 @@ namespace SilverStripe\Snapshots; +use Exception; +use SilverStripe\ORM\DataObject; use SilverStripe\Versioned\Versioned; use SilverStripe\View\ArrayData; -use Exception; +/** + * @property DataObject $Subject + */ class ActivityEntry extends ArrayData { const MODIFIED = 'MODIFIED'; @@ -22,16 +26,19 @@ class ActivityEntry extends ArrayData const UNPUBLISHED = 'UNPUBLISHED'; - public static function createFromSnapshotItem(SnapshotItem $item) + public static function createFromSnapshotItem(SnapshotItem $item): self { + /** @var DataObject|Versioned $itemObj */ $itemObj = $item->getItem(); - if ($itemObj !== null && $itemObj instanceof SnapshotEvent) { + if ($itemObj instanceof SnapshotEvent) { $flag = null; } elseif ($item->WasPublished) { $flag = self::PUBLISHED; } elseif ($item->Parent()->exists()) { - $flag = $item->WasDeleted ? self::REMOVED : self::ADDED; + $flag = $item->WasDeleted + ? self::REMOVED + : self::ADDED; } elseif ($item->WasDeleted) { $flag = self::DELETED; } elseif ($item->WasUnpublished) { @@ -45,10 +52,12 @@ public static function createFromSnapshotItem(SnapshotItem $item) // If the items been deleted then we want to get the last version of it if ($itemObj === null || $itemObj->WasDeleted) { // This gets all versions except for the deleted version so we just get the latest one + /** @var DataObject|Versioned $previousVersion */ $previousVersion = Versioned::get_all_versions($item->ObjectClass, $item->ObjectID) ->filter(['WasDeleted' => 0]) ->sort('Version', 'DESC') ->first(); + if ($previousVersion && $previousVersion->exists()) { $itemObj = $item->getItem($previousVersion->Version); // This is to deal with the case in which there is no previous version @@ -66,7 +75,7 @@ public static function createFromSnapshotItem(SnapshotItem $item) )); } - return new static([ + return static::create([ 'Subject' => $itemObj, 'Action' => $flag, 'Owner' => null, diff --git a/src/CsvBulkLoader/SaveListener.php b/src/CsvBulkLoader/SaveListener.php index a47eda8..f7abe99 100644 --- a/src/CsvBulkLoader/SaveListener.php +++ b/src/CsvBulkLoader/SaveListener.php @@ -1,8 +1,8 @@ getOwner(); $page = $owner->getArea()->getOwnerPage(); + Dispatcher::singleton()->trigger( 'elementalAreaUpdated', Event::create( diff --git a/src/Handler/CMSMain/Handler.php b/src/Handler/CMSMain/Handler.php index 9175f31..caa660d 100644 --- a/src/Handler/CMSMain/Handler.php +++ b/src/Handler/CMSMain/Handler.php @@ -1,6 +1,5 @@ getAction(); + if ($action === null) { return null; } - /* @var HTTPResponse $result */ + /** @var HTTPResponse $result */ $result = $context->get('result'); + if (!$result instanceof HTTPResponse) { return null; } + if ((int) $result->getStatusCode() !== 200) { return null; } diff --git a/src/Handler/CsvBulkLoader/Handler.php b/src/Handler/CsvBulkLoader/Handler.php index 0c2e4fb..404755c 100644 --- a/src/Handler/CsvBulkLoader/Handler.php +++ b/src/Handler/CsvBulkLoader/Handler.php @@ -1,9 +1,10 @@ get('record'); if (!$obj) { - throw new \InvalidArgumentException('Requires "record" in context'); + throw new InvalidArgumentException('Requires "record" in context'); } // Create an individual snapshot for each object to ensure they're all captured. diff --git a/src/Handler/Elemental/ArchiveElementHandler.php b/src/Handler/Elemental/ArchiveElementHandler.php index caacd18..187beef 100644 --- a/src/Handler/Elemental/ArchiveElementHandler.php +++ b/src/Handler/Elemental/ArchiveElementHandler.php @@ -1,6 +1,5 @@ getAction(); + if ($action === null) { return null; } // GraphQL 4 ?? GraphQL 3 $params = $context->get('variables') ?? $context->get('params'); + if (!$params) { return null; } + $blockID = $params['blockId'] ?? null; + if (!$blockID) { return null; } + $block = BaseElement::get()->byID($blockID); // Ensure the block is gone if ($block) { return null; } + $archivedBlock = SnapshotPublishable::get_at_last_snapshot(BaseElement::class, $blockID); + if (!$archivedBlock) { return null; } diff --git a/src/Handler/Elemental/CMSActionsHandler.php b/src/Handler/Elemental/CMSActionsHandler.php index fd0b2af..a3a5c02 100644 --- a/src/Handler/Elemental/CMSActionsHandler.php +++ b/src/Handler/Elemental/CMSActionsHandler.php @@ -1,14 +1,13 @@ getAction(); + if (!$action) { return null; } $request = $context->get('request'); + if (!$request) { return null; } $id = $request->param('ID'); + if (!$id) { return null; } $block = DataObject::get_by_id(BaseElement::class, $id); + if (!$block) { return null; } diff --git a/src/Handler/Elemental/CreateElementHandler.php b/src/Handler/Elemental/CreateElementHandler.php index 8562787..c7024f7 100644 --- a/src/Handler/Elemental/CreateElementHandler.php +++ b/src/Handler/Elemental/CreateElementHandler.php @@ -1,34 +1,35 @@ getAction(); + if ($action === null) { return null; } // GraphQL 4 ?? GraphQL 3 $params = $context->get('variables') ?? $context->get('params'); + if (!$params) { return null; } + $areaID = $params['elementalAreaID'] ?? null; + if (!$areaID) { return null; } + $area = ElementalArea::get()->byID($areaID); if (!$area) { diff --git a/src/Handler/Elemental/ModifyElementHandler.php b/src/Handler/Elemental/ModifyElementHandler.php index e2b3036..5d01ee1 100644 --- a/src/Handler/Elemental/ModifyElementHandler.php +++ b/src/Handler/Elemental/ModifyElementHandler.php @@ -1,6 +1,5 @@ getAction(); + if ($action === null) { return null; } // GraphQL 4 ?? GraphQL 3 $params = $context->get('variables') ?? $context->get('params'); + if (!$params) { return null; } + $blockID = $params['blockId'] ?? null; + if (!$blockID) { return null; } @@ -40,15 +44,19 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot } $snapshot = Snapshot::singleton()->createSnapshot($block); + if (!$snapshot) { return null; } + foreach ($snapshot->Items() as $item) { - // If it's the origin item, set published state. - if (static::hashSnapshotCompare($item->getItem(), $block)) { - $item->WasPublished = $action === 'PublishBlock'; - $item->WasUnpublished = $action === 'UnpublishBlock'; + if (!static::hashSnapshotCompare($item->getItem(), $block)) { + continue; } + + // If it's the origin item, set published state. + $item->WasPublished = $action === 'PublishBlock'; + $item->WasUnpublished = $action === 'UnpublishBlock'; } return $snapshot; diff --git a/src/Handler/Elemental/PageSaveHandler.php b/src/Handler/Elemental/PageSaveHandler.php index 107514c..889ac04 100644 --- a/src/Handler/Elemental/PageSaveHandler.php +++ b/src/Handler/Elemental/PageSaveHandler.php @@ -1,17 +1,17 @@ getRecordFromContext($context); + if (!$record) { return null; } + if (!$record->hasExtension(ElementalAreasExtension::class)) { return parent::createSnapshot($context); } + $changedElements = []; - /* @var SnapshotPublishable|ElementalAreasExtension $record */ + + /** @var SnapshotPublishable|ElementalAreasExtension $record */ foreach ($record->getElementalRelations() as $areaName) { $diffs = $record->$areaName()->getRelationDiffs(); - $elementDiffs = array_filter($diffs, function (RelationDiffer $diff) { + $elementDiffs = array_filter($diffs, static function (RelationDiffer $diff) { return $diff->getRelationClass() === BaseElement::class; }); - if (!empty($elementDiffs)) { - /* @var RelationDiffer $diff */ - foreach ($elementDiffs as $diff) { - $changedElements = array_merge($changedElements, $diff->getChanged()); - } + + /** @var RelationDiffer $diff */ + foreach ($elementDiffs as $diff) { + $changedElements = array_merge($changedElements, $diff->getChanged()); } } + // Defer to page save - if (empty($changedElements)) { + if (count($changedElements) === 0) { return parent::createSnapshot($context); } + $elements = BaseElement::get()->byIDs($changedElements); + if (count($changedElements) === 1) { return Snapshot::singleton()->createSnapshot($elements->first()); } $message = _t( - __CLASS__ . '.BLOCK_UPDATED_MANY', + self::class . '.BLOCK_UPDATED_MANY', 'Updated {count} {type}', [ 'count' => count($changedElements), @@ -70,6 +78,7 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot ] ); $snapshot = Snapshot::singleton()->createSnapshotEvent($message); + foreach ($elements as $e) { $snapshot->addOwnershipChain($e); } diff --git a/src/Handler/Elemental/SortElementsHandler.php b/src/Handler/Elemental/SortElementsHandler.php index bbf3203..5878266 100644 --- a/src/Handler/Elemental/SortElementsHandler.php +++ b/src/Handler/Elemental/SortElementsHandler.php @@ -1,43 +1,55 @@ getAction(); + if ($action === null) { return null; } + // GraphQL 4 ?? GraphQL 3 $params = $context->get('variables') ?? $context->get('params'); + if (!$params) { return null; } + $blockID = $params['blockId'] ?? null; + if (!$blockID) { return null; } - /* @var SnapshotPublishable $block */ + + /** @var BaseElement|SnapshotPublishable $block */ $block = BaseElement::get()->byID($blockID); + if (!$block) { return null; } $area = $block->Parent(); - return Snapshot::singleton()->createSnapshotEvent( - _t(__CLASS__ . '.REORDER_BLOCKS', 'Reordered blocks') - )->addOwnershipChain($area); + return Snapshot::singleton() + ->createSnapshotEvent(_t(self::class . '.REORDER_BLOCKS', 'Reordered blocks')) + ->addOwnershipChain($area); } } diff --git a/src/Handler/Form/Handler.php b/src/Handler/Form/Handler.php index 118eda6..2d2bee6 100644 --- a/src/Handler/Form/Handler.php +++ b/src/Handler/Form/Handler.php @@ -1,6 +1,5 @@ getAction(); + if ($action === null) { return null; } @@ -35,8 +35,6 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot return Snapshot::singleton()->createSnapshot($record); } - - /** * @param EventContextInterface $context * @return DataObject|null @@ -44,18 +42,20 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot protected function getPageFromContext(EventContextInterface $context): ?DataObject { $page = $context->get('page'); + if ($page) { return $page; } - /* @var HTTPRequest $request */ + /** @var HTTPRequest $request */ $request = $context->get('request'); - if (!$request || !$request instanceof HTTPRequest) { + if (!$request instanceof HTTPRequest) { return null; } $url = $request->getURL(); + return $this->getCurrentPageFromRequestUrl($url); } @@ -66,6 +66,7 @@ protected function getPageFromContext(EventContextInterface $context): ?DataObje protected function getRecordFromContext(EventContextInterface $context): ?DataObject { $record = $context->get('record'); + if ($record) { return $record; } @@ -79,13 +80,13 @@ protected function getRecordFromContext(EventContextInterface $context): ?DataOb return null; } - $refetched = DataObject::get_by_id($record->baseClass(), $record->ID); + $reFetched = DataObject::get_by_id($record->baseClass(), $record->ID); // If the record was deleted, return the version still linked to the form - if (!$refetched && $record->hasExtension(Versioned::class)) { + if (!$reFetched && $record->hasExtension(Versioned::class)) { return $record; } - return $refetched; + return $reFetched; } } diff --git a/src/Handler/Form/PublishHandler.php b/src/Handler/Form/PublishHandler.php index 76c6ff5..5176d7d 100644 --- a/src/Handler/Form/PublishHandler.php +++ b/src/Handler/Form/PublishHandler.php @@ -1,11 +1,10 @@ getAction(); + if ($action === null) { return null; } @@ -27,15 +32,18 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot if ($record === null || !$record->hasExtension(Versioned::class)) { return null; } + $snapshot = Snapshot::singleton()->createSnapshot($record); // Get the most recent change set to find out what was published + /** @var ChangeSet $changeSet */ $changeSet = ChangeSet::get()->filter([ 'State' => ChangeSet::STATE_PUBLISHED, 'IsInferred' => true, ]) ->sort('Created', 'DESC') ->first(); + if ($changeSet) { foreach ($changeSet->Changes() as $item) { foreach ($item->findReferenced() as $obj) { diff --git a/src/Handler/Form/SaveHandler.php b/src/Handler/Form/SaveHandler.php index dd20194..718e632 100644 --- a/src/Handler/Form/SaveHandler.php +++ b/src/Handler/Form/SaveHandler.php @@ -1,10 +1,9 @@ getRecordFromContext($context); + if ($record === null) { return parent::createSnapshot($context); } diff --git a/src/Handler/Form/UnpublishHandler.php b/src/Handler/Form/UnpublishHandler.php index d8c2630..4aa5898 100644 --- a/src/Handler/Form/UnpublishHandler.php +++ b/src/Handler/Form/UnpublishHandler.php @@ -1,32 +1,37 @@ getRecordFromContext($context); + if (!$record) { return null; } + foreach ($snapshot->Items() as $item) { - // If it's the origin item, set published state. - if (static::hashSnapshotCompare($item->getItem(), $record)) { - $item->WasUnpublished = true; + if (!static::hashSnapshotCompare($item->getItem(), $record)) { + continue; } + + // If it's the origin item, set published state. + $item->WasUnpublished = true; } return $snapshot; diff --git a/src/Handler/GraphQL/Middleware/Handler.php b/src/Handler/GraphQL/Middleware/Handler.php index 65b76b2..7a64309 100644 --- a/src/Handler/GraphQL/Middleware/Handler.php +++ b/src/Handler/GraphQL/Middleware/Handler.php @@ -1,6 +1,5 @@ getAction(); + if ($action === null) { return null; } diff --git a/src/Handler/GraphQL/Middleware/RollbackHandler.php b/src/Handler/GraphQL/Middleware/RollbackHandler.php index 6966bba..a301919 100644 --- a/src/Handler/GraphQL/Middleware/RollbackHandler.php +++ b/src/Handler/GraphQL/Middleware/RollbackHandler.php @@ -1,6 +1,5 @@ getAction(); + if ($action === null) { return null; } @@ -32,6 +33,7 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot // GraphQL 4 ?? GraphQL 3 $params = $context->get('variables') ?? $context->get('params'); + if (!$params) { return null; } @@ -44,23 +46,29 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot } $result = $context->get('result'); + if (!$result instanceof ExecutionResult) { return null; } - if (!empty($result->errors)) { + + if (count($result->errors) > 0) { return null; } + $data = $result->data; $className = $data[$action]['ClassName'] ?? null; + if (!$className) { return null; } - /* @var DataObject|SnapshotPublishable $obj */ + /** @var DataObject|SnapshotPublishable $obj */ $obj = DataObject::get_by_id($className, $id); + if (!$obj) { return null; } + $wasVersion = $obj->getPreviousSnapshotVersion(); $nowVersion = Versioned::get_version($className, $id, $toVersion); @@ -70,7 +78,7 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot $snapshot = Snapshot::singleton()->createSnapshotEvent( _t( - __CLASS__ . '.ROLLBACK_TO_VERSION', + self::class . '.ROLLBACK_TO_VERSION', 'Rolled back to version {version}', [ 'version' => $toVersion, diff --git a/src/Handler/GraphQL/Mutation/Handler.php b/src/Handler/GraphQL/Mutation/Handler.php index 9b7bfc1..a7c5ca9 100644 --- a/src/Handler/GraphQL/Mutation/Handler.php +++ b/src/Handler/GraphQL/Mutation/Handler.php @@ -1,6 +1,5 @@ getAction(); + if ($type === null) { return null; } diff --git a/src/Handler/GridField/Action/Handler.php b/src/Handler/GridField/Action/Handler.php index 7fb5362..518cfc4 100644 --- a/src/Handler/GridField/Action/Handler.php +++ b/src/Handler/GridField/Action/Handler.php @@ -1,6 +1,5 @@ getAction(); + if ($action === null) { return null; } + $grid = $context->get('gridField'); + if (!$grid) { return null; } - /* @var Form $form */ + /** @var Form $form */ $form = $grid->getForm(); if (!$form) { diff --git a/src/Handler/GridField/Action/ReorderHandler.php b/src/Handler/GridField/Action/ReorderHandler.php index 64f8c60..c7d64c1 100644 --- a/src/Handler/GridField/Action/ReorderHandler.php +++ b/src/Handler/GridField/Action/ReorderHandler.php @@ -4,33 +4,42 @@ use SilverStripe\EventDispatcher\Event\EventContextInterface; use SilverStripe\Forms\GridField\GridField; +use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Snapshot; use SilverStripe\Snapshots\SnapshotEvent; class ReorderHandler extends Handler { + /** + * @throws ValidationException + */ protected function createSnapshot(EventContextInterface $context): ?Snapshot { $snapshot = parent::createSnapshot($context); + if (!$snapshot) { return null; } - /* @var GridField $grid */ + + /** @var GridField $grid */ $grid = $context->get('gridField'); + if (!$grid) { return null; } + $model = $grid->getModelClass(); - $pluralName = $model::singleton()->i18n_plural_name(); + $pluralName = DataObject::singleton($model)->i18n_plural_name(); $event = SnapshotEvent::create([ 'Title' => _t( - __CLASS__ . '.REORDER_ROWS', + self::class . '.REORDER_ROWS', 'Reordered {title}', ['title' => $pluralName] ), ]); $event->write(); - return $snapshot - ->applyOrigin($event); + + return $snapshot->applyOrigin($event); } } diff --git a/src/Handler/GridField/Alteration/Handler.php b/src/Handler/GridField/Alteration/Handler.php index 4a44c4e..c2ad11b 100644 --- a/src/Handler/GridField/Alteration/Handler.php +++ b/src/Handler/GridField/Alteration/Handler.php @@ -1,16 +1,13 @@ getAction(); + if ($action === null) { return null; } + $args = $context->get('args'); + if (!$args) { return null; } + // Warning: this relies on convention. There's no guarantee an action provider uses // "RecordID" as its argument name. $recordID = $args['RecordID'] ?? null; + if ($recordID === null) { return null; } - /* @var GridField $grid */ + /** @var GridField $grid */ $grid = $context->get('gridField'); + if (!$grid) { return null; } + $class = $grid->getModelClass(); + if (!is_subclass_of($class, DataObject::class)) { return null; } @@ -53,8 +58,10 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot $record = DataObject::get_by_id($class, $recordID); // @todo Move this to a proper archive handler + if (!$record) { $record = $this->getDeletedVersion($class, $recordID); + if (!$record) { return null; } diff --git a/src/Handler/HandlerAbstract.php b/src/Handler/HandlerAbstract.php index d1546a8..27ec5ab 100644 --- a/src/Handler/HandlerAbstract.php +++ b/src/Handler/HandlerAbstract.php @@ -1,6 +1,5 @@ config()->get('messages'); + if (isset($messages[$action])) { return $messages[$action]; } $key = static::class . '.HANDLER_' . $action; + return _t($key, $action); } @@ -60,12 +63,19 @@ protected function getDeletedVersion(string $recordClass, int $id): ?DataObject ->byID($id); } + /** + * @param EventContextInterface $context + * @throws ValidationException + */ public function fire(EventContextInterface $context): void { $snapshot = $this->createSnapshot($context); - if ($snapshot) { - $snapshot->write(); + + if (!$snapshot) { + return; } + + $snapshot->write(); } /** @@ -87,7 +97,6 @@ public function getPageContextProvider(): PageContextProvider return $this->pageContextProvider; } - /** * @param EventContextInterface $context * @return Snapshot|null diff --git a/src/ImplicitModification.php b/src/ImplicitModification.php index f7dfe00..8832887 100644 --- a/src/ImplicitModification.php +++ b/src/ImplicitModification.php @@ -1,6 +1,5 @@ getMessagesForDiff($diff)); } - $this->Title = implode("\n", $messages); + $this->Title = implode(PHP_EOL, $messages); return $this; } @@ -39,6 +39,7 @@ private function getMessagesForDiff(RelationDiffer $diff): array 'removed' => ['Removed', 'Deleted'], 'changed' => ['Modified', 'Modified'], ]; + foreach ($i18nGraph as $category => $labels) { $getter = 'get' . ucfirst($category); // Number of records in 'added', or 'removed', etc. @@ -46,34 +47,39 @@ private function getMessagesForDiff(RelationDiffer $diff): array // e.g. MANY_MANY, HAS_MANY $i18nRelationKey = strtoupper($relationType); // e.g. use "Added" for many_many, "Created" for has_many - list ($manyManyLabel, $hasManyLabel) = $labels; - $action = $relationType === 'many_many' ? $manyManyLabel : $hasManyLabel; + [$manyManyLabel, $hasManyLabel] = $labels; + $action = $relationType === 'many_many' + ? $manyManyLabel + : $hasManyLabel; // e.g. ADDED, for MANY_MANY_ADDED $i18nActionKey = strtoupper($action); - // If singular, be specific with the record if ($ct === 1) { + // If singular, be specific with the record $map = $diff->$getter(); $id = $map[0] ?? 0; $record = DataObject::get_by_id($class, $id); + if ($record) { $messages[] = _t( - __CLASS__ . '.' . $i18nRelationKey . '_' . $i18nActionKey . '_ONE', + self::class . '.' . $i18nRelationKey . '_' . $i18nActionKey . '_ONE', $action . ' {type} {title}', [ 'type' => $sng->singular_name(), - 'title' => !empty($record->getTitle()) ? '"' . $record->getTitle() . '"' : '', + 'title' => $record->getTitle() + ? '"' . $record->getTitle() . '"' + : '', ] ); } - // Otherwise, just give a count } elseif ($ct > 1) { + // Otherwise, just give a count $messages[] = _t( - __CLASS__ . '.' . $i18nRelationKey . '_' . $i18nActionKey . '_MANY', + self::class . '.' . $i18nRelationKey . '_' . $i18nActionKey . '_MANY', $action . ' {count} {name}', [ 'count' => $ct, - 'name' => $ct > 1 ? $sng->plural_name() : $sng->singular_name() + 'name' => $sng->plural_name(), ] ); } diff --git a/src/InvalidSnapshotException.php b/src/InvalidSnapshotException.php index f23b8de..8f7f15d 100644 --- a/src/InvalidSnapshotException.php +++ b/src/InvalidSnapshotException.php @@ -7,7 +7,7 @@ class InvalidSnapshotException extends RuntimeException { - public function __construct($snapshot = "", $code = 0, Throwable $previous = null) + public function __construct($snapshot = '', $code = 0, Throwable $previous = null) { parent::__construct(sprintf('Invalid snapshot: %s', $snapshot), $code, $previous); } diff --git a/src/Migration/Job.php b/src/Migration/Job.php index 87ef2b1..4d6a781 100644 --- a/src/Migration/Job.php +++ b/src/Migration/Job.php @@ -1,6 +1,5 @@ addMessage('Prepping database...'); $this->getMigrator()->setup(); @@ -40,9 +40,9 @@ public function setup() /** * @return string */ - public function getSignature() + public function getSignature(): string { - return md5(get_class($this)); + return md5(static::class); } /** @@ -52,23 +52,23 @@ public function process(): void { $remainingChildren = $this->classesToProcess; $this->addMessage(sizeof($remainingChildren) . ' classes remaining'); - if (empty($remainingChildren)) { + + if (count($remainingChildren) === 0) { $this->isComplete = true; + return; } + $baseClass = array_shift($remainingChildren); - $this->addMessage("Migrating $baseClass"); + $this->addMessage('Migrating ' . $baseClass); $rows = $this->getMigrator()->migrate($baseClass); - $this->addMessage("Base ID " . $this->getMigrator()->getBaseID()); - $this->addMessage("Migrated $rows records"); + $this->addMessage('Base ID ' . $this->getMigrator()->getBaseID()); + $this->addMessage(sprintf('Migrated %d records', $rows)); $this->classesToProcess = $remainingChildren; - $this->currentStep++; + $this->currentStep += 1; } - /** - * @return void - */ public function afterComplete(): void { parent::afterComplete(); @@ -81,15 +81,12 @@ public function afterComplete(): void /** * @return string */ - public function getTitle() + public function getTitle(): string { - return _t(__CLASS__ . '.MIGRATE', 'Migrate versions tables to snapshots'); + return _t(self::class . '.MIGRATE', 'Migrate versions tables to snapshots'); } - /** - * @return string - */ - public function getJobType() + public function getJobType(): int { return QueuedJob::QUEUED; } @@ -98,7 +95,7 @@ public function getJobType() * @param MigrationService $migrator * @return $this */ - public function setMigrator(MigrationService $migrator) + public function setMigrator(MigrationService $migrator): self { $this->migrator = $migrator; diff --git a/src/Migration/MigrationService.php b/src/Migration/MigrationService.php index 5276e78..b9f336d 100644 --- a/src/Migration/MigrationService.php +++ b/src/Migration/MigrationService.php @@ -1,10 +1,10 @@ baseTable(); $versionsTable = $baseTable . '_Versions'; $rows = $this->migrateSnapshots($versionsTable); @@ -81,14 +81,17 @@ public function migrate(string $baseClass): int * * @throws ReflectionException * @throws ValidationException + * @throws Exception */ public function seedRelationTracking(): void { foreach (ClassInfo::subclassesFor(DataObject::class, false) as $class) { $tracking = $class::config()->uninherited('snapshot_relation_tracking'); - if (empty($tracking)) { + + if (!$tracking) { continue; } + foreach ($class::get() as $record) { SnapshotItem::create() ->hydrateFromDataObject($record) @@ -214,7 +217,7 @@ private function migrateItems(string $versionsTable): int return (int) DB::affected_rows(); } - private function createTemporaryTable() + private function createTemporaryTable(): void { DB::query("DROP TABLE IF EXISTS \"__ClassNameLookup\""); DB::create_table( @@ -225,6 +228,7 @@ private function createTemporaryTable() ] ); $lines = []; + foreach ($this->getClassMap() as $className => $baseClassName) { $lines[] = sprintf( "('%s', '%s')", @@ -232,6 +236,7 @@ private function createTemporaryTable() $this->sanitiseClassName($baseClassName) ); } + $values = implode(",\n", $lines); $query = <<get($class); + if (!$sng->hasExtension(Versioned::class)) { continue; } @@ -273,19 +280,20 @@ private function generateClassMap(): void $baseClass = $sng->baseClass(); $map[$class] = $baseClass; } + $this->classMap = $map; } /** - * @param $class + * @param string $class * @return string */ - private function sanitiseClassName($class): string + private function sanitiseClassName(string $class): string { return str_replace('\\', '\\\\', $class); } - public function getBaseID() + public function getBaseID(): int { return $this->baseID; } diff --git a/src/Migration/Task.php b/src/Migration/Task.php index db3806c..06ff0fb 100644 --- a/src/Migration/Task.php +++ b/src/Migration/Task.php @@ -1,15 +1,18 @@ get(LoggerInterface::class); @@ -45,10 +51,11 @@ public function run($request) $logger->info('Migrating ' . sizeof($classes) . ' classes'); foreach ($classes as $class) { - $logger->info("Migrating $class"); + $logger->info('Migrating ' . $class); $rows = $this->migrator->migrate($class); - $logger->info("$rows records migrated"); + $logger->info($rows . ' records migrated'); } + $this->migrator->seedRelationTracking(); $this->migrator->tearDown(); } diff --git a/src/PageContextProvider.php b/src/PageContextProvider.php index 77246d5..42795ec 100644 --- a/src/PageContextProvider.php +++ b/src/PageContextProvider.php @@ -2,7 +2,6 @@ namespace SilverStripe\Snapshots\Handler; -use Page; use SilverStripe\Admin\AdminRootController; use SilverStripe\CMS\Controllers\CMSMain; use SilverStripe\CMS\Controllers\CMSPageEditController; @@ -22,6 +21,7 @@ */ class PageContextProvider { + use Injectable; /** @@ -43,7 +43,7 @@ public function __construct(?HTTPRequest $request = null) */ public function getCurrentPageFromController($controller): ?DataObject { - while ($controller && ($controller instanceof Form || $controller instanceof GridFieldDetailForm_ItemRequest)) { + while ($controller instanceof Form || $controller instanceof GridFieldDetailForm_ItemRequest) { $controller = $controller->getController(); } @@ -57,7 +57,7 @@ public function getCurrentPageFromController($controller): ?DataObject $page = $controller->currentPage(); - if ($page === null || !$page instanceof SiteTree) { + if (!$page instanceof SiteTree) { return null; } @@ -74,6 +74,7 @@ public function getCurrentPageFromController($controller): ?DataObject public function getCurrentPageFromRequestUrl(?string $url): ?SiteTree { $url = trim($url, '/ '); + if (!$url) { return null; } @@ -84,9 +85,11 @@ public function getCurrentPageFromRequestUrl(?string $url): ?SiteTree $urlBase = $controller->config()->get('url_segment'); $baseURL = Path::join($adminSegment, $urlBase); $pattern = '#^' . $baseURL .'#'; + if (!preg_match($pattern, $url)) { return null; } + $slug = preg_replace($pattern, '', $url); $request = new HTTPRequest('GET', $slug); $params = $request->match($controller->config()->get('url_rule')); @@ -98,9 +101,11 @@ public function getCurrentPageFromRequestUrl(?string $url): ?SiteTree // find page by ID $page = DataObject::get_by_id(SiteTree::class, (int) $pageId); + if (!$page) { return null; } + // re-fetch the page with proper type $page = DataObject::get_by_id($page->ClassName, $pageId); @@ -120,7 +125,9 @@ public function getRequest(): ?HTTPRequest return $this->request; } - return Controller::has_curr() ? Controller::curr()->getRequest() : null; + return Controller::has_curr() + ? Controller::curr()->getRequest() + : null; } /** @@ -140,12 +147,15 @@ public function setRequest(HTTPRequest $request): self public function getPageFromReferrer(): ?SiteTree { $request = $this->getRequest(); + if (!$request) { return null; } + $url = $request->getHeader('referer'); $url = parse_url($url, PHP_URL_PATH); $url = ltrim($url, '/'); + return $this->getCurrentPageFromRequestUrl($url); } } diff --git a/src/RelationDiffer.php b/src/RelationDiffer.php index 2e3ddc7..aac13d1 100644 --- a/src/RelationDiffer.php +++ b/src/RelationDiffer.php @@ -1,12 +1,11 @@ hasExtension(SnapshotPublishable::class)) { - throw new InvalidArgumentException(sprintf('DataObject must use the %s extension', SnapshotPublishable::class)); + throw new InvalidArgumentException( + sprintf('DataObject must use the %s extension', SnapshotPublishable::class) + ); } + $this->relationClass = $relationClass; $this->relationType = $relationType; $this->previousVersionMapping = $previousVersionMapping; @@ -91,11 +95,14 @@ private function diff(): void if (!isset($this->currentVersionMapping[$prevID])) { continue; } + $currentVersion = $this->currentVersionMapping[$prevID]; + // Versioned extension not applied if ($prevVersion === null && $currentVersion === null) { continue; } + // New version is higher than old version. It's a change. if ($currentVersion > $prevVersion) { $changed[] = $prevID; @@ -110,7 +117,7 @@ private function diff(): void */ public function hasChanges(): bool { - return !empty($this->added) || !empty($this->removed) || !empty($this->changed); + return count($this->added) > 0 || count($this->removed) > 0 || count($this->changed) > 0; } /** @@ -121,11 +128,13 @@ public function getRecords(): array if (!$this->hasChanges()) { return []; } + $ids = array_merge( $this->added, $this->removed, $this->changed ); + return DataList::create($this->relationClass)->byIDs($ids)->toArray(); } diff --git a/src/Snapshot.php b/src/Snapshot.php index ffddf2e..12ee463 100644 --- a/src/Snapshot.php +++ b/src/Snapshot.php @@ -2,6 +2,7 @@ namespace SilverStripe\Snapshots; +use Exception; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\HasManyList; use SilverStripe\ORM\Queries\SQLSelect; @@ -10,7 +11,6 @@ use SilverStripe\Security\Permission; use SilverStripe\Security\Security; use SilverStripe\Versioned\Versioned; -use Exception; /** * Class Snapshot @@ -95,22 +95,31 @@ class Snapshot extends DataObject /** * @return SnapshotItem|null */ - public function getOriginItem() + public function getOriginItem(): ?DataObject { - return $this->Items()->filter([ - 'ObjectHash' => $this->OriginHash, - ])->first(); + $item = $this->Items() + ->filter([ + 'ObjectHash' => $this->OriginHash, + ]) + ->first(); + + if ($item instanceof DataObject) { + return $item; + } + + return null; } /** - * Shortcut for adding items by their associated dataobjects + * Shortcut for adding items by their associated data objects + * * @param DataObject $obj * @return $this * @throws Exception */ public function addObject(DataObject $obj): self { - if ($this->Items()->count() >= $this->config()->item_limit) { + if ($this->Items()->count() >= $this->config()->get('item_limit')) { return $this; } @@ -120,6 +129,7 @@ public function addObject(DataObject $obj): self return $this; } } + $this->Items()->add($obj); } else { // Ensure uniqueness @@ -128,6 +138,7 @@ public function addObject(DataObject $obj): self return $this; } } + $item = SnapshotItem::create() ->hydrateFromDataObject($obj); @@ -138,11 +149,12 @@ public function addObject(DataObject $obj): self } /** - * @return SnapshotItem|null + * @return DataObject|null */ - public function getOriginVersion() + public function getOriginVersion(): ?DataObject { $originItem = $this->getOriginItem(); + if ($originItem) { return Versioned::get_version( $originItem->ObjectClass, @@ -157,20 +169,23 @@ public function getOriginVersion() /** * @return string */ - public function getDate() + public function getDate(): string { return $this->LastEdited; } /** * @return string + * @throws Exception */ public function getActivityDescription(): string { + /** @var SnapshotItem $item */ $item = $this->getOriginItem(); + return $item ? ActivityEntry::createFromSnapshotItem($item)->getDescription() - : _t(__CLASS__ . 'ACTIVITY_NONE', 'none'); + : _t(self::class . 'ACTIVITY_NONE', 'none'); } public function getActivityAgo(): string @@ -184,8 +199,10 @@ public function getActivityAgo(): string public function getIsLiveSnapshot(): bool { $table = DataObject::getSchema()->tableName(SnapshotItem::class); - /* @var Versioned|DataObject $originVersion */ + + /** @var Versioned|DataObject $originVersion */ $originVersion = $this->getOriginVersion(); + if (!$originVersion) { return false; } @@ -193,16 +210,18 @@ public function getIsLiveSnapshot(): bool if ($originVersion->hasStages() && !$originVersion->isLiveVersion()) { return false; } + $liveVersionNumber = SnapshotPublishable::get_published_version_number( $originVersion->baseClass(), $originVersion->ID ); + $latestPublishID = SQLSelect::create() ->setSelect('MAX("SnapshotID")') - ->setFrom($table) + ->setFrom("\"$table\"") ->addWhere([ - '"Version" = ?' => $liveVersionNumber, - '"ObjectHash" = ?' => static::hashObjectForSnapshot($originVersion), + "\"$table\".\"Version\" = ?" => $liveVersionNumber, + "\"$table\".\"ObjectHash\" = ?" => static::hashObjectForSnapshot($originVersion), ]) ->execute() ->value(); @@ -216,7 +235,9 @@ public function getIsLiveSnapshot(): bool */ public function getActivityType(): ?string { + /** @var SnapshotItem $item */ $item = $this->getOriginItem(); + return $item ? ActivityEntry::createFromSnapshotItem($item)->Action : null; @@ -262,7 +283,7 @@ public function canView($member = null, $context = []) return true; } - public function onBeforeWrite() + public function onBeforeWrite(): void { parent::onBeforeWrite(); @@ -270,16 +291,17 @@ public function onBeforeWrite() } /** - * * @param DataObject|null $origin * @param array $extraObjects * @return Snapshot|null * @throws ValidationException + * @throws Exception */ public function createSnapshot( DataObject $origin, array $extraObjects = [] ): ?Snapshot { + /** @var DataObject|SnapshotPublishable $origin */ if (!$origin->hasExtension(SnapshotPublishable::class)) { return null; } @@ -301,14 +323,17 @@ public function createSnapshot( $event->write(); $snapshot->applyOrigin($event); $eventItem = $snapshot->getOriginItem(); - /* @var RelationDiffer $diff */ + + /** @var RelationDiffer $diff */ foreach ($diffs as $diff) { foreach ($diff->getRecords() as $obj) { $item = SnapshotItem::create() ->hydrateFromDataObject($obj); + if ($diff->isRemoved($obj->ID ?: $obj->OldID)) { $item->WasDeleted = true; } + $eventItem->Children()->add($item); $snapshot->addObject($item); } @@ -330,7 +355,7 @@ public function createSnapshot( * @return Snapshot * @throws ValidationException */ - public function createSnapshotEvent(string $message, $extraObjects = []): Snapshot + public function createSnapshotEvent(string $message, array $extraObjects = []): Snapshot { $event = SnapshotEvent::create([ 'Title' => $message, @@ -344,10 +369,13 @@ public function createSnapshotEvent(string $message, $extraObjects = []): Snapsh * sets the related snapshot items to not modified * * items with modifications are used to determine the owner's modification - * status (eg in site tree's status flags) + * status (e.g. in site tree's status flags) + * + * @throws ValidationException */ public function markNoModifications(): void { + /** @var SnapshotItem $item */ foreach ($this->Items() as $item) { $item->Modification = false; $item->write(); @@ -377,6 +405,7 @@ public function applyOrigin(DataObject $origin): self public function addOwnershipChain(DataObject $obj): self { $this->addObject($obj); + foreach ($obj->getIntermediaryObjects() as $o) { $this->addObject($o); } diff --git a/src/SnapshotChangeSetItem.php b/src/SnapshotChangeSetItem.php index 45b2d45..db91cc0 100644 --- a/src/SnapshotChangeSetItem.php +++ b/src/SnapshotChangeSetItem.php @@ -1,30 +1,40 @@ owner->getComponent('Object'); if (!$obj->hasExtension(SnapshotPublishable::class)) { return; } - if ($obj->hasOwnedModifications()) { - $type = ChangeSetItem::CHANGE_MODIFIED; + if (!$obj->hasOwnedModifications()) { + return; } + + $type = ChangeSetItem::CHANGE_MODIFIED; } } diff --git a/src/SnapshotHasher.php b/src/SnapshotHasher.php index ac6fff1..1fc5657 100644 --- a/src/SnapshotHasher.php +++ b/src/SnapshotHasher.php @@ -2,7 +2,6 @@ namespace SilverStripe\Snapshots; -use SilverStripe\Core\ClassInfo; use SilverStripe\ORM\DataObject; /** @@ -13,11 +12,11 @@ trait SnapshotHasher /** * Generates a hash for versioned snapshots * - * @param $class - * @param $id + * @param string|null $class + * @param int|null $id * @return string */ - public static function hashForSnapshot($class, $id): string + public static function hashForSnapshot(?string $class, ?int $id): string { return md5(sprintf('%s:%s', $class, $id)); } diff --git a/src/SnapshotItem.php b/src/SnapshotItem.php index 535c9b7..81a6590 100644 --- a/src/SnapshotItem.php +++ b/src/SnapshotItem.php @@ -24,8 +24,10 @@ * @property int $SnapshotID * @property int $ObjectID * @property string $ObjectClass + * @property string $Modification * @method Snapshot Snapshot() * @method DataObject Object() + * @method SnapshotItem Parent() * @package SilverStripe\Snapshots */ class SnapshotItem extends DataObject diff --git a/src/SnapshotPublishable.php b/src/SnapshotPublishable.php index eec8808..6511221 100644 --- a/src/SnapshotPublishable.php +++ b/src/SnapshotPublishable.php @@ -3,6 +3,7 @@ namespace SilverStripe\Snapshots; use Exception; +use InvalidArgumentException; use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\DataList; @@ -11,7 +12,6 @@ use SilverStripe\ORM\Queries\SQLSelect; use SilverStripe\Versioned\RecursivePublishable; use SilverStripe\Versioned\Versioned; -use InvalidArgumentException; /** * Class SnapshotPublishable @@ -35,15 +35,17 @@ class SnapshotPublishable extends RecursivePublishable private static $relationDiffs = []; /** - * A more resillient wrapper for the Versioned function that holds up against unstaged versioned + * A more resilient wrapper for the Versioned function that holds up against un-staged versioned * implementations + * * @param string $class * @param int $id * @return int|null */ public static function get_published_version_number(string $class, int $id): ?int { - $inst = $class::singleton(); + $inst = DataObject::singleton($class); + if (!$inst->hasExtension(Versioned::class)) { throw new InvalidArgumentException(sprintf( 'Class %s does not have the %s extension', @@ -52,27 +54,31 @@ public static function get_published_version_number(string $class, int $id): ?in )); } - /* @var Versioned|DataObject $inst */ - $stage = $inst->hasStages() ? Versioned::LIVE : Versioned::DRAFT; + /** @var Versioned|DataObject $inst */ + $stage = $inst->hasStages() + ? Versioned::LIVE + : Versioned::DRAFT; return Versioned::get_versionnumber_by_stage($class, $stage, $id); } /** - * @param $class - * @param $id + * @param string $class + * @param int $id * @param string|int $snapshot A snapshot ID or a Y-m-d h:i:s date formatted string - * @return DataObject + * @return DataObject|null */ - public static function get_at_snapshot($class, $id, $snapshot) + public static function get_at_snapshot(string $class, int $id, $snapshot): ?DataObject { $baseClass = DataObject::getSchema()->baseDataClass($class); - $snapshotDate = null; + if (is_numeric($snapshot)) { $record = DataObject::get_by_id(Snapshot::class, $snapshot); + if (!$record) { throw new InvalidSnapshotException($snapshot); } + $snapshotDate = $record->Created; } else { $snapshotDate = $snapshot; @@ -90,7 +96,9 @@ public static function get_at_snapshot($class, $id, $snapshot) public static function get_at_last_snapshot(string $class, int $id): ?DataObject { + /** @var SnapshotItem $lastItem */ $lastItem = static::get_last_snapshot_item($class, $id); + if (!$lastItem) { return null; } @@ -103,11 +111,12 @@ 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 static function get_last_snapshot_item(string $class, int $id): ?DataObject { - return SnapshotItem::get()->filter([ - 'ObjectHash' => static::hashForSnapshot($class, $id) - ]) + return SnapshotItem::get() + ->filter([ + 'ObjectHash' => static::hashForSnapshot($class, $id), + ]) ->sort('Created', 'DESC') ->first(); } @@ -115,25 +124,24 @@ public static function get_last_snapshot_item(string $class, int $id): ? DataObj /** * @return DataList */ - public static function getSnapshots() + public static function getSnapshots(): DataList { $snapshotTable = DataObject::getSchema()->tableName(Snapshot::class); $itemTable = DataObject::getSchema()->tableName(SnapshotItem::class); - $result = Snapshot::get() + return Snapshot::get() ->innerJoin($itemTable, "\"$snapshotTable\".\"ID\" = \"$itemTable\".\"SnapshotID\""); - - return $result; } /** * @return DataList */ - public function getRelevantSnapshots() + public function getRelevantSnapshots(): DataList { + $itemTable = DataObject::getSchema()->tableName(SnapshotItem::class); $snapshots = $this->owner->getSnapshots() ->where([ - ['"ObjectHash" = ?' => static::hashObjectForSnapshot($this->owner)] + ["\"$itemTable\".\"ObjectHash\" = ?" => static::hashObjectForSnapshot($this->owner)] ]); $this->owner->extend('updateRelevantSnapshots', $snapshots); @@ -145,56 +153,53 @@ public function getRelevantSnapshots() * @param int $sinceVersion * @return DataList */ - public function getSnapshotsSinceVersion($sinceVersion) + public function getSnapshotsSinceVersion($sinceVersion): DataList { $sinceVersion = (int) $sinceVersion; $itemTable = DataObject::getSchema()->tableName(SnapshotItem::class); $where = [ // last published version - ['"Version" >= ?' => $sinceVersion], + ["\"$itemTable\".\"Version\" >= ?" => $sinceVersion], // is not a snapshot of the last publishing [ sprintf( - '"SnapshotID" > + "\"$itemTable\".\"SnapshotID\" > COALESCE(( SELECT - MAX("SnapshotID") + MAX(\"$itemTable\".\"SnapshotID\") FROM - "%s" + \"%s\" WHERE - "ObjectHash" = ? + \"$itemTable\".\"ObjectHash\" = ? AND - "Version" = ? + \"$itemTable\".\"Version\" = ? AND - "WasPublished" = 1 - ), 0)', + \"$itemTable\".\"WasPublished\" = 1 + ), 0)", $itemTable ) => [ static::hashObjectForSnapshot($this->owner), - $sinceVersion - ] + $sinceVersion, + ], ], ]; - $result = $this->owner->getRelevantSnapshots() + return $this->owner->getRelevantSnapshots() ->where($where); - - return $result; } /** * @return DataList */ - public function getSnapshotsSinceLastPublish() + public function getSnapshotsSinceLastPublish(): DataList { $class = $this->owner->baseClass(); $id = $this->owner->ID; $publishedVersion = static::get_published_version_number($class, $id); - $snapshots = $this->owner->getSnapshotsSinceVersion($publishedVersion); - return $snapshots; + return $this->owner->getSnapshotsSinceVersion($publishedVersion); } /** @@ -204,10 +209,9 @@ public function getSnapshotsSinceLastPublish() * @param int $min Minimal version to start looking with (inclusive) * @param int|null $max Maximal version to look until (inclusive) * @param bool $includeAll Include snapshot items that have no modifications - * * @return array list of filters for using in ORM APIs */ - protected function getSnapshotsBetweenVersionsFilters($min, $max = null, $includeAll = false) + protected function getSnapshotsBetweenVersionsFilters(int $min, ?int $max = null, bool $includeAll = false): array { $itemTable = DataObject::getSchema()->tableName(SnapshotItem::class); @@ -216,8 +220,8 @@ protected function getSnapshotsBetweenVersionsFilters($min, $max = null, $includ "MIN(\"$itemTable\".\"SnapshotID\")", "\"$itemTable\"", [ - '"ObjectHash" = ?' => $hash, - '"Version" = ?' => $min, + "\"$itemTable\".\"ObjectHash\" = ?" => $hash, + "\"$itemTable\".\"Version\" = ?" => $min, ] ); $minShotSQL = $minShot->sql($minParams); @@ -226,8 +230,8 @@ protected function getSnapshotsBetweenVersionsFilters($min, $max = null, $includ "MAX(\"$itemTable\".\"SnapshotID\")", "\"$itemTable\"", [ - '"ObjectHash" = ?' => $hash, - '"Version" = ?' => $max, + "\"$itemTable\".\"ObjectHash\" = ?" => $hash, + "\"$itemTable\".\"Version\" = ?" => $max, ] ); $maxShotSQL = $maxShot->sql($maxParams); @@ -236,19 +240,20 @@ protected function getSnapshotsBetweenVersionsFilters($min, $max = null, $includ ? sprintf("\"$itemTable\".\"SnapshotID\" >= (%s)", $minShotSQL) : sprintf("\"$itemTable\".\"SnapshotID\" BETWEEN (%s) and (%s)", $minShotSQL, $maxShotSQL); - $condtionStatement = [ + $conditionStatement = [ $condition => $max === null ? $minParams : array_merge($minParams, $maxParams), - '"ObjectHash"' => $hash, - 'NOT ("Version" = ? AND "WasPublished" = 1)' => $min, + "\"$itemTable\".\"ObjectHash\"" => $hash, + "NOT (\"$itemTable\".\"Version\" = ? AND \"$itemTable\".\"WasPublished\" = 1)" => $min, ]; + if (!$includeAll) { - $condtionStatement[] = '"Modification" = 1'; + $conditionStatement[] = "\"$itemTable\".\"Modification\" = 1"; } $query = SQLSelect::create( "\"$itemTable\".\"SnapshotID\"", "\"$itemTable\"", - $condtionStatement + $conditionStatement ); $sql = $query->sql($params); @@ -258,9 +263,9 @@ protected function getSnapshotsBetweenVersionsFilters($min, $max = null, $includ } /** - * @return boolean + * @return bool */ - public function hasOwnedModifications() + public function hasOwnedModifications(): bool { if (!$this->owner->hasExtension(Versioned::class)) { return false; @@ -283,26 +288,29 @@ public function hasOwnedModifications() /** * @return int */ - public function getPublishableItemsCount() + public function getPublishableItemsCount(): int { - $snapShotIDs = $this->getSnapshotsSinceLastPublish()->column('ID'); - if (empty($snapShotIDs)) { + $snapshotIds = $this->getSnapshotsSinceLastPublish()->column('ID'); + + if (count($snapshotIds) === 0) { return 0; } - return $this->publishableItemsQuery($snapShotIDs)->execute()->numRecords(); + + return $this->publishableItemsQuery($snapshotIds)->execute()->numRecords(); } /** * @return ArrayList */ - public function getPublishableObjects() + public function getPublishableObjects(): ArrayList { - $snapShotIDs = $this->getSnapshotsSinceLastPublish()->column('ID'); - if (empty($snapShotIDs)) { + $snapshotIds = $this->getSnapshotsSinceLastPublish()->column('ID'); + + if (count($snapshotIds) === 0) { return ArrayList::create(); } - $query = $this->publishableItemsQuery($snapShotIDs); + $query = $this->publishableItemsQuery($snapshotIds); $query->addSelect($groupByFields = ['"ObjectClass"', '"ObjectID"']); $query->addGroupBy($groupByFields); @@ -312,7 +320,7 @@ public function getPublishableObjects() foreach ($items as $row) { $class = $row['ObjectClass']; $id = $row['ObjectID']; - /* @var DataObject|SnapshotPublishable $obj */ + /** @var DataObject|SnapshotPublishable $obj */ $obj = DataObject::get_by_id($class, $id); $map[static::hashObjectForSnapshot($obj)] = $obj; } @@ -328,10 +336,13 @@ public function getRelationTracking(): array $owner = $this->owner; $tracking = $owner->config()->get('snapshot_relation_tracking') ?? []; $data = []; + foreach ($tracking as $relation) { - if ($owner->hasMethod($relation) && $owner->getRelationClass($relation)) { - $data[$relation] = $owner->$relation()->map('ID', 'Version')->toArray(); + if (!$owner->hasMethod($relation) || !$owner->getRelationClass($relation)) { + continue; } + + $data[$relation] = $owner->$relation()->map('ID', 'Version')->toArray(); } return $data; @@ -340,9 +351,9 @@ public function getRelationTracking(): array /** * @param int|string $snapshot A snapshot ID or date formatted string - * @return DataObject + * @return DataObject|null */ - public function getAtSnapshot($snapshot) + public function getAtSnapshot($snapshot): ?DataObject { return static::get_at_snapshot($this->owner->baseClass(), $this->owner->ID, $snapshot); } @@ -350,9 +361,10 @@ public function getAtSnapshot($snapshot) /** * @return DataObject|null */ - public function getAtLastSnapshot() + public function getAtLastSnapshot(): ?DataObject { $lastItem = $this->owner->getPreviousSnapshotItem(); + if (!$lastItem) { return null; } @@ -362,8 +374,9 @@ public function getAtLastSnapshot() /** * Tidy up all the irrelevant snapshot records now that the changes have been reverted. + * Extension point in @see Versioned::doRevertToLive() */ - public function onAfterRevertToLive() + public function onAfterRevertToLive(): void { $snapshots = $this->getSnapshotsSinceVersion($this->owner->Version) ->filter([ @@ -378,9 +391,10 @@ public function onAfterRevertToLive() */ public function getPreviousSnapshotItem(): ?DataObject { - return SnapshotItem::get()->filter([ - 'ObjectHash' => static::hashObjectForSnapshot($this->owner), - ]) + return SnapshotItem::get() + ->filter([ + 'ObjectHash' => static::hashObjectForSnapshot($this->owner), + ]) ->sort('Created', 'DESC') ->first(); } @@ -393,12 +407,15 @@ public function atPreviousSnapshot(callable $callback) { // Get the last time this record was in a snapshot. Doesn't matter where or why. We just need a // timestamp prior to now, because the Version may be unchanged. - $lastSnapshot = SnapshotItem::get()->filter([ - 'ObjectHash' => static::hashObjectForSnapshot($this->owner), - ])->max('LastEdited'); + $lastSnapshot = SnapshotItem::get() + ->filter([ + 'ObjectHash' => static::hashObjectForSnapshot($this->owner), + ]) + ->max('LastEdited'); - return Versioned::withVersionedMode(function () use ($callback, $lastSnapshot) { + return Versioned::withVersionedMode(static function () use ($callback, $lastSnapshot) { Versioned::reading_archived_date($lastSnapshot); + return $callback($lastSnapshot); }); } @@ -422,14 +439,15 @@ public function getPreviousSnapshotVersion(): ?DataObject */ public function isModifiedSinceLastSnapshot(): bool { + /** @var SnapshotItem $previous */ $previous = $this->getPreviousSnapshotVersion(); - return $previous ? $previous->Version < $this->owner->Version : true; + return !$previous || $previous->Version < $this->owner->Version; } /** * @return RelationDiffer[] - * @todo Memoise / cache + * @todo Memorise / cache */ public function getRelationDiffs(): array { @@ -438,14 +456,19 @@ public function getRelationDiffs(): array if (!$date) { return []; } + $record = DataObject::get_by_id($this->owner->baseClass(), $this->owner->ID, false); + if (!$record) { return []; } - /* @var DataObject|SnapshotPublishable $record */ + + /** @var DataObject|SnapshotPublishable $record */ return $record->getRelationTracking(); }); + $currentTracking = $this->owner->getRelationTracking(); + foreach ($currentTracking as $relationName => $currentMap) { $class = $this->owner->getRelationClass($relationName); $type = $this->owner->getRelationType($relationName); @@ -462,7 +485,7 @@ public function getRelationDiffs(): array * @param bool $cache * @return bool */ - public function hasRelationChanges($cache = true): bool + public function hasRelationChanges(bool $cache = true): bool { foreach ($this->getRelationDiffs($cache) as $diff) { if ($diff->hasChanges()) { @@ -479,11 +502,10 @@ public function hasRelationChanges($cache = true): bool */ public function getPreviousVersion($version = null): ?DataObject { - $previous = null; $record = $this->owner; if ($record->Version == 1) { - $previous = Injector::inst()->create(get_class($record)); + $previous = Injector::inst()->create($record->ClassName); } else { if ($version === null) { $version = $record->Version - 1; @@ -499,12 +521,12 @@ public function getPreviousVersion($version = null): ?DataObject * @param array $snapShotIDs * @return SQLSelect */ - protected function publishableItemsQuery($snapShotIDs) + protected function publishableItemsQuery(array $snapShotIDs): SQLSelect { $snapshotTable = DataObject::getSchema()->tableName(Snapshot::class); $itemTable = DataObject::getSchema()->tableName(SnapshotItem::class); - $query = new SQLSelect( + $query = SQLSelect::create( ['MaxID' => "MAX(\"$itemTable\".\"ID\")"], "\"$itemTable\"" ); @@ -513,12 +535,12 @@ protected function publishableItemsQuery($snapShotIDs) "\"$snapshotTable\".\"ID\" = \"$itemTable\".\"SnapshotID\"" ); $query->setWhere([ - ['"SnapshotID" IN (' . DB::placeholders($snapShotIDs) . ')' => $snapShotIDs], - ['"WasPublished" = ?' => 0], - ['"WasDeleted" = ?' => 0], - '"ObjectHash" = "OriginHash" OR "ParentID" != 0', + ["\"$itemTable\".\"SnapshotID\" IN (" . DB::placeholders($snapShotIDs) . ')' => $snapShotIDs], + ["\"$itemTable\".\"WasPublished\" = ?" => 0], + ["\"$itemTable\".\"WasDeleted\" = ?" => 0], + "\"$itemTable\".\"ObjectHash\" = \"$snapshotTable\".\"OriginHash\" OR \"$itemTable\".\"ParentID\" != 0", ]) - ->setGroupBy(['"ObjectHash"', "\"$itemTable\".\"Created\", \"$itemTable\".\"ID\""]) + ->setGroupBy(["\"$itemTable\".\"ObjectHash\"", "\"$itemTable\".\"Created\", \"$itemTable\".\"ID\""]) ->setOrderBy("\"$itemTable\".\"Created\", \"$itemTable\".\"ID\""); return $query; @@ -541,27 +563,35 @@ protected function getChangedOwnership(DataObject $previous): array } $hasOneLookup = array_flip($owner->hasOne()); + foreach ($lookup[get_class($owner)] as $info) { - if (isset($hasOneLookup[$info['class']])) { - $map[$hasOneLookup[$info['class']] . 'ID'] = $info['class']; + if (!isset($hasOneLookup[$info['class']])) { + continue; } + + $map[$hasOneLookup[$info['class']] . 'ID'] = $info['class']; } $result = []; + foreach ($map as $field => $class) { - $previousValue = (int) $previous->$field; - $currentValue = (int) $owner->$field; + $previousValue = (int) $previous->{$field}; + $currentValue = (int) $owner->{$field}; + if ($previousValue === $currentValue) { continue; } $class = $map[$field]; + $previousOwner = DataObject::get_by_id($class, $previousValue); - if (!$previousOwner = DataObject::get_by_id($class, $previousValue)) { + if (!$previousOwner) { continue; } - if (!$currentOwner = DataObject::get_by_id($class, $currentValue)) { + $currentOwner = DataObject::get_by_id($class, $currentValue); + + if (!$currentOwner) { continue; } @@ -580,6 +610,7 @@ protected function getChangedOwnership(DataObject $previous): array * is nothing the user can do about it. Recursive publishing the old owner * will not affect this record, as it is no longer in its ownership graph. * + * @throws Exception */ public function reconcileOwnershipChanges(?DataObject $previous = null): void { @@ -588,12 +619,13 @@ public function reconcileOwnershipChanges(?DataObject $previous = null): void } $changes = $this->getChangedOwnership($previous); + foreach ($changes as $spec) { - /* @var DataObject|SnapshotPublishable|Versioned $previousOwner */ + /** @var DataObject|SnapshotPublishable|Versioned $previousOwner */ $previousOwner = $spec['previous']; $previousOwners = array_merge([$previousOwner], $previousOwner->findOwners()->toArray()); - /* @var DataObject|SnapshotPublishable|Versioned $currentOwner */ + /** @var DataObject|SnapshotPublishable|Versioned $currentOwner */ $currentOwner = $spec['current']; $currentOwners = array_merge([$currentOwner], $currentOwner->findOwners()->toArray()); @@ -601,8 +633,9 @@ public function reconcileOwnershipChanges(?DataObject $previous = null): void // Get the earliest snapshot where the previous owner was published. $cutoff = $previousOwner->getSnapshotsSinceLastPublish() - ->sort('"ID" ASC') + ->sort('ID', 'ASC') ->first(); + if (!$cutoff) { return; } @@ -620,18 +653,21 @@ public function reconcileOwnershipChanges(?DataObject $previous = null): void 'ObjectHash' => $previousHashes, 'SnapshotID' => $snapshot->ID, ]); - if ($itemsToDelete->exists()) { - // Rip out the old owners - $itemsToDelete->removeAll(); + if (!$itemsToDelete->exists()) { + continue; + } + + // Rip out the old owners + $itemsToDelete->removeAll(); + + /** @var DataObject|SnapshotPublishable $owner */ + foreach ($currentOwners as $owner) { // Replace them with the new owners - /* @var DataObject|SnapshotPublishable $owner */ - foreach ($currentOwners as $owner) { - $item = SnapshotItem::create(); - $item->hydrateFromDataObject($owner); - $item->SnapshotID = $snapshot->ID; - $item->write(); - } + $item = SnapshotItem::create(); + $item->hydrateFromDataObject($owner); + $item->SnapshotID = $snapshot->ID; + $item->write(); } } } @@ -642,13 +678,16 @@ public function reconcileOwnershipChanges(?DataObject $previous = null): void */ public function getIntermediaryObjects(): array { - /* @var SnapshotPublishable|Versioned|DataObject $record */ + /** @var SnapshotPublishable|Versioned|DataObject $record */ $record = $this->owner; + if (!$record->hasExtension(static::class)) { return []; } + $intermediaryObjects = $record->findOwners(); $extraObjects = []; + foreach ($intermediaryObjects as $extra) { $extraObjects[SnapshotHasher::hashObjectForSnapshot($extra)] = $extra; } @@ -657,16 +696,16 @@ public function getIntermediaryObjects(): array } /** - * @param $min - * @param null $max + * @param int $min + * @param int|null $max * @return DataList */ - public function getActivityBetweenVersions($min, $max = null) + public function getActivityBetweenVersions(int $min, ?int $max = null): DataList { $snapshotTable = DataObject::getSchema()->tableName(Snapshot::class); $itemTable = DataObject::getSchema()->tableName(SnapshotItem::class); - $items = SnapshotItem::get() + return SnapshotItem::get() ->innerJoin($snapshotTable, "\"$snapshotTable\".\"ID\" = \"$itemTable\".\"SnapshotID\"") ->leftJoin($itemTable, "\"ChildItem\".\"ParentID\" = \"$itemTable\".\"ID\"", "ChildItem") ->where([ @@ -684,19 +723,16 @@ public function getActivityBetweenVersions($min, $max = null) "\"$itemTable\".\"SnapshotID\"" => "ASC", "\"$itemTable\".\"ID\"" => "ASC", ]); - - return $items; } /** * Returns a list of ActivityEntry ordered by creation datetime * * @param int|null $minVersion version to start with (or last published if null) * @param int|null $maxVersion version to end with (or till the end, including everything unpublished) - * * @return ArrayList list of ActivityEntry * @throws Exception */ - public function getActivityFeed($minVersion = null, $maxVersion = null) + public function getActivityFeed(?int $minVersion = null, ?int $maxVersion = null): ArrayList { if (is_null($minVersion)) { $class = $this->owner->baseClass(); @@ -711,6 +747,7 @@ public function getActivityFeed($minVersion = null, $maxVersion = null) $items = $this->getActivityBetweenVersions($minVersion, $maxVersion); $list = ArrayList::create(); + foreach ($items as $item) { $list->push(ActivityEntry::createFromSnapshotItem($item)); } diff --git a/src/SnapshotSiteTree.php b/src/SnapshotSiteTree.php index 4c8c09a..616ac48 100644 --- a/src/SnapshotSiteTree.php +++ b/src/SnapshotSiteTree.php @@ -5,40 +5,46 @@ use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormAction; -use SilverStripe\Forms\LiteralField; use SilverStripe\ORM\DataExtension; class SnapshotSiteTree extends DataExtension { /** - * @param $flags + * Extension point in @see SiteTree::getStatusFlags() + * + * @param mixed $flags */ - public function updateStatusFlags(&$flags) + public function updateStatusFlags(&$flags): void { $owner = $this->owner; + if (!$owner->hasExtension(SnapshotPublishable::class)) { return; } - /* @var SnapshotPublishable|SiteTree $owner */ - if ($owner->hasOwnedModifications()) { - $flags['modified'] = [ - 'text' => _t(SiteTree::class . '.MODIFIEDONDRAFTSHORT', 'Modified'), - 'title' => _t(SiteTree::class . '.MODIFIEDONDRAFTHELP', 'Page has owned modifications'), - ]; + + /** @var SnapshotPublishable|SiteTree $owner */ + if (!$owner->hasOwnedModifications()) { + return; } + + $flags['modified'] = [ + 'text' => _t(SiteTree::class . '.MODIFIEDONDRAFTSHORT', 'Modified'), + 'title' => _t(SiteTree::class . '.MODIFIEDONDRAFTHELP', 'Page has owned modifications'), + ]; } /** * @param FieldList $actions */ - public function updateCMSActions(FieldList $actions) + public function updateCMSActions(FieldList $actions): void { $owner = $this->owner; + if (!$owner->hasExtension(SnapshotPublishable::class)) { return; } - /* @var SnapshotPublishable|SiteTree $owner */ + /** @var SnapshotPublishable|SiteTree $owner */ $canPublish = $owner->canPublish(); $canEdit = $owner->canEdit(); $hasOwned = $owner->hasOwnedModifications(); @@ -57,14 +63,19 @@ public function updateCMSActions(FieldList $actions) ); } } + $publish = $actions->fieldByName('MajorActions.action_publish'); + if (!$publish) { return; } - if ($hasOwned && $canPublish) { - $publish->addExtraClass('btn-primary font-icon-rocket'); - $publish->setTitle(_t(SiteTree::class . '.BUTTONSAVEPUBLISH', 'Publish')); - $publish->removeExtraClass('btn-outline-primary font-icon-tick'); + + if (!$hasOwned || !$canPublish) { + return; } + + $publish->addExtraClass('btn-primary font-icon-rocket'); + $publish->setTitle(_t(SiteTree::class . '.BUTTONSAVEPUBLISH', 'Publish')); + $publish->removeExtraClass('btn-outline-primary font-icon-tick'); } } diff --git a/src/Thirdparty/EmbargoExpiryExtension.php b/src/Thirdparty/EmbargoExpiryExtension.php index f743bb8..f8b5a28 100644 --- a/src/Thirdparty/EmbargoExpiryExtension.php +++ b/src/Thirdparty/EmbargoExpiryExtension.php @@ -6,11 +6,18 @@ use SilverStripe\EventDispatcher\Symfony\Event; use SilverStripe\ORM\DataExtension; use SilverStripe\ORM\DataObject; +use Terraformers\EmbargoExpiry\Job\PublishTargetJob; +use Terraformers\EmbargoExpiry\Job\UnPublishTargetJob; class EmbargoExpiryExtension extends DataExtension { const EVENT_NAME = 'embargoExpiryJob'; + /** + * Extension point in @see PublishTargetJob::process() + * + * @param array $options + */ public function afterPublishTargetJob(array $options): void { Dispatcher::singleton()->trigger( @@ -25,6 +32,11 @@ public function afterPublishTargetJob(array $options): void ); } + /** + * Extension point in @see UnPublishTargetJob::process() + * + * @param array $options + */ public function afterUnPublishTargetJob(array $options): void { Dispatcher::singleton()->trigger( diff --git a/src/Thirdparty/UsedOnTableExtension.php b/src/Thirdparty/UsedOnTableExtension.php index 1dd5601..0790b31 100644 --- a/src/Thirdparty/UsedOnTableExtension.php +++ b/src/Thirdparty/UsedOnTableExtension.php @@ -2,6 +2,7 @@ namespace SilverStripe\Snapshots\Thirdparty; +use SilverStripe\Admin\Forms\UsedOnTable; use SilverStripe\ORM\DataExtension; use SilverStripe\Snapshots\Snapshot; use SilverStripe\Snapshots\SnapshotEvent; @@ -11,6 +12,7 @@ class UsedOnTableExtension extends DataExtension { /** * Exclude snapshot data objects from appearing in Used On tab in Files section + * Extension point in @see UsedOnTable::usage() * * @param array $excludedClasses */ diff --git a/src/Workflow/WorkflowExtension.php b/src/Workflow/WorkflowExtension.php index 6ac887d..5c4ce06 100644 --- a/src/Workflow/WorkflowExtension.php +++ b/src/Workflow/WorkflowExtension.php @@ -1,6 +1,5 @@ ID, false); Dispatcher::singleton()->trigger('workflowComplete', new Event('publish', [ @@ -18,12 +17,11 @@ public function onAfterWorkflowPublish(DataObject $target) ])); } - public function onAfterWorkflowUnpublish(DataObject $target) + public function onAfterWorkflowUnpublish(DataObject $target): void { $record = DataObject::get_by_id(get_class($target), $target->ID, false); Dispatcher::singleton()->trigger('workflowComplete', new Event('publish', [ 'record' => $record, ])); } - } diff --git a/tests/Handler/CMSMain/HandlerTest.php b/tests/Handler/CMSMain/HandlerTest.php index d75889a..97aed2a 100644 --- a/tests/Handler/CMSMain/HandlerTest.php +++ b/tests/Handler/CMSMain/HandlerTest.php @@ -4,15 +4,19 @@ use SilverStripe\Control\HTTPResponse; use SilverStripe\EventDispatcher\Symfony\Event; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Handler\CMSMain\Handler; use SilverStripe\Snapshots\Tests\SnapshotTest\BlockPage; use SilverStripe\Snapshots\Tests\SnapshotTestAbstract; class HandlerTest extends SnapshotTestAbstract { - public function testHandlerDoesntFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { - $handler = new Handler(); + $handler = Handler::create(); $this->mockSnapshot() ->expects($this->never()) ->method('createSnapshotEvent'); @@ -26,7 +30,7 @@ public function testHandlerDoesntFire() $context = Event::create( 'action', [ - 'result' => HTTPResponse::create('response', 400) + 'result' => HTTPResponse::create('response', 400), ] ); $handler->fire($context); @@ -34,7 +38,7 @@ public function testHandlerDoesntFire() $context = Event::create( 'action', [ - 'result' => HTTPResponse::create('response', 200) + 'result' => HTTPResponse::create('response', 200), ] ); $handler->fire($context); @@ -50,9 +54,12 @@ public function testHandlerDoesntFire() $handler->fire($context); } - public function testHandlerDoesFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesFire(): void { - $handler = new Handler(); + $handler = Handler::create(); $this->mockSnapshot() ->expects($this->once()) ->method('createSnapshotEvent'); diff --git a/tests/Handler/Elemental/ArchiveElementHandlerTest.php b/tests/Handler/Elemental/ArchiveElementHandlerTest.php index e30d0ce..9603eb6 100644 --- a/tests/Handler/Elemental/ArchiveElementHandlerTest.php +++ b/tests/Handler/Elemental/ArchiveElementHandlerTest.php @@ -5,21 +5,29 @@ use DNADesign\Elemental\Extensions\ElementalPageExtension; use DNADesign\Elemental\Models\BaseElement; use SilverStripe\EventDispatcher\Symfony\Event; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Handler\Elemental\ArchiveElementHandler; use SilverStripe\Snapshots\Tests\SnapshotTest\BlockPage; use SilverStripe\Snapshots\Tests\SnapshotTestAbstract; +use SilverStripe\Versioned\Versioned; class ArchiveElementHandlerTest extends SnapshotTestAbstract { - protected function setUp() - { - parent::setUp(); - BlockPage::add_extension(ElementalPageExtension::class); - } + /** + * @var array + */ + protected static $required_extensions = [ + BlockPage::class => [ + ElementalPageExtension::class, + ], + ]; - public function testHandlerDoesntFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { - $handler = new ArchiveElementHandler(); + $handler = ArchiveElementHandler::create(); $this->mockSnapshot() ->expects($this->never()) ->method('createSnapshot'); @@ -33,7 +41,7 @@ public function testHandlerDoesntFire() $context = Event::create( 'action', [ - 'params' => [] + 'params' => [], ] ); $handler->fire($context); @@ -41,7 +49,7 @@ public function testHandlerDoesntFire() $context = Event::create( 'action', [ - 'params' => ['blockId' => 5] + 'params' => ['blockId' => 5], ] ); $handler->fire($context); @@ -49,19 +57,23 @@ public function testHandlerDoesntFire() $context = Event::create( 'action', [ - 'params' => ['blockId' => $id] + 'params' => ['blockId' => $id], ] ); $handler->fire($context); } - public function testHandlerDoesFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesFire(): void { - $handler = new ArchiveElementHandler(); + $handler = ArchiveElementHandler::create(); $this->mockSnapshot() ->expects($this->once()) ->method('createSnapshot'); + /** @var BaseElement|Versioned $elem */ $elem = BaseElement::create(); $elem->write(); $this->createHistory($elem); @@ -69,7 +81,7 @@ public function testHandlerDoesFire() $context = Event::create( 'action', [ - 'params' => ['blockId' => $elem->ID] + 'params' => ['blockId' => $elem->ID], ] ); $handler->fire($context); diff --git a/tests/Handler/Elemental/CMSActionsHandlerTest.php b/tests/Handler/Elemental/CMSActionsHandlerTest.php index 4940a31..d0895de 100644 --- a/tests/Handler/Elemental/CMSActionsHandlerTest.php +++ b/tests/Handler/Elemental/CMSActionsHandlerTest.php @@ -6,20 +6,26 @@ use DNADesign\Elemental\Models\BaseElement; use SilverStripe\Control\HTTPRequest; use SilverStripe\EventDispatcher\Symfony\Event; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Handler\Elemental\CMSActionsHandler; use SilverStripe\Snapshots\Tests\SnapshotTest\BlockPage; use SilverStripe\Snapshots\Tests\SnapshotTestAbstract; class CMSActionsHandlerTest extends SnapshotTestAbstract { + /** + * @var array + */ + protected static $required_extensions = [ + BlockPage::class => [ + ElementalPageExtension::class, + ], + ]; - protected function setUp() - { - parent::setUp(); - BlockPage::add_extension(ElementalPageExtension::class); - } - - public function testHandlerDoesntFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { $handler = new CMSActionsHandler(); $this->mockSnapshot() @@ -35,7 +41,7 @@ public function testHandlerDoesntFire() $context = Event::create( 'action', [ - 'request' => new HTTPRequest('GET', '/') + 'request' => new HTTPRequest('GET', '/'), ] ); $handler->fire($context); @@ -44,16 +50,19 @@ public function testHandlerDoesntFire() 'action', [ 'request' => (new HTTPRequest('GET', '/'))->setRouteParams([ - 'ID' => 5 - ]) + 'ID' => 5, + ]), ] ); $handler->fire($context); } - public function testHandlerDoesFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesFire(): void { - $handler = new CMSActionsHandler(); + $handler = CMSActionsHandler::create(); $this->mockSnapshot() ->expects($this->once()) ->method('createSnapshot'); diff --git a/tests/Handler/Elemental/CreateElementHandlerTest.php b/tests/Handler/Elemental/CreateElementHandlerTest.php index 9b4bf50..72d721d 100644 --- a/tests/Handler/Elemental/CreateElementHandlerTest.php +++ b/tests/Handler/Elemental/CreateElementHandlerTest.php @@ -5,22 +5,28 @@ use DNADesign\Elemental\Extensions\ElementalPageExtension; use DNADesign\Elemental\Models\ElementalArea; use SilverStripe\EventDispatcher\Symfony\Event; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Handler\Elemental\CreateElementHandler; use SilverStripe\Snapshots\Tests\SnapshotTest\BlockPage; use SilverStripe\Snapshots\Tests\SnapshotTestAbstract; class CreateElementHandlerTest extends SnapshotTestAbstract { + /** + * @var array + */ + protected static $required_extensions = [ + BlockPage::class => [ + ElementalPageExtension::class, + ], + ]; - protected function setUp() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { - parent::setUp(); - BlockPage::add_extension(ElementalPageExtension::class); - } - - public function testHandlerDoesntFire() - { - $handler = new CreateElementHandler(); + $handler = CreateElementHandler::create(); $this->mockSnapshot() ->expects($this->never()) ->method('createSnapshot'); @@ -42,15 +48,18 @@ public function testHandlerDoesntFire() $context = Event::create( 'action', [ - 'params' => ['elementalAreaID' => 5] + 'params' => ['elementalAreaID' => 5], ] ); $handler->fire($context); } - public function testHandlerDoesFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesFire(): void { - $handler = new CreateElementHandler(); + $handler = CreateElementHandler::create(); $this->mockSnapshot() ->expects($this->once()) ->method('createSnapshot'); diff --git a/tests/Handler/Elemental/ModifyElementHandlerTest.php b/tests/Handler/Elemental/ModifyElementHandlerTest.php index 6dbd543..077381a 100644 --- a/tests/Handler/Elemental/ModifyElementHandlerTest.php +++ b/tests/Handler/Elemental/ModifyElementHandlerTest.php @@ -5,6 +5,7 @@ use DNADesign\Elemental\Extensions\ElementalPageExtension; use DNADesign\Elemental\Models\BaseElement; use SilverStripe\EventDispatcher\Symfony\Event; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Handler\Elemental\ModifyElementHandler; use SilverStripe\Snapshots\Snapshot; use SilverStripe\Snapshots\Tests\SnapshotTest\BlockPage; @@ -12,16 +13,21 @@ class ModifyElementHandlerTest extends SnapshotTestAbstract { + /** + * @var array + */ + protected static $required_extensions = [ + BlockPage::class => [ + ElementalPageExtension::class, + ], + ]; - protected function setUp() - { - parent::setUp(); - BlockPage::add_extension(ElementalPageExtension::class); - } - - public function testHandlerDoesntFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { - $handler = new ModifyElementHandler(); + $handler = ModifyElementHandler::create(); $this->mockSnapshot() ->expects($this->never()) ->method('createSnapshot'); @@ -43,28 +49,31 @@ public function testHandlerDoesntFire() $context = Event::create( 'action', [ - 'params' => ['blockId' => 5] + 'params' => ['blockId' => 5], ] ); $handler->fire($context); } - public function testHandlerDoesFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesFire(): void { - $handler = new ModifyElementHandler(); + $handler = ModifyElementHandler::create(); $block = BaseElement::create(); $block->write(); $this->mockSnapshot() ->expects($this->once()) ->method('createSnapshot') - ->with($this->callback(function ($arg) use ($block) { + ->with($this->callback(static function ($arg) use ($block) { return $arg instanceof BaseElement && $arg->ID == $block->ID; })); $context = Event::create('action', [ 'params' => [ - 'blockId' => $block->ID + 'blockId' => $block->ID, ], ]); @@ -72,24 +81,26 @@ public function testHandlerDoesFire() } /** - * @throws \SilverStripe\ORM\ValidationException + * @throws ValidationException * @dataProvider dataProvider */ - public function testHandlerSetsPublishState($actionName, $wasPublished, $wasUnpublished) + public function testHandlerSetsPublishState(string $actionName, bool $wasPublished, bool $wasUnpublished): void { - $handler = new ModifyElementHandler(); + $handler = ModifyElementHandler::create(); $block = BaseElement::create(); $block->write(); $context = Event::create($actionName, [ 'params' => [ - 'blockId' => $block->ID + 'blockId' => $block->ID, ], ]); $handler->fire($context); - /* @var Snapshot $snapshot */ - $snapshot = Snapshot::get()->sort('ID', 'DESC')->first(); + /** @var Snapshot $snapshot */ + $snapshot = Snapshot::get() + ->sort('ID', 'DESC') + ->first(); $this->assertNotNull($snapshot); $item = $snapshot->getOriginItem(); @@ -101,7 +112,7 @@ public function testHandlerSetsPublishState($actionName, $wasPublished, $wasUnpu /** * @return array */ - public function dataProvider() + public function dataProvider(): array { return [ ['PublishBlock', true, false], diff --git a/tests/Handler/Elemental/PageSaveHandlerTest.php b/tests/Handler/Elemental/PageSaveHandlerTest.php index a42028b..ff20550 100644 --- a/tests/Handler/Elemental/PageSaveHandlerTest.php +++ b/tests/Handler/Elemental/PageSaveHandlerTest.php @@ -10,6 +10,7 @@ use SilverStripe\EventDispatcher\Symfony\Event; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Handler\Elemental\PageSaveHandler; use SilverStripe\Snapshots\RelationDiffer; use SilverStripe\Snapshots\SnapshotPublishable; @@ -19,16 +20,21 @@ class PageSaveHandlerTest extends SnapshotTestAbstract { - + /** + * @var array + */ protected static $required_extensions = [ BlockPage::class => [ ElementalPageExtension::class, - ] + ], ]; - public function testHandlerDoesntFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { - $handler = new PageSaveHandler(); + $handler = PageSaveHandler::create(); $ext = $this->getMockBuilder(SnapshotPublishable::class) ->setMethods(['getRelationDiffs']) ->getMock(); @@ -36,13 +42,16 @@ public function testHandlerDoesntFire() ->method('getRelationDiffs') ->will($this->returnValue([])); Injector::inst()->registerService($ext, RecursivePublishable::class); - $area = ElementalArea::create([ - 'OwnerClassName' => BlockPage::class, - ]); + + $area = ElementalArea::create(); + $area->OwnerClassName = BlockPage::class; $area->write(); + + /** @var BlockPage|ElementalPageExtension $blockPage */ $blockPage = BlockPage::create(); $blockPage->ElementalAreaID = $area->ID; $blockPage->write(); + $this->createHistory($blockPage); $this->mockSnapshot() ->expects($this->never()) @@ -51,22 +60,23 @@ public function testHandlerDoesntFire() ->expects($this->never()) ->method('createSnapshotEvent'); - $form = Form::create(new Controller(), 'TestForm', FieldList::create(), FieldList::create()); + $form = Form::create(Controller::create(), 'TestForm', FieldList::create(), FieldList::create()); $form->loadDataFrom($blockPage); $context = Event::create('action', [ 'form' => $form, ]); + $handler->fire($context); } /** - * @param $many - * @throws \SilverStripe\ORM\ValidationException + * @param bool $many + * @throws ValidationException * @dataProvider dataProvider */ - public function testHandlerDoesFireMany($many) + public function testHandlerDoesFireMany(bool $many): void { - $handler = new PageSaveHandler(); + $handler = PageSaveHandler::create(); $block1 = BaseElement::create(); $block1->write(); $block2 = BaseElement::create(); @@ -94,10 +104,11 @@ public function testHandlerDoesFireMany($many) Injector::inst()->registerService($ext, RecursivePublishable::class); // Now that the mock is registered, we can create the object - $area = ElementalArea::create([ - 'OwnerClassName' => BlockPage::class, - ]); + $area = ElementalArea::create(); + $area->OwnerClassName = BlockPage::class; $area->write(); + + /** @var BlockPage|ElementalPageExtension $blockPage */ $blockPage = BlockPage::create(); $blockPage->ElementalAreaID = $area->ID; $blockPage->write(); @@ -108,7 +119,7 @@ public function testHandlerDoesFireMany($many) // If only one, expect a standard snapshot with block1 $mock->expects($many ? $this->never() : $this->once()) ->method('createSnapshot') - ->with($this->callback(function ($sub) use ($block1) { + ->with($this->callback(static function ($sub) use ($block1) { return $sub->ClassName === $block1->ClassName && $sub->ID = $block1->ID; })); @@ -122,30 +133,31 @@ public function testHandlerDoesFireMany($many) ->method('addOwnershipChain') ->withConsecutive( [ - $this->callback(function ($sub) use ($block1) { + $this->callback(static function ($sub) use ($block1) { return $sub->ClassName === $block1->ClassName && $sub->ID = $block1->ID; - }) + }), ], [ - $this->callback(function ($sub) use ($block2) { + $this->callback(static function ($sub) use ($block2) { return $sub->ClassName === $block2->ClassName && $sub->ID = $block2->ID; - }) + }), ] ); - $form = Form::create(new Controller(), 'TestForm', FieldList::create(), FieldList::create()); + $form = Form::create(Controller::create(), 'TestForm', FieldList::create(), FieldList::create()); $form->loadDataFrom($blockPage); $context = Event::create('action', [ 'form' => $form, ]); + $handler->fire($context); } - public function dataProvider() + public function dataProvider(): array { return [ [true], - [false] + [false], ]; } } diff --git a/tests/Handler/Elemental/SortElementsHandlerTest.php b/tests/Handler/Elemental/SortElementsHandlerTest.php index cc14ed8..7e3e627 100644 --- a/tests/Handler/Elemental/SortElementsHandlerTest.php +++ b/tests/Handler/Elemental/SortElementsHandlerTest.php @@ -6,22 +6,28 @@ use DNADesign\Elemental\Models\BaseElement; use DNADesign\Elemental\Models\ElementalArea; use SilverStripe\EventDispatcher\Symfony\Event; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Handler\Elemental\SortElementsHandler; use SilverStripe\Snapshots\Tests\SnapshotTest\BlockPage; use SilverStripe\Snapshots\Tests\SnapshotTestAbstract; class SortElementsHandlerTest extends SnapshotTestAbstract { + /** + * @var array + */ + protected static $required_extensions = [ + BlockPage::class => [ + ElementalPageExtension::class, + ], + ]; - protected function setUp() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { - parent::setUp(); - BlockPage::add_extension(ElementalPageExtension::class); - } - - public function testHandlerDoesntFire() - { - $handler = new SortElementsHandler(); + $handler = SortElementsHandler::create(); $mock = $this->mockSnapshot(); $mock ->expects($this->never()) @@ -47,19 +53,25 @@ public function testHandlerDoesntFire() $context = Event::create( 'action', [ - 'params' => ['blockId' => 5] + 'params' => ['blockId' => 5], ] ); $handler->fire($context); } - public function testHandlerDoesFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesFire(): void { - $handler = new SortElementsHandler(); + $handler = SortElementsHandler::create(); $area = ElementalArea::create(); $area->write(); - $block = BaseElement::create(['ParentID' => $area->ID]); + + $block = BaseElement::create(); + $block->ParentID = $area->ID; $block->write(); + $mock = $this->mockSnapshot(); $mock ->expects($this->once()) @@ -69,13 +81,13 @@ public function testHandlerDoesFire() $mock ->expects($this->once()) ->method('addOwnershipChain') - ->with($this->callback(function ($arg) use ($area) { + ->with($this->callback(static function ($arg) use ($area) { return $arg instanceof ElementalArea && $arg->ID == $area->ID; })); $context = Event::create('action', [ 'params' => [ - 'blockId' => $block->ID + 'blockId' => $block->ID, ], ]); diff --git a/tests/Handler/GraphQL/FakePageContextProvider.php b/tests/Handler/GraphQL/FakePageContextProvider.php index 6d7db84..1c82845 100644 --- a/tests/Handler/GraphQL/FakePageContextProvider.php +++ b/tests/Handler/GraphQL/FakePageContextProvider.php @@ -1,6 +1,5 @@ registerService( - new FakePageContextProvider(), + FakePageContextProvider::create(), PageContextProvider::class ); } - public function testHandlerDoesntFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { $handler = Handler::create(); $this->mockSnapshot() @@ -39,7 +39,10 @@ public function testHandlerDoesntFire() $handler->fire($context); } - public function testHandlerDoesFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesFire(): void { $handler = Handler::create(); $blockPage = SiteTree::create(); @@ -50,7 +53,7 @@ public function testHandlerDoesFire() $this->mockSnapshot() ->expects($this->once()) ->method('createSnapshot') - ->with($this->callback(function ($arg) use ($blockPage) { + ->with($this->callback(static function ($arg) use ($blockPage) { return $arg instanceof SiteTree && $arg->ID == $blockPage->ID; })); diff --git a/tests/Handler/GraphQL/Middleware/RollbackHandlerTest.php b/tests/Handler/GraphQL/Middleware/RollbackHandlerTest.php index 2a8f481..b0245c7 100644 --- a/tests/Handler/GraphQL/Middleware/RollbackHandlerTest.php +++ b/tests/Handler/GraphQL/Middleware/RollbackHandlerTest.php @@ -5,12 +5,16 @@ use GraphQL\Executor\ExecutionResult; use SilverStripe\CMS\Model\SiteTree; use SilverStripe\EventDispatcher\Symfony\Event; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Handler\GraphQL\Middleware\RollbackHandler; use SilverStripe\Snapshots\Tests\SnapshotTestAbstract; class RollbackHandlerTest extends SnapshotTestAbstract { - public function testHandlerDoesntFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { $handler = RollbackHandler::create(); $this->mockSnapshot() @@ -30,7 +34,7 @@ public function testHandlerDoesntFire() 'params' => [ 'id' => 5, 'toVersion' => 8, - ] + ], ]); $handler->fire($context); @@ -42,13 +46,16 @@ public function testHandlerDoesntFire() 'params' => [ 'id' => $page->ID, 'toVersion' => $currentVersion * 100, - ] + ], ]); $handler->fire($context); } - public function testHandlerDoesFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesFire(): void { $handler = RollbackHandler::create(); diff --git a/tests/Handler/GraphQL/Mutation/HandlerTest.php b/tests/Handler/GraphQL/Mutation/HandlerTest.php index 3039b78..5096eaf 100644 --- a/tests/Handler/GraphQL/Mutation/HandlerTest.php +++ b/tests/Handler/GraphQL/Mutation/HandlerTest.php @@ -12,7 +12,7 @@ class HandlerTest extends SnapshotTestAbstract { - protected function setUp() + protected function setUp(): void { parent::setUp(); Injector::inst()->registerService( diff --git a/tests/Handler/GridField/Action/HandlerTest.php b/tests/Handler/GridField/Action/HandlerTest.php index a9107fc..b3dee17 100644 --- a/tests/Handler/GridField/Action/HandlerTest.php +++ b/tests/Handler/GridField/Action/HandlerTest.php @@ -7,13 +7,17 @@ use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\GridField\GridField; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Handler\GridField\Action\Handler; use SilverStripe\Snapshots\Tests\SnapshotTest\Block; use SilverStripe\Snapshots\Tests\SnapshotTestAbstract; class HandlerTest extends SnapshotTestAbstract { - public function testHandlerDoesntFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { $handler = Handler::create(); $this->mockSnapshot() @@ -34,7 +38,10 @@ public function testHandlerDoesntFire() $handler->fire($context); } - public function testHandlerDoesFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesFire(): void { $handler = Handler::create(); $block = Block::create(); @@ -43,7 +50,7 @@ public function testHandlerDoesFire() $this->mockSnapshot() ->expects($this->once()) ->method('createSnapshot') - ->with($this->callback(function ($arg) use ($block) { + ->with($this->callback(static function ($arg) use ($block) { return $arg instanceof Block && $arg->ID == $block->ID; })); diff --git a/tests/Handler/GridField/Action/ReorderHandlerTest.php b/tests/Handler/GridField/Action/ReorderHandlerTest.php index 69e94b7..4ef7495 100644 --- a/tests/Handler/GridField/Action/ReorderHandlerTest.php +++ b/tests/Handler/GridField/Action/ReorderHandlerTest.php @@ -14,7 +14,7 @@ class ReorderHandlerTest extends SnapshotTestAbstract { - public function testHandlerDoesFire() + public function testHandlerDoesFire(): void { $handler = ReorderHandler::create(); $block = Block::create(); @@ -25,9 +25,9 @@ public function testHandlerDoesFire() $mock ->expects($this->once()) ->method('applyOrigin') - ->with($this->callback(function ($arg) use ($block) { + ->with($this->callback(static function ($arg) use ($block) { return $arg instanceof SnapshotEvent && - $arg->Title == 'Reordered ' . $block->i18n_plural_name(); + $arg->Title === 'Reordered ' . $block->i18n_plural_name(); })); $form = Form::create(Controller::create(), 'TestForm', FieldList::create(), FieldList::create()) diff --git a/tests/Handler/GridField/Alteration/HandlerTest.php b/tests/Handler/GridField/Alteration/HandlerTest.php index d30a8d0..672470d 100644 --- a/tests/Handler/GridField/Alteration/HandlerTest.php +++ b/tests/Handler/GridField/Alteration/HandlerTest.php @@ -7,13 +7,17 @@ use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\GridField\GridField; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Handler\GridField\Alteration\Handler; use SilverStripe\Snapshots\Tests\SnapshotTest\Block; use SilverStripe\Snapshots\Tests\SnapshotTestAbstract; class HandlerTest extends SnapshotTestAbstract { - public function testHandlerDoesntFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesntFire(): void { $handler = Handler::create(); $this->mockSnapshot() @@ -46,7 +50,10 @@ public function testHandlerDoesntFire() $handler->fire($context); } - public function testHandlerDoesFire() + /** + * @throws ValidationException + */ + public function testHandlerDoesFire(): void { $handler = Handler::create(); $block = Block::create(); @@ -55,7 +62,7 @@ public function testHandlerDoesFire() $this->mockSnapshot() ->expects($this->once()) ->method('createSnapshot') - ->with($this->callback(function ($arg) use ($block) { + ->with($this->callback(static function ($arg) use ($block) { return $arg instanceof Block && $arg->ID == $block->ID; })); diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index 7fdd097..618be12 100644 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -1,10 +1,8 @@ set( @@ -70,7 +79,11 @@ protected function setUp() ); } - public function testFundamentals() + /** + * @throws ValidationException + * @throws Exception + */ + public function testFundamentals(): void { // Model: // BlockPage @@ -78,14 +91,16 @@ public function testFundamentals() // -> (has_many/owns) -> Gallery // -> (many_many/owns) -> GalleryImage - /* @var DataObject|SnapshotPublishable $a1 */ - $a1 = new BlockPage(['Title' => 'A1 Block Page']); + /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = BlockPage::create(); + $a1->Title = 'A1 Block Page'; $this->editingPage($a1); $this->formSaveObject($a1); $this->formPublishObject($a1); - /* @var DataObject|SnapshotPublishable $a2 */ - $a2 = new BlockPage(['Title' => 'A2 Block Page']); + /** @var DataObject|SnapshotPublishable $a2 */ + $a2 = BlockPage::create(); + $a2->Title = 'A2 Block Page'; $this->editingPage($a2); $this->formSaveObject($a2); $this->formPublishObject($a2); @@ -93,11 +108,16 @@ public function testFundamentals() // A1 Block page edits $this->editingPage($a1); - /* @var DataObject|SnapshotPublishable $a1Block1 */ - $a1Block1 = new Block(['Title' => 'Block 1 on A1', 'ParentID' => $a1->ID]); - + /** @var Block|SnapshotPublishable $a1Block1 */ + $a1Block1 = Block::create(); + $a1Block1->Title = 'Block 1 on A1'; + $a1Block1->ParentID = $a1->ID; $this->formSaveObject($a1Block1); - $a1Block2 = new Block(['Title' => 'Block 2 on A1', 'ParentID' => $a1->ID]); + + /** @var Block|SnapshotPublishable $a1Block2 */ + $a1Block2 = Block::create(); + $a1Block2->Title = 'Block 2 on A1'; + $a1Block2->ParentID = $a1->ID; $this->formSaveObject($a1Block2); // A1 @@ -106,8 +126,10 @@ public function testFundamentals() $this->editingPage($a2); - /* @var DataObject|SnapshotPublishable $a2Block1 */ - $a2Block1 = new Block(['Title' => 'Block 1 on A2', 'ParentID' => $a2->ID]); + /** @var Block|SnapshotPublishable $a2Block1 */ + $a2Block1 = Block::create(); + $a2Block1->Title = 'Block 1 on A2'; + $a2Block1->ParentID = $a2->ID; $this->formSaveObject($a2Block1); // A1 @@ -160,15 +182,17 @@ public function testFundamentals() [ $a1Block1, $a1Block2, - $a1 + $a1, ] ); $this->editingPage($a1); // Testing third level - /* @var DataObject|SnapshotPublishable|Versioned $gallery1 */ - $gallery1 = new Gallery(['Title' => 'Gallery 1 on Block 1 on A1', 'BlockID' => $a1Block1->ID]); + /** @var Gallery|SnapshotPublishable|Versioned $gallery1 */ + $gallery1 = Gallery::create(); + $gallery1->Title = 'Gallery 1 on Block 1 on A1'; + $gallery1->BlockID = $a1Block1->ID; $this->formSaveObject($gallery1); // A1 (draft, modified) @@ -253,10 +277,13 @@ public function testFundamentals() ); // Testing many_many - /* @var DataObject|SnapshotPublishable $galleryItem1 */ - $galleryItem1 = new GalleryImage(['URL' => '/gallery/image/1']); - /* @var DataObject|SnapshotPublishable $galleryItem2 */ - $galleryItem2 = new GalleryImage(['URL' => '/gallery/image/2']); + /** @var GalleryImage|SnapshotPublishable $galleryItem1 */ + $galleryItem1 = GalleryImage::create(); + $galleryItem1->URL = '/gallery/image/1'; + + /** @var GalleryImage|SnapshotPublishable $galleryItem2 */ + $galleryItem2 = GalleryImage::create(); + $galleryItem2->URL = '/gallery/image/2'; $this->formSaveRelations($gallery1, 'Images', [$galleryItem1, $galleryItem2]); @@ -283,7 +310,8 @@ public function testFundamentals() [$gallery1, ActivityEntry::MODIFIED], [$galleryItem1, ActivityEntry::ADDED], [$galleryItem2, ActivityEntry::ADDED], - [ImplicitModification::singleton(), null], ] + [ImplicitModification::singleton(), null], + ], ); $this->assertPublishableObjectsContains( @@ -300,8 +328,10 @@ public function testFundamentals() $this->editingPage($a2); - /* @var DataObject|SnapshotPublishable $gallery1a */ - $gallery1a = new Gallery(['Title' => 'Gallery 1 on Block 1 on A2', 'BlockID' => $a2Block1->ID]); + /** @var Gallery|SnapshotPublishable $gallery1a */ + $gallery1a = Gallery::create(); + $gallery1a->Title = 'Gallery 1 on Block 1 on A2'; + $gallery1a->BlockID = $a2Block1->ID; $this->formSaveObject($gallery1a); $this->formSaveRelations($gallery1a, 'Images', [$galleryItem1]); @@ -454,11 +484,15 @@ public function testFundamentals() $this->assertEmpty($a2->getPublishableObjects()); } - public function testRevertChanges() + /** + * @throws ValidationException + * @throws Exception + */ + public function testRevertChanges(): void { - /* @var DataObject|SnapshotPublishable $a1 */ - /* @var DataObject|SnapshotPublishable $gallery1 */ - list ($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var DataObject|SnapshotPublishable $a1 */ + /** @var DataObject|SnapshotPublishable|Versioned $gallery1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $this->editingPage($a1); $this->formPublishObject($a1); @@ -493,14 +527,18 @@ public function testRevertChanges() $this->assertEmpty($a1->getActivityFeed()); } - public function testIntermediaryObjects() + /** + * @throws ValidationException + */ + public function testIntermediaryObjects(): void { - /* @var DataObject|SnapshotPublishable $a1 */ - /* @var DataObject|SnapshotPublishable $a2 */ - /* @var DataObject|SnapshotPublishable $a1Block1 */ - /* @var DataObject|SnapshotPublishable $gallery1 */ - list ($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var DataObject|SnapshotPublishable $a1 */ + /** @var DataObject|SnapshotPublishable $a2 */ + /** @var DataObject|SnapshotPublishable $a1Block1 */ + /** @var DataObject|SnapshotPublishable $gallery1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $this->publish($a1); $this->editingPage($a1); $gallery1->Title = 'Gallery 1 changed'; @@ -558,7 +596,7 @@ public function testIntermediaryObjects() $this->assertFalse($a1->hasOwnedModifications()); - $a1Block1->Title = "A1 is changed again"; + $a1Block1->Title = 'A1 is changed again'; $this->formSaveObject($a1Block1); // A1 (draft, modified) * @@ -571,14 +609,18 @@ public function testIntermediaryObjects() $this->assertTrue($a1->hasOwnedModifications()); } - public function testChangeOwnershipStructure() + /** + * @throws ValidationException + * @throws Exception + */ + public function testChangeOwnershipStructure(): void { - /* @var DataObject|SnapshotPublishable $a1 */ - /* @var DataObject|SnapshotPublishable $a2 */ - /* @var DataObject|SnapshotPublishable $a1Block1 */ - /* @var DataObject|SnapshotPublishable $gallery1 */ - /* @var DataObject|SnapshotPublishable $gallery2 */ - list ($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var DataObject|SnapshotPublishable $a1 */ + /** @var DataObject|SnapshotPublishable $a2 */ + /** @var Block|SnapshotPublishable $a1Block1 */ + /** @var DataObject|SnapshotPublishable $gallery1 */ + /** @var DataObject|SnapshotPublishable $gallery2 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $this->editingPage($a1); $this->formPublishObject($a1); @@ -671,13 +713,14 @@ public function testChangeOwnershipStructure() $this->editingPage($a2); - $blockMoved->Title = "The moved block is modified"; + $blockMoved->Title = 'The moved block is modified'; $this->formSaveObject($blockMoved); - $gallery1->Title = "The gallery that belongs to the moved block is modified"; + $gallery1->Title = 'The gallery that belongs to the moved block is modified'; $this->formSaveObject($gallery1); - $item = new GalleryImage(['URL' => '/belongs/to/moved/block']); + $item = GalleryImage::create(); + $item->URL = '/belongs/to/moved/block'; $item->write(); $this->formSaveRelations($gallery1, 'Images', [$item]); @@ -706,6 +749,7 @@ public function testChangeOwnershipStructure() // Move the block back to A1 // Refresh the block so that changed fields flushes + /** @var Block $blockMoved */ $blockMoved = DataObject::get_by_id(Block::class, $blockMoved->ID, false); $blockMoved->ParentID = $a1->ID; @@ -795,13 +839,17 @@ public function testChangeOwnershipStructure() ]); } - public function testPartialActivityMigration() + /** + * @throws ValidationException + * @throws Exception + */ + public function testPartialActivityMigration(): void { - /* @var DataObject|SnapshotPublishable $a1 */ - /* @var DataObject|SnapshotPublishable $a2 */ - /* @var DataObject|SnapshotPublishable $a1Block1 */ - /* @var DataObject|SnapshotPublishable $a1Block2 */ - list ($a1, $a2, $a1Block1, $a1Block2) = $this->buildState(); + /** @var DataObject|SnapshotPublishable $a1 */ + /** @var DataObject|SnapshotPublishable $a2 */ + /** @var Block|SnapshotPublishable $a1Block1 */ + /** @var Block|SnapshotPublishable $a1Block2 */ + [$a1, $a2, $a1Block1, $a1Block2] = $this->buildState(); // Test that we can transplant a node and relevant activity will be migrated // but unrelated activity will be preserved. @@ -815,7 +863,9 @@ public function testPartialActivityMigration() $a1Block2->Title = 'You got this'; $this->formSaveObject($a1Block2); - $gallery = new Gallery(['Title' => 'A new gallery for block 2', 'BlockID' => $a1Block2->ID]); + $gallery = Gallery::create(); + $gallery->Title = 'A new gallery for block 2'; + $gallery->BlockID = $a1Block2->ID; $this->formSaveObject($gallery); // A1 (published) @@ -866,15 +916,19 @@ public function testPartialActivityMigration() ]); } - public function testDeletions() + /** + * @throws ValidationException + * @throws Exception + */ + public function testDeletions(): void { - /* @var DataObject|SnapshotPublishable $a1 */ - /* @var DataObject|SnapshotPublishable $a2 */ - /* @var DataObject|SnapshotPublishable $a1Block1 */ - /* @var DataObject|SnapshotPublishable $a1Block2 */ - /* @var DataObject|SnapshotPublishable $a2Block1 */ - /* @var DataObject|SnapshotPublishable $gallery2 */ - list ($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var DataObject|SnapshotPublishable $a1 */ + /** @var DataObject|SnapshotPublishable $a2 */ + /** @var DataObject|SnapshotPublishable $a1Block1 */ + /** @var DataObject|SnapshotPublishable $a1Block2 */ + /** @var DataObject|SnapshotPublishable $a2Block1 */ + /** @var DataObject|SnapshotPublishable $gallery2 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $this->editingPage($a1); $this->formPublishObject($a1); @@ -940,17 +994,21 @@ public function testDeletions() ]); } - public function testGetAtSnapshot() + /** + * @throws ValidationException + * @throws Exception + */ + public function testGetAtSnapshot(): void { $stamp0 = $this->sleep(1); - /* @var DataObject|SnapshotPublishable $a1 */ - /* @var DataObject|SnapshotPublishable $a2 */ - /* @var DataObject|SnapshotPublishable $a1Block1 */ - /* @var DataObject|SnapshotPublishable $a1Block2 */ - /* @var DataObject|SnapshotPublishable $a2Block1 */ - /* @var DataObject|SnapshotPublishable $gallery2 */ - list ($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + /** @var BlockPage|SnapshotPublishable $a2 */ + /** @var Block|SnapshotPublishable $a1Block1 */ + /** @var Block|SnapshotPublishable $a1Block2 */ + /** @var Block|SnapshotPublishable $a2Block1 */ + /** @var DataObject|SnapshotPublishable $gallery2 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $this->editingPage($a1); $this->formPublishObject($a1); @@ -986,10 +1044,9 @@ public function testGetAtSnapshot() $this->editingPage($a2); - $a2Block2 = new Block([ - 'Title' => 'Block 2 on A2', - 'ParentID' => $a2->ID, - ]); + $a2Block2 = Block::create(); + $a2Block2->Title = 'Block 2 on A2'; + $a2Block2->ParentID = $a2->ID; $this->formSaveObject($a2Block2); @@ -1064,9 +1121,15 @@ public function testGetAtSnapshot() $this->assertCount(1, $oldA2->Blocks()); } - public function testWonkyOwner() + /** + * @throws ValidationException + * @throws Exception + */ + public function testWonkyOwner(): void { - $page = new BlockPage(['Title' => 'The Page']); + /** @var BlockPage|SnapshotPublishable $page */ + $page = BlockPage::create(); + $page->Title = 'The Page'; $this->editingPage($page); $this->formSaveObject($page); $this->formPublishObject($page); @@ -1074,7 +1137,9 @@ public function testWonkyOwner() // This block is saved in isolation $this->editingPage(null); - $block = new Block(['Title' => 'The Block', 'ParentID' => 0]); + $block = Block::create(); + $block->Title = 'The Block'; + $block->ParentID = 0; $block->write(); $block->ParentID = $page->ID; @@ -1086,21 +1151,28 @@ public function testWonkyOwner() $this->assertActivityContains( $activity, [ - [$block, ActivityEntry::MODIFIED] + [$block, ActivityEntry::MODIFIED], ] ); } - public function testChangeToUnpublishedOwner() + /** + * @throws ValidationException + * @throws Exception + */ + public function testChangeToUnpublishedOwner(): void { - $page = new BlockPage(['Title' => 'The Page']); + /** @var BlockPage|SnapshotPublishable $page */ + $page = BlockPage::create(); + $page->Title = 'The Page'; $this->editingPage($page); $this->formSaveObject($page); $this->editingPage(null); - $block = new Block(['Title' => 'The Block']); + $block = Block::create(['Title' => 'The Block']); + $block->Title = 'The Block'; $block->write(); $this->editingPage($page); @@ -1114,21 +1186,31 @@ public function testChangeToUnpublishedOwner() $activity, [ [$page, ActivityEntry::CREATED], - [$block, ActivityEntry::MODIFIED] + [$block, ActivityEntry::MODIFIED], ] ); } - public function testMany() + /** + * @throws ValidationException + * @throws Exception + */ + public function testMany(): void { - $p = new BlockPage(['Title' => 'The Page']); + /** @var BlockPage|SnapshotPublishable $p */ + $p = BlockPage::create(); + $p->Title = 'The Page'; $this->editingPage($p); $this->formSaveObject($p); - $b = new Block(['Title' => 'The Block on The Page', 'ParentID' => $p->ID]); + $b = Block::create(); + $b->Title = 'The Block on The Page'; + $b->ParentID = $p->ID; $this->formSaveObject($b); - $g = new Gallery(['Title' => 'The Gallery on The Block on The Page', 'BlockID' => $b->ID]); + $g = Gallery::create(); + $g->Title = 'The Gallery on The Block on The Page'; + $g->BlockID = $b->ID; $this->formSaveObject($g); $this->formPublishObject($p); @@ -1137,7 +1219,8 @@ public function testMany() $this->assertCount(0, $p->getActivityFeed()); $this->assertCount(0, $p->getPublishableObjects()); - $i = new GalleryImage(['URL' => '/gallery/image/1']); + $i = GalleryImage::create(); + $i->URL = '/gallery/image/1'; $this->formSaveRelations($g, 'Images', [$i]); $activity = $p->getActivityFeed(); @@ -1146,14 +1229,19 @@ public function testMany() $activity, [ [$i, ActivityEntry::ADDED, $g], - [ImplicitModification::singleton(), null] + [ImplicitModification::singleton(), null], ] ); } - public function testPlainActivityFeed() + /** + * @throws ValidationException + * @throws Exception + */ + public function testPlainActivityFeed(): void { - $page = new BlockPage(); + /** @var BlockPage|SnapshotPublishable $page */ + $page = BlockPage::create(); $this->editingPage($page); $page->Title = 'The Page -- version 1'; $this->formSaveObject($page); @@ -1232,7 +1320,7 @@ public function testPlainActivityFeed() [$page, ActivityEntry::MODIFIED], [$page, ActivityEntry::PUBLISHED], [$page, ActivityEntry::MODIFIED], - [$page, ActivityEntry::MODIFIED] + [$page, ActivityEntry::MODIFIED], ] ); @@ -1263,16 +1351,26 @@ public function testPlainActivityFeed() ); } - public function testNestedActivityFeed() + /** + * @throws ValidationException + * @throws Exception + */ + public function testNestedActivityFeed(): void { - $p = new BlockPage(['Title' => 'Page -- v01']); + /** @var BlockPage|SnapshotPublishable $p */ + $p = BlockPage::create(); + $p->Title = 'Page -- v01'; $this->editingPage($p); $this->formSaveObject($p); - $b = new Block(['Title' => 'Block -- v01', 'ParentID' => $p->ID]); + $b = Block::create(); + $b->Title = 'Block -- v01'; + $b->ParentID = $p->ID; $this->formSaveObject($b); - $g = new Gallery(['Title' => 'Gallery -- v01', 'BlockID' => $b->ID]); + $g = Gallery::create(); + $g->Title = 'Gallery -- v01'; + $g->BlockID = $b->ID; $this->formSaveObject($g); $this->formPublishObject($p); @@ -1281,7 +1379,8 @@ public function testNestedActivityFeed() $this->assertCount(0, $p->getActivityFeed()); $this->assertCount(0, $p->getPublishableObjects()); - $i = new GalleryImage(['URL' => '/gallery/image/1']); + $i = GalleryImage::create(); + $i->URL = '/gallery/image/1'; $this->formSaveRelations($g, 'Images', [$i]); $activity = $p->getActivityFeed(); @@ -1307,42 +1406,48 @@ public function testNestedActivityFeed() $this->assertActivityContains( $activity, [ - [$i, ActivityEntry::MODIFIED] + [$i, ActivityEntry::MODIFIED], ] ); $this->formPublishObject($p); $a = $p->getActivityFeed(2); - $this->assertCount(6, $a); + $this->assertCount(8, $a); $this->assertActivityContains($a, [ [$i, ActivityEntry::ADDED], [ImplicitModification::singleton(), null], [$b, ActivityEntry::MODIFIED], - [$p, ActivityEntry::PUBLISHED], + [$b, ActivityEntry::PUBLISHED], + [ImplicitModification::singleton(), null], [$i, ActivityEntry::MODIFIED], - [$p, ActivityEntry::PUBLISHED], + [$b, ActivityEntry::PUBLISHED], + [ImplicitModification::singleton(), null], ]); $a = $p->getActivityFeed(2, 4); - $this->assertCount(6, $a); + $this->assertCount(8, $a); + $this->assertActivityContains($a, [ [$i, ActivityEntry::ADDED, $g], [ImplicitModification::singleton(), null], [$b, ActivityEntry::MODIFIED], - [$p, ActivityEntry::PUBLISHED], + [$b, ActivityEntry::PUBLISHED], + [ImplicitModification::singleton(), null], [$i, ActivityEntry::MODIFIED], - [$p, ActivityEntry::PUBLISHED], + [$b, ActivityEntry::PUBLISHED], + [ImplicitModification::singleton(), null], ]); $a = $p->getActivityFeed(2, 3); - $this->assertCount(5, $a); + $this->assertCount(6, $a); $this->assertActivityContains($a, [ [$i, ActivityEntry::ADDED, $g], [ImplicitModification::singleton(), null], [$b, ActivityEntry::MODIFIED], - [$p, ActivityEntry::PUBLISHED], - [$i, ActivityEntry::MODIFIED] + [$b, ActivityEntry::PUBLISHED], + [ImplicitModification::singleton(), null], + [$i, ActivityEntry::MODIFIED], ]); } @@ -1350,14 +1455,16 @@ public function testNestedActivityFeed() * @param ArrayList $activity * @param array $objs */ - private function assertActivityContains(ArrayList $activity, $objs = []) + private function assertActivityContains(ArrayList $activity, array $objs = []): void { foreach ($activity as $i => $entry) { - /* @var DataObject|SnapshotPublishable $obj */ - list ($obj, $action) = $objs[$i]; + /** @var DataObject|SnapshotPublishable $obj */ + [$obj, $action] = $objs[$i]; + if ($obj instanceof SnapshotEvent) { continue; } + $expectedHash = $obj->isInDB() ? SnapshotPublishable::hashObjectForSnapshot($obj) : SnapshotPublishable::hashForSnapshot($obj->ClassName, $obj->OldID); @@ -1373,12 +1480,13 @@ private function assertActivityContains(ArrayList $activity, $objs = []) * @param ArrayList $items * @param array $objs */ - private function assertPublishableObjectsContains(ArrayList $items, $objs = []) + private function assertPublishableObjectsContains(ArrayList $items, array $objs = []): void { foreach ($items as $i => $dataObject) { if ($dataObject instanceof SnapshotEvent) { continue; } + $obj= $objs[$i]; $expectedHash = $obj->isInDB() ? SnapshotPublishable::hashObjectForSnapshot($obj) @@ -1390,9 +1498,10 @@ private function assertPublishableObjectsContains(ArrayList $items, $objs = []) } } - private function debugActivity($activity) + private function debugActivity($activity): string { $list = []; + foreach ($activity as $entry) { $list[] = sprintf( '[%s] %s #%s (%s)', @@ -1403,12 +1512,13 @@ private function debugActivity($activity) ); } - return implode("\n", $list); + return implode(PHP_EOL, $list); } - private function debugPublishable($items) + private function debugPublishable($items): string { $list = []; + foreach ($items as $item) { $list[] = sprintf( '%s #%s (%s)', @@ -1418,19 +1528,29 @@ private function debugPublishable($items) ); } - return implode("\n", $list); + return implode(PHP_EOL, $list); } - private function formSaveObject(DataObject $object) + /** + * @param DataObject $object + * @throws ValidationException + */ + private function formSaveObject(DataObject $object): void { $object->write(); - $actionName = $object instanceof SiteTree ? 'save' : 'doSave'; + $actionName = $object instanceof SiteTree + ? 'save' + : 'doSave'; $event = $this->createEvent($object, $actionName); $this->dispatch($event); $this->sleep(3); } - private function formPublishObject(DataObject $object) + /** + * @param DataObject|RecursivePublishable $object + * @throws ValidationException + */ + private function formPublishObject(DataObject $object): void { $object->write(); $object->publishRecursive(); @@ -1439,7 +1559,10 @@ private function formPublishObject(DataObject $object) $this->sleep(3); } - private function formUnpublishObject(DataObject $object) + /** + * @param DataObject|Versioned $object + */ + private function formUnpublishObject(DataObject $object): void { $object->doUnpublish(); $event = $this->createEvent($object, 'unpublish'); @@ -1447,7 +1570,10 @@ private function formUnpublishObject(DataObject $object) $this->sleep(3); } - private function formDeleteObject(DataObject $object) + /** + * @param DataObject|Versioned $object + */ + private function formDeleteObject(DataObject $object): void { $object->doArchive(); $event = $this->createEvent($object, 'doDelete'); @@ -1458,24 +1584,33 @@ private function formDeleteObject(DataObject $object) /** * Relation saves need to be wrapped in NOW() increments because they rely on * timestamp driven history + * * @param DataObject $object * @param string $component * @param DataObject[] $items * @param string $type */ - private function formSaveRelations(DataObject $object, $component, array $items, $type = ActivityEntry::ADDED) - { + private function formSaveRelations( + DataObject $object, + string $component, + array $items, + string $type = ActivityEntry::ADDED + ): void { $this->sleep(2); - $method = $type === ActivityEntry::ADDED ? 'add' : 'remove'; + $method = $type === ActivityEntry::ADDED + ? 'add' + : 'remove'; + foreach ($items as $item) { $object->$component()->$method($item); } + $event = $this->createEvent($object, 'doSave'); $this->dispatch($event); $this->sleep(2); } - private function dispatch(EventContextInterface $event) + private function dispatch(EventContextInterface $event): void { Dispatcher::singleton()->trigger(Listener::EVENT_NAME, $event); } @@ -1485,6 +1620,7 @@ private function createEvent(DataObject $object, string $actionName): Event if (!$this->currentPage) { return Event::create($actionName); } + $form = Form::create( CMSPageEditController::singleton(), 'EditForm', @@ -1506,7 +1642,7 @@ private function createEvent(DataObject $object, string $actionName): Event ); } - private function editingPage(?DataObject $page = null) + private function editingPage(?DataObject $page = null): void { $this->currentPage = $page; } diff --git a/tests/PageContextProviderTest.php b/tests/PageContextProviderTest.php index ff5daf6..432d5c3 100644 --- a/tests/PageContextProviderTest.php +++ b/tests/PageContextProviderTest.php @@ -1,6 +1,5 @@ page->ID, $testURL); $result = $provider->getCurrentPageFromRequestUrl($url); + if ($shouldSucceed) { $this->assertInstanceOf(SiteTree::class, $result); $this->assertEquals($this->page->ID, $result->ID); @@ -41,7 +47,7 @@ public function testPageFromURL(string $testURL, bool $shouldSucceed) } } - public function dataProvider() + public function dataProvider(): array { // This is called before the database is ready, so [ID] is used as a placeholder return [ diff --git a/tests/RelationDifferTest.php b/tests/RelationDifferTest.php index a791670..9489ec2 100644 --- a/tests/RelationDifferTest.php +++ b/tests/RelationDifferTest.php @@ -3,24 +3,28 @@ namespace SilverStripe\Snapshots\Tests; use SilverStripe\Dev\SapphireTest; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\RelationDiffer; use SilverStripe\Snapshots\Tests\SnapshotTest\Block; class RelationDifferTest extends SapphireTest { + /** + * @var array + */ protected static $extra_dataobjects = [ Block::class, ]; - public function testDiffRemoved() + public function testDiffRemoved(): void { - $differ = new RelationDiffer( + $differ = RelationDiffer::create( Block::class, 'has_many', [ '1' => 5, '2' => 12, - '5' => 8 + '5' => 8, ], [] ); @@ -30,16 +34,16 @@ public function testDiffRemoved() $this->assertEquals(['1','2','5'], $differ->getRemoved()); } - public function testDiffAdded() + public function testDiffAdded(): void { - $differ = new RelationDiffer( + $differ = RelationDiffer::create( Block::class, 'has_many', [], [ '1' => 5, '2' => 12, - '5' => 8 + '5' => 8, ] ); $this->assertTrue($differ->hasChanges()); @@ -48,9 +52,9 @@ public function testDiffAdded() $this->assertEquals(['1','2','5'], $differ->getAdded()); } - public function testDiffChanged() + public function testDiffChanged(): void { - $differ = new RelationDiffer( + $differ = RelationDiffer::create( Block::class, 'has_many', [ @@ -76,9 +80,9 @@ public function testDiffChanged() $this->assertEquals(['1','9','16'], $changed); } - public function testDiffMixed() + public function testDiffMixed(): void { - $differ = new RelationDiffer( + $differ = RelationDiffer::create( Block::class, 'has_many', [ @@ -115,9 +119,9 @@ public function testDiffMixed() $this->assertFalse($differ->isChanged(16)); } - public function testNoChanges() + public function testNoChanges(): void { - $differ = new RelationDiffer( + $differ = RelationDiffer::create( Block::class, 'has_many', [ @@ -146,7 +150,10 @@ public function testNoChanges() $this->assertEmpty($differ->getRemoved()); } - public function testGetRecords() + /** + * @throws ValidationException + */ + public function testGetRecords(): void { $block1 = Block::create(); $block1->write(); @@ -155,7 +162,7 @@ public function testGetRecords() $block3 = Block::create(); $block3->write(); - $differ = new RelationDiffer( + $differ = RelationDiffer::create( Block::class, 'has_many', [ @@ -166,20 +173,22 @@ public function testGetRecords() $block2->ID => $block2->Version, ] ); + $this->assertTrue($differ->hasChanges()); $this->assertCount(3, $differ->getRecords()); $expected = [$block1->ID, $block2->ID, $block3->ID]; sort($expected); - $actual = array_map(function ($record) { + + $actual = array_map(static function ($record) { return $record->ID; }, $differ->getRecords()); sort($actual); $this->assertEquals($expected, $actual); } - public function testException() + public function testException(): void { $this->expectException('InvalidArgumentException'); - new RelationDiffer(static::class, 'test'); + RelationDiffer::create(static::class, 'test'); } } diff --git a/tests/SnapshotItemTest.php b/tests/SnapshotItemTest.php index d325c9d..e57eb65 100644 --- a/tests/SnapshotItemTest.php +++ b/tests/SnapshotItemTest.php @@ -2,20 +2,27 @@ namespace SilverStripe\Snapshots\Tests; +use Exception; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\SnapshotHasher; use SilverStripe\Snapshots\SnapshotItem; use SilverStripe\Snapshots\Tests\SnapshotTest\Block; +use SilverStripe\Versioned\Versioned; class SnapshotItemTest extends SnapshotTestAbstract { - public function testGetItem() + /** + * @throws ValidationException + */ + public function testGetItem(): void { + /** @var Block|Versioned $block */ $block = Block::create(); $block->write(); $item = SnapshotItem::create([ 'ObjectClass' => Block::class, 'ObjectID' => $block->ID, - 'Version' => $block->Version * 100 + 'Version' => $block->Version * 100, ]); $this->assertNull($item->getItem()); @@ -27,8 +34,13 @@ public function testGetItem() $this->assertEquals($block->Version, $item->getItem()->Version); } - public function testHydration() + /** + * @throws ValidationException + * @throws Exception + */ + public function testHydration(): void { + /** @var Block|Versioned $block */ $block = Block::create(); $block->write(); diff --git a/tests/SnapshotPublishableTest.php b/tests/SnapshotPublishableTest.php index bf4e295..31e6cba 100644 --- a/tests/SnapshotPublishableTest.php +++ b/tests/SnapshotPublishableTest.php @@ -4,17 +4,20 @@ use SilverStripe\Core\Config\Config; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\Snapshot; use SilverStripe\Snapshots\SnapshotPublishable; use SilverStripe\Snapshots\Tests\SnapshotTest\Block; use SilverStripe\Snapshots\Tests\SnapshotTest\BlockPage; +use SilverStripe\Snapshots\Tests\SnapshotTest\Gallery; use SilverStripe\Versioned\Versioned; class SnapshotPublishableTest extends SnapshotTestAbstract { - public function testGetAtSnapshot() + public function testGetAtSnapshot(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage $a1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $firstSnapshot = Snapshot::get()->sort('Created ASC')->first(); $result = SnapshotPublishable::get_at_snapshot(BlockPage::class, $a1->ID, $firstSnapshot->Created); $param = $result->getSourceQueryParam('Versioned.date'); @@ -22,9 +25,13 @@ public function testGetAtSnapshot() $this->assertEquals($firstSnapshot->Created, $param); } - public function testGetAtLastSnapshot() + /** + * @throws ValidationException + */ + public function testGetAtLastSnapshot(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage $a1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $a1->Title = 'changed'; $a1->write(); @@ -33,27 +40,31 @@ public function testGetAtLastSnapshot() $this->assertEquals('A1 Block Page', $result->Title); } - public function testGetLastSnapshotItem() + public function testGetLastSnapshotItem(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|Versioned $a1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $a1->Title = 'changed'; $this->snapshot($a1); + /** @var DataObject|Versioned $result */ $result = SnapshotPublishable::get_last_snapshot_item(BlockPage::class, $a1->ID); $this->assertNotNull($result); $this->assertEquals($a1->Version, $result->Version); } - public function testGetSnapshots() + public function testGetSnapshots(): void { - $arr = $this->buildState(); + $state = $this->buildState(); $snapshots = SnapshotPublishable::getSnapshots(); - $this->assertOrigins($snapshots, $arr); + $this->assertOrigins($snapshots, $state); } - public function testGetRelevantSnapshots() + public function testGetRelevantSnapshots(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + /** @var BlockPage|SnapshotPublishable $a2 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $a1->Title = 'changed'; $this->snapshot($a1); @@ -80,14 +91,20 @@ public function testGetRelevantSnapshots() [ $a2, $a2Block1, - $gallery2 + $gallery2, ] ); } - public function testGetSnapshotsSinceVersion() + public function testGetSnapshotsSinceVersion(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + /** @var Block $a1Block1 */ + /** @var Block $a1Block2 */ + /** @var Block $a2Block1 */ + /** @var Gallery $gallery1 */ + /** @var Gallery $gallery2 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $this->publish($a1); $fromVersion = Versioned::get_versionnumber_by_stage(BlockPage::class, Versioned::LIVE, $a1->ID); @@ -128,9 +145,13 @@ public function testGetSnapshotsSinceVersion() ); } - public function testHasOwnedModifications() + public function testHasOwnedModifications(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + /** @var BlockPage|SnapshotPublishable $a2 */ + /** @var Block $a1Block1 */ + /** @var Block $a1Block2 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $this->publish($a1); $this->publish($a2); @@ -147,9 +168,14 @@ public function testHasOwnedModifications() $this->assertFalse($a2->hasOwnedModifications()); } - public function testPublishableItems() + public function testPublishableItems(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + /** @var BlockPage|SnapshotPublishable $a2 */ + /** @var Block $a1Block1 */ + /** @var Block $a2Block1 */ + /** @var Gallery $gallery1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $this->publish($a1); $this->publish($a2); @@ -191,21 +217,26 @@ public function testPublishableItems() $this->assertEquals($classes, $a2->getPublishableObjects()->column('ClassName')); } - public function testGetRelationTracking() + public function testGetRelationTracking(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + /** @var Block|Versioned $a1Block1 */ + /** @var Block|Versioned $a1Block2 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $this->assertEmpty($a1->getRelationTracking()); Config::modify()->set(BlockPage::class, 'snapshot_relation_tracking', ['Blocks', 'Fail']); $result = $a1->getRelationTracking(); + $this->assertArrayHasKey('Blocks', $result); $this->assertArrayNotHasKey('Fail', $result); $this->assertEquals($a1Block1->Version, $result['Blocks'][$a1Block1->ID]); $this->assertEquals($a1Block2->Version, $result['Blocks'][$a1Block2->ID]); } - public function testPreviousSnapshotItem() + public function testPreviousSnapshotItem(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $a1->Title = 'changed'; $this->snapshot($a1); $version = Versioned::get_versionnumber_by_stage(BlockPage::class, Versioned::DRAFT, $a1->ID); @@ -214,9 +245,13 @@ public function testPreviousSnapshotItem() $this->assertHashCompare($a1, $item->getItem()); } - public function testPreviousSnapshot() + /** + * @throws ValidationException + */ + public function testPreviousSnapshot(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $a1->Title = 'changed'; $this->snapshot($a1); @@ -225,8 +260,10 @@ public function testPreviousSnapshot() $result = $a1->atPreviousSnapshot(function ($date) use ($a1) { $this->assertNotNull($date); + return DataObject::get_by_id(BlockPage::class, $a1->ID); }); + $this->assertNotNull($result); $this->assertEquals('changed', $result->Title); @@ -235,27 +272,42 @@ public function testPreviousSnapshot() $this->assertEquals('changed', $result->Title); } + /** + * @throws ValidationException + */ public function testIsModifiedSinceLastSnapshot() { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $this->assertFalse($a1->isModifiedSinceLastSnapshot()); $a1->Title = 'changed'; $a1->write(); + + /** @var BlockPage|SnapshotPublishable $obj */ $obj = DataObject::get_by_id(BlockPage::class, $a1->ID); $this->assertTrue($obj->isModifiedSinceLastSnapshot()); } - public function testGetIntermediaryObjects() + public function testGetIntermediaryObjects(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + /** @var Block|SnapshotPublishable $a1Block1 */ + /** @var Gallery|SnapshotPublishable $gallery1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $objs = $gallery1->getIntermediaryObjects(); $this->assertHashCompareList([$a1Block1, $a1], $objs); } - public function testGetRelationDiffs() + /** + * @throws ValidationException + */ + public function testGetRelationDiffs(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + /** @var Block|SnapshotPublishable $a1Block1 */ + /** @var Block|SnapshotPublishable $a1Block2 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); Config::modify()->set(BlockPage::class, 'snapshot_relation_tracking', ['Blocks']); $this->assertCount(1, $a1->getRelationDiffs()); @@ -305,9 +357,10 @@ public function testGetRelationDiffs() $this->assertEquals([$a1Block1->ID], $diff->getChanged()); } - public function testGetPreviousVersion() + public function testGetPreviousVersion(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $originalTitle = $a1->Title; $a1->Title = 'changed'; $this->snapshot($a1); diff --git a/tests/SnapshotTest.php b/tests/SnapshotTest.php index 1bf8016..e75ba86 100644 --- a/tests/SnapshotTest.php +++ b/tests/SnapshotTest.php @@ -1,10 +1,10 @@ write(); - $item = SnapshotItem::create([ - 'ObjectClass' => Block::class, - 'ObjectID' => 5, - 'SnapshotID' => $snapshot->ID, - ]); + + $item = SnapshotItem::create(); + $item->ObjectClass = Block::class; + $item->ObjectID = 5; + $item->SnapshotID = $snapshot->ID; $item->write(); - $item = SnapshotItem::create([ - 'ObjectClass' => Block::class, - 'ObjectID' => 7, - 'SnapshotID' => $snapshot->ID, - ]); + + $item = SnapshotItem::create(); + $item->ObjectClass = Block::class; + $item->ObjectID = 7; + $item->SnapshotID = $snapshot->ID; $item->write(); + $snapshot->OriginClass = $item->ObjectClass; $snapshot->OriginID = $item->ObjectID; $snapshot->write(); @@ -44,19 +52,29 @@ public function testGetOriginItem() $this->assertEquals($item->ObjectID, $origin->ObjectID); } - public function testAddObjectLimit() + /** + * @throws ValidationException + * @throws Exception + */ + public function testAddObjectLimit(): void { Config::modify()->set(Snapshot::class, 'item_limit', 5); $snapshot = Snapshot::create(); - for ($i = 0; $i < 10; $i++) { + + for ($i = 0; $i < 10; $i += 1) { $b = Block::create(); $b->write(); $snapshot->addObject($b); } + $this->assertCount(5, $snapshot->Items()); } - public function testAddObjectDuplication() + /** + * @throws ValidationException + * @throws Exception + */ + public function testAddObjectDuplication(): void { $snapshot = Snapshot::create(); $block1 = Block::create(); @@ -81,7 +99,11 @@ public function testAddObjectDuplication() $this->assertEquals($expected, $ids); } - public function testAddObjectAsSnapshotItem() + /** + * @throws ValidationException + * @throws Exception + */ + public function testAddObjectAsSnapshotItem(): void { $snapshot = Snapshot::create(); $block1 = Block::create(); @@ -109,32 +131,41 @@ public function testAddObjectAsSnapshotItem() $this->assertEquals($expected, $ids); } - public function testGetOriginVersion() + /** + * @throws ValidationException + */ + public function testGetOriginVersion(): void { $snapshot = Snapshot::create(); $snapshot->write(); + + /** @var Block|Versioned $block1 */ $block1 = Block::create(); $block1->write(); - $item = SnapshotItem::create([ - 'ObjectClass' => $block1->baseClass(), - 'ObjectID' => $block1->ID, - 'SnapshotID' => $snapshot->ID, - 'Version' => $block1->Version, - ]); + + $item = SnapshotItem::create(); + $item->ObjectClass = $block1->baseClass(); + $item->ObjectID = $block1->ID; + $item->SnapshotID = $snapshot->ID; + $item->Version = $block1->Version; $item->write(); - $block2 = Block::create(['Title' => 'Original title']); + + /** @var Block|Versioned $block2 */ + $block2 = Block::create(); + $block2->Title = 'Original title'; $block2->write(); + $expectedVersion = $block2->Version; $block2->Title = 'changed title'; $block2->write(); - $item = SnapshotItem::create([ - 'ObjectClass' => $block2->baseClass(), - 'ObjectID' => $block2->ID, - 'SnapshotID' => $snapshot->ID, - 'Version' => $expectedVersion, - ]); + $item = SnapshotItem::create(); + $item->ObjectClass = $block2->baseClass(); + $item->ObjectID = $block2->ID; + $item->SnapshotID = $snapshot->ID; + $item->Version = $expectedVersion; $item->write(); + $snapshot->OriginClass = $item->ObjectClass; $snapshot->OriginID = $item->ObjectID; $snapshot->write(); @@ -145,9 +176,16 @@ public function testGetOriginVersion() $this->assertEquals('Original title', $v->Title); } - public function testCreateSnapshotNoRelations() + /** + * @throws ValidationException + */ + public function testCreateSnapshotNoRelations(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage $a1 */ + /** @var Block $a1Block1 */ + /** @var Block $a2Block1 */ + /** @var Gallery $gallery1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $gallery1->Title = 'changed'; $snapshot = $this->snapshot($gallery1); $this->assertCount(3, $snapshot->Items()); @@ -196,9 +234,15 @@ public function testCreateSnapshotNoRelations() ); } - public function testCreateSnapshotWithImplicitModifications() + /** + * @throws ValidationException + */ + public function testCreateSnapshotWithImplicitModifications(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage $a1 */ + /** @var Block $a1Block1 */ + /** @var Gallery $gallery1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $image = GalleryImage::create(); $image->write(); @@ -265,7 +309,10 @@ public function testCreateSnapshotWithImplicitModifications() $this->assertRegExp('/Added ' . $name . '/', $origin->Title); } - public function testCreateSnapshotEvent() + /** + * @throws ValidationException + */ + public function testCreateSnapshotEvent(): void { $snapshot = Snapshot::singleton()->createSnapshotEvent('test event'); $snapshot->write(); @@ -290,14 +337,21 @@ public function testCreateSnapshotEvent() [ $event, $block1, - $block2 + $block2, ] ); } - public function testAddOwnershipChain() + /** + * @throws ValidationException + * @throws Exception + */ + public function testAddOwnershipChain(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage $a1 */ + /** @var Block $a1Block1 */ + /** @var Gallery $gallery1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); $snapshot = Snapshot::create(); $this->assertEmpty($snapshot->Items()); $snapshot->addOwnershipChain($gallery1); @@ -313,22 +367,28 @@ public function testAddOwnershipChain() ); } - public function testApplyOrigin() + /** + * @throws ValidationException + * @throws Exception + */ + public function testApplyOrigin(): void { $snapshot = Snapshot::create(); - $this->assertFalse($snapshot->getOriginItem()); + $this->assertNull($snapshot->getOriginItem()); $block = Block::create(); $block->write(); $snapshot->applyOrigin($block); - $this->assertNotFalse($snapshot->getOriginItem()); + $this->assertNotNull($snapshot->getOriginItem()); $item = $snapshot->getOriginItem()->getItem(); $this->assertNotNull($item); $this->assertHashCompare($item, $block); } - public function testIsLiveSnapshot() + public function testIsLiveSnapshot(): void { - list($a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2) = $this->buildState(); + /** @var BlockPage $a1 */ + /** @var Block $a1Block1 */ + [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); Snapshot::get()->removeAll(); SnapshotItem::get()->removeAll(); $this->assertCount(0, Snapshot::get()); @@ -340,13 +400,16 @@ public function testIsLiveSnapshot() $this->assertCount(4, Snapshot::get()); $liveSnapshots = []; + foreach (Snapshot::get() as $s) { - if ($s->getIsLiveSnapshot()) { - $liveSnapshots[] = $s; + if (!$s->getIsLiveSnapshot()) { + continue; } + + $liveSnapshots[] = $s; } - $this->assertCount(1, $liveSnapshots); + $this->assertCount(1, $liveSnapshots); $this->assertEquals(Snapshot::get()->max('ID'), $liveSnapshots[0]->ID); } } diff --git a/tests/SnapshotTest/BaseJoin.php b/tests/SnapshotTest/BaseJoin.php index fcd1eef..a526c66 100644 --- a/tests/SnapshotTest/BaseJoin.php +++ b/tests/SnapshotTest/BaseJoin.php @@ -11,6 +11,9 @@ */ class BaseJoin extends DataObject implements TestOnly { + /** + * @var array + */ private static $extensions = [ Versioned::class, ]; diff --git a/tests/SnapshotTest/Block.php b/tests/SnapshotTest/Block.php index 5f397ba..dd7704c 100644 --- a/tests/SnapshotTest/Block.php +++ b/tests/SnapshotTest/Block.php @@ -1,31 +1,56 @@ 'Varchar', ]; + /** + * @var array + */ private static $has_one = [ 'Parent' => BlockPage::class, ]; + /** + * @var array + */ private static $has_many = [ 'Galleries' => Gallery::class, ]; + /** + * @var array + */ private static $extensions = [ Versioned::class, ]; - private static $owns = [ 'Galleries' ]; + /** + * @var array + */ + private static $owns = [ + 'Galleries', + ]; + /** + * @var string + */ private static $table_name = 'SnapshotTest_Block'; } diff --git a/tests/SnapshotTest/BlockPage.php b/tests/SnapshotTest/BlockPage.php index a8b90d4..0b3622a 100644 --- a/tests/SnapshotTest/BlockPage.php +++ b/tests/SnapshotTest/BlockPage.php @@ -1,28 +1,47 @@ 'Varchar', ]; + /** + * @var array + */ private static $has_many = [ 'Blocks' => Block::class, ]; - private static $owns = [ 'Blocks' ]; + /** + * @var array + */ + private static $owns = [ + 'Blocks', + ]; + /** + * @var array + */ private static $extensions = [ Versioned::class, ]; + /** + * @var string + */ private static $table_name = 'SnapshotTest_BlockPage'; } diff --git a/tests/SnapshotTest/Gallery.php b/tests/SnapshotTest/Gallery.php index c5b9660..c61a302 100644 --- a/tests/SnapshotTest/Gallery.php +++ b/tests/SnapshotTest/Gallery.php @@ -1,35 +1,61 @@ 'Varchar', ]; + /** + * @var array + */ private static $has_one = [ 'Block' => Block::class, ]; + /** + * @var array + */ private static $many_many = [ 'Images' => [ 'through' => GalleryImageJoin::class, 'from' => 'Gallery', 'to' => 'Image', - ] + ], ]; + /** + * @var array + */ private static $extensions = [ Versioned::class, ]; - private static $owns = [ 'Images' ]; + /** + * @var array + */ + private static $owns = [ + 'Images', + ]; + /** + * @var string + */ private static $table_name = 'SnapshotTest_Gallery'; } diff --git a/tests/SnapshotTest/GalleryImage.php b/tests/SnapshotTest/GalleryImage.php index 017aaca..58ff288 100644 --- a/tests/SnapshotTest/GalleryImage.php +++ b/tests/SnapshotTest/GalleryImage.php @@ -1,25 +1,39 @@ 'Varchar', ]; + /** + * @var array + */ private static $belongs_many_many = [ 'Gallery' => Gallery::class, ]; + /** + * @var array + */ private static $extensions = [ Versioned::class, ]; + /** + * @var string + */ private static $table_name = 'SnapshotTest_GalleryImage'; } diff --git a/tests/SnapshotTest/GalleryImageJoin.php b/tests/SnapshotTest/GalleryImageJoin.php index fc095ee..953f9ac 100644 --- a/tests/SnapshotTest/GalleryImageJoin.php +++ b/tests/SnapshotTest/GalleryImageJoin.php @@ -7,14 +7,23 @@ class GalleryImageJoin extends BaseJoin implements TestOnly { + /** + * @var array + */ private static $has_one = [ 'Gallery' => Gallery::class, 'Image' => GalleryImage::class, ]; + /** + * @var array + */ private static $extensions = [ Versioned::class, ]; + /** + * @var string + */ private static $table_name = 'SnapshotTest_GalleryImageJoin'; } diff --git a/tests/SnapshotTestAbstract.php b/tests/SnapshotTestAbstract.php index 62391d4..988c7cc 100644 --- a/tests/SnapshotTestAbstract.php +++ b/tests/SnapshotTestAbstract.php @@ -1,20 +1,21 @@ getMockBuilder(Snapshot::class) @@ -56,6 +67,12 @@ protected function mockSnapshot() return $mock; } + /** + * @param DataObject $obj + * @return Snapshot + * @throws ValidationException + * @throws Exception + */ protected function createHistory(DataObject $obj): Snapshot { $snapshot = Snapshot::create(); @@ -72,18 +89,26 @@ protected function createHistory(DataObject $obj): Snapshot * * @param int $minutes * @return string + * @throws Exception */ - protected function sleep($minutes) + protected function sleep(int $minutes): string { $now = DBDatetime::now(); $date = DateTime::createFromFormat('Y-m-d H:i:s', $now->getValue()); - $date->modify("+{$minutes} minutes"); - $stamp = $date->format('Y-m-d H:i:s'); - DBDatetime::set_mock_now($stamp); + $date->modify(sprintf('+%d minutes', $minutes)); + $dateTime = $date->format('Y-m-d H:i:s'); + DBDatetime::set_mock_now($dateTime); - return $stamp; + return $dateTime; } + /** + * @param DataObject $obj + * @param array $extraObjects + * @return Snapshot + * @throws ValidationException + * @throws Exception + */ protected function snapshot(DataObject $obj, array $extraObjects = []): Snapshot { $obj->write(); @@ -95,82 +120,107 @@ protected function snapshot(DataObject $obj, array $extraObjects = []): Snapshot return $snapshot; } + /** + * @param DataObject|RecursivePublishable $obj + * @param array $extraObjects + * @return Snapshot + * @throws ValidationException + */ protected function publish(DataObject $obj, array $extraObjects = []): Snapshot { $obj->publishRecursive(); $obj = DataObject::get_by_id($obj->ClassName, $obj->ID, false); $snapshot = Snapshot::singleton()->createSnapshot($obj, $extraObjects); + foreach ($snapshot->Items() as $item) { $item->WasPublished = 1; } + $snapshot->write(); return $snapshot; } - protected function buildState() + /** + * @return array + * @throws ValidationException + */ + protected function buildState(): array { - /* @var DataObject|SnapshotPublishable $a1 */ - $a1 = new BlockPage(['Title' => 'A1 Block Page']); + /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = BlockPage::create(); + $a1->Title = 'A1 Block Page'; $this->snapshot($a1); - /* @var DataObject|SnapshotPublishable $a2 */ - $a2 = new BlockPage(['Title' => 'A2 Block Page']); + /** @var BlockPage|SnapshotPublishable $a2 */ + $a2 = BlockPage::create(); + $a2->Title = 'A2 Block Page'; $this->snapshot($a2); - /* @var DataObject|SnapshotPublishable $a1Block1 */ - $a1Block1 = new Block(['Title' => 'Block 1 on A1', 'ParentID' => $a1->ID]); + /** @var Block|SnapshotPublishable $a1Block1 */ + $a1Block1 = Block::create(); + $a1Block1->Title = 'Block 1 on A1'; + $a1Block1->ParentID = $a1->ID; $this->snapshot($a1Block1); - $a1Block2 = new Block(['Title' => 'Block 2 on A1', 'ParentID' => $a1->ID]); + $a1Block2 = Block::create(); + $a1Block2->Title = 'Block 2 on A1'; + $a1Block2->ParentID = $a1->ID; $this->snapshot($a1Block2); - /* @var DataObject|SnapshotPublishable $a2Block1 */ - $a2Block1 = new Block(['Title' => 'Block 1 on A2', 'ParentID' => $a2->ID]); + /** @var Block|SnapshotPublishable $a2Block1 */ + $a2Block1 = Block::create(); + $a2Block1->Title = 'Block 1 on A2'; + $a2Block1->ParentID = $a2->ID; $this->snapshot($a2Block1); - /* @var DataObject|SnapshotPublishable|Versioned $gallery1 */ - $gallery1 = new Gallery(['Title' => 'Gallery 1 on Block 1 on A1', 'BlockID' => $a1Block1->ID]); + /** @var Gallery|SnapshotPublishable|Versioned $gallery1 */ + $gallery1 = Gallery::create(); + $gallery1->Title = 'Gallery 1 on Block 1 on A1'; + $gallery1->BlockID = $a1Block1->ID; $this->snapshot($gallery1); - /* @var DataObject|SnapshotPublishable|Versioned $gallery1 */ - $gallery2 = new Gallery(['Title' => 'Gallery 2 on Block 1 on A2', 'BlockID' => $a2Block1->ID]); + /** @var Gallery|SnapshotPublishable|Versioned $gallery1 */ + $gallery2 = Gallery::create(); + $gallery2->Title = 'Gallery 2 on Block 1 on A2'; + $gallery2->BlockID = $a2Block1->ID; $this->snapshot($gallery2); return [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2]; } - protected function assertItems(SS_List $result, array $objects, $column = 'ObjectHash') + protected function assertItems(SS_List $result, array $objects, string $column = 'ObjectHash'): void { $hashes = array_unique($result->column($column)); $this->assertCount(count($objects), $hashes); + foreach ($objects as $o) { $hash = SnapshotHasher::hashObjectForSnapshot($o); $this->assertTrue(in_array($hash, $hashes)); } } - protected function assertObjects(SS_List $result, array $objects) + protected function assertObjects(SS_List $result, array $objects): void { - return $this->assertItems($result, $objects, 'ObjectHash'); + $this->assertItems($result, $objects, 'ObjectHash'); } - protected function assertOrigins(SS_List $result, array $objects) + protected function assertOrigins(SS_List $result, array $objects): void { - return $this->assertItems($result, $objects, 'OriginHash'); + $this->assertItems($result, $objects, 'OriginHash'); } - protected function assertHashCompare(DataObject $obj1, DataObject $obj2) + protected function assertHashCompare(DataObject $obj1, DataObject $obj2): void { $this->assertTrue(SnapshotHasher::hashSnapshotCompare($obj1, $obj2)); } - protected function assertHashCompareList(array $objs1, array $objs2) + protected function assertHashCompareList(array $objs1, array $objs2): void { - $hashes1 = array_map(function ($o) { + $hashes1 = array_map(static function ($o) { return SnapshotHasher::hashObjectForSnapshot($o); }, $objs1); - $hashes2 = array_map(function ($o) { + $hashes2 = array_map(static function ($o) { return SnapshotHasher::hashObjectForSnapshot($o); }, $objs2); sort($hashes1); From 1df61eec74847e41abd8a119c80bba735a39343c Mon Sep 17 00:00:00 2001 From: Mojmir Fendek Date: Fri, 25 Mar 2022 13:14:05 +1300 Subject: [PATCH 2/2] PR fixes. --- src/ActivityEntry.php | 3 +- src/Handler/Form/PublishHandler.php | 9 +- src/Snapshot.php | 6 +- src/SnapshotPublishable.php | 13 +- .../Handler/GraphQL/Mutation/HandlerTest.php | 6 +- tests/IntegrationTest.php | 83 ++++++++----- tests/RelationDifferTest.php | 4 + tests/SnapshotPublishableTest.php | 111 +++++++++++++++--- tests/SnapshotTest.php | 23 +++- tests/SnapshotTestAbstract.php | 10 +- 10 files changed, 197 insertions(+), 71 deletions(-) diff --git a/src/ActivityEntry.php b/src/ActivityEntry.php index eaab54a..7d67063 100644 --- a/src/ActivityEntry.php +++ b/src/ActivityEntry.php @@ -54,9 +54,8 @@ public static function createFromSnapshotItem(SnapshotItem $item): self // This gets all versions except for the deleted version so we just get the latest one /** @var DataObject|Versioned $previousVersion */ $previousVersion = Versioned::get_all_versions($item->ObjectClass, $item->ObjectID) - ->filter(['WasDeleted' => 0]) ->sort('Version', 'DESC') - ->first(); + ->find('WasDeleted', 0); if ($previousVersion && $previousVersion->exists()) { $itemObj = $item->getItem($previousVersion->Version); diff --git a/src/Handler/Form/PublishHandler.php b/src/Handler/Form/PublishHandler.php index 5176d7d..eb93e26 100644 --- a/src/Handler/Form/PublishHandler.php +++ b/src/Handler/Form/PublishHandler.php @@ -37,10 +37,11 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot // Get the most recent change set to find out what was published /** @var ChangeSet $changeSet */ - $changeSet = ChangeSet::get()->filter([ - 'State' => ChangeSet::STATE_PUBLISHED, - 'IsInferred' => true, - ]) + $changeSet = ChangeSet::get() + ->filter([ + 'State' => ChangeSet::STATE_PUBLISHED, + 'IsInferred' => true, + ]) ->sort('Created', 'DESC') ->first(); diff --git a/src/Snapshot.php b/src/Snapshot.php index 12ee463..ba147e5 100644 --- a/src/Snapshot.php +++ b/src/Snapshot.php @@ -97,11 +97,7 @@ class Snapshot extends DataObject */ public function getOriginItem(): ?DataObject { - $item = $this->Items() - ->filter([ - 'ObjectHash' => $this->OriginHash, - ]) - ->first(); + $item = $this->Items()->find('ObjectHash', $this->OriginHash); if ($item instanceof DataObject) { return $item; diff --git a/src/SnapshotPublishable.php b/src/SnapshotPublishable.php index 6511221..f5aa5a6 100644 --- a/src/SnapshotPublishable.php +++ b/src/SnapshotPublishable.php @@ -114,11 +114,8 @@ public static function get_at_last_snapshot(string $class, int $id): ?DataObject public static function get_last_snapshot_item(string $class, int $id): ?DataObject { return SnapshotItem::get() - ->filter([ - 'ObjectHash' => static::hashForSnapshot($class, $id), - ]) ->sort('Created', 'DESC') - ->first(); + ->find('ObjectHash', static::hashForSnapshot($class, $id)); } /** @@ -392,11 +389,8 @@ public function onAfterRevertToLive(): void public function getPreviousSnapshotItem(): ?DataObject { return SnapshotItem::get() - ->filter([ - 'ObjectHash' => static::hashObjectForSnapshot($this->owner), - ]) ->sort('Created', 'DESC') - ->first(); + ->find('ObjectHash', static::hashObjectForSnapshot($this->owner)); } /** @@ -447,7 +441,8 @@ public function isModifiedSinceLastSnapshot(): bool /** * @return RelationDiffer[] - * @todo Memorise / cache + * @throws Exception + * TODO Memoise / cache / enable cache once it's confirmed that this feature is needed */ public function getRelationDiffs(): array { diff --git a/tests/Handler/GraphQL/Mutation/HandlerTest.php b/tests/Handler/GraphQL/Mutation/HandlerTest.php index 5096eaf..2409efd 100644 --- a/tests/Handler/GraphQL/Mutation/HandlerTest.php +++ b/tests/Handler/GraphQL/Mutation/HandlerTest.php @@ -21,7 +21,7 @@ protected function setUp(): void ); } - public function testHandlerDoesntFire() + public function testHandlerDoesntFire(): void { $handler = Handler::create(); $this->mockSnapshot() @@ -35,7 +35,7 @@ public function testHandlerDoesntFire() $handler->fire($context); } - public function testHandlerDoesFire() + public function testHandlerDoesFire(): void { $handler = Handler::create(); $blockPage = SiteTree::create(); @@ -46,7 +46,7 @@ public function testHandlerDoesFire() $this->mockSnapshot() ->expects($this->once()) ->method('createSnapshot') - ->with($this->callback(function ($arg) use ($blockPage) { + ->with($this->callback(static function ($arg) use ($blockPage) { return $arg instanceof SiteTree && $arg->ID == $blockPage->ID; })); diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index 618be12..6b0f054 100644 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -490,9 +490,11 @@ public function testFundamentals(): void */ public function testRevertChanges(): void { - /** @var DataObject|SnapshotPublishable $a1 */ - /** @var DataObject|SnapshotPublishable|Versioned $gallery1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $state = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; + /** @var Gallery|SnapshotPublishable|Versioned $gallery1 */ + $gallery1 = $state['gallery1']; $this->editingPage($a1); $this->formPublishObject($a1); @@ -532,11 +534,13 @@ public function testRevertChanges(): void */ public function testIntermediaryObjects(): void { - /** @var DataObject|SnapshotPublishable $a1 */ - /** @var DataObject|SnapshotPublishable $a2 */ - /** @var DataObject|SnapshotPublishable $a1Block1 */ - /** @var DataObject|SnapshotPublishable $gallery1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $state = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; + /** @var Block|SnapshotPublishable $a1Block1 */ + $a1Block1 = $state['a1Block1']; + /** @var Gallery|SnapshotPublishable $gallery1 */ + $gallery1 = $state['gallery1']; $this->publish($a1); $this->editingPage($a1); @@ -615,12 +619,17 @@ public function testIntermediaryObjects(): void */ public function testChangeOwnershipStructure(): void { - /** @var DataObject|SnapshotPublishable $a1 */ - /** @var DataObject|SnapshotPublishable $a2 */ + $state = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; + /** @var BlockPage|SnapshotPublishable $a2 */ + $a2 = $state['a2']; /** @var Block|SnapshotPublishable $a1Block1 */ - /** @var DataObject|SnapshotPublishable $gallery1 */ - /** @var DataObject|SnapshotPublishable $gallery2 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1Block1 = $state['a1Block1']; + /** @var Gallery|SnapshotPublishable $gallery1 */ + $gallery1 = $state['gallery1']; + /** @var Gallery|SnapshotPublishable $gallery2 */ + $gallery2 = $state['gallery2']; $this->editingPage($a1); $this->formPublishObject($a1); @@ -845,11 +854,15 @@ public function testChangeOwnershipStructure(): void */ public function testPartialActivityMigration(): void { - /** @var DataObject|SnapshotPublishable $a1 */ - /** @var DataObject|SnapshotPublishable $a2 */ + $state = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; + /** @var BlockPage|SnapshotPublishable $a2 */ + $a2 = $state['a2']; /** @var Block|SnapshotPublishable $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Block|SnapshotPublishable $a1Block2 */ - [$a1, $a2, $a1Block1, $a1Block2] = $this->buildState(); + $a1Block2 = $state['a1Block2']; // Test that we can transplant a node and relevant activity will be migrated // but unrelated activity will be preserved. @@ -922,13 +935,17 @@ public function testPartialActivityMigration(): void */ public function testDeletions(): void { - /** @var DataObject|SnapshotPublishable $a1 */ - /** @var DataObject|SnapshotPublishable $a2 */ - /** @var DataObject|SnapshotPublishable $a1Block1 */ - /** @var DataObject|SnapshotPublishable $a1Block2 */ - /** @var DataObject|SnapshotPublishable $a2Block1 */ - /** @var DataObject|SnapshotPublishable $gallery2 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $state = $this->buildState(); + /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; + /** @var BlockPage|SnapshotPublishable $a2 */ + $a2 = $state['a2']; + /** @var Block|SnapshotPublishable $a1Block1 */ + $a1Block1 = $state['a1Block1']; + /** @var Block|SnapshotPublishable $a2Block1 */ + $a2Block1 = $state['a2Block1']; + /** @var Gallery|SnapshotPublishable $gallery2 */ + $gallery2 = $state['gallery2']; $this->editingPage($a1); $this->formPublishObject($a1); @@ -1000,15 +1017,21 @@ public function testDeletions(): void */ public function testGetAtSnapshot(): void { - $stamp0 = $this->sleep(1); + $this->sleep(1); + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; /** @var BlockPage|SnapshotPublishable $a2 */ + $a2 = $state['a2']; /** @var Block|SnapshotPublishable $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Block|SnapshotPublishable $a1Block2 */ + $a1Block2 = $state['a1Block2']; /** @var Block|SnapshotPublishable $a2Block1 */ - /** @var DataObject|SnapshotPublishable $gallery2 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a2Block1 = $state['a2Block1']; + /** @var Gallery|SnapshotPublishable $gallery1 */ + $gallery1 = $state['gallery1']; $this->editingPage($a1); $this->formPublishObject($a1); @@ -1050,12 +1073,12 @@ public function testGetAtSnapshot(): void $this->formSaveObject($a2Block2); - $stamp6 = $this->sleep(1); + $this->sleep(1); $a2Block1->Title = 'A2 Block 1 changed'; $this->formSaveObject($a2Block1); - $stamp7 = $this->sleep(1); + $this->sleep(1); $a2->Title = 'The new A2'; $this->formSaveObject($a2); @@ -1534,6 +1557,7 @@ private function debugPublishable($items): string /** * @param DataObject $object * @throws ValidationException + * @throws Exception */ private function formSaveObject(DataObject $object): void { @@ -1561,6 +1585,7 @@ private function formPublishObject(DataObject $object): void /** * @param DataObject|Versioned $object + * @throws Exception */ private function formUnpublishObject(DataObject $object): void { @@ -1572,6 +1597,7 @@ private function formUnpublishObject(DataObject $object): void /** * @param DataObject|Versioned $object + * @throws Exception */ private function formDeleteObject(DataObject $object): void { @@ -1589,6 +1615,7 @@ private function formDeleteObject(DataObject $object): void * @param string $component * @param DataObject[] $items * @param string $type + * @throws Exception */ private function formSaveRelations( DataObject $object, diff --git a/tests/RelationDifferTest.php b/tests/RelationDifferTest.php index 9489ec2..93a78ed 100644 --- a/tests/RelationDifferTest.php +++ b/tests/RelationDifferTest.php @@ -6,6 +6,7 @@ use SilverStripe\ORM\ValidationException; use SilverStripe\Snapshots\RelationDiffer; use SilverStripe\Snapshots\Tests\SnapshotTest\Block; +use SilverStripe\Versioned\Versioned; class RelationDifferTest extends SapphireTest { @@ -155,10 +156,13 @@ public function testNoChanges(): void */ public function testGetRecords(): void { + /** @var Block|Versioned $block1 */ $block1 = Block::create(); $block1->write(); + /** @var Block|Versioned $block2 */ $block2 = Block::create(); $block2->write(); + /** @var Block|Versioned $block3 */ $block3 = Block::create(); $block3->write(); diff --git a/tests/SnapshotPublishableTest.php b/tests/SnapshotPublishableTest.php index 31e6cba..7303c37 100644 --- a/tests/SnapshotPublishableTest.php +++ b/tests/SnapshotPublishableTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\Snapshots\Tests; +use Exception; use SilverStripe\Core\Config\Config; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\ValidationException; @@ -14,12 +15,18 @@ class SnapshotPublishableTest extends SnapshotTestAbstract { + /** + * @throws ValidationException + */ public function testGetAtSnapshot(): void { + $state = $this->buildState(); /** @var BlockPage $a1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1 = $state['a1']; + $firstSnapshot = Snapshot::get()->sort('Created ASC')->first(); $result = SnapshotPublishable::get_at_snapshot(BlockPage::class, $a1->ID, $firstSnapshot->Created); + $param = $result->getSourceQueryParam('Versioned.date'); $this->assertNotNull($param); $this->assertEquals($firstSnapshot->Created, $param); @@ -30,8 +37,10 @@ public function testGetAtSnapshot(): void */ public function testGetAtLastSnapshot(): void { + $state = $this->buildState(); /** @var BlockPage $a1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1 = $state['a1']; + $a1->Title = 'changed'; $a1->write(); @@ -40,10 +49,14 @@ public function testGetAtLastSnapshot(): void $this->assertEquals('A1 Block Page', $result->Title); } + /** + * @throws ValidationException + */ public function testGetLastSnapshotItem(): void { + $state = $this->buildState(); /** @var BlockPage|Versioned $a1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1 = $state['a1']; $a1->Title = 'changed'; $this->snapshot($a1); @@ -53,6 +66,9 @@ public function testGetLastSnapshotItem(): void $this->assertEquals($a1->Version, $result->Version); } + /** + * @throws ValidationException + */ public function testGetSnapshots(): void { $state = $this->buildState(); @@ -60,11 +76,26 @@ public function testGetSnapshots(): void $this->assertOrigins($snapshots, $state); } + /** + * @throws ValidationException + */ public function testGetRelevantSnapshots(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; /** @var BlockPage|SnapshotPublishable $a2 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a2 = $state['a2']; + /** @var Block $a1Block1 */ + $a1Block1 = $state['a1Block1']; + /** @var Block $a1Block2 */ + $a1Block2 = $state['a1Block2']; + /** @var Block $a2Block1 */ + $a2Block1 = $state['a2Block1']; + /** @var Gallery $gallery1 */ + $gallery1 = $state['gallery1']; + /** @var Gallery $gallery2 */ + $gallery2 = $state['gallery2']; $a1->Title = 'changed'; $this->snapshot($a1); @@ -96,15 +127,24 @@ public function testGetRelevantSnapshots(): void ); } + /** + * @throws ValidationException + */ public function testGetSnapshotsSinceVersion(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; /** @var Block $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Block $a1Block2 */ + $a1Block2 = $state['a1Block2']; /** @var Block $a2Block1 */ + $a2Block1 = $state['a2Block1']; /** @var Gallery $gallery1 */ + $gallery1 = $state['gallery1']; /** @var Gallery $gallery2 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $gallery2 = $state['gallery2']; $this->publish($a1); $fromVersion = Versioned::get_versionnumber_by_stage(BlockPage::class, Versioned::LIVE, $a1->ID); @@ -145,13 +185,20 @@ public function testGetSnapshotsSinceVersion(): void ); } + /** + * @throws ValidationException + */ public function testHasOwnedModifications(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; /** @var BlockPage|SnapshotPublishable $a2 */ + $a2 = $state['a2']; /** @var Block $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Block $a1Block2 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1Block2 = $state['a1Block2']; $this->publish($a1); $this->publish($a2); @@ -168,14 +215,22 @@ public function testHasOwnedModifications(): void $this->assertFalse($a2->hasOwnedModifications()); } + /** + * @throws ValidationException + */ public function testPublishableItems(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; /** @var BlockPage|SnapshotPublishable $a2 */ + $a2 = $state['a2']; /** @var Block $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Block $a2Block1 */ + $a2Block1 = $state['a2Block1']; /** @var Gallery $gallery1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $gallery1 = $state['gallery1']; $this->publish($a1); $this->publish($a2); @@ -217,12 +272,18 @@ public function testPublishableItems(): void $this->assertEquals($classes, $a2->getPublishableObjects()->column('ClassName')); } + /** + * @throws ValidationException + */ public function testGetRelationTracking(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; /** @var Block|Versioned $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Block|Versioned $a1Block2 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1Block2 = $state['a1Block2']; $this->assertEmpty($a1->getRelationTracking()); Config::modify()->set(BlockPage::class, 'snapshot_relation_tracking', ['Blocks', 'Fail']); $result = $a1->getRelationTracking(); @@ -233,10 +294,14 @@ public function testGetRelationTracking(): void $this->assertEquals($a1Block2->Version, $result['Blocks'][$a1Block2->ID]); } + /** + * @throws ValidationException + */ public function testPreviousSnapshotItem(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1 = $state['a1']; $a1->Title = 'changed'; $this->snapshot($a1); $version = Versioned::get_versionnumber_by_stage(BlockPage::class, Versioned::DRAFT, $a1->ID); @@ -250,8 +315,9 @@ public function testPreviousSnapshotItem(): void */ public function testPreviousSnapshot(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1 = $state['a1']; $a1->Title = 'changed'; $this->snapshot($a1); @@ -275,10 +341,11 @@ public function testPreviousSnapshot(): void /** * @throws ValidationException */ - public function testIsModifiedSinceLastSnapshot() + public function testIsModifiedSinceLastSnapshot(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1 = $state['a1']; $this->assertFalse($a1->isModifiedSinceLastSnapshot()); $a1->Title = 'changed'; $a1->write(); @@ -289,25 +356,35 @@ public function testIsModifiedSinceLastSnapshot() $this->assertTrue($obj->isModifiedSinceLastSnapshot()); } + /** + * @throws ValidationException + */ public function testGetIntermediaryObjects(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; /** @var Block|SnapshotPublishable $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Gallery|SnapshotPublishable $gallery1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $gallery1 = $state['gallery1']; $objs = $gallery1->getIntermediaryObjects(); $this->assertHashCompareList([$a1Block1, $a1], $objs); } /** * @throws ValidationException + * @throws Exception */ public function testGetRelationDiffs(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ + $a1 = $state['a1']; /** @var Block|SnapshotPublishable $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Block|SnapshotPublishable $a1Block2 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1Block2 = $state['a1Block2']; Config::modify()->set(BlockPage::class, 'snapshot_relation_tracking', ['Blocks']); $this->assertCount(1, $a1->getRelationDiffs()); @@ -357,10 +434,14 @@ public function testGetRelationDiffs(): void $this->assertEquals([$a1Block1->ID], $diff->getChanged()); } + /** + * @throws ValidationException + */ public function testGetPreviousVersion(): void { + $state = $this->buildState(); /** @var BlockPage|SnapshotPublishable $a1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1 = $state['a1']; $originalTitle = $a1->Title; $a1->Title = 'changed'; $this->snapshot($a1); diff --git a/tests/SnapshotTest.php b/tests/SnapshotTest.php index e75ba86..c998713 100644 --- a/tests/SnapshotTest.php +++ b/tests/SnapshotTest.php @@ -181,11 +181,15 @@ public function testGetOriginVersion(): void */ public function testCreateSnapshotNoRelations(): void { + $state = $this->buildState(); /** @var BlockPage $a1 */ + $a1 = $state['a1']; /** @var Block $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Block $a2Block1 */ + $a2Block1 = $state['a2Block1']; /** @var Gallery $gallery1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $gallery1 = $state['gallery1']; $gallery1->Title = 'changed'; $snapshot = $this->snapshot($gallery1); $this->assertCount(3, $snapshot->Items()); @@ -239,10 +243,13 @@ public function testCreateSnapshotNoRelations(): void */ public function testCreateSnapshotWithImplicitModifications(): void { + $state = $this->buildState(); /** @var BlockPage $a1 */ + $a1 = $state['a1']; /** @var Block $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Gallery $gallery1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $gallery1 = $state['gallery1']; $image = GalleryImage::create(); $image->write(); @@ -348,10 +355,13 @@ public function testCreateSnapshotEvent(): void */ public function testAddOwnershipChain(): void { + $state = $this->buildState(); /** @var BlockPage $a1 */ + $a1 = $state['a1']; /** @var Block $a1Block1 */ + $a1Block1 = $state['a1Block1']; /** @var Gallery $gallery1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $gallery1 = $state['gallery1']; $snapshot = Snapshot::create(); $this->assertEmpty($snapshot->Items()); $snapshot->addOwnershipChain($gallery1); @@ -384,11 +394,16 @@ public function testApplyOrigin(): void $this->assertHashCompare($item, $block); } + /** + * @throws ValidationException + */ public function testIsLiveSnapshot(): void { + $state = $this->buildState(); /** @var BlockPage $a1 */ + $a1 = $state['a1']; /** @var Block $a1Block1 */ - [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2] = $this->buildState(); + $a1Block1 = $state['a1Block1']; Snapshot::get()->removeAll(); SnapshotItem::get()->removeAll(); $this->assertCount(0, Snapshot::get()); diff --git a/tests/SnapshotTestAbstract.php b/tests/SnapshotTestAbstract.php index 988c7cc..2aa20b1 100644 --- a/tests/SnapshotTestAbstract.php +++ b/tests/SnapshotTestAbstract.php @@ -186,7 +186,15 @@ protected function buildState(): array $gallery2->BlockID = $a2Block1->ID; $this->snapshot($gallery2); - return [$a1, $a2, $a1Block1, $a1Block2, $a2Block1, $gallery1, $gallery2]; + return [ + 'a1' => $a1, + 'a2' => $a2, + 'a1Block1' => $a1Block1, + 'a1Block2' => $a1Block2, + 'a2Block1' => $a2Block1, + 'gallery1' => $gallery1, + 'gallery2' => $gallery2, + ]; } protected function assertItems(SS_List $result, array $objects, string $column = 'ObjectHash'): void