From 60efd6371b5c60da375088a5dbba54ba828e459b Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Tue, 29 Sep 2020 15:06:32 +0200 Subject: [PATCH] Fix DeleteGroup implementation and staticchecks --- accounts/pkg/config/config.go | 3 ++ accounts/pkg/service/v0/groups.go | 4 --- accounts/pkg/storage/cs3.go | 56 ++++++++++++------------------- accounts/pkg/storage/disk.go | 13 ++++--- accounts/pkg/storage/repo.go | 1 + 5 files changed, 34 insertions(+), 43 deletions(-) diff --git a/accounts/pkg/config/config.go b/accounts/pkg/config/config.go index 723703045..956c12cea 100644 --- a/accounts/pkg/config/config.go +++ b/accounts/pkg/config/config.go @@ -61,15 +61,18 @@ type Log struct { Color bool } +// Repo defines which storage implementation is to be used. type Repo struct { Disk Disk CS3 CS3 } +// Disk is the local disk implementation of the storage. type Disk struct { Path string } +// CS3 is the cs3 implementation of the storage. type CS3 struct { ProviderAddr string DriverURL string diff --git a/accounts/pkg/service/v0/groups.go b/accounts/pkg/service/v0/groups.go index c7a102d01..94660c8fc 100644 --- a/accounts/pkg/service/v0/groups.go +++ b/accounts/pkg/service/v0/groups.go @@ -4,7 +4,6 @@ import ( "context" "os" "path/filepath" - "sync" "github.com/CiscoM31/godata" "github.com/blevesearch/bleve" @@ -15,9 +14,6 @@ import ( "github.com/owncloud/ocis/accounts/pkg/provider" ) -// accLock mutually exclude readers from writers on group files -var groupLock sync.Mutex - func (s Service) indexGroups(path string) (err error) { var f *os.File if f, err = os.Open(path); err != nil { diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index 27ade503b..0545813c7 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -22,6 +22,7 @@ import ( "google.golang.org/grpc/metadata" ) +// CS3Repo provides a cs3 implementation of the Repo interface type CS3Repo struct { serviceID string cfg *config.Config @@ -29,6 +30,7 @@ type CS3Repo struct { storageClient provider.ProviderAPIClient } +// NewCS3Repo creates a new cs3 repo func NewCS3Repo(serviceID string, cfg *config.Config) (Repo, error) { tokenManager, err := jwt.New(map[string]interface{}{ "secret": cfg.TokenManager.JWTSecret, @@ -51,6 +53,7 @@ func NewCS3Repo(serviceID string, cfg *config.Config) (Repo, error) { }, nil } +// WriteAccount writes an account via cs3 and modifies the provided account (e.g. with a generated id). func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) { t, err := r.authenticate(ctx) if err != nil { @@ -67,7 +70,7 @@ func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) } - ureq, err := http.NewRequest("PUT", r.accountUrl(a.Id), bytes.NewReader(by)) + ureq, err := http.NewRequest("PUT", r.accountURL(a.Id), bytes.NewReader(by)) if err != nil { return err } @@ -84,15 +87,14 @@ func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) return nil } +// LoadAccount loads an account via cs3 by id and writes it to the provided account func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) { t, err := r.authenticate(ctx) if err != nil { return err } - ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - - ureq, err := http.NewRequest("GET", r.accountUrl(id), nil) + ureq, err := http.NewRequest("GET", r.accountURL(id), nil) if err != nil { return err } @@ -120,6 +122,7 @@ func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) ( return nil } +// DeleteAccount deletes an account via cs3 by id func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) { t, err := r.authenticate(ctx) if err != nil { @@ -130,17 +133,13 @@ func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) { _, err = r.storageClient.Delete(ctx, &provider.DeleteRequest{ Ref: &provider.Reference{ - Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/accounts/%s", id)}, + Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", accountsFolder, id)}, }, }) - - if err != nil { - return err - } - - return nil + return err } +// WriteGroup writes a group via cs3 and modifies the provided group (e.g. with a generated id). func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { t, err := r.authenticate(ctx) if err != nil { @@ -157,7 +156,7 @@ func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) } - ureq, err := http.NewRequest("PUT", r.groupUrl(g.Id), bytes.NewReader(by)) + ureq, err := http.NewRequest("PUT", r.groupURL(g.Id), bytes.NewReader(by)) if err != nil { return err } @@ -174,15 +173,14 @@ func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { return nil } +// LoadGroup loads a group via cs3 by id and writes it to the provided group func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err error) { t, err := r.authenticate(ctx) if err != nil { return err } - ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - - ureq, err := http.NewRequest("GET", r.groupUrl(id), nil) + ureq, err := http.NewRequest("GET", r.groupURL(id), nil) if err != nil { return err } @@ -210,6 +208,7 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return nil } +// DeleteGroup deletes a group via cs3 by id func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { t, err := r.authenticate(ctx) if err != nil { @@ -217,24 +216,13 @@ func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { } ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - ureq, err := http.NewRequest("DELETE", r.groupUrl(id), nil) - if err != nil { - return err - } - ureq.Header.Add("x-access-token", t) - cl := http.Client{ - Transport: http.DefaultTransport, - } - - resp, err := cl.Do(ureq) - if err != nil { - return err - } - - defer resp.Body.Close() - - return nil + _, err = r.storageClient.Delete(ctx, &provider.DeleteRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", groupsFolder, id)}, + }, + }) + return err } func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { @@ -244,11 +232,11 @@ func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { }) } -func (r CS3Repo) accountUrl(id string) string { +func (r CS3Repo) accountURL(id string) string { return singleJoiningSlash(r.cfg.Repo.CS3.DriverURL, path.Join(r.cfg.Repo.CS3.DataPrefix, accountsFolder, id)) } -func (r CS3Repo) groupUrl(id string) string { +func (r CS3Repo) groupURL(id string) string { return singleJoiningSlash(r.cfg.Repo.CS3.DriverURL, path.Join(r.cfg.Repo.CS3.DataPrefix, groupsFolder, id)) } diff --git a/accounts/pkg/storage/disk.go b/accounts/pkg/storage/disk.go index dfb0a5627..1c7f21e20 100644 --- a/accounts/pkg/storage/disk.go +++ b/accounts/pkg/storage/disk.go @@ -16,12 +16,14 @@ import ( var groupLock sync.Mutex +// DiskRepo provides a local filesystem implementation of the Repo interface type DiskRepo struct { serviceID string cfg *config.Config log olog.Logger } +// NewDiskRepo creates a new disk repo func NewDiskRepo(serviceID string, cfg *config.Config, log olog.Logger) DiskRepo { return DiskRepo{ serviceID: serviceID, @@ -30,7 +32,7 @@ func NewDiskRepo(serviceID string, cfg *config.Config, log olog.Logger) DiskRepo } } -// WriteAccount to the storage +// WriteAccount to the local filesystem func (r DiskRepo) WriteAccount(ctx context.Context, a *proto.Account) (err error) { // leave only the group id r.deflateMemberOf(a) @@ -48,7 +50,7 @@ func (r DiskRepo) WriteAccount(ctx context.Context, a *proto.Account) (err error return } -// LoadAccount from the storage +// LoadAccount from the local filesystem func (r DiskRepo) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, id) @@ -63,7 +65,7 @@ func (r DiskRepo) LoadAccount(ctx context.Context, id string, a *proto.Account) return } -// DeleteAccount from the storage +// DeleteAccount from the local filesystem func (r DiskRepo) DeleteAccount(ctx context.Context, id string) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, id) if err = os.Remove(path); err != nil { @@ -74,7 +76,7 @@ func (r DiskRepo) DeleteAccount(ctx context.Context, id string) (err error) { return nil } -// WriteGroup persists a given group to the storage +// WriteGroup to the local filesystem func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { // leave only the member id r.deflateMembers(g) @@ -94,7 +96,7 @@ func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { return } -// LoadGroup from the storage +// LoadGroup from the local filesystem func (r DiskRepo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, id) @@ -112,6 +114,7 @@ func (r DiskRepo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return } +// DeleteGroup from the local filesystem func (r DiskRepo) DeleteGroup(ctx context.Context, id string) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, id) if err = os.Remove(path); err != nil { diff --git a/accounts/pkg/storage/repo.go b/accounts/pkg/storage/repo.go index 12f9327ee..27f278876 100644 --- a/accounts/pkg/storage/repo.go +++ b/accounts/pkg/storage/repo.go @@ -10,6 +10,7 @@ const ( groupsFolder = "groups" ) +// Repo defines the storage operations type Repo interface { WriteAccount(ctx context.Context, a *proto.Account) (err error) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error)