[ticket/14168] Add attachment resync class

PHPBB3-14168

phpBB/config/default/container/services_files.yml

@@ -1,4 +1,19 @@
 services:
+    attachment.delete:
+        class: phpbb\attachment\delete
+        scope: prototype
+        arguments:
+            - @config
+            - @dbal.conn
+            - @dispatcher
+            - @attachment.resync
+
+    attachment.resync:
+        class: phpbb\attachment\resync
+        scope: prototype
+        arguments:
+            - @dbal.conn
+
     attachment.upload:
         class: phpbb\attachment\upload
         scope: prototype
@@ -14,14 +29,6 @@ services:
             - @user
             - %core.root_path%
 
-    attachment.delete:
-        class: phpbb\attachment\delete
-        scope: prototype
-        arguments:
-            - @config
-            - @dbal.conn
-            - @dispatcher
-
     filesystem:
         class: phpbb\filesystem\filesystem
 

phpBB/phpbb/attachment/delete.php

@@ -32,6 +32,9 @@ class delete
 	/** @var \phpbb\event\dispatcher */
 	protected $dispatcher;
 
+	/** @var \phpbb\attachment\resync */
+	protected $resync;
+
 	/** @var array Attachement IDs */
 	protected $ids;
 
@@ -61,13 +64,15 @@ class delete
 	 *
 	 * @param config $config
 	 * @param driver_interface $db
-	 * @param dispatcher>>>>>>> 85b6020... [ticket/14168] Move function for attachment deletion into class
+	 * @param dispatcher $dispatcher
+	 * @param resync $resync
 	 */
-	public function __construct(config $config, driver_interface $db, dispatcher $dispatcher)
+	public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, resync $resync)
 	{
 		$this->config = $config;
 		$this->db = $db;
 		$this->dispatcher = $dispatcher;
+		$this->resync = $resync;
 	}
 
 	/**
@@ -158,95 +163,14 @@ class delete
 		// No more use for the original ids
 		unset($ids);
 
-		// Now, we need to resync posts, messages, topics. We go through every one of them
 		// Update post indicators for posts now no longer having attachments
-		if (sizeof($this->post_ids))
+		$this->resync->resync('post', $this->post_ids);
-		{
-			// Just check which posts are still having an assigned attachment not orphaned by querying the attachments table
-			$sql = 'SELECT post_msg_id
-			FROM ' . ATTACHMENTS_TABLE . '
-			WHERE ' . $this->db->sql_in_set('post_msg_id', $this->post_ids) . '
-				AND in_message = 0
-				AND is_orphan = 0';
-			$result = $this->db->sql_query($sql);
-
-			$remaining_ids = array();
-			while ($row = $this->db->sql_fetchrow($result))
-			{
-				$remaining_ids[] = $row['post_msg_id'];
-			}
-			$this->db->sql_freeresult($result);
-
-			// Now only unset those ids remaining
-			$this->post_ids = array_diff($this->post_ids, $remaining_ids);
-
-			if (sizeof($this->post_ids))
-			{
-				$sql = 'UPDATE ' . POSTS_TABLE . '
-				SET post_attachment = 0
-				WHERE ' . $this->db->sql_in_set('post_id', $this->post_ids);
-				$this->db->sql_query($sql);
-			}
-		}
 
 		// Update message table if messages are affected
-		if (sizeof($this->message_ids))
+		$this->resync->resync('message', $this->message_ids);
-		{
-			// Just check which messages are still having an assigned attachment not orphaned by querying the attachments table
-			$sql = 'SELECT post_msg_id
-			FROM ' . ATTACHMENTS_TABLE . '
-			WHERE ' . $this->db->sql_in_set('post_msg_id', $this->message_ids) . '
-				AND in_message = 1
-				AND is_orphan = 0';
-			$result = $this->db->sql_query($sql);
-
-			$remaining_ids = array();
-			while ($row = $this->db->sql_fetchrow($result))
-			{
-				$remaining_ids[] = $row['post_msg_id'];
-			}
-			$this->db->sql_freeresult($result);
-
-			// Now only unset those ids remaining
-			$this->message_ids = array_diff($this->message_ids, $remaining_ids);
-
-			if (sizeof($this->message_ids))
-			{
-				$sql = 'UPDATE ' . PRIVMSGS_TABLE . '
-				SET message_attachment = 0
-				WHERE ' . $this->db->sql_in_set('msg_id', $this->message_ids);
-				$this->db->sql_query($sql);
-			}
-		}
 
 		// Now update the topics. This is a bit trickier, because there could be posts still having attachments within the topic
-		if (sizeof($this->topic_ids))
+		$this->resync->resync('topic', $this->topic_ids);
-		{
-			// Just check which topics are still having an assigned attachment not orphaned by querying the attachments table (much less entries expected)
-			$sql = 'SELECT topic_id
-			FROM ' . ATTACHMENTS_TABLE . '
-			WHERE ' . $this->db->sql_in_set('topic_id', $this->topic_ids) . '
-				AND is_orphan = 0';
-			$result = $this->db->sql_query($sql);
-
-			$remaining_ids = array();
-			while ($row = $this->db->sql_fetchrow($result))
-			{
-				$remaining_ids[] = $row['topic_id'];
-			}
-			$this->db->sql_freeresult($result);
-
-			// Now only unset those ids remaining
-			$this->topic_ids = array_diff($this->topic_ids, $remaining_ids);
-
-			if (sizeof($this->topic_ids))
-			{
-				$sql = 'UPDATE ' . TOPICS_TABLE . '
-				SET topic_attachment = 0
-				WHERE ' . $this->db->sql_in_set('topic_id', $this->topic_ids);
-				$this->db->sql_query($sql);
-			}
-		}
 
 		return $this->num_deleted;
 	}
@@ -384,7 +308,7 @@ class delete
 
 		// Delete attachments
 		$sql = 'DELETE FROM ' . ATTACHMENTS_TABLE . '
-		WHERE ' . $this->db->sql_in_set($this->sql_id, $this->ids);
+			WHERE ' . $this->db->sql_in_set($this->sql_id, $this->ids);
 
 		$sql .= $this->sql_where;
 

phpBB/phpbb/attachment/resync.php

@@ -0,0 +1,123 @@
+<?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ * For full copyright and license information, please see
+ * the docs/CREDITS.txt file.
+ *
+ */
+
+namespace phpbb\attachment;
+
+use \phpbb\db\driver\driver_interface;
+
+/**
+ * Attachment delete class
+ */
+
+class resync
+{
+	/** @var driver_interface */
+	protected $db;
+
+	/** @var string Attachment table SQL ID */
+	private $attach_sql_id;
+
+	/** @var string Resync table SQL ID  */
+	private $resync_sql_id;
+
+	/** @var string Resync SQL table */
+	private $resync_table;
+
+	/** @var string SQL where statement */
+	private $sql_where;
+
+	/**
+	 * Constructor for attachment resync class
+	 *
+	 * @param driver_interface $db Database driver
+	 */
+	public function __construct(driver_interface $db)
+	{
+		$this->db = $db;
+	}
+
+	/**
+	 * Set type constraints for attachment resync
+	 *
+	 * @param string $type Type of resync; can be: message|post|topic
+	 */
+	protected function set_type_constraints($type)
+	{
+		switch ($type)
+		{
+			case 'message':
+				$this->attach_sql_id = 'post_msg_id';
+				$this->sql_where = ' AND in_message = 1
+					AND is_orphan = 0';
+				$this->resync_table = PRIVMSGS_TABLE;
+				$this->resync_sql_id = 'msg_id';
+			break;
+
+			case 'post':
+				$this->attach_sql_id = 'post_msg_id';
+				$this->sql_where = ' AND in_message = 0
+					AND is_orphan = 0';
+				$this->resync_table = POSTS_TABLE;
+				$this->resync_sql_id = 'post_id';
+			break;
+
+			case 'topic':
+				$this->attach_sql_id = 'topic_id';
+				$this->sql_where = ' AND is_orphan = 0';
+				$this->resync_table = TOPICS_TABLE;
+				$this->resync_sql_id = 'topic_id';
+			break;
+		}
+	}
+
+	/**
+	 * Resync specified type
+	 *
+	 * @param string $type Type of resync
+	 * @param array $ids IDs to resync
+	 */
+	public function resync($type, $ids)
+	{
+		if (empty($type) || !is_array($ids) || !sizeof($ids) || !in_array($type, array('post', 'topic', 'message')))
+		{
+			return;
+		}
+
+		// Just check which elements are still having an assigned attachment
+		// not orphaned by querying the attachments table
+		$sql = 'SELECT ' . $this->attach_sql_id . '
+			FROM ' . ATTACHMENTS_TABLE . '
+			WHERE ' . $this->db->sql_in_set($this->attach_sql_id, $ids)
+				. $this->sql_where;
+		$result = $this->db->sql_query($sql);
+
+		$remaining_ids = array();
+		while ($row = $this->db->sql_fetchrow($result))
+		{
+			$remaining_ids[] = $row[$this->attach_sql_id];
+		}
+		$this->db->sql_freeresult($result);
+
+		// Now only unset those ids remaining
+		$ids = array_diff($ids, $remaining_ids);
+
+		if (sizeof($ids))
+		{
+			$sql = 'UPDATE ' . POSTS_TABLE . '
+				SET ' . $type . '_attachment = 0
+				WHERE ' . $this->db->sql_in_set($this->resync_sql_id, $ids);
+			$this->db->sql_query($sql);
+		}
+	}
+
+}

