From e2a957c1dca96227c4929267b7c25298a32a9bad Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Thu, 8 May 2025 19:13:58 +0200 Subject: [PATCH 1/4] Library endpoint changes --- Model/OPDSParser/OPDSParser.h | 2 +- Model/OPDSParser/OPDSParser.mm | 4 ++-- Model/OPDSParser/OPDSParser.swift | 8 ++++---- Model/Utilities/String+Extension.swift | 5 +++++ Model/Utilities/URL.swift | 18 ++++++++++++++++++ Tests/LibraryRefreshViewModelTest.swift | 2 +- Tests/OPDSParserTests.swift | 6 +++--- ViewModel/LibraryViewModel.swift | 22 ++++++++++++++-------- Views/BuildingBlocks/Favicon.swift | 2 +- 9 files changed, 49 insertions(+), 20 deletions(-) diff --git a/Model/OPDSParser/OPDSParser.h b/Model/OPDSParser/OPDSParser.h index ae6b6945..19af30d2 100644 --- a/Model/OPDSParser/OPDSParser.h +++ b/Model/OPDSParser/OPDSParser.h @@ -21,7 +21,7 @@ NS_ASSUME_NONNULL_BEGIN @interface OPDSParser : NSObject - (nonnull instancetype)init; -- (BOOL)parseData:(nonnull NSData *)data NS_REFINED_FOR_SWIFT; +- (BOOL)parseData:(nonnull NSData *)data using: (nonnull NSString *)urlHost NS_REFINED_FOR_SWIFT; - (nonnull NSSet *)getZimFileIDs NS_REFINED_FOR_SWIFT; - (nullable ZimFileMetaData *)getZimFileMetaData:(nonnull NSUUID *)identifier NS_REFINED_FOR_SWIFT; diff --git a/Model/OPDSParser/OPDSParser.mm b/Model/OPDSParser/OPDSParser.mm index fe8c76c1..6a37c7aa 100644 --- a/Model/OPDSParser/OPDSParser.mm +++ b/Model/OPDSParser/OPDSParser.mm @@ -39,7 +39,7 @@ return self; } -- (BOOL)parseData:(nonnull NSData *)data { +- (BOOL)parseData:(nonnull NSData *)data using: (nonnull NSString *)urlHost { try { NSString *content = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; if (content == nil) { @@ -47,7 +47,7 @@ } std::shared_ptr manager = std::make_shared(self.library); return manager->readOpds([content cStringUsingEncoding:NSUTF8StringEncoding], - [@"https://library.kiwix.org" cStringUsingEncoding:NSUTF8StringEncoding]); + [urlHost cStringUsingEncoding:NSUTF8StringEncoding]); } catch (std::exception) { return false; } diff --git a/Model/OPDSParser/OPDSParser.swift b/Model/OPDSParser/OPDSParser.swift index 5edbd842..b47db87b 100644 --- a/Model/OPDSParser/OPDSParser.swift +++ b/Model/OPDSParser/OPDSParser.swift @@ -15,7 +15,7 @@ protocol Parser { var zimFileIDs: Set { get } - func parse(data: Data) throws + func parse(data: Data, urlHost: String) throws func getMetaData(id: UUID) -> ZimFileMetaData? } @@ -24,8 +24,8 @@ extension OPDSParser: Parser { __getZimFileIDs() as? Set ?? Set() } - func parse(data: Data) throws { - if !self.__parseData(data) { + func parse(data: Data, urlHost: String) throws { + if !self.__parseData(data, using: urlHost) { throw LibraryRefreshError.parse } } @@ -42,7 +42,7 @@ extension OPDSParser: Parser { struct DeletingParser: Parser { let zimFileIDs: Set = .init() - func parse(data: Data) throws { + func parse(data: Data, urlHost: String) throws { } func getMetaData(id: UUID) -> ZimFileMetaData? { diff --git a/Model/Utilities/String+Extension.swift b/Model/Utilities/String+Extension.swift index 7fcdc86c..366ad14a 100644 --- a/Model/Utilities/String+Extension.swift +++ b/Model/Utilities/String+Extension.swift @@ -21,6 +21,11 @@ extension String { guard hasPrefix(value) else { return self } return String(dropFirst(value.count)) } + + func removingSuffix(_ value: String) -> String { + guard hasSuffix(value) else { return self } + return String(dropLast(value.count)) + } func replacingRegex( matching pattern: String, diff --git a/Model/Utilities/URL.swift b/Model/Utilities/URL.swift index 108ae837..ceac0a0f 100644 --- a/Model/Utilities/URL.swift +++ b/Model/Utilities/URL.swift @@ -95,4 +95,22 @@ extension URL { components.scheme = "zim" return components.url ?? self } + + func trim(pathComponents: [String]) -> URL { + var result = self + for component in pathComponents.reversed() { + if result.lastPathComponent == component { + result = result.deletingLastPathComponent() + } + } + return result.deletingPathExtension() + } + + func withoutQueryParams() -> URL { + guard var components = URLComponents(url: self, resolvingAgainstBaseURL: false) else { + return self + } + components.queryItems = nil + return components.url ?? self + } } diff --git a/Tests/LibraryRefreshViewModelTest.swift b/Tests/LibraryRefreshViewModelTest.swift index c003f5e8..32766213 100644 --- a/Tests/LibraryRefreshViewModelTest.swift +++ b/Tests/LibraryRefreshViewModelTest.swift @@ -172,7 +172,7 @@ final class LibraryRefreshViewModelTest: XCTestCase { let zimFileID = UUID() HTTPTestingURLProtocol.handler = { urlProtocol in let response = HTTPURLResponse( - url: URL.mock(), + url: URL(string: "https://library.kiwix.org/catalog/v2/entries?count=-1")!, statusCode: 200, httpVersion: nil, headerFields: [:] )! let data = self.makeOPDSData(zimFileID: zimFileID).data(using: .utf8)! diff --git a/Tests/OPDSParserTests.swift b/Tests/OPDSParserTests.swift index 106883fc..7727e675 100644 --- a/Tests/OPDSParserTests.swift +++ b/Tests/OPDSParserTests.swift @@ -22,7 +22,7 @@ final class OPDSParserTests: XCTestCase { XCTExpectFailure("Requires work in dependency to resolve the issue.") let content = "Invalid OPDS Data" XCTAssertThrowsError( - try OPDSParser().parse(data: content.data(using: .utf8)!) + try OPDSParser().parse(data: content.data(using: .utf8)!, urlHost: "") ) } @@ -31,7 +31,7 @@ final class OPDSParserTests: XCTestCase { let incompatibleEncodings: [String.Encoding] = [.unicode, .utf16, .utf32] try incompatibleEncodings.forEach { encoding in XCTAssertThrowsError( - try OPDSParser().parse(data: content.data(using: encoding)!), + try OPDSParser().parse(data: content.data(using: encoding)!, urlHost: ""), "parsing with enconding \(encoding.description) should fail" ) } @@ -76,7 +76,7 @@ final class OPDSParserTests: XCTestCase { // Parse data let parser = OPDSParser() - XCTAssertNoThrow(try parser.parse(data: content.data(using: .utf8)!)) + XCTAssertNoThrow(try parser.parse(data: content.data(using: .utf8)!, urlHost: "https://library.kiwix.org")) // check one zim file is populated let zimFileID = UUID(uuidString: "1ec90eab-5724-492b-9529-893959520de4")! diff --git a/ViewModel/LibraryViewModel.swift b/ViewModel/LibraryViewModel.swift index 1cd3b293..3c043204 100644 --- a/ViewModel/LibraryViewModel.swift +++ b/ViewModel/LibraryViewModel.swift @@ -95,7 +95,7 @@ final class LibraryViewModel: ObservableObject { private var insertionCount = 0 private var deletionCount = 0 - private static let catalogURL = URL(string: "https://library.kiwix.org/catalog/v2/entries?count=-1")! + private static let catalogURL = URL(string: "https://opds.library.kiwix.org/v2/entries?count=-1")! @MainActor init( @@ -131,7 +131,7 @@ final class LibraryViewModel: ObservableObject { process.state = .inProgress // refresh library - guard let data = try await fetchData() else { + guard case (var data, let responseURL)? = try await fetchData() else { // 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 @@ -156,7 +156,7 @@ final class LibraryViewModel: ObservableObject { return } } - let parser = try await parse(data: data) + let parser = try await parse(data: data, urlHost: responseURL) // delete all old ISO Lang Code entries if needed, by passing in an empty parser if defaults[.libraryUsingOldISOLangCodes] { try await process(parser: DeletingParser()) @@ -252,7 +252,7 @@ final class LibraryViewModel: ObservableObject { } } - private func fetchData() async throws -> Data? { + private func fetchData() async throws -> (Data, URL)? { do { var request = URLRequest(url: Self.catalogURL, timeoutInterval: 20) request.allHTTPHeaderFields = ["If-None-Match": defaults[.libraryETag]] @@ -260,19 +260,20 @@ final class LibraryViewModel: ObservableObject { guard let response = response as? HTTPURLResponse else { return nil } switch response.statusCode { case 200: + let responseURL = response.url ?? Self.catalogURL if let eTag = response.allHeaderFields["Etag"] as? String { defaults[.libraryETag] = eTag } // OK to process further os_log("Retrieved OPDS Data, size: %llu bytes", log: Log.OPDS, type: .info, data.count) - return data + return (data, responseURL) case 304: return nil // already downloaded default: throw LibraryRefreshError.retrieve(description: "HTTP Status \(response.statusCode).") } } catch { - os_log("Error retrieving OPDS Data: %s", log: Log.OPDS, type: .error) + os_log("Error retrieving OPDS Data: %s", log: Log.OPDS, type: .error, error.localizedDescription) if let error = error as? LibraryRefreshError { throw error } else { @@ -281,11 +282,16 @@ final class LibraryViewModel: ObservableObject { } } - private func parse(data: Data) async throws -> OPDSParser { + private func parse(data: Data, urlHost: URL) async throws -> OPDSParser { try await withCheckedThrowingContinuation { continuation in let parser = OPDSParser() do { - try parser.parse(data: data) + let urlHostString = urlHost + .withoutQueryParams() + .trim(pathComponents: ["catalog", "v2", "entries"]) + .absoluteString + .removingSuffix("/") + try parser.parse(data: data, urlHost: urlHostString) continuation.resume(returning: parser) } catch { continuation.resume(throwing: error) diff --git a/Views/BuildingBlocks/Favicon.swift b/Views/BuildingBlocks/Favicon.swift index 277bd236..ee3a2d42 100644 --- a/Views/BuildingBlocks/Favicon.swift +++ b/Views/BuildingBlocks/Favicon.swift @@ -63,7 +63,7 @@ struct Favicon_Previews: PreviewProvider { category: .wikipedia, imageData: nil, imageURL: URL( - string: "https://library.kiwix.org/meta?name=favicon&content=wikipedia_en_climate_change_maxi_2021-12" + string: "https://library.kiwix.org/catalog/v2/illustration/e82e6816-a2dc-a7f0-2d15-58d24709db93/?size=48" )! ).frame(width: 200, height: 200).previewLayout(.sizeThatFits) Favicon( From a81c612b74f1e246f0737dfa33e749e67efd3a52 Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Fri, 9 May 2025 19:57:35 +0200 Subject: [PATCH 2/4] Add tests and comments --- Model/Utilities/URL.swift | 15 ++++++++++----- Tests/URLContentPathTests.swift | 6 ++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Model/Utilities/URL.swift b/Model/Utilities/URL.swift index ceac0a0f..4c9e14c1 100644 --- a/Model/Utilities/URL.swift +++ b/Model/Utilities/URL.swift @@ -96,16 +96,21 @@ extension URL { return components.url ?? self } + + /// Remove the defined components one by one if found + /// - Parameter pathComponents: eg: /package/details/more can be defined as: ["package", "details", "more"] + /// - Returns: the modified url func trim(pathComponents: [String]) -> URL { var result = self - for component in pathComponents.reversed() { - if result.lastPathComponent == component { - result = result.deletingLastPathComponent() - } + for component in pathComponents.reversed() where component == result.lastPathComponent { + result = result.deletingLastPathComponent() } - return result.deletingPathExtension() + return result } + + /// Removes everything after ? or & + /// - Returns: the modified URL, or the same if it fails to find the components func withoutQueryParams() -> URL { guard var components = URLComponents(url: self, resolvingAgainstBaseURL: false) else { return self diff --git a/Tests/URLContentPathTests.swift b/Tests/URLContentPathTests.swift index 0e4950cf..34310dc2 100644 --- a/Tests/URLContentPathTests.swift +++ b/Tests/URLContentPathTests.swift @@ -43,5 +43,11 @@ final class URLContentPathTests: XCTestCase { "widgets.wp.com/likes/master.html?ver=20240530" ]) } + + func test_trimming() { + let inputURL = URL(string: "https://library.kiwix.org/catalog/v2/entries?count=-1")! + let expectedURL = URL(string: "https://library.kiwix.org/")! + XCTAssertEqual(inputURL.withoutQueryParams().trim(pathComponents: ["catalog", "v2", "entries"]), expectedURL) + } } From a568afa304faaa4ed00cab9f0d374c0171f443d7 Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Fri, 9 May 2025 21:36:10 +0200 Subject: [PATCH 3/4] Replace URLs --- Model/OPDSParser/OPDSParser.swift | 2 +- Tests/LibraryRefreshViewModelTest.swift | 5 +++-- Tests/OPDSParserTests.swift | 12 ++++++++++-- ViewModel/LibraryViewModel.swift | 1 - Views/BuildingBlocks/Favicon.swift | 2 +- 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/Model/OPDSParser/OPDSParser.swift b/Model/OPDSParser/OPDSParser.swift index b47db87b..510b7586 100644 --- a/Model/OPDSParser/OPDSParser.swift +++ b/Model/OPDSParser/OPDSParser.swift @@ -25,7 +25,7 @@ extension OPDSParser: Parser { } func parse(data: Data, urlHost: String) throws { - if !self.__parseData(data, using: urlHost) { + if !self.__parseData(data, using: urlHost.removingSuffix("/")) { throw LibraryRefreshError.parse } } diff --git a/Tests/LibraryRefreshViewModelTest.swift b/Tests/LibraryRefreshViewModelTest.swift index 32766213..3c1535e9 100644 --- a/Tests/LibraryRefreshViewModelTest.swift +++ b/Tests/LibraryRefreshViewModelTest.swift @@ -171,8 +171,9 @@ final class LibraryRefreshViewModelTest: XCTestCase { func testNewZimFileAndProperties() async throws { let zimFileID = UUID() HTTPTestingURLProtocol.handler = { urlProtocol in + let responseTestURL = URL(string: "https://response-testing.com/catalog/v2/entries?count=-1")! let response = HTTPURLResponse( - url: URL(string: "https://library.kiwix.org/catalog/v2/entries?count=-1")!, + url: responseTestURL, statusCode: 200, httpVersion: nil, headerFields: [:] )! let data = self.makeOPDSData(zimFileID: zimFileID).data(using: .utf8)! @@ -211,7 +212,7 @@ final class LibraryRefreshViewModelTest: XCTestCase { XCTAssertNil(zimFile.faviconData) XCTAssertEqual( zimFile.faviconURL, - URL(string: "https://library.kiwix.org/catalog/v2/illustration/1ec90eab-5724-492b-9529-893959520de4/") + URL(string: "https://response-testing.com/catalog/v2/illustration/1ec90eab-5724-492b-9529-893959520de4/") ) XCTAssertEqual(zimFile.fileDescription, "A selection of the best 50,000 Wikipedia articles") XCTAssertEqual(zimFile.fileID, zimFileID) diff --git a/Tests/OPDSParserTests.swift b/Tests/OPDSParserTests.swift index 7727e675..5c71bbd3 100644 --- a/Tests/OPDSParserTests.swift +++ b/Tests/OPDSParserTests.swift @@ -74,9 +74,17 @@ final class OPDSParserTests: XCTestCase { """ + + let responseTestURL = URL(string: "https://resp-test.org/")! + // Parse data let parser = OPDSParser() - XCTAssertNoThrow(try parser.parse(data: content.data(using: .utf8)!, urlHost: "https://library.kiwix.org")) + XCTAssertNoThrow( + try parser.parse( + data: content.data(using: .utf8)!, + urlHost: responseTestURL.absoluteString + ) + ) // check one zim file is populated let zimFileID = UUID(uuidString: "1ec90eab-5724-492b-9529-893959520de4")! @@ -108,7 +116,7 @@ final class OPDSParserTests: XCTestCase { ) XCTAssertEqual( metadata.faviconURL, - URL(string: "https://library.kiwix.org/catalog/v2/illustration/1ec90eab-5724-492b-9529-893959520de4/") + URL(string: "https://resp-test.org/catalog/v2/illustration/1ec90eab-5724-492b-9529-893959520de4/") ) XCTAssertEqual(metadata.flavor, "maxi") } diff --git a/ViewModel/LibraryViewModel.swift b/ViewModel/LibraryViewModel.swift index 3c043204..9140ecb7 100644 --- a/ViewModel/LibraryViewModel.swift +++ b/ViewModel/LibraryViewModel.swift @@ -290,7 +290,6 @@ final class LibraryViewModel: ObservableObject { .withoutQueryParams() .trim(pathComponents: ["catalog", "v2", "entries"]) .absoluteString - .removingSuffix("/") try parser.parse(data: data, urlHost: urlHostString) continuation.resume(returning: parser) } catch { diff --git a/Views/BuildingBlocks/Favicon.swift b/Views/BuildingBlocks/Favicon.swift index ee3a2d42..ec08491f 100644 --- a/Views/BuildingBlocks/Favicon.swift +++ b/Views/BuildingBlocks/Favicon.swift @@ -63,7 +63,7 @@ struct Favicon_Previews: PreviewProvider { category: .wikipedia, imageData: nil, imageURL: URL( - string: "https://library.kiwix.org/catalog/v2/illustration/e82e6816-a2dc-a7f0-2d15-58d24709db93/?size=48" + string: "https://opds.library.kiwix.org/v2/illustration/e82e6816-a2dc-a7f0-2d15-58d24709db93/?size=48" )! ).frame(width: 200, height: 200).previewLayout(.sizeThatFits) Favicon( From 072da3be2df8db34a519047b9798fa9798498e1f Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Fri, 9 May 2025 22:11:45 +0200 Subject: [PATCH 4/4] Fixlint --- Model/Utilities/URL.swift | 2 -- Tests/OPDSParserTests.swift | 4 +--- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/Model/Utilities/URL.swift b/Model/Utilities/URL.swift index 4c9e14c1..2116eb20 100644 --- a/Model/Utilities/URL.swift +++ b/Model/Utilities/URL.swift @@ -96,7 +96,6 @@ extension URL { return components.url ?? self } - /// Remove the defined components one by one if found /// - Parameter pathComponents: eg: /package/details/more can be defined as: ["package", "details", "more"] /// - Returns: the modified url @@ -108,7 +107,6 @@ extension URL { return result } - /// Removes everything after ? or & /// - Returns: the modified URL, or the same if it fails to find the components func withoutQueryParams() -> URL { diff --git a/Tests/OPDSParserTests.swift b/Tests/OPDSParserTests.swift index 5c71bbd3..282ff68f 100644 --- a/Tests/OPDSParserTests.swift +++ b/Tests/OPDSParserTests.swift @@ -73,11 +73,9 @@ final class OPDSParserTests: XCTestCase { """ - - - let responseTestURL = URL(string: "https://resp-test.org/")! // Parse data + let responseTestURL = URL(string: "https://resp-test.org/")! let parser = OPDSParser() XCTAssertNoThrow( try parser.parse(