From 04cda387fce732b6aa16cf6182292cb918ab5e62 Mon Sep 17 00:00:00 2001 From: Mohamed Afifi Date: Sun, 19 Nov 2023 21:30:23 -0500 Subject: [PATCH 1/2] Save success.txt in reading directory to ensure it's downloaded --- Core/SystemDependencies/FileSystem.swift | 6 ++++ .../FileSystemFake.swift | 4 +++ .../Sources/ImageDataService.swift | 11 ++++++ .../Sources/OnDemandResource.swift | 8 ++--- .../Sources/ReadingResourcesService.swift | 34 ++++++++++--------- .../Tests/ReadingResourcesServiceTests.swift | 2 +- 6 files changed, 44 insertions(+), 21 deletions(-) diff --git a/Core/SystemDependencies/FileSystem.swift b/Core/SystemDependencies/FileSystem.swift index 22b8f7b5..8301e906 100644 --- a/Core/SystemDependencies/FileSystem.swift +++ b/Core/SystemDependencies/FileSystem.swift @@ -15,6 +15,8 @@ public protocol FileSystem: Sendable { func removeItem(at url: URL) throws func contentsOfDirectory(at url: URL, includingPropertiesForKeys keys: [URLResourceKey]?) throws -> [URL] func resourceValues(at url: URL, forKeys keys: Set) throws -> ResourceValues + + func writeToFile(at path: URL, content: String) throws } public struct DefaultFileSystem: FileSystem { @@ -47,6 +49,10 @@ public struct DefaultFileSystem: FileSystem { public func copyItem(at srcURL: URL, to dstURL: URL) throws { try FileManager.default.copyItem(at: srcURL, to: dstURL) } + + public func writeToFile(at path: URL, content: String) throws { + try "Success".write(to: path, atomically: true, encoding: .utf8) + } } public protocol ResourceValues { diff --git a/Core/SystemDependenciesFake/FileSystemFake.swift b/Core/SystemDependenciesFake/FileSystemFake.swift index eeb1db82..71b9f4d0 100644 --- a/Core/SystemDependenciesFake/FileSystemFake.swift +++ b/Core/SystemDependenciesFake/FileSystemFake.swift @@ -94,6 +94,10 @@ public final class FileSystemFake: FileSystem, Sendable { resourceValuesByURL[url] = ResourceValuesFake(fileSize: fileSize) } + public func writeToFile(at path: URL, content: String) throws { + files.insert(path) + } + // MARK: Private private let state = ManagedCriticalState(State()) diff --git a/Domain/ImageService/Sources/ImageDataService.swift b/Domain/ImageService/Sources/ImageDataService.swift index 2d170687..83710780 100644 --- a/Domain/ImageService/Sources/ImageDataService.swift +++ b/Domain/ImageService/Sources/ImageDataService.swift @@ -8,6 +8,7 @@ import QuranGeometry import QuranKit import UIKit +import VLogging import WordFramePersistence import WordFrameService @@ -32,6 +33,9 @@ public struct ImageDataService { public func imageForPage(_ page: Page) async throws -> ImagePage { let imageURL = imageURLForPage(page) guard let image = UIImage(contentsOfFile: imageURL.path) else { + logFiles(directory: imagesURL) // /images/width/ + logFiles(directory: imagesURL.deletingLastPathComponent()) // /images/ + logFiles(directory: imagesURL.deletingLastPathComponent().deletingLastPathComponent()) // / fatalError("No image found for page '\(page)'") } @@ -51,6 +55,13 @@ public struct ImageDataService { private let cropInsets: UIEdgeInsets private let imagesURL: URL + private func logFiles(directory: URL) { + let fileManager = FileManager.default + let files = (try? fileManager.contentsOfDirectory(at: directory, includingPropertiesForKeys: nil)) ?? [] + let fileNames = files.map(\.lastPathComponent) + logger.error("Images: Directory \(directory) contains files \(fileNames)") + } + private func imageURLForPage(_ page: Page) -> URL { imagesURL.appendingPathComponent("page\(page.pageNumber.as3DigitString()).png") } diff --git a/Domain/ReadingService/Sources/OnDemandResource.swift b/Domain/ReadingService/Sources/OnDemandResource.swift index f7b04461..c2fa74d3 100644 --- a/Domain/ReadingService/Sources/OnDemandResource.swift +++ b/Domain/ReadingService/Sources/OnDemandResource.swift @@ -21,9 +21,9 @@ struct OnDemandResource { // MARK: Internal func fetch(onProgressChange: @Sendable @escaping (Double) -> Void) async throws { - logger.info("Fetching resources \(request.tags)") + logger.info("Resources: Fetch resources \(request.tags)") let available = await request.conditionallyBeginAccessingResources() - logger.info("Resources \(request.tags) availability \(available)") + logger.info("Resources: \(request.tags) has been downloaded before? \(available)") if available { return } @@ -35,9 +35,9 @@ struct OnDemandResource { } do { try await request.beginAccessingResources() - logger.info("Resources \(request.tags) downloaded") + logger.info("Resources: \(request.tags) has been downloaded") } catch { - logger.error("Resources \(request.tags) failed. Error: \(error)") + logger.error("Resources: \(request.tags) has failed to download. Error: \(error)") throw error } } diff --git a/Domain/ReadingService/Sources/ReadingResourcesService.swift b/Domain/ReadingService/Sources/ReadingResourcesService.swift index 617399f8..44c23592 100644 --- a/Domain/ReadingService/Sources/ReadingResourcesService.swift +++ b/Domain/ReadingService/Sources/ReadingResourcesService.swift @@ -88,7 +88,9 @@ public actor ReadingResourcesService { } private func loadResource(of reading: Reading) async { - if fileManager.fileExists(at: reading.directory) { + logger.info("Resources: Start loading reading resources of: \(reading)") + if fileManager.fileExists(at: reading.successFilePath) { + logger.info("Resources: Reading \(reading) has been downloaded and saved locally before") send(.ready, from: reading) return } @@ -100,26 +102,20 @@ public actor ReadingResourcesService { self.send(.downloading(progress: progress), from: reading) }) - copyFiles(reading: reading) + try copyFiles(reading: reading) send(.ready, from: reading) } catch { send(.error(error as NSError), from: reading) } } - private func copyFiles(reading: Reading) { - if preferences.reading != reading { - return - } - if reading.directory.isReachable { - return - } - + private func copyFiles(reading: Reading) throws { + // Create `resources` directory if needed. try? fileManager.createDirectory(at: Reading.resourcesDirectory, withIntermediateDirectories: true) - + // Remove previously downloaded resources. removePreviouslyDownloadedResources() - - copyNewlyDownloadedResource(reading: reading) + // Copy new reading to `resources` directory. + try copyNewlyDownloadedResource(reading: reading) } private func removePreviouslyDownloadedResources() { @@ -129,16 +125,18 @@ public actor ReadingResourcesService { try? fileManager.removeItem(at: resource) } } catch { - logger.error("Resources failed to list files. Error: \(error)") + logger.error("Resources: Failed to list files in resources directory. Error: \(error)") } } - private func copyNewlyDownloadedResource(reading: Reading) { + private func copyNewlyDownloadedResource(reading: Reading) throws { do { let bundleURL = reading.url(inBundle: bundle) try fileManager.copyItem(at: bundleURL, to: reading.directory) + try fileManager.writeToFile(at: reading.successFilePath, content: "Downloaded") } catch { - logger.error("Resources \(reading.resourcesTag) failed to copy. Error: \(error)") + logger.error("Resources: \(reading) failed to copy. Error: \(error)") + throw error } } @@ -171,4 +169,8 @@ extension Reading { public var directory: URL { Self.resourcesDirectory.appendingPathComponent(resourcesTag, isDirectory: true) } + + var successFilePath: URL { + directory.appendingPathComponent("success.txt") + } } diff --git a/Domain/ReadingService/Tests/ReadingResourcesServiceTests.swift b/Domain/ReadingService/Tests/ReadingResourcesServiceTests.swift index 7b930e0a..0c81ab58 100644 --- a/Domain/ReadingService/Tests/ReadingResourcesServiceTests.swift +++ b/Domain/ReadingService/Tests/ReadingResourcesServiceTests.swift @@ -47,7 +47,7 @@ final class ReadingResourcesServiceTests: XCTestCase { } func test_resourceNotAvailable_downloaded() async throws { - fileManager.files.insert(Reading.hafs_1405.directory) + fileManager.files.insert(Reading.hafs_1405.successFilePath) BundleResourceRequestFake.resourceAvailable = false await service.startLoadingResources() From a29ae4790369da4df44ec6c1459658c48fa3b86c Mon Sep 17 00:00:00 2001 From: Mohamed Afifi Date: Mon, 20 Nov 2023 13:46:06 -0500 Subject: [PATCH 2/2] End accessing ODR after the copy operation completes --- Core/SystemDependencies/BundleResourceRequest.swift | 1 + Core/SystemDependenciesFake/BundleResourceRequestFake.swift | 6 ++++++ Domain/ReadingService/Sources/OnDemandResource.swift | 4 ++++ Domain/ReadingService/Sources/ReadingResourcesService.swift | 3 ++- 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Core/SystemDependencies/BundleResourceRequest.swift b/Core/SystemDependencies/BundleResourceRequest.swift index 29552feb..332db255 100644 --- a/Core/SystemDependencies/BundleResourceRequest.swift +++ b/Core/SystemDependencies/BundleResourceRequest.swift @@ -14,6 +14,7 @@ public protocol BundleResourceRequest: AnyObject { func conditionallyBeginAccessingResources() async -> Bool func beginAccessingResources() async throws + func endAccessingResources() } extension NSBundleResourceRequest: BundleResourceRequest { diff --git a/Core/SystemDependenciesFake/BundleResourceRequestFake.swift b/Core/SystemDependenciesFake/BundleResourceRequestFake.swift index e6ed48f1..46d61d0d 100644 --- a/Core/SystemDependenciesFake/BundleResourceRequestFake.swift +++ b/Core/SystemDependenciesFake/BundleResourceRequestFake.swift @@ -24,6 +24,7 @@ public final class BundleResourceRequestFake: BundleResourceRequest { public var progress = Progress() public var loadingPriority: Double = 0 public let tags: Set + public var inUse = false public func conditionallyBeginAccessingResources() async -> Bool { Self.resourceAvailable @@ -32,9 +33,14 @@ public final class BundleResourceRequestFake: BundleResourceRequest { public func beginAccessingResources() async throws { switch Self.downloadResult { case .success: + inUse = true progress.completedUnitCount = progress.totalUnitCount case .failure(let error): throw error } } + + public func endAccessingResources() { + inUse = false + } } diff --git a/Domain/ReadingService/Sources/OnDemandResource.swift b/Domain/ReadingService/Sources/OnDemandResource.swift index c2fa74d3..1e9d2fee 100644 --- a/Domain/ReadingService/Sources/OnDemandResource.swift +++ b/Domain/ReadingService/Sources/OnDemandResource.swift @@ -42,6 +42,10 @@ struct OnDemandResource { } } + func endAccessingResources() { + request.endAccessingResources() + } + // MARK: Private private let request: BundleResourceRequest diff --git a/Domain/ReadingService/Sources/ReadingResourcesService.swift b/Domain/ReadingService/Sources/ReadingResourcesService.swift index 44c23592..1b05b817 100644 --- a/Domain/ReadingService/Sources/ReadingResourcesService.swift +++ b/Domain/ReadingService/Sources/ReadingResourcesService.swift @@ -97,6 +97,7 @@ public actor ReadingResourcesService { let tag = reading.resourcesTag let resource = OnDemandResource(request: resourceRequestFactory([tag])) + defer { resource.endAccessingResources() } do { try await resource.fetch(onProgressChange: { progress in self.send(.downloading(progress: progress), from: reading) @@ -171,6 +172,6 @@ extension Reading { } var successFilePath: URL { - directory.appendingPathComponent("success.txt") + directory.appendingPathComponent("success-v2.txt") } }