Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5a9608e
Introduce `ABTestVariationProvider` for reading `ABTest` from Woo layer.
selanthiraiyan Jan 30, 2023
b8868f1
`AuthenticationManager` - Inject `ABTestVariationProvider` and use it…
selanthiraiyan Jan 30, 2023
3813dc4
`AppCoordinator` - Inject `ABTestVariationProvider` and use it for re…
selanthiraiyan Jan 30, 2023
1c5fdc8
`AppCoordinator` - Inject `ABTestVariationProvider` and use it for re…
selanthiraiyan Jan 30, 2023
c3414ad
Inject ABTest variation from unit tests.
selanthiraiyan Jan 30, 2023
66a0321
Stop having `variation` as public to force usage of `ABTestVariationP…
selanthiraiyan Jan 30, 2023
0033332
Unit test the REST API AB test tracks event value.
selanthiraiyan Jan 30, 2023
4e7535e
Move `MockABTestVariationProvider` to a separate file for reusability.
selanthiraiyan Jan 30, 2023
5ae2cda
Update unit test name to reflect the actual test.
selanthiraiyan Jan 30, 2023
b4b1582
Add unit tests to check the scenarios when application password authe…
selanthiraiyan Jan 30, 2023
0c32dd1
Make ABTest confirm to Codable to store it to disk
selanthiraiyan Feb 2, 2023
7c5a5cf
Introduce `VariationCache` to store the ABTest variation information …
selanthiraiyan Feb 2, 2023
ada4aa6
Add cached implementation of `ABTestVariationProvider`
selanthiraiyan Feb 2, 2023
f9107f3
Point Automattic tracks pod to beta version.
selanthiraiyan Feb 2, 2023
99b0953
ABTest - Return nil instead of assuming control as variation.
selanthiraiyan Feb 2, 2023
dcca964
Add ExperimentsTests to unit test plan.
selanthiraiyan Feb 2, 2023
9803a25
Start using `CachedABTestVariationProvider` instead of `DefaultABTest…
selanthiraiyan Feb 2, 2023
8908527
Point tracks pod to latest beta commit.
selanthiraiyan Feb 2, 2023
e0d9ee5
Use Mock `ABTest` values for unit testing.
selanthiraiyan Feb 2, 2023
c4ce4ca
Explain why we cache only logged out experiments.
selanthiraiyan Feb 2, 2023
737ee32
Avoid sending mock ABTest cases to server.
selanthiraiyan Feb 2, 2023
c0708bf
Give preference to remote value over cached value.
selanthiraiyan Feb 2, 2023
40c08c3
Point `Automattic-Tracks-iOS` to latest beta version.
selanthiraiyan Feb 2, 2023
527373f
Update podfile.lock.
selanthiraiyan Feb 2, 2023
4735cc4
Merge branch 'bugfix/8782-cache-logged-out-ab-tests' into task/test-d…
toupper Feb 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions Experiments/Experiments.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
BC10218D75FEA979BDA1E68C /* Pods_Experiments.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 33CEC0C5283FD4C9EF8C6A3C /* Pods_Experiments.framework */; };
C8E16F0EE6954B58A1C402F0 /* Pods_ExperimentsTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = AAC7C082DD376957B4676401 /* Pods_ExperimentsTests.framework */; };
CC53FB48275E426900C4CA4F /* ABTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = CC53FB47275E426900C4CA4F /* ABTest.swift */; };
EE2EDFDF29879331004E702B /* ABTestVariationProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE2EDFDE29879331004E702B /* ABTestVariationProvider.swift */; };
EEC8C0ED298A92F10047B4CB /* CachedABTestVariationProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEC8C0EC298A92F10047B4CB /* CachedABTestVariationProvider.swift */; };
EEC8C0EF298A939C0047B4CB /* VariationCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEC8C0EE298A939C0047B4CB /* VariationCache.swift */; };
EEC8C0F1298B5AFB0047B4CB /* VariationCacheTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEC8C0F0298B5AFB0047B4CB /* VariationCacheTests.swift */; };
EEC8C0F3298B5F950047B4CB /* CachedABTestVariationProviderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEC8C0F2298B5F950047B4CB /* CachedABTestVariationProviderTests.swift */; };
/* End PBXBuildFile section */

