From 30db43b0113f165e198f0e77a9f5db16ad623093 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Wed, 28 Jul 2021 10:56:23 +0200 Subject: [PATCH] Refactor listing and statting across providers for virtual views (#1925) --- .../unreleased/virtual-views-refactor.md | 3 + .../grpc/services/gateway/storageprovider.go | 102 ++++++++++++------ pkg/storage/utils/eosfs/eosfs.go | 81 +++++++------- 3 files changed, 110 insertions(+), 76 deletions(-) create mode 100644 changelog/unreleased/virtual-views-refactor.md diff --git a/changelog/unreleased/virtual-views-refactor.md b/changelog/unreleased/virtual-views-refactor.md new file mode 100644 index 0000000000..a995698681 --- /dev/null +++ b/changelog/unreleased/virtual-views-refactor.md @@ -0,0 +1,3 @@ +Enhancement: Refactor listing and statting across providers for virtual views + +https://github.com/cs3org/reva/pull/1925 diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index d9b213bb4a..5625580875 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -23,6 +23,7 @@ import ( "fmt" "net/url" "path" + "path/filepath" "strings" "sync" "time" @@ -293,9 +294,10 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag } func (s *svc) GetHome(ctx context.Context, _ *provider.GetHomeRequest) (*provider.GetHomeResponse, error) { - home := s.getHome(ctx) - homeRes := &provider.GetHomeResponse{Path: home, Status: status.NewOK(ctx)} - return homeRes, nil + return &provider.GetHomeResponse{ + Path: s.getHome(ctx), + Status: status.NewOK(ctx), + }, nil } func (s *svc) getHome(_ context.Context) string { @@ -369,7 +371,7 @@ func (s *svc) InitiateFileDownload(ctx context.Context, req *provider.InitiateFi if protocol == "webdav" { // TODO(ishank011): pass this through the datagateway service - // for now, we just expose the file server to the user + // For now, we just expose the file server to the user ep, opaque, err := s.webdavRefTransferEndpoint(ctx, statRes.Info.Target) if err != nil { return &gateway.InitiateFileDownloadResponse{ @@ -433,7 +435,7 @@ func (s *svc) InitiateFileDownload(ctx context.Context, req *provider.InitiateFi if protocol == "webdav" { // TODO(ishank011): pass this through the datagateway service - // for now, we just expose the file server to the user + // For now, we just expose the file server to the user ep, opaque, err := s.webdavRefTransferEndpoint(ctx, statRes.Info.Target, shareChild) if err != nil { return &gateway.InitiateFileDownloadResponse{ @@ -568,7 +570,7 @@ func (s *svc) InitiateFileUpload(ctx context.Context, req *provider.InitiateFile if protocol == "webdav" { // TODO(ishank011): pass this through the datagateway service - // for now, we just expose the file server to the user + // For now, we just expose the file server to the user ep, opaque, err := s.webdavRefTransferEndpoint(ctx, statRes.Info.Target) if err != nil { return &gateway.InitiateFileUploadResponse{ @@ -630,7 +632,7 @@ func (s *svc) InitiateFileUpload(ctx context.Context, req *provider.InitiateFile if protocol == "webdav" { // TODO(ishank011): pass this through the datagateway service - // for now, we just expose the file server to the user + // For now, we just expose the file server to the user ep, opaque, err := s.webdavRefTransferEndpoint(ctx, statRes.Info.Target, shareChild) if err != nil { return &gateway.InitiateFileUploadResponse{ @@ -987,10 +989,10 @@ func (s *svc) Move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo }, nil } - dp, st2 := s.getPath(ctx, req.Destination) - if st2.Code != rpc.Code_CODE_OK && st2.Code != rpc.Code_CODE_NOT_FOUND { + dp, st := s.getPath(ctx, req.Destination) + if st.Code != rpc.Code_CODE_OK && st.Code != rpc.Code_CODE_NOT_FOUND { return &provider.MoveResponse{ - Status: st2, + Status: st, }, nil } @@ -1083,6 +1085,15 @@ func (s *svc) move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo Status: status.NewStatusFromErrType(ctx, "move dst="+req.Destination.String(), err), }, nil } + + // if providers are not the same we do not implement cross storage move yet. + if len(srcList) != 1 || len(dstList) != 1 { + res := &provider.MoveResponse{ + Status: status.NewUnimplemented(ctx, nil, "gateway: cross storage copy not yet implemented"), + } + return res, nil + } + srcP, dstP := srcList[0], dstList[0] // if providers are not the same we do not implement cross storage copy yet. @@ -1247,6 +1258,12 @@ func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.St return c.Stat(ctx, req) } + return s.statAcrossProviders(ctx, req, providers) +} + +func (s *svc) statAcrossProviders(ctx context.Context, req *provider.StatRequest, providers []*registry.ProviderInfo) (*provider.StatResponse, error) { + log := appctx.GetLogger(ctx) + infoFromProviders := make([]*provider.ResourceInfo, len(providers)) errors := make([]error, len(providers)) var wg sync.WaitGroup @@ -1260,9 +1277,8 @@ func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.St var totalSize uint64 for i := range providers { if errors[i] != nil { - return &provider.StatResponse{ - Status: status.NewStatusFromErrType(ctx, "stat ref: "+req.Ref.String(), errors[i]), - }, nil + log.Warn().Msgf("statting on provider %s returned err %+v", providers[i].ProviderPath, errors[i]) + continue } if infoFromProviders[i] != nil { totalSize += infoFromProviders[i].Size @@ -1278,7 +1294,7 @@ func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.St OpaqueId: uuid.New().String(), }, Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, - Path: resPath, + Path: req.Ref.GetPath(), Size: totalSize, }, }, nil @@ -1299,7 +1315,7 @@ func (s *svc) statOnProvider(ctx context.Context, req *provider.StatRequest, res } r, err := c.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{Path: newPath}}) if err != nil { - *e = errors.Wrap(err, "gateway: error calling ListContainer") + *e = errors.Wrap(err, fmt.Sprintf("gateway: error calling Stat %s on %+v", newPath, p)) return } if res == nil { @@ -1588,6 +1604,7 @@ func (s *svc) listSharesFolder(ctx context.Context) (*provider.ListContainerResp } func (s *svc) listContainer(ctx context.Context, req *provider.ListContainerRequest) (*provider.ListContainerResponse, error) { + log := appctx.GetLogger(ctx) providers, err := s.findProviders(ctx, req.Ref) if err != nil { return &provider.ListContainerResponse{ @@ -1595,44 +1612,47 @@ func (s *svc) listContainer(ctx context.Context, req *provider.ListContainerRequ }, nil } - resPath := path.Clean(req.Ref.GetPath()) infoFromProviders := make([][]*provider.ResourceInfo, len(providers)) errors := make([]error, len(providers)) + indirects := make([]bool, len(providers)) var wg sync.WaitGroup for i, p := range providers { wg.Add(1) - go s.listContainerOnProvider(ctx, req, &infoFromProviders[i], p, &errors[i], &wg) + go s.listContainerOnProvider(ctx, req, &infoFromProviders[i], p, &indirects[i], &errors[i], &wg) } wg.Wait() infos := []*provider.ResourceInfo{} - indirects := make(map[string][]*provider.ResourceInfo) + nestedInfos := make(map[string][]*provider.ResourceInfo) for i := range providers { if errors[i] != nil { - return &provider.ListContainerResponse{ - Status: status.NewStatusFromErrType(ctx, "listContainer ref: "+req.Ref.String(), errors[i]), - }, nil + // return if there's only one mount, else skip this one + if len(providers) == 1 { + return &provider.ListContainerResponse{ + Status: status.NewStatusFromErrType(ctx, "listContainer ref: "+req.Ref.String(), errors[i]), + }, nil + } + log.Warn().Msgf("listing container on provider %s returned err %+v", providers[i].ProviderPath, errors[i]) + continue } for _, inf := range infoFromProviders[i] { - if parent := path.Dir(inf.Path); resPath != "" && resPath != parent { - parts := strings.Split(strings.TrimPrefix(inf.Path, resPath), "/") - p := path.Join(resPath, parts[1]) - indirects[p] = append(indirects[p], inf) + if indirects[i] { + p := inf.Path + nestedInfos[p] = append(nestedInfos[p], inf) } else { infos = append(infos, inf) } } } - for k, v := range indirects { + for k := range nestedInfos { inf := &provider.ResourceInfo{ Id: &provider.ResourceId{ StorageId: "/", OpaqueId: uuid.New().String(), }, Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, - Etag: etag.GenerateEtagFromResources(nil, v), Path: k, Size: 0, } @@ -1645,7 +1665,7 @@ func (s *svc) listContainer(ctx context.Context, req *provider.ListContainerRequ }, nil } -func (s *svc) listContainerOnProvider(ctx context.Context, req *provider.ListContainerRequest, res *[]*provider.ResourceInfo, p *registry.ProviderInfo, e *error, wg *sync.WaitGroup) { +func (s *svc) listContainerOnProvider(ctx context.Context, req *provider.ListContainerRequest, res *[]*provider.ResourceInfo, p *registry.ProviderInfo, ind *bool, e *error, wg *sync.WaitGroup) { defer wg.Done() c, err := s.getStorageProviderClient(ctx, p) if err != nil { @@ -1654,11 +1674,31 @@ func (s *svc) listContainerOnProvider(ctx context.Context, req *provider.ListCon } resPath := path.Clean(req.Ref.GetPath()) - newPath := req.Ref.GetPath() if resPath != "" && !strings.HasPrefix(resPath, p.ProviderPath) { - newPath = p.ProviderPath + // The path which we're supposed to list encompasses this provider + // so just return the first child and mark it as indirect + rel, err := filepath.Rel(resPath, p.ProviderPath) + if err != nil { + *e = err + return + } + parts := strings.Split(rel, "/") + p := path.Join(resPath, parts[0]) + *ind = true + *res = []*provider.ResourceInfo{ + { + Id: &provider.ResourceId{ + StorageId: "/", + OpaqueId: uuid.New().String(), + }, + Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, + Path: p, + Size: 0, + }, + } + return } - r, err := c.ListContainer(ctx, &provider.ListContainerRequest{Ref: &provider.Reference{Path: newPath}}) + r, err := c.ListContainer(ctx, req) if err != nil { *e = errors.Wrap(err, "gateway: error calling ListContainer") return diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index be8996e8c9..c58bfc547a 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -248,6 +248,32 @@ func getUser(ctx context.Context) (*userpb.User, error) { return u, nil } +func (fs *eosfs) getLayout(ctx context.Context) (layout string) { + if fs.conf.EnableHome { + u, err := getUser(ctx) + if err != nil { + panic(err) + } + layout = templates.WithUser(u, fs.conf.UserLayout) + } + return +} + +func (fs *eosfs) getInternalHome(ctx context.Context) (string, error) { + if !fs.conf.EnableHome { + return "", errtypes.NotSupported("eos: get home not supported") + } + + u, err := getUser(ctx) + if err != nil { + err = errors.Wrap(err, "eosfs: wrap: no user in ctx and home is enabled") + return "", err + } + + relativeHome := templates.WithUser(u, fs.conf.UserLayout) + return relativeHome, nil +} + func (fs *eosfs) wrapShadow(ctx context.Context, fn string) (internal string) { if fs.conf.EnableHome { layout, err := fs.getInternalHome(ctx) @@ -291,17 +317,6 @@ func (fs *eosfs) unwrap(ctx context.Context, internal string) (string, error) { return external, nil } -func (fs *eosfs) getLayout(ctx context.Context) (layout string) { - if fs.conf.EnableHome { - u, err := getUser(ctx) - if err != nil { - panic(err) - } - layout = templates.WithUser(u, fs.conf.UserLayout) - } - return -} - func (fs *eosfs) getNsMatch(internal string, nss []string) (string, error) { var match string @@ -319,7 +334,6 @@ func (fs *eosfs) getNsMatch(internal string, nss []string) (string, error) { } func (fs *eosfs) unwrapInternal(ctx context.Context, ns, np, layout string) (string, error) { - log := appctx.GetLogger(ctx) trim := path.Join(ns, layout) if !strings.HasPrefix(np, trim) { @@ -332,12 +346,10 @@ func (fs *eosfs) unwrapInternal(ctx context.Context, ns, np, layout string) (str external = "/" } - log.Debug().Msgf("eosfs: unwrapInternal: trim=%s external=%s ns=%s np=%s", trim, external, ns, np) - return external, nil } -// resolve takes in a request path or request id and returns the unwrappedNominal path. +// resolve takes in a request path or request id and returns the unwrapped path. func (fs *eosfs) resolve(ctx context.Context, ref *provider.Reference) (string, error) { if ref.ResourceId != nil { p, err := fs.getPath(ctx, ref.ResourceId) @@ -749,7 +761,6 @@ func (fs *eosfs) getMDShareFolder(ctx context.Context, p string, mdKeys []string if err != nil { return nil, err } - // TODO(labkode): diff between root (dir) and children (ref) if fs.isShareFolderRoot(ctx, p) { return fs.convertToResourceInfo(ctx, eosFileInfo) @@ -758,8 +769,6 @@ func (fs *eosfs) getMDShareFolder(ctx context.Context, p string, mdKeys []string } func (fs *eosfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) { - log := appctx.GetLogger(ctx) - p, err := fs.resolve(ctx, ref) if err != nil { return nil, errors.Wrap(err, "eosfs: error resolving reference") @@ -767,10 +776,7 @@ func (fs *eosfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys // if path is home we need to add in the response any shadow folder in the shadow homedirectory. if fs.conf.EnableHome { - log.Debug().Msg("home enabled") - if strings.HasPrefix(p, "/") { - return fs.listWithHome(ctx, "/", p) - } + return fs.listWithHome(ctx, p) } return fs.listWithNominalHome(ctx, p) @@ -813,9 +819,9 @@ func (fs *eosfs) listWithNominalHome(ctx context.Context, p string) (finfos []*p return finfos, nil } -func (fs *eosfs) listWithHome(ctx context.Context, home, p string) ([]*provider.ResourceInfo, error) { - if p == home { - return fs.listHome(ctx, home) +func (fs *eosfs) listWithHome(ctx context.Context, p string) ([]*provider.ResourceInfo, error) { + if p == "/" { + return fs.listHome(ctx) } if fs.isShareFolderRoot(ctx, p) { @@ -830,8 +836,8 @@ func (fs *eosfs) listWithHome(ctx context.Context, home, p string) ([]*provider. return fs.listWithNominalHome(ctx, p) } -func (fs *eosfs) listHome(ctx context.Context, home string) ([]*provider.ResourceInfo, error) { - fns := []string{fs.wrap(ctx, home), fs.wrapShadow(ctx, home)} +func (fs *eosfs) listHome(ctx context.Context) ([]*provider.ResourceInfo, error) { + fns := []string{fs.wrap(ctx, "/"), fs.wrapShadow(ctx, "/")} u, err := getUser(ctx) if err != nil { @@ -928,21 +934,6 @@ func (fs *eosfs) GetQuota(ctx context.Context) (uint64, uint64, error) { return qi.AvailableBytes, qi.UsedBytes, nil } -func (fs *eosfs) getInternalHome(ctx context.Context) (string, error) { - if !fs.conf.EnableHome { - return "", errtypes.NotSupported("eosfs: get home not supported") - } - - u, err := getUser(ctx) - if err != nil { - err = errors.Wrap(err, "local: wrap: no user in ctx and home is enabled") - return "", err - } - - relativeHome := templates.WithUser(u, fs.conf.UserLayout) - return relativeHome, nil -} - func (fs *eosfs) GetHome(ctx context.Context) (string, error) { if !fs.conf.EnableHome { return "", errtypes.NotSupported("eosfs: get home not supported") @@ -1130,8 +1121,8 @@ func (fs *eosfs) CreateDir(ctx context.Context, p string) error { } func (fs *eosfs) CreateReference(ctx context.Context, p string, targetURI *url.URL) error { - // TODO(labkode): for the time being we only allow to create references - // on the virtual share folder to not pollute the nominal user tree. + // TODO(labkode): for the time being we only allow creating references + // in the virtual share folder to not pollute the nominal user tree. if !fs.isShareFolder(ctx, p) { return errtypes.PermissionDenied("eosfs: cannot create references outside the share folder: share_folder=" + fs.conf.ShareFolder + " path=" + p) } @@ -1142,7 +1133,7 @@ func (fs *eosfs) CreateReference(ctx context.Context, p string, targetURI *url.U fn := fs.wrapShadow(ctx, p) - // TODO(labkode): with grpc we can create a file touching with xattrs. + // TODO(labkode): with the grpc plugin we can create a file touching with xattrs. // Current mechanism is: touch to hidden dir, set xattr, rename. dir, base := path.Split(fn) tmp := path.Join(dir, fmt.Sprintf(".sys.reva#.%s", base))