fixup: add support for old selector

Signed-off-by: Todd Baert <[email protected]>

Author: toddbaert

core/pkg/store/query.go

@@ -41,14 +41,21 @@ func NewSelector(selectorExpression string) Selector {
 	}
 }
 
-func expressionToMap(selector string) map[string]string {
+func expressionToMap(sExp string) map[string]string {
 	selectorMap := make(map[string]string)
-	if selector == "" {
+	if sExp == "" {
+		return selectorMap
+	}
+
+	if strings.Index(sExp, "=") == -1 {
+		// if no '=' is found, treat the whole string as as source (backwards compatibility)
+		// we may may support interpreting this as a flagSetId in the future as an option
+		selectorMap[sourceIndex] = sExp
 		return selectorMap
 	}
 
 	// Split the selector by commas
-	pairs := strings.Split(selector, ",")
+	pairs := strings.Split(sExp, ",")
 	for _, pair := range pairs {
 		// Split each pair by the first equal sign
 		parts := strings.Split(pair, "=")
@@ -109,9 +116,13 @@ func (s Selector) ToQuery() (indexId string, constraints []interface{}) {
 
 // SelectorToMetadata converts the selector's internal map to metadata for logging or tracing purposes.
 // Only includes known indices to avoid leaking sensitive information, and is usually returned as the "top level" metadata
-func (s Selector) ToMetadata() model.Metadata {
+func (s *Selector) ToMetadata() model.Metadata {
 	meta := model.Metadata{}
 
+	if s == nil || s.indexMap == nil {
+		return meta
+	}
+
 	if s.indexMap[flagSetIdIndex] != "" {
 		meta[flagSetIdIndex] = s.indexMap[flagSetIdIndex]
 	}

core/pkg/store/query_test.go

@@ -3,6 +3,8 @@ package store
 import (
 	"reflect"
 	"testing"
+
+	"github.com/open-feature/flagd/core/pkg/model"
 )
 
 func TestSelector_IsEmpty(t *testing.T) {
@@ -100,25 +102,92 @@ func TestSelector_ToQuery(t *testing.T) {
 }
 
 func TestSelector_ToMetadata(t *testing.T) {
-	s := Selector{indexMap: map[string]string{"flagSetId": "fsid", "source": "src", "key": "myKey"}}
-	meta := s.ToMetadata()
-	if meta["flagSetId"] != "fsid" {
-		t.Errorf("ToMetadata missing flagSetId")
-	}
-	if meta["source"] != "src" {
-		t.Errorf("ToMetadata missing source")
+	tests := []struct {
+		name     string
+		selector *Selector
+		want     model.Metadata
+	}{
+		{
+			name:     "nil selector",
+			selector: nil,
+			want:     model.Metadata{},
+		},
+		{
+			name:     "nil indexMap",
+			selector: &Selector{indexMap: nil},
+			want:     model.Metadata{},
+		},
+		{
+			name:     "empty indexMap",
+			selector: &Selector{indexMap: map[string]string{}},
+			want:     model.Metadata{},
+		},
+		{
+			name:     "flagSetId only",
+			selector: &Selector{indexMap: map[string]string{"flagSetId": "fsid"}},
+			want:     model.Metadata{"flagSetId": "fsid"},
+		},
+		{
+			name:     "source only",
+			selector: &Selector{indexMap: map[string]string{"source": "src"}},
+			want:     model.Metadata{"source": "src"},
+		},
+		{
+			name:     "flagSetId and source",
+			selector: &Selector{indexMap: map[string]string{"flagSetId": "fsid", "source": "src"}},
+			want:     model.Metadata{"flagSetId": "fsid", "source": "src"},
+		},
+		{
+			name:     "flagSetId, source, and key (key should be ignored)",
+			selector: &Selector{indexMap: map[string]string{"flagSetId": "fsid", "source": "src", "key": "myKey"}},
+			want:     model.Metadata{"flagSetId": "fsid", "source": "src"},
+		},
 	}
-	if _, ok := meta["key"]; ok {
-		t.Errorf("ToMetadata should not include key")
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.selector.ToMetadata()
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("ToMetadata() = %v, want %v", got, tt.want)
+			}
+		})
 	}
 }
 
 func TestNewSelector(t *testing.T) {
-	s := NewSelector("source=abc,flagSetId=1234")
-	if s.indexMap["source"] != "abc" {
-		t.Errorf("NewSelector did not parse source")
+	tests := []struct {
+		name    string
+		input   string
+		wantMap map[string]string
+	}{
+		{
+			name:    "source and flagSetId",
+			input:   "source=abc,flagSetId=1234",
+			wantMap: map[string]string{"source": "abc", "flagSetId": "1234"},
+		},
+		{
+			name:    "source",
+			input:   "source=abc",
+			wantMap: map[string]string{"source": "abc"},
+		},
+		{
+			name:    "no equals, treat as source",
+			input:   "mysource",
+			wantMap: map[string]string{"source": "mysource"},
+		},
+		{
+			name:    "empty string",
+			input:   "",
+			wantMap: map[string]string{},
+		},
 	}
-	if s.indexMap["flagSetId"] != "1234" {
-		t.Errorf("NewSelector did not parse flagSetId")
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s := NewSelector(tt.input)
+			if !reflect.DeepEqual(s.indexMap, tt.wantMap) {
+				t.Errorf("NewSelector(%q) indexMap = %v, want %v", tt.input, s.indexMap, tt.wantMap)
+			}
+		})
 	}
 }

core/pkg/store/store.go

@@ -154,11 +154,10 @@ func NewFlags() *Store {
 func (s *Store) Get(_ context.Context, key string, selector *Selector) (model.Flag, model.Metadata, error) {
 	s.logger.Debug(fmt.Sprintf("getting flag %s", key))
 	txn := s.db.Txn(false)
-	queryMeta := model.Metadata{}
+	queryMeta := selector.ToMetadata()
 
 	// if present, use the selector to query the flags
 	if !selector.IsEmpty() {
-		queryMeta = selector.ToMetadata()
 		selector := selector.WithIndex("key", key)
 		indexId, constraints := selector.ToQuery()
 		s.logger.Debug(fmt.Sprintf("getting flag with query: %s, %v", indexId, constraints))
@@ -218,10 +217,7 @@ func (f *Store) String() (string, error) {
 // GetAll returns a copy of the store's state (copy in order to be concurrency safe)
 func (s *Store) GetAll(ctx context.Context, selector *Selector) (map[string]model.Flag, model.Metadata, error) {
 	flags := make(map[string]model.Flag)
-	queryMeta := model.Metadata{}
-	if selector != nil && !selector.IsEmpty() {
-		queryMeta = selector.ToMetadata()
-	}
+	queryMeta := selector.ToMetadata()
 	it, err := s.selectOrAll(selector)
 
 	if err != nil {