/* Begin PBXContainerItemProxy section */
Expand Down Expand Up @@ -47,6 +52,11 @@
AAC7C082DD376957B4676401 /* Pods_ExperimentsTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_ExperimentsTests.framework; sourceTree = BUILT_PRODUCTS_DIR; };
AF72D9DB7771E7A5105C88B0 /* Pods-Experiments.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Experiments.release.xcconfig"; path = "Target Support Files/Pods-Experiments/Pods-Experiments.release.xcconfig"; sourceTree = "<group>"; };
CC53FB47275E426900C4CA4F /* ABTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ABTest.swift; sourceTree = "<group>"; };
EE2EDFDE29879331004E702B /* ABTestVariationProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ABTestVariationProvider.swift; sourceTree = "<group>"; };
EEC8C0EC298A92F10047B4CB /* CachedABTestVariationProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CachedABTestVariationProvider.swift; sourceTree = "<group>"; };
EEC8C0EE298A939C0047B4CB /* VariationCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VariationCache.swift; sourceTree = "<group>"; };
EEC8C0F0298B5AFB0047B4CB /* VariationCacheTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VariationCacheTests.swift; sourceTree = "<group>"; };
EEC8C0F2298B5F950047B4CB /* CachedABTestVariationProviderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CachedABTestVariationProviderTests.swift; sourceTree = "<group>"; };
/* End PBXFileReference section */

