From ec2d7cff2f36fb37ea6348022bbba09430ac3b9f Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 3 Mar 2015 11:14:45 +0100 Subject: [PATCH 1/5] SQLite autoincrement test --- tests/lib/files/cache/cache.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/lib/files/cache/cache.php b/tests/lib/files/cache/cache.php index 1bf838351b..f0ad6cf3ab 100644 --- a/tests/lib/files/cache/cache.php +++ b/tests/lib/files/cache/cache.php @@ -594,6 +594,19 @@ class Cache extends \Test\TestCase { $this->assertEquals($newData, $newDataFromBogus); } + public function testNoReuseOfFileId() { + $data1 = array('size' => 100, 'mtime' => 50, 'mimetype' => 'text/plain'); + $this->cache->put('somefile.txt', $data1); + $info = $this->cache->get('somefile.txt'); + $fileId = $info['fileid']; + $this->cache->remove('somefile.txt'); + $data2 = array('size' => 200, 'mtime' => 100, 'mimetype' => 'text/plain'); + $this->cache->put('anotherfile.txt', $data2); + $info2 = $this->cache->get('anotherfile.txt'); + $fileId2 = $info2['fileid']; + $this->assertNotEquals($fileId, $fileId2); + } + protected function tearDown() { if ($this->cache) { $this->cache->clear(); From 24a30c10d7a8f2e1bd2e5f57c6291d809dfdbb0c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 4 Mar 2015 14:30:07 +0100 Subject: [PATCH 2/5] Add custom sqlite platform to set auto increment --- lib/private/db/connectionfactory.php | 1 + lib/private/db/sqliteplatform.php | 35 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 lib/private/db/sqliteplatform.php diff --git a/lib/private/db/connectionfactory.php b/lib/private/db/connectionfactory.php index 44b598dcda..c488bf82d1 100644 --- a/lib/private/db/connectionfactory.php +++ b/lib/private/db/connectionfactory.php @@ -96,6 +96,7 @@ class ConnectionFactory { break; case 'sqlite3': $journalMode = $additionalConnectionParams['sqlite.journal_mode']; + $additionalConnectionParams['platform'] = new SqlitePlatform(); $eventManager->addEventSubscriber(new SQLiteSessionInit(true, $journalMode)); break; } diff --git a/lib/private/db/sqliteplatform.php b/lib/private/db/sqliteplatform.php new file mode 100644 index 0000000000..b2eec7dd7b --- /dev/null +++ b/lib/private/db/sqliteplatform.php @@ -0,0 +1,35 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\DB; + +class SqlitePlatform extends \Doctrine\DBAL\Platforms\SqlitePlatform { + /** + * {@inheritDoc} + */ + public function getColumnDeclarationSQL($name, array $field) { + $def = parent::getColumnDeclarationSQL($name, $field); + if (!empty($field['autoincrement'])) { + $def .= ' PRIMARY KEY AUTOINCREMENT'; + } + return $def; + } + + /** + * {@inheritDoc} + */ + protected function _getCreateTableSQL($name, array $columns, array $options = array()){ + // if auto increment is set the column is already defined as primary key + foreach ($columns as $column) { + if (!empty($column['autoincrement'])) { + $options['primary'] = null; + } + } + return parent::_getCreateTableSQL($name, $columns, $options); + } +} From fc0c4990d53f00c9c06342b3de0a948e59baf201 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 4 Mar 2015 14:30:33 +0100 Subject: [PATCH 3/5] triger db upgrade to set autoincrement for sqlite --- version.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.php b/version.php index 2db1411da9..2fae2e8ddd 100644 --- a/version.php +++ b/version.php @@ -3,7 +3,7 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version=array(8, 1, 0, 0); +$OC_Version=array(8, 1, 0, 1); // The human readable string $OC_VersionString='8.1 pre alpha'; From ac8254de6a4adc60733de1eec242893004bfcc5a Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 4 Mar 2015 15:05:30 +0100 Subject: [PATCH 4/5] Rename platform --- lib/private/db/connectionfactory.php | 2 +- lib/private/db/{sqliteplatform.php => ocsqliteplatform.php} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename lib/private/db/{sqliteplatform.php => ocsqliteplatform.php} (92%) diff --git a/lib/private/db/connectionfactory.php b/lib/private/db/connectionfactory.php index c488bf82d1..b86e7319ae 100644 --- a/lib/private/db/connectionfactory.php +++ b/lib/private/db/connectionfactory.php @@ -96,7 +96,7 @@ class ConnectionFactory { break; case 'sqlite3': $journalMode = $additionalConnectionParams['sqlite.journal_mode']; - $additionalConnectionParams['platform'] = new SqlitePlatform(); + $additionalConnectionParams['platform'] = new OCSqlitePlatform(); $eventManager->addEventSubscriber(new SQLiteSessionInit(true, $journalMode)); break; } diff --git a/lib/private/db/sqliteplatform.php b/lib/private/db/ocsqliteplatform.php similarity index 92% rename from lib/private/db/sqliteplatform.php rename to lib/private/db/ocsqliteplatform.php index b2eec7dd7b..fe39e20c86 100644 --- a/lib/private/db/sqliteplatform.php +++ b/lib/private/db/ocsqliteplatform.php @@ -8,7 +8,7 @@ namespace OC\DB; -class SqlitePlatform extends \Doctrine\DBAL\Platforms\SqlitePlatform { +class OCSqlitePlatform extends \Doctrine\DBAL\Platforms\SqlitePlatform { /** * {@inheritDoc} */ From f056558b72db3a2eac72ac4e1b2889f7f7940b6a Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 25 Mar 2015 18:31:49 +0100 Subject: [PATCH 5/5] Add repair step to fix SQLite autoincrement Force Doctrine to generate alter table SQL statements for SQLite to make sure the code from OCSqlitePlatform is triggered. --- lib/private/repair.php | 2 + lib/repair/sqliteautoincrement.php | 85 +++++++++++++++++++ .../lib/repair/repairsqliteautoincrement.php | 81 ++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 lib/repair/sqliteautoincrement.php create mode 100644 tests/lib/repair/repairsqliteautoincrement.php diff --git a/lib/private/repair.php b/lib/private/repair.php index 101af66e30..d63b77b9c5 100644 --- a/lib/private/repair.php +++ b/lib/private/repair.php @@ -13,6 +13,7 @@ use OC\Hooks\Emitter; use OC\Repair\AssetCache; use OC\Repair\CleanTags; use OC\Repair\Collation; +use OC\Repair\SqliteAutoincrement; use OC\Repair\DropOldTables; use OC\Repair\FillETags; use OC\Repair\InnoDB; @@ -99,6 +100,7 @@ class Repair extends BasicEmitter { $steps = array( new InnoDB(), new Collation(\OC::$server->getConfig(), \OC_DB::getConnection()), + new SqliteAutoincrement(\OC_DB::getConnection()), new SearchLuceneTables(), new RepairConfig() ); diff --git a/lib/repair/sqliteautoincrement.php b/lib/repair/sqliteautoincrement.php new file mode 100644 index 0000000000..79b1fe7870 --- /dev/null +++ b/lib/repair/sqliteautoincrement.php @@ -0,0 +1,85 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\Repair; + +use Doctrine\DBAL\Platforms\SqlitePlatform; +use Doctrine\DBAL\Schema\SchemaException; +use Doctrine\DBAL\Schema\SchemaDiff; +use Doctrine\DBAL\Schema\TableDiff; +use Doctrine\DBAL\Schema\ColumnDiff; +use OC\Hooks\BasicEmitter; + +/** + * Fixes Sqlite autoincrement by forcing the SQLite table schemas to be + * altered in order to retrigger SQL schema generation through OCSqlitePlatform. + */ +class SqliteAutoincrement extends BasicEmitter implements \OC\RepairStep { + /** + * @var \OC\DB\Connection + */ + protected $connection; + + /** + * @param \OC\DB\Connection $connection + */ + public function __construct($connection) { + $this->connection = $connection; + } + + public function getName() { + return 'Repair SQLite autoincrement'; + } + + /** + * Fix mime types + */ + public function run() { + if (!$this->connection->getDatabasePlatform() instanceof SqlitePlatform) { + return; + } + + $sourceSchema = $this->connection->getSchemaManager()->createSchema(); + + $schemaDiff = new SchemaDiff(); + + foreach ($sourceSchema->getTables() as $tableSchema) { + $primaryKey = $tableSchema->getPrimaryKey(); + if (!$primaryKey) { + continue; + } + + $columnNames = $primaryKey->getColumns(); + + // add a column diff for every primary key column, + // but do not actually change anything, this will + // force the generation of SQL statements to alter + // those tables, which will then trigger the + // specific SQL code from OCSqlitePlatform + try { + $tableDiff = new TableDiff($tableSchema->getName()); + $tableDiff->fromTable = $tableSchema; + foreach ($columnNames as $columnName) { + $columnSchema = $tableSchema->getColumn($columnName); + $columnDiff = new ColumnDiff($columnSchema->getName(), $columnSchema); + $tableDiff->changedColumns[] = $columnDiff; + $schemaDiff->changedTables[] = $tableDiff; + } + } catch (SchemaException $e) { + // ignore + } + } + + $this->connection->beginTransaction(); + foreach ($schemaDiff->toSql($this->connection->getDatabasePlatform()) as $sql) { + $this->connection->query($sql); + } + $this->connection->commit(); + } +} + diff --git a/tests/lib/repair/repairsqliteautoincrement.php b/tests/lib/repair/repairsqliteautoincrement.php new file mode 100644 index 0000000000..f81e08ba78 --- /dev/null +++ b/tests/lib/repair/repairsqliteautoincrement.php @@ -0,0 +1,81 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test\Repair; + +/** + * Tests for fixing the SQLite id recycling + */ +class TestRepairSqliteAutoincrement extends \Test\TestCase { + + /** + * @var \OC\Repair\SqliteAutoincrement + */ + private $repair; + + /** + * @var \Doctrine\DBAL\Connection + */ + private $connection; + + /** + * @var string + */ + private $tableName; + + /** + * @var \OCP\IConfig + */ + private $config; + + protected function setUp() { + parent::setUp(); + + $this->connection = \OC_DB::getConnection(); + $this->config = \OC::$server->getConfig(); + if (!$this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\SqlitePlatform) { + $this->markTestSkipped("Test only relevant on Sqlite"); + } + + $dbPrefix = $this->config->getSystemValue('dbtableprefix', 'oc_'); + $this->tableName = $this->getUniqueID($dbPrefix . 'autoinc_test'); + $this->connection->exec('CREATE TABLE ' . $this->tableName . '("someid" INTEGER NOT NULL, "text" VARCHAR(16), PRIMARY KEY("someid"))'); + + $this->repair = new \OC\Repair\SqliteAutoincrement($this->connection); + } + + protected function tearDown() { + $this->connection->getSchemaManager()->dropTable($this->tableName); + parent::tearDown(); + } + + /** + * Tests whether autoincrement works + * + * @return boolean true if autoincrement works, false otherwise + */ + protected function checkAutoincrement() { + $this->connection->executeUpdate('INSERT INTO ' . $this->tableName . ' ("text") VALUES ("test")'); + $insertId = $this->connection->lastInsertId(); + $this->connection->executeUpdate('DELETE FROM ' . $this->tableName . ' WHERE "someid" = ?', array($insertId)); + + // insert again + $this->connection->executeUpdate('INSERT INTO ' . $this->tableName . ' ("text") VALUES ("test2")'); + $newInsertId = $this->connection->lastInsertId(); + + return ($insertId !== $newInsertId); + } + + public function testConvertIdColumn() { + $this->assertFalse($this->checkAutoincrement()); + + $this->repair->run(); + + $this->assertTrue($this->checkAutoincrement()); + } +}