refactor: store cleanup (#1705)

## This PR
IStore interface usage instead of Store implementation
_Separate locks for flag data and metadata in store_ - no longer valid
point

---------

Signed-off-by: Andrey <[email protected]>

Author: andreyturkov

core/pkg/evaluator/json.go

@@ -64,13 +64,13 @@ func WithEvaluator(name string, evalFunc func(interface{}, interface{}) interfac
 
 // JSON evaluator
 type JSON struct {
-	store          *store.Store
+	store          store.IStore
 	Logger         *logger.Logger
 	jsonEvalTracer trace.Tracer
 	Resolver
 }
 
-func NewJSON(logger *logger.Logger, s *store.Store, opts ...JSONEvaluatorOption) *JSON {
+func NewJSON(logger *logger.Logger, s store.IStore, opts ...JSONEvaluatorOption) *JSON {
 	logger = logger.WithFields(
 		zap.String("component", "evaluator"),
 		zap.String("evaluator", "json"),

core/pkg/store/store.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"slices"
-	"sync"
 
 	"github.com/hashicorp/go-memdb"
 	"github.com/open-feature/flagd/core/pkg/logger"
@@ -25,12 +24,13 @@ type IStore interface {
 	Get(ctx context.Context, key string, selector *Selector) (model.Flag, model.Metadata, error)
 	GetAll(ctx context.Context, selector *Selector) (map[string]model.Flag, model.Metadata, error)
 	Watch(ctx context.Context, selector *Selector, watcher chan<- FlagQueryResult)
+	Update(source string, flags map[string]model.Flag, metadata model.Metadata) (map[string]interface{}, bool)
+	String() (string, error)
 }
 
 var _ IStore = (*Store)(nil)
 
 type Store struct {
-	mx      sync.RWMutex
 	db      *memdb.MemDB
 	logger  *logger.Logger
 	sources []string
@@ -201,8 +201,6 @@ func (s *Store) Get(_ context.Context, key string, selector *Selector) (model.Fl
 
 func (f *Store) String() (string, error) {
 	f.logger.Debug("dumping flags to string")
-	f.mx.RLock()
-	defer f.mx.RUnlock()
 
 	state, _, err := f.GetAll(context.Background(), nil)
 	if err != nil {
@@ -260,7 +258,6 @@ func (s *Store) Update(
 	selector := NewSelector(sourceIndex + "=" + source)
 	oldFlags, _, _ := s.GetAll(context.Background(), &selector)
 
-	s.mx.Lock()
 	for key := range oldFlags {
 		if _, ok := flags[key]; !ok {
 			// flag has been deleted
@@ -275,7 +272,7 @@ func (s *Store) Update(
 			continue
 		}
 	}
-	s.mx.Unlock()
+
 	for key, newFlag := range flags {
 		s.logger.Debug(fmt.Sprintf("got metadata %v", metadata))
 

core/pkg/store/store_test.go

@@ -20,7 +20,7 @@ func TestUpdateFlags(t *testing.T) {
 	t.Parallel()
 	tests := []struct {
 		name        string
-		setup       func(t *testing.T) *Store
+		setup       func(t *testing.T) IStore
 		newFlags    map[string]model.Flag
 		source      string
 		wantFlags   map[string]model.Flag
@@ -30,7 +30,7 @@ func TestUpdateFlags(t *testing.T) {
 	}{
 		{
 			name: "both nil",
-			setup: func(t *testing.T) *Store {
+			setup: func(t *testing.T) IStore {
 				s, err := NewStore(logger.NewLogger(nil, false), sources)
 				if err != nil {
 					t.Fatalf("NewStore failed: %v", err)
@@ -44,7 +44,7 @@ func TestUpdateFlags(t *testing.T) {
 		},
 		{
 			name: "both empty flags",
-			setup: func(t *testing.T) *Store {
+			setup: func(t *testing.T) IStore {
 				s, err := NewStore(logger.NewLogger(nil, false), sources)
 				if err != nil {
 					t.Fatalf("NewStore failed: %v", err)
@@ -58,7 +58,7 @@ func TestUpdateFlags(t *testing.T) {
 		},
 		{
 			name: "empty new",
-			setup: func(t *testing.T) *Store {
+			setup: func(t *testing.T) IStore {
 				s, err := NewStore(logger.NewLogger(nil, false), sources)
 				if err != nil {
 					t.Fatalf("NewStore failed: %v", err)
@@ -72,7 +72,7 @@ func TestUpdateFlags(t *testing.T) {
 		},
 		{
 			name: "update from source 1 (old flag removed)",
-			setup: func(t *testing.T) *Store {
+			setup: func(t *testing.T) IStore {
 				s, err := NewStore(logger.NewLogger(nil, false), sources)
 				if err != nil {
 					t.Fatalf("NewStore failed: %v", err)
@@ -96,7 +96,7 @@ func TestUpdateFlags(t *testing.T) {
 		},
 		{
 			name: "update from source 1 (new flag added)",
-			setup: func(t *testing.T) *Store {
+			setup: func(t *testing.T) IStore {
 				s, err := NewStore(logger.NewLogger(nil, false), sources)
 				if err != nil {
 					t.Fatalf("NewStore failed: %v", err)
@@ -118,7 +118,7 @@ func TestUpdateFlags(t *testing.T) {
 		},
 		{
 			name: "flag set inheritance",
-			setup: func(t *testing.T) *Store {
+			setup: func(t *testing.T) IStore {
 				s, err := NewStore(logger.NewLogger(nil, false), sources)
 				if err != nil {
 					t.Fatalf("NewStore failed: %v", err)

flagd/pkg/service/flag-evaluation/connect_service_test.go

@@ -84,7 +84,7 @@ func TestConnectService_UnixConnection(t *testing.T) {
 			exp := metric.NewManualReader()
 			rs := resource.NewWithAttributes("testSchema")
 			metricRecorder := telemetry.NewOTelRecorder(exp, rs, tt.name)
-			svc := NewConnectService(logger.NewLogger(nil, false), eval, &store.Store{}, metricRecorder)
+			svc := NewConnectService(logger.NewLogger(nil, false), eval, nil, metricRecorder)
 			serveConf := iservice.Configuration{
 				ReadinessProbe: func() bool {
 					return true
@@ -139,7 +139,7 @@ func TestAddMiddleware(t *testing.T) {
 	rs := resource.NewWithAttributes("testSchema")
 	metricRecorder := telemetry.NewOTelRecorder(exp, rs, "my-exporter")
 
-	svc := NewConnectService(logger.NewLogger(nil, false), nil, &store.Store{}, metricRecorder)
+	svc := NewConnectService(logger.NewLogger(nil, false), nil, nil, metricRecorder)
 
 	serveConf := iservice.Configuration{
 		ReadinessProbe: func() bool {

flagd/pkg/service/flag-sync/handler.go

@@ -21,7 +21,7 @@ import (
 // syncHandler implements the sync contract
 type syncHandler struct {
 	//mux                 *Multiplexer
-	store               *store.Store
+	store               store.IStore
 	log                 *logger.Logger
 	contextValues       map[string]any
 	deadline            time.Duration

flagd/pkg/service/flag-sync/sync_service.go

@@ -28,7 +28,7 @@ type SvcConfigurations struct {
 	Logger              *logger.Logger
 	Port                uint16
 	Sources             []string
-	Store               *store.Store
+	Store               store.IStore
 	ContextValues       map[string]any
 	CertPath            string
 	KeyPath             string

flagd/pkg/service/flag-sync/sync_service_test.go

@@ -245,7 +245,7 @@ func TestSyncServiceDeadlineEndToEnd(t *testing.T) {
 func createAndStartSyncService(
 	port int,
 	sources []string,
-	store *store.Store,
+	store store.IStore,
 	certPath string,
 	keyPath string,
 	socketPath string,

flagd/pkg/service/flag-sync/util_test.go

@@ -35,7 +35,7 @@ var testSource2Flags = map[string]model.Flag{
 }
 
 // getSimpleFlagStore is a test util which returns a flag store pre-filled with flags from sources testSource1 and testSource2.
-func getSimpleFlagStore(t testing.TB) (*store.Store, []string) {
+func getSimpleFlagStore(t testing.TB) (store.IStore, []string) {
 	t.Helper()
 
 	sources := []string{testSource1, testSource2}