From 751b6e56889ea8306b3df5d2f1bb1a97ffde0e3e Mon Sep 17 00:00:00 2001
From: Michael Kirk <michael@signal.org>
Date: Tue, 23 Oct 2018 09:37:30 -0600
Subject: [PATCH] documentation for MessageSenderJobQueue

---
 .../src/Network/MessageSenderJobQueue.swift   | 24 +++++++++++++++----
 SignalServiceKit/src/Util/JobQueue.swift      | 16 ++++++++++---
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/SignalServiceKit/src/Network/MessageSenderJobQueue.swift b/SignalServiceKit/src/Network/MessageSenderJobQueue.swift
index 6eb9ad712..9b17aeb1b 100644
--- a/SignalServiceKit/src/Network/MessageSenderJobQueue.swift
+++ b/SignalServiceKit/src/Network/MessageSenderJobQueue.swift
@@ -4,11 +4,23 @@
 
 import Foundation
 
-public extension Error {
-    var isRetryable: Bool {
-        return (self as NSError).isRetryable
-    }
-}
+/// Durably enqueues a message for sending.
+///
+/// The queue's operations (`MessageSenderOperation`) uses `MessageSender` to send a message.
+///
+/// ## Retry behavior
+///
+/// Like all JobQueue's, MessageSenderJobQueue implements retry handling for operation errors.
+///
+/// `MessageSender` also includes it's own retry logic necessary to encapsulate business logic around
+/// a user changing their Registration ID, or adding/removing devices. That is, it is sometimes *normal*
+/// for MessageSender to have to resend to a recipient multiple times before it is accepted, and doesn't
+/// represent a "failure" from the application standpoint.
+///
+/// So we have an inner non-durable retry (MessageSender) and an outer durable retry (MessageSenderJobQueue).
+///
+/// Both respect the `error.isRetryable` convention to be sure we don't keep retrying in some situations
+/// (e.g. rate limiting)
 
 @objc(SSKMessageSenderJobQueue)
 public class MessageSenderJobQueue: NSObject, JobQueue {
@@ -104,6 +116,8 @@ public class MessageSenderJobQueue: NSObject, JobQueue {
         return operationQueue
     }()
 
+    // We use a per-thread serial OperationQueue to ensure messages are delivered to the
+    // service in the order the user sent them.
     public func operationQueue(jobRecord: SSKMessageSenderJobRecord) -> OperationQueue {
         guard let threadId = jobRecord.threadId else {
             return defaultQueue
diff --git a/SignalServiceKit/src/Util/JobQueue.swift b/SignalServiceKit/src/Util/JobQueue.swift
index 42325b3a8..607278b58 100644
--- a/SignalServiceKit/src/Util/JobQueue.swift
+++ b/SignalServiceKit/src/Util/JobQueue.swift
@@ -9,12 +9,22 @@ import Foundation
 /// When work needs to be done, add it to the JobQueue.
 /// The JobQueue will persist a JobRecord to be sure that work can be restarted if the app is killed.
 ///
-/// The actual work, is carried out in an operation which the JobQueue spins off, based on the contents
+/// The actual work, is carried out in a DurableOperation which the JobQueue spins off, based on the contents
 /// of a JobRecord.
 ///
-/// For a concrete example, adding a message to MessageSenderJobQueue, first records a SSKMessageSenderJobRecord.
-/// The MessageSenderJobQueue can use that SSKMessageSenderJobRecord to create a MessageSenderOperation which
+/// For a concrete example, take message sending.
+/// Add an outgoing message to the MessageSenderJobQueue, which first records a SSKMessageSenderJobRecord.
+/// The MessageSenderJobQueue then uses that SSKMessageSenderJobRecord to create a MessageSenderOperation which
 /// takes care of the actual business of communicating with the service.
+///
+/// DurableOperations are retryable - via their `remainingRetries` logic. However, if the operation encounters
+/// an error where `error.isRetryable == false`, the operation will fail, regardless of available retries.
+
+public extension Error {
+    var isRetryable: Bool {
+        return (self as NSError).isRetryable
+    }
+}
 
 public enum JobError: Error {
     case assertionFailure(description: String)