Fix incorrectly received tracker property updates (#770)

Co-authored-by: Walid Kayhal <[email protected]>

Sources/Player/Asset.swift

@@ -39,6 +39,20 @@ public struct Asset<M>: Assetable where M: AssetMetadata {
     private let configuration: (AVPlayerItem) -> Void
     private let trackerAdapters: [TrackerAdapter<M>]
 
+    init(
+        id: UUID,
+        resource: Resource,
+        metadata: M?,
+        configuration: @escaping (AVPlayerItem) -> Void,
+        trackerAdapters: [TrackerAdapter<M>]
+    ) {
+        self.id = id
+        self.resource = resource
+        self.metadata = metadata
+        self.configuration = configuration
+        self.trackerAdapters = trackerAdapters.map { $0.withId(id) }
+    }
+
     /// Returns a simple asset playable from a URL.
     /// 
     /// - Parameters:

Sources/Player/Player.swift

@@ -321,7 +321,7 @@ private extension Player {
         .sink { [weak self] nowPlayingInfoMetadata, nowPlayingInfoPlayback, isActive in
             guard let self else { return }
             let nowPlayingInfo = isActive ? nowPlayingInfoMetadata.merging(nowPlayingInfoPlayback) { _, new in new } : [:]
-            self.updateControlCenter(nowPlayingInfo: nowPlayingInfo)
+            updateControlCenter(nowPlayingInfo: nowPlayingInfo)
         }
         .store(in: &cancellables)
     }

Sources/Player/Publishers/AVPlayerItemPublishers.swift

@@ -20,10 +20,11 @@ extension AVPlayerItem {
             publisher(for: \.duration),
             minimumTimeOffsetFromLivePublisher()
         )
-        .map { state, presentationSize, mediaSelectionProperties, timeProperties, duration, minimumTimeOffsetFromLive in
+        .map { [weak self] state, presentationSize, mediaSelectionProperties, timeProperties, duration, minimumTimeOffsetFromLive in
             let isKnown = (state != .unknown)
             return .init(
                 itemProperties: .init(
+                    item: self,
                     state: state,
                     duration: isKnown ? duration : .invalid,
                     minimumTimeOffsetFromLive: minimumTimeOffsetFromLive,

Sources/Player/Tracking/TrackerAdapter.swift

@@ -5,6 +5,7 @@
 //
 
 import Combine
+import Foundation
 
 /// An adapter which instantiates and manages a tracker of a specified type.
 ///
@@ -13,6 +14,7 @@ public class TrackerAdapter<M: AssetMetadata> {
     private let tracker: any PlayerItemTracker
     private let update: (M) -> Void
     private var cancellables = Set<AnyCancellable>()
+    private var id = UUID()
 
     /// Creates an adapter for a type of tracker with the provided mapping to its metadata format.
     /// 
@@ -30,12 +32,18 @@ public class TrackerAdapter<M: AssetMetadata> {
         self.tracker = tracker
     }
 
+    func withId(_ id: UUID) -> Self {
+        self.id = id
+        return self
+    }
+
     func enable(for player: Player) {
         tracker.enable(for: player)
 
         player.propertiesPublisher
             .sink { [weak self] properties in
-                self?.tracker.updateProperties(with: properties)
+                guard let self, properties.id == id else { return }
+                tracker.updateProperties(with: properties)
             }
             .store(in: &cancellables)
     }

Sources/Player/Types/ItemProperties.swift

@@ -4,18 +4,21 @@
 //  License information is available from the LICENSE file.
 //
 
+import AVFoundation
 import CoreMedia
 
 struct ItemProperties: Equatable {
     static var empty: Self {
         .init(
+            item: nil,
             state: .unknown,
             duration: .invalid,
             minimumTimeOffsetFromLive: .invalid,
             presentationSize: nil
         )
     }
 
+    let item: AVPlayerItem?
     let state: ItemState
 
     let duration: CMTime

Sources/Player/Types/PlayerProperties.swift

@@ -24,6 +24,10 @@ public struct PlayerProperties: Equatable {
     /// The time at which the player is currently seeking, if any.
     public let seekTime: CMTime?
 
+    var id: UUID {
+        coreProperties.itemProperties.item?.id ?? UUID()
+    }
+
     init(
         coreProperties: CoreProperties,
         timeProperties: TimeProperties,

Tests/PlayerTests/Tools/NoMetadataTrackerMock.swift → Tests/PlayerTests/Tools/TrackerLifeCycleMock.swift

@@ -7,15 +7,15 @@
 @testable import PillarboxPlayer
 
 import Combine
+import Foundation
 
-final class NoMetadataTrackerMock: PlayerItemTracker {
+final class TrackerLifeCycleMock: PlayerItemTracker {
     typealias StatePublisher = PassthroughSubject<State, Never>
 
-    enum State {
+    enum State: Equatable {
         case initialized
         case enabled
         case disabled
-        case updated
         case deinitialized
     }
 
@@ -31,9 +31,7 @@ final class NoMetadataTrackerMock: PlayerItemTracker {
         configuration.statePublisher.send(.initialized)
     }
 
-    func updateMetadata(with metadata: Void) {
-        configuration.statePublisher.send(.updated)
-    }
+    func updateMetadata(with metadata: Void) {}
 
     func updateProperties(with properties: PlayerProperties) {}
 
@@ -50,7 +48,7 @@ final class NoMetadataTrackerMock: PlayerItemTracker {
     }
 }
 
-extension NoMetadataTrackerMock {
+extension TrackerLifeCycleMock {
     static func adapter(statePublisher: StatePublisher) -> TrackerAdapter<Never> {
         adapter(configuration: Configuration(statePublisher: statePublisher))
     }

Tests/PlayerTests/Tools/TrackerMock.swift → Tests/PlayerTests/Tools/TrackerUpdateMock.swift

@@ -7,16 +7,16 @@
 @testable import PillarboxPlayer
 
 import Combine
+import Foundation
 
-final class TrackerMock<Metadata>: PlayerItemTracker where Metadata: Equatable {
+final class TrackerUpdateMock<Metadata>: PlayerItemTracker where Metadata: Equatable {
     typealias StatePublisher = PassthroughSubject<State, Never>
 
     enum State: Equatable {
-        case initialized
         case enabled
         case disabled
-        case updated(Metadata)
-        case deinitialized
+        case updatedMetadata(Metadata)
+        case updatedProperties(for: UUID)
     }
 
     struct Configuration {
@@ -28,30 +28,31 @@ final class TrackerMock<Metadata>: PlayerItemTracker where Metadata: Equatable {
 
     init(configuration: Configuration) {
         self.configuration = configuration
-        configuration.statePublisher.send(.initialized)
     }
 
     func enable(for player: Player) {
         configuration.statePublisher.send(.enabled)
     }
 
     func updateMetadata(with metadata: Metadata) {
-        configuration.statePublisher.send(.updated(metadata))
+        configuration.statePublisher.send(.updatedMetadata(metadata))
     }
 
-    func updateProperties(with properties: PlayerProperties) {}
+    func updateProperties(with properties: PlayerProperties) {
+        configuration.statePublisher.send(.updatedProperties(for: properties.id))
+    }
 
     func disable() {
         configuration.statePublisher.send(.disabled)
     }
-
-    deinit {
-        configuration.statePublisher.send(.deinitialized)
-    }
 }
 
-extension TrackerMock {
+extension TrackerUpdateMock {
     static func adapter<M>(statePublisher: StatePublisher, mapper: @escaping (M) -> Metadata) -> TrackerAdapter<M> where M: AssetMetadata {
         adapter(configuration: Configuration(statePublisher: statePublisher), mapper: mapper)
     }
+
+    static func adapter(statePublisher: StatePublisher) -> TrackerAdapter<Never> {
+        adapter(configuration: Configuration(statePublisher: statePublisher))
+    }
 }

Tests/PlayerTests/Tracking/PlayerItemTrackerLifeCycleTests.swift

@@ -0,0 +1,72 @@
+//
+//  Copyright (c) SRG SSR. All rights reserved.
+//
+//  License information is available from the LICENSE file.
+//
+
+@testable import PillarboxPlayer
+
+import Foundation
+import PillarboxCircumspect
+import PillarboxStreams
+
+final class PlayerItemTrackerLifeCycleTests: TestCase {
+    func testPlayerItemLifeCycle() {
+        let publisher = TrackerLifeCycleMock.StatePublisher()
+        expectAtLeastEqualPublished(values: [.initialized, .deinitialized], from: publisher) {
+            _ = PlayerItem.simple(
+                url: Stream.shortOnDemand.url,
+                trackerAdapters: [TrackerLifeCycleMock.adapter(statePublisher: publisher)]
+            )
+        }
+    }
+
+    func testItemPlayback() {
+        let player = Player()
+        let publisher = TrackerLifeCycleMock.StatePublisher()
+        expectEqualPublished(values: [.initialized, .enabled], from: publisher, during: .seconds(2)) {
+            player.append(.simple(
+                url: Stream.onDemand.url,
+                trackerAdapters: [TrackerLifeCycleMock.adapter(statePublisher: publisher)]
+            ))
+            player.play()
+        }
+    }
+
+    func testItemEntirePlayback() {
+        let player = Player()
+        let publisher = TrackerLifeCycleMock.StatePublisher()
+        expectAtLeastEqualPublished(values: [.initialized, .enabled, .disabled], from: publisher) {
+            player.append(.simple(
+                url: Stream.shortOnDemand.url,
+                trackerAdapters: [TrackerLifeCycleMock.adapter(statePublisher: publisher)]
+            ))
+            player.play()
+        }
+    }
+
+    func testNetworkLoadedItemEntirePlayback() {
+        let player = Player()
+        let publisher = TrackerLifeCycleMock.StatePublisher()
+        expectAtLeastEqualPublished(values: [.initialized, .enabled, .disabled], from: publisher) {
+            player.append(.mock(
+                url: Stream.shortOnDemand.url,
+                loadedAfter: 1,
+                trackerAdapters: [TrackerLifeCycleMock.adapter(statePublisher: publisher)]
+            ))
+            player.play()
+        }
+    }
+
+    func testFailedItem() {
+        let player = Player()
+        let publisher = TrackerLifeCycleMock.StatePublisher()
+        expectEqualPublished(values: [.initialized, .enabled], from: publisher, during: .milliseconds(500)) {
+            player.append(.simple(
+                url: Stream.unavailable.url,
+                trackerAdapters: [TrackerLifeCycleMock.adapter(statePublisher: publisher)]
+            ))
+            player.play()
+        }
+    }
+}