Skip to content

Commit 68599fa

Browse files
authored
fix: align fdv2 intializer to spec (#330)
**Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/v5/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Describe the solution you've provided** This PR will make it so that the initializers will keep cycling to the first intializer to report a valid basis + selector OR if there are no initializers left. In this case, the synchronizers will start. In this new flow, synchronizers could put the client into a valid state. Initialization phase will no longer throw an error unless the initializers timeout. **Describe alternatives you've considered** We may want to introduce an option for customers to enable a more strict initialization flow where the client will terminate if all initializers fail, but that would need an addendum to the specs and a new PR. **Additional context** Add any other context about the pull request here. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > FDv2 now only marks ready when an initializer provides a selector, updates full-dataset change detection to consider version increases, makes file data source emit NoSelector, and updates/extends tests including a new e2e deferral case. > > - **FDv2 core**: > - Initializers now mark ready only if `basis.ChangeSet.Selector().IsDefined()`; otherwise continue to synchronizers. > - **Store**: > - `computeChangedItemsForFullDataSet` now flags affected items when `oldItem.Version < newItem.Version` in addition to add/remove. > - **File data source (v2)**: > - Change sets are finished with `subsystems.NoSelector()`; remove selector/version usage. > - **Tests**: > - Update file data source tests to expect `NoSelector`. > - Add `TestFDV2FileInitializerWillDeferToFirstSynchronizer` end-to-end test validating deferral to the first synchronizer. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e37a5a1. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent dd83aba commit 68599fa

File tree

5 files changed

+75
-19
lines changed

5 files changed

+75
-19
lines changed

internal/datasystem/fdv2_datasystem.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -251,12 +251,14 @@ func (f *FDv2) runInitializers(ctx context.Context, closeWhenReady chan struct{}
251251
continue
252252
}
253253
f.environmentIDProvider.SetEnvironmentID(basis.EnvironmentID)
254-
f.loggers.Infof("Initialized via %s", initializer.Name())
255254
f.store.Apply(basis.ChangeSet, basis.Persist)
256-
f.readyOnce.Do(func() {
257-
close(closeWhenReady)
258-
})
259-
return
255+
if basis.ChangeSet.Selector().IsDefined() {
256+
f.loggers.Infof("Initialized via %s", initializer.Name())
257+
f.readyOnce.Do(func() {
258+
close(closeWhenReady)
259+
})
260+
return
261+
}
260262
}
261263
}
262264

