diff --git a/changelog/unreleased/fix-simple-upload.md b/changelog/unreleased/fix-simple-upload.md new file mode 100644 index 0000000000..7d436fffd5 --- /dev/null +++ b/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 + diff --git a/pkg/storage/utils/decomposedfs/upload.go b/pkg/storage/utils/decomposedfs/upload.go index d0b84f6a79..eba3c91808 100644 --- a/pkg/storage/utils/decomposedfs/upload.go +++ b/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 } diff --git a/pkg/storage/utils/decomposedfs/upload/upload.go b/pkg/storage/utils/decomposedfs/upload/upload.go index 427af92dbc..f460c526ed 100644 --- a/pkg/storage/utils/decomposedfs/upload/upload.go +++ b/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()