Merge pull request #503 from creative-commoners/pulls/3/remove-depr

API Remove deprecated code

lang/en.yml

@@ -70,6 +70,8 @@ en:
     SubsiteField: Subsite
   SilverStripe\Subsites\Reports\SubsiteReportWrapper:
     ReportDropdown: Sites
+    ReportDropdownSubsite: Subsite
+    ReportDropdownAll: All
   Subsite:
     COPYSTRUCTURE: 'Copy structure from:'
     NOTEMPLATE: 'No template'

src/Extensions/SiteTreeSubsites.php

@@ -2,7 +2,6 @@
 
 namespace SilverStripe\Subsites\Extensions;
 
-use SilverStripe\Dev\Deprecation;
 use Page;
 use SilverStripe\CMS\Forms\SiteTreeURLSegmentField;
 use SilverStripe\CMS\Model\SiteTree;
@@ -10,7 +9,6 @@ use SilverStripe\Control\Controller;
 use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTP;
 use SilverStripe\Core\Config\Config;
-use SilverStripe\Core\Convert;
 use SilverStripe\Forms\CheckboxField;
 use SilverStripe\Forms\DropdownField;
 use SilverStripe\Forms\FieldList;
@@ -76,13 +74,7 @@ class SiteTreeSubsites extends DataExtension
             return;
         }
 
-        $subsiteID = null;
-        if (Subsite::$force_subsite) {
-            $subsiteID = Subsite::$force_subsite;
-        } else {
         $subsiteID = SubsiteState::singleton()->getSubsiteId();
-        }
-
         if ($subsiteID === null) {
             return;
         }
@@ -423,7 +415,7 @@ class SiteTreeSubsites extends DataExtension
         // This helps deal with Link() returning an absolute URL.
         $url = Director::absoluteURL($this->owner->Link($action));
         if ($this->owner->SubsiteID) {
-            $url = preg_replace('/\/\/[^\/]+\//', '//' . $this->owner->Subsite()->domain() . '/', $url ?? '');
+            $url = preg_replace('#//[^/]+#', '//' . $this->owner->Subsite()->domain(), $url);
         }
         return $url;
     }
@@ -444,22 +436,6 @@ class SiteTreeSubsites extends DataExtension
         return $link;
     }
 
-    /**
-     * This function is marked as deprecated for removal in 5.0.0 in silverstripe/cms
-     * so now simply passes execution to where the functionality exists for backwards compatiblity.
-     * CMS 4.0.0 SiteTree already throws a SilverStripe deprecation error before calling this function.
-     * @deprecated 2.2.0 Use updatePreviewLink() instead
-     *
-     * @param string|null $action
-     * @return string
-     */
-    public function alternatePreviewLink($action = null)
-    {
-        Deprecation::notice('2.2.0', 'Use updatePreviewLink() instead');
-        $link = '';
-        return $this->updatePreviewLink($link, $action);
-    }
-
     /**
      * Inject the subsite ID into the content so it can be used by frontend scripts.
      * @param $tags
@@ -525,22 +501,14 @@ class SiteTreeSubsites extends DataExtension
     public function augmentValidURLSegment()
     {
         // If this page is being filtered in the current subsite, then no custom validation query is required.
-        $subsite = Subsite::$force_subsite ?: SubsiteState::singleton()->getSubsiteId();
+        $subsiteID = SubsiteState::singleton()->getSubsiteId();
-        if (empty($this->owner->SubsiteID) || $subsite == $this->owner->SubsiteID) {
+        if (empty($this->owner->SubsiteID) || $subsiteID == $this->owner->SubsiteID) {
             return null;
         }
-
+        return SubsiteState::singleton()->withState(function (SubsiteState $newState) {
-        // Backup forced subsite
+            $newState->setSubsiteId($this->owner->SubsiteID);
-        $prevForceSubsite = Subsite::$force_subsite;
+            return (bool) $this->owner->validURLSegment();
-        Subsite::$force_subsite = $this->owner->SubsiteID;
+        });
-
-        // Repeat validation in the correct subsite
-        $isValid = $this->owner->validURLSegment();
-
-        // Restore
-        Subsite::$force_subsite = $prevForceSubsite;
-
-        return (bool)$isValid;
     }
 
     /**

src/Model/Subsite.php

@@ -52,14 +52,6 @@ class Subsite extends DataObject
      */
     public static $disable_subsite_filter = false;
 
