fix: fixed the objc tests that failed the request consolidator tests

jsalaber · jsalaber · commit c2e0ac3dd84b · 2022-10-14T14:02:07.000-04:00
diff --git a/DevCycle/DVCUser.swift b/DevCycle/DVCUser.swift
@@ -74,7 +74,8 @@ public class UserBuilder {
     
     public func build() throws -> DVCUser {
         guard let _ = self.user.userId,
-              let _ = self.user.isAnonymous
+              let _ = self.user.isAnonymous,
+              self.user.userId != ""
         else {
             throw UserError.MissingUserIdAndIsAnonymousFalse
         }
diff --git a/DevCycle/Models/Cache.swift b/DevCycle/Models/Cache.swift
@@ -9,8 +9,6 @@ import Foundation
 protocol CacheServiceProtocol {
     func load() -> Cache
     func save(user: DVCUser)
-    func save(config: Data)
-    func save(config: UserConfig)
 }
 
 struct Cache {
@@ -41,16 +39,6 @@ class CacheService: CacheServiceProtocol {
         return Cache(config: userConfig, user: dvcUser)
     }
     
-    func save(config: Data) {
-        let defaults = UserDefaults.standard
-        defaults.set(config, forKey: CacheKeys.config)
-    }
-    
-    func save(config: UserConfig) {
-        let defaults = UserDefaults.standard
-        defaults.set(config, forKey: CacheKeys.config)
-    }
-    
     func save(user: DVCUser) {
         let defaults = UserDefaults.standard
         if let data = try? JSONEncoder().encode(user) {
diff --git a/DevCycle/Networking/DevCycleService.swift b/DevCycle/Networking/DevCycleService.swift
@@ -197,10 +197,6 @@ class DevCycleService: DevCycleServiceProtocol {
         }.resume()
     }
     
-    func makeAndProcessRequest() {
-        
-    }
-    
     func createConfigRequest(user: DVCUser, enableEdgeDB: Bool) -> URLRequest {
         var userQueryItems: [URLQueryItem] = user.toQueryItems()
         let queryItem = URLQueryItem(name: "enableEdgeDB", value: String(enableEdgeDB))
diff --git a/DevCycle/ObjC/ObjCDVCUser.swift b/DevCycle/ObjC/ObjCDVCUser.swift
@@ -17,10 +17,19 @@ public class ObjCUser: NSObject {
     @objc public var customData: [String: Any]?
     @objc public var privateCustomData: [String: Any]?
     
+    public override init() {
+        self.isAnonymous = true
+    }
+    
     @objc(initializeWithUserId:)
     public static func initialize(userId: String?) -> ObjCUser {
         let builder = ObjCUser()
-        builder.userId = userId
+        if (userId == nil) {
+            builder.isAnonymous = true
+        } else {
+            builder.userId = userId
+            builder.isAnonymous = false
+        }
         return builder
     }
     
diff --git a/DevCycle/Utils/RequestConsolidator.swift b/DevCycle/Utils/RequestConsolidator.swift
@@ -10,44 +10,73 @@ import Foundation
 class RequestConsolidator {
     var requestCallbacks: [RequestWithCallback]
     var service: DevCycleServiceProtocol
+    var requestInFlight: Bool
     
     init(service: DevCycleServiceProtocol) {
         self.service = service
         self.requestCallbacks = []
+        self.requestInFlight = false
     }
     
     func queue(request: URLRequest, callback: @escaping ConfigCompletionHandler) {
-        let configComplete: ConfigCompletionHandler = { config in
-            if let lastRequestCallback = self.requestCallbacks.last, lastRequestCallback.request == request {
-                for requestCallback in self.requestCallbacks {
-                    requestCallback.callback(config)
+        if (self.requestInFlight) {
+            self.requestCallbacks.append(
+                RequestWithCallback(
+                    callback: callback,
+                    request: request
+                )
+            )
+            return
+        }
+        
+        self.requestInFlight = true
+        service.makeRequest(request: request) { response in
+            if (self.requestCallbacks.isEmpty) {
+                guard let config = processConfig(response.data) else {
+                    callback((nil, response.error))
+                    return
+                }
+                callback((config, response.error))
+                self.requestInFlight = false
+            } else {
+                self.requestCallbacks.insert(
+                    RequestWithCallback(
+                        callback: callback,
+                        request: request
+                    ),
+                    at: 0
+                )
+                self.makeLastRequestInQueue {
+                    self.requestInFlight = false
                 }
-                self.requestCallbacks = []
             }
         }
-        self.requestCallbacks.append(
-            RequestWithCallback(
-                callback: callback,
-                request: request,
-                service: self.service,
-                finish: configComplete
-            )
-        )
+    }
+    
+    func makeLastRequestInQueue(complete: (() -> Void)?) {
+        guard let lastRequest = self.requestCallbacks.last?.request else {
+            print("No last request to make in queue")
+            return
+        }
+        service.makeRequest(request: lastRequest) { response in
+            for requestCallback in self.requestCallbacks {
+                guard let config = processConfig(response.data) else {
+                    requestCallback.callback((nil, response.error))
+                    return
+                }
+                requestCallback.callback((config, response.error))
+            }
+            self.requestCallbacks = []
+            complete?()
+        }
     }
 }
 
-class RequestWithCallback {
+struct RequestWithCallback {
     var callback: ConfigCompletionHandler
     var request: URLRequest
-    init(callback: @escaping ConfigCompletionHandler, request: URLRequest, service: DevCycleServiceProtocol, finish: @escaping ConfigCompletionHandler) {
+    init(callback: @escaping ConfigCompletionHandler, request: URLRequest) {
         self.callback = callback
         self.request = request
-        service.makeRequest(request: request) { response in
-            guard let config = processConfig(response.data) else {
-                finish((nil, response.error))
-                return
-            }
-            finish((config, response.error))
-        }
     }
 }
diff --git a/DevCycleTests/Models/DVCClientTest.swift b/DevCycleTests/Models/DVCClientTest.swift
@@ -116,7 +116,7 @@ class DVCClientTest: XCTestCase {
 }
 
 extension DVCClientTest {
-    class MockService: DevCycleServiceProtocol {
+    private class MockService: DevCycleServiceProtocol {
         public var publishCallCount: Int = 0
         
         func getConfig(user: DVCUser, enableEdgeDB: Bool, completion: @escaping ConfigCompletionHandler) {
diff --git a/DevCycleTests/Networking/DevCycleServiceTests.swift b/DevCycleTests/Networking/DevCycleServiceTests.swift
@@ -36,7 +36,6 @@ class DevCycleServiceTests: XCTestCase {
     }
     
     func testProcessConfigReturnsNilIfMissingProperties() throws {
-        let service = getService()
         let data = "{\"config\":\"key\"}".data(using: .utf8)
         let config = processConfig(data)
         XCTAssertNil(config)
@@ -63,14 +62,6 @@ extension DevCycleServiceTests {
         func save(user: DVCUser) {
             self.saveUserCalled = true
         }
-        
-        func save(config: Data) {
-            self.saveConfigCalled = true
-        }
-        
-        func save(config: UserConfig) {
-            self.saveConfigCalled = true
-        }
     }
 
     func getService() -> DevCycleService {
diff --git a/DevCycleTests/ObjC/ObjCDVCClientTests.m b/DevCycleTests/ObjC/ObjCDVCClientTests.m
@@ -14,43 +14,44 @@ @interface ObjcDVCClientTests : XCTestCase
 @implementation ObjcDVCClientTests
 
 - (void)testBuilderReturnsErrorIfNoEnvKey {
+    XCTestExpectation *expectation = [self expectationWithDescription:@"Builder returns error if no env key"];
     DVCUser *user = [DVCUser initializeWithUserId:@"my_user"];
     DVCClient *client = [DVCClient initialize:nil user:user options:nil onInitialized:^(NSError * _Nullable err) {
         XCTAssertNil(client);
         XCTAssertNotNil(err);
+        [expectation fulfill];
     }];
+    [self waitForExpectationsWithTimeout:5.0 handler:nil];
 }
 
 - (void)testBuilderReturnsErrorIfNoUser {
+    XCTestExpectation *expectation = [self expectationWithDescription:@"Builder returns error if no user"];
     DVCClient *client = [DVCClient initialize:@"my_env_key" user:nil options:nil onInitialized:^(NSError * _Nullable err) {
         XCTAssertNil(client);
         XCTAssertNotNil(err);
+        [expectation fulfill];
     }];
+    [self waitForExpectationsWithTimeout:5.0 handler:nil];
 }
 
 - (void)testBuilderCreatesClientWithUserAndEnvKey {
     DVCUser *user = [DVCUser initializeWithUserId:@"my_user"];
-    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:^(NSError * _Nullable err) {
-        XCTAssertNil(err);
-        XCTAssertNotNil(client);
-    }];
+    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:nil];
+    XCTAssertNotNil(client);
 }
 
 #pragma mark - Variable Tests
 
 - (void)testVariableIsCreated {
     DVCUser *user = [DVCUser initializeWithUserId:@"my_user"];
-    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:^(NSError * _Nullable err) {
-        XCTAssertNil(err);
-        XCTAssertNotNil(client);
-        
-        DVCVariable *variable = [client stringVariableWithKey:@"my-key" defaultValue:@"default-value"];
-        XCTAssertNotNil(variable);
-        XCTAssertNil(variable.type);
-        XCTAssertNil(variable.evalReason);
-        XCTAssertEqual(variable.value, @"default-value");
-        XCTAssertEqual(variable.defaultValue, @"default-value");
-    }];
+    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:nil];
+    XCTAssertNotNil(client);
+    DVCVariable *variable = [client stringVariableWithKey:@"my-key" defaultValue:@"default-value"];
+    XCTAssertNotNil(variable);
+    XCTAssertTrue([variable.type isEqualToString:@"String"]);
+    XCTAssertNil(variable.evalReason);
+    XCTAssertEqual(variable.value, @"default-value");
+    XCTAssertEqual(variable.defaultValue, @"default-value");
 }
 
 @end
diff --git a/DevCycleTests/ObjC/ObjcDVCUserTests.m b/DevCycleTests/ObjC/ObjcDVCUserTests.m
@@ -15,32 +15,29 @@ @implementation ObjcDVCUserTests
 
 - (void)testCreateUser {
     DVCUser *user = [DVCUser initializeWithUserId:@"my_user"];
-    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:^(NSError * _Nullable err) {
-        XCTAssertNil(err);
-    }];
+    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:nil];
+    XCTAssertNotNil(user);
+    XCTAssertNotNil(client);
 }
 
 - (void)testAnonUser {
     DVCUser *user = [[DVCUser alloc] init];
-    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:^(NSError * _Nullable err) {
-        XCTAssertNil(err);
-        XCTAssertTrue(user.isAnonymous);
-    }];
+    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:nil];
+    XCTAssertNotNil(user);
+    XCTAssertTrue(user.isAnonymous);
 }
 
 - (void)testNonUserIdPropertiesAreNil {
     DVCUser *user = [DVCUser initializeWithUserId:@"my_user"];
-    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:^(NSError * _Nullable err) {
-        XCTAssertNil(err);
-        XCTAssertNotNil(user);
-        XCTAssert([user.userId isEqual:@"my_user"]);
-        XCTAssertFalse([user.isAnonymous boolValue]);
-        XCTAssertNil(user.email);
-        XCTAssertNil(user.name);
-        XCTAssertNil(user.country);
-        XCTAssertNil(user.customData);
-        XCTAssertNil(user.privateCustomData);
-    }];
+    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:nil];
+    XCTAssertNotNil(user);
+    XCTAssert([user.userId isEqual:@"my_user"]);
+    XCTAssertFalse([user.isAnonymous boolValue]);
+    XCTAssertNil(user.email);
+    XCTAssertNil(user.name);
+    XCTAssertNil(user.country);
+    XCTAssertNil(user.customData);
+    XCTAssertNil(user.privateCustomData);
 }
 
 - (void)testNonUserIdPropertiesAreNotNil {
@@ -49,15 +46,13 @@ - (void)testNonUserIdPropertiesAreNotNil {
     user.email = @"email.com";
     user.name = @"Jason Smith";
     user.country = @"CAN";
-    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:^(NSError * _Nullable err) {
-        XCTAssertNil(err);
-        XCTAssertNotNil(user);
-        XCTAssert([user.userId isEqual:@"my_user"]);
-        XCTAssertFalse([user.isAnonymous boolValue]);
-        XCTAssertEqual(user.email, @"email.com");
-        XCTAssertEqual(user.name, @"Jason Smith");
-        XCTAssertEqual(user.country, @"CAN");
-    }];
+    DVCClient *client = [DVCClient initialize:@"my_env_key" user:user options:nil onInitialized:nil];
+    XCTAssertNotNil(user);
+    XCTAssert([user.userId isEqual:@"my_user"]);
+    XCTAssertFalse([user.isAnonymous boolValue]);
+    XCTAssertEqual(user.email, @"email.com");
+    XCTAssertEqual(user.name, @"Jason Smith");
+    XCTAssertEqual(user.country, @"CAN");
 }
 
 @end
diff --git a/DevCycleTests/Utils/RequestConsolidatorTests.swift b/DevCycleTests/Utils/RequestConsolidatorTests.swift
@@ -19,7 +19,8 @@ class RequestConsolidatorTests: XCTestCase {
             XCTAssertEqual(response.config?.variables["testVar"]?.value as! String, "any_value")
             expectation.fulfill()
         }
-        waitForExpectations(timeout: 2)
+        waitForExpectations(timeout: 2.0)
+        XCTAssertFalse(requestConsolidator.requestInFlight)
     }
     
     func testMultipleRequestFinishesWithLatestURLConfig() {
@@ -31,18 +32,26 @@ class RequestConsolidatorTests: XCTestCase {
         let expectation = expectation(description: "Multiple request completes")
         expectation.expectedFulfillmentCount = 3
         requestConsolidator.queue(request: request1) { response in
+            print("testVar variable 1: \(response.config?.variables["testVar"]?.value as! String)")
             XCTAssertEqual(response.config?.variables["testVar"]?.value as! String, "thirdPage")
+            print("Fulfill 1")
             expectation.fulfill()
         }
+        XCTAssertTrue(requestConsolidator.requestInFlight)
         requestConsolidator.queue(request: request2) { response in
+            print("testVar variable 2: \(response.config?.variables["testVar"]?.value as! String)")
             XCTAssertEqual(response.config?.variables["testVar"]?.value as! String, "thirdPage")
+            print("Fulfill 2")
             expectation.fulfill()
         }
         requestConsolidator.queue(request: request3) { response in
+            print("testVar variable 3: \(response.config?.variables["testVar"]?.value as! String)")
             XCTAssertEqual(response.config?.variables["testVar"]?.value as! String, "thirdPage")
+            print("Fulfill 3")
             expectation.fulfill()
         }
-        waitForExpectations(timeout: 2)
+        waitForExpectations(timeout: 5.0)
+        XCTAssertFalse(requestConsolidator.requestInFlight)
     }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,8 @@ public class UserBuilder {`
`74`	`74`
`75`	`75`	`public func build() throws -> DVCUser {`
`76`	`76`	`guard let _ = self.user.userId,`
`77`		`- let _ = self.user.isAnonymous`
	`77`	`+ let _ = self.user.isAnonymous,`
	`78`	`+ self.user.userId != ""`
`78`	`79`	`else {`
`79`	`80`	`throw UserError.MissingUserIdAndIsAnonymousFalse`
`80`	`81`	`}`
Original file line number	Diff line number	Diff line change
`@@ -116,7 +116,7 @@ class DVCClientTest: XCTestCase {`
`116`	`116`	`}`
`117`	`117`
`118`	`118`	`extension DVCClientTest {`
`119`		`- class MockService: DevCycleServiceProtocol {`
	`119`	`+ private class MockService: DevCycleServiceProtocol {`
`120`	`120`	`public var publishCallCount: Int = 0`
`121`	`121`
`122`	`122`	`func getConfig(user: DVCUser, enableEdgeDB: Bool, completion: @escaping ConfigCompletionHandler) {`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,8 @@ class RequestConsolidatorTests: XCTestCase {`
`19`	`19`	`XCTAssertEqual(response.config?.variables["testVar"]?.value as! String, "any_value")`
`20`	`20`	`expectation.fulfill()`
`21`	`21`	`}`
`22`		`- waitForExpectations(timeout: 2)`
	`22`	`+ waitForExpectations(timeout: 2.0)`
	`23`	`+ XCTAssertFalse(requestConsolidator.requestInFlight)`
`23`	`24`	`}`
`24`	`25`
`25`	`26`	`func testMultipleRequestFinishesWithLatestURLConfig() {`
`@@ -31,18 +32,26 @@ class RequestConsolidatorTests: XCTestCase {`
`31`	`32`	`let expectation = expectation(description: "Multiple request completes")`
`32`	`33`	`expectation.expectedFulfillmentCount = 3`
`33`	`34`	`requestConsolidator.queue(request: request1) { response in`
	`35`	`+ print("testVar variable 1: \(response.config?.variables["testVar"]?.value as! String)")`
`34`	`36`	`XCTAssertEqual(response.config?.variables["testVar"]?.value as! String, "thirdPage")`
	`37`	`+ print("Fulfill 1")`
`35`	`38`	`expectation.fulfill()`
`36`	`39`	`}`
	`40`	`+ XCTAssertTrue(requestConsolidator.requestInFlight)`
`37`	`41`	`requestConsolidator.queue(request: request2) { response in`
	`42`	`+ print("testVar variable 2: \(response.config?.variables["testVar"]?.value as! String)")`
`38`	`43`	`XCTAssertEqual(response.config?.variables["testVar"]?.value as! String, "thirdPage")`
	`44`	`+ print("Fulfill 2")`
`39`	`45`	`expectation.fulfill()`
`40`	`46`	`}`
`41`	`47`	`requestConsolidator.queue(request: request3) { response in`
	`48`	`+ print("testVar variable 3: \(response.config?.variables["testVar"]?.value as! String)")`
`42`	`49`	`XCTAssertEqual(response.config?.variables["testVar"]?.value as! String, "thirdPage")`
	`50`	`+ print("Fulfill 3")`
`43`	`51`	`expectation.fulfill()`
`44`	`52`	`}`
`45`		`- waitForExpectations(timeout: 2)`
	`53`	`+ waitForExpectations(timeout: 5.0)`
	`54`	`+ XCTAssertFalse(requestConsolidator.requestInFlight)`
`46`	`55`	`}`
`47`	`56`	`}`
`48`	`57`