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>

10 files changed, 67 insertions, 179 deletions

diff --git a/apps/user_ldap/lib/AppInfo/Application.php b/apps/user_ldap/lib/AppInfo/Application.php
index 93f7ccb379d..8dad63fbaf8 100644
--- a/apps/user_ldap/lib/AppInfo/Application.php
+++ b/apps/user_ldap/lib/AppInfo/Application.php
@@ -46,11 +46,9 @@ use OCP\AppFramework\Bootstrap\IBootstrap;
 use OCP\AppFramework\Bootstrap\IRegistrationContext;
 use OCP\AppFramework\IAppContainer;
 use OCP\EventDispatcher\IEventDispatcher;
-use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IServerContainer;
-use OCP\IUserSession;
 use OCP\Notification\IManager as INotificationManager;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
@@ -86,28 +84,23 @@ class Application extends App implements IBootstrap {
 	}
 
 	public function boot(IBootContext $context): void {
-		$context->injectFn(function (IConfig $config,
-									 INotificationManager $notificationManager,
-									 IUserSession $userSession,
-									 IAppContainer $appContainer,
-									 EventDispatcherInterface $legacyDispatcher,
-									 IEventDispatcher $dispatcher,
-									 IGroupManager $groupManager) {
-			$helper = new Helper($config);
+		$context->injectFn(function (
+			INotificationManager $notificationManager,
+			IAppContainer $appContainer,
+			EventDispatcherInterface $legacyDispatcher,
+			IEventDispatcher $dispatcher,
+			IGroupManager $groupManager,
+			User_Proxy $userBackend,
+			Group_Proxy $groupBackend,
+			Helper $helper
+		) {
 			$configPrefixes = $helper->getServerConfigurationPrefixes(true);
 			if (count($configPrefixes) > 0) {
-				$ldapWrapper = new LDAP();
-
 				$notificationManager->registerNotifierService(Notifier::class);
 
 				$userPluginManager = $appContainer->get(UserPluginManager::class);
 				$groupPluginManager = $appContainer->get(GroupPluginManager::class);
 
-				$userBackend = new User_Proxy(
-					$configPrefixes, $ldapWrapper, $config, $notificationManager, $userSession, $userPluginManager
-				);
-				$groupBackend = new Group_Proxy($configPrefixes, $ldapWrapper, $groupPluginManager);
-
 				\OC_User::useBackend($userBackend);
 				$groupManager->addBackend($groupBackend);
 
diff --git a/apps/user_ldap/lib/Command/Search.php b/apps/user_ldap/lib/Command/Search.php
index 5086f3a0c1a..2ee83e0f22b 100644
--- a/apps/user_ldap/lib/Command/Search.php
+++ b/apps/user_ldap/lib/Command/Search.php
@@ -29,11 +29,9 @@
 namespace OCA\User_LDAP\Command;
 
 use OCA\User_LDAP\Group_Proxy;
-use OCA\User_LDAP\GroupPluginManager;
 use OCA\User_LDAP\Helper;
 use OCA\User_LDAP\LDAP;
 use OCA\User_LDAP\User_Proxy;
-use OCA\User_LDAP\UserPluginManager;
 use OCP\IConfig;
 
 use Symfony\Component\Console\Command\Command;
@@ -45,13 +43,16 @@ use Symfony\Component\Console\Output\OutputInterface;
 class Search extends Command {
 	/** @var \OCP\IConfig */
 	protected $ocConfig;
+	/** @var User_Proxy */
+	private $userProxy;
+	/** @var Group_Proxy */
+	private $groupProxy;
 
-	/**
-	 * @param \OCP\IConfig $ocConfig
-	 */
-	public function __construct(IConfig $ocConfig) {
-		$this->ocConfig = $ocConfig;
+	public function __construct(IConfig $ocConfig, User_Proxy $userProxy, Group_Proxy $groupProxy) {
 		parent::__construct();
+		$this->ocConfig = $ocConfig;
+		$this->userProxy = $userProxy;
+		$this->groupProxy = $groupProxy;
 	}
 
 	protected function configure() {
@@ -117,7 +118,7 @@ class Search extends Command {
 		$this->validateOffsetAndLimit($offset, $limit);
 
 		if ($input->getOption('group')) {
-			$proxy = new Group_Proxy($configPrefixes, $ldapWrapper, \OC::$server->query(GroupPluginManager::class));
+			$proxy = $this->groupProxy;
 			$getMethod = 'getGroups';
 			$printID = false;
 			// convert the limit of groups to null. This will show all the groups available instead of
@@ -126,14 +127,7 @@ class Search extends Command {
 				$limit = null;
 			}
 		} else {
-			$proxy = new User_Proxy(
-				$configPrefixes,
-				$ldapWrapper,
-				$this->ocConfig,
-				\OC::$server->getNotificationManager(),
-				\OC::$server->getUserSession(),
-				\OC::$server->query(UserPluginManager::class)
-			);
+			$proxy = $this->userProxy;
 			$getMethod = 'getDisplayNames';
 			$printID = true;
 		}
diff --git a/apps/user_ldap/lib/Group_Proxy.php b/apps/user_ldap/lib/Group_Proxy.php
index 4b17c020d59..490eab44462 100644
--- a/apps/user_ldap/lib/Group_Proxy.php
+++ b/apps/user_ldap/lib/Group_Proxy.php
@@ -34,13 +34,9 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet
 	private $backends = [];
 	private $refBackend = null;
 
-	/**
-	 * Constructor
-	 *
-	 * @param string[] $serverConfigPrefixes array containing the config Prefixes
-	 */
-	public function __construct($serverConfigPrefixes, ILDAPWrapper $ldap, GroupPluginManager $groupPluginManager) {
+	public function __construct(Helper $helper, ILDAPWrapper $ldap, GroupPluginManager $groupPluginManager) {
 		parent::__construct($ldap);
+		$serverConfigPrefixes = $helper->getServerConfigurationPrefixes(true);
 		foreach ($serverConfigPrefixes as $configPrefix) {
 			$this->backends[$configPrefix] =
 				new \OCA\User_LDAP\Group_LDAP($this->getAccess($configPrefix), $groupPluginManager);
diff --git a/apps/user_ldap/lib/Helper.php b/apps/user_ldap/lib/Helper.php
index a8d66998148..86712534f40 100644
--- a/apps/user_ldap/lib/Helper.php
+++ b/apps/user_ldap/lib/Helper.php
@@ -76,7 +76,7 @@ class Helper {
 	 * except the default (first) server shall be connected to.
 	 *
 	 */
-	public function getServerConfigurationPrefixes($activeConfigurations = false) {
+	public function getServerConfigurationPrefixes($activeConfigurations = false): array {
 		$referenceConfigkey = 'ldap_configuration_active';
 
 		$keys = $this->getServersConfig($referenceConfigkey);
@@ -188,18 +188,11 @@ class Helper {
 
 	/**
 	 * checks whether there is one or more disabled LDAP configurations
-	 *
-	 * @return bool
-	 * @throws \Exception
 	 */
-	public function haveDisabledConfigurations() {
+	public function haveDisabledConfigurations(): bool {
 		$all = $this->getServerConfigurationPrefixes(false);
 		$active = $this->getServerConfigurationPrefixes(true);
 
-		if (!is_array($all) || !is_array($active)) {
-			throw new \Exception('Unexpected Return Value');
-		}
-
 		return count($all) !== count($active) || count($all) === 0;
 	}
 
@@ -312,20 +305,7 @@ class Helper {
 			throw new \Exception('key uid is expected to be set in $param');
 		}
 
-		//ain't it ironic?
-		$helper = new Helper(\OC::$server->getConfig());
-
-		$configPrefixes = $helper->getServerConfigurationPrefixes(true);
-		$ldapWrapper = new LDAP();
-		$ocConfig = \OC::$server->getConfig();
-		$notificationManager = \OC::$server->getNotificationManager();
-
-		$userSession = \OC::$server->getUserSession();
-		$userPluginManager = \OC::$server->query(UserPluginManager::class);
-
-		$userBackend = new User_Proxy(
-			$configPrefixes, $ldapWrapper, $ocConfig, $notificationManager, $userSession, $userPluginManager
-		);
+		$userBackend = \OC::$server->get(User_Proxy::class);
 		$uid = $userBackend->loginName2UserName($param['uid']);
 		if ($uid !== false) {
 			$param['uid'] = $uid;
diff --git a/apps/user_ldap/lib/Jobs/CleanUp.php b/apps/user_ldap/lib/Jobs/CleanUp.php
index 8f729ae4372..6bb44b3e6ae 100644
--- a/apps/user_ldap/lib/Jobs/CleanUp.php
+++ b/apps/user_ldap/lib/Jobs/CleanUp.php
@@ -35,7 +35,6 @@ use OCA\User_LDAP\Mapping\UserMapping;
 use OCA\User_LDAP\User\DeletedUsersIndex;
 use OCA\User_LDAP\User_LDAP;
 use OCA\User_LDAP\User_Proxy;
-use OCA\User_LDAP\UserPluginManager;
 
 /**
  * Class CleanUp
@@ -69,10 +68,11 @@ class CleanUp extends TimedJob {
 	/** @var DeletedUsersIndex */
 	protected $dui;
 
-	public function __construct() {
+	public function __construct(User_Proxy $userBackend) {
 		$minutes = \OC::$server->getConfig()->getSystemValue(
 			'ldapUserCleanupInterval', (string)$this->defaultIntervalMin);
 		$this->setInterval((int)$minutes * 60);
+		$this->userBackend = $userBackend;
 	}
 
 	/**
@@ -99,15 +99,6 @@ class CleanUp extends TimedJob {
 
 		if (isset($arguments['userBackend'])) {
 			$this->userBackend = $arguments['userBackend'];
-		} else {
-			$this->userBackend = new User_Proxy(
-				$this->ldapHelper->getServerConfigurationPrefixes(true),
-				new LDAP(),
-				$this->ocConfig,
-				\OC::$server->getNotificationManager(),
-				\OC::$server->getUserSession(),
-				\OC::$server->query(UserPluginManager::class)
-			);
 		}
 
 		if (isset($arguments['db'])) {
diff --git a/apps/user_ldap/lib/Jobs/UpdateGroups.php b/apps/user_ldap/lib/Jobs/UpdateGroups.php
index 33587f8bdb2..34dddb3d287 100644
--- a/apps/user_ldap/lib/Jobs/UpdateGroups.php
+++ b/apps/user_ldap/lib/Jobs/UpdateGroups.php
@@ -33,42 +33,36 @@
 
 namespace OCA\User_LDAP\Jobs;
 
-use OCA\User_LDAP\Access;
-use OCA\User_LDAP\Connection;
-use OCA\User_LDAP\FilesystemHelper;
-use OCA\User_LDAP\GroupPluginManager;
-use OCA\User_LDAP\Helper;
-use OCA\User_LDAP\LDAP;
-use OCA\User_LDAP\LogWrapper;
-use OCA\User_LDAP\Mapping\GroupMapping;
-use OCA\User_LDAP\Mapping\UserMapping;
-use OCA\User_LDAP\User\Manager;
+use OC\BackgroundJob\TimedJob;
+use OCA\User_LDAP\Group_Proxy;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Group\Events\UserAddedEvent;
 use OCP\Group\Events\UserRemovedEvent;
 use OCP\ILogger;
 
-class UpdateGroups extends \OC\BackgroundJob\TimedJob {
-	private static $groupsFromDB;
+class UpdateGroups extends TimedJob {
+	private $groupsFromDB;
 
-	private static $groupBE;
+	/** @var Group_Proxy */
+	private $groupBackend;
 
-	public function __construct() {
-		$this->interval = self::getRefreshInterval();
+	public function __construct(Group_Proxy $groupBackend) {
+		$this->interval = $this->getRefreshInterval();
+		$this->groupBackend = $groupBackend;
 	}
 
 	/**
 	 * @param mixed $argument
 	 */
 	public function run($argument) {
-		self::updateGroups();
+		$this->updateGroups();
 	}
 
-	public static function updateGroups() {
+	public function updateGroups() {
 		\OCP\Util::writeLog('user_ldap', 'Run background job "updateGroups"', ILogger::DEBUG);
 
-		$knownGroups = array_keys(self::getKnownGroups());
-		$actualGroups = self::getGroupBE()->getGroups();
+		$knownGroups = array_keys($this->getKnownGroups());
+		$actualGroups = $this->groupBackend->getGroups();
 
 		if (empty($actualGroups) && empty($knownGroups)) {
 			\OCP\Util::writeLog('user_ldap',
@@ -77,9 +71,9 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob {
 			return;
 		}
 
-		self::handleKnownGroups(array_intersect($actualGroups, $knownGroups));
-		self::handleCreatedGroups(array_diff($actualGroups, $knownGroups));
-		self::handleRemovedGroups(array_diff($knownGroups, $actualGroups));
+		$this->handleKnownGroups(array_intersect($actualGroups, $knownGroups));
+		$this->handleCreatedGroups(array_diff($actualGroups, $knownGroups));
+		$this->handleRemovedGroups(array_diff($knownGroups, $actualGroups));
 
 		\OCP\Util::writeLog('user_ldap', 'bgJ "updateGroups" – Finished.', ILogger::DEBUG);
 	}
@@ -87,7 +81,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob {
 	/**
 	 * @return int
 	 */
-	private static function getRefreshInterval() {
+	private function getRefreshInterval() {
 		//defaults to every hour
 		return \OC::$server->getConfig()->getAppValue('user_ldap', 'bgjRefreshInterval', 3600);
 	}
@@ -95,7 +89,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob {
 	/**
 	 * @param string[] $groups
 	 */
-	private static function handleKnownGroups($groups) {
+	private function handleKnownGroups($groups) {
 		/** @var IEventDispatcher $dispatcher */
 		$dispatcher = \OC::$server->query(IEventDispatcher::class);
 		$groupManager = \OC::$server->getGroupManager();
@@ -107,10 +101,12 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob {
 			SET `owncloudusers` = ?
 			WHERE `owncloudname` = ?
 		');
+		if (!is_array($this->groupsFromDB)) {
+			$this->getKnownGroups();
+		}
 		foreach ($groups as $group) {
-			//we assume, that self::$groupsFromDB has been retrieved already
-			$knownUsers = unserialize(self::$groupsFromDB[$group]['owncloudusers']);
-			$actualUsers = self::getGroupBE()->usersInGroup($group);
+			$knownUsers = unserialize($this->groupsFromDB[$group]['owncloudusers']);
+			$actualUsers = $this->groupBackend->usersInGroup($group);
 			$hasChanged = false;
 
 			$groupObject = $groupManager->get($group);
@@ -142,7 +138,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob {
 	/**
 	 * @param string[] $createdGroups
 	 */
-	private static function handleCreatedGroups($createdGroups) {
+	private function handleCreatedGroups($createdGroups) {
 		\OCP\Util::writeLog('user_ldap', 'bgJ "updateGroups" – dealing with created Groups.', ILogger::DEBUG);
 		$query = \OC_DB::prepare('
 			INSERT
@@ -153,7 +149,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob {
 			\OCP\Util::writeLog('user_ldap',
 				'bgJ "updateGroups" – new group "'.$createdGroup.'" found.',
 				ILogger::INFO);
-			$users = serialize(self::getGroupBE()->usersInGroup($createdGroup));
+			$users = serialize($this->groupBackend->usersInGroup($createdGroup));
 			$query->execute([$createdGroup, $users]);
 		}
 		\OCP\Util::writeLog('user_ldap',
@@ -164,7 +160,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob {
 	/**
 	 * @param string[] $removedGroups
 	 */
-	private static function handleRemovedGroups($removedGroups) {
+	private function handleRemovedGroups($removedGroups) {
 		\OCP\Util::writeLog('user_ldap', 'bgJ "updateGroups" – dealing with removed groups.', ILogger::DEBUG);
 		$query = \OC_DB::prepare('
 			DELETE
@@ -183,58 +179,22 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob {
 	}
 
 	/**
-	 * @return \OCA\User_LDAP\Group_LDAP|\OCA\User_LDAP\Group_Proxy
-	 */
-	private static function getGroupBE() {
-		if (!is_null(self::$groupBE)) {
-			return self::$groupBE;
-		}
-		$helper = new Helper(\OC::$server->getConfig());
-		$configPrefixes = $helper->getServerConfigurationPrefixes(true);
-		$ldapWrapper = new LDAP();
-		if (count($configPrefixes) === 1) {
-			//avoid the proxy when there is only one LDAP server configured
-			$dbc = \OC::$server->getDatabaseConnection();
-			$userManager = new Manager(
-				\OC::$server->getConfig(),
-				new FilesystemHelper(),
-				new LogWrapper(),
-				\OC::$server->getAvatarManager(),
-				new \OCP\Image(),
-				$dbc,
-				\OC::$server->getUserManager(),
-				\OC::$server->getNotificationManager());
-			$connector = new Connection($ldapWrapper, $configPrefixes[0]);
-			$ldapAccess = new Access($connector, $ldapWrapper, $userManager, $helper, \OC::$server->getConfig(), \OC::$server->getUserManager());
-			$groupMapper = new GroupMapping($dbc);
-			$userMapper = new UserMapping($dbc);
-			$ldapAccess->setGroupMapper($groupMapper);
-			$ldapAccess->setUserMapper($userMapper);
-			self::$groupBE = new \OCA\User_LDAP\Group_LDAP($ldapAccess, \OC::$server->query(GroupPluginManager::class));
-		} else {
-			self::$groupBE = new \OCA\User_LDAP\Group_Proxy($configPrefixes, $ldapWrapper, \OC::$server->query(GroupPluginManager::class));
-		}
-
-		return self::$groupBE;
-	}
-
-	/**
 	 * @return array
 	 */
-	private static function getKnownGroups() {
-		if (is_array(self::$groupsFromDB)) {
-			return self::$groupsFromDB;
+	private function getKnownGroups() {
+		if (is_array($this->groupsFromDB)) {
+			$this->groupsFromDB;
 		}
 		$query = \OC_DB::prepare('
 			SELECT `owncloudname`, `owncloudusers`
 			FROM `*PREFIX*ldap_group_members`
 		');
 		$result = $query->execute()->fetchAll();
-		self::$groupsFromDB = [];
+		$this->groupsFromDB = [];
 		foreach ($result as $dataset) {
-			self::$groupsFromDB[$dataset['owncloudname']] = $dataset;
+			$this->groupsFromDB[$dataset['owncloudname']] = $dataset;
 		}
 
-		return self::$groupsFromDB;
+		return $this->groupsFromDB;
 	}
 }
diff --git a/apps/user_ldap/lib/Migration/UUIDFix.php b/apps/user_ldap/lib/Migration/UUIDFix.php
index 8648f979211..ba2264c3c63 100644
--- a/apps/user_ldap/lib/Migration/UUIDFix.php
+++ b/apps/user_ldap/lib/Migration/UUIDFix.php
@@ -50,11 +50,4 @@ abstract class UUIDFix extends QueuedJob {
 			}
 		}
 	}
-
-	/**
-	 * @param Proxy $proxy
-	 */
-	public function overrideProxy(Proxy $proxy) {
-		$this->proxy = $proxy;
-	}
 }
diff --git a/apps/user_ldap/lib/Migration/UUIDFixGroup.php b/apps/user_ldap/lib/Migration/UUIDFixGroup.php
index b40cc8881e6..2cf961f550a 100644
--- a/apps/user_ldap/lib/Migration/UUIDFixGroup.php
+++ b/apps/user_ldap/lib/Migration/UUIDFixGroup.php
@@ -26,18 +26,12 @@
 
 namespace OCA\User_LDAP\Migration;
 
-use OCA\User_LDAP\Helper;
-use OCA\User_LDAP\LDAP;
+use OCA\User_LDAP\Group_Proxy;
 use OCA\User_LDAP\Mapping\GroupMapping;
-use OCA\User_LDAP\User_Proxy;
-use OCA\User_LDAP\UserPluginManager;
-use OCP\IConfig;
 
 class UUIDFixGroup extends UUIDFix {
-	public function __construct(GroupMapping $mapper, LDAP $ldap, IConfig $config, Helper $helper) {
+	public function __construct(GroupMapping $mapper, Group_Proxy $proxy) {
 		$this->mapper = $mapper;
-		$this->proxy = new User_Proxy($helper->getServerConfigurationPrefixes(true), $ldap, $config,
-			\OC::$server->getNotificationManager(), \OC::$server->getUserSession(),
-			\OC::$server->query(UserPluginManager::class));
+		$this->proxy = $proxy;
 	}
 }
diff --git a/apps/user_ldap/lib/Migration/UUIDFixUser.php b/apps/user_ldap/lib/Migration/UUIDFixUser.php
index f2be0c5ed96..4ea58c45620 100644
--- a/apps/user_ldap/lib/Migration/UUIDFixUser.php
+++ b/apps/user_ldap/lib/Migration/UUIDFixUser.php
@@ -26,17 +26,12 @@
 
 namespace OCA\User_LDAP\Migration;
 
-use OCA\User_LDAP\Group_Proxy;
-use OCA\User_LDAP\GroupPluginManager;
-use OCA\User_LDAP\Helper;
-use OCA\User_LDAP\LDAP;
+use OCA\User_LDAP\User_Proxy;
 use OCA\User_LDAP\Mapping\UserMapping;
-use OCP\IConfig;
 
 class UUIDFixUser extends UUIDFix {
-	public function __construct(UserMapping $mapper, LDAP $ldap, IConfig $config, Helper $helper) {
+	public function __construct(UserMapping $mapper, User_Proxy $proxy) {
 		$this->mapper = $mapper;
-		$groupPluginManager = \OC::$server->query(GroupPluginManager::class);
-		$this->proxy = new Group_Proxy($helper->getServerConfigurationPrefixes(true), $ldap, $groupPluginManager);
+		$this->proxy = $proxy;
 	}
 }
diff --git a/apps/user_ldap/lib/User_Proxy.php b/apps/user_ldap/lib/User_Proxy.php
index 6445af89112..e8d0a6d6948 100644
--- a/apps/user_ldap/lib/User_Proxy.php
+++ b/apps/user_ldap/lib/User_Proxy.php
@@ -42,17 +42,8 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface,
 	/** @var User_LDAP */
 	private $refBackend = null;
 
-	/**
-	 * Constructor
-	 *
-	 * @param array $serverConfigPrefixes array containing the config Prefixes
-	 * @param ILDAPWrapper $ldap
-	 * @param IConfig $ocConfig
-	 * @param INotificationManager $notificationManager
-	 * @param IUserSession $userSession
-	 */
 	public function __construct(
-		array $serverConfigPrefixes,
+		Helper $helper,
 		ILDAPWrapper $ldap,
 		IConfig $ocConfig,
 		INotificationManager $notificationManager,
@@ -60,6 +51,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface,
 		UserPluginManager $userPluginManager
 	) {
 		parent::__construct($ldap);
+		$serverConfigPrefixes = $helper->getServerConfigurationPrefixes(true);
 		foreach ($serverConfigPrefixes as $configPrefix) {
 			$this->backends[$configPrefix] =
 				new User_LDAP($this->getAccess($configPrefix), $ocConfig, $notificationManager, $userSession, $userPluginManager);