Address review feedback

Author: paulb777

Example/Auth/Tests/FIRAuthTests.m

@@ -21,7 +21,7 @@
 #import <FirebaseAuthInterop/FIRAuthInterop.h>
 #import <FirebaseCore/FIRAppInternal.h>
 #import <FirebaseCore/FIRComponent.h>
-#import <FirebaseCore/FIRComponentRegistrant.h>
+#import <FirebaseCore/FIRLibrary.h>
 
 #import "FIRAdditionalUserInfo.h"
 #import "FIRAuth_Internal.h"
@@ -225,7 +225,7 @@
 static const NSTimeInterval kWaitInterval = .5;
 
 /** Category for FIRAuth to expose FIRComponentRegistrant conformance. */
-@interface FIRAuth () <FIRComponentRegistrant>
+@interface FIRAuth () <FIRLibrary>
 @end
 
 /** @class FIRAuthTests

Example/Core/Tests/FIRAppTest.m

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

Example/Core/Tests/FIRTestComponents.h

@@ -16,7 +16,7 @@
 
 #import <FirebaseCore/FIRComponent.h>
 #import <FirebaseCore/FIRComponentContainer.h>
-#import <FirebaseCore/FIRComponentRegistrant.h>
+#import <FirebaseCore/FIRLibrary.h>
 
 @protocol FIRComponentRegistrant;
 
@@ -28,13 +28,12 @@
 @end
 
 /// A test class that is a component registrant.
-@interface FIRTestClass
-    : NSObject <FIRTestProtocol, FIRComponentRegistrant, FIRComponentLifecycleMaintainer>
+@interface FIRTestClass : NSObject <FIRTestProtocol, FIRComponentLifecycleMaintainer, FIRLibrary>
 @end
 
 /// A test class that is a component registrant, a duplicate of FIRTestClass.
 @interface FIRTestClassDuplicate
-    : NSObject <FIRTestProtocol, FIRComponentRegistrant, FIRComponentLifecycleMaintainer>
+    : NSObject <FIRTestProtocol, FIRComponentLifecycleMaintainer, FIRLibrary>
 @end
 
 #pragma mark - Eager Component
@@ -47,7 +46,7 @@
 /// A test class that is a component registrant that provides a component requiring eager
 /// instantiation, and is cached for easier validation that it was instantiated.
 @interface FIRTestClassEagerCached
-    : NSObject <FIRTestProtocolEagerCached, FIRComponentRegistrant, FIRComponentLifecycleMaintainer>
+    : NSObject <FIRTestProtocol, FIRComponentLifecycleMaintainer, FIRLibrary>
 @end
 
 #pragma mark - Cached Component
@@ -59,5 +58,5 @@
 /// A test class that is a component registrant that provides a component that requests to be
 /// cached.
 @interface FIRTestClassCached
-    : NSObject <FIRTestProtocolCached, FIRComponentRegistrant, FIRComponentLifecycleMaintainer>
+    : NSObject <FIRTestProtocol, FIRComponentLifecycleMaintainer, FIRLibrary>
 @end

Example/Core/Tests/FIRTestComponents.m

@@ -47,7 +47,7 @@ @implementation FIRTestClassDuplicate
 - (void)doSomething {
 }
 
-/// FIRComponentRegistrant conformance.
+/// FIRLibrary conformance.
 + (nonnull NSArray<FIRComponent *> *)componentsToRegister {
   FIRComponent *testComponent =
       [FIRComponent componentWithProtocol:@protocol(FIRTestProtocol)
@@ -72,7 +72,7 @@ @implementation FIRTestClassEagerCached
 - (void)doSomethingFaster {
 }
 
-/// FIRComponentRegistrant conformance.
+/// FIRLibrary conformance.
 + (nonnull NSArray<FIRComponent *> *)componentsToRegister {
   FIRComponent *testComponent = [FIRComponent
       componentWithProtocol:@protocol(FIRTestProtocolEagerCached)
@@ -92,13 +92,16 @@ - (void)doSomethingFaster {
 - (void)appWillBeDeleted:(FIRApp *)app {
 }
 
+- (void)doSomething {
+}
+
 @end
 
 #pragma mark - Cached Component
 
 @implementation FIRTestClassCached
 
-/// FIRComponentRegistrant conformance.
+/// FIRLibrary conformance.
 + (nonnull NSArray<FIRComponent *> *)componentsToRegister {
   FIRComponent *testComponent = [FIRComponent
       componentWithProtocol:@protocol(FIRTestProtocolCached)
@@ -115,4 +118,7 @@ @implementation FIRTestClassCached
 - (void)appWillBeDeleted:(FIRApp *)app {
 }
 
+- (void)doSomething {
+}
+
 @end

Example/Storage/Tests/Unit/FIRStorageComponentTests.m

@@ -19,15 +19,15 @@
 
 #import <FirebaseCore/FIRAppInternal.h>
 #import <FirebaseCore/FIRComponent.h>
-#import <FirebaseCore/FIRComponentRegistrant.h>
+#import <FirebaseCore/FIRLibrary.h>
 #import <FirebaseCore/FIROptions.h>
 
 #import "FIRComponentTestUtilities.h"
 #import "FIRStorageComponent.h"
 
 // Make FIRComponentRegistrant conformance visible to the tests and expose the initializer.
-@interface FIRStorageComponent () <FIRComponentRegistrant>
-/// Internal intializer.
+@interface FIRStorageComponent () <FIRLibrary>
+/// Internal initializer.
 - (instancetype)initWithApp:(FIRApp *)app;
 @end
 

Firebase/Auth/Source/FIRAuth.m

@@ -27,8 +27,7 @@
 #import <FirebaseCore/FIRAppInternal.h>
 #import <FirebaseCore/FIRComponent.h>
 #import <FirebaseCore/FIRComponentContainer.h>
-#import <FirebaseCore/FIRComponentRegistrant.h>
-#import <FirebaseCore/FIRCoreConfigurable.h>
+#import <FirebaseCore/FIRLibrary.h>
 #import <FirebaseCore/FIRLogger.h>
 #import <FirebaseCore/FIROptions.h>
 #import <GoogleUtilities/GULAppEnvironmentUtil.h>
@@ -226,9 +225,10 @@ + (FIRActionCodeOperation)actionCodeOperationForRequestType:(NSString *)requestT
 #pragma mark - FIRAuth
 
 #if TARGET_OS_IOS
-@interface FIRAuth () <FIRAuthAppDelegateHandler, FIRAuthInterop, FIRComponentRegistrant, FIRCoreConfigurable, FIRComponentLifecycleMaintainer>
+@interface FIRAuth () <FIRAuthAppDelegateHandler, FIRAuthInterop, FIRLibrary,
+                       FIRComponentLifecycleMaintainer>
 #else
-@interface FIRAuth () <FIRAuthInterop, FIRComponentRegistrant, FIRCoreConfigurable, FIRComponentLifecycleMaintainer>
+@interface FIRAuth () <FIRAuthInterop, FIRLibrary, FIRComponentLifecycleMaintainer>
 #endif
 
 /** @property firebaseAppId
@@ -308,8 +308,9 @@ @implementation FIRAuth {
 }
 
 + (void)load {
-  [FIRApp registerLibrary:@"FirebaseAuth" withVersion:[NSString stringWithUTF8String:FirebaseAuthVersionStr] withComponentRegistrant:self withConfigurable:self];
-  [FIRComponentContainer registerAsComponentRegistrant:self];
+  [FIRApp registerLibrary:(id<FIRLibrary>)self
+                 withName:@"auth"
+              withVersion:[NSString stringWithUTF8String:FirebaseAuthVersionStr]];
   [FIRApp registerAsConfigurable:self];
 }
 

Firebase/Core/FIRApp.m

@@ -14,15 +14,15 @@
 
 #include <sys/utsname.h>
 
-#import "FIRApp.h"
-#import "FIRConfiguration.h"
 #import "Private/FIRAnalyticsConfiguration+Internal.h"
 #import "Private/FIRAppInternal.h"
 #import "Private/FIRBundleUtil.h"
 #import "Private/FIRComponentContainerInternal.h"
-#import "Private/FIRCoreConfigurable.h"
+#import "Private/FIRLibrary.h"
 #import "Private/FIRLogger.h"
 #import "Private/FIROptionsInternal.h"
+#import "Public/FIRApp.h"
+#import "Public/FIRConfiguration.h"
 
 NSString *const kFIRServiceAdMob = @"AdMob";
 NSString *const kFIRServiceAuth = @"Auth";
@@ -81,7 +81,7 @@
  * An array of all classes that registered as `FIRCoreConfigurable` in order to receive lifecycle
  * events from Core.
  */
