From 4d3f2bf3333552f77f702610267058b24db7936b Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Mon, 6 Jan 2025 11:21:59 +0100 Subject: [PATCH 1/4] Wrapper for Defaults --- Model/CategoriesToLanguage.swift | 30 +++++++++++++---------- Model/Defaulting.swift | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 Model/Defaulting.swift diff --git a/Model/CategoriesToLanguage.swift b/Model/CategoriesToLanguage.swift index 030409d3..2b6f458f 100644 --- a/Model/CategoriesToLanguage.swift +++ b/Model/CategoriesToLanguage.swift @@ -13,17 +13,23 @@ // You should have received a copy of the GNU General Public License // along with Kiwix; If not, see https://www.gnu.org/licenses/. -// -// CategoriesToLanguage.swift -// Kiwix -// - import Foundation -import Defaults -struct CategoriesToLanguages { +protocol CategoriesProtocol { + func has(category: Category, inLanguages langCodes: Set) -> Bool + func save(_ dictionary: [Category: Set]) + func allCategories() -> [Category] +} - private let dictionary: [Category: Set] = Defaults[.categoriesToLanguages] +struct CategoriesToLanguages: CategoriesProtocol { + + private let defaults: Defaulting + private let dictionary: [Category: Set] + + init(withDefaults defaults: Defaulting = UDefaults()) { + self.defaults = defaults + self.dictionary = defaults[.categoriesToLanguages] + } func has(category: Category, inLanguages langCodes: Set) -> Bool { guard !langCodes.isEmpty, !dictionary.isEmpty else { @@ -35,13 +41,13 @@ struct CategoriesToLanguages { return !languages.isDisjoint(with: langCodes) } - static func save(_ dictionary: [Category: Set]) { - Defaults[.categoriesToLanguages] = dictionary + func save(_ dictionary: [Category: Set]) { + defaults[.categoriesToLanguages] = dictionary } - static func allCategories() -> [Category] { + func allCategories() -> [Category] { let categoriesToLanguages = CategoriesToLanguages() - let contentLanguages = Defaults[.libraryLanguageCodes] + let contentLanguages = defaults[.libraryLanguageCodes] return Category.allCases.filter { (category: Category) in categoriesToLanguages.has(category: category, inLanguages: contentLanguages) } diff --git a/Model/Defaulting.swift b/Model/Defaulting.swift new file mode 100644 index 00000000..a120ed47 --- /dev/null +++ b/Model/Defaulting.swift @@ -0,0 +1,41 @@ +// This file is part of Kiwix for iOS & macOS. +// +// Kiwix is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by +// the Free Software Foundation; either version 3 of the License, or +// any later version. +// +// Kiwix is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Kiwix; If not, see https://www.gnu.org/licenses/. + +import Foundation +import Defaults + +protocol Defaulting: NSObjectProtocol { + subscript(key: Defaults.Key) -> Value { get set } +} + +final class UDefaults: NSObject, Defaulting { + subscript(key: Defaults.Key) -> Value where Value : DefaultsSerializable { + get { + Defaults[key] + } + set { + Defaults[key] = newValue + } + } +} + +/* + static subscript(key: Key) -> Value { + get { key.suite[key] } + set { + key.suite[key] = newValue + } + } + */ From 82d590b9c1bd100d60f82abdc2501237b95dc179 Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Mon, 6 Jan 2025 16:53:36 +0100 Subject: [PATCH 2/4] Wrap user defaults to stabilize unit tests --- Model/CategoriesToLanguage.swift | 3 +- Model/Defaulting.swift | 4 +-- SwiftUI/Model/Enum.swift | 2 +- Tests/LibraryRefreshViewModelTest.swift | 43 ++++++++++++++++++++---- Tests/TestDefaults.swift | 41 +++++++++++++++++++++++ ViewModel/LibraryViewModel.swift | 44 +++++++++++++++---------- Views/Library/Library.swift | 7 ++-- Views/Library/ZimFilesCategories.swift | 7 ++-- 8 files changed, 117 insertions(+), 34 deletions(-) create mode 100644 Tests/TestDefaults.swift diff --git a/Model/CategoriesToLanguage.swift b/Model/CategoriesToLanguage.swift index 2b6f458f..b70227ed 100644 --- a/Model/CategoriesToLanguage.swift +++ b/Model/CategoriesToLanguage.swift @@ -46,10 +46,9 @@ struct CategoriesToLanguages: CategoriesProtocol { } func allCategories() -> [Category] { - let categoriesToLanguages = CategoriesToLanguages() let contentLanguages = defaults[.libraryLanguageCodes] return Category.allCases.filter { (category: Category) in - categoriesToLanguages.has(category: category, inLanguages: contentLanguages) + has(category: category, inLanguages: contentLanguages) } } } diff --git a/Model/Defaulting.swift b/Model/Defaulting.swift index a120ed47..11a62373 100644 --- a/Model/Defaulting.swift +++ b/Model/Defaulting.swift @@ -16,12 +16,12 @@ import Foundation import Defaults -protocol Defaulting: NSObjectProtocol { +public protocol Defaulting: NSObjectProtocol { subscript(key: Defaults.Key) -> Value { get set } } final class UDefaults: NSObject, Defaulting { - subscript(key: Defaults.Key) -> Value where Value : DefaultsSerializable { + subscript(key: Defaults.Key) -> Value where Value: DefaultsSerializable { get { Defaults[key] } diff --git a/SwiftUI/Model/Enum.swift b/SwiftUI/Model/Enum.swift index d7bf55bd..271cc144 100644 --- a/SwiftUI/Model/Enum.swift +++ b/SwiftUI/Model/Enum.swift @@ -36,7 +36,7 @@ enum ActiveSheet: Hashable, Identifiable { case safari(url: URL) } -enum Category: String, CaseIterable, Identifiable, LosslessStringConvertible { +enum Category: String, CaseIterable, Identifiable, LosslessStringConvertible, Hashable { var description: String { rawValue } var id: String { rawValue } diff --git a/Tests/LibraryRefreshViewModelTest.swift b/Tests/LibraryRefreshViewModelTest.swift index 4b1acece..384f188c 100644 --- a/Tests/LibraryRefreshViewModelTest.swift +++ b/Tests/LibraryRefreshViewModelTest.swift @@ -60,6 +60,7 @@ final class LibraryRefreshViewModelTest: XCTestCase { } private func makeOPDSData(zimFileID: UUID) -> String { + // swiftlint:disable line_length """ """ + // swiftlint:enable line_length } /// Test time out fetching library data. @@ -96,9 +98,12 @@ final class LibraryRefreshViewModelTest: XCTestCase { HTTPTestingURLProtocol.handler = { urlProtocol in urlProtocol.client?.urlProtocol(urlProtocol, didFailWithError: URLError(URLError.Code.timedOut)) } - + let testDefaults = TestDefaults() + testDefaults.setup() let viewModel = LibraryViewModel(urlSession: urlSession, - processFactory: { LibraryProcess() }) + processFactory: { LibraryProcess(defaultState: .initial) }, + defaults: testDefaults, + categories: CategoriesToLanguages(withDefaults: testDefaults)) await viewModel.start(isUserInitiated: true) XCTAssert(viewModel.error is LibraryRefreshError) XCTAssertEqual( @@ -119,8 +124,12 @@ final class LibraryRefreshViewModelTest: XCTestCase { urlProtocol.client?.urlProtocolDidFinishLoading(urlProtocol) } + let testDefaults = TestDefaults() + testDefaults.setup() let viewModel = LibraryViewModel(urlSession: urlSession, - processFactory: { LibraryProcess() }) + processFactory: { LibraryProcess(defaultState: .initial) }, + defaults: testDefaults, + categories: CategoriesToLanguages(withDefaults: testDefaults)) await viewModel.start(isUserInitiated: true) XCTAssert(viewModel.error is LibraryRefreshError) XCTAssertEqual( @@ -142,8 +151,12 @@ final class LibraryRefreshViewModelTest: XCTestCase { urlProtocol.client?.urlProtocolDidFinishLoading(urlProtocol) } + let testDefaults = TestDefaults() + testDefaults.setup() let viewModel = LibraryViewModel(urlSession: urlSession, - processFactory: { LibraryProcess() }) + processFactory: { LibraryProcess(defaultState: .initial) }, + defaults: testDefaults, + categories: CategoriesToLanguages(withDefaults: testDefaults)) await viewModel.start(isUserInitiated: true) XCTExpectFailure("Requires work in dependency to resolve the issue.") XCTAssertEqual( @@ -154,6 +167,7 @@ final class LibraryRefreshViewModelTest: XCTestCase { /// Test zim file entity is created, and metadata are saved when new zim file becomes available in online catalog. @MainActor + // swiftlint:disable:next function_body_length func testNewZimFileAndProperties() async throws { let zimFileID = UUID() HTTPTestingURLProtocol.handler = { urlProtocol in @@ -166,9 +180,12 @@ final class LibraryRefreshViewModelTest: XCTestCase { urlProtocol.client?.urlProtocol(urlProtocol, didReceive: response, cacheStoragePolicy: .notAllowed) urlProtocol.client?.urlProtocolDidFinishLoading(urlProtocol) } - + let testDefaults = TestDefaults() + testDefaults.setup() let viewModel = LibraryViewModel(urlSession: urlSession, - processFactory: { LibraryProcess() }) + processFactory: { LibraryProcess(defaultState: .initial) }, + defaults: testDefaults, + categories: CategoriesToLanguages(withDefaults: testDefaults)) await viewModel.start(isUserInitiated: true) // check no error has happened @@ -185,6 +202,7 @@ final class LibraryRefreshViewModelTest: XCTestCase { XCTAssertEqual(zimFile.id, zimFileID) XCTAssertEqual(zimFile.articleCount, 50001) XCTAssertEqual(zimFile.category, Category.wikipedia.rawValue) + // swiftlint:disable:next force_try XCTAssertEqual(zimFile.created, try! Date("2023-01-07T00:00:00Z", strategy: .iso8601)) XCTAssertEqual( zimFile.downloadURL, @@ -211,14 +229,21 @@ final class LibraryRefreshViewModelTest: XCTestCase { XCTAssertEqual(zimFile.persistentID, "wikipedia_en_top") XCTAssertEqual(zimFile.requiresServiceWorkers, false) XCTAssertEqual(zimFile.size, 6515656704) + + // clean up + context.delete(zimFile) } /// Test zim file deprecation @MainActor func testZimFileDeprecation() async throws { + let testDefaults = TestDefaults() + testDefaults.setup() // refresh library for the first time, which should create one zim file let viewModel = LibraryViewModel(urlSession: urlSession, - processFactory: { LibraryProcess() }) + processFactory: { LibraryProcess(defaultState: .initial) }, + defaults: testDefaults, + categories: CategoriesToLanguages(withDefaults: testDefaults)) await viewModel.start(isUserInitiated: true) let context = Database.shared.viewContext let zimFile1 = try XCTUnwrap(try context.fetch(ZimFile.fetchRequest()).first) @@ -241,6 +266,10 @@ final class LibraryRefreshViewModelTest: XCTestCase { // check there are two zim files in the database, and zim file 2 is not deprecated XCTAssertEqual(zimFiles.count, 2) XCTAssertEqual(zimFiles.filter({ $0.fileID == zimFile2.fileID }).count, 1) + + // clean up + context.delete(zimFile1) + context.delete(zimFile2) } } diff --git a/Tests/TestDefaults.swift b/Tests/TestDefaults.swift new file mode 100644 index 00000000..c122614d --- /dev/null +++ b/Tests/TestDefaults.swift @@ -0,0 +1,41 @@ +// This file is part of Kiwix for iOS & macOS. +// +// Kiwix is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by +// the Free Software Foundation; either version 3 of the License, or +// any later version. +// +// Kiwix is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Kiwix; If not, see https://www.gnu.org/licenses/. + +import Foundation +import Defaults +@testable import Kiwix + +final class TestDefaults: NSObject, Defaulting { + + var dict: [Defaults.AnyKey: any DefaultsSerializable] = [:] + + func setup() { + self[.categoriesToLanguages] = [:] + self[.libraryAutoRefresh] = false + self[.libraryETag] = "" + self[.libraryUsingOldISOLangCodes] = false + self[.libraryLanguageCodes] = Set() + } + + subscript(key: Defaults.Key) -> Value where Value: DefaultsSerializable { + get { + // swiftlint:disable:next force_cast + dict[key] as! Value + } + set { + dict[key] = newValue + } + } +} diff --git a/ViewModel/LibraryViewModel.swift b/ViewModel/LibraryViewModel.swift index e277568f..ce0d6649 100644 --- a/ViewModel/LibraryViewModel.swift +++ b/ViewModel/LibraryViewModel.swift @@ -15,7 +15,6 @@ import CoreData import Combine -import Defaults import os enum LibraryState { @@ -24,8 +23,8 @@ enum LibraryState { case complete case error - static func defaultState() -> LibraryState { - if Defaults[.libraryLastRefresh] == nil { + static func defaultState(defaults: Defaulting = UDefaults()) -> LibraryState { + if defaults[.libraryLastRefresh] == nil { return .initial } else { return .complete @@ -52,6 +51,8 @@ final class LibraryViewModel: ObservableObject { @MainActor @Published var state: LibraryState @MainActor private let process: LibraryProcess private var cancellables = Set() + private let defaults: Defaulting + private let categories: CategoriesProtocol private let urlSession: URLSession private var insertionCount = 0 @@ -60,9 +61,16 @@ final class LibraryViewModel: ObservableObject { private static let catalogURL = URL(string: "https://library.kiwix.org/catalog/v2/entries?count=-1")! @MainActor - init(urlSession: URLSession? = nil, processFactory: @MainActor () -> LibraryProcess = { .shared }) { + init( + urlSession: URLSession? = nil, + processFactory: @MainActor () -> LibraryProcess = { .shared }, + defaults: Defaulting = UDefaults(), + categories: CategoriesProtocol = CategoriesToLanguages(withDefaults: UDefaults()) + ) { self.urlSession = urlSession ?? URLSession.shared self.process = processFactory() + self.defaults = defaults + self.categories = categories state = process.state process.$state.sink { [weak self] newState in self?.state = newState @@ -78,8 +86,8 @@ final class LibraryViewModel: ObservableObject { guard process.state != .inProgress else { return } do { // decide if refresh should proceed - let lastRefresh: Date? = Defaults[.libraryLastRefresh] - let hasAutoRefresh: Bool = Defaults[.libraryAutoRefresh] + let lastRefresh: Date? = defaults[.libraryLastRefresh] + let hasAutoRefresh: Bool = defaults[.libraryAutoRefresh] let isStale = (lastRefresh?.timeIntervalSinceNow ?? -3600) <= -3600 guard isUserInitiated || (hasAutoRefresh && isStale) else { return } @@ -90,7 +98,7 @@ final class LibraryViewModel: ObservableObject { // this is the case when we have no new data (304 http) // but we still need to refresh the memory only stored // zimfile categories to languages dictionary - if CategoriesToLanguages.allCategories().count < 2 { + if categories.allCategories().count < 2 { let context = Database.shared.viewContext if let zimFiles: [ZimFile] = try? context.fetch(ZimFile.fetchRequest()) { saveCategoryAvailableInLanguages(fromDBZimFiles: zimFiles) @@ -113,15 +121,15 @@ final class LibraryViewModel: ObservableObject { } let parser = try await parse(data: data) // delete all old ISO Lang Code entries if needed, by passing in an empty parser - if Defaults[.libraryUsingOldISOLangCodes] { + if defaults[.libraryUsingOldISOLangCodes] { try await process(parser: DeletingParser()) - Defaults[.libraryUsingOldISOLangCodes] = false + defaults[.libraryUsingOldISOLangCodes] = false } // process the feed try await process(parser: parser) // update library last refresh timestamp - Defaults[.libraryLastRefresh] = Date() + defaults[.libraryLastRefresh] = Date() saveCategoryAvailableInLanguages(using: parser) @@ -156,7 +164,7 @@ final class LibraryViewModel: ObservableObject { dictionary[category] = allLanguagesForCategory } } - CategoriesToLanguages.save(dictionary) + categories.save(dictionary) } private func saveCategoryAvailableInLanguages(fromDBZimFiles zimFiles: [ZimFile]) { @@ -172,7 +180,7 @@ final class LibraryViewModel: ObservableObject { } dictionary[category] = allLanguagesForCategory } - CategoriesToLanguages.save(dictionary) + categories.save(dictionary) } /// The fetched content is filtered by the languages set in settings. @@ -183,10 +191,10 @@ final class LibraryViewModel: ObservableObject { let validCodes = Set(languages.map { $0.code }) // preserve only valid selections by: // converting earlier user selections, and filtering out invalid ones - Defaults[.libraryLanguageCodes] = LanguagesConverter.convert(codes: Defaults[.libraryLanguageCodes], + defaults[.libraryLanguageCodes] = LanguagesConverter.convert(codes: defaults[.libraryLanguageCodes], validCodes: validCodes) - guard Defaults[.libraryLanguageCodes].isEmpty else { + guard defaults[.libraryLanguageCodes].isEmpty else { return // what was earlier set by the user or picked by default is valid } @@ -201,22 +209,22 @@ final class LibraryViewModel: ObservableObject { let deviceLangSet = Set([deviceLang].compactMap { $0 }) let validDefaults = LanguagesConverter.convert(codes: deviceLangSet, validCodes: validCodes) if validDefaults.isEmpty { // meaning the device language isn't valid (or nil) - Defaults[.libraryLanguageCodes] = [fallbackToEnglish] + defaults[.libraryLanguageCodes] = [fallbackToEnglish] } else { - Defaults[.libraryLanguageCodes] = validDefaults + defaults[.libraryLanguageCodes] = validDefaults } } private func fetchData() async throws -> Data? { do { var request = URLRequest(url: Self.catalogURL, timeoutInterval: 20) - request.allHTTPHeaderFields = ["If-None-Match": Defaults[.libraryETag]] + request.allHTTPHeaderFields = ["If-None-Match": defaults[.libraryETag]] let (data, response) = try await self.urlSession.data(for: request) guard let response = response as? HTTPURLResponse else { return nil } switch response.statusCode { case 200: if let eTag = response.allHeaderFields["Etag"] as? String { - Defaults[.libraryETag] = eTag + defaults[.libraryETag] = eTag } // OK to process further os_log("Retrieved OPDS Data, size: %llu bytes", log: Log.OPDS, type: .info, data.count) diff --git a/Views/Library/Library.swift b/Views/Library/Library.swift index 78d0da67..9a562c99 100644 --- a/Views/Library/Library.swift +++ b/Views/Library/Library.swift @@ -26,9 +26,12 @@ struct Library: View { private let categories: [Category] let dismiss: (() -> Void)? - init(dismiss: (() -> Void)?) { + init( + dismiss: (() -> Void)?, + categories: [Category] = CategoriesToLanguages().allCategories() + ) { self.dismiss = dismiss - categories = CategoriesToLanguages.allCategories() + self.categories = categories } var body: some View { diff --git a/Views/Library/ZimFilesCategories.swift b/Views/Library/ZimFilesCategories.swift index 29226601..13edeed1 100644 --- a/Views/Library/ZimFilesCategories.swift +++ b/Views/Library/ZimFilesCategories.swift @@ -24,8 +24,11 @@ struct ZimFilesCategories: View { private var categories: [Category] private let dismiss: (() -> Void)? - init(dismiss: (() -> Void)?) { - categories = CategoriesToLanguages.allCategories() + init( + dismiss: (() -> Void)?, + categories: [Category] = CategoriesToLanguages().allCategories() + ) { + self.categories = categories selected = categories.first ?? .wikipedia self.dismiss = dismiss } From fe8a3e63a08d4d61ebed1a4697d07254c014d3e5 Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Mon, 6 Jan 2025 17:01:12 +0100 Subject: [PATCH 3/4] Clean up --- Model/Defaulting.swift | 9 --------- 1 file changed, 9 deletions(-) diff --git a/Model/Defaulting.swift b/Model/Defaulting.swift index 11a62373..795ef8df 100644 --- a/Model/Defaulting.swift +++ b/Model/Defaulting.swift @@ -30,12 +30,3 @@ final class UDefaults: NSObject, Defaulting { } } } - -/* - static subscript(key: Key) -> Value { - get { key.suite[key] } - set { - key.suite[key] = newValue - } - } - */ From 68508dc49c2a4a2128b8e7e58d9247201ce89bab Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Mon, 6 Jan 2025 17:04:39 +0100 Subject: [PATCH 4/4] Fixlint --- Tests/LibraryRefreshViewModelTest.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/LibraryRefreshViewModelTest.swift b/Tests/LibraryRefreshViewModelTest.swift index 384f188c..c003f5e8 100644 --- a/Tests/LibraryRefreshViewModelTest.swift +++ b/Tests/LibraryRefreshViewModelTest.swift @@ -146,7 +146,7 @@ final class LibraryRefreshViewModelTest: XCTestCase { url: URL.mock(), statusCode: 200, httpVersion: nil, headerFields: [:] )! - urlProtocol.client?.urlProtocol(urlProtocol, didLoad: "Invalid OPDS Data".data(using: .utf8)!) + urlProtocol.client?.urlProtocol(urlProtocol, didLoad: Data("Invalid OPDS Data".utf8)) urlProtocol.client?.urlProtocol(urlProtocol, didReceive: response, cacheStoragePolicy: .notAllowed) urlProtocol.client?.urlProtocolDidFinishLoading(urlProtocol) } @@ -256,7 +256,7 @@ final class LibraryRefreshViewModelTest: XCTestCase { XCTAssertNotEqual(zimFile1.fileID, zimFile2.fileID) // set fileURLBookmark of zim file 2 - zimFile2.fileURLBookmark = "/Users/tester/Downloads/file_url.zim".data(using: .utf8) + zimFile2.fileURLBookmark = Data("/Users/tester/Downloads/file_url.zim".utf8) try context.save() // refresh library for the third time