prepare 6.9.1 release (#269)

## [6.9.1] - 2023-10-19 ### Fixed: - The big segments synchronization process will now handle empty versions correctly when using DynamoDB. --------- Co-authored-by: Eli Bishop <[email protected]> Co-authored-by: LaunchDarklyCI <[email protected]> Co-authored-by: hroederld <[email protected]> Co-authored-by: LaunchDarklyReleaseBot <[email protected]> Co-authored-by: Dan Richelson <[email protected]> Co-authored-by: Dan Richelson <[email protected]> Co-authored-by: Ben Woskow <[email protected]> Co-authored-by: Ben Woskow <[email protected]> Co-authored-by: Louis Chan <[email protected]> Co-authored-by: Louis Chan <[email protected]> Co-authored-by: Moshe Good <[email protected]> Co-authored-by: Moshe Good <[email protected]> Co-authored-by: Casey Waldren <[email protected]> Co-authored-by: Matthew M. Keeler <[email protected]> Co-authored-by: Ryan Lamb <[email protected]>
launchdarkly · Oct 20, 2023 · b357983 · b357983
1 parent cf39019
commit b357983
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 13 deletions.
diff --git a/internal/core/bigsegments/store_common_test.go b/internal/core/bigsegments/store_common_test.go
@@ -22,10 +22,26 @@ func testGenericAll(
 	t *testing.T,
 	withBigSegmentStore func(t *testing.T, action func(BigSegmentStore, bigSegmentOperations)),
 ) {
-	patch1 := newPatchBuilder("segment.g1", "1", "").
+	// When syncing from a real big segments endpoint the first series of patches will be from a paginated
+	// API. The "version" and "previousVersion" will be blank for the first page of entries excluding the final
+	// entry. At which point the "version" becomes a cursor used to request the next page of entries.
+	// Each subsequent page will have a "version" equal to the "after" parameter of the request. The "previous" version
+	// will be the version from the previous request.
+	//
+	// For each version several patches may be cumulatively applied to that version. So the empty version may
+	// receive several patches, as may each subsequent version.
+	//
+	// So it could be imagined that patch1, patch2, and patch 3 represent the first page of results returned from
+	// the API. First creating the empty version, then updating it, and then creating a new version by updating the cursor.
+	// Patch4 would represent the second page of results.
+	patch1 := newPatchBuilder("segment.g1", "", "").
 		addIncludes("included1", "included2").addExcludes("excluded1", "excluded2").build()
-	patch2 := newPatchBuilder("segment.g1", "2", "1").
+	patch2 := newPatchBuilder("segment.g1", "", "").
+		addIncludes("included3", "included4").addExcludes("excluded3", "excluded4").build()
+	patch3 := newPatchBuilder("segment.g1", "652ec9b74997e612346d9482", "").
 		removeIncludes("included1").removeExcludes("excluded1").build()
+	patch4 := newPatchBuilder("segment.g1", "4b748182441b808549a03c19", "652ec9b74997e612346d9482").
+		removeIncludes("included2").removeExcludes("excluded2").build()
 
 	t.Run("synchronizedOn", func(t *testing.T) {
 		withBigSegmentStore(t, func(store BigSegmentStore, operations bigSegmentOperations) {
@@ -62,18 +78,31 @@ func testGenericAll(
 			require.NoError(t, err)
 			assert.Equal(t, true, membership)
 
-			membership, err = operations.isUserExcluded(patch1.SegmentID, patch1.Changes.Excluded.Add[0])
+			// second patch adds some more users
+			success, err = store.applyPatch(patch2)
+			require.NoError(t, err)
+			require.True(t, success)
+
+			cursor, err = store.getCursor()
+			require.NoError(t, err)
+			require.Equal(t, patch2.Version, cursor)
+
+			membership, err = operations.isUserIncluded(patch2.SegmentID, patch2.Changes.Included.Add[0])
 			require.NoError(t, err)
 			assert.Equal(t, true, membership)
 
-			// apply second patch in sequence that removes users
-			success, err = store.applyPatch(patch2)
+			membership, err = operations.isUserExcluded(patch2.SegmentID, patch2.Changes.Excluded.Add[0])
+			require.NoError(t, err)
+			assert.Equal(t, true, membership)
+
+			// apply third patch in sequence that removes users
+			success, err = store.applyPatch(patch3)
 			require.NoError(t, err)
 			require.True(t, success)
 
 			cursor, err = store.getCursor()
 			require.NoError(t, err)
-			assert.Equal(t, patch2.Version, cursor)
+			assert.Equal(t, patch3.Version, cursor)
 
 			membership, err = operations.isUserIncluded(patch1.SegmentID, patch1.Changes.Included.Add[0])
 			require.NoError(t, err)
@@ -83,15 +112,32 @@ func testGenericAll(
 			require.NoError(t, err)
 			assert.Equal(t, false, membership)
 
+			// apply a fourth patch in sequence that removes more users
+			success, err = store.applyPatch(patch4)
+			require.NoError(t, err)
+			require.True(t, success)
+
+			cursor, err = store.getCursor()
+			require.NoError(t, err)
+			assert.Equal(t, patch4.Version, cursor)
+
+			membership, err = operations.isUserIncluded(patch1.SegmentID, patch1.Changes.Included.Add[1])
+			require.NoError(t, err)
+			assert.Equal(t, false, membership)
+
+			membership, err = operations.isUserExcluded(patch1.SegmentID, patch1.Changes.Excluded.Add[1])
+			require.NoError(t, err)
+			assert.Equal(t, false, membership)
+
 			// apply old patch
 			success, err = store.applyPatch(patch1)
 			require.NoError(t, err)
 			require.False(t, success)
 
-			// verify that the stored cursor was updated
+			// verify that the stored cursor was not updated
 			cursor, err = store.getCursor()
 			require.NoError(t, err)
-			assert.Equal(t, patch2.Version, cursor)
+			assert.Equal(t, patch4.Version, cursor)
 
 			// verify that the sync time is still there
 			syncTime, err := store.GetSynchronizedOn()
@@ -106,7 +152,7 @@ func testGenericAll(
 			assert.Equal(t, newSyncTime, syncTime)
 			cursor, err = store.getCursor()
 			require.NoError(t, err)
-			assert.Equal(t, patch2.Version, cursor)
+			assert.Equal(t, patch4.Version, cursor)
 		})
 	})
 

diff --git a/internal/core/bigsegments/store_dynamodb.go b/internal/core/bigsegments/store_dynamodb.go
@@ -110,12 +110,15 @@ func (store *dynamoDBBigSegmentStore) makeTransactionItem(updateExpression, attr
 
 func makeCursorUpdateCondition(previousVersion string) (string, map[string]string, map[string]types.AttributeValue) {
 	names := map[string]string{"#0": dynamoDBCursorAttr}
-	if previousVersion == "" {
-		return "attribute_not_exists(#0)", names, nil
-	}
-	return "#0 = :0", names, map[string]types.AttributeValue{
+	parameters := map[string]types.AttributeValue{
 		":0": attrValueOfString(previousVersion),
 	}
+	// Each version may receive multiple patches. The initial set of patches may contain multiple entries where the
+	// "version" and "previousVersion" are empty strings.
+	if previousVersion == "" {
+		return "attribute_not_exists(#0) or #0 = :0", names, parameters
+	}
+	return "#0 = :0", names, parameters
 }
 
 func (store *dynamoDBBigSegmentStore) applyPatch(patch bigSegmentPatch) (bool, error) {