Merge remote-tracking branch 'nickvergessen/ticket/11691' into develop

# By Joas Schilling
# Via Joas Schilling
* nickvergessen/ticket/11691:
  [ticket/11691] Fix conversion test
  [ticket/11691] Fix table names and arguments/docs
  [ticket/11691] Move purge code into new function
  [ticket/11691] Fix typo in comment
  [ticket/11691] Add order by to the query
  [ticket/11691] Fix some minor comments
  [ticket/11691] Fix some more problems with softdelete update
  [ticket/11691] Change detection of effectively_installed()
  [ticket/11691] Stagger user notification reconversion even more
  [ticket/11691] Stagger the convertion of soft delete updates

phpBB/phpbb/db/migration/data/v310/notification_options_reconvert.php

@@ -19,35 +19,44 @@ class notification_options_reconvert extends \phpbb\db\migration\migration
 	public function update_data()
 	{
 		return array(
+			array('custom', array(array($this, 'purge_notifications'))),
 			array('custom', array(array($this, 'convert_notifications'))),
 		);
 	}
 
-	public function convert_notifications()
+	public function purge_notifications()
 	{
-		$insert_table = $this->table_prefix . 'user_notifications';
+		$sql = 'DELETE FROM ' . $this->table_prefix . 'user_notifications';
-		$insert_buffer = new \phpbb\db\sql_insert_buffer($this->db, $insert_table);
+		$this->sql_query($sql);
+	}
 
-		$this->perform_conversion($insert_buffer, $insert_table);
+	public function convert_notifications($start)
+	{
+		$insert_buffer = new \phpbb\db\sql_insert_buffer($this->db, $this->table_prefix . 'user_notifications');
+
+		return $this->perform_conversion($insert_buffer, $start);
 	}
 
 	/**
 	* Perform the conversion (separate for testability)
 	*
-	* @param \phpbb\db\sql_insert_buffer $insert_buffer
+	* @param \phpbb\db\sql_insert_buffer		$insert_buffer
-	* @param string $insert_table
+	* @param int			$start		Start of staggering step
+	* @return		mixed		int start of the next step, null if the end was reached
 	*/
