read records from DB for lists at once, not one by one.

Keep a runtime cache of dn-id-mapping

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

4 files changed, 80 insertions, 12 deletions

diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php
index c087211cec7..a564f6183bc 100644
--- a/apps/user_ldap/lib/Access.php
+++ b/apps/user_ldap/lib/Access.php
@@ -871,9 +871,17 @@ class Access extends LDAPUtility {
 		if (!$forceApplyAttributes) {
 			$isBackgroundJobModeAjax = $this->config
 					->getAppValue('core', 'backgroundjobs_mode', 'ajax') === 'ajax';
-			$recordsToUpdate = array_filter($ldapRecords, function ($record) use ($isBackgroundJobModeAjax) {
+			$listOfDNs = array_reduce($ldapRecords, function ($listOfDNs, $entry) {
+				$listOfDNs[] = $entry['dn'][0];
+				return $listOfDNs;
+			}, []);
+			$idsByDn = $this->userMapper->getListOfIdsByDn($listOfDNs);
+			$recordsToUpdate = array_filter($ldapRecords, function($record) use ($isBackgroundJobModeAjax, $idsByDn) {
 				$newlyMapped = false;
-				$uid = $this->dn2ocname($record['dn'][0], null, true, $newlyMapped, $record);
+				$uid = $idsByDn[$record['dn'][0]] ?? null;
+				if($uid === null) {
+					$uid = $this->dn2ocname($record['dn'][0], null, true, $newlyMapped, $record);
+				}
 				if (is_string($uid)) {
 					$this->cacheUserExists($uid);
 				}
@@ -925,9 +933,19 @@ class Access extends LDAPUtility {
 	 */
 	public function fetchListOfGroups($filter, $attr, $limit = null, $offset = null) {
 		$groupRecords = $this->searchGroups($filter, $attr, $limit, $offset);
-		array_walk($groupRecords, function ($record) {
+
+		$listOfDNs = array_reduce($groupRecords, function ($listOfDNs, $entry) {
+			$listOfDNs[] = $entry['dn'][0];
+			return$listOfDNs;
+		}, []);
+		$idsByDn = $this->groupMapper->getListOfIdsByDn($listOfDNs);
+
+		array_walk($groupRecords, function($record) use ($idsByDn) {
 			$newlyMapped = false;
-			$gid = $this->dn2ocname($record['dn'][0], null, false, $newlyMapped, $record);
+			$gid = $uidsByDn[$record['dn'][0]] ?? null;
+			if($gid === null) {
+				$gid = $this->dn2ocname($record['dn'][0], null, false, $newlyMapped, $record);
+			}
 			if (!$newlyMapped && is_string($gid)) {
 				$this->cacheGroupExists($gid);
 			}
diff --git a/apps/user_ldap/lib/Mapping/AbstractMapping.php b/apps/user_ldap/lib/Mapping/AbstractMapping.php
index e14c9a572de..c88b84635f9 100644
--- a/apps/user_ldap/lib/Mapping/AbstractMapping.php
+++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php
@@ -26,8 +26,11 @@
 
 namespace OCA\User_LDAP\Mapping;
 
+use OC\DB\QueryBuilder\QueryBuilder;
+
 /**
  * Class AbstractMapping
+*
  * @package OCA\User_LDAP\Mapping
  */
 abstract class AbstractMapping {
@@ -40,7 +43,7 @@ abstract class AbstractMapping {
 	 * returns the DB table name which holds the mappings
 	 * @return string
 	 */
-	abstract protected function getTableName();
+	abstract protected function getTableName(bool $includePrefix = true);
 
 	/**
 	 * @param \OCP\IDBConnection $dbc
@@ -49,6 +52,9 @@ abstract class AbstractMapping {
 		$this->dbc = $dbc;
 	}
 
+	/** @var array caches Names (value) by DN (key) */
+	protected $cache = [];
+
 	/**
 	 * checks whether a provided string represents an existing table col
 	 * @param string $col
@@ -111,7 +117,12 @@ abstract class AbstractMapping {
 	 * @return string|false
 	 */
 	public function getDNByName($name) {
-		return $this->getXbyY('ldap_dn', 'owncloud_name', $name);
+		$dn = array_search($name, $this->cache);
+		if($dn === false) {
+			$dn = $this->getXbyY('ldap_dn', 'owncloud_name', $name);
+			$this->cache[$dn] = $name;
+		}
+		return $dn;
 	}
 
 	/**
@@ -121,13 +132,21 @@ abstract class AbstractMapping {
 	 * @return bool
 	 */
 	public function setDNbyUUID($fdn, $uuid) {
+		$oldDn = $this->getDnByUUID($uuid);
 		$query = $this->dbc->prepare('
 			UPDATE `' . $this->getTableName() . '`
 			SET `ldap_dn` = ?
 			WHERE `directory_uuid` = ?
 		');
 
-		return $this->modify($query, [$fdn, $uuid]);
+		$r = $this->modify($query, [$fdn, $uuid]);
+
+		if($r && is_string($oldDn) && isset($this->cache[$oldDn])) {
+			$this->cache[$fdn] = $this->cache[$oldDn];
+			unset($this->cache[$oldDn]);
+		}
+
+		return $r;
 	}
 
 	/**
@@ -146,6 +165,8 @@ abstract class AbstractMapping {
 			WHERE `ldap_dn` = ?
 		');
 
+		unset($this->cache[$fdn]);
+
 		return $this->modify($query, [$uuid, $fdn]);
 	}
 
@@ -155,7 +176,27 @@ abstract class AbstractMapping {
 	 * @return string|false
 	 */
 	public function getNameByDN($fdn) {
-		return $this->getXbyY('owncloud_name', 'ldap_dn', $fdn);
+		if(!isset($this->cache[$fdn])) {
+			$this->cache[$fdn] = $this->getXbyY('owncloud_name', 'ldap_dn', $fdn);
+		}
+		return $this->cache[$fdn];
+	}
+
+	public function getListOfIdsByDn(array $fdns): array {
+		$qb = $this->dbc->getQueryBuilder();
+		$qb->select('owncloud_name', 'ldap_dn')
+			->from($this->getTableName(false))
+			->where($qb->expr()->in('ldap_dn', $qb->createNamedParameter($fdns, QueryBuilder::PARAM_STR_ARRAY)));
+		$stmt = $qb->execute();
+
+		$results = $stmt->fetchAll(\Doctrine\DBAL\FetchMode::ASSOCIATIVE);
+		foreach ($results as $key => $entry) {
+			unset($results[$key]);
+			$results[$entry['ldap_dn']] = $entry['owncloud_name'];
+			$this->cache[$entry['ldap_dn']] = $entry['owncloud_name'];
+		}
+
+		return $results;
 	}
 
 	/**
@@ -191,6 +232,10 @@ abstract class AbstractMapping {
 		return $this->getXbyY('owncloud_name', 'directory_uuid', $uuid);
 	}
 
+	public function getDnByUUID($uuid) {
+		return $this->getXbyY('ldap_dn', 'directory_uuid', $uuid);
+	}
+
 	/**
 	 * Gets the UUID based on the provided LDAP DN
 	 * @param string $dn
@@ -249,6 +294,9 @@ abstract class AbstractMapping {
 
 		try {
 			$result = $this->dbc->insertIfNotExist($this->getTableName(), $row);
+			if((bool)$result === true) {
+				$this->cache[$fdn] = $name;
+			}
 			// insertIfNotExist returns values as int
 			return (bool)$result;
 		} catch (\Exception $e) {
diff --git a/apps/user_ldap/lib/Mapping/GroupMapping.php b/apps/user_ldap/lib/Mapping/GroupMapping.php
index b2c1b9c99af..703cc56a02a 100644
--- a/apps/user_ldap/lib/Mapping/GroupMapping.php
+++ b/apps/user_ldap/lib/Mapping/GroupMapping.php
@@ -33,7 +33,8 @@ class GroupMapping extends AbstractMapping {
 	 * returns the DB table name which holds the mappings
 	 * @return string
 	 */
-	protected function getTableName() {
-		return '*PREFIX*ldap_group_mapping';
+	protected function getTableName(bool $includePrefix = true) {
+		$p = $includePrefix ? '*PREFIX*' : '';
+		return $p . 'ldap_group_mapping';
 	}
 }
diff --git a/apps/user_ldap/lib/Mapping/UserMapping.php b/apps/user_ldap/lib/Mapping/UserMapping.php
index 556f7ecf1a4..b70cb866904 100644
--- a/apps/user_ldap/lib/Mapping/UserMapping.php
+++ b/apps/user_ldap/lib/Mapping/UserMapping.php
@@ -33,7 +33,8 @@ class UserMapping extends AbstractMapping {
 	 * returns the DB table name which holds the mappings
 	 * @return string
 	 */
-	protected function getTableName() {
-		return '*PREFIX*ldap_user_mapping';
+	protected function getTableName(bool $includePrefix = true) {
+		$p = $includePrefix ? '*PREFIX*' : '';
+		return $p . 'ldap_user_mapping';
 	}
 }