Merge pull request #969 from praveek/migrateOffUserDefaults

AEPIntegrationTests/IdentityIntegrationTests.swift

@@ -207,14 +207,16 @@ class IdentityIntegrationTests: XCTestCase {
     func testGetExperienceCloudIdWithinPermissibleTimeOnInstall() {
         initExtensionsAndWait()
 
-        let getECIDExpectation = XCTestExpectation(description: "getExperienceCloudId should return within 0.5 seconds when Configuration is available on Install")
+        let getECIDExpectation = XCTestExpectation(description: "getExperienceCloudId should return within 1 seconds when Configuration is available on Install")
         MobileCore.updateConfigurationWith(configDict: ["experienceCloud.org": "orgid", "experienceCloud.server": "test.com", "global.privacy": "optedin"])
         Identity.getExperienceCloudId { ecid, error in
             XCTAssertFalse(ecid!.isEmpty)
             XCTAssertNil(error)
             getECIDExpectation.fulfill()
         }
-        wait(for: [getECIDExpectation], timeout: 0.5)
+        // getExperienceCloudId returns within 0.5 sec when config is bundled with the app. 
+        // Increasing timeout to 1 sec to avoid race conditions
+        wait(for: [getECIDExpectation], timeout: 1)
     }
 
     func testGetExperienceCloudIdWithinPermissibleTimeOnLaunch() {
@@ -346,18 +348,9 @@ class IdentityIntegrationTests: XCTestCase {
         waitForBootupHit(initialConfig: ["experienceCloud.org": "orgid", "experienceCloud.server": "test.com", "global.privacy": "optedin"])
 
         let resetHitExpectation = XCTestExpectation(description: "new sync from reset identities")
-        resetHitExpectation.assertForOverFulfill = true
-
         let mockNetworkService = TestableNetworkService()
         ServiceProvider.shared.networkService = mockNetworkService
-
         mockNetworkService.mock { request in
-            // assert new ECID on last hit
-            props.loadFromPersistence()
-            XCTAssertNotEqual(firstEcid.ecidString, props.ecid?.ecidString)
-            XCTAssertTrue(request.url.absoluteString.contains(props.ecid!.ecidString))
-            XCTAssertFalse(request.url.absoluteString.contains(firstEcid.ecidString))
-
             resetHitExpectation.fulfill()
             return nil
         }
@@ -366,6 +359,20 @@ class IdentityIntegrationTests: XCTestCase {
         MobileCore.resetIdentities()
 
         wait(for: [resetHitExpectation], timeout: 2)
+
+        // Wait for 500ms so that the new ECID gets persisted and to avoid race conditions
+        usleep(500)
+
+        // assert new ECID on last hit
+        props.loadFromPersistence()
+        guard let newEcid = props.ecid else {
+            XCTFail("New ECID is not generated")
+            return
+        }
+
+        XCTAssertNotEqual(firstEcid.ecidString, newEcid.ecidString)
+        XCTAssertTrue(mockNetworkService.requests[0].url.absoluteString.contains(newEcid.ecidString))
+        XCTAssertFalse(mockNetworkService.requests[0].url.absoluteString.contains(firstEcid.ecidString))
     }
 
     private func waitForBootupHit(initialConfig: [String: String]) {

AEPServices/Sources/ServiceProvider.swift

@@ -116,7 +116,6 @@ public class ServiceProvider {
         queue.async {
             self.defaultSystemInfoService = ApplicationSystemInfoService()
             self.defaultKeyValueService = FileSystemNamedCollection()
-//            self.defaultKeyValueService = UserDefaultsNamedCollection()
             self.defaultNetworkService = NetworkService()
             self.defaultCacheService = DiskCacheService()
             self.defaultDataQueueService = DataQueueService()

AEPServices/Sources/cache/DiskCacheService.swift

@@ -18,43 +18,51 @@ class DiskCacheService: Caching {
     let cachePrefix = "com.adobe.mobile.diskcache/"
     let fileManager = FileManager.default
     private let LOG_PREFIX = "DiskCacheService"
-    private let PATH = "PATH"
+    private let EXPIRY_DATE = "expirydate"
+    private let METADATA = "metadata"
+    private let METADATA_KEY_PATH = "PATH"
 
     // MARK: Caching
 
     public func set(cacheName: String, key: String, entry: CacheEntry) throws {
         try createDirectoryIfNeeded(cacheName: cacheName)
         let path = filePath(for: cacheName, with: key)
         _ = fileManager.createFile(atPath: path, contents: entry.data, attributes: nil)
-        try fileManager.setAttributes([.modificationDate: entry.expiry.date], ofItemAtPath: path)
-        var newMetadata = [PATH: path]
+
+        var newMetadata = [METADATA_KEY_PATH: path]
         if let meta = entry.metadata, !meta.isEmpty {
             newMetadata.merge(meta) { current, _ in current }
         }
-        let newCache = CacheEntry(data: entry.data, expiry: entry.expiry, metadata: newMetadata)
-        Log.trace(label: LOG_PREFIX, "Updating cache '\(cacheName)' - setting key '\(key)' to value: \n\(newCache.metadata as AnyObject)")
-        dataStore.set(key: dataStoreKey(for: cacheName, with: key), value: newCache.metadata)
+
+        var attributes: [String: Any] = [
+            EXPIRY_DATE: entry.expiry.date.timeIntervalSince1970,
+            METADATA: newMetadata
+        ]
+        Log.trace(label: LOG_PREFIX, "Updating cache '\(cacheName)' - setting key '\(key)' to value: \n\(attributes as AnyObject)")
+        dataStore.set(key: dataStoreKey(for: cacheName, with: key), value: attributes)
     }
 
     public func get(cacheName: String, key: String) -> CacheEntry? {
         try? createDirectoryIfNeeded(cacheName: cacheName)
         let path = filePath(for: cacheName, with: key)
         guard let data = try? Data(contentsOf: URL(fileURLWithPath: path)) else { return nil }
-        let attributes = try? fileManager.attributesOfItem(atPath: path)
+        let attributes = dataStore.getDictionary(key: dataStoreKey(for: cacheName, with: key)) as? [String: Any]
 
-        guard let expiryDate = attributes?[.modificationDate] as? Date else {
+        guard let attributes = attributes, let expiryDate = attributes[EXPIRY_DATE] as? Double else {
+            // Expiry date attribute is either missing or unreadable, remove from cache
+            try? remove(cacheName: cacheName, key: key)
             return nil
         }
 
-        let expiry = CacheExpiry.date(expiryDate)
+        let expiry = CacheExpiry.date(Date(timeIntervalSince1970: expiryDate))
         if expiry.isExpired {
             // item is expired, remove from cache
             try? remove(cacheName: cacheName, key: key)
             return nil
         }
 
-        let meta = dataStore.getDictionary(key: dataStoreKey(for: cacheName, with: key)) as? [String: String]
-        return CacheEntry(data: data, expiry: .date(expiryDate), metadata: meta)
+        let meta = attributes[METADATA] as? [String: String]        
+        return CacheEntry(data: data, expiry: expiry, metadata: meta)
     }
 
     public func remove(cacheName: String, key: String) throws {
@@ -101,7 +109,7 @@ class DiskCacheService: Caching {
     ///   - cacheName: name of the cache
     ///   - key: key for the entry
     /// - Returns: the key to be used in the datastore for the entry
-    private func dataStoreKey(for cacheName: String, with key: String) -> String {
+    func dataStoreKey(for cacheName: String, with key: String) -> String {
         return "\(cacheName.alphanumeric)/\(key.alphanumeric)"
     }
 }

AEPServices/Sources/storage/FileSystemNamedCollection.swift

@@ -49,7 +49,7 @@ class FileSystemNamedCollection: NamedCollectionProcessing {
                 do {
                     try updatedStorageData.write(to: fileUrl, options: .atomic)
                 } catch {
-                    Log.warning(label: self.LOG_TAG, "Error when writing to file: \(error)")
+                    Log.warning(label: self.LOG_TAG, "Error '\(error)' when writing '\(key)' to collection '\(collectionName)'")
                 }
             }
         }
@@ -75,7 +75,7 @@ class FileSystemNamedCollection: NamedCollectionProcessing {
                     do {
                         try updatedStorageData.write(to: fileUrl, options: .atomic)
                     } catch {
-                        Log.warning(label: self.LOG_TAG, "Error when attempting to remove value from file: \(error)")
+                        Log.warning(label: self.LOG_TAG, "Error '\(error)' when attempting to remove key '\(key)' from collection '\(collectionName)'")
                     }
                 }
             }
@@ -92,13 +92,16 @@ class FileSystemNamedCollection: NamedCollectionProcessing {
             return nil
         }
 
-        if let storageData = try? Data(contentsOf: fileUrl) {
-            if let jsonResult = try? JSONSerialization.jsonObject(with: storageData) as? [String: Any] {
-                return jsonResult
-            }
+        guard let storageData = try? Data(contentsOf: fileUrl) else {
+            return nil
+        }
+
+        guard let jsonResult = try? JSONSerialization.jsonObject(with: storageData) as? [String: Any] else {
+            Log.warning(label: LOG_TAG, "Failed to read dictionary from collection '\(collectionName)'")
+            return nil
         }
-        Log.warning(label: LOG_TAG, "Failed to get Dictionary from data store")
-        return nil
+
+        return jsonResult
     }
 
     ///

AEPServices/Sources/utility/unzip/FileManager+ZIP.swift

@@ -61,11 +61,12 @@ extension FileManager {
     class func attributes(from entry: ZipEntry) -> [FileAttributeKey: Any] {
         let centralDirectoryStructure = entry.centralDirectoryStructure
         let entryType = entry.type
-        let entryTime = centralDirectoryStructure.lastModFileTime
-        let entryDate = centralDirectoryStructure.lastModFileDate
         let defaultPermissions = FileUnzipperConstants.defaultFilePermissions
         var attributes = [.posixPermissions: defaultPermissions] as [FileAttributeKey: Any]
-        attributes[.modificationDate] = Date(dateTime: (entryDate, entryTime))
+        // `modificationDate` attribute is now marked as a restricted API. We will not set it as we don't have any use case based on this attribute.
+        // let entryTime = centralDirectoryStructure.lastModFileTime
+        // let entryDate = centralDirectoryStructure.lastModFileDate
+        // attributes[.modificationDate] = Date(dateTime: (entryDate, entryTime))
         let versionMadeBy = centralDirectoryStructure.versionMadeBy
         guard let osType = ZipEntry.OSType(rawValue: UInt(versionMadeBy >> 8)) else { return attributes }
 

AEPServices/Tests/services/DiskCacheServiceTests.swift

@@ -22,12 +22,15 @@ private struct Person: Codable, Equatable {
 
 class DiskCacheServiceTests: XCTestCase {
     var diskCache = DiskCacheService()
+    let DATA_STORE_NAME = "DiskCacheService"
     let CACHE_NAME = "DiskCacheServiceTests"
     let ENTRY_KEY = "testEntryKey"
+    var mockDataStore: MockDataStore!
     var dateOneMinInFuture: Date!
 
     override func setUp() {
-        ServiceProvider.shared.namedKeyValueService = MockDataStore()
+        mockDataStore = MockDataStore()
+        ServiceProvider.shared.namedKeyValueService = mockDataStore
         dateOneMinInFuture = Calendar.current.date(byAdding: .minute, value: 1, to: Date())
     }
 
@@ -39,7 +42,7 @@ class DiskCacheServiceTests: XCTestCase {
     }
 
     func originalEntry(_ original: CacheEntry, equals cached: CacheEntry) -> Bool {
-        guard original.data == cached.data, original.expiry == cached.expiry else {
+        guard original.data == cached.data, original.expiry.date.equal(other: cached.expiry.date) else {
             return false
         }
 
@@ -80,6 +83,34 @@ class DiskCacheServiceTests: XCTestCase {
         let storedEntry = diskCache.get(cacheName: CACHE_NAME, key: ENTRY_KEY)
         XCTAssertTrue(originalEntry(entry, equals: storedEntry!))
     }
+
+    func testSetGetMissingAttributes() {
+        // setup
+        let data = "Test".data(using: .utf8)!
+        let entry = CacheEntry(data: data, expiry: .date(dateOneMinInFuture), metadata: nil)
+
+        // test
+        try! diskCache.set(cacheName: CACHE_NAME, key: ENTRY_KEY, entry: entry)
+        // remove attributes for this entry
+        mockDataStore.remove(collectionName: "DiskCacheService", key: diskCache.dataStoreKey(for: CACHE_NAME, with: ENTRY_KEY))
+
+        // verify
+        XCTAssertNil(diskCache.get(cacheName: CACHE_NAME, key: ENTRY_KEY))
+    }
+
+    func testSetGetMissingExpiryDate() {
+        // setup
+        let data = "Test".data(using: .utf8)!
+        let entry = CacheEntry(data: data, expiry: .date(dateOneMinInFuture), metadata: nil)
+
+        // test
+        try! diskCache.set(cacheName: CACHE_NAME, key: ENTRY_KEY, entry: entry)
+        // overwrite attributes for this entry
+        mockDataStore.set(collectionName: DATA_STORE_NAME, key: diskCache.dataStoreKey(for: CACHE_NAME, with: ENTRY_KEY), value: ["key": "value"])
+
+        // verify
+        XCTAssertNil(diskCache.get(cacheName: CACHE_NAME, key: ENTRY_KEY))
+    }
 
     func testSetGetNoMetadata() {
         // setup
@@ -184,3 +215,16 @@ class DiskCacheServiceTests: XCTestCase {
         XCTAssertNil(diskCache.get(cacheName: CACHE_NAME, key: ENTRY_KEY))
     }
 }
+
+private extension Date {
+    func equal(other: Date?) -> Bool {
+        guard let other = other else {
+            return false;
+        }
+
+        // As date stores the timestamp in milliseconds, compare double value with accuracy greater than milliseconds.
+        let accuracy = 0.00001;
+        return abs(self.timeIntervalSince1970 - other.timeIntervalSince1970) < accuracy;
+    }
+}
+