From e07a1b84ad5c24616fe3cc8c1ce93eeebad7dc2a Mon Sep 17 00:00:00 2001 From: Maxime Epain Date: Thu, 24 Mar 2022 15:22:37 +0100 Subject: [PATCH 01/13] RUMM-1870 Add data encryption interface --- Datadog/Datadog.xcodeproj/project.pbxproj | 6 +++ Sources/Datadog/Core/Feature.swift | 4 ++ .../Datadog/Core/FeaturesConfiguration.swift | 4 +- .../Core/Persistence/DataEncryption.swift | 29 ++++++++++++ .../Datadog/Core/Persistence/DataFormat.swift | 6 +-- .../Core/Persistence/Reading/FileReader.swift | 45 ++++++++++++++----- .../Core/Persistence/Writing/FileWriter.swift | 27 ++++++++--- Sources/Datadog/Datadog.swift | 3 +- Sources/Datadog/DatadogConfiguration.swift | 8 ++++ .../DatadogConfiguration+objc.swift | 37 +++++++++++++++ .../BenchmarkMocks.swift | 3 +- .../Persistence/Reading/FileReaderTests.swift | 26 +++++++++++ .../Persistence/Writing/FileWriterTests.swift | 27 +++++++++++ .../DatadogConfigurationBuilderTests.swift | 3 ++ .../Datadog/Mocks/CoreMocks.swift | 36 ++++++++++++--- .../SystemFrameworks/FoundationMocks.swift | 8 ++++ .../DatadogObjc/DDConfigurationTests.swift | 34 ++++++++++++++ .../ObjcAPITests/DDConfiguration+apiTests.m | 18 ++++++++ 18 files changed, 292 insertions(+), 32 deletions(-) create mode 100644 Sources/Datadog/Core/Persistence/DataEncryption.swift diff --git a/Datadog/Datadog.xcodeproj/project.pbxproj b/Datadog/Datadog.xcodeproj/project.pbxproj index 86cba2396f..894fa074d6 100644 --- a/Datadog/Datadog.xcodeproj/project.pbxproj +++ b/Datadog/Datadog.xcodeproj/project.pbxproj @@ -981,6 +981,8 @@ D2CB6FF327C5369600A62B57 /* DatadogCrashReporting.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = D2CB6FD127C5348200A62B57 /* DatadogCrashReporting.framework */; }; D2DC4BBC27F234D600E4FB96 /* CITestIntegration.swift in Sources */ = {isa = PBXBuildFile; fileRef = E11625D727B681D200E428C6 /* CITestIntegration.swift */; }; D2DC4BBD27F234E000E4FB96 /* CITestIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E143CCAE27D236F600F4018A /* CITestIntegrationTests.swift */; }; + D2DC4BF627F484AA00E4FB96 /* DataEncryption.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2DC4BF527F484AA00E4FB96 /* DataEncryption.swift */; }; + D2DC4BF727F484AA00E4FB96 /* DataEncryption.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2DC4BF527F484AA00E4FB96 /* DataEncryption.swift */; }; D2EFF3D32731822A00D09F33 /* RUMViewsHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2EFF3D22731822A00D09F33 /* RUMViewsHandler.swift */; }; D2F1B81126D795F3009F3293 /* DDNoopRUMMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2F1B81026D795F3009F3293 /* DDNoopRUMMonitor.swift */; }; D2F1B81326D8DA68009F3293 /* DDNoopRUMMonitorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2F1B81226D8DA68009F3293 /* DDNoopRUMMonitorTests.swift */; }; @@ -1736,6 +1738,7 @@ D2CB6FB027C5217A00A62B57 /* DatadogObjc.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = DatadogObjc.framework; sourceTree = BUILT_PRODUCTS_DIR; }; D2CB6FD127C5348200A62B57 /* DatadogCrashReporting.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = DatadogCrashReporting.framework; sourceTree = BUILT_PRODUCTS_DIR; }; D2CB6FEC27C5352300A62B57 /* DatadogCrashReportingTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = DatadogCrashReportingTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; + D2DC4BF527F484AA00E4FB96 /* DataEncryption.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DataEncryption.swift; sourceTree = ""; }; D2EFF3D22731822A00D09F33 /* RUMViewsHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RUMViewsHandler.swift; sourceTree = ""; }; D2F1B81026D795F3009F3293 /* DDNoopRUMMonitor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DDNoopRUMMonitor.swift; sourceTree = ""; }; D2F1B81226D8DA68009F3293 /* DDNoopRUMMonitorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DDNoopRUMMonitorTests.swift; sourceTree = ""; }; @@ -2098,6 +2101,7 @@ children = ( 61AD4E3724531500006E34EA /* DataFormat.swift */, 6137C571271DAD4B00EFC4A1 /* DataOrchestrator.swift */, + D2DC4BF527F484AA00E4FB96 /* DataEncryption.swift */, 61133BA92423979B00786299 /* FilesOrchestrator.swift */, 613E79412577C08900DFCC17 /* Writing */, 613E79422577C09B00DFCC17 /* Reading */, @@ -5020,6 +5024,7 @@ 61133BDF2423979B00786299 /* SwiftExtensions.swift in Sources */, 61D3E0D3277B23F1008BE766 /* KronosDNSResolver.swift in Sources */, 6149FB3A2529D17F00EE387A /* InternalURLsFilter.swift in Sources */, + D2DC4BF627F484AA00E4FB96 /* DataEncryption.swift in Sources */, 611529A525E3DD51004F740E /* ValuePublisher.swift in Sources */, 61FFFB89278457D400401A28 /* KronosMonitor.swift in Sources */, 618DCFD724C7265300589570 /* RUMUUID.swift in Sources */, @@ -5590,6 +5595,7 @@ D2CB6E4427C50EAE00A62B57 /* UIApplicationSwizzler.swift in Sources */, D2CB6E4527C50EAE00A62B57 /* RUMResourceScope.swift in Sources */, D2CB6E4627C50EAE00A62B57 /* RUMSessionScope.swift in Sources */, + D2DC4BF727F484AA00E4FB96 /* DataEncryption.swift in Sources */, D2CB6E4727C50EAE00A62B57 /* TracingUUID.swift in Sources */, D2CB6E4827C50EAE00A62B57 /* ServerDateProvider.swift in Sources */, D2CB6E4927C50EAE00A62B57 /* AttributesSanitizer.swift in Sources */, diff --git a/Sources/Datadog/Core/Feature.swift b/Sources/Datadog/Core/Feature.swift index 813d3b520b..fdc9161742 100644 --- a/Sources/Datadog/Core/Feature.swift +++ b/Sources/Datadog/Core/Feature.swift @@ -32,6 +32,7 @@ internal struct FeaturesCommonDependencies { let carrierInfoProvider: CarrierInfoProviderType let launchTimeProvider: LaunchTimeProviderType let appStateListener: AppStateListening + let encryption: DataEncryption? } internal struct FeatureStorage { @@ -82,12 +83,14 @@ internal struct FeatureStorage { let unauthorizedFileWriter = FileWriter( dataFormat: dataFormat, orchestrator: unauthorizedFilesOrchestrator, + encryption: commonDependencies.encryption, internalMonitor: internalMonitor ) let authorizedFileWriter = FileWriter( dataFormat: dataFormat, orchestrator: authorizedFilesOrchestrator, + encryption: commonDependencies.encryption, internalMonitor: internalMonitor ) @@ -116,6 +119,7 @@ internal struct FeatureStorage { fileReader: FileReader( dataFormat: dataFormat, orchestrator: authorizedFilesOrchestrator, + encryption: commonDependencies.encryption, internalMonitor: internalMonitor ) ) diff --git a/Sources/Datadog/Core/FeaturesConfiguration.swift b/Sources/Datadog/Core/FeaturesConfiguration.swift index bb2efefb4a..417b8fa609 100644 --- a/Sources/Datadog/Core/FeaturesConfiguration.swift +++ b/Sources/Datadog/Core/FeaturesConfiguration.swift @@ -22,6 +22,7 @@ internal struct FeaturesConfiguration { let origin: String? let sdkVersion: String let proxyConfiguration: [AnyHashable: Any]? + let encryption: DataEncryption? } struct Logging { @@ -174,7 +175,8 @@ extension FeaturesConfiguration { source: source, origin: CITestIntegration.active?.origin, sdkVersion: sdkVersion, - proxyConfiguration: configuration.proxyConfiguration + proxyConfiguration: configuration.proxyConfiguration, + encryption: configuration.encryption ) if configuration.loggingEnabled { diff --git a/Sources/Datadog/Core/Persistence/DataEncryption.swift b/Sources/Datadog/Core/Persistence/DataEncryption.swift new file mode 100644 index 0000000000..59e4d16541 --- /dev/null +++ b/Sources/Datadog/Core/Persistence/DataEncryption.swift @@ -0,0 +1,29 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2019-2022 Datadog, Inc. + */ + +import Foundation + +/// Interface that allows storing data in encrypted format. Encryption/decryption round should +/// return exactly the same data as it given for the encryption originally (even if decryption +/// happens in another process/app launch). +public protocol DataEncryption { + /// Encrypts given `Data` with user-chosen encryption. + /// + /// - Parameter data: Data to encrypt. + /// - Returns: The encrypted data. + func encrypt(data: Data) throws -> Data + + /// Decrypts given `Data` with user-chosen encryption. + /// + /// Beware that data to decrypt could be encrypted in a previous + /// app launch, so implementation should be aware of the case when decryption could + /// fail (for example, key used for encryption is different from key used for decryption, if + /// they are unique for every app launch). + /// + /// - Parameter data: Data to decrypt. + /// - Returns: The decrypted data. + func decrypt(data: Data) throws -> Data +} diff --git a/Sources/Datadog/Core/Persistence/DataFormat.swift b/Sources/Datadog/Core/Persistence/DataFormat.swift index 88dfec9f9b..ec07d64f32 100644 --- a/Sources/Datadog/Core/Persistence/DataFormat.swift +++ b/Sources/Datadog/Core/Persistence/DataFormat.swift @@ -13,17 +13,17 @@ internal struct DataFormat { /// Suffixes the batch payload read from file. let suffixData: Data /// Separates entities written to file. - let separatorData: Data + let separatorByte: UInt8 // MARK: - Initialization init( prefix: String, suffix: String, - separator: String + separator: Character ) { self.prefixData = prefix.data(using: .utf8)! // swiftlint:disable:this force_unwrapping self.suffixData = suffix.data(using: .utf8)! // swiftlint:disable:this force_unwrapping - self.separatorData = separator.data(using: .utf8)! // swiftlint:disable:this force_unwrapping + self.separatorByte = separator.asciiValue! // swiftlint:disable:this force_unwrapping } } diff --git a/Sources/Datadog/Core/Persistence/Reading/FileReader.swift b/Sources/Datadog/Core/Persistence/Reading/FileReader.swift index 2a88da1c8f..b8f47363db 100644 --- a/Sources/Datadog/Core/Persistence/Reading/FileReader.swift +++ b/Sources/Datadog/Core/Persistence/Reading/FileReader.swift @@ -12,42 +12,63 @@ internal final class FileReader: Reader { private let dataFormat: DataFormat /// Orchestrator producing reference to readable file. private let orchestrator: FilesOrchestrator + private let encryption: DataEncryption? private let internalMonitor: InternalMonitor? /// Files marked as read. - private var filesRead: [ReadableFile] = [] + private var filesRead: Set = [] init( dataFormat: DataFormat, orchestrator: FilesOrchestrator, + encryption: DataEncryption? = nil, internalMonitor: InternalMonitor? = nil ) { self.dataFormat = dataFormat self.orchestrator = orchestrator + self.encryption = encryption self.internalMonitor = internalMonitor } // MARK: - Reading batches func readNextBatch() -> Batch? { - if let file = orchestrator.getReadableFile(excludingFilesNamed: Set(filesRead.map { $0.name })) { - do { - let fileData = try file.read() - let batchData = dataFormat.prefixData + fileData + dataFormat.suffixData - return Batch(data: batchData, file: file) - } catch { - internalMonitor?.sdkLogger.error("Failed to read data from file", error: error) - return nil - } + guard let file = orchestrator.getReadableFile(excludingFilesNamed: filesRead) else { + return nil } - return nil + do { + let fileData = try decode(data: file.read()) + let batchData = dataFormat.prefixData + fileData + dataFormat.suffixData + return Batch(data: batchData, file: file) + } catch { + internalMonitor?.sdkLogger.error("Failed to read data from file", error: error) + return nil + } + } + + func decode(data: Data) -> Data { + guard let encryption = encryption else { + return data + } + + return data + // split data + .split(separator: dataFormat.separatorByte) + // decode base64 - allow failure + .compactMap { Data(base64Encoded: $0) } + // decrypt data - allow failure + .compactMap { try? encryption.decrypt(data: $0) } + // concat data + .reduce(Data()) { $0 + $1 + [dataFormat.separatorByte] } + // drop last separator + .dropLast() } // MARK: - Accepting batches func markBatchAsRead(_ batch: Batch) { orchestrator.delete(readableFile: batch.file) - filesRead.append(batch.file) + filesRead.insert(batch.file.name) } } diff --git a/Sources/Datadog/Core/Persistence/Writing/FileWriter.swift b/Sources/Datadog/Core/Persistence/Writing/FileWriter.swift index c3f3791cc0..e314c980a3 100644 --- a/Sources/Datadog/Core/Persistence/Writing/FileWriter.swift +++ b/Sources/Datadog/Core/Persistence/Writing/FileWriter.swift @@ -14,16 +14,19 @@ internal final class FileWriter: Writer { private let orchestrator: FilesOrchestrator /// JSON encoder used to encode data. private let jsonEncoder: JSONEncoder + private let encryption: DataEncryption? private let internalMonitor: InternalMonitor? init( dataFormat: DataFormat, orchestrator: FilesOrchestrator, + encryption: DataEncryption? = nil, internalMonitor: InternalMonitor? = nil ) { self.dataFormat = dataFormat self.orchestrator = orchestrator - self.jsonEncoder = JSONEncoder.default() + self.jsonEncoder = .default() + self.encryption = encryption self.internalMonitor = internalMonitor } @@ -32,18 +35,28 @@ internal final class FileWriter: Writer { /// Encodes given value to JSON data and writes it to the file. func write(value: T) { do { - let data = try jsonEncoder.encode(value) + var data = try encode(value: value) let file = try orchestrator.getWritableFile(writeSize: UInt64(data.count)) - if try file.size() == 0 { - try file.append(data: data) - } else { - let atomicData = dataFormat.separatorData + data - try file.append(data: atomicData) + if try file.size() > 0 { + data.insert(dataFormat.separatorByte, at: 0) } + + try file.append(data: data) } catch { userLogger.error("🔥 Failed to write data: \(error)") internalMonitor?.sdkLogger.error("Failed to write data to file", error: error) } } + + func encode(value: T) throws -> Data { + let data = try jsonEncoder.encode(value) + + guard let encryption = encryption else { + return data + } + + return try encryption.encrypt(data: data) + .base64EncodedData() + } } diff --git a/Sources/Datadog/Datadog.swift b/Sources/Datadog/Datadog.swift index 9a1da7f4d2..b9f812100d 100644 --- a/Sources/Datadog/Datadog.swift +++ b/Sources/Datadog/Datadog.swift @@ -227,7 +227,8 @@ public class Datadog { networkConnectionInfoProvider: networkConnectionInfoProvider, carrierInfoProvider: carrierInfoProvider, launchTimeProvider: launchTimeProvider, - appStateListener: AppStateListener(dateProvider: dateProvider) + appStateListener: AppStateListener(dateProvider: dateProvider), + encryption: configuration.common.encryption ) if let internalMonitoringConfiguration = configuration.internalMonitoring { diff --git a/Sources/Datadog/DatadogConfiguration.swift b/Sources/Datadog/DatadogConfiguration.swift index f55693dee6..4c1f0067f4 100644 --- a/Sources/Datadog/DatadogConfiguration.swift +++ b/Sources/Datadog/DatadogConfiguration.swift @@ -267,6 +267,7 @@ extension Datadog { private(set) var uploadFrequency: UploadFrequency private(set) var additionalConfiguration: [String: Any] private(set) var proxyConfiguration: [AnyHashable: Any]? + private(set) var encryption: DataEncryption? /// The client token autorizing internal monitoring data to be sent to Datadog org. private(set) var internalMonitoringClientToken: String? @@ -770,6 +771,13 @@ extension Datadog { return self } + /// Sets data encryption to use for on-disk data persistency. + /// - Parameter encryption: An encryption object complying with `DataEncryption` protocol. + public func set(encryption: DataEncryption) -> Builder { + configuration.encryption = encryption + return self + } + /// Builds `Datadog.Configuration` object. public func build() -> Configuration { return configuration diff --git a/Sources/DatadogObjc/DatadogConfiguration+objc.swift b/Sources/DatadogObjc/DatadogConfiguration+objc.swift index a51b468486..3cea5f4e4c 100644 --- a/Sources/DatadogObjc/DatadogConfiguration+objc.swift +++ b/Sources/DatadogObjc/DatadogConfiguration+objc.swift @@ -142,6 +142,38 @@ public enum DDUploadFrequency: Int { } } +@objc +public protocol DDDataEncryption: AnyObject { + /// Encrypts given `Data` with user-chosen encryption. + /// + /// - Parameter data: Data to encrypt. + /// - Returns: The encrypted data. + func encrypt(data: Data) throws -> Data + + /// Decrypts given `Data` with user-chosen encryption. + /// + /// Beware that data to decrypt could be encrypted in a previous + /// app launch, so implementation should be aware of the case when decryption could + /// fail (for example, key used for encryption is different from key used for decryption, if + /// they are unique for every app launch). + /// + /// - Parameter data: Data to decrypt. + /// - Returns: The decrypted data. + func decrypt(data: Data) throws -> Data +} + +internal struct DDDataEncryptionBridge: DataEncryption { + let objcEncryption: DDDataEncryption + + func encrypt(data: Data) throws -> Data { + return try objcEncryption.encrypt(data: data) + } + + func decrypt(data: Data) throws -> Data { + return try objcEncryption.decrypt(data: data) + } +} + @objc public class DDConfiguration: NSObject { internal let sdkConfiguration: Datadog.Configuration @@ -362,6 +394,11 @@ public class DDConfigurationBuilder: NSObject { _ = sdkBuilder.set(proxyConfiguration: proxyConfiguration) } + @objc + public func set(encryption: DDDataEncryption) { + _ = sdkBuilder.set(encryption: DDDataEncryptionBridge(objcEncryption: encryption)) + } + @objc public func build() -> DDConfiguration { return DDConfiguration(sdkConfiguration: sdkBuilder.build()) diff --git a/Tests/DatadogBenchmarkTests/BenchmarkMocks.swift b/Tests/DatadogBenchmarkTests/BenchmarkMocks.swift index cacfefc72d..eaaca2e6dc 100644 --- a/Tests/DatadogBenchmarkTests/BenchmarkMocks.swift +++ b/Tests/DatadogBenchmarkTests/BenchmarkMocks.swift @@ -30,7 +30,8 @@ extension FeaturesCommonDependencies { networkConnectionInfoProvider: NetworkConnectionInfoProvider(), carrierInfoProvider: CarrierInfoProvider(), launchTimeProvider: LaunchTimeProvider(), - appStateListener: AppStateListener(dateProvider: SystemDateProvider()) + appStateListener: AppStateListener(dateProvider: SystemDateProvider()), + encryption: nil ) } } diff --git a/Tests/DatadogTests/Datadog/Core/Persistence/Reading/FileReaderTests.swift b/Tests/DatadogTests/Datadog/Core/Persistence/Reading/FileReaderTests.swift index 3980f9a84c..a001f3d06d 100644 --- a/Tests/DatadogTests/Datadog/Core/Persistence/Reading/FileReaderTests.swift +++ b/Tests/DatadogTests/Datadog/Core/Persistence/Reading/FileReaderTests.swift @@ -37,6 +37,32 @@ class FileReaderTests: XCTestCase { XCTAssertEqual(batch?.data, "[ABCD]".utf8Data) } + func testItReadsSingleEncryptedBatch() throws { + // Given + _ = try temporaryDirectory + .createFile(named: Date.mockAny().toFileName) + // base64(foo) = Zm9v + .append(data: "Zm9v,Zm9v,Zm9v".utf8Data) + + let reader = FileReader( + dataFormat: .mockWith(prefix: "[", suffix: "]", separator: ","), + orchestrator: FilesOrchestrator( + directory: temporaryDirectory, + performance: StoragePerformanceMock.readAllFiles, + dateProvider: SystemDateProvider() + ), + encryption: DataEncryptionMock( + decrypt: { _ in "bar".utf8Data } + ) + ) + + // When + let batch = reader.readNextBatch() + + // Then + XCTAssertEqual(batch?.data, "[bar,bar,bar]".utf8Data) + } + func testItMarksBatchesAsRead() throws { let dateProvider = RelativeDateProvider(advancingBySeconds: 60) let reader = FileReader( diff --git a/Tests/DatadogTests/Datadog/Core/Persistence/Writing/FileWriterTests.swift b/Tests/DatadogTests/Datadog/Core/Persistence/Writing/FileWriterTests.swift index 96ce39d2a5..6a7f482edc 100644 --- a/Tests/DatadogTests/Datadog/Core/Persistence/Writing/FileWriterTests.swift +++ b/Tests/DatadogTests/Datadog/Core/Persistence/Writing/FileWriterTests.swift @@ -171,4 +171,31 @@ class FileWriterTests: XCTestCase { XCTAssertGreaterThan(writtenData.count, 0) XCTAssertLessThanOrEqual(writtenData.count, 300) } + + func testItWritesEncryptedDataToSingleFile() throws { + // Given + let writer = FileWriter( + dataFormat: DataFormat(prefix: "[", suffix: "]", separator: ","), + orchestrator: FilesOrchestrator( + directory: temporaryDirectory, + performance: PerformancePreset(batchSize: .medium, uploadFrequency: .average, bundleType: .iOSApp), + dateProvider: SystemDateProvider() + ), + encryption: DataEncryptionMock( + encrypt: { _ in "foo".utf8Data } + ) + ) + + // When + writer.write(value: ["key1": "value1"]) + writer.write(value: ["key2": "value3"]) + writer.write(value: ["key3": "value3"]) + + // Then + XCTAssertEqual(try temporaryDirectory.files().count, 1) + XCTAssertEqual( + try temporaryDirectory.files()[0].read(), + "Zm9v,Zm9v,Zm9v".utf8Data // base64(foo) = Zm9v + ) + } } diff --git a/Tests/DatadogTests/Datadog/DatadogConfigurationBuilderTests.swift b/Tests/DatadogTests/Datadog/DatadogConfigurationBuilderTests.swift index 703a2d0375..2d81173565 100644 --- a/Tests/DatadogTests/Datadog/DatadogConfigurationBuilderTests.swift +++ b/Tests/DatadogTests/Datadog/DatadogConfigurationBuilderTests.swift @@ -59,6 +59,7 @@ class DatadogConfigurationBuilderTests: XCTestCase { XCTAssertEqual(configuration.batchSize, .medium) XCTAssertEqual(configuration.uploadFrequency, .average) XCTAssertEqual(configuration.additionalConfiguration.count, 0) + XCTAssertNil(configuration.encryption) } } @@ -108,6 +109,7 @@ class DatadogConfigurationBuilderTests: XCTestCase { kCFProxyUsernameKey: "proxyuser", kCFProxyPasswordKey: "proxypass", ]) + .set(encryption: DataEncryptionMock()) return builder } @@ -165,6 +167,7 @@ class DatadogConfigurationBuilderTests: XCTestCase { XCTAssertEqual(configuration.proxyConfiguration?[kCFNetworkProxiesHTTPProxy] as? String, "www.example.com") XCTAssertEqual(configuration.proxyConfiguration?[kCFProxyUsernameKey] as? String, "proxyuser") XCTAssertEqual(configuration.proxyConfiguration?[kCFProxyPasswordKey] as? String, "proxypass") + XCTAssertTrue(configuration.encryption is DataEncryptionMock) } XCTAssertTrue(rumConfigurationWithDefaultValues.rumUIKitViewsPredicate is DefaultUIKitRUMViewsPredicate) diff --git a/Tests/DatadogTests/Datadog/Mocks/CoreMocks.swift b/Tests/DatadogTests/Datadog/Mocks/CoreMocks.swift index 732706118c..d666d0033c 100644 --- a/Tests/DatadogTests/Datadog/Mocks/CoreMocks.swift +++ b/Tests/DatadogTests/Datadog/Mocks/CoreMocks.swift @@ -196,7 +196,8 @@ extension FeaturesConfiguration.Common { source: String = .mockAny(), origin: String? = nil, sdkVersion: String = .mockAny(), - proxyConfiguration: [AnyHashable: Any]? = nil + proxyConfiguration: [AnyHashable: Any]? = nil, + encryption: DataEncryption? = nil ) -> Self { return .init( applicationName: applicationName, @@ -208,7 +209,8 @@ extension FeaturesConfiguration.Common { source: source, origin: origin, sdkVersion: sdkVersion, - proxyConfiguration: proxyConfiguration + proxyConfiguration: proxyConfiguration, + encryption: encryption ) } } @@ -365,6 +367,22 @@ extension AppContext { } } +struct DataEncryptionMock: DataEncryption { + let enc: (Data) throws -> Data + let dec: (Data) throws -> Data + + init( + encrypt: @escaping (Data) throws -> Data = { $0 }, + decrypt: @escaping (Data) throws -> Data = { $0 } + ) { + enc = encrypt + dec = decrypt + } + + func encrypt(data: Data) throws -> Data { try enc(data) } + func decrypt(data: Data) throws -> Data { try dec(data) } +} + // MARK: - PerformancePreset Mocks struct StoragePerformanceMock: StoragePerformancePreset { @@ -492,7 +510,8 @@ extension FeaturesCommonDependencies { ), carrierInfoProvider: CarrierInfoProviderType = CarrierInfoProviderMock.mockAny(), launchTimeProvider: LaunchTimeProviderType = LaunchTimeProviderMock.mockAny(), - appStateListener: AppStateListening = AppStateListenerMock.mockAny() + appStateListener: AppStateListening = AppStateListenerMock.mockAny(), + encryption: DataEncryption? = nil ) -> FeaturesCommonDependencies { let httpClient: HTTPClient @@ -527,7 +546,8 @@ extension FeaturesCommonDependencies { networkConnectionInfoProvider: networkConnectionInfoProvider, carrierInfoProvider: carrierInfoProvider, launchTimeProvider: launchTimeProvider, - appStateListener: appStateListener + appStateListener: appStateListener, + encryption: encryption ) } @@ -544,7 +564,8 @@ extension FeaturesCommonDependencies { networkConnectionInfoProvider: NetworkConnectionInfoProviderType? = nil, carrierInfoProvider: CarrierInfoProviderType? = nil, launchTimeProvider: LaunchTimeProviderType? = nil, - appStateListener: AppStateListening? = nil + appStateListener: AppStateListening? = nil, + encryption: DataEncryption? = nil ) -> FeaturesCommonDependencies { return FeaturesCommonDependencies( consentProvider: consentProvider ?? self.consentProvider, @@ -558,7 +579,8 @@ extension FeaturesCommonDependencies { networkConnectionInfoProvider: networkConnectionInfoProvider ?? self.networkConnectionInfoProvider, carrierInfoProvider: carrierInfoProvider ?? self.carrierInfoProvider, launchTimeProvider: launchTimeProvider ?? self.launchTimeProvider, - appStateListener: appStateListener ?? self.appStateListener + appStateListener: appStateListener ?? self.appStateListener, + encryption: encryption ) } } @@ -613,7 +635,7 @@ extension DataFormat { static func mockWith( prefix: String = .mockAny(), suffix: String = .mockAny(), - separator: String = .mockAny() + separator: Character = .mockAny() ) -> DataFormat { return DataFormat( prefix: prefix, diff --git a/Tests/DatadogTests/Datadog/Mocks/SystemFrameworks/FoundationMocks.swift b/Tests/DatadogTests/Datadog/Mocks/SystemFrameworks/FoundationMocks.swift index ddfccdd4c3..3cc6de5267 100644 --- a/Tests/DatadogTests/Datadog/Mocks/SystemFrameworks/FoundationMocks.swift +++ b/Tests/DatadogTests/Datadog/Mocks/SystemFrameworks/FoundationMocks.swift @@ -243,6 +243,14 @@ extension String: AnyMockable, RandomMockable { static let decimalDigits = "0123456789" } +extension Character: AnyMockable, RandomMockable { + static func mockAny() -> Character { "c" } + + static func mockRandom() -> Character { + String.alphanumerics.randomElement()! + } +} + extension Bool: RandomMockable { static func mockRandom() -> Bool { return .random() diff --git a/Tests/DatadogTests/DatadogObjc/DDConfigurationTests.swift b/Tests/DatadogTests/DatadogObjc/DDConfigurationTests.swift index d5502b4b29..0b17e03bec 100644 --- a/Tests/DatadogTests/DatadogObjc/DDConfigurationTests.swift +++ b/Tests/DatadogTests/DatadogObjc/DDConfigurationTests.swift @@ -64,6 +64,7 @@ class DDConfigurationTests: XCTestCase { XCTAssertNil(configuration.rumActionEventMapper) XCTAssertNil(configuration.rumErrorEventMapper) XCTAssertEqual(configuration.additionalConfiguration.count, 0) + XCTAssertNil(configuration.encryption) } } @@ -197,6 +198,14 @@ class DDConfigurationTests: XCTestCase { XCTAssertEqual(objcBuilder.build().sdkConfiguration.proxyConfiguration?[kCFNetworkProxiesHTTPProxy] as? String, "www.example.com") XCTAssertEqual(objcBuilder.build().sdkConfiguration.proxyConfiguration?[kCFProxyUsernameKey] as? String, "proxyuser") XCTAssertEqual(objcBuilder.build().sdkConfiguration.proxyConfiguration?[kCFProxyPasswordKey] as? String, "proxypass") + + class ObjCDataEncryption: DDDataEncryption { + func encrypt(data: Data) throws -> Data { data } + func decrypt(data: Data) throws -> Data { data } + } + let dataEncryption = ObjCDataEncryption() + objcBuilder.set(encryption: dataEncryption) + XCTAssertTrue((objcBuilder.build().sdkConfiguration.encryption as? DDDataEncryptionBridge)?.objcEncryption === dataEncryption) } func testScrubbingRUMEvents() { @@ -285,6 +294,31 @@ class DDConfigurationTests: XCTestCase { XCTAssertNil(configuration.rumLongTaskEventMapper?(.mockRandom())) } + func testDataEncryption() throws { + // Given + class ObjCDataEncryption: DDDataEncryption { + let encData: Data = .mockRandom() + let decData: Data = .mockRandom() + func encrypt(data: Data) throws -> Data { encData } + func decrypt(data: Data) throws -> Data { decData } + } + + let encryption = ObjCDataEncryption() + + // When + let objcBuilder = DDConfiguration.builder( + rumApplicationID: "rum-app-id", + clientToken: "abc-123", + environment: "tests" + ) + objcBuilder.set(encryption: encryption) + let configuration = objcBuilder.build().sdkConfiguration + + // Then + XCTAssertEqual(try configuration.encryption?.encrypt(data: .mockRandom()), encryption.encData) + XCTAssertEqual(try configuration.encryption?.decrypt(data: .mockRandom()), encryption.decData) + } + func testDeprecatedTrackUIActions() { let objcBuilder = DDConfiguration.builder(clientToken: "abc-123", environment: "tests") diff --git a/Tests/DatadogTests/DatadogObjc/ObjcAPITests/DDConfiguration+apiTests.m b/Tests/DatadogTests/DatadogObjc/ObjcAPITests/DDConfiguration+apiTests.m index 30d1d9dcd0..ae0a88851f 100644 --- a/Tests/DatadogTests/DatadogObjc/ObjcAPITests/DDConfiguration+apiTests.m +++ b/Tests/DatadogTests/DatadogObjc/ObjcAPITests/DDConfiguration+apiTests.m @@ -34,6 +34,23 @@ - (DDRUMAction * _Nullable)rumActionWithPress:(enum UIPressType)type targetView: @end +// MARK: - DDDataEncryption + +@interface CustomDDDataEncryption: NSObject +@end + +@implementation CustomDDDataEncryption + +- (NSData * _Nullable)decryptWithData:(NSData * _Nonnull)data error:(NSError * _Nullable __autoreleasing * _Nullable)error { + return data; +} + +- (NSData * _Nullable)encryptWithData:(NSData * _Nonnull)data error:(NSError * _Nullable __autoreleasing * _Nullable)error { + return data; +} + +@end + // MARK: - Tests @interface DDConfiguration_apiTests : XCTestCase @@ -112,6 +129,7 @@ - (void)testDDConfigurationBuilderAPI { [builder setWithBatchSize:DDBatchSizeMedium]; [builder setWithUploadFrequency:DDUploadFrequencyAverage]; [builder setWithAdditionalConfiguration:@{}]; + [builder setWithEncryption:[CustomDDDataEncryption new]]; [builder build]; } From 1373bc3d7f56f5d72c4b5cb1024e20cdb5a2c4b8 Mon Sep 17 00:00:00 2001 From: Maxime Epain Date: Thu, 31 Mar 2022 15:26:51 +0200 Subject: [PATCH 02/13] RUMM-1870 Update CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48dae2a2f2..f4c15bf0b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ * [FEATURE] Web-view tracking. See [#729][] * [FEATURE] Integration with CI Visibility Tests. See[#761][] * [FEATURE] Add tvOS Support. See [#793][] +* [FEATURE] Add data encryption interface on-disk data storage. See [#797][] * [BUGFIX] Strip query parameters from span resource. See [#728][] * [BUGFIX] Stop reporting pre-warmed application launch time. See [#789][] * [BUGFIX] Allow log event dropping. See [#795][] @@ -334,6 +335,7 @@ [#793]: https://github.com/DataDog/dd-sdk-ios/issues/793 [#794]: https://github.com/DataDog/dd-sdk-ios/issues/794 [#795]: https://github.com/DataDog/dd-sdk-ios/issues/795 +[#797]: https://github.com/DataDog/dd-sdk-ios/pull/797 [@00FA9A]: https://github.com/00FA9A [@Britton-Earnin]: https://github.com/Britton-Earnin [@Hengyu]: https://github.com/Hengyu From 210571d46bb1653c201dedd00bd903b335cc484b Mon Sep 17 00:00:00 2001 From: Maxime Epain Date: Mon, 4 Apr 2022 15:07:50 +0200 Subject: [PATCH 03/13] RUMM-1870 Fix FeaturesCommonDependencies mocks Co-authored-by: Maciek Grzybowski --- Tests/DatadogTests/Datadog/Mocks/CoreMocks.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/DatadogTests/Datadog/Mocks/CoreMocks.swift b/Tests/DatadogTests/Datadog/Mocks/CoreMocks.swift index d666d0033c..241e8d6498 100644 --- a/Tests/DatadogTests/Datadog/Mocks/CoreMocks.swift +++ b/Tests/DatadogTests/Datadog/Mocks/CoreMocks.swift @@ -580,7 +580,7 @@ extension FeaturesCommonDependencies { carrierInfoProvider: carrierInfoProvider ?? self.carrierInfoProvider, launchTimeProvider: launchTimeProvider ?? self.launchTimeProvider, appStateListener: appStateListener ?? self.appStateListener, - encryption: encryption + encryption: encryption ?? self.encryption ) } } From bb6b2281cab770c9f906cfc6105775ad35e45050 Mon Sep 17 00:00:00 2001 From: Maxime Epain Date: Mon, 4 Apr 2022 15:50:14 +0200 Subject: [PATCH 04/13] RUMM-1870 Add encryption in feature tests --- .../InternalMonitoring/InternalMonitoringFeatureTests.swift | 4 +++- Tests/DatadogTests/Datadog/Logging/LoggingFeatureTests.swift | 4 +++- Tests/DatadogTests/Datadog/RUM/RUMFeatureTests.swift | 4 +++- Tests/DatadogTests/Datadog/Tracing/TracingFeatureTests.swift | 4 +++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Tests/DatadogTests/Datadog/InternalMonitoring/InternalMonitoringFeatureTests.swift b/Tests/DatadogTests/Datadog/InternalMonitoring/InternalMonitoringFeatureTests.swift index 90573dd1fc..9d8bf0ed34 100644 --- a/Tests/DatadogTests/Datadog/InternalMonitoring/InternalMonitoringFeatureTests.swift +++ b/Tests/DatadogTests/Datadog/InternalMonitoring/InternalMonitoringFeatureTests.swift @@ -35,6 +35,7 @@ class InternalMonitoringFeatureTests: XCTestCase { let randomDeviceModel: String = .mockRandom() let randomDeviceOSName: String = .mockRandom() let randomDeviceOSVersion: String = .mockRandom() + let randomEncryption: DataEncryption? = Bool.random() ? DataEncryptionMock() : nil let server = ServerMock(delivery: .success(response: .mockResponseWith(statusCode: 200))) @@ -47,7 +48,8 @@ class InternalMonitoringFeatureTests: XCTestCase { applicationVersion: randomApplicationVersion, source: randomSource, origin: randomOrigin, - sdkVersion: randomSDKVersion + sdkVersion: randomSDKVersion, + encryption: randomEncryption ), logsUploadURL: randomUploadURL, clientToken: randomClientToken diff --git a/Tests/DatadogTests/Datadog/Logging/LoggingFeatureTests.swift b/Tests/DatadogTests/Datadog/Logging/LoggingFeatureTests.swift index d862f9bf84..d209ecba7b 100644 --- a/Tests/DatadogTests/Datadog/Logging/LoggingFeatureTests.swift +++ b/Tests/DatadogTests/Datadog/Logging/LoggingFeatureTests.swift @@ -35,6 +35,7 @@ class LoggingFeatureTests: XCTestCase { let randomDeviceModel: String = .mockRandom() let randomDeviceOSName: String = .mockRandom() let randomDeviceOSVersion: String = .mockRandom() + let randomEncryption: DataEncryption? = Bool.random() ? DataEncryptionMock() : nil let server = ServerMock(delivery: .success(response: .mockResponseWith(statusCode: 200))) @@ -47,7 +48,8 @@ class LoggingFeatureTests: XCTestCase { applicationVersion: randomApplicationVersion, source: randomSource, origin: randomOrigin, - sdkVersion: randomSDKVersion + sdkVersion: randomSDKVersion, + encryption: randomEncryption ), uploadURL: randomUploadURL, clientToken: randomClientToken diff --git a/Tests/DatadogTests/Datadog/RUM/RUMFeatureTests.swift b/Tests/DatadogTests/Datadog/RUM/RUMFeatureTests.swift index f052c57467..2afe37bb99 100644 --- a/Tests/DatadogTests/Datadog/RUM/RUMFeatureTests.swift +++ b/Tests/DatadogTests/Datadog/RUM/RUMFeatureTests.swift @@ -37,6 +37,7 @@ class RUMFeatureTests: XCTestCase { let randomDeviceModel: String = .mockRandom() let randomDeviceOSName: String = .mockRandom() let randomDeviceOSVersion: String = .mockRandom() + let randomEncryption: DataEncryption? = Bool.random() ? DataEncryptionMock() : nil let server = ServerMock(delivery: .success(response: .mockResponseWith(statusCode: 200))) @@ -51,7 +52,8 @@ class RUMFeatureTests: XCTestCase { environment: randomEnvironmentName, source: randomSource, origin: randomOrigin, - sdkVersion: randomSDKVersion + sdkVersion: randomSDKVersion, + encryption: randomEncryption ), uploadURL: randomUploadURL, clientToken: randomClientToken diff --git a/Tests/DatadogTests/Datadog/Tracing/TracingFeatureTests.swift b/Tests/DatadogTests/Datadog/Tracing/TracingFeatureTests.swift index 06fc2a8502..e7a870a995 100644 --- a/Tests/DatadogTests/Datadog/Tracing/TracingFeatureTests.swift +++ b/Tests/DatadogTests/Datadog/Tracing/TracingFeatureTests.swift @@ -35,6 +35,7 @@ class TracingFeatureTests: XCTestCase { let randomDeviceModel: String = .mockRandom() let randomDeviceOSName: String = .mockRandom() let randomDeviceOSVersion: String = .mockRandom() + let randomEncryption: DataEncryption? = Bool.random() ? DataEncryptionMock() : nil let server = ServerMock(delivery: .success(response: .mockResponseWith(statusCode: 200))) @@ -47,7 +48,8 @@ class TracingFeatureTests: XCTestCase { applicationVersion: randomApplicationVersion, source: randomSource, origin: randomOrigin, - sdkVersion: randomSDKVersion + sdkVersion: randomSDKVersion, + encryption: randomEncryption ), uploadURL: randomUploadURL, clientToken: randomClientToken From 1a9ea42d6ce27e9cfafb9357d96a41218096d598 Mon Sep 17 00:00:00 2001 From: Maciek Grzybowski Date: Tue, 5 Apr 2022 11:27:05 +0200 Subject: [PATCH 05/13] Update `dogfood.py` to support version 2 for `package.resolved` files --- tools/distribution/dogfood.py | 6 + .../src/dogfood/package_resolved.py | 289 +++++++++++++----- .../tests/dogfood/test_package_resolved.py | 256 ++++++++++++++++ 3 files changed, 480 insertions(+), 71 deletions(-) create mode 100644 tools/distribution/tests/dogfood/test_package_resolved.py diff --git a/tools/distribution/dogfood.py b/tools/distribution/dogfood.py index 5144cd1017..d4256a6730 100755 --- a/tools/distribution/dogfood.py +++ b/tools/distribution/dogfood.py @@ -28,6 +28,12 @@ def dogfood(dry_run: bool, repository_url: str, repository_name: str, repository os.system(f'swift package --package-path {dd_sdk_package_path} resolve') dd_sdk_ios_package = PackageResolvedFile(path=f'{dd_sdk_package_path}/Package.resolved') + if dd_sdk_ios_package.version != 1: + raise Exception( + f'`dogfood.py` expects the `package.resolved` in `dd-sdk-ios` to use version 1 ' + + f'but version {dd_sdk_ios_package.version} was detected. Update `dogfood.py` to use this version.' + ) + # Clone dependant repo to temporary location and update its `Package.resolved` (one or many) so it points # to the current `dd-sdk-ios` commit. After that, push changes to dependant repo and create dogfooding PR. with TemporaryDirectory() as temp_dir: diff --git a/tools/distribution/src/dogfood/package_resolved.py b/tools/distribution/src/dogfood/package_resolved.py index 3afd5ba3ef..e5fe987e50 100644 --- a/tools/distribution/src/dogfood/package_resolved.py +++ b/tools/distribution/src/dogfood/package_resolved.py @@ -6,53 +6,137 @@ import json from copy import deepcopy +from typing import Optional -class PackageResolvedFile: +class PackageResolvedContent: + """An interface for manipulating `package.resolved` content.""" + + def has_dependency(self, package_name: str) -> bool: + """Checks if dependency with given name exists.""" + pass + + def update_dependency(self, package_name: str, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): + """ + Updates dependency resolution values. + :param package_name: the name of the package to update + :param new_branch: the new branch name (pass `None` for `null`) + :param new_revision: the new revision (pass `None` for `null`) + :param new_version: the new version name (pass `None` for `null`) + :return: + """ + pass + + def add_dependency(self, package_name: str, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): + """ + Adds new dependency resolution. + """ + pass + + def read_dependency_names(self) -> [str]: + """ + Returns package names for all dependencies. + :return: list of package names (strings) + """ + pass + + def read_dependency(self, package_name) -> dict: + """ + Returns resolution info for given dependency. + :param package_name: the name of dependency + :return: the `pin` object from `Package.resolved` + """ + pass + + +class PackageResolvedFile(PackageResolvedContent): """ Abstracts operations on `Package.resolved` file. """ - __SUPPORTED_PACKAGE_RESOLVED_VERSION = 1 + version: int + wrapped: PackageResolvedContent def __init__(self, path: str): print(f'⚙️ Opening {path}') self.path = path with open(path, 'r') as file: self.packages = json.load(file) - version = self.packages['version'] - if version != self.__SUPPORTED_PACKAGE_RESOLVED_VERSION: + self.version = self.packages['version'] + if self.version == 1: + self.wrapped = PackageResolvedContentV1(self.path, self.packages) + elif self.version == 2: + self.wrapped = PackageResolvedContentV2(self.path, self.packages) + else: raise Exception( - f'{path} uses version {version} but `package_resolved.py` supports ' + - f'version {self.__SUPPORTED_PACKAGE_RESOLVED_VERSION}. Update `package_resolved.py` to new format.' + f'{path} uses version {self.version} but `PackageResolvedFile` only supports ' + + f'versions `1` and `2`. Update `PackageResolvedFile` to support new version.' ) + def save(self): + """ + Saves changes to initial `path`. + """ + print(f'⚙️ Saving {self.path}') + with open(self.path, 'w') as file: + json.dump( + self.packages, + fp=file, + indent=2, # preserve `swift package` indentation + sort_keys=True # preserve `swift package` packages sorting + ) + file.write('\n') # add new line to the EOF + + def print(self): + """ + Prints the content of this file. + """ + with open(self.path, 'r') as file: + print(f'⚙️ Content of {file.name}:') + print(file.read()) + + def has_dependency(self, package_name: str) -> bool: + return self.wrapped.has_dependency(package_name) + + def update_dependency(self, package_name: str, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): + self.wrapped.update_dependency(package_name, new_branch, new_revision, new_version) + + def add_dependency(self, package_name: str, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): + self.wrapped.add_dependency(package_name, repository_url, branch, revision, version) + + def read_dependency_names(self) -> [str]: + return self.wrapped.read_dependency_names() + + def read_dependency(self, package_name) -> dict: + return self.wrapped.read_dependency(package_name) + + +class PackageResolvedContentV1(PackageResolvedContent): + """ + In `package.resolved` version `1`, sample package pin looks this: + + { + "package": "DatadogSDK", + "repositoryURL": "https://github.com/DataDog/dd-sdk-ios", + "state": { + "branch": "dogfooding", + "revision": "4e93a8f1f662d9126074a0f355b4b6d20f9f30a7", + "version": null + } + } + """ + + def __init__(self, path: str, json_content: dict): + self.path = path + self.packages = json_content + def has_dependency(self, package_name: str): pins = self.packages['object']['pins'] return package_name in [p['package'] for p in pins] - def update_dependency(self, package_name: str, new_branch: str, new_revision: str, new_version): - """ - Updates dependency resolution values. - :param package_name: the name of the package to update - :param new_branch: the new branch name (pass `None` for `null`) - :param new_revision: the new revision (pass `None` for `null`) - :param new_version: the new version name (pass `None` for `null`) - :return: - """ + def update_dependency(self, package_name: str, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): package = self.__get_package(package_name=package_name) - # Individual package pin looks this: - # { - # "package": "DatadogSDK", - # "repositoryURL": "https://github.com/DataDog/dd-sdk-ios", - # "state": { - # "branch": "dogfooding", - # "revision": "4e93a8f1f662d9126074a0f355b4b6d20f9f30a7", - # "version": null - # } - # } - old_state = deepcopy(package['state']) package['state']['branch'] = new_branch @@ -70,11 +154,7 @@ def update_dependency(self, package_name: str, new_branch: str, new_revision: st else: print(f'✏️️ "{package_name}" is up-to-date in {self.path}') - def add_dependency(self, package_name: str, repository_url: str, branch: str, revision: str, version): - """ - Inserts new dependency resolution to this `Package.resolved`. - """ - + def add_dependency(self, package_name: str, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): pins = self.packages['object']['pins'] # Find the index in `pins` array where the new dependency should be inserted. @@ -82,17 +162,6 @@ def add_dependency(self, package_name: str, repository_url: str, branch: str, re # - I've seen `Package.resolved` where some dependencies were misplaced. index = next((i for i in range(len(pins)) if pins[i]['package'].lower() > package_name.lower()), len(pins)) - # Individual package pin looks this: - # { - # "package": "DatadogSDK", - # "repositoryURL": "https://github.com/DataDog/dd-sdk-ios", - # "state": { - # "branch": "dogfooding", - # "revision": "4e93a8f1f662d9126074a0f355b4b6d20f9f30a7", - # "version": null - # } - # } - new_pin = { 'package': package_name, 'repositoryURL': repository_url, @@ -111,48 +180,126 @@ def add_dependency(self, package_name: str, repository_url: str, branch: str, re print(f' → version: {version}') def read_dependency_names(self): - """ - Returns package names for all dependencies in this `Package.resolved` file. - :return: list of package names (strings) - """ pins = self.packages['object']['pins'] package_names = [pin['package'] for pin in pins] return package_names def read_dependency(self, package_name): - """ - Returns resolution info for given dependency. - :param package_name: the name of dependency - :return: the `pin` object from `Package.resolved` - """ package = self.__get_package(package_name=package_name) return deepcopy(package) - def save(self): - """ - Saves changes to initial `path`. - """ - print(f'⚙️ Saving {self.path}') - with open(self.path, 'w') as file: - json.dump( - self.packages, - fp=file, - indent=2, # preserve `swift package` indentation - sort_keys=True # preserve `swift package` packages sorting + def __get_package(self, package_name: str): + pins = self.packages['object']['pins'] + package_pins = [index for index, p in enumerate(pins) if p['package'] == package_name] + + if len(package_pins) == 0: + raise Exception( + f'{self.path} does not contain pin named "{package_name}"' ) - file.write('\n') # add new line to the EOF - def print(self): - """ - Prints the content of this file. - """ - with open(self.path, 'r') as file: - print(f'⚙️ Content of {file.name}:') - print(file.read()) + package_pin_index = package_pins[0] + return self.packages['object']['pins'][package_pin_index] + + +class PackageResolvedContentV2(PackageResolvedContent): + """ + In `package.resolved` version `2`, sample package pin looks this: + + { + "identity" : "dd-sdk-ios", + "kind" : "remoteSourceControl", + "location" : "https://github.com/DataDog/dd-sdk-ios", + "state" : { + "branch" : "dogfooding", + "revision" : "6f662103771eb4523164e64f7f936bf9276f6bd0" + } + } + + It can also have `version`, e.g. `version: 1.0.0` and then `branch` is not set. In V1 it was + using `null` value to indicate this mutual exclusion. + """ + + def __init__(self, path: str, json_content: dict): + self.path = path + self.packages = json_content + + def has_dependency(self, package_name: str): + pins = self.packages['object']['pins'] + return package_name in [p['identity'] for p in pins] + + def update_dependency(self, package_name: str, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): + package = self.__get_package(package_name=package_name) + + old_state = deepcopy(package['state']) + + if new_branch: + package['state']['branch'] = new_branch + else: + package['state'].pop('branch', None) # delete key regardless of whether it exists + + if new_revision: + package['state']['revision'] = new_revision + else: + package['state'].pop('revision', None) + + if new_version: + package['state']['version'] = new_version + else: + package['state'].pop('version', None) + + new_state = deepcopy(package['state']) + + diff = old_state.items() ^ new_state.items() + + if len(diff) > 0: + print(f'✏️️ Updated "{package_name}" in {self.path}:') + print(f' → old: {old_state}') + print(f' → new: {new_state}') + else: + print(f'✏️️ "{package_name}" is up-to-date in {self.path}') + + def add_dependency(self, package_name: str, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): + pins = self.packages['object']['pins'] + + # Find the index in `pins` array where the new dependency should be inserted. + # The `pins` array seems to follow the alphabetical order. + index = next((i for i in range(len(pins)) if pins[i]['identity'].lower() > package_name.lower()), len(pins)) + + new_pin = { + 'identity': package_name, + 'kind': 'remoteSourceControl', + 'location': repository_url, + 'state': {} + } + + if branch: + new_pin['state']['branch'] = branch + + if revision: + new_pin['state']['revision'] = revision + + if version: + new_pin['state']['version'] = version + + pins.insert(index, new_pin) + + print(f'✏️️ Added "{package_name}" at index {index} in {self.path}:') + print(f' → branch: {branch}') + print(f' → revision: {revision}') + print(f' → version: {version}') + + def read_dependency_names(self): + pins = self.packages['object']['pins'] + package_names = [pin['identity'] for pin in pins] + return package_names + + def read_dependency(self, package_name): + package = self.__get_package(package_name=package_name) + return deepcopy(package) def __get_package(self, package_name: str): pins = self.packages['object']['pins'] - package_pins = [index for index, p in enumerate(pins) if p['package'] == package_name] + package_pins = [index for index, p in enumerate(pins) if p['identity'] == package_name] if len(package_pins) == 0: raise Exception( diff --git a/tools/distribution/tests/dogfood/test_package_resolved.py b/tools/distribution/tests/dogfood/test_package_resolved.py new file mode 100644 index 0000000000..576034b79a --- /dev/null +++ b/tools/distribution/tests/dogfood/test_package_resolved.py @@ -0,0 +1,256 @@ +# ----------------------------------------------------------- +# Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019-2020 Datadog, Inc. +# ----------------------------------------------------------- + + +import unittest +from tempfile import TemporaryDirectory, NamedTemporaryFile +from src.dogfood.package_resolved import PackageResolvedFile + + +class PackageResolvedFileTestCase(unittest.TestCase): + v1_file_content = b''' + { + "object": { + "pins": [ + { + "package": "A", + "repositoryURL": "https://github.com/A-org/a.git", + "state": { + "branch": "a-branch", + "revision": "a-revision", + "version": null + } + }, + { + "package": "B", + "repositoryURL": "https://github.com/B-org/b.git", + "state": { + "branch": null, + "revision": "b-revision", + "version": "1.0.0" + } + } + ] + }, + "version": 1 + } + ''' + + v2_file_content = b''' + { + "object": { + "pins": [ + { + "identity" : "a", + "kind" : "remoteSourceControl", + "location" : "https://github.com/A-org/a", + "state" : { + "branch" : "a-branch", + "revision" : "a-revision" + } + }, + { + "identity" : "b", + "kind" : "remoteSourceControl", + "location" : "https://github.com/B-org/b.git", + "state" : { + "revision" : "b-revision", + "version" : "1.0.0" + } + } + ] + }, + "version": 2 + } + ''' + + def test_it_reads_version1_files(self): + with NamedTemporaryFile() as file: + file.write(self.v1_file_content) + file.seek(0) + + package_resolved = PackageResolvedFile(path=file.name) + self.assertTrue(package_resolved.has_dependency(package_name='A')) + self.assertTrue(package_resolved.has_dependency(package_name='B')) + self.assertFalse(package_resolved.has_dependency(package_name='C')) + self.assertListEqual(['A', 'B'], package_resolved.read_dependency_names()) + self.assertDictEqual( + { + 'package': 'A', + 'repositoryURL': 'https://github.com/A-org/a.git', + 'state': {'branch': 'a-branch', 'revision': 'a-revision', 'version': None} + }, + package_resolved.read_dependency(package_name='A') + ) + self.assertDictEqual( + { + 'package': 'B', + 'repositoryURL': 'https://github.com/B-org/b.git', + 'state': {'branch': None, 'revision': 'b-revision', 'version': '1.0.0'} + }, + package_resolved.read_dependency(package_name='B') + ) + + def test_it_changes_version1_files(self): + with NamedTemporaryFile() as file: + file.write(self.v1_file_content) + file.seek(0) + + package_resolved = PackageResolvedFile(path=file.name) + package_resolved.update_dependency( + package_name='B', new_branch='b-branch-new', new_revision='b-revision-new', new_version=None + ) + package_resolved.add_dependency( + package_name='C', repository_url='https://github.com/C-org/c.git', + branch='c-branch', revision='c-revision', version=None + ) + package_resolved.add_dependency( + package_name='D', repository_url='https://github.com/D-org/d.git', + branch=None, revision='d-revision', version='1.1.0' + ) + package_resolved.save() + + actual_new_content = file.read().decode('utf-8') + expected_new_content = '''{ + "object": { + "pins": [ + { + "package": "A", + "repositoryURL": "https://github.com/A-org/a.git", + "state": { + "branch": "a-branch", + "revision": "a-revision", + "version": null + } + }, + { + "package": "B", + "repositoryURL": "https://github.com/B-org/b.git", + "state": { + "branch": "b-branch-new", + "revision": "b-revision-new", + "version": null + } + }, + { + "package": "C", + "repositoryURL": "https://github.com/C-org/c.git", + "state": { + "branch": "c-branch", + "revision": "c-revision", + "version": null + } + }, + { + "package": "D", + "repositoryURL": "https://github.com/D-org/d.git", + "state": { + "branch": null, + "revision": "d-revision", + "version": "1.1.0" + } + } + ] + }, + "version": 1 +} +''' + self.assertEqual(expected_new_content, actual_new_content) + + def test_it_reads_version2_files(self): + with NamedTemporaryFile() as file: + file.write(self.v2_file_content) + file.seek(0) + + package_resolved = PackageResolvedFile(path=file.name) + self.assertTrue(package_resolved.has_dependency(package_name='a')) + self.assertTrue(package_resolved.has_dependency(package_name='b')) + self.assertFalse(package_resolved.has_dependency(package_name='c')) + self.assertListEqual(['a', 'b'], package_resolved.read_dependency_names()) + self.assertDictEqual( + { + 'identity': 'a', + 'kind': 'remoteSourceControl', + 'location': 'https://github.com/A-org/a', + 'state': {'branch': 'a-branch', 'revision': 'a-revision'} + }, + package_resolved.read_dependency(package_name='a') + ) + self.assertDictEqual( + { + 'identity': 'b', + 'kind': 'remoteSourceControl', + 'location': 'https://github.com/B-org/b.git', + 'state': {'revision': 'b-revision', 'version': '1.0.0'} + }, + package_resolved.read_dependency(package_name='b') + ) + + def test_it_changes_version2_files(self): + with NamedTemporaryFile() as file: + file.write(self.v2_file_content) + file.seek(0) + + package_resolved = PackageResolvedFile(path=file.name) + package_resolved.update_dependency( + package_name='b', new_branch='b-branch-new', new_revision='b-revision-new', new_version=None + ) + package_resolved.add_dependency( + package_name='c', repository_url='https://github.com/C-org/c.git', + branch='c-branch', revision='c-revision', version=None + ) + package_resolved.add_dependency( + package_name='d', repository_url='https://github.com/D-org/d.git', + branch=None, revision='d-revision', version='1.1.0' + ) + package_resolved.save() + + actual_new_content = file.read().decode('utf-8') + expected_new_content = '''{ + "object": { + "pins": [ + { + "identity": "a", + "kind": "remoteSourceControl", + "location": "https://github.com/A-org/a", + "state": { + "branch": "a-branch", + "revision": "a-revision" + } + }, + { + "identity": "b", + "kind": "remoteSourceControl", + "location": "https://github.com/B-org/b.git", + "state": { + "branch": "b-branch-new", + "revision": "b-revision-new" + } + }, + { + "identity": "c", + "kind": "remoteSourceControl", + "location": "https://github.com/C-org/c.git", + "state": { + "branch": "c-branch", + "revision": "c-revision" + } + }, + { + "identity": "d", + "kind": "remoteSourceControl", + "location": "https://github.com/D-org/d.git", + "state": { + "revision": "d-revision", + "version": "1.1.0" + } + } + ] + }, + "version": 2 +} +''' + self.assertEqual(expected_new_content, actual_new_content) From 8095bac918213a921ab414f7e9303306aaed640d Mon Sep 17 00:00:00 2001 From: Maciek Grzybowski Date: Tue, 5 Apr 2022 11:37:07 +0200 Subject: [PATCH 06/13] Support identifying dependencies by their name (v1) or identity (v2) to make it smooth for both `package.resolved` version formats --- tools/distribution/src/dogfood/package_resolved.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tools/distribution/src/dogfood/package_resolved.py b/tools/distribution/src/dogfood/package_resolved.py index e5fe987e50..2888732095 100644 --- a/tools/distribution/src/dogfood/package_resolved.py +++ b/tools/distribution/src/dogfood/package_resolved.py @@ -9,6 +9,17 @@ from typing import Optional +class PackageID: + """Identifies package in `package.resolved` file.""" + + v1: str # used in `package.resolved` version 1 + v2: str # used in `package.resolved` version 2 + + def __init__(self, package_name: str, repository_url: str): + self.v1 = package_name + self.v2 = package_name + + class PackageResolvedContent: """An interface for manipulating `package.resolved` content.""" From b5d9243ecf5377daaf304b9feef13f7121c2df21 Mon Sep 17 00:00:00 2001 From: Maciek Grzybowski Date: Tue, 5 Apr 2022 13:42:54 +0200 Subject: [PATCH 07/13] Support new JSON structure in v2's `package.resolved` --- tools/distribution/dogfood.py | 14 +- .../src/dogfood/package_resolved.py | 206 ++++++++++-------- .../tests/dogfood/test_package_resolved.py | 170 ++++++++------- 3 files changed, 212 insertions(+), 178 deletions(-) diff --git a/tools/distribution/dogfood.py b/tools/distribution/dogfood.py index d4256a6730..53e903a061 100755 --- a/tools/distribution/dogfood.py +++ b/tools/distribution/dogfood.py @@ -11,7 +11,7 @@ import os import traceback from tempfile import TemporaryDirectory -from src.dogfood.package_resolved import PackageResolvedFile +from src.dogfood.package_resolved import PackageResolvedFile, PackageID from src.dogfood.dogfooded_commit import DogfoodedCommit from src.dogfood.repository import Repository from src.utils import remember_cwd @@ -52,26 +52,26 @@ def dogfood(dry_run: bool, repository_url: str, repository_name: str, repository # Update version of `dd-sdk-ios`: for package in packages: package.update_dependency( - package_name='DatadogSDK', + package_id=PackageID(v1='DatadogSDK', v2='dd-sdk-ios'), new_branch='dogfooding', new_revision=dd_sdk_ios_commit.hash, new_version=None ) # Add or update `dd-sdk-ios` dependencies - for dependency_name in dd_sdk_ios_package.read_dependency_names(): - dependency = dd_sdk_ios_package.read_dependency(package_name=dependency_name) + for dependency_id in dd_sdk_ios_package.read_dependency_ids(): + dependency = dd_sdk_ios_package.read_dependency(package_id=dependency_id) - if package.has_dependency(package_name=dependency_name): + if package.has_dependency(package_id=dependency_id): package.update_dependency( - package_name=dependency_name, + package_id=dependency_id, new_branch=dependency['state']['branch'], new_revision=dependency['state']['revision'], new_version=dependency['state']['version'], ) else: package.add_dependency( - package_name=dependency_name, + package_id=dependency_id, repository_url=dependency['repositoryURL'], branch=dependency['state']['branch'], revision=dependency['state']['revision'], diff --git a/tools/distribution/src/dogfood/package_resolved.py b/tools/distribution/src/dogfood/package_resolved.py index 2888732095..f0784d5118 100644 --- a/tools/distribution/src/dogfood/package_resolved.py +++ b/tools/distribution/src/dogfood/package_resolved.py @@ -5,32 +5,42 @@ # ----------------------------------------------------------- import json +from dataclasses import dataclass from copy import deepcopy from typing import Optional +@dataclass() class PackageID: - """Identifies package in `package.resolved` file.""" + """ + Identifies package in `package.resolved` file. + It supports `version: 1` and `version: 2` of `package.resolved` format: + - v1 uses package name (e.g. `DatadogSDK`) as package identifier + - v2 uses package identity (e.g. `dd-sdk-ios`) as package identifier + - v2 is not backward compatible with v1 - the v1 package name cannot be read from v2's `package.resolved` + - v1 is forward compatible with v2 - the v2 package identity can be read from `repositoryURL` in v1's `package.resolved` + """ + v1: Optional[str] # can be `None` if read from v2's `package.resolved` + v2: str - v1: str # used in `package.resolved` version 1 - v2: str # used in `package.resolved` version 2 - def __init__(self, package_name: str, repository_url: str): - self.v1 = package_name - self.v2 = package_name +def v2_package_id_from_repository_url(repository_url: str) -> str: + """Reads v2 package id from repository URL.""" + components = repository_url.split('/') # e.g. ['https:/', '', 'github.com', 'A-org', 'abc.git'] + return components[-1].split('.')[0] class PackageResolvedContent: """An interface for manipulating `package.resolved` content.""" - def has_dependency(self, package_name: str) -> bool: - """Checks if dependency with given name exists.""" + def has_dependency(self, package_id: PackageID) -> bool: + """Checks if dependency with given ID exists.""" pass - def update_dependency(self, package_name: str, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): + def update_dependency(self, package_id: PackageID, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): """ Updates dependency resolution values. - :param package_name: the name of the package to update + :param package_id: identifies dependency to update :param new_branch: the new branch name (pass `None` for `null`) :param new_revision: the new revision (pass `None` for `null`) :param new_version: the new version name (pass `None` for `null`) @@ -38,23 +48,23 @@ def update_dependency(self, package_name: str, new_branch: Optional[str], new_re """ pass - def add_dependency(self, package_name: str, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): + def add_dependency(self, package_id: PackageID, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): """ Adds new dependency resolution. """ pass - def read_dependency_names(self) -> [str]: + def read_dependency_ids(self) -> [PackageID]: """ - Returns package names for all dependencies. - :return: list of package names (strings) + Returns package IDs for all dependencies. + :return: list of package IDs (PackageIDs) """ pass - def read_dependency(self, package_name) -> dict: + def read_dependency(self, package_id: PackageID) -> dict: """ Returns resolution info for given dependency. - :param package_name: the name of dependency + :param package_id: the `PackageID` of dependency :return: the `pin` object from `Package.resolved` """ pass @@ -106,47 +116,55 @@ def print(self): print(f'⚙️ Content of {file.name}:') print(file.read()) - def has_dependency(self, package_name: str) -> bool: - return self.wrapped.has_dependency(package_name) + def has_dependency(self, package_id: PackageID) -> bool: + return self.wrapped.has_dependency(package_id) - def update_dependency(self, package_name: str, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): - self.wrapped.update_dependency(package_name, new_branch, new_revision, new_version) + def update_dependency(self, package_id: PackageID, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): + self.wrapped.update_dependency(package_id, new_branch, new_revision, new_version) - def add_dependency(self, package_name: str, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): - self.wrapped.add_dependency(package_name, repository_url, branch, revision, version) + def add_dependency(self, package_id: PackageID, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): + self.wrapped.add_dependency(package_id, repository_url, branch, revision, version) - def read_dependency_names(self) -> [str]: - return self.wrapped.read_dependency_names() + def read_dependency_ids(self) -> [PackageID]: + return self.wrapped.read_dependency_ids() - def read_dependency(self, package_name) -> dict: - return self.wrapped.read_dependency(package_name) + def read_dependency(self, package_id: PackageID) -> dict: + return self.wrapped.read_dependency(package_id) class PackageResolvedContentV1(PackageResolvedContent): """ - In `package.resolved` version `1`, sample package pin looks this: - - { - "package": "DatadogSDK", - "repositoryURL": "https://github.com/DataDog/dd-sdk-ios", - "state": { - "branch": "dogfooding", - "revision": "4e93a8f1f662d9126074a0f355b4b6d20f9f30a7", - "version": null + Example of `package.resolved` in version `1` looks this:: + + { + "object": { + "pins": [ + { + "package": "DatadogSDK", + "repositoryURL": "https://github.com/DataDog/dd-sdk-ios", + "state": { + "branch": "dogfooding", + "revision": "4e93a8f1f662d9126074a0f355b4b6d20f9f30a7", + "version": null + } + }, + ... + ] + }, + "version": 1 } - } """ def __init__(self, path: str, json_content: dict): self.path = path self.packages = json_content - def has_dependency(self, package_name: str): + def has_dependency(self, package_id: PackageID): pins = self.packages['object']['pins'] - return package_name in [p['package'] for p in pins] + return package_id.v1 in [p['package'] for p in pins] - def update_dependency(self, package_name: str, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): - package = self.__get_package(package_name=package_name) + def update_dependency(self, package_id: PackageID, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): + package = self.__get_package(package_id=package_id) old_state = deepcopy(package['state']) @@ -159,22 +177,22 @@ def update_dependency(self, package_name: str, new_branch: Optional[str], new_re diff = old_state.items() ^ new_state.items() if len(diff) > 0: - print(f'✏️️ Updated "{package_name}" in {self.path}:') + print(f'✏️️ Updated "{package_id.v1}" in {self.path}:') print(f' → old: {old_state}') print(f' → new: {new_state}') else: - print(f'✏️️ "{package_name}" is up-to-date in {self.path}') + print(f'✏️️ "{package_id.v1}" is up-to-date in {self.path}') - def add_dependency(self, package_name: str, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): + def add_dependency(self, package_id: PackageID, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): pins = self.packages['object']['pins'] # Find the index in `pins` array where the new dependency should be inserted. # The `pins` array seems to follow the alphabetical order, but not always # - I've seen `Package.resolved` where some dependencies were misplaced. - index = next((i for i in range(len(pins)) if pins[i]['package'].lower() > package_name.lower()), len(pins)) + index = next((i for i in range(len(pins)) if pins[i]['package'].lower() > package_id.v1.lower()), len(pins)) new_pin = { - 'package': package_name, + 'package': package_id.v1, 'repositoryURL': repository_url, 'state': { 'branch': branch, @@ -185,27 +203,27 @@ def add_dependency(self, package_name: str, repository_url: str, branch: Optiona pins.insert(index, new_pin) - print(f'✏️️ Added "{package_name}" at index {index} in {self.path}:') + print(f'✏️️ Added "{package_id.v1}" at index {index} in {self.path}:') print(f' → branch: {branch}') print(f' → revision: {revision}') print(f' → version: {version}') - def read_dependency_names(self): + def read_dependency_ids(self): pins = self.packages['object']['pins'] - package_names = [pin['package'] for pin in pins] - return package_names + package_ids = [PackageID(v1=pin['package'], v2=v2_package_id_from_repository_url(pin['repositoryURL'])) for pin in pins] + return package_ids - def read_dependency(self, package_name): - package = self.__get_package(package_name=package_name) + def read_dependency(self, package_id: PackageID): + package = self.__get_package(package_id=package_id) return deepcopy(package) - def __get_package(self, package_name: str): + def __get_package(self, package_id: PackageID): pins = self.packages['object']['pins'] - package_pins = [index for index, p in enumerate(pins) if p['package'] == package_name] + package_pins = [index for index, p in enumerate(pins) if p['package'] == package_id.v1] if len(package_pins) == 0: raise Exception( - f'{self.path} does not contain pin named "{package_name}"' + f'{self.path} does not contain pin named "{package_id.v1}"' ) package_pin_index = package_pins[0] @@ -214,32 +232,38 @@ def __get_package(self, package_name: str): class PackageResolvedContentV2(PackageResolvedContent): """ - In `package.resolved` version `2`, sample package pin looks this: - - { - "identity" : "dd-sdk-ios", - "kind" : "remoteSourceControl", - "location" : "https://github.com/DataDog/dd-sdk-ios", - "state" : { - "branch" : "dogfooding", - "revision" : "6f662103771eb4523164e64f7f936bf9276f6bd0" - } - } - - It can also have `version`, e.g. `version: 1.0.0` and then `branch` is not set. In V1 it was - using `null` value to indicate this mutual exclusion. + Example of `package.resolved` in version `2` looks this:: + + { + "pins": [ + { + "identity" : "dd-sdk-ios", + "kind" : "remoteSourceControl", + "location" : "https://github.com/DataDog/dd-sdk-ios", + "state": { + "branch": "dogfooding", + "revision" : "6f662103771eb4523164e64f7f936bf9276f6bd0" + } + }, + ... + ] + "version": 2 + } + + In v2 `branch` and `version` are mutually exclusive: if one is set, the other + is not present (unlike v1, where one was always set to `null`). """ def __init__(self, path: str, json_content: dict): self.path = path self.packages = json_content - def has_dependency(self, package_name: str): - pins = self.packages['object']['pins'] - return package_name in [p['identity'] for p in pins] + def has_dependency(self, package_id: PackageID): + pins = self.packages['pins'] + return package_id.v2 in [p['identity'] for p in pins] - def update_dependency(self, package_name: str, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): - package = self.__get_package(package_name=package_name) + def update_dependency(self, package_id: PackageID, new_branch: Optional[str], new_revision: str, new_version: Optional[str]): + package = self.__get_package(package_id=package_id) old_state = deepcopy(package['state']) @@ -263,21 +287,21 @@ def update_dependency(self, package_name: str, new_branch: Optional[str], new_re diff = old_state.items() ^ new_state.items() if len(diff) > 0: - print(f'✏️️ Updated "{package_name}" in {self.path}:') + print(f'✏️️ Updated "{package_id.v2}" in {self.path}:') print(f' → old: {old_state}') print(f' → new: {new_state}') else: - print(f'✏️️ "{package_name}" is up-to-date in {self.path}') + print(f'✏️️ "{package_id.v2}" is up-to-date in {self.path}') - def add_dependency(self, package_name: str, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): - pins = self.packages['object']['pins'] + def add_dependency(self, package_id: PackageID, repository_url: str, branch: Optional[str], revision: str, version: Optional[str]): + pins = self.packages['pins'] # Find the index in `pins` array where the new dependency should be inserted. # The `pins` array seems to follow the alphabetical order. - index = next((i for i in range(len(pins)) if pins[i]['identity'].lower() > package_name.lower()), len(pins)) + index = next((i for i in range(len(pins)) if pins[i]['identity'].lower() > package_id.v2.lower()), len(pins)) new_pin = { - 'identity': package_name, + 'identity': package_id.v2, 'kind': 'remoteSourceControl', 'location': repository_url, 'state': {} @@ -294,28 +318,28 @@ def add_dependency(self, package_name: str, repository_url: str, branch: Optiona pins.insert(index, new_pin) - print(f'✏️️ Added "{package_name}" at index {index} in {self.path}:') + print(f'✏️️ Added "{package_id.v2}" at index {index} in {self.path}:') print(f' → branch: {branch}') print(f' → revision: {revision}') print(f' → version: {version}') - def read_dependency_names(self): - pins = self.packages['object']['pins'] - package_names = [pin['identity'] for pin in pins] - return package_names + def read_dependency_ids(self) -> [PackageID]: + pins = self.packages['pins'] + package_ids = [PackageID(v1=None, v2=pin['identity']) for pin in pins] + return package_ids - def read_dependency(self, package_name): - package = self.__get_package(package_name=package_name) + def read_dependency(self, package_id: PackageID): + package = self.__get_package(package_id=package_id) return deepcopy(package) - def __get_package(self, package_name: str): - pins = self.packages['object']['pins'] - package_pins = [index for index, p in enumerate(pins) if p['identity'] == package_name] + def __get_package(self, package_id: PackageID): + pins = self.packages['pins'] + package_pins = [index for index, p in enumerate(pins) if p['identity'] == package_id.v2] if len(package_pins) == 0: raise Exception( - f'{self.path} does not contain pin named "{package_name}"' + f'{self.path} does not contain pin named "{package_id.v2}"' ) package_pin_index = package_pins[0] - return self.packages['object']['pins'][package_pin_index] + return self.packages['pins'][package_pin_index] diff --git a/tools/distribution/tests/dogfood/test_package_resolved.py b/tools/distribution/tests/dogfood/test_package_resolved.py index 576034b79a..d6620ef06e 100644 --- a/tools/distribution/tests/dogfood/test_package_resolved.py +++ b/tools/distribution/tests/dogfood/test_package_resolved.py @@ -6,8 +6,8 @@ import unittest -from tempfile import TemporaryDirectory, NamedTemporaryFile -from src.dogfood.package_resolved import PackageResolvedFile +from tempfile import NamedTemporaryFile +from src.dogfood.package_resolved import PackageResolvedFile, PackageID, v2_package_id_from_repository_url class PackageResolvedFileTestCase(unittest.TestCase): @@ -41,28 +41,26 @@ class PackageResolvedFileTestCase(unittest.TestCase): v2_file_content = b''' { - "object": { - "pins": [ - { - "identity" : "a", - "kind" : "remoteSourceControl", - "location" : "https://github.com/A-org/a", - "state" : { - "branch" : "a-branch", - "revision" : "a-revision" - } - }, - { - "identity" : "b", - "kind" : "remoteSourceControl", - "location" : "https://github.com/B-org/b.git", - "state" : { - "revision" : "b-revision", - "version" : "1.0.0" - } + "pins": [ + { + "identity" : "a", + "kind" : "remoteSourceControl", + "location" : "https://github.com/A-org/a", + "state" : { + "branch" : "a-branch", + "revision" : "a-revision" } - ] - }, + }, + { + "identity" : "b", + "kind" : "remoteSourceControl", + "location" : "https://github.com/B-org/b.git", + "state" : { + "revision" : "b-revision", + "version" : "1.0.0" + } + } + ], "version": 2 } ''' @@ -73,17 +71,20 @@ def test_it_reads_version1_files(self): file.seek(0) package_resolved = PackageResolvedFile(path=file.name) - self.assertTrue(package_resolved.has_dependency(package_name='A')) - self.assertTrue(package_resolved.has_dependency(package_name='B')) - self.assertFalse(package_resolved.has_dependency(package_name='C')) - self.assertListEqual(['A', 'B'], package_resolved.read_dependency_names()) + self.assertTrue(package_resolved.has_dependency(package_id=PackageID(v1='A', v2='a'))) + self.assertTrue(package_resolved.has_dependency(package_id=PackageID(v1='B', v2='b'))) + self.assertFalse(package_resolved.has_dependency(package_id=PackageID(v1='C', v2='c'))) + self.assertListEqual( + [PackageID(v1='A', v2='a'), PackageID(v1='B', v2='b')], + package_resolved.read_dependency_ids() + ) self.assertDictEqual( { 'package': 'A', 'repositoryURL': 'https://github.com/A-org/a.git', 'state': {'branch': 'a-branch', 'revision': 'a-revision', 'version': None} }, - package_resolved.read_dependency(package_name='A') + package_resolved.read_dependency(package_id=PackageID(v1='A', v2='a')) ) self.assertDictEqual( { @@ -91,7 +92,7 @@ def test_it_reads_version1_files(self): 'repositoryURL': 'https://github.com/B-org/b.git', 'state': {'branch': None, 'revision': 'b-revision', 'version': '1.0.0'} }, - package_resolved.read_dependency(package_name='B') + package_resolved.read_dependency(package_id=PackageID(v1='B', v2='b')) ) def test_it_changes_version1_files(self): @@ -101,14 +102,15 @@ def test_it_changes_version1_files(self): package_resolved = PackageResolvedFile(path=file.name) package_resolved.update_dependency( - package_name='B', new_branch='b-branch-new', new_revision='b-revision-new', new_version=None + package_id=PackageID(v1='B', v2='b'), + new_branch='b-branch-new', new_revision='b-revision-new', new_version=None ) package_resolved.add_dependency( - package_name='C', repository_url='https://github.com/C-org/c.git', + package_id=PackageID(v1='C', v2='c'), repository_url='https://github.com/C-org/c.git', branch='c-branch', revision='c-revision', version=None ) package_resolved.add_dependency( - package_name='D', repository_url='https://github.com/D-org/d.git', + package_id=PackageID(v1='D', v2='d'), repository_url='https://github.com/D-org/d.git', branch=None, revision='d-revision', version='1.1.0' ) package_resolved.save() @@ -166,10 +168,13 @@ def test_it_reads_version2_files(self): file.seek(0) package_resolved = PackageResolvedFile(path=file.name) - self.assertTrue(package_resolved.has_dependency(package_name='a')) - self.assertTrue(package_resolved.has_dependency(package_name='b')) - self.assertFalse(package_resolved.has_dependency(package_name='c')) - self.assertListEqual(['a', 'b'], package_resolved.read_dependency_names()) + self.assertTrue(package_resolved.has_dependency(package_id=PackageID(v1=None, v2='a'))) + self.assertTrue(package_resolved.has_dependency(package_id=PackageID(v1=None, v2='b'))) + self.assertFalse(package_resolved.has_dependency(package_id=PackageID(v1=None, v2='c'))) + self.assertListEqual( + [PackageID(v1=None, v2='a'), PackageID(v1=None, v2='b')], + package_resolved.read_dependency_ids() + ) self.assertDictEqual( { 'identity': 'a', @@ -177,7 +182,7 @@ def test_it_reads_version2_files(self): 'location': 'https://github.com/A-org/a', 'state': {'branch': 'a-branch', 'revision': 'a-revision'} }, - package_resolved.read_dependency(package_name='a') + package_resolved.read_dependency(package_id=PackageID(v1=None, v2='a')) ) self.assertDictEqual( { @@ -186,7 +191,7 @@ def test_it_reads_version2_files(self): 'location': 'https://github.com/B-org/b.git', 'state': {'revision': 'b-revision', 'version': '1.0.0'} }, - package_resolved.read_dependency(package_name='b') + package_resolved.read_dependency(PackageID(v1=None, v2='b')) ) def test_it_changes_version2_files(self): @@ -196,61 +201,66 @@ def test_it_changes_version2_files(self): package_resolved = PackageResolvedFile(path=file.name) package_resolved.update_dependency( - package_name='b', new_branch='b-branch-new', new_revision='b-revision-new', new_version=None + package_id=PackageID(v1=None, v2='b'), new_branch='b-branch-new', + new_revision='b-revision-new', new_version=None ) package_resolved.add_dependency( - package_name='c', repository_url='https://github.com/C-org/c.git', + package_id=PackageID(v1=None, v2='c'), repository_url='https://github.com/C-org/c.git', branch='c-branch', revision='c-revision', version=None ) package_resolved.add_dependency( - package_name='d', repository_url='https://github.com/D-org/d.git', + package_id=PackageID(v1=None, v2='d'), repository_url='https://github.com/D-org/d.git', branch=None, revision='d-revision', version='1.1.0' ) package_resolved.save() actual_new_content = file.read().decode('utf-8') expected_new_content = '''{ - "object": { - "pins": [ - { - "identity": "a", - "kind": "remoteSourceControl", - "location": "https://github.com/A-org/a", - "state": { - "branch": "a-branch", - "revision": "a-revision" - } - }, - { - "identity": "b", - "kind": "remoteSourceControl", - "location": "https://github.com/B-org/b.git", - "state": { - "branch": "b-branch-new", - "revision": "b-revision-new" - } - }, - { - "identity": "c", - "kind": "remoteSourceControl", - "location": "https://github.com/C-org/c.git", - "state": { - "branch": "c-branch", - "revision": "c-revision" - } - }, - { - "identity": "d", - "kind": "remoteSourceControl", - "location": "https://github.com/D-org/d.git", - "state": { - "revision": "d-revision", - "version": "1.1.0" - } + "pins": [ + { + "identity": "a", + "kind": "remoteSourceControl", + "location": "https://github.com/A-org/a", + "state": { + "branch": "a-branch", + "revision": "a-revision" } - ] - }, + }, + { + "identity": "b", + "kind": "remoteSourceControl", + "location": "https://github.com/B-org/b.git", + "state": { + "branch": "b-branch-new", + "revision": "b-revision-new" + } + }, + { + "identity": "c", + "kind": "remoteSourceControl", + "location": "https://github.com/C-org/c.git", + "state": { + "branch": "c-branch", + "revision": "c-revision" + } + }, + { + "identity": "d", + "kind": "remoteSourceControl", + "location": "https://github.com/D-org/d.git", + "state": { + "revision": "d-revision", + "version": "1.1.0" + } + } + ], "version": 2 } ''' self.assertEqual(expected_new_content, actual_new_content) + + def test_v2_package_id_from_repository_url(self): + self.assertEqual('abc', v2_package_id_from_repository_url(repository_url='https://github.com/A-org/abc.git')) + self.assertEqual('abc', v2_package_id_from_repository_url(repository_url='https://github.com/A-org/abc')) + self.assertEqual('abc', v2_package_id_from_repository_url(repository_url='git@github.com:DataDog/abc.git')) + self.assertEqual('abc', v2_package_id_from_repository_url(repository_url='git@github.com:DataDog/abc')) From 21d7584a7fd6934606819889ccd29e58e009e3bc Mon Sep 17 00:00:00 2001 From: Maciek Grzybowski Date: Tue, 5 Apr 2022 13:51:37 +0200 Subject: [PATCH 08/13] Fix json separators generated in v2's `package.resolved` --- .../src/dogfood/package_resolved.py | 9 +-- .../tests/dogfood/test_package_resolved.py | 56 +++++++++---------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/tools/distribution/src/dogfood/package_resolved.py b/tools/distribution/src/dogfood/package_resolved.py index f0784d5118..c034ab4aab 100644 --- a/tools/distribution/src/dogfood/package_resolved.py +++ b/tools/distribution/src/dogfood/package_resolved.py @@ -104,6 +104,7 @@ def save(self): self.packages, fp=file, indent=2, # preserve `swift package` indentation + separators=(',', ': ' if self.version == 1 else ' : '), # v1: `"key": "value"`, v2: `"key" : "value"` sort_keys=True # preserve `swift package` packages sorting ) file.write('\n') # add new line to the EOF @@ -235,19 +236,19 @@ class PackageResolvedContentV2(PackageResolvedContent): Example of `package.resolved` in version `2` looks this:: { - "pins": [ + "pins" : [ { "identity" : "dd-sdk-ios", "kind" : "remoteSourceControl", "location" : "https://github.com/DataDog/dd-sdk-ios", - "state": { - "branch": "dogfooding", + "state" : { + "branch" : "dogfooding", "revision" : "6f662103771eb4523164e64f7f936bf9276f6bd0" } }, ... ] - "version": 2 + "version" : 2 } In v2 `branch` and `version` are mutually exclusive: if one is set, the other diff --git a/tools/distribution/tests/dogfood/test_package_resolved.py b/tools/distribution/tests/dogfood/test_package_resolved.py index d6620ef06e..39835e51c6 100644 --- a/tools/distribution/tests/dogfood/test_package_resolved.py +++ b/tools/distribution/tests/dogfood/test_package_resolved.py @@ -41,7 +41,7 @@ class PackageResolvedFileTestCase(unittest.TestCase): v2_file_content = b''' { - "pins": [ + "pins" : [ { "identity" : "a", "kind" : "remoteSourceControl", @@ -61,7 +61,7 @@ class PackageResolvedFileTestCase(unittest.TestCase): } } ], - "version": 2 + "version" : 2 } ''' @@ -216,45 +216,45 @@ def test_it_changes_version2_files(self): actual_new_content = file.read().decode('utf-8') expected_new_content = '''{ - "pins": [ + "pins" : [ { - "identity": "a", - "kind": "remoteSourceControl", - "location": "https://github.com/A-org/a", - "state": { - "branch": "a-branch", - "revision": "a-revision" + "identity" : "a", + "kind" : "remoteSourceControl", + "location" : "https://github.com/A-org/a", + "state" : { + "branch" : "a-branch", + "revision" : "a-revision" } }, { - "identity": "b", - "kind": "remoteSourceControl", - "location": "https://github.com/B-org/b.git", - "state": { - "branch": "b-branch-new", - "revision": "b-revision-new" + "identity" : "b", + "kind" : "remoteSourceControl", + "location" : "https://github.com/B-org/b.git", + "state" : { + "branch" : "b-branch-new", + "revision" : "b-revision-new" } }, { - "identity": "c", - "kind": "remoteSourceControl", - "location": "https://github.com/C-org/c.git", - "state": { - "branch": "c-branch", - "revision": "c-revision" + "identity" : "c", + "kind" : "remoteSourceControl", + "location" : "https://github.com/C-org/c.git", + "state" : { + "branch" : "c-branch", + "revision" : "c-revision" } }, { - "identity": "d", - "kind": "remoteSourceControl", - "location": "https://github.com/D-org/d.git", - "state": { - "revision": "d-revision", - "version": "1.1.0" + "identity" : "d", + "kind" : "remoteSourceControl", + "location" : "https://github.com/D-org/d.git", + "state" : { + "revision" : "d-revision", + "version" : "1.1.0" } } ], - "version": 2 + "version" : 2 } ''' self.assertEqual(expected_new_content, actual_new_content) From 040bf39127d6dfa322d3c69c1ba05d9cb898ed63 Mon Sep 17 00:00:00 2001 From: Maxime Epain Date: Mon, 4 Apr 2022 16:13:44 +0200 Subject: [PATCH 09/13] RUMM-1870 Improve documentation --- .../Datadog/Core/Persistence/DataEncryption.swift | 8 ++++---- .../Core/Persistence/Reading/FileReader.swift | 13 +++++++++++-- .../Core/Persistence/Writing/FileWriter.swift | 8 +++++++- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/Sources/Datadog/Core/Persistence/DataEncryption.swift b/Sources/Datadog/Core/Persistence/DataEncryption.swift index 59e4d16541..40b8fa4e0c 100644 --- a/Sources/Datadog/Core/Persistence/DataEncryption.swift +++ b/Sources/Datadog/Core/Persistence/DataEncryption.swift @@ -18,10 +18,10 @@ public protocol DataEncryption { /// Decrypts given `Data` with user-chosen encryption. /// - /// Beware that data to decrypt could be encrypted in a previous - /// app launch, so implementation should be aware of the case when decryption could - /// fail (for example, key used for encryption is different from key used for decryption, if - /// they are unique for every app launch). + /// Beware that data to decrypt could be encrypted in a previous app launch, so + /// implementation should be aware of the case when decryption could fail (for example, + /// key used for encryption is different from key used for decryption, if they are unique + /// for every app launch). /// /// - Parameter data: Data to decrypt. /// - Returns: The decrypted data. diff --git a/Sources/Datadog/Core/Persistence/Reading/FileReader.swift b/Sources/Datadog/Core/Persistence/Reading/FileReader.swift index b8f47363db..bb3bdddbc6 100644 --- a/Sources/Datadog/Core/Persistence/Reading/FileReader.swift +++ b/Sources/Datadog/Core/Persistence/Reading/FileReader.swift @@ -38,7 +38,7 @@ internal final class FileReader: Reader { } do { - let fileData = try decode(data: file.read()) + let fileData = try decrypt(data: file.read()) let batchData = dataFormat.prefixData + fileData + dataFormat.suffixData return Batch(data: batchData, file: file) } catch { @@ -47,7 +47,16 @@ internal final class FileReader: Reader { } } - func decode(data: Data) -> Data { + /// Decrypts data if encryption is available. + /// + /// When encryption is provided, the data is splitted using data-format separator, each slices + /// is then decoded from base64 and decrypted. Data is finally re-joined with data-format separator. + /// + /// If no encryption, the data is returned. + /// + /// - Parameter data: The data to decrypt. + /// - Returns: Decrypted data. + private func decrypt(data: Data) -> Data { guard let encryption = encryption else { return data } diff --git a/Sources/Datadog/Core/Persistence/Writing/FileWriter.swift b/Sources/Datadog/Core/Persistence/Writing/FileWriter.swift index e314c980a3..3d374b6d1e 100644 --- a/Sources/Datadog/Core/Persistence/Writing/FileWriter.swift +++ b/Sources/Datadog/Core/Persistence/Writing/FileWriter.swift @@ -49,7 +49,13 @@ internal final class FileWriter: Writer { } } - func encode(value: T) throws -> Data { + /// Encodes the given encodable value and encrypt it if encryption is available. + /// + /// If encryption is available, encryption result is base64 encoded. + /// + /// - Parameter value: The value to encode. + /// - Returns: Data representation of the value. + private func encode(value: T) throws -> Data { let data = try jsonEncoder.encode(value) guard let encryption = encryption else { From 8851d46b81e30751fbc11df1c836fad59017825a Mon Sep 17 00:00:00 2001 From: Maxime Epain Date: Mon, 4 Apr 2022 16:42:56 +0200 Subject: [PATCH 10/13] RUMM-1870 Report decryption failure --- .../Core/Persistence/Reading/FileReader.swift | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/Sources/Datadog/Core/Persistence/Reading/FileReader.swift b/Sources/Datadog/Core/Persistence/Reading/FileReader.swift index bb3bdddbc6..421dd4adc8 100644 --- a/Sources/Datadog/Core/Persistence/Reading/FileReader.swift +++ b/Sources/Datadog/Core/Persistence/Reading/FileReader.swift @@ -61,13 +61,32 @@ internal final class FileReader: Reader { return data } + var failure: String? = nil + defer { + failure.map { userLogger.error($0) } + } + return data // split data .split(separator: dataFormat.separatorByte) - // decode base64 - allow failure - .compactMap { Data(base64Encoded: $0) } - // decrypt data - allow failure - .compactMap { try? encryption.decrypt(data: $0) } + // decode base64 - report failure + .compactMap { + if let data = Data(base64Encoded: $0) { + return data + } + + failure = "🔥 Failed to decode base64 data before decryption" + return nil + } + // decrypt data - report failure + .compactMap { (data: Data) in + do { + return try encryption.decrypt(data: data) + } catch { + failure = "🔥 Failed to decrypt data with error: \(error)" + return nil + } + } // concat data .reduce(Data()) { $0 + $1 + [dataFormat.separatorByte] } // drop last separator From a42c76dfd1d3adc8545c944cec0ae2e915c5753c Mon Sep 17 00:00:00 2001 From: Ignacio Bonafonte Date: Tue, 5 Apr 2022 17:36:19 +0200 Subject: [PATCH 11/13] Fix CI Visiblity testing --- .../xcshareddata/xcschemes/Datadog tvOS.xcscheme | 9 +++++++++ .../xcschemes/DatadogCrashReporting tvOS.xcscheme | 9 +++++++++ .../xcshareddata/xcschemes/DatadogObjc tvOS.xcscheme | 9 +++++++++ Makefile | 6 ++++-- 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/Datadog/Datadog.xcodeproj/xcshareddata/xcschemes/Datadog tvOS.xcscheme b/Datadog/Datadog.xcodeproj/xcshareddata/xcschemes/Datadog tvOS.xcscheme index 0e7e7d0541..f468d7a520 100644 --- a/Datadog/Datadog.xcodeproj/xcshareddata/xcschemes/Datadog tvOS.xcscheme +++ b/Datadog/Datadog.xcodeproj/xcshareddata/xcschemes/Datadog tvOS.xcscheme @@ -30,6 +30,15 @@ enableThreadSanitizer = "YES" codeCoverageEnabled = "YES" onlyGenerateCoverageForSpecifiedTargets = "YES"> + + + + + + + + + + + + Date: Tue, 5 Apr 2022 18:32:49 +0200 Subject: [PATCH 12/13] Fix env variables in makefile --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 1b2e5e36c2..0ea3ae1265 100644 --- a/Makefile +++ b/Makefile @@ -4,10 +4,10 @@ all: dependencies xcodeproj-httpservermock templates DD_SDK_SWIFT_TESTING_VERSION = 1.1.1 define DD_SDK_TESTING_XCCONFIG_CI -FRAMEWORK_SEARCH_PATHS[sdk=iphonesimulator*]=$(inherited) $(SRCROOT)/../instrumented-tests/DatadogSDKTesting.xcframework/ios-arm64_x86_64-simulator/\n -LD_RUNPATH_SEARCH_PATHS[sdk=iphonesimulator*]=$(inherited) $(SRCROOT)/../instrumented-tests/DatadogSDKTesting.xcframework/ios-arm64_x86_64-simulator/\n -FRAMEWORK_SEARCH_PATHS[sdk=appletvsimulator*]==$(inherited) $(SRCROOT)/../instrumented-tests/DatadogSDKTesting.xcframework/tvos-arm64_x86_64-simulator/\n -LD_RUNPATH_SEARCH_PATHS[sdk=appletvsimulator*]=$(inherited) $(SRCROOT)/../instrumented-tests/DatadogSDKTesting.xcframework/tvos-arm64_x86_64-simulator/\n +FRAMEWORK_SEARCH_PATHS[sdk=iphonesimulator*]=$$(inherited) $$(SRCROOT)/../instrumented-tests/DatadogSDKTesting.xcframework/ios-arm64_x86_64-simulator/\n +LD_RUNPATH_SEARCH_PATHS[sdk=iphonesimulator*]=$$(inherited) $$(SRCROOT)/../instrumented-tests/DatadogSDKTesting.xcframework/ios-arm64_x86_64-simulator/\n +FRAMEWORK_SEARCH_PATHS[sdk=appletvsimulator*]==$$(inherited) $$(SRCROOT)/../instrumented-tests/DatadogSDKTesting.xcframework/tvos-arm64_x86_64-simulator/\n +LD_RUNPATH_SEARCH_PATHS[sdk=appletvsimulator*]=$$(inherited) $$(SRCROOT)/../instrumented-tests/DatadogSDKTesting.xcframework/tvos-arm64_x86_64-simulator/\n OTHER_LDFLAGS=$$(inherited) -framework DatadogSDKTesting\n DD_TEST_RUNNER=1\n DD_SDK_SWIFT_TESTING_SERVICE=dd-sdk-ios\n From 3d746ed1582bf06f486ba09f42d677ed49bedcfe Mon Sep 17 00:00:00 2001 From: Maxime Epain Date: Tue, 5 Apr 2022 16:03:03 +0200 Subject: [PATCH 13/13] RUMM-2079 `time_spent` can't be lower than 1ns --- Sources/Datadog/RUM/RUMMonitor/Scopes/RUMViewScope.swift | 4 ++-- .../Datadog/RUM/RUMMonitor/Scopes/RUMViewScopeTests.swift | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Datadog/RUM/RUMMonitor/Scopes/RUMViewScope.swift b/Sources/Datadog/RUM/RUMMonitor/Scopes/RUMViewScope.swift index 93680472a8..224a37a569 100644 --- a/Sources/Datadog/RUM/RUMMonitor/Scopes/RUMViewScope.swift +++ b/Sources/Datadog/RUM/RUMMonitor/Scopes/RUMViewScope.swift @@ -368,8 +368,8 @@ internal class RUMViewScope: RUMScope, RUMContextProvider { // RUMM-1779 Keep view active as long as we have ongoing resources let isActive = isActiveView || !resourceScopes.isEmpty - - let timeSpent = command.time.timeIntervalSince(viewStartTime) + // RUMM-2079 `time_spent` can't be lower than 1ns + let timeSpent = max(1e-9, command.time.timeIntervalSince(viewStartTime)) let cpuInfo = vitalInfoSampler.cpu let memoryInfo = vitalInfoSampler.memory let refreshRateInfo = vitalInfoSampler.refreshRate diff --git a/Tests/DatadogTests/Datadog/RUM/RUMMonitor/Scopes/RUMViewScopeTests.swift b/Tests/DatadogTests/Datadog/RUM/RUMMonitor/Scopes/RUMViewScopeTests.swift index 8834c3f72e..2a615efd15 100644 --- a/Tests/DatadogTests/Datadog/RUM/RUMMonitor/Scopes/RUMViewScopeTests.swift +++ b/Tests/DatadogTests/Datadog/RUM/RUMMonitor/Scopes/RUMViewScopeTests.swift @@ -154,7 +154,7 @@ class RUMViewScopeTests: XCTestCase { XCTAssertEqual(event.view.name, "ViewName") let viewIsActive = try XCTUnwrap(event.view.isActive) XCTAssertTrue(viewIsActive) - XCTAssertEqual(event.view.timeSpent, 0) + XCTAssertEqual(event.view.timeSpent, 1) // Minimum `time_spent of 1 nanosecond XCTAssertEqual(event.view.action.count, 1, "The initial view update must have come with `application_start` action sent.") XCTAssertEqual(event.view.error.count, 0) XCTAssertEqual(event.view.resource.count, 0) @@ -195,7 +195,7 @@ class RUMViewScopeTests: XCTestCase { XCTAssertEqual(event.view.name, "ViewName") let viewIsActive = try XCTUnwrap(event.view.isActive) XCTAssertTrue(viewIsActive) - XCTAssertEqual(event.view.timeSpent, 0) + XCTAssertEqual(event.view.timeSpent, 1) // Minimum `time_spent of 1 nanosecond XCTAssertEqual(event.view.action.count, isInitialView ? 1 : 0, "It must track application start action only if this is an initial view") XCTAssertEqual(event.view.error.count, 0) XCTAssertEqual(event.view.resource.count, 0)