Merge pull request #445 from creative-commoners/pulls/3/protect-hooks

API Set extension hook implementation visibility to protected
silverstripe · May 21, 2024 · 7e744f9 · 7e744f9
2 parents 606698e + b1f567a
commit 7e744f9
Show file tree

Hide file tree

Showing 11 changed files with 34 additions and 28 deletions.
diff --git a/src/ChangeSetItem.php b/src/ChangeSetItem.php
@@ -83,7 +83,7 @@ class ChangeSetItem extends DataObject implements Thumbnail
         ]
     ];
 
-    public function onBeforeWrite()
+    protected function onBeforeWrite()
     {
         // Make sure ObjectClass refers to the base data class in the case of old or wrong code
         $this->ObjectClass = $this->getSchema()->baseDataClass($this->ObjectClass);

diff --git a/src/Dev/VersionedTestSessionExtension.php b/src/Dev/VersionedTestSessionExtension.php
@@ -54,7 +54,7 @@ protected function getReadingmode()
      *
      * @param string $link
      */
-    public function updateGetURL(&$link)
+    protected function updateGetURL(&$link)
     {
         $this->updateLink($link);
     }
@@ -64,7 +64,7 @@ public function updateGetURL(&$link)
      *
      * @param string $link
      */
-    public function updatePostURL(&$link)
+    protected function updatePostURL(&$link)
     {
         // Default to same as http get
         $this->updateLink($link);

diff --git a/src/RecursivePublishable.php b/src/RecursivePublishable.php
@@ -432,7 +432,7 @@ public function unlinkDisownedRelationship($source, $targetStage, $relationship)
      * @param bool $doWrite
      * @param array|null|false $relations
      */
-    public function onBeforeDuplicate($original, &$doWrite, &$relations)
+    protected function onBeforeDuplicate($original, &$doWrite, &$relations)
     {
         // If relations to duplicate are declared (or forced off) don't rewrite
         if ($relations || $relations === false) {
@@ -457,7 +457,7 @@ public function onBeforeDuplicate($original, &$doWrite, &$relations)
      * Make sure to flush cached data in case the data changes
      * Extension point in @see DataObject::onAfterWrite()
      */
-    public function onAfterWrite(): void
+    protected function onAfterWrite(): void
     {
         RecursiveStagesService::reset();
     }
@@ -466,7 +466,7 @@ public function onAfterWrite(): void
      * Make sure to flush cached data in case the data changes
      * Extension point in @see DataObject::onAfterDelete()
      */
-    public function onAfterDelete(): void
+    protected function onAfterDelete(): void
     {
         RecursiveStagesService::reset();
     }

diff --git a/src/RecursivePublishableHandler.php b/src/RecursivePublishableHandler.php
@@ -17,7 +17,7 @@ class RecursivePublishableHandler extends Extension
      * Ensure that non-versioned records are published on save.
      * @param DataObject $record
      */
-    public function onAfterSave(DataObject $record)
+    protected function onAfterSave(DataObject $record)
     {
         // Assume that any versioned record has an explicit publish already
         if (!$record->hasExtension(Versioned::class)) {

diff --git a/src/Versioned.php b/src/Versioned.php
@@ -313,7 +313,7 @@ public static function reset()
      * @param SQLSelect $query
      * @param DataQuery $dataQuery
      */
-    public function augmentDataQueryCreation(SQLSelect &$query, DataQuery &$dataQuery)
+    protected function augmentDataQueryCreation(SQLSelect &$query, DataQuery &$dataQuery)
     {
         // Convert reading mode to dataquery params and assign
         $args = ReadingMode::toDataQueryParams(Versioned::get_reading_mode());
@@ -405,7 +405,7 @@ protected function getLastEditedAndStageForVersion($version)
      *
      * @param array $params
      */
-    public function updateInheritableQueryParams(&$params)
+    protected function updateInheritableQueryParams(&$params)
     {
         // Skip if versioned isn't set
         if (!isset($params['Versioned.mode'])) {
@@ -455,7 +455,7 @@ public function updateInheritableQueryParams(&$params)
      * @param DataQuery|null $dataQuery
      * @throws InvalidArgumentException
      */
-    public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
+    protected function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
     {
         if (!$dataQuery) {
             return;
@@ -896,7 +896,7 @@ protected function isTableVersioned($table)
      * @param DataQuery $dataQuery
      * @param DataObject $dataObject
      */
-    public function augmentLoadLazyFields(SQLSelect &$query, DataQuery &$dataQuery = null, $dataObject)
+    protected function augmentLoadLazyFields(SQLSelect &$query, DataQuery &$dataQuery = null, $dataObject)
     {
         // The VersionedMode local variable ensures that this decorator only applies to
         // queries that have originated from the Versioned object, and have the Versioned
@@ -917,7 +917,7 @@ public function augmentLoadLazyFields(SQLSelect &$query, DataQuery &$dataQuery =
         }
     }
 
-    public function augmentDatabase()
+    protected function augmentDatabase()
     {
         $owner = $this->owner;
         $class = get_class($owner);
@@ -1257,7 +1257,7 @@ protected function createDeletedVersion($stages = [])
         $this->owner->extend('onAfterVersionDelete');
     }
 
-    public function augmentWrite(&$manipulation)
+    protected function augmentWrite(&$manipulation)
     {
         // get Version number from base data table on write
         $version = null;
@@ -1348,7 +1348,7 @@ public function writeWithoutVersion()
     /**
      *
      */
-    public function onAfterWrite()
+    protected function onAfterWrite()
     {
         $this->setNextWriteWithoutVersion(false);
     }
@@ -1416,7 +1416,7 @@ protected function suppressDeletedVersion($callback)
      * If a write was skipped, then we need to ensure that we don't leave a
      * migrateVersion() value lying around for the next write.
      */
-    public function onAfterSkippedWrite()
+    protected function onAfterSkippedWrite()
     {
         $this->setMigratingVersion(null);
     }
@@ -1887,7 +1887,7 @@ public function doUnpublish()
         return true;
     }
 
-    public function onAfterDelete()
+    protected function onAfterDelete()
     {
         // Create deleted record for current stage
         $this->createDeletedVersion(static::get_stage());
@@ -2452,7 +2452,7 @@ private static function determineVersionNumberByStage($class, $stage, $id, $cach
      * @param array $options A map of hints about what should be cached. "numChildrenMethod" and
      *                       "childrenMethod" are allowed keys.
      */
-    public function onPrepopulateTreeDataCache($recordList = null, array $options = [])
+    protected function onPrepopulateTreeDataCache($recordList = null, array $options = [])
     {
         $idList = is_array($recordList) ? $recordList :
             ($recordList instanceof DataList ? $recordList->column('ID') : null);
@@ -2888,15 +2888,15 @@ public static function get_all_versions($class, $id)
     /**
      * @param array $labels
      */
-    public function updateFieldLabels(&$labels)
+    protected function updateFieldLabels(&$labels)
     {
         $labels['Versions'] = _t(__CLASS__ . '.has_many_Versions', 'Versions', 'Past Versions of this record');
     }
 
     /**
      * @param FieldList $fields
      */
-    public function updateCMSFields(FieldList $fields)
+    protected function updateCMSFields(FieldList $fields)
     {
         // remove the version field from the CMS as this should be left
         // entirely up to the extension (not the cms user).
@@ -2909,7 +2909,7 @@ public function updateCMSFields(FieldList $fields)
      * @param DataObject $source Record this was duplicated from
      * @param bool $doWrite
      */
-    public function onBeforeDuplicate($source, $doWrite)
+    protected function onBeforeDuplicate($source, $doWrite)
     {
         $this->owner->Version = 0;
     }
@@ -2925,7 +2925,7 @@ public function flushCache()
      *
      * @return string
      */
-    public function cacheKeyComponent()
+    protected function cacheKeyComponent()
     {
         return 'versionedmode-' . static::get_reading_mode();
     }

diff --git a/src/VersionedGridFieldArchiveExtension.php b/src/VersionedGridFieldArchiveExtension.php
@@ -13,7 +13,7 @@
  */
 class VersionedGridFieldArchiveExtension extends Extension
 {
-    public function updateConfig()
+    protected function updateConfig()
     {
         $owner = $this->getOwner();
         $owner->addComponent(new GridFieldArchiveAction(), GridFieldDeleteAction::class);

diff --git a/src/VersionedGridFieldDetailForm.php b/src/VersionedGridFieldDetailForm.php
@@ -20,7 +20,7 @@ class VersionedGridFieldDetailForm extends Extension
      * @param RequestHandler $requestHandler
      * @param string $assignedClass Name of class explicitly assigned to this component
      */
-    public function updateItemRequestClass(&$class, $gridField, $record, $requestHandler, $assignedClass = null)
+    protected function updateItemRequestClass(&$class, $gridField, $record, $requestHandler, $assignedClass = null)
     {
         // Avoid overriding explicitly assigned class name if set using setItemRequestClass()
         if ($assignedClass) {

diff --git a/src/VersionedGridFieldStateExtension.php b/src/VersionedGridFieldStateExtension.php
@@ -13,7 +13,7 @@
  */
 class VersionedGridFieldStateExtension extends Extension
 {
-    public function updateConfig()
+    protected function updateConfig()
     {
         $owner = $this->getOwner();
         if (!$owner->getComponentByType(VersionedGridFieldState::class)) {

diff --git a/src/VersionedTableDataQueryExtension.php b/src/VersionedTableDataQueryExtension.php
@@ -21,7 +21,7 @@ class VersionedTableDataQueryExtension extends Extension
      * @param string $table
      * @param string $updated
      */
-    public function updateJoinTableName($class, $table, &$updated)
+    protected function updateJoinTableName($class, $table, &$updated)
     {
         $inst = Injector::inst()->get($class);
         if (!$inst->hasExtension(Versioned::class) || !$inst->hasStages()) {

diff --git a/tests/php/VersionedNumberCacheTest.php b/tests/php/VersionedNumberCacheTest.php
@@ -5,6 +5,7 @@
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Versioned\Versioned;
 use SilverStripe\Versioned\Tests\VersionedTest\TestObject;
+use ReflectionMethod;
 
 /**
  * @internal Only test the right values are returned, not that the cache is actually used.
@@ -100,7 +101,12 @@ public function testVersionNumberCache($stage, $ID, $cache, $expected)
      */
     public function testPrepopulatedVersionNumberCache($stage, $ID, $cache, $expected)
     {
-        TestObject::singleton()->onPrepopulateTreeDataCache();
+        $owner = TestObject::singleton();
+        $ext = new Versioned();
+        $ext->setOwner($owner);
+        $method = new ReflectionMethod(Versioned::class, 'onPrepopulateTreeDataCache');
+        $method->setAccessible(true);
+        $method->invoke($ext);
         $actual = Versioned::get_versionnumber_by_stage(TestObject::class, $stage, self::${$ID}, $cache);
         $this->assertEquals(self::$expectedVersions[$expected], $actual);
     }

diff --git a/tests/php/VersionedTest/TestObject.php b/tests/php/VersionedTest/TestObject.php
@@ -72,15 +72,15 @@ public function canView($member = null)
         return true;
     }
 
-    public function onAfterPublish($original)
+    protected function onAfterPublish($original)
     {
         if (self::$setNameWithoutVersionAfterPublish !== null) {
             $this->Name = self::$setNameWithoutVersionAfterPublish;
             $this->writeWithoutVersion();
         }
     }
 
-    public function onAfterDelete()
+    protected function onAfterDelete()
     {
         parent::onAfterDelete();
         self::$nameValueOfObjectJustDeleted = $this->Name;