Skip to content

Commit c4b0564

Browse files
MartinCardozo-SDKMartinCardozo-SDK
authored
Merge pull request #735 from splitio/fix/SameFlag
Fix/Same Flag Counter
2 parents 0c4bb8f + f30a37c commit c4b0564

File tree

8 files changed

+105
-33
lines changed

8 files changed

+105
-33
lines changed

‎Split/Network/Sync/FeatureFlagsSynchronizer.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class DefaultFeatureFlagsSynchronizer: FeatureFlagsSynchronizer {
8989

9090
// MARK: Important. This should be called before loadLocal()
9191
// MARK: Part of /memberships hits optimization
92-
if storageContainer.generalInfoStorage.getSegmentsInUse() == nil && storageContainer.splitsStorage.changeNumber > -1 {
92+
if shouldForceParse() {
9393
Logger.v("Force Parsing flags")
9494
splitsStorage.forceParsing()
9595
ruleBasedSegmentsStorage.forceParsing()
@@ -248,4 +248,8 @@ class DefaultFeatureFlagsSynchronizer: FeatureFlagsSynchronizer {
248248
}
249249
return nil
250250
}
251+
252+
private func shouldForceParse() -> Bool {
253+
storageContainer.generalInfoStorage.getSegmentsInUse() == nil && storageContainer.generalInfoStorage.getSplitsChangeNumber() > -1
254+
}
251255
}

‎Split/Storage/GeneralInfo/GeneralInfoStorage.swift

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ protocol GeneralInfoStorage {
1010
func getFlagSpec() -> String
1111
func setFlagSpec(flagsSpec: String)
1212

13+
// Splits methods
14+
func getSplitsChangeNumber() -> Int64
15+
func setSplitsChangeNumber(changeNumber: Int64)
16+
1317
// Rule based segments methods
1418
func getRuleBasedSegmentsChangeNumber() -> Int64
1519
func setRuleBasedSegmentsChangeNumber(changeNumber: Int64)
@@ -78,6 +82,14 @@ class DefaultGeneralInfoStorage: GeneralInfoStorage {
7882
func setRuleBasedSegmentsChangeNumber(changeNumber: Int64) {
7983
generalInfoDao.update(info: .ruleBasedSegmentsChangeNumber, longValue: changeNumber)
8084
}
85+
86+
func getSplitsChangeNumber() -> Int64 {
87+
return generalInfoDao.longValue(info: .splitsChangeNumber) ?? -1
88+
}
89+
90+
func setSplitsChangeNumber(changeNumber: Int64) {
91+
generalInfoDao.update(info: .splitsChangeNumber, longValue: changeNumber)
92+
}
8193

8294
func getLastProxyUpdateTimestamp() -> Int64 {
8395
return generalInfoDao.longValue(info: .lastProxyUpdateTimestamp) ?? 0
@@ -92,14 +104,12 @@ class DefaultGeneralInfoStorage: GeneralInfoStorage {
92104
if segmentsInUse == nil { // This happens just on start
93105
segmentsInUse = generalInfoDao.longValue(info: .segmentsInUse)
94106
}
107+
return segmentsInUse
95108
}
96-
return segmentsInUse
97109
}
98110

99111
func setSegmentsInUse(_ count: Int64) {
100112
segmentsInUse = count
101-
queue.async { [weak self] in
102-
self?.generalInfoDao.update(info: .segmentsInUse, longValue: count)
103-
}
113+
generalInfoDao.update(info: .segmentsInUse, longValue: count)
104114
}
105115
}

‎Split/Storage/RuleBasedSegments/RuleBasedSegmentsStorage.swift

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,43 @@ class DefaultRuleBasedSegmentsStorage: RuleBasedSegmentsStorage {
130130
generalInfoStorage.setSegmentsInUse(segmentsInUse)
131131
}
132132

133-
fileprivate func parseSegment(_ segment: RuleBasedSegment) -> RuleBasedSegment? {
133+
private func parseSegment(_ segment: RuleBasedSegment) -> RuleBasedSegment? {
134134
guard let parsedSegment = try? Json.decodeFrom(json: segment.json, to: RuleBasedSegment.self) else { return nil }
135135
return parsedSegment
136136
}
137137

138-
fileprivate func updateSegmentsCount(_ segment: RuleBasedSegment) -> Int64 {
138+
private func updateSegmentsCount(_ segment: RuleBasedSegment) -> Int64 {
139139
var segmentsInUse: Int64 = 0
140+
let segmentName = segment.name?.lowercased() ?? ""
141+
let inMemorySegment = inMemorySegments.value(forKey: segmentName)
140142

141-
if let segmentName = segment.name?.lowercased(), segment.status == .active, inMemorySegments.value(forKey: segmentName) == nil, StorageHelper.usesSegments(segment.conditions) {
143+
// 1. New Segment
144+
if inMemorySegment == nil, segment.status == .active, StorageHelper.usesSegments(segment.conditions) {
142145
segmentsInUse += 1
143-
} else if inMemorySegments.value(forKey: segment.name?.lowercased() ?? "") != nil, segment.status != .active, StorageHelper.usesSegments(segment.conditions) {
144-
segmentsInUse -= 1
146+
}
147+
148+
// 2. Known Segment
149+
if inMemorySegment != nil, segment.status == .active {
150+
151+
if StorageHelper.usesSegments(segment.conditions ?? []) {
152+
153+
// Previously not using Segments?
154+
if StorageHelper.usesSegments(inMemorySegment?.conditions ?? []) == false {
155+
return 1
156+
}
157+
} else {
158+
// Not using Segments but previously yes?
159+
if StorageHelper.usesSegments(inMemorySegment?.conditions ?? []) {
160+
return -1
161+
}
162+
}
163+
}
164+
165+
// 3. Known segment just archived
166+
if inMemorySegment != nil, segment.status == .archived {
167+
if StorageHelper.usesSegments(segment.conditions ?? []) {
168+
return -1
169+
}
145170
}
146171

147172
return segmentsInUse

‎Split/Storage/Splits/SplitsStorage.swift

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,13 +244,34 @@ class DefaultSplitsStorage: SplitsStorage {
244244
}
245245

246246
func updateSegmentsCount(split: Split) -> Int64 { // Keep count of Flags with Segments (used to optimize "/memberships" hits)
247-
guard let splitName = split.name else { return 0 }
247+
guard let splitName = split.name?.lowercased() else { return 0 }
248+
let inMemorySplit = inMemorySplits.value(forKey: splitName)
248249

249-
if inMemorySplits.value(forKey: splitName) == nil, split.status == .active { // If new Split and active
250+
// 1. New Split using Segments
251+
if inMemorySplit == nil, split.status == .active {
250252
if StorageHelper.usesSegments(split.conditions ?? []) {
251253
return 1
252254
}
253-
} else if inMemorySplits.value(forKey: splitName) != nil && split.status != .active { // If known Split and archived
255+
}
256+
257+
// 2. Known Split
258+
if inMemorySplit != nil, split.status == .active {
259+
if StorageHelper.usesSegments(split.conditions ?? []) {
260+
261+
// Previously not using Segments?
262+
if StorageHelper.usesSegments(inMemorySplit?.conditions ?? []) == false {
263+
return 1
264+
}
265+
} else {
266+
// Not using Segments but previously yes?
267+
if StorageHelper.usesSegments(inMemorySplit?.conditions ?? []) {
268+
return -1
269+
}
270+
}
271+
}
272+
273+
// 3. Known Split just archived
274+
if inMemorySplit != nil && split.status == .archived {
254275
if StorageHelper.usesSegments(split.conditions ?? []) {
255276
return -1
256277
}

‎SplitTests/Fake/Storage/GeneralInfoStorageMock.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ class GeneralInfoStorageMock: GeneralInfoStorage {
99
var splitsFilterQueryString: String = ""
1010
var flagsSpec = ""
1111
var ruleBasedSegmentsChangeNumber: Int64 = -1
12+
var splitsChangeNumber: Int64 = -1
1213
var lastProxyUpdateTimestamp: Int64 = 0
1314
var segmentsInUse: Int64? = nil
1415

@@ -51,6 +52,14 @@ class GeneralInfoStorageMock: GeneralInfoStorage {
5152
func setRuleBasedSegmentsChangeNumber(changeNumber: Int64) {
5253
ruleBasedSegmentsChangeNumber = changeNumber
5354
}
55+
56+
func getSplitsChangeNumber() -> Int64 {
57+
return splitsChangeNumber
58+
}
59+
60+
func setSplitsChangeNumber(changeNumber: Int64) {
61+
splitsChangeNumber = changeNumber
62+
}
5463

5564
func getLastProxyUpdateTimestamp() -> Int64 {
5665
return lastProxyUpdateTimestamp

‎SplitTests/Fake/Storage/PersistentSplitsStorageStub.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,14 @@ class PersistentSplitsStorageStub: PersistentSplitsStorage {
4545

4646
func update(splitChange: ProcessedSplitChange) {
4747
processedSplitChange = splitChange
48+
changeNumber = splitChange.changeNumber
4849
updateCalled = true
4950
}
5051

5152
func update(split: Split) {
5253
updateSplitCalled = true
5354
splits[split.name ?? ""] = split
54-
snapshot = SplitsSnapshot(changeNumber: snapshot.changeNumber, splits: splits.values.compactMap { $0 },
55+
snapshot = SplitsSnapshot(changeNumber: changeNumber, splits: splits.values.compactMap { $0 },
5556
updateTimestamp: snapshot.updateTimestamp)
5657
}
5758

‎SplitTests/Storage/SplitsStorageTests.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,23 +318,26 @@ class SplitsStorageTest: XCTestCase {
318318
let split4 = SplitTestHelper.newSplitWithMatcherType("split4", .inLargeSegment)
319319
let split5 = SplitTestHelper.newSplitWithMatcherType("split5", .inLargeSegment)
320320
let split6 = SplitTestHelper.newSplitWithMatcherType("split6", .inLargeSegment)
321+
322+
let splitNotUsingSegments = newSplit(name: "added")
323+
let splitsToAdd = [split, split2, split3, split4, splitNotUsingSegments, split5]
321324

322-
persistentStorage.snapshot = getTestSnapshot()
325+
persistentStorage.snapshot = SplitsSnapshot(changeNumber: dummyChangeNumber, splits: [], updateTimestamp: dummyUpdateTimestamp)
323326
splitsStorage.loadLocal()
324327

325328
// 1. Check Segments count is in 0
326329
XCTAssertEqual(generalInfoStorage.getSegmentsInUse(), 0)
327330

328-
let splitNotUsingSegments = newSplit(name: "added")
329331

330332
// 2. Add 6 Splits (1 not using Segments)
331-
var processedChange = ProcessedSplitChange(activeSplits: [split, split2, split3, split4, splitNotUsingSegments, split5],
333+
var processedChange = ProcessedSplitChange(activeSplits: splitsToAdd,
332334
archivedSplits: [],
333335
changeNumber: 999, updateTimestamp: 888)
334336

335337
_ = splitsStorage.update(splitChange: processedChange)
336338
XCTAssertEqual(generalInfoStorage.getSegmentsInUse(), 5) // One should have been ignored, so 5
337339
XCTAssertTrue(persistentStorage.updateCalled)
340+
persistentStorage.snapshot = SplitsSnapshot(changeNumber: dummyChangeNumber, splits: splitsToAdd, updateTimestamp: dummyUpdateTimestamp)
338341

339342
// 3. Add 2 previously added (should be ignored by the counter), and a new one
340343
processedChange = ProcessedSplitChange(activeSplits: [split, split2, split6],

‎SplitTests/Streaming/FeatureFlagsSynchronizerTest.swift

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class FeatureFlagsSynchronizerTest: XCTestCase {
3737
synchronizer = buildSynchronizer()
3838
}
3939

40-
func buildSynchronizer(syncEnabled: Bool = true, splitFilters: [SplitFilter]? = nil, generalInfoStorage: GeneralInfoStorageMock? = nil, Database: SplitDatabase? = nil) -> FeatureFlagsSynchronizer {
40+
func buildSynchronizer(syncEnabled: Bool = true, splitFilters: [SplitFilter]? = nil, splitsStorage: SplitsStorageStub? = nil, generalInfoStorage: GeneralInfoStorageMock? = nil, Database: SplitDatabase? = nil, withChangeNumber: Int64 = 100) -> FeatureFlagsSynchronizer {
4141

4242
eventsManager = SplitEventsManagerStub()
4343
persistentSplitsStorage = PersistentSplitsStorageStub()
@@ -47,26 +47,22 @@ class FeatureFlagsSynchronizerTest: XCTestCase {
4747
syncWorkerFactory = SyncWorkerFactoryStub()
4848
syncWorkerFactory.splitsSyncWorker = splitsSyncWorker
4949
syncWorkerFactory.periodicSplitsSyncWorker = periodicSplitsSyncWorker
50-
splitsStorage = SplitsStorageStub()
5150
ruleBasedSegmentsStorage = RuleBasedSegmentsStorageStub()
5251
broadcasterChannel = SyncEventBroadcasterStub()
5352

54-
if generalInfoStorage == nil {
55-
self.generalInfoStorage = GeneralInfoStorageMock()
56-
}
57-
58-
if Database == nil {
59-
self.database = TestingHelper.createTestDatabase(name: "pepe")
60-
}
53+
self.splitsStorage = splitsStorage ?? SplitsStorageStub()
54+
self.generalInfoStorage = generalInfoStorage ?? GeneralInfoStorageMock()
55+
self.database = Database ?? TestingHelper.createTestDatabase(name: "pepe")
6156

57+
persistentSplitsStorage.changeNumber = withChangeNumber
6258
telemetryStorageStub = TelemetryStorageStub()
63-
_ = splitsStorage.update(splitChange: ProcessedSplitChange(activeSplits: [], archivedSplits: [],
64-
changeNumber: 100, updateTimestamp: 100))
59+
_ = self.splitsStorage!.update(splitChange: ProcessedSplitChange(activeSplits: [], archivedSplits: [],
60+
changeNumber: withChangeNumber, updateTimestamp: 100))
6561

6662
self.generalInfoStorage.setFlagSpec(flagsSpec: "1.2")
6763

6864
let storageContainer = SplitStorageContainer(splitDatabase: database,
69-
splitsStorage: splitsStorage,
65+
splitsStorage: self.splitsStorage!,
7066
persistentSplitsStorage: persistentSplitsStorage,
7167
impressionsStorage: ImpressionsStorageStub(),
7268
persistentImpressionsStorage: PersistentImpressionsStorageStub(),
@@ -81,7 +77,7 @@ class FeatureFlagsSynchronizerTest: XCTestCase {
8177
flagSetsCache: FlagSetsCacheMock(),
8278
persistentHashedImpressionsStorage: PersistentHashedImpressionStorageMock(),
8379
hashedImpressionsStorage: HashedImpressionsStorageMock(),
84-
generalInfoStorage: self.generalInfoStorage,
80+
generalInfoStorage: self.generalInfoStorage!,
8581
ruleBasedSegmentsStorage: ruleBasedSegmentsStorage,
8682
persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub())
8783

@@ -353,10 +349,13 @@ class FeatureFlagsSynchronizerTest: XCTestCase {
353349
}
354350

355351
func testForceParcing() {
356-
var generalInfoStorage = GeneralInfoStorageMock()
352+
let splitsStorage = SplitsStorageStub()
353+
let generalInfoStorage = GeneralInfoStorageMock()
354+
splitsStorage.changeNumber = 100
355+
generalInfoStorage.setSplitsChangeNumber(changeNumber: 100)
357356

358-
synchronizer = buildSynchronizer(generalInfoStorage: GeneralInfoStorageMock()) // SegmentsInUse = nil, changeNumber = 100
359-
synchronizer.load()
357+
let synchronizer2 = buildSynchronizer(splitsStorage: splitsStorage, generalInfoStorage: generalInfoStorage, withChangeNumber: 100) // SegmentsInUse = nil, changeNumber = 100
358+
synchronizer2.load()
360359

361360
ThreadUtils.delay(seconds: 2)
362361

0 commit comments

Comments
 (0)