From 1eeca031f863a652d07ebfa2f75339232bf60dc1 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Wed, 12 Aug 2015 20:03:11 +0100 Subject: [PATCH] Split backend identifiers from the class name Prior to this, the storage class name was stored in mount.json under the "class" parameter, and the auth mechanism class name under the "authMechanism" parameter. This decouples the class name from the identifier used to retrieve the backend or auth mechanism. Now, backends/auth mechanisms have a unique identifier, which is saved in the "backend" or "authMechanism" parameter in mount.json respectively. An identifier is considered unique for the object it references, but the underlying class may change (e.g. files_external gets pulled into core and namespaces are modified). --- .../controller/globalstoragescontroller.php | 24 ++--- .../controller/storagescontroller.php | 14 +-- .../controller/userstoragescontroller.php | 26 +++--- apps/files_external/js/settings.js | 32 +++---- .../files_external/lib/auth/authmechanism.php | 9 +- .../files_external/lib/auth/nullmechanism.php | 1 + apps/files_external/lib/backend/backend.php | 10 +-- apps/files_external/lib/config.php | 41 ++++++--- apps/files_external/lib/identifiertrait.php | 68 ++++++++++++++ apps/files_external/lib/storageconfig.php | 4 +- .../files_external/service/backendservice.php | 40 ++++++--- .../service/storagesservice.php | 39 ++++---- apps/files_external/templates/settings.php | 12 +-- .../controller/storagescontrollertest.php | 8 +- .../service/globalstoragesservicetest.php | 89 ++++++++++++++----- .../tests/service/storagesservicetest.php | 50 +++++++---- .../tests/service/userstoragesservicetest.php | 20 ++--- .../tests/storageconfigtest.php | 12 +-- 18 files changed, 327 insertions(+), 172 deletions(-) create mode 100644 apps/files_external/lib/identifiertrait.php diff --git a/apps/files_external/controller/globalstoragescontroller.php b/apps/files_external/controller/globalstoragescontroller.php index 2c7b6af419..756a34fc5d 100644 --- a/apps/files_external/controller/globalstoragescontroller.php +++ b/apps/files_external/controller/globalstoragescontroller.php @@ -63,8 +63,8 @@ class GlobalStoragesController extends StoragesController { * Create an external storage entry. * * @param string $mountPoint storage mount point - * @param string $backendClass backend class name - * @param string $authMechanismClass authentication mechanism class + * @param string $backend backend identifier + * @param string $authMechanism authentication mechanism identifier * @param array $backendOptions backend-specific options * @param array $mountOptions mount-specific options * @param array $applicableUsers users for which to mount the storage @@ -75,8 +75,8 @@ class GlobalStoragesController extends StoragesController { */ public function create( $mountPoint, - $backendClass, - $authMechanismClass, + $backend, + $authMechanism, $backendOptions, $mountOptions, $applicableUsers, @@ -85,8 +85,8 @@ class GlobalStoragesController extends StoragesController { ) { $newStorage = $this->createStorage( $mountPoint, - $backendClass, - $authMechanismClass, + $backend, + $authMechanism, $backendOptions, $mountOptions, $applicableUsers, @@ -117,8 +117,8 @@ class GlobalStoragesController extends StoragesController { * * @param int $id storage id * @param string $mountPoint storage mount point - * @param string $backendClass backend class name - * @param string $authMechanismClass authentication mechansim class + * @param string $backend backend identifier + * @param string $authMechanism authentication mechansim identifier * @param array $backendOptions backend-specific options * @param array $mountOptions mount-specific options * @param array $applicableUsers users for which to mount the storage @@ -130,8 +130,8 @@ class GlobalStoragesController extends StoragesController { public function update( $id, $mountPoint, - $backendClass, - $authMechanismClass, + $backend, + $authMechanism, $backendOptions, $mountOptions, $applicableUsers, @@ -140,8 +140,8 @@ class GlobalStoragesController extends StoragesController { ) { $storage = $this->createStorage( $mountPoint, - $backendClass, - $authMechanismClass, + $backend, + $authMechanism, $backendOptions, $mountOptions, $applicableUsers, diff --git a/apps/files_external/controller/storagescontroller.php b/apps/files_external/controller/storagescontroller.php index 5f3779dc8b..f1d1625bdc 100644 --- a/apps/files_external/controller/storagescontroller.php +++ b/apps/files_external/controller/storagescontroller.php @@ -77,8 +77,8 @@ abstract class StoragesController extends Controller { * Create a storage from its parameters * * @param string $mountPoint storage mount point - * @param string $backendClass backend class name - * @param string $authMechanismClass authentication mechanism class name + * @param string $backend backend identifier + * @param string $authMechanism authentication mechanism identifier * @param array $backendOptions backend-specific options * @param array|null $mountOptions mount-specific options * @param array|null $applicableUsers users for which to mount the storage @@ -89,8 +89,8 @@ abstract class StoragesController extends Controller { */ protected function createStorage( $mountPoint, - $backendClass, - $authMechanismClass, + $backend, + $authMechanism, $backendOptions, $mountOptions = null, $applicableUsers = null, @@ -100,8 +100,8 @@ abstract class StoragesController extends Controller { try { return $this->service->createStorage( $mountPoint, - $backendClass, - $authMechanismClass, + $backend, + $authMechanism, $backendOptions, $mountOptions, $applicableUsers, @@ -145,7 +145,7 @@ abstract class StoragesController extends Controller { return new DataResponse( array( 'message' => (string)$this->l10n->t('Invalid storage backend "%s"', [ - $storage->getBackend()->getClass() + $storage->getBackend()->getIdentifier() ]) ), Http::STATUS_UNPROCESSABLE_ENTITY diff --git a/apps/files_external/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php index e72b51ff65..0d41e088a7 100644 --- a/apps/files_external/controller/userstoragescontroller.php +++ b/apps/files_external/controller/userstoragescontroller.php @@ -83,7 +83,7 @@ class UserStoragesController extends StoragesController { return new DataResponse( array( 'message' => (string)$this->l10n->t('Admin-only storage backend "%s"', [ - $storage->getBackend()->getClass() + $storage->getBackend()->getIdentifier() ]) ), Http::STATUS_UNPROCESSABLE_ENTITY @@ -108,8 +108,8 @@ class UserStoragesController extends StoragesController { * Create an external storage entry. * * @param string $mountPoint storage mount point - * @param string $backendClass backend class name - * @param string $authMechanismClass authentication mechanism class + * @param string $backend backend identifier + * @param string $authMechanism authentication mechanism identifier * @param array $backendOptions backend-specific options * @param array $mountOptions backend-specific mount options * @@ -119,15 +119,15 @@ class UserStoragesController extends StoragesController { */ public function create( $mountPoint, - $backendClass, - $authMechanismClass, + $backend, + $authMechanism, $backendOptions, $mountOptions ) { $newStorage = $this->createStorage( $mountPoint, - $backendClass, - $authMechanismClass, + $backend, + $authMechanism, $backendOptions, $mountOptions ); @@ -154,8 +154,8 @@ class UserStoragesController extends StoragesController { * * @param int $id storage id * @param string $mountPoint storage mount point - * @param string $backendClass backend class name - * @param string $authMechanismClass authentication mechanism class + * @param string $backend backend identifier + * @param string $authMechanism authentication mechanism identifier * @param array $backendOptions backend-specific options * @param array $mountOptions backend-specific mount options * @@ -166,15 +166,15 @@ class UserStoragesController extends StoragesController { public function update( $id, $mountPoint, - $backendClass, - $authMechanismClass, + $backend, + $authMechanism, $backendOptions, $mountOptions ) { $storage = $this->createStorage( $mountPoint, - $backendClass, - $authMechanismClass, + $backend, + $authMechanism, $backendOptions, $mountOptions ); diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 7240c246ea..7288f90fa7 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -214,18 +214,18 @@ StorageConfig.prototype = { mountPoint: '', /** - * Backend class name + * Backend * * @type string */ - backendClass: null, + backend: null, /** - * Authentication mechanism class name + * Authentication mechanism * * @type string */ - authMechanismClass: null, + authMechanism: null, /** * Backend-specific configuration @@ -279,8 +279,8 @@ StorageConfig.prototype = { getData: function() { var data = { mountPoint: this.mountPoint, - backendClass: this.backendClass, - authMechanismClass: this.authMechanismClass, + backend: this.backend, + authMechanism: this.authMechanism, backendOptions: this.backendOptions }; if (this.id) { @@ -704,14 +704,14 @@ MountConfigListView.prototype = { $el.find('tbody').append($tr.clone()); $el.find('tbody tr').last().find('.mountPoint input').val(''); var selected = $target.find('option:selected').text(); - var backendClass = $target.val(); + var backend = $target.val(); $tr.find('.backend').text(selected); if ($tr.find('.mountPoint input').val() === '') { $tr.find('.mountPoint input').val(this._suggestMountPoint(selected)); } - $tr.addClass(backendClass); - $tr.find('.backend').data('class', backendClass); - var backendConfiguration = this._allBackends[backendClass]; + $tr.addClass(backend); + $tr.find('.backend').data('class', backend); + var backendConfiguration = this._allBackends[backend]; var selectAuthMechanism = $(''); $.each(this._allAuthMechanisms, function(authClass, authMechanism) { @@ -730,7 +730,7 @@ MountConfigListView.prototype = { var priorityEl = $(''); $tr.append(priorityEl); - if (backendConfiguration['custom'] && $el.find('tbody tr.'+backendClass.replace(/\\/g, '\\\\')).length === 1) { + if (backendConfiguration['custom'] && $el.find('tbody tr.'+backend.replace(/\\/g, '\\\\')).length === 1) { OC.addScript('files_external', backendConfiguration['custom']); } $td.children().not('[type=hidden]').first().focus(); @@ -747,12 +747,12 @@ MountConfigListView.prototype = { var $target = $(event.target); var $tr = $target.closest('tr'); - var authMechanismClass = $target.val(); - var authMechanism = this._allAuthMechanisms[authMechanismClass]; + var authMechanism = $target.val(); + var authMechanismConfiguration = this._allAuthMechanisms[authMechanism]; var $td = $tr.find('td.configuration'); $td.find('.auth-param').remove(); - $.each(authMechanism['configuration'], _.partial( + $.each(authMechanismConfiguration['configuration'], _.partial( this.writeParameterInput, $td, _, _, ['auth-param'] )); }, @@ -792,8 +792,8 @@ MountConfigListView.prototype = { } var storage = new this._storageConfigClass(storageId); storage.mountPoint = $tr.find('.mountPoint input').val(); - storage.backendClass = $tr.find('.backend').data('class'); - storage.authMechanismClass = $tr.find('.selectAuthMechanism').val(); + storage.backend = $tr.find('.backend').data('class'); + storage.authMechanism = $tr.find('.selectAuthMechanism').val(); var classOptions = {}; var configuration = $tr.find('.configuration input'); diff --git a/apps/files_external/lib/auth/authmechanism.php b/apps/files_external/lib/auth/authmechanism.php index 7da57662db..a89ee823d5 100644 --- a/apps/files_external/lib/auth/authmechanism.php +++ b/apps/files_external/lib/auth/authmechanism.php @@ -23,6 +23,7 @@ namespace OCA\Files_External\Lib\Auth; use \OCA\Files_External\Lib\StorageConfig; use \OCA\Files_External\Lib\VisibilityTrait; +use \OCA\Files_External\Lib\IdentifierTrait; use \OCA\Files_External\Lib\FrontendDefinitionTrait; use \OCA\Files_External\Lib\StorageModifierTrait; @@ -59,17 +60,11 @@ class AuthMechanism implements \JsonSerializable { use VisibilityTrait; use FrontendDefinitionTrait; use StorageModifierTrait; + use IdentifierTrait; /** @var string */ protected $scheme; - /** - * @return string - */ - public function getClass() { - return '\\'.get_class($this); - } - /** * Get the authentication scheme implemented * See self::SCHEME_* constants diff --git a/apps/files_external/lib/auth/nullmechanism.php b/apps/files_external/lib/auth/nullmechanism.php index 396649d731..1765fc6739 100644 --- a/apps/files_external/lib/auth/nullmechanism.php +++ b/apps/files_external/lib/auth/nullmechanism.php @@ -32,6 +32,7 @@ class NullMechanism extends AuthMechanism { public function __construct(IL10N $l) { $this + ->setIdentifier('null::null') ->setScheme(self::SCHEME_NULL) ->setText($l->t('None')) ; diff --git a/apps/files_external/lib/backend/backend.php b/apps/files_external/lib/backend/backend.php index 634bcb7bfb..90d5d38ed9 100644 --- a/apps/files_external/lib/backend/backend.php +++ b/apps/files_external/lib/backend/backend.php @@ -27,6 +27,7 @@ use \OCA\Files_External\Lib\FrontendDefinitionTrait; use \OCA\Files_External\Lib\PriorityTrait; use \OCA\Files_External\Lib\DependencyTrait; use \OCA\Files_External\Lib\StorageModifierTrait; +use \OCA\Files_External\Lib\IdentifierTrait; use \OCA\Files_External\Lib\Auth\AuthMechanism; /** @@ -60,6 +61,7 @@ class Backend implements \JsonSerializable { use PriorityTrait; use DependencyTrait; use StorageModifierTrait; + use IdentifierTrait; /** @var string storage class */ private $storageClass; @@ -70,14 +72,6 @@ class Backend implements \JsonSerializable { /** @var AuthMechanism|callable authentication mechanism fallback */ private $legacyAuthMechanism; - /** - * @return string - */ - public function getClass() { - // return storage class for legacy compat - return $this->getStorageClass(); - } - /** * @return string */ diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index e720677480..c0ded522de 100644 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -117,14 +117,17 @@ class OC_Mount_Config { // Global mount points (is this redundant?) if (isset($mountConfig[self::MOUNT_TYPE_GLOBAL])) { foreach ($mountConfig[self::MOUNT_TYPE_GLOBAL] as $mountPoint => $options) { - $backend = $backendService->getBackend($options['class']); + if (!isset($options['backend'])) { + $options['backend'] = $options['class']; + } + $backend = $backendService->getBackend($options['backend']); $options['personal'] = false; $options['options'] = self::decryptPasswords($options['options']); if (!isset($options['priority'])) { $options['priority'] = $backend->getPriority(); } if (!isset($options['authMechanism'])) { - $options['authMechanism'] = $backend->getLegacyAuthMechanism($options['options'])->getClass(); + $options['authMechanism'] = $backend->getLegacyAuthMechanism($options['options'])->getIdentifier(); } // Override if priority greater @@ -145,14 +148,17 @@ class OC_Mount_Config { foreach ($options as &$option) { $option = self::setUserVars($user, $option); } - $backend = $backendService->getBackend($options['class']); + if (!isset($options['backend'])) { + $options['backend'] = $options['class']; + } + $backend = $backendService->getBackend($options['backend']); $options['personal'] = false; $options['options'] = self::decryptPasswords($options['options']); if (!isset($options['priority'])) { $options['priority'] = $backend->getPriority(); } if (!isset($options['authMechanism'])) { - $options['authMechanism'] = $backend->getLegacyAuthMechanism($options['options'])->getClass(); + $options['authMechanism'] = $backend->getLegacyAuthMechanism($options['options'])->getIdentifier(); } // Override if priority greater @@ -174,14 +180,17 @@ class OC_Mount_Config { foreach ($options as &$option) { $option = self::setUserVars($user, $option); } - $backend = $backendService->getBackend($options['class']); + if (!isset($options['backend'])) { + $options['backend'] = $options['class']; + } + $backend = $backendService->getBackend($options['backend']); $options['personal'] = false; $options['options'] = self::decryptPasswords($options['options']); if (!isset($options['priority'])) { $options['priority'] = $backend->getPriority(); } if (!isset($options['authMechanism'])) { - $options['authMechanism'] = $backend->getLegacyAuthMechanism($options['options'])->getClass(); + $options['authMechanism'] = $backend->getLegacyAuthMechanism($options['options'])->getIdentifier(); } // Override if priority greater or if priority type different @@ -206,14 +215,17 @@ class OC_Mount_Config { foreach ($options as &$option) { $option = self::setUserVars($user, $option); } - $backend = $backendService->getBackend($options['class']); + if (!isset($options['backend'])) { + $options['backend'] = $options['class']; + } + $backend = $backendService->getBackend($options['backend']); $options['personal'] = false; $options['options'] = self::decryptPasswords($options['options']); if (!isset($options['priority'])) { $options['priority'] = $backend->getPriority(); } if (!isset($options['authMechanism'])) { - $options['authMechanism'] = $backend->getLegacyAuthMechanism($options['options'])->getClass(); + $options['authMechanism'] = $backend->getLegacyAuthMechanism($options['options'])->getIdentifier(); } // Override if priority greater or if priority type different @@ -234,12 +246,15 @@ class OC_Mount_Config { $mountConfig = self::readData($user); if (isset($mountConfig[self::MOUNT_TYPE_USER][$user])) { foreach ($mountConfig[self::MOUNT_TYPE_USER][$user] as $mountPoint => $options) { - $backend = $backendService->getBackend($options['class']); + if (!isset($options['backend'])) { + $options['backend'] = $options['class']; + } + $backend = $backendService->getBackend($options['backend']); if ($backend->isVisibleFor(BackendService::VISIBILITY_PERSONAL)) { $options['personal'] = true; $options['options'] = self::decryptPasswords($options['options']); if (!isset($options['authMechanism'])) { - $options['authMechanism'] = $backend->getLegacyAuthMechanism($options['options'])->getClass(); + $options['authMechanism'] = $backend->getLegacyAuthMechanism($options['options'])->getIdentifier(); } // Always override previous config @@ -809,7 +824,8 @@ class OC_Mount_Config { public static function makeConfigHash($config) { $data = json_encode( array( - 'c' => $config['class'], + 'c' => $config['backend'], + 'a' => $config['authMechanism'], 'm' => $config['mountpoint'], 'o' => $config['options'], 'p' => isset($config['priority']) ? $config['priority'] : -1, @@ -858,7 +874,8 @@ class OC_Mount_Config { return false; } - $class = $options['class']; + $service = self::$appContainer->query('OCA\Files_External\Service\BackendService'); + $class = $service->getBackend($options['backend'])->getStorageClass(); try { /** @var \OC\Files\Storage\Storage $storage */ $storage = new $class($options['options']); diff --git a/apps/files_external/lib/identifiertrait.php b/apps/files_external/lib/identifiertrait.php new file mode 100644 index 0000000000..139911580f --- /dev/null +++ b/apps/files_external/lib/identifiertrait.php @@ -0,0 +1,68 @@ + + * + * @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 + * + */ + +namespace OCA\Files_External\Lib; + +/** + * Trait for objects requiring an identifier (and/or identifier aliases) + */ +trait IdentifierTrait { + + /** @var string */ + protected $identifier; + + /** @var string[] */ + protected $identifierAliases = []; + + /** + * @return string + */ + public function getIdentifier() { + return $this->identifier; + } + + /** + * @param string $identifier + * @return self + */ + public function setIdentifier($identifier) { + $this->identifier = $identifier; + $this->identifierAliases[] = $identifier; + return $this; + } + + /** + * @return string[] + */ + public function getIdentifierAliases() { + return $this->identifierAliases; + } + + /** + * @param string $alias + * @return self + */ + public function addIdentifierAlias($alias) { + $this->identifierAliases[] = $alias; + return $this; + } + +} diff --git a/apps/files_external/lib/storageconfig.php b/apps/files_external/lib/storageconfig.php index 96ba2f72ae..aeb8f52707 100644 --- a/apps/files_external/lib/storageconfig.php +++ b/apps/files_external/lib/storageconfig.php @@ -322,8 +322,8 @@ class StorageConfig implements \JsonSerializable { $result['id'] = $this->id; } $result['mountPoint'] = $this->mountPoint; - $result['backendClass'] = $this->backend->getClass(); - $result['authMechanismClass'] = $this->authMechanism->getClass(); + $result['backend'] = $this->backend->getIdentifier(); + $result['authMechanism'] = $this->authMechanism->getIdentifier(); $result['backendOptions'] = $this->backendOptions; if (!is_null($this->priority)) { $result['priority'] = $this->priority; diff --git a/apps/files_external/service/backendservice.php b/apps/files_external/service/backendservice.php index c1abbcf2b7..bee08ecbd1 100644 --- a/apps/files_external/service/backendservice.php +++ b/apps/files_external/service/backendservice.php @@ -83,7 +83,9 @@ class BackendService { if (!$this->isAllowedUserBackend($backend)) { $backend->removeVisibility(BackendService::VISIBILITY_PERSONAL); } - $this->backends[$backend->getClass()] = $backend; + foreach ($backend->getIdentifierAliases() as $alias) { + $this->backends[$alias] = $backend; + } } /** @@ -103,7 +105,9 @@ class BackendService { if (!$this->isAllowedAuthMechanism($authMech)) { $authMech->removeVisibility(BackendService::VISIBILITY_PERSONAL); } - $this->authMechanisms[$authMech->getClass()] = $authMech; + foreach ($authMech->getIdentifierAliases() as $alias) { + $this->authMechanisms[$alias] = $authMech; + } } /** @@ -121,7 +125,12 @@ class BackendService { * @return Backend[] */ public function getBackends() { - return $this->backends; + // only return real identifiers, no aliases + $backends = []; + foreach ($this->backends as $backend) { + $backends[$backend->getIdentifier()] = $backend; + } + return $backends; } /** @@ -160,12 +169,12 @@ class BackendService { } /** - * @param string $class Backend class name + * @param string $identifier * @return Backend|null */ - public function getBackend($class) { - if (isset($this->backends[$class])) { - return $this->backends[$class]; + public function getBackend($identifier) { + if (isset($this->backends[$identifier])) { + return $this->backends[$identifier]; } return null; } @@ -176,7 +185,12 @@ class BackendService { * @return AuthMechanism[] */ public function getAuthMechanisms() { - return $this->authMechanisms; + // only return real identifiers, no aliases + $mechanisms = []; + foreach ($this->authMechanisms as $mechanism) { + $mechanisms[$mechanism->getIdentifier()] = $mechanism; + } + return $mechanisms; } /** @@ -217,12 +231,12 @@ class BackendService { /** - * @param string $class + * @param string $identifier * @return AuthMechanism|null */ - public function getAuthMechanism($class) { - if (isset($this->authMechanisms[$class])) { - return $this->authMechanisms[$class]; + public function getAuthMechanism($identifier) { + if (isset($this->authMechanisms[$identifier])) { + return $this->authMechanisms[$identifier]; } return null; } @@ -242,7 +256,7 @@ class BackendService { */ protected function isAllowedUserBackend(Backend $backend) { if ($this->userMountingAllowed && - in_array($backend->getClass(), $this->userMountingBackends) + !empty(array_intersect($backend->getIdentifierAliases(), $this->userMountingBackends)) ) { return true; } diff --git a/apps/files_external/service/storagesservice.php b/apps/files_external/service/storagesservice.php index b8a1824ba2..e89af6bc75 100644 --- a/apps/files_external/service/storagesservice.php +++ b/apps/files_external/service/storagesservice.php @@ -81,9 +81,9 @@ abstract class StoragesService { $applicable, $storageOptions ) { - $backend = $this->backendService->getBackend($storageOptions['class']); + $backend = $this->backendService->getBackend($storageOptions['backend']); if (!$backend) { - throw new \UnexpectedValueException('Invalid backend class'); + throw new \UnexpectedValueException('Invalid backend '.$storageOptions['backend']); } $storageConfig->setBackend($backend); @@ -94,7 +94,7 @@ abstract class StoragesService { $storageOptions['authMechanism'] = 'null'; // to make error handling easier } if (!$authMechanism) { - throw new \UnexpectedValueException('Invalid authentication mechanism class'); + throw new \UnexpectedValueException('Invalid authentication mechanism '.$storageOptions['authMechanism']); } $storageConfig->setAuthMechanism($authMechanism); @@ -140,9 +140,10 @@ abstract class StoragesService { * - $mountPath is the mount point path (where the storage must be mounted) * - $storageOptions is a map of storage options: * - "priority": storage priority - * - "backend": backend class name + * - "backend": backend identifier + * - "class": LEGACY backend class name * - "options": backend-specific options - * - "authMechanism": authentication mechanism class name + * - "authMechanism": authentication mechanism identifier * - "mountOptions": mount-specific options (ex: disable previews, scanner, etc) */ @@ -186,6 +187,13 @@ abstract class StoragesService { // options might be needed for the config hash $storageOptions['options'] = \OC_Mount_Config::decryptPasswords($storageOptions['options']); + if (!isset($storageOptions['backend'])) { + $storageOptions['backend'] = $storageOptions['class']; // legacy compat + } + if (!isset($storageOptions['authMechanism'])) { + $storageOptions['authMechanism'] = null; // ensure config hash works + } + if (isset($storageOptions['id'])) { $configId = (int)$storageOptions['id']; if (isset($storages[$configId])) { @@ -271,8 +279,9 @@ abstract class StoragesService { $options = [ 'id' => $storageConfig->getId(), - 'class' => $storageConfig->getBackend()->getClass(), - 'authMechanism' => $storageConfig->getAuthMechanism()->getClass(), + 'backend' => $storageConfig->getBackend()->getIdentifier(), + //'class' => $storageConfig->getBackend()->getClass(), + 'authMechanism' => $storageConfig->getAuthMechanism()->getIdentifier(), 'options' => $storageConfig->getBackendOptions(), ]; @@ -350,8 +359,8 @@ abstract class StoragesService { * Create a storage from its parameters * * @param string $mountPoint storage mount point - * @param string $backendClass backend class name - * @param string $authMechanismClass authentication mechanism class + * @param string $backendIdentifier backend identifier + * @param string $authMechanismIdentifier authentication mechanism identifier * @param array $backendOptions backend-specific options * @param array|null $mountOptions mount-specific options * @param array|null $applicableUsers users for which to mount the storage @@ -362,21 +371,21 @@ abstract class StoragesService { */ public function createStorage( $mountPoint, - $backendClass, - $authMechanismClass, + $backendIdentifier, + $authMechanismIdentifier, $backendOptions, $mountOptions = null, $applicableUsers = null, $applicableGroups = null, $priority = null ) { - $backend = $this->backendService->getBackend($backendClass); + $backend = $this->backendService->getBackend($backendIdentifier); if (!$backend) { - throw new \InvalidArgumentException('Unable to get backend for backend class '.$backendClass); + throw new \InvalidArgumentException('Unable to get backend for '.$backendIdentifier); } - $authMechanism = $this->backendService->getAuthMechanism($authMechanismClass); + $authMechanism = $this->backendService->getAuthMechanism($authMechanismIdentifier); if (!$authMechanism) { - throw new \InvalidArgumentException('Unable to get authentication mechanism for class '.$authMechanismClass); + throw new \InvalidArgumentException('Unable to get authentication mechanism for '.$authMechanismIdentifier); } $newStorage = new StorageConfig(); $newStorage->setMountPoint($mountPoint); diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php index 2328ea9565..589574bbae 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -71,7 +71,7 @@ - + @@ -80,7 +80,7 @@ data-mountpoint="getMountPoint(), '/')); ?>" placeholder="t('Folder name')); ?>" /> - getBackend()->getText()); ?> + getBackend()->getText()); ?> @@ -166,7 +166,7 @@ }); ?> - + @@ -206,7 +206,7 @@

class="hidden"> t('Allow users to mount the following external storage')); ?>
- isVisibleFor(BackendService::VISIBILITY_PERSONAL)) print_unescaped(' checked="checked"'); ?> /> + isVisibleFor(BackendService::VISIBILITY_PERSONAL)) print_unescaped(' checked="checked"'); ?> />
diff --git a/apps/files_external/tests/controller/storagescontrollertest.php b/apps/files_external/tests/controller/storagescontrollertest.php index 735e760c09..5a9683306c 100644 --- a/apps/files_external/tests/controller/storagescontrollertest.php +++ b/apps/files_external/tests/controller/storagescontrollertest.php @@ -53,8 +53,8 @@ abstract class StoragesControllerTest extends \Test\TestCase { ->getMock(); $backend->method('getStorageClass') ->willReturn($storageClass); - $backend->method('getClass') - ->willReturn($storageClass); + $backend->method('getIdentifier') + ->willReturn('identifier:'.$class); return $backend; } @@ -64,8 +64,8 @@ abstract class StoragesControllerTest extends \Test\TestCase { ->getMock(); $authMech->method('getScheme') ->willReturn($scheme); - $authMech->method('getClass') - ->willReturn($class); + $authMech->method('getIdentifier') + ->willReturn('identifier:'.$class); return $authMech; } diff --git a/apps/files_external/tests/service/globalstoragesservicetest.php b/apps/files_external/tests/service/globalstoragesservicetest.php index d5f99431a5..f6ed9f1422 100644 --- a/apps/files_external/tests/service/globalstoragesservicetest.php +++ b/apps/files_external/tests/service/globalstoragesservicetest.php @@ -40,8 +40,8 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { protected function makeTestStorageData() { return $this->makeStorageConfig([ 'mountPoint' => 'mountpoint', - 'backendClass' => '\OC\Files\Storage\SMB', - 'authMechanismClass' => '\Auth\Mechanism', + 'backendIdentifier' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanismIdentifier' => 'identifier:\Auth\Mechanism', 'backendOptions' => [ 'option1' => 'value1', 'option2' => 'value2', @@ -62,8 +62,8 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { [ [ 'mountPoint' => 'mountpoint', - 'backendClass' => '\OC\Files\Storage\SMB', - 'authMechanismClass' => '\Auth\Mechanism', + 'backendIdentifier' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanismIdentifier' => 'identifier:\Auth\Mechanism', 'backendOptions' => [ 'option1' => 'value1', 'option2' => 'value2', @@ -78,8 +78,8 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { [ [ 'mountPoint' => 'mountpoint', - 'backendClass' => '\OC\Files\Storage\SMB', - 'authMechanismClass' => '\Auth\Mechanism', + 'backendIdentifier' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanismIdentifier' => 'identifier:\Auth\Mechanism', 'backendOptions' => [ 'option1' => 'value1', 'option2' => 'value2', @@ -94,8 +94,8 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { [ [ 'mountPoint' => 'mountpoint', - 'backendClass' => '\OC\Files\Storage\SMB', - 'authMechanismClass' => '\Auth\Mechanism', + 'backendIdentifier' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanismIdentifier' => 'identifier:\Auth\Mechanism', 'backendOptions' => [ 'option1' => 'value1', 'option2' => 'value2', @@ -110,8 +110,8 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { [ [ 'mountPoint' => 'mountpoint', - 'backendClass' => '\OC\Files\Storage\SMB', - 'authMechanismClass' => '\Auth\Mechanism', + 'backendIdentifier' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanismIdentifier' => 'identifier:\Auth\Mechanism', 'backendOptions' => [ 'option1' => 'value1', 'option2' => 'value2', @@ -159,8 +159,8 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { $updatedStorage = $this->makeStorageConfig($updatedStorageParams); $storage = $this->makeStorageConfig([ 'mountPoint' => 'mountpoint', - 'backendClass' => '\OC\Files\Storage\SMB', - 'authMechanismClass' => '\Auth\Mechanism', + 'backendIdentifier' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanismIdentifier' => 'identifier:\Auth\Mechanism', 'backendOptions' => [ 'option1' => 'value1', 'option2' => 'value2', @@ -647,8 +647,8 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { $mountPointOptions = current($mountPointData); $this->assertEquals(1, $mountPointOptions['id']); - $this->assertEquals('\OC\Files\Storage\SMB', $mountPointOptions['class']); - $this->assertEquals('\Auth\Mechanism', $mountPointOptions['authMechanism']); + $this->assertEquals('identifier:\OCA\Files_External\Lib\Backend\SMB', $mountPointOptions['backend']); + $this->assertEquals('identifier:\Auth\Mechanism', $mountPointOptions['authMechanism']); $this->assertEquals(15, $mountPointOptions['priority']); $this->assertEquals(false, $mountPointOptions['mountOptions']['preview']); @@ -688,8 +688,8 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { $mountPointOptions = current($mountPointData); $this->assertEquals(1, $mountPointOptions['id']); - $this->assertEquals('\OC\Files\Storage\SMB', $mountPointOptions['class']); - $this->assertEquals('\Auth\Mechanism', $mountPointOptions['authMechanism']); + $this->assertEquals('identifier:\OCA\Files_External\Lib\Backend\SMB', $mountPointOptions['backend']); + $this->assertEquals('identifier:\Auth\Mechanism', $mountPointOptions['authMechanism']); $this->assertEquals(15, $mountPointOptions['priority']); $this->assertEquals(false, $mountPointOptions['mountOptions']['preview']); @@ -706,8 +706,8 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { $mountPointOptions = current($mountPointData); $this->assertEquals(1, $mountPointOptions['id']); - $this->assertEquals('\OC\Files\Storage\SMB', $mountPointOptions['class']); - $this->assertEquals('\Auth\Mechanism', $mountPointOptions['authMechanism']); + $this->assertEquals('identifier:\OCA\Files_External\Lib\Backend\SMB', $mountPointOptions['backend']); + $this->assertEquals('identifier:\Auth\Mechanism', $mountPointOptions['authMechanism']); $this->assertEquals(15, $mountPointOptions['priority']); $this->assertEquals(false, $mountPointOptions['mountOptions']['preview']); @@ -732,15 +732,15 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { $legacyBackendOptions = \OC_Mount_Config::encryptPasswords($legacyBackendOptions); $legacyConfig = [ - 'class' => '\OC\Files\Storage\SMB', - 'authMechanism' => '\Auth\Mechanism', + 'backend' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanism' => 'identifier:\Auth\Mechanism', 'options' => $legacyBackendOptions, 'mountOptions' => ['preview' => false], ]; // different mount options $legacyConfig2 = [ - 'class' => '\OC\Files\Storage\SMB', - 'authMechanism' => '\Auth\Mechanism', + 'backend' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanism' => 'identifier:\Auth\Mechanism', 'options' => $legacyBackendOptions, 'mountOptions' => ['preview' => true], ]; @@ -751,8 +751,8 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { // different config $legacyConfig3 = [ - 'class' => '\OC\Files\Storage\SMB', - 'authMechanism' => '\Auth\Mechanism', + 'backend' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanism' => 'identifier:\Auth\Mechanism', 'options' => $legacyBackendOptions2, 'mountOptions' => ['preview' => true], ]; @@ -826,4 +826,45 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { $this->assertEquals([], $storage4->getApplicableGroups()); $this->assertEquals(['preview' => true], $storage4->getMountOptions()); } + + public function testReadLegacyConfigClass() { + $configFile = $this->dataDir . '/mount.json'; + + $json = [ + 'user' => [ + 'user1' => [ + '/$user/files/somemount' => [ + 'class' => 'identifier:\OCA\Files_External\Lib\Backend\SFTP', + 'authMechanism' => 'identifier:\Auth\Mechanism', + 'options' => [], + 'mountOptions' => [], + ], + '/$user/files/othermount' => [ + 'class' => 'identifier:sftp_alias', + 'authMechanism' => 'identifier:\Auth\Mechanism', + 'options' => [], + 'mountOptions' => [], + ], + ] + ] + ]; + + file_put_contents($configFile, json_encode($json)); + + $allStorages = $this->service->getAllStorages(); + + $this->assertCount(2, $allStorages); + + $storage1 = $allStorages[1]; + $storage2 = $allStorages[2]; + + $this->assertEquals('/somemount', $storage1->getMountPoint()); + $this->assertEquals('identifier:\OCA\Files_External\Lib\Backend\SFTP', $storage1->getBackend()->getIdentifier()); + $this->assertEquals('identifier:\Auth\Mechanism', $storage1->getAuthMechanism()->getIdentifier()); + + $this->assertEquals('/othermount', $storage2->getMountPoint()); + $this->assertEquals('identifier:\OCA\Files_External\Lib\Backend\SFTP', $storage2->getBackend()->getIdentifier()); + $this->assertEquals('identifier:\Auth\Mechanism', $storage2->getAuthMechanism()->getIdentifier()); + } + } diff --git a/apps/files_external/tests/service/storagesservicetest.php b/apps/files_external/tests/service/storagesservicetest.php index 1429fb1818..397f2a2e5c 100644 --- a/apps/files_external/tests/service/storagesservicetest.php +++ b/apps/files_external/tests/service/storagesservicetest.php @@ -66,9 +66,9 @@ abstract class StoragesServiceTest extends \Test\TestCase { ->getMock(); $authMechanisms = [ - '\Auth\Mechanism' => $this->getAuthMechMock('null', '\Auth\Mechanism'), - '\Other\Auth\Mechanism' => $this->getAuthMechMock('null', '\Other\Auth\Mechanism'), - '\OCA\Files_External\Lib\Auth\NullMechanism' => $this->getAuthMechMock(), + 'identifier:\Auth\Mechanism' => $this->getAuthMechMock('null', '\Auth\Mechanism'), + 'identifier:\Other\Auth\Mechanism' => $this->getAuthMechMock('null', '\Other\Auth\Mechanism'), + 'identifier:\OCA\Files_External\Lib\Auth\NullMechanism' => $this->getAuthMechMock(), ]; $this->backendService->method('getAuthMechanism') ->will($this->returnCallback(function($class) use ($authMechanisms) { @@ -86,12 +86,14 @@ abstract class StoragesServiceTest extends \Test\TestCase { $this->backendService->method('getAuthMechanisms') ->will($this->returnValue($authMechanisms)); + $sftpBackend = $this->getBackendMock('\OCA\Files_External\Lib\Backend\SFTP', '\OC\Files\Storage\SFTP'); $backends = [ - '\OC\Files\Storage\SMB' => $this->getBackendMock('\OCA\Files_External\Lib\Backend\SMB', '\OC\Files\Storage\SMB'), - '\OC\Files\Storage\SFTP' => $this->getBackendMock('\OCA\Files_External\Lib\Backend\SFTP', '\OC\Files\Storage\SFTP'), + 'identifier:\OCA\Files_External\Lib\Backend\SMB' => $this->getBackendMock('\OCA\Files_External\Lib\Backend\SMB', '\OC\Files\Storage\SMB'), + 'identifier:\OCA\Files_External\Lib\Backend\SFTP' => $sftpBackend, + 'identifier:sftp_alias' => $sftpBackend, ]; - $backends['\OC\Files\Storage\SFTP']->method('getLegacyAuthMechanism') - ->willReturn($authMechanisms['\Other\Auth\Mechanism']); + $backends['identifier:\OCA\Files_External\Lib\Backend\SFTP']->method('getLegacyAuthMechanism') + ->willReturn($authMechanisms['identifier:\Other\Auth\Mechanism']); $this->backendService->method('getBackend') ->will($this->returnCallback(function($backendClass) use ($backends) { if (isset($backends[$backendClass])) { @@ -111,6 +113,14 @@ abstract class StoragesServiceTest extends \Test\TestCase { Filesystem::signal_delete_mount, get_class($this), 'deleteHookCallback'); + $containerMock = $this->getMock('\OCP\AppFramework\IAppContainer'); + $containerMock->method('query') + ->will($this->returnCallback(function($name) { + if ($name === 'OCA\Files_External\Service\BackendService') { + return $this->backendService; + } + })); + \OC_Mount_Config::initApp($containerMock); } public function tearDown() { @@ -124,8 +134,8 @@ abstract class StoragesServiceTest extends \Test\TestCase { ->getMock(); $backend->method('getStorageClass') ->willReturn($storageClass); - $backend->method('getClass') - ->willReturn($storageClass); + $backend->method('getIdentifier') + ->willReturn('identifier:'.$class); return $backend; } @@ -135,8 +145,8 @@ abstract class StoragesServiceTest extends \Test\TestCase { ->getMock(); $authMech->method('getScheme') ->willReturn($scheme); - $authMech->method('getClass') - ->willReturn($class); + $authMech->method('getIdentifier') + ->willReturn('identifier:'.$class); return $authMech; } @@ -157,10 +167,16 @@ abstract class StoragesServiceTest extends \Test\TestCase { if (!isset($data['backend'])) { // data providers are run before $this->backendService is initialised // so $data['backend'] can be specified directly - $data['backend'] = $this->backendService->getBackend($data['backendClass']); + $data['backend'] = $this->backendService->getBackend($data['backendIdentifier']); + } + if (!isset($data['backend'])) { + throw new \Exception('oops, no backend'); } if (!isset($data['authMechanism'])) { - $data['authMechanism'] = $this->backendService->getAuthMechanism($data['authMechanismClass']); + $data['authMechanism'] = $this->backendService->getAuthMechanism($data['authMechanismIdentifier']); + } + if (!isset($data['authMechanism'])) { + throw new \Exception('oops, no auth mechanism'); } $storage->setBackend($data['backend']); $storage->setAuthMechanism($data['authMechanism']); @@ -185,8 +201,8 @@ abstract class StoragesServiceTest extends \Test\TestCase { * @expectedException \OCA\Files_external\NotFoundException */ public function testNonExistingStorage() { - $backend = $this->backendService->getBackend('\OC\Files\Storage\SMB'); - $authMechanism = $this->backendService->getAuthMechanism('\Auth\Mechanism'); + $backend = $this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\SMB'); + $authMechanism = $this->backendService->getAuthMechanism('identifier:\Auth\Mechanism'); $storage = new StorageConfig(255); $storage->setMountPoint('mountpoint'); $storage->setBackend($backend); @@ -195,8 +211,8 @@ abstract class StoragesServiceTest extends \Test\TestCase { } public function testDeleteStorage() { - $backend = $this->backendService->getBackend('\OC\Files\Storage\SMB'); - $authMechanism = $this->backendService->getAuthMechanism('\Auth\Mechanism'); + $backend = $this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\SMB'); + $authMechanism = $this->backendService->getAuthMechanism('identifier:\Auth\Mechanism'); $storage = new StorageConfig(255); $storage->setMountPoint('mountpoint'); $storage->setBackend($backend); diff --git a/apps/files_external/tests/service/userstoragesservicetest.php b/apps/files_external/tests/service/userstoragesservicetest.php index 1e57eedd32..0d5b82e2f8 100644 --- a/apps/files_external/tests/service/userstoragesservicetest.php +++ b/apps/files_external/tests/service/userstoragesservicetest.php @@ -54,8 +54,8 @@ class UserStoragesServiceTest extends StoragesServiceTest { private function makeTestStorageData() { return $this->makeStorageConfig([ 'mountPoint' => 'mountpoint', - 'backendClass' => '\OC\Files\Storage\SMB', - 'authMechanismClass' => '\Auth\Mechanism', + 'backendIdentifier' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanismIdentifier' => 'identifier:\Auth\Mechanism', 'backendOptions' => [ 'option1' => 'value1', 'option2' => 'value2', @@ -100,8 +100,8 @@ class UserStoragesServiceTest extends StoragesServiceTest { public function testUpdateStorage() { $storage = $this->makeStorageConfig([ 'mountPoint' => 'mountpoint', - 'backendClass' => '\OC\Files\Storage\SMB', - 'authMechanismClass' => '\Auth\Mechanism', + 'backendIdentifier' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanismIdentifier' => 'identifier:\Auth\Mechanism', 'backendOptions' => [ 'option1' => 'value1', 'option2' => 'value2', @@ -194,8 +194,8 @@ class UserStoragesServiceTest extends StoragesServiceTest { $mountPointOptions = current($mountPointData); $this->assertEquals(1, $mountPointOptions['id']); - $this->assertEquals('\OC\Files\Storage\SMB', $mountPointOptions['class']); - $this->assertEquals('\Auth\Mechanism', $mountPointOptions['authMechanism']); + $this->assertEquals('identifier:\OCA\Files_External\Lib\Backend\SMB', $mountPointOptions['backend']); + $this->assertEquals('identifier:\Auth\Mechanism', $mountPointOptions['authMechanism']); $this->assertEquals(false, $mountPointOptions['mountOptions']['preview']); $backendOptions = $mountPointOptions['options']; @@ -218,15 +218,15 @@ class UserStoragesServiceTest extends StoragesServiceTest { $legacyBackendOptions = \OC_Mount_Config::encryptPasswords($legacyBackendOptions); $legacyConfig = [ - 'class' => '\OC\Files\Storage\SMB', - 'authMechanism' => '\Auth\Mechanism', + 'backend' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanism' => 'identifier:\Auth\Mechanism', 'options' => $legacyBackendOptions, 'mountOptions' => ['preview' => false], ]; // different mount options $legacyConfig2 = [ - 'class' => '\OC\Files\Storage\SMB', - 'authMechanism' => '\Auth\Mechanism', + 'backend' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanism' => 'identifier:\Auth\Mechanism', 'options' => $legacyBackendOptions, 'mountOptions' => ['preview' => true], ]; diff --git a/apps/files_external/tests/storageconfigtest.php b/apps/files_external/tests/storageconfigtest.php index 8039990999..dba5105d7d 100644 --- a/apps/files_external/tests/storageconfigtest.php +++ b/apps/files_external/tests/storageconfigtest.php @@ -29,14 +29,14 @@ class StorageConfigTest extends \Test\TestCase { $backend = $this->getMockBuilder('\OCA\Files_External\Lib\Backend\Backend') ->disableOriginalConstructor() ->getMock(); - $backend->method('getClass') - ->willReturn('\OC\Files\Storage\SMB'); + $backend->method('getIdentifier') + ->willReturn('storage::identifier'); $authMech = $this->getMockBuilder('\OCA\Files_External\Lib\Auth\AuthMechanism') ->disableOriginalConstructor() ->getMock(); - $authMech->method('getClass') - ->willReturn('\Auth\Mechanism'); + $authMech->method('getIdentifier') + ->willReturn('auth::identifier'); $storageConfig = new StorageConfig(1); $storageConfig->setMountPoint('test'); @@ -52,8 +52,8 @@ class StorageConfigTest extends \Test\TestCase { $this->assertEquals(1, $json['id']); $this->assertEquals('/test', $json['mountPoint']); - $this->assertEquals('\OC\Files\Storage\SMB', $json['backendClass']); - $this->assertEquals('\Auth\Mechanism', $json['authMechanismClass']); + $this->assertEquals('storage::identifier', $json['backend']); + $this->assertEquals('auth::identifier', $json['authMechanism']); $this->assertEquals('test', $json['backendOptions']['user']); $this->assertEquals('password123', $json['backendOptions']['password']); $this->assertEquals(128, $json['priority']);