]> source.dussan.org Git - nextcloud-server.git/commitdiff
Fix namespace duplication and other issues in repairlegacystorages
authorJoas Schilling <nickvergessen@owncloud.com>
Mon, 13 Apr 2015 14:34:10 +0000 (16:34 +0200)
committerJoas Schilling <nickvergessen@owncloud.com>
Mon, 13 Apr 2015 14:34:10 +0000 (16:34 +0200)
lib/private/repair/repairlegacystorages.php [deleted file]
lib/private/user.php
lib/repair/repairlegacystorages.php [new file with mode: 0644]
tests/lib/repair/repairlegacystorage.php

diff --git a/lib/private/repair/repairlegacystorages.php b/lib/private/repair/repairlegacystorages.php
deleted file mode 100644 (file)
index c90b144..0000000
+++ /dev/null
@@ -1,260 +0,0 @@
-<?php
-/**
- * @author Morris Jobke <hey@morrisjobke.de>
- * @author Vincent Petry <pvince81@owncloud.com>
- *
- * @copyright Copyright (c) 2015, 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 <http://www.gnu.org/licenses/>
- *
- */
-
-namespace OC\Repair;
-
-use OC\Hooks\BasicEmitter;
-
-class RepairLegacyStorages extends BasicEmitter {
-       /**
-        * @var \OCP\IConfig
-        */
-       protected $config;
-
-       /**
-        * @var \OC\DB\Connection
-        */
-       protected $connection;
-
-       protected $findStorageInCacheStatement;
-       protected $renameStorageStatement;
-
-       /**
-        * @param \OCP\IConfig $config
-        * @param \OC\DB\Connection $connection
-        */
-       public function __construct($config, $connection) {
-               $this->connection = $connection;
-               $this->config = $config;
-
-               $this->findStorageInCacheStatement = $this->connection->prepare(
-                       'SELECT DISTINCT `storage` FROM `*PREFIX*filecache`'
-                       . ' WHERE `storage` in (?, ?)'
-               );
-               $this->renameStorageStatement = $this->connection->prepare(
-                       'UPDATE `*PREFIX*storages`'
-                       . ' SET `id` = ?'
-                       . ' WHERE `id` = ?'
-               );
-       }
-
-       public function getName() {
-               return 'Repair legacy storages';
-       }
-
-       /**
-        * Extracts the user id from a legacy storage id
-        *
-        * @param string $storageId legacy storage id in the
-        * format "local::/path/to/datadir/userid"
-        * @return string user id extracted from the storage id
-        */
-       private function extractUserId($storageId) {
-               $storageId = rtrim($storageId, '/');
-               $pos = strrpos($storageId, '/');
-               return substr($storageId, $pos + 1);
-       }
-
-       /**
-        * Fix the given legacy storage by renaming the old id
-        * to the new id. If the new id already exists, whichever
-        * storage that has data in the file cache will be used.
-        * If both have data, nothing will be done and false is
-        * returned.
-        *
-        * @param string $oldId old storage id
-        * @param int $oldNumericId old storage numeric id
-        *
-        * @return bool true if fixed, false otherwise
-        */
-       private function fixLegacyStorage($oldId, $oldNumericId, $userId = null) {
-               // check whether the new storage already exists
-               if (is_null($userId)) {
-                       $userId = $this->extractUserId($oldId);
-               }
-               $newId = 'home::' . $userId;
-
-               // check if target id already exists
-               $newNumericId = \OC\Files\Cache\Storage::getNumericStorageId($newId);
-               if (!is_null($newNumericId)) {
-                       $newNumericId = (int)$newNumericId;
-                       // try and resolve the conflict
-                       // check which one of "local::" or "home::" needs to be kept
-                       $result = $this->findStorageInCacheStatement->execute(array($oldNumericId, $newNumericId));
-                       $row1 = $this->findStorageInCacheStatement->fetch();
-                       $row2 = $this->findStorageInCacheStatement->fetch();
-                       $this->findStorageInCacheStatement->closeCursor();
-                       if ($row2 !== false) {
-                               // two results means both storages have data, not auto-fixable
-                               throw new \OC\RepairException(
-                                       'Could not automatically fix legacy storage '
-                                       . '"' . $oldId . '" => "' . $newId . '"'
-                                       . ' because they both have data.'
-                               );
-                       }
-                       if ($row1 === false || (int)$row1['storage'] === $oldNumericId) {
-                               // old storage has data, then delete the empty new id
-                               $toDelete = $newId;
-                       } else if ((int)$row1['storage'] === $newNumericId) {
-                               // new storage has data, then delete the empty old id
-                               $toDelete = $oldId;
-                       } else {
-                               // unknown case, do not continue
-                               return false;
-                       }
-
-                       // delete storage including file cache
-                       \OC\Files\Cache\Storage::remove($toDelete);
-
-                       // if we deleted the old id, the new id will be used
-                       // automatically
-                       if ($toDelete === $oldId) {
-                               // nothing more to do
-                               return true;
-                       }
-               }
-
-               // rename old id to new id
-               $newId = \OC\Files\Cache\Storage::adjustStorageId($newId);
-               $oldId = \OC\Files\Cache\Storage::adjustStorageId($oldId);
-               $rowCount = $this->renameStorageStatement->execute(array($newId, $oldId));
-               $this->renameStorageStatement->closeCursor();
-               return ($rowCount === 1);
-       }
-
-       /**
-        * Converts legacy home storage ids in the format
-        * "local::/data/dir/path/userid/" to the new format "home::userid"
-        */
-       public function run() {
-               // only run once
-               if ($this->config->getAppValue('core', 'repairlegacystoragesdone') === 'yes') {
-                       return;
-               }
-
-               $dataDir = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/');
-               $dataDir = rtrim($dataDir, '/') . '/';
-               $dataDirId = 'local::' . $dataDir;
-
-               $count = 0;
-               $hasWarnings = false;
-
-               $this->connection->beginTransaction();
-
-               // note: not doing a direct UPDATE with the REPLACE function
-               // because regexp search/extract is needed and it is not guaranteed
-               // to work on all database types
-               $sql = 'SELECT `id`, `numeric_id` FROM `*PREFIX*storages`'
-                       . ' WHERE `id` LIKE ?'
-                       . ' ORDER BY `id`';
-               $result = $this->connection->executeQuery($sql, array($dataDirId . '%'));
-
-               while ($row = $result->fetch()) {
-                       $currentId = $row['id'];
-                       // one entry is the datadir itself
-                       if ($currentId === $dataDirId) {
-                               continue;
-                       }
-
-                       try {
-                               if ($this->fixLegacyStorage($currentId, (int)$row['numeric_id'])) {
-                                       $count++;
-                               }
-                       }
-                       catch (\OC\RepairException $e) {
-                               $hasWarnings = true;
-                               $this->emit(
-                                       '\OC\Repair',
-                                       'warning',
-                                       array('Could not repair legacy storage ' . $currentId . ' automatically.')
-                               );
-                       }
-               }
-
-               // check for md5 ids, not in the format "prefix::"
-               $sql = 'SELECT COUNT(*) AS "c" FROM `*PREFIX*storages`'
-                       . ' WHERE `id` NOT LIKE \'%::%\'';
-               $result = $this->connection->executeQuery($sql);
-               $row = $result->fetch();
-
-               // find at least one to make sure it's worth
-               // querying the user list
-               if ((int)$row['c'] > 0) {
-                       $userManager = \OC_User::getManager();
-
-                       // use chunks to avoid caching too many users in memory
-                       $limit = 30;
-                       $offset = 0;
-
-                       do {
-                               // query the next page of users
-                               $results = $userManager->search('', $limit, $offset);
-                               $storageIds = array();
-                               $userIds = array();
-                               foreach ($results as $uid => $userObject) {
-                                       $storageId = $dataDirId . $uid . '/';
-                                       if (strlen($storageId) <= 64) {
-                                               // skip short storage ids as they were handled in the previous section
-                                               continue;
-                                       }
-                                       $storageIds[$uid] = $storageId;
-                               }
-
-                               if (count($storageIds) > 0) {
-                                       // update the storages of these users
-                                       foreach ($storageIds as $uid => $storageId) {
-                                               $numericId = \OC\Files\Cache\Storage::getNumericStorageId($storageId);
-                                               try {
-                                                       if (!is_null($numericId) && $this->fixLegacyStorage($storageId, (int)$numericId)) {
-                                                               $count++;
-                                                       }
-                                               }
-                                               catch (\OC\RepairException $e) {
-                                                       $hasWarnings = true;
-                                                       $this->emit(
-                                                               '\OC\Repair',
-                                                               'warning',
-                                                               array('Could not repair legacy storage ' . $storageId . ' automatically.')
-                                                       );
-                                               }
-                                       }
-                               }
-                               $offset += $limit;
-                       } while (count($results) >= $limit);
-               }
-
-               $this->emit('\OC\Repair', 'info', array('Updated ' . $count . ' legacy home storage ids'));
-
-               $this->connection->commit();
-
-               if ($hasWarnings) {
-                       $this->emit(
-                               '\OC\Repair',
-                               'warning',
-                               array('Some legacy storages could not be repaired. Please manually fix them then re-run ./occ maintenance:repair')
-                       );
-               } else {
-                       // if all were done, no need to redo the repair during next upgrade
-                       $this->config->setAppValue('core', 'repairlegacystoragesdone', 'yes');
-               }
-       }
-}
index e6e3e8a7f2090aff95b61ee9b1a3360401ce3e5e..b3677233825a9283716fea430824cf1019744b68 100644 (file)
@@ -216,7 +216,7 @@ class OC_User {
         * @return bool
         *
         * Deletes a user
-        * @deprecated Use \OC::$server->getUserManager->delete()
+        * @deprecated Use \OC::$server->getUserManager()->get() and then run delete() on the return
         */
        public static function deleteUser($uid) {
                $user = self::getManager()->get($uid);
diff --git a/lib/repair/repairlegacystorages.php b/lib/repair/repairlegacystorages.php
new file mode 100644 (file)
index 0000000..7086f2a
--- /dev/null
@@ -0,0 +1,262 @@
+<?php
+/**
+ * @author Morris Jobke <hey@morrisjobke.de>
+ * @author Vincent Petry <pvince81@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, 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 <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OC\Repair;
+
+use OC\Files\Cache\Storage;
+use OC\Hooks\BasicEmitter;
+use OC\RepairException;
+
+class RepairLegacyStorages extends BasicEmitter {
+       /**
+        * @var \OCP\IConfig
+        */
+       protected $config;
+
+       /**
+        * @var \OCP\IDBConnection
+        */
+       protected $connection;
+
+       protected $findStorageInCacheStatement;
+       protected $renameStorageStatement;
+
+       /**
+        * @param \OCP\IConfig $config
+        * @param \OCP\IDBConnection $connection
+        */
+       public function __construct($config, $connection) {
+               $this->connection = $connection;
+               $this->config = $config;
+
+               $this->findStorageInCacheStatement = $this->connection->prepare(
+                       'SELECT DISTINCT `storage` FROM `*PREFIX*filecache`'
+                       . ' WHERE `storage` in (?, ?)'
+               );
+               $this->renameStorageStatement = $this->connection->prepare(
+                       'UPDATE `*PREFIX*storages`'
+                       . ' SET `id` = ?'
+                       . ' WHERE `id` = ?'
+               );
+       }
+
+       public function getName() {
+               return 'Repair legacy storages';
+       }
+
+       /**
+        * Extracts the user id from a legacy storage id
+        *
+        * @param string $storageId legacy storage id in the
+        * format "local::/path/to/datadir/userid"
+        * @return string user id extracted from the storage id
+        */
+       private function extractUserId($storageId) {
+               $storageId = rtrim($storageId, '/');
+               $pos = strrpos($storageId, '/');
+               return substr($storageId, $pos + 1);
+       }
+
+       /**
+        * Fix the given legacy storage by renaming the old id
+        * to the new id. If the new id already exists, whichever
+        * storage that has data in the file cache will be used.
+        * If both have data, nothing will be done and false is
+        * returned.
+        *
+        * @param string $oldId old storage id
+        * @param int $oldNumericId old storage numeric id
+        * @param string $userId
+        * @return bool true if fixed, false otherwise
+        * @throws RepairException
+        */
+       private function fixLegacyStorage($oldId, $oldNumericId, $userId = null) {
+               // check whether the new storage already exists
+               if (is_null($userId)) {
+                       $userId = $this->extractUserId($oldId);
+               }
+               $newId = 'home::' . $userId;
+
+               // check if target id already exists
+               $newNumericId = Storage::getNumericStorageId($newId);
+               if (!is_null($newNumericId)) {
+                       $newNumericId = (int)$newNumericId;
+                       // try and resolve the conflict
+                       // check which one of "local::" or "home::" needs to be kept
+                       $result = $this->findStorageInCacheStatement->execute(array($oldNumericId, $newNumericId));
+                       $row1 = $this->findStorageInCacheStatement->fetch();
+                       $row2 = $this->findStorageInCacheStatement->fetch();
+                       $this->findStorageInCacheStatement->closeCursor();
+                       if ($row2 !== false) {
+                               // two results means both storages have data, not auto-fixable
+                               throw new RepairException(
+                                       'Could not automatically fix legacy storage '
+                                       . '"' . $oldId . '" => "' . $newId . '"'
+                                       . ' because they both have data.'
+                               );
+                       }
+                       if ($row1 === false || (int)$row1['storage'] === $oldNumericId) {
+                               // old storage has data, then delete the empty new id
+                               $toDelete = $newId;
+                       } else if ((int)$row1['storage'] === $newNumericId) {
+                               // new storage has data, then delete the empty old id
+                               $toDelete = $oldId;
+                       } else {
+                               // unknown case, do not continue
+                               return false;
+                       }
+
+                       // delete storage including file cache
+                       Storage::remove($toDelete);
+
+                       // if we deleted the old id, the new id will be used
+                       // automatically
+                       if ($toDelete === $oldId) {
+                               // nothing more to do
+                               return true;
+                       }
+               }
+
+               // rename old id to new id
+               $newId = Storage::adjustStorageId($newId);
+               $oldId = Storage::adjustStorageId($oldId);
+               $rowCount = $this->renameStorageStatement->execute(array($newId, $oldId));
+               $this->renameStorageStatement->closeCursor();
+               return ($rowCount === 1);
+       }
+
+       /**
+        * Converts legacy home storage ids in the format
+        * "local::/data/dir/path/userid/" to the new format "home::userid"
+        */
+       public function run() {
+               // only run once
+               if ($this->config->getAppValue('core', 'repairlegacystoragesdone') === 'yes') {
+                       return;
+               }
+
+               $dataDir = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/');
+               $dataDir = rtrim($dataDir, '/') . '/';
+               $dataDirId = 'local::' . $dataDir;
+
+               $count = 0;
+               $hasWarnings = false;
+
+               $this->connection->beginTransaction();
+
+               // note: not doing a direct UPDATE with the REPLACE function
+               // because regexp search/extract is needed and it is not guaranteed
+               // to work on all database types
+               $sql = 'SELECT `id`, `numeric_id` FROM `*PREFIX*storages`'
+                       . ' WHERE `id` LIKE ?'
+                       . ' ORDER BY `id`';
+               $result = $this->connection->executeQuery($sql, array($dataDirId . '%'));
+
+               while ($row = $result->fetch()) {
+                       $currentId = $row['id'];
+                       // one entry is the datadir itself
+                       if ($currentId === $dataDirId) {
+                               continue;
+                       }
+
+                       try {
+                               if ($this->fixLegacyStorage($currentId, (int)$row['numeric_id'])) {
+                                       $count++;
+                               }
+                       }
+                       catch (RepairException $e) {
+                               $hasWarnings = true;
+                               $this->emit(
+                                       '\OC\Repair',
+                                       'warning',
+                                       array('Could not repair legacy storage ' . $currentId . ' automatically.')
+                               );
+                       }
+               }
+
+               // check for md5 ids, not in the format "prefix::"
+               $sql = 'SELECT COUNT(*) AS "c" FROM `*PREFIX*storages`'
+                       . ' WHERE `id` NOT LIKE \'%::%\'';
+               $result = $this->connection->executeQuery($sql);
+               $row = $result->fetch();
+
+               // find at least one to make sure it's worth
+               // querying the user list
+               if ((int)$row['c'] > 0) {
+                       $userManager = \OC::$server->getUserManager();
+
+                       // use chunks to avoid caching too many users in memory
+                       $limit = 30;
+                       $offset = 0;
+
+                       do {
+                               // query the next page of users
+                               $results = $userManager->search('', $limit, $offset);
+                               $storageIds = array();
+                               foreach ($results as $uid => $userObject) {
+                                       $storageId = $dataDirId . $uid . '/';
+                                       if (strlen($storageId) <= 64) {
+                                               // skip short storage ids as they were handled in the previous section
+                                               continue;
+                                       }
+                                       $storageIds[$uid] = $storageId;
+                               }
+
+                               if (count($storageIds) > 0) {
+                                       // update the storages of these users
+                                       foreach ($storageIds as $uid => $storageId) {
+                                               $numericId = Storage::getNumericStorageId($storageId);
+                                               try {
+                                                       if (!is_null($numericId) && $this->fixLegacyStorage($storageId, (int)$numericId)) {
+                                                               $count++;
+                                                       }
+                                               }
+                                               catch (RepairException $e) {
+                                                       $hasWarnings = true;
+                                                       $this->emit(
+                                                               '\OC\Repair',
+                                                               'warning',
+                                                               array('Could not repair legacy storage ' . $storageId . ' automatically.')
+                                                       );
+                                               }
+                                       }
+                               }
+                               $offset += $limit;
+                       } while (count($results) >= $limit);
+               }
+
+               $this->emit('\OC\Repair', 'info', array('Updated ' . $count . ' legacy home storage ids'));
+
+               $this->connection->commit();
+
+               if ($hasWarnings) {
+                       $this->emit(
+                               '\OC\Repair',
+                               'warning',
+                               array('Some legacy storages could not be repaired. Please manually fix them then re-run ./occ maintenance:repair')
+                       );
+               } else {
+                       // if all were done, no need to redo the repair during next upgrade
+                       $this->config->setAppValue('core', 'repairlegacystoragesdone', 'yes');
+               }
+       }
+}
index 2df0f6b94b495c9c0082774ce012427ceeccc722..e1edf70442402bcb365e3cc1ed72587fd5e6d645 100644 (file)
@@ -5,18 +5,25 @@
  * later.
  * See the COPYING-README file.
  */
+
 namespace Test\Repair;
 
+use OC\Files\Cache\Cache;
+use OC\Files\Cache\Storage;
+use Test\TestCase;
+
 /**
  * Tests for the converting of legacy storages to home storages.
  *
  * @see \OC\Repair\RepairLegacyStorages
  */
-class RepairLegacyStorages extends \Test\TestCase {
-
+class RepairLegacyStorages extends TestCase {
+       /** @var \OCP\IDBConnection */
        private $connection;
+       /** @var \OCP\IConfig */
        private $config;
        private $user;
+       /** @var \OC\Repair\RepairLegacyStorages */
        private $repair;
 
        private $dataDir;
@@ -31,7 +38,7 @@ class RepairLegacyStorages extends \Test\TestCase {
                parent::setUp();
 
                $this->config = \OC::$server->getConfig();
-               $this->connection = \OC_DB::getConnection();
+               $this->connection = \OC::$server->getDatabaseConnection();
                $this->oldDataDir = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/');
 
                $this->repair = new \OC\Repair\RepairLegacyStorages($this->config, $this->connection);
@@ -44,40 +51,49 @@ class RepairLegacyStorages extends \Test\TestCase {
        }
 
        protected function tearDown() {
-               \OC_User::deleteUser($this->user);
+               $user = \OC::$server->getUserManager()->get($this->user);
+               if ($user) {
+                       $user->delete();
+               }
 
                $sql = 'DELETE FROM `*PREFIX*storages`';
                $this->connection->executeQuery($sql);
                $sql = 'DELETE FROM `*PREFIX*filecache`';
                $this->connection->executeQuery($sql);
-               \OCP\Config::setSystemValue('datadirectory', $this->oldDataDir);
+               $this->config->setSystemValue('datadirectory', $this->oldDataDir);
                $this->config->setAppValue('core', 'repairlegacystoragesdone', 'no');
 
                parent::tearDown();
        }
 
+       /**
+        * @param string $dataDir
+        * @param string $userId
+        * @throws \Exception
+        */
        function prepareSettings($dataDir, $userId) {
                // hard-coded string as we want a predictable fixed length
                // no data will be written there
                $this->dataDir = $dataDir;
-               \OCP\Config::setSystemValue('datadirectory', $this->dataDir);
+               $this->config->setSystemValue('datadirectory', $this->dataDir);
 
                $this->user = $userId;
                $this->legacyStorageId = 'local::' . $this->dataDir . $this->user . '/';
                $this->newStorageId = 'home::' . $this->user;
-               \OC_User::createUser($this->user, $this->user);
+               \OC::$server->getUserManager()->createUser($this->user, $this->user);
        }
 
        /**
         * Create a storage entry
         *
         * @param string $storageId
+        * @return int
         */
        private function createStorage($storageId) {
                $sql = 'INSERT INTO `*PREFIX*storages` (`id`)'
                        . ' VALUES (?)';
 
-               $storageId = \OC\Files\Cache\Storage::adjustStorageId($storageId);
+               $storageId = Storage::adjustStorageId($storageId);
                $numRows = $this->connection->executeUpdate($sql, array($storageId));
                $this->assertEquals(1, $numRows);
 
@@ -87,11 +103,11 @@ class RepairLegacyStorages extends \Test\TestCase {
        /**
         * Returns the storage id based on the numeric id
         *
-        * @param int $numericId numeric id of the storage
+        * @param int $storageId numeric id of the storage
         * @return string storage id or null if not found
         */
        private function getStorageId($storageId) {
-               $numericId = \OC\Files\Cache\Storage::getNumericStorageId($storageId);
+               $numericId = Storage::getNumericStorageId($storageId);
                if (!is_null($numericId)) {
                        return (int)$numericId;
                }
@@ -104,7 +120,7 @@ class RepairLegacyStorages extends \Test\TestCase {
         * @param string $storageId storage id
         */
        private function createData($storageId) {
-               $cache = new \OC\Files\Cache\Cache($storageId);
+               $cache = new Cache($storageId);
                $cache->put(
                        'dummyfile.txt',
                        array('size' => 5, 'mtime' => 12, 'mimetype' => 'text/plain')
@@ -113,7 +129,11 @@ class RepairLegacyStorages extends \Test\TestCase {
 
        /**
         * Test that existing home storages are left alone when valid.
+        *
         * @dataProvider settingsProvider
+        *
+        * @param string $dataDir
+        * @param string $userId
         */
        public function testNoopWithExistingHomeStorage($dataDir, $userId) {
                $this->prepareSettings($dataDir, $userId);
@@ -128,7 +148,11 @@ class RepairLegacyStorages extends \Test\TestCase {
        /**
         * Test that legacy storages are converted to home storages when
         * the latter does not exist.
+        *
         * @dataProvider settingsProvider
+        *
+        * @param string $dataDir
+        * @param string $userId
         */
        public function testConvertLegacyToHomeStorage($dataDir, $userId) {
                $this->prepareSettings($dataDir, $userId);
@@ -143,12 +167,16 @@ class RepairLegacyStorages extends \Test\TestCase {
        /**
         * Test that legacy storages are converted to home storages
         * when home storage already exists but has no data.
+        *
         * @dataProvider settingsProvider
+        *
+        * @param string $dataDir
+        * @param string $userId
         */
        public function testConvertLegacyToExistingEmptyHomeStorage($dataDir, $userId) {
                $this->prepareSettings($dataDir, $userId);
                $legacyStorageNumId = $this->createStorage($this->legacyStorageId);
-               $newStorageNumId = $this->createStorage($this->newStorageId);
+               $this->createStorage($this->newStorageId);
 
                $this->createData($this->legacyStorageId);
 
@@ -162,11 +190,15 @@ class RepairLegacyStorages extends \Test\TestCase {
         * Test that legacy storages are converted to home storages
         * when home storage already exists and the legacy storage
         * has no data.
+        *
         * @dataProvider settingsProvider
+        *
+        * @param string $dataDir
+        * @param string $userId
         */
        public function testConvertEmptyLegacyToHomeStorage($dataDir, $userId) {
                $this->prepareSettings($dataDir, $userId);
-               $legacyStorageNumId = $this->createStorage($this->legacyStorageId);
+               $this->createStorage($this->legacyStorageId);
                $newStorageNumId = $this->createStorage($this->newStorageId);
 
                $this->createData($this->newStorageId);
@@ -180,7 +212,11 @@ class RepairLegacyStorages extends \Test\TestCase {
        /**
         * Test that nothing is done when both conflicting legacy
         * and home storage have data.
+        *
         * @dataProvider settingsProvider
+        *
+        * @param string $dataDir
+        * @param string $userId
         */
        public function testConflictNoop($dataDir, $userId) {
                $this->prepareSettings($dataDir, $userId);
@@ -205,7 +241,11 @@ class RepairLegacyStorages extends \Test\TestCase {
 
        /**
         * Test that the data dir local entry is left alone
+        *
         * @dataProvider settingsProvider
+        *
+        * @param string $dataDir
+        * @param string $userId
         */
        public function testDataDirEntryNoop($dataDir, $userId) {
                $this->prepareSettings($dataDir, $userId);
@@ -219,7 +259,11 @@ class RepairLegacyStorages extends \Test\TestCase {
 
        /**
         * Test that external local storages are left alone
+        *
         * @dataProvider settingsProvider
+        *
+        * @param string $dataDir
+        * @param string $userId
         */
        public function testLocalExtStorageNoop($dataDir, $userId) {
                $this->prepareSettings($dataDir, $userId);
@@ -233,7 +277,11 @@ class RepairLegacyStorages extends \Test\TestCase {
 
        /**
         * Test that other external storages are left alone
+        *
         * @dataProvider settingsProvider
+        *
+        * @param string $dataDir
+        * @param string $userId
         */
        public function testExtStorageNoop($dataDir, $userId) {
                $this->prepareSettings($dataDir, $userId);