From ba56340386aaf21c94408931aae976728f248741 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Tue, 22 Oct 2024 16:44:55 +0100 Subject: [PATCH 1/8] Remove caching on PhishingDetectionClient --- Sources/PhishingDetection/PhishingDetectionClient.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/PhishingDetection/PhishingDetectionClient.swift b/Sources/PhishingDetection/PhishingDetectionClient.swift index 942075b71..2293f68a0 100644 --- a/Sources/PhishingDetection/PhishingDetectionClient.swift +++ b/Sources/PhishingDetection/PhishingDetectionClient.swift @@ -161,6 +161,7 @@ extension PhishingDetectionAPIClient { var request = URLRequest(url: url) request.httpMethod = "GET" request.allHTTPHeaderFields = headers + request.cachePolicy = .reloadIgnoringLocalCacheData // Disable caching do { let (data, _) = try await session.data(for: request) From 8c43f96b73fbdf5e45a9831087acded685912ba2 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Tue, 22 Oct 2024 16:45:37 +0100 Subject: [PATCH 2/8] Ensure latest embedded dataset replaces on-disk dataset when needed --- .../PhishingDetectionDataStore.swift | 43 ++++++++++++++++--- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/Sources/PhishingDetection/PhishingDetectionDataStore.swift b/Sources/PhishingDetection/PhishingDetectionDataStore.swift index ab761f272..0894b3dc6 100644 --- a/Sources/PhishingDetection/PhishingDetectionDataStore.swift +++ b/Sources/PhishingDetection/PhishingDetectionDataStore.swift @@ -136,10 +136,16 @@ public class PhishingDetectionDataStore: PhishingDetectionDataSaving { } private func loadHashPrefix() -> Set { - guard let data = fileStorageManager.read(from: hashPrefixFilename) else { return dataProvider.loadEmbeddedHashPrefixes() } + guard let data = fileStorageManager.read(from: hashPrefixFilename) else { + return dataProvider.loadEmbeddedHashPrefixes() + } let decoder = JSONDecoder() do { - return Set(try decoder.decode(Set.self, from: data)) + if loadRevisionFromDisk() < dataProvider.embeddedRevision { + return dataProvider.loadEmbeddedHashPrefixes() + } + let onDiskHashPrefixes = Set(try decoder.decode(Set.self, from: data)) + return onDiskHashPrefixes } catch { Logger.phishingDetectionDataStore.error("Error decoding \(self.hashPrefixFilename): \(error.localizedDescription)") return dataProvider.loadEmbeddedHashPrefixes() @@ -147,18 +153,26 @@ public class PhishingDetectionDataStore: PhishingDetectionDataSaving { } private func loadFilterSet() -> Set { - guard let data = fileStorageManager.read(from: filterSetFilename) else { return dataProvider.loadEmbeddedFilterSet() } + guard let data = fileStorageManager.read(from: filterSetFilename) else { + return dataProvider.loadEmbeddedFilterSet() + } let decoder = JSONDecoder() do { - return Set(try decoder.decode(Set.self, from: data)) + if loadRevisionFromDisk() < dataProvider.embeddedRevision { + return dataProvider.loadEmbeddedFilterSet() + } + let onDiskFilterSet = Set(try decoder.decode(Set.self, from: data)) + return onDiskFilterSet } catch { Logger.phishingDetectionDataStore.error("Error decoding \(self.filterSetFilename): \(error.localizedDescription)") return dataProvider.loadEmbeddedFilterSet() } } - private func loadRevision() -> Int { - guard let data = fileStorageManager.read(from: revisionFilename) else { return dataProvider.embeddedRevision } + private func loadRevisionFromDisk() -> Int { + guard let data = fileStorageManager.read(from: revisionFilename) else { + return dataProvider.embeddedRevision + } let decoder = JSONDecoder() do { return try decoder.decode(Int.self, from: data) @@ -167,6 +181,23 @@ public class PhishingDetectionDataStore: PhishingDetectionDataSaving { return dataProvider.embeddedRevision } } + + private func loadRevision() -> Int { + guard let data = fileStorageManager.read(from: revisionFilename) else { + return dataProvider.embeddedRevision + } + let decoder = JSONDecoder() + do { + let loadedRevision = try decoder.decode(Int.self, from: data) + if loadedRevision < dataProvider.embeddedRevision { + return dataProvider.embeddedRevision + } + return loadedRevision + } catch { + Logger.phishingDetectionDataStore.error("Error decoding \(self.revisionFilename): \(error.localizedDescription)") + return dataProvider.embeddedRevision + } + } } extension PhishingDetectionDataStore { From 1549f59148797191d4fe578bd77d4740a52f4b6b Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Tue, 22 Oct 2024 16:45:56 +0100 Subject: [PATCH 3/8] Add test for embedded phishing dataset updates --- .../PhishingDetectionDataStoreTests.swift | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift b/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift index 95a51eb05..7ece419cb 100644 --- a/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift +++ b/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift @@ -60,6 +60,22 @@ class PhishingDetectionDataStoreTests: XCTestCase { XCTAssertEqual(actualFilterSet, expectedFilerSet) XCTAssertEqual(actualHashPrefix, expectedHashPrefix) } + + func testWhenEmbeddedRevisionNewerThanOnDisk_ThenLoadEmbedded() async { + mockDataProvider.embeddedRevision = 5 + let expectedFilerSet = Set([Filter(hashValue: "some", regex: "some")]) + let expectedHashPrefix = Set(["sassa"]) + mockDataProvider.shouldReturnFilterSet(set: expectedFilerSet) + mockDataProvider.shouldReturnHashPrefixes(set: expectedHashPrefix) + + let actualRevision = dataStore.currentRevision + let actualFilterSet = dataStore.filterSet + let actualHashPrefix = dataStore.hashPrefixes + + XCTAssertEqual(actualFilterSet, expectedFilerSet) + XCTAssertEqual(actualHashPrefix, expectedHashPrefix) + XCTAssertEqual(actualRevision, 5) + } func testWriteAndLoadData() async { // Get and write data From ef00346d54779421c820ccc39174c18f2f004f01 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Tue, 22 Oct 2024 17:01:53 +0100 Subject: [PATCH 4/8] Swiftlint fix --- .../PhishingDetectionDataStoreTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift b/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift index 7ece419cb..189eed8fc 100644 --- a/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift +++ b/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift @@ -60,14 +60,14 @@ class PhishingDetectionDataStoreTests: XCTestCase { XCTAssertEqual(actualFilterSet, expectedFilerSet) XCTAssertEqual(actualHashPrefix, expectedHashPrefix) } - + func testWhenEmbeddedRevisionNewerThanOnDisk_ThenLoadEmbedded() async { mockDataProvider.embeddedRevision = 5 let expectedFilerSet = Set([Filter(hashValue: "some", regex: "some")]) let expectedHashPrefix = Set(["sassa"]) mockDataProvider.shouldReturnFilterSet(set: expectedFilerSet) mockDataProvider.shouldReturnHashPrefixes(set: expectedHashPrefix) - + let actualRevision = dataStore.currentRevision let actualFilterSet = dataStore.filterSet let actualHashPrefix = dataStore.hashPrefixes From eac223786df7c9e6c39c2de9220772487095a25a Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 28 Oct 2024 11:58:34 +0000 Subject: [PATCH 5/8] Undo remove caching --- Sources/PhishingDetection/PhishingDetectionClient.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/PhishingDetection/PhishingDetectionClient.swift b/Sources/PhishingDetection/PhishingDetectionClient.swift index 2293f68a0..942075b71 100644 --- a/Sources/PhishingDetection/PhishingDetectionClient.swift +++ b/Sources/PhishingDetection/PhishingDetectionClient.swift @@ -161,7 +161,6 @@ extension PhishingDetectionAPIClient { var request = URLRequest(url: url) request.httpMethod = "GET" request.allHTTPHeaderFields = headers - request.cachePolicy = .reloadIgnoringLocalCacheData // Disable caching do { let (data, _) = try await session.data(for: request) From cda34acd538347c80d4a08db8d4f6425fd86f903 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 28 Oct 2024 12:42:08 +0000 Subject: [PATCH 6/8] Add more explicit test cases for onDisk and embedded datasets --- .../PhishingDetectionDataStoreTests.swift | 50 ++++++++++++++++--- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift b/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift index 189eed8fc..cb641602b 100644 --- a/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift +++ b/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift @@ -62,20 +62,58 @@ class PhishingDetectionDataStoreTests: XCTestCase { } func testWhenEmbeddedRevisionNewerThanOnDisk_ThenLoadEmbedded() async { + let encoder = JSONEncoder() + // On Disk Data Setup + fileStorageManager.write(data: "1".utf8data, to: "revision.txt") + let onDiskFilterSet = Set([Filter(hashValue: "other", regex: "other")]) + let filterSetData = try! encoder.encode(Array(onDiskFilterSet)) + let onDiskHashPrefix = Set(["faffa"]) + let hashPrefixData = try! encoder.encode(Array(onDiskHashPrefix)) + fileStorageManager.write(data: filterSetData, to: "filterSet.json") + fileStorageManager.write(data: hashPrefixData, to: "hashPrefixes.json") + + // Embedded Data Setup mockDataProvider.embeddedRevision = 5 - let expectedFilerSet = Set([Filter(hashValue: "some", regex: "some")]) - let expectedHashPrefix = Set(["sassa"]) - mockDataProvider.shouldReturnFilterSet(set: expectedFilerSet) - mockDataProvider.shouldReturnHashPrefixes(set: expectedHashPrefix) + let embeddedFilterSet = Set([Filter(hashValue: "some", regex: "some")]) + let embeddedHashPrefix = Set(["sassa"]) + mockDataProvider.shouldReturnFilterSet(set: embeddedFilterSet) + mockDataProvider.shouldReturnHashPrefixes(set: embeddedHashPrefix) let actualRevision = dataStore.currentRevision let actualFilterSet = dataStore.filterSet let actualHashPrefix = dataStore.hashPrefixes - XCTAssertEqual(actualFilterSet, expectedFilerSet) - XCTAssertEqual(actualHashPrefix, expectedHashPrefix) + XCTAssertEqual(actualFilterSet, embeddedFilterSet) + XCTAssertEqual(actualHashPrefix, embeddedHashPrefix) XCTAssertEqual(actualRevision, 5) } + + func testWhenEmbeddedRevisionOlderThanOnDisk_ThenDontLoadEmbedded() async { + let encoder = JSONEncoder() + // On Disk Data Setup + fileStorageManager.write(data: "6".utf8data, to: "revision.txt") + let onDiskFilterSet = Set([Filter(hashValue: "other", regex: "other")]) + let filterSetData = try! encoder.encode(Array(onDiskFilterSet)) + let onDiskHashPrefix = Set(["faffa"]) + let hashPrefixData = try! encoder.encode(Array(onDiskHashPrefix)) + fileStorageManager.write(data: filterSetData, to: "filterSet.json") + fileStorageManager.write(data: hashPrefixData, to: "hashPrefixes.json") + + // Embedded Data Setup + mockDataProvider.embeddedRevision = 1 + let embeddedFilterSet = Set([Filter(hashValue: "some", regex: "some")]) + let embeddedHashPrefix = Set(["sassa"]) + mockDataProvider.shouldReturnFilterSet(set: embeddedFilterSet) + mockDataProvider.shouldReturnHashPrefixes(set: embeddedHashPrefix) + + let actualRevision = dataStore.currentRevision + let actualFilterSet = dataStore.filterSet + let actualHashPrefix = dataStore.hashPrefixes + + XCTAssertEqual(actualFilterSet, onDiskFilterSet) + XCTAssertEqual(actualHashPrefix, onDiskHashPrefix) + XCTAssertEqual(actualRevision, 6) + } func testWriteAndLoadData() async { // Get and write data From 9b9d038efbc6f5df569c61a1ebd221545e7f901c Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 28 Oct 2024 13:59:08 +0000 Subject: [PATCH 7/8] Ensure lazy-loading is not returning stale data --- .../PhishingDetectionDataStore.swift | 6 +-- .../PhishingDetectionDataStoreTests.swift | 47 +++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/Sources/PhishingDetection/PhishingDetectionDataStore.swift b/Sources/PhishingDetection/PhishingDetectionDataStore.swift index 0894b3dc6..f247f90b8 100644 --- a/Sources/PhishingDetection/PhishingDetectionDataStore.swift +++ b/Sources/PhishingDetection/PhishingDetectionDataStore.swift @@ -202,18 +202,18 @@ public class PhishingDetectionDataStore: PhishingDetectionDataSaving { extension PhishingDetectionDataStore { public func saveFilterSet(set: Set) { - writeFilterSet() self.filterSet = set + writeFilterSet() } public func saveHashPrefixes(set: Set) { - writeHashPrefixes() self.hashPrefixes = set + writeHashPrefixes() } public func saveRevision(_ revision: Int) { - writeRevision() self.currentRevision = revision + writeRevision() } } diff --git a/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift b/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift index cb641602b..82dda6f19 100644 --- a/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift +++ b/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift @@ -147,4 +147,51 @@ class PhishingDetectionDataStoreTests: XCTestCase { XCTFail("Failed to decode stored PhishingDetection data") } } + + func testLazyLoadingDoesNotReturnStaleData() async { + clearDatasets() + + // Set up initial data + let initialFilterSet = Set([Filter(hashValue: "initial", regex: "initial")]) + let initialHashPrefixes = Set(["initialPrefix"]) + mockDataProvider.shouldReturnFilterSet(set: initialFilterSet) + mockDataProvider.shouldReturnHashPrefixes(set: initialHashPrefixes) + + // Access the lazy-loaded properties to trigger loading + let loadedFilterSet = dataStore.filterSet + let loadedHashPrefixes = dataStore.hashPrefixes + + // Validate loaded data matches initial data + XCTAssertEqual(loadedFilterSet, initialFilterSet) + XCTAssertEqual(loadedHashPrefixes, initialHashPrefixes) + + // Update in-memory data + let updatedFilterSet = Set([Filter(hashValue: "updated", regex: "updated")]) + let updatedHashPrefixes = Set(["updatedPrefix"]) + dataStore.saveFilterSet(set: updatedFilterSet) + dataStore.saveHashPrefixes(set: updatedHashPrefixes) + + // Access lazy-loaded properties again + let reloadedFilterSet = dataStore.filterSet + let reloadedHashPrefixes = dataStore.hashPrefixes + + // Validate reloaded data matches updated data + XCTAssertEqual(reloadedFilterSet, updatedFilterSet) + XCTAssertEqual(reloadedHashPrefixes, updatedHashPrefixes) + + // Validate on-disk data is also updated + let storedFilterSetData = fileStorageManager.read(from: "filterSet.json") + let storedHashPrefixesData = fileStorageManager.read(from: "hashPrefixes.json") + + let decoder = JSONDecoder() + if let storedFilterSet = try? decoder.decode(Set.self, from: storedFilterSetData!), + let storedHashPrefixes = try? decoder.decode(Set.self, from: storedHashPrefixesData!) { + + XCTAssertEqual(storedFilterSet, updatedFilterSet) + XCTAssertEqual(storedHashPrefixes, updatedHashPrefixes) + } else { + XCTFail("Failed to decode stored PhishingDetection data after update") + } + } + } From 07a4dee445cf92727578adb73210cc4d5d222c77 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 1 Nov 2024 10:51:05 +0000 Subject: [PATCH 8/8] Swiftlint fix --- .../PhishingDetectionDataStoreTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift b/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift index 82dda6f19..79e9fb500 100644 --- a/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift +++ b/Tests/PhishingDetectionTests/PhishingDetectionDataStoreTests.swift @@ -87,7 +87,7 @@ class PhishingDetectionDataStoreTests: XCTestCase { XCTAssertEqual(actualHashPrefix, embeddedHashPrefix) XCTAssertEqual(actualRevision, 5) } - + func testWhenEmbeddedRevisionOlderThanOnDisk_ThenDontLoadEmbedded() async { let encoder = JSONEncoder() // On Disk Data Setup @@ -186,7 +186,7 @@ class PhishingDetectionDataStoreTests: XCTestCase { let decoder = JSONDecoder() if let storedFilterSet = try? decoder.decode(Set.self, from: storedFilterSetData!), let storedHashPrefixes = try? decoder.decode(Set.self, from: storedHashPrefixesData!) { - + XCTAssertEqual(storedFilterSet, updatedFilterSet) XCTAssertEqual(storedHashPrefixes, updatedHashPrefixes) } else {