From 0a369b0850db696844c2c4bfd40938b5f0c823e6 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 22 Oct 2018 21:09:48 +0200 Subject: [PATCH 1/5] Add mariadb 10.3 to Drone --- .drone.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.drone.yml b/.drone.yml index 807621379d3c..6231beabb933 100644 --- a/.drone.yml +++ b/.drone.yml @@ -423,7 +423,7 @@ pipeline: services: mariadb: - image: mariadb:10.2 + image: ${MARIADB_IMAGE=mariadb:10.2} environment: - MYSQL_USER=owncloud - MYSQL_PASSWORD=owncloud @@ -625,6 +625,13 @@ matrix: COVERAGE: true INSTALL_SERVER: true + - PHP_VERSION: 7.1 + DB_TYPE: mariadb + MARIADB_IMAGE: mariadb:10.3 + TEST_SUITE: phpunit + COVERAGE: true + INSTALL_SERVER: true + - PHP_VERSION: 7.1 DB_TYPE: oracle TEST_SUITE: phpunit From 9f355ddf77e5fe15cc072f5643e9d01b594a6608 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 7 Nov 2018 17:17:42 +0100 Subject: [PATCH 2/5] Detect MariaDB > 10.3.1 and assume mb4 support --- lib/private/DB/MySqlTools.php | 46 ++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/lib/private/DB/MySqlTools.php b/lib/private/DB/MySqlTools.php index 61409b8bd490..b4009c86e9bd 100644 --- a/lib/private/DB/MySqlTools.php +++ b/lib/private/DB/MySqlTools.php @@ -22,17 +22,13 @@ namespace OC\DB; use OCP\IDBConnection; +use Doctrine\DBAL\DBALException; /** * Various MySQL specific helper functions. */ class MySqlTools { - - /** - * @param Connection $connection - * @return bool - */ - public function supports4ByteCharset(IDBConnection $connection) { + private function detectBarracuda(IDBConnection $connection) { foreach (['innodb_file_format' => 'Barracuda', 'innodb_large_prefix' => 'ON', 'innodb_file_per_table' => 'ON'] as $var => $val) { $result = $connection->executeQuery("SHOW VARIABLES LIKE '$var'"); $rows = $result->fetch(); @@ -44,6 +40,44 @@ public function supports4ByteCharset(IDBConnection $connection) { return false; } } + return true; } + + /** + * Detect MariaDB server version, including hack for some mariadb distributions + * that starts with the prefix '5.5.5-' + * + * @param string $versionString Version string as returned by mariadb server, i.e. '5.5.5-Mariadb-10.0.8-xenial' + * @return string + * @throws DBALException + */ + private function getMariaDbMysqlVersionNumber($versionString) { + if (!\preg_match('/^(?:5\.5\.5-)?(mariadb-)?(?P\d+)\.(?P\d+)\.(?P\d+)/i', $versionString, $versionParts)) { + throw DBALException::invalidPlatformVersionSpecified( + $versionString, + '^(?:5\.5\.5-)?(mariadb-)?..' + ); + } + return $versionParts['major'] . '.' . $versionParts['minor'] . '.' . $versionParts['patch']; + } + + /** + * @param Connection $connection + * @return bool + */ + public function supports4ByteCharset(IDBConnection $connection) { + if ($this->detectBarracuda($connection)) { + return true; + } + + $version = $connection->getDatabaseVersionString(); + $mariadb = \stripos($version, 'mariadb') !== false; + if ($mariadb && \version_compare($this->getMariaDbMysqlVersionNumber($version), '10.3.1', '>=')) { + // mb4 supported by default on MariaDB since 10.3.1 and innodb_file_format vars were removed + return true; + } + + return false; + } } From dddedc9046d0c71f4988195d2962fac55df7d03b Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 7 Nov 2018 17:18:17 +0100 Subject: [PATCH 3/5] Remove obsolete MariaDB 10.2 workaround Since the DBAL update, the schema wrapper is not needed any more. --- lib/private/DB/ConnectionFactory.php | 4 - .../MySqlSchemaColumnDefinitionListener.php | 244 ------------------ 2 files changed, 248 deletions(-) delete mode 100644 lib/private/DB/MySqlSchemaColumnDefinitionListener.php diff --git a/lib/private/DB/ConnectionFactory.php b/lib/private/DB/ConnectionFactory.php index 5d087c89cca5..eab0135b65c7 100644 --- a/lib/private/DB/ConnectionFactory.php +++ b/lib/private/DB/ConnectionFactory.php @@ -117,10 +117,6 @@ public function getConnection($type, $additionalConnectionParams) { case 'mysql': $eventManager->addEventSubscriber( new SQLSessionInit("SET SESSION AUTOCOMMIT=1")); - $eventManager->addEventListener( - Events::onSchemaColumnDefinition, - new MySqlSchemaColumnDefinitionListener() - ); break; case 'oci': $eventManager->addEventSubscriber(new OracleSessionInit); diff --git a/lib/private/DB/MySqlSchemaColumnDefinitionListener.php b/lib/private/DB/MySqlSchemaColumnDefinitionListener.php deleted file mode 100644 index 6dbcc8ddd9bd..000000000000 --- a/lib/private/DB/MySqlSchemaColumnDefinitionListener.php +++ /dev/null @@ -1,244 +0,0 @@ - - * - * @copyright Copyright (c) 2018, ownCloud GmbH - * @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\DBALException; -use Doctrine\DBAL\Event\SchemaColumnDefinitionEventArgs; -use Doctrine\DBAL\Platforms\MySqlPlatform; -use Doctrine\DBAL\Schema\Column; -use Doctrine\DBAL\Types\Type; - -/** - * Class MySqlSchemaColumnDefinitionListener - * - * @package OC\DB - * - * This class contains the smallest portion of native Doctrine code taken from - * Doctrine\DBAL\Platforms\MySqlSchemaManager that allows to bypass the call to - * native _getPortableTableColumnDefinition method - * - * TODO: remove once https://github.com/owncloud/core/issues/28695 is fixed and Doctrine upgraded - */ -class MySqlSchemaColumnDefinitionListener { - /** - * @var \Doctrine\DBAL\Platforms\AbstractPlatform - */ - protected $_platform; - - public function onSchemaColumnDefinition(SchemaColumnDefinitionEventArgs $eventArgs) { - // We need an instance of platform with ownCloud-specific mappings - // this part can't be moved to constructor - it leads to an infinite recursion - if ($this->_platform === null) { - $this->_platform = \OC::$server->getDatabaseConnection()->getDatabasePlatform(); - } - - $version = \OC::$server->getDatabaseConnection()->getDatabaseVersionString(); - $mariadb = \stripos($version, 'mariadb') !== false; - if ($mariadb && \version_compare($this->getMariaDbMysqlVersionNumber($version), '10.2.7', '>=')) { - $tableColumn = $eventArgs->getTableColumn(); - try { - $column = $this->_getPortableTableColumnDefinition($tableColumn); - $eventArgs->preventDefault(); - $eventArgs->setColumn($column); - } catch (DBALException $e) { - // Pass - } - } - } - - /** - * Given a table comment this method tries to extract a typehint for Doctrine Type, or returns - * the type given as default. - * - * @param string $comment - * @param string $currentType - * - * @return string - */ - public function extractDoctrineTypeFromComment($comment, $currentType) { - if (\preg_match("(\(DC2Type:([a-zA-Z0-9_]+)\))", $comment, $match)) { - $currentType = $match[1]; - } - - return $currentType; - } - - /** - * @param string $comment - * @param string $type - * - * @return string - */ - public function removeDoctrineTypeFromComment($comment, $type) { - return \str_replace('(DC2Type:'.$type.')', '', $comment); - } - - protected function _getPortableTableColumnDefinition($tableColumn) { - $tableColumn = \array_change_key_case($tableColumn, CASE_LOWER); - - $dbType = \strtolower($tableColumn['type']); - $dbType = \strtok($dbType, '(), '); - if (isset($tableColumn['length'])) { - $length = $tableColumn['length']; - } else { - $length = \strtok('(), '); - } - - $fixed = null; - - if (! isset($tableColumn['name'])) { - $tableColumn['name'] = ''; - } - - $scale = null; - $precision = null; - - $type = $this->_platform->getDoctrineTypeMapping($dbType); - - // In cases where not connected to a database DESCRIBE $table does not return 'Comment' - if (isset($tableColumn['comment'])) { - $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type); - $tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type); - } - - switch ($dbType) { - case 'char': - case 'binary': - $fixed = true; - break; - case 'float': - case 'double': - case 'real': - case 'numeric': - case 'decimal': - if (\preg_match('([A-Za-z]+\(([0-9]+)\,([0-9]+)\))', $tableColumn['type'], $match)) { - $precision = $match[1]; - $scale = $match[2]; - $length = null; - } - break; - case 'tinytext': - $length = MySqlPlatform::LENGTH_LIMIT_TINYTEXT; - break; - case 'text': - $length = MySqlPlatform::LENGTH_LIMIT_TEXT; - break; - case 'mediumtext': - $length = MySqlPlatform::LENGTH_LIMIT_MEDIUMTEXT; - break; - case 'tinyblob': - $length = MySqlPlatform::LENGTH_LIMIT_TINYBLOB; - break; - case 'blob': - $length = MySqlPlatform::LENGTH_LIMIT_BLOB; - break; - case 'mediumblob': - $length = MySqlPlatform::LENGTH_LIMIT_MEDIUMBLOB; - break; - case 'tinyint': - case 'smallint': - case 'mediumint': - case 'int': - case 'integer': - case 'bigint': - case 'year': - $length = null; - break; - } - - $length = ((int) $length == 0) ? null : (int) $length; - - $default = isset($tableColumn['default']) ? $tableColumn['default'] : null; - $columnDefault = $this->getMariaDb1027ColumnDefault($default); - - $options = [ - 'length' => $length, - 'unsigned' => (bool) (\strpos($tableColumn['type'], 'unsigned') !== false), - 'fixed' => (bool) $fixed, - // This line was changed to fix breaking change introduced in MariaDB 10.2.6 - 'default' => $columnDefault, - 'notnull' => (bool) ($tableColumn['null'] != 'YES'), - 'scale' => null, - 'precision' => null, - 'autoincrement' => (bool) (\strpos($tableColumn['extra'], 'auto_increment') !== false), - 'comment' => isset($tableColumn['comment']) && $tableColumn['comment'] !== '' - ? $tableColumn['comment'] - : null, - ]; - - if ($scale !== null && $precision !== null) { - $options['scale'] = $scale; - $options['precision'] = $precision; - } - - $column = new Column($tableColumn['field'], Type::getType($type), $options); - - if (isset($tableColumn['collation'])) { - $column->setPlatformOption('collation', $tableColumn['collation']); - } - - return $column; - } - - /** - * Return Doctrine/Mysql-compatible column default values for MariaDB 10.2.7+ servers. - * - * - Since MariaDb 10.2.7 column defaults stored in information_schema are now quoted - * to distinguish them from expressions (see MDEV-10134). - * - Note: Quoted 'NULL' is not enforced by Maria, it is technically possible to have - * null in some circumstances (see https://jira.mariadb.org/browse/MDEV-14053) - * - * @link https://mariadb.com/kb/en/library/information-schema-columns-table/ - * @link https://jira.mariadb.org/browse/MDEV-13132 - * - * @param null|string $columnDefault default value as stored in information_schema for MariaDB >= 10.2.7 - * @return string - */ - private function getMariaDb1027ColumnDefault($columnDefault) { - if ($columnDefault === 'NULL' || $columnDefault === null) { - return null; - } - if ($columnDefault[0] === "'") { - return \preg_replace('/^\'(.*)\'$/', '$1', $columnDefault); - } - - return $columnDefault; - } - - /** - * Detect MariaDB server version, including hack for some mariadb distributions - * that starts with the prefix '5.5.5-' - * - * @param string $versionString Version string as returned by mariadb server, i.e. '5.5.5-Mariadb-10.0.8-xenial' - * @return string - * @throws DBALException - */ - private function getMariaDbMysqlVersionNumber($versionString) { - if (!\preg_match('/^(?:5\.5\.5-)?(mariadb-)?(?P\d+)\.(?P\d+)\.(?P\d+)/i', $versionString, $versionParts)) { - throw DBALException::invalidPlatformVersionSpecified( - $versionString, - '^(?:5\.5\.5-)?(mariadb-)?..' - ); - } - return $versionParts['major'] . '.' . $versionParts['minor'] . '.' . $versionParts['patch']; - } -} From 152821172d5f0d4bc4e59feff158d6bd8274a753 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 8 Nov 2018 14:58:57 +0100 Subject: [PATCH 4/5] Add MySQL 8 which defaults to mb4 mode --- .drone.yml | 22 +++++++++++++++++----- lib/private/DB/MySqlTools.php | 6 ++++++ tests/drone/install-server.sh | 4 ++-- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/.drone.yml b/.drone.yml index 6231beabb933..522adf0d7e8a 100644 --- a/.drone.yml +++ b/.drone.yml @@ -434,7 +434,7 @@ services: DB_TYPE: mariadb mysql: - image: mysql:5.5 + image: ${MYSQL_IMAGE=mysql:5.5} environment: - MYSQL_USER=owncloud - MYSQL_PASSWORD=owncloud @@ -444,8 +444,10 @@ services: matrix: DB_TYPE: mysql - mysqlmb4: - image: mysql:5.7 + mysql8: + image: ${MYSQL_IMAGE=mysql:8.0} + # see http://php.net/manual/en/mysqli.requirements.php + command: --default-authentication-plugin=mysql_native_password environment: - MYSQL_USER=owncloud - MYSQL_PASSWORD=owncloud @@ -453,7 +455,7 @@ services: - MYSQL_ROOT_PASSWORD=owncloud when: matrix: - DB_TYPE: mysqlmb4 + DB_TYPE: mysql8 postgres: image: ${POSTGRES_IMAGE=postgres:9.4} @@ -602,8 +604,17 @@ matrix: INSTALL_SERVER: true - PHP_VERSION: 7.1 - DB_TYPE: mysqlmb4 + DB_TYPE: mysql + # mb4 support, with innodb_file_format=Barracuda + MYSQL_IMAGE: mysql:5.7 + TEST_SUITE: phpunit + INSTALL_SERVER: true + + - PHP_VERSION: 7.1 + # mb4 support by default + DB_TYPE: mysql8 TEST_SUITE: phpunit + COVERAGE: false INSTALL_SERVER: true # - PHP_VERSION: 7.1 @@ -627,6 +638,7 @@ matrix: - PHP_VERSION: 7.1 DB_TYPE: mariadb + # mb4 support by default MARIADB_IMAGE: mariadb:10.3 TEST_SUITE: phpunit COVERAGE: true diff --git a/lib/private/DB/MySqlTools.php b/lib/private/DB/MySqlTools.php index b4009c86e9bd..62e084c7b0f3 100644 --- a/lib/private/DB/MySqlTools.php +++ b/lib/private/DB/MySqlTools.php @@ -23,6 +23,7 @@ use OCP\IDBConnection; use Doctrine\DBAL\DBALException; +use Doctrine\DBAL\Platforms\MySQL80Platform; /** * Various MySQL specific helper functions. @@ -71,6 +72,11 @@ public function supports4ByteCharset(IDBConnection $connection) { return true; } + if ($connection->getDatabasePlatform() instanceof MySQL80Platform) { + // mb4 supported by default since MySQL 8.0 and innodb_file_format vars were removed + return true; + } + $version = $connection->getDatabaseVersionString(); $mariadb = \stripos($version, 'mariadb') !== false; if ($mariadb && \version_compare($this->getMariaDbMysqlVersionNumber($version), '10.3.1', '>=')) { diff --git a/tests/drone/install-server.sh b/tests/drone/install-server.sh index 4e480abf97e3..04f256bbeab5 100755 --- a/tests/drone/install-server.sh +++ b/tests/drone/install-server.sh @@ -83,8 +83,8 @@ case "${DB_TYPE}" in wait-for-it mysql:3306 DB=mysql ;; - mysqlmb4) - wait-for-it mysqlmb4:3306 + mysql8) + wait-for-it mysql8:3306 DB=mysql ;; postgres) From a2078b3440543770aa4f1f88dae03c82f5a69f85 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 15 Nov 2018 22:04:44 +0100 Subject: [PATCH 5/5] Unit tests for mysql mb4 detection Unit tests to verify implementation of mb4 detection without touching the database. --- lib/private/DB/MySqlTools.php | 10 ++- tests/lib/DB/MySqlToolsTest.php | 134 ++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 tests/lib/DB/MySqlToolsTest.php diff --git a/lib/private/DB/MySqlTools.php b/lib/private/DB/MySqlTools.php index 62e084c7b0f3..34cd90bf9790 100644 --- a/lib/private/DB/MySqlTools.php +++ b/lib/private/DB/MySqlTools.php @@ -1,6 +1,7 @@ + * @author Vincent Petry * * @copyright Copyright (c) 2018, ownCloud GmbH * @license AGPL-3.0 @@ -24,6 +25,7 @@ use OCP\IDBConnection; use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Platforms\MySQL80Platform; +use OC\DB\Connection; /** * Various MySQL specific helper functions. @@ -64,10 +66,12 @@ private function getMariaDbMysqlVersionNumber($versionString) { } /** - * @param Connection $connection - * @return bool + * Returns whether the database from the given connection supports 4-byte characters. + * + * @param Connection $connection connection to check + * @return bool true if supported, false otherwise */ - public function supports4ByteCharset(IDBConnection $connection) { + public function supports4ByteCharset(Connection $connection) { if ($this->detectBarracuda($connection)) { return true; } diff --git a/tests/lib/DB/MySqlToolsTest.php b/tests/lib/DB/MySqlToolsTest.php new file mode 100644 index 000000000000..c626f3590f1e --- /dev/null +++ b/tests/lib/DB/MySqlToolsTest.php @@ -0,0 +1,134 @@ + + * + * @copyright Copyright (c) 2018, ownCloud GmbH + * @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 OCP\IDBConnection; +use Doctrine\DBAL\Driver\Statement; +use OC\DB\MySqlTools; +use Doctrine\DBAL\Platforms\MySQL80Platform; +use OC\DB\Connection; + +/** + * Class MySqlToolsTest + */ +class MySqlToolsTest extends \Test\TestCase { + + /** + * @var IDBConnection + */ + private $db; + + /** + * @var MySqlTools + */ + + public function setUp() { + parent::setUp(); + + $this->db = $this->createMock(Connection::class); + $this->tools = new MySqlTools(); + } + + public function providesDbVars() { + return [ + [ + ['innodb_file_format' => 'Barracuda', 'innodb_large_prefix' => 'ON', 'innodb_file_per_table' => 'ON'], true, + ['innodb_file_format' => 'Barracuda', 'innodb_large_prefix' => 'ON', 'innodb_file_per_table' => 'OFF'], false, + ['innodb_file_format' => 'Antelope', 'innodb_large_prefix' => 'ON', 'innodb_file_per_table' => 'ON'], false, + ['innodb_file_format' => 'Barracuda', 'innodb_large_prefix' => 'OFF', 'innodb_file_per_table' => 'ON'], false, + ], + ]; + } + + private function mockDatabaseVars($vars) { + $map = []; + foreach ($vars as $key => $value) { + $statementMock = $this->createMock(Statement::class); + $statementMock->expects($this->once())->method('fetch')->willReturn(['Value' => $value]); + $map[] = ["SHOW VARIABLES LIKE '$key'", [], [], null, $statementMock]; + } + // mock vars + $this->db->expects($this->any()) + ->method('executeQuery') + ->will($this->returnValueMap($map)); + } + + private function mockDatabaseEmptyVars() { + $statementMock = $this->createMock(Statement::class); + $statementMock->expects($this->any())->method('fetch')->willReturn(false); + + $this->db->expects($this->any()) + ->method('executeQuery') + ->willReturn($statementMock); + } + + /** + * Test barracuda detection based on MySQL vars + * + * @dataProvider providesDbVars + * @param array $vars + * @param bool $expectedResult + */ + public function testMb4WithBarracuda($vars, $expectedResult) { + $this->mockDatabaseVars($vars); + + $this->assertEquals($expectedResult, $this->tools->supports4ByteCharset($this->db)); + } + + public function testDetectMb4WithMySQL8() { + $this->mockDatabaseEmptyVars(); + + $this->db->expects($this->once()) + ->method('getDatabasePlatform') + ->willReturn($this->createMock(MySQL80Platform::class)); + + $this->assertTrue($this->tools->supports4ByteCharset($this->db)); + } + + public function providesVersionString() { + return [ + ['10.3.2-oracle', false], + ['10.3.0-mariadb', false], + ['mariadb-10.3.0', false], + ['10.3.1-mariadb', true], + ['mariadb-10.3.1', true], + ]; + } + + /** + * Tests whether MariaDB 10.3 is properly detected. + * + * @dataProvider providesVersionString + * @param string $versionString + * @param bool $expectedResult + */ + public function testDetectMb4WithMariaDB103($versionString, $expectedResult) { + $this->mockDatabaseEmptyVars(); + + $this->db->expects($this->once()) + ->method('getDatabaseVersionString') + ->willReturn($versionString); + + $this->assertEquals($expectedResult, $this->tools->supports4ByteCharset($this->db)); + } +}