Merge pull request #571 from creative-commoners/pulls/4/protect-hooks

API Set extension hook implementation visibility to protected

src/Extensions/BaseElementSubsites.php

@@ -20,7 +20,7 @@ class BaseElementSubsites extends DataExtension
      * @param string|null $action
      * @return string
      */
-    public function updatePreviewLink(&$link)
+    protected function updatePreviewLink(&$link)
     {
         // Get subsite ID from the element or from its page. Defaults to 0 automatically.
         $subsiteID = $this->owner->SubsiteID;

src/Extensions/CMSPageAddControllerExtension.php

@@ -13,7 +13,7 @@ use SilverStripe\Subsites\State\SubsiteState;
  */
 class CMSPageAddControllerExtension extends Extension
 {
-    public function updatePageOptions(FieldList $fields)
+    protected function updatePageOptions(FieldList $fields)
     {
         $fields->push(HiddenField::create('SubsiteID', 'SubsiteID', SubsiteState::singleton()->getSubsiteId()));
     }

src/Extensions/ErrorPageSubsite.php

@@ -28,7 +28,7 @@ class ErrorPageSubsite extends DataExtension
      * @param string $name
      * @param int $statusCode
      */
-    public function updateErrorFilename(&$name, &$statusCode)
+    protected function updateErrorFilename(&$name, &$statusCode)
     {
         $static_filepath = Config::inst()->get($this->owner->ClassName, 'static_filepath');
         $subdomainPart = '';

src/Extensions/FileSubsites.php

@@ -49,7 +49,7 @@ class FileSubsites extends DataExtension
      * @param SQLSelect $query
      * @param DataQuery|null $dataQuery
      */
-    public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
+    protected function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
     {
         if (Subsite::$disable_subsite_filter) {
             return;
@@ -85,7 +85,7 @@ class FileSubsites extends DataExtension
         }
     }
 
-    public function onBeforeWrite()
+    protected function onBeforeWrite()
     {
         if (!$this->owner->ID && !$this->owner->SubsiteID) {
             if ($this->owner->config()->get('default_root_folders_global')) {
@@ -96,7 +96,7 @@ class FileSubsites extends DataExtension
         }
     }
 
-    public function onAfterUpload()
+    protected function onAfterUpload()
     {
         // If we have a parent, use it's subsite as our subsite
         if ($this->owner->Parent()) {
@@ -132,7 +132,7 @@ class FileSubsites extends DataExtension
      *
      * @return string
      */
-    public function cacheKeyComponent()
+    protected function cacheKeyComponent()
     {
         return 'subsite-' . SubsiteState::singleton()->getSubsiteId();
     }

src/Extensions/FolderFormFactoryExtension.php

@@ -18,7 +18,7 @@ class FolderFormFactoryExtension extends Extension
      * Add subsites-specific fields to the folder editor.
      * @param FieldList $fields
      */
-    public function updateFormFields(FieldList $fields)
+    protected function updateFormFields(FieldList $fields)
     {
         $sites = Subsite::accessible_sites('CMS_ACCESS_AssetAdmin');
         $values = [];

src/Extensions/GroupSubsites.php

@@ -78,7 +78,7 @@ class GroupSubsites extends DataExtension implements PermissionProvider
         }
     }
 
-    public function updateCMSFields(FieldList $fields)
+    protected function updateCMSFields(FieldList $fields)
     {
         if ($this->owner->canEdit()) {
             // i18n tab
@@ -132,7 +132,7 @@ class GroupSubsites extends DataExtension implements PermissionProvider
      *
      * @param string $title
      */
-    public function updateTreeTitle(&$title)
+    protected function updateTreeTitle(&$title)
     {
         if ($this->owner->AccessAllSubsites) {
             $title = _t(__CLASS__ . '.GlobalGroup', 'global group');
@@ -148,7 +148,7 @@ class GroupSubsites extends DataExtension implements PermissionProvider
      * @param SQLSelect $query
      * @param DataQuery|null $dataQuery
      */
-    public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
+    protected function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
     {
         if (Subsite::$disable_subsite_filter) {
             return;
@@ -203,7 +203,7 @@ class GroupSubsites extends DataExtension implements PermissionProvider
         }
     }
 
-    public function onBeforeWrite()
+    protected function onBeforeWrite()
     {
         // New record test approximated by checking whether the ID has changed.
         // Note also that the after write test is only used when we're *not* on a subsite
@@ -212,7 +212,7 @@ class GroupSubsites extends DataExtension implements PermissionProvider
         }
     }
 
-    public function onAfterWrite()
+    protected function onAfterWrite()
     {
         // New record test approximated by checking whether the ID has changed.
         // Note also that the after write test is only used when we're on a subsite

src/Extensions/HintsCacheKeyExtension.php

@@ -16,7 +16,7 @@ use SilverStripe\Subsites\State\SubsiteState;
  */
 class HintsCacheKeyExtension extends Extension
 {
-    public function updateHintsCacheKey(&$baseKey)
+    protected function updateHintsCacheKey(&$baseKey)
     {
         $baseKey .= '_Subsite:' . SubsiteState::singleton()->getSubsiteId();
     }

src/Extensions/LeftAndMainSubsites.php

@@ -56,7 +56,7 @@ class LeftAndMainSubsites extends LeftAndMainExtension
         return $subsite ? Convert::raw2xml($subsite->Title) : _t(__CLASS__.'.SITECONTENTLEFT', 'Site Content');
     }
 
-    public function updatePageOptions(&$fields)
+    protected function updatePageOptions(&$fields)
     {
         $fields->push(HiddenField::create('SubsiteID', 'SubsiteID', SubsiteState::singleton()->getSubsiteId()));
     }
@@ -266,7 +266,7 @@ class LeftAndMainSubsites extends LeftAndMainExtension
      * fully re-synchronised with the internal session. This is better than risking some panels
      * showing data from another subsite.
      */
-    public function onBeforeInit()
+    protected function onBeforeInit()
     {
         $request = Controller::curr()->getRequest();
         $session = $request->getSession();
@@ -381,13 +381,13 @@ class LeftAndMainSubsites extends LeftAndMainExtension
         return;
     }
 
-    public function augmentNewSiteTreeItem(&$item)
+    protected function augmentNewSiteTreeItem(&$item)
     {
         $request = Controller::curr()->getRequest();
         $item->SubsiteID = $request->postVar('SubsiteID') ?: SubsiteState::singleton()->getSubsiteId();
     }
 
-    public function onAfterSave($record)
+    protected function onAfterSave($record)
     {
         if ($record->hasMethod('NormalRelated') && ($record->NormalRelated() || $record->ReverseRelated())) {
             $this->owner->response->addHeader(

src/Extensions/SiteConfigSubsites.php

@@ -29,7 +29,7 @@ class SiteConfigSubsites extends DataExtension
      * @param SQLSelect $query
      * @param DataQuery|null $dataQuery
      */
-    public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
+    protected function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
     {
         if (Subsite::$disable_subsite_filter) {
             return;
@@ -60,7 +60,7 @@ class SiteConfigSubsites extends DataExtension
         $query->addWhere("\"$tableName\".\"SubsiteID\" IN ($subsiteID)");
     }
 
-    public function onBeforeWrite()
+    protected function onBeforeWrite()
     {
         if ((!is_numeric($this->owner->ID) || !$this->owner->ID) && !$this->owner->SubsiteID) {
             $this->owner->SubsiteID = SubsiteState::singleton()->getSubsiteId();
@@ -70,12 +70,12 @@ class SiteConfigSubsites extends DataExtension
     /**
      * Return a piece of text to keep DataObject cache keys appropriately specific
      */
-    public function cacheKeyComponent()
+    protected function cacheKeyComponent()
     {
         return 'subsite-' . SubsiteState::singleton()->getSubsiteId();
     }
 
-    public function updateCMSFields(FieldList $fields)
+    protected function updateCMSFields(FieldList $fields)
     {
         $fields->push(HiddenField::create('SubsiteID', 'SubsiteID', SubsiteState::singleton()->getSubsiteId()));
     }

src/Extensions/SiteTreeSubsites.php

@@ -63,7 +63,7 @@ class SiteTreeSubsites extends DataExtension
      * @param SQLSelect $query
      * @param DataQuery $dataQuery
      */
-    public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
+    protected function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
     {
         if (Subsite::$disable_subsite_filter) {
             return;
@@ -98,7 +98,7 @@ class SiteTreeSubsites extends DataExtension
         }
     }
 
-    public function onBeforeWrite()
+    protected function onBeforeWrite()
     {
         if (!$this->owner->ID && !$this->owner->SubsiteID) {
             $this->owner->SubsiteID = SubsiteState::singleton()->getSubsiteId();
@@ -107,7 +107,7 @@ class SiteTreeSubsites extends DataExtension
         parent::onBeforeWrite();
     }
 
-    public function updateCMSFields(FieldList $fields)
+    protected function updateCMSFields(FieldList $fields)
     {
         $subsites = Subsite::accessible_sites('CMS_ACCESS_CMSMain');
         if ($subsites && $subsites->count()) {
@@ -225,7 +225,7 @@ class SiteTreeSubsites extends DataExtension
     /**
      * When duplicating a page, assign the current subsite ID from the state
      */
-    public function onBeforeDuplicate()
+    protected function onBeforeDuplicate()
     {
         $subsiteId = SubsiteState::singleton()->getSubsiteId();
         if ($subsiteId !== null) {
@@ -290,7 +290,7 @@ class SiteTreeSubsites extends DataExtension
     /**
      * @return SiteConfig
      */
-    public function alternateSiteConfig()
+    protected function alternateSiteConfig()
     {
         if (!$this->owner->SubsiteID) {
             return false;
@@ -433,7 +433,7 @@ class SiteTreeSubsites extends DataExtension
      * @param string|null $action
      * @return string
      */
-    public function updatePreviewLink(&$link, $action = null)
+    protected function updatePreviewLink(&$link, $action = null)
     {
         $url = Director::absoluteURL($this->owner->Link($action));
         $link = HTTP::setGetVar('SubsiteID', $this->owner->SubsiteID, $url);
@@ -454,7 +454,7 @@ class SiteTreeSubsites extends DataExtension
         return $tags;
     }
 
-    public function augmentSyncLinkTracking()
+    protected function augmentSyncLinkTracking()
     {
         // Set LinkTracking appropriately
         $links = HTTP::getLinksIn($this->owner->Content);
@@ -502,7 +502,7 @@ class SiteTreeSubsites extends DataExtension
      *
      * @return null|bool Either true or false, or null to not influence result
      */
-    public function augmentValidURLSegment()
+    protected function augmentValidURLSegment()
     {
         // If this page is being filtered in the current subsite, then no custom validation query is required.
         $subsiteID = SubsiteState::singleton()->getSubsiteId();
@@ -518,7 +518,7 @@ class SiteTreeSubsites extends DataExtension
     /**
      * Return a piece of text to keep DataObject cache keys appropriately specific
      */
-    public function cacheKeyComponent()
+    protected function cacheKeyComponent()
     {
         return 'subsite-' . SubsiteState::singleton()->getSubsiteId();
     }

src/Model/Subsite.php

@@ -819,7 +819,7 @@ class Subsite extends DataObject
      *
      * @return void
      */
-    public function onAfterWrite()
+    protected function onAfterWrite()
     {
         Subsite::writeHostMap();
         if ($this->isChanged('ID')) {

src/Model/SubsiteDomain.php

@@ -95,7 +95,7 @@ class SubsiteDomain extends DataObject
      *
      * @return void
      */
-    public function onAfterWrite()
+    protected function onAfterWrite()
     {
         Subsite::writeHostMap();
         parent::onAfterWrite();

src/Pages/SubsitesVirtualPage.php

@@ -192,7 +192,7 @@ class SubsitesVirtualPage extends VirtualPage
         Subsite::$disable_subsite_filter = $oldState;
     }
 
-    public function onBeforeWrite()
+    protected function onBeforeWrite()
     {
         parent::onBeforeWrite();
 

tests/php/FileSubsitesTest.php

@@ -9,6 +9,7 @@ use SilverStripe\Forms\FieldList;
 use SilverStripe\Subsites\Extensions\FileSubsites;
 use SilverStripe\Subsites\Model\Subsite;
 use SilverStripe\Security\Member;
+use ReflectionMethod;
 
 class FileSubsitesTest extends BaseSubsiteTest
 {
@@ -25,7 +26,11 @@ class FileSubsitesTest extends BaseSubsiteTest
         $this->assertEquals('FileTitle', $file->getTreeTitle());
         $this->assertInstanceOf(FieldList::class, singleton(Folder::class)->getCMSFields());
         Subsite::changeSubsite(1);
-        $this->assertEquals('subsite-1', $file->getExtensionInstance(FileSubsites::class)->cacheKeyComponent());
+        $ext = $file->getExtensionInstance(FileSubsites::class);
+        $method = new ReflectionMethod(FileSubsites::class, 'cacheKeyComponent');
+        $method->setAccessible(true);
+        $result = $method->invoke($ext);
+        $this->assertEquals('subsite-1', $result);
     }
 
     public function testWritingSubsiteID()
@@ -35,10 +40,13 @@ class FileSubsitesTest extends BaseSubsiteTest
         $subsite = $this->objFromFixture(Subsite::class, 'domaintest1');
         Config::modify()->set(FileSubsites::class, 'default_root_folders_global', true);
 
+        $method = new ReflectionMethod(File::class, 'onAfterUpload');
+        $method->setAccessible(true);
+
         Subsite::changeSubsite(0);
         $file = new File();
         $file->write();
-        $file->onAfterUpload();
+        $method->invoke($file);
         $this->assertEquals((int)$file->SubsiteID, 0);
 
         Subsite::changeSubsite($subsite->ID);
@@ -63,7 +71,7 @@ class FileSubsitesTest extends BaseSubsiteTest
         Config::modify()->set(FileSubsites::class, 'default_root_folders_global', true);
         $file = new File();
         $file->ParentID = $folder->ID;
-        $file->onAfterUpload();
+        $method->invoke($file);
         $this->assertEquals($folder->SubsiteID, $file->SubsiteID);
     }