Merge pull request #55629 from nextcloud/carl/cleanup-propagator

refactor(IPropagator): Cleanup implementation

Author: susnux

lib/private/Files/Cache/HomePropagator.php

@@ -1,37 +1,19 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 namespace OC\Files\Cache;
 
+use OCP\Files\Storage\IStorage;
 use OCP\IDBConnection;
 
 class HomePropagator extends Propagator {
-	private $ignoredBaseFolders;
-
-	/**
-	 * @param \OC\Files\Storage\Storage $storage
-	 */
-	public function __construct(\OC\Files\Storage\Storage $storage, IDBConnection $connection) {
-		parent::__construct($storage, $connection);
-		$this->ignoredBaseFolders = ['files_encryption'];
-	}
-
-
-	/**
-	 * @param string $internalPath
-	 * @param int $time
-	 * @param int $sizeDifference number of bytes the file has grown
-	 */
-	public function propagateChange($internalPath, $time, $sizeDifference = 0) {
-		[$baseFolder] = explode('/', $internalPath, 2);
-		if (in_array($baseFolder, $this->ignoredBaseFolders)) {
-			return [];
-		} else {
-			parent::propagateChange($internalPath, $time, $sizeDifference);
-		}
+	public function __construct(IStorage $storage, IDBConnection $connection) {
+		parent::__construct($storage, $connection, ignore: ['files_encryption']);
 	}
 }

lib/private/Files/Cache/Propagator.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -12,60 +14,40 @@
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Files\Cache\IPropagator;
 use OCP\Files\Storage\IReliableEtagStorage;
+use OCP\Files\Storage\IStorage;
 use OCP\IDBConnection;
 use OCP\Server;
+use Override;
 use Psr\Clock\ClockInterface;
 use Psr\Log\LoggerInterface;
 
-/**
- * Propagate etags and mtimes within the storage
- */
 class Propagator implements IPropagator {
 	public const MAX_RETRIES = 3;
-	private $inBatch = false;
-
-	private $batch = [];
-
-	/**
-	 * @var \OC\Files\Storage\Storage
-	 */
-	protected $storage;
-
-	/**
-	 * @var IDBConnection
-	 */
-	private $connection;
-
-	/**
-	 * @var array
-	 */
-	private $ignore = [];
 
+	private bool $inBatch = false;
+	private array $batch = [];
 	private ClockInterface $clock;
 
-	public function __construct(\OC\Files\Storage\Storage $storage, IDBConnection $connection, array $ignore = []) {
-		$this->storage = $storage;
-		$this->connection = $connection;
-		$this->ignore = $ignore;
+	public function __construct(
+		protected readonly IStorage $storage,
+		private readonly IDBConnection $connection,
+		private readonly array $ignore = [],
+	) {
 		$this->clock = Server::get(ClockInterface::class);
 	}
 
-	/**
-	 * @param string $internalPath
-	 * @param int $time
-	 * @param int $sizeDifference number of bytes the file has grown
-	 */
-	public function propagateChange($internalPath, $time, $sizeDifference = 0) {
+	#[Override]
+	public function propagateChange(string $internalPath, int $time, int $sizeDifference = 0): void {
 		// Do not propagate changes in ignored paths
 		foreach ($this->ignore as $ignore) {
 			if (str_starts_with($internalPath, $ignore)) {
 				return;
 			}
 		}
 
-		$time = min((int)$time, $this->clock->now()->getTimestamp());
+		$time = min($time, $this->clock->now()->getTimestamp());
 
-		$storageId = $this->storage->getStorageCache()->getNumericId();
+		$storageId = $this->storage->getCache()->getNumericStorageId();
 
 		$parents = $this->getParents($internalPath);
 
@@ -137,7 +119,10 @@ public function propagateChange($internalPath, $time, $sizeDifference = 0) {
 		}
 	}
 
-	protected function getParents($path) {
+	/**
+	 * @return string[]
+	 */
+	protected function getParents(string $path): array {
 		$parts = explode('/', $path);
 		$parent = '';
 		$parents = [];
@@ -148,19 +133,12 @@ protected function getParents($path) {
 		return $parents;
 	}
 
-	/**
-	 * Mark the beginning of a propagation batch
-	 *
-	 * Note that not all cache setups support propagation in which case this will be a noop
-	 *
-	 * Batching for cache setups that do support it has to be explicit since the cache state is not fully consistent
-	 * before the batch is committed.
-	 */
-	public function beginBatch() {
+	#[Override]
+	public function beginBatch(): void {
 		$this->inBatch = true;
 	}
 
-	private function addToBatch($internalPath, $time, $sizeDifference) {
+	private function addToBatch(string $internalPath, int $time, int $sizeDifference): void {
 		if (!isset($this->batch[$internalPath])) {
 			$this->batch[$internalPath] = [
 				'hash' => md5($internalPath),
@@ -175,10 +153,8 @@ private function addToBatch($internalPath, $time, $sizeDifference) {
 		}
 	}
 
-	/**
-	 * Commit the active propagation batch
-	 */
-	public function commitBatch() {
+	#[Override]
+	public function commitBatch(): void {
 		if (!$this->inBatch) {
 			throw new \BadMethodCallException('Not in batch');
 		}
@@ -187,7 +163,7 @@ public function commitBatch() {
 		$this->connection->beginTransaction();
 
 		$query = $this->connection->getQueryBuilder();
-		$storageId = (int)$this->storage->getStorageCache()->getNumericId();
+		$storageId = $this->storage->getCache()->getNumericStorageId();
 
 		$query->update('filecache')
 			->set('mtime', $query->func()->greatest('mtime', $query->createParameter('time')))

lib/private/Files/Cache/Wrapper/JailPropagator.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -8,21 +10,14 @@
 
 use OC\Files\Cache\Propagator;
 use OC\Files\Storage\Wrapper\Jail;
+use Override;
 
 class JailPropagator extends Propagator {
-	/**
-	 * @var Jail
-	 */
-	protected $storage;
-
-	/**
-	 * @param string $internalPath
-	 * @param int $time
-	 * @param int $sizeDifference
-	 */
-	public function propagateChange($internalPath, $time, $sizeDifference = 0) {
-		/** @var \OC\Files\Storage\Storage $storage */
-		[$storage, $sourceInternalPath] = $this->storage->resolvePath($internalPath);
+	#[Override]
+	public function propagateChange(string $internalPath, int $time, int $sizeDifference = 0): void {
+		/** @var Jail $jail */
+		$jail = $this->storage;
+		[$storage, $sourceInternalPath] = $jail->resolvePath($internalPath);
 		$storage->getPropagator()->propagateChange($sourceInternalPath, $time, $sizeDifference);
 	}
 }

lib/private/Files/Storage/Wrapper/Jail.php

@@ -215,7 +215,9 @@ public function changeLock(string $path, int $type, ILockingProvider $provider):
 	}
 
 	/**
-	 * Resolve the path for the source of the share
+	 * Resolve the path for the source of the share.
+	 *
+	 * @return array{0: IStorage, 1: string}
 	 */
 	public function resolvePath(string $path): array {
 		return [$this->getWrapperStorage(), $this->getUnjailedPath($path)];

lib/public/Files/Cache/IPropagator.php

@@ -1,20 +1,26 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
+
 namespace OCP\Files\Cache;
 
+use OCP\AppFramework\Attribute\Consumable;
+
 /**
- * Propagate etags and mtimes within the storage
+ * Propagate ETags and mtimes within the storage.
  *
  * @since 9.0.0
  */
+#[Consumable(since: '9.0.0')]
 interface IPropagator {
 	/**
-	 * Mark the beginning of a propagation batch
+	 * Mark the beginning of a propagation batch.
 	 *
 	 * Note that not all cache setups support propagation in which case this will be a noop
 	 *
@@ -23,20 +29,20 @@ interface IPropagator {
 	 *
 	 * @since 9.1.0
 	 */
-	public function beginBatch();
+	public function beginBatch(): void;
 
 	/**
-	 * Commit the active propagation batch
+	 * Commit the active propagation batch.
 	 *
 	 * @since 9.1.0
 	 */
-	public function commitBatch();
+	public function commitBatch(): void;
 
 	/**
 	 * @param string $internalPath
 	 * @param int $time
-	 * @param int $sizeDifference
+	 * @param int $sizeDifference The number of bytes the file has grown.
 	 * @since 9.0.0
 	 */
-	public function propagateChange($internalPath, $time, $sizeDifference = 0);
+	public function propagateChange(string $internalPath, int $time, int $sizeDifference = 0): void;
 }