Adding a new topic subscription/unsubcription with completion handler method (#1001)

* add new topic subscription/unsubscription method with handler

13 files changed, 115 insertions, 101 deletions

diff --git a/Firebase/Messaging/FIRMessagingClient.h b/Firebase/Messaging/FIRMessagingClient.h
index 1726428..cb76e98 100644
--- a/Firebase/Messaging/FIRMessagingClient.h
+++ b/Firebase/Messaging/FIRMessagingClient.h
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-#import "FIRMessagingTopicsCommon.h"
+#import "FIRMessaging.h"
 
 @class FIRReachabilityChecker;
 @class GPBMessage;
diff --git a/Firebase/Messaging/FIRMessagingClient.m b/Firebase/Messaging/FIRMessagingClient.m
index 51a62b7..d36778d 100644
--- a/Firebase/Messaging/FIRMessagingClient.m
+++ b/Firebase/Messaging/FIRMessagingClient.m
@@ -18,6 +18,7 @@
 
 #import <FirebaseCore/FIRReachabilityChecker.h>
 
+#import "FIRMessaging.h"
 #import "FIRMessagingConnection.h"
 #import "FIRMessagingConstants.h"
 #import "FIRMessagingDataMessageManager.h"
@@ -168,8 +169,7 @@ static NSUInteger FIRMessagingServerPort() {
 
   _FIRMessagingDevAssert(handler != nil, @"Invalid handler to FIRMessaging subscribe");
 
-  FIRMessagingTopicOperationCompletion completion =
-      ^void(FIRMessagingTopicOperationResult result, NSError * error) {
+  FIRMessagingTopicOperationCompletion completion = ^void(NSError *error) {
     if (error) {
       FIRMessagingLoggerError(kFIRMessagingMessageCodeClient001, @"Failed to subscribe to topic %@",
                               error);
@@ -182,7 +182,7 @@ static NSUInteger FIRMessagingServerPort() {
                                @"Successfully subscribed to topic %@", topic);
       }
     }
-    handler(result, error);
+    handler(error);
   };
 
   [self.registrar tryToLoadValidCheckinInfo];
diff --git a/Firebase/Messaging/FIRMessagingPendingTopicsList.h b/Firebase/Messaging/FIRMessagingPendingTopicsList.h
index c5a306a..a8108bf 100644
--- a/Firebase/Messaging/FIRMessagingPendingTopicsList.h
+++ b/Firebase/Messaging/FIRMessagingPendingTopicsList.h
@@ -16,6 +16,7 @@
 
 #import <Foundation/Foundation.h>
 
+#import "FIRMessaging.h"
 #import "FIRMessagingTopicsCommon.h"
 
 NS_ASSUME_NONNULL_BEGIN
diff --git a/Firebase/Messaging/FIRMessagingPendingTopicsList.m b/Firebase/Messaging/FIRMessagingPendingTopicsList.m
index 8bab770..b10b552 100644
--- a/Firebase/Messaging/FIRMessagingPendingTopicsList.m
+++ b/Firebase/Messaging/FIRMessagingPendingTopicsList.m
@@ -217,46 +217,49 @@ NSString *const kPendingTopicsTimestampEncodingKey = @"ts";
     [self.topicsInFlight addObject:topic];
   }
   FIRMessaging_WEAKIFY(self);
-  [self.delegate pendingTopicsList:self
-           requestedUpdateForTopic:topic
-                            action:self.currentBatch.action
-                        completion:^(FIRMessagingTopicOperationResult result, NSError * error) {
-    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_BACKGROUND, 0), ^{
-      FIRMessaging_STRONGIFY(self);
-      @synchronized (self) {
-        [self.topicsInFlight removeObject:topic];
-
-        BOOL recoverableError = [self subscriptionErrorIsRecoverable:error];
-        if (result == FIRMessagingTopicOperationResultSucceeded ||
-            result == FIRMessagingTopicOperationResultCancelled ||
-            !recoverableError) {
-          // Notify our handlers and remove the topic from our batch
-          NSMutableArray *handlers = self.currentBatch.topicHandlers[topic];
-          if (handlers.count) {
-            dispatch_async(dispatch_get_main_queue(), ^{
-              for (FIRMessagingTopicOperationCompletion handler in handlers) {
-                handler(result, error);
-              }
-              [handlers removeAllObjects];
-            });
-          }
-          [self.currentBatch.topics removeObject:topic];
-          [self.currentBatch.topicHandlers removeObjectForKey:topic];
-          if (self.currentBatch.topics.count == 0) {
-            // All topic updates successfully finished in this batch, move on to the next batch
-            [self.topicBatches removeObject:self.currentBatch];
-            self.currentBatch = nil;
-          }
-          [self.delegate pendingTopicsListDidUpdate:self];
-          FIRMessaging_WEAKIFY(self)
-          dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_BACKGROUND, 0), ^{
-            FIRMessaging_STRONGIFY(self)
-            [self resumeOperationsIfNeeded];
-          });
-        }
-      }
-    });
-  }];
+  [self.delegate
+            pendingTopicsList:self
+      requestedUpdateForTopic:topic
+                       action:self.currentBatch.action
+                   completion:^(NSError *error) {
+                     dispatch_async(
+                         dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_BACKGROUND, 0), ^{
+                           FIRMessaging_STRONGIFY(self);
+                           @synchronized(self) {
+                             [self.topicsInFlight removeObject:topic];
+
+                             BOOL recoverableError = [self subscriptionErrorIsRecoverable:error];
+                             if (!error || !recoverableError) {
+                               // Notify our handlers and remove the topic from our batch
+                               NSMutableArray *handlers = self.currentBatch.topicHandlers[topic];
+                               if (handlers.count) {
+                                 dispatch_async(dispatch_get_main_queue(), ^{
+                                   for (FIRMessagingTopicOperationCompletion handler in handlers) {
+                                     handler(error);
+                                   }
+                                   [handlers removeAllObjects];
+                                 });
+                               }
+                               [self.currentBatch.topics removeObject:topic];
+                               [self.currentBatch.topicHandlers removeObjectForKey:topic];
+                               if (self.currentBatch.topics.count == 0) {
+                                 // All topic updates successfully finished in this batch, move on
+                                 // to the next batch
+                                 [self.topicBatches removeObject:self.currentBatch];
+                                 self.currentBatch = nil;
+                               }
+                               [self.delegate pendingTopicsListDidUpdate:self];
+                               FIRMessaging_WEAKIFY(self);
+                               dispatch_async(
+                                   dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_BACKGROUND, 0),
+                                   ^{
+                                     FIRMessaging_STRONGIFY(self);
+                                     [self resumeOperationsIfNeeded];
+                                   });
+                             }
+                           }
+                         });
+                   }];
 }
 
 @end
