diff options
| -rwxr-xr-x | plugins/UserCountry/GeoIPAutoUpdater.php | 26 | ||||
| -rw-r--r-- | tests/PHPUnit/Plugins/UserCountryTest.php | 70 |
2 files changed, 81 insertions, 15 deletions
diff --git a/plugins/UserCountry/GeoIPAutoUpdater.php b/plugins/UserCountry/GeoIPAutoUpdater.php index 562a47d315..912425e998 100755 --- a/plugins/UserCountry/GeoIPAutoUpdater.php +++ b/plugins/UserCountry/GeoIPAutoUpdater.php @@ -501,9 +501,11 @@ class Piwik_UserCountry_GeoIPAutoUpdater * This is a safety measure used to make sure tracking isn't affected if strange * update errors occur. * - * Databases are renamed to ${original}.broken.N where 'N' is a number. + * Databases are renamed to ${original}.broken . + * + * Note: method is protected for testability. */ - private function performRedundantDbChecks() + protected function performRedundantDbChecks() { $databaseTypes = array_keys(Piwik_UserCountry_LocationProvider_GeoIp::$dbNames); @@ -534,6 +536,11 @@ class Piwik_UserCountry_GeoIPAutoUpdater if ($oldPath !== false && $newPath !== false) { + if (file_exists($newPath)) + { + unlink($newPath); + } + rename($oldPath, $newPath); } } @@ -546,7 +553,7 @@ class Piwik_UserCountry_GeoIPAutoUpdater * @param array $possibleDbNames The possible names of the database. * @return array Array with two elements, the path to the existing database, and * the path to rename it to if it is broken. The second will end - * with something like .broken.N where N is a number. + * with something like .broken . */ private function getOldAndNewPathsForBrokenDb( $possibleDbNames ) { @@ -555,18 +562,7 @@ class Piwik_UserCountry_GeoIPAutoUpdater if ($pathToDb !== false) { - // 250 used as upper limit just for sanity (updating should not fail that much) - $brokenPath = false; - for ($i = 0; $i != 250; ++$i) - { - $brokenPath = $pathToDb.".broken.$i"; - if (!file_exists($brokenPath)) - { - break; - } - } - - $newPath = $brokenPath; + $newPath = $pathToDb.".broken"; } return array($pathToDb, $newPath); diff --git a/tests/PHPUnit/Plugins/UserCountryTest.php b/tests/PHPUnit/Plugins/UserCountryTest.php index 0dea828590..68f2732339 100644 --- a/tests/PHPUnit/Plugins/UserCountryTest.php +++ b/tests/PHPUnit/Plugins/UserCountryTest.php @@ -7,6 +7,7 @@ */ require_once PIWIK_INCLUDE_PATH.'/plugins/UserCountry/UserCountry.php'; require_once 'UserCountry/functions.php'; +require_once PIWIK_INCLUDE_PATH.'/core/DataFiles/Countries.php'; class Test_Piwik_UserCountry extends PHPUnit_Framework_Testcase { @@ -70,5 +71,74 @@ class Test_Piwik_UserCountry extends PHPUnit_Framework_Testcase $this->assertArrayHasKey($country, $countries, $filename); } } + + // test that redundant checks work + public function testGeoIpUpdaterRedundantChecks() + { + Piwik_UserCountry_LocationProvider_GeoIp::$geoIPDatabaseDir = 'tests/lib/geoip-files'; + Piwik_UserCountry_LocationProvider::$providers = null; + + // create empty ISP & Org files + $this->createEmptyISPOrgFiles(); + + // run redundant checks + $updater = new Piwik_UserCountry_GeoIPAutoUpdater_publictestRedundantChecks(); + $updater->performRedundantDbChecks(); + + // check that files are renamed correctly + $this->checkBrokenGeoIPState(); + + // create empty files again & run checks again + $this->createEmptyISPOrgFiles(); + $updater->performRedundantDbChecks(); + + // check that w/ broken files already there, redundant checks still work correctly + $this->checkBrokenGeoIPState(); + } + + public function tearDown() + { + $geoIpDirPath = PIWIK_INCLUDE_PATH.'/tests/lib/geoip-files'; + if (file_exists($geoIpDirPath.'/GeoIPISP.dat.broken')) + { + unlink($geoIpDirPath.'/GeoIPISP.dat.broken'); + } + + if (file_exists($geoIpDirPath.'/GeoIPOrg.dat.broken')) + { + unlink($geoIpDirPath.'/GeoIPOrg.dat.broken'); + } + } + + private function createEmptyISPOrgFiles() + { + $geoIpDir = PIWIK_INCLUDE_PATH.'/tests/lib/geoip-files'; + + $fd = fopen($geoIpDir.'/GeoIPISP.dat', 'w'); + fclose($fd); + + $fd = fopen($geoIpDir.'/GeoIPOrg.dat', 'w'); + fclose($fd); + } + + private function checkBrokenGeoIPState() + { + $geoIpDir = PIWIK_INCLUDE_PATH.'/tests/lib/geoip-files'; + + $this->assertFalse(file_exists($geoIpDir.'/GeoIPCity.dat.broken')); + + $this->assertFalse(file_exists($geoIpDir.'/GeoIPISP.dat')); + $this->assertTrue(file_exists($geoIpDir.'/GeoIPISP.dat.broken')); + + $this->assertFalse(file_exists($geoIpDir.'/GeoIPOrg.dat')); + $this->assertTrue(file_exists($geoIpDir.'/GeoIPOrg.dat.broken')); + } } +class Piwik_UserCountry_GeoIPAutoUpdater_publictestRedundantChecks extends Piwik_UserCountry_GeoIPAutoUpdater +{ + public function performRedundantDbChecks() + { + parent::performRedundantDbChecks(); + } +} |