Foxtrott has uploaded a new change for review. https://gerrit.wikimedia.org/r/174954
Change subject: Introduces monitoring of multiple files for cache invalidation ...................................................................... Introduces monitoring of multiple files for cache invalidation * adds composer.lock to default list of files monitored for cache triggering * adds 'cachetriggers' key to keys recognized by ResourceLoaderBootstrapModule * adds 'cachetriggers' key to $wgResourcemodules[ 'ext.bootstrap.styles' ] Change-Id: I32a198f1b6f34d8fa93c4e650cd2f3f6f781c5de --- M Bootstrap.php M src/BootstrapManager.php M src/Hooks/SetupAfterCache.php M src/ResourceLoaderBootstrapModule.php M tests/phpunit/BootstrapManagerTest.php M tests/phpunit/Hooks/SetupAfterCacheTest.php M tests/phpunit/ResourceLoaderBootstrapModuleTest.php 7 files changed, 196 insertions(+), 47 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Bootstrap refs/changes/54/174954/1 diff --git a/Bootstrap.php b/Bootstrap.php index 96ee9a7..b82ff09 100644 --- a/Bootstrap.php +++ b/Bootstrap.php @@ -49,7 +49,7 @@ /** * The extension version */ - define( 'BS_VERSION', '1.0.2-alpha' ); + define( 'BS_VERSION', '1.1-alpha' ); // register the extension $GLOBALS[ 'wgExtensionCredits' ][ 'other' ][ ] = array( @@ -77,6 +77,7 @@ $configuration = array(); $configuration[ 'localBasePath' ] = str_replace( DIRECTORY_SEPARATOR . 'extensions' . DIRECTORY_SEPARATOR . 'Bootstrap', '/vendor/twitter/bootstrap', __DIR__ ); $configuration[ 'remoteBasePath' ] = str_replace( $GLOBALS[ 'IP' ], $GLOBALS[ 'wgScriptPath' ], $configuration[ 'localBasePath' ] ); + $configuration[ 'IP' ] = $GLOBALS[ 'IP' ]; $setupAfterCache = new \Bootstrap\Hooks\SetupAfterCache( $configuration ); $setupAfterCache->process(); @@ -89,6 +90,10 @@ 'styles' => array(), 'variables' => array(), 'dependencies' => array(), + 'cachetriggers' => array( + 'LocalSettings.php' => null, + 'composer.lock' => null, + ), ); $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.scripts' ] = array( diff --git a/src/BootstrapManager.php b/src/BootstrapManager.php index 225aef2..7248320 100644 --- a/src/BootstrapManager.php +++ b/src/BootstrapManager.php @@ -113,23 +113,22 @@ /** * @param string $filetype 'styles'|'scripts' - * @param array|string $description + * @param mixed[] $description * @param $fileExt - * - * @internal param $relativePath */ protected function addFilesToGlobalResourceModules ( $filetype, $description, $fileExt ) { if ( isset( $description[ $filetype ] ) ) { - $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.' . $filetype ][ $filetype ] = - array_merge( - $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.' . $filetype ][ $filetype ], - array_map( - function ( $filename ) use ( $fileExt ) { return $filename . $fileExt; }, - (array) $description[ $filetype ] - ) - ); + $files = array_map( + function ( $filename ) use ( $fileExt ) { + return $filename . $fileExt; + }, + (array) $description[ $filetype ] + ); + + $this->adjustArrayElementOfResourceModuleDescription( $filetype, $files, $filetype ); + } } @@ -151,7 +150,7 @@ * @internal param string $path */ public function addExternalModule( $file, $remotePath = '' ) { - $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.styles' ][ 'external styles' ][ $file ] = $remotePath; + $this->adjustArrayElementOfResourceModuleDescription( 'external styles', array( $file => $remotePath ) ); } /** @@ -167,14 +166,18 @@ /** * @since 1.0 * - * @param $variables + * @param mixed[] $variables */ public function setLessVariables( $variables ) { - $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.styles' ][ 'variables' ] = - array_merge( - $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.styles' ][ 'variables' ], - $variables - ); + $this->adjustArrayElementOfResourceModuleDescription( 'variables', $variables ); + } + + /** + * @since 1.1 + * @param string|string[] $files + */ + public function addCacheTriggerFile( $files ){ + $this->adjustArrayElementOfResourceModuleDescription( 'cachetriggers', $files ); } protected function initCoreModules() { @@ -182,4 +185,21 @@ $this->addBootstrapModule( $this->moduleDefinition->get( 'core' ) ); } + /** + * @param string $key + * @param mixed $value + * @param string $filetype 'styles'|'scripts' + */ + protected function adjustArrayElementOfResourceModuleDescription( $key, $value, $filetype = 'styles' ) { + + if (!isset($GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.' . $filetype ][ $key ])) { + $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.' . $filetype ][ $key ] = $value; + } else { + $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.' . $filetype ][ $key ] = + array_merge( + $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.' . $filetype ][ $key ], + (array) $value + ); + } + } } diff --git a/src/Hooks/SetupAfterCache.php b/src/Hooks/SetupAfterCache.php index 1554b0d..db8e5da 100644 --- a/src/Hooks/SetupAfterCache.php +++ b/src/Hooks/SetupAfterCache.php @@ -60,9 +60,7 @@ */ public function process() { - if ( !$this->hasConfiguration( 'localBasePath' ) || !$this->hasConfiguration( 'remoteBasePath' ) ) { - throw new InvalidArgumentException( 'Expected a valid configuration' ); - } + $this->assertAcceptableConfiguration(); $this->removeLegacyLessCompilerFromComposerAutoloader(); @@ -70,6 +68,8 @@ $this->isReadablePath( $this->configuration['localBasePath'] ), $this->configuration[ 'remoteBasePath' ] ); + + $this->registerCacheTriggers(); return true; } @@ -147,4 +147,34 @@ throw new RuntimeException( "Expected an accessible {$localBasePath} path" ); } + protected function registerCacheTriggers() { + + $defaultRecacheTriggers = array( + 'LocalSettings.php' => $this->configuration[ 'IP' ] . '/LocalSettings.php', + 'composer.lock' => $this->configuration[ 'IP' ] . '/composer.lock', + ); + + foreach ( $defaultRecacheTriggers as $key => $filename ) { + if ( array_key_exists( $key, $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.styles' ][ 'cachetriggers' ] ) && + $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.styles' ][ 'cachetriggers' ][ $key ] === null ) { + $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.styles' ][ 'cachetriggers' ][ $key ] = $filename; + } + } + } + + public function assertAcceptableConfiguration() { + + $configElements = array( + 'localBasePath' => 'Local base path to Bootstrap modules not found.', + 'remoteBasePath' => 'Remote base path to Bootstrap modules not found.', + 'IP' => 'Full path to working directory ($IP) not found.', + ); + + foreach ( $configElements as $key => $errorMessage ) { + if ( !$this->hasConfiguration( $key ) ) { + throw new InvalidArgumentException( $errorMessage ); + } + } + } + } diff --git a/src/ResourceLoaderBootstrapModule.php b/src/ResourceLoaderBootstrapModule.php index a73741d..ab2ada5 100644 --- a/src/ResourceLoaderBootstrapModule.php +++ b/src/ResourceLoaderBootstrapModule.php @@ -3,7 +3,6 @@ namespace Bootstrap; use Less_Parser; -use ResourceLoader; use ResourceLoaderContext; use ResourceLoaderFileModule; use BagOStuff; @@ -57,6 +56,7 @@ protected $variables = array(); protected $paths = array(); protected $extStyles = array(); + protected $cacheTriggers = array(); protected $styleText = null; @@ -65,17 +65,7 @@ parent::__construct( $options, $localBasePath, $remoteBasePath ); - if ( isset( $options[ 'variables' ] ) ) { - $this->variables = $options[ 'variables' ]; - } - - if ( isset( $options[ 'paths' ] ) ) { - $this->paths = $options[ 'paths' ]; - } - - if ( isset( $options[ 'external styles' ] ) ) { - $this->extStyles = $options[ 'external styles' ]; - } + $this->applyOptions( $options ); } /** @@ -137,14 +127,13 @@ if ( is_array( $cacheResult ) ) { - if ( $cacheResult[ 'storetime' ] >= $this->getLocalSettingsModificationTime() ) { - - $this->styleText = $cacheResult[ 'styles' ]; - - wfDebug( __METHOD__ . " ext.bootstrap: Cache hit: Got styles from cache.\n" ); + if ( $this->isCacheOutdated( $cacheResult[ 'storetime' ] ) ) { + wfDebug( __METHOD__ . " ext.bootstrap: Cache miss: Cache outdated.\n" ); } else { - wfDebug( __METHOD__ . " ext.bootstrap: Cache miss: Cache outdated, LocalSettings have changed.\n" ); + $this->styleText = $cacheResult[ 'styles' ]; + wfDebug( __METHOD__ . " ext.bootstrap: Cache hit: Got styles from cache.\n" ); } + } else { wfDebug( __METHOD__ . " ext.bootstrap: Cache miss: Styles not found in cache.\n" ); } @@ -160,10 +149,6 @@ protected function purgeCache( ResourceLoaderContext $context ) { $this->getCache()->delete( $this->getCacheKey( $context ) ); - } - - protected function getLocalSettingsModificationTime() { - return filemtime( $GLOBALS[ 'IP' ] . '/LocalSettings.php' ); } protected function compileStyles( ResourceLoaderContext $context ) { @@ -196,4 +181,38 @@ } + /** + * @param mixed[] $options + */ + protected function applyOptions( $options ) { + $mapConfigToLocalVar = array ( + 'variables' => 'variables', + 'paths' => 'paths', + 'external styles' => 'extStyles', + 'cachetriggers' => 'cacheTriggers', + ); + + foreach ( $mapConfigToLocalVar as $config => $local ) { + if ( isset( $options[ $config ] ) ) { + $this->$local = $options[ $config ]; + } + } + } + + /** + * @param int $cacheStoreTime + * + * @return bool + */ + protected function isCacheOutdated( $cacheStoreTime ) { + + foreach ( $this->cacheTriggers as $triggerFile ) { + if ( $triggerFile !== null && $cacheStoreTime < filemtime( $triggerFile ) ) { + return true; + } + } + + return false; + } + } diff --git a/tests/phpunit/BootstrapManagerTest.php b/tests/phpunit/BootstrapManagerTest.php index 810b522..5da6070 100644 --- a/tests/phpunit/BootstrapManagerTest.php +++ b/tests/phpunit/BootstrapManagerTest.php @@ -130,6 +130,32 @@ ); } + public function testAddCacheTriggerFiles() { + + $moduleDefinition = $this->getMockBuilder( '\Bootstrap\Definition\ModuleDefinition' ) + ->disableOriginalConstructor() + ->setMethods( array( 'get' ) ) + ->getMock(); + + $moduleDefinition->expects( $this->atLeastOnce() ) + ->method( 'get' ) + ->will( $this->returnValue( array() ) ); + + $instance = new BootstrapManager( $moduleDefinition ); + $instance-> addCacheTriggerFile( array( 'foo' => 'bar') ); + $instance-> addCacheTriggerFile( 'ichi' ); + + $triggers = $this->getGlobalResourceModuleBootstrapCacheTriggers(); + + $this->assertTrue( + isset( $triggers[ 'foo' ] ) && $triggers[ 'foo' ] === 'bar' + ); + + $this->assertTrue( + array_search( 'ichi', $triggers ) !== false + ); + } + public function testAddExternalModule() { $moduleDefinition = $this->getMockBuilder( '\Bootstrap\Definition\ModuleDefinition' ) @@ -162,4 +188,8 @@ return $GLOBALS['wgResourceModules'][ 'ext.bootstrap.styles' ]['external styles']; } + private function getGlobalResourceModuleBootstrapCacheTriggers() { + return $GLOBALS['wgResourceModules'][ 'ext.bootstrap.styles' ]['cachetriggers']; + } + } diff --git a/tests/phpunit/Hooks/SetupAfterCacheTest.php b/tests/phpunit/Hooks/SetupAfterCacheTest.php index f3549f5..76125c2 100644 --- a/tests/phpunit/Hooks/SetupAfterCacheTest.php +++ b/tests/phpunit/Hooks/SetupAfterCacheTest.php @@ -41,7 +41,8 @@ $configuration = array( 'localBasePath' => $this->localBootstrapVendorPath, - 'remoteBasePath' => '' + 'remoteBasePath' => '', + 'IP' => 'someIP', ); $instance = new SetupAfterCache( $configuration ); @@ -49,11 +50,37 @@ $this->assertTrue( $instance->process() ); } + public function testProcess_setsDefaultCacheTriggers() { + + $configuration = array( + 'localBasePath' => $this->localBootstrapVendorPath, + 'remoteBasePath' => '', + 'IP' => 'someIP', + ); + + $this->resetGlobals(); + + $instance = new SetupAfterCache( $configuration ); + + $this->assertTrue( $instance->process() ); + + $this->assertEquals( + 'someIP/LocalSettings.php', + $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.styles' ][ 'cachetriggers' ][ 'LocalSettings.php' ] + ); + + $this->assertEquals( + 'someIP/composer.lock', + $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.styles' ][ 'cachetriggers' ][ 'composer.lock' ] + ); + } + public function testProcessWithAccessibilityOnAddedLocalResourcePaths() { $configuration = array( 'localBasePath' => $this->localBootstrapVendorPath, - 'remoteBasePath' => '' + 'remoteBasePath' => '', + 'IP' => 'someIP', ); $instance = new SetupAfterCache( $configuration ); @@ -83,7 +110,8 @@ $configuration = array( 'localBasePath' => 'Foo', - 'remoteBasePath' => '' + 'remoteBasePath' => '', + 'IP' => 'someIP', ); $instance = new SetupAfterCache( $configuration ); @@ -119,4 +147,17 @@ $this->assertTrue( is_readable( $path ) ); } + private function resetGlobals() { + $GLOBALS[ 'wgResourceModules' ][ 'ext.bootstrap.styles' ] = array ( + 'class' => 'Bootstrap\ResourceLoaderBootstrapModule', + 'styles' => array (), + 'variables' => array (), + 'dependencies' => array (), + 'cachetriggers' => array ( + 'LocalSettings.php' => null, + 'composer.lock' => null, + ), + ); + } + } diff --git a/tests/phpunit/ResourceLoaderBootstrapModuleTest.php b/tests/phpunit/ResourceLoaderBootstrapModuleTest.php index 42d7d35..542a934 100644 --- a/tests/phpunit/ResourceLoaderBootstrapModuleTest.php +++ b/tests/phpunit/ResourceLoaderBootstrapModuleTest.php @@ -84,4 +84,8 @@ $this->assertContains( 'LESS compile error', $result['all'] ); } + public function testSupportsURLLoading() { + $instance = new ResourceLoaderBootstrapModule(); + $this->assertFalse( $instance->supportsURLLoading() ); + } } -- To view, visit https://gerrit.wikimedia.org/r/174954 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I32a198f1b6f34d8fa93c4e650cd2f3f6f781c5de Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Bootstrap Gerrit-Branch: master Gerrit-Owner: Foxtrott <s7ep...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits