Merge branch 'origin/stable-2.0' into 'next-release/stable-2.0'

go.mod

@@ -63,7 +63,7 @@ require (
 	github.com/onsi/ginkgo/v2 v2.23.3
 	github.com/onsi/gomega v1.36.3
 	github.com/open-policy-agent/opa v1.2.0
-	github.com/opencloud-eu/reva/v2 v2.29.4
+	github.com/opencloud-eu/reva/v2 v2.29.5
 	github.com/orcaman/concurrent-map v1.0.0
 	github.com/owncloud/libre-graph-api-go v1.0.5-0.20240829135935-80dc00d6f5ea
 	github.com/pkg/errors v0.9.1

go.sum

@@ -865,8 +865,8 @@ github.com/onsi/gomega v1.36.3 h1:hID7cr8t3Wp26+cYnfcjR6HpJ00fdogN6dqZ1t6IylU=
 github.com/onsi/gomega v1.36.3/go.mod h1:8D9+Txp43QWKhM24yyOBEdpkzN8FvJyAwecBgsU4KU0=
 github.com/open-policy-agent/opa v1.2.0 h1:88NDVCM0of1eO6Z4AFeL3utTEtMuwloFmWWU7dRV1z0=
 github.com/open-policy-agent/opa v1.2.0/go.mod h1:30euUmOvuBoebRCcJ7DMF42bRBOPznvt0ACUMYDUGVY=
-github.com/opencloud-eu/reva/v2 v2.29.4 h1:UaykCqG3FNEpaeZzixsJBGi+j/Ihl3qASQ1WgcYTDb0=
-github.com/opencloud-eu/reva/v2 v2.29.4/go.mod h1:+nkCU7w6E6cyNSsKRYj1rb0cCI7QswEQ7KOPljctebM=
+github.com/opencloud-eu/reva/v2 v2.29.5 h1:T4RjTSDk650PVn0hAL8HpF+61ChqQ/UwNoWMYYAMOGU=
+github.com/opencloud-eu/reva/v2 v2.29.5/go.mod h1:+nkCU7w6E6cyNSsKRYj1rb0cCI7QswEQ7KOPljctebM=
 github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
 github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs=
 github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=

tests/acceptance/features/apiContract/propfind.feature

@@ -120,3 +120,26 @@ Feature: Propfind test
       | Manager      | RDNVWZP       |
       | Space Editor | DNVW          |
       | Space Viewer |               |
+
+  @issue-1523
+  Scenario: propfind response contains a restored folder with correct name
+    Given user "Alice" has created a folder "folderMain" in space "Personal"
+    And user "Alice" has deleted folder "folderMain"
+    And user "Alice" has created a folder "folderMain" in space "Personal"
+    When user "Alice" restores the folder with original path "/folderMain" to "/folderMain (1)" using the trashbin API
+    And user "Alice" sends PROPFIND request to space "Personal" using the WebDAV API
+    Then the HTTP status code should be "207"
+    And as user "Alice" the PROPFIND response should contain a resource "folderMain" with these key and value pairs:
+      | key            | value             |
+      | oc:fileid      | %file_id_pattern% |
+      | oc:file-parent | %file_id_pattern% |
+      | oc:name        | folderMain        |
+      | oc:permissions | RDNVCKZP          |
+      | oc:size        | 0                 |
+    And as user "Alice" the PROPFIND response should contain a resource "folderMain (1)" with these key and value pairs:
+      | key            | value             |
+      | oc:fileid      | %file_id_pattern% |
+      | oc:file-parent | %file_id_pattern% |
+      | oc:name        | folderMain (1)    |
+      | oc:permissions | RDNVCKZP          |
+      | oc:size        | 0                 |

tests/acceptance/features/coreApiTrashbinRestore/trashbinRestore.feature

@@ -567,3 +567,52 @@ Feature: restore deleted files/folders
       | dav-path-version |
       | spaces           |
       | new              |
+
+  @issue-1523
+  Scenario Outline: restore deleted folder when folder with same name exists
+    Given using <dav-path-version> DAV path
+    And user "Alice" has created folder "new"
+    And user "Alice" has uploaded file with content "content" to "new/test.txt"
+    And user "Alice" has deleted folder "new"
+    And user "Alice" has created folder "new"
+    And user "Alice" has uploaded file with content "new content" to "new/new-file.txt"
+    When user "Alice" restores the folder with original path "/new" to "/new (1)" using the trashbin API
+    Then the HTTP status code should be "201"
+    And as "Alice" the following folders should exist
+      | path          |
+      | /new     |
+      | /new (1) |
+    And as "Alice" the following files should exist
+      | path               |
+      | /new/new-file.txt   |
+      | /new (1)/test.txt  |
+    Examples:
+      | dav-path-version |
+      | spaces           |
+      | new              |
+
+  @issue-1523
+  Scenario Outline: restore deleted folder with files when folder with same name exists
+    Given using <dav-path-version> DAV path
+    And user "Alice" has created folder "folder-a"
+    And user "Alice" has uploaded file with content "content b" to "folder-a/b.txt"
+    And user "Alice" has uploaded file with content "content c" to "folder-a/c.txt"
+    And user "Alice" has deleted file "folder-a/b.txt"
+    And user "Alice" has deleted folder "folder-a"
+    And user "Alice" has created folder "folder-a"
+    When user "Alice" restores the file with original path "folder-a/b.txt" using the trashbin API
+    Then the HTTP status code should be "201"
+    When user "Alice" restores the folder with original path "/folder-a" to "/folder-a (1)" using the trashbin API
+    Then the HTTP status code should be "201"
+    And as "Alice" the following folders should exist
+      | path          |
+      | /folder-a     |
+      | /folder-a (1) |
+    And as "Alice" the following files should exist
+      | path               |
+      | /folder-a/b.txt    |
+      | /folder-a (1)/c.txt |
+    Examples:
+      | dav-path-version |
+      | spaces           |
+      | new              |

vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/blobstore/blobstore.go

@@ -20,13 +20,21 @@
 package blobstore
 
 import (
-	"bufio"
+	"context"
+	"fmt"
 	"io"
 	"os"
 	"path/filepath"
+	"time"
+
+	"github.com/pkg/errors"
+	"github.com/pkg/xattr"
 
 	"github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/node"
-	"github.com/pkg/errors"
+)
+
+const (
+	TMPDir = ".oc-tmp"
 )
 
 // Blobstore provides an interface to an filesystem based blobstore
@@ -41,61 +49,106 @@ func New(root string) (*Blobstore, error) {
 	}, nil
 }
 
