Skip to content

Commit

Permalink
Make owncloudsql spaces aware (#2472)
Browse files Browse the repository at this point in the history
* Add ListStorages method

* Implement ListStorageSpaces in owncloudsql

* ResourceInfos do no longer contain the full path but only the basename

* Handle references relative to a root

* Fix space lookup, extract space functionality into a separate file

* Use oc_mounts to find the storage roots

This way it also works with storages with hashed IDs (that happens when
the id exceeds a certain length).

* Fix shares

* Implement GetPath, fix GetPathById

* Include the storage id when listing shares

* Fix accepting declined shares

* Ignore setting-grants-not-supported errors, storage grants are optional

* Add changelog

* Fix missing storage id from resource info

* Fix field mask

* Do not log error messages for unsupported grant calls

* Fix hound issue

* Fix changelog URL

* Fix linter issue

* Remove unfinished GetPath() code

* Adapt expected failures

* Cache user lookups in the oc10-sql share manager

That leads to a massive performance boost.
  • Loading branch information
aduffeck authored Jan 28, 2022
1 parent 52630ab commit 376754b
Show file tree
Hide file tree
Showing 17 changed files with 497 additions and 83 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/make-owncloudsql-spaces-aware.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Bugfix: make owncloudsql work with the spaces registry

owncloudsql now works properly with the spaces registry.

https://github.com/cs3org/reva/pull/2372

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (

// GatewayClient describe the interface of a gateway client
type GatewayClient interface {
GetPath(ctx context.Context, in *provider.GetPathRequest, opts ...grpc.CallOption) (*provider.GetPathResponse, error)
Stat(ctx context.Context, in *provider.StatRequest, opts ...grpc.CallOption) (*provider.StatResponse, error)
Move(ctx context.Context, in *provider.MoveRequest, opts ...grpc.CallOption) (*provider.MoveResponse, error)
Delete(ctx context.Context, in *provider.DeleteRequest, opts ...grpc.CallOption) (*provider.DeleteResponse, error)
Expand Down
15 changes: 15 additions & 0 deletions internal/grpc/services/storageprovider/storageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,11 @@ func (s *service) DenyGrant(ctx context.Context, req *provider.DenyGrantRequest)
if err != nil {
var st *rpc.Status
switch err.(type) {
case errtypes.NotSupported:
// ignore - setting storage grants is optional
return &provider.DenyGrantResponse{
Status: status.NewOK(ctx),
}, nil
case errtypes.IsNotFound:
st = status.NewNotFound(ctx, "path not found when setting grants")
case errtypes.PermissionDenied:
Expand Down Expand Up @@ -1135,6 +1140,11 @@ func (s *service) AddGrant(ctx context.Context, req *provider.AddGrantRequest) (
if err != nil {
var st *rpc.Status
switch err.(type) {
case errtypes.NotSupported:
// ignore - setting storage grants is optional
return &provider.AddGrantResponse{
Status: status.NewOK(ctx),
}, nil
case errtypes.IsNotFound:
st = status.NewNotFound(ctx, "path not found when setting grants")
case errtypes.PermissionDenied:
Expand Down Expand Up @@ -1170,6 +1180,11 @@ func (s *service) UpdateGrant(ctx context.Context, req *provider.UpdateGrantRequ
if err := s.storage.UpdateGrant(ctx, req.Ref, req.Grant); err != nil {
var st *rpc.Status
switch err.(type) {
case errtypes.NotSupported:
// ignore - setting storage grants is optional
return &provider.UpdateGrantResponse{
Status: status.NewOK(ctx),
}, nil
case errtypes.IsNotFound:
st = status.NewNotFound(ctx, "path not found when updating grant")
case errtypes.PermissionDenied:
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) {
return
}

share, ocsResponse := getShareFromID(ctx, client, shareID)
rs, ocsResponse := getReceivedShareFromID(ctx, client, shareID)
if ocsResponse != nil {
response.WriteOCSResponse(w, r, *ocsResponse, nil)
return
}

sharedResource, ocsResponse := getSharedResource(ctx, client, share)
sharedResource, ocsResponse := getSharedResource(ctx, client, rs.Share.Share.ResourceId)
if ocsResponse != nil {
response.WriteOCSResponse(w, r, *ocsResponse, nil)
return
Expand All @@ -76,7 +76,7 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) {
var mountPoints []string
sharesToAccept := map[string]bool{shareID: true}
for _, s := range lrs.Shares {
if utils.ResourceIDEqual(s.Share.ResourceId, share.Share.GetResourceId()) {
if utils.ResourceIDEqual(s.Share.ResourceId, rs.Share.Share.GetResourceId()) {
if s.State == collaboration.ShareState_SHARE_STATE_ACCEPTED {
mount = s.MountPoint.Path
} else {
Expand Down Expand Up @@ -183,9 +183,9 @@ func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, sh
response.WriteOCSSuccess(w, r, []*conversions.ShareData{data})
}

// getShareFromID uses a client to the gateway to fetch a share based on its ID.
func getShareFromID(ctx context.Context, client GatewayClient, shareID string) (*collaboration.GetShareResponse, *response.Response) {
s, err := client.GetShare(ctx, &collaboration.GetShareRequest{
// getReceivedShareFromID uses a client to the gateway to fetch a share based on its ID.
func getReceivedShareFromID(ctx context.Context, client GatewayClient, shareID string) (*collaboration.GetReceivedShareResponse, *response.Response) {
s, err := client.GetReceivedShare(ctx, &collaboration.GetReceivedShareRequest{
Ref: &collaboration.ShareReference{
Spec: &collaboration.ShareReference_Id{
Id: &collaboration.ShareId{
Expand Down Expand Up @@ -213,10 +213,10 @@ func getShareFromID(ctx context.Context, client GatewayClient, shareID string) (
}

// getSharedResource attempts to get a shared resource from the storage from the resource reference.
func getSharedResource(ctx context.Context, client GatewayClient, share *collaboration.GetShareResponse) (*provider.StatResponse, *response.Response) {
func getSharedResource(ctx context.Context, client GatewayClient, resID *provider.ResourceId) (*provider.StatResponse, *response.Response) {
res, err := client.Stat(ctx, &provider.StatRequest{
Ref: &provider.Reference{
ResourceId: share.Share.GetResourceId(),
ResourceId: resID,
},
})
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,11 @@ var _ = Describe("The ocs API", func() {
)

BeforeEach(func() {
client.On("GetShare", mock.Anything, mock.Anything).Return(&collaboration.GetShareResponse{
client.On("GetReceivedShare", mock.Anything, mock.Anything).Return(&collaboration.GetReceivedShareResponse{
Status: status.NewOK(context.Background()),
Share: share,
Share: &collaboration.ReceivedShare{
Share: share,
},
}, nil)

client.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ type GatewayClient interface {

ListShares(ctx context.Context, in *collaboration.ListSharesRequest, opts ...grpc.CallOption) (*collaboration.ListSharesResponse, error)
GetShare(ctx context.Context, in *collaboration.GetShareRequest, opts ...grpc.CallOption) (*collaboration.GetShareResponse, error)
GetReceivedShare(ctx context.Context, in *collaboration.GetReceivedShareRequest, opts ...grpc.CallOption) (*collaboration.GetReceivedShareResponse, error)
CreateShare(ctx context.Context, in *collaboration.CreateShareRequest, opts ...grpc.CallOption) (*collaboration.CreateShareResponse, error)
RemoveShare(ctx context.Context, in *collaboration.RemoveShareRequest, opts ...grpc.CallOption) (*collaboration.RemoveShareResponse, error)
ListReceivedShares(ctx context.Context, in *collaboration.ListReceivedSharesRequest, opts ...grpc.CallOption) (*collaboration.ListReceivedSharesResponse, error)
Expand Down Expand Up @@ -329,7 +330,7 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) {
MountPoint: s.MountPoint,
State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
},
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"state, mount_point"}},
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"state", "mount_point"}},
}

shareRes, err := client.UpdateReceivedShare(granteeCtx, updateRequest)
Expand Down
30 changes: 28 additions & 2 deletions pkg/share/manager/sql/conversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ package sql
import (
"context"
"strings"
"time"

"github.com/ReneKroon/ttlcache/v2"
grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
userprovider "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
Expand Down Expand Up @@ -65,17 +67,34 @@ type UserConverter interface {
// GatewayUserConverter converts usernames and ids using the gateway
type GatewayUserConverter struct {
gwAddr string

IDCache *ttlcache.Cache
NameCache *ttlcache.Cache
}

// NewGatewayUserConverter returns a instance of GatewayUserConverter
func NewGatewayUserConverter(gwAddr string) *GatewayUserConverter {
IDCache := ttlcache.NewCache()
_ = IDCache.SetTTL(30 * time.Second)
IDCache.SkipTTLExtensionOnHit(true)
NameCache := ttlcache.NewCache()
_ = NameCache.SetTTL(30 * time.Second)
NameCache.SkipTTLExtensionOnHit(true)

return &GatewayUserConverter{
gwAddr: gwAddr,
gwAddr: gwAddr,
IDCache: IDCache,
NameCache: NameCache,
}
}

// UserIDToUserName converts a user ID to an username
func (c *GatewayUserConverter) UserIDToUserName(ctx context.Context, userid *userpb.UserId) (string, error) {
username, err := c.NameCache.Get(userid.String())
if err == nil {
return username.(string), nil
}

gwConn, err := pool.GetGatewayServiceClient(c.gwAddr)
if err != nil {
return "", err
Expand All @@ -89,11 +108,17 @@ func (c *GatewayUserConverter) UserIDToUserName(ctx context.Context, userid *use
if getUserResponse.Status.Code != rpc.Code_CODE_OK {
return "", status.NewErrorFromCode(getUserResponse.Status.Code, "gateway")
}
_ = c.NameCache.Set(userid.String(), getUserResponse.User.Username)
return getUserResponse.User.Username, nil
}

// UserNameToUserID converts a username to an user ID
func (c *GatewayUserConverter) UserNameToUserID(ctx context.Context, username string) (*userpb.UserId, error) {
id, err := c.IDCache.Get(username)
if err == nil {
return id.(*userpb.UserId), nil
}

gwConn, err := pool.GetGatewayServiceClient(c.gwAddr)
if err != nil {
return nil, err
Expand All @@ -108,6 +133,7 @@ func (c *GatewayUserConverter) UserNameToUserID(ctx context.Context, username st
if getUserResponse.Status.Code != rpc.Code_CODE_OK {
return nil, status.NewErrorFromCode(getUserResponse.Status.Code, "gateway")
}
_ = c.IDCache.Set(username, getUserResponse.User.Id)
return getUserResponse.User.Id, nil
}

Expand Down Expand Up @@ -233,7 +259,7 @@ func (m *mgr) convertToCS3Share(ctx context.Context, s DBShare, storageMountID s
OpaqueId: s.ID,
},
ResourceId: &provider.ResourceId{
StorageId: storageMountID + "!" + s.ItemStorage,
StorageId: s.ItemStorage,
OpaqueId: s.FileSource,
},
Permissions: &collaboration.SharePermissions{Permissions: permissions},
Expand Down
24 changes: 16 additions & 8 deletions pkg/share/manager/sql/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,15 @@ func (m *mgr) UpdateShare(ctx context.Context, ref *collaboration.ShareReference

func (m *mgr) ListShares(ctx context.Context, filters []*collaboration.Filter) ([]*collaboration.Share, error) {
uid := ctxpkg.ContextMustGetUser(ctx).Username
query := "select coalesce(uid_owner, '') as uid_owner, coalesce(uid_initiator, '') as uid_initiator, coalesce(share_with, '') as share_with, coalesce(file_source, '') as file_source, file_target, id, stime, permissions, share_type FROM oc_share WHERE (uid_owner=? or uid_initiator=?)"
query := `
SELECT
coalesce(s.uid_owner, '') as uid_owner, coalesce(s.uid_initiator, '') as uid_initiator,
coalesce(s.share_with, '') as share_with, coalesce(s.file_source, '') as file_source,
s.file_target, s.id, s.stime, s.permissions, s.share_type, fc.storage as storage
FROM oc_share s
LEFT JOIN oc_filecache fc ON fc.fileid = file_source
WHERE (uid_owner=? or uid_initiator=?)
`
params := []interface{}{uid, uid}

var (
Expand Down Expand Up @@ -310,7 +318,7 @@ func (m *mgr) ListShares(ctx context.Context, filters []*collaboration.Filter) (
var s DBShare
shares := []*collaboration.Share{}
for rows.Next() {
if err := rows.Scan(&s.UIDOwner, &s.UIDInitiator, &s.ShareWith, &s.FileSource, &s.FileTarget, &s.ID, &s.STime, &s.Permissions, &s.ShareType); err != nil {
if err := rows.Scan(&s.UIDOwner, &s.UIDInitiator, &s.ShareWith, &s.FileSource, &s.FileTarget, &s.ID, &s.STime, &s.Permissions, &s.ShareType, &s.ItemStorage); err != nil {
continue
}
share, err := m.convertToCS3Share(ctx, s, m.storageMountID)
Expand Down Expand Up @@ -357,7 +365,7 @@ func (m *mgr) ListReceivedShares(ctx context.Context, filters []*collaboration.F
)
SELECT COALESCE(r.uid_owner, '') AS uid_owner, COALESCE(r.uid_initiator, '') AS uid_initiator, COALESCE(r.share_with, '')
AS share_with, COALESCE(r.file_source, '') AS file_source, COALESCE(r2.file_target, r.file_target), r.id, r.stime, r.permissions, r.share_type, COALESCE(r2.accepted, r.accepted),
r.numeric_id, COALESCE(r.parent, -1) AS parent FROM results r LEFT JOIN results r2 ON r.id = r2.parent WHERE r.parent IS NULL;`
r.numeric_id, COALESCE(r.parent, -1) AS parent FROM results r LEFT JOIN results r2 ON r.id = r2.parent WHERE r.parent IS NULL`

filterQuery, filterParams, err := translateFilters(filters)
if err != nil {
Expand All @@ -368,7 +376,7 @@ func (m *mgr) ListReceivedShares(ctx context.Context, filters []*collaboration.F
if filterQuery != "" {
query = fmt.Sprintf("%s AND (%s)", query, filterQuery)
}

query += ";"
rows, err := m.db.Query(query, params...)
if err != nil {
return nil, err
Expand Down Expand Up @@ -543,7 +551,7 @@ func (m *mgr) getReceivedByID(ctx context.Context, id *collaboration.ShareId) (*
SELECT s.*, storages.numeric_id
FROM oc_share s
LEFT JOIN oc_storages storages ON ` + homeConcat + `
WHERE s.id=? OR s.parent=?` + userSelect + `
WHERE s.id=? OR s.parent=? ` + userSelect + `
)
SELECT COALESCE(r.uid_owner, '') AS uid_owner, COALESCE(r.uid_initiator, '') AS uid_initiator, COALESCE(r.share_with, '')
AS share_with, COALESCE(r.file_source, '') AS file_source, COALESCE(r2.file_target, r.file_target), r.id, r.stime, r.permissions, r.share_type, COALESCE(r2.accepted, r.accepted),
Expand Down Expand Up @@ -621,7 +629,7 @@ func translateFilters(filters []*collaboration.Filter) (string, []interface{}, e
case collaboration.Filter_TYPE_RESOURCE_ID:
filterQuery += "("
for i, f := range filters {
filterQuery += "item_source=?"
filterQuery += "file_source=?"
params = append(params, f.GetResourceId().OpaqueId)

if i != len(filters)-1 {
Expand All @@ -632,7 +640,7 @@ func translateFilters(filters []*collaboration.Filter) (string, []interface{}, e
case collaboration.Filter_TYPE_GRANTEE_TYPE:
filterQuery += "("
for i, f := range filters {
filterQuery += "share_type=?"
filterQuery += "r.share_type=?"
params = append(params, granteeTypeToShareType(f.GetGranteeType()))

if i != len(filters)-1 {
Expand All @@ -642,7 +650,7 @@ func translateFilters(filters []*collaboration.Filter) (string, []interface{}, e
filterQuery += ")"
case collaboration.Filter_TYPE_EXCLUDE_DENIALS:
// TODO this may change once the mapping of permission to share types is completed (cf. pkg/cbox/utils/conversions.go)
filterQuery += "permissions > 0"
filterQuery += "r.permissions > 0"
default:
return "", nil, fmt.Errorf("filter type is not supported")
}
Expand Down
Loading

0 comments on commit 376754b

Please sign in to comment.