From 046c7aafa5a625af37a4469c70f5400702014b24 Mon Sep 17 00:00:00 2001 From: Sebastian Villena <97059974+ruisebas@users.noreply.github.com> Date: Thu, 7 Dec 2023 12:30:36 -0500 Subject: [PATCH] fix(Analytics): Fixing session duration being reported for non-stopped sessions. (#3403) --- .../PinpointEvent+PinpointClientTypes.swift | 18 ++++++++++- .../Session/PinpointSession.swift | 7 +++-- .../EventRecorderTests.swift | 31 +++++++++++++++++++ .../Mocks/MockPinpointClient.swift | 2 ++ 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/AmplifyPlugins/Internal/Sources/InternalAWSPinpoint/Analytics/PinpointEvent+PinpointClientTypes.swift b/AmplifyPlugins/Internal/Sources/InternalAWSPinpoint/Analytics/PinpointEvent+PinpointClientTypes.swift index 42dc924992..a916d7dc2b 100644 --- a/AmplifyPlugins/Internal/Sources/InternalAWSPinpoint/Analytics/PinpointEvent+PinpointClientTypes.swift +++ b/AmplifyPlugins/Internal/Sources/InternalAWSPinpoint/Analytics/PinpointEvent+PinpointClientTypes.swift @@ -10,7 +10,14 @@ import AWSPluginsCore import Foundation extension PinpointEvent { - var clientTypeSession: PinpointClientTypes.Session { + var clientTypeSession: PinpointClientTypes.Session? { + #if os(watchOS) + // If the session duration cannot be represented by Int, return a nil session instead. + // This is extremely unlikely to happen since a session's stopTime is set when the app is closed + if let duration = session.duration, duration > Int.max { + return nil + } + #endif return PinpointClientTypes.Session(duration: Int(session.duration), id: session.sessionId, startTimestamp: session.startTime.asISO8601String, @@ -48,3 +55,12 @@ extension Bundle { object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String ?? "" } } + +private extension Int { + init?(_ value: Int64?) { + guard let value = value else { + return nil + } + self.init(value) + } +} diff --git a/AmplifyPlugins/Internal/Sources/InternalAWSPinpoint/Session/PinpointSession.swift b/AmplifyPlugins/Internal/Sources/InternalAWSPinpoint/Session/PinpointSession.swift index 86514dff25..819d76bc43 100644 --- a/AmplifyPlugins/Internal/Sources/InternalAWSPinpoint/Session/PinpointSession.swift +++ b/AmplifyPlugins/Internal/Sources/InternalAWSPinpoint/Session/PinpointSession.swift @@ -35,8 +35,11 @@ public class PinpointSession: Codable { return stopTime != nil } - var duration: Date.Millisecond { - let endTime = stopTime ?? Date() + var duration: Date.Millisecond? { + /// According to Pinpoint's documentation, `duration` is only required if `stopTime` is not nil. + guard let endTime = stopTime else { + return nil + } return endTime.millisecondsSince1970 - startTime.millisecondsSince1970 } diff --git a/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/EventRecorderTests.swift b/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/EventRecorderTests.swift index bf7a70f69c..ee604010b9 100644 --- a/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/EventRecorderTests.swift +++ b/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/EventRecorderTests.swift @@ -94,6 +94,37 @@ class EventRecorderTests: XCTestCase { XCTAssertEqual(storage.deleteEventCallCount, 2) } + /// - Given: a event recorder with events saved in the local storage with active and stopped sessions + /// - When: submitAllEvents is invoked + /// - Then: the input is generated accordingly by including duration only for the stopped session + func testSubmitAllEvents_withActiveAndStoppedSessions_shouldGenerateRightInput() async throws { + let activeSession = PinpointSession( + sessionId: "active", + startTime: Date(), + stopTime: nil + ) + let stoppedSession = PinpointSession( + sessionId: "stopped", + startTime: Date().addingTimeInterval(-10), + stopTime: Date() + ) + storage.events = [ + .init(id: "1", eventType: "eventType1", eventDate: Date(), session: activeSession), + .init(id: "2", eventType: "eventType2", eventDate: Date(), session: stoppedSession) + ] + + _ = try? await recorder.submitAllEvents() + XCTAssertEqual(pinpointClient.putEventsCount, 1) + let input = try XCTUnwrap(pinpointClient.putEventsLastInput) + let batchItem = try XCTUnwrap(input.eventsRequest?.batchItem?.first?.value as? PinpointClientTypes.EventsBatch) + let events = try XCTUnwrap(batchItem.events) + XCTAssertEqual(events.count, 2) + XCTAssertNotNil(events["1"]?.session, "Expected session for eventType1") + XCTAssertNil(events["1"]?.session?.duration, "Expected nil session duration for eventType") + XCTAssertNotNil(events["2"]?.session, "Expected session for eventType2") + XCTAssertNotNil(events["2"]?.session?.duration, "Expected session duration for eventType2") + } + /// - Given: a event recorder with events saved in the local storage /// - When: submitAllEvents is invoked and fails with a non-retryable error /// - Then: the events are marked as dirty diff --git a/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockPinpointClient.swift b/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockPinpointClient.swift index e0380c028e..4bc8c814d8 100644 --- a/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockPinpointClient.swift +++ b/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockPinpointClient.swift @@ -367,8 +367,10 @@ class MockPinpointClient: PinpointClientProtocol { var putEventsCount = 0 var putEventsResult: Result = .failure(CancellationError()) + var putEventsLastInput: PutEventsInput? func putEvents(input: PutEventsInput) async throws -> PutEventsOutput { putEventsCount += 1 + putEventsLastInput = input return try putEventsResult.get() }