-static NSMutableArray<Class<FIRCoreConfigurable>> *gRegisteredAsConfigurable;
+static NSMutableArray<Class<FIRLibrary>> *gRegisteredAsConfigurable;
 
 @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<FIRCoreConfigurable> library in gRegisteredAsConfigurable) {
+  for (Class<FIRLibrary> library in gRegisteredAsConfigurable) {
     [library configureWithApp:app];
   }
 }
@@ -471,49 +471,45 @@ + (NSError *)errorForInvalidAppID {
                          userInfo:errorDict];
 }
 
-+ (void)registerAsConfigurable:(Class<FIRCoreConfigurable>)klass {
++ (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(FIRCoreConfigurable)],
-           @"The class being registered (%@) must conform to `FIRCoreConfigurable`.", klass);
+  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:(nonnull NSString *)library
-                withVersion:(nonnull NSString *)version
-    withComponentRegistrant:(nullable Class)componentRegistrant
-           withConfigurable:(nullable Class<FIRCoreConfigurable>)configurableRegistrant {
++ (void)registerLibrary:(nullable id<FIRLibrary>)library
+               withName:(nonnull NSString *)name
+            withVersion:(nonnull NSString *)version {
+  if (library) {
+    [FIRComponentContainer registerAsComponentRegistrant:library];
+  }
   // Create the set of characters which aren't allowed, only if this feature is used.
   NSMutableCharacterSet *allowedSet = [NSMutableCharacterSet alphanumericCharacterSet];
   [allowedSet addCharactersInString:@"-_."];
   NSCharacterSet *disallowedSet = [allowedSet invertedSet];
   // Make sure the library name and version strings do not contain unexpected characters, and
   // add the name/version pair to the dictionary.
-  if ([library rangeOfCharacterFromSet:disallowedSet].location == NSNotFound &&
+  if ([name rangeOfCharacterFromSet:disallowedSet].location == NSNotFound &&
       [version rangeOfCharacterFromSet:disallowedSet].location == NSNotFound) {
     if (!sLibraryVersions) {
       sLibraryVersions = [[NSMutableDictionary alloc] init];
     }
-    sLibraryVersions[library] = version;
+    sLibraryVersions[name] = version;
   } else {
     FIRLogError(kFIRLoggerCore, @"I-COR000027",
                 @"The library name (%@) or version number (%@) contain illegal characters. "
                 @"Only alphanumeric, dash, underscore and period characters are allowed.",
-                library, version);
-  }
-  if (configurableRegistrant) {
-    [FIRApp registerAsConfigurable:configurableRegistrant];
-  }
-  if (componentRegistrant) {
-    [FIRComponentContainer registerAsComponentRegistrant:componentRegistrant];
+                name, version);
   }
 }
 

Firebase/Core/FIRComponentContainer.m

@@ -18,7 +18,7 @@
 
 #import "Private/FIRAppInternal.h"
 #import "Private/FIRComponent.h"
-#import "Private/FIRComponentRegistrant.h"
+#import "Private/FIRLibrary.h"
 #import "Private/FIRLogger.h"
 
 NS_ASSUME_NONNULL_BEGIN
@@ -41,13 +41,13 @@ @implementation FIRComponentContainer
 
 #pragma mark - Public Registration
 
-+ (void)registerAsComponentRegistrant:(Class)klass {
++ (void)registerAsComponentRegistrant:(id<FIRLibrary>)klass {
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
     gFIRComponentRegistrants = [[NSMutableSet<Class> alloc] init];
   });
 
-  [self registerAsComponentRegistrant:klass inSet:gFIRComponentRegistrants];
+  [self registerAsComponentRegistrant:(Class)klass inSet:gFIRComponentRegistrants];
 }
 
 + (void)registerAsComponentRegistrant:(Class)klass inSet:(NSMutableSet<Class> *)allRegistrants {
@@ -59,7 +59,7 @@ + (void)registerAsComponentRegistrant:(Class)klass inSet:(NSMutableSet<Class> *)
   }
 
   // Ensure the class given conforms to the proper protocol.