-    /**
-     * Allows you to force a specific subsite ID, or comma separated list of IDs.
-     * Only works for reading. An object cannot be written to more than 1 subsite.
-     *
-     * @deprecated 2.0.0 Use SubsiteState::singleton()->withState() instead.
-     */
-    public static $force_subsite = null;
-
     /**
      * Whether to write a host-map.php file
      *
@@ -191,24 +183,6 @@ class Subsite extends DataObject
         return Subsite::get()->byID(SubsiteState::singleton()->getSubsiteId());
     }
 
-    /**
-     * This function gets the current subsite ID from the session. It used in the backend so Ajax requests
-     * use the correct subsite. The frontend handles subsites differently. It calls getSubsiteIDForDomain
-     * directly from ModelAsController::getNestedController. Only gets Subsite instances which have their
-     * {@link IsPublic} flag set to TRUE.
-     *
-     * You can simulate subsite access without creating virtual hosts by appending ?SubsiteID=<ID> to the request.
-     *
-     * @return int ID of the current subsite instance
-     *
-     * @deprecated 2.0.0 Use SubsiteState::singleton()->getSubsiteId() instead
-     */
-    public static function currentSubsiteID()
-    {
-        Deprecation::notice('2.0.0', 'Use SubsiteState::singleton()->getSubsiteId() instead');
-        return SubsiteState::singleton()->getSubsiteId();
-    }
-
     /**
      * Switch to another subsite through storing the subsite identifier in the current PHP session.
      * Only takes effect when {@link SubsiteState::singleton()->getUseSessions()} is set to TRUE.
@@ -218,7 +192,7 @@ class Subsite extends DataObject
     public static function changeSubsite($subsite)
     {
         // Session subsite change only meaningful if the session is active.
-        // Otherwise we risk setting it to wrong value, e.g. if we rely on currentSubsiteID.
+        // Otherwise we risk setting it to wrong value
         if (!SubsiteState::singleton()->getUseSessions()) {
             return;
         }
@@ -352,6 +326,15 @@ class Subsite extends DataObject
         self::$disable_subsite_filter = $disabled;
     }
 
+    public static function withDisabledSubsiteFilter(callable $callable, bool $disabled = true): mixed
+    {
+        $orig = self::$disable_subsite_filter;
+        self::disable_subsite_filter($disabled);
+        $ret = $callable();
+        self::disable_subsite_filter($orig);
+        return $ret;
+    }
+
     /**
      * Flush caches on database reset
      */

src/Reports/SubsiteReportWrapper.php

@@ -2,10 +2,11 @@
 
 namespace SilverStripe\Subsites\Reports;
 
+use SilverStripe\Forms\DropdownField;
 use SilverStripe\Forms\FieldList;
-use SilverStripe\Forms\TreeMultiselectField;
 use SilverStripe\Reports\ReportWrapper;
 use SilverStripe\Subsites\Model\Subsite;
