featureManager: Add DynamicConfigRequirement requirement

We expect the vast majority of requirements and features to be defined
in services as possible. However, there are some "complex" requirements
that require additional application/HTTP request state. Unfortunately,
service wiring is done before some of that state is available.

I65702426 attempted to work around this by requiring clients of the
Feature Manager to pass that additional state on every interaction with
the system. Those complex requirements would then select the parts of
the state that they required when it was required. However
implementations of \IContextSource are God objects and their use should
be limited.

Whilst reviewing I65702426, Stephen Niedzielski mentioned that the
application state being available is a requirement. This remarkably
simple solution:

- Keeps the Requirement interface and FeatureManager API free of God
  objects;

- Is true to the nature of the Feature Manager - it makes clear and
  centralizes the various checks for application state being available
  across the codebase; and

- Inject a Requirement implementations' dependencies at construction
  time

It just so happens that the $wgFullyInitialised variable flags whether
the application state is available...

Changes:

- Add the the FeatureManager\Requirements\DynamicConfigRequirement class
  and tests. The DynamicConfigRequirement lazily evaluates a single
  configuration value whenever ::isMet is invoked

- Register an DynamicConfigRequirement instance, configured to evaluate
  $wgFullyInitialised while constructing the Vector.FeatureManager
  service

Bug: T244481
Change-Id: I7a2cdc2dfdf20d78e4548f07cf53994563b234b3
This commit is contained in:
Sam Smith 2020-03-13 17:22:59 +00:00
parent 864cc97092
commit 84226b41b6
5 changed files with 196 additions and 26 deletions

View File

@ -60,6 +60,17 @@ final class Constants {
*/
public const PREF_KEY_SKIN_VERSION = 'VectorSkinVersion';
// These are used in the Feature Management System.
/**
* @var string
*/
public const CONFIG_KEY_FULLY_INITIALISED = 'FullyInitialised';
/**
* @var string
*/
public const REQUIREMENT_FULLY_INITIALISED = 'FullyInitialised';
/**
* This class is for namespacing constants only. Forbid construction.
* @throws FatalError

View File

@ -0,0 +1,97 @@
<?php
/**
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
* http://www.gnu.org/copyleft/gpl.html
*
* @file
* @since 1.35
*/
namespace Vector\FeatureManagement\Requirements;
use Vector\FeatureManagement\Requirement;
/**
* Some application state changes throughout the lifetime of the application, e.g.
* `wgFullyInitialised`, which signals whether the application boot process has finished and
* critical resources like database connections are available.
*
* The `DynamicStateRequirement` allows us to define requirements that lazily evaluate the
* application state statically, e.g.
*
* ```lang=php
* $featureManager->registerComplexRequirement(
* new DynamicConfigRequirement(
* $config,
* 'FullyInitialised',
* 'Foo'
* )
* );
* ```
*
* registers a requirement that will evaluate to true only when `mediawiki/includes/Setup.php` has
* finished executing (after all service wiring has executed).
*
* NOTE: This API hasn't settled. It may change at any time without warning. Please don't bind to
* it unless you absolutely need to
*
* @unstable
*
* @package FeatureManagement
* @internal
*/
final class DynamicConfigRequirement implements Requirement {
/**
* @var \Config
*/
private $config;
/**
* @var string
*/
private $configName;
/**
* @var string
*/
private $requirementName;
/**
* @param \Config $config
* @param string $configName
* @param string $requirementName The name of the requirement presented to the Feature Manager
*/
public function __construct( \Config $config, string $configName, string $requirementName ) {
$this->config = $config;
$this->configName = $configName;
$this->requirementName = $requirementName;
}
/**
* @inheritDoc
*/
public function getName() : string {
return $this->requirementName;
}
/**
* @inheritDoc
*/
public function isMet() : bool {
return (bool)$this->config->get( $this->configName );
}
}

View File

@ -6,28 +6,6 @@ API and associated scaffolding classes (see https://phabricator.wikimedia.org/T2
https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/572323/). This document aims to list
the steps required to get from this system to something as powerful as Piotr's.
1. ~~Decide whether "set" is the correct name~~
2. Add support for sets that utilise contextual information that isn't available at boot time, e.g.
```php
use Vector\Constants;
use IContextSource;
$featureManager->registerRequirement( Constants::LOGGED_IN_REQ, function( IContextSource $context ) {
$user = $context->getUser();
return $user
&& $user->isSafeToLoad()
&& $user->isLoggedIn();
} );
$featureManager->registerRequirement( Constants::MAINSPACE_REQ, function ( IContextSource $context ) {
$title = $context->getTitle();
return $title && $title->inNamespace( NS_MAIN );
} );
```
3. Consider supporing memoization of those requirements (see https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/573626/7/includes/FeatureManagement/FeatureManager.php@68)
4. Add support for getting all requirements
5. Add support for getting all features enabled when a requirement is enabled/disabled
1. Consider supporing memoization of those requirements (see https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/573626/7/includes/FeatureManagement/FeatureManager.php@68)
2. Add support for getting all requirements
3. Add support for getting all features enabled when a requirement is enabled/disabled

View File

@ -25,12 +25,22 @@
use MediaWiki\MediaWikiServices;
use Vector\Constants;
use Vector\FeatureManagement\FeatureManager;
use Vector\FeatureManagement\Requirements\DynamicConfigRequirement;
return [
Constants::SERVICE_CONFIG => function ( MediaWikiServices $services ) {
return $services->getService( 'ConfigFactory' )->makeConfig( Constants::SKIN_NAME );
},
Constants::SERVICE_FEATURE_MANAGER => function ( MediaWikiServices $services ) {
return new FeatureManager();
$requirement = new DynamicConfigRequirement(
$services->getMainConfig(),
Constants::CONFIG_KEY_FULLY_INITIALISED,
Constants::REQUIREMENT_FULLY_INITIALISED
);
$featureManager = new FeatureManager();
$featureManager->registerComplexRequirement( $requirement );
return $featureManager;
}
];

View File

@ -0,0 +1,74 @@
<?php
/**
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
* http://www.gnu.org/copyleft/gpl.html
*
* @file
* @since 1.35
*/
namespace Vector\FeatureManagement\Tests;
use Vector\FeatureManagement\Requirements\DynamicConfigRequirement;
/**
* @group Vector
* @group FeatureManagement
* @coversDefaultClass \Vector\FeatureManagement\Requirements\DynamicConfigRequirement
*/
class DynamicConfigRequirementTest extends \MediaWikiUnitTestCase {
/**
* @covers ::isMet
*/
public function testItFetchesAndReturnsConfigValue() {
$config = $this->createMock( \Config::class );
$config->expects( $this->once() )
->method( 'get' )
->with( 'Foo' )
->willReturn( true );
$requirement = new DynamicConfigRequirement( $config, 'Foo', 'Bar' );
$this->assertTrue( $requirement->isMet() );
}
/**
* @covers ::isMet
*/
public function testItCastsConfigValue() {
$config = new \HashConfig( [
'Foo' => new \stdClass(),
] );
$requirement = new DynamicConfigRequirement( $config, 'Foo', 'Bar' );
$this->assertTrue( $requirement->isMet() );
}
/**
* @covers ::getName
*/
public function testItReturnsName() {
$requirement = new DynamicConfigRequirement(
new \HashConfig(),
'Foo',
'Bar'
);
$this->assertEquals( 'Bar', $requirement->getName() );
}
}