Address review feedback

Author: paulb777

Example/Core/Tests/FIRAppTest.m

@@ -778,23 +778,23 @@ - (void)testIsDefaultAppConfigured {
 }
 
 - (void)testIllegalLibraryName {
-  [FIRApp registerLibrary:nil withName:@"Oops>" withVersion:@"1.0.0"];
+  [FIRApp registerLibrary:@"Oops>" withVersion:@"1.0.0"];
   XCTAssertTrue([[FIRApp firebaseUserAgent] isEqualToString:@""]);
 }
 
 - (void)testIllegalLibraryVersion {
-  [FIRApp registerLibrary:nil withName:@"LegalName" withVersion:@"1.0.0+"];
+  [FIRApp registerLibrary:@"LegalName" withVersion:@"1.0.0+"];
   XCTAssertTrue([[FIRApp firebaseUserAgent] isEqualToString:@""]);
 }
 
 - (void)testSingleLibrary {
-  [FIRApp registerLibrary:nil withName:@"LegalName" withVersion:@"1.0.0"];
+  [FIRApp registerLibrary:@"LegalName" withVersion:@"1.0.0"];
   XCTAssertTrue([[FIRApp firebaseUserAgent] containsString:@"LegalName/1.0.0"]);
 }
 
 - (void)testMultipleLibraries {
-  [FIRApp registerLibrary:nil withName:@"LegalName" withVersion:@"1.0.0"];
-  [FIRApp registerLibrary:nil withName:@"LegalName2" withVersion:@"2.0.0"];
+  [FIRApp registerLibrary:@"LegalName" withVersion:@"1.0.0"];
+  [FIRApp registerLibrary:@"LegalName2" withVersion:@"2.0.0"];
   XCTAssertTrue([[FIRApp firebaseUserAgent] containsString:@"LegalName/1.0.0 LegalName2/2.0.0"]);
 }
 

Firebase/Auth/Source/FIRAuth.m

