Merge pull request #6224 from annando/dba-delete-contact

Avoid duplicated contacts, improve contact deletion, avoid memory issues

config/dbstructure.config.php

@@ -34,7 +34,7 @@
 use Friendica\Database\DBA;
 
 if (!defined('DB_UPDATE_VERSION')) {
-	define('DB_UPDATE_VERSION', 1290);
+	define('DB_UPDATE_VERSION', 1291);
 }
 
 return [
@@ -643,6 +643,7 @@ return [
 			"uid_contactid_created" => ["uid", "contact-id", "created"],
 			"authorid_created" => ["author-id", "created"],
 			"ownerid" => ["owner-id"],
+			"contact-id" => ["contact-id"],
 			"uid_uri" => ["uid", "uri(190)"],
 			"resource-id" => ["resource-id"],
 			"deleted_changed" => ["deleted", "changed"],
@@ -894,7 +895,9 @@ return [
 			"fid" => ["type" => "int unsigned", "not null" => "1", "relation" => ["fcontact" => "id"], "comment" => ""],
 		],
 		"indexes" => [
-			"PRIMARY" => ["iid", "server"]
+			"PRIMARY" => ["iid", "server"],
+			"cid" => ["cid"],
+			"fid" => ["fid"]
 		]
 	],
 	"pconfig" => [

src/Core/Update.php

@@ -118,6 +118,8 @@ class Update
 					Lock::release('dbupdate');
 				}
 			}
+		} elseif ($force) {
+			DBStructure::update($verbose, true);
 		}
 
 		return '';

src/Database/DBA.php

@@ -1044,12 +1044,11 @@ class DBA
 	 * @param array   $options
 	 *                - cascade: If true we delete records in other tables that depend on the one we're deleting through
 	 *                           relations (default: true)
-	 * @param boolean $in_process  Internal use: Only do a commit after the last delete
 	 * @param array   $callstack   Internal use: prevent endless loops
 	 *
-	 * @return boolean|array was the delete successful? When $in_process is set: deletion data
+	 * @return boolean was the delete successful?
 	 */