tests/content_visibility/delete_post_test.php

@@ -312,7 +312,7 @@ class phpbb_content_visibility_delete_post_test extends phpbb_database_test_case
 		$lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx);
 		$lang = new \phpbb\language\language($lang_loader);
 		$user = new \phpbb\user($lang, '\phpbb\datetime');
-		$attachment_delete = new \phpbb\attachment\delete($config, $db);
+		$attachment_delete = new \phpbb\attachment\delete($config, $db, new \phpbb\attachment\resync($db));
 
 		$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
 

tests/functions_user/delete_user_test.php

@@ -36,7 +36,7 @@ class phpbb_functions_user_delete_user_test extends phpbb_database_test_case
 		$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
 		$phpbb_container = new phpbb_mock_container_builder();
 		$phpbb_container->set('notification_manager', new phpbb_mock_notification_manager());
-		$phpbb_container->set('attachment.delete', new \phpbb\attachment\delete($config, $db));
+		$phpbb_container->set('attachment.delete', new \phpbb\attachment\delete($config, $db, new \phpbb\attachment\resync($db)));
 		$phpbb_container->set(
 			'auth.provider.db',
 			new phpbb_mock_auth_provider()

tests/privmsgs/delete_user_pms_test.php

@@ -91,7 +91,7 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case
 
 		$phpbb_container = new phpbb_mock_container_builder();
 		$phpbb_container->set('notification_manager', new phpbb_mock_notification_manager());
-		$phpbb_container->set('attachment.delete', new \phpbb\attachment\delete(new \phpbb\config\config(array()), $db));
+		$phpbb_container->set('attachment.delete', new \phpbb\attachment\delete(new \phpbb\config\config(array()), $db, new \phpbb\attachment\resync($db)));
 
 		phpbb_delete_user_pms($delete_user);