Merge pull request #18733 from owncloud/ext-dependencycheck

Make checkDependencies a real method

11 files changed, 162 insertions, 64 deletions

diff --git a/apps/files_external/lib/backend/amazons3.php b/apps/files_external/lib/backend/amazons3.php
index 880d47621f3..1cf62d7cb09 100644
--- a/apps/files_external/lib/backend/amazons3.php
+++ b/apps/files_external/lib/backend/amazons3.php
@@ -26,10 +26,14 @@ use \OCA\Files_External\Lib\Backend\Backend;
 use \OCA\Files_External\Lib\DefinitionParameter;
 use \OCA\Files_External\Lib\Auth\AuthMechanism;
 use \OCA\Files_External\Service\BackendService;
+use \OCA\Files_External\Lib\LegacyDependencyCheckPolyfill;
+
 use \OCA\Files_External\Lib\Auth\AmazonS3\AccessKey;
 
 class AmazonS3 extends Backend {
 
+	use LegacyDependencyCheckPolyfill;
+
 	public function __construct(IL10N $l, AccessKey $legacyAuth) {
 		$this
 			->setIdentifier('amazons3')
@@ -49,7 +53,6 @@ class AmazonS3 extends Backend {
 				(new DefinitionParameter('use_path_style', $l->t('Enable Path Style')))
 					->setType(DefinitionParameter::VALUE_BOOLEAN),
 			])
-			->setDependencyCheck('\OC\Files\Storage\AmazonS3::checkDependencies')
 			->addAuthScheme(AccessKey::SCHEME_AMAZONS3_ACCESSKEY)
 			->setLegacyAuthMechanism($legacyAuth)
 		;
diff --git a/apps/files_external/lib/backend/dav.php b/apps/files_external/lib/backend/dav.php
index 5ae6d122588..c4f446548e1 100644
--- a/apps/files_external/lib/backend/dav.php
+++ b/apps/files_external/lib/backend/dav.php
@@ -26,11 +26,14 @@ use \OCA\Files_External\Lib\Backend\Backend;
 use \OCA\Files_External\Lib\DefinitionParameter;
 use \OCA\Files_External\Lib\Auth\AuthMechanism;
 use \OCA\Files_External\Service\BackendService;
+use \OCA\Files_External\Lib\LegacyDependencyCheckPolyfill;
 
 use \OCA\Files_External\Lib\Auth\Password\Password;
 
 class DAV extends Backend {
 
+	use LegacyDependencyCheckPolyfill;
+
 	public function __construct(IL10N $l, Password $legacyAuth) {
 		$this
 			->setIdentifier('dav')
@@ -44,7 +47,6 @@ class DAV extends Backend {
 				(new DefinitionParameter('secure', $l->t('Secure https://')))
 					->setType(DefinitionParameter::VALUE_BOOLEAN),
 			])
-			->setDependencyCheck('\OC\Files\Storage\DAV::checkDependencies')
 			->addAuthScheme(AuthMechanism::SCHEME_PASSWORD)
 			->setLegacyAuthMechanism($legacyAuth)
 		;
diff --git a/apps/files_external/lib/backend/dropbox.php b/apps/files_external/lib/backend/dropbox.php
index bfd2e4cddc4..3e595cb0a9c 100644
--- a/apps/files_external/lib/backend/dropbox.php
+++ b/apps/files_external/lib/backend/dropbox.php
@@ -26,10 +26,14 @@ use \OCA\Files_External\Lib\Backend\Backend;
 use \OCA\Files_External\Lib\DefinitionParameter;
 use \OCA\Files_External\Lib\Auth\AuthMechanism;
 use \OCA\Files_External\Service\BackendService;
+use \OCA\Files_External\Lib\LegacyDependencyCheckPolyfill;
+
 use \OCA\Files_External\Lib\Auth\OAuth1\OAuth1;
 
 class Dropbox extends Backend {
 
+	use LegacyDependencyCheckPolyfill;
+
 	public function __construct(IL10N $l, OAuth1 $legacyAuth) {
 		$this
 			->setIdentifier('dropbox')
@@ -39,7 +43,6 @@ class Dropbox extends Backend {
 			->addParameters([
 				// all parameters handled in OAuth1 mechanism
 			])
-			->setDependencyCheck('\OC\Files\Storage\Dropbox::checkDependencies')
 			->addAuthScheme(AuthMechanism::SCHEME_OAUTH1)
 			->setLegacyAuthMechanism($legacyAuth)
 		;
diff --git a/apps/files_external/lib/backend/ftp.php b/apps/files_external/lib/backend/ftp.php
index df6ca37679e..1caf3a8fcb8 100644
--- a/apps/files_external/lib/backend/ftp.php
+++ b/apps/files_external/lib/backend/ftp.php
@@ -26,11 +26,14 @@ use \OCA\Files_External\Lib\Backend\Backend;
 use \OCA\Files_External\Lib\DefinitionParameter;
 use \OCA\Files_External\Lib\Auth\AuthMechanism;
 use \OCA\Files_External\Service\BackendService;
+use \OCA\Files_External\Lib\LegacyDependencyCheckPolyfill;
 
 use \OCA\Files_External\Lib\Auth\Password\Password;
 
 class FTP extends Backend {
 
+	use LegacyDependencyCheckPolyfill;
+
 	public function __construct(IL10N $l, Password $legacyAuth) {
 		$this
 			->setIdentifier('ftp')
@@ -44,7 +47,6 @@ class FTP extends Backend {
 				(new DefinitionParameter('secure', $l->t('Secure ftps://')))
 					->setType(DefinitionParameter::VALUE_BOOLEAN),
 			])
-			->setDependencyCheck('\OC\Files\Storage\FTP::checkDependencies')
 			->addAuthScheme(AuthMechanism::SCHEME_PASSWORD)
 			->setLegacyAuthMechanism($legacyAuth)
 		;
diff --git a/apps/files_external/lib/backend/google.php b/apps/files_external/lib/backend/google.php
index b46b2f653a6..bc0b52c464b 100644
--- a/apps/files_external/lib/backend/google.php
+++ b/apps/files_external/lib/backend/google.php
@@ -26,10 +26,14 @@ use \OCA\Files_External\Lib\Backend\Backend;
 use \OCA\Files_External\Lib\DefinitionParameter;
 use \OCA\Files_External\Lib\Auth\AuthMechanism;
 use \OCA\Files_External\Service\BackendService;
+use \OCA\Files_External\Lib\LegacyDependencyCheckPolyfill;
+
 use \OCA\Files_External\Lib\Auth\OAuth2\OAuth2;
 
 class Google extends Backend {
 
+	use LegacyDependencyCheckPolyfill;
+
 	public function __construct(IL10N $l, OAuth2 $legacyAuth) {
 		$this
 			->setIdentifier('googledrive')
@@ -39,7 +43,6 @@ class Google extends Backend {
 			->addParameters([
 				// all parameters handled in OAuth2 mechanism
 			])
-			->setDependencyCheck('\OC\Files\Storage\Google::checkDependencies')
 			->addAuthScheme(AuthMechanism::SCHEME_OAUTH2)
 			->setLegacyAuthMechanism($legacyAuth)
 		;
diff --git a/apps/files_external/lib/backend/legacybackend.php b/apps/files_external/lib/backend/legacybackend.php
index 0f60c2caa47..83a5b45940d 100644
--- a/apps/files_external/lib/backend/legacybackend.php
+++ b/apps/files_external/lib/backend/legacybackend.php
@@ -24,12 +24,21 @@ namespace OCA\Files_External\Lib\Backend;
 use \OCA\Files_External\Lib\DefinitionParameter;
 use \OCA\Files_External\Lib\Backend\Backend;
 use \OCA\Files_External\Lib\Auth\Builtin;
+use \OCA\Files_External\Lib\MissingDependency;
+use \OCA\Files_External\Lib\LegacyDependencyCheckPolyfill;
 
 /**
  * Legacy compatibility for OC_Mount_Config::registerBackend()
  */
 class LegacyBackend extends Backend {
 
+	use LegacyDependencyCheckPolyfill {
+		LegacyDependencyCheckPolyfill::checkDependencies as doCheckDependencies;
+	}
+
+	/** @var bool */
+	protected $hasDependencies = false;
+
 	/**
 	 * @param string $class
 	 * @param array $definition
@@ -78,8 +87,18 @@ class LegacyBackend extends Backend {
 			$this->setCustomJs($definition['custom']);
 		}
 		if (isset($definition['has_dependencies']) && $definition['has_dependencies']) {
-			$this->setDependencyCheck($class . '::checkDependencies');
+			$this->hasDependencies = true;
+		}
+	}
+
+	/**
+	 * @return MissingDependency[]
+	 */
+	public function checkDependencies() {
+		if ($this->hasDependencies) {
+			return $this->doCheckDependencies();
 		}
+		return [];
 	}
 
 }
diff --git a/apps/files_external/lib/backend/smb.php b/apps/files_external/lib/backend/smb.php
index 3d950a80c31..350eca1de34 100644
--- a/apps/files_external/lib/backend/smb.php
+++ b/apps/files_external/lib/backend/smb.php
@@ -26,11 +26,14 @@ use \OCA\Files_External\Lib\Backend\Backend;
 use \OCA\Files_External\Lib\DefinitionParameter;
 use \OCA\Files_External\Lib\Auth\AuthMechanism;
 use \OCA\Files_External\Service\BackendService;
+use \OCA\Files_External\Lib\LegacyDependencyCheckPolyfill;
 
 use \OCA\Files_External\Lib\Auth\Password\Password;
 
 class SMB extends Backend {
 
+	use LegacyDependencyCheckPolyfill;
+
 	public function __construct(IL10N $l, Password $legacyAuth) {
 		$this
 			->setIdentifier('smb')
@@ -43,7 +46,6 @@ class SMB extends Backend {
 				(new DefinitionParameter('root', $l->t('Remote subfolder')))
 					->setFlag(DefinitionParameter::FLAG_OPTIONAL),
 			])
-			->setDependencyCheck('\OC\Files\Storage\SMB::checkDependencies')
 			->addAuthScheme(AuthMechanism::SCHEME_PASSWORD)
 			->setLegacyAuthMechanism($legacyAuth)
 		;
diff --git a/apps/files_external/lib/dependencytrait.php b/apps/files_external/lib/dependencytrait.php
index 116421eab14..f0d6d6080e5 100644
--- a/apps/files_external/lib/dependencytrait.php
+++ b/apps/files_external/lib/dependencytrait.php
@@ -28,58 +28,13 @@ use \OCA\Files_External\Lib\MissingDependency;
  */
 trait DependencyTrait {
 
-	/** @var callable|null dependency check */
-	private $dependencyCheck = null;
-
-	/**
-	 * @return bool
-	 */
-	public function hasDependencies() {
-		return !is_null($this->dependencyCheck);
-	}
-
-	/**
-	 * @param callable $dependencyCheck
-	 * @return self
-	 */
-	public function setDependencyCheck(callable $dependencyCheck) {
-		$this->dependencyCheck = $dependencyCheck;
-		return $this;
-	}
-
 	/**
 	 * Check if object is valid for use
 	 *
 	 * @return MissingDependency[] Unsatisfied dependencies
 	 */
 	public function checkDependencies() {
-		$ret = [];
-
-		if ($this->hasDependencies()) {
-			$result = call_user_func($this->dependencyCheck);
-			if ($result !== true) {
-				if (!is_array($result)) {
-					$result = [$result];
-				}
-				foreach ($result as $key => $value) {
-					if (!($value instanceof MissingDependency)) {
-						$module = null;
-						$message = null;
-						if (is_numeric($key)) {
-							$module = $value;
-						} else {
-							$module = $key;
-							$message = $value;
-						}
-						$value = new MissingDependency($module, $this);
-						$value->setMessage($message);
-					}
-					$ret[] = $value;
-				}
-			}
-		}
-
-		return $ret;
+		return []; // no dependencies by default
 	}
 
 }
diff --git a/apps/files_external/lib/legacydependencycheckpolyfill.php b/apps/files_external/lib/legacydependencycheckpolyfill.php
new file mode 100644
index 00000000000..7bb137fb3e1
--- /dev/null
+++ b/apps/files_external/lib/legacydependencycheckpolyfill.php
@@ -0,0 +1,70 @@
+<?php
+/**
+ * @author Robin McCorkell <rmccorkell@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 OCA\Files_External\Lib;
+
+use \OCA\Files_External\Lib\MissingDependency;
+
+/**
+ * Polyfill for checking dependencies using legacy Storage::checkDependencies()
+ */
+trait LegacyDependencyCheckPolyfill {
+
+	/**
+	 * @return string
+	 */
+	abstract public function getStorageClass();
+
+	/**
+	 * Check if object is valid for use
+	 *
+	 * @return MissingDependency[] Unsatisfied dependencies
+	 */
+	public function checkDependencies() {
+		$ret = [];
+
+		$result = call_user_func([$this->getStorageClass(), 'checkDependencies']);
+		if ($result !== true) {
+			if (!is_array($result)) {
+				$result = [$result];
+			}
+			foreach ($result as $key => $value) {
+				if (!($value instanceof MissingDependency)) {
+					$module = null;
+					$message = null;
+					if (is_numeric($key)) {
+						$module = $value;
+					} else {
+						$module = $key;
+						$message = $value;
+					}
+					$value = new MissingDependency($module, $this);
+					$value->setMessage($message);
+				}
+				$ret[] = $value;
+			}
+		}
+
+		return $ret;
+	}
+
+}
+
diff --git a/apps/files_external/tests/backend/legacybackendtest.php b/apps/files_external/tests/backend/legacybackendtest.php
index 44cb16a4986..d57810de29b 100644
--- a/apps/files_external/tests/backend/legacybackendtest.php
+++ b/apps/files_external/tests/backend/legacybackendtest.php
@@ -23,15 +23,25 @@ namespace OCA\Files_External\Tests\Backend;
 
 use \OCA\Files_External\Lib\Backend\LegacyBackend;
 use \OCA\Files_External\Lib\DefinitionParameter;
+use \OCA\Files_External\Lib\MissingDependency;
 
 class LegacyBackendTest extends \Test\TestCase {
 
+	/**
+	 * @return MissingDependency[]
+	 */
+	public static function checkDependencies() {
+		return [
+			(new MissingDependency('abc'))->setMessage('foobar')
+		];
+	}
+
 	public function testConstructor() {
 		$auth = $this->getMockBuilder('\OCA\Files_External\Lib\Auth\Builtin')
 			->disableOriginalConstructor()
 			->getMock();
 
-		$class = '\OC\Files\Storage\SMB';
+		$class = '\OCA\Files_External\Tests\Backend\LegacyBackendTest';
 		$definition = [
 			'configuration' => [
 				'textfield' => 'Text field',
@@ -49,14 +59,18 @@ class LegacyBackendTest extends \Test\TestCase {
 
 		$backend = new LegacyBackend($class, $definition, $auth);
 
-		$this->assertEquals('\OC\Files\Storage\SMB', $backend->getStorageClass());
+		$this->assertEquals('\OCA\Files_External\Tests\Backend\LegacyBackendTest', $backend->getStorageClass());
 		$this->assertEquals('Backend text', $backend->getText());
 		$this->assertEquals(123, $backend->getPriority());
 		$this->assertEquals('foo/bar.js', $backend->getCustomJs());
-		$this->assertEquals(true, $backend->hasDependencies());
 		$this->assertArrayHasKey('builtin', $backend->getAuthSchemes());
 		$this->assertEquals($auth, $backend->getLegacyAuthMechanism());
 
+		$dependencies = $backend->checkDependencies();
+		$this->assertCount(1, $dependencies);
+		$this->assertEquals('abc', $dependencies[0]->getDependency());
+		$this->assertEquals('foobar', $dependencies[0]->getMessage());
+
 		$parameters = $backend->getParameters();
 		$this->assertEquals('Text field', $parameters['textfield']->getText());
 		$this->assertEquals(DefinitionParameter::VALUE_TEXT, $parameters['textfield']->getType());
@@ -78,4 +92,22 @@ class LegacyBackendTest extends \Test\TestCase {
 		$this->assertEquals(DefinitionParameter::FLAG_OPTIONAL, $parameters['optionalpassword']->getFlags());
 	}
 
+	public function testNoDependencies() {
+		$auth = $this->getMockBuilder('\OCA\Files_External\Lib\Auth\Builtin')
+			->disableOriginalConstructor()
+			->getMock();
+
+		$class = '\OCA\Files_External\Tests\Backend\LegacyBackendTest';
+		$definition = [
+			'configuration' => [
+			],
+			'backend' => 'Backend text',
+		];
+
+		$backend = new LegacyBackend($class, $definition, $auth);
+
+		$dependencies = $backend->checkDependencies();
+		$this->assertCount(0, $dependencies);
+	}
+
 }
diff --git a/apps/files_external/tests/dependencytraittest.php b/apps/files_external/tests/legacydependencycheckpolyfilltest.php
index 5706d97053d..49d825d77aa 100644
--- a/apps/files_external/tests/dependencytraittest.php
+++ b/apps/files_external/tests/legacydependencycheckpolyfilltest.php
@@ -23,16 +23,23 @@ namespace OCA\Files_External\Tests;
 
 use \OCA\Files_External\Lib\MissingDependency;
 
-class DependencyTraitTest extends \Test\TestCase {
+class LegacyDependencyCheckPolyfillTest extends \Test\TestCase {
+
+	/**
+	 * @return MissingDependency[]
+	 */
+	public static function checkDependencies() {
+		return [
+			(new MissingDependency('dependency'))->setMessage('missing dependency'),
+			(new MissingDependency('program'))->setMessage('cannot find program'),
+		];
+	}
 
 	public function testCheckDependencies() {
-		$trait = $this->getMockForTrait('\OCA\Files_External\Lib\DependencyTrait');
-		$trait->setDependencyCheck(function() {
-			return [
-				(new MissingDependency('dependency'))->setMessage('missing dependency'),
-				(new MissingDependency('program'))->setMessage('cannot find program'),
-			];
-		});
+		$trait = $this->getMockForTrait('\OCA\Files_External\Lib\LegacyDependencyCheckPolyfill');
+		$trait->expects($this->once())
+			->method('getStorageClass')
+			->willReturn('\OCA\Files_External\Tests\LegacyDependencyCheckPolyfillTest');
 
 		$dependencies = $trait->checkDependencies();
 		$this->assertCount(2, $dependencies);