Merge pull request #335 from creative-commoners/pulls/2.0/more-config…

…ging

API Convert most of Subsite public statics to config properties

.travis.yml

@@ -46,7 +46,7 @@ before_script:
 script:
   - if [[ $PHPUNIT_TEST ]]; then vendor/bin/phpunit; fi
   - if [[ $PHPUNIT_COVERAGE_TEST ]]; then phpdbg -qrr vendor/bin/phpunit --coverage-clover=coverage.xml; fi
-  - if [[ $PHPCS_TEST ]]; then vendor/bin/phpcs --standard=vendor/silverstripe/framework/phpcs.xml.dist src tests *.php ; fi
+  - if [[ $PHPCS_TEST ]]; then vendor/bin/phpcs --standard=vendor/silverstripe/framework/phpcs.xml.dist src tests *.php --ignore=host-map.php; fi
   - if [[ $BEHAT_TEST ]]; then vendor/bin/behat @subsites; fi
 
 after_success:

CHANGELOG.md

@@ -8,6 +8,13 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 * Updating to be compatible with SilverStripe 4
 * Subsite specific theme is now added to default theme, as themes are now cascadable
+* Global subsite information moved to injectable `SubsiteState` singleton service
+* `FileExtension:::default_root_folders_global` converted to a configuration property
+* `Subsite::$check_is_public` converted to a configuration property
+* `Subsite::$strict_subdomain_matching` converted to a configuration property
+* `Subsite::$force_subsite` deprecated and will be removed in future -  use `SubsiteState::singleton()->withState()` instead
+* `Subsite::$write_hostmap` converted to a configuration property
+* `Subsite::$allowed_themes` made protected
 
 ## [1.2.3]
 

src/Extensions/FileSubsites.php