-  if (![klass conformsToProtocol:@protocol(FIRComponentRegistrant)] ||
+  if (![klass conformsToProtocol:@protocol(FIRLibrary)] ||
       ![klass respondsToSelector:@selector(componentsToRegister)]) {
     [NSException raise:NSInvalidArgumentException
                 format:
@@ -91,7 +91,7 @@ - (instancetype)initWithApp:(FIRApp *)app registrants:(NSMutableSet<Class> *)all
 
 - (void)populateComponentsFromRegisteredClasses:(NSSet<Class> *)classes forApp:(FIRApp *)app {
   // Loop through the verified component registrants and populate the components array.
-  for (Class<FIRComponentRegistrant> klass in classes) {
+  for (Class<FIRLibrary> klass in classes) {
     // Loop through all the components being registered and store them as appropriate.
     // Classes which do not provide functionality should use a dummy FIRComponentRegistrant
     // protocol.

Firebase/Core/FIROptions.m

@@ -113,30 +113,23 @@ + (void)initialize {
   NSRange major = NSMakeRange(0, 1);
   NSRange minor = NSMakeRange(1, 2);
   NSRange patch = NSMakeRange(3, 2);
-  [FIRApp registerLibrary:@"fire-ios"
-                  withVersion:[NSString
-                                  stringWithFormat:@"%@.%d.%d",
-                                                   [kFIRLibraryVersionID substringWithRange:major],
-                                                   [[kFIRLibraryVersionID substringWithRange:minor]
-                                                       intValue],
-                                                   [[kFIRLibraryVersionID substringWithRange:patch]
-                                                       intValue]]
-      withComponentRegistrant:nil
-             withConfigurable:nil];
+  [FIRApp
+      registerLibrary:nil
+             withName:@"fire-ios"
+          withVersion:[NSString stringWithFormat:@"%@.%d.%d",
+                                                 [kFIRLibraryVersionID substringWithRange:major],
+                                                 [[kFIRLibraryVersionID substringWithRange:minor]
+                                                     intValue],
+                                                 [[kFIRLibraryVersionID substringWithRange:patch]
+                                                     intValue]]];
   NSDictionary<NSString *, id> *info = [[NSBundle mainBundle] infoDictionary];
   NSString *xcodeVersion = info[@"DTXcodeBuild"];
   NSString *sdkVersion = info[@"DTSDKBuild"];
   if (xcodeVersion) {
-    [FIRApp registerLibrary:@"xcode"
-                    withVersion:xcodeVersion
-        withComponentRegistrant:nil
-               withConfigurable:nil];
+    [FIRApp registerLibrary:nil withName:@"xcode" withVersion:xcodeVersion];
   }
   if (sdkVersion) {
-    [FIRApp registerLibrary:@"apple-sdk"
-                    withVersion:sdkVersion
-        withComponentRegistrant:nil
-               withConfigurable:nil];
+    [FIRApp registerLibrary:nil withName:@"apple-sdk" withVersion:sdkVersion];
   }
 }