Skip to content

Commit

Permalink
Merge pull request #38 from spotify/sent-property
Browse files Browse the repository at this point in the history
Remove SENT property from flag applier stored cache
  • Loading branch information
fabriziodemaria committed Jul 19, 2023
2 parents 640f8d3 + 61ef012 commit 8e727cc
Show file tree
Hide file tree
Showing 12 changed files with 35 additions and 145 deletions.
13 changes: 1 addition & 12 deletions Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ final class FlagApplierWithRetries: FlagApplier {
self.write(resolveToken: resolveToken, name: flagName, applyTime: applyTime)
return
}
self.setEventSent(resolveToken: resolveToken, name: flagName)
self.triggerBatch()
}
}
Expand All @@ -63,21 +62,11 @@ final class FlagApplierWithRetries: FlagApplier {
return
}
// Remove events from storage that were successfully sent
// Set 'sent' property of apply events to true
self.remove(resolveToken: resolveEvent.resolveToken)
resolveEvent.events.forEach { applyEvent in
self.setEventSent(resolveToken: resolveEvent.resolveToken, name: applyEvent.name)
}
}
}
}

private func setEventSent(resolveToken: String, name: String) {
Task {
await self.cacheDataInteractor.setEventSent(resolveToken: resolveToken, name: name)
}
}

