diff --git a/mParticle-Apple-SDK-Swift/Sources/DataModel/MPCookie.swift b/mParticle-Apple-SDK-Swift/Sources/DataModel/MPCookie.swift index db34e78d0..67b63fa9e 100644 --- a/mParticle-Apple-SDK-Swift/Sources/DataModel/MPCookie.swift +++ b/mParticle-Apple-SDK-Swift/Sources/DataModel/MPCookie.swift @@ -96,32 +96,28 @@ public final class MPCookiePRIVATE: NSObject, NSSecureCoding { if let expiration { coder.encode(expiration, forKey: CodingKeys.expiration) } } - /// Ported verbatim from the deleted Objective-C wrapper, including a defect: `content`, - /// `domain` and `expiration` are encoded as strings but decoded with `NSDictionary` as the - /// expected class, so they never survive a round trip and a restored cookie carries only its - /// name. + /// `content`, `domain` and `expiration` are encoded as strings, so they are decoded as strings. /// - /// The production persistence path does not go through here — cookies are written as raw - /// sqlite columns, and the only production archives are `MPUploadSettings` and a - /// configuration dictionary. Cookies *are* archivable, though: `MPConsumerInfo` conforms to - /// `NSSecureCoding` and encodes its `cookies` array, and `MPConsumerInfoTests.testInstance` - /// and `testConsumerInfoEncoding` exercise exactly that. Neither asserts the cookie fields, - /// and `isEqual(toCookie:)` compares names only, which is why the loss goes unnoticed there - /// and in `testCookie`. + /// The deleted Objective-C wrapper passed `NSDictionary` as the expected class for all three, + /// which never matches an `NSString`, so every restored cookie carried only its name. /// - /// Left as-is deliberately: repairing it would change what a decoded cookie contains, which is - /// a behaviour change rather than a migration. Tracked as a follow-up. + /// It went unnoticed because no production path archives a cookie — cookies are written as raw + /// sqlite columns, and the only production archives are `MPUploadSettings` and a configuration + /// dictionary. Cookies are archivable in principle: `MPConsumerInfo` conforms to + /// `NSSecureCoding` and encodes its `cookies` array, and `MPConsumerInfoTests.testInstance` + /// and `testConsumerInfoEncoding` do exercise that. Neither asserts the cookie fields, and + /// `isEqual(toCookie:)` compares names only, so the round-trip assertions passed over the loss. public convenience init?(coder: NSCoder) { let name = coder.decodeObject(of: NSString.self, forKey: CodingKeys.name) as String? let configuration = NSMutableDictionary() - if let content = coder.decodeObject(of: NSDictionary.self, forKey: CodingKeys.content) { + if let content = coder.decodeObject(of: NSString.self, forKey: CodingKeys.content) { configuration[Keys.content] = content } - if let domain = coder.decodeObject(of: NSDictionary.self, forKey: CodingKeys.domain) { + if let domain = coder.decodeObject(of: NSString.self, forKey: CodingKeys.domain) { configuration[Keys.domain] = domain } - if let expiration = coder.decodeObject(of: NSDictionary.self, forKey: CodingKeys.expiration) { + if let expiration = coder.decodeObject(of: NSString.self, forKey: CodingKeys.expiration) { configuration[Keys.expiration] = expiration } diff --git a/mParticle-Apple-SDK-Swift/Test/Utils/MPDataModelTests.swift b/mParticle-Apple-SDK-Swift/Test/Utils/MPDataModelTests.swift index fd2af1980..52556f46a 100644 --- a/mParticle-Apple-SDK-Swift/Test/Utils/MPDataModelTests.swift +++ b/mParticle-Apple-SDK-Swift/Test/Utils/MPDataModelTests.swift @@ -252,12 +252,9 @@ final class MPDataModelTests: XCTestCase { XCTAssertNil(cookie?.dictionaryRepresentation()) } - /// Pins the ported defect rather than endorsing it: `content`, `domain` and `expiration` are - /// encoded as strings but decoded expecting `NSDictionary`, so only the name survives. - /// No production path archives a cookie, and equality is by name, so the loss is invisible - /// even to `MPConsumerInfoTests`, which does archive cookies through `MPConsumerInfo` but - /// asserts only `uniqueIdentifier`. Change this test the day the decode is repaired. - func testCookieArchiveRoundTripKeepsOnlyTheName() throws { + /// The wrapper decoded these three expecting `NSDictionary`, which never matches the `NSString` + /// that was encoded, so a restored cookie used to carry only its name. + func testCookieArchiveRoundTripPreservesEveryField() throws { let cookie = try XCTUnwrap(MPCookiePRIVATE(name: "uid", configuration: ["c": "g=abc", "d": "example.com", "e": "2035-05-26T22:43:31.505262Z"])) @@ -266,9 +263,24 @@ final class MPDataModelTests: XCTestCase { let restored = try XCTUnwrap(NSKeyedUnarchiver.unarchivedObject(ofClass: MPCookiePRIVATE.self, from: data)) XCTAssertEqual(restored.name, "uid") - XCTAssertEqual(restored, cookie, "equality is by name, which is why the loss below is invisible") + XCTAssertEqual(restored.content, cookie.content) + XCTAssertEqual(restored.domain, cookie.domain) + XCTAssertEqual(restored.expiration, cookie.expiration) + XCTAssertFalse(restored.expired, "a restored future expiration must still read as unexpired") + } + + /// A cookie that never had the optional fields still restores, and stays "expired" because an + /// absent expiration means expired. + func testCookieArchiveRoundTripWithOnlyANameStillRestores() throws { + let cookie = try XCTUnwrap(MPCookiePRIVATE(name: "uid", configuration: [:])) + + let data = try NSKeyedArchiver.archivedData(withRootObject: cookie, requiringSecureCoding: false) + let restored = try XCTUnwrap(NSKeyedUnarchiver.unarchivedObject(ofClass: MPCookiePRIVATE.self, from: data)) + + XCTAssertEqual(restored.name, "uid") XCTAssertNil(restored.content) XCTAssertNil(restored.domain) XCTAssertNil(restored.expiration) + XCTAssertTrue(restored.expired) } }