mirror of
https://gerrit.wikimedia.org/r/mediawiki/skins/Vector.git
synced 2024-11-23 23:33:54 +00:00
features: Add minimalist feature manager
With complex additions to Vector's codebase like the Desktop Improvement Program upcoming, it's important that we have a shared, intuitive language to talk about features and their requirements. Centralising the registration of features and creating an API satisfies does exactly this. This change introduces a greatly-reduced version of Piotr Miazga's (polishdeveloper, pmiazga) original proposed API and associated scaffolding classes for feature management in Vector, which itself was based upon his work in MobileFrontend/MinervaNeue. This is done to establish a foundation upon which we can build the more sophisticated parts of Piotr's proposal in a piecemeal basis, thereby minimising risk. Distinct from Piotr's proposed API is the ability to register sets and features that are always enabled or disabled. Additionally: - A Vector.FeatureManager service is registered but not used - A list of proposed immediate next steps is included Bug: T244481 Change-Id: Ie53c41d479eaf15559d5bb00f269774760360bde
This commit is contained in:
parent
e19f6da3e0
commit
9177c22365
182
includes/FeatureManagement/FeatureManager.php
Normal file
182
includes/FeatureManagement/FeatureManager.php
Normal file
|
@ -0,0 +1,182 @@
|
|||
<?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;
|
||||
|
||||
use Wikimedia\Assert\Assert;
|
||||
|
||||
/**
|
||||
* A simple feature manager.
|
||||
*
|
||||
* 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 FeatureManager {
|
||||
|
||||
/**
|
||||
* A map of feature name to the array of requirements. A feature is only considered enabled when
|
||||
* all of its requirements are met.
|
||||
*
|
||||
* See FeatureManager::registerFeature for additional detail.
|
||||
*
|
||||
* @var Array<string,string[]>
|
||||
*/
|
||||
private $features = [];
|
||||
|
||||
/**
|
||||
* A map of set name to whether the set is enabled.
|
||||
*
|
||||
* @var Array<string,bool>
|
||||
*/
|
||||
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];
|
||||
}
|
||||
}
|
33
includes/FeatureManagement/TODO.md
Normal file
33
includes/FeatureManagement/TODO.md
Normal file
|
@ -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
|
32
includes/ServiceWiring.php
Normal file
32
includes/ServiceWiring.php
Normal file
|
@ -0,0 +1,32 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* Service Wirings for Vector skin
|
||||
*
|
||||
* 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
|
||||
*/
|
||||
|
||||
use MediaWiki\MediaWikiServices;
|
||||
use Vector\FeatureManagement\FeatureManager;
|
||||
|
||||
return [
|
||||
'Vector.FeatureManager' => function ( MediaWikiServices $services ) {
|
||||
return new FeatureManager();
|
||||
}
|
||||
];
|
|
@ -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
|
||||
}
|
||||
|
|
156
tests/phpunit/unit/FeatureManagement/FeatureManagerTest.php
Normal file
156
tests/phpunit/unit/FeatureManagement/FeatureManagerTest.php
Normal file
|
@ -0,0 +1,156 @@
|
|||
<?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\FeatureManager;
|
||||
|
||||
/**
|
||||
* @group Vector
|
||||
* @group FeatureManagement
|
||||
* @coversDefaultClass \Vector\FeatureManagement\FeatureManager
|
||||
*/
|
||||
class FeatureManagerTest extends \MediaWikiUnitTestCase {
|
||||
|
||||
/**
|
||||
* @covers ::registerSet
|
||||
*/
|
||||
public function testRegisterSetThrowsWhenSetIsRegisteredTwice() {
|
||||
$this->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' );
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue