remove old code, add tests for FeatureStoreWrapper

feature_store.go

@@ -4,9 +4,6 @@ import (
 	"log"
 	"os"
 	"sync"
-	"time"
-
-	cache "github.com/patrickmn/go-cache"
 )
 
 // FeatureStore is an interface describing a structure that maintains the live collection of features
@@ -161,143 +158,3 @@ func (store *InMemoryFeatureStore) Initialized() bool {
 	defer store.RUnlock()
 	return store.isInitialized
 }
-
-// FeatureStoreHelper is a helper type that can provide caching behavior for a FeatureStore
-// implementation.
-type FeatureStoreHelper struct {
-	cache    *cache.Cache
-	cacheTTL time.Duration
-}
-
-// NewFeatureStoreHelper creates an instance of FeatureStoreHelper. If cacheTTL is
-// non-zero, it will create an in-memory cache with the specified TTL and all of the
-// FeatureStoreHelper methods will use this cache. If cacheTTL is zero, it will not
-// create a cache and the methods will simply delegate to the underlying data store.
-func NewFeatureStoreHelper(cacheTTL time.Duration) *FeatureStoreHelper {
-	ret := FeatureStoreHelper{cacheTTL: cacheTTL}
-	if cacheTTL > 0 {
-		ret.cache = cache.New(cacheTTL, 5*time.Minute)
-	}
-	return &ret
-}
-
-func featureStoreCacheKey(kind VersionedDataKind, key string) string {
-	return kind.GetNamespace() + ":" + key
-}
-
-func featureStoreAllItemsCacheKey(kind VersionedDataKind) string {
-	return "all:" + kind.GetNamespace()
-}
-
-// Init performs an update of the entire data store, with optional caching. The uncachedInit
-// function updates the underlying data.
-func (fsh *FeatureStoreHelper) Init(allData map[VersionedDataKind]map[string]VersionedData,
-	uncachedInit func(map[VersionedDataKind]map[string]VersionedData) error) error {
-	if fsh.cache == nil {
-		return uncachedInit(allData)
-	}
-	fsh.cache.Flush()
-	err := uncachedInit(allData)
-	if err != nil {
-		return err
-	}
-	for kind, items := range allData {
-		fsh.putAllItemsInCache(kind, items)
-	}
-	return nil
-}
-
-func (fsh *FeatureStoreHelper) putAllItemsInCache(kind VersionedDataKind, items map[string]VersionedData) {
-	if fsh.cache == nil {
-		return
-	}
-	// We do some filtering here so that deleted items are not included in the full cached data set
-	// that's used by All. This is so that All doesn't have to do that filtering itself. However,
-	// since Get does know to filter out deleted items, we will still cache those individually,
-	filteredItems := make(map[string]VersionedData, len(items))
-	for key, item := range items {
-		fsh.cache.Set(featureStoreCacheKey(kind, key), item, fsh.cacheTTL)
-		if !item.IsDeleted() {
-			filteredItems[key] = item
-		}
-	}
-	fsh.cache.Set(featureStoreAllItemsCacheKey(kind), filteredItems, fsh.cacheTTL)
-}
-
-// Get retrieves a single item by key, with optional caching. The uncachedGet function attempts
-// to retrieve the item from the underlying data store.
-func (fsh *FeatureStoreHelper) Get(kind VersionedDataKind, key string,
-	uncachedGet func(VersionedDataKind, string) (VersionedData, error)) (VersionedData, error) {
-	if fsh.cache == nil {
-		item, err := uncachedGet(kind, key)
-		return itemOnlyIfNotDeleted(item), err
-	}
-	cacheKey := featureStoreCacheKey(kind, key)
-	if data, present := fsh.cache.Get(cacheKey); present {
-		if data == nil { // If present is true but data is nil, we have cached the absence of an item
-			return nil, nil
-		}
-		if item, ok := data.(VersionedData); ok {
-			return itemOnlyIfNotDeleted(item), nil
-		}
-	}
-	// Item was not cached or cached value was not valid
-	item, err := uncachedGet(kind, key)
-	if err == nil {
-		fsh.cache.Set(cacheKey, item, fsh.cacheTTL)
-	}
-	return itemOnlyIfNotDeleted(item), err
-}
-
-func itemOnlyIfNotDeleted(item VersionedData) VersionedData {
-	if item != nil && item.IsDeleted() {
-		return nil
-	}
-	return item
-}
-
-// All retrieves all items of the specified kind, with optional caching. The uncachedAll function
-// retrieves the items from the underlying data store.
-func (fsh *FeatureStoreHelper) All(kind VersionedDataKind,
-	uncachedAll func(VersionedDataKind) (map[string]VersionedData, error)) (map[string]VersionedData, error) {
-	if fsh.cache == nil {
-		return uncachedAll(kind)
-	}
-	// Check whether we have a cache item for the entire data set
-	cacheKey := featureStoreAllItemsCacheKey(kind)
-	if data, present := fsh.cache.Get(cacheKey); present {
-		if items, ok := data.(map[string]VersionedData); ok {
-			return items, nil
-		}
-	}
-	// Data set was not cached or cached value was not valid
-	items, err := uncachedAll(kind)
-	if err == nil {
-		fsh.putAllItemsInCache(kind, items)
-	}
-	return items, err
-}
-
-// Upsert updates or adds an item, with optional caching. The uncachedUpsert function performs
-// an upsert to the underlying data store.
-func (fsh *FeatureStoreHelper) Upsert(kind VersionedDataKind, item VersionedData,
-	uncachedUpsert func(VersionedDataKind, VersionedData) error) error {
-	if fsh.cache == nil {
-		return uncachedUpsert(kind, item)
-	}
-	err := uncachedUpsert(kind, item)
-	if err == nil {
-		fsh.cache.Set(featureStoreCacheKey(kind, item.GetKey()), item, fsh.cacheTTL)
-		fsh.cache.Delete(featureStoreAllItemsCacheKey(kind))
-	}
-	return err
-}
-
-// Delete deletes an item, with optional caching. The uncachedUpsert function performs an
-// upsert to the underlying data store - Delete is implemented by upserting an item that
-// is in a deleted state.
-func (fsh *FeatureStoreHelper) Delete(kind VersionedDataKind, key string, version int,
-	uncachedUpdate func(VersionedDataKind, VersionedData) error) error {
-	deletedItem := kind.MakeDeletedItem(key, version)
-	return fsh.Upsert(kind, deletedItem, uncachedUpdate)
-}

