From d9804ec15caa93b5d7612402f4b80496aa57f1bf Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 22 Oct 2024 14:52:04 -0700 Subject: [PATCH] feat: add support for client-side prerequisite events (#201) This commit updates `AllFlagsState` to track prerequisite evaluations. This didn't require modifying the eval module, as it already exposes a `PrerequisiteEventRecorder` interface with the necessary info. The `AllFlagsState` public API allows for fetching a flag's details (`FlagState`) from the top-level `AllFlags` object. This struct now has prerequisite information exposed. Additionally when the `AllFlags` is marshaled to JSON, it will now contain the prerequisite relationships for each flag. --- .github/workflows/common_ci.yml | 2 +- interfaces/flagstate/flags_state.go | 11 + interfaces/flagstate/flags_state_test.go | 63 ++++- ldclient.go | 8 +- ldclient_evaluation_all_flags_test.go | 285 ++++++++++++++++++++++- testservice/service.go | 1 + testservice/servicedef/service_params.go | 1 + 7 files changed, 366 insertions(+), 5 deletions(-) diff --git a/.github/workflows/common_ci.yml b/.github/workflows/common_ci.yml index 0b078aa9..1cd93a70 100644 --- a/.github/workflows/common_ci.yml +++ b/.github/workflows/common_ci.yml @@ -42,7 +42,7 @@ jobs: run: make workspace - name: Start test service in background run: make start-contract-test-service-bg - - uses: launchdarkly/gh-actions/actions/contract-tests@contract-tests-v1.0.0 + - uses: launchdarkly/gh-actions/actions/contract-tests@contract-tests-v1.1.0 continue-on-error: true with: test_service_port: ${{ env.TEST_SERVICE_PORT }} diff --git a/interfaces/flagstate/flags_state.go b/interfaces/flagstate/flags_state.go index dc1290f6..23dedec0 100644 --- a/interfaces/flagstate/flags_state.go +++ b/interfaces/flagstate/flags_state.go @@ -67,6 +67,10 @@ type FlagState struct { // OmitDetails is true if, based on the options passed to AllFlagsState and the flag state, some of the // metadata can be left out of the JSON representation. OmitDetails bool + + // Prerequisites is an ordered list of direct prerequisites that were evaluated in the process of evaluating this + // flag. + Prerequisites []string } // Option is the interface for optional parameters that can be passed to LDClient.AllFlagsState. @@ -156,6 +160,13 @@ func (a AllFlags) MarshalJSON() ([]byte, error) { flagObj.Maybe("trackEvents", flag.TrackEvents).Bool(flag.TrackEvents) flagObj.Maybe("trackReason", flag.TrackReason).Bool(flag.TrackReason) flagObj.Maybe("debugEventsUntilDate", flag.DebugEventsUntilDate > 0).Float64(float64(flag.DebugEventsUntilDate)) + if len(flag.Prerequisites) > 0 { + prerequisites := flagObj.Name("prerequisites").Array() + for _, p := range flag.Prerequisites { + prerequisites.String(p) + } + prerequisites.End() + } flagObj.End() } stateObj.End() diff --git a/interfaces/flagstate/flags_state_test.go b/interfaces/flagstate/flags_state_test.go index 1178f595..3e02f2e5 100644 --- a/interfaces/flagstate/flags_state_test.go +++ b/interfaces/flagstate/flags_state_test.go @@ -99,6 +99,7 @@ func TestAllFlagsJSON(t *testing.T) { Reason: ldreason.NewEvalReasonFallthrough(), TrackEvents: true, DebugEventsUntilDate: ldtime.UnixMillisecondTime(100000), + Prerequisites: []string{"flag2", "flag3", "flag4"}, }, }, } @@ -109,7 +110,8 @@ func TestAllFlagsJSON(t *testing.T) { "$valid":true, "flag1": "value1", "$flagsState":{ - "flag1": {"variation":1,"version":1000,"reason":{"kind":"FALLTHROUGH"},"trackEvents":true,"debugEventsUntilDate":100000} + "flag1": {"variation":1,"version":1000,"reason":{"kind":"FALLTHROUGH"},"trackEvents":true,"debugEventsUntilDate":100000, + "prerequisites": ["flag2","flag3","flag4"]} } }`, string(bytes)) }) @@ -140,7 +142,7 @@ func TestAllFlagsJSON(t *testing.T) { }`, string(bytes)) }) - t.Run("omitting details", func(t *testing.T) { + t.Run("omitting details, no prerequisites present", func(t *testing.T) { a := AllFlags{ valid: true, flags: map[string]FlagState{ @@ -162,6 +164,32 @@ func TestAllFlagsJSON(t *testing.T) { "$flagsState":{ "flag1": {"variation":1} } +}`, string(bytes)) + }) + + t.Run("omitting details, prerequisites present", func(t *testing.T) { + a := AllFlags{ + valid: true, + flags: map[string]FlagState{ + "flag1": { + Value: ldvalue.String("value1"), + Variation: ldvalue.NewOptionalInt(1), + Version: 1000, + Reason: ldreason.NewEvalReasonFallthrough(), + OmitDetails: true, + Prerequisites: []string{"flag2", "flag3", "flag4"}, + }, + }, + } + bytes, err := a.MarshalJSON() + assert.NoError(t, err) + assert.JSONEq(t, + `{ + "$valid":true, + "flag1": "value1", + "$flagsState":{ + "flag1": {"variation":1, "prerequisites": ["flag2","flag3","flag4"]} + } }`, string(bytes)) }) } @@ -295,6 +323,37 @@ func TestAllFlagsBuilder(t *testing.T) { "flag5": flag5, }, a.flags) }) + + t.Run("add flags with prerequisites", func(t *testing.T) { + b := NewAllFlagsBuilder() + + flag1 := FlagState{ + Value: ldvalue.String("value1"), + Variation: ldvalue.NewOptionalInt(1), + Version: 1000, + Prerequisites: []string{"flag2"}, + } + flag2 := FlagState{ + Value: ldvalue.String("value2"), + Version: 2000, + Prerequisites: []string{"flag3"}, + } + flag3 := FlagState{ + Value: ldvalue.String("value3"), + Version: 3000, + } + + b.AddFlag("flag1", flag1) + b.AddFlag("flag2", flag2) + b.AddFlag("flag3", flag3) + + a := b.Build() + assert.Equal(t, map[string]FlagState{ + "flag1": flag1, + "flag2": flag2, + "flag3": flag3, + }, a.flags) + }) } func TestAllFlagsOptions(t *testing.T) { diff --git a/ldclient.go b/ldclient.go index 2dabf92b..b40c88b0 100644 --- a/ldclient.go +++ b/ldclient.go @@ -684,7 +684,12 @@ func (client *LDClient) AllFlagsState(context ldcontext.Context, options ...flag continue } - result := client.evaluator.Evaluate(flag, context, nil) + var prerequisites []string + result := client.evaluator.Evaluate(flag, context, func(event ldeval.PrerequisiteFlagEvent) { + if event.TargetFlagKey == flag.Key { + prerequisites = append(prerequisites, event.PrerequisiteFlag.Key) + } + }) state.AddFlag( item.Key, @@ -696,6 +701,7 @@ func (client *LDClient) AllFlagsState(context ldcontext.Context, options ...flag TrackEvents: flag.TrackEvents || result.IsExperiment, TrackReason: result.IsExperiment, DebugEventsUntilDate: flag.DebugEventsUntilDate, + Prerequisites: prerequisites, }, ) } diff --git a/ldclient_evaluation_all_flags_test.go b/ldclient_evaluation_all_flags_test.go index 869055a6..4ead0168 100644 --- a/ldclient_evaluation_all_flags_test.go +++ b/ldclient_evaluation_all_flags_test.go @@ -2,6 +2,7 @@ package ldclient import ( "errors" + "github.com/stretchr/testify/require" "testing" "github.com/launchdarkly/go-server-sdk/v7/internal/sharedtest/mocks" @@ -128,7 +129,7 @@ func TestAllFlagsStateCanFilterForOnlyClientSideFlags(t *testing.T) { func TestAllFlagsStateCanOmitDetailForUntrackedFlags(t *testing.T) { futureTime := ldtime.UnixMillisNow() + 100000 - // flag1 does not get full detials because neither event tracking nor debugging is on and there's no experiment + // flag1 does not get full details because neither event tracking nor debugging is on and there's no experiment flag1 := ldbuilders.NewFlagBuilder("key1").Version(100).OffVariation(0).Variations(ldvalue.String("value1")).Build() // flag2 gets full details because event tracking is on @@ -246,3 +247,285 @@ func TestAllFlagsStateReturnsInvalidStateIfStoreReturnsError(t *testing.T) { assert.Len(t, mockLoggers.GetOutput(ldlog.Warn), 1) assert.Contains(t, mockLoggers.GetOutput(ldlog.Warn)[0], "Unable to fetch flags") } + +func TestAllFlagsStateReturnsPrerequisites(t *testing.T) { + + // Creates a boolean flag that is on (true). + booleanFlag := func(key string) *ldbuilders.FlagBuilder { + return ldbuilders.NewFlagBuilder(key). + Version(100). + On(true). + OffVariation(0). + FallthroughVariation(1). + Variations(ldvalue.Bool(false), ldvalue.Bool(true)) + } + + t.Run("only top-level (direct) prerequisites are included in the prerequisites field", func(t *testing.T) { + + // Toplevel has one direct prerequisite. + toplevel := booleanFlag("toplevel"). + AddPrerequisite("prereq1", 1).Build() + + // Prereq1 also has one direct prerequisite. + prereq1 := booleanFlag("prereq1"). + AddPrerequisite("prereq2", 1).Build() + + // Prereq2 has no prerequisites itself. + prereq2 := booleanFlag("prereq2").Build() + + // We expect that toplevel and prereq1 should list only their direct prerequisites. That is, + // toplevel should not list [prereq1, prereq2], but only [prereq1]. + + withClientEvalTestParams(func(p clientEvalTestParams) { + p.data.UsePreconfiguredFlag(toplevel) + p.data.UsePreconfiguredFlag(prereq1) + p.data.UsePreconfiguredFlag(prereq2) + + state := p.client.AllFlagsState(lduser.NewUser("userkey")) + require.True(t, state.IsValid()) + + toplevelState, ok := state.GetFlag("toplevel") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq1"}, toplevelState.Prerequisites) + } + + prereq1State, ok := state.GetFlag("prereq1") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq2"}, prereq1State.Prerequisites) + } + + prereq2State, ok := state.GetFlag("prereq2") + if assert.True(t, ok) { + assert.Empty(t, prereq2State.Prerequisites) + } + }) + }) + + t.Run("the prerequisites field should hold prerequisites in evaluation order", func(t *testing.T) { + + // These tests ensure all direct prerequisites of a flag (that is on) are included in the + // prerequisites field. The sub-tests are a sanity check to make sure we're not sorting the array + // accidentally - the array should follow eval order. + t.Run("depth one, all on", func(t *testing.T) { + t.Run("ascending alphabetic", func(t *testing.T) { + toplevel := booleanFlag("toplevel"). + AddPrerequisite("prereq1", 1). + AddPrerequisite("prereq2", 1). + AddPrerequisite("prereq3", 1).Build() + + prereq1 := booleanFlag("prereq1").Build() + prereq2 := booleanFlag("prereq2").Build() + prereq3 := booleanFlag("prereq3").Build() + + withClientEvalTestParams(func(p clientEvalTestParams) { + p.data.UsePreconfiguredFlag(toplevel) + p.data.UsePreconfiguredFlag(prereq1) + p.data.UsePreconfiguredFlag(prereq2) + p.data.UsePreconfiguredFlag(prereq3) + + state := p.client.AllFlagsState(lduser.NewUser("userkey")) + require.True(t, state.IsValid()) + + toplevelState, ok := state.GetFlag("toplevel") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq1", "prereq2", "prereq3"}, toplevelState.Prerequisites) + } + }) + }) + + t.Run("descending alphabetic", func(t *testing.T) { + toplevel := booleanFlag("toplevel"). + AddPrerequisite("prereq3", 1). + AddPrerequisite("prereq2", 1). + AddPrerequisite("prereq1", 1).Build() + + prereq1 := booleanFlag("prereq1").Build() + prereq2 := booleanFlag("prereq2").Build() + prereq3 := booleanFlag("prereq3").Build() + + withClientEvalTestParams(func(p clientEvalTestParams) { + p.data.UsePreconfiguredFlag(toplevel) + p.data.UsePreconfiguredFlag(prereq1) + p.data.UsePreconfiguredFlag(prereq2) + p.data.UsePreconfiguredFlag(prereq3) + + state := p.client.AllFlagsState(lduser.NewUser("userkey")) + require.True(t, state.IsValid()) + + toplevelState, ok := state.GetFlag("toplevel") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq3", "prereq2", "prereq1"}, toplevelState.Prerequisites) + } + }) + }) + }) + + t.Run("depth three, toplevel flag is off", func(t *testing.T) { + toplevel := booleanFlag("toplevel"). + AddPrerequisite("prereq1", 1).On(false).Build() + + prereq1 := booleanFlag("prereq1"). + AddPrerequisite("prereq2", 1).Build() + + prereq2 := booleanFlag("prereq2"). + AddPrerequisite("prereq3", 1).Build() + + prereq3 := booleanFlag("prereq3").Build() + + withClientEvalTestParams(func(p clientEvalTestParams) { + p.data.UsePreconfiguredFlag(toplevel) + p.data.UsePreconfiguredFlag(prereq1) + p.data.UsePreconfiguredFlag(prereq2) + p.data.UsePreconfiguredFlag(prereq3) + + state := p.client.AllFlagsState(lduser.NewUser("userkey")) + require.True(t, state.IsValid()) + + // If toplevel were on, then we'd expect to see the single prerequisite. Since it's not, we shouldn't + // see any. + toplevelState, ok := state.GetFlag("toplevel") + if assert.True(t, ok) { + assert.Empty(t, toplevelState.Prerequisites) + } + + // The other prerequisites are themselves top-level flags when evaluated, so they should have their + // own prerequisites included since they're all on. + prereq1State, ok := state.GetFlag("prereq1") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq2"}, prereq1State.Prerequisites) + } + + prereq2State, ok := state.GetFlag("prereq2") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq3"}, prereq2State.Prerequisites) + } + + // Prereq1 had no prereqs. + prereq3State, ok := state.GetFlag("prereq3") + if assert.True(t, ok) { + assert.Empty(t, prereq3State.Prerequisites) + } + }) + }) + + t.Run("depth three, first prerequisite is off", func(t *testing.T) { + toplevel := booleanFlag("toplevel"). + AddPrerequisite("prereq1", 1).Build() + + prereq1 := booleanFlag("prereq1"). + AddPrerequisite("prereq2", 1).On(false).Build() + + prereq2 := booleanFlag("prereq2"). + AddPrerequisite("prereq3", 1).Build() + + prereq3 := booleanFlag("prereq3").Build() + + withClientEvalTestParams(func(p clientEvalTestParams) { + p.data.UsePreconfiguredFlag(toplevel) + p.data.UsePreconfiguredFlag(prereq1) + p.data.UsePreconfiguredFlag(prereq2) + p.data.UsePreconfiguredFlag(prereq3) + + state := p.client.AllFlagsState(lduser.NewUser("userkey")) + require.True(t, state.IsValid()) + + toplevelState, ok := state.GetFlag("toplevel") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq1"}, toplevelState.Prerequisites) + } + + prereq1State, ok := state.GetFlag("prereq1") + if assert.True(t, ok) { + assert.Empty(t, prereq1State.Prerequisites) + } + + prereq2State, ok := state.GetFlag("prereq2") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq3"}, prereq2State.Prerequisites) + } + + prereq3State, ok := state.GetFlag("prereq3") + if assert.True(t, ok) { + assert.Empty(t, prereq3State.Prerequisites) + } + }) + }) + + t.Run("depth three, second prerequisite is off", func(t *testing.T) { + toplevel := booleanFlag("toplevel"). + AddPrerequisite("prereq1", 1).Build() + + prereq1 := booleanFlag("prereq1"). + AddPrerequisite("prereq2", 1).Build() + + prereq2 := booleanFlag("prereq2"). + AddPrerequisite("prereq3", 1).On(false).Build() + + prereq3 := booleanFlag("prereq3").Build() + + withClientEvalTestParams(func(p clientEvalTestParams) { + p.data.UsePreconfiguredFlag(toplevel) + p.data.UsePreconfiguredFlag(prereq1) + p.data.UsePreconfiguredFlag(prereq2) + p.data.UsePreconfiguredFlag(prereq3) + + state := p.client.AllFlagsState(lduser.NewUser("userkey")) + require.True(t, state.IsValid()) + + toplevelState, ok := state.GetFlag("toplevel") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq1"}, toplevelState.Prerequisites) + } + + prereq1State, ok := state.GetFlag("prereq1") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq2"}, prereq1State.Prerequisites) + } + + prereq2State, ok := state.GetFlag("prereq2") + if assert.True(t, ok) { + assert.Empty(t, prereq2State.Prerequisites) + } + + prereq3State, ok := state.GetFlag("prereq3") + if assert.True(t, ok) { + assert.Empty(t, prereq3State.Prerequisites) + } + }) + }) + + }) + + t.Run("prerequisite flags not visible to client SDKs should still be referenced in visible flags", func(t *testing.T) { + toplevel := booleanFlag("toplevel"). + ClientSideUsingEnvironmentID(true). + AddPrerequisite("prereq1", 1). + AddPrerequisite("prereq2", 1).Build() + + prereq1 := booleanFlag("prereq1"). + ClientSideUsingEnvironmentID(false).Build() + + prereq2 := booleanFlag("prereq2"). + ClientSideUsingEnvironmentID(false).Build() + + withClientEvalTestParams(func(p clientEvalTestParams) { + p.data.UsePreconfiguredFlag(toplevel) + p.data.UsePreconfiguredFlag(prereq1) + p.data.UsePreconfiguredFlag(prereq2) + + state := p.client.AllFlagsState(lduser.NewUser("userkey"), flagstate.OptionClientSideOnly()) + require.True(t, state.IsValid()) + + toplevelState, ok := state.GetFlag("toplevel") + if assert.True(t, ok) { + assert.Equal(t, []string{"prereq1", "prereq2"}, toplevelState.Prerequisites) + } + + _, ok = state.GetFlag("prereq1") + assert.False(t, ok) + + _, ok = state.GetFlag("prereq2") + assert.False(t, ok) + }) + }) +} diff --git a/testservice/service.go b/testservice/service.go index b74ae4aa..2b348f89 100644 --- a/testservice/service.go +++ b/testservice/service.go @@ -43,6 +43,7 @@ var capabilities = []string{ servicedef.CapabilityOmitAnonymousContexts, servicedef.CapabilityEventGzip, servicedef.CapabilityOptionalEventGzip, + servicedef.CapabilityClientPrereqEvents, } // gets the specified environment variable, or the default if not set diff --git a/testservice/servicedef/service_params.go b/testservice/servicedef/service_params.go index 5e9d6313..b82d6e18 100644 --- a/testservice/servicedef/service_params.go +++ b/testservice/servicedef/service_params.go @@ -24,6 +24,7 @@ const ( CapabilityOmitAnonymousContexts = "omit-anonymous-contexts" CapabilityEventGzip = "event-gzip" CapabilityOptionalEventGzip = "optional-event-gzip" + CapabilityClientPrereqEvents = "client-prereq-events" ) type StatusRep struct {