@@ -311,10 +311,9 @@ @implementation FIRAuth {
 }
 
 + (void)load {
-  [FIRApp registerLibrary:(id<FIRLibrary>)self
-                 withName:@"auth"
+  [FIRApp registerInternalLibrary:(Class<FIRLibrary>)self
+                 withName:@"fire-auth"
               withVersion:[NSString stringWithUTF8String:FirebaseAuthVersionStr]];
-  [FIRApp registerAsConfigurable:self];
 }
 
 + (void)initialize {

Firebase/Core/FIRApp.m

@@ -17,7 +17,7 @@
 #import "Private/FIRAnalyticsConfiguration+Internal.h"
 #import "Private/FIRAppInternal.h"
 #import "Private/FIRBundleUtil.h"
-#import "Private/FIRComponentContainerInternal.h"
+#import "FIRComponentContainerInternal.h"
 #import "Private/FIRLibrary.h"
 #import "Private/FIRLogger.h"
 #import "Private/FIROptionsInternal.h"
@@ -81,7 +81,7 @@
  * An array of all classes that registered as `FIRCoreConfigurable` in order to receive lifecycle
  * events from Core.
  */
-static NSMutableArray<Class<FIRLibrary>> *gRegisteredAsConfigurable;
+static NSMutableArray<Class<FIRLibrary>> *sRegisteredAsConfigurable;
 
 @interface FIRApp ()
 
@@ -431,7 +431,7 @@ + (void)sendNotificationsToSDKs:(FIRApp *)app {
 
   // This is the new way of sending information to SDKs.
   // TODO: Do we want this on a background thread, maybe?
-  for (Class<FIRLibrary> library in gRegisteredAsConfigurable) {
+  for (Class<FIRLibrary> library in sRegisteredAsConfigurable) {
     [library configureWithApp:app];
   }
 }
@@ -471,28 +471,11 @@ + (NSError *)errorForInvalidAppID {
                          userInfo:errorDict];
 }
 
-+ (void)registerAsConfigurable:(Class<FIRLibrary>)klass {
-  // This is called at +load time, keep the work to a minimum.
-  static dispatch_once_t onceToken;
-  dispatch_once(&onceToken, ^{
-    gRegisteredAsConfigurable = [[NSMutableArray alloc] initWithCapacity:1];
-  });
-  NSAssert([(Class)klass conformsToProtocol:@protocol(FIRLibrary)] &&
-               [(Class)klass respondsToSelector:@selector(configureWithApp:)],
-           @"The class being registered (%@) must conform to `FIRLibrary`.", klass);
-  [gRegisteredAsConfigurable addObject:klass];
-}
-
 + (BOOL)isDefaultAppConfigured {
   return (sDefaultApp != nil);
 }
 
-+ (void)registerLibrary:(nullable id<FIRLibrary>)library
-               withName:(nonnull NSString *)name
-            withVersion:(nonnull NSString *)version {
-  if (library) {
-    [FIRComponentContainer registerAsComponentRegistrant:library];
-  }
++ (void)registerLibrary:(nonnull NSString *)name withVersion:(nonnull NSString *)version {
   // Create the set of characters which aren't allowed, only if this feature is used.
   NSMutableCharacterSet *allowedSet = [NSMutableCharacterSet alphanumericCharacterSet];
   [allowedSet addCharactersInString:@"-_."];
@@ -513,6 +496,21 @@ + (void)registerLibrary:(nullable id<FIRLibrary>)library
   }
 }
 
++ (void)registerInternalLibrary:(nonnull Class<FIRLibrary>)library
+                       withName:(nonnull NSString *)name
+                    withVersion:(nonnull NSString *)version {
+  // This is called at +load time, keep the work to a minimum.
+  [FIRComponentContainer registerAsComponentRegistrant:library];
+  if ([(Class)library respondsToSelector:@selector(configureWithApp:)]) {
+    static dispatch_once_t onceToken;
+    dispatch_once(&onceToken, ^{
+      sRegisteredAsConfigurable = [[NSMutableArray alloc] init];
+    });
+    [sRegisteredAsConfigurable addObject:library];
+  }
+  [self registerLibrary:name withVersion:version];
+}
+
 + (NSString *)firebaseUserAgent {
   NSMutableArray<NSString *> *libraries =
       [[NSMutableArray<NSString *> alloc] initWithCapacity:sLibraryVersions.count];

Firebase/Core/FIRComponentContainer.m

@@ -37,17 +37,17 @@ @interface FIRComponentContainer ()
 @implementation FIRComponentContainer
 
 // Collection of all classes that register to provide components.
-static NSMutableSet<Class> *gFIRComponentRegistrants;
+static NSMutableSet<Class> *sFIRComponentRegistrants;
 
 #pragma mark - Public Registration
 
-+ (void)registerAsComponentRegistrant:(id<FIRLibrary>)klass {
++ (void)registerAsComponentRegistrant:(Class<FIRLibrary>)klass {
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
-    gFIRComponentRegistrants = [[NSMutableSet<Class> alloc] init];
+    sFIRComponentRegistrants = [[NSMutableSet<Class> alloc] init];
   });
 
-  [self registerAsComponentRegistrant:(Class)klass inSet:gFIRComponentRegistrants];
+  [self registerAsComponentRegistrant:klass inSet:sFIRComponentRegistrants];
 }
 
 + (void)registerAsComponentRegistrant:(Class)klass inSet:(NSMutableSet<Class> *)allRegistrants {
@@ -59,7 +59,8 @@ + (void)registerAsComponentRegistrant:(Class)klass inSet:(NSMutableSet<Class> *)
   }
 
   // Ensure the class given conforms to the proper protocol.
-  if (![klass conformsToProtocol:@protocol(FIRLibrary)] ||
+  // TODO restore protocol check after new code propagates internally
+  if (  //![klass conformsToProtocol:@protocol(FIRLibrary)] ||
       ![klass respondsToSelector:@selector(componentsToRegister)]) {
     [NSException raise:NSInvalidArgumentException
                 format:
@@ -74,7 +75,7 @@ + (void)registerAsComponentRegistrant:(Class)klass inSet:(NSMutableSet<Class> *)
 #pragma mark - Internal Initialization
 
 - (instancetype)initWithApp:(FIRApp *)app {
-  return [self initWithApp:app registrants:gFIRComponentRegistrants];
+  return [self initWithApp:app registrants:sFIRComponentRegistrants];
 }
 
 - (instancetype)initWithApp:(FIRApp *)app registrants:(NSMutableSet<Class> *)allRegistrants {

Firebase/Core/FIRComponentType.m

@@ -16,7 +16,7 @@
 
 #import "Private/FIRComponentType.h"
 
-#import "Private/FIRComponentContainerInternal.h"
+#import "FIRComponentContainerInternal.h"
 
 @implementation FIRComponentType
 

Firebase/Core/FIROptions.m

@@ -114,8 +114,7 @@ + (void)initialize {
   NSRange minor = NSMakeRange(1, 2);
   NSRange patch = NSMakeRange(3, 2);
   [FIRApp
-      registerLibrary:nil
-             withName:@"fire-ios"
+      registerLibrary:@"fire-ios"
           withVersion:[NSString stringWithFormat:@"%@.%d.%d",
                                                  [kFIRLibraryVersionID substringWithRange:major],
                                                  [[kFIRLibraryVersionID substringWithRange:minor]
@@ -126,10 +125,10 @@ + (void)initialize {
   NSString *xcodeVersion = info[@"DTXcodeBuild"];
   NSString *sdkVersion = info[@"DTSDKBuild"];
   if (xcodeVersion) {
-    [FIRApp registerLibrary:nil withName:@"xcode" withVersion:xcodeVersion];
+    [FIRApp registerLibrary:@"xcode" withVersion:xcodeVersion];
   }
   if (sdkVersion) {
-    [FIRApp registerLibrary:nil withName:@"apple-sdk" withVersion:sdkVersion];
+    [FIRApp registerLibrary:@"apple-sdk" withVersion:sdkVersion];
   }
 }
 

Firebase/Core/Private/FIRAppInternal.h

@@ -164,23 +164,25 @@ typedef NSString *_Nullable (^FIRAppGetUIDImplementation)(void);
 + (BOOL)isDefaultAppConfigured;
 
 /**
- * Register a class that conforms to `FIRCoreConfigurable`. Each SDK should have one class that
- * registers in order to provide critical information for interoperability and lifecycle events.
- * TODO(wilsonryan): Write more documentation.
+ * Registers a given third-party library with the given version number to be reported for
+ * analytics.
+ *
+ * @param name Name of the library.
+ * @param version Version of the library.
  */
-+ (void)registerAsConfigurable:(Class<FIRLibrary>)klass;
++ (void)registerLibrary:(nonnull NSString *)name withVersion:(nonnull NSString *)version;
 
 /**
- * Registers a given third-party library with the given version number to be reported for
+ * Registers a given internal library with the given version number to be reported for
  * analytics.
  *
  * @param library Optional parameter for component registration.
- * @param name Name of the library. It's typically the CocoaPod name.
+ * @param name Name of the library.
  * @param version Version of the library.
  */
-+ (void)registerLibrary:(nullable id<FIRLibrary>)library
-               withName:(nonnull NSString *)name
-            withVersion:(nonnull NSString *)version;
++ (void)registerInternalLibrary:(nonnull Class<FIRLibrary>)library
+                       withName:(nonnull NSString *)name
+                    withVersion:(nonnull NSString *)version;
 
 /**
  * A concatenated string representing all the third-party libraries and version numbers.

Firebase/Core/Private/FIRComponentContainer.h

@@ -39,10 +39,6 @@ NS_SWIFT_NAME(FirebaseComponentContainer)
 /// Unavailable. Use the `container` property on `FIRApp`.
 - (instancetype)init NS_UNAVAILABLE;
 
-/// Register a class to provide components for the interoperability system. The class should conform
-/// to `FIRComponentRegistrant` and provide an array of `FIRComponent` objects.
-+ (void)registerAsComponentRegistrant:(id<FIRLibrary>)klass;
-
 @end
 
 NS_ASSUME_NONNULL_END

Firebase/Core/Private/FIRComponentContainerInternal.h

@@ -34,6 +34,10 @@ NS_ASSUME_NONNULL_BEGIN
 /// Remove all of the cached instances stored and allow them to clean up after themselves.
 - (void)removeAllCachedInstances;
 
+/// Register a class to provide components for the interoperability system. The class should conform
+/// to `FIRComponentRegistrant` and provide an array of `FIRComponent` objects.
++ (void)registerAsComponentRegistrant:(Class<FIRLibrary>)klass;
+
 @end
 
 NS_ASSUME_NONNULL_END

Firebase/Core/Private/FIRLibrary.h

@@ -28,14 +28,13 @@ NS_ASSUME_NONNULL_BEGIN
 NS_SWIFT_NAME(Library)
 @protocol FIRLibrary
 
-//@optional
 /// Returns one or more FIRComponents that will be registered in
 /// FIRApp and participate in dependency resolution and injection.
 + (NSArray<FIRComponent *> *)componentsToRegister;
 
 @optional
-/// Configure the SDK if needed ahead of time. This method is called when the developer calls
-/// `FirebaseApp.configure()`.
+/// Implement this method if the library needs notifications for lifecycle events. This method is
+/// called when the developer calls `FirebaseApp.configure()`.
 + (void)configureWithApp:(FIRApp *)app;
 
 @end

Firebase/Database/Api/FIRDatabaseComponent.m

@@ -55,7 +55,9 @@ - (instancetype)initWithApp:(FIRApp *)app {
 #pragma mark - Lifecycle
 
 + (void)load {
-  [FIRApp registerLibrary:(id<FIRLibrary>)self withName:@"db" withVersion:[FIRDatabase sdkVersion]];
+  [FIRApp registerInternalLibrary:(Class<FIRLibrary>)self
+                         withName:@"fire-db"
+                      withVersion:[FIRDatabase sdkVersion]];
 }
 
 #pragma mark - FIRComponentRegistrant

Firebase/DynamicLinks/FIRDynamicLinks.m

@@ -115,7 +115,7 @@ @implementation FIRDynamicLinks {
 #ifdef FIRDynamicLinks3P
 
 + (void)load {
-  [FIRApp registerLibrary:(id<FIRLibrary>)self withName:@"dynlinks" withVersion:kFIRDLVersion];
+  [FIRApp registerInternalLibrary:self withName:@"fire-dynlinks" withVersion:kFIRDLVersion];
 }
 
 + (nonnull NSArray<FIRComponent *> *)componentsToRegister {

Firebase/Messaging/FIRMessaging.m

@@ -212,10 +212,9 @@ - (void)dealloc {
 #pragma mark - Config
 
 + (void)load {
-  [FIRApp registerLibrary:(id<FIRLibrary>)self
-                 withName:@"msg"
+  [FIRApp registerInternalLibrary:(Class<FIRLibrary>)self
+                 withName:@"fire-msg"
               withVersion:FIRMessagingCurrentLibraryVersion()];
-  [FIRApp registerAsConfigurable:self];
 }
 
 + (nonnull NSArray<FIRComponent *> *)componentsToRegister {

Firebase/Storage/FIRStorageComponent.m

@@ -52,9 +52,9 @@ - (instancetype)initWithApp:(FIRApp *)app {
 #pragma mark - Lifecycle
 
 + (void)load {
-  [FIRApp registerLibrary:(id<FIRLibrary>)self
-                 withName:@"stor"
-              withVersion:[NSString stringWithUTF8String:FIRStorageVersionString]];
+  [FIRApp registerInternalLibrary:(Class<FIRLibrary>)self
+                         withName:@"fire-storage"
+                      withVersion:[NSString stringWithUTF8String:FIRStorageVersionString]];
 }
 
 #pragma mark - FIRComponentRegistrant