[FSSDK-6907]: Adds a check to only save valid datafile in cache. (#514)

yasirfolio3 · web-flow · commit 236d76dfed46 · 2023-06-26T17:02:47.000-04:00
* Adding check to only save datafile in cache if valid.
diff --git a/Sources/Customization/DefaultDatafileHandler.swift b/Sources/Customization/DefaultDatafileHandler.swift
@@ -1,5 +1,5 @@
 //
-// Copyright 2019-2022, Optimizely, Inc. and contributors
+// Copyright 2019-2023, Optimizely, Inc. and contributors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -158,12 +158,18 @@ open class DefaultDatafileHandler: OPTDatafileHandler {
     open func getResponseData(sdkKey: String, response: HTTPURLResponse, url: URL?) -> Data? {
         if let url = url, let data = try? Data(contentsOf: url) {
             self.logger.d { String(data: data, encoding: .utf8) ?? "" }
-            self.saveDatafile(sdkKey: sdkKey, dataFile: data)
-            if let lastModified = response.getLastModified() {
-                self.sharedDataStore.setLastModified(sdkKey: sdkKey, lastModified: lastModified)
+            // Check datafile is valid json
+            do {
+                // Try deserializing datafile, isValidJSONObject is not applicable here
+                _ = try JSONSerialization.jsonObject(with: data)
+                self.saveDatafile(sdkKey: sdkKey, dataFile: data)
+                if let lastModified = response.getLastModified() {
+                    self.sharedDataStore.setLastModified(sdkKey: sdkKey, lastModified: lastModified)
+                }
+                return data
+            } catch {
+                self.logger.w("Error deserializing datafile: \(error.localizedDescription)")
             }
-            
-            return data
         }
         
         return nil
diff --git a/Tests/OptimizelyTests-Common/DatafileHandlerTests.swift b/Tests/OptimizelyTests-Common/DatafileHandlerTests.swift
@@ -1,5 +1,5 @@
 //
-// Copyright 2019, 2021, Optimizely, Inc. and contributors
+// Copyright 2019, 2021, 2023, Optimizely, Inc. and contributors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -65,7 +65,65 @@ class DatafileHandlerTests: XCTestCase {
         // This is an example of a functional test case.
         // Use XCTAssert and related functions to verify your tests produce the correct results.
     }
+    
+    func testDatafileDownloadWith200AndValidDatafile() {
+        // Datafile and last updated should not be saved in this case
+        let handler = MockDatafileHandler(statusCode: 200, localResponseData:"{}")
+        let expectation = XCTestExpectation(description: "no-nil data")
+        
+        handler.downloadDatafile(sdkKey: sdkKey) { (result) in
+            if case let .success(data) = result  {
+                XCTAssert(data != nil)
+                expectation.fulfill()
+            }
+        }
+        
+        wait(for: [expectation], timeout: 3)
+        XCTAssertTrue(handler.isDatafileSaved(sdkKey: sdkKey))
+        XCTAssertNotNil(handler.sharedDataStore.getLastModified(sdkKey: sdkKey))
+    }
+
 
+    func testDatafileDownloadWith200AndInvalidDatafile() {
+        // Datafile and last updated should not be saved in this case
+        let handler = MockDatafileHandler(statusCode: 200, localResponseData: "1231")
+        let expectation = XCTestExpectation(description: "wait to get failure")
+        
+        handler.downloadDatafile(sdkKey: sdkKey) { (result) in
+            if case .success(_) = result  {
+                XCTFail()
+            }
+            if case .failure(_) = result  {
+                expectation.fulfill()
+            }
+        }
+        
+        wait(for: [expectation], timeout: 3)
+        XCTAssertFalse(handler.isDatafileSaved(sdkKey: sdkKey))
+        XCTAssertNil(handler.sharedDataStore.getLastModified(sdkKey: sdkKey))
+    }
+    
+    func testStartWith200AndInvalidDatafile() {
+        let handler = MockDatafileHandler(statusCode: 200, localResponseData: "1231")
+        let expectation = XCTestExpectation(description: "wait to get failure")
+        
+        let optimizely = OptimizelyClient(sdkKey: sdkKey,
+                                          datafileHandler: handler,
+                                          periodicDownloadInterval: 1)
+        
+        optimizely.start(completion: { result in
+            if case let .failure(error) = result  {
+                print(error)
+                XCTAssert(true)
+                expectation.fulfill()
+            }
+        })
+        
+        wait(for: [expectation], timeout: 3)
+        XCTAssertFalse(handler.isDatafileSaved(sdkKey: sdkKey))
+        XCTAssertNil(handler.sharedDataStore.getLastModified(sdkKey: sdkKey))
+    }
+    
     func testDatafileDownload500() {
         OTUtils.createDatafileCache(sdkKey: sdkKey)
 
@@ -184,7 +242,7 @@ class DatafileHandlerTests: XCTestCase {
 
     func testPeriodicDownload() {
         let expection = XCTestExpectation(description: "Expect 10 periodic downloads")
-        let handler = MockDatafileHandler(statusCode: 200)
+        let handler = MockDatafileHandler(statusCode: 200, localResponseData: "{}")
         let now = Date()
         var count = 0
         var seconds = 0
@@ -205,7 +263,7 @@ class DatafileHandlerTests: XCTestCase {
     
     func testPeriodicDownload_PollingShouldNotBeAccumulatedWhileInBackground() {
         let expectation = XCTestExpectation(description: "polling")
-        let handler = MockDatafileHandler(statusCode: 200)
+        let handler = MockDatafileHandler(statusCode: 200, localResponseData: "{}")
         let now = Date()
         
         let updateInterval = 1
@@ -336,7 +394,7 @@ class DatafileHandlerTests: XCTestCase {
                 // will return 500
                 return MockUrlSession(withError: true)
             } else {
-                return MockUrlSession(statusCode: 200)
+                return MockUrlSession(statusCode: 200, localResponseData: "{}")
             }
         }
     }
diff --git a/Tests/OptimizelyTests-Common/NetworkReachabilityTests.swift b/Tests/OptimizelyTests-Common/NetworkReachabilityTests.swift
@@ -1,5 +1,5 @@
 //
-// Copyright 2021, Optimizely, Inc. and contributors 
+// Copyright 2021, 2023 Optimizely, Inc. and contributors 
 // 
 // Licensed under the Apache License, Version 2.0 (the "License");  
 // you may not use this file except in compliance with the License.
@@ -119,7 +119,7 @@ class NetworkReachabilityTests: XCTestCase {
     }
     
     func testFetchDatafile_numContiguousFails() {
-        let handler = MockDatafileHandler(withError: true)
+        let handler = MockDatafileHandler(withError: true, localResponseData: "{}")
         let reachability = handler.reachability
 
         var exp = expectation(description: "r")
@@ -164,7 +164,7 @@ class NetworkReachabilityTests: XCTestCase {
     }
 
     func testFetchDatafile_checkReachability() {
-        let handler = MockDatafileHandler(withError: true)
+        let handler = MockDatafileHandler(withError: true, localResponseData: "{}")
         let reachability = handler.reachability
         
         reachability.stop()
@@ -196,7 +196,7 @@ class NetworkReachabilityTests: XCTestCase {
         
         // no valid datafile cache
         
-        let handler = MockDatafileHandler(withError: true)
+        let handler = MockDatafileHandler(withError: true, localResponseData: "{}")
         let reachability = handler.reachability
         
         reachability.stop()
diff --git a/Tests/OptimizelyTests-MultiClients/DatafileHandlerTests_MultiClients.swift b/Tests/OptimizelyTests-MultiClients/DatafileHandlerTests_MultiClients.swift
@@ -1,5 +1,5 @@
 //
-// Copyright 2021, Optimizely, Inc. and contributors 
+// Copyright 2021, 2023, Optimizely, Inc. and contributors
 // 
 // Licensed under the Apache License, Version 2.0 (the "License");  
 // you may not use this file except in compliance with the License.
@@ -39,7 +39,7 @@ class DatafileHandlerTests_MultiClients: XCTestCase {
     
     func testConcurrentDownloadDatafiles() {
         // use a shared DatafileHandler instance
-        let mockHandler = MockDatafileHandler(statusCode: 200)
+        let mockHandler = MockDatafileHandler(statusCode: 200, localResponseData: "{}")
         
         sdkKeys = OTUtils.makeRandomSdkKeys(100)
 
@@ -153,7 +153,7 @@ class DatafileHandlerTests_MultiClients: XCTestCase {
     
     func testConcurrentAccessLastModified() {
         // use a shared DatafileHandler instance
-        let mockHandler = MockDatafileHandler(statusCode: 200)
+        let mockHandler = MockDatafileHandler(statusCode: 200, localResponseData: "{}")
 
         sdkKeys = OTUtils.makeRandomSdkKeys(100)
 
@@ -241,7 +241,7 @@ class DatafileHandlerTests_MultiClients: XCTestCase {
         }
         
         // use a shared DatafileHandler instance
-        let mockHandler = MockDatafileHandler(statusCode: 200)
+        let mockHandler = MockDatafileHandler(statusCode: 200, localResponseData: "{}")
         
         let numSdks = 50
         sdkKeys = OTUtils.makeRandomSdkKeys(numSdks)
diff --git a/Tests/TestUtils/MockUrlSession.swift b/Tests/TestUtils/MockUrlSession.swift
@@ -28,6 +28,7 @@ class MockUrlSession: URLSession {
     var localResponseData: String?
     var settingsMap: [String: (Int, Bool)]?
     var handler: MockDatafileHandler?
+    let lock = DispatchQueue(label: "mock-session-lock")
     
     class MockDownloadTask: URLSessionDownloadTask {
         var task: () -> Void
@@ -54,15 +55,19 @@ class MockUrlSession: URLSession {
     }
 
     init(handler: MockDatafileHandler? = nil, statusCode: Int = 0, withError: Bool = false, localResponseData: String? = nil) {
-        Self.validSessions += 1
+        lock.async {
+            Self.validSessions += 1
+        }
         self.handler = handler
         self.statusCode = statusCode
         self.withError = withError
         self.localResponseData = localResponseData
     }
    
     init(handler: MockDatafileHandler? = nil, settingsMap: [String: (Int, Bool)]) {
-        Self.validSessions += 1
+        lock.async {
+            Self.validSessions += 1
+        }
         self.handler = handler
         self.statusCode = 0
         self.withError = false
@@ -113,6 +118,8 @@ class MockUrlSession: URLSession {
     }
 
     override func finishTasksAndInvalidate() {
-        Self.validSessions -= 1
+        lock.async {
+            Self.validSessions -= 1
+        }
     }
 }
