diff --git a/core/DataAccess/ArchiveWriter.php b/core/DataAccess/ArchiveWriter.php index 1ea6b0d08c26a5622eaa4a29f62f75f28de7441c..7e0086d79584055e79a12b6f070746ca3e156a12 100644 --- a/core/DataAccess/ArchiveWriter.php +++ b/core/DataAccess/ArchiveWriter.php @@ -11,7 +11,6 @@ namespace Piwik\DataAccess; use Exception; use Piwik\ArchiveProcessor\Rules; use Piwik\ArchiveProcessor; -use Piwik\Common; use Piwik\Db; use Piwik\Db\BatchInsert; use Piwik\Period; @@ -138,29 +137,11 @@ class ArchiveWriter } protected function allocateNewArchiveId() - { - $this->idArchive = $this->insertNewArchiveId(); - return $this->idArchive; - } - - /** - * Locks the archive table to generate a new archive ID. - * - * We lock to make sure that - * if several archiving processes are running at the same time (for different websites and/or periods) - * then they will each use a unique archive ID. - * - * @return int - */ - protected function insertNewArchiveId() { $numericTable = $this->getTableNumeric(); - $idSite = $this->idSite; - $date = date("Y-m-d H:i:s"); - $id = $this->getModel()->insertNewArchiveId($numericTable, $idSite, $date); - - return $id; + $this->idArchive = $this->getModel()->allocateNewArchiveId($numericTable); + return $this->idArchive; } private function getModel() diff --git a/core/DataAccess/Model.php b/core/DataAccess/Model.php index 6c0d1761417242a8e68dbb48f59f9b3e1e686a81..e49eb02502ab8ab545bf32be4d91792302bf2af1 100644 --- a/core/DataAccess/Model.php +++ b/core/DataAccess/Model.php @@ -184,53 +184,35 @@ class Model } try { - $sequence = new Sequence($tableName); - $sequence->create(); + if (ArchiveTableCreator::NUMERIC_TABLE === ArchiveTableCreator::getTypeFromTableName($tableName)) { + $sequence = new Sequence($tableName); + $sequence->create(); + } } catch (Exception $e) { } } - /** - * Locks the archive table to generate a new archive ID. - * - * We lock to make sure that - * if several archiving processes are running at the same time (for different websites and/or periods) - * then they will each use a unique archive ID. - * - * @return int - */ - public function insertNewArchiveId($numericTable, $idSite, $date) + public function allocateNewArchiveId($numericTable) { $sequence = new Sequence($numericTable); $idarchive = $sequence->getNextId(); - $insertSql = "INSERT INTO $numericTable " - . " SELECT IFNULL( MAX(idarchive), 0 ) + 1, - '" . (int)$idarchive . "', - " . (int)$idSite . ", - '" . $date . "', - '" . $date . "', - 0, - '" . $date . "', - 0 " - . " FROM $numericTable as tb1"; - Db::get()->exec($insertSql); - return $idarchive; } public function deletePreviousArchiveStatus($numericTable, $archiveId, $doneFlag) { + $dbLockName = "deletePreviousArchiveStatus.$numericTable.$archiveId"; + // without advisory lock here, the DELETE would acquire Exclusive Lock - $this->acquireArchiveTableLock($numericTable); + $this->acquireArchiveTableLock($dbLockName); - Db::query("DELETE FROM $numericTable WHERE idarchive = ? AND (name = '" . $doneFlag - . "' OR name LIKE '" . self::PREFIX_SQL_LOCK . "%')", + Db::query("DELETE FROM $numericTable WHERE idarchive = ? AND (name = '" . $doneFlag . "')", array($archiveId) ); - $this->releaseArchiveTableLock($numericTable); + $this->releaseArchiveTableLock($dbLockName); } public function insertRecord($tableName, $fields, $record, $name, $value) @@ -260,24 +242,16 @@ class Model return "((name IN ($allDoneFlags)) AND (value IN (" . implode(',', $possibleValues) . ")))"; } - protected function acquireArchiveTableLock($numericTable) + protected function acquireArchiveTableLock($dbLockName) { - $dbLockName = $this->getArchiveLockName($numericTable); - if (Db::getDbLock($dbLockName, $maxRetries = 30) === false) { - throw new Exception("allocateNewArchiveId: Cannot get named lock $dbLockName."); + throw new Exception("Cannot get named lock $dbLockName."); } } - protected function releaseArchiveTableLock($numericTable) + protected function releaseArchiveTableLock($dbLockName) { - $dbLockName = $this->getArchiveLockName($numericTable); Db::releaseDbLock($dbLockName); } - protected function getArchiveLockName($numericTable) - { - return "allocateNewArchiveId.$numericTable"; - } - } diff --git a/core/Sequence.php b/core/Sequence.php index 68a5f30f22e118e3f1a9758a19ed269a5994d6b6..655332dcedeb522240803bbbdadfdc3e14d066a3 100644 --- a/core/Sequence.php +++ b/core/Sequence.php @@ -10,10 +10,24 @@ namespace Piwik; use Exception; +/** + * Used for generating auto increment ids. + * + * Example: + * + * $sequence = new Sequence('my_sequence_name'); + * $id = $sequence->getNextId(); + * $db->insert('anytable', array('id' => $id, '...' => '...')); + */ class Sequence { private $name; + /** + * The name of the table or sequence you want to get an id for. + * + * @param string $name eg 'archive_numeric_2014_11' + */ public function __construct($name) { $this->name = $name; @@ -24,21 +38,18 @@ class Sequence return Common::prefixTable('sequence'); } - public function getCurrentId() + /** + * Creates / initializes a new sequence. + * + * @param int $initialValue + * @return int The actually used value to initialize the table. + * + * @throws \Exception in case a sequence having this name already exists. + */ + public function create($initialValue = 0) { - $table = $this->getTableName(); - $sql = 'SELECT value FROM ' . $table . ' WHERE name = ? LIMIT 1'; - - $db = Db::get(); - $id = $db->fetchOne($sql, array($this->name)); + $initialValue = (int) $initialValue; - if (!empty($id)) { - return (int) $id; - } - } - - public function create($initialValue = 1) - { $table = $this->getTableName(); $db = $this->getDb(); @@ -47,20 +58,19 @@ class Sequence return $initialValue; } - public function getQueryToCreateSequence($initialValue) - { - $table = $this->getTableName(); - $query = sprintf("INSERT INTO %s (name, value) VALUES ('%s', %d)", $table, $this->name, $initialValue); - - return $query; - } - - public function getNextId() + /** + * Get / allocate / reserve a new id for the current sequence. Important: Getting the next id will fail in case + * no such sequence exists. Make sure to create one if needed, see {@link create()}. + * + * @return int + * @throws Exception + */ + public function getNextId() { $table = $this->getTableName(); $sql = 'UPDATE ' . $table . ' SET value = LAST_INSERT_ID(value + 1) WHERE name = ?'; - $db = Db::get(); + $db = $this->getDb(); $result = $db->query($sql, array($this->name)); $rowCount = $result->rowCount(); @@ -73,6 +83,24 @@ class Sequence return (int) $createdId; } + /** + * Returns the current max id. + * @return int + * @internal + */ + public function getCurrentId() + { + $table = $this->getTableName(); + $sql = 'SELECT value FROM ' . $table . ' WHERE name = ?'; + + $db = $this->getDb(); + $id = $db->fetchOne($sql, array($this->name)); + + if (!empty($id) || '0' === $id || 0 === $id) { + return (int) $id; + } + } + private function getDb() { return Db::get(); diff --git a/core/Updates/2.9.0-b1.php b/core/Updates/2.9.0-b1.php index f14f130b002e045360e17e90684abe16aebd58de..5720bf076e814733e9bd6d3dda1adc3dc9472635 100644 --- a/core/Updates/2.9.0-b1.php +++ b/core/Updates/2.9.0-b1.php @@ -9,85 +9,17 @@ namespace Piwik\Updates; use Piwik\Common; -use Piwik\DataAccess\ArchiveTableCreator; use Piwik\Db; use Piwik\Option; use Piwik\Plugin\Manager; -use Piwik\Sequence; use Piwik\Updater; use Piwik\Updates; class Updates_2_9_0_b1 extends Updates { static function getSql() - { - $sql = self::getSqlsForMigratingUserSettingsToDevicesDetection(); - $sql = self::addQueryToCreateSequenceTable($sql); - $sql = self::addArchivingIdMigrationQueries($sql); - - return $sql; - } - - static function update() - { - Updater::updateDatabase(__FILE__, self::getSql()); - - try { - Manager::getInstance()->activatePlugin('TestRunner'); - } catch (\Exception $e) { - - } - } - - private static function addArchivingIdMigrationQueries($sql) - { - $tables = ArchiveTableCreator::getTablesArchivesInstalled(); - - foreach ($tables as $table) { - $type = ArchiveTableCreator::getTypeFromTableName($table); - - if ($type === ArchiveTableCreator::NUMERIC_TABLE) { - $maxId = Db::fetchOne('SELECT MAX(idarchive) FROM ' . $table); - - if (!empty($maxId)) { - $maxId = (int) $maxId + 500; - } else { - $maxId = 1; - } - - $sequence = new Sequence($table); - $query = $sequence->getQueryToCreateSequence($maxId); - $sql[$query] = false; - } - } - - return $sql; - } - - /** - * @return string - */ - private static function addQueryToCreateSequenceTable($sql) - { - $dbSettings = new Db\Settings(); - $engine = $dbSettings->getEngine(); - $table = Common::prefixTable('sequence'); - - $query = "CREATE TABLE `$table` ( - `name` VARCHAR(120) NOT NULL, - `value` BIGINT(20) UNSIGNED NOT NULL, - PRIMARY KEY(`name`) - ) ENGINE=$engine DEFAULT CHARSET=utf8"; - - $sql[$query] = 1050; - - return $sql; - } - - private static function getSqlsForMigratingUserSettingsToDevicesDetection() { $sql = array(); - $sql = self::updateBrowserEngine($sql); return $sql; diff --git a/core/Updates/2.9.0-b4.php b/core/Updates/2.9.0-b4.php new file mode 100644 index 0000000000000000000000000000000000000000..8aab257694d42697f335294bafb36cedb838698f --- /dev/null +++ b/core/Updates/2.9.0-b4.php @@ -0,0 +1,90 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Updates; + +use Piwik\Common; +use Piwik\DataAccess\ArchiveTableCreator; +use Piwik\Db; +use Piwik\Updater; +use Piwik\Updates; + +class Updates_2_9_0_b4 extends Updates +{ + static function getSql() + { + $sql = array(); + $sql = self::addCreateSequenceTableQuery($sql); + $sql = self::addArchivingIdMigrationQueries($sql); + + return $sql; + } + + static function update() + { + Updater::updateDatabase(__FILE__, self::getSql()); + } + + private static function addArchivingIdMigrationQueries($sql) + { + $tables = ArchiveTableCreator::getTablesArchivesInstalled(); + + foreach ($tables as $table) { + $type = ArchiveTableCreator::getTypeFromTableName($table); + + if ($type === ArchiveTableCreator::NUMERIC_TABLE) { + $maxId = Db::fetchOne('SELECT MAX(idarchive) FROM ' . $table); + + if (!empty($maxId)) { + $maxId = (int) $maxId + 500; + } else { + $maxId = 1; + } + + $query = self::getQueryToCreateSequence($table, $maxId); + $sql[$query] = false; + } + } + + return $sql; + } + + private static function getQueryToCreateSequence($name, $initialValue) + { + $table = self::getSequenceTableName(); + $query = sprintf("INSERT INTO %s (name, value) VALUES ('%s', %d)", $table, $name, $initialValue); + + return $query; + } + + /** + * @return string + */ + private static function addCreateSequenceTableQuery($sql) + { + $dbSettings = new Db\Settings(); + $engine = $dbSettings->getEngine(); + $table = self::getSequenceTableName(); + + $query = "CREATE TABLE `$table` ( + `name` VARCHAR(120) NOT NULL, + `value` BIGINT(20) UNSIGNED NOT NULL, + PRIMARY KEY(`name`) + ) ENGINE=$engine DEFAULT CHARSET=utf8"; + + $sql[$query] = 1050; + + return $sql; + } + + private static function getSequenceTableName() + { + return Common::prefixTable('sequence'); + } + +} diff --git a/core/Version.php b/core/Version.php index 789d7d29d3b590949cd5e4b73c37eb8311fb92e1..fc28e761691b6042f4a8073425345a88e40f5012 100644 --- a/core/Version.php +++ b/core/Version.php @@ -12,7 +12,6 @@ namespace Piwik; /** * Piwik version information. * - * * @api */ final class Version diff --git a/plugins/TestRunner/Commands/TestsSetupFixture.php b/plugins/TestRunner/Commands/TestsSetupFixture.php index 8c053b17117fa1ce1a37dba125e28b033c4dab6e..cda1a5c585072373e3408badb6815d98c4b3b071 100644 --- a/plugins/TestRunner/Commands/TestsSetupFixture.php +++ b/plugins/TestRunner/Commands/TestsSetupFixture.php @@ -237,10 +237,12 @@ class TestsSetupFixture extends ConsoleCommand require_once PIWIK_INCLUDE_PATH . '/tests/PHPUnit/IntegrationTestCase.php'; $fixturesToLoad = array( + '/tests/PHPUnit/Fixtures/*.php', '/tests/PHPUnit/UI/Fixtures/*.php', '/plugins/*/tests/Fixtures/*.php', '/plugins/*/Test/Fixtures/*.php', ); + foreach($fixturesToLoad as $fixturePath) { foreach (glob(PIWIK_INCLUDE_PATH . $fixturePath) as $file) { require_once $file; diff --git a/tests/PHPUnit/Integration/DataAccess/ModelTest.php b/tests/PHPUnit/Integration/DataAccess/ModelTest.php index 62ccaeaade9cbc6baa56583e5f7790b3fe00384f..270253b82a727334a20135fe02cefd72eaa6c000 100644 --- a/tests/PHPUnit/Integration/DataAccess/ModelTest.php +++ b/tests/PHPUnit/Integration/DataAccess/ModelTest.php @@ -31,17 +31,17 @@ class Core_DataAccess_ModelTest extends IntegrationTestCase public function test_insertNewArchiveId() { - $this->assertCreatedArchiveId(1); - $this->assertCreatedArchiveId(2); - $this->assertCreatedArchiveId(3); - $this->assertCreatedArchiveId(4); - $this->assertCreatedArchiveId(5, 2); - $this->assertCreatedArchiveId(6, 2); + $this->assertAllocatedArchiveId(1); + $this->assertAllocatedArchiveId(2); + $this->assertAllocatedArchiveId(3); + $this->assertAllocatedArchiveId(4); + $this->assertAllocatedArchiveId(5); + $this->assertAllocatedArchiveId(6); } - private function assertCreatedArchiveId($expectedId, $siteId = 1) + private function assertAllocatedArchiveId($expectedId) { - $id = $this->model->insertNewArchiveId($this->tableName, $siteId, '2014-01-01 00:01:02'); + $id = $this->model->allocateNewArchiveId($this->tableName); $this->assertEquals($expectedId, $id); } diff --git a/tests/PHPUnit/Integration/SequenceTest.php b/tests/PHPUnit/Integration/SequenceTest.php new file mode 100644 index 0000000000000000000000000000000000000000..f42617cf0691006cf279213dd78e8162f60246df --- /dev/null +++ b/tests/PHPUnit/Integration/SequenceTest.php @@ -0,0 +1,112 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ +use Piwik\Db; +use Piwik\Sequence; +use Piwik\Tests\Framework\TestCase\IntegrationTestCase; + +/** + * Class Core_SequenceTest + * + * @group Core + * @group Sequence + */ +class Core_SequenceTest extends IntegrationTestCase +{ + /** + * @var Sequence + */ + private $sequence; + + public function setUp() + { + parent::setUp(); + + $this->sequence = new Sequence('mySequence0815'); + $this->sequence->create(); + } + + public function test_create_shouldAddNewSequenceWithInitalId1() + { + $sequence = $this->getEmptySequence(); + + $id = $sequence->create(); + $this->assertSame(0, $id); + + // verify + $id = $sequence->getCurrentId(); + $this->assertSame(0, $id); + } + + public function test_create_WithCustomInitialValue() + { + $sequence = $this->getEmptySequence(); + + $id = $sequence->create(11); + $this->assertSame(11, $id); + + // verify + $id = $sequence->getCurrentId(); + $this->assertSame(11, $id); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage Duplicate entry + */ + public function test_create_shouldFailIfSequenceAlreadyExists() + { + $this->sequence->create(); + } + + public function test_getNextId_shouldGenerateNextId() + { + $this->assertNextIdGenerated(1); + $this->assertNextIdGenerated(2); + $this->assertNextIdGenerated(3); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage Sequence 'notCreatedSequence' not found + */ + public function test_getNextId_shouldFailIfThereIsNoSequenceHavingThisName() + { + $sequence = $this->getEmptySequence(); + $sequence->getNextId(); + } + + private function assertNextIdGenerated($expectedId) + { + $id = $this->sequence->getNextId(); + $this->assertSame($expectedId, $id); + + // verify + $id = $this->sequence->getCurrentId(); + $this->assertSame($expectedId, $id); + } + + public function test_getCurrentId_shouldReturnTheCurrentIdAsInt() + { + $id = $this->sequence->getCurrentId(); + $this->assertSame(0, $id); + } + + public function test_getCurrentId_shouldReturnNullIfSequenceDoesNotExist() + { + $sequence = $this->getEmptySequence(); + $id = $sequence->getCurrentId(); + $this->assertNull($id); + } + + private function getEmptySequence() + { + return new Sequence('notCreatedSequence'); + } + + +} \ No newline at end of file diff --git a/tests/resources/OmniFixture-dump.sql.gz b/tests/resources/OmniFixture-dump.sql.gz index f1f56e6d9bec5a341fc674cf2b7a096c9091d4c5..a7151a5d6bb250084b352be3017c5fa57f01e5e5 100644 Binary files a/tests/resources/OmniFixture-dump.sql.gz and b/tests/resources/OmniFixture-dump.sql.gz differ