diff --git a/includes/Constants.php b/includes/Constants.php index a39359c..6527da3 100644 --- a/includes/Constants.php +++ b/includes/Constants.php @@ -72,6 +72,16 @@ final class Constants { */ public const REQUIREMENT_FULLY_INITIALISED = 'FullyInitialised'; + /** + * @var string + */ + public const REQUIREMENT_LATEST_SKIN_VERSION = 'LatestSkinVersion'; + + /** + * @var string + */ + public const FEATURE_LATEST_SKIN = 'LatestSkin'; + // These are used for query parameters. /** * Override the skin version user preference and site Config. See readme. diff --git a/includes/SkinVersionLookup.php b/includes/FeatureManagement/Requirements/LatestSkinVersionRequirement.php similarity index 56% rename from includes/SkinVersionLookup.php rename to includes/FeatureManagement/Requirements/LatestSkinVersionRequirement.php index 8b43a3f..ba766c5 100644 --- a/includes/SkinVersionLookup.php +++ b/includes/FeatureManagement/Requirements/LatestSkinVersionRequirement.php @@ -20,36 +20,43 @@ * @since 1.35 */ -namespace Vector; +namespace Vector\FeatureManagement\Requirements; use Config; use User; +use Vector\Constants; +use Vector\FeatureManagement\Requirement; use WebRequest; /** - * Given initial dependencies, retrieve the current skin version. This class does no parsing, just - * the lookup. + * Retrieve the skin version for the request and compare it with `Constants::SKIN_VERSION_LATEST`. + * This requirement is met if the two are equal. * * Skin version is evaluated in the following order: * - * - useskinversion URL query parameter override. See readme. + * - `useskinversion` URL query parameter override. See `README.md`. * - * - User preference. The User object for new and existing accounts are updated by hook according to - * VectorDefaultSkinVersionForNewAccounts and VectorDefaultSkinVersionForExistingAccounts. See - * Hooks and skin.json. + * - User preference. The `User` object for new and existing accounts are updated by hook according + * to the `VectorDefaultSkinVersionForNewAccounts` and + * `VectorDefaultSkinVersionForExistingAccounts` config values. See the `Vector\Hooks` class and + * `skin.json`. * - * If the skin version is evaluated prior to User preference hook invocations, an incorrect + * If the skin version is evaluated prior to `User` preference hook invocations, an incorrect * version may be returned as only query parameter and site configuration will be known. * - * - Site configuration default. The default is controlled by VectorDefaultSkinVersion. This is used - * for anonymous users and as a fallback configuration. See skin.json. + * - Site configuration default. The default is controlled by the `VectorDefaultSkinVersion` config + * value. This is used for anonymous users and as a fallback configuration. See `skin.json`. + * + * This majority of this class is taken from Stephen Niedzielski's `Vector\SkinVersionLookup` class, + * which was introduced in `d1072d0fdfb1`. * * @unstable * - * @package Vector + * @package Vector\FeatureManagement\Requirements * @internal */ -final class SkinVersionLookup { +final class LatestSkinVersionRequirement implements Requirement { + /** * @var WebRequest */ @@ -64,8 +71,7 @@ final class SkinVersionLookup { private $config; /** - * This constructor accepts all dependencies needed to obtain the skin version. The dependencies - * are lazily evaluated, not cached, meaning they always return the current results. + * This constructor accepts all dependencies needed to obtain the skin version. * * @param WebRequest $request * @param User $user @@ -78,24 +84,30 @@ final class SkinVersionLookup { } /** - * Whether or not the legacy skin is being used. - * - * @return bool - * @throws \ConfigException + * @inheritDoc */ - public function isLegacy() { - return $this->getVersion() === Constants::SKIN_VERSION_LEGACY; + public function getName() : string { + return Constants::REQUIREMENT_LATEST_SKIN_VERSION; } /** - * The skin version as a string. E.g., `Constants::SKIN_VERSION_LEGACY`, - * `Constants::SKIN_VERSION_LATEST`, or maybe 'beta'. Note: it's likely someone will put arbitrary - * strings in the query parameter which means this function returns those strings as is. + * @inheritDoc + * @throws \ConfigException + */ + public function isMet() : bool { + return $this->getVersion() === Constants::SKIN_VERSION_LATEST; + } + + /** + * The skin version as a string. E.g., `Constants::SKIN_VERSION_LATEST`, + * `Constants::SKIN_VERSION_LATEST`, or maybe 'beta'. Note: it's likely someone will put + * arbitrary strings in the query parameter which means this function returns those strings as + * is. * * @return string * @throws \ConfigException */ - public function getVersion() { + private function getVersion() : string { // Obtain the skin version from the `useskinversion` URL query parameter override, the user // preference, or the configured default. return (string)$this->request->getVal( diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index ea9b7f0..7178974 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -26,6 +26,7 @@ use MediaWiki\MediaWikiServices; use Vector\Constants; use Vector\FeatureManagement\FeatureManager; use Vector\FeatureManagement\Requirements\DynamicConfigRequirement; +use Vector\FeatureManagement\Requirements\LatestSkinVersionRequirement; return [ Constants::SERVICE_CONFIG => function ( MediaWikiServices $services ) { @@ -41,6 +42,26 @@ return [ ); $featureManager->registerComplexRequirement( $requirement ); + // Feature: Latest skin + // ==================== + $context = RequestContext::getMain(); + + $featureManager->registerComplexRequirement( + new LatestSkinVersionRequirement( + $context->getRequest(), + $context->getUser(), + $context->getConfig() + ) + ); + + $featureManager->registerFeature( + Constants::FEATURE_LATEST_SKIN, + [ + Constants::REQUIREMENT_FULLY_INITIALISED, + Constants::REQUIREMENT_LATEST_SKIN_VERSION, + ] + ); + return $featureManager; } ]; diff --git a/includes/SkinVector.php b/includes/SkinVector.php index f061826..a64f2b7 100644 --- a/includes/SkinVector.php +++ b/includes/SkinVector.php @@ -21,7 +21,9 @@ * @file * @ingroup Skins */ -use Vector\SkinVersionLookup; + +use MediaWiki\MediaWikiServices; +use Vector\Constants; /** * Skin subclass for Vector @@ -36,21 +38,6 @@ class SkinVector extends SkinTemplate { private $responsiveMode = false; - /** - * @var SkinVersionLookup - */ - private $skinVersionLookup; - - /** - * @inheritDoc - */ - public function __construct( $skinname = null ) { - parent::__construct( $skinname ); - - $this->skinVersionLookup = - new SkinVersionLookup( $this->getRequest(), $this->getUser(), $this->getConfig() ); - } - /** * Enables the responsive mode */ @@ -82,7 +69,7 @@ class SkinVector extends SkinTemplate { public function getDefaultModules() { $modules = parent::getDefaultModules(); // add vector skin styles and vector module - $module = $this->skinVersionLookup->isLegacy() + $module = $this->isLegacy() ? 'skins.vector.styles.legacy' : 'skins.vector.styles'; $modules['styles']['skin'][] = $module; $modules['core'][] = 'skins.vector.js'; @@ -101,7 +88,7 @@ class SkinVector extends SkinTemplate { */ protected function setupTemplate( $classname ) { $tp = new TemplateParser( __DIR__ . '/templates' ); - return new VectorTemplate( $this->getConfig(), $tp, $this->skinVersionLookup->isLegacy() ); + return new VectorTemplate( $this->getConfig(), $tp, $this->isLegacy() ); } /** @@ -112,4 +99,17 @@ class SkinVector extends SkinTemplate { public function shouldPreloadLogo() { return true; } + + /** + * Whether or not the legacy version of the skin is being used. + * + * @return bool + */ + private function isLegacy() : bool { + $isLatestSkinFeatureEnabled = MediaWikiServices::getInstance() + ->getService( Constants::SERVICE_FEATURE_MANAGER ) + ->isFeatureEnabled( Constants::FEATURE_LATEST_SKIN ); + + return !$isLatestSkinFeatureEnabled; + } } diff --git a/tests/phpunit/integration/FeatureManagement/Requirements/LatestSkinVersionRequirementTest.php b/tests/phpunit/integration/FeatureManagement/Requirements/LatestSkinVersionRequirementTest.php new file mode 100644 index 0000000..dce38e9 --- /dev/null +++ b/tests/phpunit/integration/FeatureManagement/Requirements/LatestSkinVersionRequirementTest.php @@ -0,0 +1,101 @@ +getMockBuilder( \WebRequest::class )->getMock(); + $request + ->expects( $this->exactly( 1 ) ) + ->method( 'getVal' ) + ->with( $this->anything(), $this->equalTo( '1' ) ) + ->willReturn( 'beta' ); + + $user = $this->createMock( \User::class ); + $user + ->expects( $this->exactly( 1 ) ) + ->method( 'getOption' ) + ->with( $this->anything(), $this->equalTo( '2' ) ) + ->willReturn( '1' ); + + $config = new \HashConfig( [ 'VectorDefaultSkinVersion' => '2' ] ); + + $requirement = new LatestSkinVersionRequirement( $request, $user, $config ); + + $this->assertFalse( + $requirement->isMet(), + 'WebRequest::getVal takes precedence. "beta" isn\'t considered latest.' + ); + } + + /** + * @covers ::isMet + * @covers ::getVersion + */ + public function testUserPreference() { + $request = new WebRequest(); + + $user = $this->createMock( \User::class ); + $user + ->expects( $this->exactly( 1 ) ) + ->method( 'getOption' ) + ->with( $this->anything(), $this->equalTo( '2' ) ) + ->willReturn( '1' ); + + $config = new \HashConfig( [ 'VectorDefaultSkinVersion' => '2' ] ); + + $requirement = new LatestSkinVersionRequirement( $request, $user, $config ); + + $this->assertFalse( + $requirement->isMet(), + 'User preference takes second place. "1" (legacy) isn\'t considered latest.' + ); + } + + /** + * @covers ::isMet + * @covers ::getVersion + */ + public function testConfig() { + $request = new WebRequest(); + $user = \MediaWikiTestCase::getTestUser() + ->getUser(); + + $config = new HashConfig( [ 'VectorDefaultSkinVersion' => '2' ] ); + + $requirement = new LatestSkinVersionRequirement( $request, $user, $config ); + + $this->assertTrue( + $requirement->isMet(), + 'Config takes third place. "2" is considered latest.' + ); + } +} diff --git a/tests/phpunit/integration/SkinVersionLookupTest.php b/tests/phpunit/integration/SkinVersionLookupTest.php deleted file mode 100644 index 59fa77a..0000000 --- a/tests/phpunit/integration/SkinVersionLookupTest.php +++ /dev/null @@ -1,133 +0,0 @@ -getMockBuilder( \WebRequest::class )->getMock(); - $request - ->expects( $this->exactly( 2 ) ) - ->method( 'getVal' ) - ->with( $this->anything(), $this->equalTo( '1' ) ) - ->willReturn( 'beta' ); - - $user = $this->createMock( \User::class ); - $user - ->expects( $this->exactly( 2 ) ) - ->method( 'getOption' ) - ->with( $this->anything(), $this->equalTo( '2' ) ) - ->willReturn( '1' ); - - $config = new HashConfig( [ 'VectorDefaultSkinVersion' => '2' ] ); - - $skinVersionLookup = new SkinVersionLookup( $request, $user, $config ); - - $this->assertSame( - $skinVersionLookup->getVersion(), - 'beta', - 'Query parameter is the first priority.' - ); - $this->assertSame( - $skinVersionLookup->isLegacy(), - false, - 'Version is non-legacy.' - ); - } - - /** - * @covers ::getVersion - * @covers ::isLegacy - */ - public function testUserPreference() { - $request = $this->getMockBuilder( \WebRequest::class )->getMock(); - $request - ->expects( $this->exactly( 2 ) ) - ->method( 'getVal' ) - ->with( $this->anything(), $this->equalTo( '1' ) ) - ->willReturn( '1' ); - - $user = $this->createMock( \User::class ); - $user - ->expects( $this->exactly( 2 ) ) - ->method( 'getOption' ) - ->with( $this->anything(), $this->equalTo( '2' ) ) - ->willReturn( '1' ); - - $config = new HashConfig( [ 'VectorDefaultSkinVersion' => '2' ] ); - - $skinVersionLookup = new SkinVersionLookup( $request, $user, $config ); - - $this->assertSame( - $skinVersionLookup->getVersion(), - '1', - 'User preference is the second priority.' - ); - $this->assertSame( - $skinVersionLookup->isLegacy(), - true, - 'Version is legacy.' - ); - } - - /** - * @covers ::getVersion - * @covers ::isLegacy - */ - public function testConfig() { - $request = $this->getMockBuilder( \WebRequest::class )->getMock(); - $request - ->expects( $this->exactly( 2 ) ) - ->method( 'getVal' ) - ->with( $this->anything(), $this->equalTo( '2' ) ) - ->willReturn( '2' ); - - $user = $this->createMock( \User::class ); - $user - ->expects( $this->exactly( 2 ) ) - ->method( 'getOption' ) - ->with( $this->anything(), $this->equalTo( '2' ) ) - ->willReturn( '2' ); - - $config = new HashConfig( [ 'VectorDefaultSkinVersion' => '2' ] ); - - $skinVersionLookup = new SkinVersionLookup( $request, $user, $config ); - - $this->assertSame( - $skinVersionLookup->getVersion(), - '2', - 'Config is the third priority.' - ); - $this->assertSame( - $skinVersionLookup->isLegacy(), - false, - 'Version is non-legacy.' - ); - } -}