From ecf809c7ae508b71bc0fcdf5956660eb129fa141 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 1 Jun 2019 06:54:47 +0000 Subject: [PATCH] The delivery counter now counts only successful deliveries --- src/Protocol/DFRN.php | 26 ----------------- src/Protocol/Diaspora.php | 25 ++++------------- src/Worker/Delivery.php | 59 ++++++++++++++++++++++----------------- 3 files changed, 38 insertions(+), 72 deletions(-) diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index 01159b1bae..fe2a608ec6 100644 --- a/src/Protocol/DFRN.php +++ b/src/Protocol/DFRN.php @@ -1229,7 +1229,6 @@ class DFRN $curlResult = Network::curl($url); if ($curlResult->isTimeout()) { - Contact::markForArchival($contact); return -2; // timed out } @@ -1237,29 +1236,24 @@ class DFRN $curl_stat = $curlResult->getReturnCode(); if (empty($curl_stat)) { - Contact::markForArchival($contact); return -3; // timed out } Logger::log('dfrn_deliver: ' . $xml, Logger::DATA); if (empty($xml)) { - Contact::markForArchival($contact); return 3; } if (strpos($xml, 'status) != 0) || !strlen($res->challenge) || !strlen($res->dfrn_id)) { - Contact::markForArchival($contact); - if (empty($res->status)) { $status = 3; } else { @@ -1315,7 +1309,6 @@ class DFRN if ($final_dfrn_id != $orig_id) { Logger::log('dfrn_deliver: wrong dfrn_id.'); // did not decode properly - cannot trust this site - Contact::markForArchival($contact); return 3; } @@ -1351,7 +1344,6 @@ class DFRN default: Logger::log("rino: invalid requested version '$rino_remote_version'"); - Contact::markForArchival($contact); return -8; } @@ -1391,26 +1383,22 @@ class DFRN $curl_stat = $postResult->getReturnCode(); if (empty($curl_stat) || empty($xml)) { - Contact::markForArchival($contact); return -9; // timed out } if (($curl_stat == 503) && stristr($postResult->getHeader(), 'retry-after')) { - Contact::markForArchival($contact); return -10; } if (strpos($xml, 'status)) { - Contact::markForArchival($contact); return -11; } @@ -1423,10 +1411,6 @@ class DFRN Logger::log('Delivery returned status '.$res->status.' - '.$res->message, Logger::DEBUG); } - if (($res->status >= 200) && ($res->status <= 299)) { - Contact::unmarkForArchival($contact); - } - return intval($res->status); } @@ -1454,7 +1438,6 @@ class DFRN if (empty($contact['addr'])) { Logger::log('Unable to find contact handle for ' . $contact['id'] . ' - ' . $contact['url']); - Contact::markForArchival($contact); return -21; } } @@ -1462,7 +1445,6 @@ class DFRN $fcontact = Diaspora::personByHandle($contact['addr']); if (empty($fcontact)) { Logger::log('Unable to find contact details for ' . $contact['id'] . ' - ' . $contact['addr']); - Contact::markForArchival($contact); return -22; } $pubkey = $fcontact['pubkey']; @@ -1491,26 +1473,22 @@ class DFRN $curl_stat = $postResult->getReturnCode(); if (empty($curl_stat) || empty($xml)) { Logger::log('Empty answer from ' . $contact['id'] . ' - ' . $dest_url); - Contact::markForArchival($contact); return -9; // timed out } if (($curl_stat == 503) && (stristr($postResult->getHeader(), 'retry-after'))) { - Contact::markForArchival($contact); return -10; } if (strpos($xml, 'status)) { - Contact::markForArchival($contact); return -23; } @@ -1518,10 +1496,6 @@ class DFRN Logger::log('Transmit to ' . $dest_url . ' returned status '.$res->status.' - '.$res->message, Logger::DEBUG); } - if (($res->status >= 200) && ($res->status <= 299)) { - Contact::unmarkForArchival($contact); - } - return intval($res->status); } diff --git a/src/Protocol/Diaspora.php b/src/Protocol/Diaspora.php index 818f078bb1..e341de2080 100644 --- a/src/Protocol/Diaspora.php +++ b/src/Protocol/Diaspora.php @@ -3110,13 +3110,12 @@ class Diaspora * @param string $envelope The message that is to be transmitted * @param bool $public_batch Is it a public post? * @param string $guid message guid - * @param bool $no_defer Don't defer a failing delivery * * @return int Result of the transmission * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function transmit(array $owner, array $contact, $envelope, $public_batch, $guid = "", $no_defer = false) + private static function transmit(array $owner, array $contact, $envelope, $public_batch, $guid = "") { $enabled = intval(Config::get("system", "diaspora_enabled")); if (!$enabled) { @@ -3155,20 +3154,6 @@ class Diaspora Logger::log("transmit: ".$logid."-".$guid." to ".$dest_url." returns: ".$return_code); - if (!$return_code || (($return_code == 503) && (stristr($postResult->getHeader(), "retry-after")))) { - if (!$no_defer && !empty($contact['contact-type']) && ($contact['contact-type'] != Contact::TYPE_RELAY)) { - Logger::info('defer message', ['log' => $logid, 'guid' => $guid, 'destination' => $dest_url]); - // defer message for redelivery - Worker::defer(); - } - - // The message could not be delivered. We mark the contact as "dead" - Contact::markForArchival($contact); - } elseif (($return_code >= 200) && ($return_code <= 299)) { - // We successfully delivered a message, the contact is alive - Contact::unmarkForArchival($contact); - } - return $return_code ? $return_code : -1; } @@ -3197,13 +3182,12 @@ class Diaspora * @param array $message The message data * @param bool $public_batch Is it a public post? * @param string $guid message guid - * @param bool $no_defer Don't defer a failing delivery * * @return int Result of the transmission * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - private static function buildAndTransmit(array $owner, array $contact, $type, $message, $public_batch = false, $guid = "", $no_defer = false) + private static function buildAndTransmit(array $owner, array $contact, $type, $message, $public_batch = false, $guid = "") { $msg = self::buildPostXml($type, $message); @@ -3217,7 +3201,7 @@ class Diaspora $envelope = self::buildMessage($msg, $owner, $contact, $owner['uprvkey'], $contact['pubkey'], $public_batch); - $return_code = self::transmit($owner, $contact, $envelope, $public_batch, $guid, $no_defer); + $return_code = self::transmit($owner, $contact, $envelope, $public_batch, $guid); Logger::log("guid: ".$guid." result ".$return_code, Logger::DEBUG); @@ -4215,9 +4199,10 @@ class Diaspora $message = self::createProfileData($uid); + // @ToDo Split this into single worker jobs foreach ($recips as $recip) { Logger::log("Send updated profile data for user ".$uid." to contact ".$recip["id"], Logger::DEBUG); - self::buildAndTransmit($owner, $recip, "profile", $message, false, '', true); + self::buildAndTransmit($owner, $recip, "profile", $message); } } diff --git a/src/Worker/Delivery.php b/src/Worker/Delivery.php index 904e9904ef..786b9774da 100644 --- a/src/Worker/Delivery.php +++ b/src/Worker/Delivery.php @@ -178,18 +178,10 @@ class Delivery extends BaseObject case Protocol::DFRN: self::deliverDFRN($cmd, $contact, $owner, $items, $target_item, $public_message, $top_level, $followup); - - if (in_array($cmd, [Delivery::POST, Delivery::COMMENT])) { - Model\ItemDeliveryData::incrementQueueDone($target_id); - } break; case Protocol::DIASPORA: self::deliverDiaspora($cmd, $contact, $owner, $items, $target_item, $public_message, $top_level, $followup); - - if (in_array($cmd, [Delivery::POST, Delivery::COMMENT])) { - Model\ItemDeliveryData::incrementQueueDone($target_id); - } break; case Protocol::OSTATUS: @@ -321,21 +313,19 @@ class Delivery extends BaseObject Logger::info('DFRN Delivery', ['cmd' => $cmd, 'url' => $contact['url'], 'guid' => defaults($target_item, 'guid', $target_item['id']), 'return' => $deliver_status]); - if ($deliver_status < 0) { - Logger::info('Delivery failed: defer message', ['id' => defaults($target_item, 'guid', $target_item['id'])]); - Worker::defer(); - } - if (($deliver_status >= 200) && ($deliver_status <= 299)) { // We successfully delivered a message, the contact is alive Model\Contact::unmarkForArchival($contact); + + if (in_array($cmd, [Delivery::POST, Delivery::COMMENT])) { + Model\ItemDeliveryData::incrementQueueDone($target_item['id']); + } } else { // The message could not be delivered. We mark the contact as "dead" Model\Contact::markForArchival($contact); - // Transmit via Diaspora when all other methods (legacy DFRN and new one) are failing. - // This is a fallback for systems that don't know the new methods. - self::deliverDiaspora($cmd, $contact, $owner, $items, $target_item, $public_message, $top_level, $followup); + Logger::info('Delivery failed: defer message', ['id' => defaults($target_item, 'guid', $target_item['id'])]); + Worker::defer(); } } @@ -380,32 +370,49 @@ class Delivery extends BaseObject if (!$contact['pubkey'] && !$public_message) { return; } + if ($cmd == self::RELOCATION) { - Diaspora::sendAccountMigration($owner, $contact, $owner['uid']); - return; + $deliver_status = Diaspora::sendAccountMigration($owner, $contact, $owner['uid']); } elseif ($target_item['deleted'] && (($target_item['uri'] === $target_item['parent-uri']) || $followup)) { // top-level retraction Logger::log('diaspora retract: ' . $loc); - Diaspora::sendRetraction($target_item, $owner, $contact, $public_message); - return; + $deliver_status = Diaspora::sendRetraction($target_item, $owner, $contact, $public_message); } elseif ($followup) { // send comments and likes to owner to relay Logger::log('diaspora followup: ' . $loc); - Diaspora::sendFollowup($target_item, $owner, $contact, $public_message); - return; + $deliver_status = Diaspora::sendFollowup($target_item, $owner, $contact, $public_message); } elseif ($target_item['uri'] !== $target_item['parent-uri']) { // we are the relay - send comments, likes and relayable_retractions to our conversants Logger::log('diaspora relay: ' . $loc); - Diaspora::sendRelay($target_item, $owner, $contact, $public_message); - return; + $deliver_status = Diaspora::sendRelay($target_item, $owner, $contact, $public_message); } elseif ($top_level && !$walltowall) { // currently no workable solution for sending walltowall Logger::log('diaspora status: ' . $loc); - Diaspora::sendStatus($target_item, $owner, $contact, $public_message); + $deliver_status = Diaspora::sendStatus($target_item, $owner, $contact, $public_message); + } else { + Logger::log('Unknown mode ' . $cmd . ' for ' . $loc); return; } - Logger::log('Unknown mode ' . $cmd . ' for ' . $loc); + if (($deliver_status >= 200) && ($deliver_status <= 299)) { + // We successfully delivered a message, the contact is alive + Model\Contact::unmarkForArchival($contact); + + if (in_array($cmd, [Delivery::POST, Delivery::COMMENT])) { + Model\ItemDeliveryData::incrementQueueDone($target_item['id']); + } + } else { + // The message could not be delivered. We mark the contact as "dead" + Model\Contact::markForArchival($contact); + + if (!empty($contact['contact-type']) && ($contact['contact-type'] != Model\Contact::TYPE_RELAY)) { + Logger::info('Delivery failed: defer message', ['id' => defaults($target_item, 'guid', $target_item['id'])]); + // defer message for redelivery + Worker::defer(); + } elseif (in_array($cmd, [Delivery::POST, Delivery::COMMENT])) { + Model\ItemDeliveryData::incrementQueueDone($target_item['id']); + } + } } /**