review feedback and test adjustment

Author: paulb777

Example/Core/Tests/FIRComponentContainerTest.m

@@ -27,8 +27,26 @@ @interface FIRComponentContainer (TestInternal)
 @property(nonatomic, strong) NSMutableDictionary<NSString *, id> *cachedInstances;
 
 + (void)registerAsComponentRegistrant:(Class)klass inSet:(NSMutableSet<Class> *)allRegistrants;
-
 - (instancetype)initWithApp:(FIRApp *)app registrants:(NSMutableSet<Class> *)allRegistrants;
++ (void)setRegistrantSet:(NSMutableSet<Class> *)testInput;
++ (NSMutableSet<Class> *)registrantSet;
+@end
+
+@interface FIRComponentContainer (TestInternalImplementations)
+- (instancetype)initWithApp:(FIRApp *)app
+                 components:(NSDictionary<NSString *, FIRComponentCreationBlock> *)components;
+@end
+
+@implementation FIRComponentContainer (TestInternalImplementations)
+
+- (instancetype)initWithApp:(FIRApp *)app
+                 components:(NSDictionary<NSString *, FIRComponentCreationBlock> *)components {
+  self = [self initWithApp:app registrants:[[NSMutableSet alloc] init]];
+  if (self) {
+    components = [components mutableCopy];
+  }
+  return self;
+}
 
 @end
 
@@ -41,10 +59,10 @@ @implementation FIRComponentContainerTest
 #pragma mark - Registration Tests
 
 - (void)testRegisteringConformingClass {
-  NSMutableSet<Class> *allRegistrants = [NSMutableSet<Class> set];
+  [FIRComponentContainer setRegistrantSet:[NSMutableSet<Class> set]];
   Class testClass = [FIRTestClass class];
-  [FIRComponentContainer registerAsComponentRegistrant:testClass inSet:allRegistrants];
-  XCTAssertTrue([allRegistrants containsObject:testClass]);
+  [FIRComponentContainer registerAsComponentRegistrant:testClass];
+  XCTAssertTrue([[FIRComponentContainer registrantSet] containsObject:testClass]);
 }
 
 - (void)testRegisteringNonConformingClass {
@@ -143,14 +161,15 @@ - (void)testProtocolAlreadyRegistered {
 /// Create a container that has registered the test class.
 - (FIRComponentContainer *)containerWithRegistrants:(NSArray<Class> *)registrants {
   id appMock = OCMClassMock([FIRApp class]);
-  NSMutableSet<Class> *allRegistrants = [NSMutableSet<Class> set];
+  [FIRComponentContainer setRegistrantSet:[NSMutableSet<Class> set]];
 
   // Initialize the container with the test classes.
   for (Class c in registrants) {
-    [FIRComponentContainer registerAsComponentRegistrant:c inSet:allRegistrants];
+    [FIRComponentContainer registerAsComponentRegistrant:c];
   }
 
-  return [[FIRComponentContainer alloc] initWithApp:appMock registrants:allRegistrants];
+  return [[FIRComponentContainer alloc] initWithApp:appMock
+                                        registrants:[FIRComponentContainer registrantSet]];
 }
 
 @end

Firebase/Core/FIRApp.m

@@ -500,6 +500,17 @@ + (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.
+
+  // Ensure the class given conforms to the proper protocol.
+  if (![(Class)library conformsToProtocol:@protocol(FIRLibrary)] ||
+      ![(Class)library respondsToSelector:@selector(componentsToRegister)]) {
+    [NSException raise:NSInvalidArgumentException
+                format:
+                    @"Class %@ attempted to register components, but it does not conform to "
+                    @"`FIRLibrary or provide a `componentsToRegister:` method.",
+                    library];
+  }
+
   [FIRComponentContainer registerAsComponentRegistrant:library];
   if ([(Class)library respondsToSelector:@selector(configureWithApp:)]) {
     static dispatch_once_t onceToken;

Firebase/Core/FIRComponentContainer.m

@@ -47,29 +47,7 @@ + (void)registerAsComponentRegistrant:(Class<FIRLibrary>)klass {
     sFIRComponentRegistrants = [[NSMutableSet<Class> alloc] init];
   });
 
-  [self registerAsComponentRegistrant:klass inSet:sFIRComponentRegistrants];
-}
-
-+ (void)registerAsComponentRegistrant:(Class)klass inSet:(NSMutableSet<Class> *)allRegistrants {
-  // Validate the array to store the components is initialized.
-  if (!allRegistrants) {
-    FIRLogWarning(kFIRLoggerCore, @"I-COR000025",
-                  @"Attempted to store registered components in an empty set.");
-    return;
-  }
-
-  // Ensure the class given conforms to the proper protocol.
-  // TODO restore protocol check after new code propagates internally
-  if (  //![klass conformsToProtocol:@protocol(FIRLibrary)] ||
-      ![klass respondsToSelector:@selector(componentsToRegister)]) {
-    [NSException raise:NSInvalidArgumentException
-                format:
-                    @"Class %@ attempted to register components, but it does not conform to "
-                    @"`FIRComponentRegistrant` or provide a `componentsToRegister:` method.",
-                    klass];
-  }
-
-  [allRegistrants addObject:klass];
+  [sFIRComponentRegistrants addObject:klass];
 }
 
 #pragma mark - Internal Initialization
@@ -188,17 +166,14 @@ - (void)removeAllCachedInstances {
   [self.cachedInstances removeAllObjects];
 }
 
-#pragma mark - Testing Initializers
+#pragma mark - Testing Accessors
 
-// TODO(wilsonryan): Set up a testing flag so this only is compiled in with unit tests.
-/// Initialize an instance with an app and existing components.
-- (instancetype)initWithApp:(FIRApp *)app
-                 components:(NSDictionary<NSString *, FIRComponentCreationBlock> *)components {
-  self = [self initWithApp:app registrants:[[NSMutableSet alloc] init]];
-  if (self) {
-    _components = [components mutableCopy];
-  }
-  return self;
++ (void)setRegistrantSet:(NSMutableSet<Class> *)testInput {
+  sFIRComponentRegistrants = testInput;
+}
+
++ (NSMutableSet<Class> *)registrantSet {
+  return sFIRComponentRegistrants;
 }
 
 @end