Merge pull request #117 from tidal-music/alberto/offlineengine-use-no…

…tificationhandler

[Player] Refactor OfflineEngine Listener setup and handling

Sources/Player/Common/Event/NotificationsHandler.swift

@@ -2,10 +2,12 @@ import Foundation
 
 final class NotificationsHandler {
 	private let listener: PlayerListener
+	private let offlineEngineListener: OfflineEngineListener?
 	private let queue: DispatchQueue
 
-	init(listener: PlayerListener, queue: DispatchQueue) {
+	init(listener: PlayerListener, offlineEngineListener: OfflineEngineListener?, queue: DispatchQueue) {
 		self.listener = listener
+		self.offlineEngineListener = offlineEngineListener
 		self.queue = queue
 	}
 
@@ -58,4 +60,40 @@ final class NotificationsHandler {
 			self.listener.djSessionTransitioned(to: transition)
 		}
 	}
+
+	func offliningStarted(for mediaProduct: MediaProduct) {
+		queue.async {
+			self.offlineEngineListener?.offliningStarted(for: mediaProduct)
+		}
+	}
+
+	func offliningProgress(for mediaProduct: MediaProduct, is downloadPercentage: Double) {
+		queue.async {
+			self.offlineEngineListener?.offliningProgressed(for: mediaProduct, is: downloadPercentage)
+		}
+	}
+
+	func offliningCompleted(for mediaProduct: MediaProduct) {
+		queue.async {
+			self.offlineEngineListener?.offliningCompleted(for: mediaProduct)
+		}
+	}
+
+	func offliningFailed(for mediaProduct: MediaProduct) {
+		queue.async {
+			self.offlineEngineListener?.offliningFailed(for: mediaProduct)
+		}
+	}
+
+	func offlinedDeleted(for mediaProduct: MediaProduct) {
+		queue.async {
+			self.offlineEngineListener?.offlinedDeleted(for: mediaProduct)
+		}
+	}
+
+	func allOfflinedMediaProductsDeleted() {
+		queue.async {
+			self.offlineEngineListener?.allOfflinedMediaProductsDeleted()
+		}
+	}
 }

Sources/Player/Mocks/OfflineEngine/OfflineEngineListenerMock.swift

@@ -0,0 +1,15 @@
+import Foundation
+
+public final class OfflineEngineListenerMock: OfflineEngineListener {
+	public func offliningStarted(for mediaProduct: MediaProduct) {}
+
+	public func offliningProgressed(for mediaProduct: MediaProduct, is downloadPercentage: Double) {}
+
+	public func offliningCompleted(for mediaProduct: MediaProduct) {}
+
+	public func offliningFailed(for mediaProduct: MediaProduct) {}
+
+	public func offlinedDeleted(for mediaProduct: MediaProduct) {}
+
+	public func allOfflinedMediaProductsDeleted() {}
+}

Sources/Player/OfflineEngine/OfflineEngine.swift

