fix: incorrect metadata returned per source (#1599)
This PR fixes an issue where the wrong metadata is returned when a selector is used. This bug is easy to fix, but was also easy to write. We eventually need to refactor the flag memory store (`flags.go`) to store all flags per source. This was retrofitted in to support selectors, from a single-flagSet model. To test, do the manual test specified in https://github.com/open-feature/flagd/issues/1597. Fixes: https://github.com/open-feature/flagd/issues/1597 Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
This commit is contained in:
parent
9ee0c573d6
commit
b333e11ecf
|
|
@ -70,7 +70,7 @@ func (f *State) Get(_ context.Context, key string) (model.Flag, model.Metadata,
|
||||||
metadata := f.getMetadata()
|
metadata := f.getMetadata()
|
||||||
flag, ok := f.Flags[key]
|
flag, ok := f.Flags[key]
|
||||||
if ok {
|
if ok {
|
||||||
metadata = f.getMetadataForSource(flag.Source)
|
metadata = f.GetMetadataForSource(flag.Source)
|
||||||
}
|
}
|
||||||
|
|
||||||
return flag, metadata, ok
|
return flag, metadata, ok
|
||||||
|
|
@ -319,7 +319,7 @@ func (f *State) Merge(
|
||||||
return notifications, resyncRequired
|
return notifications, resyncRequired
|
||||||
}
|
}
|
||||||
|
|
||||||
func (f *State) getMetadataForSource(source string) model.Metadata {
|
func (f *State) GetMetadataForSource(source string) model.Metadata {
|
||||||
perSource, ok := f.MetadataPerSource[source]
|
perSource, ok := f.MetadataPerSource[source]
|
||||||
if ok && perSource != nil {
|
if ok && perSource != nil {
|
||||||
return maps.Clone(perSource)
|
return maps.Clone(perSource)
|
||||||
|
|
|
||||||
|
|
@ -199,6 +199,8 @@ func (r *Multiplexer) reFill() error {
|
||||||
|
|
||||||
// for all flags, sort them into their correct selector
|
// for all flags, sort them into their correct selector
|
||||||
for source, flags := range collector {
|
for source, flags := range collector {
|
||||||
|
// store the corresponding metadata
|
||||||
|
metadata := r.store.GetMetadataForSource(source)
|
||||||
bytes, err := json.Marshal(map[string]interface{}{"flags": flags, "metadata": metadata})
|
bytes, err := json.Marshal(map[string]interface{}{"flags": flags, "metadata": metadata})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("unable to marshal flags: %w", err)
|
return fmt.Errorf("unable to marshal flags: %w", err)
|
||||||
|
|
|
||||||
|
|
@ -168,35 +168,94 @@ func TestGetAllFlags(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
// when - get all with open scope
|
// when - get all with open scope
|
||||||
flags, err := mux.GetAllFlags("")
|
flagConfig, err := mux.GetAllFlags("")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal("error when retrieving all flags")
|
t.Fatal("error when retrieving all flags")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(flags) == 0 {
|
if len(flagConfig) == 0 {
|
||||||
t.Fatal("expected no empty flags")
|
t.Fatal("expected no empty flags")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// when - get all with a scope
|
// when - get all with a scope
|
||||||
flags, err = mux.GetAllFlags("A")
|
flagConfig, err = mux.GetAllFlags("A")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal("error when retrieving all flags")
|
t.Fatal("error when retrieving all flags")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(flags) == 0 || !strings.Contains(flags, fmt.Sprintf("\"source\":\"%s\"", "A")) {
|
if len(flagConfig) == 0 || !strings.Contains(flagConfig, fmt.Sprintf("\"source\":\"%s\"", "A")) {
|
||||||
t.Fatal("expected flags to be scoped")
|
t.Fatal("expected flags to be scoped")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// when - get all for a flagless-scope
|
// when - get all for a flagless-scope
|
||||||
flags, err = mux.GetAllFlags("C")
|
flagConfig, err = mux.GetAllFlags("C")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal("error when retrieving all flags")
|
t.Fatal("error when retrieving all flags")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
assert.Equal(t, flags, emptyConfigString)
|
assert.Equal(t, flagConfig, emptyConfigString)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestGetAllFlagsMetadata(t *testing.T) {
|
||||||
|
// given
|
||||||
|
mux, err := NewMux(getSimpleFlagStore())
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal("error during flag extraction")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// when - get all with open scope
|
||||||
|
flagConfig, err := mux.GetAllFlags("")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal("error when retrieving all flags")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(flagConfig) == 0 {
|
||||||
|
t.Fatal("expected no empty flags")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if !strings.Contains(flagConfig, "\"keyA\":\"valueA\"") {
|
||||||
|
t.Fatal("expected unique metadata key for A to be present")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if !strings.Contains(flagConfig, "\"keyB\":\"valueB\"") {
|
||||||
|
t.Fatal("expected unique metadata key for B to be present")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// duplicated keys are removed
|
||||||
|
if strings.Contains(flagConfig, "\"keyDuped\":\"value\"") {
|
||||||
|
t.Fatal("expected duplicated metadata key NOT to be present")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// when - get all with a scope
|
||||||
|
flagConfig, err = mux.GetAllFlags("A")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal("error when retrieving all flags")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(flagConfig) == 0 {
|
||||||
|
t.Fatal("expected no empty flags")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if !strings.Contains(flagConfig, "\"keyA\":\"valueA\"") {
|
||||||
|
t.Fatal("expected unique metadata key to be present")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if !strings.Contains(flagConfig, "\"keyDuped\":\"value\"") {
|
||||||
|
t.Fatal("expected duplicated metadata key to be present")
|
||||||
|
return
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -34,6 +34,16 @@ func getSimpleFlagStore() (*store.State, []string) {
|
||||||
Source: "B",
|
Source: "B",
|
||||||
})
|
})
|
||||||
|
|
||||||
|
flagStore.MetadataPerSource["A"] = model.Metadata{
|
||||||
|
"keyDuped": "value",
|
||||||
|
"keyA": "valueA",
|
||||||
|
}
|
||||||
|
|
||||||
|
flagStore.MetadataPerSource["B"] = model.Metadata{
|
||||||
|
"keyDuped": "value",
|
||||||
|
"keyB": "valueB",
|
||||||
|
}
|
||||||
|
|
||||||
return flagStore, []string{"A", "B", "C"}
|
return flagStore, []string{"A", "B", "C"}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue