feat: Return previous value on Provider STALE (#98)

* Revert "fix: change STALE reason in case of context change" This reverts commit 6d9ec6c. * feat: resolve previous value on provider stale
spotify · Apr 23, 2024 · 896be5e · 896be5e
1 parent f0bf363
commit 896be5e
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 53 deletions.
diff --git a/Sources/ConfidenceProvider/ConfidenceClient/ConfidenceClient.swift b/Sources/ConfidenceProvider/ConfidenceClient/ConfidenceClient.swift
@@ -18,6 +18,7 @@ public struct ResolvedValue: Codable, Equatable {
         case targetingKeyError = 2
         case generalError = 3
         case disabled = 4
+        case stale = 5
     }
 }
 

diff --git a/Sources/ConfidenceProvider/ConfidenceClient/LocalStorageResolver.swift b/Sources/ConfidenceProvider/ConfidenceClient/LocalStorageResolver.swift
@@ -16,7 +16,9 @@ public class LocalStorageResolver: Resolver {
             throw OpenFeatureError.flagNotFoundError(key: flag)
         }
         guard getResult.needsUpdate == false else {
-            throw ConfidenceError.cachedValueExpired
+            var resolveValueStale = getResult.resolvedValue
+            resolveValueStale.resolveReason = .stale
+            return .init(resolvedValue: resolveValueStale, resolveToken: getResult.resolveToken)
         }
         return .init(resolvedValue: getResult.resolvedValue, resolveToken: getResult.resolveToken)
     }

diff --git a/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift b/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift
@@ -260,43 +260,34 @@ public class ConfidenceFeatureProvider: FeatureProvider {
             throw OpenFeatureError.invalidContextError
         }
 
-        do {
-            let resolverResult = try resolver.resolve(flag: path.flag, ctx: ctx)
-
-            guard let value = resolverResult.resolvedValue.value else {
-                return resolveFlagNoValue(
-                    defaultValue: defaultValue,
-                    resolverResult: resolverResult,
-                    ctx: ctx
-                )
-            }
+        let resolverResult = try resolver.resolve(flag: path.flag, ctx: ctx)
 
-            let pathValue: Value = try getValue(path: path.path, value: value)
-            guard let typedValue: T = pathValue == .null ? defaultValue : pathValue.getTyped() else {
-                throw OpenFeatureError.parseError(message: "Unable to parse flag value: \(pathValue)")
-            }
-
-            let evaluationResult = ProviderEvaluation(
-                value: typedValue,
-                variant: resolverResult.resolvedValue.variant,
-                reason: Reason.targetingMatch.rawValue
-            )
-
-            processResultForApply(
+        guard let value = resolverResult.resolvedValue.value else {
+            return resolveFlagNoValue(
+                defaultValue: defaultValue,
                 resolverResult: resolverResult,
-                ctx: ctx,
-                applyTime: Date.backport.now
+                ctx: ctx
             )
-            return evaluationResult
-        } catch ConfidenceError.cachedValueExpired {
-            return ProviderEvaluation(
-                value: defaultValue,
-                variant: nil,
-                reason: Reason.error.rawValue,
-                errorCode: ErrorCode.providerNotReady)
-        } catch {
-            throw error
         }
+
+        let pathValue: Value = try getValue(path: path.path, value: value)
+        guard let typedValue: T = pathValue == .null ? defaultValue : pathValue.getTyped() else {
+            throw OpenFeatureError.parseError(message: "Unable to parse flag value: \(pathValue)")
+        }
+
+        let isStale = resolverResult.resolvedValue.resolveReason == .stale
+        let evaluationResult = ProviderEvaluation(
+            value: typedValue,
+            variant: resolverResult.resolvedValue.variant,
+            reason: isStale ? Reason.stale.rawValue : Reason.targetingMatch.rawValue
+        )
+
+        processResultForApply(
+            resolverResult: resolverResult,
+            ctx: ctx,
+            applyTime: Date.backport.now
+        )
+        return evaluationResult
     }
 
     private func resolveFlagNoValue<T>(defaultValue: T, resolverResult: ResolveResult, ctx: EvaluationContext)
@@ -338,6 +329,12 @@ public class ConfidenceFeatureProvider: FeatureProvider {
                 reason: Reason.error.rawValue,
                 errorCode: ErrorCode.general,
                 errorMessage: "General error in the Confidence backend")
+        case .stale:
+            return ProviderEvaluation(
+                value: defaultValue,
+                variant: resolverResult.resolvedValue.variant,
+                reason: Reason.stale.rawValue
+            )
         }
     }
 