@@ -7,25 +7,31 @@ public final class OfflineEngine {
 	private let offlineStorage: OfflineStorage
 	private let playerEventSender: PlayerEventSender
 
-	private weak var offlinerDelegate: OfflinerDelegate?
-
-	init(downloader: Downloader, offlineStorage: OfflineStorage, playerEventSender: PlayerEventSender) {
+	@Atomic private var notificationsHandler: NotificationsHandler?
+
+	init(
+		downloader: Downloader,
+		offlineStorage: OfflineStorage,
+		playerEventSender: PlayerEventSender,
+		notificationsHandler: NotificationsHandler
+	) {
 		self.downloader = downloader
 		self.offlineStorage = offlineStorage
 		self.playerEventSender = playerEventSender
+		self.notificationsHandler = notificationsHandler
 		self.downloader.setObserver(observer: self)
 	}
 
 	public func offline(mediaProduct: MediaProduct) -> Bool {
 		guard let offlineEntry = try? offlineStorage.get(key: mediaProduct.productId) else {
-			offlinerDelegate?.offliningStarted(for: mediaProduct)
+			notificationsHandler?.offliningStarted(for: mediaProduct)
 			downloader.download(mediaProduct: mediaProduct, sessionType: .DOWNLOAD)
 			return true
 		}
 
 		guard offlineEntry.state == .OFFLINED_AND_VALID else {
 			delete(offlineEntry: offlineEntry)
-			offlinerDelegate?.offliningStarted(for: mediaProduct)
+			notificationsHandler?.offliningStarted(for: mediaProduct)
 			downloader.download(mediaProduct: mediaProduct, sessionType: .DOWNLOAD)
 			return true
 		}
@@ -45,7 +51,7 @@ public final class OfflineEngine {
 	public func deleteAllOfflinedMediaProducts() -> Bool {
 		downloader.cancellAll()
 		try? offlineStorage.clear()
-		offlinerDelegate?.allOfflinedMediaProductsDeleted()
+		notificationsHandler?.allOfflinedMediaProductsDeleted()
 		return true
 	}
 
@@ -55,10 +61,6 @@ public final class OfflineEngine {
 		}
 		return offlineEntry.state.publicState
 	}
-
-	public func setOfflinerDelegate(_ offlinerDelegate: OfflinerDelegate) {
-		self.offlinerDelegate = offlinerDelegate
-	}
 }
 
 // MARK: DownloadObserver
@@ -69,25 +71,25 @@ extension OfflineEngine: DownloadObserver {
 	}
 
 	func downloadStarted(for mediaProduct: MediaProduct) {
-		offlinerDelegate?.offliningStarted(for: mediaProduct)
+		notificationsHandler?.offliningStarted(for: mediaProduct)
 	}
 
 	func downloadProgress(for mediaProduct: MediaProduct, is percentage: Double) {
-		offlinerDelegate?.offliningProgress(for: mediaProduct, is: percentage)
+		notificationsHandler?.offliningProgress(for: mediaProduct, is: percentage)
 	}
 
 	func downloadCompleted(for mediaProduct: MediaProduct, offlineEntry: OfflineEntry) {
 		do {
 			try offlineStorage.save(offlineEntry)
-			offlinerDelegate?.offliningCompleted(for: mediaProduct)
+			notificationsHandler?.offliningCompleted(for: mediaProduct)
 		} catch {
 			PlayerWorld.logger?.log(loggable: PlayerLoggable.saveOfflinedItemFailed(error: error))
-			offlinerDelegate?.offliningFailed(for: mediaProduct)
+			notificationsHandler?.offliningFailed(for: mediaProduct)
 		}
 	}
 
 	func downloadFailed(for mediaProduct: MediaProduct, with error: Error) {
-		offlinerDelegate?.offliningFailed(for: mediaProduct)
+		notificationsHandler?.offliningFailed(for: mediaProduct)
 	}
 }
 
@@ -103,7 +105,7 @@ private extension OfflineEngine {
 			}
 			try offlineStorage.delete(key: offlineEntry.productId)
 			let mediaProduct = MediaProduct(productType: offlineEntry.productType, productId: offlineEntry.productId)
-			offlinerDelegate?.offlinedDeleted(for: mediaProduct)
+			notificationsHandler?.offlinedDeleted(for: mediaProduct)
 		} catch {
 			PlayerWorld.logger?.log(loggable: PlayerLoggable.deleteOfflinedItemFailed(error: error))
 		}

Sources/Player/OfflineEngine/OfflinerDelegate.swift → Sources/Player/OfflineEngine/OfflineEngineListener.swift

@@ -1,8 +1,8 @@
 import Foundation
 
