launchdarkly / java-server-sdk

LaunchDarkly Server-Side SDK for Java
Other
83 stars 56 forks source link

`isInitialized` & `dataStore.isInitialized()` #280

Closed t3hnar closed 2 years ago

t3hnar commented 2 years ago

Hey guys, I'm going through code in order to assess java client before adding is as dependency

I can see repeated pattern of two checks like:

if (!isInitialized()) { <-- first check
      if (dataStore.isInitialized()) { <-- second check
        evaluationLogger.warn("allFlagsState() was called before client initialized; using last known values from data store");
      } else {
        evaluationLogger.warn("allFlagsState() was called before client initialized; data store unavailable, returning no data");
        return builder.valid(false).build();
      }
    }

https://github.com/launchdarkly/java-server-sdk/blob/main/src/main/java/com/launchdarkly/sdk/server/LDClient.java#L341

however

public boolean isInitialized() {
    return dataSource.isInitialized();
  }

This should be either bug or I must be missing something. Could you please help?

eli-darkly commented 2 years ago

You didn't say why it looks like a bug to you, so I don't know whether you're missing something, but here's a summary of what the intention is:

eli-darkly commented 2 years ago

So, the specific scenarios that that first block is there to handle are:

  1. The SDK has not (yet) completed a successful initialization because it has not been able to connect to LaunchDarkly. But, when it calls dataStore.isInitialized(), it returns true, indicating that we are using a database and we detected some flag data already in the database (either from a previous run of the application, or put there by another process such as the Relay Proxy). So, we use that data, and we print a warning ("using last known values from data store").
  2. Or, the SDK has not yet completed a successful initialization, and there is not already data in the data store. So we have no data, there is no way to evaluate flags, and we print a different warning ("data store unavailable").
t3hnar commented 2 years ago

ok makes sense, I was confused by similar naming [dataS]tore with [dataS]ource, and thought that those both are the same thing.

I'm closing the issue now.