wikimedia/mediawiki-skins-Vector
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/skins/Vector.git synced 2024-12-18 02:40:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

featureManager: Set -> Requirement

Browse source 
As was noted in https://phabricator.wikimedia.org/T244481#5859513, the
term "set" doesn't seem natural. Piotr Miazga (polishdeveloper, pmiazga)
and Nicholas Ray (nray) suggested a number of good replacements,
including "requirement." Serendipitously, this term is already used in
FeatureManager's documentation.

Bug: T244481
Change-Id: I559c2d4149db69235cdd4bb880697deb1a145743
This commit is contained in:
Sam Smith 2020-02-28 15:42:05 +00:00
parent 3683a52f91
commit 3f95820467
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 = []; 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> * @var Array<string,bool>
*/ */
private $sets = []; private $requirements = [];
/** /**
* Register a feature and its 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 * 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 * 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 * 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. * colleagues.
* *
* ```php * ```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 * ```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. * defines the "featureB" feature, which is enabled when the "requirementA" and "requirementB"
* Note well that the feature is only enabled when _all_ requirements are met, i.e. the * requirements are met. Note well that the feature is only enabled when _all_ requirements are
* requirements are evaluated in order and logically `AND`ed together. * met, i.e. the requirements are evaluated in order and logically `AND`ed together.
* *
* @param string $feature The name of the feature * @param string $feature The name of the feature
* @param string|array $requirements Which sets the feature requires to be enabled. As above, * @param string|array $requirements The feature's requirements. As above, you can define a
* you can define a feature that requires a single set via the shorthand * feature that requires a single requirement via the shorthand
* *
* ```php * ```php
* $featureManager->registerFeature( 'feature', 'set' ); * $featureManager->registerFeature( 'feature', 'requirementA' );
* // Equivalent to $featureManager->registerFeature( 'feature', [ 'set' ] ); * // Equivalent to $featureManager->registerFeature( 'feature', [ 'requirementA' ] );
* ``` * ```
* *
* @throws \LogicException If the feature is already registered * @throws \LogicException If the feature is already registered
* @throws \Wikimedia\Assert\ParameterAssertionException If the feature's requirements aren't * @throws \Wikimedia\Assert\ParameterAssertionException If the feature's requirements aren't
* the name of a single set or an array of sets * the name of a single requirement or a list of requirements
* @throws \InvalidArgumentException If the feature requires a set that isn't registered * @throws \InvalidArgumentException If the feature references a requirement that isn't
* registered
*/ */
public function registerFeature( $feature, $requirements ) { public function registerFeature( $feature, $requirements ) {
// //
// Validation // Validation
if ( array_key_exists( $feature, $this->features ) ) { 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' ); Assert::parameterType( 'string|array', $requirements, 'requirements' );
@ -103,11 +107,13 @@ final class FeatureManager {
Assert::parameterElementType( 'string', $requirements, 'requirements' ); Assert::parameterElementType( 'string', $requirements, 'requirements' );
foreach ( $requirements as $set ) { foreach ( $requirements as $name ) {
if ( !array_key_exists( $set, $this->sets ) ) { if ( !array_key_exists( $name, $this->requirements ) ) {
throw new \InvalidArgumentException( throw new \InvalidArgumentException( sprintf(
"Feature \"{$feature}\" references set \"{$set}\", which hasn't been registered" 'Feature "%s" references requirement "%s", which hasn\'t been registered',
); $feature,
$name
) );
} }
} }
@ -130,8 +136,8 @@ final class FeatureManager {
$requirements = $this->features[$feature]; $requirements = $this->features[$feature];
foreach ( $requirements as $set ) { foreach ( $requirements as $name ) {
if ( !$this->sets[$set] ) { if ( !$this->requirements[$name] ) {
return false; 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 * At the moment, these conditions can only be evaluated when the requirement is being defined,
* boot time. At that time, certain objects mightn't have been fully loaded * i.e. at boot time. At that time, certain objects mightn't have been fully loaded (see
* (see User::isSafeToLoad). See TODO.md for the proposed list of steps to allow this feature * User::isSafeToLoad). See TODO.md for the proposed list of steps to allow this feature
* manager to handle that scenario. * manager to handle that scenario.
* *
* @param string $set The name of the set * @param string $name The name of the requirement
* @param bool $isEnabled Whether the set is enabled * @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 ) { public function registerRequirement( $name, $isMet ) {
if ( array_key_exists( $set, $this->sets ) ) { if ( array_key_exists( $name, $this->requirements ) ) {
throw new \LogicException( "Set \"{$set}\" is already registered." ); 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 * @return bool
* *
* @throws \InvalidArgumentException If the set isn't registerd * @throws \InvalidArgumentException If the requirement isn't registered
*/ */
public function isSetEnabled( $set ) { public function isRequirementMet( $name ) {
if ( !array_key_exists( $set, $this->sets ) ) { if ( !array_key_exists( $name, $this->requirements ) ) {
throw new \InvalidArgumentException( "Set \"{$set}\" isn't registered." ); 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 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. 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. 2. Add support for sets that utilise contextual information that isn't available at boot time, e.g.
```php ```php
use Vector\Constants; use Vector\Constants;
use IContextSource; use IContextSource;
$featureManager->registerSet( Constants::LOGGED_IN_SET, function( IContextSource $context ) { $featureManager->registerRequirement( Constants::LOGGED_IN_REQ, function( IContextSource $context ) {
$user = $context->getUser(); $user = $context->getUser();
return $user return $user
@ -21,13 +21,13 @@ $featureManager->registerSet( Constants::LOGGED_IN_SET, function( IContextSource
&& $user->isLoggedIn(); && $user->isLoggedIn();
} ); } );
$featureManager->registerSet( Constants::MAINSPACE_SET, function ( IContextSource $context ) { $featureManager->registerRequirement( Constants::MAINSPACE_REQ, function ( IContextSource $context ) {
$title = $context->getTitle(); $title = $context->getTitle();
return $title && $title->inNamespace( NS_MAIN ); 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) 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 sets 4. Add support for getting all requirements
5. Add support for getting all features enabled when a set is enabled/disabled 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 { class FeatureManagerTest extends \MediaWikiUnitTestCase {
/** /**
* @covers ::registerSet * @covers ::registerRequirement
*/ */
public function testRegisterSetThrowsWhenSetIsRegisteredTwice() { public function testRegisterRequirementThrowsWhenRequirementIsRegisteredTwice() {
$this->expectException( \LogicException::class ); $this->expectException( \LogicException::class );
$featureManager = new FeatureManager(); $featureManager = new FeatureManager();
$featureManager->registerSet( 'setA', true ); $featureManager->registerRequirement( 'requirementA', true );
$featureManager->registerSet( 'setA', true ); $featureManager->registerRequirement( 'requirementA', true );
} }
/** /**
* @covers ::registerSet * @covers ::registerRequirement
*/ */
public function testRegisterSetValidatesIsEnabled() { public function testRegisterRequirementValidatesIsEnabled() {
$this->expectException( \Wikimedia\Assert\ParameterAssertionException::class ); $this->expectException( \Wikimedia\Assert\ParameterAssertionException::class );
$featureManager = new FeatureManager(); $featureManager = new FeatureManager();
$featureManager->registerSet( 'setA', 'foo' ); $featureManager->registerRequirement( 'requirementA', 'foo' );
} }
public static function provideInvalidFeatureConfig() { public static function provideInvalidFeatureConfig() {
@ -60,7 +60,7 @@ class FeatureManagerTest extends \MediaWikiUnitTestCase {
[ 1 ], [ 1 ],
], ],
// The "bar" set hasn't been registered. // The "bar" requirement hasn't been registered.
[ [
\InvalidArgumentException::class, \InvalidArgumentException::class,
[ [
@ -78,30 +78,30 @@ class FeatureManagerTest extends \MediaWikiUnitTestCase {
$this->expectException( $expectedExceptionType ); $this->expectException( $expectedExceptionType );
$featureManager = new FeatureManager(); $featureManager = new FeatureManager();
$featureManager->registerSet( 'set', true ); $featureManager->registerRequirement( 'requirement', true );
$featureManager->registerFeature( 'feature', $config ); $featureManager->registerFeature( 'feature', $config );
} }
/** /**
* @covers ::isSetEnabled * @covers ::isRequirementMet
*/ */
public function testIsSetEnabled() { public function testIsRequirementMet() {
$featureManager = new FeatureManager(); $featureManager = new FeatureManager();
$featureManager->registerSet( 'enabled', true ); $featureManager->registerRequirement( 'enabled', true );
$featureManager->registerSet( 'disabled', false ); $featureManager->registerRequirement( 'disabled', false );
$this->assertTrue( $featureManager->isSetEnabled( 'enabled' ) ); $this->assertTrue( $featureManager->isRequirementMet( 'enabled' ) );
$this->assertFalse( $featureManager->isSetEnabled( 'disabled' ) ); $this->assertFalse( $featureManager->isRequirementMet( 'disabled' ) );
} }
/** /**
* @covers ::isSetEnabled * @covers ::isRequirementMet
*/ */
public function testIsSetEnabledThrowsExceptionWhenSetIsntRegistered() { public function testIsRequirementMetThrowsExceptionWhenRequirementIsntRegistered() {
$this->expectException( \InvalidArgumentException::class ); $this->expectException( \InvalidArgumentException::class );
$featureManager = new FeatureManager(); $featureManager = new FeatureManager();
$featureManager->isSetEnabled( 'foo' ); $featureManager->isRequirementMet( 'foo' );
} }
/** /**
@ -120,25 +120,25 @@ class FeatureManagerTest extends \MediaWikiUnitTestCase {
*/ */
public function testIsFeatureEnabled() { public function testIsFeatureEnabled() {
$featureManager = new FeatureManager(); $featureManager = new FeatureManager();
$featureManager->registerSet( 'foo', false ); $featureManager->registerRequirement( 'foo', false );
$featureManager->registerFeature( 'requiresFoo', 'foo' ); $featureManager->registerFeature( 'requiresFoo', 'foo' );
$this->assertFalse( $this->assertFalse(
$featureManager->isFeatureEnabled( 'requiresFoo' ), $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->registerRequirement( 'bar', true );
$featureManager->registerSet( 'baz', true ); $featureManager->registerRequirement( 'baz', true );
$featureManager->registerFeature( 'requiresFooBar', [ 'foo', 'bar' ] ); $featureManager->registerFeature( 'requiresFooBar', [ 'foo', 'bar' ] );
$featureManager->registerFeature( 'requiresBarBaz', [ 'bar', 'baz' ] ); $featureManager->registerFeature( 'requiresBarBaz', [ 'bar', 'baz' ] );
$this->assertFalse( $this->assertFalse(
$featureManager->isFeatureEnabled( 'requiresFooBar' ), $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' ) ); $this->assertTrue( $featureManager->isFeatureEnabled( 'requiresBarBaz' ) );