Conversation
…ed flags - Send subjectAttributes as nested ContextAttributes with numericAttributes and categoricalAttributes separated - Use salt directly as string for MD5 hashing instead of base64 decoding - Update tests to use salt directly
- Add PrecomputedConfigurationWireTests using shared sdk-test-data - Add ContextAttributes unit tests to PrecomputedRequestorTests
leoromanovsky
left a comment
leoromanovsky
left a comment
There was a problem hiding this comment.
Cool thanks for the follow up.
|
|
||
| private func loadPrecomputedConfig() throws -> PrecomputedConfiguration { | ||
| let fileURL = Bundle.module.url( | ||
| forResource: "Resources/test-data/configuration-wire/precomputed-v1.json", |
There was a problem hiding this comment.
Good idea testing against this 👍
| for (key, value) in attributes { | ||
| if value.isNumeric() { | ||
| numeric[key] = value | ||
| } else if !value.isNull() { | ||
| categorical[key] = value | ||
| } | ||
| } |
There was a problem hiding this comment.
Intended to match deduceAttributeContext in js-sdk-common
| /// Tests the precomputed configuration wire format using shared test data from sdk-test-data. | ||
| class PrecomputedConfigurationWireTests: XCTestCase { | ||
|
|
||
| func testSaltUsedDirectlyForFlagKeyHashing() throws { |
There was a problem hiding this comment.
The test I should have had using the shared test data to verify that we are using the salt as-is
| XCTAssertTrue(decodingError.errorDescription?.contains("Failed to decode response") ?? false) | ||
| } | ||
|
|
||
| // MARK: - ContextAttributes Tests |
There was a problem hiding this comment.
I believe we have to do this with isolated unit tests instead of using the shared test data (which would typically be preferable) because there are cases where the SDKs cannot know if a numerical was meant to be categorical, e.g. "buildNumber"
| // todo: make this a build argument (FF-1944) | ||
| public let sdkName = "ios" | ||
| public let sdkVersion = "5.3.1" | ||
| public let sdkVersion = "5.3.2" |
There was a problem hiding this comment.
No change to the public API, so I'm going with a patch bump here
sameerank
deleted the
sameerank/FFESUPPORT-501-subject-attributes-and-salt-issues
branch
2 participants
🎟️ Fixes FFESUPPORT-501
Motivation and Context
EppoPrecomputedClient.initialize()was failing in production due to two issues:subjectAttributesin a nested format withnumericAttributesandcategoricalAttributesseparated, but the iOS SDK was sending a flat dictionaryDescription
ContextAttributesstruct that separates attributes intonumericAttributes(integers/doubles) andcategoricalAttributes(strings/booleans), matching the JS SDK implementationPrecomputedConfiguration.decode()to use salt directly without base64 decodingContextAttributesattribute separation logicHow has this been documented?
No documentation required - external API unchanged.
How has this been tested?
PrecomputedConfigurationWireTeststhat validates salt handling and flag assignments using shared test data fromsdk-test-data/configuration-wire/precomputed-v1.jsonContextAttributesunit tests inPrecomputedRequestorTeststo verify numeric/categorical attribute separation