Merge branch '2.4' into 2

MNT Add dev version of framework

.github/workflows/ci.yml

@@ -0,0 +1,11 @@
+name: CI
+
+on:
+  push:
+  pull_request:
+  workflow_dispatch:
+
+jobs:
+  ci:
+    name: CI
+    uses: silverstripe/gha-ci/.github/workflows/ci.yml@v1

.github/workflows/deploy-userhelp-docs.yml

@@ -0,0 +1,16 @@
+name: Deploy Userhelp Docs
+on:
+  push:
+    branches:
+      - '3'
+      - '2'
+      - 'master'
+    paths:
+      - 'docs/en/userguide/**'
+jobs:
+  deploy:
+    name: deploy-userhelp-docs
+    runs-on: ubuntu-latest
+    steps:
+      - name: Run build hook
+        run: curl -X POST -d {} https://api.netlify.com/build_hooks/${{ secrets.NETLIFY_BUILD_HOOK }}

.github/workflows/dispatch-ci.yml

@@ -0,0 +1,16 @@
+name: Dispatch CI
+
+on:
+  # At 11:10 AM UTC, only on Sunday and Monday
+  schedule:
+    - cron: '10 11 * * 0,1'
+
+jobs:
+  dispatch-ci:
+    name: Dispatch CI
+    # Only run cron on the silverstripe account
+    if: (github.event_name == 'schedule' && github.repository_owner == 'silverstripe') || (github.event_name != 'schedule')
+    runs-on: ubuntu-latest
+    steps:
+      - name: Dispatch CI
+        uses: silverstripe/gha-dispatch-ci@v1

.github/workflows/keepalive.yml

@@ -0,0 +1,17 @@
+name: Keepalive
+
+on:
+  workflow_dispatch:
+  # The 8th of every month at 11:50am UTC
+  schedule:
+    - cron: '50 11 8 * *'
+
+jobs:
+  keepalive:
+    name: Keepalive
+    # Only run cron on the silverstripe account
+    if: (github.event_name == 'schedule' && github.repository_owner == 'silverstripe') || (github.event_name != 'schedule')
+    runs-on: ubuntu-latest
+    steps:
+      - name: Keepalive
+        uses: silverstripe/gha-keepalive@v1

.scrutinizer.yml

