From 1bae7738331618242badfb27a0fca71f694a89b1 Mon Sep 17 00:00:00 2001 From: Thomas Poignant Date: Mon, 7 Oct 2024 20:45:43 +0200 Subject: [PATCH] chore: Coverage increase (#2478) --- cmd/relayproxy/api/server_test.go | 84 +++++++++++++ codecov.yml | 1 + config_test.go | 9 ++ exporter/gcstorageexporter/exporter.go | 4 +- go.mod | 1 + go.sum | 2 + internal/flagstate/all_flags_test.go | 47 +++++++ model/dto/converter_test.go | 2 +- retriever/mongodbretriever/retriever.go | 3 + retriever/mongodbretriever/retriever_test.go | 122 ++++++++++++++----- testutils/mock/exporter_mock.go | 4 +- testutils/mongodb_mock.go | 62 +--------- 12 files changed, 249 insertions(+), 92 deletions(-) create mode 100644 internal/flagstate/all_flags_test.go diff --git a/cmd/relayproxy/api/server_test.go b/cmd/relayproxy/api/server_test.go index f10e6a54d02..1d3ceb9b8cd 100644 --- a/cmd/relayproxy/api/server_test.go +++ b/cmd/relayproxy/api/server_test.go @@ -2,6 +2,7 @@ package api_test import ( "net/http" + "strings" "testing" "time" @@ -133,3 +134,86 @@ func Test_Starting_RelayProxy_with_monitoring_on_different_port(t *testing.T) { assert.NoError(t, err) assert.Equal(t, http.StatusOK, responseI1.StatusCode) } + +func Test_CheckOFREPAPIExists(t *testing.T) { + proxyConf := &config.Config{ + Retriever: &config.RetrieverConf{ + Kind: "file", + Path: "../../../testdata/flag-config.yaml", + }, + ListenPort: 11024, + AuthorizedKeys: config.APIKeys{ + Admin: nil, + Evaluation: []string{"test"}, + }, + } + log := log.InitLogger() + defer func() { _ = log.ZapLogger.Sync() }() + + metricsV2, err := metric.NewMetrics() + if err != nil { + log.ZapLogger.Error("impossible to initialize prometheus metrics", zap.Error(err)) + } + wsService := service.NewWebsocketService() + defer wsService.Close() // close all the open connections + prometheusNotifier := metric.NewPrometheusNotifier(metricsV2) + proxyNotifier := service.NewNotifierWebsocket(wsService) + goff, err := service.NewGoFeatureFlagClient(proxyConf, log.ZapLogger, []notifier.Notifier{ + prometheusNotifier, + proxyNotifier, + }) + if err != nil { + panic(err) + } + + services := service.Services{ + MonitoringService: service.NewMonitoring(goff), + WebsocketService: wsService, + GOFeatureFlagService: goff, + Metrics: metricsV2, + } + + s := api.New(proxyConf, services, log.ZapLogger) + go func() { s.Start() }() + defer s.Stop() + + time.Sleep(10 * time.Millisecond) + + req, err := http.NewRequest("GET", + "http://localhost:11024/ofrep/v1/configuration", nil) + assert.NoError(t, err) + req.Header.Add("Authorization", "Bearer test") + response, err := http.DefaultClient.Do(req) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, response.StatusCode) + + req, err = http.NewRequest("POST", + "http://localhost:11024/ofrep/v1/evaluate/flags", + strings.NewReader(`{ "context":{"targetingKey":"some-key"}}`)) + assert.NoError(t, err) + req.Header.Add("Authorization", "Bearer test") + req.Header.Add("Content-Type", "application/json") + response, err = http.DefaultClient.Do(req) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, response.StatusCode) + + req, err = http.NewRequest("POST", + "http://localhost:11024/ofrep/v1/evaluate/flags/some-key", + strings.NewReader(`{ "context":{"targetingKey":"some-key"}}`)) + assert.NoError(t, err) + req.Header.Add("Authorization", "Bearer test") + req.Header.Add("Content-Type", "application/json") + response, err = http.DefaultClient.Do(req) + assert.NoError(t, err) + assert.Equal(t, http.StatusNotFound, response.StatusCode) + + req, err = http.NewRequest("POST", + "http://localhost:11024/ofrep/v1/evaluate/flags/test-flag", + strings.NewReader(`{ "context":{"targetingKey":"some-key"}}`)) + assert.NoError(t, err) + req.Header.Add("Authorization", "Bearer test") + req.Header.Add("Content-Type", "application/json") + response, err = http.DefaultClient.Do(req) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, response.StatusCode) +} diff --git a/codecov.yml b/codecov.yml index fe588c1f662..b2f4c883e74 100644 --- a/codecov.yml +++ b/codecov.yml @@ -12,3 +12,4 @@ ignore: - "cmd/relayproxy/docs/" - "cmd/jsonschema-generator/" - "cmd/relayproxy/modeldocs/" + - "model/dto/converter_v0.go" diff --git a/config_test.go b/config_test.go index db62667fac0..af2b40538c1 100644 --- a/config_test.go +++ b/config_test.go @@ -113,3 +113,12 @@ func TestConfig_GetRetrievers(t *testing.T) { }) } } + +func TestOfflineConfig(t *testing.T) { + c := ffClient.Config{ + Offline: true, + } + assert.True(t, c.IsOffline()) + c.SetOffline(false) + assert.False(t, c.IsOffline()) +} diff --git a/exporter/gcstorageexporter/exporter.go b/exporter/gcstorageexporter/exporter.go index 5ddbc075557..48b38e2a9e1 100644 --- a/exporter/gcstorageexporter/exporter.go +++ b/exporter/gcstorageexporter/exporter.go @@ -95,7 +95,7 @@ func (f *Exporter) Export(ctx context.Context, logger *fflog.FFLogger, featureEv for _, file := range files { of, err := os.Open(outputDir + "/" + file.Name()) if err != nil { - logger.Error("[GCP DeprecatedExporter] impossible to open the file", + logger.Error("[GCP Exporter] impossible to open the file", slog.String("path", outputDir+"/"+file.Name())) continue } @@ -111,7 +111,7 @@ func (f *Exporter) Export(ctx context.Context, logger *fflog.FFLogger, featureEv _, err = io.Copy(wc, of) _ = wc.Close() if err != nil { - return fmt.Errorf("error: [DeprecatedExporter] impossible to copy the file from %s to bucket %s: %v", + return fmt.Errorf("error: [GCP Exporter] impossible to copy the file from %s to bucket %s: %v", source, f.Bucket, err) } } diff --git a/go.mod b/go.mod index 3fbe4eb6aab..dfd5d941387 100644 --- a/go.mod +++ b/go.mod @@ -44,6 +44,7 @@ require ( github.com/swaggo/echo-swagger v1.4.1 github.com/swaggo/swag v1.16.3 github.com/testcontainers/testcontainers-go v0.33.0 + github.com/testcontainers/testcontainers-go/modules/mongodb v0.33.0 github.com/testcontainers/testcontainers-go/modules/redis v0.33.0 github.com/thejerf/slogassert v0.3.4 github.com/xitongsys/parquet-go v1.6.2 diff --git a/go.sum b/go.sum index 0c53371a288..fd3f40dbe04 100644 --- a/go.sum +++ b/go.sum @@ -862,6 +862,8 @@ github.com/swaggo/swag v1.16.3 h1:PnCYjPCah8FK4I26l2F/KQ4yz3sILcVUN3cTlBFA9Pg= github.com/swaggo/swag v1.16.3/go.mod h1:DImHIuOFXKpMFAQjcC7FG4m3Dg4+QuUgUzJmKjI/gRk= github.com/testcontainers/testcontainers-go v0.33.0 h1:zJS9PfXYT5O0ZFXM2xxXfk4J5UMw/kRiISng037Gxdw= github.com/testcontainers/testcontainers-go v0.33.0/go.mod h1:W80YpTa8D5C3Yy16icheD01UTDu+LmXIA2Keo+jWtT8= +github.com/testcontainers/testcontainers-go/modules/mongodb v0.33.0 h1:iXVA84s5hKMS5gn01GWOYHE3ymy/2b+0YkpFeTxB2XY= +github.com/testcontainers/testcontainers-go/modules/mongodb v0.33.0/go.mod h1:R6tMjTojRiaoo89fh/hf7tOmfzohdqSU17R9DwSVSog= github.com/testcontainers/testcontainers-go/modules/redis v0.33.0 h1:S/QvMOwpr00MM2aWH+krzP73Erlp/Ug0dr2rkgZYI5s= github.com/testcontainers/testcontainers-go/modules/redis v0.33.0/go.mod h1:gudb3+6uZ9SsAysOVoLs7nazbjGlkHegBW8nqPXvDMI= github.com/thejerf/slogassert v0.3.4 h1:VoTsXixRbXMrRSSxDjYTiEDCM4VWbsYPW5rB/hX24kM= diff --git a/internal/flagstate/all_flags_test.go b/internal/flagstate/all_flags_test.go new file mode 100644 index 00000000000..04d4e8dd6f2 --- /dev/null +++ b/internal/flagstate/all_flags_test.go @@ -0,0 +1,47 @@ +package flagstate_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/thomaspoignant/go-feature-flag/internal/flag" + "github.com/thomaspoignant/go-feature-flag/internal/flagstate" +) + +func TestAllFlags(t *testing.T) { + afs := flagstate.NewAllFlags() + assert.NotNil(t, afs.GetFlags()) + assert.Equal(t, 0, len(afs.GetFlags())) + assert.True(t, afs.IsValid()) + + fs := flagstate.FlagState{ + Value: 20, + Timestamp: time.Date(2022, 8, 1, 0, 0, 10, 0, time.UTC).Unix(), + VariationType: "var_a", + TrackEvents: false, + Failed: false, + Reason: flag.ReasonStatic, + } + afs.AddFlag("my-key", fs) + assert.Equal(t, 1, len(afs.GetFlags())) + assert.True(t, afs.IsValid()) + fs2 := flagstate.FlagState{ + Value: 20, + Timestamp: time.Date(2022, 8, 1, 0, 0, 10, 0, time.UTC).Unix(), + VariationType: "var_a", + TrackEvents: false, + Failed: true, + ErrorCode: flag.ErrorCodeTargetingKeyMissing, + ErrorDetails: "The targeting key is missing", + Reason: flag.ReasonError, + } + afs.AddFlag("my-key-2", fs2) + assert.Equal(t, 2, len(afs.GetFlags())) + assert.False(t, afs.IsValid()) + + want := "{\"flags\":{\"my-key\":{\"value\":20,\"timestamp\":1659312010,\"variationType\":\"var_a\",\"trackEvents\":false,\"errorCode\":\"\",\"reason\":\"STATIC\"},\"my-key-2\":{\"value\":20,\"timestamp\":1659312010,\"variationType\":\"var_a\",\"trackEvents\":false,\"errorCode\":\"TARGETING_KEY_MISSING\",\"errorDetails\":\"The targeting key is missing\",\"reason\":\"ERROR\"}},\"valid\":false}\n" + got, err := afs.MarshalJSON() + assert.NoError(t, err) + assert.JSONEq(t, want, string(got)) +} diff --git a/model/dto/converter_test.go b/model/dto/converter_test.go index 72c724587d8..7bd44013ffa 100644 --- a/model/dto/converter_test.go +++ b/model/dto/converter_test.go @@ -137,7 +137,7 @@ func TestConvertV1DtoToInternalFlag(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := dto.ConvertV1DtoToInternalFlag(tt.input) + result := tt.input.Convert(nil, "random-flag-name") assert.Equal(t, tt.expected, result) }) } diff --git a/retriever/mongodbretriever/retriever.go b/retriever/mongodbretriever/retriever.go index de073e9e05e..32494d1bca0 100644 --- a/retriever/mongodbretriever/retriever.go +++ b/retriever/mongodbretriever/retriever.go @@ -44,6 +44,9 @@ func (r *Retriever) Init(ctx context.Context, logger *fflog.FFLogger) error { // Status returns the current status of the retriever func (r *Retriever) Status() retriever.Status { + if r == nil || r.status == "" { + return retriever.RetrieverNotReady + } return r.status } diff --git a/retriever/mongodbretriever/retriever_test.go b/retriever/mongodbretriever/retriever_test.go index f417d35e0d0..e7171e18787 100644 --- a/retriever/mongodbretriever/retriever_test.go +++ b/retriever/mongodbretriever/retriever_test.go @@ -1,11 +1,21 @@ -package mongodbretriever +//go:build docker +// +build docker + +package mongodbretriever_test import ( "context" "encoding/json" + "github.com/stretchr/testify/require" + "github.com/thomaspoignant/go-feature-flag/retriever" + "github.com/thomaspoignant/go-feature-flag/retriever/mongodbretriever" + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/mongo" + "go.mongodb.org/mongo-driver/mongo/options" "testing" "github.com/stretchr/testify/assert" + "github.com/testcontainers/testcontainers-go/modules/mongodb" "github.com/thomaspoignant/go-feature-flag/testutils" "github.com/thomaspoignant/go-feature-flag/utils/fflog" "go.mongodb.org/mongo-driver/mongo/integration/mtest" @@ -17,64 +27,120 @@ func Test_MongoDBRetriever_Retrieve(t *testing.T) { tests := []struct { name string want []byte - mocker *func(t *mtest.T) + data string wantErr bool }{ { name: "Returns well formed flag definition document", - mocker: &testutils.MockSuccessFind, + data: testutils.MongoFindResultString, want: []byte(testutils.QueryResult), wantErr: false, }, { name: "One of the Flag definition document does not have 'flag' key/value (ignore this document)", - mocker: &testutils.MockNoFlagKeyResult, + data: testutils.MongoMissingFlagKey, want: []byte(testutils.MissingFlagKeyResult), wantErr: false, }, { name: "Flag definition document 'flag' key does not have 'string' value (ignore this document)", - mocker: &testutils.MockFlagNotStrResult, + data: testutils.MongoFindResultFlagNoStr, want: []byte(testutils.FlagKeyNotStringResult), wantErr: false, }, { name: "No flags found on DB", - mocker: &testutils.MockNoFlags, - want: nil, + want: []byte("{}"), wantErr: true, }, } for _, tt := range tests { mtDB.Run(tt.name, func(t *mtest.T) { - mdb := Retriever{ - URI: "mongouri", - Collection: "collection", - Database: "database", - dbConnection: t.DB, - } + mongodbContainer, err := mongodb.Run(context.TODO(), "mongo:6") + require.NoError(t, err) + defer func() { + err := mongodbContainer.Terminate(context.TODO()) + require.NoError(t, err) + }() - if tt.mocker != nil { - (*tt.mocker)(t) - } + uri, err := mongodbContainer.ConnectionString(context.Background()) - _ = mdb.Init(context.TODO(), &fflog.FFLogger{}) - got, err := mdb.Retrieve(context.Background()) + if tt.data != "" { + // insert data + client, err := mongo.Connect(context.TODO(), options.Client().ApplyURI(uri)) + coll := client.Database("database").Collection("collection") + var documents []bson.M + err = json.Unmarshal([]byte(tt.data), &documents) + require.NoError(t, err) - if tt.wantErr { - assert.Error(t, err, "Retrieve() error = %v, wantErr %v", err, tt.wantErr) - return + for _, doc := range documents { + _, err := coll.InsertOne(context.TODO(), doc) + require.NoError(t, err) + } } - var gotUnm, wantUn interface{} - if err := json.Unmarshal(tt.want, &wantUn); err != nil { - assert.Fail(t, "could not json unmarshall wanted flag structure") - } - if err := json.Unmarshal(got, &gotUnm); err != nil { - assert.Fail(t, "could not json unmarshall got flag structure") + // retriever + mdb := mongodbretriever.Retriever{ + URI: uri, + Collection: "collection", + Database: "database", } + assert.Equal(t, retriever.RetrieverNotReady, mdb.Status()) + err = mdb.Init(context.TODO(), &fflog.FFLogger{}) + assert.NoError(t, err) + defer func() { _ = mdb.Shutdown(context.TODO()) }() + assert.Equal(t, retriever.RetrieverReady, mdb.Status()) - assert.Equal(t, wantUn, gotUnm) + got, err := mdb.Retrieve(context.Background()) + if tt.want == nil { + assert.Nil(t, got) + } else { + modifiedGot, err := removeIDFromJSON(string(got)) + require.NoError(t, err) + assert.JSONEq(t, string(tt.want), modifiedGot) + } }) } } + +func Test_MongoDBRetriever_InvalidURI(t *testing.T) { + mdb := mongodbretriever.Retriever{ + URI: "invalidURI", + Collection: "collection", + Database: "database", + } + assert.Equal(t, retriever.RetrieverNotReady, mdb.Status()) + err := mdb.Init(context.TODO(), &fflog.FFLogger{}) + assert.Error(t, err) + assert.Equal(t, retriever.RetrieverError, mdb.Status()) +} + +func removeIDFromJSON(jsonStr string) (string, error) { + var data interface{} + if err := json.Unmarshal([]byte(jsonStr), &data); err != nil { + return "", err + } + + removeIDFields(data) + + modifiedJSON, err := json.Marshal(data) + if err != nil { + return "", err + } + + return string(modifiedJSON), nil +} + +func removeIDFields(data interface{}) { + switch v := data.(type) { + case map[string]interface{}: + delete(v, "_id") + for _, value := range v { + removeIDFields(value) + } + case []interface{}: + for _, item := range v { + removeIDFields(item) + } + } +} diff --git a/testutils/mock/exporter_mock.go b/testutils/mock/exporter_mock.go index 64478282f32..e8ce2b63511 100644 --- a/testutils/mock/exporter_mock.go +++ b/testutils/mock/exporter_mock.go @@ -2,10 +2,10 @@ package mock import ( "context" - "log" "sync" "github.com/thomaspoignant/go-feature-flag/exporter" + "github.com/thomaspoignant/go-feature-flag/utils/fflog" ) type Exporter struct { @@ -19,7 +19,7 @@ type Exporter struct { once sync.Once } -func (m *Exporter) Export(ctx context.Context, logger *log.Logger, events []exporter.FeatureEvent) error { +func (m *Exporter) Export(ctx context.Context, _ *fflog.FFLogger, events []exporter.FeatureEvent) error { m.once.Do(m.initMutex) m.mutex.Lock() defer m.mutex.Unlock() diff --git a/testutils/mongodb_mock.go b/testutils/mongodb_mock.go index f4f5f9dd1a2..b28b0acb8bb 100644 --- a/testutils/mongodb_mock.go +++ b/testutils/mongodb_mock.go @@ -1,69 +1,13 @@ package testutils -import ( - "encoding/json" - "fmt" +var MongoFindResultString = `[{"flag":"test-flag","variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"random-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false},{"flag":"test-flag2","variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"not-a-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false}]` - "go.mongodb.org/mongo-driver/bson" - "go.mongodb.org/mongo-driver/mongo/integration/mtest" -) - -var mongoFindResultString = `[{"flag":"test-flag","variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"random-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false},{"flag":"test-flag2","variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"not-a-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false}]` - -var mongoMissingFlagKey = `[{"flag":"test-flag","variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"random-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false},{"variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"not-a-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false}]` +var MongoMissingFlagKey = `[{"flag":"test-flag","variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"random-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false},{"variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"not-a-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false}]` var MissingFlagKeyResult = `{"test-flag":{"variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"random-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false}}` -var mongoFindResultFlagNoStr = `[{"flag":123456,"variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"random-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false},{"flag":"test-flag","variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"random-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false}]` +var MongoFindResultFlagNoStr = `[{"flag":123456,"variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"random-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false},{"flag":"test-flag","variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"random-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false}]` var FlagKeyNotStringResult = MissingFlagKeyResult var QueryResult = `{"test-flag":{"variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"random-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false},"test-flag2":{"variations":{"true_var":true,"false_var":false},"targeting":[{"query":"key eq \"not-a-key\"","percentage":{"true_var":0,"false_var":100}}],"defaultRule":{"variation":"false_var"},"trackEvents":false}}` - -func mapToBsonD(inputMap []map[string]interface{}) []bson.D { - bsonData := make([]bson.D, 0, 10) - - for _, v := range inputMap { - var b []bson.E - - for k, v := range v { - b = append(b, bson.E{Key: k, Value: v}) - } - - bsonData = append(bsonData, bson.D(b)) - } - - return bsonData -} - -func succesQueryMockerFactory(queryResult string) func(t *mtest.T) { - return func(t *mtest.T) { - var resultMap []map[string]interface{} - - // Unmarshal the JSON string into the map - err := json.Unmarshal([]byte(queryResult), &resultMap) - if err != nil { - fmt.Println("Error:", err) - return - } - - // Convert the map to a BSON document (bson.D) - bsonDocuments := mapToBsonD(resultMap) - - t.AddMockResponses(mtest.CreateCursorResponse(1, "dummyDB.dummyCollection", - mtest.FirstBatch, - bsonDocuments..., - )) - } -} - -var MockSuccessFind = succesQueryMockerFactory(mongoFindResultString) - -var MockNoFlagKeyResult = succesQueryMockerFactory(mongoMissingFlagKey) - -var MockFlagNotStrResult = succesQueryMockerFactory(mongoFindResultFlagNoStr) - -var MockNoFlags = func(t *mtest.T) { - - t.AddMockResponses(mtest.CreateCommandErrorResponse(mtest.CommandError{})) -}