diff --git a/Firebase/Messaging/FIRMessagingPubSub.h b/Firebase/Messaging/FIRMessagingPubSub.h
index 2ce8ed4..a2141e4 100644
--- a/Firebase/Messaging/FIRMessagingPubSub.h
+++ b/Firebase/Messaging/FIRMessagingPubSub.h
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-#import "FIRMessagingTopicsCommon.h"
+#import "FIRMessaging.h"
 
 @class FIRMessagingClient;
 @class FIRMessagingPubSubCache;
diff --git a/Firebase/Messaging/FIRMessagingPubSub.m b/Firebase/Messaging/FIRMessagingPubSub.m
index 74a5292..09491b4 100644
--- a/Firebase/Messaging/FIRMessagingPubSub.m
+++ b/Firebase/Messaging/FIRMessagingPubSub.m
@@ -60,8 +60,7 @@ static NSString *const kPendingSubscriptionsListKey =
   _FIRMessagingDevAssert([token length], @"FIRMessaging error no token specified");
   _FIRMessagingDevAssert([topic length], @"FIRMessaging error Invalid empty topic specified");
   if (!self.client) {
-    handler(FIRMessagingTopicOperationResultError,
-            [NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubFIRMessagingNotSetup]);
+    handler([NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubFIRMessagingNotSetup]);
     return;
   }
 
@@ -75,8 +74,7 @@ static NSString *const kPendingSubscriptionsListKey =
   if (![[self class] isValidTopicWithPrefix:topic]) {
     FIRMessagingLoggerError(kFIRMessagingMessageCodePubSub000,
                             @"Invalid FIRMessaging Pubsub topic %@", topic);
-    handler(FIRMessagingTopicOperationResultError,
-            [NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubInvalidTopic]);
+    handler([NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubInvalidTopic]);
     return;
   }
 
@@ -93,11 +91,9 @@ static NSString *const kPendingSubscriptionsListKey =
                                      topic:topic
                                    options:options
                               shouldDelete:NO
