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

Reply via email to