From 6bc265fb22f6d059de3f9d7648e612b5748992a7 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Fri, 27 Sep 2024 17:54:22 -0400 Subject: [PATCH 01/17] Handle logger creation failure --- .../kotlin/io/bitdrift/capture/Capture.kt | 84 ++++++++++---- .../io/bitdrift/capture/CaptureJniLibrary.kt | 4 +- .../kotlin/io/bitdrift/capture/LoggerImpl.kt | 25 ++++- .../kotlin/io/bitdrift/capture/CaptureTest.kt | 6 +- .../io/bitdrift/capture/ConfigurationTest.kt | 95 ++++++++++++++++ platform/swift/source/Capture.swift | 35 +++++- platform/swift/source/Logger.swift | 103 +++++++++++------- platform/swift/source/LoggerBridge.swift | 12 +- .../swift/source/LoggerBridgingFactory.swift | 2 +- .../LoggerBridgingFactoryProvider.swift | 2 +- .../swift/benchmark/ClockTimeProfiler.swift | 4 +- .../unit_integration/ConfigurationTests.swift | 30 +++++ .../unit_integration/LoggerSharedTests.swift | 10 +- .../swift/unit_integration/LoggerTests.swift | 26 ++--- .../SessionStrategyTests.swift | 8 +- .../URLSessionIntegrationTests.swift | 2 +- .../unit_integration/mocks/Logger+Tests.swift | 29 ++--- .../mocks/MockLoggerBridgingFactory.swift | 8 +- .../network/LoggerE2ETest.swift | 42 +++---- .../helper/BaseNetworkingTestCase.swift | 24 ++-- 20 files changed, 407 insertions(+), 144 deletions(-) create mode 100644 platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt index 7750c9fd..97ab60e1 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt @@ -22,11 +22,17 @@ import okhttp3.HttpUrl import java.util.concurrent.atomic.AtomicReference import kotlin.time.Duration +internal sealed class LoggerState { + data object NotConfigured : LoggerState() + class Configured(val logger: LoggerImpl) : LoggerState() + data object ConfigurationFailure : LoggerState() +} + /** * Top level namespace Capture SDK. */ object Capture { - private val default: AtomicReference = AtomicReference(null) + private val default: AtomicReference = AtomicReference(LoggerState.NotConfigured) /** * Returns a handle to the underlying logger instance, if Capture has been configured. @@ -34,7 +40,11 @@ object Capture { * @return ILogger a logger handle */ fun logger(): ILogger? { - return default.get() + return when (val state = default.get()) { + is LoggerState.NotConfigured -> null + is LoggerState.Configured -> state.logger + is LoggerState.ConfigurationFailure -> null + } } /** @@ -82,6 +92,29 @@ object Capture { fieldProviders: List = listOf(), dateProvider: DateProvider? = null, apiUrl: HttpUrl = defaultCaptureApiUrl, + ) { + configure( + apiKey, + sessionStrategy, + configuration, + fieldProviders, + dateProvider, + apiUrl, + CaptureJniLibrary, + ) + } + + @Synchronized + @JvmStatic + @JvmOverloads + internal fun configure( + apiKey: String, + sessionStrategy: SessionStrategy, + configuration: Configuration = Configuration(), + fieldProviders: List = listOf(), + dateProvider: DateProvider? = null, + apiUrl: HttpUrl = defaultCaptureApiUrl, + bridge: IBridge, ) { // Note that we need to use @Synchronized to prevent multiple loggers from being initialized, // while subsequent logger access relies on volatile reads. @@ -96,22 +129,28 @@ object Capture { return } - // If the logger has already been configured, do nothing. - if (default.get() != null) { - Log.w("capture", "Attempted to initialize Capture more than once") - return + default.getAndUpdate { + if (it is LoggerState.NotConfigured) { + try { + val logger = LoggerImpl( + apiKey = apiKey, + apiUrl = apiUrl, + fieldProviders = fieldProviders, + dateProvider = dateProvider ?: SystemDateProvider(), + configuration = configuration, + sessionStrategy = sessionStrategy, + bridge = bridge, + ) + LoggerState.Configured(logger) + } catch (e: Throwable) { + Log.w("capture", "Capture initialization failed") + LoggerState.ConfigurationFailure + } + } else { + Log.w("capture", "Attempted to initialize Capture more than once") + it + } } - - val logger = LoggerImpl( - apiKey = apiKey, - apiUrl = apiUrl, - fieldProviders = fieldProviders, - dateProvider = dateProvider ?: SystemDateProvider(), - configuration = configuration, - sessionStrategy = sessionStrategy, - ) - - default.set(logger) } /** @@ -333,7 +372,7 @@ object Capture { */ @JvmStatic fun log(httpRequestInfo: HttpRequestInfo) { - default.get()?.log(httpRequestInfo) + logger()?.log(httpRequestInfo) } /** @@ -344,7 +383,14 @@ object Capture { */ @JvmStatic fun log(httpResponseInfo: HttpResponseInfo) { - default.get()?.log(httpResponseInfo) + logger()?.log(httpResponseInfo) + } + + /** + * Used for testing purposes. + */ + internal fun resetShared() { + default.set(LoggerState.NotConfigured) } } } diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/CaptureJniLibrary.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/CaptureJniLibrary.kt index 6e51bf73..2f363e67 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/CaptureJniLibrary.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/CaptureJniLibrary.kt @@ -25,7 +25,7 @@ interface StackTraceProvider { } @Suppress("UndocumentedPublicClass") -internal object CaptureJniLibrary { +internal object CaptureJniLibrary : IBridge { /** * Loads the shared library. This is safe to call multiple times. @@ -49,7 +49,7 @@ internal object CaptureJniLibrary { * @param preferences the preferences storage to use for persistent storage of simple settings and configuration. * @param errorReporter the error reporter to use for reporting error to bitdrift services. */ - external fun createLogger( + external override fun createLogger( sdkDirectory: String, apiKey: String, sessionStrategy: SessionStrategyConfiguration, diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt index 6c34120b..05617069 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt @@ -36,6 +36,7 @@ import io.bitdrift.capture.events.performance.ResourceUtilizationTarget import io.bitdrift.capture.events.span.Span import io.bitdrift.capture.network.HttpRequestInfo import io.bitdrift.capture.network.HttpResponseInfo +import io.bitdrift.capture.network.ICaptureNetwork import io.bitdrift.capture.network.okhttp.OkHttpApiClient import io.bitdrift.capture.network.okhttp.OkHttpNetwork import io.bitdrift.capture.providers.DateProvider @@ -44,6 +45,7 @@ import io.bitdrift.capture.providers.FieldProvider import io.bitdrift.capture.providers.FieldValue import io.bitdrift.capture.providers.MetadataProvider import io.bitdrift.capture.providers.session.SessionStrategy +import io.bitdrift.capture.providers.session.SessionStrategyConfiguration import io.bitdrift.capture.providers.toFields import okhttp3.HttpUrl import java.io.File @@ -57,6 +59,22 @@ typealias LoggerId = Long internal typealias InternalFieldsList = List internal typealias InternalFieldsMap = Map +internal interface IBridge { + fun createLogger( + sdkDirectory: String, + apiKey: String, + sessionStrategy: SessionStrategyConfiguration, + metadataProvider: IMetadataProvider, + resourceUtilizationTarget: IResourceUtilizationTarget, + eventsListenerTarget: IEventsListenerTarget, + applicationId: String, + applicationVersion: String, + network: ICaptureNetwork, + preferences: IPreferences, + errorReporter: IErrorReporter, + ): Long +} + internal class LoggerImpl( apiKey: String, apiUrl: HttpUrl, @@ -76,6 +94,7 @@ internal class LoggerImpl( private val apiClient: OkHttpApiClient = OkHttpApiClient(apiUrl, apiKey), private var deviceCodeService: DeviceCodeService = DeviceCodeService(apiClient), private val activityManager: ActivityManager = context.getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager, + private val bridge: IBridge = CaptureJniLibrary, ) : ILogger { private val metadataProvider: MetadataProvider @@ -144,7 +163,7 @@ internal class LoggerImpl( processingQueue, ) - this.loggerId = CaptureJniLibrary.createLogger( + val loggerId = bridge.createLogger( sdkDirectory, apiKey, sessionStrategy.createSessionStrategyConfiguration { appExitSaveCurrentSessionId(it) }, @@ -164,6 +183,10 @@ internal class LoggerImpl( localErrorReporter, ) + check(loggerId != -1L) { "initialization of the rust logger failed" } + + this.loggerId = loggerId + runtime = JniRuntime(this.loggerId) diskUsageMonitor.runtime = runtime diff --git a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt index 1d55bc52..79ca01a4 100644 --- a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt +++ b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt @@ -30,7 +30,7 @@ class CaptureTest { // This Test needs to run first since the following tests need to initialize // the ContextHolder before they can run. @Test - fun a_configure_skips_logger_creation_when_context_not_initialized() { + fun aConfigureSkipsLoggerCreationWhenContextNotInitialized() { assertThat(Capture.logger()).isNull() Logger.configure( @@ -45,7 +45,7 @@ class CaptureTest { // Accessing fields prior to the configuration of the logger may lead to crash since it can // potentially call into a native method that's used to sanitize passed url path. @Test - fun b_does_not_access_fields_if_logger_not_configured() { + fun bDoesNotAccessFieldsIfLoggerNotConfigured() { assertThat(Capture.logger()).isNull() val requestInfo = HttpRequestInfo("GET", path = HttpUrlPath("/foo/12345")) @@ -65,7 +65,7 @@ class CaptureTest { } @Test - fun c_idempotent_configure() { + fun cIdempotentConfigure() { val initializer = ContextHolder() initializer.create(ApplicationProvider.getApplicationContext()) diff --git a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt new file mode 100644 index 00000000..3064c0d0 --- /dev/null +++ b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt @@ -0,0 +1,95 @@ +package io.bitdrift.capture + +import androidx.test.core.app.ApplicationProvider +import com.nhaarman.mockitokotlin2.anyOrNull +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.times +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.whenever +import io.bitdrift.capture.providers.session.SessionStrategy +import org.assertj.core.api.Assertions +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [21]) +class ConfigurationTest { + @Test + fun configurationFailure() { + val initializer = ContextHolder() + initializer.create(ApplicationProvider.getApplicationContext()) + + val bridge: IBridge = mock {} + whenever( + bridge.createLogger( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + ), + ).thenReturn(-1L) + + // We start without configured logger. + Assertions.assertThat(Capture.logger()).isNull() + + Capture.Logger.configure( + apiKey = "test1", + sessionStrategy = SessionStrategy.Fixed(), + dateProvider = null, + bridge = bridge, + ) + + // The configuration failed so the logger is still `null`. + Assertions.assertThat(Capture.logger()).isNull() + + // We confirm that we actually tried to configure the logger. + verify(bridge, times(1)).createLogger( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + ) + + // We perform another attempt to configure the logger to verify that + // consecutive configure calls are no-ops. + Capture.Logger.configure( + apiKey = "test1", + sessionStrategy = SessionStrategy.Fixed(), + dateProvider = null, + bridge = bridge, + ) + + Assertions.assertThat(Capture.logger()).isNull() + + // We verify that the second configure call was a no-op. + verify(bridge, times(1)).createLogger( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + ) + } +} diff --git a/platform/swift/source/Capture.swift b/platform/swift/source/Capture.swift index f2fe5b55..6ffaeea1 100644 --- a/platform/swift/source/Capture.swift +++ b/platform/swift/source/Capture.swift @@ -19,8 +19,9 @@ extension Logger { // MARK: - General - /// An instance of an underlying logger, if the Capture SDK has been configured. Returns `nil` only if - /// `configure(...)` method has not been called. + /// An instance of the underlying logger, if the Capture SDK is configured. Returns `nil` if the + /// `configure(...)` method has not been called or if Logger configuration failed due to an internal + /// error. public static var shared: Logging? { self.getShared(assert: false) } @@ -40,8 +41,7 @@ extension Logger { /// instructed otherwise during discussions with bitdrift. Defaults to /// bitdrift's hosted Compose API base URL. /// - /// - returns: A logger integrator that can be used to enable various SDK integration and get access - /// to non-optional `Logging` instance. + /// - returns: A logger integrator that can be used to enable various SDK integration. @discardableResult public static func configure( withAPIKey apiKey: String, @@ -51,7 +51,29 @@ extension Logger { dateProvider: DateProvider? = nil, // swiftlint:disable:next force_unwrapping use_static_string_url_init apiURL: URL = URL(string: "https://api.bitdrift.io")! - ) -> LoggerIntegrator + ) -> LoggerIntegrator? + { + return self.configure( + withAPIKey: apiKey, + sessionStrategy: sessionStrategy, + configuration: configuration, + fieldProviders: fieldProviders, + dateProvider: dateProvider, + apiURL: apiURL, + loggerBridgingFactoryProvider: LoggerBridgingFactory() + ) + } + + @discardableResult + static func configure( + withAPIKey apiKey: String, + sessionStrategy: SessionStrategy, + configuration: Configuration, + fieldProviders: [FieldProvider], + dateProvider: DateProvider?, + apiURL: URL, + loggerBridgingFactoryProvider: LoggerBridgingFactoryProvider + ) -> LoggerIntegrator? { return self.createOnce { return Logger( @@ -60,7 +82,8 @@ extension Logger { configuration: configuration, sessionStrategy: sessionStrategy, dateProvider: dateProvider, - fieldProviders: fieldProviders + fieldProviders: fieldProviders, + loggerBridgingFactoryProvider: loggerBridgingFactoryProvider ) } } diff --git a/platform/swift/source/Logger.swift b/platform/swift/source/Logger.swift index ab0328fb..90e24e41 100644 --- a/platform/swift/source/Logger.swift +++ b/platform/swift/source/Logger.swift @@ -11,6 +11,12 @@ import Foundation // swiftlint:disable file_length public final class Logger { + enum State { + case notConfigured + case configured(LoggerIntegrator) + case configurationFailure + } + private let underlyingLogger: CoreLogging private let timeProvider: TimeProvider @@ -24,7 +30,7 @@ public final class Logger { private let sessionURLBase: URL - private static let syncedShared = Atomic(nil) + private static let syncedShared = Atomic(.notConfigured) private let network: URLSessionNetworkClient? // Used for benchmarking purposes. @@ -46,7 +52,7 @@ public final class Logger { /// attributes to attach to emitted logs. /// - parameter loggerBridgingFactoryProvider: A class to use for Rust bridging. Used for testing /// purposes. - convenience init( + convenience init?( withAPIKey apiKey: String, apiURL: URL, configuration: Configuration, @@ -94,7 +100,7 @@ public final class Logger { /// - parameter timeProvider: The time source to use by the logger. /// - parameter loggerBridgingFactoryProvider: A class to use for Rust bridging. Used for testing /// purposes. - init( + init?( withAPIKey apiKey: String, bufferDirectory: URL?, apiURL: URL, @@ -111,10 +117,6 @@ public final class Logger { { self.timeProvider = timeProvider let start = timeProvider.uptime() - defer { - let duration = timeProvider.timeIntervalSince(start) - self.underlyingLogger.logSDKConfigured(fields: [:], duration: duration) - } // Order of providers matters in here, the latter in the list the higher their priority in // case of key conflicts. @@ -157,27 +159,36 @@ public final class Logger { ) self.eventsListenerTarget = EventsListenerTarget() - self.underlyingLogger = CoreLogger( - logger: loggerBridgingFactoryProvider.makeLogger( - apiKey: apiKey, - bufferDirectoryPath: directoryURL?.path, - sessionStrategy: sessionStrategy, - metadataProvider: metadataProvider, - // TODO(Augustyniak): Pass `resourceUtilizationTarget` and `eventsListenerTarget` as part of - // the `self.underlyingLogger.start()` method call instead. - // Pass the event listener target here and finish setting up - // before the logger is actually started. - resourceUtilizationTarget: self.resourceUtilizationTarget, - // Pass the event listener target here and finish setting up - // before the logger is actually started. - eventsListenerTarget: self.eventsListenerTarget, - appID: clientAttributes.appID, - releaseVersion: clientAttributes.appVersion, - network: network, - errorReporting: self.remoteErrorReporter - ) + let logger = loggerBridgingFactoryProvider.makeLogger( + apiKey: apiKey, + bufferDirectoryPath: directoryURL?.path, + sessionStrategy: sessionStrategy, + metadataProvider: metadataProvider, + // TODO(Augustyniak): Pass `resourceUtilizationTarget` and `eventsListenerTarget` as part of + // the `self.underlyingLogger.start()` method call instead. + // Pass the event listener target here and finish setting up + // before the logger is actually started. + resourceUtilizationTarget: self.resourceUtilizationTarget, + // Pass the event listener target here and finish setting up + // before the logger is actually started. + eventsListenerTarget: self.eventsListenerTarget, + appID: clientAttributes.appID, + releaseVersion: clientAttributes.appVersion, + network: network, + errorReporting: self.remoteErrorReporter ) + guard let logger else { + return nil + } + + self.underlyingLogger = CoreLogger(logger: logger) + + defer { + let duration = timeProvider.timeIntervalSince(start) + self.underlyingLogger.logSDKConfigured(fields: [:], duration: duration) + } + self.eventsListenerTarget.setUp( logger: self.underlyingLogger, appStateAttributes: appStateAttributes, @@ -247,19 +258,25 @@ public final class Logger { // MARK: - Static - static func createOnce(_ createLogger: () -> Logger) -> LoggerIntegrator { - let logger = self.syncedShared.update { logger in - guard logger == nil else { + static func createOnce(_ createLogger: () -> Logger?) -> LoggerIntegrator? { + let state = self.syncedShared.update { state in + guard case .notConfigured = state else { return } - logger = LoggerIntegrator(logger: createLogger()) + if let createdLogger = createLogger() { + state = .configured(LoggerIntegrator(logger: createdLogger)) + } else { + state = .configurationFailure + } } - // Safety: - // The logger instance is guaranteed to be created after the first call to createOnce method. - // swiftlint:disable force_unwrapping - return logger! + return switch state { + case .configured(let logger): + logger + case .notConfigured, .configurationFailure: + nil + } } /// Retrieves a shared instance of logger if one has been configured. @@ -268,7 +285,8 @@ public final class Logger { /// /// - returns: The shared instance of logger. static func getShared(assert: Bool = true) -> Logging? { - guard let integrator = Self.syncedShared.load() else { + return switch Self.syncedShared.load() { + case .notConfigured: { if assert { assertionFailure( """ @@ -279,16 +297,25 @@ public final class Logger { } return nil + }() + case .configured(let integrator): + integrator.logger + case .configurationFailure: + nil } - - return integrator.logger } /// Internal for testing purposes only. /// /// - parameter logger: The logger to use. static func resetShared(logger: Logging? = nil) { - Self.syncedShared.update { $0 = logger.flatMap(LoggerIntegrator.init(logger:)) } + Self.syncedShared.update { state in + if let logger { + state = .configured(LoggerIntegrator(logger: logger)) + } else { + state = .notConfigured + } + } } // Returns the location to use for storing files related to the Capture SDK, including the disk persisted diff --git a/platform/swift/source/LoggerBridge.swift b/platform/swift/source/LoggerBridge.swift index 41d68166..820b7a7e 100644 --- a/platform/swift/source/LoggerBridge.swift +++ b/platform/swift/source/LoggerBridge.swift @@ -16,7 +16,7 @@ final class LoggerBridge: LoggerBridging { let loggerID: LoggerID private var blockingShutdown = false - init( + init?( apiKey: String, bufferDirectoryPath: String?, sessionStrategy: SessionStrategy, @@ -28,7 +28,7 @@ final class LoggerBridge: LoggerBridging { network: Network?, errorReporting: RemoteErrorReporting ) { - self.loggerID = capture_create_logger( + let loggerID = capture_create_logger( bufferDirectoryPath, apiKey, sessionStrategy.makeSessionStrategyProvider(), @@ -40,6 +40,12 @@ final class LoggerBridge: LoggerBridging { network, errorReporting ) + + if loggerID == -1 { + return nil + } + + self.loggerID = loggerID } deinit { @@ -61,7 +67,7 @@ final class LoggerBridge: LoggerBridging { releaseVersion: String, network: Network?, errorReporting: RemoteErrorReporting - ) -> LoggerBridging { + ) -> LoggerBridging? { return LoggerBridge( apiKey: apiKey, bufferDirectoryPath: bufferDirectoryPath, diff --git a/platform/swift/source/LoggerBridgingFactory.swift b/platform/swift/source/LoggerBridgingFactory.swift index 5c8f8f5a..ed55498e 100644 --- a/platform/swift/source/LoggerBridgingFactory.swift +++ b/platform/swift/source/LoggerBridgingFactory.swift @@ -19,7 +19,7 @@ final class LoggerBridgingFactory: LoggerBridgingFactoryProvider { releaseVersion: String, network: Network?, errorReporting: RemoteErrorReporting - ) -> LoggerBridging { + ) -> LoggerBridging? { return LoggerBridge( apiKey: apiKey, bufferDirectoryPath: bufferDirectoryPath, diff --git a/platform/swift/source/LoggerBridgingFactoryProvider.swift b/platform/swift/source/LoggerBridgingFactoryProvider.swift index cd57d0d8..15a1d33b 100644 --- a/platform/swift/source/LoggerBridgingFactoryProvider.swift +++ b/platform/swift/source/LoggerBridgingFactoryProvider.swift @@ -34,5 +34,5 @@ protocol LoggerBridgingFactoryProvider { releaseVersion: String, network: Network?, errorReporting: RemoteErrorReporting - ) -> LoggerBridging + ) -> LoggerBridging? } diff --git a/test/platform/swift/benchmark/ClockTimeProfiler.swift b/test/platform/swift/benchmark/ClockTimeProfiler.swift index b639f913..7eac0cf6 100644 --- a/test/platform/swift/benchmark/ClockTimeProfiler.swift +++ b/test/platform/swift/benchmark/ClockTimeProfiler.swift @@ -10,6 +10,7 @@ import Benchmark import CapturePassable import CaptureTestBridge import Foundation +import XCTest // swiftlint:disable:next force_unwrapping private let kAPIURL = URL(string: "https://api-tests.bitdrift.io")! @@ -276,7 +277,7 @@ private let kPostConfigLogBenchmark = BenchmarkSuite(name: "Logging - Post-Confi private extension Logger { static func make(directoryURL: URL? = nil) throws -> Logger { let directoryURLFallback = try makeTmpDirectory() - return Logger( + return try XCTUnwrap(Logger( withAPIKey: "foo", bufferDirectory: directoryURL ?? directoryURLFallback, apiURL: kAPIURL, @@ -289,6 +290,7 @@ private extension Logger { storageProvider: Storage.shared, timeProvider: SystemTimeProvider() ) + ) } } diff --git a/test/platform/swift/unit_integration/ConfigurationTests.swift b/test/platform/swift/unit_integration/ConfigurationTests.swift index 9aeca81a..3b883035 100644 --- a/test/platform/swift/unit_integration/ConfigurationTests.swift +++ b/test/platform/swift/unit_integration/ConfigurationTests.swift @@ -42,4 +42,34 @@ final class ConfigurationTests: XCTestCase { XCTAssertNotNil(Logger.getShared()) } + + func testConfigurationFailure() { + let factory = MockLoggerBridgingFactory(logger: nil) + + Logger.configure( + withAPIKey: "test", + sessionStrategy: .fixed(), + configuration: .init(), + fieldProviders: [], + dateProvider: nil, + apiURL: URL(staticString: "https://api.bitdrift.io"), + loggerBridgingFactoryProvider: factory + ) + + XCTAssertEqual(1, factory.makeLoggerCallsCount) + XCTAssertNil(Logger.shared) + + Logger.configure( + withAPIKey: "test", + sessionStrategy: .fixed(), + configuration: .init(), + fieldProviders: [], + dateProvider: nil, + apiURL: URL(staticString: "https://api.bitdrift.io"), + loggerBridgingFactoryProvider: factory + ) + + XCTAssertEqual(1, factory.makeLoggerCallsCount) + XCTAssertNil(Logger.shared) + } } diff --git a/test/platform/swift/unit_integration/LoggerSharedTests.swift b/test/platform/swift/unit_integration/LoggerSharedTests.swift index b9f44119..2d5e03b2 100644 --- a/test/platform/swift/unit_integration/LoggerSharedTests.swift +++ b/test/platform/swift/unit_integration/LoggerSharedTests.swift @@ -19,15 +19,17 @@ final class LoggerSharedTests: XCTestCase { Logger.resetShared() } - func testIntegrationsAreEnabledOnlyOnce() { + func testIntegrationsAreEnabledOnlyOnce() throws { var integrationStartsCount = 0 let integration = Integration { _, _ in integrationStartsCount += 1 } - let integrator = Logger.configure( - withAPIKey: "foo", - sessionStrategy: .fixed() + let integrator = try XCTUnwrap( + Logger.configure( + withAPIKey: "foo", + sessionStrategy: .fixed() + ) ) integrator.enableIntegrations([integration]) diff --git a/test/platform/swift/unit_integration/LoggerTests.swift b/test/platform/swift/unit_integration/LoggerTests.swift index 4f94272f..a41c70a1 100644 --- a/test/platform/swift/unit_integration/LoggerTests.swift +++ b/test/platform/swift/unit_integration/LoggerTests.swift @@ -11,8 +11,8 @@ import Foundation import XCTest final class LoggerTests: XCTestCase { - func testPropertiesReturnsCorrectValues() { - let logger = Logger.testLogger( + func testPropertiesReturnsCorrectValues() throws { + let logger = try Logger.testLogger( withAPIKey: "test_api_key", bufferDirectory: Logger.tempBufferDirectory(), configuration: .init(sessionReplayConfiguration: nil) @@ -23,8 +23,8 @@ final class LoggerTests: XCTestCase { } // Basic test to ensure we can create a logger and call log. - func testLogger() { - let logger = Logger.testLogger( + func testLogger() throws { + let logger = try Logger.testLogger( withAPIKey: "test_api_key", bufferDirectory: Logger.tempBufferDirectory(), configuration: .init(sessionReplayConfiguration: nil) @@ -37,7 +37,7 @@ final class LoggerTests: XCTestCase { // Verifies that we don't end up recursing forever (resulting in a stack overflow) when a provider ends // up calling back into the logger. - func testPreventsLoggingReEntryFromWithinRegisteredProviders() { + func testPreventsLoggingReEntryFromWithinRegisteredProviders() throws { var logger: Logger? let expectation = self.expectation(description: "'SDKConfigured' log is emitted") @@ -49,7 +49,7 @@ final class LoggerTests: XCTestCase { return [:] } - logger = Logger.testLogger( + logger = try Logger.testLogger( withAPIKey: "test_api_key", bufferDirectory: Logger.tempBufferDirectory(), fieldProviders: [fieldProvider], @@ -59,7 +59,7 @@ final class LoggerTests: XCTestCase { XCTAssertEqual(.completed, XCTWaiter().wait(for: [expectation], timeout: 1)) } - func testRunsProvidersOffCallerThread() { + func testRunsProvidersOffCallerThread() throws { let dateProvider = MockDateProvider() // Verify that test is executed on the main queue. @@ -93,7 +93,7 @@ final class LoggerTests: XCTestCase { return Date() } - let logger = Logger.testLogger( + let logger = try Logger.testLogger( withAPIKey: "test_api_key", bufferDirectory: Logger.tempBufferDirectory(), sessionStrategy: SessionStrategy.fixed(), @@ -112,7 +112,7 @@ final class LoggerTests: XCTestCase { XCTAssertEqual(.completed, XCTWaiter().wait(for: expectations, timeout: 1)) } - func testFailingEncodableField() { + func testFailingEncodableField() throws { struct FailingEncodable: Encodable { struct Error: Swift.Error {} @@ -123,7 +123,7 @@ final class LoggerTests: XCTestCase { let bridge = MockLoggerBridging() - let logger = Logger.testLogger( + let logger = try Logger.testLogger( bufferDirectory: Logger.tempBufferDirectory(), configuration: .init(), loggerBridgingFactoryProvider: MockLoggerBridgingFactory(logger: bridge) @@ -144,7 +144,7 @@ final class LoggerTests: XCTestCase { let bridge = MockLoggerBridging() - let logger = Logger.testLogger( + let logger = try Logger.testLogger( withAPIKey: "test_api_key", bufferDirectory: Logger.tempBufferDirectory(), configuration: .init(sessionReplayConfiguration: .init(captureIntervalSeconds: 5)), @@ -192,7 +192,7 @@ final class LoggerTests: XCTestCase { let bridge = MockLoggerBridging() - let logger = Logger.testLogger( + let logger = try Logger.testLogger( withAPIKey: "test_api_key", bufferDirectory: Logger.tempBufferDirectory(), configuration: .init(sessionReplayConfiguration: .init(captureIntervalSeconds: 5)), @@ -242,7 +242,7 @@ final class LoggerTests: XCTestCase { let bridge = MockLoggerBridging() - let logger = Logger.testLogger( + let logger = try Logger.testLogger( withAPIKey: "test_api_key", bufferDirectory: Logger.tempBufferDirectory(), configuration: .init(sessionReplayConfiguration: .init(captureIntervalSeconds: 5)), diff --git a/test/platform/swift/unit_integration/SessionStrategyTests.swift b/test/platform/swift/unit_integration/SessionStrategyTests.swift index 0cc43ac5..d0a1a941 100644 --- a/test/platform/swift/unit_integration/SessionStrategyTests.swift +++ b/test/platform/swift/unit_integration/SessionStrategyTests.swift @@ -15,10 +15,10 @@ final class SessionStrategyTests: XCTestCase { Storage.shared.clear() } - func testFixedSessionStrategy() { + func testFixedSessionStrategy() throws { var generatedSessionIDs = [String]() - let logger = Logger.testLogger( + let logger = try Logger.testLogger( withAPIKey: "test_api_key", bufferDirectory: Logger.tempBufferDirectory(), sessionStrategy: SessionStrategy.fixed { @@ -40,11 +40,11 @@ final class SessionStrategyTests: XCTestCase { XCTAssertEqual(logger.sessionID, generatedSessionIDs[1]) } - func testActivityBasedSessionStrategy() { + func testActivityBasedSessionStrategy() throws { let expectation = self.expectation(description: "onSessionIDChange called") var observedSessionID: String? - let logger = Logger.testLogger( + let logger = try Logger.testLogger( withAPIKey: "test_api_key", bufferDirectory: Logger.tempBufferDirectory(), sessionStrategy: SessionStrategy.activityBased { sessionID in diff --git a/test/platform/swift/unit_integration/integrations/URLSessionIntegrationTests.swift b/test/platform/swift/unit_integration/integrations/URLSessionIntegrationTests.swift index 9d91162e..627222d6 100644 --- a/test/platform/swift/unit_integration/integrations/URLSessionIntegrationTests.swift +++ b/test/platform/swift/unit_integration/integrations/URLSessionIntegrationTests.swift @@ -56,7 +56,7 @@ final class URLSessionIntegrationTests: XCTestCase { self.logger = MockLogging() Logger.resetShared(logger: self.logger) Logger - .configure(withAPIKey: "123", sessionStrategy: .fixed()) + .configure(withAPIKey: "123", sessionStrategy: .fixed())? .enableIntegrations([ .urlSession(), ]) diff --git a/test/platform/swift/unit_integration/mocks/Logger+Tests.swift b/test/platform/swift/unit_integration/mocks/Logger+Tests.swift index 970e6c1b..fbaa9647 100644 --- a/test/platform/swift/unit_integration/mocks/Logger+Tests.swift +++ b/test/platform/swift/unit_integration/mocks/Logger+Tests.swift @@ -7,6 +7,7 @@ @testable import Capture import Foundation +import XCTest extension Logger { static func testLogger( @@ -17,20 +18,22 @@ extension Logger { fieldProviders: [FieldProvider] = [], configuration: Configuration, loggerBridgingFactoryProvider: LoggerBridgingFactoryProvider = LoggerBridgingFactory() - ) -> Logger + ) throws -> Logger { - return Logger( - withAPIKey: apiKey, - bufferDirectory: bufferDirectory, - apiURL: URL(staticString: "https://api-tests.bitdrift.io"), - remoteErrorReporter: nil, - configuration: configuration, - sessionStrategy: sessionStrategy, - dateProvider: dateProvider, - fieldProviders: fieldProviders, - storageProvider: MockStorageProvider(), - timeProvider: SystemTimeProvider(), - loggerBridgingFactoryProvider: loggerBridgingFactoryProvider + return try XCTUnwrap( + Logger( + withAPIKey: apiKey, + bufferDirectory: bufferDirectory, + apiURL: URL(staticString: "https://api-tests.bitdrift.io"), + remoteErrorReporter: nil, + configuration: configuration, + sessionStrategy: sessionStrategy, + dateProvider: dateProvider, + fieldProviders: fieldProviders, + storageProvider: MockStorageProvider(), + timeProvider: SystemTimeProvider(), + loggerBridgingFactoryProvider: loggerBridgingFactoryProvider + ) ) } diff --git a/test/platform/swift/unit_integration/mocks/MockLoggerBridgingFactory.swift b/test/platform/swift/unit_integration/mocks/MockLoggerBridgingFactory.swift index c5f8cf74..9ab04f17 100644 --- a/test/platform/swift/unit_integration/mocks/MockLoggerBridgingFactory.swift +++ b/test/platform/swift/unit_integration/mocks/MockLoggerBridgingFactory.swift @@ -10,9 +10,10 @@ import CaptureLoggerBridge import Foundation final class MockLoggerBridgingFactory: LoggerBridgingFactoryProvider { - private let logger: LoggerBridging + private let logger: LoggerBridging? + private(set) var makeLoggerCallsCount = 0 - init(logger: LoggerBridging) { + init(logger: LoggerBridging?) { self.logger = logger } @@ -27,7 +28,8 @@ final class MockLoggerBridgingFactory: LoggerBridgingFactoryProvider { releaseVersion _: String, network _: Network?, errorReporting _: RemoteErrorReporting - ) -> LoggerBridging { + ) -> LoggerBridging? { + self.makeLoggerCallsCount += 1 return self.logger } } diff --git a/test/platform/swift/unit_integration/network/LoggerE2ETest.swift b/test/platform/swift/unit_integration/network/LoggerE2ETest.swift index 60bbcb32..fa2c5d7d 100644 --- a/test/platform/swift/unit_integration/network/LoggerE2ETest.swift +++ b/test/platform/swift/unit_integration/network/LoggerE2ETest.swift @@ -64,26 +64,28 @@ final class CaptureE2ENetworkTests: BaseNetworkingTestCase { self.storage = MockStorageProvider() let apiURL = self.setUpTestServer() - let logger = Logger( - withAPIKey: "test!", - bufferDirectory: self.setUpSDKDirectory(), - apiURL: apiURL, - remoteErrorReporter: MockRemoteErrorReporter(), - configuration: configuration, - sessionStrategy: SessionStrategy.fixed(sessionIDGenerator: { "mock-group-id" }), - dateProvider: MockDateProvider(), - fieldProviders: [ - MockFieldProvider( - getFieldsClosure: { - [ - "field_provider": "mock_field_provider", - "failing_to_convert_and_should_be_skipped_field": MockEncodable(), - ] - } - ), - ], - storageProvider: self.storage, - timeProvider: SystemTimeProvider() + let logger = try XCTUnwrap( + Logger( + withAPIKey: "test!", + bufferDirectory: self.setUpSDKDirectory(), + apiURL: apiURL, + remoteErrorReporter: MockRemoteErrorReporter(), + configuration: configuration, + sessionStrategy: SessionStrategy.fixed(sessionIDGenerator: { "mock-group-id" }), + dateProvider: MockDateProvider(), + fieldProviders: [ + MockFieldProvider( + getFieldsClosure: { + [ + "field_provider": "mock_field_provider", + "failing_to_convert_and_should_be_skipped_field": MockEncodable(), + ] + } + ), + ], + storageProvider: self.storage, + timeProvider: SystemTimeProvider() + ) ) self.logger = logger diff --git a/test/platform/swift/unit_integration/network/helper/BaseNetworkingTestCase.swift b/test/platform/swift/unit_integration/network/helper/BaseNetworkingTestCase.swift index 687b3a34..05348fcc 100644 --- a/test/platform/swift/unit_integration/network/helper/BaseNetworkingTestCase.swift +++ b/test/platform/swift/unit_integration/network/helper/BaseNetworkingTestCase.swift @@ -68,17 +68,19 @@ open class BaseNetworkingTestCase: XCTestCase { self.network = network - let loggerBridge = LoggerBridge( - apiKey: "test!", - bufferDirectoryPath: sdkDirectory.path, - sessionStrategy: .fixed(), - metadataProvider: MockMetadataProvider(), - resourceUtilizationTarget: MockResourceUtilizationTarget(), - eventsListenerTarget: MockEventsListenerTarget(), - appID: "io.bitdrift.capture.test", - releaseVersion: "", - network: network, - errorReporting: MockRemoteErrorReporter() + let loggerBridge = try XCTUnwrap( + LoggerBridge( + apiKey: "test!", + bufferDirectoryPath: sdkDirectory.path, + sessionStrategy: .fixed(), + metadataProvider: MockMetadataProvider(), + resourceUtilizationTarget: MockResourceUtilizationTarget(), + eventsListenerTarget: MockEventsListenerTarget(), + appID: "io.bitdrift.capture.test", + releaseVersion: "", + network: network, + errorReporting: MockRemoteErrorReporter() + ) ) loggerBridge.start() From a62783f618058fe2a9a45543e0e110afa55ac846 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Fri, 27 Sep 2024 17:55:48 -0400 Subject: [PATCH 02/17] move interface to a separate file --- .../kotlin/io/bitdrift/capture/IBridge.kt | 21 +++++++++++++++++++ .../kotlin/io/bitdrift/capture/LoggerImpl.kt | 16 -------------- 2 files changed, 21 insertions(+), 16 deletions(-) create mode 100644 platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/IBridge.kt diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/IBridge.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/IBridge.kt new file mode 100644 index 00000000..664aa832 --- /dev/null +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/IBridge.kt @@ -0,0 +1,21 @@ +package io.bitdrift.capture + +import io.bitdrift.capture.error.IErrorReporter +import io.bitdrift.capture.network.ICaptureNetwork +import io.bitdrift.capture.providers.session.SessionStrategyConfiguration + +internal interface IBridge { + fun createLogger( + sdkDirectory: String, + apiKey: String, + sessionStrategy: SessionStrategyConfiguration, + metadataProvider: IMetadataProvider, + resourceUtilizationTarget: IResourceUtilizationTarget, + eventsListenerTarget: IEventsListenerTarget, + applicationId: String, + applicationVersion: String, + network: ICaptureNetwork, + preferences: IPreferences, + errorReporter: IErrorReporter, + ): Long +} diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt index 05617069..ab7030c7 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt @@ -59,22 +59,6 @@ typealias LoggerId = Long internal typealias InternalFieldsList = List internal typealias InternalFieldsMap = Map -internal interface IBridge { - fun createLogger( - sdkDirectory: String, - apiKey: String, - sessionStrategy: SessionStrategyConfiguration, - metadataProvider: IMetadataProvider, - resourceUtilizationTarget: IResourceUtilizationTarget, - eventsListenerTarget: IEventsListenerTarget, - applicationId: String, - applicationVersion: String, - network: ICaptureNetwork, - preferences: IPreferences, - errorReporter: IErrorReporter, - ): Long -} - internal class LoggerImpl( apiKey: String, apiUrl: HttpUrl, From 92499640415d6f4b0a3656f964e61a1e46ae2ce4 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Mon, 30 Sep 2024 15:51:52 -0400 Subject: [PATCH 03/17] format fix --- .../capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt index ab7030c7..4a85687d 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt @@ -36,7 +36,6 @@ import io.bitdrift.capture.events.performance.ResourceUtilizationTarget import io.bitdrift.capture.events.span.Span import io.bitdrift.capture.network.HttpRequestInfo import io.bitdrift.capture.network.HttpResponseInfo -import io.bitdrift.capture.network.ICaptureNetwork import io.bitdrift.capture.network.okhttp.OkHttpApiClient import io.bitdrift.capture.network.okhttp.OkHttpNetwork import io.bitdrift.capture.providers.DateProvider @@ -45,7 +44,6 @@ import io.bitdrift.capture.providers.FieldProvider import io.bitdrift.capture.providers.FieldValue import io.bitdrift.capture.providers.MetadataProvider import io.bitdrift.capture.providers.session.SessionStrategy -import io.bitdrift.capture.providers.session.SessionStrategyConfiguration import io.bitdrift.capture.providers.toFields import okhttp3.HttpUrl import java.io.File From b4fd016cc4795cfc48545058b2967cf2259c86be Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Mon, 30 Sep 2024 15:52:51 -0400 Subject: [PATCH 04/17] fix --- platform/swift/source/Logger.swift | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/platform/swift/source/Logger.swift b/platform/swift/source/Logger.swift index 90e24e41..a060c820 100644 --- a/platform/swift/source/Logger.swift +++ b/platform/swift/source/Logger.swift @@ -159,7 +159,7 @@ public final class Logger { ) self.eventsListenerTarget = EventsListenerTarget() - let logger = loggerBridgingFactoryProvider.makeLogger( + guard let logger = loggerBridgingFactoryProvider.makeLogger( apiKey: apiKey, bufferDirectoryPath: directoryURL?.path, sessionStrategy: sessionStrategy, @@ -176,9 +176,7 @@ public final class Logger { releaseVersion: clientAttributes.appVersion, network: network, errorReporting: self.remoteErrorReporter - ) - - guard let logger else { + ) else { return nil } From cfb7b4c5724803378cbbe65fa782a3a9a32fe612 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Mon, 30 Sep 2024 15:53:23 -0400 Subject: [PATCH 05/17] add ? when working with optional --- examples/swift/hello_world/LoggerCustomer.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/swift/hello_world/LoggerCustomer.swift b/examples/swift/hello_world/LoggerCustomer.swift index 22e48e41..16ed099d 100644 --- a/examples/swift/hello_world/LoggerCustomer.swift +++ b/examples/swift/hello_world/LoggerCustomer.swift @@ -90,7 +90,7 @@ final class LoggerCustomer: NSObject, URLSessionDelegate { configuration: .init(), fieldProviders: [CustomFieldProvider()], apiURL: kBitdriftURL - ) + )? .enableIntegrations([.urlSession()], disableSwizzling: true) Logger.addField(withKey: "field_container_field_key", value: "field_container_value") From cc598811bce49a64dd4ca781ee9067816a726594 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Mon, 30 Sep 2024 16:02:44 -0400 Subject: [PATCH 06/17] add missing license headers --- .../capture/src/main/kotlin/io/bitdrift/capture/IBridge.kt | 7 +++++++ .../test/kotlin/io/bitdrift/capture/ConfigurationTest.kt | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/IBridge.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/IBridge.kt index 664aa832..89a04c63 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/IBridge.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/IBridge.kt @@ -1,3 +1,10 @@ +// capture-sdk - bitdrift's client SDK +// Copyright Bitdrift, Inc. All rights reserved. +// +// Use of this source code is governed by a source available license that can be found in the +// LICENSE file or at: +// https://polyformproject.org/wp-content/uploads/2020/06/PolyForm-Shield-1.0.0.txt + package io.bitdrift.capture import io.bitdrift.capture.error.IErrorReporter diff --git a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt index 3064c0d0..18ee04a6 100644 --- a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt +++ b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt @@ -1,3 +1,10 @@ +// capture-sdk - bitdrift's client SDK +// Copyright Bitdrift, Inc. All rights reserved. +// +// Use of this source code is governed by a source available license that can be found in the +// LICENSE file or at: +// https://polyformproject.org/wp-content/uploads/2020/06/PolyForm-Shield-1.0.0.txt + package io.bitdrift.capture import androidx.test.core.app.ApplicationProvider From 33ed31d06c37136ea206b8150d35064f353fe1f8 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Tue, 1 Oct 2024 10:19:10 -0400 Subject: [PATCH 07/17] suppress warning --- .../jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt index 97ab60e1..19b39af1 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt @@ -107,6 +107,7 @@ object Capture { @Synchronized @JvmStatic @JvmOverloads + @Suppress("SwallowedException") internal fun configure( apiKey: String, sessionStrategy: SessionStrategy, From bda538113df7abf141704cfc82da32d7f8d95146 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Tue, 1 Oct 2024 17:34:56 -0400 Subject: [PATCH 08/17] use getAndSet instead of getAndUpdate --- .../src/main/kotlin/io/bitdrift/capture/Capture.kt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt index 19b39af1..ea18f915 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt @@ -25,6 +25,7 @@ import kotlin.time.Duration internal sealed class LoggerState { data object NotConfigured : LoggerState() class Configured(val logger: LoggerImpl) : LoggerState() + data object ConfigurationStarted : LoggerState() data object ConfigurationFailure : LoggerState() } @@ -43,6 +44,7 @@ object Capture { return when (val state = default.get()) { is LoggerState.NotConfigured -> null is LoggerState.Configured -> state.logger + is LoggerState.ConfigurationStarted -> null is LoggerState.ConfigurationFailure -> null } } @@ -130,8 +132,9 @@ object Capture { return } - default.getAndUpdate { - if (it is LoggerState.NotConfigured) { + // Ideally we would use `getAndUpdate` in here but it's available for API 24 and up only. + when (default.getAndSet(LoggerState.ConfigurationStarted)) { + is LoggerState.NotConfigured -> { try { val logger = LoggerImpl( apiKey = apiKey, @@ -142,14 +145,13 @@ object Capture { sessionStrategy = sessionStrategy, bridge = bridge, ) - LoggerState.Configured(logger) + default.set(LoggerState.Configured(logger)) } catch (e: Throwable) { Log.w("capture", "Capture initialization failed") - LoggerState.ConfigurationFailure + default.set(LoggerState.ConfigurationFailure) } - } else { + } else -> { Log.w("capture", "Attempted to initialize Capture more than once") - it } } } From 5407f345f21c0f4af928c4d7ce1d9d60d8ef7b91 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Tue, 1 Oct 2024 17:48:27 -0400 Subject: [PATCH 09/17] remove resetShared --- .../capture/src/main/kotlin/io/bitdrift/capture/Capture.kt | 7 ------- 1 file changed, 7 deletions(-) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt index ea18f915..26bd557d 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt @@ -388,12 +388,5 @@ object Capture { fun log(httpResponseInfo: HttpResponseInfo) { logger()?.log(httpResponseInfo) } - - /** - * Used for testing purposes. - */ - internal fun resetShared() { - default.set(LoggerState.NotConfigured) - } } } From b46bcf7a08f0b048f7af3d2b9fcbe1b7fc71c924 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Tue, 1 Oct 2024 17:48:43 -0400 Subject: [PATCH 10/17] add docs --- .../main/kotlin/io/bitdrift/capture/Capture.kt | 15 +++++++++++++++ platform/swift/source/Logger.swift | 5 +++++ 2 files changed, 20 insertions(+) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt index 26bd557d..fe7366ca 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt @@ -23,9 +23,24 @@ import java.util.concurrent.atomic.AtomicReference import kotlin.time.Duration internal sealed class LoggerState { + /** + * The logger has not yet been configured. + */ data object NotConfigured : LoggerState() + + /** + * The logger has been successfully configured and is ready for use. Subsequent attempts to configure the logger will be ignored. + */ class Configured(val logger: LoggerImpl) : LoggerState() + + /** + * The configuration has started but is not yet complete. Subsequent attempts to configure the logger will be ignored. + */ data object ConfigurationStarted : LoggerState() + + /** + * The configuration was attempted but failed. Subsequent attempts to configure the logger will be ignored. + */ data object ConfigurationFailure : LoggerState() } diff --git a/platform/swift/source/Logger.swift b/platform/swift/source/Logger.swift index a060c820..3e0c5f12 100644 --- a/platform/swift/source/Logger.swift +++ b/platform/swift/source/Logger.swift @@ -12,8 +12,13 @@ import Foundation // swiftlint:disable file_length public final class Logger { enum State { + // The logger has not yet been configured. case notConfigured + // The logger has been successfully configured and is ready for use. + // Subsequent attempts to configure the logger will be ignored. case configured(LoggerIntegrator) + // The configuration was attempted but failed. + // Subsequent attempts to configure the logger will be ignored. case configurationFailure } From 03c38110de969b2635a955a5cbc44c1ebc6468b1 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Tue, 1 Oct 2024 18:03:10 -0400 Subject: [PATCH 11/17] fix flaky test --- .../capture/src/main/kotlin/io/bitdrift/capture/Capture.kt | 6 ++++++ .../src/test/kotlin/io/bitdrift/capture/CaptureTest.kt | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt index fe7366ca..a35170dc 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt @@ -403,5 +403,11 @@ object Capture { fun log(httpResponseInfo: HttpResponseInfo) { logger()?.log(httpResponseInfo) } + /** + * Used for testing purposes. + */ + internal fun resetShared() { + default.set(LoggerState.NotConfigured) + } } } diff --git a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt index 79ca01a4..25ede99e 100644 --- a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt +++ b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt @@ -15,6 +15,7 @@ import io.bitdrift.capture.network.HttpResponseInfo import io.bitdrift.capture.network.HttpUrlPath import io.bitdrift.capture.providers.session.SessionStrategy import org.assertj.core.api.Assertions.assertThat +import org.junit.Before import org.junit.FixMethodOrder import org.junit.Test import org.junit.runner.RunWith @@ -27,6 +28,11 @@ import org.robolectric.annotation.Config @FixMethodOrder(MethodSorters.NAME_ASCENDING) class CaptureTest { + @Before + fun setUp() { + Capture.Logger.resetShared() + } + // This Test needs to run first since the following tests need to initialize // the ContextHolder before they can run. @Test From f817a9f614d331c9e49faa2689c0ac193a4686ed Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Tue, 1 Oct 2024 18:03:21 -0400 Subject: [PATCH 12/17] add empty line --- .../jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt index a35170dc..27a7fefe 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt @@ -403,6 +403,7 @@ object Capture { fun log(httpResponseInfo: HttpResponseInfo) { logger()?.log(httpResponseInfo) } + /** * Used for testing purposes. */ From 675d77ed1265b42864d847bb27ce9e5ef133a0d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Augustyniak?= Date: Tue, 1 Oct 2024 18:04:31 -0400 Subject: [PATCH 13/17] Update platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Miguel Juárez López Signed-off-by: Rafał Augustyniak --- .../jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt index 27a7fefe..fdf888df 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt @@ -162,7 +162,7 @@ object Capture { ) default.set(LoggerState.Configured(logger)) } catch (e: Throwable) { - Log.w("capture", "Capture initialization failed") + Log.w("capture", "Capture initialization failed", e) default.set(LoggerState.ConfigurationFailure) } } else -> { From 9fd7a77ab8b6125dd459b7a4f688c6f34462d902 Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Tue, 1 Oct 2024 18:05:09 -0400 Subject: [PATCH 14/17] remove suppress --- .../jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt index fdf888df..b8b1c38b 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt @@ -124,7 +124,6 @@ object Capture { @Synchronized @JvmStatic @JvmOverloads - @Suppress("SwallowedException") internal fun configure( apiKey: String, sessionStrategy: SessionStrategy, From ae821270c7ec14e16f2f7bd55f59d5d377f5fd6e Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Tue, 1 Oct 2024 18:27:15 -0400 Subject: [PATCH 15/17] fix teardown --- .../src/test/kotlin/io/bitdrift/capture/CaptureTest.kt | 6 ------ .../test/kotlin/io/bitdrift/capture/ConfigurationTest.kt | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt index 25ede99e..eb981b36 100644 --- a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt +++ b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt @@ -27,12 +27,6 @@ import org.robolectric.annotation.Config @Config(sdk = [21]) @FixMethodOrder(MethodSorters.NAME_ASCENDING) class CaptureTest { - - @Before - fun setUp() { - Capture.Logger.resetShared() - } - // This Test needs to run first since the following tests need to initialize // the ContextHolder before they can run. @Test diff --git a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt index 18ee04a6..a95d14dd 100644 --- a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt +++ b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt @@ -15,6 +15,7 @@ import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import io.bitdrift.capture.providers.session.SessionStrategy import org.assertj.core.api.Assertions +import org.junit.After import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -99,4 +100,9 @@ class ConfigurationTest { anyOrNull(), ) } + + @After + fun tearDown() { + Capture.Logger.resetShared() + } } From dbb64260da8d2aa6c66e7a3f8223055a960971eb Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Tue, 1 Oct 2024 18:34:52 -0400 Subject: [PATCH 16/17] format fix --- .../capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt index eb981b36..0b7f141a 100644 --- a/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt +++ b/platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureTest.kt @@ -15,7 +15,6 @@ import io.bitdrift.capture.network.HttpResponseInfo import io.bitdrift.capture.network.HttpUrlPath import io.bitdrift.capture.providers.session.SessionStrategy import org.assertj.core.api.Assertions.assertThat -import org.junit.Before import org.junit.FixMethodOrder import org.junit.Test import org.junit.runner.RunWith From 3a533d4c8214d71036c4606f9a9847dddf2e107c Mon Sep 17 00:00:00 2001 From: Rafal Augustyniak Date: Tue, 1 Oct 2024 19:01:20 -0400 Subject: [PATCH 17/17] fix test --- .../kotlin/io/bitdrift/capture/Capture.kt | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt index b8b1c38b..f037e2ad 100644 --- a/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt +++ b/platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/Capture.kt @@ -147,26 +147,24 @@ object Capture { } // Ideally we would use `getAndUpdate` in here but it's available for API 24 and up only. - when (default.getAndSet(LoggerState.ConfigurationStarted)) { - is LoggerState.NotConfigured -> { - try { - val logger = LoggerImpl( - apiKey = apiKey, - apiUrl = apiUrl, - fieldProviders = fieldProviders, - dateProvider = dateProvider ?: SystemDateProvider(), - configuration = configuration, - sessionStrategy = sessionStrategy, - bridge = bridge, - ) - default.set(LoggerState.Configured(logger)) - } catch (e: Throwable) { - Log.w("capture", "Capture initialization failed", e) - default.set(LoggerState.ConfigurationFailure) - } - } else -> { - Log.w("capture", "Attempted to initialize Capture more than once") + if (default.compareAndSet(LoggerState.NotConfigured, LoggerState.ConfigurationStarted)) { + try { + val logger = LoggerImpl( + apiKey = apiKey, + apiUrl = apiUrl, + fieldProviders = fieldProviders, + dateProvider = dateProvider ?: SystemDateProvider(), + configuration = configuration, + sessionStrategy = sessionStrategy, + bridge = bridge, + ) + default.set(LoggerState.Configured(logger)) + } catch (e: Throwable) { + Log.w("capture", "Capture initialization failed", e) + default.set(LoggerState.ConfigurationFailure) } + } else { + Log.w("capture", "Attempted to initialize Capture more than once") } }