call CreateDir without splitting name from ref

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

.drone.star

@@ -103,7 +103,6 @@ def main(ctx):
     litmusOcisOldWebdav(),
     litmusOcisNewWebdav(),
     litmusOcisSpacesDav(),
-    localIntegrationTestsOcis(),
   ] + ocisIntegrationTests(6) + s3ngIntegrationTests(12)
 
 
@@ -574,67 +573,6 @@ def litmusOcisSpacesDav():
         ]
       },
     ],
-  } 
-
-def localIntegrationTestsOcis():
-  return {
-    "kind": "pipeline",
-    "type": "docker",
-    "name": "local-integration-tests-ocis",
-    "platform": {
-      "os": "linux",
-      "arch": "amd64",
-    },
-    "trigger": {
-      "event": {
-        "include": [
-          "pull_request",
-          "tag",
-        ],
-      },
-    },
-    "steps": [
-      makeStep("build-ci"),
-      {
-        "name": "revad-services",
-        "image": "registry.cern.ch/docker.io/library/golang:1.16",
-        "detach": True,
-        "commands": [
-          "cd /drone/src/tests/oc-integration-tests/drone/",
-          "/drone/src/cmd/revad/revad -c frontend.toml &",
-          "/drone/src/cmd/revad/revad -c gateway.toml &",
-          "/drone/src/cmd/revad/revad -c shares.toml &",
-          "/drone/src/cmd/revad/revad -c storage-home-ocis.toml &",
-          "/drone/src/cmd/revad/revad -c storage-oc-ocis.toml &",
-          "/drone/src/cmd/revad/revad -c storage-publiclink-ocis.toml &",
-          "/drone/src/cmd/revad/revad -c ldap-users.toml",
-        ],
-      },
-      cloneOc10TestReposStep(),
-      {
-        "name": "localAPIAcceptanceTestsOcisStorage",
-        "image": "registry.cern.ch/docker.io/owncloudci/php:7.4",
-        "commands": [
-          "make test-acceptance-api",
-        ],
-        "environment": {
-          "TEST_SERVER_URL": "http://revad-services:20080",
-          "OCIS_REVA_DATA_ROOT": "/drone/src/tmp/reva/data/",
-          "DELETE_USER_DATA_CMD": "rm -rf /drone/src/tmp/reva/data/nodes/root/* /drone/src/tmp/reva/data/nodes/*-*-*-*",
-          "STORAGE_DRIVER": "OCIS",
-          "SKELETON_DIR": "/drone/src/tmp/testing/data/apiSkeleton",
-          "TEST_WITH_LDAP": "true",
-          "REVA_LDAP_HOSTNAME": "ldap",
-          "TEST_REVA": "true",
-          "SEND_SCENARIO_LINE_REFERENCES": "true",
-          "BEHAT_FILTER_TAGS": "~@skipOnOcis-OCIS-Storage",
-          "PATH_TO_CORE": "/drone/src/tmp/testrunner",
-        }
-      },
-    ],
-    "services": [
-      ldapService(),
-    ],
   }
 
 def ocisIntegrationTests(parallelRuns, skipExceptParts = []):

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
 	}