-// Upload stores some data in the blobstore under the given key
-func (bs *Blobstore) Upload(node *node.Node, source, copyTarget string) error {
-	path := node.InternalPath()
+// Upload is responsible for transferring data from a source file (upload) to its final location;
+// the file operation is done atomically using a temporary file followed by a rename
+func (bs *Blobstore) Upload(n *node.Node, source, copyTarget string) error {
+	tempName := filepath.Join(n.SpaceRoot.InternalPath(), TMPDir, filepath.Base(source))
 
-	// preserve the mtime of the file
-	fi, _ := os.Stat(path)
-
-	file, err := os.Open(source)
-	if err != nil {
-		return errors.Wrap(err, "Decomposedfs: posix blobstore: Can not open source file to upload")
-	}
-	defer file.Close()
-
-	f, err := os.OpenFile(node.InternalPath(), os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0700)
-	if err != nil {
-		return errors.Wrapf(err, "could not open blob '%s' for writing", node.InternalPath())
-	}
-	defer f.Close()
-
-	w := bufio.NewWriter(f)
-	_, err = w.ReadFrom(file)
-	if err != nil {
-		return errors.Wrapf(err, "could not write blob '%s'", node.InternalPath())
-	}
-	err = w.Flush()
-	if err != nil {
-		return err
-	}
-	err = os.Chtimes(path, fi.ModTime(), fi.ModTime())
-	if err != nil {
+	// there is no guarantee that the space root TMPDir exists at this point, so we create the directory if needed
+	if err := os.MkdirAll(filepath.Dir(tempName), 0700); err != nil {
 		return err
 	}
 
-	if copyTarget != "" {
-		// also "upload" the file to a local path, e.g. for keeping the "current" version of the file
-		err := os.MkdirAll(filepath.Dir(copyTarget), 0700)
-		if err != nil {
-			return err
+	sourceFile, err := os.Open(source)
+	if err != nil {
+		return fmt.Errorf("failed to open source file '%s': %v", source, err)
+	}
+	defer func() {
+		_ = sourceFile.Close()
+	}()
+
+	tempFile, err := os.OpenFile(tempName, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0700)
+	if err != nil {
+		return fmt.Errorf("unable to create temp file '%s': %v", tempName, err)
+	}
+
+	if _, err := tempFile.ReadFrom(sourceFile); err != nil {
+		return fmt.Errorf("failed to write data from source file '%s' to temp file '%s' - %v", source, tempName, err)
+	}
+
+	if err := tempFile.Sync(); err != nil {
+		return fmt.Errorf("failed to sync temp file '%s' - %v", tempName, err)
+	}
+
+	if err := tempFile.Close(); err != nil {
+		return fmt.Errorf("failed to close temp file '%s' - %v", tempName, err)
+	}
+
+	nodeAttributes, err := n.Xattrs(context.Background())
+	if err != nil {
+		return fmt.Errorf("failed to get xattrs for node '%s': %v", n.InternalPath(), err)
+	}
+
+	var mtime *time.Time
+	for k, v := range nodeAttributes {
+		if err := xattr.Set(tempName, k, v); err != nil {
+			return fmt.Errorf("failed to set xattr '%s' on temp file '%s' - %v", k, tempName, err)
 		}
 
-		_, err = file.Seek(0, 0)
-		if err != nil {
-			return err
+		if k == "user.oc.mtime" {
+			tv, err := time.Parse(time.RFC3339Nano, string(v))
+			if err == nil {
+				mtime = &tv
+			}
+		}
+	}
+
+	// the extended attributes should always contain a mtime, but in case they don't, we fetch it from the node
+	if mtime == nil {
+		switch nodeMtime, err := n.GetMTime(context.Background()); {
+		case err != nil:
+			return fmt.Errorf("failed to get mtime for node '%s' - %v", n.InternalPath(), err)
+		default:
+			mtime = &nodeMtime
 		}
 
-		copyFile, err := os.OpenFile(copyTarget, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600)
-		if err != nil {
-			return errors.Wrapf(err, "could not open copy target '%s' for writing", copyTarget)
-		}
-		defer copyFile.Close()
+	}
 
-		_, err = copyFile.ReadFrom(file)
-		if err != nil {
-			return errors.Wrapf(err, "could not write blob copy of '%s' to '%s'", node.InternalPath(), copyTarget)
-		}
+	// etags rely on the id and the mtime, so we need to ensure the mtime is set correctly
+	if err := os.Chtimes(tempName, *mtime, *mtime); err != nil {
+		return fmt.Errorf("failed to set mtime on temp file '%s' - %v", tempName, err)
+	}
+
+	// atomically move the file to its final location,
+	// on Windows systems (unsupported oc os) os.Rename is not atomic
+	if err := os.Rename(tempName, n.InternalPath()); err != nil {
+		return fmt.Errorf("failed to move temp file '%s' to node '%s' - %v", tempName, n.InternalPath(), err)
+	}
+
+	// upload successfully, now handle the copy target if set
+	if copyTarget == "" {
+		return nil
+	}
+
+	// also "upload" the file to a local path, e.g., for keeping the "current" version of the file
+	if err := os.MkdirAll(filepath.Dir(copyTarget), 0700); err != nil {
+		return err
+	}
+
+	if _, err := sourceFile.Seek(0, 0); err != nil {
+		return err
+	}
+
+	copyFile, err := os.OpenFile(copyTarget, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600)
+	if err != nil {
+		return errors.Wrapf(err, "could not open copy target '%s' for writing", copyTarget)
+	}
+	defer func() {
+		_ = copyFile.Close()
+	}()
+
+	if _, err := copyFile.ReadFrom(sourceFile); err != nil {
+		return errors.Wrapf(err, "could not write blob copy of '%s' to '%s'", n.InternalPath(), copyTarget)
 	}
 
 	return nil

vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/trashbin/trashbin.go

@@ -32,6 +32,7 @@ import (
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
+
 	"github.com/opencloud-eu/reva/v2/pkg/errtypes"
 	"github.com/opencloud-eu/reva/v2/pkg/storage"
 	"github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/lookup"
@@ -333,10 +334,12 @@ func (tb *Trashbin) RestoreRecycleItem(ctx context.Context, spaceID string, key,
 		return nil, fmt.Errorf("trashbin: parent id not found for %s", restorePath)
 	}
 
-	trashNode := &trashNode{spaceID: spaceID, id: id, path: trashPath}
-	err = tb.lu.MetadataBackend().Set(ctx, trashNode, prefixes.ParentidAttr, []byte(parentID))
-	if err != nil {
-		return nil, err
+	trashedNode := &trashNode{spaceID: spaceID, id: id, path: trashPath}
+	if err = tb.lu.MetadataBackend().SetMultiple(ctx, trashedNode, map[string][]byte{
+		prefixes.NameAttr:     []byte(filepath.Base(restorePath)),
+		prefixes.ParentidAttr: []byte(parentID),
+	}, true); err != nil {
+		return nil, fmt.Errorf("posixfs: failed to update trashed node metadata: %w", err)
 	}
 
 	// restore the item

vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/tree/tree.go

@@ -38,9 +38,11 @@ import (
 	"golang.org/x/sync/errgroup"
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+
 	"github.com/opencloud-eu/reva/v2/pkg/appctx"
 	"github.com/opencloud-eu/reva/v2/pkg/errtypes"
 	"github.com/opencloud-eu/reva/v2/pkg/events"
+	"github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/blobstore"
 	"github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/lookup"
 	"github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/options"
 	"github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/trashbin"
@@ -649,6 +651,10 @@ func (t *Tree) createDirNode(ctx context.Context, n *node.Node) (err error) {
 	return n.SetXattrsWithContext(ctx, attributes, false)
 }
 
+func (t *Tree) isTemporary(path string) bool {
+	return path == blobstore.TMPDir
+}
+
 func (t *Tree) isIgnored(path string) bool {
 	return isLockFile(path) || isTrash(path) || t.isUpload(path) || t.isInternal(path)
 }
@@ -664,7 +670,7 @@ func (t *Tree) isIndex(path string) bool {
 func (t *Tree) isInternal(path string) bool {
 	return path == t.options.Root ||
 		path == filepath.Join(t.options.Root, "users") ||
-		t.isIndex(path) || strings.Contains(path, lookup.MetadataDir)
+		t.isIndex(path) || strings.Contains(path, lookup.MetadataDir) || t.isTemporary(path)
 }
 
 func isLockFile(path string) bool {

vendor/github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/upload/upload.go

@@ -286,6 +286,17 @@ func (session *DecomposedFsSession) Finalize(ctx context.Context) (err error) {
 	revisionNode := node.New(session.SpaceID(), session.NodeID(), "", "", session.Size(), session.ID(),
 		provider.ResourceType_RESOURCE_TYPE_FILE, session.SpaceOwner(), session.store.lu)
 
+	switch spaceRoot, err := session.store.lu.NodeFromSpaceID(ctx, session.SpaceID()); {
+	case err != nil:
+		return fmt.Errorf("failed to get space root for space id %s: %v", session.SpaceID(), err)
+	case spaceRoot == nil:
+		return fmt.Errorf("space root for space id %s not found", session.SpaceID())
+	case spaceRoot.InternalPath() == "":
+		return fmt.Errorf("space root for space id %s has no valid internal path", session.SpaceID())
+	default:
+		revisionNode.SpaceRoot = spaceRoot
+	}
+
 	// upload the data to the blobstore
 	_, subspan := tracer.Start(ctx, "WriteBlob")
 	err = session.store.tp.WriteBlob(revisionNode, session.binPath())

vendor/modules.txt

@@ -1198,7 +1198,7 @@ github.com/open-policy-agent/opa/v1/types
 github.com/open-policy-agent/opa/v1/util
 github.com/open-policy-agent/opa/v1/util/decoding
 github.com/open-policy-agent/opa/v1/version
-# github.com/opencloud-eu/reva/v2 v2.29.4
+# github.com/opencloud-eu/reva/v2 v2.29.5
 ## explicit; go 1.24.1
 github.com/opencloud-eu/reva/v2/cmd/revad/internal/grace
 github.com/opencloud-eu/reva/v2/cmd/revad/runtime