From e70909cc1accfbd17c828a2c635952fee4a1007d Mon Sep 17 00:00:00 2001 From: Pradip Parmar Date: Tue, 1 Mar 2022 07:58:04 +0530 Subject: [PATCH 1/4] simplify logic for delete Signed-off-by: Pradip Parmar --- .../0chain.net/blobbercore/allocation/deletefilechange.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/code/go/0chain.net/blobbercore/allocation/deletefilechange.go b/code/go/0chain.net/blobbercore/allocation/deletefilechange.go index 618791755..338b9187a 100644 --- a/code/go/0chain.net/blobbercore/allocation/deletefilechange.go +++ b/code/go/0chain.net/blobbercore/allocation/deletefilechange.go @@ -3,6 +3,7 @@ package allocation import ( "context" "encoding/json" + "os" "github.com/0chain/blobber/code/go/0chain.net/blobbercore/datastore" "github.com/0chain/blobber/code/go/0chain.net/blobbercore/filestore" @@ -56,15 +57,19 @@ func (nf *DeleteFileChange) DeleteTempFile() error { func (nf *DeleteFileChange) CommitToFileStore(ctx context.Context) error { db := datastore.GetStore().GetTransaction(ctx) + var finalErr error for contenthash := range nf.ContentHash { var count int64 err := db.Table((&reference.Ref{}).TableName()).Where(db.Where(&reference.Ref{ThumbnailHash: contenthash}).Or(&reference.Ref{ContentHash: contenthash})).Where("deleted_at IS null").Where(&reference.Ref{AllocationID: nf.AllocationID}).Count(&count).Error if err == nil && count == 0 { Logger.Info("Deleting content file", zap.String("content_hash", contenthash)) if err := filestore.GetFileStore().DeleteFile(nf.AllocationID, contenthash); err != nil { + if os.IsNotExist(err) { + finalErr = err + } Logger.Error("FileStore_DeleteFile", zap.String("allocation_id", nf.AllocationID), zap.Error(err)) } } } - return nil + return finalErr } From 2ba2d972b7aef34736e9277f2e44b67d18805d1c Mon Sep 17 00:00:00 2001 From: Pradip Parmar Date: Tue, 1 Mar 2022 08:06:35 +0530 Subject: [PATCH 2/4] commit handler logic update. Signed-off-by: Pradip Parmar --- .../blobbercore/handler/object_operation_handler.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/code/go/0chain.net/blobbercore/handler/object_operation_handler.go b/code/go/0chain.net/blobbercore/handler/object_operation_handler.go index 4fd456ff1..dbf92480c 100644 --- a/code/go/0chain.net/blobbercore/handler/object_operation_handler.go +++ b/code/go/0chain.net/blobbercore/handler/object_operation_handler.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "encoding/json" "errors" + "os" "github.com/0chain/blobber/code/go/0chain.net/blobbercore/blobberhttp" "github.com/0chain/blobber/code/go/0chain.net/blobbercore/stats" @@ -515,6 +516,9 @@ func (fsh *StorageHandler) CommitWrite(ctx context.Context, r *http.Request) (*b } err = connectionObj.CommitToFileStore(ctx) if err != nil { + if os.IsNotExist(err) { + return nil, common.NewErrorfWithStatusCode(204, "invalid_file", "File does not exist at path") + } return nil, common.NewError("file_store_error", "Error committing to file store. "+err.Error()) } From f556e5d8e39581298a5882462943736d637881f7 Mon Sep 17 00:00:00 2001 From: Pradip Parmar Date: Wed, 2 Mar 2022 06:44:31 +0530 Subject: [PATCH 3/4] status handling for respond method Signed-off-by: Pradip Parmar --- code/go/0chain.net/core/common/handler.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/code/go/0chain.net/core/common/handler.go b/code/go/0chain.net/core/common/handler.go index 8f24c11b0..6afd09cba 100644 --- a/code/go/0chain.net/core/common/handler.go +++ b/code/go/0chain.net/core/common/handler.go @@ -44,12 +44,16 @@ func Respond(w http.ResponseWriter, data interface{}, err error) { if err != nil { data := make(map[string]interface{}, 2) data["error"] = err.Error() + statusCode := 400 if cerr, ok := err.(*Error); ok { data["code"] = cerr.Code + if cerr.StatusCode != 0 { + statusCode = cerr.StatusCode + } } buf := bytes.NewBuffer(nil) json.NewEncoder(buf).Encode(data) //nolint:errcheck // checked in previous step - http.Error(w, buf.String(), 400) + http.Error(w, buf.String(), statusCode) } else if data != nil { json.NewEncoder(w).Encode(data) //nolint:errcheck // checked in previous step } From 448c7f9686498b0ac9fba7b403336a90f8428f48 Mon Sep 17 00:00:00 2001 From: Lz Date: Wed, 2 Mar 2022 20:20:56 +0800 Subject: [PATCH 4/4] feat(delete): improved status code 204 logic --- .../allocation/deletefilechange.go | 8 ++- .../handler/file_command_delete.go | 13 +++- .../handler/grpc_commit_handler.go | 2 +- .../0chain.net/blobbercore/handler/handler.go | 41 +++++++++-- .../handler/object_operation_handler.go | 9 +-- code/go/0chain.net/core/common/constants.go | 3 + code/go/0chain.net/core/common/errors.go | 10 +-- code/go/0chain.net/core/common/handler.go | 68 ++++++++++++++++--- 8 files changed, 120 insertions(+), 34 deletions(-) diff --git a/code/go/0chain.net/blobbercore/allocation/deletefilechange.go b/code/go/0chain.net/blobbercore/allocation/deletefilechange.go index 338b9187a..c5896c367 100644 --- a/code/go/0chain.net/blobbercore/allocation/deletefilechange.go +++ b/code/go/0chain.net/blobbercore/allocation/deletefilechange.go @@ -8,6 +8,7 @@ import ( "github.com/0chain/blobber/code/go/0chain.net/blobbercore/datastore" "github.com/0chain/blobber/code/go/0chain.net/blobbercore/filestore" "github.com/0chain/blobber/code/go/0chain.net/blobbercore/reference" + "github.com/0chain/blobber/code/go/0chain.net/core/common" . "github.com/0chain/blobber/code/go/0chain.net/core/logging" "go.uber.org/zap" @@ -57,7 +58,7 @@ func (nf *DeleteFileChange) DeleteTempFile() error { func (nf *DeleteFileChange) CommitToFileStore(ctx context.Context) error { db := datastore.GetStore().GetTransaction(ctx) - var finalErr error + var errFileWasDeleted error for contenthash := range nf.ContentHash { var count int64 err := db.Table((&reference.Ref{}).TableName()).Where(db.Where(&reference.Ref{ThumbnailHash: contenthash}).Or(&reference.Ref{ContentHash: contenthash})).Where("deleted_at IS null").Where(&reference.Ref{AllocationID: nf.AllocationID}).Count(&count).Error @@ -65,11 +66,12 @@ func (nf *DeleteFileChange) CommitToFileStore(ctx context.Context) error { Logger.Info("Deleting content file", zap.String("content_hash", contenthash)) if err := filestore.GetFileStore().DeleteFile(nf.AllocationID, contenthash); err != nil { if os.IsNotExist(err) { - finalErr = err + errFileWasDeleted = common.ErrFileWasDeleted + continue } Logger.Error("FileStore_DeleteFile", zap.String("allocation_id", nf.AllocationID), zap.Error(err)) } } } - return finalErr + return errFileWasDeleted } diff --git a/code/go/0chain.net/blobbercore/handler/file_command_delete.go b/code/go/0chain.net/blobbercore/handler/file_command_delete.go index 786d5f7c0..60c26fdea 100644 --- a/code/go/0chain.net/blobbercore/handler/file_command_delete.go +++ b/code/go/0chain.net/blobbercore/handler/file_command_delete.go @@ -2,6 +2,7 @@ package handler import ( "context" + "errors" "net/http" "github.com/0chain/blobber/code/go/0chain.net/blobbercore/allocation" @@ -9,6 +10,7 @@ import ( "github.com/0chain/blobber/code/go/0chain.net/blobbercore/reference" "github.com/0chain/blobber/code/go/0chain.net/core/common" "github.com/0chain/gosdk/constants" + "gorm.io/gorm" ) // FileCommandDelete command for deleting file @@ -28,12 +30,17 @@ func (cmd *FileCommandDelete) IsAuthorized(ctx context.Context, req *http.Reques if path == "" { return common.NewError("invalid_parameters", "Invalid path") } - cmd.exisitingFileRef, _ = reference.GetReference(ctx, allocationObj.ID, path) - if cmd.exisitingFileRef == nil { - return common.NewErrorfWithStatusCode(204, "invalid_file", "File does not exist at path") + fileRef, err := reference.GetReference(ctx, allocationObj.ID, path) + if err != nil { + if errors.Is(gorm.ErrRecordNotFound, err) { + return common.ErrFileWasDeleted + } + return common.NewError("bad_db_operation", err.Error()) } + cmd.exisitingFileRef = fileRef + return nil } diff --git a/code/go/0chain.net/blobbercore/handler/grpc_commit_handler.go b/code/go/0chain.net/blobbercore/handler/grpc_commit_handler.go index cb522cbd2..0286f1abf 100644 --- a/code/go/0chain.net/blobbercore/handler/grpc_commit_handler.go +++ b/code/go/0chain.net/blobbercore/handler/grpc_commit_handler.go @@ -31,7 +31,7 @@ func (b *blobberGRPCService) Commit(ctx context.Context, req *blobbergrpc.Commit httpRequestWithMetaData(r, getGRPCMetaDataFromCtx(ctx), req.Allocation) r.Header.Set("Content-Type", writer.FormDataContentType()) - resp, err := CommitHandler(ctx, r) + resp, _, err := CommitHandler(ctx, r) if err != nil { return nil, err } diff --git a/code/go/0chain.net/blobbercore/handler/handler.go b/code/go/0chain.net/blobbercore/handler/handler.go index bc2eba128..cb7a2710d 100644 --- a/code/go/0chain.net/blobbercore/handler/handler.go +++ b/code/go/0chain.net/blobbercore/handler/handler.go @@ -51,7 +51,7 @@ func SetupHandlers(r *mux.Router) { r.HandleFunc("/v1/dir/{allocation}", common.ToJSONResponse(WithConnection(CreateDirHandler))).Methods(http.MethodPost, http.MethodOptions) r.HandleFunc("/v1/dir/{allocation}", common.ToJSONResponse(WithConnection(CreateDirHandler))).Methods(http.MethodDelete, http.MethodOptions) - r.HandleFunc("/v1/connection/commit/{allocation}", common.ToJSONResponse(WithConnection(CommitHandler))) + r.HandleFunc("/v1/connection/commit/{allocation}", common.ToStatusCode(WithStatusConnection(CommitHandler))) r.HandleFunc("/v1/file/commitmetatxn/{allocation}", common.ToJSONResponse(WithConnection(CommitMetaTxnHandler))) r.HandleFunc("/v1/file/collaborator/{allocation}", common.ToJSONResponse(WithConnection(CollaboratorHandler))) r.HandleFunc("/v1/file/calculatehash/{allocation}", common.ToJSONResponse(WithConnection(CalculateHashHandler))) @@ -121,6 +121,34 @@ func WithConnection(handler common.JSONResponderF) common.JSONResponderF { } } +func WithStatusConnection(handler common.StatusCodeResponderF) common.StatusCodeResponderF { + return func(ctx context.Context, r *http.Request) (resp interface{}, statusCode int, err error) { + ctx = GetMetaDataStore().CreateTransaction(ctx) + resp, statusCode, err = handler(ctx, r) + + defer func() { + if err != nil { + var rollErr = GetMetaDataStore().GetTransaction(ctx). + Rollback().Error + if rollErr != nil { + Logger.Error("couldn't rollback", zap.Error(err)) + } + } + }() + + if err != nil { + Logger.Error("Error in handling the request." + err.Error()) + return + } + err = GetMetaDataStore().GetTransaction(ctx).Commit().Error + if err != nil { + return resp, statusCode, common.NewErrorf("commit_error", + "error committing to meta store: %v", err) + } + return + } +} + func setupHandlerContext(ctx context.Context, r *http.Request) context.Context { var vars = mux.Vars(r) ctx = context.WithValue(ctx, constants.ContextKeyClient, @@ -239,15 +267,20 @@ func ListHandler(ctx context.Context, r *http.Request) (interface{}, error) { } /*CommitHandler is the handler to respond to upload requests fro clients*/ -func CommitHandler(ctx context.Context, r *http.Request) (interface{}, error) { +func CommitHandler(ctx context.Context, r *http.Request) (interface{}, int, error) { ctx = setupHandlerContext(ctx, r) response, err := storageHandler.CommitWrite(ctx, r) + if err != nil { - return nil, err + + if errors.Is(common.ErrFileWasDeleted, err) { + return response, http.StatusNoContent, nil + } + return nil, http.StatusBadRequest, err } - return response, nil + return response, http.StatusOK, nil } func ReferencePathHandler(ctx context.Context, r *http.Request) (interface{}, error) { diff --git a/code/go/0chain.net/blobbercore/handler/object_operation_handler.go b/code/go/0chain.net/blobbercore/handler/object_operation_handler.go index dbf92480c..bddb45e4a 100644 --- a/code/go/0chain.net/blobbercore/handler/object_operation_handler.go +++ b/code/go/0chain.net/blobbercore/handler/object_operation_handler.go @@ -5,7 +5,6 @@ import ( "encoding/hex" "encoding/json" "errors" - "os" "github.com/0chain/blobber/code/go/0chain.net/blobbercore/blobberhttp" "github.com/0chain/blobber/code/go/0chain.net/blobbercore/stats" @@ -516,10 +515,9 @@ func (fsh *StorageHandler) CommitWrite(ctx context.Context, r *http.Request) (*b } err = connectionObj.CommitToFileStore(ctx) if err != nil { - if os.IsNotExist(err) { - return nil, common.NewErrorfWithStatusCode(204, "invalid_file", "File does not exist at path") + if !errors.Is(common.ErrFileWasDeleted, err) { + return nil, common.NewError("file_store_error", "Error committing to file store. "+err.Error()) } - return nil, common.NewError("file_store_error", "Error committing to file store. "+err.Error()) } result.Changes = connectionObj.Changes @@ -533,6 +531,9 @@ func (fsh *StorageHandler) CommitWrite(ctx context.Context, r *http.Request) (*b result.Success = true result.ErrorMessage = "" + if errors.Is(common.ErrFileWasDeleted, err) { + return &result, err + } return &result, nil } diff --git a/code/go/0chain.net/core/common/constants.go b/code/go/0chain.net/core/common/constants.go index 2345aebe1..4314f134b 100644 --- a/code/go/0chain.net/core/common/constants.go +++ b/code/go/0chain.net/core/common/constants.go @@ -32,4 +32,7 @@ var ( // ErrEntityNotFound entity can't found in db ErrEntityNotFound = errors.New("entity not found") + + // ErrFileWasDeleted file already was deleted + ErrFileWasDeleted = errors.New("file was deleted") ) diff --git a/code/go/0chain.net/core/common/errors.go b/code/go/0chain.net/core/common/errors.go index 052696808..9f09e5b06 100644 --- a/code/go/0chain.net/core/common/errors.go +++ b/code/go/0chain.net/core/common/errors.go @@ -6,9 +6,8 @@ import ( /*Error type for a new application error */ type Error struct { - Code string `json:"code,omitempty"` - Msg string `json:"msg"` - StatusCode int `json:"status_code,omitempty"` + Code string `json:"code,omitempty"` + Msg string `json:"msg"` } func (err *Error) Error() string { @@ -25,11 +24,6 @@ func NewErrorf(code, format string, args ...interface{}) *Error { return &Error{Code: code, Msg: fmt.Sprintf(format, args...)} } -/*NewErrorf - create a new error with format */ -func NewErrorfWithStatusCode(statusCode int, errCode, format string, args ...interface{}) *Error { - return &Error{StatusCode: statusCode, Code: errCode, Msg: fmt.Sprintf(format, args...)} -} - /*InvalidRequest - create error messages that are needed when validating request input */ func InvalidRequest(msg string) error { return NewError("invalid_request", fmt.Sprintf("Invalid request (%v)", msg)) diff --git a/code/go/0chain.net/core/common/handler.go b/code/go/0chain.net/core/common/handler.go index 6afd09cba..0dd0d4fcd 100644 --- a/code/go/0chain.net/core/common/handler.go +++ b/code/go/0chain.net/core/common/handler.go @@ -44,16 +44,12 @@ func Respond(w http.ResponseWriter, data interface{}, err error) { if err != nil { data := make(map[string]interface{}, 2) data["error"] = err.Error() - statusCode := 400 if cerr, ok := err.(*Error); ok { data["code"] = cerr.Code - if cerr.StatusCode != 0 { - statusCode = cerr.StatusCode - } } buf := bytes.NewBuffer(nil) json.NewEncoder(buf).Encode(data) //nolint:errcheck // checked in previous step - http.Error(w, buf.String(), statusCode) + http.Error(w, buf.String(), 400) } else if data != nil { json.NewEncoder(w).Encode(data) //nolint:errcheck // checked in previous step } @@ -66,18 +62,14 @@ func ToByteStream(handler JSONResponderF) ReqRespHandlerf { ctx := r.Context() data, err := handler(ctx, r) if err != nil { - statusCode := 400 if cerr, ok := err.(*Error); ok { w.Header().Set(AppErrorHeader, cerr.Code) - if cerr.StatusCode != 0 { - statusCode = cerr.StatusCode - } } if data != nil { responseString, _ := json.Marshal(data) - http.Error(w, string(responseString), statusCode) + http.Error(w, string(responseString), 400) } else { - http.Error(w, err.Error(), statusCode) + http.Error(w, err.Error(), 400) } } else if data != nil { rawdata, ok := data.([]byte) @@ -158,3 +150,57 @@ func JSONString(json map[string]interface{}, field string, required bool) (strin return fmt.Sprintf("%v", sval), nil } } + +type StatusCodeResponderF func(ctx context.Context, r *http.Request) (interface{}, int, error) + +func ToStatusCode(handler StatusCodeResponderF) ReqRespHandlerf { + return func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Access-Control-Allow-Origin", "*") // CORS for all. + if r.Method == "OPTIONS" { + SetupCORSResponse(w, r) + return + } + + ctx := r.Context() + + data, statusCode, err := handler(ctx, r) + + if err != nil { + if statusCode == 0 { + statusCode = http.StatusBadRequest + } + + w.WriteHeader(statusCode) + w.Header().Set("Content-Type", "application/json") + + if data != nil { + json.NewEncoder(w).Encode(data) //nolint:errcheck + } else { + //nolint:errcheck + json.NewEncoder(w).Encode(map[string]string{ + "error": err.Error(), + }) + } + + return + } + + if statusCode == 0 { + statusCode = http.StatusOK + } + + w.WriteHeader(statusCode) + + if data != nil { + + rawdata, ok := data.([]byte) + if ok { + w.Header().Set("Content-Type", "application/octet-stream") + w.Write(rawdata) //nolint:errcheck + } else { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(data) //nolint:errcheck + } + } + } +}