internal/datasystem/store.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -399,10 +399,11 @@ func (s *Store) computeChangedItemsForFullDataSet(
399399
}
400400
}
401401
for _, key := range allKeys {
402-
_, haveOld := oldItems[key]
403-
_, haveNew := newItems[key]
402+
oldItem, haveOld := oldItems[key]
403+
newItem, haveNew := newItems[key]
404+
404405
if haveOld || haveNew {
405-
if !haveOld || !haveNew {
406+
if !haveOld || !haveNew || oldItem.Version < newItem.Version {
406407
s.dependencyTracker.addAffectedItems(affectedItems, toposort.NewVertex(kind, key))
407408
}
408409
}

ldclient_end_to_end_fdv2_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/launchdarkly/go-sdk-common/v3/ldlog"
1111
"github.com/launchdarkly/go-server-sdk/v7/internal/sharedtest"
12+
"github.com/launchdarkly/go-server-sdk/v7/ldfiledatav2"
1213
"github.com/launchdarkly/go-server-sdk/v7/subsystems"
1314
"github.com/launchdarkly/go-server-sdk/v7/testhelpers/ldservicesv2"
1415

@@ -17,6 +18,7 @@ import (
1718
"github.com/launchdarkly/go-server-sdk/v7/ldcomponents"
1819
"github.com/launchdarkly/go-server-sdk/v7/testhelpers/ldservices"
1920

21+
testHelpers "github.com/launchdarkly/go-test-helpers/v3"
2022
"github.com/launchdarkly/go-test-helpers/v3/httphelpers"
2123

2224
"github.com/stretchr/testify/assert"
@@ -354,3 +356,49 @@ func TestFDV2StreamingSynchronizerTimesOut(t *testing.T) {
354356
assert.Len(t, logCapture.GetOutput(ldlog.Error), 0)
355357
})
356358
}
359+
360+
func TestFDV2FileInitializerWillDeferToFirstSynchronizer(t *testing.T) {
361+
data := ldservicesv2.NewServerSDKData().Flags(alwaysTrueFlag)
362+
363+
protocol := ldservicesv2.NewStreamingProtocol().
364+
WithIntent(subsystems.ServerIntent{Payload: subsystems.Payload{
365+
ID: "fake-id", Target: 0, Code: subsystems.IntentTransferFull, Reason: "payload-missing",
366+
}}).
367+
WithPutObjects(data.ToPutObjects()).
368+
WithTransferred("state", 1)
369+
370+
streamHandler, _ := ldservices.ServerSideStreamingV2ServiceProtocolHandler(protocol)
371+
372+
handler, requestsCh := httphelpers.RecordingHandler(streamHandler)
373+
374+
testHelpers.WithTempFileData([]byte(`{"flags": {"`+alwaysFalseFlag.Key+`": {"on": false}}, "segments": {}}`), func(filename string) {
375+
376+
httphelpers.WithServer(handler, func(server *httptest.Server) {
377+
logCapture := ldlogtest.NewMockLog()
378+
379+
config := Config{
380+
Logging: ldcomponents.Logging().Loggers(logCapture.Loggers),
381+
DataSystem: ldcomponents.DataSystem().Custom().
382+
Initializers(
383+
ldfiledatav2.DataSource().FilePaths(filename).AsInitializer(),
384+
).
385+
Synchronizers(
386+
ldcomponents.StreamingDataSourceV2().BaseURI(server.URL),
387+
nil,
388+
),
389+
}
390+
391+
client, err := MakeCustomClient(testSdkKey, config, time.Second*5)
392+
393+
<-requestsCh
394+
395+
require.NoError(t, err)
396+
defer client.Close()
397+
398+
assert.Equal(t, string(interfaces.DataSourceStateValid), string(client.GetDataSourceStatusProvider().GetStatus().State))
399+
400+
value, _ := client.BoolVariation(alwaysTrueFlag.Key, testUser, false)
401+
assert.True(t, value)
402+
})
403+
})
404+
}

ldfiledatav2/file_data_source_impl.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"os"
99
"path/filepath"
10-
"strconv"
1110
"strings"
1211
"sync"
1312
"sync/atomic"
@@ -29,7 +28,9 @@ import (
2928
type fileDataSource struct {
3029
changeSetBroadcaster *internal.Broadcaster[subsystems.ChangeSet]
3130
statusBroadcaster *internal.Broadcaster[interfaces.DataSynchronizerStatus]
32-
version int
31+
// NOTE: this is not really used anymore because file data sources at this
32+
// moment will not report a selector.
33+
version int
3334

3435
absFilePaths []string
3536
duplicateKeysHandling DuplicateKeysHandling
@@ -349,7 +350,11 @@ func mergeFileData(
349350
}
350351
}
351352

352-
return builder.Finish(subsystems.NewSelector(strconv.Itoa(version), version))
353+
// File data source will not have a selector for now.
354+
// NOTE: If we start supporting FDv2 data from file then this statement might change.
355+
// When that happens we will construct the selector the same way that we construct it
356+
// in the FDv2 polling data source.
357+
return builder.Finish(subsystems.NoSelector())
353358
}
354359

355360
func makeFlagWithValue(key string, v interface{}) *ldmodel.FeatureFlag {

ldfiledatav2/file_data_source_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func TestSuccessfullyLoadsJsonFlags(t *testing.T) {
3434
assert.Contains(t, keys, "my-flag")
3535
assert.Contains(t, keys, "my-segment")
3636

37-
assert.Equal(t, 1, result.ChangeSet.Selector().Version())
37+
assert.Equal(t, subsystems.NoSelector(), result.ChangeSet.Selector())
3838
assert.Equal(t, subsystems.IntentTransferFull, result.ChangeSet.IntentCode())
3939
assert.Equal(t, interfaces.DataSourceStateValid, result.State)
4040
assert.Equal(t, interfaces.DataSourceErrorInfo{}, result.Error)
@@ -54,7 +54,7 @@ func TestSuccessfullyLoadsJsonFlagValues(t *testing.T) {
5454
assert.NotNil(t, result.ChangeSet)
5555
assert.Len(t, result.ChangeSet.Changes(), 1)
5656
assert.Equal(t, "my-flag", result.ChangeSet.Changes()[0].Key)
57-
assert.Equal(t, 1, result.ChangeSet.Selector().Version())
57+
assert.Equal(t, subsystems.NoSelector(), result.ChangeSet.Selector())
5858
assert.Equal(t, subsystems.IntentTransferFull, result.ChangeSet.IntentCode())
5959
assert.Equal(t, interfaces.DataSourceStateValid, result.State)
6060
assert.Equal(t, interfaces.DataSourceErrorInfo{}, result.Error)
@@ -80,7 +80,7 @@ flags:
8080
assert.NotNil(t, result.ChangeSet)
8181
assert.Len(t, result.ChangeSet.Changes(), 1)
8282
assert.Equal(t, "my-flag", result.ChangeSet.Changes()[0].Key)
83-
assert.Equal(t, 1, result.ChangeSet.Selector().Version())
83+
assert.Equal(t, subsystems.NoSelector(), result.ChangeSet.Selector())
8484
assert.Equal(t, subsystems.IntentTransferFull, result.ChangeSet.IntentCode())
8585
assert.Equal(t, interfaces.DataSourceStateValid, result.State)
8686
assert.Equal(t, interfaces.DataSourceErrorInfo{}, result.Error)
@@ -105,7 +105,7 @@ flagValues:
105105
assert.NotNil(t, result.ChangeSet)
106106
assert.Len(t, result.ChangeSet.Changes(), 1)
107107
assert.Equal(t, "my-flag", result.ChangeSet.Changes()[0].Key)
108-
assert.Equal(t, 1, result.ChangeSet.Selector().Version())
108+
assert.Equal(t, subsystems.NoSelector(), result.ChangeSet.Selector())
109109
assert.Equal(t, subsystems.IntentTransferFull, result.ChangeSet.IntentCode())
110110
assert.Equal(t, interfaces.DataSourceStateValid, result.State)
111111
assert.Equal(t, interfaces.DataSourceErrorInfo{}, result.Error)
@@ -133,7 +133,7 @@ func TestSuccessfullyLoadsJsonFlagsFromMultipleFiles(t *testing.T) {
133133
assert.Contains(t, keys, "my-flag")
134134
assert.Contains(t, keys, "your-flag")
135135

136-
assert.Equal(t, 1, result.ChangeSet.Selector().Version())
136+
assert.Equal(t, subsystems.NoSelector(), result.ChangeSet.Selector())
137137
assert.Equal(t, subsystems.IntentTransferFull, result.ChangeSet.IntentCode())
138138
assert.Equal(t, interfaces.DataSourceStateValid, result.State)
139139
assert.Equal(t, interfaces.DataSourceErrorInfo{}, result.Error)
@@ -183,7 +183,7 @@ func TestIsValidWhenLoadingConflictingFilesWhenIgnoringDuplicates(t *testing.T)
183183
// It uses the first duplicate key.
184184
assert.Equal(t, 1, keys["my-flag"].Version)
185185

186-
assert.Equal(t, 1, result.ChangeSet.Selector().Version())
186+
assert.Equal(t, subsystems.NoSelector(), result.ChangeSet.Selector())
187187
assert.Equal(t, subsystems.IntentTransferFull, result.ChangeSet.IntentCode())
188188
assert.Equal(t, interfaces.DataSourceStateValid, result.State)
189189
assert.Equal(t, interfaces.DataSourceErrorInfo{}, result.Error)
@@ -238,7 +238,7 @@ func TestRecoversFromInvalidDataWhenFileUpdated(t *testing.T) {
238238
assert.NotNil(t, result.ChangeSet)
239239
assert.Len(t, result.ChangeSet.Changes(), 1)
240240
assert.Equal(t, "my-flag", result.ChangeSet.Changes()[0].Key)
241-
assert.Equal(t, 1, result.ChangeSet.Selector().Version())
241+
assert.Equal(t, subsystems.NoSelector(), result.ChangeSet.Selector())
242242
assert.Equal(t, subsystems.IntentTransferFull, result.ChangeSet.IntentCode())
243243
assert.Equal(t, interfaces.DataSourceStateValid, result.State)
244244
})
@@ -297,7 +297,7 @@ func TestSuccessfullyLoadsJsonFlagsAsInitializer(t *testing.T) {
297297
assert.Contains(t, keys, "my-flag")
298298
assert.Contains(t, keys, "my-segment")
299299

300-
assert.Equal(t, 1, basis.ChangeSet.Selector().Version())
300+
assert.Equal(t, subsystems.NoSelector(), basis.ChangeSet.Selector())
301301
assert.Equal(t, subsystems.IntentTransferFull, basis.ChangeSet.IntentCode())
302302
})
303303
}

0 commit comments

Comments
 (0)