NEW Record deprecated config

This commit is contained in:
Steve Boyd 2022-10-31 19:00:59 +13:00
parent 59b980edd7
commit b1dc861aac
3 changed files with 191 additions and 49 deletions

View File

@ -3,7 +3,6 @@
namespace SilverStripe\Dev; namespace SilverStripe\Dev;
use BadMethodCallException; use BadMethodCallException;
use Exception;
use SilverStripe\Control\Director; use SilverStripe\Control\Director;
use SilverStripe\Core\Environment; use SilverStripe\Core\Environment;
use SilverStripe\Core\Injector\InjectorLoader; use SilverStripe\Core\Injector\InjectorLoader;
@ -41,6 +40,7 @@ class Deprecation
const SCOPE_METHOD = 1; const SCOPE_METHOD = 1;
const SCOPE_CLASS = 2; const SCOPE_CLASS = 2;
const SCOPE_GLOBAL = 4; const SCOPE_GLOBAL = 4;
const SCOPE_CONFIG = 8;
/** /**
* @var string * @var string
@ -67,8 +67,10 @@ class Deprecation
* must be available before this to avoid infinite loops. * 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 if present
*
* @internal - Marked as internal so this and other private static's are not treated as config
*/ */
protected static bool $is_enabled = false; private static bool $is_enabled = false;
/** /**
* @var array * @var array
@ -76,20 +78,35 @@ class Deprecation
*/ */
protected static $module_version_overrides = []; protected static $module_version_overrides = [];
protected static bool $inside_notice = false; /**
* @internal
*/
private static bool $inside_notice = false;
/** /**
* Switched out by unit-testing to E_USER_NOTICE because E_USER_DEPRECATED is not * @var array
* caught by $this->expectDeprecated() by default * @deprecated 4.12.0 Will be removed without equivalent functionality to replace it
* https://github.com/laminas/laminas-di/pull/30#issuecomment-927585210
*
* @var int
*/ */
public static $notice_level = E_USER_DEPRECATED; public static $notice_level = E_USER_DEPRECATED;
/**
* Buffer of user_errors to be raised when enabled() is called
*
* This is used when setting deprecated config via yaml, before Deprecation::enable() has been called in _config.php
* 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
*
* @internal
*/
private static array $user_error_message_buffer = [];
public static function enable(): void public static function enable(): void
{ {
static::$is_enabled = true; static::$is_enabled = true;
foreach (self::$user_error_message_buffer as $message) {
user_error($message, E_USER_DEPRECATED);
}
self::$user_error_message_buffer = [];
} }
public static function disable(): void public static function disable(): void
@ -167,10 +184,7 @@ class Deprecation
if (!Director::isDev()) { if (!Director::isDev()) {
return false; return false;
} }
if (Environment::getEnv('SS_DEPRECATION_ENABLED')) { return static::$is_enabled || Environment::getEnv('SS_DEPRECATION_ENABLED');
return true;
}
return static::$is_enabled;
} }
/** /**
@ -202,13 +216,16 @@ class Deprecation
// try block needs to wrap all code in case anything inside the try block // try block needs to wrap all code in case anything inside the try block
// calls something else that calls Deprecation::notice() // calls something else that calls Deprecation::notice()
try { try {
if (!self::get_is_enabled()) { if ($scope === self::SCOPE_CONFIG) {
return; if (self::get_is_enabled()) {
user_error($string, E_USER_DEPRECATED);
} else {
self::$user_error_message_buffer[] = $string;
} }
} else {
// If you pass #.#, assume #.#.0 if (!self::get_is_enabled()) {
if (preg_match('/^[0-9]+\.[0-9]+$/', $atVersion ?? '')) { // Do not add to self::$user_error_message_buffer, as the backtrace is too expensive
$atVersion .= '.0'; return;
} }
// Getting a backtrace is slow, so we only do it if we need it // Getting a backtrace is slow, so we only do it if we need it
@ -233,9 +250,10 @@ class Deprecation
$string .= " Called from " . self::get_called_method_from_trace($backtrace, 2) . '.'; $string .= " Called from " . self::get_called_method_from_trace($backtrace, 2) . '.';
if ($caller) { if ($caller) {
user_error($caller . ' is deprecated.' . ($string ? ' ' . $string : ''), self::$notice_level); user_error($caller . ' is deprecated.' . ($string ? ' ' . $string : ''), E_USER_DEPRECATED);
} else { } else {
user_error($string, self::$notice_level); user_error($string, E_USER_DEPRECATED);
}
} }
} catch (BadMethodCallException $e) { } catch (BadMethodCallException $e) {
if ($e->getMessage() === InjectorLoader::NO_MANIFESTS_AVAILABLE) { if ($e->getMessage() === InjectorLoader::NO_MANIFESTS_AVAILABLE) {

View File

@ -2,16 +2,43 @@
namespace SilverStripe\Dev\Tests; namespace SilverStripe\Dev\Tests;
use PHPUnit\Framework\Error\Deprecated;
use SilverStripe\Core\Config\Config;
use SilverStripe\Dev\Deprecation; use SilverStripe\Dev\Deprecation;
use SilverStripe\Dev\SapphireTest; use SilverStripe\Dev\SapphireTest;
use SilverStripe\Dev\Tests\DeprecationTest\TestDeprecation; use SilverStripe\Dev\Tests\DeprecationTest\DeprecationTestObject;
class DeprecationTest extends SapphireTest class DeprecationTest extends SapphireTest
{ {
protected static $extra_dataobjects = [
DeprecationTestObject::class,
];
private $oldHandler = null;
protected function setup(): void
{
// Use custom error handler for two reasons:
// - Filter out errors for deprecated class properities unrelated to this unit test
// - Allow the use of expectDeprecation(), which doesn't work with E_USER_DEPRECATION by default
// https://github.com/laminas/laminas-di/pull/30#issuecomment-927585210
parent::setup();
$this->oldHandler = set_error_handler(function (int $errno, string $errstr, string $errfile, int $errline) {
if (str_contains($errstr, 'SilverStripe\\Dev\\Tests\\DeprecationTest')) {
throw new Deprecated($errstr, $errno, '', 1);
}
if (is_callable($this->oldHandler)) {
return call_user_func($this->oldHandler, $errno, $errstr, $errfile, $errline);
}
return false;
});
}
protected function tearDown(): void protected function tearDown(): void
{ {
Deprecation::$notice_level = E_USER_DEPRECATED;
Deprecation::disable(); Deprecation::disable();
set_error_handler($this->oldHandler);
$this->oldHandler = null;
parent::tearDown(); parent::tearDown();
} }
@ -22,9 +49,8 @@ class DeprecationTest extends SapphireTest
'My message.', 'My message.',
'Called from PHPUnit\Framework\TestCase->runTest.' 'Called from PHPUnit\Framework\TestCase->runTest.'
]); ]);
$this->expectError(); $this->expectDeprecation();
$this->expectErrorMessage($message); $this->expectDeprecationMessage($message);
Deprecation::$notice_level = E_USER_NOTICE;
Deprecation::enable(); Deprecation::enable();
Deprecation::notice('1.2.3', 'My message'); Deprecation::notice('1.2.3', 'My message');
} }
@ -34,8 +60,71 @@ class DeprecationTest extends SapphireTest
*/ */
public function testDisabled() public function testDisabled()
{ {
Deprecation::$notice_level = E_USER_NOTICE;
// test that no error error is raised because by default Deprecation is disabled // test that no error error is raised because by default Deprecation is disabled
Deprecation::notice('4.5.6', 'My message'); Deprecation::notice('4.5.6', 'My message');
} }
// The following tests would be better to put in the silverstripe/config module, however this is not
// possible to do in a clean way as the config for the DeprecationTestObject will not load if it
// is inside the silverstripe/config directory, as there is no _config.php file or _config folder.
// Adding a _config.php file will break existing unit-tests within silverstripe/config
// It is possible to put DeprecationTestObject in framework and the unit tests in config, however
// that's probably messier then just having everything within framework
public function testConfigGetFirst()
{
$message = implode(' ', [
'Config SilverStripe\Dev\Tests\DeprecationTest\DeprecationTestObject.first_config is deprecated.',
'My first config message.'
]);
$this->expectDeprecation();
$this->expectDeprecationMessage($message);
Deprecation::enable();
Config::inst()->get(DeprecationTestObject::class, 'first_config');
}
public function testConfigGetSecond()
{
$message = implode(' ', [
'Config SilverStripe\Dev\Tests\DeprecationTest\DeprecationTestObject.second_config is deprecated.',
'My second config message.'
]);
$this->expectDeprecation();
$this->expectDeprecationMessage($message);
Deprecation::enable();
Config::inst()->get(DeprecationTestObject::class, 'second_config');
}
public function testConfigGetThird()
{
$message = 'Config SilverStripe\Dev\Tests\DeprecationTest\DeprecationTestObject.third_config is deprecated.';
$this->expectDeprecation();
$this->expectDeprecationMessage($message);
Deprecation::enable();
Config::inst()->get(DeprecationTestObject::class, 'third_config');
}
public function testConfigSet()
{
$message = implode(' ', [
'Config SilverStripe\Dev\Tests\DeprecationTest\DeprecationTestObject.first_config is deprecated.',
'My first config message.'
]);
$this->expectDeprecation();
$this->expectDeprecationMessage($message);
Deprecation::enable();
Config::modify()->set(DeprecationTestObject::class, 'first_config', 'abc');
}
public function testConfigMerge()
{
$message = implode(' ', [
'Config SilverStripe\Dev\Tests\DeprecationTest\DeprecationTestObject.array_config is deprecated.',
'My array config message.'
]);
$this->expectDeprecation();
$this->expectDeprecationMessage($message);
Deprecation::enable();
Config::modify()->merge(DeprecationTestObject::class, 'array_config', ['abc']);
}
} }

View File

@ -0,0 +1,35 @@
<?php
namespace SilverStripe\Dev\Tests\DeprecationTest;
use SilverStripe\Dev\TestOnly;
use SilverStripe\ORM\DataObject;
class DeprecationTestObject extends DataObject implements TestOnly
{
private static $db = [
"Name" => "Varchar"
];
private static $table_name = 'DeprecatedTestObject';
/**
* @deprecated 1.2.3 My first config message
*/
private static $first_config = 'ABC';
/**
* @deprecated My second config message
*/
private static $second_config = 'DEF';
/**
* @deprecated
*/
private static $third_config = 'XYZ';
/**
* @deprecated 1.2.3 My array config message
*/
private static $array_config = ['lorem', 'ipsum'];
}