/* Begin PBXFrameworksBuildPhase section */
Expand Down Expand Up @@ -100,6 +110,9 @@
0270C0A427069B8900FC799F /* DefaultFeatureFlagService.swift */,
0270C0A627069BA500FC799F /* BuildConfiguration.swift */,
CC53FB47275E426900C4CA4F /* ABTest.swift */,
EE2EDFDE29879331004E702B /* ABTestVariationProvider.swift */,
EEC8C0EC298A92F10047B4CB /* CachedABTestVariationProvider.swift */,
EEC8C0EE298A939C0047B4CB /* VariationCache.swift */,
);
path = Experiments;
sourceTree = "<group>";
Expand All @@ -108,6 +121,8 @@
isa = PBXGroup;
children = (
0270C09127069A8900FC799F /* Info.plist */,
EEC8C0F0298B5AFB0047B4CB /* VariationCacheTests.swift */,
EEC8C0F2298B5F950047B4CB /* CachedABTestVariationProviderTests.swift */,
);
path = ExperimentsTests;
sourceTree = "<group>";
Expand Down Expand Up @@ -203,6 +218,7 @@
};
0270C08927069A8900FC799F = {
CreatedOnToolsVersion = 12.5.1;
LastSwiftMigration = 1420;
};
};
};
Expand Down Expand Up @@ -311,10 +327,13 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
EEC8C0EF298A939C0047B4CB /* VariationCache.swift in Sources */,
0270C0A327069B7800FC799F /* FeatureFlagService.swift in Sources */,
0270C0A527069B8900FC799F /* DefaultFeatureFlagService.swift in Sources */,
0270C09C27069AE700FC799F /* FeatureFlag.swift in Sources */,
EE2EDFDF29879331004E702B /* ABTestVariationProvider.swift in Sources */,
0270C0A727069BA500FC799F /* BuildConfiguration.swift in Sources */,
EEC8C0ED298A92F10047B4CB /* CachedABTestVariationProvider.swift in Sources */,
CC53FB48275E426900C4CA4F /* ABTest.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand All @@ -323,6 +342,8 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
EEC8C0F1298B5AFB0047B4CB /* VariationCacheTests.swift in Sources */,
EEC8C0F3298B5F950047B4CB /* CachedABTestVariationProviderTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down Expand Up @@ -517,6 +538,7 @@
baseConfigurationReference = 3022E2766134CE2735C73FC6 /* Pods-ExperimentsTests.debug.xcconfig */;
buildSettings = {
ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = "${inherited}";
CLANG_ENABLE_MODULES = YES;
CODE_SIGN_STYLE = Automatic;
INFOPLIST_FILE = ExperimentsTests/Info.plist;
LD_RUNPATH_SEARCH_PATHS = (
Expand All @@ -526,6 +548,7 @@
);
PRODUCT_BUNDLE_IDENTIFIER = com.automattic.ExperimentsTests;
PRODUCT_NAME = "$(TARGET_NAME)";
SWIFT_OPTIMIZATION_LEVEL = "-Onone";
SWIFT_VERSION = 5.0;
TARGETED_DEVICE_FAMILY = "1,2";
};
Expand All @@ -536,6 +559,7 @@
baseConfigurationReference = 7C831644164B49828A485590 /* Pods-ExperimentsTests.release.xcconfig */;
buildSettings = {
ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = "${inherited}";
CLANG_ENABLE_MODULES = YES;
CODE_SIGN_STYLE = Automatic;
INFOPLIST_FILE = ExperimentsTests/Info.plist;
LD_RUNPATH_SEARCH_PATHS = (
Expand Down Expand Up @@ -640,6 +664,7 @@
baseConfigurationReference = 3F9DB5FBFF7A42EFBCB746F3 /* Pods-ExperimentsTests.release-alpha.xcconfig */;
buildSettings = {
ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = "${inherited}";
CLANG_ENABLE_MODULES = YES;
CODE_SIGN_STYLE = Automatic;
INFOPLIST_FILE = ExperimentsTests/Info.plist;
LD_RUNPATH_SEARCH_PATHS = (
Expand Down
27 changes: 18 additions & 9 deletions Experiments/Experiments/ABTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ import AutomatticTracks

/// ABTest adds A/B testing experiments and runs the tests based on their variations from the ExPlat service.
///
public enum ABTest: String, CaseIterable {
/// Throwaway case, to prevent a compiler error:
/// `An enum with no cases cannot declare a raw type`
case null
public enum ABTest: String, Codable, CaseIterable {
/// Mocks for unit testing
case mockLoggedIn, mockLoggedOut

/// A/A test to make sure there is no bias in the logged out state.
/// Experiment ref: pbxNRc-1QS-p2
Expand All @@ -20,8 +19,9 @@ public enum ABTest: String, CaseIterable {
case applicationPasswordAuthentication = "woocommerceios_login_rest_api_project_202301_v2"

/// Returns a variation for the given experiment
public var variation: Variation {
ExPlat.shared?.experiment(rawValue) ?? .control
///
public var variation: Variation? {
ExPlat.shared?.experiment(rawValue)
}

/// Returns the context for the given experiment.
Expand All @@ -33,18 +33,27 @@ public enum ABTest: String, CaseIterable {
return .loggedIn
case .aaTestLoggedOut, .applicationPasswordAuthentication:
return .loggedOut
case .null:
return .none
// Mocks
case .mockLoggedIn:
return .loggedIn
case .mockLoggedOut:
return .loggedOut
}
}

// Returns only the genuine ABTest cases. (After removing unit test mocks)
//
static var genuineCases: [ABTest] {
ABTest.allCases.filter { [.mockLoggedIn, .mockLoggedOut].contains($0) == false }
}
}

public extension ABTest {
/// Start the AB Testing platform if any experiment exists for the provided context
///
@MainActor
static func start(for context: ExperimentContext) async {
let experiments = ABTest.allCases.filter { $0.context == context }
let experiments = ABTest.genuineCases.filter { $0.context == context }

await withCheckedContinuation { continuation in
guard !experiments.isEmpty else {
Expand Down
16 changes: 16 additions & 0 deletions Experiments/Experiments/ABTestVariationProvider.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import AutomatticTracks

/// For getting the variation of a `ABTest`
public protocol ABTestVariationProvider {
/// Returns the `Variation` for the provided `ABTest`
func variation(for abTest: ABTest) -> Variation
}

/// Default implementation of `ABTestVariationProvider`
public struct DefaultABTestVariationProvider: ABTestVariationProvider {
public init() { }

public func variation(for abTest: ABTest) -> Variation {
abTest.variation ?? .control
}
}
30 changes: 30 additions & 0 deletions Experiments/Experiments/CachedABTestVariationProvider.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import AutomatticTracks

/// Cache based implementation of `ABTestVariationProvider`
///
public struct CachedABTestVariationProvider: ABTestVariationProvider {

private let cache: VariationCache

public init(cache: VariationCache = VariationCache(userDefaults: .standard)) {
self.cache = cache
}

public func variation(for abTest: ABTest) -> Variation {
// We cache only logged out ABTests as they are assigned based on `anonId` by `ExPlat`.
// There will be one value assigned to one device and it won't change.
//
guard abTest.context == .loggedOut else {
return abTest.variation ?? .control
}

if let variation = abTest.variation {
try? cache.assign(variation: variation, for: abTest)
return variation
} else if let cachedVariation = cache.variation(for: abTest) {
return cachedVariation
} else {
return .control
}
}
}
47 changes: 47 additions & 0 deletions Experiments/Experiments/VariationCache.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import AutomatticTracks

enum VariationCacheError: Error {
case onlyLoggedOutExperimentsShouldBeCached
}

public struct VariationCache {
private let variationKey = "VariationCacheKey"

private let userDefaults: UserDefaults

public init(userDefaults: UserDefaults) {
self.userDefaults = userDefaults
}

func variation(for abTest: ABTest) -> Variation? {
guard abTest.context == .loggedOut else {
return nil
}

guard let data = userDefaults.object(forKey: variationKey) as? Data,
let cachedVariations = try? JSONDecoder().decode([CachedVariation].self, from: data),
case let variation = cachedVariations.first(where: { $0.abTest == abTest })?.variation
else {
return nil
}

return variation
}

func assign(variation: Variation, for abTest: ABTest) throws {
guard abTest.context == .loggedOut else {
throw VariationCacheError.onlyLoggedOutExperimentsShouldBeCached
}

var variations = userDefaults.object(forKey: variationKey) as? [CachedVariation] ?? []
variations.append(CachedVariation(abTest: abTest, variation: variation))

let encodedVariation = try JSONEncoder().encode(variations)
userDefaults.set(encodedVariation, forKey: variationKey)
}
}

public struct CachedVariation: Codable {
let abTest: ABTest
let variation: Variation
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import XCTest
@testable import Experiments

final class CachedABTestVariationProviderTests: XCTestCase {
func test_variation_is_control_when_the_value_does_not_exist() throws {
// Given
let userDefaults = try XCTUnwrap(UserDefaults(suiteName: UUID().uuidString))

// When
let cache = VariationCache(userDefaults: userDefaults)
let provider = CachedABTestVariationProvider(cache: cache)

// Then
XCTAssertEqual(provider.variation(for: .mockLoggedOut), .control)
}

func test_correct_variation_is_returned_after_caching_it() throws {
// Given
let userDefaults = try XCTUnwrap(UserDefaults(suiteName: UUID().uuidString))
let cache = VariationCache(userDefaults: userDefaults)
let provider = CachedABTestVariationProvider(cache: cache)

// When
try cache.assign(variation: .treatment, for: .mockLoggedOut)

// Then
XCTAssertEqual(provider.variation(for: .mockLoggedOut), .treatment)
}
}
36 changes: 36 additions & 0 deletions Experiments/ExperimentsTests/VariationCacheTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import XCTest
@testable import Experiments

final class VariationCacheTests: XCTestCase {
func test_variation_is_nil_when_the_value_does_not_exist() throws {
// Given
let userDefaults = try XCTUnwrap(UserDefaults(suiteName: UUID().uuidString))

// When
let cache = VariationCache(userDefaults: userDefaults)

// Then
XCTAssertNil(cache.variation(for: .mockLoggedOut))
}

func test_correct_variation_is_returned_after_setting_it() throws {
// Given
let userDefaults = try XCTUnwrap(UserDefaults(suiteName: UUID().uuidString))
let cache = VariationCache(userDefaults: userDefaults)

// When
try cache.assign(variation: .treatment, for: .mockLoggedOut)

// Then
XCTAssertEqual(cache.variation(for: .mockLoggedOut), .treatment)
}

func test_it_throws_when_trying_to_cache_logged_in_experiment() throws {
// Given
let userDefaults = try XCTUnwrap(UserDefaults(suiteName: UUID().uuidString))
let cache = VariationCache(userDefaults: userDefaults)

// When
XCTAssertThrowsError(try cache.assign(variation: .treatment, for: .mockLoggedIn))
}
}
2 changes: 1 addition & 1 deletion Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def aztec
end

def tracks
pod 'Automattic-Tracks-iOS', '~> 1.0.0'
pod 'Automattic-Tracks-iOS', '~> 1.1.0-beta.1'
# pod 'Automattic-Tracks-iOS', :git => 'https://github.com/Automattic/Automattic-Tracks-iOS.git', :branch => 'trunk'
# pod 'Automattic-Tracks-iOS', :git => 'https://github.com/Automattic/Automattic-Tracks-iOS.git', :commit => ''
# pod 'Automattic-Tracks-iOS', :path => '../Automattic-Tracks-iOS'
Expand Down
8 changes: 4 additions & 4 deletions Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ PODS:
- AppAuth/Core (1.6.0)
- AppAuth/ExternalUserAgent (1.6.0):
- AppAuth/Core
- Automattic-Tracks-iOS (1.0.0):
- Automattic-Tracks-iOS (1.1.0-beta.1):
- Sentry (~> 7.25)
- Sodium (>= 0.9.1)
- UIDeviceIdentifier (~> 2.0)
Expand Down Expand Up @@ -75,7 +75,7 @@ PODS:

DEPENDENCIES:
- Alamofire (~> 4.8)
- Automattic-Tracks-iOS (~> 1.0.0)
- Automattic-Tracks-iOS (~> 1.1.0-beta.1)
- CocoaLumberjack (~> 3.7.4)
- CocoaLumberjack/Swift (~> 3.7.4)
- Gridicons (~> 1.2.0)
Expand Down Expand Up @@ -135,7 +135,7 @@ SPEC REPOS:
SPEC CHECKSUMS:
Alamofire: 3ec537f71edc9804815215393ae2b1a8ea33a844
AppAuth: 8fca6b5563a5baef2c04bee27538025e4ceb2add
Automattic-Tracks-iOS: 93df154824af31eba947718110023acce1ce7905
Automattic-Tracks-iOS: a30ebcffcdc33e5dd41aaa8a60999b4ef0e86700
CocoaLumberjack: 543c79c114dadc3b1aba95641d8738b06b05b646
GoogleSignIn: fd381840dbe7c1137aa6dc30849a5c3e070c034a
Gridicons: 4455b9f366960121430e45997e32112ae49ffe1d
Expand Down Expand Up @@ -169,6 +169,6 @@ SPEC CHECKSUMS:
ZendeskSupportProvidersSDK: 2bdf8544f7cd0fd4c002546f5704b813845beb2a
ZendeskSupportSDK: 3a8e508ab1d9dd22dc038df6c694466414e037ba

PODFILE CHECKSUM: 5611275f2e251a88706eac841eb27ae63383a7ac
PODFILE CHECKSUM: 7b77ea6b3b7ca7a6a8b75a3c174d2ca7b1a4bbc2

COCOAPODS: 1.11.3
Loading