-                                   handler:
-      ^void(FIRMessagingTopicOperationResult result, NSError * error) {
-
-    handler(result, error);
-  }];
+                                   handler:^void(NSError *error) {
+                                     handler(error);
+                                   }];
 }
 
 - (void)unsubscribeWithToken:(NSString *)token
@@ -108,8 +104,7 @@ static NSString *const kPendingSubscriptionsListKey =
   _FIRMessagingDevAssert([topic length], @"FIRMessaging error Invalid empty topic specified");
 
   if (!self.client) {
-    handler(FIRMessagingTopicOperationResultError,
-            [NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubFIRMessagingNotSetup]);
+    handler([NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubFIRMessagingNotSetup]);
     return;
   }
 
@@ -122,8 +117,7 @@ static NSString *const kPendingSubscriptionsListKey =
   if (![[self class] isValidTopicWithPrefix:topic]) {
     FIRMessagingLoggerError(kFIRMessagingMessageCodePubSub002,
                             @"Invalid FIRMessaging Pubsub topic %@", topic);
-    handler(FIRMessagingTopicOperationResultError,
-            [NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubInvalidTopic]);
+    handler([NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubInvalidTopic]);
     return;
   }
   if (![self verifyPubSubOptions:options]) {
@@ -139,11 +133,9 @@ static NSString *const kPendingSubscriptionsListKey =
                                      topic:topic
                                    options:options
                               shouldDelete:YES
-                                   handler:
-      ^void(FIRMessagingTopicOperationResult result, NSError * error) {
-
-    handler(result, error);
-  }];
+                                   handler:^void(NSError *error) {
+                                     handler(error);
+                                   }];
 }
 
 - (void)subscribeToTopic:(NSString *)topic
diff --git a/Firebase/Messaging/FIRMessagingRegistrar.h b/Firebase/Messaging/FIRMessagingRegistrar.h
index 5b60437..35f3ff3 100644
--- a/Firebase/Messaging/FIRMessagingRegistrar.h
+++ b/Firebase/Messaging/FIRMessagingRegistrar.h
@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
+#import "FIRMessaging.h"
 #import "FIRMessagingCheckinService.h"
-#import "FIRMessagingTopicsCommon.h"
 
 @class FIRMessagingCheckinStore;
 @class FIRMessagingPubSubRegistrar;
diff --git a/Firebase/Messaging/FIRMessagingRegistrar.m b/Firebase/Messaging/FIRMessagingRegistrar.m
index ab57b9e..bb1e9ad 100644
--- a/Firebase/Messaging/FIRMessagingRegistrar.m
+++ b/Firebase/Messaging/FIRMessagingRegistrar.m
@@ -83,7 +83,7 @@
     FIRMessagingLoggerDebug(kFIRMessagingMessageCodeRegistrar000,
                             @"Device check in error, no auth credentials found");
     NSError *error = [NSError errorWithFCMErrorCode:kFIRMessagingErrorCodeMissingDeviceID];
-    handler(FIRMessagingTopicOperationResultError, error);
+    handler(error);
   }
 }
 
diff --git a/Firebase/Messaging/FIRMessagingTopicOperation.h b/Firebase/Messaging/FIRMessagingTopicOperation.h
index e4bbde8..ea98e6d 100644
--- a/Firebase/Messaging/FIRMessagingTopicOperation.h
+++ b/Firebase/Messaging/FIRMessagingTopicOperation.h
@@ -16,6 +16,7 @@
 
 #import <Foundation/Foundation.h>
 
+#import "FIRMessaging.h"
 #import "FIRMessagingCheckinService.h"
 #import "FIRMessagingTopicsCommon.h"
 
