Merge pull request #4920 from butonic/fix-simple-upload

fix simple upload status code

changelog/unreleased/fix-simple-upload.md

@@ -0,0 +1,6 @@
+Bugfix: Return correct status codes for simple uploads
+
+Decomposedfs now returns the correct precondition failed status code when the etag does not match. This allows the jsoncs3 share managers optimistic locking to handle concurrent writes correctly
+
+https://github.com/cs3org/reva/pull/4920
+

pkg/storage/utils/decomposedfs/upload.go

@@ -26,10 +26,11 @@ import (
 	"strings"
 	"time"
 
+	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/google/uuid"
+	"github.com/pkg/errors"
 	tusd "github.com/tus/tusd/v2/pkg/handler"
 
-	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/errtypes"
@@ -40,7 +41,6 @@ import (
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload"
 	"github.com/cs3org/reva/v2/pkg/storagespace"
 	"github.com/cs3org/reva/v2/pkg/utils"
-	"github.com/pkg/errors"
 )
 
 // Upload uploads data to the given resource
@@ -90,7 +90,7 @@ func (fs *Decomposedfs) Upload(ctx context.Context, req storage.UploadRequest, u
 		}
 	}
 
-	if err := session.FinishUpload(ctx); err != nil {
+	if err := session.FinishUploadDecomposed(ctx); err != nil {
 		return &provider.ResourceInfo{}, err
 	}
 
@@ -321,7 +321,7 @@ func (fs *Decomposedfs) InitiateUpload(ctx context.Context, ref *provider.Refere
 
 	if uploadLength == 0 {
 		// Directly finish this upload
-		err = session.FinishUpload(ctx)
+		err = session.FinishUploadDecomposed(ctx)
 		if err != nil {
 			return nil, err
 		}

pkg/storage/utils/decomposedfs/upload/upload.go

@@ -33,6 +33,12 @@ import (
 
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	"github.com/golang-jwt/jwt"
+	"github.com/pkg/errors"
+	tusd "github.com/tus/tusd/v2/pkg/handler"
+	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/trace"
+
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/errtypes"
@@ -41,11 +47,6 @@ import (
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/metadata/prefixes"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node"
 	"github.com/cs3org/reva/v2/pkg/utils"
-	"github.com/golang-jwt/jwt"
-	"github.com/pkg/errors"
-	tusd "github.com/tus/tusd/v2/pkg/handler"
-	"go.opentelemetry.io/otel"
-	"go.opentelemetry.io/otel/trace"
 )
 
 var (
@@ -106,7 +107,25 @@ func (session *OcisSession) GetReader(ctx context.Context) (io.ReadCloser, error
 }
 
 // FinishUpload finishes an upload and moves the file to the internal destination
+// implements tusd.DataStore interface
+// returns tusd errors
 func (session *OcisSession) FinishUpload(ctx context.Context) error {
+	err := session.FinishUploadDecomposed(ctx)
+
+	//  we need to return a tusd error here to make the tusd handler return the correct status code
+	switch err.(type) {
+	case errtypes.AlreadyExists:
+		return tusd.NewError("ERR_ALREADY_EXISTS", err.Error(), http.StatusConflict)
+	case errtypes.Aborted:
+		return tusd.NewError("ERR_PRECONDITION_FAILED", err.Error(), http.StatusPreconditionFailed)
+	default:
+		return err
+	}
+}
+
+// FinishUploadDecomposed finishes an upload and moves the file to the internal destination
+// retures errtypes errors
+func (session *OcisSession) FinishUploadDecomposed(ctx context.Context) error {
 	ctx, span := tracer.Start(session.Context(ctx), "FinishUpload")
 	defer span.End()
 	log := appctx.GetLogger(ctx)
@@ -167,16 +186,7 @@ func (session *OcisSession) FinishUpload(ctx context.Context) error {
 
 	n, err := session.store.CreateNodeForUpload(session, attrs)
 	if err != nil {
-		session.store.Cleanup(ctx, session, true, false, false)
-		//  we need to return a tusd error here to make the tusd handler return the correct status code
-		switch err.(type) {
-		case errtypes.AlreadyExists:
-			return ErrAlreadyExists
-		case errtypes.Aborted:
-			return tusd.NewError("ERR_PRECONDITION_FAILED", err.Error(), http.StatusPreconditionFailed)
-		default:
-			return err
-		}
+		return err
 	}
 	// increase the processing counter for every started processing
 	// will be decreased in Cleanup()