From e75f6139ffdecd56068ab55524459b0e173fcf05 Mon Sep 17 00:00:00 2001
From: Chen Liang <chliang@google.com>
Date: Fri, 15 Jun 2018 19:39:57 -0700
Subject: [PATCH 1/6] Handle all the completion cases for topic
 subscription/unsubscription

---
 .../Messaging/Tests/FIRMessagingServiceTest.m | 238 +++++++++++-------
 Firebase/Messaging/FIRMessaging.m             |  74 +++---
 Firebase/Messaging/Public/FIRMessaging.h      |   6 +
 3 files changed, 206 insertions(+), 112 deletions(-)

diff --git a/Example/Messaging/Tests/FIRMessagingServiceTest.m b/Example/Messaging/Tests/FIRMessagingServiceTest.m
index 073adad1a29..4983b58e089 100644
--- a/Example/Messaging/Tests/FIRMessagingServiceTest.m
+++ b/Example/Messaging/Tests/FIRMessagingServiceTest.m
@@ -26,6 +26,11 @@
 #import "InternalHeaders/FIRMessagingInternalUtilities.h"
 #import "NSError+FIRMessaging.h"
 
+static NSString *const kFakeToken =
+    @"fE1e1PZJFSQ:APA91bFAOjp1ahBWn9rTlbjArwBEm_"
+    @"yUTTzK6dhIvLqzqqCSabaa4TQVM0pGTmF6r7tmMHPe6VYiGMHuCwJFgj5v97xl78sUNMLwuPPhoci8z_"
+    @"QGlCrTbxCFGzEUfvA3fGpGgIVQU2W6";
+
 @interface FIRMessaging () <FIRMessagingClientDelegate>
 
 @property(nonatomic, readwrite, strong) FIRMessagingClient *client;
@@ -40,22 +45,35 @@ @interface FIRMessagingPubSub ()
 
 @end
 
-
-@interface FIRMessagingServiceTest : XCTestCase
+@interface FIRMessagingServiceTest : XCTestCase {
+  FIRMessaging *_messaging;
+  id _mockPubSub;
+}
 
 @end
 
 @implementation FIRMessagingServiceTest
 
