From 3c94f24c0810ad1c689a89ad5e81fd9da897fbb4 Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Sat, 2 Mar 2024 08:58:33 +0100 Subject: [PATCH 1/2] Clarify the purpose of fileURLBookmark --- Model/Entities/Entities.swift | 14 ++++++++++---- Model/ZimFileService/ZimFileService.swift | 22 ++++++++++++++-------- SwiftUI/Model/LibraryOperations.swift | 8 ++++---- Tests/LibraryRefreshViewModelTest.swift | 2 +- ViewModel/LibraryViewModel.swift | 2 +- Views/Library/ZimFilesOpened.swift | 2 +- Views/SearchResults.swift | 2 +- 7 files changed, 32 insertions(+), 20 deletions(-) diff --git a/Model/Entities/Entities.swift b/Model/Entities/Entities.swift index ce628ce7..1dc04695 100644 --- a/Model/Entities/Entities.swift +++ b/Model/Entities/Entities.swift @@ -148,7 +148,7 @@ struct URLContent { let size: UInt } -class ZimFile: NSManagedObject, Identifiable { +final class ZimFile: NSManagedObject, Identifiable { var id: UUID { fileID } @NSManaged var articleCount: Int64 @@ -159,6 +159,7 @@ class ZimFile: NSManagedObject, Identifiable { @NSManaged var faviconURL: URL? @NSManaged var fileDescription: String @NSManaged var fileID: UUID + /// System file URL, if not nil, it means it's downloaded @NSManaged var fileURLBookmark: Data? @NSManaged var flavor: String? @NSManaged var hasDetails: Bool @@ -184,11 +185,16 @@ class ZimFile: NSManagedObject, Identifiable { }.joined(separator: ",") } + enum Predicate { + static let isDownloaded = NSPredicate(format: "fileURLBookmark != nil") + static let notDownloaded = NSPredicate(format: "fileURLBookmark == nil") + static let notMissing = NSPredicate(format: "isMissing == false") + } + static var openedPredicate = NSCompoundPredicate(andPredicateWithSubpredicates: [ - NSPredicate(format: "fileURLBookmark != nil"), - NSPredicate(format: "isMissing == false") + Predicate.isDownloaded, + Predicate.notMissing ]) - static var withFileURLBookmarkPredicate = NSPredicate(format: "fileURLBookmark != nil") class func fetchRequest( predicate: NSPredicate? = nil, sortDescriptors: [NSSortDescriptor] = [] diff --git a/Model/ZimFileService/ZimFileService.swift b/Model/ZimFileService/ZimFileService.swift index be26cfd5..0580d227 100644 --- a/Model/ZimFileService/ZimFileService.swift +++ b/Model/ZimFileService/ZimFileService.swift @@ -16,27 +16,27 @@ extension ZimFileService { // MARK: - Reader Management - /// Open a zim file from bookmark data + /// Open a zim file from system file URL bookmark data /// - Parameter bookmark: url bookmark data of the zim file to open /// - Returns: new url bookmark data if the one used to open the zim file is stale @discardableResult - func open(bookmark: Data) throws -> Data? { + func open(fileURLBookmark data: Data) throws -> Data? { // resolve url var isStale: Bool = false #if os(macOS) guard let url = try? URL( - resolvingBookmarkData: bookmark, + resolvingBookmarkData: data, options: [.withSecurityScope], bookmarkDataIsStale: &isStale ) else { throw ZimFileOpenError.missing } #else - guard let url = try? URL(resolvingBookmarkData: bookmark, bookmarkDataIsStale: &isStale) else { + guard let url = try? URL(resolvingBookmarkData: data, bookmarkDataIsStale: &isStale) else { throw ZimFileOpenError.missing } #endif __open(url) - return isStale ? ZimFileService.getBookmarkData(url: url) : nil + return isStale ? ZimFileService.getFileURLBookmarkData(for: url) : nil } /// Close a zim file @@ -57,9 +57,15 @@ extension ZimFileService { __getMetaData(withFileURL: url) } - // MARK: - URL Bookmark - - static func getBookmarkData(url: URL) -> Data? { + // MARK: - URL System Bookmark + + /// System URL bookmark for the ZIM file itself + /// "bookmark data that can later be resolved into a URL object for a file + /// even if the user moves or renames it" + /// Not to be confused with the article bookmarks + /// - Parameter url: file system URL + /// - Returns: data that can later be resolved into a URL object + static func getFileURLBookmarkData(for url: URL) -> Data? { _ = url.startAccessingSecurityScopedResource() defer { url.stopAccessingSecurityScopedResource() } #if os(macOS) diff --git a/SwiftUI/Model/LibraryOperations.swift b/SwiftUI/Model/LibraryOperations.swift index 5a15fe91..df9cb976 100644 --- a/SwiftUI/Model/LibraryOperations.swift +++ b/SwiftUI/Model/LibraryOperations.swift @@ -26,11 +26,11 @@ struct LibraryOperations { @discardableResult static func open(url: URL, onComplete: (() -> Void)? = nil) -> ZimFileMetaData? { guard let metadata = ZimFileService.getMetaData(url: url), - let fileURLBookmark = ZimFileService.getBookmarkData(url: url) else { return nil } + let fileURLBookmark = ZimFileService.getFileURLBookmarkData(for: url) else { return nil } // open the file do { - try ZimFileService.shared.open(bookmark: fileURLBookmark) + try ZimFileService.shared.open(fileURLBookmark: fileURLBookmark) } catch { return nil } @@ -57,7 +57,7 @@ struct LibraryOperations { static func reopen(onComplete: (() -> Void)?) { var successCount = 0 let context = Database.shared.container.viewContext - let request = ZimFile.fetchRequest(predicate: ZimFile.withFileURLBookmarkPredicate) + let request = ZimFile.fetchRequest(predicate: ZimFile.Predicate.isDownloaded) guard let zimFiles = try? context.fetch(request) else { onComplete?() @@ -66,7 +66,7 @@ struct LibraryOperations { zimFiles.forEach { zimFile in guard let data = zimFile.fileURLBookmark else { return } do { - if let data = try ZimFileService.shared.open(bookmark: data) { + if let data = try ZimFileService.shared.open(fileURLBookmark: data) { zimFile.fileURLBookmark = data } zimFile.isMissing = false diff --git a/Tests/LibraryRefreshViewModelTest.swift b/Tests/LibraryRefreshViewModelTest.swift index 7aa81f32..e7172998 100644 --- a/Tests/LibraryRefreshViewModelTest.swift +++ b/Tests/LibraryRefreshViewModelTest.swift @@ -219,7 +219,7 @@ final class LibraryRefreshViewModelTest: XCTestCase { XCTAssertNotEqual(zimFile1.fileID, zimFile2.fileID) // set fileURLBookmark of zim file 2 - zimFile2.fileURLBookmark = "some file url bookmark data".data(using: .utf8) + zimFile2.fileURLBookmark = "/Users/tester/Downloads/file_url.zim".data(using: .utf8) try context.save() // refresh library for the third time diff --git a/ViewModel/LibraryViewModel.swift b/ViewModel/LibraryViewModel.swift index 5faa7f77..96f8416d 100644 --- a/ViewModel/LibraryViewModel.swift +++ b/ViewModel/LibraryViewModel.swift @@ -195,7 +195,7 @@ public class LibraryViewModel: ObservableObject { // delete old zim entries not included in the feed let fetchRequest: NSFetchRequest = ZimFile.fetchRequest() fetchRequest.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [ - NSPredicate(format: "fileURLBookmark == nil"), + ZimFile.Predicate.notDownloaded, NSPredicate(format: "NOT fileID IN %@", parser.zimFileIDs) ]) let deleteRequest = NSBatchDeleteRequest(fetchRequest: fetchRequest) diff --git a/Views/Library/ZimFilesOpened.swift b/Views/Library/ZimFilesOpened.swift index 46c4ac0c..20c742a1 100644 --- a/Views/Library/ZimFilesOpened.swift +++ b/Views/Library/ZimFilesOpened.swift @@ -14,7 +14,7 @@ struct ZimFilesOpened: View { @Environment(\.horizontalSizeClass) private var horizontalSizeClass @FetchRequest( sortDescriptors: [NSSortDescriptor(keyPath: \ZimFile.size, ascending: false)], - predicate: ZimFile.withFileURLBookmarkPredicate, + predicate: ZimFile.Predicate.isDownloaded, animation: .easeInOut ) private var zimFiles: FetchedResults @State private var isFileImporterPresented = false diff --git a/Views/SearchResults.swift b/Views/SearchResults.swift index 159f78cd..b8f1ab62 100644 --- a/Views/SearchResults.swift +++ b/Views/SearchResults.swift @@ -18,7 +18,7 @@ struct SearchResults: View { @EnvironmentObject private var viewModel: SearchViewModel @FetchRequest( sortDescriptors: [NSSortDescriptor(keyPath: \ZimFile.size, ascending: false)], - predicate: ZimFile.withFileURLBookmarkPredicate, + predicate: ZimFile.Predicate.isDownloaded, animation: .easeInOut ) private var zimFiles: FetchedResults @State private var isClearSearchConfirmationPresented = false From 8e6e506ffb912320c7f6e1d5cec838fb0793652b Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Sun, 3 Mar 2024 00:22:19 +0100 Subject: [PATCH 2/2] Clean up --- Model/ZimFileService/ZimFileService.h | 2 - Model/ZimFileService/ZimFileService.mm | 95 +++++++++-------------- Model/ZimFileService/ZimFileService.swift | 12 +-- Support/Kiwix-Bridging-Header.h | 1 - 4 files changed, 38 insertions(+), 72 deletions(-) diff --git a/Model/ZimFileService/ZimFileService.h b/Model/ZimFileService/ZimFileService.h index 1ef3faee..6658c9b6 100644 --- a/Model/ZimFileService/ZimFileService.h +++ b/Model/ZimFileService/ZimFileService.h @@ -26,8 +26,6 @@ # pragma mark - Metadata -- (nullable ZimFileMetaData *)getMetaData:(nonnull NSUUID *)zimFileID NS_REFINED_FOR_SWIFT; -- (nullable NSData *)getFavicon:(nonnull NSUUID *)zimFileID NS_REFINED_FOR_SWIFT; + (nullable ZimFileMetaData *)getMetaDataWithFileURL:(nonnull NSURL *)url NS_REFINED_FOR_SWIFT; # pragma mark - URL Handling diff --git a/Model/ZimFileService/ZimFileService.mm b/Model/ZimFileService/ZimFileService.mm index ce3a6f0a..7d30a801 100644 --- a/Model/ZimFileService/ZimFileService.mm +++ b/Model/ZimFileService/ZimFileService.mm @@ -23,7 +23,7 @@ @interface ZimFileService () -@property (assign) std::unordered_map *archives; +@property (assign) std::unordered_map *archives; // (NSUUID_c: Archive) @property (strong) NSMutableDictionary *fileURLs; // [NSUUID: URL] @end @@ -68,7 +68,7 @@ if (![pathExtension isEqualToString:@"zim"]) { return; } - + // if we have previously added this url, skip it if ([[self.fileURLs allKeysForObject:url] count] > 0) { return; @@ -78,7 +78,7 @@ [url startAccessingSecurityScopedResource]; zim::Archive archive = zim::Archive([url fileSystemRepresentation]); self.archives->insert(std::make_pair(std::string(archive.getUuid()), archive)); - + // store file URL NSUUID *zimFileID = [[NSUUID alloc] initWithUUIDBytes:(unsigned char *)archive.getUuid().data]; self.fileURLs[zimFileID] = url; @@ -88,7 +88,7 @@ } - (void)close:(NSUUID *)zimFileID { - self.archives->erase([[[zimFileID UUIDString] lowercaseString] cStringUsingEncoding:NSUTF8StringEncoding]); + self.archives->erase([self zimfileID_C: zimFileID]); [self.fileURLs[zimFileID] stopAccessingSecurityScopedResource]; [self.fileURLs removeObjectForKey:zimFileID]; } @@ -103,33 +103,6 @@ # pragma mark - Metadata -- (ZimFileMetaData *)getMetaData:(NSUUID *)zimFileID { - std::string zimFileID_C = [[[zimFileID UUIDString] lowercaseString] cStringUsingEncoding:NSUTF8StringEncoding]; - auto found = self.archives->find(zimFileID_C); - if (found == self.archives->end()) { - return nil; - } else { - kiwix::Book book = kiwix::Book(); - book.update(found->second); - return [[ZimFileMetaData alloc] initWithBook: &book]; - } -} - -- (NSData *)getFavicon:(NSUUID *)zimFileID { - std::string zimFileID_C = [[[zimFileID UUIDString] lowercaseString] cStringUsingEncoding:NSUTF8StringEncoding]; - auto found = self.archives->find(zimFileID_C); - if (found == self.archives->end()) { - return nil; - } else { - try { - zim::Blob blob = found->second.getIllustrationItem().getData(); - return [NSData dataWithBytes:blob.data() length:blob.size()]; - } catch (std::exception) { - return nil; - } - } -} - + (ZimFileMetaData *)getMetaDataWithFileURL:(NSURL *)url { ZimFileMetaData *metaData = nil; [url startAccessingSecurityScopedResource]; @@ -148,29 +121,23 @@ return self.fileURLs[zimFileID]; } -- (NSString *_Nullable)getRedirectedPath:(NSUUID *_Nonnull)zimFileID contentPath:(NSString *_Nonnull)contentPath { - std::string zimFileID_C = [[[zimFileID UUIDString] lowercaseString] cStringUsingEncoding:NSUTF8StringEncoding]; - auto found = self.archives->find(zimFileID_C); - if (found == self.archives->end()) { - return nil; - } +- (NSString *_Nullable) getRedirectedPath:(NSUUID *_Nonnull)zimFileID contentPath:(NSString *_Nonnull)contentPath { + zim::Archive *archive = [self archiveBy: zimFileID]; + if (archive == nil) { return nil; } try { std::string contentPathC = [contentPath cStringUsingEncoding:NSUTF8StringEncoding]; - zim::Item item = found->second.getEntryByPath(contentPathC).getRedirect(); - return [NSString stringWithUTF8String:item.getPath().c_str()]; + zim::Item item = archive->getEntryByPath(contentPathC).getRedirect(); + return [NSString stringWithUTF8String: item.getPath().c_str()]; } catch (std::exception) { return nil; } } - (NSString *)getMainPagePath:(NSUUID *)zimFileID { - std::string zimFileID_C = [[[zimFileID UUIDString] lowercaseString] cStringUsingEncoding:NSUTF8StringEncoding]; - auto found = self.archives->find(zimFileID_C); - if (found == self.archives->end()) { - return nil; - } + zim::Archive *archive = [self archiveBy: zimFileID]; + if (archive == nil) { return nil; } try { - zim::Entry entry = found->second.getMainEntry(); + zim::Entry entry = archive->getMainEntry(); zim::Item item = entry.getItem(entry.isRedirect()); return [NSString stringWithCString:item.getPath().c_str() encoding:NSUTF8StringEncoding]; } catch (std::exception) { @@ -179,13 +146,10 @@ } - (NSString *)getRandomPagePath:(NSUUID *)zimFileID { - std::string zimFileID_C = [[[zimFileID UUIDString] lowercaseString] cStringUsingEncoding:NSUTF8StringEncoding]; - auto found = self.archives->find(zimFileID_C); - if (found == self.archives->end()) { - return nil; - } + zim::Archive *archive = [self archiveBy: zimFileID]; + if (archive == nil) { return nil; } try { - zim::Entry entry = found->second.getRandomEntry(); + zim::Entry entry = archive->getRandomEntry(); zim::Item item = entry.getItem(entry.isRedirect()); return [NSString stringWithCString:item.getPath().c_str() encoding:NSUTF8StringEncoding]; } catch (std::exception) { @@ -194,18 +158,15 @@ } - (NSDictionary *)getContent:(NSUUID *)zimFileID contentPath:(NSString *)contentPath - start:(NSUInteger)start end:(NSUInteger)end { + start:(NSUInteger)start end:(NSUInteger)end { if ([contentPath hasPrefix:@"/"]) { contentPath = [contentPath substringFromIndex:1]; } - - std::string zimFileID_C = [[[zimFileID UUIDString] lowercaseString] cStringUsingEncoding:NSUTF8StringEncoding]; - auto found = self.archives->find(zimFileID_C); - if (found == self.archives->end()) { - return nil; - } + + zim::Archive *archive = [self archiveBy: zimFileID]; + if (archive == nil) { return nil; } try { - zim::Entry entry = found->second.getEntryByPath([contentPath cStringUsingEncoding:NSUTF8StringEncoding]); + zim::Entry entry = archive->getEntryByPath([contentPath cStringUsingEncoding:NSUTF8StringEncoding]); zim::Item item = entry.getItem(entry.isRedirect()); zim::Blob blob; if (start == 0 && end == 0) { @@ -227,4 +188,20 @@ } } +# pragma mark - private + +/// Converts the UUID to a C representation +- (std::string) zimfileID_C: (NSUUID *_Nonnull) zimFileID { + return [[[zimFileID UUIDString] lowercaseString] cStringUsingEncoding:NSUTF8StringEncoding]; +} + +- (zim::Archive *_Nullable) archiveBy: (NSUUID *_Nonnull) zimFileID { + std::string zimFileID_C = [self zimfileID_C: zimFileID]; + auto found = self.archives->find(zimFileID_C); + if (found == self.archives->end()) { + return nil; + } + return &(found->second); +} + @end diff --git a/Model/ZimFileService/ZimFileService.swift b/Model/ZimFileService/ZimFileService.swift index 0580d227..2f64a10d 100644 --- a/Model/ZimFileService/ZimFileService.swift +++ b/Model/ZimFileService/ZimFileService.swift @@ -12,8 +12,8 @@ extension ZimFileService { static let shared = ZimFileService.__sharedInstance() /// IDs of currently opened zim files - var fileIDs: [UUID] { get { return __getReaderIdentifiers().compactMap({ $0 as? UUID }) } } - + private var fileIDs: [UUID] { get { return __getReaderIdentifiers().compactMap({ $0 as? UUID }) } } + // MARK: - Reader Management /// Open a zim file from system file URL bookmark data @@ -44,14 +44,6 @@ extension ZimFileService { func close(fileID: UUID) { __close(fileID) } // MARK: - Metadata - - func getMetaData(id: UUID) -> ZimFileMetaData? { - __getMetaData(id) - } - - func getFavicon(id: UUID) -> Data? { - __getFavicon(id) - } static func getMetaData(url: URL) -> ZimFileMetaData? { __getMetaData(withFileURL: url) diff --git a/Support/Kiwix-Bridging-Header.h b/Support/Kiwix-Bridging-Header.h index 590a34d2..c88c6a4b 100644 --- a/Support/Kiwix-Bridging-Header.h +++ b/Support/Kiwix-Bridging-Header.h @@ -15,7 +15,6 @@ NS_INLINE NSException * _Nullable objCTryBlock(void(^_Nonnull tryBlock)(void)) { return nil; } @catch (NSException *exception) { - NSLog(@"Exception: %s", exception); return exception; } }