Skip to content

Commit

Permalink
fix: Sort private attributes for improved stable encoding (#401)
Browse files Browse the repository at this point in the history
To help increase cache hits, we want to always use a stable encoding for
the context. This was done in a previous commit when we added the
`encoder.outputFormatting = [.sortedKeys]` modification.

A context's private attributes are sorted as a set, which makes no
guarantee about encoding order. To address this, we are sorting the
private attributes prior to the JSON encoding phase. As a result, we
would expect to see an increase in cache hits for customers.
  • Loading branch information
keelerm84 authored Aug 5, 2024
1 parent dc3ebd6 commit 90bf896
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 1 deletion.
3 changes: 2 additions & 1 deletion LaunchDarkly/LaunchDarkly/Models/Context/LDContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ public struct LDContext: Encodable, Equatable {
let includePrivateAttributes = encoder.userInfo[UserInfoKeys.includePrivateAttributes] as? Bool ?? false

if let privateAttributes = privateAttributes, !privateAttributes.isEmpty, includePrivateAttributes {
try container.encodeIfPresent(privateAttributes, forKey: .privateAttributes)
let sorted = privateAttributes.sorted { $0.raw() < $1.raw() }
try container.encodeIfPresent(sorted, forKey: .privateAttributes)
}

if let redactedAttributes = redactedAttributes, !redactedAttributes.isEmpty {
Expand Down
55 changes: 55 additions & 0 deletions LaunchDarkly/LaunchDarklyTests/Models/Context/LDContextSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -386,11 +386,63 @@ final class LDContextSpec: XCTestCase {
}
}

func testAttributeOrderDefinitionDoesNotAffectJsonEncoding() {
var builder1 = LDContextBuilder(key: "context-key")
builder1.kind("org")
builder1.trySetValue("example-attribute", LDValue(stringLiteral: "Hi there"))
builder1.name("Example name")
builder1.addPrivateAttribute(Reference.init(literal: "first"))
builder1.addPrivateAttribute(Reference.init(literal: "second"))
builder1.addPrivateAttribute(Reference.init(literal: "third"))

guard case .success(let context1) = builder1.build()
else {
XCTFail("builder1.build should not have failed")
return
}

var builder2 = LDContextBuilder(key: "context-key")
builder2.trySetValue("example-attribute", LDValue(stringLiteral: "Hi there"))
builder2.name("Example name")
builder2.kind("org")
builder2.addPrivateAttribute(Reference.init(literal: "second"))
builder2.addPrivateAttribute(Reference.init(literal: "third"))
builder2.addPrivateAttribute(Reference.init(literal: "first"))

guard case .success(let context2) = builder2.build()
else {
XCTFail("builder1.build should not have failed")
return
}

let encoder = JSONEncoder()
encoder.userInfo[LDContext.UserInfoKeys.includePrivateAttributes] = true
encoder.userInfo[LDContext.UserInfoKeys.redactAttributes] = false
encoder.outputFormatting = [.sortedKeys]

guard let context1JsonData = try? encoder.encode(context1)
else {
XCTFail("failed to encode context1")
return
}

guard let context2JsonData = try? encoder.encode(context2)
else {
XCTFail("failed to encode context2")
return
}

XCTAssertEqual(context1JsonData, context2JsonData)
}

func testAttributeOrderDefinitionDoesNotAffectContextHash() {
var builder1 = LDContextBuilder(key: "context-key")
builder1.kind("org")
builder1.trySetValue("example-attribute", LDValue(stringLiteral: "Hi there"))
builder1.name("Example name")
builder1.addPrivateAttribute(Reference.init(literal: "first"))
builder1.addPrivateAttribute(Reference.init(literal: "second"))
builder1.addPrivateAttribute(Reference.init(literal: "third"))

guard case .success(let context1) = builder1.build()
else {
Expand All @@ -402,6 +454,9 @@ final class LDContextSpec: XCTestCase {
builder2.trySetValue("example-attribute", LDValue(stringLiteral: "Hi there"))
builder2.name("Example name")
builder2.kind("org")
builder2.addPrivateAttribute(Reference.init(literal: "second"))
builder2.addPrivateAttribute(Reference.init(literal: "third"))
builder2.addPrivateAttribute(Reference.init(literal: "first"))

guard case .success(let context2) = builder2.build()
else {
Expand Down

0 comments on commit 90bf896

Please sign in to comment.