+- (void)setUp {
+  _messaging = [FIRMessaging messaging];
+  _messaging.defaultFcmToken = kFakeToken;
+  _mockPubSub = OCMPartialMock(_messaging.pubsub);
+  [super setUp];
+}
+
+- (void)tearDown {
+  [_mockPubSub stopMocking];
+  [super tearDown];
+}
+
 - (void)testSubscribe {
   id mockClient = OCMClassMock([FIRMessagingClient class]);
-  FIRMessaging *service = [FIRMessaging messaging];
-  [service setClient:mockClient];
-  [service.pubsub setClient:mockClient];
+  [_messaging setClient:mockClient];
+  [_mockPubSub setClient:mockClient];
 
   XCTestExpectation *subscribeExpectation =
       [self expectationWithDescription:@"Should call subscribe on FIRMessagingClient"];
-  NSString *token = @"abcdefghijklmn";
+  NSString *token = kFakeToken;
   NSString *topic = @"/topics/some-random-topic";
 
   [[[mockClient stub]
@@ -68,12 +86,12 @@ - (void)testSubscribe {
                      shouldDelete:NO
                           handler:OCMOCK_ANY];
 
-  [service.pubsub subscribeWithToken:token
-                               topic:topic
-                             options:nil
-                             handler:^(NSError *error){
-                                 // not a nil block
-                             }];
+  [_mockPubSub subscribeWithToken:token
+                            topic:topic
+                          options:nil
+                          handler:^(NSError *error){
+                              // not a nil block
+                          }];
 
   // should call updateSubscription
   [self waitForExpectationsWithTimeout:0.1
@@ -85,14 +103,13 @@ - (void)testSubscribe {
 
 - (void)testUnsubscribe {
   id mockClient = OCMClassMock([FIRMessagingClient class]);
-  FIRMessaging *messaging = [FIRMessaging messaging];
-  [messaging setClient:mockClient];
-  [messaging.pubsub setClient:mockClient];
+  [_messaging setClient:mockClient];
+  [_mockPubSub setClient:mockClient];
 
   XCTestExpectation *subscribeExpectation =
       [self expectationWithDescription:@"Should call unsubscribe on FIRMessagingClient"];
 
-  NSString *token = @"abcdefghijklmn";
+  NSString *token = kFakeToken;
   NSString *topic = @"/topics/some-random-topic";
 
   [[[mockClient stub] andDo:^(NSInvocation *invocation) {
@@ -109,12 +126,12 @@ - (void)testUnsubscribe {
                      shouldDelete:YES
                           handler:OCMOCK_ANY];
 
-  [messaging.pubsub unsubscribeWithToken:token
-                                   topic:topic
-                                 options:nil
-                                 handler:^(NSError *error){
+  [_mockPubSub unsubscribeWithToken:token
+                              topic:topic
+                            options:nil
+                            handler:^(NSError *error){
 
-                                 }];
+                            }];
 
   // should call updateSubscription
   [self waitForExpectationsWithTimeout:0.1
@@ -128,8 +145,8 @@ - (void)testUnsubscribe {
  *  Test using PubSub without explicitly starting FIRMessagingService.
  */
 - (void)testSubscribeWithoutStart {
-  [[[FIRMessaging messaging] pubsub]
-      subscribeWithToken:@"abcdef1234"
+  [_mockPubSub
+      subscribeWithToken:kFakeToken
                    topic:@"/topics/hello-world"
                  options:nil
                  handler:^(NSError *error) {
@@ -141,19 +158,18 @@ - (void)testSubscribeWithoutStart {
 // TODO(chliangGoogle) Investigate why invalid token can't throw assertion but the rest can under
 // release build.
 - (void)testSubscribeWithInvalidTopic {
-  FIRMessaging *messaging = [FIRMessaging messaging];
 
   XCTestExpectation *exceptionExpectation =
   [self expectationWithDescription:@"Should throw exception for invalid token"];
   @try {
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wnonnull"
-    [messaging.pubsub subscribeWithToken:@"abcdef1234"
-                                   topic:nil
-                                 options:nil
-                                 handler:^(NSError *error) {
-                                   XCTFail(@"Should not invoke the handler");
-                                 }];
+    [_mockPubSub subscribeWithToken:kFakeToken
+                              topic:nil
+                            options:nil
+                            handler:^(NSError *error) {
+                              XCTFail(@"Should not invoke the handler");
+                            }];
 #pragma clang diagnostic pop
   }
   @catch (NSException *exception) {
@@ -167,19 +183,17 @@ - (void)testSubscribeWithInvalidTopic {
 }
 
 - (void)testUnsubscribeWithInvalidTopic {
-  FIRMessaging *messaging = [FIRMessaging messaging];
-
   XCTestExpectation *exceptionExpectation =
       [self expectationWithDescription:@"Should throw exception for invalid token"];
   @try {
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wnonnull"
-    [messaging.pubsub unsubscribeWithToken:@"abcdef1234"
-                                     topic:nil
-                                   options:nil
-                                   handler:^(NSError *error) {
-                                     XCTFail(@"Should not invoke the handler");
-                                   }];
+    [_mockPubSub unsubscribeWithToken:kFakeToken
+                                topic:nil
+                              options:nil
+                              handler:^(NSError *error) {
+                                XCTFail(@"Should not invoke the handler");
+                              }];
 #pragma clang diagnostic pop
   }
   @catch (NSException *exception) {
@@ -193,68 +207,124 @@ - (void)testUnsubscribeWithInvalidTopic {
 }
 
 - (void)testSubscribeWithNoTopicPrefix {
-  FIRMessaging *messaging = [FIRMessaging messaging];
-  FIRMessagingPubSub *pubSub = messaging.pubsub;
-  id mockPubSub = OCMClassMock([FIRMessagingPubSub class]);
 
   NSString *topicName = @"topicWithoutPrefix";
   NSString *topicNameWithPrefix = [FIRMessagingPubSub addPrefixToTopic:topicName];
-  messaging.pubsub = mockPubSub;
-  messaging.defaultFcmToken = @"fake-default-token";
-  OCMExpect([messaging.pubsub subscribeToTopic:[OCMArg isEqual:topicNameWithPrefix]
-                                       handler:[OCMArg any]]);
-  [messaging subscribeToTopic:topicName];
-  OCMVerifyAll(mockPubSub);
-  // Need to swap back since it's a singleton and hence will live beyond the scope of this test.
-  messaging.pubsub = pubSub;
+  OCMExpect(
+      [_mockPubSub subscribeToTopic:[OCMArg isEqual:topicNameWithPrefix] handler:[OCMArg any]]);
+  [_messaging subscribeToTopic:topicName];
+  OCMVerifyAll(_mockPubSub);
 }
 
 - (void)testSubscribeWithTopicPrefix {
-  FIRMessaging *messaging = [FIRMessaging messaging];
-  FIRMessagingPubSub *pubSub = messaging.pubsub;
-  id mockPubSub = OCMClassMock([FIRMessagingPubSub class]);
-
   NSString *topicName = @"/topics/topicWithoutPrefix";
-  messaging.pubsub = mockPubSub;
-  messaging.defaultFcmToken = @"fake-default-token";
-  OCMExpect([messaging.pubsub subscribeToTopic:[OCMArg isEqual:topicName] handler:[OCMArg any]]);
-  [messaging subscribeToTopic:topicName];
-  OCMVerifyAll(mockPubSub);
-  // Need to swap back since it's a singleton and hence will live beyond the scope of this test.
-  messaging.pubsub = pubSub;
+  OCMExpect([_mockPubSub subscribeToTopic:[OCMArg isEqual:topicName] handler:[OCMArg any]]);
+  [_messaging subscribeToTopic:topicName];
+  OCMVerifyAll(_mockPubSub);
 }
 
 - (void)testUnsubscribeWithNoTopicPrefix {
-  FIRMessaging *messaging = [FIRMessaging messaging];
-  FIRMessagingPubSub *pubSub = messaging.pubsub;
-  id mockPubSub = OCMClassMock([FIRMessagingPubSub class]);
-
   NSString *topicName = @"topicWithoutPrefix";
   NSString *topicNameWithPrefix = [FIRMessagingPubSub addPrefixToTopic:topicName];
-  messaging.pubsub = mockPubSub;
-  messaging.defaultFcmToken = @"fake-default-token";
-  OCMExpect([messaging.pubsub unsubscribeFromTopic:[OCMArg isEqual:topicNameWithPrefix]
-                                           handler:[OCMArg any]]);
-  [messaging unsubscribeFromTopic:topicName];
-  OCMVerifyAll(mockPubSub);
-  // Need to swap back since it's a singleton and hence will live beyond the scope of this test.
-  messaging.pubsub = pubSub;
+  OCMExpect(
+      [_mockPubSub unsubscribeFromTopic:[OCMArg isEqual:topicNameWithPrefix] handler:[OCMArg any]]);
+  [_messaging unsubscribeFromTopic:topicName];
+  OCMVerifyAll(_mockPubSub);
 }
 
 - (void)testUnsubscribeWithTopicPrefix {
-  FIRMessaging *messaging = [FIRMessaging messaging];
-  FIRMessagingPubSub *pubSub = messaging.pubsub;
-  id mockPubSub = OCMClassMock([FIRMessagingPubSub class]);
-
   NSString *topicName = @"/topics/topicWithPrefix";
-  messaging.pubsub = mockPubSub;
-  messaging.defaultFcmToken = @"fake-default-token";
-  OCMExpect([messaging.pubsub unsubscribeFromTopic:[OCMArg isEqual:topicName]
-                                           handler:[OCMArg any]]);
-  [messaging unsubscribeFromTopic:topicName];
-  OCMVerifyAll(mockPubSub);
-  // Need to swap back since it's a singleton and hence will live beyond the scope of this test.
-  messaging.pubsub = pubSub;
+  OCMExpect([_mockPubSub unsubscribeFromTopic:[OCMArg isEqual:topicName] handler:[OCMArg any]]);
+  [_messaging unsubscribeFromTopic:topicName];
+  OCMVerifyAll(_mockPubSub);
+}
+
+- (void)testSubScriptionCompletionHandlerWithInvalidToken {
+  XCTestExpectation *subscriptionCompletionExpectation =
+      [self expectationWithDescription:@"Subscription is complete"];
+  _messaging.defaultFcmToken = nil;
+  [_messaging subscribeToTopic:@"news"
+                    completion:^(NSError *error) {
+                      XCTAssertNotNil(error);
+                      XCTAssertEqual(error.code, FIRMessagingErrorTokenNotAvailable);
+                      [subscriptionCompletionExpectation fulfill];
+                    }];
+  [self waitForExpectationsWithTimeout:0.2
+                               handler:^(NSError *_Nullable error){
+                               }];
+}
+
+- (void)testSubscriptionCompletionHandlerWithInvalidTopicName {
+  XCTestExpectation *subscriptionCompletionExpectation =
+      [self expectationWithDescription:@"Subscription is complete"];
+  [_messaging subscribeToTopic:@"!@#$%^&*()"
+                    completion:^(NSError *_Nullable error) {
+                      XCTAssertNotNil(error);
+                      XCTAssertEqual(error.code, FIRMessagingErrorInvalidTopicName);
+                      [subscriptionCompletionExpectation fulfill];
+                    }];
+  [self waitForExpectationsWithTimeout:0.2
+                               handler:^(NSError *_Nullable error){
+                               }];
+}
+
+- (void)testSubscriptionCompletionHandlerWithSuccess {
+  OCMStub([_mockPubSub subscribeToTopic:[OCMArg any]
+                                handler:([OCMArg invokeBlockWithArgs:[NSNull null], nil])]);
+  XCTestExpectation *subscriptionCompletionExpectation =
+      [self expectationWithDescription:@"Subscription is complete"];
+  [_messaging subscribeToTopic:@"news"
+                    completion:^(NSError *error) {
+                      XCTAssertNil(error);
+                      [subscriptionCompletionExpectation fulfill];
+                    }];
+  [self waitForExpectationsWithTimeout:0.2
+                               handler:^(NSError *_Nullable error){
+                               }];
+}
+
+- (void)testUnsubscribeCompletionHandlerWithInvalidToken {
+  XCTestExpectation *unsubscriptionCompletionExpectation =
+      [self expectationWithDescription:@"Unsubscription is complete"];
+  _messaging.defaultFcmToken = nil;
+  [_messaging unsubscribeFromTopic:@""
+                        completion:^(NSError *error) {
+                          XCTAssertNotNil(error);
+                          XCTAssertEqual(error.code, FIRMessagingErrorTokenNotAvailable);
+                          [unsubscriptionCompletionExpectation fulfill];
+                        }];
+  [self waitForExpectationsWithTimeout:0.2
+                               handler:^(NSError *_Nullable error){
+                               }];
+}
+
+- (void)testUnsubscribeCompletionHandlerWithInvalidTopic {
+  XCTestExpectation *unsubscriptionCompletionExpectation =
+      [self expectationWithDescription:@"Unsubscription is complete"];
+  [_messaging unsubscribeFromTopic:@"!@#$%^&*()"
+                        completion:^(NSError *error) {
+                          XCTAssertNotNil(error);
+                          XCTAssertEqual(error.code, FIRMessagingErrorInvalidTopicName);
+                          [unsubscriptionCompletionExpectation fulfill];
+                        }];
+  [self waitForExpectationsWithTimeout:0.2
+                               handler:^(NSError *_Nullable error){
+                               }];
+}
+
+- (void)testUnsubscribeCompletionHandlerWithSuccess {
+  OCMStub([_mockPubSub unsubscribeFromTopic:[OCMArg any]
+                                    handler:([OCMArg invokeBlockWithArgs:[NSNull null], nil])]);
+  XCTestExpectation *unsubscriptionCompletionExpectation =
+      [self expectationWithDescription:@"Unsubscription is complete"];
+  [_messaging unsubscribeFromTopic:@"news"
+                        completion:^(NSError *_Nullable error) {
+                          XCTAssertNil(error);
+                          [unsubscriptionCompletionExpectation fulfill];
+                        }];
+  [self waitForExpectationsWithTimeout:0.2
+                               handler:^(NSError *_Nullable error){
+                               }];
 }
 
 - (void)testFIRMessagingSDKVersionInFIRMessagingService {
diff --git a/Firebase/Messaging/FIRMessaging.m b/Firebase/Messaging/FIRMessaging.m
index 3c4d999196b..083afcb0d12 100644
--- a/Firebase/Messaging/FIRMessaging.m
+++ b/Firebase/Messaging/FIRMessaging.m
@@ -492,6 +492,7 @@ - (void)setAutoInitEnabled:(BOOL)autoInitEnabled {
     self.defaultFcmToken = self.instanceID.token;
 #pragma clang diagnostic pop
   }
+  [_messagingUserDefaults synchronize];
 }
 
 - (NSString *)FCMToken {
@@ -680,6 +681,9 @@ - (void)disconnect {
 #pragma mark - Topics
 
 + (NSString *)normalizeTopic:(NSString *)topic {
+  if (!topic.length) {
+    return nil;
+  }
   if (![FIRMessagingPubSub hasTopicsPrefix:topic]) {
     topic = [FIRMessagingPubSub addPrefixToTopic:topic];
   }
@@ -695,23 +699,30 @@ - (void)subscribeToTopic:(NSString *)topic {
 
 - (void)subscribeToTopic:(NSString *)topic
               completion:(nullable FIRMessagingTopicOperationCompletion)completion {
-  if (self.defaultFcmToken.length && topic.length) {
-    NSString *normalizeTopic = [[self class ] normalizeTopic:topic];
-    if ([FIRMessagingPubSub hasTopicsPrefix:topic]) {
-      FIRMessagingLoggerWarn(kFIRMessagingMessageCodeTopicFormatIsDeprecated,
-                             @"Format '%@' is deprecated. Only '%@' should be used in "
-                             @"subscribeToTopic.", topic, normalizeTopic);
-    }
-    if (normalizeTopic.length) {
-      [self.pubsub subscribeToTopic:normalizeTopic handler:completion];
-    } else {
-      FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging009,
-                              @"Cannot parse topic name %@. Will not subscribe.", topic);
-    }
-  } else {
+  if (!self.defaultFcmToken.length) {
     FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging010,
                             @"Cannot subscribe to topic: %@ with token: %@", topic,
                             self.defaultFcmToken);
+    if (completion) {
+      completion([NSError fcm_errorWithCode:FIRMessagingErrorTokenNotAvailable userInfo:nil]);
+    }
+    return;
+  }
+  NSString *normalizeTopic = [[self class] normalizeTopic:topic];
+  if ([FIRMessagingPubSub hasTopicsPrefix:topic]) {
+    FIRMessagingLoggerWarn(kFIRMessagingMessageCodeTopicFormatIsDeprecated,
+                           @"Format '%@' is deprecated. Only '%@' should be used in "
+                           @"subscribeToTopic.",
+                           topic, normalizeTopic);
+  }
+  if (normalizeTopic.length) {
+    [self.pubsub subscribeToTopic:normalizeTopic handler:completion];
+    return;
+  }
+  FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging009,
+                          @"Cannot parse topic name %@. Will not subscribe.", topic);
+  if (completion) {
+    completion([NSError fcm_errorWithCode:FIRMessagingErrorInvalidTopicName userInfo:nil]);
   }
 }
 
@@ -721,23 +732,30 @@ - (void)unsubscribeFromTopic:(NSString *)topic {
 
 - (void)unsubscribeFromTopic:(NSString *)topic
                   completion:(nullable FIRMessagingTopicOperationCompletion)completion {
-  if (self.defaultFcmToken.length && topic.length) {
-    NSString *normalizeTopic = [[self class] normalizeTopic:topic];
-    if ([FIRMessagingPubSub hasTopicsPrefix:topic]) {
-      FIRMessagingLoggerWarn(kFIRMessagingMessageCodeTopicFormatIsDeprecated,
-                             @"Format '%@' is deprecated. Only '%@' should be used in "
-                             @"unsubscribeFromTopic.", topic, normalizeTopic);
-    }
-    if (normalizeTopic.length) {
-      [self.pubsub unsubscribeFromTopic:normalizeTopic handler:completion];
-    } else {
-      FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging011,
-                              @"Cannot parse topic name %@. Will not unsubscribe.", topic);
-    }
-  } else {
+  if (!self.defaultFcmToken.length) {
     FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging012,
                             @"Cannot unsubscribe to topic: %@ with token: %@", topic,
                             self.defaultFcmToken);
+    if (completion) {
+      completion([NSError fcm_errorWithCode:FIRMessagingErrorTokenNotAvailable userInfo:nil]);
+    }
+    return;
+  }
+  NSString *normalizeTopic = [[self class] normalizeTopic:topic];
+  if ([FIRMessagingPubSub hasTopicsPrefix:topic]) {
+    FIRMessagingLoggerWarn(kFIRMessagingMessageCodeTopicFormatIsDeprecated,
+                           @"Format '%@' is deprecated. Only '%@' should be used in "
+                           @"unsubscribeFromTopic.",
+                           topic, normalizeTopic);
+  }
+  if (normalizeTopic.length) {
+    [self.pubsub unsubscribeFromTopic:normalizeTopic handler:completion];
+    return;
+  }
+  FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging011,
+                          @"Cannot parse topic name %@. Will not unsubscribe.", topic);
+  if (completion) {
+    completion([NSError fcm_errorWithCode:FIRMessagingErrorInvalidTopicName userInfo:nil]);
   }
 }
 
diff --git a/Firebase/Messaging/Public/FIRMessaging.h b/Firebase/Messaging/Public/FIRMessaging.h
index e58a216c2ca..823e41b3b3d 100644
--- a/Firebase/Messaging/Public/FIRMessaging.h
+++ b/Firebase/Messaging/Public/FIRMessaging.h
@@ -189,6 +189,12 @@ typedef NS_ENUM(NSUInteger, FIRMessagingError) {
 
   /// Some parameters of the request were invalid.
   FIRMessagingErrorInvalidRequest = 7,
+
+  /// Token is not available for topic subscription/unsubscription.
+  FIRMessagingErrorTokenNotAvailable = 8,
+
+  /// Topic name is invalid for subscription/unsubscription.
+  FIRMessagingErrorInvalidTopicName = 9,
 } NS_SWIFT_NAME(MessagingError);
 
 /// Status for the downstream message received by the app.

From 97d1e2ee19fb635de0152aaaa20fed651c9bd66d Mon Sep 17 00:00:00 2001
From: Chen Liang <chliang@google.com>
Date: Tue, 19 Jun 2018 13:02:33 -0700
Subject: [PATCH 2/6] Corrected the deprecation warning when subscribing to or
 unsubscribing from a topic

---
 Firebase/Messaging/FIRMessaging.m       | 6 ++++--
 Firebase/Messaging/FIRMessagingPubSub.h | 9 +++++++++
 Firebase/Messaging/FIRMessagingPubSub.m | 8 ++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Firebase/Messaging/FIRMessaging.m b/Firebase/Messaging/FIRMessaging.m
index a97374b9764..efafcc2457a 100644
--- a/Firebase/Messaging/FIRMessaging.m
+++ b/Firebase/Messaging/FIRMessaging.m
@@ -700,7 +700,8 @@ - (void)subscribeToTopic:(NSString *)topic
     if ([FIRMessagingPubSub hasTopicsPrefix:topic]) {
       FIRMessagingLoggerWarn(kFIRMessagingMessageCodeTopicFormatIsDeprecated,
                              @"Format '%@' is deprecated. Only '%@' should be used in "
-                             @"subscribeToTopic.", topic, normalizeTopic);
+                             @"subscribeToTopic.", topic,
+                             [FIRMessagingPubSub removePrefixFromTopic:topic]);
     }
     if (normalizeTopic.length) {
       [self.pubsub subscribeToTopic:normalizeTopic handler:completion];
@@ -726,7 +727,8 @@ - (void)unsubscribeFromTopic:(NSString *)topic
     if ([FIRMessagingPubSub hasTopicsPrefix:topic]) {
       FIRMessagingLoggerWarn(kFIRMessagingMessageCodeTopicFormatIsDeprecated,
                              @"Format '%@' is deprecated. Only '%@' should be used in "
-                             @"unsubscribeFromTopic.", topic, normalizeTopic);
+                             @"unsubscribeFromTopic.", topic,
+                             [FIRMessagingPubSub removePrefixFromTopic:topic]);
     }
     if (normalizeTopic.length) {
       [self.pubsub unsubscribeFromTopic:normalizeTopic handler:completion];
diff --git a/Firebase/Messaging/FIRMessagingPubSub.h b/Firebase/Messaging/FIRMessagingPubSub.h
index 1c615d18d48..5eb2e8934c4 100644
--- a/Firebase/Messaging/FIRMessagingPubSub.h
+++ b/Firebase/Messaging/FIRMessagingPubSub.h
@@ -138,6 +138,15 @@ NS_ASSUME_NONNULL_BEGIN
  */
 + (NSString *)addPrefixToTopic:(NSString *)topic;
 
+/**
+ *  Removes the "/topics/" prefix from the topic.
+ *
+ *  @param topic The topic to remove the prefix from.
+ *
+ *  @return The new topic name with the "/topics/" prefix removed.
+ */
++ (NSString *)removePrefixFromTopic:(NSString *)topic;
+
 /**
  *  Check if the topic name has "/topics/" prefix.
  *
diff --git a/Firebase/Messaging/FIRMessagingPubSub.m b/Firebase/Messaging/FIRMessagingPubSub.m
index 09491b439bd..3f954e8ab5e 100644
--- a/Firebase/Messaging/FIRMessagingPubSub.m
+++ b/Firebase/Messaging/FIRMessagingPubSub.m
@@ -231,6 +231,14 @@ + (NSString *)addPrefixToTopic:(NSString *)topic {
   }
 }
 
++ (NSString *)removePrefixFromTopic:(NSString *)topic {
+  if ([self hasTopicsPrefix:topic]) {
+    return [topic substringFromIndex:kTopicsPrefix.length];
+  } else {
+    return [topic copy];
+  }
+}
+
 + (BOOL)hasTopicsPrefix:(NSString *)topic {
   return [topic hasPrefix:kTopicsPrefix];
 }

From a82f4ef15e0fa9e723f3c4e519067e8f3e38c796 Mon Sep 17 00:00:00 2001
From: Chen Liang <chliang@google.com>
Date: Tue, 19 Jun 2018 13:06:35 -0700
Subject: [PATCH 3/6] add unit test

---
 Example/Messaging/Tests/FIRMessagingPubSubTest.m | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Example/Messaging/Tests/FIRMessagingPubSubTest.m b/Example/Messaging/Tests/FIRMessagingPubSubTest.m
index 3af1402bba5..e1260f5cc23 100644
--- a/Example/Messaging/Tests/FIRMessagingPubSubTest.m
+++ b/Example/Messaging/Tests/FIRMessagingPubSubTest.m
@@ -78,4 +78,13 @@ - (void)testAddTopicPrefix_withPrefix {
   XCTAssertTrue([FIRMessagingPubSub isValidTopicWithPrefix:topic]);
 }
 
+- (void)testRemoveTopicPrefix {
+    NSString *topic = [NSString stringWithFormat:@"/topics/%@", kTopicName];
+    topic = [FIRMessagingPubSub removePrefixFromTopic:topic];
+    XCTAssertEqualObjects(topic, kTopicName);
+    // if the topic doesn't have the prefix, should return topic itself.
+    topic = [FIRMessagingPubSub removePrefixFromTopic:kTopicName];
+    XCTAssertEqualObjects(topic, kTopicName);
+}
+
 @end

From 2cc3b2ba2894830819ad05abb3184fd7eece7f99 Mon Sep 17 00:00:00 2001
From: Chen Liang <chliang@google.com>
Date: Tue, 19 Jun 2018 15:00:48 -0700
Subject: [PATCH 4/6] update this PR to only add a warning message when token
 is not available, because topic operation will eventually resume when token
 is back

---
 .../Messaging/Tests/FIRMessagingServiceTest.m | 58 -------------------
 Firebase/Messaging/FIRMessaging.m             |  6 +-
 2 files changed, 3 insertions(+), 61 deletions(-)

diff --git a/Example/Messaging/Tests/FIRMessagingServiceTest.m b/Example/Messaging/Tests/FIRMessagingServiceTest.m
index 4983b58e089..afbae465dbc 100644
--- a/Example/Messaging/Tests/FIRMessagingServiceTest.m
+++ b/Example/Messaging/Tests/FIRMessagingServiceTest.m
@@ -239,35 +239,6 @@ - (void)testUnsubscribeWithTopicPrefix {
   OCMVerifyAll(_mockPubSub);
 }
 
-- (void)testSubScriptionCompletionHandlerWithInvalidToken {
-  XCTestExpectation *subscriptionCompletionExpectation =
-      [self expectationWithDescription:@"Subscription is complete"];
-  _messaging.defaultFcmToken = nil;
-  [_messaging subscribeToTopic:@"news"
-                    completion:^(NSError *error) {
-                      XCTAssertNotNil(error);
-                      XCTAssertEqual(error.code, FIRMessagingErrorTokenNotAvailable);
-                      [subscriptionCompletionExpectation fulfill];
-                    }];
-  [self waitForExpectationsWithTimeout:0.2
-                               handler:^(NSError *_Nullable error){
-                               }];
-}
-
-- (void)testSubscriptionCompletionHandlerWithInvalidTopicName {
-  XCTestExpectation *subscriptionCompletionExpectation =
-      [self expectationWithDescription:@"Subscription is complete"];
-  [_messaging subscribeToTopic:@"!@#$%^&*()"
-                    completion:^(NSError *_Nullable error) {
-                      XCTAssertNotNil(error);
-                      XCTAssertEqual(error.code, FIRMessagingErrorInvalidTopicName);
-                      [subscriptionCompletionExpectation fulfill];
-                    }];
-  [self waitForExpectationsWithTimeout:0.2
-                               handler:^(NSError *_Nullable error){
-                               }];
-}
-
 - (void)testSubscriptionCompletionHandlerWithSuccess {
   OCMStub([_mockPubSub subscribeToTopic:[OCMArg any]
                                 handler:([OCMArg invokeBlockWithArgs:[NSNull null], nil])]);
@@ -283,35 +254,6 @@ - (void)testSubscriptionCompletionHandlerWithSuccess {
                                }];
 }
 
-- (void)testUnsubscribeCompletionHandlerWithInvalidToken {
-  XCTestExpectation *unsubscriptionCompletionExpectation =
-      [self expectationWithDescription:@"Unsubscription is complete"];
-  _messaging.defaultFcmToken = nil;
-  [_messaging unsubscribeFromTopic:@""
-                        completion:^(NSError *error) {
-                          XCTAssertNotNil(error);
-                          XCTAssertEqual(error.code, FIRMessagingErrorTokenNotAvailable);
-                          [unsubscriptionCompletionExpectation fulfill];
-                        }];
-  [self waitForExpectationsWithTimeout:0.2
-                               handler:^(NSError *_Nullable error){
-                               }];
-}
-
-- (void)testUnsubscribeCompletionHandlerWithInvalidTopic {
-  XCTestExpectation *unsubscriptionCompletionExpectation =
-      [self expectationWithDescription:@"Unsubscription is complete"];
-  [_messaging unsubscribeFromTopic:@"!@#$%^&*()"
-                        completion:^(NSError *error) {
-                          XCTAssertNotNil(error);
-                          XCTAssertEqual(error.code, FIRMessagingErrorInvalidTopicName);
-                          [unsubscriptionCompletionExpectation fulfill];
-                        }];
-  [self waitForExpectationsWithTimeout:0.2
-                               handler:^(NSError *_Nullable error){
-                               }];
-}
-
 - (void)testUnsubscribeCompletionHandlerWithSuccess {
   OCMStub([_mockPubSub unsubscribeFromTopic:[OCMArg any]
                                     handler:([OCMArg invokeBlockWithArgs:[NSNull null], nil])]);
diff --git a/Firebase/Messaging/FIRMessaging.m b/Firebase/Messaging/FIRMessaging.m
index d8ea587be3f..771d44dccec 100644
--- a/Firebase/Messaging/FIRMessaging.m
+++ b/Firebase/Messaging/FIRMessaging.m
@@ -706,7 +706,7 @@ - (void)subscribeToTopic:(NSString *)topic
                            topic, [FIRMessagingPubSub removePrefixFromTopic:topic]);
   }
   if (!self.defaultFcmToken.length) {
-    FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging010,
+    FIRMessagingLoggerWarn(kFIRMessagingMessageCodeMessaging010,
                             @"The subscription operation is suspended because you don't have a "
                             @"token. The operation will resume once you get an FCM token.");
   }
@@ -725,14 +725,14 @@ - (void)unsubscribeFromTopic:(NSString *)topic {
 
 - (void)unsubscribeFromTopic:(NSString *)topic
                   completion:(nullable FIRMessagingTopicOperationCompletion)completion {
- if ([FIRMessagingPubSub hasTopicsPrefix:topic]) {
+  if ([FIRMessagingPubSub hasTopicsPrefix:topic]) {
     FIRMessagingLoggerWarn(kFIRMessagingMessageCodeTopicFormatIsDeprecated,
                            @"Format '%@' is deprecated. Only '%@' should be used in "
                            @"unsubscribeFromTopic.",
                            topic, [FIRMessagingPubSub removePrefixFromTopic:topic]);
   }
   if (!self.defaultFcmToken.length) {
-    FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging012,
+    FIRMessagingLoggerWarn(kFIRMessagingMessageCodeMessaging012,
                             @"The unsubscription operation is suspended because you don't have a "
                             @"token. The operation will resume once you get an FCM token.");
   }

From 5b75a76c26b15e38b5610314f2de8c6c54ad896f Mon Sep 17 00:00:00 2001
From: Chen Liang <chliang@google.com>
Date: Wed, 26 Sep 2018 15:08:47 -0700
Subject: [PATCH 5/6] Fix the issue that when topic is invalid, the callback
 was not triggered.

---
 .../Messaging/Tests/FIRMessagingServiceTest.m | 28 +++++++++++++++++++
 Firebase/Messaging/FIRMessaging.m             |  6 ++++
 Firebase/Messaging/Public/FIRMessaging.h      |  3 ++
 3 files changed, 37 insertions(+)

diff --git a/Example/Messaging/Tests/FIRMessagingServiceTest.m b/Example/Messaging/Tests/FIRMessagingServiceTest.m
index afbae465dbc..1924a7ffc1a 100644
--- a/Example/Messaging/Tests/FIRMessagingServiceTest.m
+++ b/Example/Messaging/Tests/FIRMessagingServiceTest.m
@@ -269,6 +269,34 @@ - (void)testUnsubscribeCompletionHandlerWithSuccess {
                                }];
 }
 
+- (void)testSubscriptionCompletionHandlerWithInvalidTopicName {
+  XCTestExpectation *subscriptionCompletionExpectation =
+      [self expectationWithDescription:@"Subscription is complete"];
+  [_messaging subscribeToTopic:@"!@#$%^&*()"
+                    completion:^(NSError *_Nullable error) {
+                      XCTAssertNotNil(error);
+                      XCTAssertEqual(error.code, FIRMessagingErrorInvalidTopicName);
+                      [subscriptionCompletionExpectation fulfill];
+                    }];
+  [self waitForExpectationsWithTimeout:0.2
+                               handler:^(NSError *_Nullable error){
+                               }];
+}
+
+- (void)testUnsubscribeCompletionHandlerWithInvalidTopicName {
+  XCTestExpectation *unsubscriptionCompletionExpectation =
+      [self expectationWithDescription:@"Unsubscription is complete"];
+  [_messaging unsubscribeFromTopic:@"!@#$%^&*()"
+                        completion:^(NSError *error) {
+                          XCTAssertNotNil(error);
+                          XCTAssertEqual(error.code, FIRMessagingErrorInvalidTopicName);
+                          [unsubscriptionCompletionExpectation fulfill];
+                        }];
+  [self waitForExpectationsWithTimeout:0.2
+                               handler:^(NSError *_Nullable error){
+                               }];
+}
+
 - (void)testFIRMessagingSDKVersionInFIRMessagingService {
   Class versionClass = NSClassFromString(kFIRMessagingSDKClassString);
   SEL versionSelector = NSSelectorFromString(kFIRMessagingSDKVersionSelectorString);
diff --git a/Firebase/Messaging/FIRMessaging.m b/Firebase/Messaging/FIRMessaging.m
index d158e51d3c5..186afdf3359 100644
--- a/Firebase/Messaging/FIRMessaging.m
+++ b/Firebase/Messaging/FIRMessaging.m
@@ -760,6 +760,9 @@ - (void)subscribeToTopic:(NSString *)topic
   }
   FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging009,
                           @"Cannot parse topic name %@. Will not subscribe.", topic);
+  if (completion) {
+    completion([NSError fcm_errorWithCode:FIRMessagingErrorInvalidTopicName userInfo:nil]);
+  }
 }
 
 - (void)unsubscribeFromTopic:(NSString *)topic {
@@ -786,6 +789,9 @@ - (void)unsubscribeFromTopic:(NSString *)topic
   }
   FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging011,
                           @"Cannot parse topic name %@. Will not unsubscribe.", topic);
+  if (completion) {
+    completion([NSError fcm_errorWithCode:FIRMessagingErrorInvalidTopicName userInfo:nil]);
+  }
 }
 
 #pragma mark - Send
diff --git a/Firebase/Messaging/Public/FIRMessaging.h b/Firebase/Messaging/Public/FIRMessaging.h
index acf79fa2712..a5b82a9308e 100644
--- a/Firebase/Messaging/Public/FIRMessaging.h
+++ b/Firebase/Messaging/Public/FIRMessaging.h
@@ -189,6 +189,9 @@ typedef NS_ENUM(NSUInteger, FIRMessagingError) {
 
   /// Some parameters of the request were invalid.
   FIRMessagingErrorInvalidRequest = 7,
+
+  /// Topic name is invalid for subscription/unsubscription.
+  FIRMessagingErrorInvalidTopicName = 8,
 } NS_SWIFT_NAME(MessagingError);
 
 /// Status for the downstream message received by the app.

From 03828dec5491499d9b137fe12fdc964fec2caa0c Mon Sep 17 00:00:00 2001
From: Chen Liang <chliang@google.com>
Date: Wed, 26 Sep 2018 15:20:47 -0700
Subject: [PATCH 6/6] fix merge errors

---
 Firebase/Messaging/FIRMessaging.m        | 1 -
 Firebase/Messaging/Public/FIRMessaging.h | 2 --
 2 files changed, 3 deletions(-)

diff --git a/Firebase/Messaging/FIRMessaging.m b/Firebase/Messaging/FIRMessaging.m
index 3528cec5788..186afdf3359 100644
--- a/Firebase/Messaging/FIRMessaging.m
+++ b/Firebase/Messaging/FIRMessaging.m
@@ -523,7 +523,6 @@ - (void)setAutoInitEnabled:(BOOL)autoInitEnabled {
     self.defaultFcmToken = self.instanceID.token;
 #pragma clang diagnostic pop
   }
-  [_messagingUserDefaults synchronize];
 }
 
 - (NSString *)FCMToken {
diff --git a/Firebase/Messaging/Public/FIRMessaging.h b/Firebase/Messaging/Public/FIRMessaging.h
index 5e6d9b4692e..c498d1f75ab 100644
--- a/Firebase/Messaging/Public/FIRMessaging.h
+++ b/Firebase/Messaging/Public/FIRMessaging.h
@@ -193,8 +193,6 @@ typedef NS_ENUM(NSUInteger, FIRMessagingError) {
   /// Topic name is invalid for subscription/unsubscription.
   FIRMessagingErrorInvalidTopicName = 8,
 
-  /// Topic name is invalid for subscription/unsubscription.
-  FIRMessagingErrorInvalidTopicName = 9,
 } NS_SWIFT_NAME(MessagingError);
 
 /// Status for the downstream message received by the app.