refs #6417 trying to get rid of the unnecessary first archive insert

refs #6417 this should fix some tests. Start initial value by 0 so first generated id will be 1

fix omnifixture creation was broken

refs #6417 wondering if omnifixture creation worked

refs #6417 acquire a lock per archive id which should be better

refs #6417 we have to generate the SQL in the update itself otherwise the update would fail at some point in the future if the tables signature changes

refs #6417 split updates into a separate update file otherwise it would not be executed for devs using Piwik from git

refs #6417 added some documentation

refs #6417 Piwik 2.9.0-b2 was released meaning we have to move it to b3

refs #6417 Piwik 2.9.0-b3 was released meaning we have to move it to b4

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()

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";
-    }
-
 }

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();

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;

core/Updates/2.9.0-b4.php

+<?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');
+    }
+
+}

core/Version.php

@@ -12,7 +12,6 @@ namespace Piwik;
 /**
  * Piwik version information.
  *
- *
  * @api
  */
 final class Version

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;

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);
     }

tests/PHPUnit/Integration/SequenceTest.php

+<?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