Make owncloudsql spaces aware (#2472)

* 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.

changelog/unreleased/make-owncloudsql-spaces-aware.md

@@ -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

internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go

@@ -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)

internal/grpc/services/storageprovider/storageprovider.go

@@ -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:
@@ -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:
@@ -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:

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go

@@ -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
@@ -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 {
@@ -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{
@@ -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 {

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go

@@ -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{

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go

@@ -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)
@@ -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)

pkg/share/manager/sql/conversions.go

@@ -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"
@@ -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
@@ -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
@@ -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
 }
 
@@ -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},

pkg/share/manager/sql/sql.go

@@ -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 (
@@ -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)
@@ -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 {
@@ -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
@@ -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),
@@ -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 {
@@ -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 {
@@ -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")
  }