diff --git a/Tests/ConfidenceProviderTests/ConfidenceFeatureProviderTest.swift b/Tests/ConfidenceProviderTests/ConfidenceFeatureProviderTest.swift
@@ -312,25 +312,18 @@ class ConfidenceFeatureProviderTest: XCTestCase {
 
     func testStaleEvaluationContextInCache() throws {
         let resolve: [String: MockedResolveClientURLProtocol.ResolvedTestFlag] = [
-            "user1": .init(variant: "control", value: .structure(["size": .integer(3)]))
+            "user0": .init(variant: "control", value: .structure(["size": .integer(3)]))
         ]
 
         let flags: [String: MockedResolveClientURLProtocol.TestFlag] = [
             "flags/flag": .init(resolve: resolve)
         ]
 
         let session = MockedResolveClientURLProtocol.mockedSession(flags: flags)
-        // Simulating a cache with an old evaluation context
-
-        let data = [ResolvedValue(flag: "flag", resolveReason: .match)]
-            .toCacheData(context: MutableContext(targetingKey: "user0"), resolveToken: "token0")
-
-        let storage = try StorageMock(data: data)
 
         let provider =
         builder
             .with(session: session)
-            .with(storage: storage)
             .build()
         try withExtendedLifetime(
             provider.observe().sink { event in
@@ -347,13 +340,12 @@ class ConfidenceFeatureProviderTest: XCTestCase {
                 defaultValue: 0,
                 context: MutableContext(targetingKey: "user1"))
 
-            XCTAssertEqual(evaluation.value, 0)
+            XCTAssertEqual(evaluation.value, 3)
+            XCTAssertNil(evaluation.errorCode)
             XCTAssertNil(evaluation.errorMessage)
-            XCTAssertNil(evaluation.variant)
-            XCTAssertEqual(evaluation.errorCode, ErrorCode.providerNotReady)
-            XCTAssertEqual(evaluation.reason, Reason.error.rawValue)
+            XCTAssertEqual(evaluation.variant, "control")
+            XCTAssertEqual(evaluation.reason, Reason.stale.rawValue)
             XCTAssertEqual(MockedResolveClientURLProtocol.resolveStats, 1)
-
             // TODO: Check this - how do we check for something not called?
             XCTAssertEqual(flagApplier.applyCallCount, 0)
         }

diff --git a/Tests/ConfidenceProviderTests/Helpers/ClientMock.swift b/Tests/ConfidenceProviderTests/Helpers/ClientMock.swift
@@ -48,6 +48,9 @@ class ClientMock: ConfidenceResolveClient {
     }
 
     func resolve(flag: String, ctx: EvaluationContext) throws -> ResolveResult {
-        return ResolveResult(resolvedValue: ResolvedValue(flag: "flag1", resolveReason: .match), resolveToken: "")
+        return ResolveResult(
+            resolvedValue: ResolvedValue(flag: "flag1", resolveReason: .match),
+            resolveToken: ""
+        )
     }
 }
diff --git a/Tests/ConfidenceProviderTests/LocalStorageResolverTest.swift b/Tests/ConfidenceProviderTests/LocalStorageResolverTest.swift
@@ -12,12 +12,9 @@ class LocalStorageResolverTest: XCTestCase {
         let resolver = LocalStorageResolver(cache: cache)
 
         let ctx = MutableContext(targetingKey: "key", structure: MutableStructure())
-        XCTAssertThrowsError(
+        XCTAssertNoThrow(
             try resolver.resolve(flag: "test", ctx: ctx)
-        ) { error in
-            XCTAssertEqual(
-                error as? ConfidenceError, ConfidenceError.cachedValueExpired)
-        }
+        )
     }
 
     func testMissingValueFromCache() throws {