diff --git a/.github/workflows/swift.yml b/.github/workflows/swift.yml index 715f75cd..6324de4e 100644 --- a/.github/workflows/swift.yml +++ b/.github/workflows/swift.yml @@ -22,11 +22,11 @@ env: jobs: lint_markdown_files: - uses: optimizely/swift-sdk/.github/workflows/lint_markdown.yml@fix-release-process + uses: optimizely/swift-sdk/.github/workflows/lint_markdown.yml@master integration_tests: if: "${{ github.event.inputs.PREP == '' && github.event.inputs.RELEASE == '' }}" - uses: optimizely/swift-sdk/.github/workflows/integration_tests.yml@fix-release-process + uses: optimizely/swift-sdk/.github/workflows/integration_tests.yml@master secrets: CI_USER_TOKEN: ${{ secrets.CI_USER_TOKEN }} @@ -46,7 +46,7 @@ jobs: unittests: if: "${{ github.event.inputs.PREP == '' && github.event.inputs.RELEASE == '' }}" - uses: optimizely/swift-sdk/.github/workflows/unit_tests.yml@fix-release-process + uses: optimizely/swift-sdk/.github/workflows/unit_tests.yml@master prepare_for_release: runs-on: macos-15 if: "${{ github.event.inputs.PREP == 'true' && github.event_name == 'workflow_dispatch' }}" diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..310bfdca --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,335 @@ +# Optimizely Swift SDK - Claude Code Context + +## Project Overview +This is the Optimizely Swift SDK for Feature Experimentation and Full Stack. It provides A/B testing and feature management capabilities for iOS, tvOS, and watchOS platforms. + +## Getting Started + +### Platform Support +- iOS 10.0+ +- tvOS 10.0+ +- watchOS 3.0+ +- Swift 5+ + +### Installation Methods +- Swift Package Manager (preferred) +- CocoaPods + +### Dependencies +- SwiftLint (development) + +### Initial Setup +```bash +# Install dependencies +pod install + +# Build the SDK +swift build + +# Verify setup (see "Testing" section for detailed commands) +swift test +``` + +## Project Structure + +### Source Code Organization + +#### Core Modules +- **Sources/Optimizely/**: Main SDK entry point and client implementation + - `OptimizelyClient.swift`: Primary SDK interface + - `OptimizelyConfig.swift`: Configuration management + - `VuidManager.swift`: Visitor unique ID management + +- **Sources/Optimizely+Decide/**: Decision-making and user context + - `OptimizelyUserContext.swift`: User context for decision-making + - `OptimizelyDecision.swift`: Decision results + - `OptimizelyDecideOption.swift`: Decision options and flags + +- **Sources/Data Model/**: Data structures for experiments, features, and events + - Core entities: Experiment, FeatureFlag, Variation, Event, Audience + - CMAB (Contextual Multi-Armed Bandit) models + - Holdout configurations + +- **Sources/Implementation/**: Core business logic + - `DefaultDecisionService.swift`: Decision-making engine + - `DefaultBucketer.swift`: User bucketing logic + - Event handling and batch processing + +- **Sources/CMAB/**: Contextual Multi-Armed Bandit implementation + - `CmabClient.swift`: Client for CMAB predictions + - `CmabConfig.swift`: Configuration for CMAB + - `CmabService.swift`: Service layer for CMAB operations + +- **Sources/ODP/**: Optimizely Data Platform integration + - Event and segment management + - API managers for ODP communication + +- **Sources/Customization/**: Extensibility points + - Protocol definitions for custom handlers + - Default implementations (logger, event dispatcher, datafile handler) + +- **Sources/Utils/**: Shared utilities + - Atomic properties and thread-safe collections + - Hashing (MurmurHash3) + - Network reachability + +#### Test Organization +- **Tests/OptimizelyTests-Common/**: Common utility and core functionality tests +- **Tests/OptimizelyTests-APIs/**: Public API tests +- **Tests/OptimizelyTests-Batch/**: Event batching and dispatching tests +- **Tests/OptimizelyTests-DataModel/**: Data model tests +- **Tests/TestData/**: JSON fixture files for test data +- Test naming convention: `{FeatureName}Tests.swift` +- Test data fixtures: Predefined JSON files with sample configurations + +## Coding Standards + +### Style Guide +We follow the [Ray Wenderlich Swift Style Guide](https://github.com/raywenderlich/swift-style-guide) for readability and consistency. + +### Linting +SwiftLint is enforced. Before committing: +```bash +swiftlint +``` +Fix all warnings and errors. Configuration in `.swiftlint.yml`. + +### Common Patterns + +#### Protocol-Oriented Design +The SDK uses protocols for extensibility: +- `OPTLogger`: Custom logging +- `OPTEventDispatcher`: Custom event dispatching +- `OPTDatafileHandler`: Custom datafile management +- `OPTUserProfileService`: Custom user profile persistence + +#### Thread Safety +- Use `AtomicProperty`, `AtomicArray`, `AtomicDictionary` for thread-safe state +- All atomic utilities are located in `Sources/Utils/` +- Ensure event dispatchers and managers are thread-safe + +#### Error Handling +- Use `OptimizelyError` enum for SDK-specific errors +- Use `OptimizelyResult` for result types +- Handle errors gracefully with meaningful messages + +#### Logging +- Use `ThreadSafeLogger` or custom logger implementing `OPTLogger` +- Log levels: debug, info, warning, error +- Use appropriate log levels for different message types + +## Testing + +### Running Tests + +#### Using Swift Package Manager +```bash +# Run all tests +swift test + +# Run with verbose output +swift test --verbose +``` + +#### Using Xcode +```bash +# Run all tests for iOS +xcodebuild test \ + -workspace OptimizelySwiftSDK.xcworkspace \ + -scheme OptimizelySwiftSDK-iOS \ + -destination 'platform=iOS Simulator,name=iPhone 16' + +# Run a specific test target +xcodebuild test \ + -workspace OptimizelySwiftSDK.xcworkspace \ + -scheme OptimizelySwiftSDK-iOS \ + -destination 'platform=iOS Simulator,name=iPhone 16' \ + -only-testing:TestTarget/TestClass + +# Run a specific test method +xcodebuild test \ + -workspace OptimizelySwiftSDK.xcworkspace \ + -scheme OptimizelySwiftSDK-iOS \ + -destination 'platform=iOS Simulator,name=iPhone 16' \ + -only-testing:TestTarget/TestClass/testMethodName +``` + +### Test Targets +- `OptimizelyTests-Common-iOS`: Common utilities and core functionality +- `OptimizelyTests-APIs-iOS`: Public API tests +- `OptimizelyTests-Batch-iOS`: Event batching and dispatching +- `OptimizelyTests-DataModel-iOS`: Data models +- `OptimizelyTests-Legacy-iOS`: Legacy compatibility +- `OptimizelyTests-MultiClients-iOS`: Multi-client scenarios +- `OptimizelyTests-Others-iOS`: Miscellaneous tests +- `OptimizelyTests-iOS`: Main test suite + +Similar test targets exist for tvOS and other platforms. + +### Testing Best Practices +- All code must have test coverage +- Use XCTest framework +- Use `.sortedKeys` for JSONEncoder in tests to ensure deterministic JSON output +- Override network calls in test mocks to avoid timeouts +- Use JSON fixtures from `Tests/TestData/` for consistent test data +- Each test should use unique file names for persistent storage + +## Development Workflow + +### Branch Strategy +- Main branch: `master` +- Create feature branches: `YOUR_NAME/branch_name` +- Don't commit on master branch, create new branch before committing any changes + +### Making Changes + +1. **Create a branch** (see "Helpful Commands > Git Commands") +2. **Make your changes** following coding standards +3. **Write or update tests** (see "Testing" section) +4. **Run linting** (see "Coding Standards > Linting") +5. **Run tests** to verify changes (see "Testing" section) + +### Pull Request Process + +When creating a pull request, follow this checklist: + +1. **Ensure all tests pass** (see "Testing" section) +2. **Run SwiftLint and fix all issues** (see "Coding Standards > Linting") +3. **Verify no merge conflicts with `master`** + ```bash + git fetch origin + git merge origin/master + ``` + +4. **Follow the PR template** (located at `pull_request_template.md`) + + Your PR description MUST include: + + **## Summary** + - Bullet points describing "what" changed (each logical change) + - Context explaining "why" the changes were made + + **## Test plan** + - Describe how the changes were tested + - List specific test cases added or modified + - Include manual testing steps if applicable + + **## Issues** + - Reference related issues: "THING-1234" or "Fixes #123" + - If no issue exists, explain why the change is needed + + Example: + ```markdown + ## Summary + - Fixed flaky tests by replacing asyncAfter with DispatchGroup.wait() + - Updated MockCmabService to override both sync and async methods + + Recent async retry refactoring introduced timing issues in tests. Tests + were using unreliable asyncAfter delays instead of proper synchronization. + + ## Test plan + - Ran EventDispatcherRetryTests suite 20 times, all passed + - Verified on GitHub CI across multiple Xcode versions + - Added capturedOptions array to MockCmabService for thread-safe tracking + + ## Issues + - Fixes #456 + ``` + +5. **Get review from maintainer** + - Request review from code owners + - Address all feedback and comments + +6. **Don't update SDK version** + - Version updates are handled by maintainers during release process + +## Key APIs & Usage + +### Initialization +Initialize the SDK with an SDK key and start fetching the datafile: +```swift +let optimizely = OptimizelyClient(sdkKey: "YOUR_SDK_KEY") +optimizely.start { result in + switch result { + case .success: + // SDK ready + case .failure(let error): + // Handle error + } +} +``` + +### Decision Making +Create a user context and make feature flag decisions: +```swift +let user = optimizely.createUserContext(userId: "user123") +let decision = user.decide(key: "feature_key") +if decision.enabled { + // Feature is enabled +} +``` + +### Event Tracking +Track custom events for analytics: +```swift +try optimizely.track(eventKey: "purchase", userId: "user123") +``` + +## Helpful Commands + +### Finding Files +```bash +# Find implementation files by pattern +find Sources -name "*ClassName*.swift" + +# Find test files by pattern +find Tests -name "*TestName*.swift" + +# List all files in a specific module +find Sources/ModuleName -name "*.swift" +``` + +### Searching Code +```bash +# Find protocol definitions +grep -r "^protocol" Sources/ --include="*.swift" + +# Search for specific functions or classes +grep -r "class ClassName" Sources/ --include="*.swift" +grep -r "func functionName" Sources/ --include="*.swift" + +# Find TODO or FIXME comments +grep -r "TODO\|FIXME" Sources/ --include="*.swift" +``` + +### Git Commands +```bash +# Create a new branch +git checkout -b YOUR_NAME/feature-name + +# View recent commits +git log --oneline -10 + +# Check what changed in a specific commit +git show + +# View file changes +git diff + +# Fetch and merge latest from master +git fetch origin +git merge origin/master +``` + +### Xcode +```bash +# List available simulators +xcrun simctl list devices available + +# List schemes and build targets +xcodebuild -workspace OptimizelySwiftSDK.xcworkspace -list + +# Show build settings for a scheme +xcodebuild -workspace OptimizelySwiftSDK.xcworkspace \ + -scheme OptimizelySwiftSDK-iOS -showBuildSettings +``` diff --git a/Sources/Extensions/ArrayEventForDispatch+Extension.swift b/Sources/Extensions/ArrayEventForDispatch+Extension.swift index d662fb28..dd4d5ff1 100644 --- a/Sources/Extensions/ArrayEventForDispatch+Extension.swift +++ b/Sources/Extensions/ArrayEventForDispatch+Extension.swift @@ -113,7 +113,11 @@ extension Array where Element == EventForDispatch { enrichDecisions: true, region: base.region) - guard let data = try? JSONEncoder().encode(batchEvent) else { + let encoder = JSONEncoder() + if #available(iOS 11.0, tvOS 11.0, watchOS 4.0, macOS 10.13, *) { + encoder.outputFormatting = .sortedKeys + } + guard let data = try? encoder.encode(batchEvent) else { return nil } diff --git a/Sources/Utils/Utils.swift b/Sources/Utils/Utils.swift index 25bf0954..8be935ac 100644 --- a/Sources/Utils/Utils.swift +++ b/Sources/Utils/Utils.swift @@ -79,7 +79,13 @@ class Utils { #endif } - private static let jsonEncoder = JSONEncoder() + private static let jsonEncoder: JSONEncoder = { + let encoder = JSONEncoder() + if #available(iOS 11.0, tvOS 11.0, watchOS 4.0, macOS 10.13, *) { + encoder.outputFormatting = .sortedKeys + } + return encoder + }() // @objc NSNumber can be casted either Bool, Int, or Double // more filtering required to avoid NSNumber(false, true) interpreted as Int(0, 1) instead of Bool diff --git a/Tests/OptimizelyTests-Batch-iOS/EventDispatcherTests_Batch.swift b/Tests/OptimizelyTests-Batch-iOS/EventDispatcherTests_Batch.swift index a03c58b1..37bd7d55 100644 --- a/Tests/OptimizelyTests-Batch-iOS/EventDispatcherTests_Batch.swift +++ b/Tests/OptimizelyTests-Batch-iOS/EventDispatcherTests_Batch.swift @@ -670,7 +670,6 @@ extension EventDispatcherTests_Batch { var notifEvent: [String: Any]? _ = optimizely.notificationCenter!.addLogEventNotificationListener { (url, event) in - print("LogEvent Notification called") notifUrl = url notifEvent = event } @@ -929,18 +928,18 @@ extension EventDispatcherTests_Batch { func makeEventForDispatch(url: String, event: BatchEvent) -> EventForDispatch { let encoder = JSONEncoder() -// if #available(iOS 11.0, tvOS 11.0, watchOS 4.0, macOS 10.13, *) { -// encoder.outputFormatting = .sortedKeys -// } + if #available(iOS 11.0, tvOS 11.0, watchOS 4.0, macOS 10.13, *) { + encoder.outputFormatting = .sortedKeys + } let data = try! encoder.encode(event) return EventForDispatch(url: URL(string: url), body: data) } func makeInvalidEventForDispatchWithNilUrl() -> EventForDispatch { let encoder = JSONEncoder() -// if #available(iOS 11.0, tvOS 11.0, watchOS 4.0, macOS 10.13, *) { -// encoder.outputFormatting = .sortedKeys -// } + if #available(iOS 11.0, tvOS 11.0, watchOS 4.0, macOS 10.13, *) { + encoder.outputFormatting = .sortedKeys + } let data = try! encoder.encode(batchEventA) return EventForDispatch(url: nil, body: data) } @@ -1035,7 +1034,7 @@ class TestEventDispatcher: DefaultEventDispatcher { override func sendEvent(event: EventForDispatch, completionHandler: @escaping DispatchCompletionHandler) { sendRequestedEvents.append(event) - + do { let decodedEvent = try JSONDecoder().decode(BatchEvent.self, from: event.body) numReceivedVisitors += decodedEvent.visitors.count @@ -1044,21 +1043,20 @@ class TestEventDispatcher: DefaultEventDispatcher { // invalid event format detected // - invalid events are supposed to be filtered out when batching (converting to nil, so silently dropped) // - an exeption is that an invalid event is alone in the queue, when validation is skipped for performance on common path - + // pass through invalid events, so server can filter them out } - - // must call completionHandler to complete synchronization - super.sendEvent(event: event) { _ in - if self.forceError { - completionHandler(.failure(.eventDispatchFailed("forced"))) - } else { - // return success to clear store after sending events - completionHandler(.success(Data())) - } - - self.exp?.fulfill() + NotificationCenter.default.post(name: .willSendOptimizelyEvents, object: event) + // Simulate network call completion immediately in tests to avoid timeouts + // Instead of making actual network requests to dummy URLs, directly call the completion handler + if self.forceError { + completionHandler(.failure(.eventDispatchFailed("forced"))) + } else { + // return success to clear store after sending events + completionHandler(.success(Data())) } + + self.exp?.fulfill() } } diff --git a/Tests/OptimizelyTests-Common/DecisionServiceTests_Experiments.swift b/Tests/OptimizelyTests-Common/DecisionServiceTests_Experiments.swift index 1fda868e..b5ac3a89 100644 --- a/Tests/OptimizelyTests-Common/DecisionServiceTests_Experiments.swift +++ b/Tests/OptimizelyTests-Common/DecisionServiceTests_Experiments.swift @@ -469,7 +469,7 @@ extension DecisionServiceTests_Experiments { extension DecisionServiceTests_Experiments { func testDoesMeetAudienceConditionsWithInvalidType() { - MockLogger.expectedLog = OptimizelyError.userAttributeInvalidType("{\"match\":\"gt\",\"value\":17,\"name\":\"age\",\"type\":\"\"}").localizedDescription + MockLogger.expectedLog = OptimizelyError.userAttributeInvalidType("{\"match\":\"gt\",\"name\":\"age\",\"type\":\"\",\"value\":17}").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -485,7 +485,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithInvalidMatchType() { - MockLogger.expectedLog = OptimizelyError.userAttributeInvalidMatch("{\"match\":\"\",\"value\":17,\"name\":\"age\",\"type\":\"custom_attribute\"}").localizedDescription + MockLogger.expectedLog = OptimizelyError.userAttributeInvalidMatch("{\"match\":\"\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":17}").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -501,7 +501,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithInvalidName() { - MockLogger.expectedLog = OptimizelyError.userAttributeInvalidName("{\"type\":\"custom_attribute\",\"match\":\"gt\",\"value\":17}").localizedDescription + MockLogger.expectedLog = OptimizelyError.userAttributeInvalidName("{\"match\":\"gt\",\"type\":\"custom_attribute\",\"value\":17}").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -517,7 +517,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithMissingAttributeValue() { - MockLogger.expectedLog = OptimizelyError.missingAttributeValue("{\"match\":\"gt\",\"value\":17,\"name\":\"age\",\"type\":\"custom_attribute\"}", "age").localizedDescription + MockLogger.expectedLog = OptimizelyError.missingAttributeValue("{\"match\":\"gt\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":17}", "age").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -533,7 +533,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithNilUserAttributeValue() { - MockLogger.expectedLog = OptimizelyError.userAttributeNilValue("{\"name\":\"age\",\"type\":\"custom_attribute\",\"match\":\"gt\"}").localizedDescription + MockLogger.expectedLog = OptimizelyError.userAttributeNilValue("{\"match\":\"gt\",\"name\":\"age\",\"type\":\"custom_attribute\"}").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -549,7 +549,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithNilAttributeValue() { - MockLogger.expectedLog = OptimizelyError.nilAttributeValue("{\"match\":\"gt\",\"value\":17,\"name\":\"age\",\"type\":\"custom_attribute\"}", "age").localizedDescription + MockLogger.expectedLog = OptimizelyError.nilAttributeValue("{\"match\":\"gt\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":17}", "age").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -565,7 +565,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithExactMatchAndInvalidValue() { - MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidCondition("{\"match\":\"exact\",\"value\":{\"invalid\":{}},\"name\":\"age\",\"type\":\"custom_attribute\"}").localizedDescription + MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidCondition("{\"match\":\"exact\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":{\"invalid\":{}}}").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -579,7 +579,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithExactMatchAndInvalidAttributeValue() { - MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidType("{\"match\":\"exact\",\"value\":\"us\",\"name\":\"country\",\"type\":\"custom_attribute\"}",["invalid"],"country").localizedDescription + MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidType("{\"match\":\"exact\",\"name\":\"country\",\"type\":\"custom_attribute\",\"value\":\"us\"}",["invalid"],"country").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -595,7 +595,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithExactMatchAndInfiniteAttributeValue() { - MockLogger.expectedLog = OptimizelyError.evaluateAttributeValueOutOfRange("{\"match\":\"exact\",\"value\":17,\"name\":\"age\",\"type\":\"custom_attribute\"}","age").localizedDescription + MockLogger.expectedLog = OptimizelyError.evaluateAttributeValueOutOfRange("{\"match\":\"exact\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":17}","age").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -611,7 +611,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithGreaterMatchAndInvalidValue() { - MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidCondition("{\"match\":\"gt\",\"value\":{\"invalid\":{}},\"name\":\"age\",\"type\":\"custom_attribute\"}").localizedDescription + MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidCondition("{\"match\":\"gt\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":{\"invalid\":{}}}").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -626,7 +626,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithGreaterMatchAndInvalidAttributeValue() { - MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidType("{\"match\":\"gt\",\"value\":17,\"name\":\"age\",\"type\":\"custom_attribute\"}", ["invalid"], "age").localizedDescription + MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidType("{\"match\":\"gt\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":17}", ["invalid"], "age").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -642,7 +642,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithLessMatchAndInvalidValue() { - MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidCondition("{\"match\":\"lt\",\"value\":{\"invalid\":{}},\"name\":\"age\",\"type\":\"custom_attribute\"}").localizedDescription + MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidCondition("{\"match\":\"lt\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":{\"invalid\":{}}}").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -657,7 +657,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithLessMatchAndInvalidAttributeValue() { - MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidType("{\"match\":\"lt\",\"value\":17,\"name\":\"age\",\"type\":\"custom_attribute\"}", ["invalid"], "age").localizedDescription + MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidType("{\"match\":\"lt\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":17}", ["invalid"], "age").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -673,7 +673,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithSubstringMatchAndInvalidValue() { - MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidCondition("{\"match\":\"substring\",\"value\":151,\"name\":\"age\",\"type\":\"custom_attribute\"}").localizedDescription + MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidCondition("{\"match\":\"substring\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":151}").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) @@ -689,7 +689,7 @@ extension DecisionServiceTests_Experiments { } func testDoesMeetAudienceConditionsWithSubstringMatchAndInvalidAttributeValue() { - MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidType("{\"match\":\"substring\",\"value\":\"twelve\",\"name\":\"age\",\"type\":\"custom_attribute\"}", ["invalid"], "age").localizedDescription + MockLogger.expectedLog = OptimizelyError.evaluateAttributeInvalidType("{\"match\":\"substring\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":\"twelve\"}", ["invalid"], "age").localizedDescription self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData) experiment = try! OTUtils.model(from: sampleExperimentData) diff --git a/Tests/OptimizelyTests-Common/EventDispatcherRetryTests.swift b/Tests/OptimizelyTests-Common/EventDispatcherRetryTests.swift index a79c95a6..fef7783a 100644 --- a/Tests/OptimizelyTests-Common/EventDispatcherRetryTests.swift +++ b/Tests/OptimizelyTests-Common/EventDispatcherRetryTests.swift @@ -22,6 +22,7 @@ class EventDispatcherRetryTests: XCTestCase { override func setUp() { OTUtils.createDocumentDirectoryIfNotAvailable() + OTUtils.clearAllEventQueues() } override func tearDown() { @@ -158,12 +159,12 @@ class EventDispatcherRetryTests: XCTestCase { dispatcher.flushEvents() - // Wait for all retries to exhaust - let expectation = XCTestExpectation(description: "All retries exhausted") - dispatcher.queueLock.asyncAfter(deadline: .now() + 1.0) { - expectation.fulfill() - } - wait(for: [expectation], timeout: 2.0) + // Ensure flush async block has started + dispatcher.queueLock.sync {} + + // Wait for all retries to complete using the notify DispatchGroup + // This is more reliable than asyncAfter as it actually waits for completion + _ = dispatcher.notify.wait(timeout: .now() + 5.0) // Should have 3 send attempts (1 initial + 2 retries) XCTAssertEqual(dispatcher.sendCount, 3) @@ -260,9 +261,14 @@ class EventDispatcherRetryTests: XCTestCase { XCTAssertTrue(dispatcher.reachability.shouldBlockNetworkAccess()) dispatcher.flushEvents() + + // Ensure flush async block has started dispatcher.queueLock.sync {} - // When network is down, sendEvent returns early + // Wait for flush to complete using the notify DispatchGroup + _ = dispatcher.notify.wait(timeout: .now() + 5.0) + + // When network is down, sendEvent should not be called // No retries should happen XCTAssertEqual(dispatcher.sendCount, 0) } diff --git a/Tests/OptimizelyTests-Common/OptimizelyUserContextTests_Decide_CMAB.swift b/Tests/OptimizelyTests-Common/OptimizelyUserContextTests_Decide_CMAB.swift index eafb6fe9..d69f8ff1 100644 --- a/Tests/OptimizelyTests-Common/OptimizelyUserContextTests_Decide_CMAB.swift +++ b/Tests/OptimizelyTests-Common/OptimizelyUserContextTests_Decide_CMAB.swift @@ -28,7 +28,7 @@ class OptimizelyUserContextTests_Decide_CMAB: XCTestCase { override func setUp() { super.setUp() - + let datafile = OTUtils.loadJSONDatafile("decide_datafile")! mockCmabService = MockCmabService() decisionService = DefaultDecisionService(userProfileService: DefaultUserProfileService(), cmabService: mockCmabService) @@ -39,8 +39,11 @@ class OptimizelyUserContextTests_Decide_CMAB: XCTestCase { self.config = self.optimizely.config try! optimizely.start(datafile: datafile) } - + override func tearDown() { + super.tearDown() + // Reset mock service state + mockCmabService?.reset() optimizely = nil mockCmabService = nil decisionService = nil @@ -215,14 +218,14 @@ class OptimizelyUserContextTests_Decide_CMAB: XCTestCase { let exp2 = XCTestExpectation(description: "Second call") let exp3 = XCTestExpectation(description: "Third call") - + // Set up the CMAB experiment let cmab: Cmab = try! OTUtils.model(from: ["trafficAllocation": 10000, "attributeIds": ["10389729780"]]) var experiments = optimizely.config!.project.experiments experiments[0].cmab = cmab optimizely.config?.project.experiments = experiments mockCmabService.variationId = "10389729780" // corresponds to variation "a" - + // Create user with attributes that match CMAB experiment let user = optimizely.createUserContext( userId: kUserId, @@ -230,21 +233,27 @@ class OptimizelyUserContextTests_Decide_CMAB: XCTestCase { ) user.decideAsync(key: "feature_1", options: [.ignoreCmabCache]) { decision in XCTAssertEqual(decision.variationKey, "a") - XCTAssertTrue(self.mockCmabService.ignoreCacheUsed) exp1.fulfill() } user.decideAsync(key: "feature_1", options: [.resetCmabCache]) { decision in XCTAssertEqual(decision.variationKey, "a") - XCTAssertTrue(self.mockCmabService.resetCacheCache) exp2.fulfill() } user.decideAsync(key: "feature_1", options: [.invalidateUserCmabCache]) { decision in XCTAssertEqual(decision.variationKey, "a") - XCTAssertTrue(self.mockCmabService.invalidateUserCmabCache) exp3.fulfill() } wait(for: [exp1, exp2, exp3], timeout: 1) + // Ensure all queued decision work has completed before checking captured options + // decideAsync uses optimizely.decisionQueue, so sync on it to ensure all tasks finish + optimizely.decisionQueue.sync {} + + // Verify options were passed correctly for each call (thread-safe check after all async calls complete) + XCTAssertEqual(self.mockCmabService.capturedOptions.count, 3, "Expected 3 calls to getDecision") + XCTAssertTrue(self.mockCmabService.capturedOptions[0].contains(.ignoreCmabCache), "First call should have ignoreCmabCache option") + XCTAssertTrue(self.mockCmabService.capturedOptions[1].contains(.resetCmabCache), "Second call should have resetCmabCache option") + XCTAssertTrue(self.mockCmabService.capturedOptions[2].contains(.invalidateUserCmabCache), "Third call should have invalidateUserCmabCache option") } func testDecideAsync_cmabError() { @@ -282,31 +291,70 @@ fileprivate class MockCmabService: DefaultCmabService { var ignoreCacheUsed = false var resetCacheCache = false var invalidateUserCmabCache = false - + + // Thread-safe tracking of options for each call to avoid race conditions + private let optionsLock = DispatchQueue(label: "MockCmabService.optionsLock") + private var _capturedOptions: [[OptimizelyDecideOption]] = [] + var capturedOptions: [[OptimizelyDecideOption]] { + optionsLock.sync { + _capturedOptions + } + } + init() { super.init(cmabClient: DefaultCmabClient(), cmabCache: CmabCache(size: 10, timeoutInSecs: 10)) } - + + func reset() { + optionsLock.sync { + self.variationId = nil + self.error = nil + self.decisionCalled = false + self.decisionCallCount = 0 + self.lastRuleId = nil + self.ignoreCacheUsed = false + self.resetCacheCache = false + self.invalidateUserCmabCache = false + self._capturedOptions.removeAll() + } + } + + // Override both sync and async versions to ensure all call paths are tracked + override func getDecision(config: ProjectConfig, userContext: OptimizelyUserContext, ruleId: String, options: [OptimizelyDecideOption]) -> Result { - decisionCalled = true - lastRuleId = ruleId - ignoreCacheUsed = options.contains(.ignoreCmabCache) - resetCacheCache = options.contains(.resetCmabCache) - invalidateUserCmabCache = options.contains(.invalidateUserCmabCache) - decisionCallCount += 1 + // Thread-safe state tracking + optionsLock.sync { + self.decisionCalled = true + self.lastRuleId = ruleId + self.ignoreCacheUsed = options.contains(.ignoreCmabCache) + self.resetCacheCache = options.contains(.resetCmabCache) + self.invalidateUserCmabCache = options.contains(.invalidateUserCmabCache) + self.decisionCallCount += 1 + self._capturedOptions.append(options) + } + + // Return mock error if set if let error = error { return .failure(error) } - + + // Return mock decision if variationId is set if let variationId = variationId { return .success(CmabDecision( variationId: variationId, cmabUUID: "test-uuid" )) } - + + // Otherwise return error return .failure(CmabClientError.fetchFailed("No variation set")) } + + override func getDecision(config: ProjectConfig, userContext: OptimizelyUserContext, ruleId: String, options: [OptimizelyDecideOption], completion: @escaping CmabDecisionCompletionHandler) { + // Use sync version to maintain consistency + let result = getDecision(config: config, userContext: userContext, ruleId: ruleId, options: options) + completion(result) + } } extension OptimizelyUserContextTests_Decide_CMAB { diff --git a/Tests/OptimizelyTests-Common/OptimizelyUserContextTests_Decide_Reasons.swift b/Tests/OptimizelyTests-Common/OptimizelyUserContextTests_Decide_Reasons.swift index 7ece916e..c0acf4d5 100644 --- a/Tests/OptimizelyTests-Common/OptimizelyUserContextTests_Decide_Reasons.swift +++ b/Tests/OptimizelyTests-Common/OptimizelyUserContextTests_Decide_Reasons.swift @@ -109,8 +109,8 @@ extension OptimizelyUserContextTests_Decide_Reasons { let featureKey = "feature_1" let audienceId = "invalid_condition" setAudienceForFeatureTest(featureKey: featureKey, audienceId: audienceId) - - let condition = "{\"match\":\"gt\",\"value\":\"US\",\"name\":\"age\",\"type\":\"custom_attribute\"}" + + let condition = "{\"match\":\"gt\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":\"US\"}" user.setAttribute(key: "age", value: 25) var decision = user.decide(key: featureKey) @@ -123,8 +123,8 @@ extension OptimizelyUserContextTests_Decide_Reasons { let featureKey = "feature_1" let audienceId = "13389130056" setAudienceForFeatureTest(featureKey: featureKey, audienceId: audienceId) - - let condition = "{\"match\":\"exact\",\"value\":\"US\",\"name\":\"country\",\"type\":\"custom_attribute\"}" + + let condition = "{\"match\":\"exact\",\"name\":\"country\",\"type\":\"custom_attribute\",\"value\":\"US\"}" let attributeKey = "country" let attributeValue = 25 user.setAttribute(key: attributeKey, value: attributeValue) @@ -139,8 +139,8 @@ extension OptimizelyUserContextTests_Decide_Reasons { let featureKey = "feature_1" let audienceId = "age_18" setAudienceForFeatureTest(featureKey: featureKey, audienceId: audienceId) - - let condition = "{\"match\":\"gt\",\"value\":18,\"name\":\"age\",\"type\":\"custom_attribute\"}" + + let condition = "{\"match\":\"gt\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":18}" user.setAttribute(key: "age", value: pow(2,54) as Double) // TOO-BIG value var decision = user.decide(key: featureKey) @@ -153,8 +153,8 @@ extension OptimizelyUserContextTests_Decide_Reasons { let featureKey = "feature_1" let audienceId = "invalid_type" setAudienceForFeatureTest(featureKey: featureKey, audienceId: audienceId) - - let condition = "{\"match\":\"gt\",\"value\":18,\"name\":\"age\",\"type\":\"invalid\"}" + + let condition = "{\"match\":\"gt\",\"name\":\"age\",\"type\":\"invalid\",\"value\":18}" user.setAttribute(key: "age", value: 25) var decision = user.decide(key: featureKey) @@ -167,8 +167,8 @@ extension OptimizelyUserContextTests_Decide_Reasons { let featureKey = "feature_1" let audienceId = "invalid_match" setAudienceForFeatureTest(featureKey: featureKey, audienceId: audienceId) - - let condition = "{\"match\":\"invalid\",\"value\":18,\"name\":\"age\",\"type\":\"custom_attribute\"}" + + let condition = "{\"match\":\"invalid\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":18}" user.setAttribute(key: "age", value: 25) var decision = user.decide(key: featureKey) @@ -181,8 +181,8 @@ extension OptimizelyUserContextTests_Decide_Reasons { let featureKey = "feature_1" let audienceId = "nil_value" setAudienceForFeatureTest(featureKey: featureKey, audienceId: audienceId) - - let condition = "{\"name\":\"age\",\"type\":\"custom_attribute\",\"match\":\"gt\"}" + + let condition = "{\"match\":\"gt\",\"name\":\"age\",\"type\":\"custom_attribute\"}" user.setAttribute(key: "age", value: 25) var decision = user.decide(key: featureKey) @@ -195,8 +195,8 @@ extension OptimizelyUserContextTests_Decide_Reasons { let featureKey = "feature_1" let audienceId = "invalid_name" setAudienceForFeatureTest(featureKey: featureKey, audienceId: audienceId) - - let condition = "{\"type\":\"custom_attribute\",\"match\":\"gt\",\"value\":18}" + + let condition = "{\"match\":\"gt\",\"type\":\"custom_attribute\",\"value\":18}" user.setAttribute(key: "age", value: 25) var decision = user.decide(key: featureKey) @@ -209,8 +209,8 @@ extension OptimizelyUserContextTests_Decide_Reasons { let featureKey = "feature_1" let audienceId = "age_18" setAudienceForFeatureTest(featureKey: featureKey, audienceId: audienceId) - - let condition = "{\"match\":\"gt\",\"value\":18,\"name\":\"age\",\"type\":\"custom_attribute\"}" + + let condition = "{\"match\":\"gt\",\"name\":\"age\",\"type\":\"custom_attribute\",\"value\":18}" var decision = user.decide(key: featureKey) XCTAssert(decision.reasons.isEmpty) diff --git a/Tests/TestUtils/OTUtils.swift b/Tests/TestUtils/OTUtils.swift index 549a7128..b665a828 100644 --- a/Tests/TestUtils/OTUtils.swift +++ b/Tests/TestUtils/OTUtils.swift @@ -23,7 +23,11 @@ class OTUtils { static let dummyClient = OptimizelyClient(sdkKey: "any-key") static func isEqualWithEncodeThenDecode(_ model: T) -> Bool { - let jsonData = try! JSONEncoder().encode(model) + let encoder = JSONEncoder() + if #available(iOS 11.0, tvOS 11.0, watchOS 4.0, macOS 10.13, *) { + encoder.outputFormatting = .sortedKeys + } + let jsonData = try! encoder.encode(model) let modelExp = try! JSONDecoder().decode(T.self, from: jsonData) return modelExp == model } @@ -216,7 +220,11 @@ class OTUtils { static func makeEventForDispatch(url: String? = nil, event: BatchEvent? = nil) -> EventForDispatch { let targetUrl = URL(string: url ?? "https://a.b.c") - let data = try! JSONEncoder().encode(event ?? makeTestBatchEvent()) + let encoder = JSONEncoder() + if #available(iOS 11.0, tvOS 11.0, watchOS 4.0, macOS 10.13, *) { + encoder.outputFormatting = .sortedKeys + } + let data = try! encoder.encode(event ?? makeTestBatchEvent()) return EventForDispatch(url: targetUrl, body: data) }