LDAP simplify User_Proxy and Group_Proxy signatures

- make User_Proxy and Group_Proxy easy to instantiate
- simplify dependent code
- move commands to info.xml
- make UpdateGroups job class non-static

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

5 files changed, 49 insertions, 69 deletions

diff --git a/apps/user_ldap/tests/Jobs/CleanUpTest.php b/apps/user_ldap/tests/Jobs/CleanUpTest.php
index 1e88694f7bf..533bb041f75 100644
--- a/apps/user_ldap/tests/Jobs/CleanUpTest.php
+++ b/apps/user_ldap/tests/Jobs/CleanUpTest.php
@@ -26,44 +26,49 @@
 
 namespace OCA\User_LDAP\Tests\Jobs;
 
+use Exception;
 use OCA\User_LDAP\Helper;
+use OCA\User_LDAP\Jobs\CleanUp;
+use OCA\User_LDAP\User\DeletedUsersIndex;
+use OCA\User_LDAP\User_Proxy;
 use OCP\IConfig;
 use OCP\IDBConnection;
+use Test\TestCase;
 
-class CleanUpTest extends \Test\TestCase {
-	public function getMocks() {
-		$mocks = [];
-		$mocks['userBackend'] =
-			$this->getMockBuilder('\OCA\User_LDAP\User_Proxy')
-				->disableOriginalConstructor()
-				->getMock();
-		$mocks['deletedUsersIndex'] =
-			$this->getMockBuilder('\OCA\User_LDAP\User\DeletedUsersIndex')
-				->disableOriginalConstructor()
-				->getMock();
-		$mocks['ocConfig'] = $this->createMock(IConfig::class);
-		$mocks['db'] = $this->createMock(IDBConnection::class);
-		$mocks['helper'] = $this->createMock(Helper::class);
-
-		return $mocks;
+class CleanUpTest extends TestCase {
+	/** @var CleanUp */
+	protected $bgJob;
+
+	/** @var array */
+	protected $mocks;
+
+	public function setUp(): void {
+		$this->createMocks();
+		$this->bgJob = new CleanUp($this->mocks['userBackend']);
+		$this->bgJob->setArguments($this->mocks);
+	}
+
+	protected function createMocks(): void {
+		$this->mocks = [];
+		$this->mocks['userBackend'] = $this->createMock(User_Proxy::class);
+		$this->mocks['deletedUsersIndex'] = $this->createMock(DeletedUsersIndex::class);
+		$this->mocks['ocConfig'] = $this->createMock(IConfig::class);
+		$this->mocks['db'] = $this->createMock(IDBConnection::class);
+		$this->mocks['helper'] = $this->createMock(Helper::class);
 	}
 
 	/**
 	 * clean up job must not run when there are disabled configurations
 	 */
 	public function test_runNotAllowedByDisabledConfigurations() {
-		$args = $this->getMocks();
-		$args['helper']->expects($this->once())
+		$this->mocks['helper']->expects($this->once())
 			->method('haveDisabledConfigurations')
 			->willReturn(true);
 
-		$args['ocConfig']->expects($this->never())
+		$this->mocks['ocConfig']->expects($this->never())
 			->method('getSystemValue');
 
-		$bgJob = new \OCA\User_LDAP\Jobs\CleanUp();
-		$bgJob->setArguments($args);
-
-		$result = $bgJob->isCleanUpAllowed();
+		$result = $this->bgJob->isCleanUpAllowed();
 		$this->assertSame(false, $result);
 	}
 
@@ -72,18 +77,14 @@ class CleanUpTest extends \Test\TestCase {
 	 * returning unexpected results
 	 */
 	public function test_runNotAllowedByBrokenHelper() {
-		$args = $this->getMocks();
-		$args['helper']->expects($this->once())
+		$this->mocks['helper']->expects($this->once())
 			->method('haveDisabledConfigurations')
-			->will($this->throwException(new \Exception()));
+			->will($this->throwException(new Exception()));
 
-		$args['ocConfig']->expects($this->never())
+		$this->mocks['ocConfig']->expects($this->never())
 			->method('getSystemValue');
 
-		$bgJob = new \OCA\User_LDAP\Jobs\CleanUp();
-		$bgJob->setArguments($args);
-
-		$result = $bgJob->isCleanUpAllowed();
+		$result = $this->bgJob->isCleanUpAllowed();
 		$this->assertSame(false, $result);
 	}
 
@@ -91,19 +92,15 @@ class CleanUpTest extends \Test\TestCase {
 	 * clean up job must not run when it is not enabled
 	 */
 	public function test_runNotAllowedBySysConfig() {
-		$args = $this->getMocks();
-		$args['helper']->expects($this->once())
+		$this->mocks['helper']->expects($this->once())
 			->method('haveDisabledConfigurations')
 			->willReturn(false);
 
-		$args['ocConfig']->expects($this->once())
+		$this->mocks['ocConfig']->expects($this->once())
 			->method('getSystemValue')
 			->willReturn(false);
 
-		$bgJob = new \OCA\User_LDAP\Jobs\CleanUp();
-		$bgJob->setArguments($args);
-
-		$result = $bgJob->isCleanUpAllowed();
+		$result = $this->bgJob->isCleanUpAllowed();
 		$this->assertSame(false, $result);
 	}
 
@@ -111,19 +108,15 @@ class CleanUpTest extends \Test\TestCase {
 	 * clean up job is allowed to run
 	 */
 	public function test_runIsAllowed() {
-		$args = $this->getMocks();
-		$args['helper']->expects($this->once())
+		$this->mocks['helper']->expects($this->once())
 			->method('haveDisabledConfigurations')
 			->willReturn(false);
 
-		$args['ocConfig']->expects($this->once())
+		$this->mocks['ocConfig']->expects($this->once())
 			->method('getSystemValue')
 			->willReturn(true);
 
-		$bgJob = new \OCA\User_LDAP\Jobs\CleanUp();
-		$bgJob->setArguments($args);
-
-		$result = $bgJob->isCleanUpAllowed();
+		$result = $this->bgJob->isCleanUpAllowed();
 		$this->assertSame(true, $result);
 	}
 
@@ -131,12 +124,7 @@ class CleanUpTest extends \Test\TestCase {
 	 * check whether offset will be reset when it needs to
 	 */
 	public function test_OffsetResetIsNecessary() {
-		$args = $this->getMocks();
-
-		$bgJob = new \OCA\User_LDAP\Jobs\CleanUp();
-		$bgJob->setArguments($args);
-
-		$result = $bgJob->isOffsetResetNecessary($bgJob->getChunkSize() - 1);
+		$result = $this->bgJob->isOffsetResetNecessary($this->bgJob->getChunkSize() - 1);
 		$this->assertSame(true, $result);
 	}
 
@@ -144,12 +132,7 @@ class CleanUpTest extends \Test\TestCase {
 	 * make sure offset is not reset when it is not due
 	 */
 	public function test_OffsetResetIsNotNecessary() {
-		$args = $this->getMocks();
-
-		$bgJob = new \OCA\User_LDAP\Jobs\CleanUp();
-		$bgJob->setArguments($args);
-
-		$result = $bgJob->isOffsetResetNecessary($bgJob->getChunkSize());
+		$result = $this->bgJob->isOffsetResetNecessary($this->bgJob->getChunkSize());
 		$this->assertSame(false, $result);
 	}
 }
diff --git a/apps/user_ldap/tests/Migration/AbstractUUIDFixTest.php b/apps/user_ldap/tests/Migration/AbstractUUIDFixTest.php
index ec484dfe7eb..82483a54327 100644
--- a/apps/user_ldap/tests/Migration/AbstractUUIDFixTest.php
+++ b/apps/user_ldap/tests/Migration/AbstractUUIDFixTest.php
@@ -74,18 +74,13 @@ abstract class AbstractUUIDFixTest extends TestCase {
 			->willReturn(['s01', 's03']);
 	}
 
-	protected function mockProxy($className) {
-		$this->proxy = $this->createMock($className);
+	protected function instantiateJob($className) {
+		$this->job = new $className($this->mapper, $this->proxy);
 		$this->proxy->expects($this->any())
 			->method('getLDAPAccess')
 			->willReturn($this->access);
 	}
 
-	protected function instantiateJob($className) {
-		$this->job = new $className($this->mapper, $this->ldap, $this->config, $this->helper);
-		$this->job->overrideProxy($this->proxy);
-	}
-
 	public function testRunSingleRecord() {
 		$args = [
 			'records' => [
diff --git a/apps/user_ldap/tests/Migration/UUIDFixGroupTest.php b/apps/user_ldap/tests/Migration/UUIDFixGroupTest.php
index a1f04d44670..fb9d2e2331c 100644
--- a/apps/user_ldap/tests/Migration/UUIDFixGroupTest.php
+++ b/apps/user_ldap/tests/Migration/UUIDFixGroupTest.php
@@ -40,11 +40,9 @@ class UUIDFixGroupTest extends AbstractUUIDFixTest {
 		$this->isUser = false;
 		parent::setUp();
 
-		$this->isUser = false;
-
 		$this->mapper = $this->createMock(GroupMapping::class);
+		$this->proxy = $this->createMock(Group_Proxy::class);
 
-		$this->mockProxy(Group_Proxy::class);
 		$this->instantiateJob(UUIDFixGroup::class);
 	}
 }
diff --git a/apps/user_ldap/tests/Migration/UUIDFixUserTest.php b/apps/user_ldap/tests/Migration/UUIDFixUserTest.php
index 188c7b0f91d..cf80bf36230 100644
--- a/apps/user_ldap/tests/Migration/UUIDFixUserTest.php
+++ b/apps/user_ldap/tests/Migration/UUIDFixUserTest.php
@@ -40,8 +40,8 @@ class UUIDFixUserTest extends AbstractUUIDFixTest {
 		parent::setUp();
 
 		$this->mapper = $this->createMock(UserMapping::class);
+		$this->proxy = $this->createMock(User_Proxy::class);
 
-		$this->mockProxy(User_Proxy::class);
 		$this->instantiateJob(UUIDFixUser::class);
 	}
 }
diff --git a/apps/user_ldap/tests/User_ProxyTest.php b/apps/user_ldap/tests/User_ProxyTest.php
index 5753990a736..0bbea00ed44 100644
--- a/apps/user_ldap/tests/User_ProxyTest.php
+++ b/apps/user_ldap/tests/User_ProxyTest.php
@@ -29,6 +29,7 @@
 
 namespace OCA\User_LDAP\Tests;
 
+use OCA\User_LDAP\Helper;
 use OCA\User_LDAP\ILDAPWrapper;
 use OCA\User_LDAP\User_Proxy;
 use OCA\User_LDAP\UserPluginManager;
@@ -38,6 +39,8 @@ use OCP\Notification\IManager as INotificationManager;
 use Test\TestCase;
 
 class User_ProxyTest extends TestCase {
+	/** @var Helper|\PHPUnit\Framework\MockObject\MockObject */
+	protected $helper;
 	/** @var ILDAPWrapper|\PHPUnit\Framework\MockObject\MockObject */
 	private $ldapWrapper;
 	/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
@@ -54,6 +57,7 @@ class User_ProxyTest extends TestCase {
 	protected function setUp(): void {
 		parent::setUp();
 
+		$this->helper = $this->createMock(Helper::class);
 		$this->ldapWrapper = $this->createMock(ILDAPWrapper::class);
 		$this->config = $this->createMock(IConfig::class);
 		$this->notificationManager = $this->createMock(INotificationManager::class);
@@ -61,7 +65,7 @@ class User_ProxyTest extends TestCase {
 		$this->userPluginManager = $this->createMock(UserPluginManager::class);
 		$this->proxy = $this->getMockBuilder(User_Proxy::class)
 			->setConstructorArgs([
-				[],
+				$this->helper,
 				$this->ldapWrapper,
 				$this->config,
 				$this->notificationManager,