diff --git a/Firebase/Messaging/FIRMessagingTopicOperation.m b/Firebase/Messaging/FIRMessagingTopicOperation.m
index 3120240..1546d47 100644
--- a/Firebase/Messaging/FIRMessagingTopicOperation.m
+++ b/Firebase/Messaging/FIRMessagingTopicOperation.m
@@ -125,7 +125,9 @@ NSString *FIRMessagingSubscriptionsServer() {
 
 - (void)start {
   if (self.isCancelled) {
-    [self finishWithResult:FIRMessagingTopicOperationResultCancelled error:nil];
+    NSError *error =
+        [NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubOperationIsCancelled];
+    [self finishWithError:error];
     return;
   }
 
@@ -134,14 +136,14 @@ NSString *FIRMessagingSubscriptionsServer() {
   [self performSubscriptionChange];
 }
 
-- (void)finishWithResult:(FIRMessagingTopicOperationResult)result error:(NSError *)error {
+- (void)finishWithError:(NSError *)error {
   // Add a check to prevent this finish from being called more than once.
   if (self.isFinished) {
     return;
   }
   self.dataTask = nil;
   if (self.completion) {
-    self.completion(result, error);
+    self.completion(error);
   }
 
   [self setExecuting:NO];
@@ -151,7 +153,8 @@ NSString *FIRMessagingSubscriptionsServer() {
 - (void)cancel {
   [super cancel];
   [self.dataTask cancel];
-  [self finishWithResult:FIRMessagingTopicOperationResultCancelled error:nil];
+  NSError *error = [NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubOperationIsCancelled];
+  [self finishWithError:error];
 }
 
 - (void)performSubscriptionChange {
@@ -218,13 +221,12 @@ NSString *FIRMessagingSubscriptionsServer() {
       FIRMessagingLoggerDebug(kFIRMessagingMessageCodeTopicOption001,
                               @"Device registration HTTP fetch error. Error Code: %ld",
                               _FIRMessaging_L(error.code));
-      [self finishWithResult:FIRMessagingTopicOperationResultError error:error];
+      [self finishWithError:error];
       return;
     }
     NSString *response = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding];
     if (response.length == 0) {
-      [self finishWithResult:FIRMessagingTopicOperationResultError
-                       error:[NSError errorWithFCMErrorCode:kFIRMessagingErrorCodeUnknown]];
+      [self finishWithError:[NSError errorWithFCMErrorCode:kFIRMessagingErrorCodeUnknown]];
       return;
     }
     NSArray *parts = [response componentsSeparatedByString:@"="];
@@ -232,15 +234,14 @@ NSString *FIRMessagingSubscriptionsServer() {
     if (![parts[0] isEqualToString:@"token"] || parts.count <= 1) {
       FIRMessagingLoggerDebug(kFIRMessagingMessageCodeTopicOption002,
                               @"Invalid registration request, response");
-      [self finishWithResult:FIRMessagingTopicOperationResultError
-                       error:[NSError errorWithFCMErrorCode:kFIRMessagingErrorCodeUnknown]];
+      [self finishWithError:[NSError errorWithFCMErrorCode:kFIRMessagingErrorCodeUnknown]];
       return;
     }
 #if DEBUG_LOG_SUBSCRIPTION_OPERATION_DURATIONS
     NSTimeInterval duration = -[start timeIntervalSinceNow];
     FIRMessagingLoggerDebug(@"%@ change took %.2fs", self.topic, duration);
 #endif
-    [self finishWithResult:FIRMessagingTopicOperationResultSucceeded error:nil];
+    [self finishWithError:nil];
 
   };
 
diff --git a/Firebase/Messaging/FIRMessagingTopicsCommon.h b/Firebase/Messaging/FIRMessagingTopicsCommon.h
index 50d8906..030b3ff 100644
--- a/Firebase/Messaging/FIRMessagingTopicsCommon.h
+++ b/Firebase/Messaging/FIRMessagingTopicsCommon.h
@@ -26,27 +26,4 @@ typedef NS_ENUM(NSInteger, FIRMessagingTopicAction) {
   FIRMessagingTopicActionUnsubscribe
 };
 
-/**
- * Represents the possible results of a topic operation.
- */
-typedef NS_ENUM(NSInteger, FIRMessagingTopicOperationResult) {
-  FIRMessagingTopicOperationResultSucceeded,
-  FIRMessagingTopicOperationResultError,
-  FIRMessagingTopicOperationResultCancelled,
-};
-
-/**
- *  Callback to invoke once the HTTP call to FIRMessaging backend for updating
- *  subscription finishes.
- *
- *  @param result         The result of the operation. If the result is
- *                        FIRMessagingTopicOperationResultError, the error parameter will be
- *                        non-nil.
- *  @param error          The error which occurred while updating the subscription topic
- *                        on the FIRMessaging server. This will be nil in case the operation
- *                        was successful, or if the operation was cancelled.
- */
-typedef void(^FIRMessagingTopicOperationCompletion)
-    (FIRMessagingTopicOperationResult result, NSError * _Nullable error);
-
 NS_ASSUME_NONNULL_END
diff --git a/Firebase/Messaging/NSError+FIRMessaging.h b/Firebase/Messaging/NSError+FIRMessaging.h
index 9b1e214..ae25b5b 100644
--- a/Firebase/Messaging/NSError+FIRMessaging.h
+++ b/Firebase/Messaging/NSError+FIRMessaging.h
@@ -56,6 +56,7 @@ typedef NS_ENUM(NSUInteger, FIRMessagingInternalErrorCode) {
   kFIRMessagingErrorCodePubSubAlreadyUnsubscribed = 3002,
   kFIRMessagingErrorCodePubSubInvalidTopic = 3003,
   kFIRMessagingErrorCodePubSubFIRMessagingNotSetup = 3004,
+  kFIRMessagingErrorCodePubSubOperationIsCancelled = 3005,
 };
 
 @interface NSError (FIRMessaging)
diff --git a/Firebase/Messaging/Public/FIRMessaging.h b/Firebase/Messaging/Public/FIRMessaging.h
index 31e8625..f56b7ab 100644
--- a/Firebase/Messaging/Public/FIRMessaging.h
+++ b/Firebase/Messaging/Public/FIRMessaging.h
@@ -23,9 +23,9 @@
  *  If the call fails we return the appropriate `error code`, described by
  *  `FIRMessagingError`.
  *
- *  @param FCMToken The valid registration token returned by FCM.
- *  @param error The error describing why a token request failed. The error code
- *               will match a value from the FIRMessagingError enumeration.
+ *  @param FCMToken  The valid registration token returned by FCM.
+ *  @param error     The error describing why a token request failed. The error code
+ *                   will match a value from the FIRMessagingError enumeration.
  */
 typedef void(^FIRMessagingFCMTokenFetchCompletion)(NSString * _Nullable FCMToken,
     NSError * _Nullable error)
@@ -46,6 +46,16 @@ typedef void(^FIRMessagingDeleteFCMTokenCompletion)(NSError * _Nullable error)
     NS_SWIFT_NAME(MessagingDeleteFCMTokenCompletion);
 
 /**
+ *  Callback to invoke once the HTTP call to FIRMessaging backend for updating
+ *  subscription finishes.
+ *
+ *  @param error  The error which occurred while updating the subscription topic
+ *                on the FIRMessaging server. This will be nil in case the operation
+ *                was successful, or if the operation was cancelled.
+ */
+typedef void (^FIRMessagingTopicOperationCompletion)(NSError *_Nullable error);
+
+/**
  *  The completion handler invoked once the data connection with FIRMessaging is
  *  established.  The data connection is used to send a continous stream of
  *  data and all the FIRMessaging data notifications arrive through this connection.
@@ -459,12 +469,40 @@ NS_SWIFT_NAME(Messaging)
 - (void)subscribeToTopic:(nonnull NSString *)topic NS_SWIFT_NAME(subscribe(toTopic:));
 
 /**
+ *  Asynchronously subscribe to the topic. Adds to the pending list of topic operations.
+ *  Retry in case of failures. This makes a repeated attempt to subscribe to the topic
+ *  as compared to the `subscribe` method above which tries once.
+ *
+ *  @param topic       The topic name to subscribe to. Should be of the form
+ *                     `"/topics/<topic-name>"`.
+ *  @param completion  The completion that is invoked once the unsubscribe call ends.
+ *                     In case of success, nil error is returned. Otherwise, an
+ *                     appropriate error object is returned.
+ */
+- (void)subscribeToTopic:(NSString *)topic
+              completion:(nullable FIRMessagingTopicOperationCompletion)completion;
+
+/**
  *  Asynchronously unsubscribe from a topic.
  *
  *  @param topic The name of the topic, for example @"sports".
  */
 - (void)unsubscribeFromTopic:(nonnull NSString *)topic NS_SWIFT_NAME(unsubscribe(fromTopic:));
 
+/**
+ *  Asynchronously unsubscribe from the topic. Adds to the pending list of topic operations.
+ *  Retry in case of failures. This makes a repeated attempt to unsubscribe from the topic
+ *  as compared to the `unsubscribe` method above which tries once.
+ *
+ *  @param topic       The topic name to unsubscribe from. Should be of the form
+ *                     `"/topics/<topic-name>"`.
+ *  @param completion  The completion that is invoked once the unsubscribe call ends.
+ *                     In case of success, nil error is returned. Otherwise, an
+ *                     appropriate error object is returned.
+ */
+- (void)unsubscribeFromTopic:(NSString *)topic
+                  completion:(nullable FIRMessagingTopicOperationCompletion)completion;
+
 #pragma mark - Upstream
 
 /**