Merge pull request #17082 from owncloud/shared-etag-propagate-file

Fix etag propagation for single file shares

4 files changed, 51 insertions, 12 deletions

diff --git a/apps/files_sharing/lib/propagation/changewatcher.php b/apps/files_sharing/lib/propagation/changewatcher.php
index 483f436e289..3988eddf6f5 100644
--- a/apps/files_sharing/lib/propagation/changewatcher.php
+++ b/apps/files_sharing/lib/propagation/changewatcher.php
@@ -25,10 +25,17 @@ class ChangeWatcher {
 	private $baseView;
 
 	/**
+	 * @var RecipientPropagator
+	 */
+	private $recipientPropagator;
+
+	/**
 	 * @param \OC\Files\View $baseView the view for the logged in user
+	 * @param RecipientPropagator $recipientPropagator
 	 */
-	public function __construct(View $baseView) {
+	public function __construct(View $baseView, RecipientPropagator $recipientPropagator) {
 		$this->baseView = $baseView;
+		$this->recipientPropagator = $recipientPropagator;
 	}
 
 
@@ -39,6 +46,12 @@ class ChangeWatcher {
 		if ($mount instanceof SharedMount) {
 			$this->propagateForOwner($mount->getShare(), $mount->getInternalPath($fullPath), $mount->getOwnerPropagator());
 		}
+		$info = $this->baseView->getFileInfo($path);
+		if ($info) {
+			// trigger propagation if the subject of the write hook is shared.
+			// if a parent folder of $path is shared the propagation will be triggered from the change propagator hooks
+			$this->recipientPropagator->propagateById($info->getId());
+		}
 	}
 
 	public function renameHook($params) {
diff --git a/apps/files_sharing/lib/propagation/propagationmanager.php b/apps/files_sharing/lib/propagation/propagationmanager.php
index fa073be7f60..227da7cbcb8 100644
--- a/apps/files_sharing/lib/propagation/propagationmanager.php
+++ b/apps/files_sharing/lib/propagation/propagationmanager.php
@@ -75,7 +75,7 @@ class PropagationManager {
 		if (isset($this->sharePropagators[$user])) {
 			return $this->sharePropagators[$user];
 		}
-		$this->sharePropagators[$user] = new RecipientPropagator($user, $this->getChangePropagator($user), $this->config);
+		$this->sharePropagators[$user] = new RecipientPropagator($user, $this->getChangePropagator($user), $this->config, $this);
 		return $this->sharePropagators[$user];
 	}
 
@@ -101,7 +101,8 @@ class PropagationManager {
 		if (!$user) {
 			return;
 		}
-		$watcher = new ChangeWatcher(Filesystem::getView());
+		$recipientPropagator = $this->getSharePropagator($user->getUID());
+		$watcher = new ChangeWatcher(Filesystem::getView(), $recipientPropagator);
 
 		// for marking shares owned by the active user as dirty when a file inside them changes
 		$this->listenToOwnerChanges($user->getUID(), $user->getUID());
diff --git a/apps/files_sharing/lib/propagation/recipientpropagator.php b/apps/files_sharing/lib/propagation/recipientpropagator.php
index 5b7651f2ce7..c426128782a 100644
--- a/apps/files_sharing/lib/propagation/recipientpropagator.php
+++ b/apps/files_sharing/lib/propagation/recipientpropagator.php
@@ -32,16 +32,23 @@ class RecipientPropagator {
 	protected $config;
 
 	/**
+	 * @var PropagationManager
+	 */
+	private $manager;
+
+	/**
 	 * @param string $userId current user, must match the propagator's
 	 * user
 	 * @param \OC\Files\Cache\ChangePropagator $changePropagator change propagator
 	 * initialized with a view for $user
 	 * @param \OCP\IConfig $config
+	 * @param PropagationManager $manager
 	 */
-	public function __construct($userId, $changePropagator, $config) {
+	public function __construct($userId, $changePropagator, $config, PropagationManager $manager) {
 		$this->userId = $userId;
 		$this->changePropagator = $changePropagator;
 		$this->config = $config;
+		$this->manager = $manager;
 	}
 
 	/**
@@ -101,18 +108,24 @@ class RecipientPropagator {
 	 */
 	public function attachToPropagator(ChangePropagator $propagator, $owner) {
 		$propagator->listen('\OC\Files', 'propagate', function ($path, $entry) use ($owner) {
-			$shares = Share::getAllSharesForFileId($entry['fileid']);
-			foreach ($shares as $share) {
-				// propagate down the share tree
-				$this->markDirty($share, microtime(true));
+			$this->propagateById($entry['fileid']);
+		});
+	}
 
-				// propagate up the share tree
-				$user = $share['uid_owner'];
+	public function propagateById($id) {
+		$shares = Share::getAllSharesForFileId($id);
+		foreach ($shares as $share) {
+			// propagate down the share tree
+			$this->markDirty($share, microtime(true));
+
+			// propagate up the share tree
+			$user = $share['uid_owner'];
+			if($user !== $this->userId) {
 				$view = new View('/' . $user . '/files');
 				$path = $view->getPath($share['file_source']);
-				$watcher = new ChangeWatcher($view);
+				$watcher = new ChangeWatcher($view, $this->manager->getSharePropagator($user));
 				$watcher->writeHook(['path' => $path]);
 			}
-		});
+		}
 	}
 }
diff --git a/apps/files_sharing/tests/etagpropagation.php b/apps/files_sharing/tests/etagpropagation.php
index 60b7c525e35..710d7125340 100644
--- a/apps/files_sharing/tests/etagpropagation.php
+++ b/apps/files_sharing/tests/etagpropagation.php
@@ -69,9 +69,12 @@ class EtagPropagation extends TestCase {
 		$view1->mkdir('/directReshare');
 		$view1->mkdir('/sub1/sub2/folder/other');
 		$view1->mkdir('/sub1/sub2/folder/other');
+		$view1->file_put_contents('/foo.txt', 'foobar');
 		$view1->file_put_contents('/sub1/sub2/folder/file.txt', 'foobar');
 		$view1->file_put_contents('/sub1/sub2/folder/inside/file.txt', 'foobar');
 		$folderInfo = $view1->getFileInfo('/sub1/sub2/folder');
+		$fileInfo = $view1->getFileInfo('/foo.txt');
+		\OCP\Share::shareItem('file', $fileInfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31);
 		\OCP\Share::shareItem('folder', $folderInfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31);
 		\OCP\Share::shareItem('folder', $folderInfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER3, 31);
 		$folderInfo = $view1->getFileInfo('/directReshare');
@@ -179,6 +182,15 @@ class EtagPropagation extends TestCase {
 		$this->assertAllUnchaged();
 	}
 
+	public function testOwnerWritesToSingleFileShare() {
+		$this->loginAsUser(self::TEST_FILES_SHARING_API_USER1);
+		Filesystem::file_put_contents('/foo.txt', 'bar');
+		$this->assertEtagsNotChanged([self::TEST_FILES_SHARING_API_USER4, self::TEST_FILES_SHARING_API_USER3]);
+		$this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2]);
+
+		$this->assertAllUnchaged();
+	}
+
 	public function testOwnerWritesToShareWithReshare() {
 		$this->loginAsUser(self::TEST_FILES_SHARING_API_USER1);
 		Filesystem::file_put_contents('/sub1/sub2/folder/inside/bar.txt', 'bar');