launchdarkly/java-server-sdk

FlagTracker does not pick up flag changes in flagdata.json

seangatewood opened this issue · 5 comments

Describe the bug
When reading flags from a file, any listeners registered using addFlagValueChangeListener are not triggered when flags in the file change, unless you also change the flags' version property. We believe this is not ideal if the flag is in the flags map, and is not possible if the flag is in the flagValues map.

To reproduce
I made a quick test:

  @Test
  void test_flagTracker_calls_listener_when_reading_from_file() {
    String filepath = System.getProperty("user.home") + "/flagdata.json";
    LDClient ldClient = getLdClient(filepath);
    FlagTracker tracker = ldClient.getFlagTracker();
    LDUser ldUser = new LDUser("key");
    String flagKey = "my-boolean-flag-key";

    AtomicBoolean listenerWasCalled = new AtomicBoolean(false);

    tracker.addFlagValueChangeListener(flagKey, ldUser, changeEvent -> {
      listenerWasCalled.set(true);
    });
    
    boolean initialValue = ldClient.boolVariation(flagKey, ldUser, false);
    setFeatureInJsonFile(flagKey, !initialValue, filepath);
    await().atMost(15, TimeUnit.SECONDS).until(() -> ldClient.boolVariation(flagKey, ldUser, false) != initialValue);

    assertThat(listenerWasCalled.get()).isTrue(); // fails
  }

  private LDClient getLdClient(String filepath) {
    LDConfig config = new LDConfig.Builder()
        .dataSource(FileData.dataSource().filePaths(filepath).autoUpdate(true))
        .events(Components.noEvents())
        .build();
    return new LDClient("nonnull sdk key", config);
  }

Only the last assertion fails, with the following in ~/flagdata.json:

{
  "flags": {
    "flag-key-1": {
      "key": "flag-key-1",
      "on": true,
      "variations": [
        "a",
        "b"
      ],
      "version": 1
    }
  },
  "segments": {
    "segment-key-1": {
      "key": "segment-key-1",
      "includes": [
        "user-key-1"
      ],
      "version": 1
    }
  },
  "flagValues": {
    "my-string-flag-key": "value-1",
    "my-boolean-flag-key": true,
    "my-integer-flag-key": 3
  }
}

If you set a breakpoint here, you can see there is a difference between the flag in oldData vs in fullDataSetToMap(allData), even though the versions are the same.

oldData

image

fullDataSetToMap(allData)

image

So there are no changed items sent into sendChangeEvents:
image

Expected behavior
It would be nice if we could detect the change there in computeChangedItemsForFullDataSet, so the Tracker can pick up flag changes in the json file.

Logs
(I just see this from the client)

[Test worker] INFO com.launchdarkly.sdk.server.LDClient - Waiting up to 5000 milliseconds for LaunchDarkly client to start...

SDK version
We are using version 5.2.2

Language version, developer tools
Java 8

OS/platform
MacOS 10.15.7

Additional context
We are using the "Reading flags from a file" feature to provide our developers with an isolated environment for testing/prototyping purposes. If this can't be fixed in the SDK, we may have to set up our own polling to detect changes in the file to enable our developers to test features that update their state with change listeners.

You're right that the mechanism FileData is currently using to inject flag data does not play well with FlagTracker, because it's not providing version numbers. I don't think we would want to address this by making FlagTracker diff the entire flag configuration to detect changes even if the version number hasn't changed— flag configurations can be very large. But it might not be too hard to make FileData generate version numbers; I'll look into it.

The fix I'm thinking of would cause the SDK to signal a flag change for every flag whenever the data file was updated (since the file data source would set all of the flag versions to 1 on the first load, 2 on the second load, etc.). So if you were using addFlagChangeListener (not addFlagValueChangeListener), you would get false positives— which are already inherent in the LaunchDarkly update model, since there are lots of ways minor details of a flag configuration could change that wouldn't actually be significant. But using addFlagValueChangeListener as you're doing would correctly only notify you of changes that really affect the value.

Thank you for the quick response! Your solution does sound better to me, and would probably be more localized within FileData as well. 🙂

We've released version 5.2.3, which should fix this.

Verified on my end. 🎉 Thank you!