Merge pull request #4012 from 2403905/issue-1248

fix mtime if 0 size file uploaded

.drone.env

@@ -1,4 +1,4 @@
 # The test runner source for API tests
-APITESTS_COMMITID=2ea3b8c400688b14ef328786b3af445c2edbbe18
+APITESTS_COMMITID=c8f8bc55e22597bc3d53b63b2f38f345b4814b07
 APITESTS_BRANCH=master
 APITESTS_REPO_GIT_URL=https://github.com/owncloud/ocis.git

changelog/unreleased/fix-mtime.md

@@ -0,0 +1,6 @@
+Bugfix: fix mtime if 0 size file uploaded
+
+Fix mtime if 0 size file uploaded
+
+https://github.com/cs3org/reva/pull/4012
+https://github.com/owncloud/ocis/issues/1248

internal/grpc/services/storageprovider/storageprovider.go

@@ -662,13 +662,15 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta
 
 func (s *service) TouchFile(ctx context.Context, req *provider.TouchFileRequest) (*provider.TouchFileResponse, error) {
 	// FIXME these should be part of the TouchFileRequest object
+	var mtime string
 	if req.Opaque != nil {
 		if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" {
 			ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value))
 		}
+		mtime = utils.ReadPlainFromOpaque(req.Opaque, "X-OC-Mtime")
 	}
 
-	err := s.storage.TouchFile(ctx, req.Ref, utils.ExistsInOpaque(req.Opaque, "markprocessing"))
+	err := s.storage.TouchFile(ctx, req.Ref, utils.ExistsInOpaque(req.Opaque, "markprocessing"), mtime)
 
 	return &provider.TouchFileResponse{
 		Status: status.NewStatusFromErrType(ctx, "touch file", err),

internal/http/services/owncloud/ocdav/put.go

@@ -154,9 +154,17 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
+	opaque := &typespb.Opaque{}
+	if mtime := r.Header.Get(net.HeaderOCMtime); mtime != "" {
+		utils.AppendPlainToOpaque(opaque, net.HeaderOCMtime, mtime)
+
+		// TODO: find a way to check if the storage really accepted the value
+		w.Header().Set(net.HeaderOCMtime, "accepted")
+	}
 	if length == 0 {
 		tfRes, err := client.TouchFile(ctx, &provider.TouchFileRequest{
-			Ref: ref,
+			Opaque: opaque,
+			Ref:    ref,
 		})
 		if err != nil {
 			log.Error().Err(err).Msg("error sending grpc touch file request")
@@ -191,22 +199,7 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ
 		return
 	}
 
-	opaqueMap := map[string]*typespb.OpaqueEntry{
-		net.HeaderUploadLength: {
-			Decoder: "plain",
-			Value:   []byte(strconv.FormatInt(length, 10)),
-		},
-	}
-
-	if mtime := r.Header.Get(net.HeaderOCMtime); mtime != "" {
-		opaqueMap[net.HeaderOCMtime] = &typespb.OpaqueEntry{
-			Decoder: "plain",
-			Value:   []byte(mtime),
-		}
-
-		// TODO: find a way to check if the storage really accepted the value
-		w.Header().Set(net.HeaderOCMtime, "accepted")
-	}
+	utils.AppendPlainToOpaque(opaque, net.HeaderUploadLength, strconv.FormatInt(length, 10))
 
 	// curl -X PUT https://demo.owncloud.com/remote.php/webdav/testcs.bin -u demo:demo -d '123' -v -H 'OC-Checksum: SHA1:40bd001563085fc35165329ea1ff5c5ecbdbbeef'
 
@@ -231,16 +224,13 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ
 	// we do not check the algorithm here, because it might depend on the storage
 	if len(cparts) == 2 {
 		// Translate into TUS style Upload-Checksum header
-		opaqueMap[net.HeaderUploadChecksum] = &typespb.OpaqueEntry{
-			Decoder: "plain",
-			// algorithm is always lowercase, checksum is separated by space
-			Value: []byte(strings.ToLower(cparts[0]) + " " + cparts[1]),
-		}
+		// algorithm is always lowercase, checksum is separated by space
+		utils.AppendPlainToOpaque(opaque, net.HeaderUploadChecksum, strings.ToLower(cparts[0])+" "+cparts[1])
 	}
 
 	uReq := &provider.InitiateFileUploadRequest{
 		Ref:    ref,
-		Opaque: &typespb.Opaque{Map: opaqueMap},
+		Opaque: opaque,
 	}
 	if ifMatch := r.Header.Get(net.HeaderIfMatch); ifMatch != "" {
 		uReq.Options = &provider.InitiateFileUploadRequest_IfMatch{IfMatch: ifMatch}

pkg/storage/fs/cephfs/cephfs.go

@@ -165,7 +165,7 @@ func (fs *cephfs) CreateDir(ctx context.Context, ref *provider.Reference) error
 }
 
 // TouchFile as defined in the storage.FS interface
-func (fs *cephfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error {
+func (fs *cephfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error {
 	return fmt.Errorf("unimplemented: TouchFile")
 }
 

pkg/storage/fs/nextcloud/nextcloud.go

@@ -266,7 +266,7 @@ func (nc *StorageDriver) CreateDir(ctx context.Context, ref *provider.Reference)
 }
 
 // TouchFile as defined in the storage.FS interface
-func (nc *StorageDriver) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error {
+func (nc *StorageDriver) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error {
 	return fmt.Errorf("unimplemented: TouchFile")
 }
 

pkg/storage/fs/owncloudsql/owncloudsql.go

@@ -794,7 +794,7 @@ func (fs *owncloudsqlfs) CreateDir(ctx context.Context, ref *provider.Reference)
 }
 
 // TouchFile as defined in the storage.FS interface
-func (fs *owncloudsqlfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error {
+func (fs *owncloudsqlfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error {
 	ip, err := fs.resolve(ctx, ref)
 	if err != nil {
 		return err
@@ -830,15 +830,25 @@ func (fs *owncloudsqlfs) TouchFile(ctx context.Context, ref *provider.Reference,
 	if err != nil {
 		return err
 	}
-	mtime := time.Now().Unix()
+	storageMtime := time.Now().Unix()
+	mt := storageMtime
+	if mtime != "" {
+		t, err := strconv.Atoi(mtime)
+		if err != nil {
+			log.Info().
+				Str("owncloudsql", ip).
+				Msg("error mtime conversion. mtine set to system time")
+		}
+		mt = time.Unix(int64(t), 0).Unix()
+	}
 
 	data := map[string]interface{}{
 		"path":          fs.toDatabasePath(ip),
 		"etag":          calcEtag(ctx, fi),
 		"mimetype":      mime.Detect(false, ip),
 		"permissions":   int(conversions.RoleFromResourcePermissions(parentPerms, false).OCSPermissions()), // inherit permissions of parent
-		"mtime":         mtime,
-		"storage_mtime": mtime,
+		"mtime":         mt,
+		"storage_mtime": storageMtime,
 	}
 	storageID, err := fs.getStorage(ctx, ip)
 	if err != nil {

pkg/storage/fs/s3/s3.go

@@ -348,7 +348,7 @@ func (fs *s3FS) CreateDir(ctx context.Context, ref *provider.Reference) error {
 }
 
 // TouchFile as defined in the storage.FS interface
-func (fs *s3FS) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error {
+func (fs *s3FS) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error {
 	return fmt.Errorf("unimplemented: TouchFile")
 }
 

pkg/storage/storage.go

@@ -38,7 +38,7 @@ type FS interface {
 	GetHome(ctx context.Context) (string, error)
 	CreateHome(ctx context.Context) error
 	CreateDir(ctx context.Context, ref *provider.Reference) error
-	TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error
+	TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) 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, fieldMask []string) (*provider.ResourceInfo, error)

pkg/storage/utils/decomposedfs/decomposedfs.go

@@ -76,7 +76,7 @@ type Tree interface {
 	ListFolder(ctx context.Context, node *node.Node) ([]*node.Node, error)
 	// CreateHome(owner *userpb.UserId) (n *node.Node, err error)
 	CreateDir(ctx context.Context, node *node.Node) (err error)
-	TouchFile(ctx context.Context, node *node.Node, markprocessing bool) error
+	TouchFile(ctx context.Context, node *node.Node, markprocessing bool, mtime string) error
 	// CreateReference(ctx context.Context, node *node.Node, targetURI *url.URL) error
 	Move(ctx context.Context, oldNode *node.Node, newNode *node.Node) (err error)
 	Delete(ctx context.Context, node *node.Node) (err error)
@@ -616,7 +616,7 @@ func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference)
 }
 
 // TouchFile as defined in the storage.FS interface
-func (fs *Decomposedfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error {
+func (fs *Decomposedfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error {
 	parentRef := &provider.Reference{
 		ResourceId: ref.ResourceId,
 		Path:       path.Dir(ref.Path),
@@ -655,7 +655,7 @@ func (fs *Decomposedfs) TouchFile(ctx context.Context, ref *provider.Reference,
 	if err := n.CheckLock(ctx); err != nil {
 		return err
 	}
-	return fs.tp.TouchFile(ctx, n, markprocessing)
+	return fs.tp.TouchFile(ctx, n, markprocessing, mtime)
 }
 
 // CreateReference creates a reference as a node folder with the target stored in extended attributes

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

@@ -128,7 +128,7 @@ func (t *Tree) GetMD(ctx context.Context, n *node.Node) (os.FileInfo, error) {
 }
 
 // TouchFile creates a new empty file
-func (t *Tree) TouchFile(ctx context.Context, n *node.Node, markprocessing bool) error {
+func (t *Tree) TouchFile(ctx context.Context, n *node.Node, markprocessing bool, mtime string) error {
 	if n.Exists {
 		if markprocessing {
 			return n.SetXattr(prefixes.StatusPrefix, []byte(node.ProcessingStatus))
@@ -155,6 +155,11 @@ func (t *Tree) TouchFile(ctx context.Context, n *node.Node, markprocessing bool)
 	if markprocessing {
 		attributes[prefixes.StatusPrefix] = []byte(node.ProcessingStatus)
 	}
+	if mtime != "" {
+		if err := n.SetMtimeString(mtime); err != nil {
+			return errors.Wrap(err, "Decomposedfs: could not set mtime")
+		}
+	}
 	err = n.SetXattrs(attributes, true)
 	if err != nil {
 		return err

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

@@ -246,7 +246,7 @@ var _ = Describe("Tree", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(fileToBeCreated.Exists).To(BeFalse())
 
-				err = t.TouchFile(env.Ctx, fileToBeCreated, false)
+				err = t.TouchFile(env.Ctx, fileToBeCreated, false, "")
 				Expect(err).ToNot(HaveOccurred())
 
 				existingFile, err := env.Lookup.NodeFromResource(env.Ctx, ref)

pkg/storage/utils/eosfs/eosfs.go

@@ -21,6 +21,7 @@ package eosfs
 import (
 	"context"
 	"database/sql"
+	b64 "encoding/base64"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -33,8 +34,6 @@ import (
 	"strings"
 	"time"
 
-	b64 "encoding/base64"
-
 	"github.com/bluele/gcache"
 	grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
@@ -1673,7 +1672,7 @@ func (fs *eosfs) CreateDir(ctx context.Context, ref *provider.Reference) error {
 }
 
 // TouchFile as defined in the storage.FS interface
-func (fs *eosfs) TouchFile(ctx context.Context, ref *provider.Reference, _ bool) error {
+func (fs *eosfs) TouchFile(ctx context.Context, ref *provider.Reference, _ bool, _ string) error {
 	log := appctx.GetLogger(ctx)
 
 	fn, auth, err := fs.resolveRefAndGetAuth(ctx, ref)

pkg/storage/utils/localfs/localfs.go

@@ -806,7 +806,7 @@ func (fs *localfs) CreateDir(ctx context.Context, ref *provider.Reference) error
 }
 
 // TouchFile as defined in the storage.FS interface
-func (fs *localfs) TouchFile(ctx context.Context, ref *provider.Reference, _ bool) error {
+func (fs *localfs) TouchFile(ctx context.Context, ref *provider.Reference, _ bool, _ string) error {
 	return fmt.Errorf("unimplemented: TouchFile")
 }