fix: Photo sync can be gracefully terminated by the system in backgro…

…und (#1111)

* chore: Bump core to 6.2.0
chore: Bump ExpiringActivity to new version

* feat: PHAssetIdentifier returns nil if it is interupted by the system

* feat: PhotoLibraryUploader Scan will stop if system is trying to interrup work

* feat: Making sure BackgroundTasksService is aware of activity termination and can stop gracefully

* chore: SonarCloud feedback

* chore: PR Feedback

.package.resolved

@@ -86,8 +86,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/Infomaniak/ios-core",
       "state" : {
-        "revision" : "a1341fb7d6d5bc48576cc8400682476c441f2d23",
-        "version" : "6.0.0"
+        "revision" : "a1689a335a84f1396816d25955cfcd2069be62ae",
+        "version" : "6.2.0"
       }
     },
     {

Project.swift

@@ -24,7 +24,7 @@ let project = Project(name: "kDrive",
                       packages: [
                           .package(url: "https://github.com/apple/swift-algorithms", .upToNextMajor(from: "1.2.0")),
                           .package(url: "https://github.com/Alamofire/Alamofire", .upToNextMajor(from: "5.2.2")),
-                          .package(url: "https://github.com/Infomaniak/ios-core", .upToNextMajor(from: "6.0.0")),
+                          .package(url: "https://github.com/Infomaniak/ios-core", .upToNextMajor(from: "6.2.0")),
                           .package(url: "https://github.com/Infomaniak/ios-core-ui", .upToNextMajor(from: "4.1.0")),
                           .package(url: "https://github.com/Infomaniak/ios-login", .upToNextMajor(from: "6.0.1")),
                           .package(url: "https://github.com/Infomaniak/ios-dependency-injection", .upToNextMajor(from: "2.0.0")),

kDriveCore/Data/UploadQueue/Operation/UploadOperation.swift

@@ -245,7 +245,7 @@ public final class UploadOperation: AsynchronousOperation, UploadOperationable {
             Log.uploadOperation("call finish ufid:\(uploadFileId)")
 
             // Make sure we stop the expiring activity
-            self.expiringActivity?.end()
+            self.expiringActivity?.endAll()
 
             // Make sure we stop all the network requests (if any)
             self.cancelAllUploadRequests()

kDriveCore/Data/UploadQueue/Servicies/PhotoLibraryUploader+Scan.swift

@@ -111,8 +111,17 @@ public extension PhotoLibraryUploader {
                     burstCount: &burstCount
                 )
 
-                let bestResourceSHA256: String? = asset.bestResourceSHA256
-                Log.photoLibraryUploader("Asset hash:\(bestResourceSHA256)")
+                let bestResourceSHA256: String?
+                do {
+                    bestResourceSHA256 = try asset.bestResourceSHA256
+                } catch {
+                    // Error thrown while hashing a resource, we stop ASAP.
+                    Log.photoLibraryUploader("Error while hashing:\(error) asset: \(asset.localIdentifier)", level: .error)
+                    stop.pointee = true
+                    return
+                }
+
+                Log.photoLibraryUploader("Asset hash:\(String(describing: bestResourceSHA256))")
 
                 // Check if picture uploaded before
                 guard !assetAlreadyUploaded(assetName: finalName,

kDriveCore/Services/BackgroundTasksService.swift

@@ -44,6 +44,8 @@ public protocol BackgroundTasksServiceable {
 }
 
 struct BackgroundTasksService: BackgroundTasksServiceable {
+    private static let activityShouldTerminateMessage = "Notified activity should terminate"
+
     @LazyInjectService private var scheduler: BGTaskScheduler
     @LazyInjectService private var accountManager: AccountManageable
     @LazyInjectService private var uploadQueue: UploadQueue
@@ -86,25 +88,53 @@ struct BackgroundTasksService: BackgroundTasksServiceable {
     }
 
     func handleBackgroundRefresh(completion: @escaping (Bool) -> Void) {
+        let expiringActivity = ExpiringActivity(id: UUID().uuidString, delegate: nil)
+        expiringActivity.start()
+
         Log.backgroundTaskScheduling("handleBackgroundRefresh")
         // User installed the app but never logged in
-        if accountManager.accounts.isEmpty {
+        if expiringActivity.shouldTerminate || accountManager.accounts.isEmpty {
+            Log.backgroundTaskScheduling(Self.activityShouldTerminateMessage, level: .error)
             completion(false)
+            expiringActivity.endAll()
             return
         }
 
         Log.backgroundTaskScheduling("Enqueue new pictures")
         photoUploader.scheduleNewPicturesForUpload()
 
+        guard !expiringActivity.shouldTerminate else {
+            Log.backgroundTaskScheduling(Self.activityShouldTerminateMessage, level: .error)
+            completion(false)
+            expiringActivity.endAll()
+            return
+        }
+
         Log.backgroundTaskScheduling("Clean errors for all uploads")
         uploadQueue.cleanNetworkAndLocalErrorsForAllOperations()
 
+        guard !expiringActivity.shouldTerminate else {
+            Log.backgroundTaskScheduling(Self.activityShouldTerminateMessage, level: .error)
+            completion(false)
+            expiringActivity.endAll()
+            return
+        }
+
         Log.backgroundTaskScheduling("Reload operations in queue")
         uploadQueue.rebuildUploadQueueFromObjectsInRealm()
 
+        guard !expiringActivity.shouldTerminate else {
+            Log.backgroundTaskScheduling(Self.activityShouldTerminateMessage, level: .error)
+            completion(false)
+            expiringActivity.endAll()
+            return
+        }
+
         Log.backgroundTaskScheduling("waitForCompletion")
         uploadQueue.waitForCompletion {
+            Log.backgroundTaskScheduling("Background activity ended with success")
             completion(true)
+            expiringActivity.endAll()
         }
     }
 

kDriveCore/Utils/ExpiringActivity.swift

@@ -19,71 +19,107 @@
 import Foundation
 import InfomaniakCore
 
+// TODO: User core 7.0.0 version ASAP
 /// Delegation mechanism to notify the end of an `ExpiringActivity`
 public protocol ExpiringActivityDelegate: AnyObject {
     /// Called when the system is requiring us to terminate an expiring activity
     func backgroundActivityExpiring()
 }
 
+/// Something that can perform arbitrary short background tasks, with a super simple API.
 public protocol ExpiringActivityable {
+    /// Common init method
+    /// - Parameters:
+    ///   - id: Something to identify the background activity in debug
+    ///   - qos: QoS used by the underlying queues
+    ///   - delegate: The delegate to notify we should terminate
+    init(id: String, qos: DispatchQoS, delegate: ExpiringActivityDelegate?)
+
+    /// init method
+    /// - Parameters:
+    ///   - id: Something to identify the background activity in debug
+    ///   - delegate: The delegate to notify we should terminate
     init(id: String, delegate: ExpiringActivityDelegate?)
 
     /// Register with the system an expiring activity
     func start()
 
-    /// Terminate the expiring activity if needed.
-    func end()
+    /// Terminate all the expiring activities
+    func endAll()
+
+    /// True if the system asked to stop the background activity
+    var shouldTerminate: Bool { get }
 }
 
 public final class ExpiringActivity: ExpiringActivityable {
-    /// Keep track of the locks on blocks
-    private var locks = [TolerantDispatchGroup]()
+    private let qos: DispatchQoS
+
+    private let queue: DispatchQueue
 
-    /// For thread safety
-    private let queue = DispatchQueue(label: "com.infomaniak.ExpiringActivity.sync")
+    private let processInfo = ProcessInfo.processInfo
+
+    var locks = [TolerantDispatchGroup]()
 
-    /// Something to identify the background activity in debug
     let id: String
 
-    /// The delegate to notify we should terminate
+    public var shouldTerminate = false
+
     weak var delegate: ExpiringActivityDelegate?
 
     // MARK: Lifecycle
 
-    public init(id: String, delegate: ExpiringActivityDelegate?) {
+    public init(id: String, qos: DispatchQoS, delegate: ExpiringActivityDelegate?) {
         self.id = id
+        self.qos = qos
         self.delegate = delegate
+        queue = DispatchQueue(label: "com.infomaniak.ExpiringActivity.sync", qos: qos)
+    }
+
+    public convenience init(id: String, delegate: ExpiringActivityDelegate?) {
+        self.init(id: id, qos: .userInitiated, delegate: delegate)
     }
 
     deinit {
         queue.sync {
-            assert(locks.isEmpty, "please make sure to balance 'start()' and 'end()' before releasing this object")
+            assert(locks.isEmpty, "please make sure to call 'endAll()' once explicitly before releasing this object")
         }
     }
 
     public func start() {
-        let group = TolerantDispatchGroup()
+        let group = TolerantDispatchGroup(qos: qos)
 
         queue.sync {
             self.locks.append(group)
         }
 
+        #if os(macOS)
+        // We block a non cooperative queue that matches current QoS
+        DispatchQueue.global(qos: qos.qosClass).async {
+            self.processInfo.performActivity(options: .suddenTerminationDisabled, reason: self.id) {
+                // No expiration handler as we are running on macOS
+                group.enter()
+                group.wait()
+            }
+        }
+        #else
         // Make sure to not lock an unexpected thread that would deinit()
-        ProcessInfo.processInfo.performExpiringActivity(withReason: id) { [weak self] shouldTerminate in
+        processInfo.performExpiringActivity(withReason: id) { [weak self] shouldTerminate in
             guard let self else {
                 return
             }
 
             if shouldTerminate {
+                self.shouldTerminate = true
                 delegate?.backgroundActivityExpiring()
             }
 
             group.enter()
             group.wait()
         }
+        #endif
     }
 
-    public func end() {
+    public func endAll() {
         queue.sync {
             // Release locks, oldest first
             for group in locks.reversed() {

kDriveCore/Utils/PHAsset/PHAsset+Exension.swift

@@ -28,20 +28,26 @@ public extension PHAsset {
     ///
     /// Will return `nil` for any other resource type (like video)
     var baseImageSHA256: String? {
-        guard let identifier = PHAssetIdentifier(self) else {
-            return nil
-        }
+        get throws {
+            guard let identifier = PHAssetIdentifier(self) else {
+                return nil
+            }
 
-        return identifier.baseImageSHA256
+            let hash = try identifier.baseImageSHA256
+            return hash
+        }
     }
 
     /// Hash of the best resource available. Editing a video or a picture will change this hash
     var bestResourceSHA256: String? {
-        guard let identifier = PHAssetIdentifier(self) else {
-            return nil
-        }
+        get throws {
+            guard let identifier = PHAssetIdentifier(self) else {
+                return nil
+            }
 
-        return identifier.bestResourceSHA256
+            let hash = try identifier.bestResourceSHA256
+            return hash
+        }
     }
 
     // MARK: - Filename