Seb35 has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/377055 )
Change subject: Improved reading of config files
......................................................................
Improved reading of config files
1. The cached files are always prefered over the origin files, even for the
files in .php or .ser format.
2. An additional safeguard is in place for PHP files in case of parsing
errors; this will only work for PHP 7+, but is compatible with PHP 5.2.
3. Avoid edge cases when the origin file and the cached file have the same
timestamp: possibly the cached file has an old version and there is no
mean to know this fact (I guess I/O are sometimes written in batch with
the same timestamp, explaining this fact).
Change-Id: Ia76d2ad46f9f3124da8e681c73de275d0da579ef
---
M .gitignore
M src/Utils.php
M tests/phpunit/InstallationIndependantTest.php
M tests/phpunit/MediaWikiFarmTestCase.php
M tests/phpunit/MultiversionInstallationTest.php
5 files changed, 39 insertions(+), 13 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MediaWikiFarm
refs/changes/55/377055/1
diff --git a/.gitignore b/.gitignore
index 07aae69..050dc52 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,6 +14,7 @@
# Tests
/phpunitHTTP404.php
/tests/perfs/results
+/tests/phpunit/data/config/badsyntax.php
/tests/phpunit/data/config/badsyntax.json
/tests/phpunit/data/config/deployments.php
/tests/phpunit/data/config/empty.json
diff --git a/src/Utils.php b/src/Utils.php
index 59125c8..46a1ea9 100644
--- a/src/Utils.php
+++ b/src/Utils.php
@@ -46,10 +46,20 @@
$format = null;
}
- # Format PHP
- if( $format == '.php' ) {
+ # Cached version - avoid the case where the timestamps are the
same, the two files could have non-coherent versions
+ if( $cachedFile && is_string( $format ) && is_file( $cachedFile
) && filemtime( $cachedFile ) > filemtime( $prefixedFile ) ) {
- $array = include $prefixedFile;
+ return self::readFile( $filename . '.php', $cacheDir,
$log, $cacheDir . '/config', false );
+ }
+
+ # Format PHP
+ elseif( $format == '.php' ) {
+
+ try {
+ $array = @include $prefixedFile;
+ } catch( Throwable $e ) {
+ $array = null;
+ }
}
# Format 'serialisation'
@@ -63,12 +73,6 @@
else {
$array = unserialize( $content );
}
- }
-
- # Cached version
- elseif( $cachedFile && is_string( $format ) && is_file(
$cachedFile ) && filemtime( $cachedFile ) >= filemtime( $prefixedFile ) ) {
-
- return self::readFile( $filename . '.php', $cacheDir,
$log, $cacheDir . '/config', false );
}
# Format YAML
@@ -136,7 +140,8 @@
# Regular return for arrays
if( is_array( $array ) ) {
- if( $cachedFile && $directory != $cacheDir . '/config'
&& ( !is_file( $cachedFile ) || ( filemtime( $cachedFile ) < filemtime(
$prefixedFile ) ) ) ) {
+ # Cache this version - avoid the case where the
timestamps are the same, the two files could have non-coherent versions
+ if( $cachedFile && $directory != $cacheDir . '/config'
&& ( !is_file( $cachedFile ) || ( filemtime( $cachedFile ) <= filemtime(
$prefixedFile ) ) ) ) {
self::cacheFile( $array, $filename . '.php',
$cacheDir . '/config' );
}
diff --git a/tests/phpunit/InstallationIndependantTest.php
b/tests/phpunit/InstallationIndependantTest.php
index 250fd61..ea1b88a 100644
--- a/tests/phpunit/InstallationIndependantTest.php
+++ b/tests/phpunit/InstallationIndependantTest.php
@@ -242,6 +242,21 @@
}
/**
+ * Test reading a badly-formatted PHP file.
+ *
+ * @requires PHP 7.0
+ * @covers MediaWikiFarm::readFile
+ * @covers MediaWikiFarmUtils::readFile
+ * @uses MediaWikiFarm::__construct
+ * @uses MediaWikiFarm::selectFarm
+ */
+ function testBadSyntaxFileReadingPHP() {
+
+ $result = $this->farm->readFile( 'badsyntax.php',
self::$wgMediaWikiFarmConfigDir );
+ $this->assertFalse( $result );
+ }
+
+ /**
* Test reading a badly-formatted YAML file.
*
* @covers MediaWikiFarm::readFile
@@ -342,7 +357,9 @@
$farm = new MediaWikiFarm(
'a.testfarm-monoversion.example.org', null, self::$wgMediaWikiFarmConfigDir,
null, self::$wgMediaWikiFarmCacheDir );
+ # Simulate an old origin file, so that the cached version (with
current time) will be more recent as it should be
copy( self::$wgMediaWikiFarmConfigDir . '/testreading.json',
self::$wgMediaWikiFarmConfigDir . '/testreading2.json' );
+ touch( self::$wgMediaWikiFarmConfigDir . '/testreading2.json',
time() - 300 );
# Read original file and check the cached version is written
$farm->readFile( 'testreading2.json',
self::$wgMediaWikiFarmConfigDir );
@@ -360,7 +377,7 @@
# Put the original file in badsyntax and check it fallbacks to
cached version
# Touch the file to simulate a later edit by the user
copy( self::$wgMediaWikiFarmConfigDir . '/badsyntax.json',
self::$wgMediaWikiFarmConfigDir . '/testreading2.json' );
- touch( self::$wgMediaWikiFarmConfigDir . '/testreading2.json',
time() + 10 );
+ touch( self::$wgMediaWikiFarmConfigDir . '/testreading2.json',
time() + 300 );
$result = $farm->readFile( 'testreading2.json',
self::$wgMediaWikiFarmConfigDir );
$this->assertEquals(
array(
diff --git a/tests/phpunit/MediaWikiFarmTestCase.php
b/tests/phpunit/MediaWikiFarmTestCase.php
index 4d71857..6b21161 100644
--- a/tests/phpunit/MediaWikiFarmTestCase.php
+++ b/tests/phpunit/MediaWikiFarmTestCase.php
@@ -123,6 +123,7 @@
copy( self::$wgMediaWikiFarmConfigDir . '/http404.php',
'phpunitHTTP404.php' );
# Dynamically create these files to avoid CI error reports
+ file_put_contents( self::$wgMediaWikiFarmConfigDir .
'/badsyntax.php', "<?php\nreturn array()\n" );
file_put_contents( self::$wgMediaWikiFarmConfigDir .
'/badsyntax.json', "{\n\t\"element1\",\n}\n" );
file_put_contents( self::$wgMediaWikiFarmConfigDir .
'/empty.json', "null\n" );
}
@@ -150,6 +151,9 @@
if( is_file( 'phpunitHTTP404.php' ) ) {
unlink( 'phpunitHTTP404.php' );
}
+ if( is_file( self::$wgMediaWikiFarmConfigDir . '/badsyntax.php'
) ) {
+ unlink( self::$wgMediaWikiFarmConfigDir .
'/badsyntax.php' );
+ }
if( is_file( self::$wgMediaWikiFarmConfigDir .
'/badsyntax.json' ) ) {
unlink( self::$wgMediaWikiFarmConfigDir .
'/badsyntax.json' );
}
diff --git a/tests/phpunit/MultiversionInstallationTest.php
b/tests/phpunit/MultiversionInstallationTest.php
index a462936..4a0186e 100644
--- a/tests/phpunit/MultiversionInstallationTest.php
+++ b/tests/phpunit/MultiversionInstallationTest.php
@@ -530,8 +530,7 @@
$this->assertTrue( is_file( self::$wgMediaWikiFarmCacheDir .
'/LocalSettings/a.testfarm-multiversion.example.org.php' ) );
# Invalidate the existence cache
- sleep( 2 );
- $this->assertTrue( touch( self::$wgMediaWikiFarmConfigDir .
'/farms.php' ) );
+ $this->assertTrue( touch( self::$wgMediaWikiFarmConfigDir .
'/farms.php', time() + 300 ) );
# Check the existence cache is understood as invalidated
$farm = new MediaWikiFarm(
'a.testfarm-multiversion.example.org', null,
--
To view, visit https://gerrit.wikimedia.org/r/377055
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia76d2ad46f9f3124da8e681c73de275d0da579ef
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MediaWikiFarm
Gerrit-Branch: master
Gerrit-Owner: Seb35 <se...@seb35.fr>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
