From 74850c45a1ccad02edc842bb40ac136aa13df2ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20D=C3=A9fago?= Date: Sun, 10 Nov 2024 22:40:01 +0100 Subject: [PATCH] Fix player observability in response to media accessibility updates (#1061) Co-authored-by: Walid Kayhal <3347810+waliid@users.noreply.github.com> --- .../xcshareddata/swiftpm/Package.resolved | 4 +- Demo/Sources/Model/MediaDescription.swift | 2 +- Package.resolved | 4 +- .../Expectations/ExpectValue.swift | 32 +++++++++++++++ Sources/Core/Publisher.swift | 2 +- Sources/Core/Stopwatch.swift | 6 +-- Sources/Monitoring/MetricsTracker.swift | 4 +- Sources/Monitoring/Types/MetricPayload.swift | 2 +- .../MediaSelectionProperties.swift | 6 ++- Sources/Player/PlayerItem.swift | 4 +- .../Publishers/AVPlayerItemPublishers.swift | 15 ++++--- .../Publishers/PlayerItemPublishers.swift | 2 +- .../Expectations/ExpectValueTests.swift | 40 +++++++++++++++++++ .../MediaSelection/MediaSelectionTests.swift | 15 +++++++ Tests/PlayerTests/Player/QueueTests.swift | 10 ----- 15 files changed, 116 insertions(+), 32 deletions(-) create mode 100644 Sources/Circumspect/Expectations/ExpectValue.swift create mode 100644 Tests/CircumspectTests/Expectations/ExpectValueTests.swift diff --git a/Demo/Pillarbox-demo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Demo/Pillarbox-demo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index ad2d40c05..73d1f14a3 100644 --- a/Demo/Pillarbox-demo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Demo/Pillarbox-demo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -77,8 +77,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/Quick/Nimble.git", "state" : { - "revision" : "6416749c3c0488664fff6b42f8bf3ea8dc282ca1", - "version" : "13.6.0" + "revision" : "f3bedd7c2a858f4513e7d004def200a69238db2f", + "version" : "13.6.2" } }, { diff --git a/Demo/Sources/Model/MediaDescription.swift b/Demo/Sources/Model/MediaDescription.swift index d70618e8d..8fdd86907 100644 --- a/Demo/Sources/Model/MediaDescription.swift +++ b/Demo/Sources/Model/MediaDescription.swift @@ -53,7 +53,7 @@ enum MediaDescription { } static func style(for media: SRGMedia) -> Style { - media.timeAvailability(at: Date()) == .available ? .standard : .disabled + media.timeAvailability(at: .now) == .available ? .standard : .disabled } static func date(for media: SRGMedia) -> String { diff --git a/Package.resolved b/Package.resolved index 0118ce7e1..1fa702e5e 100644 --- a/Package.resolved +++ b/Package.resolved @@ -50,8 +50,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/Quick/Nimble.git", "state" : { - "revision" : "6416749c3c0488664fff6b42f8bf3ea8dc282ca1", - "version" : "13.6.0" + "revision" : "f3bedd7c2a858f4513e7d004def200a69238db2f", + "version" : "13.6.2" } }, { diff --git a/Sources/Circumspect/Expectations/ExpectValue.swift b/Sources/Circumspect/Expectations/ExpectValue.swift new file mode 100644 index 000000000..956da8202 --- /dev/null +++ b/Sources/Circumspect/Expectations/ExpectValue.swift @@ -0,0 +1,32 @@ +// +// Copyright (c) SRG SSR. All rights reserved. +// +// License information is available from the LICENSE file. +// + +import Combine +import XCTest + +public extension XCTestCase { + /// Expects a publisher to emit at least a value. + func expectValue

( + from publisher: P, + timeout: DispatchTimeInterval = .seconds(20), + file: StaticString = #file, + line: UInt = #line, + while executing: (() -> Void)? = nil + ) where P: Publisher { + expectSuccess(from: publisher.first(), timeout: timeout, file: file, line: line, while: executing) + } + + /// Expects an observable object to publish at least a change. + func expectChange( + from object: O, + timeout: DispatchTimeInterval = .seconds(20), + file: StaticString = #file, + line: UInt = #line, + while executing: (() -> Void)? = nil + ) where O: ObservableObject { + expectValue(from: object.objectWillChange, timeout: timeout, file: file, line: line, while: executing) + } +} diff --git a/Sources/Core/Publisher.swift b/Sources/Core/Publisher.swift index c5b5456fd..a9f77a009 100644 --- a/Sources/Core/Publisher.swift +++ b/Sources/Core/Publisher.swift @@ -195,7 +195,7 @@ public extension Publisher { ) -> AnyPublisher where S: Scheduler, S.SchedulerTimeType == DispatchQueue.SchedulerTimeType { measureInterval(using: scheduler) .map { stride in - let date = Date() + let date = Date.now return DateInterval(start: date.advanced(by: -TimeInterval(from: stride)), end: date) } .eraseToAnyPublisher() diff --git a/Sources/Core/Stopwatch.swift b/Sources/Core/Stopwatch.swift index 18d244830..01104576c 100644 --- a/Sources/Core/Stopwatch.swift +++ b/Sources/Core/Stopwatch.swift @@ -17,20 +17,20 @@ public final class Stopwatch { /// Starts the stopwatch. public func start() { guard date == nil else { return } - date = Date() + date = .now } /// Stops the stopwatch. public func stop() { guard let date else { return } - total += Date().timeIntervalSince(date) + total += Date.now.timeIntervalSince(date) self.date = nil } /// The time accumulated by the stopwatch. public func time() -> TimeInterval { if let date { - return total + Date().timeIntervalSince(date) + return total + Date.now.timeIntervalSince(date) } else { return total diff --git a/Sources/Monitoring/MetricsTracker.swift b/Sources/Monitoring/MetricsTracker.swift index e4015dd33..91db761f4 100644 --- a/Sources/Monitoring/MetricsTracker.swift +++ b/Sources/Monitoring/MetricsTracker.swift @@ -52,10 +52,10 @@ public final class MetricsTracker: PlayerItemTracker { sendEvent(name: .start, data: startData(from: events)) startHeartbeat() case .stall: - stallDate = Date() + stallDate = .now case .resumeAfterStall: guard let stallDate else { break } - stallDuration += Date().timeIntervalSince(stallDate) + stallDuration += Date.now.timeIntervalSince(stallDate) case let .failure(error): if !session.isStarted { session.start() diff --git a/Sources/Monitoring/Types/MetricPayload.swift b/Sources/Monitoring/Types/MetricPayload.swift index c3522a946..ae3211011 100644 --- a/Sources/Monitoring/Types/MetricPayload.swift +++ b/Sources/Monitoring/Types/MetricPayload.swift @@ -10,6 +10,6 @@ struct MetricPayload: Encodable where Data: Encodable { let version = 1 let sessionId: String let eventName: EventName - let timestamp = Date().timestamp + let timestamp = Date.now.timestamp let data: Data } diff --git a/Sources/Player/MediaSelection/MediaSelectionProperties.swift b/Sources/Player/MediaSelection/MediaSelectionProperties.swift index a4839c96a..72e7846d7 100644 --- a/Sources/Player/MediaSelection/MediaSelectionProperties.swift +++ b/Sources/Player/MediaSelection/MediaSelectionProperties.swift @@ -7,14 +7,16 @@ import AVFoundation struct MediaSelectionProperties: Equatable { - static let empty = Self(groups: [:], selection: nil) + static let empty = Self(groups: [:], selection: nil, settingsChangeDate: .now) private let groups: [AVMediaCharacteristic: AVMediaSelectionGroup] let selection: AVMediaSelection? + private let settingsChangeDate: Date - init(groups: [AVMediaCharacteristic: AVMediaSelectionGroup], selection: AVMediaSelection?) { + init(groups: [AVMediaCharacteristic: AVMediaSelectionGroup], selection: AVMediaSelection?, settingsChangeDate: Date) { self.groups = groups self.selection = selection + self.settingsChangeDate = settingsChangeDate } } diff --git a/Sources/Player/PlayerItem.swift b/Sources/Player/PlayerItem.swift index 87818e4f1..3501558f0 100644 --- a/Sources/Player/PlayerItem.swift +++ b/Sources/Player/PlayerItem.swift @@ -38,7 +38,7 @@ public final class PlayerItem: Equatable { Publishers.PublishAndRepeat(onOutputFrom: Self.trigger.signal(activatedBy: TriggerId.reset(id))) { [id] in Publishers.CombineLatest( publisher, - Just(Date()).setFailureType(to: P.Failure.self) + Just(Date.now).setFailureType(to: P.Failure.self) ) .handleEvents(receiveOutput: { asset, _ in trackerAdapters.forEach { adapter in @@ -49,7 +49,7 @@ public final class PlayerItem: Equatable { Publishers.CombineLatest3( Just(asset), metadataMapper(asset.metadata).playerMetadataPublisher(), - Just(DateInterval(start: startDate, end: Date())) + Just(DateInterval(start: startDate, end: .now)) ) } .switchToLatest() diff --git a/Sources/Player/Publishers/AVPlayerItemPublishers.swift b/Sources/Player/Publishers/AVPlayerItemPublishers.swift index 14e3f854d..0fa5516b1 100644 --- a/Sources/Player/Publishers/AVPlayerItemPublishers.swift +++ b/Sources/Player/Publishers/AVPlayerItemPublishers.swift @@ -76,12 +76,10 @@ extension AVPlayerItem { Publishers.CombineLatest3( asset.mediaSelectionGroupsPublisher(), mediaSelectionPublisher(), - NotificationCenter.default.publisher(for: kMACaptionAppearanceSettingsChangedNotification as Notification.Name) - .map { _ in } - .prepend(()) + mediaAccessibilityCaptionAppearanceSettingsChangeDatePublisher() ) - .map { groups, selection, _ in - MediaSelectionProperties(groups: groups, selection: selection) + .map { groups, selection, settingsChangeDate in + MediaSelectionProperties(groups: groups, selection: selection, settingsChangeDate: settingsChangeDate) } .prepend(.empty) .removeDuplicates() @@ -102,6 +100,13 @@ extension AVPlayerItem { .eraseToAnyPublisher() } + private func mediaAccessibilityCaptionAppearanceSettingsChangeDatePublisher() -> AnyPublisher { + NotificationCenter.default.publisher(for: kMACaptionAppearanceSettingsChangedNotification as Notification.Name) + .map { _ in .now } + .prepend(.now) + .eraseToAnyPublisher() + } + private func minimumTimeOffsetFromLivePublisher() -> AnyPublisher { asset.propertyPublisher(.minimumTimeOffsetFromLive) .replaceError(with: .invalid) diff --git a/Sources/Player/Publishers/PlayerItemPublishers.swift b/Sources/Player/Publishers/PlayerItemPublishers.swift index 7dfb807df..bcb061d0b 100644 --- a/Sources/Player/Publishers/PlayerItemPublishers.swift +++ b/Sources/Player/Publishers/PlayerItemPublishers.swift @@ -25,7 +25,7 @@ extension PlayerItem { $content .compactMap(\.dateInterval) .removeDuplicates(), - Just(Date()) + Just(Date.now) ) .map { dateInterval, startDate in MetricEvent( diff --git a/Tests/CircumspectTests/Expectations/ExpectValueTests.swift b/Tests/CircumspectTests/Expectations/ExpectValueTests.swift new file mode 100644 index 000000000..9fe7cdb0b --- /dev/null +++ b/Tests/CircumspectTests/Expectations/ExpectValueTests.swift @@ -0,0 +1,40 @@ +// +// Copyright (c) SRG SSR. All rights reserved. +// +// License information is available from the LICENSE file. +// + +@testable import PillarboxCircumspect + +import Combine +import XCTest + +private class Object: ObservableObject { + @Published var value = 0 +} + +final class ExpectValueTests: XCTestCase { + func testSingleValue() { + expectValue(from: Just(1)) + } + + func testMultipleValues() { + expectValue(from: [1, 2, 3].publisher) + } + + func testSingleChange() { + let object = Object() + expectChange(from: object) { + object.value = 1 + } + } + + func testMultipleChanges() { + let object = Object() + expectChange(from: object) { + object.value = 1 + object.value = 2 + object.value = 3 + } + } +} diff --git a/Tests/PlayerTests/MediaSelection/MediaSelectionTests.swift b/Tests/PlayerTests/MediaSelection/MediaSelectionTests.swift index 856946b65..f4a8a097c 100644 --- a/Tests/PlayerTests/MediaSelection/MediaSelectionTests.swift +++ b/Tests/PlayerTests/MediaSelection/MediaSelectionTests.swift @@ -8,6 +8,7 @@ import AVFoundation import Nimble +import PillarboxCircumspect import PillarboxStreams final class MediaSelectionTests: TestCase { @@ -272,6 +273,20 @@ final class MediaSelectionTests: TestCase { expect(player.selectedMediaOption(for: .legible)).toEventually(equal(.automatic)) expect(player.currentMediaOption(for: .legible)).to(equal(.off)) } + + func testObservabilityWhenTogglingBetweenOffAndAutomatic() { + MediaAccessibilityDisplayType.forcedOnly.apply() + + let player = Player(item: .simple(url: Stream.onDemandWithOptions.url)) + expect(player.mediaSelectionOptions(for: .legible)).toEventuallyNot(beEmpty()) + + expectChange(from: player) { + player.select(mediaOption: .automatic, for: .legible) + } + expectChange(from: player) { + player.select(mediaOption: .off, for: .legible) + } + } } private extension Player { diff --git a/Tests/PlayerTests/Player/QueueTests.swift b/Tests/PlayerTests/Player/QueueTests.swift index c3f5c5043..a6806eedc 100644 --- a/Tests/PlayerTests/Player/QueueTests.swift +++ b/Tests/PlayerTests/Player/QueueTests.swift @@ -115,16 +115,6 @@ final class QueueTests: TestCase { expect(player.currentItem).to(equal(item2)) } - func testFailingUnauthorizedItemBetweenPlayableItems() { - let item1 = PlayerItem.simple(url: Stream.shortOnDemand.url) - let item2 = PlayerItem.simple(url: Stream.unauthorized.url) - let item3 = PlayerItem.simple(url: Stream.onDemand.url) - let player = Player(items: [item1, item2, item3]) - player.play() - expect(player.urls).toEventually(beEmpty()) - expect(player.currentItem).to(equal(item2)) - } - func testFailingMp3ItemBetweenPlayableItems() { let item1 = PlayerItem.simple(url: Stream.shortOnDemand.url) let item2 = PlayerItem.simple(url: Stream.unavailableMp3.url)