@@ -1,15 +0,0 @@
-inherit: true
-
-build:
-  nodes:
-    analysis:
-      tests:
-        override: [php-scrutinizer-run]
-
-checks:
-  php:
-    code_rating: true
-    duplication: true
-
-filter:
-  paths: [src/*, tests/*]

.travis.yml

@@ -1,34 +0,0 @@
-language: php
-
-env:
-  global:
-    - COMPOSER_ROOT_VERSION=2.0.x-dev
-
-matrix:
-  include:
-    - php: 5.6
-      env: DB=MYSQL PHPCS_TEST=1 PHPUNIT_TEST=1
-    - php: 7.0
-      env: DB=MYSQL PHPUNIT_TEST=1
-    - php: 7.1
-      env: DB=PGSQL PHPUNIT_COVERAGE_TEST=1
-    - php: 7.2
-      env: DB=MYSQL PHPUNIT_TEST=1
-
-before_script:
-  # Init PHP
-  - phpenv rehash
-  - phpenv config-rm xdebug.ini
-
-  # Install composer dependencies
-  - composer validate
-  - if [[ $DB == PGSQL ]]; then composer require --no-update silverstripe/postgresql 2.0.x-dev; fi
-  - composer install --prefer-dist --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile
-
-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 src/ tests/; fi
-
-after_success:
-  - if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml; fi

.tx/config

@@ -1,8 +1,9 @@
 [main]
 host = https://www.transifex.com
 
-[silverstripe-versionfeed.master]
+[o:silverstripe:p:silverstripe-versionfeed:r:master]
 file_filter = lang/<lang>.yml
 source_file = lang/en.yml
 source_lang = en
 type        = YML
+

README.md

@@ -1,8 +1,7 @@
 # Version Feed
 
-[![Build Status](http://img.shields.io/travis/silverstripe/silverstripe-versionfeed.svg?style=flat)](https://travis-ci.org/silverstripe/silverstripe-versionfeed)
-[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/silverstripe/silverstripe-versionfeed/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/silverstripe/silverstripe-versionfeed/?branch=master)
-[![codecov](https://codecov.io/gh/silverstripe/silverstripe-versionfeed/branch/master/graph/badge.svg)](https://codecov.io/gh/silverstripe/silverstripe-versionfeed)
+[![CI](https://github.com/silverstripe/silverstripe-versionfeed/actions/workflows/ci.yml/badge.svg)](https://github.com/silverstripe/silverstripe-versionfeed/actions/workflows/ci.yml)
+[![Silverstripe supported module](https://img.shields.io/badge/silverstripe-supported-0071C4.svg)](https://www.silverstripe.org/software/addons/silverstripe-commercially-supported-module-list/)
 
 ## Overview
 
@@ -10,13 +9,13 @@ The module creates an RSS feed on each page with their change history, as well a
 
 ## Requirements
 
- * SilverStripe ^4
+ * Silverstripe ^4
 
-**Note:** For a SilverStripe 3.x compatible version, please use [the 1.x release line](https://github.com/silverstripe/silverstripe-versionfeed/tree/1.2).
+**Note:** For a Silverstripe 3.x compatible version, please use [the 1.x release line](https://github.com/silverstripe/silverstripe-versionfeed/tree/1.2).
 
 ## Installation
 
-Install with composer by running `composer require silverstripe/versionfeed` in the root of your SilverStripe project.
+Install with composer by running `composer require silverstripe/versionfeed` in the root of your Silverstripe project.
 
 ## Usage
 

_config/versionfeed.yml

@@ -6,7 +6,7 @@ SilverStripe\Core\Injector\Injector:
   ContentFilter:
     class: SilverStripe\VersionFeed\Filters\CachedContentFilter
     constructor:
-      - %$RateLimitFilter
+      - '%$RateLimitFilter'
   Psr\SimpleCache\CacheInterface.VersionFeedController:
     factory: SilverStripe\Core\Cache\CacheFactory
     constructor:
@@ -22,4 +22,4 @@ SilverStripe\CMS\Controllers\ContentController:
     - SilverStripe\VersionFeed\VersionFeedController
 SilverStripe\VersionFeed\VersionFeedController:
   dependencies:
-    ContentFilter: %$ContentFilter
+    ContentFilter: '%$ContentFilter'

composer.json

@@ -16,12 +16,14 @@
         }
     ],
     "require": {
+        "php": "^7.4 || ^8.0",
         "silverstripe/cms": "^4",
         "silverstripe/versioned": "^1",
         "silverstripe/siteconfig": "^4"
     },
     "require-dev": {
-        "phpunit/phpunit": "^5.7",
+        "phpunit/phpunit": "^9.5",
+        "silverstripe/framework": "^4.10",
         "squizlabs/php_codesniffer": "^3.0"
     },
     "autoload": {
@@ -30,11 +32,6 @@
             "SilverStripe\\VersionFeed\\Tests\\": "tests/"
         }
     },
-    "extra": {
-        "branch-alias": {
-            "dev-master": "2.x-dev"
-        }
-    },
     "minimum-stability": "dev",
     "prefer-stable": true
 }

docs/en/developer.md

@@ -9,11 +9,14 @@ the extensions defined on the extension.
 
 ### Enabling / Disabling
 
-The `allchanges` feed can be disabled by setting the `SilverStripe\VersionFeed\VersionFeed.allchanges_enabled` config to false.
+By default the `allchanges` and `changes` feed are disabled.
+
+The `allchanges` feed can be enabled by setting the `SilverStripe\VersionFeed\VersionFeed.allchanges_enabled` config to true. If this is true, then the allchanges feed can still be disabled by unchecking the "All page changes" checkbox in the "Settings" section in the CMS. 
+
+Likewise, the `changes` feed for each page can be globally enabled by setting the `SilverStripe\VersionFeed\VersionFeed.changes_enabled`
+config to true. If this is true, then each page can still be individually disabled by unchecking the
+'Make history public' checkbox in the CMS under page settings.
 
-Likewise, the `changes` feed for each page can be globally disabled by setting the `SilverStripe\VersionFeed\VersionFeed.changes_enabled`
-config to false. If this left true, then each page can still be individually disabled by unchecking the
-'Make History Public' checkbox in the CMS under page settings.
 See [user documentation on enabling / disabling](userguide/index.md#enabling--disabling).
 
 ### Default RSS action

docs/en/userguide/index.md

@@ -1,5 +1,7 @@
+---
 title: Content change RSS
 summary: Adds page or site wide RSS feeds that display content changes
+---
 
 # Content change RSS
 
@@ -16,12 +18,12 @@ Make sure that your SilverStripe installation has the [versionfeed](http://addon
 
 There are two feeds that are automatically created for each page:
 
-* Page changes: This feed will display all published versions of the page, highlighting any additions or deletions with underscores or strikethroughs. It is accessible with the `changes` action - so `http://mysite.com/mypage/changes`
+* Page changes: This feed will display all published versions of the page, highlighting any additions or deletions with underscores or strikethroughs respectively. It is accessible with the `changes` action - so `http://mysite.com/mypage/changes`
 * Site changes: This will aggregate all the per-page change feeds into one feed and display the most recent 20. It is accessible from any page with the `allchanges` action - so `http://mysite.com/home/allchanges`
 
 ## Enabling / disabling
 
-You can enable or disable the feed on a per-page basis by checking or unchecking the *Public History* checkbox in the Settings tab of each page. If a page has the Public History option, unchecked, it will not appear in the allchanges feed.
+You can enable or disable the feed on a per-page basis by checking or unchecking the *Make history public* checkbox (if available) in the Settings tab of each page. If a page has the Make history public option unchecked, it will not appear in the allchanges feed.
 
 The allchanges feed can also be disabled by unchecking the "All page changes" checkbox in the "Settings" section in the cms.
 
@@ -29,4 +31,4 @@ The allchanges feed can also be disabled by unchecking the "All page changes" ch
 
 A page's history will be completely visible when it has public history enabled, even if some updates were made when it was restricted to only being viewed by authenticated users. So if a page has ever had confidential data on it, it is best to not enable this feature unless the data has entered the public domain.
 
-There is a warning explaining this fact next to the *Public History* checkbox.
+There is a warning explaining this fact next to the *Make history public* checkbox.

lang/en.yml

@@ -6,6 +6,8 @@ en:
     TITLECHANGED: 'Title has changed:'
     Warning: 'Publicising the history will also disclose the changes that have at the time been protected from the public view.'
     Warning2: 'Changing access settings in such a way that this page or pages under it become publicly<br>accessible may result in publicising all historical changes on these pages too. Please review<br> this section''s "Public history" settings to ascertain only intended information is disclosed.'
+    db_PublicHistory: 'Public history'
   SilverStripe\VersionFeed\VersionFeedSiteConfig:
     ALLCHANGES: 'All page changes'
     ALLCHANGESLABEL: 'Make global changes feed public'
+    db_AllChangesEnabled: 'All changes enabled'

lang/eo.yml

@@ -6,6 +6,8 @@ eo:
     TITLECHANGED: 'Titolo estas ŝanĝita:'
     Warning: 'Publikigi la historion ankaŭ malkaŝos la ŝanĝojn ĝis tiam protektitajn kontraŭ publika vido.'
     Warning2: 'Ŝanĝi la alirajn agordojn tiel ke ĉi tiu paĝo, aŭ paĝoj sub ĝi, fariĝas publike alireblaj <br>eble rezultigos ke publikiĝos ĉiuj historiaj ŝanĝoj en tiuj paĝoj. Bonvole rekonsideru <br> la sekcion "Publika historio" de ĉi tiu sekcio, por certigi ke nur intencita informo publikiĝu.'
+    db_PublicHistory: 'Publika historio'
   SilverStripe\VersionFeed\VersionFeedSiteConfig:
     ALLCHANGES: 'Ĉiuj paĝaj ŝanĝoj'
     ALLCHANGESLABEL: 'Ĉieaj ŝanĝoj fluu en publikan'
+    db_AllChangesEnabled: 'Ĉiuj ŝanĝoj enŝaltitaj'

lang/it.yml

@@ -0,0 +1,11 @@
+it:
+  SilverStripe\VersionFeed\VersionFeed:
+    LABEL: 'Rendere pubblica la cronologia'
+    SINGLEPAGEFEEDTITLE: 'Aggiornare alla pagina {title}'
+    SITEFEEDTITLE: 'Aggiornare a {title}'
+    TITLECHANGED: 'Il titolo è cambiato:'
+    Warning: 'Pubblicare la cronologia divulgherà anche i cambiamenti che sono stati precedentemente protetti dalla vista pubblica.'
+    Warning2: 'Cambiare la modalità di accesso in modo che la pagina o le pagine sottostanti diventino pubbliche<br>può comportare la pubblicazione della cronologia dei cambiamenti di queste pagine. Si prega di rivedere<br>le impostazioni della sezione "Cronologia pubblica" per assicurarsi che siano divulgate solo le informazioni desiderate.'
+  SilverStripe\VersionFeed\VersionFeedSiteConfig:
+    ALLCHANGES: 'Tutti i cambiamenti pagina'
+    ALLCHANGESLABEL: 'Rendere pubblico il feed dei cambiamenti globali'

lang/sk.yml

@@ -0,0 +1,13 @@
+sk:
+  SilverStripe\VersionFeed\VersionFeed:
+    LABEL: 'Zverejniť históriu'
+    SINGLEPAGEFEEDTITLE: 'Aktualizácie stránky {title}'
+    SITEFEEDTITLE: 'Aktualizácie {title}'
+    TITLECHANGED: 'Názov sa zmenil:'
+    Warning: 'Zverejnením histórie sa zverejnia aj zmeny, ktoré boli v tom čase chránené pred zrakom verejnosti.'
+    Warning2: 'Zmena nastavení prístupu tak, aby sa táto stránka alebo stránky pod ňou stali verejne prístupné,<br>môže viesť k zverejneniu všetkých historických zmien aj na týchto stránkach.<br> Skontrolujte nastavenia v časti "Verejná história", aby ste sa uistili, že sú zverejnené iba zamýšľané informácie.'
+    db_PublicHistory: 'Verejná história'
+  SilverStripe\VersionFeed\VersionFeedSiteConfig:
+    ALLCHANGES: 'Všetky zmeny stránky'
+    ALLCHANGESLABEL: 'Zverejnite informačný kanál globálnych zmien'
+    db_AllChangesEnabled: 'Všetky zmeny povolené'

lang/sl.yml

@@ -0,0 +1,12 @@
+sl:
+  SilverStripe\VersionFeed\VersionFeed:
+    LABEL: 'Javno objavi zgodovino'
+    SINGLEPAGEFEEDTITLE: 'Posodobitve strani ''{title}'' '
+    SITEFEEDTITLE: 'Posodobitve {title}'
+    TITLECHANGED: 'Spremenjen naslov:'
+    Warning: 'Objava zgodovine bo razkrila tudi spremembe, ki do sedaj niso bile javno objavljene.'
+    Warning2: 'Sprememba nastavitev dostopa tako, da so so ta stran ali njene podstrani dostopne javnosti, <br> lahko povzroči objavo tudi vseh sprememb na omenjenih straneh. Podrobno preverite <br> seznam sprememb in se prepričajte, da boste razkrili samo tiste informacije, ki jih želite.'
+    db_PublicHistory: 'Javna zgodovina'
+  SilverStripe\VersionFeed\VersionFeedSiteConfig:
+    ALLCHANGES: 'Vse spremembe'
+    ALLCHANGESLABEL: 'Javno objavi seznam s krovnimi spremembami'

lang/zh.yml

@@ -1,8 +1,8 @@
 zh:
   SilverStripe\VersionFeed\VersionFeed:
-    LABEL: '将历史记录公开'
+    LABEL: 将历史记录公开
     SINGLEPAGEFEEDTITLE: '更新至 {title} 页面'
     SITEFEEDTITLE: '更新至 {title}'
-    TITLECHANGED: '标题已更改：'
-    Warning: '发布历史记录还会在公开视图中显示受保护事件内进行的改动。'
-    Warning2: '用这种方式更改访问设置会使得本页及下级页面变为公开的<br>可能还会使得这些页面的所有变动历史记录也变为公开的。请查阅<br>本节的“公开历史记录”设置，确保只将需要的信息披露出来。'
+    TITLECHANGED: 标题已更改：
+    Warning: 发布历史记录还会在公开视图中显示受保护事件内进行的改动。
+    Warning2: 用这种方式更改访问设置会使得本页及下级页面变为公开的<br>可能还会使得这些页面的所有变动历史记录也变为公开的。请查阅<br>本节的“公开历史记录”设置，确保只将需要的信息披露出来。

phpcs.xml.dist

@@ -2,6 +2,9 @@
 <ruleset name="SilverStripe">
     <description>CodeSniffer ruleset for SilverStripe coding conventions.</description>
 
+    <file>src</file>
+    <file>tests</file>
+
     <rule ref="PSR2" >
         <!-- Current exclusions -->
         <exclude name="PSR1.Methods.CamelCapsMethodName" />

phpunit.xml.dist

@@ -1,7 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
 <phpunit bootstrap="vendor/silverstripe/cms/tests/bootstrap.php" colors="true">
+    <testsuites>
         <testsuite name="Default">
             <directory>tests/</directory>
         </testsuite>
+    </testsuites>
     <filter>
         <whitelist addUncoveredFilesFromWhitelist="true">
             <directory suffix=".php">src/</directory>

src/Filters/ContentFilter.php

@@ -39,7 +39,7 @@ abstract class ContentFilter
     /**
      * Gets the cache to use
      *
-     * @return Zend_Cache_Frontend
+     * @return CacheInterface
      */
     protected function getCache()
     {

src/Filters/RateLimitFilter.php

@@ -5,6 +5,7 @@ namespace SilverStripe\VersionFeed\Filters;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\HTTPResponse_Exception;
+use SilverStripe\Versioned\Versioned;
 
 /**
  * Provides rate limiting of execution of a callback
@@ -67,13 +68,13 @@ class RateLimitFilter extends ContentFilter
 
         // Add global identifier
         if ($this->config()->get('lock_bypage')) {
-            $key .= '_' . md5($itemkey);
+            $key .= '_' . md5($itemkey ?? '');
         }
 
         // Add user-specific identifier
         if ($this->config()->get('lock_byuserip') && Controller::has_curr()) {
             $ip = Controller::curr()->getRequest()->getIP();
-            $key .= '_' . md5($ip);
+            $key .= '_' . md5($ip ?? '');
         }
 
         return $key;

src/VersionFeed.php

@@ -2,6 +2,7 @@
 
 namespace SilverStripe\VersionFeed;
 
+use SilverStripe\Dev\Deprecation;
 use SilverStripe\CMS\Model\SiteTreeExtension;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Forms\CheckboxField;
@@ -12,6 +13,7 @@ use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\SiteConfig\SiteConfig;
 use SilverStripe\View\Parsers\Diff;
+use SilverStripe\CMS\Model\SiteTree;
 
 class VersionFeed extends SiteTreeExtension
 {
@@ -76,7 +78,7 @@ class VersionFeed extends SiteTreeExtension
         $offset = $highestVersion ? "AND \"SiteTree_Versions\".\"Version\"<='".(int)$highestVersion."'" : '';
         // Get just enough elements for diffing. We need one more than desired to have something to compare to.
         $qLimit = (int)$limit + 1;
-        $versions = $this->owner->allVersions(
+        $versions = $this->owner->Versions(
             "\"WasPublished\"='1' AND \"CanViewType\" IN ('Anyone', 'Inherit') $offset",
             "\"SiteTree\".\"LastEdited\" DESC, \"SiteTree\".\"ID\" DESC",
             $qLimit
@@ -114,7 +116,16 @@ class VersionFeed extends SiteTreeExtension
                 }
 
                 // Copy the link so it can be cached.
-                $version->GeneratedLink = $version->AbsoluteLink();
+                $oldPage = $version->getField('object');
+                if (!$oldPage instanceof SiteTree) {
+                    // We only need enough info to generate the link...
+                    $oldPage = SiteTree::create([
+                        'ID' => $oldPage->ID,
+                        'URLSegment' => $oldPage->URLSegment,
+                        'ParentID' => $oldPage->ParentID
+                    ]);
+                }
+                $version->GeneratedLink = $oldPage->AbsoluteLink();
             }
 
             // Omit the versions that haven't been visibly changed (only takes the above fields into consideration).
@@ -161,7 +172,7 @@ class VersionFeed extends SiteTreeExtension
     /**
      * Compile a list of changes to the current page, excluding non-published and explicitly secured versions.
      *
-     * @deprecated 2.0.0 Use VersionFeed::getDiffList instead
+     * @deprecated 2.0.0 Use VersionFeed::getDiffList() instead
      *
      * @param int $highestVersion Top version number to consider.
      * @param boolean $fullHistory Set to true to get the full change history, set to false for a single diff.
@@ -171,6 +182,7 @@ class VersionFeed extends SiteTreeExtension
      */
     public function getDiffedChanges($highestVersion = null, $fullHistory = true, $limit = 100)
     {
+        Deprecation::notice('2.0.0', 'Use VersionFeed::getDiffList() instead');
         return $this->getDiffList(
             $highestVersion,
             $fullHistory ? $limit : 1

src/VersionFeedController.php

@@ -77,7 +77,7 @@ class VersionFeedController extends Extension
 
         // Cache the diffs to remove DOS possibility.
         $target = $this->owner;
-        $key = implode('_', array('changes', $this->owner->ID, $this->owner->Version));
+        $key = implode('_', array('changes', $target->ID, $target->Version));
         $entries = $this->filterContent($key, function () use ($target) {
             return $target->getDiffList(null, Config::inst()->get(VersionFeed::class, 'changes_limit'));
         });
@@ -119,7 +119,7 @@ class VersionFeedController extends Extension
         if ($lastChange) {
             // Cache the diffs to remove DOS possibility.
             $key = 'allchanges'
-                . preg_replace('#[^a-zA-Z0-9_]#', '', $lastChange['LastEdited'])
+                . preg_replace('#[^a-zA-Z0-9_]#', '', $lastChange['LastEdited'] ?? '')
                 . (Security::getCurrentUser() ? Security::getCurrentUser()->ID : 'public');
             $changeList = $this->filterContent($key, function () use ($latestChanges) {
                 $changeList = new ArrayList();
@@ -191,6 +191,7 @@ class VersionFeedController extends Extension
     {
         if (!Config::inst()->get(VersionFeed::class, 'allchanges_enabled')
             || !SiteConfig::current_site_config()->AllChangesEnabled
+            || !method_exists($this->owner, 'getSiteRSSLink')
         ) {
             return;
         }

tests/VersionFeedFunctionalTest.php

@@ -3,9 +3,11 @@
 namespace SilverStripe\VersionFeed\Tests;
 
 use Page;
+use PageController;
 use Psr\SimpleCache\CacheInterface;
 use SilverStripe\CMS\Model\SiteTree;
 use SilverStripe\Control\Director;
+use SilverStripe\Core\Cache\CacheFactory;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Dev\FunctionalTest;
@@ -22,23 +24,28 @@ class VersionFeedFunctionalTest extends FunctionalTest
 
     protected $baseURI = 'http://www.fakesite.test';
 
-    protected static $required_extensions = array(
-        'Page' => array(VersionFeed::class),
-        'PageController' => array(VersionFeedController::class),
-    );
+    protected static $required_extensions = [
+        Page::class => [VersionFeed::class],
+        PageController::class => [VersionFeedController::class],
+    ];
 
     protected $userIP;
 
-    protected function setUp()
+    /**
+     * @var CacheInterface
+     */
+    protected $cache;
+
+    protected function setUp(): void
     {
         Director::config()->set('alternate_base_url', $this->baseURI);
 
         parent::setUp();
 
-        $cache = Injector::inst()->get(
+        $this->cache = Injector::inst()->get(
             CacheInterface::class . '.VersionFeedController'
         );
-        $cache->clear();
+        $this->cache->clear();
 
         $this->userIP = isset($_SERVER['REMOTE_ADDR']) ? $_SERVER['REMOTE_ADDR'] : null;
 
@@ -52,9 +59,12 @@ class VersionFeedFunctionalTest extends FunctionalTest
         Config::modify()->set(RateLimitFilter::class, 'lock_bypage', false);
         Config::modify()->set(RateLimitFilter::class, 'lock_byuserip', false);
         Config::modify()->set(RateLimitFilter::class, 'lock_cooldown', false);
+
+        // Ensure any version based caches read from the live cache
+        Versioned::set_reading_mode(Versioned::DEFAULT_MODE);
     }
 
-    protected function tearDown()
+    protected function tearDown(): void
     {
         Director::config()->set('alternate_base_url', null);
 
@@ -63,9 +73,9 @@ class VersionFeedFunctionalTest extends FunctionalTest
         parent::tearDown();
     }
 
-    public function testPublicHistory()
+    public function testPublicHistoryPublicHistoryDisabled()
     {
-        $page = $this->createPageWithChanges(array('PublicHistory' => false));
+        $page = $this->createPageWithChanges(['PublicHistory' => false]);
 
         $response = $this->get($page->RelativeLink('changes'));
         $this->assertEquals(
@@ -76,17 +86,20 @@ class VersionFeedFunctionalTest extends FunctionalTest
 
         $response = $this->get($page->RelativeLink('allchanges'));
         $this->assertEquals(200, $response->getStatusCode());
-        $xml = simplexml_load_string($response->getBody());
+        $xml = simplexml_load_string($response->getBody() ?? '');
         $this->assertFalse(
             (bool)$xml->channel->item,
             'With Page\'s "PublicHistory" disabled, `allchanges` action should not have an item in the channel'
         );
+    }
 
-        $page = $this->createPageWithChanges(array('PublicHistory' => true));
+    public function testPublicHistoryPublicHistoryEnabled()
+    {
+        $page = $this->createPageWithChanges(['PublicHistory' => true]);
 
         $response = $this->get($page->RelativeLink('changes'));
         $this->assertEquals(200, $response->getStatusCode());
-        $xml = simplexml_load_string($response->getBody());
+        $xml = simplexml_load_string($response->getBody() ?? '');
         $this->assertTrue(
             (bool)$xml->channel->item,
             'With Page\'s "PublicHistory" enabled, `changes` action should have an item in the channel'
@@ -94,7 +107,7 @@ class VersionFeedFunctionalTest extends FunctionalTest
 
         $response = $this->get($page->RelativeLink('allchanges'));
         $this->assertEquals(200, $response->getStatusCode());
-        $xml = simplexml_load_string($response->getBody());
+        $xml = simplexml_load_string($response->getBody() ?? '');
         $this->assertTrue(
             (bool)$xml->channel->item,
             'With "PublicHistory" enabled, `allchanges` action should have an item in the channel'
@@ -107,15 +120,11 @@ class VersionFeedFunctionalTest extends FunctionalTest
         Config::modify()->set(RateLimitFilter::class, 'lock_timeout', 20);
         Config::modify()->set(CachedContentFilter::class, 'cache_enabled', true);
 
-        $page1 = $this->createPageWithChanges(array('PublicHistory' => true, 'Title' => 'Page1'));
-        $page2 = $this->createPageWithChanges(array('PublicHistory' => true, 'Title' => 'Page2'));
+        $page1 = $this->createPageWithChanges(['PublicHistory' => true, 'Title' => 'Page1']);
+        $page2 = $this->createPageWithChanges(['PublicHistory' => true, 'Title' => 'Page2']);
 
         // Artifically set cache lock
-        Config::modify()->set(RateLimitFilter::class, 'lock_byuserip', false);
-        $cache = Injector::inst()->get(
-            CacheInterface::class . '.VersionFeedController'
-        );
-        $cache->set(RateLimitFilter::CACHE_PREFIX, time() + 10);
+        $this->cache->set(RateLimitFilter::CACHE_PREFIX, time() + 10);
 
         // Test normal hit
         $response = $this->get($page1->RelativeLink('changes'));
@@ -127,13 +136,13 @@ class VersionFeedFunctionalTest extends FunctionalTest
 
         // Test page specific lock
         Config::modify()->set(RateLimitFilter::class, 'lock_bypage', true);
-        $key = implode('_', array(
+        $key = implode('_', [
             'changes',
             $page1->ID,
             Versioned::get_versionnumber_by_stage(SiteTree::class, 'Live', $page1->ID, false)
-        ));
-        $key = RateLimitFilter::CACHE_PREFIX . '_' . md5($key);
-        $cache->set($key, time() + 10);
+        ]);
+        $key = RateLimitFilter::CACHE_PREFIX . '_' . md5($key ?? '');
+        $this->cache->set($key, time() + 10);
         $response = $this->get($page1->RelativeLink('changes'));
         $this->assertEquals(429, $response->getStatusCode());
         $this->assertGreaterThan(0, $response->getHeader('Retry-After'));
@@ -144,42 +153,37 @@ class VersionFeedFunctionalTest extends FunctionalTest
         // Test rate limit hit by IP
         Config::modify()->set(RateLimitFilter::class, 'lock_byuserip', true);
         $_SERVER['REMOTE_ADDR'] = '127.0.0.1';
-        $cache->set(RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1'), time() + 10);
+        $this->cache->set(RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1'), time() + 10);
         $response = $this->get($page1->RelativeLink('changes'));
         $this->assertEquals(429, $response->getStatusCode());
         $this->assertGreaterThan(0, $response->getHeader('Retry-After'));
 
         // Test rate limit doesn't hit other IP
         $_SERVER['REMOTE_ADDR'] = '127.0.0.20';
-        $cache->set(RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1'), time() + 10);
+        $this->cache->set(RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1'), time() + 10);
         $response = $this->get($page1->RelativeLink('changes'));
         $this->assertEquals(200, $response->getStatusCode());
-
-        // Restore setting
-        Config::modify()->set(RateLimitFilter::class, 'lock_byuserip', false);
-        Config::modify()->set(RateLimitFilter::class, 'lock_timeout', 0);
-        Config::modify()->set(CachedContentFilter::class, 'cache_enabled', false);
     }
 
     public function testChangesActionContainsChangesForCurrentPageOnly()
     {
-        $page1 = $this->createPageWithChanges(array('Title' => 'Page1'));
-        $page2 = $this->createPageWithChanges(array('Title' => 'Page2'));
+        $page1 = $this->createPageWithChanges(['Title' => 'Page1']);
+        $page2 = $this->createPageWithChanges(['Title' => 'Page2']);
 
         $response = $this->get($page1->RelativeLink('changes'));
-        $xml = simplexml_load_string($response->getBody());
+        $xml = simplexml_load_string($response->getBody() ?? '');
         $titles = array_map(function ($item) {
             return (string)$item->title;
-        }, $xml->xpath('//item'));
+        }, $xml->xpath('//item') ?? []);
         // TODO Unclear if this should contain the original version
         $this->assertContains('Changed: Page1', $titles);
         $this->assertNotContains('Changed: Page2', $titles);
 
         $response = $this->get($page2->RelativeLink('changes'));
-        $xml = simplexml_load_string($response->getBody());
+        $xml = simplexml_load_string($response->getBody() ?? '');
         $titles = array_map(function ($item) {
             return (string)$item->title;
-        }, $xml->xpath('//item'));
+        }, $xml->xpath('//item') ?? []);
         // TODO Unclear if this should contain the original version
         $this->assertNotContains('Changed: Page1', $titles);
         $this->assertContains('Changed: Page2', $titles);
@@ -187,14 +191,14 @@ class VersionFeedFunctionalTest extends FunctionalTest
 
     public function testAllChangesActionContainsAllChangesForAllPages()
     {
-        $page1 = $this->createPageWithChanges(array('Title' => 'Page1'));
-        $page2 = $this->createPageWithChanges(array('Title' => 'Page2'));
+        $page1 = $this->createPageWithChanges(['Title' => 'Page1']);
+        $page2 = $this->createPageWithChanges(['Title' => 'Page2']);
 
         $response = $this->get($page1->RelativeLink('allchanges'));
-        $xml = simplexml_load_string($response->getBody());
+        $xml = simplexml_load_string($response->getBody() ?? '');
         $titles = array_map(function ($item) {
             return str_replace('Changed: ', '', (string) $item->title);
-        }, $xml->xpath('//item'));
+        }, $xml->xpath('//item') ?? []);
         $this->assertContains('Page1', $titles);
         $this->assertContains('Page2', $titles);
     }
@@ -203,32 +207,32 @@ class VersionFeedFunctionalTest extends FunctionalTest
     {
         $page = new Page();
 
-        $seed = array_merge(array(
+        $seed = array_merge([
             'Title' => 'My Title',
             'Content' => 'My Content'
-        ), $seed);
+        ], $seed);
         $page->update($seed);
         $page->write();
-        $page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
+        $page->publishSingle();
 
-        $page->update(array(
+        $page->update([
             'Title' => 'Changed: ' . $seed['Title'],
             'Content' => 'Changed: ' . $seed['Content'],
-        ));
+        ]);
         $page->write();
-        $page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
+        $page->publishSingle();
 
-        $page->update(array(
+        $page->update([
             'Title' => 'Changed again: ' . $seed['Title'],
             'Content' => 'Changed again: ' . $seed['Content'],
-        ));
+        ]);
         $page->write();
-        $page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
+        $page->publishSingle();
 
-        $page->update(array(
+        $page->update([
             'Title' => 'Unpublished: ' . $seed['Title'],
             'Content' => 'Unpublished: ' . $seed['Content'],
-        ));
+        ]);
         $page->write();
 
         return $page;
@@ -241,11 +245,11 @@ class VersionFeedFunctionalTest extends FunctionalTest
     {
 
         // Nested loop through each configuration
-        foreach (array(true, false) as $publicHistory_Page) {
-            $page = $this->createPageWithChanges(array('PublicHistory' => $publicHistory_Page, 'Title' => 'Page'));
+        foreach ([true, false] as $publicHistory_Page) {
+            $page = $this->createPageWithChanges(['PublicHistory' => $publicHistory_Page, 'Title' => 'Page']);
 
             // Test requests to 'changes' action
-            foreach (array(true, false) as $publicHistory_Config) {
+            foreach ([true, false] as $publicHistory_Config) {
                 Config::modify()->set(VersionFeed::class, 'changes_enabled', $publicHistory_Config);
                 $expectedResponse = $publicHistory_Page && $publicHistory_Config ? 200 : 404;
                 $response = $this->get($page->RelativeLink('changes'));
@@ -253,8 +257,8 @@ class VersionFeedFunctionalTest extends FunctionalTest
             }
 
             // Test requests to 'allchanges' action on each page
-            foreach (array(true, false) as $allChanges_Config) {
-                foreach (array(true, false) as $allChanges_SiteConfig) {
+            foreach ([true, false] as $allChanges_Config) {
+                foreach ([true, false] as $allChanges_SiteConfig) {
                     Config::modify()->set(VersionFeed::class, 'allchanges_enabled', $allChanges_Config);
                     $siteConfig = SiteConfig::current_site_config();
                     $siteConfig->AllChangesEnabled = $allChanges_SiteConfig;

tests/VersionFeedTest.php

@@ -44,18 +44,18 @@ class VersionFeedTest extends SapphireTest
 
         // Strip spaces from test output because they're not reliably maintained by the HTML Tidier
         $cleanDiffOutput = function ($val) {
-            return str_replace(' ', '', strip_tags($val));
+            return str_replace(' ', '', strip_tags($val ?? ''));
         };
 
         $this->assertContains(
             str_replace(' ', '', _t('RSSHistory.TITLECHANGED', 'Title has changed:') . 'My Changed Title'),
-            array_map($cleanDiffOutput, $page->getDiffList()->column('DiffTitle')),
+            array_map($cleanDiffOutput, $page->getDiffList()->column('DiffTitle') ?? []),
             'Detects published title changes'
         );
 
         $this->assertNotContains(
             str_replace(' ', '', _t('RSSHistory.TITLECHANGED', 'Title has changed:') . 'My Unpublished Changed Title'),
-            array_map($cleanDiffOutput, $page->getDiffList()->column('DiffTitle')),
+            array_map($cleanDiffOutput, $page->getDiffList()->column('DiffTitle') ?? []),
             'Ignores unpublished title changes'
         );
     }