Merge pull request #1773 from nextcloud/testing-characters-in-db

MySQL 4byte support

.drone.yml

@@ -148,6 +148,14 @@ pipeline:
       matrix:
         DB: postgres
         PHP: 5.6
+  mysqlmb4-php5.6:
+    image: nextcloudci/php5.6:php5.6-2
+    commands:
+      - NOCOVERAGE=true TEST_SELECTION=DB ./autotest.sh mysqlmb4
+    when:
+      matrix:
+        DB: mysqlmb4
+        PHP: 5.6
   integration-capabilities_features:
     image: nextcloudci/integration-php7.0:integration-php7.0-1
     commands:
@@ -368,6 +376,8 @@ matrix:
       PHP: 5.6
     - DB: postgres
       PHP: 5.6
+    - DB: mysqlmb4
+      PHP: 5.6
 
 services:
   cache:
@@ -390,3 +400,14 @@ services:
     when:
       matrix:
         DB: mysql
+  mysqlmb4:
+    image: mysql
+    environment:
+      - MYSQL_ROOT_PASSWORD=owncloud
+      - MYSQL_USER=oc_autotest
+      - MYSQL_PASSWORD=owncloud
+      - MYSQL_DATABASE=oc_autotest
+    command: [ "--innodb_large_prefix=true", "--innodb_file_format=barracuda", "--innodb_file_per_table=true" ]
+    when:
+      matrix:
+        DB: mysqlmb4

autotest.sh

@@ -21,7 +21,7 @@ ADMINLOGIN=admin$EXECUTOR_NUMBER
 BASEDIR=$PWD
 
 PRIMARY_STORAGE_CONFIGS="local swift"
-DBCONFIGS="sqlite mysql mariadb pgsql oci"
+DBCONFIGS="sqlite mysql mariadb pgsql oci mysqlmb4"
 
 # $PHP_EXE is run through 'which' and as such e.g. 'php' or 'hhvm' is usually
 # sufficient. Due to the behaviour of 'which', $PHP_EXE may also be a path
