golem
/
VectorGOLEM
9
0
Fork 0
Code Issues 1 Pull Requests Releases Wiki Activity

Merge "featureManager: Set -> Requirement"

This commit is contained in:
jenkins-bot 2020-03-06 21:45:20 +00:00 committed by Gerrit Code Review
parent 478f0b5b24 3f95820467
commit ecfd7a2b28
3 changed files with 79 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

93
includes/FeatureManagement/FeatureManager.php
View File

@ -47,11 +47,11 @@ final class FeatureManager {
private $features = [];
/**
* A map of set name to whether the set is enabled.
* A map of requirement name to whether the requirement is met.
*
* @var Array<string,bool>
*/
private $sets = [];
private $requirements = [];
/**
* Register a feature and its requirements.
@ -59,42 +59,46 @@ final class FeatureManager {
* Essentially, a "feature" is a friendly (hopefully) name for some component, however big or
* small, that has some requirements. A feature manager allows us to decouple the component's
* logic from its requirements, allowing them to vary independently. Moreover, the use of
* friendly names wherever possible allows us to define common languages with our non-technical
* friendly names wherever possible allows us to define a common language with our non-technical
* colleagues.
*
* ```php
* $featureManager->registerFeature( 'featureB', 'setA' );
* $featureManager->registerFeature( 'featureA', 'requirementA' );
* ```
*
* defines the "featureB" feature, which is enabled when the "setA" set is enabled.
* defines the "featureA" feature, which is enabled when the "requirementA" requirement is met.
*
* ```php
* $featureManager->registerFeature( 'featureC', [ 'setA', 'setB' ] );
* $featureManager->registerFeature( 'featureB', [ 'requirementA', 'requirementB' ] );
* ```
*
* defines the "featureC" feature, which is enabled when the "setA" and "setB" sets are enabled.
* Note well that the feature is only enabled when _all_ requirements are met, i.e. the
* requirements are evaluated in order and logically `AND`ed together.
* defines the "featureB" feature, which is enabled when the "requirementA" and "requirementB"
* requirements are met. Note well that the feature is only enabled when _all_ requirements are
* met, i.e. the requirements are evaluated in order and logically `AND`ed together.
*
* @param string $feature The name of the feature
* @param string|array $requirements Which sets the feature requires to be enabled. As above,
* you can define a feature that requires a single set via the shorthand
* @param string|array $requirements The feature's requirements. As above, you can define a
* feature that requires a single requirement via the shorthand
*
* ```php
* $featureManager->registerFeature( 'feature', 'set' );
* // Equivalent to $featureManager->registerFeature( 'feature', [ 'set' ] );
* $featureManager->registerFeature( 'feature', 'requirementA' );
* // Equivalent to $featureManager->registerFeature( 'feature', [ 'requirementA' ] );
* ```
*
* @throws \LogicException If the feature is already registered
* @throws \Wikimedia\Assert\ParameterAssertionException If the feature's requirements aren't
* the name of a single set or an array of sets
* @throws \InvalidArgumentException If the feature requires a set that isn't registered
* the name of a single requirement or a list of requirements
* @throws \InvalidArgumentException If the feature references a requirement that isn't
* registered
*/
public function registerFeature( $feature, $requirements ) {
//
// Validation
if ( array_key_exists( $feature, $this->features ) ) {
throw new \LogicException( "Feature \"{$feature}\" is already registered." );
throw new \LogicException( sprintf(
'Feature "%s" is already registered.',
$feature
) );
}
Assert::parameterType( 'string|array', $requirements, 'requirements' );
@ -103,11 +107,13 @@ final class FeatureManager {
Assert::parameterElementType( 'string', $requirements, 'requirements' );
foreach ( $requirements as $set ) {
if ( !array_key_exists( $set, $this->sets ) ) {
throw new \InvalidArgumentException(
"Feature \"{$feature}\" references set \"{$set}\", which hasn't been registered"
);
foreach ( $requirements as $name ) {
if ( !array_key_exists( $name, $this->requirements ) ) {
throw new \InvalidArgumentException( sprintf(
'Feature "%s" references requirement "%s", which hasn\'t been registered',
$feature,
$name
) );
}
}
@ -130,8 +136,8 @@ final class FeatureManager {
$requirements = $this->features[$feature];
foreach ( $requirements as $set ) {
if ( !$this->sets[$set] ) {
foreach ( $requirements as $name ) {
if ( !$this->requirements[$name] ) {
return false;
}
}
@ -140,43 +146,44 @@ final class FeatureManager {
}
/**
* Register a set.
* Register a requirement.
*
* A set is some condition of the application state that a feature requires to be true or false.
* A requirement is some condition of the application state that a feature requires to be true
* or false.
*
* At the moment, these conditions can only be evaluated when the set is being defined, i.e. at
* boot time. At that time, certain objects mightn't have been fully loaded
* (see User::isSafeToLoad). See TODO.md for the proposed list of steps to allow this feature
* At the moment, these conditions can only be evaluated when the requirement is being defined,
* i.e. at boot time. At that time, certain objects mightn't have been fully loaded (see
* User::isSafeToLoad). See TODO.md for the proposed list of steps to allow this feature
* manager to handle that scenario.
*
* @param string $set The name of the set
* @param bool $isEnabled Whether the set is enabled
* @param string $name The name of the requirement
* @param bool $isMet Whether the requirement is met
*
* @throws \LogicException If the set has already been registered
* @throws \LogicException If the requirement has already been registered
*/
public function registerSet( $set, $isEnabled ) {
if ( array_key_exists( $set, $this->sets ) ) {
throw new \LogicException( "Set \"{$set}\" is already registered." );
public function registerRequirement( $name, $isMet ) {
if ( array_key_exists( $name, $this->requirements ) ) {
throw new \LogicException( "The requirement \"{$name}\" is already registered." );
}
Assert::parameterType( 'boolean', $isEnabled, 'isEnabled' );
Assert::parameterType( 'boolean', $isMet, 'isMet' );
$this->sets[$set] = $isEnabled;
$this->requirements[$name] = $isMet;
}
/**
* Gets whether the set is enabled.
* Gets whether the requirement is met.
*
* @param string $set The name of the set
* @param string $name The name of the requirement
* @return bool
*
* @throws \InvalidArgumentException If the set isn't registerd
* @throws \InvalidArgumentException If the requirement isn't registered
*/
public function isSetEnabled( $set ) {
if ( !array_key_exists( $set, $this->sets ) ) {
throw new \InvalidArgumentException( "Set \"{$set}\" isn't registered." );
public function isRequirementMet( $name ) {
if ( !array_key_exists( $name, $this->requirements ) ) {
throw new \InvalidArgumentException( "Requirement \"{$name}\" isn't registered." );
}
return $this->sets[$set];
return $this->requirements[$name];
}
}

12
includes/FeatureManagement/TODO.md
View File

@ -6,14 +6,14 @@ 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
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->registerSet( Constants::LOGGED_IN_SET, function( IContextSource $context ) {
$featureManager->registerRequirement( Constants::LOGGED_IN_REQ, function( IContextSource $context ) {
$user = $context->getUser();
return $user
@ -21,13 +21,13 @@ $featureManager->registerSet( Constants::LOGGED_IN_SET, function( IContextSource
&& $user->isLoggedIn();
} );
$featureManager->registerSet( Constants::MAINSPACE_SET, function ( IContextSource $context ) {
$featureManager->registerRequirement( Constants::MAINSPACE_REQ, function ( IContextSource $context ) {
$title = $context->getTitle();
return $title && $title->inNamespace( NS_MAIN );
} );
```
3. Consider supporing memoization of those sets (see https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/573626/7/includes/FeatureManagement/FeatureManager.php@68)
4. Add support for getting all sets
5. Add support for getting all features enabled when a set is enabled/disabled
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

46
tests/phpunit/unit/FeatureManagement/FeatureManagerTest.php
View File

@ -31,24 +31,24 @@ use Vector\FeatureManagement\FeatureManager;
class FeatureManagerTest extends \MediaWikiUnitTestCase {
/**
* @covers ::registerSet
* @covers ::registerRequirement
*/
public function testRegisterSetThrowsWhenSetIsRegisteredTwice() {
public function testRegisterRequirementThrowsWhenRequirementIsRegisteredTwice() {
$this->expectException( \LogicException::class );
$featureManager = new FeatureManager();
$featureManager->registerSet( 'setA', true );
$featureManager->registerSet( 'setA', true );
$featureManager->registerRequirement( 'requirementA', true );
$featureManager->registerRequirement( 'requirementA', true );
}
/**
* @covers ::registerSet
* @covers ::registerRequirement
*/
public function testRegisterSetValidatesIsEnabled() {
public function testRegisterRequirementValidatesIsEnabled() {
$this->expectException( \Wikimedia\Assert\ParameterAssertionException::class );
$featureManager = new FeatureManager();
$featureManager->registerSet( 'setA', 'foo' );
$featureManager->registerRequirement( 'requirementA', 'foo' );
}
public static function provideInvalidFeatureConfig() {
@ -60,7 +60,7 @@ class FeatureManagerTest extends \MediaWikiUnitTestCase {
[ 1 ],
],
// The "bar" set hasn't been registered.
// The "bar" requirement hasn't been registered.
[
\InvalidArgumentException::class,
[
@ -78,30 +78,30 @@ class FeatureManagerTest extends \MediaWikiUnitTestCase {
$this->expectException( $expectedExceptionType );
$featureManager = new FeatureManager();
$featureManager->registerSet( 'set', true );
$featureManager->registerRequirement( 'requirement', true );
$featureManager->registerFeature( 'feature', $config );
}
/**
* @covers ::isSetEnabled
* @covers ::isRequirementMet
*/
public function testIsSetEnabled() {
public function testIsRequirementMet() {
$featureManager = new FeatureManager();
$featureManager->registerSet( 'enabled', true );
$featureManager->registerSet( 'disabled', false );
$featureManager->registerRequirement( 'enabled', true );
$featureManager->registerRequirement( 'disabled', false );
$this->assertTrue( $featureManager->isSetEnabled( 'enabled' ) );
$this->assertFalse( $featureManager->isSetEnabled( 'disabled' ) );
$this->assertTrue( $featureManager->isRequirementMet( 'enabled' ) );
$this->assertFalse( $featureManager->isRequirementMet( 'disabled' ) );
}
/**
* @covers ::isSetEnabled
* @covers ::isRequirementMet
*/
public function testIsSetEnabledThrowsExceptionWhenSetIsntRegistered() {
public function testIsRequirementMetThrowsExceptionWhenRequirementIsntRegistered() {
$this->expectException( \InvalidArgumentException::class );
$featureManager = new FeatureManager();
$featureManager->isSetEnabled( 'foo' );
$featureManager->isRequirementMet( 'foo' );
}
/**
@ -120,25 +120,25 @@ class FeatureManagerTest extends \MediaWikiUnitTestCase {
*/
public function testIsFeatureEnabled() {
$featureManager = new FeatureManager();
$featureManager->registerSet( 'foo', false );
$featureManager->registerRequirement( 'foo', false );
$featureManager->registerFeature( 'requiresFoo', 'foo' );
$this->assertFalse(
$featureManager->isFeatureEnabled( 'requiresFoo' ),
'A feature is disabled when the set that it requires is disabled.'
'A feature is disabled when the requirement that it requires is disabled.'
);
// ---
$featureManager->registerSet( 'bar', true );
$featureManager->registerSet( 'baz', true );
$featureManager->registerRequirement( 'bar', true );
$featureManager->registerRequirement( 'baz', true );
$featureManager->registerFeature( 'requiresFooBar', [ 'foo', 'bar' ] );
$featureManager->registerFeature( 'requiresBarBaz', [ 'bar', 'baz' ] );
$this->assertFalse(
$featureManager->isFeatureEnabled( 'requiresFooBar' ),
'A feature is disabled when at least one set that it requires is disabled.'
'A feature is disabled when at least one requirement that it requires is disabled.'
);
$this->assertTrue( $featureManager->isFeatureEnabled( 'requiresBarBaz' ) );