-	public static function delete($table, array $conditions, array $options = [], $in_process = false, array &$callstack = [])
+	public static function delete($table, array $conditions, array $options = [], array &$callstack = [])
 	{
 		if (empty($table) || empty($conditions)) {
 			Logger::log('Table and conditions have to be set');
@@ -1098,22 +1097,18 @@ class DBA
 			if ((count($conditions) == 1) && ($field == array_keys($conditions)[0])) {
 				foreach ($rel_def AS $rel_table => $rel_fields) {
 					foreach ($rel_fields AS $rel_field) {
-						$retval = self::delete($rel_table, [$rel_field => array_values($conditions)[0]], $options, true, $callstack);
+						$retval = self::delete($rel_table, [$rel_field => array_values($conditions)[0]], $options, $callstack);
-						$commands = array_merge($commands, $retval);
 					}
 				}
 				// We quit when this key already exists in the callstack.
 			} elseif (!isset($callstack[$qkey])) {
-
 				$callstack[$qkey] = true;
 
 				// Fetch all rows that are to be deleted
 				$data = self::select($table, [$field], $conditions);
 
 				while ($row = self::fetch($data)) {
-					// Now we accumulate the delete commands
+					self::delete($table, [$field => $row[$field]], $options, $callstack);
-					$retval = self::delete($table, [$field => $row[$field]], $options, true, $callstack);
-					$commands = array_merge($commands, $retval);
 				}
 
 				self::close($data);
@@ -1123,74 +1118,70 @@ class DBA
 			}
 		}
 
-		if (!$in_process) {
+		// Now we finalize the process
-			// Now we finalize the process
+		$do_transaction = !self::$in_transaction;
-			$do_transaction = !self::$in_transaction;
 
-			if ($do_transaction) {
+		if ($do_transaction) {
-				self::transaction();
+			self::transaction();
+		}
+
+		$compacted = [];
+		$counter = [];
+
+		foreach ($commands AS $command) {
+			$conditions = $command['conditions'];
+			reset($conditions);
+			$first_key = key($conditions);
+
+			$condition_string = self::buildCondition($conditions);
+
+			if ((count($command['conditions']) > 1) || is_int($first_key)) {
+				$sql = "DELETE FROM `" . $command['table'] . "`" . $condition_string;
+				Logger::log(self::replaceParameters($sql, $conditions), Logger::DATA);
+
+				if (!self::e($sql, $conditions)) {
+					if ($do_transaction) {
+						self::rollback();
+					}
+					return false;
+				}
+			} else {
+				$key_table = $command['table'];
+				$key_condition = array_keys($command['conditions'])[0];
+				$value = array_values($command['conditions'])[0];
+
+				// Split the SQL queries in chunks of 100 values
+				// We do the $i stuff here to make the code better readable
+				$i = isset($counter[$key_table][$key_condition]) ? $counter[$key_table][$key_condition] : 0;
+				if (isset($compacted[$key_table][$key_condition][$i]) && count($compacted[$key_table][$key_condition][$i]) > 100) {
+					++$i;
+				}
+
+				$compacted[$key_table][$key_condition][$i][$value] = $value;
+				$counter[$key_table][$key_condition] = $i;
 			}
+		}
+		foreach ($compacted AS $table => $values) {
+			foreach ($values AS $field => $field_value_list) {
+				foreach ($field_value_list AS $field_values) {
+					$sql = "DELETE FROM `" . $table . "` WHERE `" . $field . "` IN (" .
+						substr(str_repeat("?, ", count($field_values)), 0, -2) . ");";
 
-			$compacted = [];
+					Logger::log(self::replaceParameters($sql, $field_values), Logger::DATA);
-			$counter = [];
 
-			foreach ($commands AS $command) {
+					if (!self::e($sql, $field_values)) {
-				$conditions = $command['conditions'];
-				reset($conditions);
-				$first_key = key($conditions);
-
-				$condition_string = self::buildCondition($conditions);
-
-				if ((count($command['conditions']) > 1) || is_int($first_key)) {
-					$sql = "DELETE FROM `" . $command['table'] . "`" . $condition_string;
-					Logger::log(self::replaceParameters($sql, $conditions), Logger::DATA);
-
-					if (!self::e($sql, $conditions)) {
 						if ($do_transaction) {
 							self::rollback();
 						}
 						return false;
 					}
-				} else {
-					$key_table = $command['table'];
-					$key_condition = array_keys($command['conditions'])[0];
-					$value = array_values($command['conditions'])[0];
-
-					// Split the SQL queries in chunks of 100 values
-					// We do the $i stuff here to make the code better readable
-					$i = isset($counter[$key_table][$key_condition]) ? $counter[$key_table][$key_condition] : 0;
-					if (isset($compacted[$key_table][$key_condition][$i]) && count($compacted[$key_table][$key_condition][$i]) > 100) {
-						++$i;
-					}
-
-					$compacted[$key_table][$key_condition][$i][$value] = $value;
-					$counter[$key_table][$key_condition] = $i;
 				}
 			}
-			foreach ($compacted AS $table => $values) {
-				foreach ($values AS $field => $field_value_list) {
-					foreach ($field_value_list AS $field_values) {
-						$sql = "DELETE FROM `" . $table . "` WHERE `" . $field . "` IN (" .
-							substr(str_repeat("?, ", count($field_values)), 0, -2) . ");";
-
-						Logger::log(self::replaceParameters($sql, $field_values), Logger::DATA);
-
-						if (!self::e($sql, $field_values)) {
-							if ($do_transaction) {
-								self::rollback();
-							}
-							return false;
-						}
-					}
-				}
-			}
-			if ($do_transaction) {
-				self::commit();
-			}
-			return true;
 		}
-
+		if ($do_transaction) {
-		return $commands;
+			self::commit();
+		}
+		return true;
 	}
 
 	/**

src/Model/Contact.php

@@ -1147,7 +1147,7 @@ class Contact extends BaseObject
 
 		$url = $data["url"];
 		if (!$contact_id) {
-			DBA::insert('contact', [
+			$fields = [
 				'uid'       => $uid,
 				'created'   => DateTimeFormat::utcNow(),
 				'url'       => $data["url"],
@@ -1176,10 +1176,13 @@ class Contact extends BaseObject
 				'writable'  => 1,
 				'blocked'   => 0,
 				'readonly'  => 0,
-				'pending'   => 0]
+				'pending'   => 0];
-			);
 
-			$s = DBA::select('contact', ['id'], ['nurl' => Strings::normaliseLink($data["url"]), 'uid' => $uid], ['order' => ['id'], 'limit' => 2]);
+			$condition = ['nurl' => Strings::normaliseLink($data["url"]), 'uid' => $uid, 'deleted' => false];
+
+			DBA::update('contact', $fields, $condition, true);
+
+			$s = DBA::select('contact', ['id'], $condition, ['order' => ['id'], 'limit' => 2]);
 			$contacts = DBA::toArray($s);
 			if (!DBA::isResult($contacts)) {
 				return 0;
@@ -1204,8 +1207,10 @@ class Contact extends BaseObject
 			}
 
 			if (count($contacts) > 1 && $uid == 0 && $contact_id != 0 && $data["url"] != "") {
-				DBA::delete('contact', ["`nurl` = ? AND `uid` = 0 AND `id` != ? AND NOT `self`",
+				$condition = ["`nurl` = ? AND `uid` = ? AND `id` != ? AND NOT `self`",
-					Strings::normaliseLink($data["url"]), $contact_id]);
+					Strings::normaliseLink($data["url"]), 0, $contact_id];
+				Logger::log('Deleting duplicate contact ' . json_encode($condition), Logger::DEBUG);
+				DBA::delete('contact', $condition);
 			}
 		}