@@ -209,6 +209,48 @@ function execute_tests {
             exit 1
         fi
 	fi
+	if [ "$DB" == "mysqlmb4" ] ; then
+		if [ ! -z "$USEDOCKER" ] ; then
+			echo "Fire up the mysql docker"
+			DOCKER_CONTAINER_ID=$(docker run \
+				-v $BASEDIR/tests/docker/mysqlmb4:/etc/mysql/conf.d \
+				-e MYSQL_ROOT_PASSWORD=owncloud \
+				-e MYSQL_USER="$DATABASEUSER" \
+				-e MYSQL_PASSWORD=owncloud \
+				-e MYSQL_DATABASE="$DATABASENAME" \
+				-d mysql:5.7
+				--innodb_large_prefix=true
+				--innodb_file_format=barracuda
+				--innodb_file_per_table=true)
+
+			DATABASEHOST=$(docker inspect --format="{{.NetworkSettings.IPAddress}}" "$DOCKER_CONTAINER_ID")
+
+		else
+			if [ -z "$DRONE" ] ; then # no need to drop the DB when we are on CI
+				if [ "mysql" != "$(mysql --version | grep -o mysql)" ] ; then
+					echo "Your mysql binary is not provided by mysql"
+					echo "To use the docker container set the USEDOCKER environment variable"
+					exit -1
+				fi
+				mysql -u "$DATABASEUSER" -powncloud -e "DROP DATABASE IF EXISTS $DATABASENAME" -h $DATABASEHOST || true
+			else
+				DATABASEHOST=127.0.0.1
+			fi
+		fi
+
+		echo "Waiting for MySQL(utf8mb4) initialisation ..."
+
+		if ! apps/files_external/tests/env/wait-for-connection $DATABASEHOST 3306 60; then
+			echo "[ERROR] Waited 60 seconds, no response" >&2
+			exit 1
+		fi
+		sleep 1
+
+		echo "MySQL(utf8mb4)  is up."
+		_DB="mysql"
+
+		cp tests/docker/mysqlmb4.config.php config
+	fi
 	if [ "$DB" == "mariadb" ] ; then
 		if [ ! -z "$USEDOCKER" ] ; then
 			echo "Fire up the mariadb docker"

config/config.sample.php

@@ -129,6 +129,7 @@ $CONFIG = array(
  */
 'dbtableprefix' => '',
 
+
 /**
  * Indicates whether the Nextcloud instance was installed successfully; ``true``
  * indicates a successful installation, and ``false`` indicates an unsuccessful
@@ -1079,6 +1080,34 @@ $CONFIG = array(
  */
 'sqlite.journal_mode' => 'DELETE',
 
+/**
+ * If this setting is set to true MySQL can handle 4 byte characters instead of
+ * 3 byte characters
+ *
+ * MySQL requires a special setup for longer indexes (> 767 bytes) which are
+ * needed:
+ *
+ * [mysqld]
+ * innodb_large_prefix=true
+ * innodb_file_format=barracuda
+ * innodb_file_per_table=true
+ *
+ * Tables will be created with
+ *  * character set: utf8mb4
+ *  * collation:     utf8mb4_bin
+ *  * row_format:    compressed
+ *
+ * See:
+ * https://dev.mysql.com/doc/refman/5.7/en/charset-unicode-utf8mb4.html
+ * https://dev.mysql.com/doc/refman/5.7/en/innodb-parameters.html#sysvar_innodb_large_prefix
+ * https://mariadb.com/kb/en/mariadb/xtradbinnodb-server-system-variables/#innodb_large_prefix
+ * http://www.tocker.ca/2013/10/31/benchmarking-innodb-page-compression-performance.html
+ * http://mechanics.flite.com/blog/2014/07/29/using-innodb-large-prefix-to-avoid-error-1071/
+ *
+ * WARNING: EXPERIMENTAL
+ */
+'mysql.utf8mb4' => false,
+
 /**
  * Database types that are supported for installation.
  *

core/register_command.php

@@ -83,7 +83,7 @@ if (\OC::$server->getConfig()->getSystemValue('installed', false)) {
 	$application->add(new OC\Core\Command\Config\System\SetConfig(\OC::$server->getSystemConfig()));
 
 	$application->add(new OC\Core\Command\Db\GenerateChangeScript());
-	$application->add(new OC\Core\Command\Db\ConvertType(\OC::$server->getConfig(), new \OC\DB\ConnectionFactory()));
+	$application->add(new OC\Core\Command\Db\ConvertType(\OC::$server->getConfig(), new \OC\DB\ConnectionFactory(\OC::$server->getConfig())));
 
 	$application->add(new OC\Core\Command\Encryption\Disable(\OC::$server->getConfig()));
 	$application->add(new OC\Core\Command\Encryption\Enable(\OC::$server->getConfig(), \OC::$server->getEncryptionManager()));

lib/private/DB/AdapterMySQL.php

@@ -27,6 +27,9 @@ namespace OC\DB;
 
 class AdapterMySQL extends Adapter {
 
+	/** @var string */
+	protected $charset;
+
 	/**
 	 * @param string $tableName
 	 */
@@ -39,7 +42,16 @@ class AdapterMySQL extends Adapter {
 	}
 
 	public function fixupStatement($statement) {
-		$statement = str_replace(' ILIKE ', ' COLLATE utf8_general_ci LIKE ', $statement);
+		$statement = str_replace(' ILIKE ', ' COLLATE ' . $this->getCharset() . '_general_ci LIKE ', $statement);
 		return $statement;
 	}
+
+	protected function getCharset() {
+		if (!$this->charset) {
+			$params = $this->conn->getParams();
+			$this->charset = isset($params['charset']) ? $params['charset'] : 'utf8';
+		}
+
+		return $this->charset;
+	}
 }

lib/private/DB/ConnectionFactory.php

@@ -28,6 +28,7 @@ namespace OC\DB;
 use Doctrine\DBAL\Event\Listeners\OracleSessionInit;
 use Doctrine\DBAL\Event\Listeners\SQLSessionInit;
 use Doctrine\DBAL\Event\Listeners\MysqlSessionInit;
+use OCP\IConfig;
 
 /**
 * Takes care of creating and configuring Doctrine connections.
@@ -64,6 +65,12 @@ class ConnectionFactory {
 		),
 	);
 
+	public function __construct(IConfig $config) {
+		if($config->getSystemValue('mysql.utf8mb4', false)) {
+			$this->defaultConnectionParams['mysql']['charset'] = 'utf8mb4';
+		}
+	}
+
 	/**
 	* @brief Get default connection parameters for a given DBMS.
 	* @param string $type DBMS type
@@ -99,7 +106,9 @@ class ConnectionFactory {
 			case 'mysql':
 				// Send "SET NAMES utf8". Only required on PHP 5.3 below 5.3.6.
 				// See http://stackoverflow.com/questions/4361459/php-pdo-charset-set-names#4361485
-				$eventManager->addEventSubscriber(new MysqlSessionInit);
+				$eventManager->addEventSubscriber(new MysqlSessionInit(
+					$this->defaultConnectionParams['mysql']['charset']
+				));
 				$eventManager->addEventSubscriber(
 					new SQLSessionInit("SET SESSION AUTOCOMMIT=1"));
 				break;

lib/private/DB/MDB2SchemaReader.php

@@ -33,6 +33,7 @@ namespace OC\DB;
 
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\SchemaConfig;
+use Doctrine\DBAL\Platforms\MySqlPlatform;
 use OCP\IConfig;
 
 class MDB2SchemaReader {
@@ -54,12 +55,16 @@ class MDB2SchemaReader {
 	/** @var \Doctrine\DBAL\Schema\SchemaConfig $schemaConfig */
 	protected $schemaConfig;
 
+	/** @var IConfig */
+	protected $config;
+
 	/**
 	 * @param \OCP\IConfig $config
 	 * @param \Doctrine\DBAL\Platforms\AbstractPlatform $platform
 	 */
 	public function __construct(IConfig $config, AbstractPlatform $platform) {
 		$this->platform = $platform;
+		$this->config = $config;
 		$this->DBNAME = $config->getSystemValue('dbname', 'owncloud');
 		$this->DBTABLEPREFIX = $config->getSystemValue('dbtableprefix', 'oc_');
 
@@ -118,8 +123,15 @@ class MDB2SchemaReader {
 					$name = str_replace('*dbprefix*', $this->DBTABLEPREFIX, $name);
 					$name = $this->platform->quoteIdentifier($name);
 					$table = $schema->createTable($name);
-					$table->addOption('collate', 'utf8_bin');
 					$table->setSchemaConfig($this->schemaConfig);
+
+					if($this->platform instanceof MySqlPlatform && $this->config->getSystemValue('mysql.utf8mb4', false)) {
+						$table->addOption('charset', 'utf8mb4');
+						$table->addOption('collate', 'utf8mb4_bin');
+						$table->addOption('row_format', 'compressed');
+					} else {
+						$table->addOption('collate', 'utf8_bin');
+					}
 					break;
 				case 'create':
 				case 'overwrite':

lib/private/DB/MDB2SchemaWriter.php

@@ -43,7 +43,11 @@ class MDB2SchemaWriter {
 		$xml->addChild('name', $config->getSystemValue('dbname', 'owncloud'));
 		$xml->addChild('create', 'true');
 		$xml->addChild('overwrite', 'false');
-		$xml->addChild('charset', 'utf8');
+		if($config->getSystemValue('dbtype', 'sqlite') === 'mysql' && $config->getSystemValue('mysql.utf8mb4', false)) {
+			$xml->addChild('charset', 'utf8mb4');
+		} else {
+			$xml->addChild('charset', 'utf8');
+		}
 
 		// FIX ME: bloody work around
 		if ($config->getSystemValue('dbtype', 'sqlite') === 'oci') {

lib/private/DB/QueryBuilder/ExpressionBuilder/MySqlExpressionBuilder.php

@@ -24,18 +24,31 @@
 namespace OC\DB\QueryBuilder\ExpressionBuilder;
 
 
-use OC\DB\QueryBuilder\QueryFunction;
-use OCP\DB\QueryBuilder\IQueryBuilder;
+use OC\DB\Connection;
+use OCP\IDBConnection;
 
 class MySqlExpressionBuilder extends ExpressionBuilder {
 
+	/** @var string */
+	protected $charset;
+
+	/**
+	 * @param \OCP\IDBConnection|Connection $connection
+	 */
+	public function __construct(IDBConnection $connection) {
+		parent::__construct($connection);
+
+		$params = $connection->getParams();
+		$this->charset = isset($params['charset']) ? $params['charset'] : 'utf8';
+	}
+
 	/**
 	 * @inheritdoc
 	 */
 	public function iLike($x, $y, $type = null) {
 		$x = $this->helper->quoteColumnName($x);
 		$y = $this->helper->quoteColumnName($y);
-		return $this->expressionBuilder->comparison($x, ' COLLATE utf8_general_ci LIKE', $y);
+		return $this->expressionBuilder->comparison($x, ' COLLATE ' . $this->charset . '_general_ci LIKE', $y);
 	}
 
 }

lib/private/Repair.php

@@ -129,6 +129,7 @@ class Repair implements IOutput{
 	 */
 	public static function getRepairSteps() {
 		return [
+			new Collation(\OC::$server->getConfig(), \OC::$server->getLogger(), \OC::$server->getDatabaseConnection(), false),
 			new RepairMimeTypes(\OC::$server->getConfig()),
 			new RepairLegacyStorages(\OC::$server->getConfig(), \OC::$server->getDatabaseConnection()),
 			new AssetCache(),
@@ -179,7 +180,7 @@ class Repair implements IOutput{
 		$connection = \OC::$server->getDatabaseConnection();
 		$steps = [
 			new InnoDB(),
-			new Collation(\OC::$server->getConfig(), $connection),
+			new Collation(\OC::$server->getConfig(), \OC::$server->getLogger(), $connection, true),
 			new SqliteAutoincrement($connection),
 			new SearchLuceneTables(),
 		];

lib/private/Repair/Collation.php

@@ -24,28 +24,38 @@
 
 namespace OC\Repair;
 
+use Doctrine\DBAL\Exception\DriverException;
 use Doctrine\DBAL\Platforms\MySqlPlatform;
+use OCP\IConfig;
+use OCP\IDBConnection;
+use OCP\ILogger;
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
 
 class Collation implements IRepairStep {
-	/**
-	 * @var \OCP\IConfig
-	 */
+	/**  @var IConfig */
 	protected $config;
 
-	/**
-	 * @var \OC\DB\Connection
-	 */
+	/** @var ILogger */
+	protected $logger;
+
+	/** @var IDBConnection */
 	protected $connection;
 
+	/** @var bool */
+	protected $ignoreFailures;
+
 	/**
-	 * @param \OCP\IConfig $config
-	 * @param \OC\DB\Connection $connection
+	 * @param IConfig $config
+	 * @param ILogger $logger
+	 * @param IDBConnection $connection
+	 * @param bool $ignoreFailures
 	 */
-	public function __construct($config, $connection) {
+	public function __construct(IConfig $config, ILogger $logger, IDBConnection $connection, $ignoreFailures) {
 		$this->connection = $connection;
 		$this->config = $config;
+		$this->logger = $logger;
+		$this->ignoreFailures = $ignoreFailures;
 	}
 
 	public function getName() {
@@ -61,11 +71,21 @@ class Collation implements IRepairStep {
 			return;
 		}
 
+		$characterSet = $this->config->getSystemValue('mysql.utf8mb4', false) ? 'utf8mb4' : 'utf8';
+
 		$tables = $this->getAllNonUTF8BinTables($this->connection);
 		foreach ($tables as $table) {
 			$output->info("Change collation for $table ...");
-			$query = $this->connection->prepare('ALTER TABLE `' . $table . '` CONVERT TO CHARACTER SET utf8 COLLATE utf8_bin;');
-			$query->execute();
+			$query = $this->connection->prepare('ALTER TABLE `' . $table . '` CONVERT TO CHARACTER SET ' . $characterSet . ' COLLATE ' . $characterSet . '_bin;');
+			try {
+				$query->execute();
+			} catch (DriverException $e) {
+				// Just log this
+				$this->logger->logException($e);
+				if (!$this->ignoreFailures) {
+					throw $e;
+				}
+			}
 		}
 	}
 
@@ -75,11 +95,12 @@ class Collation implements IRepairStep {
 	 */
 	protected function getAllNonUTF8BinTables($connection) {
 		$dbName = $this->config->getSystemValue("dbname");
+		$characterSet = $this->config->getSystemValue('mysql.utf8mb4', false) ? 'utf8mb4' : 'utf8';
 		$rows = $connection->fetchAll(
 			"SELECT DISTINCT(TABLE_NAME) AS `table`" .
 			"	FROM INFORMATION_SCHEMA . COLUMNS" .
 			"	WHERE TABLE_SCHEMA = ?" .
-			"	AND (COLLATION_NAME <> 'utf8_bin' OR CHARACTER_SET_NAME <> 'utf8')" .
+			"	AND (COLLATION_NAME <> '" . $characterSet . "_bin' OR CHARACTER_SET_NAME <> '" . $characterSet . "')" .
 			"	AND TABLE_NAME LIKE \"*PREFIX*%\"",
 			array($dbName)
 		);

lib/private/Server.php

@@ -407,8 +407,8 @@ class Server extends ServerContainer implements IServerContainer {
 			return new CredentialsManager($c->getCrypto(), $c->getDatabaseConnection());
 		});
 		$this->registerService('DatabaseConnection', function (Server $c) {
-			$factory = new \OC\DB\ConnectionFactory();
 			$systemConfig = $c->getSystemConfig();
+			$factory = new \OC\DB\ConnectionFactory($c->getConfig());
 			$type = $systemConfig->getValue('dbtype', 'sqlite');
 			if (!$factory->isValidType($type)) {
 				throw new \OC\DatabaseException('Invalid database type');

lib/private/Setup/AbstractDatabase.php

@@ -134,7 +134,7 @@ abstract class AbstractDatabase {
 		}
 
 		$connectionParams = array_merge($connectionParams, $configOverwrite);
-		$cf = new ConnectionFactory();
+		$cf = new ConnectionFactory($this->config);
 		return $cf->getConnection($this->config->getSystemValue('dbtype', 'sqlite'), $connectionParams);
 	}
 

lib/private/Setup/MySQL.php

@@ -58,8 +58,9 @@ class MySQL extends AbstractDatabase {
 		try{
 			$name = $this->dbName;
 			$user = $this->dbUser;
-			//we can't use OC_BD functions here because we need to connect as the administrative user.
-			$query = "CREATE DATABASE IF NOT EXISTS `$name` CHARACTER SET utf8 COLLATE utf8_bin;";
+			//we can't use OC_DB functions here because we need to connect as the administrative user.
+			$characterSet = \OC::$server->getSystemConfig()->getValue('mysql.utf8mb4', false) ? 'utf8mb4' : 'utf8';
+			$query = "CREATE DATABASE IF NOT EXISTS `$name` CHARACTER SET $characterSet COLLATE ${characterSet}_bin;";
 			$connection->executeUpdate($query);
 		} catch (\Exception $ex) {
 			$this->logger->error('Database creation failed: {error}', [

tests/data/db_structure.xml

@@ -293,4 +293,19 @@
 
 	</table>
 
+  <table>
+
+		<name>*dbprefix*text_table</name>
+		<declaration>
+
+         <field>
+          <name>textfield</name>
+          <type>text</type>
+          <notnull>false</notnull>
+          <length>255</length>
+         </field>
+
+		</declaration>
+  </table>
+
 </database>

tests/docker/mysqlmb4.config.php

@@ -0,0 +1,5 @@
+<?php
+
+$CONFIG = array(
+	'mysql.utf8mb4' => true,
+);

tests/docker/mysqlmb4/mb4.cnf

@@ -0,0 +1,5 @@
+
+[mysqld]
+innodb_large_prefix=true
+innodb_file_format=barracuda
+innodb_file_per_table=true

tests/lib/DB/LegacyDBTest.php

@@ -46,6 +46,11 @@ class LegacyDBTest extends \Test\TestCase {
 	 */
 	private $table5;
 
+	/**
+	 * @var string
+	 */
+	private $text_table;
+
 	protected function setUp() {
 		parent::setUp();
 
@@ -63,6 +68,7 @@ class LegacyDBTest extends \Test\TestCase {
 		$this->table3 = $this->test_prefix.'vcategory';
 		$this->table4 = $this->test_prefix.'decimal';
 		$this->table5 = $this->test_prefix.'uniconst';
+		$this->text_table = $this->test_prefix.'text_table';
 	}
 
 	protected function tearDown() {
@@ -390,4 +396,33 @@ class LegacyDBTest extends \Test\TestCase {
 		$result = $query->execute(array('%ba%'));
 		$this->assertCount(1, $result->fetchAll());
 	}
+
+	/**
+	 * @dataProvider insertAndSelectDataProvider
+	 */
+	public function testInsertAndSelectData($expected, $throwsOnMysqlWithoutUTF8MB4) {
+		$table = "*PREFIX*{$this->text_table}";
+		$config = \OC::$server->getConfig();
+
+		$query = OC_DB::prepare("INSERT INTO `$table` (`textfield`) VALUES (?)");
+		if ($throwsOnMysqlWithoutUTF8MB4 && $config->getSystemValue('dbtype', 'sqlite') === 'mysql' && $config->getSystemValue('mysql.utf8mb4', false) === false) {
+			$this->markTestSkipped('MySQL requires UTF8mb4 to store value: ' . $expected);
+		}
+		$result = $query->execute(array($expected));
+		$this->assertEquals(1, $result);
+
+		$actual = OC_DB::prepare("SELECT `textfield` FROM `$table`")->execute()->fetchOne();
+		$this->assertSame($expected, $actual);
+	}
+
+	public function insertAndSelectDataProvider() {
+		return [
+			['abcdefghijklmnopqrstuvwxyzABCDEFGHIKLMNOPQRSTUVWXYZ', false],
+			['0123456789', false],
+			['äöüÄÖÜß!"§$%&/()=?#\'+*~°^`´', false],
+			['²³¼½¬{[]}\\', false],
+			['♡⚗', false],
+			['💩', true], # :hankey: on github
+		];
+	}
 }

tests/lib/Files/Cache/CacheTest.php

@@ -113,7 +113,8 @@ class CacheTest extends \Test\TestCase {
 	public function testFolder($folder) {
 		if(strpos($folder, 'F09F9890')) {
 			// 4 byte UTF doesn't work on mysql
-			if(\OC::$server->getDatabaseConnection()->getDatabasePlatform() instanceof MySqlPlatform) {
+			$params = \OC::$server->getDatabaseConnection()->getParams();
+			if(\OC::$server->getDatabaseConnection()->getDatabasePlatform() instanceof MySqlPlatform && $params['charset'] !== 'utf8mb4') {
 				$this->markTestSkipped('MySQL doesn\'t support 4 byte UTF-8');
 			}
 		}

tests/lib/Repair/RepairCollationTest.php

@@ -1,9 +1,4 @@
 <?php
-
-namespace Test\Repair;
-
-use OCP\Migration\IOutput;
-
 /**
  * Copyright (c) 2014 Thomas Müller <deepdiver@owncloud.com>
  * This file is licensed under the Affero General Public License version 3 or
@@ -11,6 +6,11 @@ use OCP\Migration\IOutput;
  * See the COPYING-README file.
  */
 
+namespace Test\Repair;
+
+use OCP\ILogger;
+use OCP\Migration\IOutput;
+
 class TestCollationRepair extends \OC\Repair\Collation {
 	/**
 	 * @param \Doctrine\DBAL\Connection $connection
@@ -50,10 +50,14 @@ class RepairCollationTest extends \Test\TestCase {
 	 */
 	private $config;
 
+	/** @var ILogger */
+	private $logger;
+
 	protected function setUp() {
 		parent::setUp();
 
 		$this->connection = \OC::$server->getDatabaseConnection();
+		$this->logger = $this->createMock(ILogger::class);
 		$this->config = \OC::$server->getConfig();
 		if (!$this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\MySqlPlatform) {
 			$this->markTestSkipped("Test only relevant on MySql");
@@ -63,7 +67,7 @@ class RepairCollationTest extends \Test\TestCase {
 		$this->tableName = $this->getUniqueID($dbPrefix . "_collation_test");
 		$this->connection->exec("CREATE TABLE $this->tableName(text VARCHAR(16)) COLLATE utf8_unicode_ci");
 
-		$this->repair = new TestCollationRepair($this->config, $this->connection);
+		$this->repair = new TestCollationRepair($this->config, $this->logger, $this->connection, false);
 	}
 
 	protected function tearDown() {