From f7f96e971ac017b217252ee8c74f1b287ea4a79e Mon Sep 17 00:00:00 2001 From: Michael Date: Tue, 30 Oct 2018 17:34:18 +0000 Subject: [PATCH 1/3] Avoid duplicated delivery to Diaspora --- src/Worker/Notifier.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Worker/Notifier.php b/src/Worker/Notifier.php index 480938ec53..d4ec33b2ca 100644 --- a/src/Worker/Notifier.php +++ b/src/Worker/Notifier.php @@ -412,13 +412,17 @@ class Notifier if (!empty($networks)) { $condition['network'] = $networks; } - $contacts = DBA::select('contact', ['id', 'url', 'network'], $condition); + $contacts = DBA::select('contact', ['id', 'url', 'network', 'batch'], $condition); $r = DBA::toArray($contacts); } // delivery loop if (DBA::isResult($r)) { foreach ($r as $contact) { + if (($contact['network'] == Protocol::DIASPORA) && $diaspora_delivery && $public_message && !empty($contact['batch']) + && !in_array($cmd, [Delivery::MAIL, Delivery::SUGGESTION]) && !$followup) { + continue; + } logger("Deliver ".$item_id." to ".$contact['url']." via network ".$contact['network'], LOGGER_DEBUG); Worker::add(['priority' => $a->queue['priority'], 'created' => $a->queue['created'], 'dont_fork' => true], From ef205878b61c64e77c2654a312181bdb88fc0440 Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 4 Nov 2018 23:17:41 +0000 Subject: [PATCH 2/3] Cleaned up the notifier --- src/Worker/Notifier.php | 105 +++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 55 deletions(-) diff --git a/src/Worker/Notifier.php b/src/Worker/Notifier.php index 8a35a34dcb..ca79656d46 100644 --- a/src/Worker/Notifier.php +++ b/src/Worker/Notifier.php @@ -174,7 +174,6 @@ class Notifier $followup = false; $recipients_followup = []; - $conversants = []; if (!in_array($cmd, [Delivery::MAIL, Delivery::SUGGESTION, Delivery::RELOCATION])) { $parent = $items[0]; @@ -245,11 +244,10 @@ class Notifier // local followup to remote post $followup = true; $public_message = false; // not public - $conversant_str = DBA::escape($parent['contact-id']); $recipients = [$parent['contact-id']]; $recipients_followup = [$parent['contact-id']]; - Logger::log('notifier: followup '.$target_item["guid"].' to '.$conversant_str, Logger::DEBUG); + Logger::log('notifier: followup ' . $target_item['guid'] . ' to ' . $parent['contact-id'], Logger::DEBUG); //if (!$target_item['private'] && $target_item['wall'] && if (!$target_item['private'] && @@ -313,14 +311,12 @@ class Notifier foreach ($items as $item) { $recipients[] = $item['contact-id']; - $conversants[] = $item['contact-id']; // pull out additional tagged people to notify (if public message) if ($public_message && strlen($item['inform'])) { $people = explode(',',$item['inform']); foreach ($people as $person) { if (substr($person,0,4) === 'cid:') { $recipients[] = intval(substr($person,4)); - $conversants[] = intval(substr($person,4)); } else { $url_recipients[] = substr($person,4); } @@ -332,13 +328,9 @@ class Notifier Logger::log('notifier: '.$target_item["guid"].' url_recipients ' . print_r($url_recipients,true)); } - $conversants = array_unique($conversants); - - $recipients = array_unique(array_merge($recipients,$allow_people,$allow_groups)); - $deny = array_unique(array_merge($deny_people,$deny_groups)); - $recipients = array_diff($recipients,$deny); - - $conversant_str = DBA::escape(implode(', ',$conversants)); + $recipients = array_unique(array_merge($recipients, $allow_people, $allow_groups)); + $deny = array_unique(array_merge($deny_people, $deny_groups)); + $recipients = array_diff($recipients, $deny); } // If the thread parent is OStatus then do some magic to distribute the messages. @@ -403,7 +395,7 @@ class Notifier } if (($cmd == Delivery::RELOCATION)) { - $r = $recipients_relocate; + $contacts = $recipients_relocate; } else { if ($followup) { $recipients = $recipients_followup; @@ -413,41 +405,19 @@ class Notifier if (!empty($networks)) { $condition['network'] = $networks; } - $contacts = DBA::select('contact', ['id', 'url', 'network', 'batch'], $condition); - $r = DBA::toArray($contacts); + $result = DBA::select('contact', ['id', 'url', 'network', 'batch'], $condition); + $contacts = DBA::toArray($result); } - // delivery loop - if (DBA::isResult($r)) { - foreach ($r as $contact) { - if (($contact['network'] == Protocol::DIASPORA) && $diaspora_delivery && $public_message && !empty($contact['batch']) - && !in_array($cmd, [Delivery::MAIL, Delivery::SUGGESTION]) && !$followup) { - continue; - } - Logger::log("Deliver ".$item_id." to ".$contact['url']." via network ".$contact['network'], Logger::DEBUG); + $conversants = []; + $batch_delivery = false; - Worker::add(['priority' => $a->queue['priority'], 'created' => $a->queue['created'], 'dont_fork' => true], - 'Delivery', $cmd, $item_id, (int)$contact['id']); - } - } - - // send salmon slaps to mentioned remote tags (@foo@example.com) in OStatus posts - // They are especially used for notifications to OStatus users that don't follow us. - if (!Config::get('system', 'dfrn_only') && count($url_recipients) && ($public_message || $push_notify) && $normal_mode) { - $slap = OStatus::salmon($target_item, $owner); - foreach ($url_recipients as $url) { - if ($url) { - Logger::log('notifier: urldelivery: ' . $url); - $deliver_status = Salmon::slapper($owner, $url, $slap); - /// @TODO Redeliver/queue these items on failure, though there is no contact record - } - } - } - - if ($public_message) { + if ($public_message && !in_array($cmd, [Delivery::MAIL, Delivery::SUGGESTION]) && !$followup) { $r1 = []; if ($diaspora_delivery) { + $batch_delivery = true; + $r1 = q("SELECT `batch`, ANY_VALUE(`id`) AS `id`, ANY_VALUE(`name`) AS `name`, ANY_VALUE(`network`) AS `network` FROM `contact` WHERE `network` = '%s' AND `batch` != '' AND `uid` = %d AND `rel` != %d AND NOT `blocked` AND NOT `pending` AND NOT `archive` GROUP BY `batch`", @@ -477,25 +447,50 @@ class Notifier Logger::log('pubdeliver '.$target_item["guid"].': '.print_r($r,true), Logger::DEBUG); foreach ($r as $rr) { - // except for Diaspora batch jobs - // Don't deliver to folks who have already been delivered to - - if (($rr['network'] !== Protocol::DIASPORA) && (in_array($rr['id'], $conversants))) { - Logger::log('notifier: already delivered id=' . $rr['id']); - continue; - } - - if (!in_array($cmd, [Delivery::MAIL, Delivery::SUGGESTION]) && !$followup) { - Logger::log('notifier: delivery agent: '.$rr['name'].' '.$rr['id'].' '.$rr['network'].' '.$target_item["guid"]); - Worker::add(['priority' => $a->queue['priority'], 'created' => $a->queue['created'], 'dont_fork' => true], - 'Delivery', $cmd, $item_id, (int)$rr['id']); - } + $conversants[] = $rr['id']; + Logger::log('notifier: delivery agent: '.$rr['name'].' '.$rr['id'].' '.$rr['network'].' '.$target_item["guid"]); + Worker::add(['priority' => $a->queue['priority'], 'created' => $a->queue['created'], 'dont_fork' => true], + 'Delivery', $cmd, $item_id, (int)$rr['id']); } } $push_notify = true; } + // delivery loop + if (DBA::isResult($contacts)) { + foreach ($contacts as $contact) { + // Don't deliver to Diaspora if it already had been done as batch delivery + if (($contact['network'] == Protocol::DIASPORA) && $batch_delivery) { + continue; + } + + // Don't deliver to folks who have already been delivered to + if (in_array($contact['id'], $conversants)) { + Logger::log('notifier: already delivered id=' . $rr['id']); + continue; + } + + Logger::log("Deliver ".$item_id." to ".$contact['url']." via network ".$contact['network'], Logger::DEBUG); + + Worker::add(['priority' => $a->queue['priority'], 'created' => $a->queue['created'], 'dont_fork' => true], + 'Delivery', $cmd, $item_id, (int)$contact['id']); + } + } + + // send salmon slaps to mentioned remote tags (@foo@example.com) in OStatus posts + // They are especially used for notifications to OStatus users that don't follow us. + if (!Config::get('system', 'dfrn_only') && count($url_recipients) && ($public_message || $push_notify) && $normal_mode) { + $slap = OStatus::salmon($target_item, $owner); + foreach ($url_recipients as $url) { + if ($url) { + Logger::log('notifier: urldelivery: ' . $url); + $deliver_status = Salmon::slapper($owner, $url, $slap); + /// @TODO Redeliver/queue these items on failure, though there is no contact record + } + } + } + // Notify PuSH subscribers (Used for OStatus distribution of regular posts) if ($push_notify) { Logger::log('Activating internal PuSH for item '.$item_id, Logger::DEBUG); From 3b565e80b6a845a067f724e7b76c2de05838608b Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 5 Nov 2018 20:35:17 +0000 Subject: [PATCH 3/3] Improved logger text --- src/Worker/Notifier.php | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/Worker/Notifier.php b/src/Worker/Notifier.php index ca79656d46..e0108ddcfe 100644 --- a/src/Worker/Notifier.php +++ b/src/Worker/Notifier.php @@ -58,7 +58,7 @@ class Notifier { $a = BaseObject::getApp(); - Logger::log('notifier: invoked: '.$cmd.': '.$item_id, Logger::DEBUG); + Logger::log('Invoked: ' . $cmd . ': ' . $item_id, Logger::DEBUG); $top_level = false; $recipients = []; @@ -148,7 +148,7 @@ class Notifier } if ((count($items) == 1) && ($items[0]['id'] === $target_item['id']) && ($items[0]['uri'] === $items[0]['parent-uri'])) { - Logger::log('notifier: top level post'); + Logger::log('Top level post'); $top_level = true; } } @@ -184,7 +184,7 @@ class Notifier $condition = ['uri' => $target_item["thr-parent"], 'uid' => $target_item["uid"]]; $thr_parent = Item::selectFirst($fields, $condition); - Logger::log('GUID: '.$target_item["guid"].': Parent is '.$parent['network'].'. Thread parent is '.$thr_parent['network'], Logger::DEBUG); + Logger::log('GUID: ' . $target_item["guid"] . ': Parent is ' . $parent['network'] . '. Thread parent is ' . $thr_parent['network'], Logger::DEBUG); // This is IMPORTANT!!!! @@ -247,7 +247,7 @@ class Notifier $recipients = [$parent['contact-id']]; $recipients_followup = [$parent['contact-id']]; - Logger::log('notifier: followup ' . $target_item['guid'] . ' to ' . $parent['contact-id'], Logger::DEBUG); + Logger::log('Followup ' . $target_item['guid'] . ' to ' . $parent['contact-id'], Logger::DEBUG); //if (!$target_item['private'] && $target_item['wall'] && if (!$target_item['private'] && @@ -277,16 +277,16 @@ class Notifier $push_notify = false; } - Logger::log("Notify ".$target_item["guid"]." via PuSH: ".($push_notify?"Yes":"No"), Logger::DEBUG); + Logger::log('Notify ' . $target_item["guid"] .' via PuSH: ' . ($push_notify ? "Yes":"No"), Logger::DEBUG); } else { $followup = false; - Logger::log('Distributing directly '.$target_item["guid"], Logger::DEBUG); + Logger::log('Distributing directly ' . $target_item["guid"], Logger::DEBUG); // don't send deletions onward for other people's stuff if ($target_item['deleted'] && !intval($target_item['wall'])) { - Logger::log('notifier: ignoring delete notification for non-wall item'); + Logger::log('Ignoring delete notification for non-wall item'); return; } @@ -325,7 +325,7 @@ class Notifier } if (count($url_recipients)) { - Logger::log('notifier: '.$target_item["guid"].' url_recipients ' . print_r($url_recipients,true)); + Logger::log('Deliver ' . $target_item["guid"] . ' to _recipients ' . json_decode($url_recipients)); } $recipients = array_unique(array_merge($recipients, $allow_people, $allow_groups)); @@ -444,11 +444,9 @@ class Notifier $r = array_merge($r2, $r1); if (DBA::isResult($r)) { - Logger::log('pubdeliver '.$target_item["guid"].': '.print_r($r,true), Logger::DEBUG); - foreach ($r as $rr) { $conversants[] = $rr['id']; - Logger::log('notifier: delivery agent: '.$rr['name'].' '.$rr['id'].' '.$rr['network'].' '.$target_item["guid"]); + Logger::log('Public delivery of item ' . $target_item["guid"] . ' (' . $item_id . ') to ' . json_encode($rr), Logger::DEBUG); Worker::add(['priority' => $a->queue['priority'], 'created' => $a->queue['created'], 'dont_fork' => true], 'Delivery', $cmd, $item_id, (int)$rr['id']); } @@ -462,17 +460,17 @@ class Notifier foreach ($contacts as $contact) { // Don't deliver to Diaspora if it already had been done as batch delivery if (($contact['network'] == Protocol::DIASPORA) && $batch_delivery) { + Logger::log('Already delivered id ' . $item_id . ' via batch to ' . json_encode($contact), Logger::DEBUG); continue; } // Don't deliver to folks who have already been delivered to if (in_array($contact['id'], $conversants)) { - Logger::log('notifier: already delivered id=' . $rr['id']); + Logger::log('Already delivered id ' . $item_id. ' to ' . json_encode($contact), Logger::DEBUG); continue; } - Logger::log("Deliver ".$item_id." to ".$contact['url']." via network ".$contact['network'], Logger::DEBUG); - + Logger::log('Delivery of item ' . $item_id . ' to ' . json_encode($contact), Logger::DEBUG); Worker::add(['priority' => $a->queue['priority'], 'created' => $a->queue['created'], 'dont_fork' => true], 'Delivery', $cmd, $item_id, (int)$contact['id']); } @@ -484,7 +482,7 @@ class Notifier $slap = OStatus::salmon($target_item, $owner); foreach ($url_recipients as $url) { if ($url) { - Logger::log('notifier: urldelivery: ' . $url); + Logger::log('Salmon delivery of item ' . $item_id . ' to ' . $url); $deliver_status = Salmon::slapper($owner, $url, $slap); /// @TODO Redeliver/queue these items on failure, though there is no contact record } @@ -499,7 +497,7 @@ class Notifier PushSubscriber::publishFeed($owner['uid'], $a->queue['priority']); } - Logger::log('notifier: calling hooks for ' . $cmd . ' ' . $item_id, Logger::DEBUG); + Logger::log('Calling hooks for ' . $cmd . ' ' . $item_id, Logger::DEBUG); if ($normal_mode) { Hook::fork($a->queue['priority'], 'notifier_normal', $target_item);