Skip to content

Commit

Permalink
revert includeTrashed hack
Browse files Browse the repository at this point in the history 
Signed-off-by: jkoberg <[email protected]>
  • Loading branch information
@kobergj
kobergj committed Feb 2, 2022
1 parent 05a45b3 commit f8a46f4
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 62 deletions.
32 changes: 11 additions & 21 deletions internal/grpc/services/gateway/storageprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,7 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp
func (s *svc) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorageSpaceRequest) (*provider.UpdateStorageSpaceResponse, error) {
// TODO: needs to be fixed
ref := &provider.Reference{ResourceId: req.StorageSpace.Root}
var restore bool
if req.Opaque != nil {
_, restore = req.Opaque.Map["restore"]
}
c, _, err := s.find(ctx, ref, restore)
c, _, err := s.find(ctx, ref)
if err != nil {
return &provider.UpdateStorageSpaceResponse{
Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find reference %+v", ref), err),
Expand Down Expand Up @@ -315,7 +311,7 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag
StorageId: storageid,
OpaqueId: opaqeid,
}}
c, _, err := s.find(ctx, ref, false)
c, _, err := s.find(ctx, ref)
if err != nil {
return &provider.DeleteStorageSpaceResponse{
Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find reference %+v", ref), err),
Expand Down Expand Up @@ -610,7 +606,7 @@ func (s *svc) CreateContainer(ctx context.Context, req *provider.CreateContainer
}

func (s *svc) TouchFile(ctx context.Context, req *provider.TouchFileRequest) (*provider.TouchFileResponse, error) {
c, _, err := s.find(ctx, req.Ref, false)
c, _, err := s.find(ctx, req.Ref)
if err != nil {
return &provider.TouchFileResponse{
Status: status.NewStatusFromErrType(ctx, "TouchFile ref="+req.Ref.String(), err),
Expand Down Expand Up @@ -748,7 +744,7 @@ func (s *svc) SetLock(ctx context.Context, req *provider.SetLockRequest) (*provi

// GetLock returns an existing lock on the given reference
func (s *svc) GetLock(ctx context.Context, req *provider.GetLockRequest) (*provider.GetLockResponse, error) {
c, _, err := s.find(ctx, req.Ref, false)
c, _, err := s.find(ctx, req.Ref)
if err != nil {
return &provider.GetLockResponse{
Status: status.NewStatusFromErrType(ctx, "GetLock ref="+req.Ref.String(), err),
Expand All @@ -768,7 +764,7 @@ func (s *svc) GetLock(ctx context.Context, req *provider.GetLockRequest) (*provi

// RefreshLock refreshes an existing lock on the given reference
func (s *svc) RefreshLock(ctx context.Context, req *provider.RefreshLockRequest) (*provider.RefreshLockResponse, error) {
c, _, err := s.find(ctx, req.Ref, false)
c, _, err := s.find(ctx, req.Ref)
if err != nil {
return &provider.RefreshLockResponse{
Status: status.NewStatusFromErrType(ctx, "RefreshLock ref="+req.Ref.String(), err),
Expand All @@ -788,7 +784,7 @@ func (s *svc) RefreshLock(ctx context.Context, req *provider.RefreshLockRequest)

// Unlock removes an existing lock from the given reference
func (s *svc) Unlock(ctx context.Context, req *provider.UnlockRequest) (*provider.UnlockResponse, error) {
c, _, err := s.find(ctx, req.Ref, false)
c, _, err := s.find(ctx, req.Ref)
if err != nil {
return &provider.UnlockResponse{
Status: status.NewStatusFromErrType(ctx, "Unlock ref="+req.Ref.String(), err),
Expand Down Expand Up @@ -981,15 +977,15 @@ func (s *svc) GetQuota(ctx context.Context, req *gateway.GetQuotaRequest) (*prov

func (s *svc) findByPath(ctx context.Context, path string) (provider.ProviderAPIClient, *registry.ProviderInfo, error) {
ref := &provider.Reference{Path: path}
return s.find(ctx, ref, false)
return s.find(ctx, ref)
}

// find looks up the provider that is responsible for the given request
// It will return a client that the caller can use to make the call, as well as the ProviderInfo. It:
// - contains the provider path, which is the mount point of the provider
// - may contain a list of storage spaces with their id and space path
func (s *svc) find(ctx context.Context, ref *provider.Reference, includeTrashed bool) (provider.ProviderAPIClient, *registry.ProviderInfo, error) {
p, err := s.findSpaces(ctx, ref, includeTrashed)
func (s *svc) find(ctx context.Context, ref *provider.Reference) (provider.ProviderAPIClient, *registry.ProviderInfo, error) {
p, err := s.findSpaces(ctx, ref)
if err != nil {
return nil, nil, err
}
Expand All @@ -1001,7 +997,7 @@ func (s *svc) find(ctx context.Context, ref *provider.Reference, includeTrashed
// FIXME findAndUnwrap currently just returns the first provider ... which may not be what is needed.
// for the ListRecycle call we need an exact match, for Stat and List we need to query all related providers
func (s *svc) findAndUnwrap(ctx context.Context, ref *provider.Reference) (provider.ProviderAPIClient, *registry.ProviderInfo, *provider.Reference, error) {
c, p, err := s.find(ctx, ref, false)
c, p, err := s.find(ctx, ref)
if err != nil {
return nil, nil, nil, err
}
Expand Down Expand Up @@ -1039,7 +1035,7 @@ func (s *svc) getStorageRegistryClient(_ context.Context, address string) (regis
return s.cache.StorageRegistryClient(c), nil
}

func (s *svc) findSpaces(ctx context.Context, ref *provider.Reference, includeTrashed bool) ([]*registry.ProviderInfo, error) {
func (s *svc) findSpaces(ctx context.Context, ref *provider.Reference) ([]*registry.ProviderInfo, error) {
switch {
case ref == nil:
return nil, errtypes.BadRequest("missing reference")
Expand All @@ -1063,12 +1059,6 @@ func (s *svc) findSpaces(ctx context.Context, ref *provider.Reference, includeTr
Opaque: &typesv1beta1.Opaque{Map: make(map[string]*typesv1beta1.OpaqueEntry)},
}

if includeTrashed {
listReq.Opaque.Map["includeTrashed"] = &typesv1beta1.OpaqueEntry{
Decoder: "plain",
Value: []byte("true"),
}
}
sdk.EncodeOpaqueMap(listReq.Opaque, filters)

return s.findProvider(ctx, listReq)
Expand Down
12 changes: 6 additions & 6 deletions internal/grpc/services/gateway/usershareprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ func (s *svc) removeReference(ctx context.Context, resourceID *provider.Resource
log := appctx.GetLogger(ctx)

idReference := &provider.Reference{ResourceId: resourceID}
storageProvider, _, err := s.find(ctx, idReference, false)
storageProvider, _, err := s.find(ctx, idReference)
if err != nil {
appctx.GetLogger(ctx).
Err(err).
Expand Down Expand Up @@ -413,7 +413,7 @@ func (s *svc) removeReference(ctx context.Context, resourceID *provider.Resource
log.Debug().Str("share_path", sharePath).Msg("remove reference of share")

sharePathRef := &provider.Reference{Path: sharePath}
homeProvider, providerInfo, err := s.find(ctx, sharePathRef, false)
homeProvider, providerInfo, err := s.find(ctx, sharePathRef)
if err != nil {
appctx.GetLogger(ctx).
Err(err).
Expand Down Expand Up @@ -477,7 +477,7 @@ func (s *svc) denyGrant(ctx context.Context, id *provider.ResourceId, g *provide
Grantee: g,
}

c, _, err := s.find(ctx, ref, false)
c, _, err := s.find(ctx, ref)
if err != nil {
appctx.GetLogger(ctx).
Err(err).
Expand Down Expand Up @@ -514,7 +514,7 @@ func (s *svc) addGrant(ctx context.Context, id *provider.ResourceId, g *provider
},
}

c, _, err := s.find(ctx, ref, false)
c, _, err := s.find(ctx, ref)
if err != nil {
appctx.GetLogger(ctx).
Err(err).
Expand Down Expand Up @@ -550,7 +550,7 @@ func (s *svc) updateGrant(ctx context.Context, id *provider.ResourceId, g *provi
},
}

c, _, err := s.find(ctx, ref, false)
c, _, err := s.find(ctx, ref)
if err != nil {
appctx.GetLogger(ctx).
Err(err).
Expand Down Expand Up @@ -587,7 +587,7 @@ func (s *svc) removeGrant(ctx context.Context, id *provider.ResourceId, g *provi
},
}

c, _, err := s.find(ctx, ref, false)
c, _, err := s.find(ctx, ref)
if err != nil {
appctx.GetLogger(ctx).
Err(err).
Expand Down
7 changes: 0 additions & 7 deletions internal/grpc/services/storageprovider/storageprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,13 +524,6 @@ func (s *service) CreateStorageSpace(ctx context.Context, req *provider.CreateSt
func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSpacesRequest) (*provider.ListStorageSpacesResponse, error) {
log := appctx.GetLogger(ctx)

var includeTrashed bool
if req.Opaque != nil {
// let's do as simple as possible until we have proper solution
_, includeTrashed = req.Opaque.Map["includeTrashed"]
ctx = context.WithValue(ctx, utils.ContextKeyIncludeTrash{}, includeTrashed)
}

spaces, err := s.storage.ListStorageSpaces(ctx, req.Filters)
if err != nil {
var st *rpc.Status
Expand Down
24 changes: 6 additions & 18 deletions pkg/storage/registry/spaces/spaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,12 +266,11 @@ func (r *registry) GetProvider(ctx context.Context, space *providerpb.StorageSpa
// below = /foo/bar/bif <=> /foo/bar -> list(spaceid, ./bif)
func (r *registry) ListProviders(ctx context.Context, filters map[string]string) ([]*registrypb.ProviderInfo, error) {
b, _ := strconv.ParseBool(filters["unique"])
it, _ := strconv.ParseBool(filters["includeTrashed"])
switch {
case filters["storage_id"] != "" && filters["opaque_id"] != "":
findMountpoint := filters["type"] == "mountpoint"
findGrant := !findMountpoint && filters["path"] == "" // relvative references, by definition, occur in the correct storage, so do not look for grants
return r.findProvidersForResource(ctx, filters["storage_id"]+"!"+filters["opaque_id"], findMountpoint, findGrant, it), nil
return r.findProvidersForResource(ctx, filters["storage_id"]+"!"+filters["opaque_id"], findMountpoint, findGrant), nil
case filters["path"] != "":
return r.findProvidersForAbsolutePathReference(ctx, filters["path"], b), nil
case len(filters) == 0:
Expand Down Expand Up @@ -326,7 +325,7 @@ func (r *registry) findProvidersForFilter(ctx context.Context, filters []*provid
p := &registrypb.ProviderInfo{
Address: address,
}
spaces, err := r.findStorageSpaceOnProvider(ctx, address, filters, false)
spaces, err := r.findStorageSpaceOnProvider(ctx, address, filters)
if err != nil {
appctx.GetLogger(ctx).Debug().Err(err).Interface("provider", provider).Msg("findStorageSpaceOnProvider by id failed, continuing")
continue
Expand Down Expand Up @@ -364,7 +363,7 @@ func (r *registry) findProvidersForFilter(ctx context.Context, filters []*provid
// findProvidersForResource looks up storage providers based on a resource id
// for the root of a space the res.StorageId is the same as the res.OpaqueId
// for share spaces the res.StorageId tells the registry the spaceid and res.OpaqueId is a node in that space
func (r *registry) findProvidersForResource(ctx context.Context, id string, findMoundpoint, findGrant bool, includeTrashed bool) []*registrypb.ProviderInfo {
func (r *registry) findProvidersForResource(ctx context.Context, id string, findMoundpoint, findGrant bool) []*registrypb.ProviderInfo {
currentUser := ctxpkg.ContextMustGetUser(ctx)
providerInfos := []*registrypb.ProviderInfo{}
for address, provider := range r.c.Providers {
Expand Down Expand Up @@ -398,7 +397,7 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string, find
},
})
}
spaces, err := r.findStorageSpaceOnProvider(ctx, address, filters, includeTrashed)
spaces, err := r.findStorageSpaceOnProvider(ctx, address, filters)
if err != nil {
appctx.GetLogger(ctx).Debug().Err(err).Interface("provider", provider).Msg("findStorageSpaceOnProvider by id failed, continuing")
continue
Expand Down Expand Up @@ -474,7 +473,7 @@ func (r *registry) findProvidersForAbsolutePathReference(ctx context.Context, pa
},
})

spaces, err = r.findStorageSpaceOnProvider(ctx, p.Address, filters, false)
spaces, err = r.findStorageSpaceOnProvider(ctx, p.Address, filters)
if err != nil {
appctx.GetLogger(ctx).Debug().Err(err).Interface("provider", provider).Msg("findStorageSpaceOnProvider failed, continuing")
continue
Expand Down Expand Up @@ -598,7 +597,7 @@ func setSpaces(providerInfo *registrypb.ProviderInfo, spaces []*providerpb.Stora
return nil
}

func (r *registry) findStorageSpaceOnProvider(ctx context.Context, addr string, filters []*providerpb.ListStorageSpacesRequest_Filter, includeTrashed bool) ([]*providerpb.StorageSpace, error) {
func (r *registry) findStorageSpaceOnProvider(ctx context.Context, addr string, filters []*providerpb.ListStorageSpacesRequest_Filter) ([]*providerpb.StorageSpace, error) {
c, err := r.getStorageProviderServiceClient(addr)
if err != nil {
return nil, err
Expand All @@ -607,17 +606,6 @@ func (r *registry) findStorageSpaceOnProvider(ctx context.Context, addr string,
Filters: filters,
}

if includeTrashed {
req.Opaque = &typesv1beta1.Opaque{
Map: map[string]*typesv1beta1.OpaqueEntry{
"includeTrashed": {
Decoder: "plain",
Value: []byte("true"),
},
},
}
}

res, err := c.ListStorageSpaces(ctx, req)
if err != nil {
// ignore errors
Expand Down
14 changes: 4 additions & 10 deletions pkg/storage/utils/decomposedfs/spaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,6 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
nodeID = spaceIDAny
)

// NOTE: this is not optimal. We should either adjust Method signature or add a new filter
includeTrashed, _ := ctx.Value(utils.ContextKeyIncludeTrash{}).(bool)

spaceTypes := []string{}

for i := range filter {
Expand Down Expand Up @@ -221,9 +218,6 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
matches := []string{}
for _, spaceType := range spaceTypes {
path := filepath.Join(fs.o.Root, "spaces", spaceType, nodeID)
if includeTrashed {
path += "*"
}
m, err := filepath.Glob(path)
if err != nil {
return nil, err
Expand Down Expand Up @@ -291,7 +285,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
}

// TODO apply more filters
space, err := fs.storageSpaceFromNode(ctx, n, spaceType, matches[i], canListAllSpaces, includeTrashed)
space, err := fs.storageSpaceFromNode(ctx, n, spaceType, matches[i], canListAllSpaces)
if err != nil {
if _, ok := err.(errtypes.IsPermissionDenied); !ok {
appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not convert to storage space")
Expand All @@ -310,7 +304,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
return nil, err
}
if n.Exists {
space, err := fs.storageSpaceFromNode(ctx, n, "*", n.InternalPath(), canListAllSpaces, includeTrashed)
space, err := fs.storageSpaceFromNode(ctx, n, "*", n.InternalPath(), canListAllSpaces)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -542,7 +536,7 @@ func (fs *Decomposedfs) createStorageSpace(ctx context.Context, spaceType, space
return nil
}

func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, spaceType, nodePath string, canListAllSpaces bool, includeTrashed bool) (*provider.StorageSpace, error) {
func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, spaceType, nodePath string, canListAllSpaces bool) (*provider.StorageSpace, error) {
owner, err := n.Owner()
if err != nil {
return nil, err
Expand Down Expand Up @@ -599,7 +593,7 @@ func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node,
return nil, errtypes.PermissionDenied(fmt.Sprintf("user %s is not allowed to Stat the space %+v", user.Username, space))
}

if !includeTrashed && strings.Contains(n.Name, node.TrashIDDelimiter) {
if strings.Contains(n.Name, node.TrashIDDelimiter) {
return nil, errtypes.PermissionDenied(fmt.Sprintf("user %s is not allowed to list deleted spaces %+v", user.Username, space))
}
}
Expand Down

0 comments on commit f8a46f4

Please sign in to comment.