+use SilverStripe\Subsites\State\SubsiteState;
 
 /**
  * Creates a subsite-aware version of another report.
@@ -13,6 +14,7 @@ use SilverStripe\Subsites\Model\Subsite;
  */
 class SubsiteReportWrapper extends ReportWrapper
 {
+    private const SUBSITE_ID_ALL = -1;
 
     /**
      * @return FieldList
@@ -20,23 +22,22 @@ class SubsiteReportWrapper extends ReportWrapper
     public function parameterFields()
     {
         $subsites = Subsite::accessible_sites('CMS_ACCESS_CMSMain', true);
-        $options = $subsites->toDropdownMap('ID', 'Title');
+        $options = [self::SUBSITE_ID_ALL => _t(__CLASS__ . '.ReportDropdownAll', 'All')] + $subsites->map()->toArray();
 
-        $subsiteField = TreeMultiselectField::create(
+        $subsiteField = DropdownField::create(
-            'Subsites',
+            'Subsite',
-            _t(__CLASS__ . '.ReportDropdown', 'Sites'),
+            _t(__CLASS__ . '.ReportDropdownSubsite', 'Subsite'),
             $options
         );
-        $subsiteField->setValue(array_keys($options ?? []));
 
         // We don't need to make the field editable if only one subsite is available
-        if (sizeof($options ?? []) <= 1) {
+        if (sizeof($options ?? []) <= 2) {
             $subsiteField = $subsiteField->performReadonlyTransformation();
         }
 
         $fields = parent::parameterFields();
         if ($fields) {
-            $fields->insertBefore($fields->First()->Name(), $subsiteField);
+            $fields->insertBefore($fields->First()->getName(), $subsiteField);
         } else {
             $fields = FieldList::create($subsiteField);
         }
@@ -49,37 +50,40 @@ class SubsiteReportWrapper extends ReportWrapper
     public function columns()
     {
         $columns = parent::columns();
-        $columns['Subsite.Title'] = Subsite::class;
+        $columns['Subsite.Title'] = _t(__CLASS__ . '.ReportDropdownSubsite', 'Subsite');
         return $columns;
     }
 
-    ///////////////////////////////////////////////////////////////////////////////////////////
+    public function sourceQuery($params)
-    // Querying
-
-    /**
-     * @param arary $params
-     * @return void
-     */
-    public function beforeQuery($params)
     {
-        // The user has select a few specific sites
+        $subsiteID = (int) ($params['Subsite'] ?? self::SUBSITE_ID_ALL);
-        if (!empty($params['Subsites'])) {
+        if ($subsiteID === self::SUBSITE_ID_ALL) {
-            Subsite::$force_subsite = $params['Subsites'];
+            return Subsite::withDisabledSubsiteFilter(function () use ($params) {
-
+                return parent::sourceQuery($params);
-            // Default: restrict to all accessible sites
+            });
-        } else {
-            $subsites = Subsite::accessible_sites('CMS_ACCESS_CMSMain');
-            $options = $subsites->toDropdownMap('ID', 'Title');
-            Subsite::$force_subsite = join(',', array_keys($options ?? []));
         }
+        return SubsiteState::singleton()->withState(function (SubsiteState $newState) use ($subsiteID, $params) {
+            $newState->setSubsiteId($subsiteID);
+            return parent::sourceQuery($params);
+        });
     }
 
-    /**
+    public function sourceRecords($params = [], $sort = null, $limit = null)
-     * @return void
-     */
-    public function afterQuery()
     {
-        // Manually manage the subsite filtering
+        $subsiteID = (int) ($params['Subsite'] ?? self::SUBSITE_ID_ALL);
-        Subsite::$force_subsite = null;
+        if ($subsiteID === self::SUBSITE_ID_ALL) {
+            return Subsite::withDisabledSubsiteFilter(function () use ($params, $sort, $limit) {
+                return parent::sourceRecords($params, $sort, $limit);
+            });
+        }
+        return SubsiteState::singleton()->withState(function (SubsiteState $newState) use (
+            $subsiteID,
+            $params,
+            $sort,
+            $limit
+        ) {
+            $newState->setSubsiteId($subsiteID);
+            return parent::sourceRecords($params, $sort, $limit);
+        });
     }
 }

src/State/SubsiteState.php

@@ -13,28 +13,16 @@ class SubsiteState implements Resettable
 {
     use Injectable;
 
-    /**
+    protected ?int $subsiteId = null;
-     * @var int|null
-     */
-    protected $subsiteId;
 
+    protected ?int $originalSubsiteId = null;
 
-    /**
+    protected ?bool $useSessions = null;
-     * @var int|null
-     */
-    protected $originalSubsiteId;
-
-    /**
-     * @var bool
-     */
-    protected $useSessions;
 
     /**
      * Get the current subsite ID
-     *
-     * @return int|null
      */
-    public function getSubsiteId()
+    public function getSubsiteId(): ?int
     {
         return $this->subsiteId;
     }
@@ -42,11 +30,8 @@ class SubsiteState implements Resettable
     /**
      * Set the current subsite ID, and track the first subsite ID set as the "original". This is used to check
      * whether the ID has been changed through a request.
-     *
-     * @param int $id
-     * @return $this
      */
-    public function setSubsiteId($id)
+    public function setSubsiteId(?int $id): static
     {
         if (is_null($this->originalSubsiteId)) {
             $this->originalSubsiteId = (int) $id;
@@ -59,21 +44,16 @@ class SubsiteState implements Resettable
 
     /**
      * Get whether to use sessions for storing the subsite ID
-     *
-     * @return bool
      */
-    public function getUseSessions()
+    public function getUseSessions(): ?bool
     {
         return $this->useSessions;
     }
 
     /**
      * Set whether to use sessions for storing the subsite ID
-     *
-     * @param bool $useSessions
-     * @return $this
      */
-    public function setUseSessions($useSessions)
+    public function setUseSessions(?bool $useSessions): static
     {
         $this->useSessions = $useSessions;
 
@@ -82,10 +62,8 @@ class SubsiteState implements Resettable
 
     /**
      * Get whether the subsite ID has been changed during a request, based on the original and current IDs
-     *
-     * @return bool
      */
-    public function getSubsiteIdWasChanged()
+    public function getSubsiteIdWasChanged(): bool
     {
         return $this->originalSubsiteId !== $this->getSubsiteId();
     }
@@ -97,7 +75,7 @@ class SubsiteState implements Resettable
      * @param callable $callback Callback to run. Will be passed the nested state as a parameter
      * @return mixed Result of callback
      */
-    public function withState(callable $callback)
+    public function withState(callable $callback): mixed
     {
         $newState = clone $this;
         try {
@@ -111,7 +89,7 @@ class SubsiteState implements Resettable
     /**
      * Reset the local cache of the singleton
      */
-    public static function reset()
+    public static function reset(): void
     {
         SubsiteState::singleton()->resetState();
     }
@@ -119,7 +97,7 @@ class SubsiteState implements Resettable
     /**
      * Reset the local cache of this object
      */
-    public function resetState()
+    public function resetState(): void
     {
         $this->originalSubsiteId = null;
         $this->subsiteId = null;

tests/php/BaseSubsiteTest.php

@@ -15,7 +15,6 @@ class BaseSubsiteTest extends SapphireTest
 
         SubsiteState::singleton()->setUseSessions(true);
         Config::modify()->set(Subsite::class, 'write_hostmap', false);
-        Subsite::$force_subsite = null;
     }
 
     /**

tests/php/SiteTreeSubsitesTest.php

@@ -275,8 +275,8 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest
 
         // Staff is shifted to top level and given a unique url segment
         $domain = $otherSubsite->domain();
-        $this->assertEquals('http://' . $domain . '/staff-2/', $staffPage2->AbsoluteLink());
+        $this->assertEquals('http://' . $domain . '/staff-2', $staffPage2->AbsoluteLink());
-        $this->assertEquals('http://' . $domain . '/contact-us-2/', $contactPage2->AbsoluteLink());
+        $this->assertEquals('http://' . $domain . '/contact-us-2', $contactPage2->AbsoluteLink());
     }
 
     public function testPageTypesBlacklistInCMSMain()
@@ -443,13 +443,13 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest
     public function provideAlternateAbsoluteLink()
     {
         return [
-            ['home', null, 'http://localhost/'],
+            ['home', null, 'http://localhost'],
             ['home', 'myaction', 'http://localhost/home/myaction'],
-            ['contact', null, 'http://localhost/contact-us/'],
+            ['contact', null, 'http://localhost/contact-us'],
             ['contact', 'myaction', 'http://localhost/contact-us/myaction'],
-            ['subsite1_home', null, 'http://subsite1.localhost/'],
+            ['subsite1_home', null, 'http://subsite1.localhost'],
             ['subsite1_home', 'myaction', 'http://subsite1.localhost/home/myaction'],
-            ['subsite1_contactus', null, 'http://subsite1.localhost/contact-us/'],
+            ['subsite1_contactus', null, 'http://subsite1.localhost/contact-us'],
             ['subsite1_contactus', 'myaction', 'http://subsite1.localhost/contact-us/myaction']
         ];
     }
@@ -463,7 +463,7 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest
     public function testAlternateAbsoluteLink($pageFixtureName, $action, $expectedAbsoluteLink)
     {
         // Setting a control value, in case base url is set for the installation under test
-        Config::modify()->set(Director::class, 'alternate_base_url', 'http://localhost/');
+        Config::modify()->set(Director::class, 'alternate_base_url', 'http://localhost');
 
         /** @var Page $page */
         $page = $this->objFromFixture(Page::class, $pageFixtureName);

tests/php/SubsiteTest.php

@@ -301,8 +301,10 @@ class SubsiteTest extends BaseSubsiteTest
         );
 
         $this->assertEquals($_SERVER['HTTP_HOST'], singleton(Subsite::class)->PrimaryDomain);
+
+        // If there is a Director::baseURL() value this will also be included
         $this->assertEquals(
-            'http://' . $_SERVER['HTTP_HOST'] . Director::baseURL(),
+            'http://' . $_SERVER['HTTP_HOST'],
             singleton(Subsite::class)->absoluteBaseURL()
         );
     }
@@ -328,22 +330,22 @@ class SubsiteTest extends BaseSubsiteTest
     public function domainProtocolProvider()
     {
         return [
-            [Subsite::class, 'domaintest2', false, 'http://two.mysite.com/'],
+            [Subsite::class, 'domaintest2', false, 'http://two.mysite.com'],
-            [SubsiteDomain::class, 'dt2a', false, 'http://two.mysite.com/'],
+            [SubsiteDomain::class, 'dt2a', false, 'http://two.mysite.com'],
-            [SubsiteDomain::class, 'dt2b', false, 'http://subsite.mysite.com/'],
+            [SubsiteDomain::class, 'dt2b', false, 'http://subsite.mysite.com'],
-            [Subsite::class, 'domaintest4', false, 'https://www.primary.com/'],
+            [Subsite::class, 'domaintest4', false, 'https://www.primary.com'],
-            [SubsiteDomain::class, 'dt4a', false, 'https://www.primary.com/'],
+            [SubsiteDomain::class, 'dt4a', false, 'https://www.primary.com'],
-            [SubsiteDomain::class, 'dt4b', false, 'http://www.secondary.com/'],
+            [SubsiteDomain::class, 'dt4b', false, 'http://www.secondary.com'],
-            [Subsite::class, 'domaintest5', false, 'http://www.tertiary.com/'],
+            [Subsite::class, 'domaintest5', false, 'http://www.tertiary.com'],
-            [SubsiteDomain::class, 'dt5', false,  'http://www.tertiary.com/'],
+            [SubsiteDomain::class, 'dt5', false,  'http://www.tertiary.com'],
-            [Subsite::class, 'domaintest2', true, 'https://two.mysite.com/'],
+            [Subsite::class, 'domaintest2', true, 'https://two.mysite.com'],
-            [SubsiteDomain::class, 'dt2a', true, 'https://two.mysite.com/'],
+            [SubsiteDomain::class, 'dt2a', true, 'https://two.mysite.com'],
-            [SubsiteDomain::class, 'dt2b', true, 'https://subsite.mysite.com/'],
+            [SubsiteDomain::class, 'dt2b', true, 'https://subsite.mysite.com'],
-            [Subsite::class, 'domaintest4', true, 'https://www.primary.com/'],
+            [Subsite::class, 'domaintest4', true, 'https://www.primary.com'],
-            [SubsiteDomain::class, 'dt4a', true, 'https://www.primary.com/'],
+            [SubsiteDomain::class, 'dt4a', true, 'https://www.primary.com'],
-            [SubsiteDomain::class, 'dt4b', true, 'http://www.secondary.com/'],
+            [SubsiteDomain::class, 'dt4b', true, 'http://www.secondary.com'],
-            [Subsite::class, 'domaintest5', true, 'http://www.tertiary.com/'],
+            [Subsite::class, 'domaintest5', true, 'http://www.tertiary.com'],
-            [SubsiteDomain::class, 'dt5', true, 'http://www.tertiary.com/'],
+            [SubsiteDomain::class, 'dt5', true, 'http://www.tertiary.com'],
         ];
     }