From 6ae5f705b64c0b0241238d75e6060798ac3a6a1b Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Tue, 3 Mar 2020 01:47:28 -0500 Subject: [PATCH 1/7] Rename Item::delete* methods to Item::markForDeletion* --- src/Model/Item.php | 31 +++++++++++++------------- src/Module/Admin/Item/Delete.php | 2 +- src/Protocol/ActivityPub/Processor.php | 4 ++-- src/Protocol/DFRN.php | 2 +- src/Protocol/Diaspora.php | 2 +- src/Protocol/OStatus.php | 2 +- src/Worker/RemoveUser.php | 2 +- 7 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index eac3b7028..de719a3c7 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1066,11 +1066,11 @@ class Item * @param integer $priority Priority for the notification * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function delete($condition, $priority = PRIORITY_HIGH) + public static function markForDeletion($condition, $priority = PRIORITY_HIGH) { $items = self::select(['id'], $condition); while ($item = self::fetch($items)) { - self::deleteById($item['id'], $priority); + self::markForDeletionById($item['id'], $priority); } DBA::close($items); } @@ -1097,7 +1097,7 @@ class Item // Delete notifications DBA::delete('notify', ['iid' => $item['id'], 'uid' => $uid]); } elseif ($item['uid'] == $uid) { - self::deleteById($item['id'], PRIORITY_HIGH); + self::markForDeletionById($item['id'], PRIORITY_HIGH); } else { Logger::log('Wrong ownership. Not deleting item ' . $item['id']); } @@ -1106,17 +1106,17 @@ class Item } /** - * Delete an item and notify others about it - if it was ours + * Mark an item for deletion, delete related data and notify others about it - if it was ours * - * @param integer $item_id Item ID that should be delete + * @param integer $item_id * @param integer $priority Priority for the notification * * @return boolean success * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function deleteById($item_id, $priority = PRIORITY_HIGH) + public static function markForDeletionById($item_id, $priority = PRIORITY_HIGH) { - Logger::notice('Delete item by id', ['id' => $item_id, 'callstack' => System::callstack()]); + Logger::notice('Mark item for deletion by id', ['id' => $item_id, 'callstack' => System::callstack()]); // locate item to be deleted $fields = ['id', 'uri', 'uid', 'parent', 'parent-uri', 'origin', 'deleted', 'file', 'resource-id', 'event-id', 'attach', @@ -1124,12 +1124,12 @@ class Item 'icid', 'iaid', 'psid']; $item = self::selectFirst($fields, ['id' => $item_id]); if (!DBA::isResult($item)) { - Logger::log('Item with ID ' . $item_id . " hasn't been found.", Logger::DEBUG); + Logger::debug('Item not found.', ['id' => $item_id]); return false; } if ($item['deleted']) { - Logger::log('Item with ID ' . $item_id . ' has already been deleted.', Logger::DEBUG); + Logger::debug('Item has already been marked for deletion.', ['id' => $item_id]); return false; } @@ -1199,7 +1199,7 @@ class Item self::deleteThread($item['id'], $item['parent-uri']); if (!self::exists(["`uri` = ? AND `uid` != 0 AND NOT `deleted`", $item['uri']])) { - self::delete(['uri' => $item['uri'], 'uid' => 0, 'deleted' => false], $priority); + self::markForDeletion(['uri' => $item['uri'], 'uid' => 0, 'deleted' => false], $priority); } ItemDeliveryData::delete($item['id']); @@ -1219,14 +1219,13 @@ class Item // If it's the parent of a comment thread, kill all the kids if ($item['id'] == $item['parent']) { - self::delete(['parent' => $item['parent'], 'deleted' => false], $priority); + self::markForDeletion(['parent' => $item['parent'], 'deleted' => false], $priority); } // Is it our comment and/or our thread? if ($item['origin'] || $parent['origin']) { - // When we delete the original post we will delete all existing copies on the server as well - self::delete(['uri' => $item['uri'], 'deleted' => false], $priority); + self::markForDeletion(['uri' => $item['uri'], 'deleted' => false], $priority); // send the notification upstream/downstream Worker::add(['priority' => $priority, 'dont_fork' => true], "Notifier", Delivery::DELETION, intval($item['id'])); @@ -1239,7 +1238,7 @@ class Item } } - Logger::log('Item with ID ' . $item_id . " has been deleted.", Logger::DEBUG); + Logger::debug('Item has been marked for deletion.', ['id' => $item_id]); return true; } @@ -3105,7 +3104,7 @@ class Item continue; } - self::deleteById($item['id'], PRIORITY_LOW); + self::markForDeletionById($item['id'], PRIORITY_LOW); ++$expired; } @@ -3246,7 +3245,7 @@ class Item // If it exists, mark it as deleted if (DBA::isResult($like_item)) { - self::deleteById($like_item['id']); + self::markForDeletionById($like_item['id']); if (!$event_verb_flag || $like_item['verb'] == $activity) { return true; diff --git a/src/Module/Admin/Item/Delete.php b/src/Module/Admin/Item/Delete.php index c98248e86..0ad20f97c 100644 --- a/src/Module/Admin/Item/Delete.php +++ b/src/Module/Admin/Item/Delete.php @@ -48,7 +48,7 @@ class Delete extends BaseAdmin } // Now that we have the GUID, drop those items, which will also delete the // associated threads. - Item::delete(['guid' => $guid]); + Item::markForDeletion(['guid' => $guid]); } info(DI::l10n()->t('Item marked for deletion.') . EOL); diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index 2e3264e09..5ff8881dd 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -220,7 +220,7 @@ class Processor $owner = Contact::getIdForURL($activity['actor']); Logger::log('Deleting item ' . $activity['object_id'] . ' from ' . $owner, Logger::DEBUG); - Item::delete(['uri' => $activity['object_id'], 'owner-id' => $owner]); + Item::markForDeletion(['uri' => $activity['object_id'], 'owner-id' => $owner]); } /** @@ -868,7 +868,7 @@ class Processor return; } - Item::delete(['uri' => $activity['object_id'], 'author-id' => $author_id, 'gravity' => GRAVITY_ACTIVITY]); + Item::markForDeletion(['uri' => $activity['object_id'], 'author-id' => $author_id, 'gravity' => GRAVITY_ACTIVITY]); } /** diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index 4c88db1d9..5c9d19ea9 100644 --- a/src/Protocol/DFRN.php +++ b/src/Protocol/DFRN.php @@ -2716,7 +2716,7 @@ class DFRN Logger::log('deleting item '.$item['id'].' uri='.$uri, Logger::DEBUG); - Item::delete(['id' => $item['id']]); + Item::markForDeletion(['id' => $item['id']]); } /** diff --git a/src/Protocol/Diaspora.php b/src/Protocol/Diaspora.php index cda428021..70965a72f 100644 --- a/src/Protocol/Diaspora.php +++ b/src/Protocol/Diaspora.php @@ -2794,7 +2794,7 @@ class Diaspora continue; } - Item::delete(['id' => $item['id']]); + Item::markForDeletion(['id' => $item['id']]); Logger::log("Deleted target ".$target_guid." (".$item["id"].") from user ".$item["uid"]." parent: ".$item["parent"], Logger::DEBUG); } diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index 96b8447b4..b707c62c5 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -585,7 +585,7 @@ class OStatus return; } - Item::delete($condition); + Item::markForDeletion($condition); Logger::log('Deleted item with uri '.$item['uri'].' for user '.$item['uid']); } diff --git a/src/Worker/RemoveUser.php b/src/Worker/RemoveUser.php index c2441adc9..018d17a46 100644 --- a/src/Worker/RemoveUser.php +++ b/src/Worker/RemoveUser.php @@ -41,7 +41,7 @@ class RemoveUser { do { $items = Item::select(['id'], $condition, ['limit' => 100]); while ($item = Item::fetch($items)) { - Item::deleteById($item['id'], PRIORITY_NEGLIGIBLE); + Item::markForDeletionById($item['id'], PRIORITY_NEGLIGIBLE); } DBA::close($items); } while (Item::exists($condition)); From 86a6268aac035cf0d789c718115dfc1999e608b3 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Tue, 3 Mar 2020 01:48:06 -0500 Subject: [PATCH 2/7] Add guid context to item deletion logging --- src/Model/Item.php | 4 ++-- src/Worker/DBClean.php | 12 ++++++------ src/Worker/Expire.php | 4 ++-- src/Worker/RemoveContact.php | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index de719a3c7..df5fc1976 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1938,7 +1938,7 @@ class Item if ($entries > 1) { // There are duplicates. We delete our just created entry. - Logger::notice('Delete duplicated item', ['id' => $current_post, 'uri' => $item['uri'], 'uid' => $item['uid']]); + Logger::notice('Delete duplicated item', ['id' => $current_post, 'uri' => $item['uri'], 'uid' => $item['uid'], 'guid' => $item['guid']]); // Yes, we could do a rollback here - but we are having many users with MyISAM. DBA::delete('item', ['id' => $current_post]); @@ -2721,7 +2721,7 @@ class Item if (!$mention) { if (($community_page || $prvgroup) && !$item['wall'] && !$item['origin'] && ($item['id'] == $item['parent'])) { - Logger::notice('Delete private group/communiy top-level item without mention', ['id' => $item_id]); + Logger::notice('Delete private group/communiy top-level item without mention', ['id' => $item_id, 'guid'=> $item['guid']]); DBA::delete('item', ['id' => $item_id]); return true; } diff --git a/src/Worker/DBClean.php b/src/Worker/DBClean.php index 7fbc65e29..19d95b675 100644 --- a/src/Worker/DBClean.php +++ b/src/Worker/DBClean.php @@ -98,7 +98,7 @@ class DBClean { $last_id = DI::config()->get('system', 'dbclean-last-id-1', 0); Logger::log("Deleting old global item entries from item table without user copy. Last ID: ".$last_id); - $r = DBA::p("SELECT `id` FROM `item` WHERE `uid` = 0 AND + $r = DBA::p("SELECT `id`, `guid` FROM `item` WHERE `uid` = 0 AND NOT EXISTS (SELECT `guid` FROM `item` AS `i` WHERE `item`.`guid` = `i`.`guid` AND `i`.`uid` != 0) AND `received` < UTC_TIMESTAMP() - INTERVAL ? DAY AND `id` >= ? ORDER BY `id` LIMIT ?", $days_unclaimed, $last_id, $limit); @@ -107,7 +107,7 @@ class DBClean { Logger::log("found global item orphans: ".$count); while ($orphan = DBA::fetch($r)) { $last_id = $orphan["id"]; - Logger::notice('Delete global orphan item', ['id' => $orphan["id"]]); + Logger::notice('Delete global orphan item', ['id' => $orphan['id'], 'guid' => $orphan['guid']]); DBA::delete('item', ['id' => $orphan["id"]]); } Worker::add(PRIORITY_MEDIUM, 'DBClean', 1, $last_id); @@ -122,7 +122,7 @@ class DBClean { $last_id = DI::config()->get('system', 'dbclean-last-id-2', 0); Logger::log("Deleting items without parents. Last ID: ".$last_id); - $r = DBA::p("SELECT `id` FROM `item` + $r = DBA::p("SELECT `id`, `guid` FROM `item` WHERE NOT EXISTS (SELECT `id` FROM `item` AS `i` WHERE `item`.`parent` = `i`.`id`) AND `id` >= ? ORDER BY `id` LIMIT ?", $last_id, $limit); $count = DBA::numRows($r); @@ -130,7 +130,7 @@ class DBClean { Logger::log("found item orphans without parents: ".$count); while ($orphan = DBA::fetch($r)) { $last_id = $orphan["id"]; - Logger::notice('Delete orphan item', ['id' => $orphan["id"]]); + Logger::notice('Delete orphan item', ['id' => $orphan['id'], 'guid' => $orphan['guid']]); DBA::delete('item', ['id' => $orphan["id"]]); } Worker::add(PRIORITY_MEDIUM, 'DBClean', 2, $last_id); @@ -319,7 +319,7 @@ class DBClean { $till_id = DI::config()->get('system', 'dbclean-last-id-8', 0); Logger::log("Deleting old global item entries from expired threads from ID ".$last_id." to ID ".$till_id); - $r = DBA::p("SELECT `id` FROM `item` WHERE `uid` = 0 AND + $r = DBA::p("SELECT `id`, `guid` FROM `item` WHERE `uid` = 0 AND NOT EXISTS (SELECT `guid` FROM `item` AS `i` WHERE `item`.`guid` = `i`.`guid` AND `i`.`uid` != 0) AND `received` < UTC_TIMESTAMP() - INTERVAL 90 DAY AND `id` >= ? AND `id` <= ? ORDER BY `id` LIMIT ?", $last_id, $till_id, $limit); @@ -328,7 +328,7 @@ class DBClean { Logger::log("found global item entries from expired threads: ".$count); while ($orphan = DBA::fetch($r)) { $last_id = $orphan["id"]; - Logger::notice('Delete expired thread item', ['id' => $orphan["id"]]); + Logger::notice('Delete expired thread item', ['id' => $orphan['id'], 'guid' => $orphan['guid']]); DBA::delete('item', ['id' => $orphan["id"]]); } Worker::add(PRIORITY_MEDIUM, 'DBClean', 9, $last_id); diff --git a/src/Worker/Expire.php b/src/Worker/Expire.php index dfbf9738e..bccb439e5 100644 --- a/src/Worker/Expire.php +++ b/src/Worker/Expire.php @@ -43,9 +43,9 @@ class Expire Logger::log('Delete expired items', Logger::DEBUG); // physically remove anything that has been deleted for more than two months $condition = ["`deleted` AND `changed` < UTC_TIMESTAMP() - INTERVAL 60 DAY"]; - $rows = DBA::select('item', ['id'], $condition); + $rows = DBA::select('item', ['id', 'guid'], $condition); while ($row = DBA::fetch($rows)) { - Logger::notice('Delete expired item', ['id' => $row["id"]]); + Logger::notice('Delete expired item', ['id' => $row['id'], 'guid' => $row['guid']]); DBA::delete('item', ['id' => $row['id']]); } DBA::close($rows); diff --git a/src/Worker/RemoveContact.php b/src/Worker/RemoveContact.php index 40e3a67fc..cfec2c184 100644 --- a/src/Worker/RemoveContact.php +++ b/src/Worker/RemoveContact.php @@ -41,9 +41,9 @@ class RemoveContact { // Now we delete the contact and all depending tables $condition = ['uid' => $contact['uid'], 'contact-id' => $id]; do { - $items = Item::select(['id'], $condition, ['limit' => 100]); + $items = Item::select(['id', 'guid'], $condition, ['limit' => 100]); while ($item = Item::fetch($items)) { - Logger::notice('Delete removed contact item', ['id' => $item["id"]]); + Logger::notice('Delete removed contact item', ['id' => $item['id'], 'guid' => $item['guid']]); DBA::delete('item', ['id' => $item['id']]); } DBA::close($items); From b4597a643d66c1035b15fa6339ef3cb6c2ac88c2 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Tue, 3 Mar 2020 01:48:21 -0500 Subject: [PATCH 3/7] Add callstack context to cascade deletion debug logging --- src/Database/Database.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index ff25e8b19..50b45d675 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1262,7 +1262,7 @@ class Database if ((count($command['conditions']) > 1) || is_int($first_key)) { $sql = "DELETE FROM " . DBA::quoteIdentifier($command['table']) . " " . $condition_string; - $this->logger->debug($this->replaceParameters($sql, $conditions)); + $this->logger->debug($this->replaceParameters($sql, $conditions), ['callstack' => System::callstack(6), 'internal_callstack' => $callstack]); if (!$this->e($sql, $conditions)) { if ($do_transaction) { @@ -1292,7 +1292,7 @@ class Database $sql = "DELETE FROM " . DBA::quoteIdentifier($table) . " WHERE " . DBA::quoteIdentifier($field) . " IN (" . substr(str_repeat("?, ", count($field_values)), 0, -2) . ");"; - $this->logger->debug($this->replaceParameters($sql, $field_values)); + $this->logger->debug($this->replaceParameters($sql, $field_values), ['callstack' => System::callstack(6), 'internal_callstack' => $callstack]); if (!$this->e($sql, $field_values)) { if ($do_transaction) { From f047944b2d28adb2239d68c81f1281d6ccffba8b Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 4 Mar 2020 15:59:19 -0500 Subject: [PATCH 4/7] Switch all item deletion logging to info level --- src/Database/Database.php | 4 ++-- src/Model/Item.php | 12 ++++++------ src/Worker/DBClean.php | 6 +++--- src/Worker/Expire.php | 2 +- src/Worker/RemoveContact.php | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 50b45d675..f86f27996 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1262,7 +1262,7 @@ class Database if ((count($command['conditions']) > 1) || is_int($first_key)) { $sql = "DELETE FROM " . DBA::quoteIdentifier($command['table']) . " " . $condition_string; - $this->logger->debug($this->replaceParameters($sql, $conditions), ['callstack' => System::callstack(6), 'internal_callstack' => $callstack]); + $this->logger->info($this->replaceParameters($sql, $conditions), ['callstack' => System::callstack(6), 'internal_callstack' => $callstack]); if (!$this->e($sql, $conditions)) { if ($do_transaction) { @@ -1292,7 +1292,7 @@ class Database $sql = "DELETE FROM " . DBA::quoteIdentifier($table) . " WHERE " . DBA::quoteIdentifier($field) . " IN (" . substr(str_repeat("?, ", count($field_values)), 0, -2) . ");"; - $this->logger->debug($this->replaceParameters($sql, $field_values), ['callstack' => System::callstack(6), 'internal_callstack' => $callstack]); + $this->logger->info($this->replaceParameters($sql, $field_values), ['callstack' => System::callstack(6), 'internal_callstack' => $callstack]); if (!$this->e($sql, $field_values)) { if ($do_transaction) { diff --git a/src/Model/Item.php b/src/Model/Item.php index df5fc1976..063a33874 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1116,7 +1116,7 @@ class Item */ public static function markForDeletionById($item_id, $priority = PRIORITY_HIGH) { - Logger::notice('Mark item for deletion by id', ['id' => $item_id, 'callstack' => System::callstack()]); + Logger::info('Mark item for deletion by id', ['id' => $item_id, 'callstack' => System::callstack()]); // locate item to be deleted $fields = ['id', 'uri', 'uid', 'parent', 'parent-uri', 'origin', 'deleted', 'file', 'resource-id', 'event-id', 'attach', @@ -1124,12 +1124,12 @@ class Item 'icid', 'iaid', 'psid']; $item = self::selectFirst($fields, ['id' => $item_id]); if (!DBA::isResult($item)) { - Logger::debug('Item not found.', ['id' => $item_id]); + Logger::info('Item not found.', ['id' => $item_id]); return false; } if ($item['deleted']) { - Logger::debug('Item has already been marked for deletion.', ['id' => $item_id]); + Logger::info('Item has already been marked for deletion.', ['id' => $item_id]); return false; } @@ -1238,7 +1238,7 @@ class Item } } - Logger::debug('Item has been marked for deletion.', ['id' => $item_id]); + Logger::info('Item has been marked for deletion.', ['id' => $item_id]); return true; } @@ -1938,7 +1938,7 @@ class Item if ($entries > 1) { // There are duplicates. We delete our just created entry. - Logger::notice('Delete duplicated item', ['id' => $current_post, 'uri' => $item['uri'], 'uid' => $item['uid'], 'guid' => $item['guid']]); + Logger::info('Delete duplicated item', ['id' => $current_post, 'uri' => $item['uri'], 'uid' => $item['uid'], 'guid' => $item['guid']]); // Yes, we could do a rollback here - but we are having many users with MyISAM. DBA::delete('item', ['id' => $current_post]); @@ -2721,7 +2721,7 @@ class Item if (!$mention) { if (($community_page || $prvgroup) && !$item['wall'] && !$item['origin'] && ($item['id'] == $item['parent'])) { - Logger::notice('Delete private group/communiy top-level item without mention', ['id' => $item_id, 'guid'=> $item['guid']]); + Logger::info('Delete private group/communiy top-level item without mention', ['id' => $item_id, 'guid'=> $item['guid']]); DBA::delete('item', ['id' => $item_id]); return true; } diff --git a/src/Worker/DBClean.php b/src/Worker/DBClean.php index 19d95b675..0316b9ebf 100644 --- a/src/Worker/DBClean.php +++ b/src/Worker/DBClean.php @@ -107,7 +107,7 @@ class DBClean { Logger::log("found global item orphans: ".$count); while ($orphan = DBA::fetch($r)) { $last_id = $orphan["id"]; - Logger::notice('Delete global orphan item', ['id' => $orphan['id'], 'guid' => $orphan['guid']]); + Logger::info('Delete global orphan item', ['id' => $orphan['id'], 'guid' => $orphan['guid']]); DBA::delete('item', ['id' => $orphan["id"]]); } Worker::add(PRIORITY_MEDIUM, 'DBClean', 1, $last_id); @@ -130,7 +130,7 @@ class DBClean { Logger::log("found item orphans without parents: ".$count); while ($orphan = DBA::fetch($r)) { $last_id = $orphan["id"]; - Logger::notice('Delete orphan item', ['id' => $orphan['id'], 'guid' => $orphan['guid']]); + Logger::info('Delete orphan item', ['id' => $orphan['id'], 'guid' => $orphan['guid']]); DBA::delete('item', ['id' => $orphan["id"]]); } Worker::add(PRIORITY_MEDIUM, 'DBClean', 2, $last_id); @@ -328,7 +328,7 @@ class DBClean { Logger::log("found global item entries from expired threads: ".$count); while ($orphan = DBA::fetch($r)) { $last_id = $orphan["id"]; - Logger::notice('Delete expired thread item', ['id' => $orphan['id'], 'guid' => $orphan['guid']]); + Logger::info('Delete expired thread item', ['id' => $orphan['id'], 'guid' => $orphan['guid']]); DBA::delete('item', ['id' => $orphan["id"]]); } Worker::add(PRIORITY_MEDIUM, 'DBClean', 9, $last_id); diff --git a/src/Worker/Expire.php b/src/Worker/Expire.php index bccb439e5..f98d56ed0 100644 --- a/src/Worker/Expire.php +++ b/src/Worker/Expire.php @@ -45,7 +45,7 @@ class Expire $condition = ["`deleted` AND `changed` < UTC_TIMESTAMP() - INTERVAL 60 DAY"]; $rows = DBA::select('item', ['id', 'guid'], $condition); while ($row = DBA::fetch($rows)) { - Logger::notice('Delete expired item', ['id' => $row['id'], 'guid' => $row['guid']]); + Logger::info('Delete expired item', ['id' => $row['id'], 'guid' => $row['guid']]); DBA::delete('item', ['id' => $row['id']]); } DBA::close($rows); diff --git a/src/Worker/RemoveContact.php b/src/Worker/RemoveContact.php index cfec2c184..28a32160a 100644 --- a/src/Worker/RemoveContact.php +++ b/src/Worker/RemoveContact.php @@ -43,7 +43,7 @@ class RemoveContact { do { $items = Item::select(['id', 'guid'], $condition, ['limit' => 100]); while ($item = Item::fetch($items)) { - Logger::notice('Delete removed contact item', ['id' => $item['id'], 'guid' => $item['guid']]); + Logger::info('Delete removed contact item', ['id' => $item['id'], 'guid' => $item['guid']]); DBA::delete('item', ['id' => $item['id']]); } DBA::close($items); From 825a7f9f93c063cc314d29b4bfacb42398cf9bf6 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 5 Mar 2020 07:16:47 +0000 Subject: [PATCH 5/7] Issue 8321: Follow items now work again --- mod/subthread.php | 150 +++----------------------------------- src/Model/Item.php | 4 + view/theme/frio/theme.php | 2 +- 3 files changed, 14 insertions(+), 142 deletions(-) diff --git a/mod/subthread.php b/mod/subthread.php index 96400b0dd..24b20d49d 100644 --- a/mod/subthread.php +++ b/mod/subthread.php @@ -20,156 +20,24 @@ */ use Friendica\App; -use Friendica\Core\Hook; +use Friendica\Network\HTTPException; use Friendica\Core\Logger; use Friendica\Core\Session; -use Friendica\Core\System; -use Friendica\Database\DBA; -use Friendica\DI; use Friendica\Model\Item; -use Friendica\Protocol\Activity; -use Friendica\Util\Security; use Friendica\Util\Strings; -use Friendica\Util\XML; - -function subthread_content(App $a) { +function subthread_content(App $a) +{ if (!Session::isAuthenticated()) { - return; + throw new HTTPException\ForbiddenException(); } - $activity = Activity::FOLLOW; - $item_id = (($a->argc > 1) ? Strings::escapeTags(trim($a->argv[1])) : 0); - $condition = ["`parent` = ? OR `parent-uri` = ? AND `parent` = `id`", $item_id, $item_id]; - $item = Item::selectFirst([], $condition); - - if (empty($item_id) || !DBA::isResult($item)) { - Logger::log('subthread: no item ' . $item_id); - return; + if (!Item::performLike($item_id, 'follow')) { + Logger::info('Following item failed', ['item' => $item_id]); + throw new HTTPException\BadRequestException(); } - - $owner_uid = $item['uid']; - - if (!Security::canWriteToUserWall($owner_uid)) { - return; - } - - $remote_owner = null; - - if (!$item['wall']) { - // The top level post may have been written by somebody on another system - $contact = DBA::selectFirst('contact', [], ['id' => $item['contact-id'], 'uid' => $item['uid']]); - if (!DBA::isResult($contact)) { - return; - } - if (!$contact['self']) { - $remote_owner = $contact; - } - } - - $owner = null; - // this represents the post owner on this system. - - $r = q("SELECT `contact`.*, `user`.`nickname` FROM `contact` LEFT JOIN `user` ON `contact`.`uid` = `user`.`uid` - WHERE `contact`.`self` = 1 AND `contact`.`uid` = %d LIMIT 1", - intval($owner_uid) - ); - - if (DBA::isResult($r)) { - $owner = $r[0]; - } - - if (!$owner) { - Logger::log('like: no owner'); - return; - } - - if (!$remote_owner) { - $remote_owner = $owner; - } - - $contact = null; - // This represents the person posting - - if (local_user() && (local_user() == $owner_uid)) { - $contact = $owner; - } else { - $contact = DBA::selectFirst('contact', [], ['id' => $_SESSION['visitor_id'], 'uid' => $owner_uid]); - if (!DBA::isResult($contact)) { - return; - } - } - - $uri = Item::newURI($owner_uid); - - $post_type = (($item['resource-id']) ? DI::l10n()->t('photo') : DI::l10n()->t('status')); - $objtype = (($item['resource-id']) ? Activity\ObjectType::IMAGE : Activity\ObjectType::NOTE ); - $link = XML::escape('' . "\n"); - $body = $item['body']; - - $obj = <<< EOT - - - $objtype - 1 - {$item['uri']} - $link - - $body - -EOT; - $bodyverb = DI::l10n()->t('%1$s is following %2$s\'s %3$s'); - - if (!isset($bodyverb)) { - return; - } - - $arr = []; - - $arr['guid'] = System::createUUID(); - $arr['uri'] = $uri; - $arr['uid'] = $owner_uid; - $arr['contact-id'] = $contact['id']; - $arr['wall'] = $item['wall']; - $arr['origin'] = 1; - $arr['gravity'] = GRAVITY_ACTIVITY; - $arr['parent'] = $item['id']; - $arr['parent-uri'] = $item['uri']; - $arr['thr-parent'] = $item['uri']; - $arr['owner-name'] = $remote_owner['name']; - $arr['owner-link'] = $remote_owner['url']; - $arr['owner-avatar'] = $remote_owner['thumb']; - $arr['author-name'] = $contact['name']; - $arr['author-link'] = $contact['url']; - $arr['author-avatar'] = $contact['thumb']; - - $ulink = '[url=' . $contact['url'] . ']' . $contact['name'] . '[/url]'; - $alink = '[url=' . $item['author-link'] . ']' . $item['author-name'] . '[/url]'; - $plink = '[url=' . DI::baseUrl() . '/display/' . $item['guid'] . ']' . $post_type . '[/url]'; - $arr['body'] = sprintf( $bodyverb, $ulink, $alink, $plink ); - - $arr['verb'] = $activity; - $arr['object-type'] = $objtype; - $arr['object'] = $obj; - $arr['allow_cid'] = $item['allow_cid']; - $arr['allow_gid'] = $item['allow_gid']; - $arr['deny_cid'] = $item['deny_cid']; - $arr['deny_gid'] = $item['deny_gid']; - $arr['visible'] = 1; - $arr['unseen'] = 1; - - $post_id = Item::insert($arr); - - if (!$item['visible']) { - Item::update(['visible' => true], ['id' => $item['id']]); - } - - $arr['id'] = $post_id; - - Hook::callAll('post_local_end', $arr); - - exit(); - + Logger::info('Followed item', ['item' => $item_id]); + return; } diff --git a/src/Model/Item.php b/src/Model/Item.php index eac3b7028..5a1e957db 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -3168,6 +3168,10 @@ class Item case 'unattendmaybe': $activity = Activity::ATTENDMAYBE; break; + case 'follow': + case 'unfollow': + $activity = Activity::FOLLOW; + break; default: Logger::log('like: unknown verb ' . $verb . ' for item ' . $item_id); return false; diff --git a/view/theme/frio/theme.php b/view/theme/frio/theme.php index bca18e78d..22afbc9a7 100644 --- a/view/theme/frio/theme.php +++ b/view/theme/frio/theme.php @@ -352,7 +352,7 @@ function frio_display_item(App $a, &$arr) $subthread = [ 'menu' => 'follow_thread', 'title' => DI::l10n()->t('Follow Thread'), - 'action' => 'dosubthread(' . $arr['item']['id'] . '); return false;', + 'action' => 'dosubthread(' . $arr['item']['id'] . ');', 'href' => '#' ]; } From a55057d97434c92cf12233869496e12b620ff3c9 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 5 Mar 2020 08:03:05 +0000 Subject: [PATCH 6/7] Function renamed to better reflect the purpose --- include/api.php | 2 +- mod/subthread.php | 2 +- src/Model/Item.php | 2 +- src/Module/Like.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/api.php b/include/api.php index 3a29184ec..ef7388912 100644 --- a/include/api.php +++ b/include/api.php @@ -5823,7 +5823,7 @@ function api_friendica_activity($type) $id = $_REQUEST['id'] ?? 0; - $res = Item::performLike($id, $verb); + $res = Item::performActivity($id, $verb); if ($res) { if ($type == "xml") { diff --git a/mod/subthread.php b/mod/subthread.php index 24b20d49d..ebec978c5 100644 --- a/mod/subthread.php +++ b/mod/subthread.php @@ -34,7 +34,7 @@ function subthread_content(App $a) $item_id = (($a->argc > 1) ? Strings::escapeTags(trim($a->argv[1])) : 0); - if (!Item::performLike($item_id, 'follow')) { + if (!Item::performActivity($item_id, 'follow')) { Logger::info('Following item failed', ['item' => $item_id]); throw new HTTPException\BadRequestException(); } diff --git a/src/Model/Item.php b/src/Model/Item.php index 5a1e957db..f3b27ac80 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -3141,7 +3141,7 @@ class Item * array $arr * 'post_id' => ID of posted item */ - public static function performLike($item_id, $verb) + public static function performActivity($item_id, $verb) { if (!Session::isAuthenticated()) { return false; diff --git a/src/Module/Like.php b/src/Module/Like.php index a85044714..c926012f1 100644 --- a/src/Module/Like.php +++ b/src/Module/Like.php @@ -50,7 +50,7 @@ class Like extends BaseModule // @TODO: Replace with parameter from router $itemId = (($app->argc > 1) ? Strings::escapeTags(trim($app->argv[1])) : 0); - if (!Item::performLike($itemId, $verb)) { + if (!Item::performActivity($itemId, $verb)) { throw new HTTPException\BadRequestException(); } From 9390748187465803a67e4e2fe80411bb43e725a2 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 5 Mar 2020 08:06:19 +0000 Subject: [PATCH 7/7] Store "sc:identifier" as GUID as well --- src/Protocol/ActivityPub/Processor.php | 2 +- src/Protocol/ActivityPub/Receiver.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index 4fa2d33f7..b902aa7ff 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -486,7 +486,7 @@ class Processor $item['created'] = DateTimeFormat::utc($activity['published']); $item['edited'] = DateTimeFormat::utc($activity['updated']); - $item['guid'] = $activity['diaspora:guid'] ?: self::getGUIDByURL($item['uri']); + $item['guid'] = $activity['diaspora:guid'] ?: $activity['sc:identifier'] ?: self::getGUIDByURL($item['uri']); $item = self::processContent($activity, $item); if (empty($item)) { diff --git a/src/Protocol/ActivityPub/Receiver.php b/src/Protocol/ActivityPub/Receiver.php index 21aba5094..daca87336 100644 --- a/src/Protocol/ActivityPub/Receiver.php +++ b/src/Protocol/ActivityPub/Receiver.php @@ -983,6 +983,7 @@ class Receiver $actor = JsonLD::fetchElement($object, 'as:actor', '@id'); } + $object_data['sc:identifier'] = JsonLD::fetchElement($object, 'sc:identifier', '@value'); $object_data['diaspora:guid'] = JsonLD::fetchElement($object, 'diaspora:guid', '@value'); $object_data['diaspora:comment'] = JsonLD::fetchElement($object, 'diaspora:comment', '@value'); $object_data['diaspora:like'] = JsonLD::fetchElement($object, 'diaspora:like', '@value');