[4.1.0.1 backport] CBG-5550: Revert "CBG-5262 fix missing changes with active_only=true and limit=" (#8418)

Author: torcolvin

db/channel_cache_single.go

@@ -440,17 +440,14 @@ func (c *singleChannelCacheImpl) GetChanges(ctx context.Context, options Changes
 
 	result := resultFromQuery
 	room := options.Limit - len(result)
-	if (options.Limit == 0 || room > 0 || options.ActiveOnly) && len(resultFromCache) > 0 {
+	if (options.Limit == 0 || room > 0) && len(resultFromCache) > 0 {
 		// Concatenate the view & cache results:
 		if len(result) > 0 && resultFromCache[0].Sequence == result[len(result)-1].Sequence {
 			resultFromCache = resultFromCache[1:]
 		}
 		n := len(resultFromCache)
-		// Limit evaluation only valid when not activeOnly, since view and cache results don't apply activeOnly filtering
-		if !options.ActiveOnly {
-			if options.Limit > 0 && room > 0 && room < n {
-				n = room
-			}
+		if options.Limit > 0 && room > 0 && room < n {
+			n = room
 		}
 		result = append(result, resultFromCache[0:n]...)
 	}

db/channel_cache_test.go

@@ -598,172 +598,3 @@ func TestChannelCacheBackgroundTaskWithIllegalTimeInterval(t *testing.T) {
 	assert.Equal(t, "CleanAgedItems", backgroundTaskError.TaskName)
 	assert.Equal(t, options.ChannelCacheAge, backgroundTaskError.Interval)
 }
-
-// - The channel cache is validFrom sequence n, with one active mutation resident in the cache
-// - There are channel removals (only) in the bucket with m < sequence < n
-// - Client issues a GetChanges request with since=m
-func TestChannelCacheActiveOnlyAndLimit(t *testing.T) {
-	ctx, db, collection := setupDBWithChannelCacheSize(t, 2)
-
-	const (
-		activeChannel   = "active"
-		inactiveChannel = "inactive"
-		doc1            = "doc1"
-		doc2            = "doc2"
-		doc3            = "doc3"
-	)
-
-	// doc1 rev1: channel active
-	// doc1 rev2: channel inactive
-	// doc2 rev1: channel active
-	// doc2 rev2: channel inactive
-	// doc3 rev1: channel active
-	revID, _, err := collection.Put(ctx, doc1, Body{"channels": activeChannel})
-	require.NoError(t, err)
-	_, _, err = collection.Put(ctx, doc1, Body{"channels": inactiveChannel, "_rev": revID})
-	require.NoError(t, err)
-	revID, _, err = collection.Put(ctx, doc2, Body{"channels": activeChannel})
-	require.NoError(t, err)
-	_, _, err = collection.Put(ctx, doc2, Body{"channels": inactiveChannel, "_rev": revID})
-	require.NoError(t, err)
-
-	_, _, err = collection.Put(ctx, doc3, Body{"channels": activeChannel})
-	require.NoError(t, err)
-
-	db.WaitForPendingChanges(t)
-
-	// prime channel cache, doc2 and doc3 should be in cache
-	changesOptions := ChangesOptions{
-		Since:      SequenceID{Seq: 0},
-		ActiveOnly: false,
-		ChangesCtx: base.TestCtx(t),
-	}
-	require.Len(t, getChanges(t, collection, base.SetOf(activeChannel), changesOptions), 3)
-
-	// whether limit or no limit, should only be 1 active entry
-	for _, limit := range []int{0, 1} {
-		t.Run("limit="+fmt.Sprint(limit), func(t *testing.T) {
-			changesOptions = ChangesOptions{
-				Since:      SequenceID{Seq: 0},
-				ActiveOnly: true,
-				ChangesCtx: base.TestCtx(t),
-				Limit:      limit,
-			}
-			require.Len(t, getChanges(t, collection, base.SetOf(activeChannel), changesOptions), 1)
-		})
-	}
-}
-
-func TestChannelCacheActiveOnlyScenarios(t *testing.T) {
-	const activeChannel = "active"
-
-	t.Run("query returns an active rev, cache is all removals", func(t *testing.T) {
-		ctx, db, collection := setupDBWithChannelCacheSize(t, 2)
-
-		// doc1: active (seq 1) - will be in backing store, not cache
-		_, _, _ = collection.Put(ctx, "doc1", Body{"channels": activeChannel})
-
-		// doc2: active (seq 2) -> inactive (seq 3) - seq 3 in cache
-		revID2, _, _ := collection.Put(ctx, "doc2", Body{"channels": activeChannel})
-		_, _, _ = collection.Put(ctx, "doc2", Body{"channels": "other", "_rev": revID2})
-
-		// doc3: active (seq 4) -> inactive (seq 5) - seq 5 in cache
-		revID3, _, _ := collection.Put(ctx, "doc3", Body{"channels": activeChannel})
-		_, _, _ = collection.Put(ctx, "doc3", Body{"channels": "other", "_rev": revID3})
-
-		db.WaitForPendingChanges(t)
-
-		// With limit 1 (before)
-		changesOptions := ChangesOptions{Since: SequenceID{Seq: 0}, ActiveOnly: true, Limit: 1, ChangesCtx: base.TestCtx(t)}
-		changes := getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
-		require.Len(t, changes, 1)
-		assert.Equal(t, "doc1", changes[0].ID)
-
-		// No limit
-		changesOptions.Limit = 0
-		changes = getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
-		require.Len(t, changes, 1)
-		assert.Equal(t, "doc1", changes[0].ID)
-
-		// With limit 1 (after)
-		changesOptions.Limit = 1
-		changes = getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
-		require.Len(t, changes, 1)
-		assert.Equal(t, "doc1", changes[0].ID)
-	})
-
-	t.Run("query returns an active rev, cache also has an active rev", func(t *testing.T) {
-		ctx, db, collection := setupDBWithChannelCacheSize(t, 2)
-
-		// doc1: active (seq 1) - backing store
-		_, _, _ = collection.Put(ctx, "doc1", Body{"channels": activeChannel})
-
-		// doc2: active (seq 2) -> inactive (seq 3) - cache
-		revID2, _, _ := collection.Put(ctx, "doc2", Body{"channels": activeChannel})
-		_, _, _ = collection.Put(ctx, "doc2", Body{"channels": "other", "_rev": revID2})
-
-		// doc3: active (seq 4) - cache
-		_, _, _ = collection.Put(ctx, "doc3", Body{"channels": activeChannel})
-
-		db.WaitForPendingChanges(t)
-
-		// With limit 1 (before)
-		changesOptions := ChangesOptions{Since: SequenceID{Seq: 0}, ActiveOnly: true, Limit: 1, ChangesCtx: base.TestCtx(t)}
-		changes := getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
-		require.Len(t, changes, 1)
-		assert.Equal(t, "doc1", changes[0].ID)
-
-		// No limit: should get doc1 and doc3
-		changesOptions.Limit = 0
-		changes = getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
-		require.Len(t, changes, 2)
-		assert.Equal(t, "doc1", changes[0].ID)
-		assert.Equal(t, "doc3", changes[1].ID)
-
-		// With limit 1 (after)
-		changesOptions.Limit = 1
-		changes = getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
-		require.Len(t, changes, 1)
-		assert.Equal(t, "doc1", changes[0].ID)
-	})
-
-	t.Run("query has no active revs, cache has no active revs", func(t *testing.T) {
-		ctx, db, collection := setupDBWithChannelCacheSize(t, 2)
-
-		// doc1: active (seq 1) -> inactive (seq 2)
-		revID1, _, _ := collection.Put(ctx, "doc1", Body{"channels": activeChannel})
-		_, _, _ = collection.Put(ctx, "doc1", Body{"channels": "other", "_rev": revID1})
-
-		// doc2: active (seq 3) -> inactive (seq 4)
-		revID2, _, _ := collection.Put(ctx, "doc2", Body{"channels": activeChannel})
-		_, _, _ = collection.Put(ctx, "doc2", Body{"channels": "other", "_rev": revID2})
-
-		db.WaitForPendingChanges(t)
-
-		// With limit 1 (before)
-		changesOptions := ChangesOptions{Since: SequenceID{Seq: 0}, ActiveOnly: true, Limit: 1, ChangesCtx: base.TestCtx(t)}
-		changes := getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
-		require.Len(t, changes, 0)
-
-		// No limit: should get nothing
-		changesOptions.Limit = 0
-		changes = getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
-		require.Len(t, changes, 0)
-
-		// With limit 1 (after)
-		changesOptions.Limit = 1
-		changes = getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
-		require.Len(t, changes, 0)
-	})
-}
-
-func setupDBWithChannelCacheSize(t *testing.T, maxLength int) (context.Context, *Database, *DatabaseCollectionWithUser) {
-	cacheOptions := DefaultCacheOptions()
-	cacheOptions.ChannelCacheMaxLength = maxLength
-	db, ctx := setupTestDBWithCacheOptions(t, cacheOptions)
-	t.Cleanup(func() { db.Close(ctx) })
-	collection, ctx := GetSingleDatabaseCollectionWithUser(ctx, t, db)
-	_, err := collection.UpdateSyncFun(ctx, channels.DocChannelsSyncFunction)
-	require.NoError(t, err)
-	return ctx, db, collection
-}