call CreateDir without splitting name from ref

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

internal/grpc/services/storageprovider/storageprovider.go

@@ -500,27 +500,13 @@ func (s *service) DeleteStorageSpace(ctx context.Context, req *provider.DeleteSt
 }
 
 func (s *service) CreateContainer(ctx context.Context, req *provider.CreateContainerRequest) (*provider.CreateContainerResponse, error) {
-	var parentRef *provider.Reference
-	var name string
-	switch {
-	case utils.IsRelativeReference(req.Ref):
-		req.Ref.Path, name = path.Split(req.Ref.Path)
-		parentRef = req.Ref
-	case utils.IsAbsoluteReference(req.Ref):
-		ref, err := s.unwrap(ctx, req.Ref)
-		if err != nil {
-			return &provider.CreateContainerResponse{
-				Status: status.NewInternal(ctx, err, "error unwrapping path"),
-			}, nil
-		}
-		parentRef = &provider.Reference{Path: path.Dir(ref.GetPath())}
-		name = path.Base(ref.GetPath())
-	default:
+	newRef, err := s.unwrap(ctx, req.Ref)
+	if err != nil {
 		return &provider.CreateContainerResponse{
-			Status: status.NewInvalidArg(ctx, "invalid reference, name required"),
+			Status: status.NewInternal(ctx, err, "error unwrapping path"),
 		}, nil
 	}
-	if err := s.storage.CreateDir(ctx, parentRef, name); err != nil {
+	if err := s.storage.CreateDir(ctx, newRef); err != nil {
 		var st *rpc.Status
 		switch err.(type) {
 		case errtypes.IsNotFound:
@@ -590,13 +576,13 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide
 	sourceRef, err := s.unwrap(ctx, req.Source)
 	if err != nil {
 		return &provider.MoveResponse{
-			Status: status.NewInvalid(ctx, err.Error()),
+			Status: status.NewInternal(ctx, err, "error unwrapping source path"),
 		}, nil
 	}
 	targetRef, err := s.unwrap(ctx, req.Destination)
 	if err != nil {
 		return &provider.MoveResponse{
-			Status: status.NewInvalid(ctx, err.Error()),
+			Status: status.NewInternal(ctx, err, "error unwrapping destination path"),
 		}, nil
 	}
 
@@ -630,23 +616,15 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide
 	)
 
 	newRef, err := s.unwrap(ctx, req.Ref)
-	var st *rpc.Status
 	if err != nil {
-		switch err.(type) {
-		case errtypes.IsNotFound:
-			st = status.NewNotFound(ctx, "path not found when unwrapping")
-		case errtypes.PermissionDenied:
-			st = status.NewPermissionDenied(ctx, err, "permission denied")
-		default:
-			st = status.NewInternal(ctx, err, "error unwrapping: "+req.String())
-		}
 		return &provider.StatResponse{
-			Status: st,
+			Status: status.NewInternal(ctx, err, "error unwrapping path"),
 		}, nil
 	}
 
 	md, err := s.storage.GetMD(ctx, newRef, req.ArbitraryMetadataKeys)
 	if err != nil {
+		var st *rpc.Status
 		switch err.(type) {
 		case errtypes.IsNotFound:
 			st = status.NewNotFound(ctx, "path not found when stating")
@@ -1237,13 +1215,12 @@ func (s *service) unwrap(ctx context.Context, ref *provider.Reference) (*provide
 	// there are two cases:
 	// 1. absolute id references (resource_id is set, path is empty)
 	// 2. relative references (resource_id is set, path starts with a `.`)
-	if ref.ResourceId != nil {
+	if ref.GetResourceId() != nil {
 		return ref, nil
 	}
 
-	// if the
 	if !strings.HasPrefix(ref.GetPath(), "/") {
-		// abort, absolute path references must start with a `/``
+		// abort, absolute path references must start with a `/`
 		return nil, errtypes.BadRequest("ref is invalid: " + ref.String())
 	}
 
@@ -1254,7 +1231,9 @@ func (s *service) unwrap(ctx context.Context, ref *provider.Reference) (*provide
 		return nil, err
 	}
 
-	return &provider.Reference{Path: fsfn}, nil
+	pathRef := &provider.Reference{Path: fsfn}
+
+	return pathRef, nil
 }
 
 func (s *service) trimMountPrefix(fn string) (string, error) {

pkg/storage/fs/owncloud/owncloud.go

@@ -1156,13 +1156,12 @@ func (fs *ocfs) GetHome(ctx context.Context) (string, error) {
 	return "", nil
 }
 
-func (fs *ocfs) CreateDir(ctx context.Context, ref *provider.Reference, name string) (err error) {
+func (fs *ocfs) CreateDir(ctx context.Context, ref *provider.Reference) (err error) {
 
-	dir, err := fs.resolve(ctx, ref)
+	ip, err := fs.resolve(ctx, ref)
 	if err != nil {
 		return err
 	}
-	ip := filepath.Join(dir, name)
 
 	// check permissions of parent dir
 	if perm, err := fs.readPermissions(ctx, filepath.Dir(ip)); err == nil {
@@ -1171,17 +1170,17 @@ func (fs *ocfs) CreateDir(ctx context.Context, ref *provider.Reference, name str
 		}
 	} else {
 		if isNotFound(err) {
-			return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip)))
+			return errtypes.NotFound(ref.Path)
 		}
 		return errors.Wrap(err, "ocfs: error reading permissions")
 	}
 
 	if err = os.Mkdir(ip, 0700); err != nil {
 		if os.IsNotExist(err) {
-			return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip)))
+			return errtypes.NotFound(ref.Path)
 		}
 		// FIXME we also need already exists error, webdav expects 405 MethodNotAllowed
-		return errors.Wrap(err, "ocfs: error creating dir "+ip)
+		return errors.Wrap(err, "ocfs: error creating dir "+ref.Path)
 	}
 	return fs.propagate(ctx, ip)
 }

pkg/storage/fs/owncloudsql/owncloudsql.go

@@ -582,10 +582,6 @@ func (fs *owncloudsqlfs) resolve(ctx context.Context, ref *provider.Reference) (
 		return fs.toInternalPath(ctx, ref.GetPath()), nil
 	}
 
-	if ref.GetPath() != "" {
-		return fs.toInternalPath(ctx, ref.GetPath()), nil
-	}
-
 	// reference is invalid
 	return "", fmt.Errorf("invalid reference %+v", ref)
 }
@@ -709,8 +705,12 @@ func (fs *owncloudsqlfs) GetHome(ctx context.Context) (string, error) {
 	return "", nil
 }
 
-func (fs *owncloudsqlfs) CreateDir(ctx context.Context, ref *provider.Reference, sp string) (err error) {
-	ip := fs.toInternalPath(ctx, sp)
+func (fs *owncloudsqlfs) CreateDir(ctx context.Context, ref *provider.Reference) (err error) {
+
+	ip, err := fs.resolve(ctx, ref)
+	if err != nil {
+		return err
+	}
 
 	// check permissions of parent dir
 	if perm, err := fs.readPermissions(ctx, filepath.Dir(ip)); err == nil {
@@ -719,17 +719,17 @@ func (fs *owncloudsqlfs) CreateDir(ctx context.Context, ref *provider.Reference,
 		}
 	} else {
 		if isNotFound(err) {
-			return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip)))
+			return errtypes.NotFound(ref.Path)
 		}
 		return errors.Wrap(err, "owncloudsql: error reading permissions")
 	}
 
 	if err = os.Mkdir(ip, 0700); err != nil {
 		if os.IsNotExist(err) {
-			return errtypes.NotFound(sp)
+			return errtypes.NotFound(ref.Path)
 		}
 		// FIXME we also need already exists error, webdav expects 405 MethodNotAllowed
-		return errors.Wrap(err, "owncloudsql: error creating dir "+ip)
+		return errors.Wrap(err, "owncloudsql: error creating dir "+fs.toStoragePath(ctx, filepath.Dir(ip)))
 	}
 
 	fi, err := os.Stat(ip)

pkg/storage/fs/s3/s3.go

@@ -293,14 +293,13 @@ func (fs *s3FS) CreateHome(ctx context.Context) error {
 	return errtypes.NotSupported("s3fs: not supported")
 }
 
-func (fs *s3FS) CreateDir(ctx context.Context, ref *provider.Reference, name string) error {
+func (fs *s3FS) CreateDir(ctx context.Context, ref *provider.Reference) error {
 	log := appctx.GetLogger(ctx)
 
-	dir, err := fs.resolve(ctx, ref)
+	fn, err := fs.resolve(ctx, ref)
 	if err != nil {
 		return nil
 	}
-	fn := path.Join(dir, name)
 
 	fn = fs.addRoot(fn) + "/" // append / to indicate folder // TODO only if fn does not end in /
 
@@ -316,11 +315,11 @@ func (fs *s3FS) CreateDir(ctx context.Context, ref *provider.Reference, name str
 		log.Error().Err(err)
 		if aerr, ok := err.(awserr.Error); ok {
 			if aerr.Code() == s3.ErrCodeNoSuchBucket {
-				return errtypes.NotFound(fn)
+				return errtypes.NotFound(ref.Path)
 			}
 		}
 		// FIXME we also need already exists error, webdav expects 405 MethodNotAllowed
-		return errors.Wrap(err, "s3fs: error creating dir "+fn)
+		return errors.Wrap(err, "s3fs: error creating dir "+ref.Path)
 	}
 
 	log.Debug().Interface("result", result) // todo cache etag?

pkg/storage/storage.go

@@ -31,7 +31,7 @@ import (
 type FS interface {
 	GetHome(ctx context.Context) (string, error)
 	CreateHome(ctx context.Context) error
-	CreateDir(ctx context.Context, ref *provider.Reference, name string) error
+	CreateDir(ctx context.Context, ref *provider.Reference) error
 	Delete(ctx context.Context, ref *provider.Reference) error
 	Move(ctx context.Context, oldRef, newRef *provider.Reference) error
 	GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error)

pkg/storage/utils/decomposedfs/decomposedfs.go

@@ -27,6 +27,7 @@ import (
 	"math"
 	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -278,7 +279,12 @@ func (fs *Decomposedfs) GetPathByID(ctx context.Context, id *provider.ResourceId
 }
 
 // CreateDir creates the specified directory
-func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference, name string) (err error) {
+func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference) (err error) {
+	name := path.Base(ref.Path)
+	if name == "" || name == "." || name == "/" {
+		return errtypes.BadRequest("Invalid path")
+	}
+	ref.Path = path.Dir(ref.Path)
 	var n *node.Node
 	if n, err = fs.lu.NodeFromResource(ctx, ref); err != nil {
 		return
@@ -288,7 +294,7 @@ func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference,
 	}
 
 	if n.Exists {
-		return errtypes.AlreadyExists(name)
+		return errtypes.AlreadyExists(ref.Path)
 	}
 	pn, err := n.Parent()
 	if err != nil {

pkg/storage/utils/decomposedfs/decomposedfs_concurrency_test.go

@@ -139,9 +139,9 @@ var _ = Describe("Decomposed", func() {
 				for i := 0; i < 10; i++ {
 					go func() {
 						defer GinkgoRecover()
-						err := fs.CreateDir(ctx, &provider.Reference{Path: "."}, "fightforit")
+						err := fs.CreateDir(ctx, &provider.Reference{Path: "/fightforit"})
 						if err != nil {
-							rinfo, err := fs.GetMD(ctx, &provider.Reference{Path: "fightforit"}, nil)
+							rinfo, err := fs.GetMD(ctx, &provider.Reference{Path: "/fightforit"}, nil)
 							Expect(err).ToNot(HaveOccurred())
 							Expect(rinfo).ToNot(BeNil())
 						}

pkg/storage/utils/decomposedfs/lookup.go

@@ -40,7 +40,6 @@ type Lookup struct {
 
 // NodeFromResource takes in a request path or request id and converts it to a Node
 func (lu *Lookup) NodeFromResource(ctx context.Context, ref *provider.Reference) (*node.Node, error) {
-
 	if ref.ResourceId != nil {
 		// check if a storage space reference is used
 		// currently, the decomposed fs uses the root node id as the space id

pkg/storage/utils/decomposedfs/testhelpers/helpers.go

@@ -21,7 +21,6 @@ package helpers
 import (
 	"context"
 	"os"
-	"path"
 	"path/filepath"
 
 	"github.com/google/uuid"
@@ -114,7 +113,7 @@ func NewTestEnv() (*TestEnv, error) {
 	}
 
 	// Create dir1
-	dir1, err := env.CreateTestDir("dir1")
+	dir1, err := env.CreateTestDir("/dir1")
 	if err != nil {
 		return nil, err
 	}
@@ -126,13 +125,13 @@ func NewTestEnv() (*TestEnv, error) {
 	}
 
 	// Create subdir1 in dir1
-	err = fs.CreateDir(ctx, &providerv1beta1.Reference{Path: "dir1"}, "subdir1")
+	err = fs.CreateDir(ctx, &providerv1beta1.Reference{Path: "/dir1/subdir1"})
 	if err != nil {
 		return nil, err
 	}
 
 	// Create emptydir
-	err = fs.CreateDir(ctx, &providerv1beta1.Reference{Path: "."}, "emptydir")
+	err = fs.CreateDir(ctx, &providerv1beta1.Reference{Path: "/emptydir"})
 	if err != nil {
 		return nil, err
 	}
@@ -147,7 +146,7 @@ func (t *TestEnv) Cleanup() {
 
 // CreateTestDir create a directory and returns a corresponding Node
 func (t *TestEnv) CreateTestDir(name string) (*node.Node, error) {
-	err := t.Fs.CreateDir(t.Ctx, &providerv1beta1.Reference{Path: path.Dir(name)}, path.Base(name))
+	err := t.Fs.CreateDir(t.Ctx, &providerv1beta1.Reference{Path: name})
 	if err != nil {
 		return nil, err
 	}

pkg/storage/utils/eosfs/eosfs.go

@@ -1158,18 +1158,17 @@ func (fs *eosfs) createUserDir(ctx context.Context, u *userpb.User, path string,
 	return nil
 }
 
-func (fs *eosfs) CreateDir(ctx context.Context, ref *provider.Reference, name string) error {
+func (fs *eosfs) CreateDir(ctx context.Context, ref *provider.Reference) error {
 	log := appctx.GetLogger(ctx)
 	u, err := getUser(ctx)
 	if err != nil {
 		return errors.Wrap(err, "eosfs: no user in ctx")
 	}
-	dir, err := fs.resolve(ctx, ref)
+	p, err := fs.resolve(ctx, ref)
 	if err != nil {
 		return nil
 	}
 
-	p := path.Join(dir, name)
 	auth, err := fs.getUserAuth(ctx, u, p)
 	if err != nil {
 		return err

pkg/storage/utils/localfs/localfs.go

@@ -751,13 +751,12 @@ func (fs *localfs) createHomeInternal(ctx context.Context, fn string) error {
 	return nil
 }
 
-func (fs *localfs) CreateDir(ctx context.Context, ref *provider.Reference, name string) error {
+func (fs *localfs) CreateDir(ctx context.Context, ref *provider.Reference) error {
 
-	dir, err := fs.resolve(ctx, ref)
+	fn, err := fs.resolve(ctx, ref)
 	if err != nil {
 		return nil
 	}
-	fn := path.Join(dir, name)
 
 	if fs.isShareFolder(ctx, fn) {
 		return errtypes.PermissionDenied("localfs: cannot create folder under the share folder")