fix(upload): [OCISDEV-1471] skip TouchFile's propagation for coordinator uploads

pkg/storage/fs/posix/tree/tree.go

@@ -41,6 +41,7 @@ import (
 	"github.com/owncloud/reva/v2/pkg/appctx"
 	"github.com/owncloud/reva/v2/pkg/errtypes"
 	"github.com/owncloud/reva/v2/pkg/events"
+	"github.com/owncloud/reva/v2/pkg/storage"
 	"github.com/owncloud/reva/v2/pkg/storage/fs/posix/lookup"
 	"github.com/owncloud/reva/v2/pkg/storage/fs/posix/options"
 	"github.com/owncloud/reva/v2/pkg/storage/fs/posix/trashbin"
@@ -247,6 +248,9 @@ func (t *Tree) TouchFile(ctx context.Context, n *node.Node, markprocessing bool,
 		return err
 	}
 
+	if storage.SkipTouchPropagation(ctx) {
+		return nil
+	}
 	return t.Propagate(ctx, n, 0)
 }
 

pkg/storage/uploads.go

@@ -31,6 +31,19 @@ import (
 // UploadFinishedFunc is a callback function used in storage drivers to indicate that an upload has finished
 type UploadFinishedFunc func(spaceOwner, executant *userpb.UserId, ref *provider.Reference)
 
+type skipTouchPropagationKey struct{}
+
+// ContextSkipTouchPropagation tells TouchFile not to propagate.
+func ContextSkipTouchPropagation(ctx context.Context) context.Context {
+	return context.WithValue(ctx, skipTouchPropagationKey{}, true)
+}
+
+// SkipTouchPropagation reports whether TouchFile should skip propagation.
+func SkipTouchPropagation(ctx context.Context) bool {
+	skip, _ := ctx.Value(skipTouchPropagationKey{}).(bool)
+	return skip
+}
+
 // UploadRequest us used in FS.Upload() to carry required upload metadata
 type UploadRequest struct {
 	Ref    *provider.Reference

pkg/storage/utils/decomposedfs/tree/tree.go

@@ -34,6 +34,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/owncloud/reva/v2/pkg/appctx"
 	"github.com/owncloud/reva/v2/pkg/errtypes"
+	"github.com/owncloud/reva/v2/pkg/storage"
 	"github.com/owncloud/reva/v2/pkg/storage/utils/decomposedfs/lookup"
 	"github.com/owncloud/reva/v2/pkg/storage/utils/decomposedfs/metadata"
 	"github.com/owncloud/reva/v2/pkg/storage/utils/decomposedfs/metadata/prefixes"
@@ -183,6 +184,9 @@ func (t *Tree) TouchFile(ctx context.Context, n *node.Node, markprocessing bool,
 		}
 	}
 
+	if storage.SkipTouchPropagation(ctx) {
+		return nil
+	}
 	return t.Propagate(ctx, n, 0)
 }
 

pkg/storage/utils/decomposedfs/tree/tree_test.go

@@ -19,6 +19,7 @@
 package tree_test
 
 import (
+	"context"
 	"errors"
 	"os"
 	"path"
@@ -27,6 +28,7 @@ import (
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/google/uuid"
 	"github.com/rs/zerolog"
+	"github.com/owncloud/reva/v2/pkg/storage"
 	"github.com/owncloud/reva/v2/pkg/storage/fs/posix/timemanager"
 	"github.com/owncloud/reva/v2/pkg/storage/utils/decomposedfs/lookup"
 	"github.com/owncloud/reva/v2/pkg/storage/utils/decomposedfs/metadata"
@@ -324,6 +326,39 @@ var _ = Describe("Tree", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(existingFile.Exists).To(BeTrue())
 			})
+
+			// Not the etag: it also follows the dir's own mtime, which the child link changes.
+			parentTMTime := func() string {
+				dir, err := env.Lookup.NodeFromID(env.Ctx, &provider.ResourceId{SpaceId: n.SpaceID, OpaqueId: n.ID})
+				Expect(err).ToNot(HaveOccurred())
+				tmtime, _ := dir.XattrString(env.Ctx, prefixes.TreeMTimeAttr)
+				return tmtime
+			}
+
+			touch := func(ctx context.Context) {
+				fileToBeCreated, err := env.Lookup.NodeFromResource(env.Ctx, &provider.Reference{
+					ResourceId: env.SpaceRootRes,
+					Path:       "emptydir/newFile",
+				})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(t.TouchFile(ctx, fileToBeCreated, false, "")).To(Succeed())
+			}
+
+			It("propagates the new file to its parent", func() {
+				before := parentTMTime()
+
+				touch(env.Ctx)
+
+				Expect(parentTMTime()).ToNot(Equal(before))
+			})
+
+			It("does not propagate when the context asks it not to", func() {
+				before := parentTMTime()
+
+				touch(storage.ContextSkipTouchPropagation(env.Ctx))
+
+				Expect(parentTMTime()).To(Equal(before))
+			})
 		})
 
 		Context("that was deleted", func() {

pkg/upload/coordinator.go

@@ -301,7 +301,8 @@ func (c *coordinator) touchNode(ctx context.Context, session Session) error {
 		Path: session.Filename(),
 	}
 	// MarkProcessing is the coordinator's own call, hence false here.
-	result, err := c.fs.TouchFile(ctx, pathRef, false, session.Metadata()["mtime"])
+	// PrepareUpload propagates the node, so TouchFile need not.
+	result, err := c.fs.TouchFile(storage.ContextSkipTouchPropagation(ctx), pathRef, false, session.Metadata()["mtime"])
 	if err != nil {
 		session.Cleanup(ctx, true, true)
 		if _, ok := err.(errtypes.IsNotFound); ok {

pkg/upload/coordinator_test.go

@@ -125,6 +125,16 @@ var _ = Describe("coordinator", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(session.NodeID()).To(Equal("real-node-id"))
 			})
+
+			// PrepareUpload propagates the node, so a second walk would be wasted.
+			It("tells TouchFile not to propagate", func() {
+				session := newSession(false)
+
+				_, err := c.finishUpload(ctx, session)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(fs.touchSkippedPropagation).To(BeTrue())
+			})
 		})
 
 		Context("for an overwrite", func() {

pkg/upload/fakefs_test.go

@@ -59,6 +59,8 @@ type fakeFS struct {
 	touchRef    *provider.Reference
 	touchMtime  string
 
+	touchSkippedPropagation bool
+
 	// hooks fire inside a driver call, to make the coordinator's next step fail.
 	afterMark, afterPrepare func()
 }
@@ -100,9 +102,10 @@ func (f *fakeFS) GetPathByID(_ context.Context, _ *provider.ResourceId) (string,
 	return f.pathByID, f.pathByIDErr
 }
 
-func (f *fakeFS) TouchFile(_ context.Context, ref *provider.Reference, markprocessing bool, mtime string) (*storage.TouchFileResult, error) {
+func (f *fakeFS) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) (*storage.TouchFileResult, error) {
 	f.record("TouchFile(markprocessing=%v)", markprocessing)
 	f.touchRef, f.touchMtime = ref, mtime
+	f.touchSkippedPropagation = storage.SkipTouchPropagation(ctx)
 	if f.touchErr != nil {
 		return nil, f.touchErr
 	}