Skip to content

Commit

Permalink
Initial Preparation for Strict Concurrency (#28)
Browse files Browse the repository at this point in the history
* Make Networking Sendable

* Make ImageProcessor Sendable

* Move task management into separate object

* Remove unused extension

* Get image data outside Task

* Remove unnecessary imports

* Fix concurrency warnings

* DiskCache -> 2.2.0

* Move equality assertions into closures

* Make MockCache Sendable

Stop trying to implement unused sync methods

* Fix mutation in concurrent code

* Fix equality tests with UIImage

* Fix indentation

* Revert changes
  • Loading branch information
mgacy authored Jul 30, 2024
1 parent 5a60145 commit 4589800
Show file tree
Hide file tree
Showing 14 changed files with 203 additions and 228 deletions.
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ let package = Package(
.library(name: "ImageFetcher", targets: ["ImageFetcher"])
],
dependencies: [
.package(url: "https://github.com/Mobelux/DiskCache.git", from: "2.0.0"),
.package(url: "https://github.com/Mobelux/DiskCache.git", from: "2.2.0"),
.package(url: "https://github.com/apple/swift-crypto.git", from: "3.2.0")
],
targets: [
Expand Down
73 changes: 0 additions & 73 deletions Sources/ImageFetcher/Extensions/URLSession+Utils.swift

This file was deleted.

53 changes: 13 additions & 40 deletions Sources/ImageFetcher/ImageFetcher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,11 @@ public final class ImageFetcher: ImageFetching {
internal let cache: Cache
internal let imageProcessor: ImageProcessing
internal let networking: Networking
private let lock = NSLock()
private var tasks: [String: Task<ImageSource, Error>] = [:]
private let taskManager = TaskManager()

/// The number of active tasks.
public var taskCount: Int {
lock.lock()
defer { lock.unlock() }
return tasks.count
taskManager.taskCount
}

/// Creates an image fetcher with the given dependencies.
Expand Down Expand Up @@ -90,21 +87,21 @@ public extension ImageFetcher {
/// - Parameter imageConfiguration: The configuation of the image to be downloaded.
/// - Returns: The parent task of the image loading operation.
func task(_ imageConfiguration: ImageConfiguration) async -> Task<ImageSource, Error> {
if let existingTask = getTask(imageConfiguration) {
if let existingTask = taskManager.getTask(imageConfiguration) {
return existingTask
} else if let cachedData = try? await cache.data(imageConfiguration.key) {
let decompressTask = Task(priority: imageConfiguration.priority) {
try await decompress(cachedData, imageConfiguration: imageConfiguration)
}

insertTask(decompressTask, key: imageConfiguration)
taskManager.insertTask(decompressTask, key: imageConfiguration)
return decompressTask
} else {
let downloadTask = Task(priority: imageConfiguration.priority) {
try await download(imageConfiguration)
}

insertTask(downloadTask, key: imageConfiguration)
taskManager.insertTask(downloadTask, key: imageConfiguration)
return downloadTask
}
}
Expand Down Expand Up @@ -132,7 +129,7 @@ public extension ImageFetcher {
/// Cancels an in-flight image load.
/// - Parameter imageConfiguration: The configuation of the image to be downloaded.
func cancel(_ imageConfiguration: ImageConfiguration) {
guard let task = removeTask(imageConfiguration) else {
guard let task = taskManager.removeTask(imageConfiguration) else {
return
}

Expand All @@ -143,14 +140,14 @@ public extension ImageFetcher {
/// - Parameter url: The url of the image to be downloaded.
/// - Returns: The parent task of the image loading operation.
subscript (_ url: URL) -> Task<ImageSource, Error>? {
getTask(ImageConfiguration(url: url))
taskManager.getTask(ImageConfiguration(url: url))
}

/// Returns the `Task` associated with the given configuration, if one exists.
/// - Parameter url: The configuration of the image to be downloaded.
/// - Returns: The parent task of the image loading operation.
subscript (_ imageConfiguration: ImageConfiguration) -> Task<ImageSource, Error>? {
getTask(imageConfiguration)
taskManager.getTask(imageConfiguration)
}

/// Deletes all images in the cache.
Expand Down Expand Up @@ -212,7 +209,7 @@ public extension ImageFetcher {
try await decompress(cachedData, imageConfiguration: key)
}

insertTask(decompressTask, key: key)
taskManager.insertTask(decompressTask, key: key)
return try? await decompressTask.value.value
}
}
Expand All @@ -223,53 +220,29 @@ private extension ImageFetcher {
do {
let data = try await networking.load(URLRequest(url: imageConfiguration.url))
let image = try await imageProcessor.process(data, configuration: imageConfiguration)
removeTask(imageConfiguration)
taskManager.removeTask(imageConfiguration)

Task.detached(priority: .medium) { [weak self] in
try? await self?.cache(image, key: imageConfiguration)
}

return .downloaded(image)
} catch {
removeTask(imageConfiguration)
taskManager.removeTask(imageConfiguration)
throw error
}
}

func decompress(_ imageData: Data, imageConfiguration: ImageConfiguration) async throws -> ImageSource {
do {
let image = try await imageProcessor.decompress(imageData)
removeTask(imageConfiguration)
taskManager.removeTask(imageConfiguration)
return .cached(image)
} catch let error as CancellationError {
removeTask(imageConfiguration)
taskManager.removeTask(imageConfiguration)
throw error
} catch {
return try await download(imageConfiguration)
}
}

// MARK: Task Access

func insertTask(_ imageFetcherTask: Task<ImageSource, Error>, key: ImageConfiguration) {
lock.lock()
defer { lock.unlock() }

tasks[key.key] = imageFetcherTask
}

func getTask(_ key: ImageConfiguration) -> Task<ImageSource, Error>? {
lock.lock()
defer { lock.unlock() }

return tasks[key.key]
}

@discardableResult
func removeTask(_ key: ImageConfiguration) -> Task<ImageSource, Error>? {
lock.lock()
defer { lock.unlock() }

return tasks.removeValue(forKey: key.key)
}
}
2 changes: 1 addition & 1 deletion Sources/ImageFetcher/ImageProcessor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ extension Operation {
static let isExecutingKey = "isExecuting"
}

final class ImageOperation: Operation {
final class ImageOperation: Operation, @unchecked Sendable {
enum Work: Equatable {
case decompress
case edit(ImageConfiguration)
Expand Down
9 changes: 5 additions & 4 deletions Sources/ImageFetcher/Networking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@
import Foundation

/// A simple wrapper for a closure performing an async network request.
public struct Networking {
public struct Networking: Sendable {
/// Downloads the contents of a URL based on the specified URL request and delivers the data asynchronously.
public let load: (URLRequest) async throws -> Data
public let load: @Sendable (URLRequest) async throws -> Data

/// Creates a wrapper to perform async network requests.
/// - Parameter load: A closure to load a request.
public init(load: @escaping (URLRequest) async throws -> Data) {
public init(load: @escaping @Sendable (URLRequest) async throws -> Data) {
self.load = load
}
}
Expand All @@ -45,6 +45,7 @@ public extension Networking {
enum ResponseValidator {
/// Throws an error if the given response was not successful.
/// - Parameter response: The response to validate.
@Sendable
public static func validate(_ response: URLResponse) throws {
guard let httpResponse = response as? HTTPURLResponse else {
throw ImageError.cannotParse
Expand All @@ -63,7 +64,7 @@ public extension Networking {
/// - validateResponse: A closure that throws an error if the response passed to it was not successful.
init(
_ configuration: URLSessionConfiguration = .default,
validateResponse: @escaping (URLResponse) throws -> Void = ResponseValidator.validate
validateResponse: @escaping @Sendable (URLResponse) throws -> Void = ResponseValidator.validate
) {
let session = URLSession(configuration: configuration)
if #available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *) {
Expand Down
6 changes: 1 addition & 5 deletions Sources/ImageFetcher/Protocols/ImageFetching.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@
// Copyright © 2018 Neighborhood Goods. All rights reserved.
//

#if os(macOS)
import AppKit
#else
import UIKit
#endif
import Foundation

/// A class of types that download and cache images from urls.
public protocol ImageURLFetching {
Expand Down
2 changes: 1 addition & 1 deletion Sources/ImageFetcher/Protocols/ImageProcessing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import Foundation

/// A class of types responsible for decompressing and processing image data.
public protocol ImageProcessing {
public protocol ImageProcessing: Sendable {
/// Decompressed an image from the given data.
/// - Parameter data: The image data.
/// - Returns: The decompressed image.
Expand Down
2 changes: 1 addition & 1 deletion Sources/ImageFetcher/Protocols/Queue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import Foundation

/// A queue that regulates the execution of operations.
public protocol Queue: AnyObject {
public protocol Queue: AnyObject, Sendable {
/// The maximum number of queued operations that can run at the same time.
var maxConcurrentOperationCount: Int { get set }

Expand Down
61 changes: 61 additions & 0 deletions Sources/ImageFetcher/TaskManager.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//
// TaskManager.swift
// Mobelux
//
// MIT License
//
// Copyright (c) 2024 Mobelux LLC
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
//

import Foundation

final class TaskManager: @unchecked Sendable {
private let lock = NSLock()
private var tasks: [String: Task<ImageSource, Error>] = [:]

var taskCount: Int {
lock.lock()
defer { lock.unlock() }
return tasks.count
}

func insertTask(_ imageFetcherTask: Task<ImageSource, Error>, key: ImageConfiguration) {
lock.lock()
defer { lock.unlock() }

tasks[key.key] = imageFetcherTask
}

func getTask(_ key: ImageConfiguration) -> Task<ImageSource, Error>? {
lock.lock()
defer { lock.unlock() }

return tasks[key.key]
}

@discardableResult
func removeTask(_ key: ImageConfiguration) -> Task<ImageSource, Error>? {
lock.lock()
defer { lock.unlock() }

return tasks.removeValue(forKey: key.key)
}
}
38 changes: 38 additions & 0 deletions Tests/ImageFetcherTests/Helpers/Counter.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//
// Counter.swift
// Mobelux
//
// MIT License
//
// Copyright (c) 2024 Mobelux LLC
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
//

import Foundation

actor Counter {
private(set) var count: Int = 0

@discardableResult
func increment() -> Int {
count += 1
return count
}
}
Loading

0 comments on commit 4589800

Please sign in to comment.