From b603df60f073b1bb687e0e4ddf600e7a9f4b6329 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Sun, 19 Jan 2025 18:42:40 +0100 Subject: [PATCH] Deprecate `close()` and `Closeable` APIs (#538) --- CHANGELOG.md | 4 ++++ .../GCDWebServer/ResourceResponse.swift | 4 ---- .../LCP/Content Protection/LCPDecryptor.swift | 4 ---- .../Audiobook/PublicationMediaLoader.swift | 4 +--- Sources/Shared/Publication/Publication.swift | 9 --------- .../Iterators/HTMLResourceContentIterator.swift | 1 - Sources/Shared/Toolkit/Archive/Archive.swift | 4 ---- Sources/Shared/Toolkit/Closeable.swift | 2 ++ Sources/Shared/Toolkit/Data/Asset/Asset.swift | 17 ----------------- .../Toolkit/Data/Container/Container.swift | 6 ------ .../Shared/Toolkit/Data/Container/Fetcher.swift | 3 --- .../Container/SingleResourceContainer.swift | 6 +----- .../Data/Container/TransformingContainer.swift | 4 ---- .../Data/Resource/BorrowedResource.swift | 6 ++---- .../Data/Resource/BufferingResource.swift | 4 ---- .../Toolkit/Data/Resource/CachingResource.swift | 4 ---- .../Data/Resource/TailCachingResource.swift | 4 ---- .../Data/Resource/TransformingResource.swift | 4 ---- Sources/Shared/Toolkit/File/FileResource.swift | 12 ------------ Sources/Shared/Toolkit/HTTP/HTTPClient.swift | 6 ------ Sources/Shared/Toolkit/PDF/CGPDF.swift | 5 ----- Sources/Shared/Toolkit/ZIP/ZIPFoundation.swift | 6 ++---- .../EPUBDeobfuscator.swift | 4 ---- .../EPUB/Services/EPUBPositionsService.swift | 1 - .../Streamer/Toolkit/Extensions/Container.swift | 2 -- .../Reader/Common/Search/SearchViewModel.swift | 6 +----- .../Services/EPUBPositionsServiceTests.swift | 4 ---- 27 files changed, 13 insertions(+), 123 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30a8b803a..1a11f4eed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ All notable changes to this project will be documented in this file. Take a look * Support for streaming ZIP packages over HTTP. This lets you open a remote EPUB, audiobook, or any other ZIP-based publication without needing to download it first. +### Deprecated + +* The `close()` and `Closeable` APIs are now deprecated. Resources are automatically released upon `deinit`, which aligns better with Swift. + ## [3.0.0-beta.2] diff --git a/Sources/Adapters/GCDWebServer/ResourceResponse.swift b/Sources/Adapters/GCDWebServer/ResourceResponse.swift index d446c7b94..5b21538ef 100644 --- a/Sources/Adapters/GCDWebServer/ResourceResponse.swift +++ b/Sources/Adapters/GCDWebServer/ResourceResponse.swift @@ -112,8 +112,4 @@ class ResourceResponse: ReadiumGCDWebServerFileResponse, Loggable { return (try? lastReadData?.get()) ?? Data() } - - override open func close() { - resource.close() - } } diff --git a/Sources/LCP/Content Protection/LCPDecryptor.swift b/Sources/LCP/Content Protection/LCPDecryptor.swift index 0c23300cc..8d5118ab9 100644 --- a/Sources/LCP/Content Protection/LCPDecryptor.swift +++ b/Sources/LCP/Content Protection/LCPDecryptor.swift @@ -101,10 +101,6 @@ final class LCPDecryptor { self.encryption = encryption } - func close() { - resource.close() - } - let sourceURL: AbsoluteURL? = nil func properties() async -> ReadResult { diff --git a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift index d6778472d..a298e9efb 100644 --- a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift +++ b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift @@ -101,9 +101,7 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log req.task.cancel() if reqs.isEmpty { - if let (_, res) = resources.removeValue(forKey: href) { - res.close() - } + resources.removeValue(forKey: href) requests.removeValue(forKey: href) } else { requests[href] = reqs diff --git a/Sources/Shared/Publication/Publication.swift b/Sources/Shared/Publication/Publication.swift index bac2a3212..d57d5b692 100644 --- a/Sources/Shared/Publication/Publication.swift +++ b/Sources/Shared/Publication/Publication.swift @@ -97,15 +97,6 @@ public class Publication: Closeable, Loggable { ?? container[href.anyURL.removingQuery().removingFragment()] } - /// Closes any opened resource associated with the `Publication`, including `services`. - public func close() { - container.close() - - for service in services { - service.close() - } - } - /// Finds the first `Publication.Service` implementing the given service type. /// /// e.g. `findService(PositionsService.self)` diff --git a/Sources/Shared/Publication/Services/Content/Iterators/HTMLResourceContentIterator.swift b/Sources/Shared/Publication/Services/Content/Iterators/HTMLResourceContentIterator.swift index 6f8f99fc7..fef188f87 100644 --- a/Sources/Shared/Publication/Services/Content/Iterators/HTMLResourceContentIterator.swift +++ b/Sources/Shared/Publication/Services/Content/Iterators/HTMLResourceContentIterator.swift @@ -102,7 +102,6 @@ public class HTMLResourceContentIterator: ContentIterator { .tryMap { try SwiftSoup.parse($0) } .tryMap { try parse(document: $0, locator: locator, beforeMaxLength: beforeMaxLength) } .asyncMap { await adjustProgressions(of: $0) } - resource.close() return try result.get() } diff --git a/Sources/Shared/Toolkit/Archive/Archive.swift b/Sources/Shared/Toolkit/Archive/Archive.swift index dc2da03ff..f509eb74b 100644 --- a/Sources/Shared/Toolkit/Archive/Archive.swift +++ b/Sources/Shared/Toolkit/Archive/Archive.swift @@ -29,7 +29,6 @@ public protocol Archive { var entries: [ArchiveEntry] { get } func entry(at path: ArchivePath) -> ArchiveEntry? func readEntry(at path: ArchivePath) -> ArchiveEntryReader? - func close() } @available(*, unavailable) @@ -56,9 +55,6 @@ public protocol ArchiveEntryReader { /// When `range` is nil, the whole content is returned. Out-of-range indexes are clamped to the available length /// automatically. func read(range: Range?) -> ArchiveResult - - /// Closes any pending resources for this entry. - func close() } @available(*, unavailable, renamed: "ArchiveOpener") diff --git a/Sources/Shared/Toolkit/Closeable.swift b/Sources/Shared/Toolkit/Closeable.swift index 539456373..ef4e08455 100644 --- a/Sources/Shared/Toolkit/Closeable.swift +++ b/Sources/Shared/Toolkit/Closeable.swift @@ -10,6 +10,7 @@ import Foundation public protocol Closeable { /// Closes this object and releases any resources associated with it. /// If the object is already closed then invoking this method has no effect. + @available(*, deprecated, message: "Handle Resource deallocation with `deinit` instead.") func close() } @@ -20,6 +21,7 @@ public extension Closeable { public extension Closeable { /// Executes the given block function on this resource and then closes it down correctly whether /// an error is thrown or not. + @available(*, deprecated, message: "The resource is automatically closed when deallocated") @inlinable func use(_ block: (Self) throws -> T) rethrows -> T { // Can't use `defer` with async functions. do { diff --git a/Sources/Shared/Toolkit/Data/Asset/Asset.swift b/Sources/Shared/Toolkit/Data/Asset/Asset.swift index d51c3e563..7ee5a6277 100644 --- a/Sources/Shared/Toolkit/Data/Asset/Asset.swift +++ b/Sources/Shared/Toolkit/Data/Asset/Asset.swift @@ -38,15 +38,6 @@ public enum Asset: AssetProtocol { } } } - - public func close() { - switch self { - case let .resource(asset): - asset.close() - case let .container(asset): - asset.close() - } - } } /// A single resource asset. @@ -58,10 +49,6 @@ public struct ResourceAsset: AssetProtocol { self.resource = resource self.format = format } - - public func close() { - resource.close() - } } /// A container asset providing access to several resources. @@ -73,8 +60,4 @@ public struct ContainerAsset: AssetProtocol { self.container = container self.format = format } - - public func close() { - container.close() - } } diff --git a/Sources/Shared/Toolkit/Data/Container/Container.swift b/Sources/Shared/Toolkit/Data/Container/Container.swift index b6f24c9da..03728bc5d 100644 --- a/Sources/Shared/Toolkit/Data/Container/Container.swift +++ b/Sources/Shared/Toolkit/Data/Container/Container.swift @@ -59,10 +59,4 @@ public class CompositeContainer: Container { container[url] } } - - public func close() { - for container in containers { - container.close() - } - } } diff --git a/Sources/Shared/Toolkit/Data/Container/Fetcher.swift b/Sources/Shared/Toolkit/Data/Container/Fetcher.swift index b4a09cc80..673ce89f9 100644 --- a/Sources/Shared/Toolkit/Data/Container/Fetcher.swift +++ b/Sources/Shared/Toolkit/Data/Container/Fetcher.swift @@ -22,9 +22,6 @@ public protocol Fetcher { /// A `Resource` is always returned, since for some cases we can't know if it exists before /// actually fetching it, such as HTTP. Therefore, errors are handled at the Resource level. func get(_ link: Link) -> Resource - - /// Closes any opened file handles, removes temporary files, etc. - func close() } /// A `Fetcher` providing no resources at all. diff --git a/Sources/Shared/Toolkit/Data/Container/SingleResourceContainer.swift b/Sources/Shared/Toolkit/Data/Container/SingleResourceContainer.swift index 186f96585..774de4c86 100644 --- a/Sources/Shared/Toolkit/Data/Container/SingleResourceContainer.swift +++ b/Sources/Shared/Toolkit/Data/Container/SingleResourceContainer.swift @@ -24,10 +24,6 @@ public class SingleResourceContainer: Container { return nil } - return resource.borrowed() - } - - public func close() { - resource.close() + return resource } } diff --git a/Sources/Shared/Toolkit/Data/Container/TransformingContainer.swift b/Sources/Shared/Toolkit/Data/Container/TransformingContainer.swift index 76743d509..81f336c4c 100644 --- a/Sources/Shared/Toolkit/Data/Container/TransformingContainer.swift +++ b/Sources/Shared/Toolkit/Data/Container/TransformingContainer.swift @@ -40,10 +40,6 @@ public final class TransformingContainer: Container { transformer(url, resource) } } - - public func close() { - container.close() - } } /// Convenient shortcuts to create a `TransformingContainer`. diff --git a/Sources/Shared/Toolkit/Data/Resource/BorrowedResource.swift b/Sources/Shared/Toolkit/Data/Resource/BorrowedResource.swift index f32f4b524..f2a4c07da 100644 --- a/Sources/Shared/Toolkit/Data/Resource/BorrowedResource.swift +++ b/Sources/Shared/Toolkit/Data/Resource/BorrowedResource.swift @@ -11,11 +11,13 @@ import Foundation /// This is useful when you want to pass a ``Resource`` to a component which /// might close it, but you want to keep using it after. public extension Resource { + @available(*, deprecated, message: "Resources are closed on deallocation now.") func borrowed() -> Resource { BorrowedResource(resource: self) } } +@available(*, deprecated, message: "Resources are closed on deallocation now.") private class BorrowedResource: Resource { private let resource: Resource @@ -23,10 +25,6 @@ private class BorrowedResource: Resource { self.resource = resource } - func close() { - // Do nothing - } - var sourceURL: AbsoluteURL? { resource.sourceURL } diff --git a/Sources/Shared/Toolkit/Data/Resource/BufferingResource.swift b/Sources/Shared/Toolkit/Data/Resource/BufferingResource.swift index d755c2de1..63f820fa2 100644 --- a/Sources/Shared/Toolkit/Data/Resource/BufferingResource.swift +++ b/Sources/Shared/Toolkit/Data/Resource/BufferingResource.swift @@ -44,10 +44,6 @@ public actor BufferingResource: Resource, Loggable { await resource.properties() } - public nonisolated func close() { - resource.close() - } - private var cachedLength: ReadResult? public func estimatedLength() async -> ReadResult { diff --git a/Sources/Shared/Toolkit/Data/Resource/CachingResource.swift b/Sources/Shared/Toolkit/Data/Resource/CachingResource.swift index 9f9d7a602..c7338990e 100644 --- a/Sources/Shared/Toolkit/Data/Resource/CachingResource.swift +++ b/Sources/Shared/Toolkit/Data/Resource/CachingResource.swift @@ -52,10 +52,6 @@ public actor CachingResource: Resource { return () } } - - public nonisolated func close() { - resource.close() - } } public extension Resource { diff --git a/Sources/Shared/Toolkit/Data/Resource/TailCachingResource.swift b/Sources/Shared/Toolkit/Data/Resource/TailCachingResource.swift index 281f4dfee..0f95f2c06 100644 --- a/Sources/Shared/Toolkit/Data/Resource/TailCachingResource.swift +++ b/Sources/Shared/Toolkit/Data/Resource/TailCachingResource.swift @@ -61,10 +61,6 @@ actor TailCachingResource: Resource, Loggable { } } - nonisolated func close() { - resource.close() - } - private var cache: ReadResult? = nil private func cachedTail() async -> ReadResult { diff --git a/Sources/Shared/Toolkit/Data/Resource/TransformingResource.swift b/Sources/Shared/Toolkit/Data/Resource/TransformingResource.swift index 87bdcc4ce..672be9f49 100644 --- a/Sources/Shared/Toolkit/Data/Resource/TransformingResource.swift +++ b/Sources/Shared/Toolkit/Data/Resource/TransformingResource.swift @@ -29,10 +29,6 @@ open class TransformingResource: Resource { await _transform!(data) } - open func close() { - resource.close() - } - // As the resource is transformed, we can't use the original source URL // as reference. public let sourceURL: AbsoluteURL? = nil diff --git a/Sources/Shared/Toolkit/File/FileResource.swift b/Sources/Shared/Toolkit/File/FileResource.swift index 53b1ffa24..d5d93d981 100644 --- a/Sources/Shared/Toolkit/File/FileResource.swift +++ b/Sources/Shared/Toolkit/File/FileResource.swift @@ -14,18 +14,6 @@ public actor FileResource: Resource, Loggable { fileURL = file } - public nonisolated func close() { - Task { await doClose() } - } - - private func doClose() async { - do { - try _handle?.getOrNil()?.close() - } catch { - log(.error, error) - } - } - public nonisolated var sourceURL: AbsoluteURL? { fileURL } private var _length: ReadResult? diff --git a/Sources/Shared/Toolkit/HTTP/HTTPClient.swift b/Sources/Shared/Toolkit/HTTP/HTTPClient.swift index 031cb2025..398be8d9b 100644 --- a/Sources/Shared/Toolkit/HTTP/HTTPClient.swift +++ b/Sources/Shared/Toolkit/HTTP/HTTPClient.swift @@ -131,12 +131,6 @@ public extension HTTPClient { } ) - do { - try fileHandle.close() - } catch { - log(.warning, error) - } - switch result { case let .success(response): return .success(HTTPDownload( diff --git a/Sources/Shared/Toolkit/PDF/CGPDF.swift b/Sources/Shared/Toolkit/PDF/CGPDF.swift index 4593b6186..f484d101f 100644 --- a/Sources/Shared/Toolkit/PDF/CGPDF.swift +++ b/Sources/Shared/Toolkit/PDF/CGPDF.swift @@ -297,11 +297,6 @@ public class CGPDFDocumentFactory: PDFDocumentFactory, Loggable { }, releaseInfo: { info in - guard let context = CGPDFDocumentFactory.context(from: info) else { - return - } - let resource = context.resource - resource.close() let info = info?.assumingMemoryBound(to: ResourceContext.self) info?.deinitialize(count: 1) info?.deallocate() diff --git a/Sources/Shared/Toolkit/ZIP/ZIPFoundation.swift b/Sources/Shared/Toolkit/ZIP/ZIPFoundation.swift index c557df79a..e03c219ba 100644 --- a/Sources/Shared/Toolkit/ZIP/ZIPFoundation.swift +++ b/Sources/Shared/Toolkit/ZIP/ZIPFoundation.swift @@ -274,6 +274,8 @@ private final class ResourceDataSource: ReadiumZIPFoundation.DataSource { self.resource = resource } + func close() throws {} + func length() async throws -> UInt64 { guard let length = try await resource.estimatedLength().get() else { throw ResourceDataSourceError.unknownContentLength @@ -298,8 +300,4 @@ private final class ResourceDataSource: ReadiumZIPFoundation.DataSource { _position += UInt64(data.count) return data } - - func close() { - // We don't own the resource, so it will not be closed - } } diff --git a/Sources/Streamer/Parser/EPUB/Resource Transformers/EPUBDeobfuscator.swift b/Sources/Streamer/Parser/EPUB/Resource Transformers/EPUBDeobfuscator.swift index 62b52f1ad..fff7b4f4d 100644 --- a/Sources/Streamer/Parser/EPUB/Resource Transformers/EPUBDeobfuscator.swift +++ b/Sources/Streamer/Parser/EPUB/Resource Transformers/EPUBDeobfuscator.swift @@ -57,10 +57,6 @@ final class EPUBDeobfuscator { self.key = key } - func close() { - resource.close() - } - let sourceURL: AbsoluteURL? = nil func estimatedLength() async -> ReadResult { diff --git a/Sources/Streamer/Parser/EPUB/Services/EPUBPositionsService.swift b/Sources/Streamer/Parser/EPUB/Services/EPUBPositionsService.swift index 95867fd7d..a1b1c4e30 100644 --- a/Sources/Streamer/Parser/EPUB/Services/EPUBPositionsService.swift +++ b/Sources/Streamer/Parser/EPUB/Services/EPUBPositionsService.swift @@ -131,7 +131,6 @@ public actor EPUBPositionsService: PositionsService { guard let resource = container[link.url()] else { return (startPosition, []) } - defer { resource.close() } let positionCount = await reflowableStrategy.positionCount(for: link, resource: resource) let positions = (1 ... positionCount).map { position in diff --git a/Sources/Streamer/Toolkit/Extensions/Container.swift b/Sources/Streamer/Toolkit/Extensions/Container.swift index 9b91c2856..794e5393b 100644 --- a/Sources/Streamer/Toolkit/Extensions/Container.swift +++ b/Sources/Streamer/Toolkit/Extensions/Container.swift @@ -28,7 +28,6 @@ extension Container { guard let resource = self[href] else { return nil } - defer { resource.close() } return try await resource.read().get() } @@ -42,7 +41,6 @@ extension Container { guard let resource = self[url] else { continue } - defer { resource.close() } switch await assetRetriever.sniffFormat(of: resource) { case let .success(format): diff --git a/TestApp/Sources/Reader/Common/Search/SearchViewModel.swift b/TestApp/Sources/Reader/Common/Search/SearchViewModel.swift index f3bd32799..bbcba00ef 100644 --- a/TestApp/Sources/Reader/Common/Search/SearchViewModel.swift +++ b/TestApp/Sources/Reader/Common/Search/SearchViewModel.swift @@ -94,11 +94,7 @@ final class SearchViewModel: ObservableObject { /// Cancels any on-going search and clears the results. func cancelSearch() { switch state { - case let .idle(iterator): - iterator.close() - - case let .loadingNext(iterator, task): - iterator.close() + case let .loadingNext(_, task): task.cancel() default: diff --git a/Tests/StreamerTests/Parser/EPUB/Services/EPUBPositionsServiceTests.swift b/Tests/StreamerTests/Parser/EPUB/Services/EPUBPositionsServiceTests.swift index 9053a5445..9e8b88d8b 100644 --- a/Tests/StreamerTests/Parser/EPUB/Services/EPUBPositionsServiceTests.swift +++ b/Tests/StreamerTests/Parser/EPUB/Services/EPUBPositionsServiceTests.swift @@ -424,8 +424,6 @@ private class MockContainer: Container { return MockResource(length: length, archiveProperties: archiveProperties) } - func close() {} - struct MockResource: Resource { private let _length: UInt64 private let _properties: ResourceProperties @@ -437,8 +435,6 @@ private class MockContainer: Container { _properties = props } - func close() {} - let sourceURL: (any AbsoluteURL)? = nil func estimatedLength() async -> ReadResult {