Merge branch '5.0' into 5

.github/workflows/ci.yml

@@ -4,15 +4,10 @@ on:
   push:
   pull_request:
   workflow_dispatch:
-  # Every Tuesday at 2:20pm UTC
-  schedule:
-    - cron: '20 14 * * 2'
 
 jobs:
   ci:
     name: CI
-    # Only run cron on the silverstripe account
-    if: (github.event_name == 'schedule' && github.repository_owner == 'silverstripe') || (github.event_name != 'schedule')
     uses: silverstripe/gha-ci/.github/workflows/ci.yml@v1
     with:
       # Turn phpcoverage off because it causes a segfault

.github/workflows/dispatch-ci.yml

@@ -0,0 +1,16 @@
+name: Dispatch CI
+
+on:
+  # At 2:20 PM UTC, only on Tuesday and Wednesday
+  schedule:
+    - cron: '20 14 * * 2,3'
+
+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

composer.json

@@ -111,4 +111,4 @@
     },
     "minimum-stability": "dev",
     "prefer-stable": true
-}
+}

src/Core/Environment.php

@@ -177,23 +177,32 @@ public static function getTimeLimitMax()
     }
 
     /**
-     * Get value of environment variable
+     * Get value of environment variable.
+     * If the value is false, you should check Environment::hasEnv() to see
+     * if the value is an actual environment variable value or if the variable
+     * simply hasn't been set.
      *
      * @param string $name
      * @return mixed Value of the environment variable, or false if not set
      */
     public static function getEnv($name)
     {
-        switch (true) {
-            case  is_array(static::$env) && array_key_exists($name, static::$env):
-                return static::$env[$name];
-            case  is_array($_ENV) && array_key_exists($name, $_ENV):
-                return $_ENV[$name];
-            case  is_array($_SERVER) && array_key_exists($name, $_SERVER):
-                return $_SERVER[$name];
-            default:
-                return getenv($name);
+        if (array_key_exists($name, static::$env)) {
+            return static::$env[$name];
         }
+        // isset() is used for $_ENV and $_SERVER instead of array_key_exists() to fix a very strange issue that
+        // occured in CI running silverstripe/recipe-kitchen-sink where PHP would timeout due apparently due to an
+        // excessively high number of array method calls. isset() is not used for static::$env above because
+        // values there may be null, and isset() will return false for null values
+        // Symfony also uses isset() for reading $_ENV and $_SERVER values
+        // https://github.com/symfony/dependency-injection/blob/6.2/EnvVarProcessor.php#L148
+        if (isset($_ENV[$name])) {
+            return $_ENV[$name];
+        }
+        if (isset($_SERVER[$name])) {
+            return $_SERVER[$name];
+        }
+        return getenv($name);
     }
 
     /**
@@ -223,6 +232,18 @@ public static function setEnv($name, $value)
         static::$env[$name] = $value;
     }
 
+    /**
+     * Check if an environment variable is set
+     */
+    public static function hasEnv(string $name): bool
+    {
+        // See getEnv() for an explanation of why isset() is used for $_ENV and $_SERVER
+        return array_key_exists($name, static::$env)
+            || isset($_ENV[$name])
+            || isset($_SERVER[$name])
+            || getenv($name) !== false;
+    }
+
     /**
      * Returns true if this script is being run from the command line rather than the web server
      *

src/Dev/Deprecation.php

@@ -29,12 +29,22 @@ class Deprecation
      * Must be configured outside of the config API, as deprecation API
      * must be available before this to avoid infinite loops.
      *
-     * This will be overriden by the SS_DEPRECATION_ENABLED environment if present
+     * This will be overriden by the SS_DEPRECATION_ENABLED environment variable if present
      *
      * @internal - Marked as internal so this and other private static's are not treated as config
      */
     private static bool $currentlyEnabled = false;
 
+    /**
+     * @internal
+     */
+    private static bool $shouldShowForHttp = false;
+
+    /**
+     * @internal
+     */
+    private static bool $shouldShowForCli = true;
+
     /**
      * @internal
      */
@@ -45,6 +55,11 @@ class Deprecation
      */
     private static bool $insideWithNoReplacement = false;
 
+    /**
+     * @internal
+     */
+    private static bool $isTriggeringError = false;
+
     /**
      * Buffer of user_errors to be raised
      *
@@ -62,12 +77,26 @@ class Deprecation
      */
     private static bool $showNoReplacementNotices = false;
 
+    /**
+     * Enable throwing deprecation warnings. By default, this excludes warnings for
+     * deprecated code which is called by core Silverstripe modules.
+     *
+     * This will be overriden by the SS_DEPRECATION_ENABLED environment variable if present.
+     *
+     * @param bool $showNoReplacementNotices If true, deprecation warnings will also be thrown
+     * for deprecated code which is called by core Silverstripe modules.
+     */
     public static function enable(bool $showNoReplacementNotices = false): void
     {
         static::$currentlyEnabled = true;
         static::$showNoReplacementNotices = $showNoReplacementNotices;
     }
 
+    /**
+     * Disable throwing deprecation warnings.
+     *
+     * This will be overriden by the SS_DEPRECATION_ENABLED environment variable if present.
+     */
     public static function disable(): void
     {
         static::$currentlyEnabled = false;
@@ -133,32 +162,93 @@ public static function isEnabled(): bool
         if (!Director::isDev()) {
             return false;
         }
-        return static::$currentlyEnabled || Environment::getEnv('SS_DEPRECATION_ENABLED');
+        if (Environment::hasEnv('SS_DEPRECATION_ENABLED')) {
+            $envVar = Environment::getEnv('SS_DEPRECATION_ENABLED');
+            return self::varAsBoolean($envVar);
+        }
+        return static::$currentlyEnabled;
+    }
+
+    /**
+     * If true, any E_USER_DEPRECATED errors should be treated as coming
+     * directly from this class.
+     */
+    public static function isTriggeringError(): bool
+    {
+        return self::$isTriggeringError;
+    }
+
+    /**
+     * Determine whether deprecation warnings should be included in HTTP responses.
+     * Does not affect logging.
+     *
+     * This will be overriden by the SS_DEPRECATION_SHOW_HTTP environment variable if present.
+     */
+    public static function setShouldShowForHttp(bool $value): void
+    {
+        self::$shouldShowForHttp = $value;
+    }
+
+    /**
+     * Determine whether deprecation warnings should be included in CLI responses.
+     * Does not affect logging.
+     *
+     * This will be overriden by the SS_DEPRECATION_SHOW_CLI environment variable if present.
+     */
+    public static function setShouldShowForCli(bool $value): void
+    {
+        self::$shouldShowForCli = $value;
+    }
+
+    /**
+     * If true, deprecation warnings should be included in HTTP responses.
+     * Does not affect logging.
+     */
+    public static function shouldShowForHttp(): bool
+    {
+        if (Environment::hasEnv('SS_DEPRECATION_SHOW_HTTP')) {
+            $envVar = Environment::getEnv('SS_DEPRECATION_SHOW_HTTP');
+            return self::varAsBoolean($envVar);
+        }
+        return self::$shouldShowForHttp;
+    }
+
+    /**
+     * If true, deprecation warnings should be included in CLI responses.
+     * Does not affect logging.
+     */
+    public static function shouldShowForCli(): bool
+    {
+        if (Environment::hasEnv('SS_DEPRECATION_SHOW_CLI')) {
+            $envVar = Environment::getEnv('SS_DEPRECATION_SHOW_CLI');
+            return self::varAsBoolean($envVar);
+        }
+        return self::$shouldShowForCli;
     }
 
     public static function outputNotices(): void
     {
         if (!self::isEnabled()) {
             return;
         }
-        $outputMessages = [];
-        // using a while loop with array_shift() to ensure that self::$userErrorMessageBuffer will have
-        // have values removed from it before calling user_error()
-        while (count(self::$userErrorMessageBuffer)) {
+
+        $count = 0;
+        $origCount = count(self::$userErrorMessageBuffer);
+        while ($origCount > $count) {
+            $count++;
             $arr = array_shift(self::$userErrorMessageBuffer);
             $message = $arr['message'];
-            // often the same deprecation message appears dozens of times, which isn't helpful
-            // only need to show a single instance of each message
-            if (in_array($message, $outputMessages)) {
-                continue;
-            }
             $calledInsideWithNoReplacement = $arr['calledInsideWithNoReplacement'];
             if ($calledInsideWithNoReplacement && !self::$showNoReplacementNotices) {
                 continue;
             }
+            self::$isTriggeringError = true;
             user_error($message, E_USER_DEPRECATED);
-            $outputMessages[] = $message;
+            self::$isTriggeringError = false;
         }
+        // Make absolutely sure the buffer is empty - array_shift seems to leave an item in the array
+        // if we're not using numeric keys.
+        self::$userErrorMessageBuffer = [];
     }
 
     /**
@@ -178,10 +268,12 @@ public static function notice($atVersion, $string = '', $scope = Deprecation::SC
         // try block needs to wrap all code in case anything inside the try block
         // calls something else that calls Deprecation::notice()
         try {
+            $data = null;
             if ($scope === self::SCOPE_CONFIG) {
                 // Deprecated config set via yaml will only be shown in the browser when using ?flush=1
                 // It will not show in CLI when running dev/build flush=1
-                self::$userErrorMessageBuffer[] = [
+                $data = [
+                    'key' => sha1($string),
                     'message' => $string,
                     'calledInsideWithNoReplacement' => self::$insideWithNoReplacement
                 ];
@@ -215,20 +307,26 @@ public static function notice($atVersion, $string = '', $scope = Deprecation::SC
                 if ($caller) {
                     $string = $caller . ' is deprecated.' . ($string ? ' ' . $string : '');
                 }
-                self::$userErrorMessageBuffer[] = [
+                $data = [
+                    'key' => sha1($string),
                     'message' => $string,
                     'calledInsideWithNoReplacement' => self::$insideWithNoReplacement
                 ];
             }
-            if (!self::$haveSetShutdownFunction && self::isEnabled()) {
+            if ($data && !array_key_exists($data['key'], self::$userErrorMessageBuffer)) {
+                // Store de-duplicated data in a buffer to be outputted when outputNotices() is called
+                self::$userErrorMessageBuffer[$data['key']] = $data;
+
                 // Use a shutdown function rather than immediately calling user_error() so that notices
                 // do not interfere with setting session varibles i.e. headers already sent error
                 // it also means the deprecation notices appear below all phpunit output in CI
                 // which is far nicer than having it spliced between phpunit output
-                register_shutdown_function(function () {
-                    self::outputNotices();
-                });
-                self::$haveSetShutdownFunction = true;
+                if (!self::$haveSetShutdownFunction && self::isEnabled()) {
+                    register_shutdown_function(function () {
+                        self::outputNotices();
+                    });
+                    self::$haveSetShutdownFunction = true;
+                }
             }
         } catch (BadMethodCallException $e) {
             if ($e->getMessage() === InjectorLoader::NO_MANIFESTS_AVAILABLE) {
@@ -242,4 +340,31 @@ public static function notice($atVersion, $string = '', $scope = Deprecation::SC
             static::$insideNotice = false;
         }
     }
+
+    private static function varAsBoolean($val): bool
+    {
+        if (is_string($val)) {
+            $truthyStrings = [
+                'on',
+                'true',
+                '1',
+            ];
+
+            if (in_array(strtolower($val), $truthyStrings, true)) {
+                return true;
+            }
+
+            $falsyStrings = [
+                'off',
+                'false',
+                '0',
+            ];
+
+            if (in_array(strtolower($val), $falsyStrings, true)) {
+                return false;
+            }
+        }
+
+        return (bool) $val;
+    }
 }

src/Forms/GridField/GridFieldFilterHeader.php

@@ -199,21 +199,25 @@ public function getManipulatedData(GridField $gridField, SS_List $dataList)
     public function canFilterAnyColumns($gridField)
     {
         $list = $gridField->getList();
-
-        if (!$this->checkDataType($list)) {
+        if (!($list instanceof Filterable) || !$this->checkDataType($list)) {
             return false;
         }
-
-        $columns = $gridField->getColumns();
-        foreach ($columns as $columnField) {
-            $metadata = $gridField->getColumnMetadata($columnField);
-            $title = $metadata['title'];
-
-            if ($title && $list->canFilterBy($columnField)) {
+        $modelClass = $gridField->getModelClass();
+        // note: searchableFields() will return summary_fields if there are no searchable_fields on the model
+        $searchableFields = array_keys($modelClass::singleton()->searchableFields());
+        $summaryFields = array_keys($modelClass::singleton()->summaryFields());
+        sort($searchableFields);
+        sort($summaryFields);
+        // searchable_fields has been explictily defined i.e. searchableFields() is not falling back to summary_fields
+        if ($searchableFields !== $summaryFields) {
+            return true;
+        }
+        // we have fallen back to summary_fields, check they are filterable
+        foreach ($searchableFields as $searchableField) {
+            if ($list->canFilterBy($searchableField)) {
                 return true;
             }
         }
-
         return false;
     }