From 27a4ec6fb76bf17b409685d146d6d3765686edf4 Mon Sep 17 00:00:00 2001 From: Thomas Steur <thomas.steur@gmail.com> Date: Sun, 13 Dec 2015 20:04:25 +0000 Subject: [PATCH] refs #9131 lock name should be less than 64 characters for MySQL compatibility --- core/DataAccess/Model.php | 7 +++++- core/Db.php | 19 ++++++++++++++- tests/PHPUnit/Integration/DbTest.php | 35 ++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/core/DataAccess/Model.php b/core/DataAccess/Model.php index 4213cd2039..a7173f16c8 100644 --- a/core/DataAccess/Model.php +++ b/core/DataAccess/Model.php @@ -281,7 +281,12 @@ class Model public function deletePreviousArchiveStatus($numericTable, $archiveId, $doneFlag) { - $dbLockName = "deletePreviousArchiveStatus.$numericTable.$archiveId"; + $tableWithoutLeadingPrefix = $numericTable; + if (strlen($numericTable) >= 23) { + $tableWithoutLeadingPrefix = substr($numericTable, strlen($numericTable) - 23); + // we need to make sure lock name is less than 64 characters see https://github.com/piwik/piwik/issues/9131 + } + $dbLockName = "rmPrevArchiveStatus.$tableWithoutLeadingPrefix.$archiveId"; // without advisory lock here, the DELETE would acquire Exclusive Lock $this->acquireArchiveTableLock($dbLockName); diff --git a/core/Db.php b/core/Db.php index f7eae8c8b2..387f9c7a4d 100644 --- a/core/Db.php +++ b/core/Db.php @@ -90,6 +90,17 @@ class Db return $dbConfig; } + /** + * For tests only. + * @param $connection + * @ignore + * @internal + */ + public static function setDatabaseObject($connection) + { + self::$connection = $connection; + } + /** * Connects to the database. * @@ -638,9 +649,14 @@ class Db * @param string $lockName The lock name. * @param int $maxRetries The max number of times to retry. * @return bool `true` if the lock was obtained, `false` if otherwise. + * @throws \Exception if Lock name is too long */ public static function getDbLock($lockName, $maxRetries = 30) { + if (strlen($lockName) > 64) { + throw new \Exception('DB lock name has to be 64 characters or less for MySQL 5.7 compatibility.'); + } + /* * the server (e.g., shared hosting) may have a low wait timeout * so instead of a single GET_LOCK() with a 30 second timeout, @@ -652,7 +668,8 @@ class Db $db = self::get(); while ($maxRetries > 0) { - if ($db->fetchOne($sql, array($lockName)) == '1') { + $result = $db->fetchOne($sql, array($lockName)); + if ($result == '1') { return true; } $maxRetries--; diff --git a/tests/PHPUnit/Integration/DbTest.php b/tests/PHPUnit/Integration/DbTest.php index 9a5595e29a..67b6709796 100644 --- a/tests/PHPUnit/Integration/DbTest.php +++ b/tests/PHPUnit/Integration/DbTest.php @@ -56,6 +56,41 @@ class DbTest extends IntegrationTestCase $this->assertSame($expected, $result); } + /** + * @expectedException \Exception + * @expectedExceptionMessagelock name has to be 64 characters or less + */ + public function test_getDbLock_shouldThrowAnException_IfDbLockNameIsTooLong() + { + Db::getDbLock(str_pad('test', 65, '1')); + } + + public function test_getDbLock_shouldGetLock() + { + $db = Db::get(); + $this->assertTrue(Db::getDbLock('MyLock')); + // same session still has lock + $this->assertTrue(Db::getDbLock('MyLock')); + + Db::setDatabaseObject(null); + // different session, should not be able to acquire lock + $this->assertFalse(Db::getDbLock('MyLock', 1)); + // different session cannot release lock + $this->assertFalse(Db::releaseDbLock('MyLock')); + Db::destroyDatabaseObject(); + + // release lock again by using previous session + Db::setDatabaseObject($db); + $this->assertTrue(Db::releaseDbLock('MyLock')); + Db::destroyDatabaseObject(); + } + + public function test_tableExists() + { + $this->assertFalse(Db::tableExists('noTExistIngTable')); + $this->assertTrue(Db::tableExists(Common::prefixTable('log_visit'))); + } + public function getDbAdapter() { return array( -- GitLab