Fixed change listener not firing when only value changes (#85)

* Fixed change listener not firing when only value changes

* Fixed a unit test variable name typo

* Cast flag value to dictionary instead of string, improved unit test

* Remove debug printout, remove unnecessary parameter

LaunchDarkly/LaunchDarkly/Service Objects/FlagChangeNotifier.swift

@@ -158,7 +158,7 @@ final class FlagChangeNotifier: FlagChangeNotifying {
                 else {
                     return true
                 }
-                return !oldFeatureFlag.matchesVariation(newFeatureFlag)
+                return !(oldFeatureFlag.matchesVariation(newFeatureFlag) && ["value": oldFeatureFlag.value as Any] == ["value": newFeatureFlag.value as Any])
         }
     }
 }

LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift

@@ -111,6 +111,8 @@ final class DarklyServiceMock: DarklyServiceProvider {
                                      includeVariations: Bool = true,
                                      includeVersions: Bool = true,
                                      includeFlagVersions: Bool = true,
+                                     alternateVariationNumber: Bool = true,
+                                     bumpFlagVersions: Bool = false,
                                      alternateValuesForKeys alternateValueKeys: [LDFlagKey] = [],
                                      eventTrackingContext: EventTrackingContext? = EventTrackingContext.stub()) -> [LDFlagKey: FeatureFlag] {
 
@@ -121,6 +123,9 @@ final class DarklyServiceMock: DarklyServiceProvider {
                                                  includeVersion: includeVersions,
                                                  includeFlagVersion: includeFlagVersions,
                                                  useAlternateValue: useAlternateValue(for: flagKey, alternateValueKeys: alternateValueKeys),
+                                                 useAlternateVersion: bumpFlagVersions && useAlternateValue(for: flagKey, alternateValueKeys: alternateValueKeys),
+                                                 useAlternateFlagVersion: bumpFlagVersions && useAlternateValue(for: flagKey, alternateValueKeys: alternateValueKeys),
+                                                 useAlternateVariationNumber: alternateVariationNumber,
                                                  eventTrackingContext: eventTrackingContext))
             }
 
@@ -151,6 +156,14 @@ final class DarklyServiceMock: DarklyServiceProvider {
             return useAlternateValue ? variation + 1 : variation
         }
 
+        private static func variation(for flagKey: LDFlagKey, includeVariation: Bool) -> Int? {
+            guard includeVariation
+                else {
+                    return nil
+            }
+            return variation
+        }
+
         private static func version(for flagKey: LDFlagKey, includeVersion: Bool, alternateValueKeys: [LDFlagKey]) -> Int? {
             return version(for: flagKey, includeVersion: includeVersion, useAlternateVersion: useAlternateValue(for: flagKey, alternateValueKeys: alternateValueKeys))
         }
@@ -183,12 +196,13 @@ final class DarklyServiceMock: DarklyServiceProvider {
                                     useAlternateValue: Bool = false,
                                     useAlternateVersion: Bool = false,
                                     useAlternateFlagVersion: Bool = false,
+                                    useAlternateVariationNumber: Bool = true,
                                     eventTrackingContext: EventTrackingContext? = EventTrackingContext.stub(),
                                     includeEvaluationReason: Bool = false,
                                     includeTrackReason: Bool = false) -> FeatureFlag {
             return FeatureFlag(flagKey: flagKey,
                                value: value(for: flagKey, useAlternateValue: useAlternateValue),
-                               variation: variation(for: flagKey, includeVariation: includeVariation, useAlternateValue: useAlternateValue),
+                               variation: useAlternateVariationNumber ? variation(for: flagKey, includeVariation: includeVariation, useAlternateValue: useAlternateValue) : variation(for: flagKey, includeVariation: includeVariation),
                                version: version(for: flagKey, includeVersion: includeVersion, useAlternateVersion: useAlternateValue || useAlternateVersion),
                                flagVersion: flagVersion(for: flagKey, includeFlagVersion: includeFlagVersion, useAlternateFlagVersion: useAlternateValue || useAlternateFlagVersion),
                                eventTrackingContext: eventTrackingContext,

LaunchDarkly/LaunchDarklyTests/Service Objects/FlagChangeNotifierSpec.swift

@@ -483,6 +483,39 @@ final class FlagChangeNotifierSpec: QuickSpec {
 
                             expect(testContext.flagChangeHandlerCallCount) == 1
                             expect(testContext.changedFlag) == targetChangedFlag
+                            let newValue = testContext.changedFlag?.newValue as? String
+                            let newValueFromChangedFlag = targetChangedFlag?.newValue as? String
+                            if newValue != nil || newValueFromChangedFlag != nil {
+                                expect(newValue) == newValueFromChangedFlag
+                            }
+                            expect(testContext.flagsUnchangedHandlerCallCount) == 0
+                        }
+                    }
+                    it("activates the change handler when the value changes but not the variation number") {
+                        DarklyServiceMock.FlagKeys.flagsWithAnAlternateValue.forEach { (key) in
+                            testContext = TestContext(
+                                keys: DarklyServiceMock.FlagKeys.knownFlags,
+                                flagChangeHandler: { (changedFlag) in
+                                    testContext.flagChangeHandlerCallCount += 1
+                                    testContext.changedFlag = changedFlag
+                            },
+                                flagsUnchangedHandler: {
+                                    testContext.flagsUnchangedHandlerCallCount += 1
+                            })
+                            oldFlags = DarklyServiceMock.Constants.stubFeatureFlags(alternateVariationNumber: false, bumpFlagVersions: true, alternateValuesForKeys: [key])
+                            targetChangedFlag = LDChangedFlag.stub(key: key, oldFlags: oldFlags, newFlags: testContext.user.flagStore.featureFlags)
+
+                            waitUntil { done in
+                                testContext.subject.notifyObservers(user: testContext.user, oldFlags: oldFlags, oldFlagSource: .server, completion: done)
+                            }
+
+                            expect(testContext.flagChangeHandlerCallCount) == 1
+                            expect(testContext.changedFlag) == targetChangedFlag
+                            let newValue = testContext.changedFlag?.newValue as? String
+                            let newValueFromChangedFlag = targetChangedFlag?.newValue as? String
+                            if newValue != nil || newValueFromChangedFlag != nil {
+                                expect(newValue) == newValueFromChangedFlag
+                            }
                             expect(testContext.flagsUnchangedHandlerCallCount) == 0
                         }
                     }