Deprecate close() and Closeable APIs (#538)

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]
 

Sources/Adapters/GCDWebServer/ResourceResponse.swift

@@ -112,8 +112,4 @@ class ResourceResponse: ReadiumGCDWebServerFileResponse, Loggable {
 
         return (try? lastReadData?.get()) ?? Data()
     }
-
-    override open func close() {
-        resource.close()
-    }
 }

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<ResourceProperties> {

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

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)`

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()
     }
 

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<UInt64>?) -> ArchiveResult<Data>
-
-    /// Closes any pending resources for this entry.
-    func close()
 }
 
 @available(*, unavailable, renamed: "ArchiveOpener")

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<T>(_ block: (Self) throws -> T) rethrows -> T {
         // Can't use `defer` with async functions.
         do {

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()
-    }
 }

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()
-        }
-    }
 }

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.

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
     }
 }

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`.

Sources/Shared/Toolkit/Data/Resource/BorrowedResource.swift

@@ -11,22 +11,20 @@ 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
 
     init(resource: Resource) {
         self.resource = resource
     }
 
-    func close() {
-        // Do nothing
-    }
-
     var sourceURL: AbsoluteURL? {
         resource.sourceURL
     }

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<UInt64?>?
 
     public func estimatedLength() async -> ReadResult<UInt64?> {

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 {

Sources/Shared/Toolkit/Data/Resource/TailCachingResource.swift

@@ -61,10 +61,6 @@ actor TailCachingResource: Resource, Loggable {
             }
     }
 
-    nonisolated func close() {
-        resource.close()
-    }
-
     private var cache: ReadResult<Data?>? = nil
 
     private func cachedTail() async -> ReadResult<Data?> {

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

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<UInt64?>?

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(

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()

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
-    }
 }

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<UInt64?> {

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

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):

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:

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<UInt64?> {