feature_store_test.go

@@ -2,9 +2,7 @@ package ldclient_test
 
 import (
 	"testing"
-	"time"
 
-	"github.com/stretchr/testify/require"
 	ld "gopkg.in/launchdarkly/go-client.v4"
 	ldtest "gopkg.in/launchdarkly/go-client.v4/shared_test"
 )
@@ -16,223 +14,3 @@ func makeInMemoryStore() ld.FeatureStore {
 func TestInMemoryFeatureStore(t *testing.T) {
 	ldtest.RunFeatureStoreTests(t, makeInMemoryStore)
 }
-
-func TestFeatureStoreHelper(t *testing.T) {
-	cacheTime := 30 * time.Second
-
-	getFunc := func(err error, item ld.VersionedData) func(ld.VersionedDataKind, string) (ld.VersionedData, error) {
-		return func(kind ld.VersionedDataKind, key string) (ld.VersionedData, error) {
-			return item, err
-		}
-	}
-
-	allFunc := func(err error, items ...ld.VersionedData) func(ld.VersionedDataKind) (map[string]ld.VersionedData, error) {
-		return func(kind ld.VersionedDataKind) (map[string]ld.VersionedData, error) {
-			ret := make(map[string]ld.VersionedData)
-			for _, item := range items {
-				ret[item.GetKey()] = item
-			}
-			return ret, err
-		}
-	}
-
-	initFunc := func(err error, wasCalled *bool) func(map[ld.VersionedDataKind]map[string]ld.VersionedData) error {
-		return func(allData map[ld.VersionedDataKind]map[string]ld.VersionedData) error {
-			if wasCalled != nil {
-				*wasCalled = true
-			}
-			return err
-		}
-	}
-
-	upsertFunc := func(err error, receivedItem *ld.VersionedData) func(ld.VersionedDataKind, ld.VersionedData) error {
-		return func(kind ld.VersionedDataKind, item ld.VersionedData) error {
-			if receivedItem != nil {
-				*receivedItem = item
-			}
-			return err
-		}
-	}
-
-	runCachedAndUncachedTests := func(t *testing.T, name string, test func(t *testing.T, isCached bool, fsh *ld.FeatureStoreHelper)) {
-		t.Run(name, func(t *testing.T) {
-			t.Run("uncached", func(t *testing.T) {
-				test(t, false, ld.NewFeatureStoreHelper(0))
-			})
-			t.Run("cached", func(t *testing.T) {
-				test(t, true, ld.NewFeatureStoreHelper(cacheTime))
-			})
-		})
-	}
-
-	runCachedAndUncachedTests(t, "Get", func(t *testing.T, isCached bool, fsh *ld.FeatureStoreHelper) {
-		flagv1 := ld.FeatureFlag{Key: "flag", Version: 1}
-		flagv2 := ld.FeatureFlag{Key: "flag", Version: 2}
-
-		item, err := fsh.Get(ld.Features, flagv1.Key, getFunc(nil, &flagv1))
-		require.NoError(t, err)
-		require.Equal(t, &flagv1, item)
-
-		item, err = fsh.Get(ld.Features, flagv1.Key, getFunc(nil, &flagv2))
-		require.NoError(t, err)
-		if isCached {
-			require.Equal(t, &flagv1, item) // returns cached value, does not call getter
-		} else {
-			require.Equal(t, &flagv2, item) // no caching, calls getter
-		}
-	})
-
-	runCachedAndUncachedTests(t, "Get with deleted item", func(t *testing.T, isCached bool, fsh *ld.FeatureStoreHelper) {
-		flagv1 := ld.FeatureFlag{Key: "flag", Version: 1, Deleted: true}
-		flagv2 := ld.FeatureFlag{Key: "flag", Version: 2, Deleted: false}
-
-		item, err := fsh.Get(ld.Features, flagv1.Key, getFunc(nil, &flagv1))
-		require.NoError(t, err)
-		require.Nil(t, item) // item is filtered out because Deleted is true
-
-		item, err = fsh.Get(ld.Features, flagv1.Key, getFunc(nil, &flagv2))
-		require.NoError(t, err)
-		if isCached {
-			require.Nil(t, item) // it used the cached deleted item rather than calling the getter
-		} else {
-			require.Equal(t, &flagv2, item) // no caching, calls getter
-		}
-	})
-
-	runCachedAndUncachedTests(t, "Get with missing item", func(t *testing.T, isCached bool, fsh *ld.FeatureStoreHelper) {
-		flag := ld.FeatureFlag{Key: "flag", Version: 1}
-
-		item, err := fsh.Get(ld.Features, flag.Key, getFunc(nil, nil))
-		require.NoError(t, err)
-		require.Nil(t, item)
-
-		item, err = fsh.Get(ld.Features, flag.Key, getFunc(nil, &flag))
-		require.NoError(t, err)
-		if isCached {
-			require.Nil(t, item) // the cache retains a nil result
-		} else {
-			require.Equal(t, &flag, item) // no caching, calls getter
-		}
-	})
-
-	t.Run("cached Get uses values from Init", func(t *testing.T) {
-		fsh := ld.NewFeatureStoreHelper(cacheTime)
-
-		flagv1 := ld.FeatureFlag{Key: "flag", Version: 1}
-		flagv2 := ld.FeatureFlag{Key: "flag", Version: 1}
-
-		allData := map[ld.VersionedDataKind]map[string]ld.VersionedData{
-			ld.Features: {flagv1.Key: &flagv1},
-		}
-		initWasCalled := false
-		err := fsh.Init(allData, initFunc(nil, &initWasCalled))
-		require.NoError(t, err)
-		require.True(t, initWasCalled)
-
-		item, err := fsh.Get(ld.Features, flagv1.Key, getFunc(nil, &flagv2))
-		require.NoError(t, err)
-		require.Equal(t, &flagv1, item) // it used the cached item rather than calling the getter
-	})
-
-	runCachedAndUncachedTests(t, "All", func(t *testing.T, isCached bool, fsh *ld.FeatureStoreHelper) {
-		flag1 := ld.FeatureFlag{Key: "flag1", Version: 1}
-		flag2 := ld.FeatureFlag{Key: "flag2", Version: 1}
-
-		items, err := fsh.All(ld.Features, allFunc(nil, &flag1, &flag2))
-		require.NoError(t, err)
-		require.Equal(t, 2, len(items))
-
-		items, err = fsh.All(ld.Features, allFunc(nil, &flag1))
-		require.NoError(t, err)
-		if isCached {
-			require.Equal(t, 2, len(items))
-		} else {
-			require.Equal(t, 1, len(items))
-		}
-	})
-
-	t.Run("cached All uses values from Init", func(t *testing.T) {
-		fsh := ld.NewFeatureStoreHelper(cacheTime)
-
-		flag1 := ld.FeatureFlag{Key: "flag1", Version: 1}
-		flag2 := ld.FeatureFlag{Key: "flag2", Version: 1}
-
-		allData := map[ld.VersionedDataKind]map[string]ld.VersionedData{
-			ld.Features: {flag1.Key: &flag1, flag2.Key: &flag2},
-		}
-		initWasCalled := false
-		err := fsh.Init(allData, initFunc(nil, &initWasCalled))
-		require.NoError(t, err)
-		require.True(t, initWasCalled)
-
-		items, err := fsh.All(ld.Features, allFunc(nil, &flag1))
-		require.NoError(t, err)
-		require.Equal(t, 2, len(items))
-	})
-
-	t.Run("cached All uses fresh values if there has been an update", func(t *testing.T) {
-		fsh := ld.NewFeatureStoreHelper(cacheTime)
-
-		flag1 := ld.FeatureFlag{Key: "flag1", Version: 1}
-		flag1v2 := ld.FeatureFlag{Key: "flag1", Version: 2}
-		flag2 := ld.FeatureFlag{Key: "flag2", Version: 1}
-		flag2v2 := ld.FeatureFlag{Key: "flag2", Version: 2}
-
-		allData := map[ld.VersionedDataKind]map[string]ld.VersionedData{
-			ld.Features: {flag1.Key: &flag1, flag2.Key: &flag2},
-		}
-		initWasCalled := false
-		err := fsh.Init(allData, initFunc(nil, &initWasCalled))
-		require.NoError(t, err)
-		require.True(t, initWasCalled)
-
-		err = fsh.Upsert(ld.Features, &flag1v2, upsertFunc(nil, nil))
-		require.NoError(t, err)
-
-		// updating any flag should force the cache for All to be flushed
-		items, err := fsh.All(ld.Features, allFunc(nil, &flag1v2, &flag2v2))
-		require.NoError(t, err)
-		require.Equal(t, 2, items[flag2.Key].GetVersion())
-	})
-
-	runCachedAndUncachedTests(t, "Upsert", func(t *testing.T, isCached bool, fsh *ld.FeatureStoreHelper) {
-		flagv1 := ld.FeatureFlag{Key: "flag1", Version: 1}
-		flagv2 := ld.FeatureFlag{Key: "flag1", Version: 2}
-
-		var receivedItem ld.VersionedData
-		err := fsh.Upsert(ld.Features, &flagv1, upsertFunc(nil, &receivedItem))
-		require.NoError(t, err)
-		require.Equal(t, &flagv1, receivedItem)
-
-		item, err := fsh.Get(ld.Features, flagv1.Key, getFunc(nil, &flagv2))
-		require.NoError(t, err)
-		if isCached {
-			require.Equal(t, &flagv1, item)
-		} else {
-			require.Equal(t, &flagv2, item)
-		}
-	})
-
-	runCachedAndUncachedTests(t, "Delete", func(t *testing.T, isCached bool, fsh *ld.FeatureStoreHelper) {
-		flagv1 := ld.FeatureFlag{Key: "flag1", Version: 1}
-		flagv2 := ld.FeatureFlag{Key: "flag1", Version: 2, Deleted: true}
-		flagv3 := ld.FeatureFlag{Key: "flag1", Version: 3}
-
-		item, err := fsh.Get(ld.Features, flagv1.Key, getFunc(nil, &flagv1))
-		require.NoError(t, err)
-		require.Equal(t, &flagv1, item)
-
-		var receivedItem ld.VersionedData
-		err = fsh.Delete(ld.Features, flagv1.Key, 2, upsertFunc(nil, &receivedItem))
-		require.NoError(t, err)
-		require.Equal(t, &flagv2, receivedItem)
-
-		item, err = fsh.Get(ld.Features, flagv1.Key, getFunc(nil, &flagv3))
-		require.NoError(t, err)
-		if isCached {
-			require.Nil(t, item)
-		} else {
-			require.Equal(t, &flagv3, item)
-		}
-	})
-}