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 1/2] 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() } From 56d5339b25f6f60b8294b08a25a0cfd3793fffeb Mon Sep 17 00:00:00 2001 From: Harsh <6162866+harsh62@users.noreply.github.com> Date: Thu, 7 Dec 2023 13:37:52 -0500 Subject: [PATCH 2/2] feat(Auth): Adding network preferences (#3379) * feat(Auth): Adding network preferences * adding tests * Update AWSCognitoNetworkPreferences.swift * Update AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoNetworkPreferences.swift Co-authored-by: Sebastian Villena <97059974+ruisebas@users.noreply.github.com> * worked on review comments --------- Co-authored-by: Sebastian Villena <97059974+ruisebas@users.noreply.github.com> --- .../AWSCognitoAuthPlugin+Configure.swift | 27 +++++++++++ .../AWSCognitoAuthPlugin.swift | 11 +++++ .../Models/AWSCognitoNetworkPreferences.swift | 31 +++++++++++++ .../AWSCognitoAuthPluginConfigTests.swift | 46 +++++++++++++++++++ 4 files changed, 115 insertions(+) create mode 100644 AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoNetworkPreferences.swift diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift index 2fb118bdfa..4fce0d3311 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift @@ -102,6 +102,15 @@ extension AWSCognitoAuthPlugin { configuration.httpClientEngine = .userAgentEngine(for: configuration) } + if let requestTimeout = networkPreferences?.timeoutIntervalForRequest { + let requestTimeOutMs = requestTimeout * 1_000 + configuration.connectTimeoutMs = UInt32(requestTimeOutMs) + } + + if let maxRetryUnwrapped = networkPreferences?.maxRetryCount { + configuration.retryStrategyOptions = RetryStrategyOptions(maxRetriesBase: Int(maxRetryUnwrapped)) + } + return CognitoIdentityProviderClient(config: configuration) default: fatalError() @@ -116,6 +125,15 @@ extension AWSCognitoAuthPlugin { ) configuration.httpClientEngine = .userAgentEngine(for: configuration) + if let requestTimeout = networkPreferences?.timeoutIntervalForRequest { + let requestTimeOutMs = requestTimeout * 1_000 + configuration.connectTimeoutMs = UInt32(requestTimeOutMs) + } + + if let maxRetryUnwrapped = networkPreferences?.maxRetryCount { + configuration.retryStrategyOptions = RetryStrategyOptions(maxRetriesBase: Int(maxRetryUnwrapped)) + } + return CognitoIdentityClient(config: configuration) default: fatalError() @@ -129,6 +147,15 @@ extension AWSCognitoAuthPlugin { private func makeURLSession() -> URLSession { let configuration = URLSessionConfiguration.default configuration.urlCache = nil + + if let timeoutIntervalForRequest = networkPreferences?.timeoutIntervalForRequest { + configuration.timeoutIntervalForRequest = timeoutIntervalForRequest + } + + if let timeoutIntervalForResource = networkPreferences?.timeoutIntervalForResource { + configuration.timeoutIntervalForResource = timeoutIntervalForResource + } + return URLSession(configuration: configuration) } diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift index 473951844d..f32209ba9c 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift @@ -32,6 +32,9 @@ public final class AWSCognitoAuthPlugin: AWSCognitoAuthPluginBehavior { var httpClientEngineProxy: HttpClientEngineProxy? + /// The user network preferences for timeout and retry + let networkPreferences: AWSCognitoNetworkPreferences? + @_spi(InternalAmplifyConfiguration) internal(set) public var jsonConfiguration: JSONValue? @@ -42,5 +45,13 @@ public final class AWSCognitoAuthPlugin: AWSCognitoAuthPluginBehavior { /// Instantiates an instance of the AWSCognitoAuthPlugin. public init() { + self.networkPreferences = nil + } + + /// Instantiates an instance of the AWSCognitoAuthPlugin with custom network preferences + /// - Parameters: + /// - networkPreferences: network preferences + public init(networkPreferences: AWSCognitoNetworkPreferences) { + self.networkPreferences = networkPreferences } } diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoNetworkPreferences.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoNetworkPreferences.swift new file mode 100644 index 0000000000..2be4f3279b --- /dev/null +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoNetworkPreferences.swift @@ -0,0 +1,31 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import Foundation + +public struct AWSCognitoNetworkPreferences { + + /// The maximum number of retries for failed requests. + public let maxRetryCount: UInt32 + + /// The timeout interval to use when waiting for additional data. + public let timeoutIntervalForRequest: TimeInterval + + /// The maximum amount of time that a resource request should be allowed to take. + /// + /// NOTE: This value is only applicable to HostedUI because the underlying Swift SDK does + /// not support resource timeouts + public let timeoutIntervalForResource: TimeInterval? + + public init(maxRetryCount: UInt32, + timeoutIntervalForRequest: TimeInterval, + timeoutIntervalForResource: TimeInterval? = nil) { + self.maxRetryCount = maxRetryCount + self.timeoutIntervalForRequest = timeoutIntervalForRequest + self.timeoutIntervalForResource = timeoutIntervalForResource + } +} diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift index 8f6fce1eff..8ba574028e 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift @@ -190,4 +190,50 @@ class AWSCognitoAuthPluginConfigTests: XCTestCase { } } + /// Test Auth configuration with valid config for user pool and identity pool, with network preferences + /// + /// - Given: Given valid config for user pool and identity pool, and network preferences + /// - When: + /// - I configure auth with the given configuration and network preferences + /// - Then: + /// - I should not get any error while configuring auth + /// + func testConfigWithUserPoolAndIdentityPoolWithNetworkPreferences() throws { + let plugin = AWSCognitoAuthPlugin( + networkPreferences: .init( + maxRetryCount: 2, + timeoutIntervalForRequest: 60, + timeoutIntervalForResource: 60)) + try Amplify.add(plugin: plugin) + + let categoryConfig = AuthCategoryConfiguration(plugins: [ + "awsCognitoAuthPlugin": [ + "CredentialsProvider": ["CognitoIdentity": ["Default": + ["PoolId": "xx", + "Region": "us-east-1"] + ]], + "CognitoUserPool": ["Default": [ + "PoolId": "xx", + "Region": "us-east-1", + "AppClientId": "xx", + "AppClientSecret": "xx"]] + ] + ]) + let amplifyConfig = AmplifyConfiguration(auth: categoryConfig) + do { + try Amplify.configure(amplifyConfig) + + let escapeHatch = plugin.getEscapeHatch() + guard case .userPoolAndIdentityPool(let userPoolClient, let identityPoolClient) = escapeHatch else { + XCTFail("Expected .userPool, got \(escapeHatch)") + return + } + XCTAssertNotNil(userPoolClient) + XCTAssertNotNil(identityPoolClient) + + } catch { + XCTFail("Should not throw error. \(error)") + } + } + }