From 220bc1f21878c550e0d1ab32265f604fc9542f87 Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <roeland@famdouma.nl>
Date: Mon, 9 Nov 2020 21:45:20 +0100
Subject: [PATCH] Make the expire shares cron job actually expire the shares

Right now we just delete the shares from the DB. Which is efficient
sure. But doesn't trigger any real cleanup. So no Admin audit entries or
any other post processing is done.

This makes sure we really trigger this.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
---
 apps/files_sharing/lib/ExpireSharesJob.php    | 44 ++++++++++++++-----
 .../tests/ExpireSharesJobTest.php             | 24 ++++------
 2 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/apps/files_sharing/lib/ExpireSharesJob.php b/apps/files_sharing/lib/ExpireSharesJob.php
index 5e48dd9ffd..eb13a13daa 100644
--- a/apps/files_sharing/lib/ExpireSharesJob.php
+++ b/apps/files_sharing/lib/ExpireSharesJob.php
@@ -24,7 +24,11 @@
 
 namespace OCA\Files_Sharing;
 
-use OC\BackgroundJob\TimedJob;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\BackgroundJob\TimedJob;
+use OCP\IDBConnection;
+use OCP\Share\Exceptions\ShareNotFound;
+use OCP\Share\IManager;
 use OCP\Share\IShare;
 
 /**
@@ -32,22 +36,29 @@ use OCP\Share\IShare;
  */
 class ExpireSharesJob extends TimedJob {
 
-	/**
-	 * sets the correct interval for this timed job
-	 */
-	public function __construct() {
+	/** @var IManager */
+	private $shareManager;
+
+	/** @var IDBConnection */
+	private $db;
+
+	public function __construct(ITimeFactory $time, IManager $shareManager, IDBConnection $db) {
+		$this->shareManager = $shareManager;
+		$this->db = $db;
+
+		parent::__construct($time);
+
 		// Run once a day
 		$this->setInterval(24 * 60 * 60);
 	}
 
+
 	/**
 	 * Makes the background job do its work
 	 *
 	 * @param array $argument unused argument
 	 */
 	public function run($argument) {
-		$connection = \OC::$server->getDatabaseConnection();
-
 		//Current time
 		$now = new \DateTime();
 		$now = $now->format('Y-m-d H:i:s');
@@ -55,8 +66,8 @@ class ExpireSharesJob extends TimedJob {
 		/*
 		 * Expire file link shares only (for now)
 		 */
-		$qb = $connection->getQueryBuilder();
-		$qb->select('id', 'file_source', 'uid_owner', 'item_type')
+		$qb = $this->db->getQueryBuilder();
+		$qb->select('id', 'share_type')
 			->from('share')
 			->where(
 				$qb->expr()->andX(
@@ -74,7 +85,20 @@ class ExpireSharesJob extends TimedJob {
 
 		$shares = $qb->execute();
 		while ($share = $shares->fetch()) {
-			\OC\Share\Share::unshare($share['item_type'], $share['file_source'], IShare::TYPE_LINK, null, $share['uid_owner']);
+			if ((int)$share['share_type'] === IShare::TYPE_LINK) {
+				$id = 'ocinternal';
+			} elseif ((int)$share['share_type'] === IShare::TYPE_EMAIL) {
+				$id = 'ocMailShare';
+			}
+
+			$id .= ':' . $share['id'];
+
+			try {
+				$share = $this->shareManager->getShareById($id);
+				$this->shareManager->deleteShare($share);
+			} catch (ShareNotFound $e) {
+				// Normally the share gets automatically expired on fetching it
+			}
 		}
 		$shares->closeCursor();
 	}
diff --git a/apps/files_sharing/tests/ExpireSharesJobTest.php b/apps/files_sharing/tests/ExpireSharesJobTest.php
index 2d853f15af..116cc2a27d 100644
--- a/apps/files_sharing/tests/ExpireSharesJobTest.php
+++ b/apps/files_sharing/tests/ExpireSharesJobTest.php
@@ -26,6 +26,8 @@
 namespace OCA\Files_Sharing\Tests;
 
 use OCA\Files_Sharing\ExpireSharesJob;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\Share\IManager;
 use OCP\Share\IShare;
 
 /**
@@ -37,24 +39,16 @@ use OCP\Share\IShare;
  */
 class ExpireSharesJobTest extends \Test\TestCase {
 
-	/**
-	 * @var ExpireSharesJob
-	 */
+	/** @var ExpireSharesJob */
 	private $job;
 
-	/**
-	 * @var \OCP\IDBConnection
-	 */
+	/** @var \OCP\IDBConnection */
 	private $connection;
 
-	/**
-	 * @var string
-	 */
+	/** @var string */
 	private $user1;
 
-	/**
-	 * @var string
-	 */
+	/** @var string */
 	private $user2;
 
 	protected function setUp(): void {
@@ -68,12 +62,12 @@ class ExpireSharesJobTest extends \Test\TestCase {
 		$this->user2 = $this->getUniqueID('user2_');
 
 		$userManager = \OC::$server->getUserManager();
-		$userManager->createUser($this->user1, 'pass');
-		$userManager->createUser($this->user2, 'pass');
+		$userManager->createUser($this->user1, 'longrandompassword');
+		$userManager->createUser($this->user2, 'longrandompassword');
 
 		\OC::registerShareHooks();
 
-		$this->job = new ExpireSharesJob();
+		$this->job = new ExpireSharesJob(\OC::$server->get(ITimeFactory::class), \OC::$server->get(IManager::class), $this->connection);
 	}
 
 	protected function tearDown(): void {