Improve iterators

Signed-off-by: Benjamin Gaussorgues <[email protected]>
nextcloud · Nov 9, 2023 · 6c2206e · 6c2206e
1 parent 347765d
commit 6c2206e
Show file tree

Hide file tree

Showing 8 changed files with 104 additions and 94 deletions.
diff --git a/Makefile b/Makefile
@@ -10,11 +10,11 @@ updater.phar: updater.php lib/*.php buildVersionFile.php
 clean:
 	rm updater.phar index.php
 
-index.php: lib/UpdateException.php lib/LogException.php lib/RecursiveDirectoryIteratorWithoutData.php lib/Updater.php index.web.php
+index.php: lib/UpdateException.php lib/LogException.php lib/RecursiveDirectoryIteratorFilter.php lib/Updater.php index.web.php
 	# First put openining php tag and license
 	awk '/^<\?php$$/,/\*\//' index.web.php > index.php
 	# Then concat all files while filtering php tag and license
-	cat lib/UpdateException.php lib/LogException.php lib/RecursiveDirectoryIteratorWithoutData.php lib/Updater.php index.web.php| grep -v "^namespace" | awk '/^<\?php$$/,/\*\//{next} 1' >> index.php
+	cat lib/UpdateException.php lib/LogException.php lib/RecursiveDirectoryIteratorFilter.php lib/Updater.php index.web.php| grep -v "^namespace" | awk '/^<\?php$$/,/\*\//{next} 1' >> index.php
 
 test/vendor:
 	cd tests && composer install

diff --git a/index.php b/index.php
@@ -41,28 +41,25 @@ class LogException extends \Exception {
 }
 
 
-class RecursiveDirectoryIteratorWithoutData extends \RecursiveFilterIterator {
-	public function accept(): bool {
-		$excludes = [
-			'.rnd',
-			'.well-known',
-			'data',
-			'..',
-		];
+class RecursiveDirectoryIteratorFilter extends \RecursiveFilterIterator {
+	private array $excludedPaths;
 
-		/** @var \SplFileInfo|false */
-		$current = $this->current();
-		if (!$current) {
-			return false;
-		}
+	public function __construct(
+		\RecursiveDirectoryIterator $iterator,
+		array $excludedPaths = ['data'],
+	) {
+		parent::__construct($iterator);
+		$this->excludedPaths = array_flip($excludedPaths);
+	}
 
