diff --git a/lib/private/DB/MDB2SchemaManager.php b/lib/private/DB/MDB2SchemaManager.php index 1d25ae9f18..79811d8c6c 100644 --- a/lib/private/DB/MDB2SchemaManager.php +++ b/lib/private/DB/MDB2SchemaManager.php @@ -84,7 +84,7 @@ class MDB2SchemaManager { } else if ($platform instanceof MySqlPlatform) { return new MySQLMigrator($this->conn, $random, $config, $dispatcher); } else if ($platform instanceof PostgreSqlPlatform) { - return new Migrator($this->conn, $random, $config, $dispatcher); + return new PostgreSqlMigrator($this->conn, $random, $config, $dispatcher); } else { return new NoCheckMigrator($this->conn, $random, $config, $dispatcher); } diff --git a/lib/private/DB/Migrator.php b/lib/private/DB/Migrator.php index 8b8a34d986..f2efd6916d 100644 --- a/lib/private/DB/Migrator.php +++ b/lib/private/DB/Migrator.php @@ -33,6 +33,8 @@ use \Doctrine\DBAL\Schema\Table; use \Doctrine\DBAL\Schema\Schema; use \Doctrine\DBAL\Schema\SchemaConfig; use \Doctrine\DBAL\Schema\Comparator; +use Doctrine\DBAL\Types\StringType; +use Doctrine\DBAL\Types\Type; use OCP\IConfig; use OCP\Security\ISecureRandom; use Symfony\Component\EventDispatcher\EventDispatcher; @@ -194,7 +196,25 @@ class Migrator { return new Table($newName, $table->getColumns(), $newIndexes, array(), 0, $table->getOptions()); } + /** + * @param Schema $targetSchema + * @param \Doctrine\DBAL\Connection $connection + * @return \Doctrine\DBAL\Schema\SchemaDiff + * @throws DBALException + */ protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $connection) { + // adjust varchar columns with a length higher then getVarcharMaxLength to clob + foreach ($targetSchema->getTables() as $table) { + foreach ($table->getColumns() as $column) { + if ($column->getType() instanceof StringType) { + if ($column->getLength() > $connection->getDatabasePlatform()->getVarcharMaxLength()) { + $column->setType(Type::getType('text')); + $column->setLength(null); + } + } + } + } + $filterExpression = $this->getFilterExpression(); $this->connection->getConfiguration()-> setFilterSchemaAssetsExpression($filterExpression); diff --git a/lib/private/DB/OracleMigrator.php b/lib/private/DB/OracleMigrator.php index ceb89cf64d..010f9213a1 100644 --- a/lib/private/DB/OracleMigrator.php +++ b/lib/private/DB/OracleMigrator.php @@ -23,6 +23,7 @@ namespace OC\DB; +use Doctrine\DBAL\Schema\ColumnDiff; use Doctrine\DBAL\Schema\Schema; class OracleMigrator extends NoCheckMigrator { @@ -39,7 +40,12 @@ class OracleMigrator extends NoCheckMigrator { $tableDiff->name = $this->connection->quoteIdentifier($tableDiff->name); foreach ($tableDiff->changedColumns as $column) { $column->oldColumnName = $this->connection->quoteIdentifier($column->oldColumnName); + // auto increment is not relevant for oracle and can anyhow not be applied on change + $column->changedProperties = array_diff($column->changedProperties, ['autoincrement', 'unsigned']); } + $tableDiff->changedColumns = array_filter($tableDiff->changedColumns, function (ColumnDiff $column) { + return count($column->changedProperties) > 0; + }); } return $schemaDiff; diff --git a/lib/private/DB/PostgreSqlMigrator.php b/lib/private/DB/PostgreSqlMigrator.php new file mode 100644 index 0000000000..75e6e0a16c --- /dev/null +++ b/lib/private/DB/PostgreSqlMigrator.php @@ -0,0 +1,55 @@ + + * + * @copyright Copyright (c) 2016, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\DB; + +use Doctrine\DBAL\Schema\Schema; + +class PostgreSqlMigrator extends Migrator { + /** + * @param Schema $targetSchema + * @param \Doctrine\DBAL\Connection $connection + * @return \Doctrine\DBAL\Schema\SchemaDiff + */ + protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $connection) { + $schemaDiff = parent::getDiff($targetSchema, $connection); + + foreach ($schemaDiff->changedTables as $tableDiff) { + // fix default value in brackets - pg 9.4 is returning a negative default value in () + // see https://github.com/doctrine/dbal/issues/2427 + foreach ($tableDiff->changedColumns as $column) { + $column->changedProperties = array_filter($column->changedProperties, function ($changedProperties) use ($column) { + if ($changedProperties !== 'default') { + return true; + } + $fromDefault = $column->fromColumn->getDefault(); + $toDefault = $column->column->getDefault(); + $fromDefault = trim($fromDefault, "()"); + + // by intention usage of != + return $fromDefault != $toDefault; + }); + } + } + + return $schemaDiff; + } +} diff --git a/lib/private/DB/SQLiteMigrator.php b/lib/private/DB/SQLiteMigrator.php index 8ea3258101..837a3a2987 100644 --- a/lib/private/DB/SQLiteMigrator.php +++ b/lib/private/DB/SQLiteMigrator.php @@ -26,6 +26,8 @@ namespace OC\DB; use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Schema\Schema; +use Doctrine\DBAL\Types\BigIntType; +use Doctrine\DBAL\Types\Type; class SQLiteMigrator extends Migrator { @@ -76,6 +78,15 @@ class SQLiteMigrator extends Migrator { $platform->registerDoctrineTypeMapping('smallint unsigned', 'integer'); $platform->registerDoctrineTypeMapping('varchar ', 'string'); + // with sqlite autoincrement columns is of type integer + foreach ($targetSchema->getTables() as $table) { + foreach ($table->getColumns() as $column) { + if ($column->getType() instanceof BigIntType && $column->getAutoincrement()) { + $column->setType(Type::getType('integer')); + } + } + } + return parent::getDiff($targetSchema, $connection); } } diff --git a/tests/lib/DB/DBSchemaTest.php b/tests/lib/DB/DBSchemaTest.php index 284fc532c2..ba17546a34 100644 --- a/tests/lib/DB/DBSchemaTest.php +++ b/tests/lib/DB/DBSchemaTest.php @@ -8,15 +8,17 @@ namespace Test\DB; +use Doctrine\DBAL\Platforms\SqlitePlatform; use OC_DB; use OCP\Security\ISecureRandom; +use Test\TestCase; /** * Class DBSchemaTest * * @group DB */ -class DBSchemaTest extends \Test\TestCase { +class DBSchemaTest extends TestCase { protected $schema_file = 'static://test_db_scheme'; protected $schema_file2 = 'static://test_db_scheme2'; protected $table1; @@ -53,7 +55,6 @@ class DBSchemaTest extends \Test\TestCase { * @medium */ public function testSchema() { - $platform = \OC::$server->getDatabaseConnection()->getDatabasePlatform(); $this->doTestSchemaCreating(); $this->doTestSchemaChanging(); $this->doTestSchemaDumping(); @@ -97,7 +98,7 @@ class DBSchemaTest extends \Test\TestCase { */ public function assertTableNotExist($table) { $platform = \OC::$server->getDatabaseConnection()->getDatabasePlatform(); - if ($platform instanceof \Doctrine\DBAL\Platforms\SqlitePlatform) { + if ($platform instanceof SqlitePlatform) { // sqlite removes the tables after closing the DB $this->assertTrue(true); } else { diff --git a/tests/lib/DB/SchemaDiffTest.php b/tests/lib/DB/SchemaDiffTest.php new file mode 100644 index 0000000000..b7bb3c2a9c --- /dev/null +++ b/tests/lib/DB/SchemaDiffTest.php @@ -0,0 +1,99 @@ + + * + * @copyright Copyright (c) 2016, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace Test\DB; + +use Doctrine\DBAL\Schema\SchemaDiff; +use OC\DB\MDB2SchemaManager; +use OC\DB\MDB2SchemaReader; +use OCP\IConfig; +use Test\TestCase; + +/** + * Class MigratorTest + * + * @group DB + * + * @package Test\DB + */ +class SchemaDiffTest extends TestCase { + /** @var \Doctrine\DBAL\Connection $connection */ + private $connection; + + /** @var MDB2SchemaManager */ + private $manager; + + /** @var IConfig */ + private $config; + + /** @var string */ + private $testPrefix; + + protected function setUp() { + parent::setUp(); + + $this->config = \OC::$server->getConfig(); + $this->connection = \OC::$server->getDatabaseConnection(); + $this->manager = new MDB2SchemaManager($this->connection); + $this->testPrefix= strtolower($this->getUniqueID($this->config->getSystemValue('dbtableprefix', 'oc_'), 3)); + } + + protected function tearDown() { + $this->manager->removeDBStructure('static://test_db_scheme'); + parent::tearDown(); + } + + /** + * @dataProvider providesSchemaFiles + * @param string $xml + */ + public function testZeroChangeOnSchemaMigrations($xml) { + + $xml = str_replace( '*dbprefix*', $this->testPrefix, $xml ); + $schemaFile = 'static://test_db_scheme'; + file_put_contents($schemaFile, $xml); + + // apply schema + $this->manager->createDbFromStructure($schemaFile); + + $schemaReader = new MDB2SchemaReader($this->config, $this->connection->getDatabasePlatform()); + $endSchema = $schemaReader->loadSchemaFromFile($schemaFile); + + // get the diff + /** @var SchemaDiff $diff */ + $migrator = $this->manager->getMigrator(); + $diff = $this->invokePrivate($migrator, 'getDiff', [$endSchema, $this->connection]); + + // no sql statement is expected + $sqls = $diff->toSql($this->connection->getDatabasePlatform()); + $this->assertEquals([], $sqls); + } + + public function providesSchemaFiles() { + return [ + 'explicit test on autoincrement' => [file_get_contents(__DIR__ . '/schemDiffData/autoincrement.xml')], + 'explicit test on clob' => [file_get_contents(__DIR__ . '/schemDiffData/clob.xml')], + 'explicit test on unsigned' => [file_get_contents(__DIR__ . '/schemDiffData/unsigned.xml')], + 'explicit test on default -1' => [file_get_contents(__DIR__ . '/schemDiffData/default-1.xml')], + 'testing core schema' => [file_get_contents(__DIR__ . '/schemDiffData/core.xml')], + ]; + } +} diff --git a/tests/lib/DB/schemDiffData/autoincrement.xml b/tests/lib/DB/schemDiffData/autoincrement.xml new file mode 100644 index 0000000000..458c5d8166 --- /dev/null +++ b/tests/lib/DB/schemDiffData/autoincrement.xml @@ -0,0 +1,16 @@ + + + + *dbprefix*external_config + + + config_id + integer + 0 + true + 1 + 6 + + +
+
diff --git a/tests/lib/DB/schemDiffData/clob.xml b/tests/lib/DB/schemDiffData/clob.xml new file mode 100644 index 0000000000..08aef18c31 --- /dev/null +++ b/tests/lib/DB/schemDiffData/clob.xml @@ -0,0 +1,14 @@ + + + + *dbprefix*external_config + + + value + text + true + 100000 + + +
+
diff --git a/tests/lib/DB/schemDiffData/core.xml b/tests/lib/DB/schemDiffData/core.xml new file mode 100644 index 0000000000..3775eb6cb3 --- /dev/null +++ b/tests/lib/DB/schemDiffData/core.xml @@ -0,0 +1,1347 @@ + + + + + + + *dbprefix*appconfig + + + + + appid + text + + true + 32 + + + + configkey + text + + true + 64 + + + + configvalue + clob + false + + + +
+ + + + + *dbprefix*storages + + + + + id + text + + false + 64 + + + + numeric_id + integer + 0 + true + 1 + 4 + + + + available + integer + 1 + true + + + + last_checked + integer + + + +
+ + + + + *dbprefix*mounts + + + + + id + integer + 0 + true + 1 + 4 + + + + storage_id + integer + true + + + + + root_id + integer + true + + + + user_id + text + true + 64 + + + + mount_point + text + true + 4000 + + + + + +
+ + + + + *dbprefix*mimetypes + + + + + id + integer + 0 + true + 1 + 4 + + + + mimetype + text + + true + 255 + + + + +
+ + + + + *dbprefix*filecache + + + + + fileid + integer + 0 + true + 1 + 4 + + + + + storage + integer + + true + 4 + + + + path + text + + false + 4000 + + + + path_hash + text + + true + 32 + + + + + parent + integer + + true + 4 + + + + name + text + + false + 250 + + + + + mimetype + integer + + true + 4 + + + + + mimepart + integer + + true + 4 + + + + size + integer + + true + 8 + + + + mtime + integer + + true + 4 + + + + storage_mtime + integer + + true + 4 + + + + encrypted + integer + 0 + true + 4 + + + + unencrypted_size + integer + 0 + true + 8 + + + + etag + text + + false + 40 + + + + permissions + integer + 0 + false + 4 + + + + checksum + text + + false + 255 + + + + + + +
+ + + + + *dbprefix*group_user + + + + + + gid + text + + true + 64 + + + + + uid + text + + true + 64 + + + + +
+ + + + + *dbprefix*group_admin + + + + + + gid + text + + true + 64 + + + + + uid + text + + true + 64 + + + + +
+ + + + + *dbprefix*groups + + + + + gid + text + + true + 64 + + + + +
+ + + + + *dbprefix*preferences + + + + + + userid + text + + true + 64 + + + + appid + text + + true + 32 + + + + configkey + text + + true + 64 + + + + configvalue + clob + false + + + + +
+ + + + + *dbprefix*properties + + + + + id + 1 + integer + 0 + true + 4 + + + + + userid + text + + true + 64 + + + + propertypath + text + + true + 255 + + + + propertyname + text + + true + 255 + + + + propertyvalue + text + true + 255 + + + + +
+ + + + + *dbprefix*share + + + + + id + 1 + integer + 0 + true + 4 + + + + + share_type + integer + 0 + true + 1 + + + + + share_with + text + + false + 255 + + + + + + uid_owner + text + + true + 64 + + + + + + uid_initiator + text + + false + 64 + + + + + + + parent + integer + false + 4 + + + + + item_type + text + + true + 64 + + + + + item_source + text + + false + 255 + + + + item_target + text + + false + 255 + + + + + file_source + integer + false + 4 + + + + file_target + text + + false + 512 + + + + + permissions + integer + 0 + true + 1 + + + + + stime + integer + 0 + true + 8 + + + + + accepted + integer + 0 + true + 1 + + + + + expiration + timestamp + + false + + + + token + text + + false + 32 + + + + mail_send + integer + 0 + true + 1 + + + + +
+ + + + + *dbprefix*jobs + + + + + id + integer + 0 + true + 1 + true + 4 + + + + class + text + + true + 255 + + + + argument + text + + true + 4000 + + + + + last_run + integer + + false + + + + + last_checked + integer + + false + + + + + reserved_at + integer + + false + + + + +
+ + + + + *dbprefix*users + + + + + uid + text + + true + 64 + + + + displayname + text + + 64 + + + + password + text + + true + 255 + + + + + +
+ + + *dbprefix*authtoken + + + + + id + integer + 0 + true + 1 + true + 4 + + + + + uid + text + + true + 64 + + + + login_name + text + + true + 64 + + + + password + clob + + false + + + + name + clob + + true + + + + token + text + + true + 200 + + + + type + integer + 0 + true + true + 2 + + + + last_activity + integer + 0 + true + true + 4 + + + +
+ + + + + *dbprefix*vcategory + + + + + id + integer + 0 + true + 1 + true + 4 + + + + + uid + text + + true + 64 + + + + type + text + + true + 64 + + + + category + text + + true + 255 + + + +
+ + + + + *dbprefix*vcategory_to_object + + + + + objid + integer + 0 + true + true + 4 + + + + + categoryid + integer + 0 + true + true + 4 + + + + type + text + + true + 64 + + + + +
+ + + + *dbprefix*systemtag + + + + + id + integer + 0 + true + 1 + true + 4 + + + + + name + text + + true + 64 + + + + + visibility + integer + 1 + true + 1 + + + + + editable + integer + 1 + true + 1 + + + +
+ + + + + *dbprefix*systemtag_object_mapping + + + + + + objectid + text + + true + 64 + + + + + objecttype + text + + true + 64 + + + + + systemtagid + integer + 0 + true + true + 4 + + + + + +
+ + + + + *dbprefix*systemtag_group + + + + + + systemtagid + integer + 0 + true + true + 4 + + + + gid + string + true + + + + +
+ + + + + *dbprefix*privatedata + + + + + keyid + integer + 0 + true + true + 4 + 1 + + + + + user + text + + true + 64 + + + + app + text + + true + 255 + + + + key + text + + true + 255 + + + + value + text + + true + 255 + + + + +
+ + + + + *dbprefix*file_locks + + + + + id + integer + 0 + true + true + 4 + 1 + + + + lock + integer + 0 + true + 4 + + + + key + text + true + 64 + + + + ttl + integer + -1 + true + 4 + + + + +
+ + + + *dbprefix*comments + + + + + id + integer + 0 + true + true + 4 + 1 + + + + parent_id + integer + 0 + true + true + 4 + + + + topmost_parent_id + integer + 0 + true + true + 4 + + + + children_count + integer + 0 + true + true + 4 + + + + actor_type + text + + true + 64 + + + + actor_id + text + + true + 64 + + + + message + clob + + false + + + + verb + text + + false + 64 + + + + creation_timestamp + timestamp + + false + + + + latest_child_timestamp + timestamp + + false + + + + object_type + text + + true + 64 + + + + object_id + text + + true + 64 + + + + +
+ + + + *dbprefix*comments_read_markers + + + + + user_id + text + + true + 64 + + + + marker_datetime + timestamp + + false + + + + object_type + text + + true + 64 + + + + object_id + text + + true + 64 + + + + +
+ + + + *dbprefix*credentials + + + + + user + text + + false + 64 + + + + identifier + text + true + 64 + + + + credentials + clob + false + + + + +
+ +
diff --git a/tests/lib/DB/schemDiffData/default-1.xml b/tests/lib/DB/schemDiffData/default-1.xml new file mode 100644 index 0000000000..39b95a1f27 --- /dev/null +++ b/tests/lib/DB/schemDiffData/default-1.xml @@ -0,0 +1,51 @@ + + + + + + + *dbprefix*file_locks + + + + + id + integer + 0 + true + true + 4 + 1 + + + + lock + integer + 0 + true + 4 + + + + key + text + true + (stupid text) + 64 + + + + ttl + integer + -1 + true + 4 + + + + +
+ +
diff --git a/tests/lib/DB/schemDiffData/unsigned.xml b/tests/lib/DB/schemDiffData/unsigned.xml new file mode 100644 index 0000000000..e89fd6a5d4 --- /dev/null +++ b/tests/lib/DB/schemDiffData/unsigned.xml @@ -0,0 +1,68 @@ + + + + + + + *dbprefix*jobs + + + + + id + integer + 0 + true + 1 + true + 4 + + + + class + text + + true + 255 + + + + argument + text + + true + 4000 + + + + + last_run + integer + + false + + + + + last_checked + integer + + false + + + + + reserved_at + integer + + false + + + + +
+ +