From 066a040cc712f25cbe752cd5f7e2b1f73a065737 Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 2 Dec 2018 16:25:25 +0000 Subject: [PATCH] Avoid duplicated contacts, improve contact deletion, avoid memory issues --- config/dbstructure.config.php | 7 +- src/Core/Update.php | 2 + src/Database/DBA.php | 121 ++++++++++++++++------------------ src/Model/Contact.php | 17 +++-- 4 files changed, 74 insertions(+), 73 deletions(-) diff --git a/config/dbstructure.config.php b/config/dbstructure.config.php index 99e3de9d06..ca34936065 100644 --- a/config/dbstructure.config.php +++ b/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" => [ diff --git a/src/Core/Update.php b/src/Core/Update.php index 252ea8ef31..f4d8b8f9a7 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -118,6 +118,8 @@ class Update Lock::release('dbupdate'); } } + } elseif ($force) { + DBStructure::update($verbose, true); } return ''; diff --git a/src/Database/DBA.php b/src/Database/DBA.php index bf3004ead4..af0c25d0dd 100644 --- a/src/Database/DBA.php +++ b/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); - $commands = array_merge($commands, $retval); + $retval = self::delete($rel_table, [$rel_field => array_values($conditions)[0]], $options, $callstack); } } // 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 - $retval = self::delete($table, [$field => $row[$field]], $options, true, $callstack); - $commands = array_merge($commands, $retval); + self::delete($table, [$field => $row[$field]], $options, $callstack); } self::close($data); @@ -1123,74 +1118,70 @@ class DBA } } - if (!$in_process) { - // Now we finalize the process - $do_transaction = !self::$in_transaction; + // Now we finalize the process + $do_transaction = !self::$in_transaction; - if ($do_transaction) { - self::transaction(); + if ($do_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 = []; - $counter = []; + Logger::log(self::replaceParameters($sql, $field_values), Logger::DATA); - 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 (!self::e($sql, $field_values)) { 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; } - - return $commands; + if ($do_transaction) { + self::commit(); + } + return true; } /** diff --git a/src/Model/Contact.php b/src/Model/Contact.php index bb6fc25851..1765a17de9 100644 --- a/src/Model/Contact.php +++ b/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`", - Strings::normaliseLink($data["url"]), $contact_id]); + $condition = ["`nurl` = ? AND `uid` = ? AND `id` != ? AND NOT `self`", + Strings::normaliseLink($data["url"]), 0, $contact_id]; + Logger::log('Deleting duplicate contact ' . json_encode($condition), Logger::DEBUG); + DBA::delete('contact', $condition); } }