diff --git a/includes/FeatureManagement/FeatureManager.php b/includes/FeatureManagement/FeatureManager.php new file mode 100644 index 0000000..0d12312 --- /dev/null +++ b/includes/FeatureManagement/FeatureManager.php @@ -0,0 +1,182 @@ + + */ + private $features = []; + + /** + * A map of set name to whether the set is enabled. + * + * @var Array + */ + private $sets = []; + + /** + * Register a feature and its requirements. + * + * 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 + * colleagues. + * + * ```php + * $featureManager->registerFeature( 'featureB', 'setA' ); + * ``` + * + * defines the "featureB" feature, which is enabled when the "setA" set is enabled. + * + * ```php + * $featureManager->registerFeature( 'featureC', [ 'setA', 'setB' ] ); + * ``` + * + * 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. + * + * @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 + * + * ```php + * $featureManager->registerFeature( 'feature', 'set' ); + * // Equivalent to $featureManager->registerFeature( 'feature', [ 'set' ] ); + * ``` + * + * @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 + */ + public function registerFeature( $feature, $requirements ) { + // + // Validation + if ( array_key_exists( $feature, $this->features ) ) { + throw new \LogicException( "Feature \"{$feature}\" is already registered." ); + } + + Assert::parameterType( 'string|array', $requirements, 'requirements' ); + + $requirements = (array)$requirements; + + 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" + ); + } + } + + // Mutation + $this->features[$feature] = $requirements; + } + + /** + * Gets whether the feature's requirements are met. + * + * @param string $feature + * @return bool + * + * @throws \InvalidArgumentException If the feature isn't registered + */ + public function isFeatureEnabled( $feature ) { + if ( !array_key_exists( $feature, $this->features ) ) { + throw new \InvalidArgumentException( "The feature \"{$feature}\" isn't registered." ); + } + + $requirements = $this->features[$feature]; + + foreach ( $requirements as $set ) { + if ( !$this->sets[$set] ) { + return false; + } + } + + return true; + } + + /** + * Register a set. + * + * A set 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 + * manager to handle that scenario. + * + * @param string $set The name of the set + * @param bool $isEnabled Whether the set is enabled + * + * @throws \LogicException If the set has already been registered + */ + public function registerSet( $set, $isEnabled ) { + if ( array_key_exists( $set, $this->sets ) ) { + throw new \LogicException( "Set \"{$set}\" is already registered." ); + } + + Assert::parameterType( 'boolean', $isEnabled, 'isEnabled' ); + + $this->sets[$set] = $isEnabled; + } + + /** + * Gets whether the set is enabled. + * + * @param string $set The name of the set + * @return bool + * + * @throws \InvalidArgumentException If the set isn't registerd + */ + public function isSetEnabled( $set ) { + if ( !array_key_exists( $set, $this->sets ) ) { + throw new \InvalidArgumentException( "Set \"{$set}\" isn't registered." ); + } + + return $this->sets[$set]; + } +} diff --git a/includes/FeatureManagement/TODO.md b/includes/FeatureManagement/TODO.md new file mode 100644 index 0000000..62b7019 --- /dev/null +++ b/includes/FeatureManagement/TODO.md @@ -0,0 +1,33 @@ +TODO +==== + +Currently the `FeatureManager` class is a very shallow interpretation of Piotr Miazga's proposed +API and associated scaffolding classes (see https://phabricator.wikimedia.org/T244481 and +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->registerSet( Constants::LOGGED_IN_SET, function( IContextSource $context ) { + $user = $context->getUser(); + + return $user + && $user->isSafeToLoad() + && $user->isLoggedIn(); +} ); + +$featureManager->registerSet( Constants::MAINSPACE_SET, 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 diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php new file mode 100644 index 0000000..432a85c --- /dev/null +++ b/includes/ServiceWiring.php @@ -0,0 +1,32 @@ + function ( MediaWikiServices $services ) { + return new FeatureManager(); + } +]; diff --git a/skin.json b/skin.json index 2b1de0b..475bfbf 100644 --- a/skin.json +++ b/skin.json @@ -26,6 +26,9 @@ "SkinVector": "includes/SkinVector.php", "VectorTemplate": "includes/VectorTemplate.php" }, + "AutoloadNamespaces": { + "Vector\\FeatureManagement\\": "includes/FeatureManagement/" + }, "Hooks": { "BeforePageDisplayMobile": "Vector\\Hooks::onBeforePageDisplayMobile" }, @@ -108,5 +111,8 @@ "value": false } }, + "ServiceWiringFiles": [ + "includes/ServiceWiring.php" + ], "manifest_version": 2 } diff --git a/tests/phpunit/unit/FeatureManagement/FeatureManagerTest.php b/tests/phpunit/unit/FeatureManagement/FeatureManagerTest.php new file mode 100644 index 0000000..c217a44 --- /dev/null +++ b/tests/phpunit/unit/FeatureManagement/FeatureManagerTest.php @@ -0,0 +1,156 @@ +expectException( \LogicException::class ); + + $featureManager = new FeatureManager(); + $featureManager->registerSet( 'setA', true ); + $featureManager->registerSet( 'setA', true ); + } + + /** + * @covers ::registerSet + */ + public function testRegisterSetValidatesIsEnabled() { + $this->expectException( \Wikimedia\Assert\ParameterAssertionException::class ); + + $featureManager = new FeatureManager(); + $featureManager->registerSet( 'setA', 'foo' ); + } + + public static function provideInvalidFeatureConfig() { + return [ + + // ::registerFeature( string, int[] ) will throw an exception. + [ + \Wikimedia\Assert\ParameterAssertionException::class, + [ 1 ], + ], + + // The "bar" set hasn't been registered. + [ + \InvalidArgumentException::class, + [ + 'bar', + ], + ], + ]; + } + + /** + * @dataProvider provideInvalidFeatureConfig + * @covers ::registerFeature + */ + public function testRegisterFeatureValidatesConfig( $expectedExceptionType, $config ) { + $this->expectException( $expectedExceptionType ); + + $featureManager = new FeatureManager(); + $featureManager->registerSet( 'set', true ); + $featureManager->registerFeature( 'feature', $config ); + } + + /** + * @covers ::isSetEnabled + */ + public function testIsSetEnabled() { + $featureManager = new FeatureManager(); + $featureManager->registerSet( 'enabled', true ); + $featureManager->registerSet( 'disabled', false ); + + $this->assertTrue( $featureManager->isSetEnabled( 'enabled' ) ); + $this->assertFalse( $featureManager->isSetEnabled( 'disabled' ) ); + } + + /** + * @covers ::isSetEnabled + */ + public function testIsSetEnabledThrowsExceptionWhenSetIsntRegistered() { + $this->expectException( \InvalidArgumentException::class ); + + $featureManager = new FeatureManager(); + $featureManager->isSetEnabled( 'foo' ); + } + + /** + * @covers ::registerFeature + */ + public function testRegisterFeatureThrowsExceptionWhenFeatureIsRegisteredTwice() { + $this->expectException( \LogicException::class ); + + $featureManager = new FeatureManager(); + $featureManager->registerFeature( 'featureA', [] ); + $featureManager->registerFeature( 'featureA', [] ); + } + + /** + * @covers ::isFeatureEnabled + */ + public function testIsFeatureEnabled() { + $featureManager = new FeatureManager(); + $featureManager->registerSet( 'foo', false ); + $featureManager->registerFeature( 'requiresFoo', 'foo' ); + + $this->assertFalse( + $featureManager->isFeatureEnabled( 'requiresFoo' ), + 'A feature is disabled when the set that it requires is disabled.' + ); + + // --- + + $featureManager->registerSet( 'bar', true ); + $featureManager->registerSet( '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.' + ); + + $this->assertTrue( $featureManager->isFeatureEnabled( 'requiresBarBaz' ) ); + } + + /** + * @covers ::isFeatureEnabled + */ + public function testIsFeatureEnabledThrowsExceptionWhenFeatureIsntRegistered() { + $this->expectException( \InvalidArgumentException::class ); + + $featureManager = new FeatureManager(); + $featureManager->isFeatureEnabled( 'foo' ); + } +}