Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate close() and Closeable APIs #538

Merged
merged 1 commit into from
Jan 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
4 changes: 0 additions & 4 deletions Sources/Adapters/GCDWebServer/ResourceResponse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,4 @@ class ResourceResponse: ReadiumGCDWebServerFileResponse, Loggable {

return (try? lastReadData?.get()) ?? Data()
}

override open func close() {
resource.close()
}
}
4 changes: 0 additions & 4 deletions Sources/LCP/Content Protection/LCPDecryptor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,6 @@ final class LCPDecryptor {
self.encryption = encryption
}

func close() {
resource.close()
}

let sourceURL: AbsoluteURL? = nil

func properties() async -> ReadResult<ResourceProperties> {
Expand Down
4 changes: 1 addition & 3 deletions Sources/Navigator/Audiobook/PublicationMediaLoader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 0 additions & 9 deletions Sources/Shared/Publication/Publication.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
4 changes: 0 additions & 4 deletions Sources/Shared/Toolkit/Archive/Archive.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
Expand Down
2 changes: 2 additions & 0 deletions Sources/Shared/Toolkit/Closeable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand All @@ -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 {
Expand Down
17 changes: 0 additions & 17 deletions Sources/Shared/Toolkit/Data/Asset/Asset.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -73,8 +60,4 @@ public struct ContainerAsset: AssetProtocol {
self.container = container
self.format = format
}

public func close() {
container.close()
}
}
6 changes: 0 additions & 6 deletions Sources/Shared/Toolkit/Data/Container/Container.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,4 @@ public class CompositeContainer: Container {
container[url]
}
}

public func close() {
for container in containers {
container.close()
}
}
}
3 changes: 0 additions & 3 deletions Sources/Shared/Toolkit/Data/Container/Fetcher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ public class SingleResourceContainer: Container {
return nil
}

return resource.borrowed()
}

public func close() {
resource.close()
return resource
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ public final class TransformingContainer: Container {
transformer(url, resource)
}
}

public func close() {
container.close()
}
}

/// Convenient shortcuts to create a `TransformingContainer`.
Expand Down
6 changes: 2 additions & 4 deletions Sources/Shared/Toolkit/Data/Resource/BorrowedResource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 0 additions & 4 deletions Sources/Shared/Toolkit/Data/Resource/BufferingResource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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?> {
Expand Down
4 changes: 0 additions & 4 deletions Sources/Shared/Toolkit/Data/Resource/CachingResource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ public actor CachingResource: Resource {
return ()
}
}

public nonisolated func close() {
resource.close()
}
}

public extension Resource {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 0 additions & 12 deletions Sources/Shared/Toolkit/File/FileResource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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?>?
Expand Down
6 changes: 0 additions & 6 deletions Sources/Shared/Toolkit/HTTP/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,6 @@ public extension HTTPClient {
}
)

do {
try fileHandle.close()
} catch {
log(.warning, error)
}

switch result {
case let .success(response):
return .success(HTTPDownload(
Expand Down
5 changes: 0 additions & 5 deletions Sources/Shared/Toolkit/PDF/CGPDF.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 2 additions & 4 deletions Sources/Shared/Toolkit/ZIP/ZIPFoundation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ final class EPUBDeobfuscator {
self.key = key
}

func close() {
resource.close()
}

let sourceURL: AbsoluteURL? = nil

func estimatedLength() async -> ReadResult<UInt64?> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions Sources/Streamer/Toolkit/Extensions/Container.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ extension Container {
guard let resource = self[href] else {
return nil
}
defer { resource.close() }
return try await resource.read().get()
}

Expand All @@ -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):
Expand Down
6 changes: 1 addition & 5 deletions TestApp/Sources/Reader/Common/Search/SearchViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -437,8 +435,6 @@ private class MockContainer: Container {
_properties = props
}

func close() {}

let sourceURL: (any AbsoluteURL)? = nil

func estimatedLength() async -> ReadResult<UInt64?> {
Expand Down