The delivery counter now counts only successful deliveries
authorMichael <heluecht@pirati.ca>
Sat, 1 Jun 2019 06:54:47 +0000 (06:54 +0000)
committerMichael <heluecht@pirati.ca>
Sat, 1 Jun 2019 06:54:47 +0000 (06:54 +0000)
src/Protocol/DFRN.php
src/Protocol/Diaspora.php
src/Worker/Delivery.php

index 01159b1..fe2a608 100644 (file)
@@ -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, '<?xml') === false) {
                        Logger::log('dfrn_deliver: no valid XML returned');
                        Logger::log('dfrn_deliver: returned XML: ' . $xml, Logger::DATA);
-                       Contact::markForArchival($contact);
                        return 3;
                }
 
                $res = XML::parseString($xml);
 
                if (!is_object($res) || (intval($res->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, '<?xml') === false) {
                        Logger::log('dfrn_deliver: phase 2: no valid XML returned');
                        Logger::log('dfrn_deliver: phase 2: returned XML: ' . $xml, Logger::DATA);
-                       Contact::markForArchival($contact);
                        return 3;
                }
 
                $res = XML::parseString($xml);
 
                if (!isset($res->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, '<?xml') === false) {
                        Logger::log('No valid XML returned from ' . $contact['id'] . ' - ' . $dest_url);
                        Logger::log('Returned XML: ' . $xml, Logger::DATA);
-                       Contact::markForArchival($contact);
                        return 3;
                }
 
                $res = XML::parseString($xml);
 
                if (empty($res->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);
        }
 
index 818f078..e341de2 100644 (file)
@@ -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);
                }
        }
 
index 904e990..786b977 100644 (file)
@@ -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']);
+                       }
+               }
        }
 
        /**