-	public function perform_conversion(\phpbb\db\sql_insert_buffer $insert_buffer, $insert_table)
+	public function perform_conversion(\phpbb\db\sql_insert_buffer $insert_buffer, $start)
 	{
-		$sql = 'DELETE FROM ' . $insert_table;
+		$limit = 250;
-		$this->db->sql_query($sql);
+		$converted_users = 0;
 
 		$sql = 'SELECT user_id, user_notify_type, user_notify_pm
-			FROM ' . USERS_TABLE;
+			FROM ' . $this->table_prefix . 'users
-		$result = $this->db->sql_query($sql);
+			ORDER BY user_id';
+		$result = $this->db->sql_query_limit($sql, $limit, $start);
 
 		while ($row = $this->db->sql_fetchrow($result))
 		{
+			$converted_users++;
 			$notification_methods = array();
 
 			// In-board notification
@@ -91,6 +100,14 @@ class notification_options_reconvert extends \phpbb\db\migration\migration
 		$this->db->sql_freeresult($result);
 
 		$insert_buffer->flush();
+
+		if ($converted_users < $limit)
+		{
+			// No more users left, we are done...
+			return;
+		}
+
+		return $start + $limit;
 	}
 
 	/**

phpBB/phpbb/db/migration/data/v310/softdelete_p1.php

@@ -101,7 +101,8 @@ class softdelete_p1 extends \phpbb\db\migration\migration
 		return array(
 			array('custom', array(array($this, 'update_post_visibility'))),
 			array('custom', array(array($this, 'update_topic_visibility'))),
-			array('custom', array(array($this, 'update_topic_forum_counts'))),
+			array('custom', array(array($this, 'update_topics_post_counts'))),
+			array('custom', array(array($this, 'update_forums_topic_and_post_counts'))),
 
 			array('permission.add', array('f_softdelete', false)),
 			array('permission.add', array('m_softdelete', false)),
@@ -122,28 +123,43 @@ class softdelete_p1 extends \phpbb\db\migration\migration
 		$this->sql_query($sql);
 	}
 
-	public function update_topic_forum_counts()
+	public function update_topics_post_counts()
 	{
+		/*
+		* Using sql_case here to avoid "BIGINT UNSIGNED value is out of range" errors.
+		* As we update all topics in 2 queries, one broken topic would stop the conversion
+		* for all topics and the surpressed error will cause the admin to not even notice it.
+		*/
 		$sql = 'UPDATE ' . $this->table_prefix . 'topics
 			SET topic_posts_approved = topic_replies + 1,
-				topic_posts_unapproved = topic_replies_real - topic_replies
+				topic_posts_unapproved = ' . $this->db->sql_case('topic_replies_real > topic_replies', 'topic_replies_real - topic_replies', '0') . '
 			WHERE topic_visibility = ' . ITEM_APPROVED;
 		$this->sql_query($sql);
 
 		$sql = 'UPDATE ' . $this->table_prefix . 'topics
 			SET topic_posts_approved = 0,
-				topic_posts_unapproved = (topic_replies_real - topic_replies) + 1
+				topic_posts_unapproved = (' . $this->db->sql_case('topic_replies_real > topic_replies', 'topic_replies_real - topic_replies', '0') . ') + 1
 			WHERE topic_visibility = ' . ITEM_UNAPPROVED;
 		$this->sql_query($sql);
+	}
+
+	public function update_forums_topic_and_post_counts($start)
+	{
+		$start = (int) $start;
+		$limit = 10;
+		$converted_forums = 0;
 
 		$sql = 'SELECT forum_id, topic_visibility, COUNT(topic_id) AS sum_topics, SUM(topic_posts_approved) AS sum_posts_approved, SUM(topic_posts_unapproved) AS sum_posts_unapproved
 			FROM ' . $this->table_prefix . 'topics
-			GROUP BY forum_id, topic_visibility';
+			GROUP BY forum_id, topic_visibility
-		$result = $this->db->sql_query($sql);
+			ORDER BY forum_id, topic_visibility';
+		$result = $this->db->sql_query_limit($sql, $limit, $start);
 
 		$update_forums = array();
 		while ($row = $this->db->sql_fetchrow($result))
 		{
+			$converted_forums++;
+
 			$forum_id = (int) $row['forum_id'];
 			if (!isset($update_forums[$forum_id]))
 			{
@@ -169,5 +185,14 @@ class softdelete_p1 extends \phpbb\db\migration\migration
 				WHERE forum_id = ' . $forum_id;
 			$this->sql_query($sql);
 		}
+
+		if ($converted_forums < $limit)
+		{
+			// There are no more topics, we are done
+			return;
+		}
+
+		// There are still more topics to query, return the next start value
+		return $start + $limit;
 	}
 }

phpBB/phpbb/db/migrator.php

@@ -208,7 +208,7 @@ class migrator
 
 		if (!isset($this->migration_state[$name]))
 		{
-			if ($migration->effectively_installed())
+			if ($state['migration_start_time'] == 0 && $migration->effectively_installed())
 			{
 				$state = array(
 					'migration_depends_on'	=> $migration->depends_on(),
@@ -227,6 +227,8 @@ class migrator
 			}
 		}
 
+		$this->set_migration_state($name, $state);
+
 		if (!$state['migration_schema_done'])
 		{
 			$this->apply_schema_changes($migration->update_schema());

tests/notification/convert_test.php

@@ -38,7 +38,7 @@ class phpbb_notification_convert_test extends phpbb_database_test_case
 	public function test_convert()
 	{
 		$buffer = new phpbb_mock_sql_insert_buffer($this->db, 'phpbb_user_notifications');
-		$this->migration->perform_conversion($buffer, 'phpbb_user_notifications');
+		$this->migration->perform_conversion($buffer, 0);
 
 		$expected = array_merge(
 			$this->create_expected('post', 1, 'email'),