@@ -16,9 +16,13 @@
  */
 class FileSubsites extends DataExtension
 {
-    // If this is set to true, all folders created will be default be
-    // considered 'global', unless set otherwise
-    public static $default_root_folders_global = false;
+    /**
+     * If this is set to true, all folders created will be default be considered 'global', unless set otherwise
+     *
+     * @config
+     * @var bool
+     */
+    private static $default_root_folders_global = false;
 
     private static $has_one = [
         'Subsite' => Subsite::class,
@@ -81,7 +85,7 @@ public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
     public function onBeforeWrite()
     {
         if (!$this->owner->ID && !$this->owner->SubsiteID) {
-            if (self::$default_root_folders_global) {
+            if ($this->owner->config()->get('default_root_folders_global')) {
                 $this->owner->SubsiteID = 0;
             } else {
                 $this->owner->SubsiteID = SubsiteState::singleton()->getSubsiteId();

src/Model/Subsite.php

@@ -8,6 +8,7 @@
 use SilverStripe\Control\Session;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Core\Manifest\ModuleLoader;
 use SilverStripe\Dev\Deprecation;
 use SilverStripe\Forms\CheckboxSetField;
 use SilverStripe\Forms\DropdownField;
@@ -46,21 +47,27 @@ class Subsite extends DataObject
 
     /**
      * @var boolean $disable_subsite_filter If enabled, bypasses the query decoration
-     * to limit DataObject::get*() calls to a specific subsite. Useful for debugging.
+     * to limit DataObject::get*() calls to a specific subsite. Useful for debugging. Note that
+     * for now this is left as a public static property to avoid having to nest and mutate the
+     * configuration manifest.
      */
     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..3.0.0 Use SubsiteState::singleton()->withState() instead.
      */
     public static $force_subsite = null;
 
     /**
+     * Whether to write a host-map.php file
      *
+     * @config
      * @var boolean
      */
-    public static $write_hostmap = true;
+    private static $write_hostmap = true;
 
     /**
      * Memory cache of accessible sites
@@ -77,23 +84,31 @@ class Subsite extends DataObject
     private static $_cache_subsite_for_domain = [];
 
     /**
-     * @var array $allowed_themes Numeric array of all themes which are allowed to be selected for all subsites.
+     * Numeric array of all themes which are allowed to be selected for all subsites.
      * Corresponds to subfolder names within the /themes folder. By default, all themes contained in this folder
      * are listed.
+     *
+     * @var array
      */
-    private static $allowed_themes = [];
+    protected static $allowed_themes = [];
 
     /**
-     * @var Boolean If set to TRUE, don't assume 'www.example.com' and 'example.com' are the same.
+     * If set to TRUE, don't assume 'www.example.com' and 'example.com' are the same.
      * Doesn't affect wildcard matching, so '*.example.com' will match 'www.example.com' (but not 'example.com')
      * in both TRUE or FALSE setting.
+     *
+     * @config
+     * @var boolean
      */
-    public static $strict_subdomain_matching = false;
+    private static $strict_subdomain_matching = false;
 
     /**
-     * @var boolean Respects the IsPublic flag when retrieving subsites
+     * Respects the IsPublic flag when retrieving subsites
+     *
+     * @config
+     * @var boolean
      */
-    public static $check_is_public = true;
+    private static $check_is_public = true;
 
     /*** @return array
      */
@@ -247,32 +262,37 @@ public static function getSubsiteIDForDomain($host = null, $checkPermissions = t
         $matchingDomains = null;
         $cacheKey = null;
         if ($host) {
-            if (!self::$strict_subdomain_matching) {
+            if (!static::config()->get('strict_subdomain_matching')) {
                 $host = preg_replace('/^www\./', '', $host);
             }
 
             $currentUserId = Security::getCurrentUser() ? Security::getCurrentUser()->ID : 0;
-            $cacheKey = implode('_', [$host, $currentUserId, self::$check_is_public]);
+            $cacheKey = implode('_', [$host, $currentUserId, static::config()->get('check_is_public')]);
             if (isset(self::$_cache_subsite_for_domain[$cacheKey])) {
                 return self::$_cache_subsite_for_domain[$cacheKey];
             }
 
             $SQL_host = Convert::raw2sql($host);
 
+            $schema = DataObject::getSchema();
+
             /** @skipUpgrade */
-            if (!in_array('SubsiteDomain', DB::table_list())) {
+            $domainTableName = $schema->tableName(SubsiteDomain::class);
+            if (!in_array($domainTableName, DB::table_list())) {
                 // Table hasn't been created yet. Might be a dev/build, skip.
                 return 0;
             }
 
+            $subsiteTableName = $schema->tableName(__CLASS__);
             /** @skipUpgrade */
             $matchingDomains = DataObject::get(
                 SubsiteDomain::class,
-                "'$SQL_host' LIKE replace(\"SubsiteDomain\".\"Domain\",'*','%')",
+                "'$SQL_host' LIKE replace(\"{$domainTableName}\".\"Domain\",'*','%')",
                 '"IsPrimary" DESC'
             )->innerJoin(
-                'Subsite',
-                '"Subsite"."ID" = "SubsiteDomain"."SubsiteID" AND "Subsite"."IsPublic"=1'
+                $subsiteTableName,
+                '"' . $subsiteTableName . '"."ID" = "SubsiteDomain"."SubsiteID" AND "'
+                    . $subsiteTableName . '"."IsPublic"=1'
             );
         }
 
@@ -530,12 +550,13 @@ public static function accessible_sites(
      */
     public static function writeHostMap($file = null)
     {
-        if (!self::$write_hostmap) {
+        if (!static::config()->get('write_hostmap')) {
             return;
         }
 
         if (!$file) {
-            $file = Director::baseFolder() . '/subsites/host-map.php';
+            $subsitesPath = ModuleLoader::getModule('silverstripe/subsites')->getRelativePath();
+            $file = Director::baseFolder() . $subsitesPath . '/host-map.php';
         }
         $hostmap = [];
 
@@ -547,7 +568,7 @@ public static function writeHostMap($file = null)
                 if ($domains) {
                     foreach ($domains as $domain) {
                         $domainStr = $domain->Domain;
-                        if (!self::$strict_subdomain_matching) {
+                        if (!static::config()->get('strict_subdomain_matching')) {
                             $domainStr = preg_replace('/^www\./', '', $domainStr);
                         }
                         $hostmap[$domainStr] = $subsite->domain();
@@ -758,7 +779,7 @@ public function fieldLabels($includerelations = true)
      */
     public function allowedThemes()
     {
-        if ($themes = $this->stat('allowed_themes')) {
+        if ($themes = self::$allowed_themes) {
             return ArrayLib::valuekey($themes);
         }
 

tests/php/BaseSubsiteTest.php

@@ -2,6 +2,7 @@
 
 namespace SilverStripe\Subsites\Tests;
 
+use SilverStripe\Core\Config\Config;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Subsites\Model\Subsite;
 use SilverStripe\Subsites\State\SubsiteState;
@@ -13,6 +14,7 @@ protected function setUp()
         parent::setUp();
 
         SubsiteState::singleton()->setUseSessions(true);
+        Config::modify()->set(Subsite::class, 'write_hostmap', false);
         Subsite::$force_subsite = null;
     }
 

tests/php/FileSubsitesTest.php

@@ -4,6 +4,7 @@
 
 use SilverStripe\Assets\File;
 use SilverStripe\Assets\Folder;
+use SilverStripe\Core\Config\Config;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Subsites\Extensions\FileSubsites;
 use SilverStripe\Subsites\Model\Subsite;
@@ -31,7 +32,7 @@ public function testWritingSubsiteID()
         $this->logInAs('admin');
 
         $subsite = $this->objFromFixture(Subsite::class, 'domaintest1');
-        FileSubsites::$default_root_folders_global = true;
+        Config::modify()->set(FileSubsites::class, 'default_root_folders_global', true);
 
         Subsite::changeSubsite(0);
         $file = new File();
@@ -47,7 +48,7 @@ public function testWritingSubsiteID()
         $this->assertEquals((int)$file->SubsiteID, 0);
         $this->assertTrue($file->canEdit());
 
-        FileSubsites::$default_root_folders_global = false;
+        Config::modify()->set(FileSubsites::class, 'default_root_folders_global', false);
 
         Subsite::changeSubsite($subsite->ID);
         $file = new File();
@@ -58,7 +59,7 @@ public function testWritingSubsiteID()
         $folder = new Folder();
         $folder->write();
         $this->assertEquals($folder->SubsiteID, $subsite->ID);
-        FileSubsites::$default_root_folders_global = true;
+        Config::modify()->set(FileSubsites::class, 'default_root_folders_global', true);
         $file = new File();
         $file->ParentID = $folder->ID;
         $file->onAfterUpload();

tests/php/SubsiteAdminTest.php

@@ -5,13 +5,21 @@
 use SilverStripe\CMS\Controllers\CMSMain;
 use SilverStripe\Control\Director;
 use SilverStripe\Control\Session;
+use SilverStripe\Core\Config\Config;
 use SilverStripe\Security\Member;
 use SilverStripe\Subsites\Model\Subsite;
 
 class SubsiteAdminTest extends BaseSubsiteTest
 {
     protected static $fixture_file = 'SubsiteTest.yml';
 
+    protected function setUp()
+    {
+        parent::setUp();
+
+        Config::modify()->set(Subsite::class, 'write_hostmap', false);
+    }
+
     protected function adminLoggedInSession()
     {
         return new Session([
@@ -24,7 +32,6 @@ protected function adminLoggedInSession()
      */
     public function testBasicView()
     {
-        Subsite::$write_hostmap = false;
         $subsite1ID = $this->objFromFixture(Subsite::class, 'domaintest1')->ID;
 
         // Open the admin area logged in as admin

tests/php/SubsiteTest.php

@@ -17,13 +17,6 @@ class SubsiteTest extends BaseSubsiteTest
 {
     protected static $fixture_file = 'SubsiteTest.yml';
 
-    /**
-     * Original value of {@see SubSite::$strict_subdomain_matching}
-     *
-     * @var bool
-     */
-    protected $origStrictSubdomainMatching = null;
-
     /**
      * Original value of $_REQUEST
      *
@@ -35,16 +28,17 @@ protected function setUp()
     {
         parent::setUp();
 
-        Config::modify()->set(Director::class, 'alternate_base_url', '/');
-        $this->origStrictSubdomainMatching = Subsite::$strict_subdomain_matching;
+        Config::modify()
+            ->set(Director::class, 'alternate_base_url', '/')
+            ->set(Subsite::class, 'strict_subdomain_matching', false)
+            ->set(Subsite::class, 'write_hostmap', false);
+
         $this->origServer = $_SERVER;
-        Subsite::$strict_subdomain_matching = false;
     }
 
     protected function tearDown()
     {
         $_SERVER = $this->origServer;
-        Subsite::$strict_subdomain_matching = $this->origStrictSubdomainMatching;
 
         parent::tearDown();
     }
@@ -54,8 +48,6 @@ protected function tearDown()
      */
     public function testSubsiteCreation()
     {
-        Subsite::$write_hostmap = false;
-
         // Create the instance
         $template = $this->objFromFixture(Subsite::class, 'main');
 
@@ -191,7 +183,7 @@ public function testStrictSubdomainMatching()
             'www.wildcard.com' => false,
         ]);
 
-        Subsite::$strict_subdomain_matching = false;
+        Config::modify()->set(Subsite::class, 'strict_subdomain_matching', false);
 
         $this->assertEquals(
             $subsite1->ID,
@@ -214,7 +206,7 @@ public function testStrictSubdomainMatching()
             'Doesn\'t match www prefix without strict check, even if a wildcard subdomain is in place'
         );
 
-        Subsite::$strict_subdomain_matching = true;
+        Config::modify()->set(Subsite::class, 'strict_subdomain_matching', true);
 
         $this->assertEquals(
             $subsite1->ID,

tests/php/SubsitesVirtualPageTest.php

@@ -29,6 +29,8 @@ protected function setUp()
     {
         parent::setUp();
 
+        Config::modify()->set(Subsite::class, 'write_hostmap', false);
+
         // Set backend root to /DataDifferencerTest
         TestAssetStore::activate('SubsitesVirtualPageTest');
 
@@ -54,8 +56,6 @@ public function tearDown()
     // Attempt to bring main:linky to subsite2:linky
     public function testVirtualPageFromAnotherSubsite()
     {
-        Subsite::$write_hostmap = false;
-
         $subsite = $this->objFromFixture(Subsite::class, 'subsite2');
 
         Subsite::changeSubsite($subsite->ID);
@@ -261,7 +261,6 @@ public function testSubsiteVirtualPageCanHaveSameUrlsegmentAsOtherSubsite()
     {
         $this->markTestIncomplete('@todo fix this test');
 
-        Subsite::$write_hostmap = false;
         $subsite1 = $this->objFromFixture(Subsite::class, 'subsite1');
         $subsite2 = $this->objFromFixture(Subsite::class, 'subsite2');
         Subsite::changeSubsite($subsite1->ID);