private func write(resolveToken: String, name: String, applyTime: Date) {
guard var storedData = try? storage.load(defaultValue: CacheData.empty()) else {
return
Expand All @@ -98,7 +87,7 @@ final class FlagApplierWithRetries: FlagApplier {
let applyFlagRequestItems = items.map { applyEvent in
AppliedFlagRequestItem(
flag: applyEvent.name,
applyTime: applyEvent.applyEvent.applyTime
applyTime: applyEvent.applyTime
)
}
let request = ApplyFlagsRequest(
Expand Down
10 changes: 3 additions & 7 deletions Sources/ConfidenceProvider/Cache/CacheDataActor.swift
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import Foundation

/**
`CacheDataActor` protocol defines an actor responsible for interactions with `CacheData`.
Implementation of CacheDataActor is thread-safe by default.
**/
/// `CacheDataActor` protocol defines an actor responsible for interactions with `CacheData`.
/// Implementation of CacheDataActor is thread-safe by default.

protocol CacheDataActor: Actor {
var cache: CacheData { get }

Expand All @@ -21,7 +20,4 @@ protocol CacheDataActor: Actor {

/// Removes single apply event from the cache.
func applyEventExists(resolveToken: String, name: String) -> Bool

/// Sets Flag Apply Event `sent` propery to `true`.
func setEventSent(resolveToken: String, name: String)
}
7 changes: 2 additions & 5 deletions Sources/ConfidenceProvider/Cache/CacheDataInteractor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,10 @@ final actor CacheDataInteractor: CacheDataActor {
cache.applyEventExists(resolveToken: resolveToken, name: name)
}

func setEventSent(resolveToken: String, name: String) {
cache.setEventSent(resolveToken: resolveToken, name: name)
}

private func loadCacheFromStorage() {
guard let storedData = try? storage.load(defaultValue: cache),
storedData.isEmpty == false else {
storedData.isEmpty == false
else {
return
}
self.cache = storedData
Expand Down
11 changes: 0 additions & 11 deletions Sources/ConfidenceProvider/Cache/Models/ApplyEvent.swift

This file was deleted.

21 changes: 5 additions & 16 deletions Sources/ConfidenceProvider/Cache/Models/CacheData.swift
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import Foundation

/**
`CacheData` represents object that encapsulates exposure events for evaluated flags.
It holds information related to apply event i.e. resolve token, flag name, timestamp .
This object is used for tracking exposure events, i.e. by storing them on disk.
*/
/// `CacheData` represents object that encapsulates exposure events for evaluated flags.
/// It holds information related to apply event i.e. resolve token, flag name, timestamp .
/// This object is used for tracking exposure events, i.e. by storing them on disk.
struct CacheData: Codable {
var resolveEvents: [ResolveApply]

Expand Down Expand Up @@ -35,16 +33,6 @@ struct CacheData: Codable {
return resolveTokenIndex != nil
}

mutating func setEventSent(resolveToken: String, name: String, sent: Bool = true) {
let flagEventIndexes = flagEventIndex(resolveToken: resolveToken, name: name)
guard let resolveIndex = flagEventIndexes.resolveEventIndex,
let flagIndex = flagEventIndexes.flagEventIndex else {
return
}

resolveEvents[resolveIndex].events[flagIndex].applyEvent.sent = sent
}

mutating func add(resolveToken: String, flagName: String, applyTime: Date) {
let resolveEventIndex = resolveEventIndex(resolveToken: resolveToken)

Expand Down Expand Up @@ -105,7 +93,8 @@ struct CacheData: Codable {

func flagEvent(resolveToken: String, name: String) -> FlagApply? {
guard let resolveTokenIndex = resolveEventIndex(resolveToken: resolveToken),
let flagEventIndex = applyEventIndex(resolveToken: resolveToken, name: name) else {
let flagEventIndex = applyEventIndex(resolveToken: resolveToken, name: name)
else {
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions Sources/ConfidenceProvider/Cache/Models/FlagApply.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import Foundation

struct FlagApply: Codable {
let name: String
var applyEvent: ApplyEvent
var applyTime: Date

init(name: String, applyTime: Date) {

Check warning on line 7 in Sources/ConfidenceProvider/Cache/Models/FlagApply.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Unneeded Synthesized Initializer Violation: This memberwise initializer would be synthesized automatically - you do not need to define it (unneeded_synthesized_initializer)
self.name = name
self.applyEvent = ApplyEvent(applyTime: applyTime)
self.applyTime = applyTime
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public class RemoteConfidenceClient: ConfidenceClient {
apply: applyOnResolve)

do {
let result = try self.httpClient.post(path: ":resolve", data: request, resultType: ResolveFlagsResponse.self)
let result = try self.httpClient.post(
path: ":resolve", data: request, resultType: ResolveFlagsResponse.self)
guard result.response.status == .ok else {
throw result.response.mapStatusToError(error: result.decodedError)
}
Expand Down
12 changes: 7 additions & 5 deletions Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -438,11 +438,13 @@ extension ConfidenceFeatureProvider {

/// Creates the `ConfidenceFeatureProvider` according to the settings specified in the builder.
public func build() -> ConfidenceFeatureProvider {
let flagApplier = flagApplier ?? FlagApplierWithRetries(
httpClient: NetworkClient(region: options.region),
storage: DefaultStorage(filePath: "applier.flags.cache"),
options: options
)
let flagApplier =
flagApplier
?? FlagApplierWithRetries(
httpClient: NetworkClient(region: options.region),
storage: DefaultStorage(filePath: "applier.flags.cache"),
options: options
)

let client = RemoteConfidenceClient(
options: options,
Expand Down
16 changes: 9 additions & 7 deletions Sources/ConfidenceProvider/Http/HttpClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ final class NetworkClient: HttpClient {
timeout: TimeInterval = 30.0,
retry: Retry = .none
) {
self.session = session ?? {
let configuration = URLSessionConfiguration.default
configuration.timeoutIntervalForRequest = timeout
configuration.httpAdditionalHeaders = defaultHeaders

return URLSession(configuration: configuration)
}()
self.session =
session
?? {
let configuration = URLSessionConfiguration.default
configuration.timeoutIntervalForRequest = timeout
configuration.httpAdditionalHeaders = defaultHeaders

return URLSession(configuration: configuration)
}()

self.headers = defaultHeaders
self.retry = retry
Expand Down
41 changes: 3 additions & 38 deletions Tests/ConfidenceProviderTests/CacheDataTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ final class CacheDataTests: XCTestCase {
let resolveEvent = try XCTUnwrap(cacheData.resolveEvents.first)
XCTAssertEqual(resolveEvent.resolveToken, "token1")
XCTAssertEqual(resolveEvent.events.count, 1)
XCTAssertEqual(resolveEvent.events.first?.applyEvent.applyTime, applyTime)
XCTAssertEqual(resolveEvent.events.first?.applyTime, applyTime)
}

func testCacheData_addEvent_emptyFlagEvents() throws {
Expand All @@ -38,7 +38,7 @@ final class CacheDataTests: XCTestCase {
let resolveEvent = try XCTUnwrap(cacheData.resolveEvents.first)
XCTAssertEqual(resolveEvent.resolveToken, "token1")
XCTAssertEqual(resolveEvent.events.count, 1)
XCTAssertEqual(resolveEvent.events.first?.applyEvent.applyTime, applyTime)
XCTAssertEqual(resolveEvent.events.first?.applyTime, applyTime)
}

func testCacheData_addEvent_prefilled() throws {
Expand Down Expand Up @@ -66,7 +66,7 @@ final class CacheDataTests: XCTestCase {

// Then apply record is not overriden
let applyEvent = try XCTUnwrap(cacheData.resolveEvents.first?.events.first)
XCTAssertEqual(applyEvent.applyEvent.applyTime, applyTime)
XCTAssertEqual(applyEvent.applyTime, applyTime)
}

func testCacheData_addEvent_multipleTokens() throws {
Expand Down Expand Up @@ -111,41 +111,6 @@ final class CacheDataTests: XCTestCase {
XCTAssertEqual(cacheData.resolveEvents.isEmpty, true)
}

func testCacheData_eventExist_isEventSent() throws {
// Given prefilled cached data
// and all apply events has sent property set to false
var cacheData = try CacheDataUtility.prefilledCacheData()
let resolve = try XCTUnwrap(cacheData.resolveEvents.first)
let sentEvents = resolve.events.filter { $0.applyEvent.sent == true }
XCTAssertEqual(sentEvents.count, 0)

// When set event sent is called for item that exists in cache
cacheData.setEventSent(resolveToken: "token0", name: "prefilled2")
let resolveAfterUpdate = try XCTUnwrap(cacheData.resolveEvents.first)

// Then apply event sent property has been set to true
let sentEventsAfterUpdate = resolveAfterUpdate.events.filter { $0.applyEvent.sent == true }
XCTAssertEqual(sentEventsAfterUpdate.count, 1)
XCTAssertEqual(sentEventsAfterUpdate.first?.name, "prefilled2")
}

func testCacheData_eventDoesNotExist_isEventSent() throws {
// Given prefilled cached data
// and all apply events has sent property set to false
var cacheData = try CacheDataUtility.prefilledCacheData()
let resolve = try XCTUnwrap(cacheData.resolveEvents.first)
let sentEvents = resolve.events.filter { $0.applyEvent.sent == true }
XCTAssertEqual(sentEvents.count, 0)

// When set event sent is called for item that does not exists in cache
cacheData.setEventSent(resolveToken: "token0", name: "prefilled45")
let resolveAfterUpdate = try XCTUnwrap(cacheData.resolveEvents.first)

// Then apply event sent property has not been changed
let sentEventsAfterUpdate = resolveAfterUpdate.events.filter { $0.applyEvent.sent == true }
XCTAssertEqual(sentEventsAfterUpdate.count, 0)
}

func testCacheData_isEmpty() {
// Given empty cached data
let cacheData = CacheData.empty()
Expand Down
41 changes: 1 addition & 40 deletions Tests/ConfidenceProviderTests/FlagApplierWithRetriesTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -162,43 +162,6 @@ class FlagApplierWithRetriesTest: XCTestCase {
XCTAssertEqual(request3.flags.first?.flag, "flags/flag1")
}

func testApply_multipleApplyCalls_sentSet() async throws {
// Given flag applier with http client that is offline
let cacheDataInteractor = CacheDataInteractor(storage: storage)
let httpClient = HttpClientMock(testMode: .error)
let expectation = XCTestExpectation(description: "Waiting for batch trigger")
expectation.expectedFulfillmentCount = 3
httpClient.expectation = expectation

let applier = FlagApplierWithRetries(
httpClient: httpClient,
storage: storage,
options: options,
cacheDataInteractor: cacheDataInteractor,
triggerBatch: false
)

// When first apply call is issued
// And http client request fails with .invalidResponse
await applier.apply(flagName: "flag1", resolveToken: "token1")

// And second apply call is issued
// With test mode .success
httpClient.testMode = .success
await applier.apply(flagName: "flag2", resolveToken: "token1")
wait(for: [expectation], timeout: 1)

Task {
// Then both requests are marked as sent in cache data
let cacheData = await cacheDataInteractor.cache
let flagEvent1 = cacheData.flagEvent(resolveToken: "token1", name: "flag1")
let flagEvent2 = cacheData.flagEvent(resolveToken: "token1", name: "flag2")

XCTAssertEqual(flagEvent1?.applyEvent.sent, true)
XCTAssertEqual(flagEvent2?.applyEvent.sent, true)
}
}

func testApply_previoslyStoredData_cleanAfterSending() async throws {
// Given storage that has previously stored data (100 records, same token)
let prefilledStorage = StorageMock()
Expand Down Expand Up @@ -263,7 +226,6 @@ class FlagApplierWithRetriesTest: XCTestCase {
await applier.apply(flagName: "flag2", resolveToken: "token1")
await applier.apply(flagName: "flag3", resolveToken: "token1")


// Then 1 resolve event record is written to disk
let storedData = try XCTUnwrap(storage.load(defaultValue: CacheData.empty()))
let data = try XCTUnwrap(storedData.resolveEvents.first { $0.resolveToken == "token1" })
Expand Down Expand Up @@ -333,8 +295,7 @@ class FlagApplierWithRetriesTest: XCTestCase {
let newResolveEvent = try XCTUnwrap(storedData.resolveEvents.first { $0.resolveToken == "token0" })
XCTAssertEqual(newResolveEvent.events.count, 1)
XCTAssertEqual(newResolveEvent.events[0].name, "flag1")
XCTAssertEqual(newResolveEvent.events[0].applyEvent.applyTime, Date(timeIntervalSince1970: 1000))
XCTAssertEqual(newResolveEvent.events[0].applyEvent.sent, false)
XCTAssertEqual(newResolveEvent.events[0].applyTime, Date(timeIntervalSince1970: 1000))
}

func testApplyOffline_previoslyStoredData_100records() async throws {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,5 @@ class FlagApplierMock: FlagApplier {

func apply(flagName: String, resolveToken: String) async {
applyCallCount += 1
print("increment apply call counter by 1")
}
}

0 comments on commit 8e727cc

Please sign in to comment.