make archiver id based (cs3org#2429)

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

changelog/unreleased/make-archiver-id-based.md

@@ -0,0 +1,5 @@
+Change: make archiver id based
+
+The archiver now uses ids to walk the tree instead of paths
+
+https://github.com/cs3org/reva/pull/2429

internal/http/services/archiver/handler.go

@@ -119,52 +119,60 @@ func (c *Config) init() {
 	c.GatewaySvc = sharedconf.GetGatewaySVC(c.GatewaySvc)
 }
 
-func (s *svc) getFiles(ctx context.Context, files, ids []string) ([]string, error) {
-	if len(files) == 0 && len(ids) == 0 {
-		return nil, errtypes.BadRequest("file and id lists are both empty")
+func (s *svc) getResources(ctx context.Context, paths, ids []string) ([]*provider.ResourceId, error) {
+	if len(paths) == 0 && len(ids) == 0 {
+		return nil, errtypes.BadRequest("path and id lists are both empty")
 	}
 
-	f := make([]string, 0, len(files)+len(ids))
+	resources := make([]*provider.ResourceId, 0, len(paths)+len(ids))
 
 	for _, id := range ids {
 		// id is base64 encoded and after decoding has the form <storage_id>:<resource_id>
 
-		ref := resourceid.OwnCloudResourceIDUnwrap(id)
-		if ref == nil {
+		decodedID := resourceid.OwnCloudResourceIDUnwrap(id)
+		if decodedID == nil {
 			return nil, errors.New("could not unwrap given file id")
 		}
 
+		resources = append(resources, decodedID)
+
+	}
+
+	for _, p := range paths {
+		// id is base64 encoded and after decoding has the form <storage_id>:<resource_id>
+
 		resp, err := s.gtwClient.Stat(ctx, &provider.StatRequest{
 			Ref: &provider.Reference{
-				ResourceId: ref,
+				Path: p,
 			},
 		})
 
 		switch {
 		case err != nil:
 			return nil, err
 		case resp.Status.Code == rpc.Code_CODE_NOT_FOUND:
-			return nil, errtypes.NotFound(id)
+			return nil, errtypes.NotFound(p)
 		case resp.Status.Code != rpc.Code_CODE_OK:
-			return nil, errtypes.InternalError(fmt.Sprintf("error getting stats from %s", id))
+			return nil, errtypes.InternalError(fmt.Sprintf("error stating %s", p))
 		}
 
-		f = append(f, resp.Info.Path)
+		resources = append(resources, resp.Info.Id)
 
 	}
 
-	f = append(f, files...)
-
 	// check if all the folders are allowed to be archived
-	err := s.allAllowed(f)
+	/* FIXME bring back filtering
+	err := s.allAllowed(resources)
 	if err != nil {
 		return nil, err
 	}
+	*/
 
-	return f, nil
+	return resources, nil
 }
 
 // return true if path match with at least with one allowed folder regex
+/*
 func (s *svc) isPathAllowed(path string) bool {
 	for _, reg := range s.allowedFolders {
 		if reg.MatchString(path) {
@@ -187,6 +195,7 @@ func (s *svc) allAllowed(paths []string) error {
 	}
 	return nil
 }
+*/
 
 func (s *svc) writeHTTPError(rw http.ResponseWriter, err error) {
 	s.log.Error().Msg(err.Error())
@@ -220,13 +229,13 @@ func (s *svc) Handler() http.Handler {
 			ids = []string{}
 		}
 
-		files, err := s.getFiles(ctx, paths, ids)
+		resources, err := s.getResources(ctx, paths, ids)
 		if err != nil {
 			s.writeHTTPError(rw, err)
 			return
 		}
 
-		arch, err := manager.NewArchiver(files, s.walker, s.downloader, manager.Config{
+		arch, err := manager.NewArchiver(resources, s.walker, s.downloader, manager.Config{
 			MaxNumFiles: s.config.MaxNumFiles,
 			MaxSize:     s.config.MaxSize,
 		})
@@ -244,7 +253,7 @@ func (s *svc) Handler() http.Handler {
 			archName += ".tar"
 		}
 
-		s.log.Debug().Msg("Requested the following files/folders to archive: " + render.Render(files))
+		s.log.Debug().Msg("Requested the following resoucres to archive: " + render.Render(resources))
 
 		rw.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=\"%s\"", archName))
 		rw.Header().Set("Content-Transfer-Encoding", "binary")

internal/http/services/archiver/manager/archiver.go

@@ -23,7 +23,6 @@ import (
 	"archive/zip"
 	"context"
 	"io"
-	"path"
 	"path/filepath"
 	"time"
 
@@ -40,88 +39,36 @@ type Config struct {
 
 // Archiver is the struct able to create an archive
 type Archiver struct {
-	files      []string
-	dir        string
+	resources  []*provider.ResourceId
 	walker     walker.Walker
 	downloader downloader.Downloader
 	config     Config
 }
 
 // NewArchiver creates a new archiver able to create an archive containing the files in the list
-func NewArchiver(files []string, w walker.Walker, d downloader.Downloader, config Config) (*Archiver, error) {
-	if len(files) == 0 {
+func NewArchiver(r []*provider.ResourceId, w walker.Walker, d downloader.Downloader, config Config) (*Archiver, error) {
+	if len(r) == 0 {
 		return nil, ErrEmptyList{}
 	}
 
-	dir := getDeepestCommonDir(files)
-	if pathIn(files, dir) {
-		dir = filepath.Dir(dir)
-	}
-
 	arc := &Archiver{
-		dir:        dir,
-		files:      files,
+		resources:  r,
 		walker:     w,
 		downloader: d,
 		config:     config,
 	}
 	return arc, nil
 }
 
-// pathIn verifies that the path `f`is in the `files`list
-func pathIn(files []string, f string) bool {
-	f = filepath.Clean(f)
-	for _, file := range files {
-		if filepath.Clean(file) == f {
-			return true
-		}
-	}
-	return false
-}
-
-func getDeepestCommonDir(files []string) string {
-
-	if len(files) == 0 {
-		return ""
-	}
-
-	// find the maximum common substring from left
-	res := path.Clean(files[0]) + "/"
-
-	for _, file := range files[1:] {
-		file = path.Clean(file) + "/"
-
-		if len(file) < len(res) {
-			res, file = file, res
-		}
-
-		for i := 0; i < len(res); i++ {
-			if res[i] != file[i] {
-				res = res[:i]
-			}
-		}
-
-	}
-
-	// the common substring could be between two / - inside a file name
-	for i := len(res) - 1; i >= 0; i-- {
-		if res[i] == '/' {
-			res = res[:i+1]
-			break
-		}
-	}
-	return filepath.Clean(res)
-}
-
 // CreateTar creates a tar and write it into the dst Writer
 func (a *Archiver) CreateTar(ctx context.Context, dst io.Writer) error {
 	w := tar.NewWriter(dst)
 
 	var filesCount, sizeFiles int64
 
-	for _, root := range a.files {
+	for _, root := range a.resources {
 
-		err := a.walker.Walk(ctx, root, func(path string, info *provider.ResourceInfo, err error) error {
+		err := a.walker.Walk(ctx, root, func(wd string, info *provider.ResourceInfo, err error) error {
 			if err != nil {
 				return err
 			}
@@ -143,15 +90,8 @@ func (a *Archiver) CreateTar(ctx context.Context, dst io.Writer) error {
 				}
 			}
 
-			// TODO (gdelmont): remove duplicates if the resources requested overlaps
-			fileName, err := filepath.Rel(a.dir, path)
-
-			if err != nil {
-				return err
-			}
-
 			header := tar.Header{
-				Name:    fileName,
+				Name:    filepath.Join(wd, info.Path),
 				ModTime: time.Unix(int64(info.Mtime.Seconds), 0),
 			}
 
@@ -171,7 +111,7 @@ func (a *Archiver) CreateTar(ctx context.Context, dst io.Writer) error {
 			}
 
 			if !isDir {
-				err = a.downloader.Download(ctx, path, w)
+				err = a.downloader.Download(ctx, info.Id, w)
 				if err != nil {
 					return err
 				}
@@ -193,9 +133,9 @@ func (a *Archiver) CreateZip(ctx context.Context, dst io.Writer) error {
 
 	var filesCount, sizeFiles int64
 
-	for _, root := range a.files {
+	for _, root := range a.resources {
 
-		err := a.walker.Walk(ctx, root, func(path string, info *provider.ResourceInfo, err error) error {
+		err := a.walker.Walk(ctx, root, func(wd string, info *provider.ResourceInfo, err error) error {
 			if err != nil {
 				return err
 			}
@@ -217,18 +157,8 @@ func (a *Archiver) CreateZip(ctx context.Context, dst io.Writer) error {
 				}
 			}
 
-			// TODO (gdelmont): remove duplicates if the resources requested overlaps
-			fileName, err := filepath.Rel(a.dir, path)
-			if err != nil {
-				return err
-			}
-
-			if fileName == "" {
-				return nil
-			}
-
 			header := zip.FileHeader{
-				Name:     fileName,
+				Name:     filepath.Join(wd, info.Path),
 				Modified: time.Unix(int64(info.Mtime.Seconds), 0),
 			}
 
@@ -244,7 +174,7 @@ func (a *Archiver) CreateZip(ctx context.Context, dst io.Writer) error {
 			}
 
 			if !isDir {
-				err = a.downloader.Download(ctx, path, dst)
+				err = a.downloader.Download(ctx, info.Id, dst)
 				if err != nil {
 					return err
 				}