From 96722e8e05daffa9cfd23824cb447865b1e908dd Mon Sep 17 00:00:00 2001 From: Daniel Adam Date: Mon, 27 May 2024 16:59:36 +0200 Subject: [PATCH] fixup! Introduce snippet-service --- snippet-service/service/grpc/server.go | 2 +- .../service/http/createConfiguration_test.go | 1 - .../service/http/getConfigurations_test.go | 120 +++++++++++++++++ snippet-service/service/http/service.go | 22 ++- .../service/http/updateConfiguration_test.go | 1 - .../store/mongodb/configuration.go | 23 ++-- .../store/mongodb/configuration_test.go | 127 +++--------------- snippet-service/test/test.go | 15 +++ 8 files changed, 175 insertions(+), 136 deletions(-) create mode 100644 snippet-service/service/http/getConfigurations_test.go diff --git a/snippet-service/service/grpc/server.go b/snippet-service/service/grpc/server.go index 5b1c15fc8..80acaf111 100644 --- a/snippet-service/service/grpc/server.go +++ b/snippet-service/service/grpc/server.go @@ -75,7 +75,7 @@ func (s *SnippetServiceServer) UpdateConfiguration(ctx context.Context, conf *pb func (s *SnippetServiceServer) GetConfigurations(req *pb.GetConfigurationsRequest, srv pb.SnippetService_GetConfigurationsServer) error { owner, err := s.checkOwner(srv.Context(), "") if err != nil { - return s.logger.LogAndReturnError(status.Errorf(codes.PermissionDenied, "cannot update configuration: %v", err)) + return s.logger.LogAndReturnError(status.Errorf(codes.PermissionDenied, "cannot get configurations: %v", err)) } err = s.store.GetConfigurations(srv.Context(), owner, req, func(ctx context.Context, iter store.Iterator[store.Configuration]) error { diff --git a/snippet-service/service/http/createConfiguration_test.go b/snippet-service/service/http/createConfiguration_test.go index aae3e55b1..76de8fdad 100644 --- a/snippet-service/service/http/createConfiguration_test.go +++ b/snippet-service/service/http/createConfiguration_test.go @@ -57,7 +57,6 @@ func TestRequestHandlerCreateConfiguration(t *testing.T) { tests := []struct { name string args args - wantData map[string]interface{} wantHTTPCode int wantErr bool }{ diff --git a/snippet-service/service/http/getConfigurations_test.go b/snippet-service/service/http/getConfigurations_test.go new file mode 100644 index 000000000..0f8f2f937 --- /dev/null +++ b/snippet-service/service/http/getConfigurations_test.go @@ -0,0 +1,120 @@ +package http_test + +import ( + "context" + "crypto/tls" + "errors" + "io" + "net/http" + "testing" + + "github.com/plgd-dev/go-coap/v3/message" + pkgHttp "github.com/plgd-dev/hub/v2/pkg/net/http" + "github.com/plgd-dev/hub/v2/snippet-service/pb" + snippetHttp "github.com/plgd-dev/hub/v2/snippet-service/service/http" + "github.com/plgd-dev/hub/v2/snippet-service/test" + hubTest "github.com/plgd-dev/hub/v2/test" + "github.com/plgd-dev/hub/v2/test/config" + httpTest "github.com/plgd-dev/hub/v2/test/http" + oauthTest "github.com/plgd-dev/hub/v2/test/oauth-server/test" + "github.com/plgd-dev/hub/v2/test/service" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials" +) + +func TestRequestHandlerGetConfigurations(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), config.TEST_TIMEOUT) + defer cancel() + + shutDown := service.SetUpServices(context.Background(), t, service.SetUpServicesOAuth) + defer shutDown() + + snippetCfg := test.MakeConfig(t) + shutdownHttp := test.New(t, snippetCfg) + defer shutdownHttp() + + conn, err := grpc.NewClient(config.SNIPPET_SERVICE_HOST, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{ + RootCAs: hubTest.GetRootCertificatePool(t), + }))) + require.NoError(t, err) + defer func() { + _ = conn.Close() + }() + c := pb.NewSnippetServiceClient(conn) + confs := test.AddConfigurations(ctx, t, snippetCfg.APIs.GRPC.Authorization.OwnerClaim, c, 30) + + type args struct { + accept string + token string + } + tests := []struct { + name string + args args + wantHTTPCode int + wantErr bool + want func(*testing.T, []*pb.Configuration) + }{ + { + name: test.ConfigurationOwner(1) + "/all", + args: args{ + accept: pkgHttp.ApplicationProtoJsonContentType, + token: oauthTest.GetAccessToken(t, config.OAUTH_SERVER_HOST, oauthTest.ClientTest, map[string]interface{}{ + snippetCfg.APIs.GRPC.Authorization.OwnerClaim: test.ConfigurationOwner(1), + }), + }, + wantHTTPCode: http.StatusOK, + want: func(t *testing.T, values []*pb.Configuration) { + require.NotEmpty(t, values) + for _, v := range values { + conf, ok := confs[v.GetId()] + require.True(t, ok) + test.ConfigurationContains(t, conf, v) + } + }, + }, + { + name: "missing owner", + args: args{ + accept: pkgHttp.ApplicationProtoJsonContentType, + token: oauthTest.GetAccessToken(t, config.OAUTH_SERVER_HOST, oauthTest.ClientTest, map[string]interface{}{ + snippetCfg.APIs.GRPC.Authorization.OwnerClaim: nil, + }), + }, + wantHTTPCode: http.StatusForbidden, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rb := httpTest.NewRequest(http.MethodGet, test.HTTPURI(snippetHttp.Configurations), nil).AuthToken(tt.args.token) + rb = rb.Accept(tt.args.accept).ContentType(message.AppCBOR.String()) + resp := httpTest.Do(t, rb.Build(ctx, t)) + defer func() { + _ = resp.Body.Close() + }() + require.Equal(t, tt.wantHTTPCode, resp.StatusCode) + + if tt.wantErr { + return + } + + values := make([]*pb.Configuration, 0, 1) + for { + var value pb.Configuration + err = httpTest.Unmarshal(resp.StatusCode, resp.Body, &value) + if errors.Is(err, io.EOF) { + break + } + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + values = append(values, &value) + } + tt.want(t, values) + }) + } +} diff --git a/snippet-service/service/http/service.go b/snippet-service/service/http/service.go index e772cdea1..72fb26714 100644 --- a/snippet-service/service/http/service.go +++ b/snippet-service/service/http/service.go @@ -2,7 +2,6 @@ package http import ( "fmt" - "strings" "github.com/plgd-dev/hub/v2/http-gateway/uri" "github.com/plgd-dev/hub/v2/pkg/fsnotify" @@ -23,15 +22,14 @@ type Service struct { // New parses configuration and creates new Server with provided store and bus func New(serviceName string, config Config, snippetServiceServer *grpcService.SnippetServiceServer, validator *validator.Validator, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider) (*Service, error) { service, err := httpService.New(httpService.Config{ - HTTPConnection: config.Connection, - HTTPServer: config.Server, - ServiceName: serviceName, - AuthRules: kitNetHttp.NewDefaultAuthorizationRules(uri.API), - FileWatcher: fileWatcher, - Logger: logger, - TraceProvider: tracerProvider, - Validator: validator, - QueryCaseInsensitive: queryCaseInsensitive, + HTTPConnection: config.Connection, + HTTPServer: config.Server, + ServiceName: serviceName, + AuthRules: kitNetHttp.NewDefaultAuthorizationRules(uri.API), + FileWatcher: fileWatcher, + Logger: logger, + TraceProvider: tracerProvider, + Validator: validator, }) if err != nil { return nil, fmt.Errorf("cannot create http service: %w", err) @@ -48,7 +46,3 @@ func New(serviceName string, config Config, snippetServiceServer *grpcService.Sn requestHandler: requestHandler, }, nil } - -var queryCaseInsensitive = map[string]string{ - strings.ToLower(ConfigurationIDKey): "id", -} diff --git a/snippet-service/service/http/updateConfiguration_test.go b/snippet-service/service/http/updateConfiguration_test.go index 550e9e8ae..a18cc6dfa 100644 --- a/snippet-service/service/http/updateConfiguration_test.go +++ b/snippet-service/service/http/updateConfiguration_test.go @@ -66,7 +66,6 @@ func TestRequestHandlerUpdateConfiguration(t *testing.T) { tests := []struct { name string args args - wantData map[string]interface{} wantHTTPCode int wantErr bool }{ diff --git a/snippet-service/store/mongodb/configuration.go b/snippet-service/store/mongodb/configuration.go index 216da33dc..95d91edb9 100644 --- a/snippet-service/store/mongodb/configuration.go +++ b/snippet-service/store/mongodb/configuration.go @@ -76,31 +76,31 @@ func compareIdFilter(i, j *pb.IDFilter) int { return cmp.Compare(i.GetValue(), j.GetValue()) } -func normalizeIdFilter(idfilter []*pb.IDFilter) []*pb.IDFilter { +func checkEmptyIdFilter(idfilter []*pb.IDFilter) []*pb.IDFilter { // if an empty query is provided, return all if len(idfilter) == 0 { return nil } - slices.SortFunc(idfilter, compareIdFilter) - // if the first filter is All, we can ignore all other filters first := idfilter[0] if first.GetId() == "" && first.GetAll() { return nil } + return idfilter +} + +func normalizeIdFilter(idfilter []*pb.IDFilter) []*pb.IDFilter { + idfilter = checkEmptyIdFilter(idfilter) + if len(idfilter) == 0 { + return nil + } updatedFilter := make([]*pb.IDFilter, 0) var idAll bool var idLatest bool var idValue bool var idValueVersion uint64 - setNextInitial := func(idf *pb.IDFilter) { - idAll = idf.GetAll() - idLatest = idf.GetLatest() - idValue = !idAll && !idLatest - idValueVersion = idf.GetValue() - } setNextLatest := func(idf *pb.IDFilter) { // we already have the latest filter if idLatest { @@ -123,7 +123,10 @@ func normalizeIdFilter(idfilter []*pb.IDFilter) []*pb.IDFilter { prevID := "" for _, idf := range idfilter { if idf.GetId() != prevID { - setNextInitial(idf) + idAll = idf.GetAll() + idLatest = idf.GetLatest() + idValue = !idAll && !idLatest + idValueVersion = idf.GetValue() updatedFilter = append(updatedFilter, idf) } diff --git a/snippet-service/store/mongodb/configuration_test.go b/snippet-service/store/mongodb/configuration_test.go index 9a72929ec..ffa39f529 100644 --- a/snippet-service/store/mongodb/configuration_test.go +++ b/snippet-service/store/mongodb/configuration_test.go @@ -4,7 +4,6 @@ import ( "cmp" "context" "slices" - "strconv" "testing" "github.com/google/uuid" @@ -232,103 +231,13 @@ func TestUpdateConfiguration(t *testing.T) { } } -var testConfigurationIDs = make(map[int]string) - -func testConfigurationID(i int) string { - if id, ok := testConfigurationIDs[i]; ok { - return id - } - id := uuid.New().String() - testConfigurationIDs[i] = id - return id -} - -func testConfigurationName(i int) string { - return "cfg" + strconv.Itoa(i) -} - -func testConfigurationOwner(i int) string { - return "owner" + strconv.Itoa(i) -} - -func testConfigurationResources(t *testing.T, start, n int) []*pb.Configuration_Resource { - resources := make([]*pb.Configuration_Resource, 0, n) - for i := start; i < start+n; i++ { - resources = append(resources, &pb.Configuration_Resource{ - Href: hubTest.TestResourceLightInstanceHref(strconv.Itoa(i)), - Content: &commands.Content{ - Data: hubTest.EncodeToCbor(t, map[string]interface{}{ - "power": i, - }), - ContentType: message.AppOcfCbor.String(), - CoapContentFormat: int32(message.AppOcfCbor), - }, - TimeToLive: 1337, - }) - } - return resources -} - -func addConfigurationsToStore(ctx context.Context, t *testing.T, s store.Store, n int) map[string]store.Configuration { - const numConfigs = 10 - const numOwners = 3 - versions := make(map[int]uint64, numConfigs) - owners := make(map[int]string, numConfigs) - configurations := make(map[string]store.Configuration) - for i := 0; i < n; i++ { - version, ok := versions[i%numConfigs] - if !ok { - version = 0 - versions[i%numConfigs] = version - } - versions[i%numConfigs]++ - owner, ok := owners[i%numConfigs] - if !ok { - owner = testConfigurationOwner(i % numOwners) - owners[i%numConfigs] = owner - } - confIn := &pb.Configuration{ - Id: testConfigurationID(i % numConfigs), - Version: version, - Resources: testConfigurationResources(t, i%16, (i%5)+1), - Owner: owner, - } - var conf *pb.Configuration - var err error - if !ok { - confIn.Name = testConfigurationName(i % numConfigs) - conf, err = s.CreateConfiguration(ctx, confIn) - require.NoError(t, err) - } else { - conf, err = s.UpdateConfiguration(ctx, confIn) - require.NoError(t, err) - } - - configuration, ok := configurations[conf.GetId()] - if !ok { - configuration = store.Configuration{ - Id: conf.GetId(), - Owner: conf.GetOwner(), - Name: conf.GetName(), - } - configurations[conf.GetId()] = configuration - } - configuration.Versions = append(configuration.Versions, store.ConfigurationVersion{ - Version: conf.GetVersion(), - Resources: conf.GetResources(), - }) - configurations[conf.GetId()] = configuration - } - return configurations -} - func TestStoreGetConfigurations(t *testing.T) { s, cleanUpStore := test.NewMongoStore(t) defer cleanUpStore() ctx, cancel := context.WithTimeout(context.Background(), config.TEST_TIMEOUT) defer cancel() - confs := addConfigurationsToStore(ctx, t, s, 500) + confs := test.AddConfigurationsToStore(ctx, t, s, 500) type args struct { owner string @@ -353,13 +262,13 @@ func TestStoreGetConfigurations(t *testing.T) { { name: "owner0", args: args{ - owner: testConfigurationOwner(0), + owner: test.ConfigurationOwner(0), query: nil, }, want: func(t *testing.T, configurations []*store.Configuration) { require.NotEmpty(t, configurations) for _, c := range configurations { - require.Equal(t, testConfigurationOwner(0), c.Owner) + require.Equal(t, test.ConfigurationOwner(0), c.Owner) conf, ok := confs[c.Id] require.True(t, ok) test.CmpJSON(t, &conf, c) @@ -373,7 +282,7 @@ func TestStoreGetConfigurations(t *testing.T) { query: &pb.GetConfigurationsRequest{ IdFilter: []*pb.IDFilter{ { - Id: testConfigurationID(1), + Id: test.ConfigurationID(1), Version: &pb.IDFilter_All{ All: true, }, @@ -392,11 +301,11 @@ func TestStoreGetConfigurations(t *testing.T) { { name: "owner2/id2/all", args: args{ - owner: testConfigurationOwner(2), + owner: test.ConfigurationOwner(2), query: &pb.GetConfigurationsRequest{ IdFilter: []*pb.IDFilter{ { - Id: testConfigurationID(2), + Id: test.ConfigurationID(2), Version: &pb.IDFilter_All{ All: true, }, @@ -409,8 +318,8 @@ func TestStoreGetConfigurations(t *testing.T) { c := configurations[0] conf, ok := confs[c.Id] require.True(t, ok) - require.Equal(t, testConfigurationID(2), conf.Id) - require.Equal(t, testConfigurationOwner(2), conf.Owner) + require.Equal(t, test.ConfigurationID(2), conf.Id) + require.Equal(t, test.ConfigurationOwner(2), conf.Owner) test.CmpJSON(t, &conf, c) }, }, @@ -439,7 +348,7 @@ func TestStoreGetConfigurations(t *testing.T) { { name: "owner1/latest", args: args{ - owner: testConfigurationOwner(1), + owner: test.ConfigurationOwner(1), query: &pb.GetConfigurationsRequest{ IdFilter: []*pb.IDFilter{ { @@ -455,7 +364,7 @@ func TestStoreGetConfigurations(t *testing.T) { for _, c := range configurations { conf, ok := confs[c.Id] require.True(t, ok) - require.Equal(t, testConfigurationOwner(1), conf.Owner) + require.Equal(t, test.ConfigurationOwner(1), conf.Owner) require.Len(t, c.Versions, 1) } }, @@ -463,11 +372,11 @@ func TestStoreGetConfigurations(t *testing.T) { { name: "owner1/latest - non-matching owner", args: args{ - owner: testConfigurationOwner(2), + owner: test.ConfigurationOwner(2), query: &pb.GetConfigurationsRequest{ IdFilter: []*pb.IDFilter{ { - Id: testConfigurationID(1), + Id: test.ConfigurationID(1), Version: &pb.IDFilter_Latest{ Latest: true, }, @@ -481,7 +390,7 @@ func TestStoreGetConfigurations(t *testing.T) { }, { name: "owner2{latest, id2/latest, id5/latest} - non-matching owner", args: args{ - owner: testConfigurationOwner(2), + owner: test.ConfigurationOwner(2), query: &pb.GetConfigurationsRequest{ IdFilter: []*pb.IDFilter{ { @@ -490,13 +399,13 @@ func TestStoreGetConfigurations(t *testing.T) { }, }, { - Id: testConfigurationID(2), + Id: test.ConfigurationID(2), Version: &pb.IDFilter_Latest{ Latest: true, }, }, { - Id: testConfigurationID(5), + Id: test.ConfigurationID(5), Version: &pb.IDFilter_Latest{ Latest: true, }, @@ -509,7 +418,7 @@ func TestStoreGetConfigurations(t *testing.T) { for _, c := range configurations { conf, ok := confs[c.Id] require.True(t, ok) - require.Equal(t, testConfigurationOwner(2), conf.Owner) + require.Equal(t, test.ConfigurationOwner(2), conf.Owner) require.Len(t, c.Versions, 1) } }, @@ -538,7 +447,7 @@ func TestStoreGetConfigurations(t *testing.T) { }, { name: "owner3/version/{13, 37, 42}", args: args{ - owner: testConfigurationOwner(2), + owner: test.ConfigurationOwner(2), query: &pb.GetConfigurationsRequest{ IdFilter: []*pb.IDFilter{ { @@ -564,7 +473,7 @@ func TestStoreGetConfigurations(t *testing.T) { }, // filter with Id should be ignored if there are filters without Id { - Id: testConfigurationID(2), + Id: test.ConfigurationID(2), Version: &pb.IDFilter_Value{ Value: 37, }, diff --git a/snippet-service/test/test.go b/snippet-service/test/test.go index b252d9081..53adbcb75 100644 --- a/snippet-service/test/test.go +++ b/snippet-service/test/test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/plgd-dev/hub/v2/snippet-service/pb" + "github.com/plgd-dev/hub/v2/snippet-service/store" "github.com/plgd-dev/hub/v2/test" "github.com/plgd-dev/kit/v2/codec/json" "github.com/stretchr/testify/require" @@ -39,3 +40,17 @@ func CmpConfiguration(t *testing.T, want, got *pb.Configuration) { } CmpJSON(t, want, got) } + +func ConfigurationContains(t *testing.T, storeConf store.Configuration, conf *pb.Configuration) { + require.Equal(t, storeConf.Id, conf.GetId()) + require.Equal(t, storeConf.Owner, conf.GetOwner()) + require.Equal(t, storeConf.Name, conf.GetName()) + for _, v := range storeConf.Versions { + if v.Version != conf.GetVersion() { + continue + } + test.CheckProtobufs(t, v.Resources, conf.GetResources(), test.RequireToCheckFunc(require.Equal)) + return + } + require.Fail(t, "version not found") +}