From 2ae4959afb696557712b7000d85357b11d7008e4 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Fri, 4 Mar 2022 15:42:46 +0100 Subject: [PATCH] sonarcloud suggestions Signed-off-by: jkoberg --- settings/pkg/store/metadata/assignments.go | 22 +++++++++++-------- .../pkg/store/metadata/assignments_test.go | 7 +++--- settings/pkg/store/metadata/bundles.go | 16 +++++++++----- settings/pkg/store/metadata/store.go | 17 +++++++------- settings/pkg/store/metadata/store_test.go | 12 ++-------- settings/pkg/store/metadata/values.go | 14 ++++++++---- 6 files changed, 48 insertions(+), 40 deletions(-) diff --git a/settings/pkg/store/metadata/assignments.go b/settings/pkg/store/metadata/assignments.go index 4fe6c9173..bf80cb71e 100644 --- a/settings/pkg/store/metadata/assignments.go +++ b/settings/pkg/store/metadata/assignments.go @@ -2,6 +2,7 @@ package store import ( + "context" "encoding/json" "fmt" @@ -16,14 +17,15 @@ func (s *Store) ListRoleAssignments(accountUUID string) ([]*settingsmsg.UserRole return defaultRoleAssignments(accountUUID), nil } s.Init() - assIDs, err := s.mdc.ReadDir(nil, accountPath(accountUUID)) + ctx := context.TODO() + assIDs, err := s.mdc.ReadDir(ctx, accountPath(accountUUID)) if err != nil { return nil, err } - var ass []*settingsmsg.UserRoleAssignment + ass := make([]*settingsmsg.UserRoleAssignment, 0, len(assIDs)) for _, assID := range assIDs { - b, err := s.mdc.SimpleDownload(nil, assignmentPath(accountUUID, assID)) + b, err := s.mdc.SimpleDownload(ctx, assignmentPath(accountUUID, assID)) if err != nil { return nil, err } @@ -42,13 +44,14 @@ func (s *Store) ListRoleAssignments(accountUUID string) ([]*settingsmsg.UserRole // WriteRoleAssignment appends the given role assignment to the existing assignments of the respective account. func (s *Store) WriteRoleAssignment(accountUUID, roleID string) (*settingsmsg.UserRoleAssignment, error) { s.Init() + ctx := context.TODO() // as per https://github.com/owncloud/product/issues/103 "Each user can have exactly one role" - err := s.mdc.Delete(nil, accountPath(accountUUID)) + err := s.mdc.Delete(ctx, accountPath(accountUUID)) if err != nil { // TODO: How to differentiate between 'not found' and other errors? } - err = s.mdc.MakeDirIfNotExist(nil, accountPath(accountUUID)) + err = s.mdc.MakeDirIfNotExist(ctx, accountPath(accountUUID)) if err != nil { return nil, err } @@ -62,20 +65,21 @@ func (s *Store) WriteRoleAssignment(accountUUID, roleID string) (*settingsmsg.Us if err != nil { return nil, err } - return ass, s.mdc.SimpleUpload(nil, assignmentPath(accountUUID, ass.Id), b) + return ass, s.mdc.SimpleUpload(ctx, assignmentPath(accountUUID, ass.Id), b) } // RemoveRoleAssignment deletes the given role assignment from the existing assignments of the respective account. func (s *Store) RemoveRoleAssignment(assignmentID string) error { s.Init() - accounts, err := s.mdc.ReadDir(nil, accountsFolderLocation) + ctx := context.TODO() + accounts, err := s.mdc.ReadDir(ctx, accountsFolderLocation) if err != nil { return err } // TODO: use indexer to avoid spamming Metadata service for _, accID := range accounts { - assIDs, err := s.mdc.ReadDir(nil, accountPath(accID)) + assIDs, err := s.mdc.ReadDir(ctx, accountPath(accID)) if err != nil { // TODO: error? continue @@ -83,7 +87,7 @@ func (s *Store) RemoveRoleAssignment(assignmentID string) error { for _, assID := range assIDs { if assID == assignmentID { - return s.mdc.Delete(nil, assignmentPath(accID, assID)) + return s.mdc.Delete(ctx, assignmentPath(accID, assID)) } } } diff --git a/settings/pkg/store/metadata/assignments_test.go b/settings/pkg/store/metadata/assignments_test.go index 5dae88ae9..8c4369279 100644 --- a/settings/pkg/store/metadata/assignments_test.go +++ b/settings/pkg/store/metadata/assignments_test.go @@ -15,16 +15,16 @@ var ( //marie = "3c054db3-eec1-4ca4-b985-bc56dcf560cb" s = &Store{ - l: &sync.Mutex{}, + Logger: logger, + l: &sync.Mutex{}, } + logger = olog.NewLogger( olog.Color(true), olog.Pretty(true), olog.Level("info"), ) - mdc = NewMDC(s) - bundles = []*settingsmsg.Bundle{ { Id: "f36db5e6-a03c-40df-8413-711c67e40b47", @@ -87,6 +87,7 @@ var ( ) func init() { + NewMDC(s) setupRoles() } diff --git a/settings/pkg/store/metadata/bundles.go b/settings/pkg/store/metadata/bundles.go index 3c3d449db..816f0e4aa 100644 --- a/settings/pkg/store/metadata/bundles.go +++ b/settings/pkg/store/metadata/bundles.go @@ -2,6 +2,7 @@ package store import ( + "context" "encoding/json" "errors" "fmt" @@ -18,9 +19,10 @@ func (s *Store) ListBundles(bundleType settingsmsg.Bundle_Type, bundleIDs []stri return defaultBundle(bundleType, bundleIDs[0]), nil } s.Init() + ctx := context.TODO() if len(bundleIDs) == 0 { - bIDs, err := s.mdc.ReadDir(nil, bundleFolderLocation) + bIDs, err := s.mdc.ReadDir(ctx, bundleFolderLocation) if err != nil { return nil, err } @@ -29,7 +31,7 @@ func (s *Store) ListBundles(bundleType settingsmsg.Bundle_Type, bundleIDs []stri } var bundles []*settingsmsg.Bundle for _, id := range bundleIDs { - b, err := s.mdc.SimpleDownload(nil, bundlePath(id)) + b, err := s.mdc.SimpleDownload(ctx, bundlePath(id)) if err != nil { return nil, err } @@ -54,7 +56,8 @@ func (s *Store) ReadBundle(bundleID string) (*settingsmsg.Bundle, error) { return defaultBundle(settingsmsg.Bundle_TYPE_ROLE, bundleID)[0], nil } s.Init() - b, err := s.mdc.SimpleDownload(nil, bundlePath(bundleID)) + ctx := context.TODO() + b, err := s.mdc.SimpleDownload(ctx, bundlePath(bundleID)) if err != nil { return nil, err } @@ -66,8 +69,9 @@ func (s *Store) ReadBundle(bundleID string) (*settingsmsg.Bundle, error) { // ReadSetting tries to find a setting by the given id from the metadata service func (s *Store) ReadSetting(settingID string) (*settingsmsg.Setting, error) { s.Init() + ctx := context.TODO() - ids, err := s.mdc.ReadDir(nil, bundleFolderLocation) + ids, err := s.mdc.ReadDir(ctx, bundleFolderLocation) if err != nil { return nil, err } @@ -92,11 +96,13 @@ func (s *Store) ReadSetting(settingID string) (*settingsmsg.Setting, error) { // WriteBundle sends the givens record to the metadataclient. returns `record` for legacy reasons func (s *Store) WriteBundle(record *settingsmsg.Bundle) (*settingsmsg.Bundle, error) { s.Init() + ctx := context.TODO() + b, err := json.Marshal(record) if err != nil { return nil, err } - return record, s.mdc.SimpleUpload(nil, bundlePath(record.Id), b) + return record, s.mdc.SimpleUpload(ctx, bundlePath(record.Id), b) } // AddSettingToBundle adds the given setting to the bundle with the given bundleID. diff --git a/settings/pkg/store/metadata/store.go b/settings/pkg/store/metadata/store.go index 703579db2..6ba5fcab2 100644 --- a/settings/pkg/store/metadata/store.go +++ b/settings/pkg/store/metadata/store.go @@ -58,13 +58,11 @@ func (s *Store) Init() { s.l.Lock() defer s.l.Unlock() - var err error //s.init.Do(func() { - err = s.initMetadataClient(NewMetadataClient(s.cfg.Metadata)) - //}) - if err != nil { + if err := s.initMetadataClient(NewMetadataClient(s.cfg.Metadata)); err != nil { s.Logger.Error().Err(err).Msg("error initializing metadata client") } + //}) } // New creates a new store @@ -96,7 +94,8 @@ func NewMetadataClient(cfg config.Metadata) MetadataClient { // we need to lazy initialize the MetadataClient because metadata service might not be ready func (s *Store) initMetadataClient(mdc MetadataClient) error { - err := mdc.Init(nil, settingsSpaceID) + ctx := context.TODO() + err := mdc.Init(ctx, settingsSpaceID) if err != nil { return err } @@ -107,7 +106,7 @@ func (s *Store) initMetadataClient(mdc MetadataClient) error { bundleFolderLocation, valuesFolderLocation, } { - err = mdc.MakeDirIfNotExist(nil, p) + err = mdc.MakeDirIfNotExist(ctx, p) if err != nil { return err } @@ -118,7 +117,7 @@ func (s *Store) initMetadataClient(mdc MetadataClient) error { if err != nil { return err } - err = mdc.SimpleUpload(nil, bundlePath(p.Id), b) + err = mdc.SimpleUpload(ctx, bundlePath(p.Id), b) if err != nil { return err } @@ -127,7 +126,7 @@ func (s *Store) initMetadataClient(mdc MetadataClient) error { for _, p := range defaults.DefaultRoleAssignments() { accountUUID := p.AccountUuid roleID := p.RoleId - err = mdc.MakeDirIfNotExist(nil, accountPath(accountUUID)) + err = mdc.MakeDirIfNotExist(ctx, accountPath(accountUUID)) if err != nil { return err } @@ -141,7 +140,7 @@ func (s *Store) initMetadataClient(mdc MetadataClient) error { if err != nil { return err } - err = mdc.SimpleUpload(nil, assignmentPath(accountUUID, ass.Id), b) + err = mdc.SimpleUpload(ctx, assignmentPath(accountUUID, ass.Id), b) if err != nil { return err } diff --git a/settings/pkg/store/metadata/store_test.go b/settings/pkg/store/metadata/store_test.go index b13158094..8772ac296 100644 --- a/settings/pkg/store/metadata/store_test.go +++ b/settings/pkg/store/metadata/store_test.go @@ -42,7 +42,7 @@ type MockedMetadataClient struct { } // NewMDC instantiates a mocked MetadataClient -func NewMDC(s *Store) MetadataClient { +func NewMDC(s *Store) error { var mdc MetadataClient switch testtype { case "unit": @@ -50,15 +50,7 @@ func NewMDC(s *Store) MetadataClient { case "integration": mdc = NewMetadataClient(config.DefaultConfig().Metadata) } - s.initMetadataClient(mdc) - return nil -} - -func keys(m map[string][]byte) (s []string) { - for k := range m { - s = append(s, k) - } - return + return s.initMetadataClient(mdc) } // SimpleDownload returns nil if not found diff --git a/settings/pkg/store/metadata/values.go b/settings/pkg/store/metadata/values.go index 212527160..9dd73b16d 100644 --- a/settings/pkg/store/metadata/values.go +++ b/settings/pkg/store/metadata/values.go @@ -2,6 +2,7 @@ package store import ( + "context" "encoding/json" "errors" "fmt" @@ -16,8 +17,9 @@ import ( // If the accountUUID is not empty, values with an empty or with a matching accountUUID are returned. func (s *Store) ListValues(bundleID, accountUUID string) ([]*settingsmsg.Value, error) { s.Init() + ctx := context.TODO() - vIDs, err := s.mdc.ReadDir(nil, valuesFolderLocation) + vIDs, err := s.mdc.ReadDir(ctx, valuesFolderLocation) if err != nil { return nil, err } @@ -25,7 +27,7 @@ func (s *Store) ListValues(bundleID, accountUUID string) ([]*settingsmsg.Value, // TODO: refine logic not to spam metadata service var values []*settingsmsg.Value for _, vid := range vIDs { - b, err := s.mdc.SimpleDownload(nil, valuePath(vid)) + b, err := s.mdc.SimpleDownload(ctx, valuePath(vid)) if err != nil { return nil, err } @@ -56,7 +58,9 @@ func (s *Store) ListValues(bundleID, accountUUID string) ([]*settingsmsg.Value, // ReadValue tries to find a value by the given valueId within the dataPath func (s *Store) ReadValue(valueID string) (*settingsmsg.Value, error) { s.Init() - b, err := s.mdc.SimpleDownload(nil, valuePath(valueID)) + ctx := context.TODO() + + b, err := s.mdc.SimpleDownload(ctx, valuePath(valueID)) if err != nil { return nil, err } @@ -73,6 +77,8 @@ func (s *Store) ReadValueByUniqueIdentifiers(accountUUID, settingID string) (*se // WriteValue writes the given value into a file within the dataPath func (s *Store) WriteValue(value *settingsmsg.Value) (*settingsmsg.Value, error) { s.Init() + ctx := context.TODO() + if value.Id == "" { value.Id = uuid.Must(uuid.NewV4()).String() } @@ -80,7 +86,7 @@ func (s *Store) WriteValue(value *settingsmsg.Value) (*settingsmsg.Value, error) if err != nil { return nil, err } - return value, s.mdc.SimpleUpload(nil, valuePath(value.Id), b) + return value, s.mdc.SimpleUpload(ctx, valuePath(value.Id), b) } func valuePath(id string) string {