-		return !(in_array($current->getFilename(), $excludes, true) || $current->isDir());
+	public function accept(): bool {
+		return !isset($this->excludedPaths[$this->current()->getFilename()]);
 	}
 }
 
 
 class Updater {
-	private string $baseDir;
+	private string $nextcloudDir;
 	private array $configValues = [];
 	private string $currentVersion = 'unknown';
 	private string $buildTime;
@@ -75,13 +72,15 @@ class Updater {
 	 * @param string $baseDir the absolute path to the /updater/ directory in the Nextcloud root
 	 * @throws \Exception
 	 */
-	public function __construct(string $baseDir) {
-		$this->baseDir = $baseDir;
+	public function __construct(
+		private string $baseDir
+	) {
+		$this->nextcloudDir = realpath(dirname($baseDir));
 
 		if ($dir = getenv('NEXTCLOUD_CONFIG_DIR')) {
-			$configFileName = rtrim($dir, '/') . '/config.php';
+			$configFileName = realpath($dir . '/config.php');
 		} else {
-			$configFileName = $this->baseDir . '/../config/config.php';
+			$configFileName = $this->nextcloudDir . '/config/config.php';
 		}
 		if (!file_exists($configFileName)) {
 			throw new \Exception('Could not find config.php. Is this file in the "updater" subfolder of Nextcloud?');
@@ -102,7 +101,7 @@ public function __construct(string $baseDir) {
 			throw new \Exception('Could not read data directory from config.php.');
 		}
 
-		$versionFileName = $this->baseDir . '/../version.php';
+		$versionFileName = $this->nextcloudDir . '/version.php';
 		if (!file_exists($versionFileName)) {
 			// fallback to version in config.php
 			$version = $this->getConfigOptionString('version');
@@ -133,27 +132,23 @@ public function __construct(string $baseDir) {
 
 	/**
 	 * Returns whether the web updater is disabled
-	 *
-	 * @return bool
 	 */
-	public function isDisabled() {
+	public function isDisabled(): bool {
 		return $this->disabled;
 	}
 
 	/**
 	 * Returns current version or "unknown" if this could not be determined.
-	 *
-	 * @return string
 	 */
-	public function getCurrentVersion() {
+	public function getCurrentVersion(): string {
 		return $this->currentVersion;
 	}
 
 	/**
 	 * Returns currently used release channel
 	 */
 	private function getCurrentReleaseChannel(): string {
-		return ($this->getConfigOptionString('updater.release.channel') ?? 'stable');
+		return $this->getConfigOptionString('updater.release.channel') ?? 'stable';
 	}
 
 	/**
@@ -323,16 +318,19 @@ private function getAppDirectories(): array {
 	/**
 	 * Gets the recursive directory iterator over the Nextcloud folder
 	 *
-	 * @return \RecursiveIteratorIterator<\RecursiveDirectoryIterator>
+	 * @return \RecursiveIteratorIterator<\RecursiveDirectoryIterator|RecursiveDirectoryIteratorFilter>
 	 */
-	private function getRecursiveDirectoryIterator(?string $folder = null): \RecursiveIteratorIterator {
+	private function getRecursiveDirectoryIterator(?string $folder = null, array $excludedPaths = []): \RecursiveIteratorIterator {
 		if ($folder === null) {
 			$folder = $this->baseDir . '/../';
 		}
-		return new \RecursiveIteratorIterator(
-			new \RecursiveDirectoryIterator($folder, \RecursiveDirectoryIterator::SKIP_DOTS),
-			\RecursiveIteratorIterator::CHILD_FIRST
-		);
+
+		$iterator = new \RecursiveDirectoryIterator($folder, \FilesystemIterator::SKIP_DOTS);
+		if (!empty($excludedPaths)) {
+			$iterator = new RecursiveDirectoryIteratorFilter($iterator, $excludedPaths);
+		}
+
+		return new \RecursiveIteratorIterator($iterator, \RecursiveIteratorIterator::CHILD_FIRST);
 	}
 
 	/**
@@ -343,7 +341,7 @@ public function checkForExpectedFilesAndFolders(): void {
 
 		$expectedElements = $this->getExpectedElementsList();
 		$unexpectedElements = [];
-		foreach (new \DirectoryIterator($this->baseDir . '/../') as $fileInfo) {
+		foreach (new \DirectoryIterator($this->nextcloudDir) as $fileInfo) {
 			if (array_search($fileInfo->getFilename(), $expectedElements) === false) {
 				$unexpectedElements[] = $fileInfo->getFilename();
 			}
@@ -361,15 +359,21 @@ public function checkForExpectedFilesAndFolders(): void {
 	public function checkWritePermissions(): void {
 		$this->silentLog('[info] checkWritePermissions()');
 
-		$notWritablePaths = array();
-		$dir = new \RecursiveDirectoryIterator($this->baseDir . '/../');
-		$filter = new RecursiveDirectoryIteratorWithoutData($dir);
-		/** @var iterable<string, \SplFileInfo> */
-		$it = new \RecursiveIteratorIterator($filter);
+		$excludedPaths = [
+			'.rnd' => true,
+			'.well-known' => true,
+			'data' => true,
+		];
+
+		$it = new \DirectoryIterator($this->nextcloudDir);
 
-		foreach ($it as $path => $dir) {
-			if (!is_writable($path)) {
-				$notWritablePaths[] = $path;
+		$notWritablePaths = [];
+		foreach ($it as $path => $fileInfo) {
+			if ($fileInfo->isDot() || isset($excludedPaths[$fileInfo->getFilename()])) {
+				continue;
+			}
+			if (!$fileInfo->isWritable()) {
+				$notWritablePaths[] = $fileInfo->getFilename();
 			}
 		}
 		if (count($notWritablePaths) > 0) {
@@ -442,7 +446,7 @@ public function createBackup(): void {
 		 * @var string $path
 		 * @var \SplFileInfo $fileInfo
 		 */
-		foreach ($this->getRecursiveDirectoryIterator($currentDir) as $path => $fileInfo) {
+		foreach ($this->getRecursiveDirectoryIterator($currentDir, $excludedElements) as $path => $fileInfo) {
 			$fileName = explode($currentDir, $path)[1];
 			$folderStructure = explode('/', $fileName, -1);
 
@@ -971,8 +975,9 @@ public function deleteOldFiles(): void {
 	}
 
 	/**
-	 * Moves the specified filed except the excluded elements to the correct position
+	 * Moves the specified files except the excluded elements to the correct position
 	 *
+	 * @param string[] $excludedElements
 	 * @throws \Exception
 	 */
 	private function moveWithExclusions(string $dataLocation, array $excludedElements): void {

diff --git a/...RecursiveDirectoryIteratorWithoutData.php → lib/RecursiveDirectoryIteratorFilter.php b/...RecursiveDirectoryIteratorWithoutData.php → lib/RecursiveDirectoryIteratorFilter.php
@@ -22,21 +22,18 @@
 
 namespace NC\Updater;
 
-class RecursiveDirectoryIteratorWithoutData extends \RecursiveFilterIterator {
-	public function accept(): bool {
-		$excludes = [
-			'.rnd',
-			'.well-known',
-			'data',
-			'..',
-		];
+class RecursiveDirectoryIteratorFilter extends \RecursiveFilterIterator {
+	private array $excludedPaths;
 
-		/** @var \SplFileInfo|false */
-		$current = $this->current();
-		if (!$current) {
-			return false;
-		}
+	public function __construct(
+		\RecursiveDirectoryIterator $iterator,
+		array $excludedPaths = ['data'],
+	) {
+		parent::__construct($iterator);
+		$this->excludedPaths = array_flip($excludedPaths);
+	}
 
-		return !(in_array($current->getFilename(), $excludes, true) || $current->isDir());
+	public function accept(): bool {
+		return !isset($this->excludedPaths[$this->current()->getFilename()]);
 	}
 }
diff --git a/lib/Updater.php b/lib/Updater.php
@@ -24,7 +24,7 @@
 namespace NC\Updater;
 
 class Updater {
-	private string $baseDir;
+	private string $nextcloudDir;
 	private array $configValues = [];
 	private string $currentVersion = 'unknown';
 	private string $buildTime;
@@ -37,13 +37,15 @@ class Updater {
 	 * @param string $baseDir the absolute path to the /updater/ directory in the Nextcloud root
 	 * @throws \Exception
 	 */
-	public function __construct(string $baseDir) {
-		$this->baseDir = $baseDir;
+	public function __construct(
+		private string $baseDir
+	) {
+		$this->nextcloudDir = realpath(dirname($baseDir));
 
 		if ($dir = getenv('NEXTCLOUD_CONFIG_DIR')) {
-			$configFileName = rtrim($dir, '/') . '/config.php';
+			$configFileName = realpath($dir . '/config.php');
 		} else {
-			$configFileName = $this->baseDir . '/../config/config.php';
+			$configFileName = $this->nextcloudDir . '/config/config.php';
 		}
 		if (!file_exists($configFileName)) {
 			throw new \Exception('Could not find config.php. Is this file in the "updater" subfolder of Nextcloud?');
@@ -64,7 +66,7 @@ public function __construct(string $baseDir) {
 			throw new \Exception('Could not read data directory from config.php.');
 		}
 
-		$versionFileName = $this->baseDir . '/../version.php';
+		$versionFileName = $this->nextcloudDir . '/version.php';
 		if (!file_exists($versionFileName)) {
 			// fallback to version in config.php
 			$version = $this->getConfigOptionString('version');
@@ -95,27 +97,23 @@ public function __construct(string $baseDir) {
 
 	/**
 	 * Returns whether the web updater is disabled
-	 *
-	 * @return bool
 	 */
-	public function isDisabled() {
+	public function isDisabled(): bool {
 		return $this->disabled;
 	}
 
 	/**
 	 * Returns current version or "unknown" if this could not be determined.
-	 *
-	 * @return string
 	 */
-	public function getCurrentVersion() {
+	public function getCurrentVersion(): string {
 		return $this->currentVersion;
 	}
 
 	/**
 	 * Returns currently used release channel
 	 */
 	private function getCurrentReleaseChannel(): string {
-		return ($this->getConfigOptionString('updater.release.channel') ?? 'stable');
+		return $this->getConfigOptionString('updater.release.channel') ?? 'stable';
 	}
 
 	/**
@@ -285,16 +283,19 @@ private function getAppDirectories(): array {
 	/**
 	 * Gets the recursive directory iterator over the Nextcloud folder
 	 *
-	 * @return \RecursiveIteratorIterator<\RecursiveDirectoryIterator>
+	 * @return \RecursiveIteratorIterator<\RecursiveDirectoryIterator|RecursiveDirectoryIteratorFilter>
 	 */
-	private function getRecursiveDirectoryIterator(?string $folder = null): \RecursiveIteratorIterator {
+	private function getRecursiveDirectoryIterator(?string $folder = null, array $excludedPaths = []): \RecursiveIteratorIterator {
 		if ($folder === null) {
 			$folder = $this->baseDir . '/../';
 		}
-		return new \RecursiveIteratorIterator(
-			new \RecursiveDirectoryIterator($folder, \RecursiveDirectoryIterator::SKIP_DOTS),
-			\RecursiveIteratorIterator::CHILD_FIRST
-		);
+
+		$iterator = new \RecursiveDirectoryIterator($folder, \FilesystemIterator::SKIP_DOTS);
+		if (!empty($excludedPaths)) {
+			$iterator = new RecursiveDirectoryIteratorFilter($iterator, $excludedPaths);
+		}
+
+		return new \RecursiveIteratorIterator($iterator, \RecursiveIteratorIterator::CHILD_FIRST);
 	}
 
 	/**
@@ -305,7 +306,7 @@ public function checkForExpectedFilesAndFolders(): void {
 
 		$expectedElements = $this->getExpectedElementsList();
 		$unexpectedElements = [];
-		foreach (new \DirectoryIterator($this->baseDir . '/../') as $fileInfo) {
+		foreach (new \DirectoryIterator($this->nextcloudDir) as $fileInfo) {
 			if (array_search($fileInfo->getFilename(), $expectedElements) === false) {
 				$unexpectedElements[] = $fileInfo->getFilename();
 			}
@@ -323,15 +324,21 @@ public function checkForExpectedFilesAndFolders(): void {
 	public function checkWritePermissions(): void {
 		$this->silentLog('[info] checkWritePermissions()');
 
-		$notWritablePaths = array();
-		$dir = new \RecursiveDirectoryIterator($this->baseDir . '/../');
-		$filter = new RecursiveDirectoryIteratorWithoutData($dir);
-		/** @var iterable<string, \SplFileInfo> */
-		$it = new \RecursiveIteratorIterator($filter);
+		$excludedPaths = [
+			'.rnd' => true,
+			'.well-known' => true,
+			'data' => true,
+		];
+
+		$it = new \DirectoryIterator($this->nextcloudDir);
 
-		foreach ($it as $path => $dir) {
-			if (!is_writable($path)) {
-				$notWritablePaths[] = $path;
+		$notWritablePaths = [];
+		foreach ($it as $path => $fileInfo) {
+			if ($fileInfo->isDot() || isset($excludedPaths[$fileInfo->getFilename()])) {
+				continue;
+			}
+			if (!$fileInfo->isWritable()) {
+				$notWritablePaths[] = $fileInfo->getFilename();
 			}
 		}
 		if (count($notWritablePaths) > 0) {
@@ -404,7 +411,7 @@ public function createBackup(): void {
 		 * @var string $path
 		 * @var \SplFileInfo $fileInfo
 		 */
-		foreach ($this->getRecursiveDirectoryIterator($currentDir) as $path => $fileInfo) {
+		foreach ($this->getRecursiveDirectoryIterator($currentDir, $excludedElements) as $path => $fileInfo) {
 			$fileName = explode($currentDir, $path)[1];
 			$folderStructure = explode('/', $fileName, -1);
 
@@ -933,8 +940,9 @@ public function deleteOldFiles(): void {
 	}
 
 	/**
-	 * Moves the specified filed except the excluded elements to the correct position
+	 * Moves the specified files except the excluded elements to the correct position
 	 *
+	 * @param string[] $excludedElements
 	 * @throws \Exception
 	 */
 	private function moveWithExclusions(string $dataLocation, array $excludedElements): void {

diff --git a/updater.phar b/updater.phar
diff --git a/vendor/composer/autoload_classmap.php b/vendor/composer/autoload_classmap.php
@@ -24,7 +24,7 @@
     'JsonException' => $vendorDir . '/symfony/polyfill-php73/Resources/stubs/JsonException.php',
     'NC\\Updater\\CommandApplication' => $baseDir . '/lib/CommandApplication.php',
     'NC\\Updater\\LogException' => $baseDir . '/lib/LogException.php',
-    'NC\\Updater\\RecursiveDirectoryIteratorWithoutData' => $baseDir . '/lib/RecursiveDirectoryIteratorWithoutData.php',
+    'NC\\Updater\\RecursiveDirectoryIteratorFilter' => $baseDir . '/lib/RecursiveDirectoryIteratorFilter.php',
     'NC\\Updater\\UpdateCommand' => $baseDir . '/lib/UpdateCommand.php',
     'NC\\Updater\\UpdateException' => $baseDir . '/lib/UpdateException.php',
     'NC\\Updater\\Updater' => $baseDir . '/lib/Updater.php',