-public protocol OfflinerDelegate: AnyObject {
+public protocol OfflineEngineListener: AnyObject {
 	func offliningStarted(for mediaProduct: MediaProduct)
-	func offliningProgress(for mediaProduct: MediaProduct, is downloadPercentage: Double)
+	func offliningProgressed(for mediaProduct: MediaProduct, is downloadPercentage: Double)
 	func offliningCompleted(for mediaProduct: MediaProduct)
 	func offliningFailed(for mediaProduct: MediaProduct)
 	func offlinedDeleted(for mediaProduct: MediaProduct)

Sources/Player/Player.swift

@@ -108,7 +108,8 @@ public extension Player {
 	/// logging is actually performed.
 	/// - Returns: Instance of Player if not initialized yet, or nil if initized already.
 	static func bootstrap(
-		listener: PlayerListener,
+		playerListener: PlayerListener,
+		offlineEngineListener: OfflineEngineListener? = nil,
 		listenerQueue: DispatchQueue = .main,
 		featureFlagProvider: FeatureFlagProvider = .standard,
 		externalPlayers: [GenericMediaPlayer.Type] = [],
@@ -168,7 +169,11 @@ public extension Player {
 		)
 
 		let networkMonitor = NetworkMonitor()
-		let notificationsHandler = NotificationsHandler(listener: listener, queue: listenerQueue)
+		let notificationsHandler = NotificationsHandler(
+			listener: playerListener,
+			offlineEngineListener: offlineEngineListener,
+			queue: listenerQueue
+		)
 
 		// For now, OfflineStorage and OfflineEngine can be optional.
 		// Once the functionality is finalized, Player should not work with a missing OfflineEngine.
@@ -184,7 +189,8 @@ public extension Player {
 					networkMonitor,
 					fairplayLicenseFetcher,
 					featureFlagProvider,
-					credentialsProvider
+					credentialsProvider,
+					notificationsHandler
 				)
 			}
 		}
@@ -380,11 +386,6 @@ public extension Player {
 		return offlineEngine?.getOfflineState(mediaProduct: mediaProduct) ?? .NOT_OFFLINED
 	}
 
-	func setOfflinerDelegate(_ offlinerDelegate: OfflinerDelegate) {
-		initializeOfflineEngineIfNeeded()
-		offlineEngine?.setOfflinerDelegate(offlinerDelegate)
-	}
-
 	func startDjSession(title: String) {
 		queue.dispatch {
 			let now = PlayerWorld.timeProvider.timestamp()
@@ -418,7 +419,8 @@ private extension Player {
 					networkMonitor,
 					fairplayLicenseFetcher,
 					featureFlagProvider,
-					credentialsProvider
+					credentialsProvider,
+					notificationsHandler
 				)
 			}
 		}
@@ -503,7 +505,8 @@ private extension Player {
 		_ networkMonitor: NetworkMonitor,
 		_ fairplayLicenseFetcher: FairPlayLicenseFetcher,
 		_ featureFlagProvider: FeatureFlagProvider,
-		_ credentialsProvider: CredentialsProvider
+		_ credentialsProvider: CredentialsProvider,
+		_ notificationsHandler: NotificationsHandler
 	) -> OfflineEngine {
 		let offlinerPlaybackInfoFetcher = PlaybackInfoFetcher(
 			with: configuration,
@@ -518,8 +521,12 @@ private extension Player {
 			fairPlayLicenseFetcher: fairplayLicenseFetcher,
 			networkMonitor: networkMonitor
 		)
-
-		return OfflineEngine(downloader: downloader, offlineStorage: storage, playerEventSender: playerEventSender)
+		return OfflineEngine(
+			downloader: downloader,
+			offlineStorage: storage,
+			playerEventSender: playerEventSender,
+			notificationsHandler: notificationsHandler
+		)
 	}
 }
 

TestApps/Player/PlayerTestApp/PlayerViewModel.swift

@@ -7,7 +7,7 @@ public typealias PlayerState = State
 
 // MARK: - PlayerViewModel
 
-final class PlayerViewModel: ObservableObject, PlayerListener {
+final class PlayerViewModel: ObservableObject {
 	private let CLIENT_ID = "YOUR_CLIENT_ID"
 	private let CLIENT_SECRET = "YOUR_CLIENT_SECRET"
 	private let CREDENTIALS_KEY = "YOUR_CREDENTIALS_KEY"
@@ -55,7 +55,7 @@ final class PlayerViewModel: ObservableObject, PlayerListener {
 
 	private func initPlayer() {
 		player = Player.bootstrap(
-			listener: self,
+			playerListener: self,
 			credentialsProvider: auth,
 			eventSender: eventSender
 		)
@@ -70,7 +70,11 @@ final class PlayerViewModel: ObservableObject, PlayerListener {
 		)
 		player?.play()
 	}
+}
+
+// MARK: PlayerListener
 
+extension PlayerViewModel: PlayerListener {
 	func stateChanged(to state: State) {
 		playerState = state
 	}

Tests/PlayerTests/EventsTests.swift

@@ -114,7 +114,7 @@ final class EventsTests: XCTestCase {
 
 		listener = PlayerListenerMock()
 		listenerQueue = DispatchQueue(label: "com.tidal.queue.for.testing")
-		notificationsHandler = NotificationsHandler(listener: listener, queue: listenerQueue)
+		notificationsHandler = .mock(listener: listener, queue: listenerQueue)
 
 		djProducer = DJProducer(
 			httpClient: httpClient,

Tests/PlayerTests/FeatureFlagProviderTests.swift

@@ -20,7 +20,8 @@ final class FeatureFlagProviderTests: XCTestCase {
 		shouldUseOfflineEngine = false
 
 		let playerInstance = Player.bootstrap(
-			listener: PlayerListenerMock(),
+			playerListener: PlayerListenerMock(),
+			offlineEngineListener: OfflineEngineListenerMock(),
 			listenerQueue: DispatchQueue(label: "com.tidal.queue.for.testing"),
 			featureFlagProvider: featureFlagProvider,
 			credentialsProvider: CredentialsProviderMock(),
@@ -37,7 +38,8 @@ final class FeatureFlagProviderTests: XCTestCase {
 		shouldUseOfflineEngine = true
 
 		let playerInstance = Player.bootstrap(
-			listener: PlayerListenerMock(),
+			playerListener: PlayerListenerMock(),
+			offlineEngineListener: OfflineEngineListenerMock(),
 			listenerQueue: DispatchQueue(label: "com.tidal.queue.for.testing"),
 			featureFlagProvider: featureFlagProvider,
 			credentialsProvider: CredentialsProviderMock(),
@@ -54,7 +56,8 @@ final class FeatureFlagProviderTests: XCTestCase {
 		shouldUseOfflineEngine = false
 
 		let playerInstance = Player.bootstrap(
-			listener: PlayerListenerMock(),
+			playerListener: PlayerListenerMock(),
+			offlineEngineListener: OfflineEngineListenerMock(),
 			listenerQueue: DispatchQueue(label: "com.tidal.queue.for.testing"),
 			featureFlagProvider: featureFlagProvider,
 			credentialsProvider: CredentialsProviderMock(),

Tests/PlayerTests/Mocks/Common/NotificationHandler/NotificationsHandler+Mock.swift

@@ -4,8 +4,13 @@ import Foundation
 extension NotificationsHandler {
 	static func mock(
 		listener: PlayerListener = PlayerListenerMock(),
+		offlineEngineListener: OfflineEngineListener = OfflineEngineListenerMock(),
 		queue: DispatchQueue = DispatchQueue(label: "com.tidal.queue.for.testing")
 	) -> NotificationsHandler {
-		NotificationsHandler(listener: listener, queue: queue)
+		NotificationsHandler(
+			listener: listener,
+			offlineEngineListener: offlineEngineListener,
+			queue: queue
+		)
 	}
 }

Tests/PlayerTests/Mocks/Player+Mock.swift

@@ -19,10 +19,7 @@ extension PlayerEngine {
 		storage: OfflineStorage = OfflineStorageMock(),
 		playerLoader: PlayerLoader = PlayerLoaderMock(),
 		featureFlagProvider: FeatureFlagProvider = .mock,
-		notificationsHandler: NotificationsHandler? = NotificationsHandler(
-			listener: PlayerListenerMock(),
-			queue: DispatchQueue(label: "com.tidal.queue.for.testing")
-		)
+		notificationsHandler: NotificationsHandler? = .mock(queue: DispatchQueue(label: "com.tidal.queue.for.testing"))
 	) -> PlayerEngine {
 		PlayerEngine(
 			with: queue,

Tests/PlayerTests/Player/PlaybackEngine/PlayerEngineTests.swift

@@ -75,7 +75,7 @@ final class PlayerEngineTests: XCTestCase {
 		playerEventSender = PlayerEventSenderMock()
 		listener = PlayerListenerMock()
 		listenerQueue = DispatchQueue(label: "com.tidal.queue.for.testing")
-		notificationsHandler = NotificationsHandler(listener: listener, queue: listenerQueue)
+		notificationsHandler = .mock(listener: listener, queue: listenerQueue)
 		credentialsProvider = CredentialsProviderMock()
 		featureFlagProvider = FeatureFlagProvider.mock
 		featureFlagProvider.shouldUseImprovedCaching = {

Tests/PlayerTests/PlayerStateTests.swift

@@ -46,7 +46,7 @@ final class PlayerStateTests: XCTestCase {
 
 		listener = PlayerListenerMock()
 		listenerQueue = DispatchQueue(label: "com.tidal.queue.for.testing")
-		notificationsHandler = NotificationsHandler(listener: listener, queue: listenerQueue)
+		notificationsHandler = .mock(listener: listener, queue: listenerQueue)
 
 		credentialsProvider = CredentialsProviderMock()
 		featureFlagProvider = FeatureFlagProvider.mock