Refactor code to split common loop

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

1 files changed, 15 insertions, 37 deletions

diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php
index 3db3c6c7664..b0d5909fd81 100644
--- a/apps/user_ldap/lib/Group_LDAP.php
+++ b/apps/user_ldap/lib/Group_LDAP.php
@@ -305,7 +305,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
 			$fetcher = function ($memberDN) use (&$seen) {
 				return $this->_groupMembers($memberDN, $seen);
 			};
-			$allMembers = $this->walkNestedGroups($dnGroup, $fetcher, $members, $seen);
+			$allMembers = $this->walkNestedGroupsReturnDNs($dnGroup, $fetcher, $members, $seen);
 		}
 
 		$allMembers += $this->getDynamicGroupMembers($dnGroup);
@@ -352,29 +352,11 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
 			return $nestedGroups;
 		};
 
-		$groups = $this->walkNestedGroups($dn, $fetcher, $groups);
+		$groups = $this->walkNestedGroupsReturnDNs($dn, $fetcher, $groups);
 		return $this->filterValidGroups($groups);
 	}
 
-	private function walkNestedGroups(string $dn, Closure $fetcher, array $list, array &$seen = []): array {
-		$nesting = (int)$this->access->connection->ldapNestedGroups;
-		// depending on the input, we either have a list of DNs or a list of LDAP records
-		// also, the output expects either DNs or records. Testing the first element should suffice.
-		$recordMode = isset($list[0]) && is_array($list[0]) && isset($list[0]['dn'][0]);
-
-		if ($nesting !== 1) {
-			if ($recordMode) {
-				// the keys are numeric, but should hold the DN
-				return array_reduce($list, function ($transformed, $record) use ($dn) {
-					if ($record['dn'][0] != $dn) {
-						$transformed[$record['dn'][0]] = $record;
-					}
-					return $transformed;
-				}, []);
-			}
-			return $list;
-		}
-
+	private function processListFromWalkingNestedGroups(array &$list, array &$seen, string $dn, Closure $fetcher): void {
 		while ($record = array_shift($list)) {
 			$recordDN = $record['dn'][0] ?? $record;
 			if ($recordDN === $dn || array_key_exists($recordDN, $seen)) {
@@ -387,9 +369,17 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
 				$seen[$recordDN] = $record;
 			}
 		}
+	}
+
+	private function walkNestedGroupsReturnDNs(string $dn, Closure $fetcher, array $list, array &$seen = []): array {
+		$nesting = (int)$this->access->connection->ldapNestedGroups;
+
+		if ($nesting !== 1) {
+			return $list;
+		}
 
-		// on record mode, filter out intermediate state
-		return $recordMode ? array_filter($seen, 'is_array') : array_keys($seen);
+		$this->processListFromWalkingNestedGroups($list, $seen, $dn, $fetcher);
+		return array_keys($seen);
 	}
 
 	private function walkNestedGroupsReturnRecords(string $dn, Closure $fetcher, array $list, array &$seen = []): array {
@@ -405,20 +395,8 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
 			}, []);
 		}
 
-		while ($record = array_shift($list)) {
-			$recordDN = $record['dn'][0] ?? $record;
-			if ($recordDN === $dn || array_key_exists($recordDN, $seen)) {
-				// Prevent loops
-				continue;
-			}
-			$fetched = $fetcher($record);
-			$list = array_merge($list, $fetched);
-			if (!isset($seen[$recordDN]) || is_bool($seen[$recordDN]) && is_array($record)) {
-				$seen[$recordDN] = $record;
-			}
-		}
-
-		// on record mode, filter out intermediate state
+		$this->processListFromWalkingNestedGroups($list, $seen, $dn, $fetcher);
+		// filter out intermediate state
 		return array_filter($seen, 'is_array');
 	}