From 6767bfa2d2b15d4a666ae35702e4c0f44ac7910b Mon Sep 17 00:00:00 2001 From: Prakash Senthil Vel <23444145+prakashsvmx@users.noreply.github.com> Date: Thu, 7 Dec 2023 03:58:46 +0530 Subject: [PATCH] add encoded filename as part of delete api url (#3141) --- integration/user_api_bucket_test.go | 9 +++-- portal-ui/src/api/consoleApi.ts | 2 +- .../ListObjects/DeleteMultipleObjects.tsx | 40 +++++++++++++++---- .../Objects/ListObjects/DeleteNonCurrent.tsx | 2 +- .../Objects/ListObjects/DeleteObject.tsx | 2 +- .../permissions-3/bucketDeleteAllVersions.ts | 12 ++---- restapi/embedded_spec.go | 4 +- .../object/delete_object_parameters.go | 16 ++++---- .../object/delete_object_urlbuilder.go | 8 ++-- restapi/user_objects.go | 7 +++- swagger.yml | 2 +- 11 files changed, 65 insertions(+), 39 deletions(-) diff --git a/integration/user_api_bucket_test.go b/integration/user_api_bucket_test.go index daa44672a5..4e7ce88c06 100644 --- a/integration/user_api_bucket_test.go +++ b/integration/user_api_bucket_test.go @@ -423,8 +423,9 @@ func DeleteObject(bucketName, path string, recursive, allVersions bool) (*http.R DELETE: {{baseUrl}}/buckets/bucketName/objects?path=Y2VzYXJpby50eHQ=&recursive=false&all_versions=false */ - url := "http://localhost:9090/api/v1/buckets/" + bucketName + "/objects?path=" + - path + "&recursive=" + strconv.FormatBool(recursive) + "&all_versions=" + + prefixEncoded := base64.StdEncoding.EncodeToString([]byte(path)) + url := "http://localhost:9090/api/v1/buckets/" + bucketName + "/objects?prefix=" + + prefixEncoded + "&recursive=" + strconv.FormatBool(recursive) + "&all_versions=" + strconv.FormatBool(allVersions) request, err := http.NewRequest( "DELETE", @@ -1639,7 +1640,6 @@ func TestDeleteObject(t *testing.T) { assert := assert.New(t) bucketName := "testdeleteobjectbucket1" fileName := "testdeleteobjectfile" - path := "dGVzdGRlbGV0ZW9iamVjdGZpbGUxLnR4dA==" // fileName encoded base64 numberOfFiles := 2 // 1. Create bucket @@ -1662,8 +1662,9 @@ func TestDeleteObject(t *testing.T) { } } + objPathFull := fileName + "1.txt" // would be encoded in DeleteObject util method. // 3. Delete only one object from the bucket. - deleteResponse, deleteError := DeleteObject(bucketName, path, false, false) + deleteResponse, deleteError := DeleteObject(bucketName, objPathFull, false, false) assert.Nil(deleteError) if deleteError != nil { log.Println(deleteError) diff --git a/portal-ui/src/api/consoleApi.ts b/portal-ui/src/api/consoleApi.ts index 9937cb28a3..6121c177d2 100644 --- a/portal-ui/src/api/consoleApi.ts +++ b/portal-ui/src/api/consoleApi.ts @@ -2092,7 +2092,7 @@ export class Api< deleteObject: ( bucketName: string, query: { - path: string; + prefix: string; version_id?: string; recursive?: boolean; all_versions?: boolean; diff --git a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteMultipleObjects.tsx b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteMultipleObjects.tsx index af09467e1e..fd57c1bf20 100644 --- a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteMultipleObjects.tsx +++ b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteMultipleObjects.tsx @@ -25,6 +25,8 @@ import { hasPermission } from "../../../../../../common/SecureComponent"; import { IAM_SCOPES } from "../../../../../../common/SecureComponent/permissions"; import { useSelector } from "react-redux"; import { BucketVersioningResponse } from "api/consoleApi"; +import { api } from "../../../../../../api"; +import { encodeURLString } from "../../../../../../common/utils"; interface IDeleteObjectProps { closeDeleteModalAndRefresh: (refresh: boolean) => void; @@ -86,13 +88,37 @@ const DeleteObject = ({ } if (toSend) { - invokeDeleteApi( - "POST", - `/api/v1/buckets/${selectedBucket}/delete-objects?all_versions=${deleteVersions}${ - bypassGovernance ? "&bypass=true" : "" - }`, - toSend, - ); + if (selectedObjects.length === 1) { + const firstObject = selectedObjects[0]; + api.buckets + .deleteObject(selectedBucket, { + prefix: encodeURLString(firstObject), + all_versions: deleteVersions, + bypass: bypassGovernance, + recursive: firstObject.endsWith("/"), //if it is just a prefix + }) + .then(onDelSuccess) + .catch((err) => { + dispatch( + setErrorSnackMessage({ + errorMessage: `Could not delete object. ${err.statusText}. ${ + retentionConfig + ? "Please check retention mode and if object is WORM protected." + : "" + }`, + detailedError: "", + }), + ); + }); + } else { + invokeDeleteApi( + "POST", + `/api/v1/buckets/${selectedBucket}/delete-objects?all_versions=${deleteVersions}${ + bypassGovernance ? "&bypass=true" : "" + }`, + toSend, + ); + } } }; diff --git a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteNonCurrent.tsx b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteNonCurrent.tsx index f79573df74..a56b60be9d 100644 --- a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteNonCurrent.tsx +++ b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteNonCurrent.tsx @@ -59,7 +59,7 @@ const DeleteNonCurrentVersions = ({ if (deleteLoading) { api.buckets .deleteObject(selectedBucket, { - path: selectedObject, + prefix: selectedObject, non_current_versions: true, bypass: bypassGovernance, }) diff --git a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteObject.tsx b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteObject.tsx index b70a0e9f68..d5e371e486 100644 --- a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteObject.tsx +++ b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/DeleteObject.tsx @@ -80,7 +80,7 @@ const DeleteObject = ({ const recursive = decodedSelectedObject.endsWith("/"); invokeDeleteApi( "DELETE", - `/api/v1/buckets/${selectedBucket}/objects?path=${selectedObject}${ + `/api/v1/buckets/${selectedBucket}/objects?prefix=${selectedObject}${ selectedVersion !== "" ? `&version_id=${selectedVersion}` : `&recursive=${recursive}&all_versions=${deleteVersions}` diff --git a/portal-ui/tests/permissions-3/bucketDeleteAllVersions.ts b/portal-ui/tests/permissions-3/bucketDeleteAllVersions.ts index 1a44bc4567..76141a9903 100644 --- a/portal-ui/tests/permissions-3/bucketDeleteAllVersions.ts +++ b/portal-ui/tests/permissions-3/bucketDeleteAllVersions.ts @@ -17,11 +17,8 @@ import * as roles from "../utils/roles"; import * as elements from "../utils/elements"; import * as functions from "../utils/functions"; -import { bucketsElement } from "../utils/elements-menu"; import { testBucketBrowseButtonFor } from "../utils/functions"; import { Selector } from "testcafe"; -import * as constants from "../utils/constants"; -import { deleteAllVersions } from "../utils/elements"; fixture("For user with Bucket Read & Write permissions").page( "http://localhost:9090", @@ -45,6 +42,9 @@ test .setFilesToUpload(elements.uploadInput, "../uploads/test.txt") .wait(1000); })("All versions of an object can be deleted from a bucket", async (t) => { + const versionRows = Selector( + "div.ReactVirtualized__Grid.ReactVirtualized__Table__Grid > div > div:nth-child(1)", + ); await t .useRole(roles.bucketReadWrite) .navigateTo("http://localhost:9090/browser") @@ -55,11 +55,7 @@ test .click(elements.deleteButton) .click(elements.deleteAllVersions) .click(Selector("button:enabled").withExactText("Delete").nth(1)) - .expect( - Selector( - "div.ReactVirtualized__Grid.ReactVirtualized__Table__Grid > div > div:nth-child(1)", - ).exists, - ) + .expect(versionRows.exists) .notOk(); }) .after(async (t) => { diff --git a/restapi/embedded_spec.go b/restapi/embedded_spec.go index bc0393e98a..bec6a42ea0 100644 --- a/restapi/embedded_spec.go +++ b/restapi/embedded_spec.go @@ -1534,7 +1534,7 @@ func init() { }, { "type": "string", - "name": "path", + "name": "prefix", "in": "query", "required": true }, @@ -10684,7 +10684,7 @@ func init() { }, { "type": "string", - "name": "path", + "name": "prefix", "in": "query", "required": true }, diff --git a/restapi/operations/object/delete_object_parameters.go b/restapi/operations/object/delete_object_parameters.go index 39f2acf14f..d6d99701af 100644 --- a/restapi/operations/object/delete_object_parameters.go +++ b/restapi/operations/object/delete_object_parameters.go @@ -71,7 +71,7 @@ type DeleteObjectParams struct { Required: true In: query */ - Path string + Prefix string /* In: query */ @@ -113,8 +113,8 @@ func (o *DeleteObjectParams) BindRequest(r *http.Request, route *middleware.Matc res = append(res, err) } - qPath, qhkPath, _ := qs.GetOK("path") - if err := o.bindPath(qPath, qhkPath, route.Formats); err != nil { + qPrefix, qhkPrefix, _ := qs.GetOK("prefix") + if err := o.bindPrefix(qPrefix, qhkPrefix, route.Formats); err != nil { res = append(res, err) } @@ -216,10 +216,10 @@ func (o *DeleteObjectParams) bindNonCurrentVersions(rawData []string, hasKey boo return nil } -// bindPath binds and validates parameter Path from query. -func (o *DeleteObjectParams) bindPath(rawData []string, hasKey bool, formats strfmt.Registry) error { +// bindPrefix binds and validates parameter Prefix from query. +func (o *DeleteObjectParams) bindPrefix(rawData []string, hasKey bool, formats strfmt.Registry) error { if !hasKey { - return errors.Required("path", "query", rawData) + return errors.Required("prefix", "query", rawData) } var raw string if len(rawData) > 0 { @@ -229,10 +229,10 @@ func (o *DeleteObjectParams) bindPath(rawData []string, hasKey bool, formats str // Required: true // AllowEmptyValue: false - if err := validate.RequiredString("path", "query", raw); err != nil { + if err := validate.RequiredString("prefix", "query", raw); err != nil { return err } - o.Path = raw + o.Prefix = raw return nil } diff --git a/restapi/operations/object/delete_object_urlbuilder.go b/restapi/operations/object/delete_object_urlbuilder.go index b20ede9563..93bb65231b 100644 --- a/restapi/operations/object/delete_object_urlbuilder.go +++ b/restapi/operations/object/delete_object_urlbuilder.go @@ -38,7 +38,7 @@ type DeleteObjectURL struct { AllVersions *bool Bypass *bool NonCurrentVersions *bool - Path string + Prefix string Recursive *bool VersionID *string @@ -107,9 +107,9 @@ func (o *DeleteObjectURL) Build() (*url.URL, error) { qs.Set("non_current_versions", nonCurrentVersionsQ) } - pathQ := o.Path - if pathQ != "" { - qs.Set("path", pathQ) + prefixQ := o.Prefix + if prefixQ != "" { + qs.Set("prefix", prefixQ) } var recursiveQ string diff --git a/restapi/user_objects.go b/restapi/user_objects.go index eca470a917..e1420349d4 100644 --- a/restapi/user_objects.go +++ b/restapi/user_objects.go @@ -63,6 +63,7 @@ func registerObjectsHandlers(api *operations.ConsoleAPI) { }) // delete object api.ObjectDeleteObjectHandler = objectApi.DeleteObjectHandlerFunc(func(params objectApi.DeleteObjectParams, session *models.Principal) middleware.Responder { + fmt.Println("ObjectDeleteObjectHandler", params.Prefix) if err := getDeleteObjectResponse(session, params); err != nil { return objectApi.NewDeleteObjectDefault(err.Code).WithPayload(err.APIError) } @@ -70,6 +71,8 @@ func registerObjectsHandlers(api *operations.ConsoleAPI) { }) // delete multiple objects api.ObjectDeleteMultipleObjectsHandler = objectApi.DeleteMultipleObjectsHandlerFunc(func(params objectApi.DeleteMultipleObjectsParams, session *models.Principal) middleware.Responder { + fmt.Println("ObjectDeleteMultipleObjectsHandler", params) + if err := getDeleteMultiplePathsResponse(session, params); err != nil { return objectApi.NewDeleteMultipleObjectsDefault(err.Code).WithPayload(err.APIError) } @@ -764,8 +767,8 @@ func getDeleteObjectResponse(session *models.Principal, params objectApi.DeleteO ctx, cancel := context.WithCancel(params.HTTPRequest.Context()) defer cancel() var prefix string - if params.Path != "" { - encodedPrefix := SanitizeEncodedPrefix(params.Path) + if params.Prefix != "" { + encodedPrefix := SanitizeEncodedPrefix(params.Prefix) decodedPrefix, err := base64.StdEncoding.DecodeString(encodedPrefix) if err != nil { return ErrorWithContext(ctx, err) diff --git a/swagger.yml b/swagger.yml index 9dfcb47e1e..ad2bfa1830 100644 --- a/swagger.yml +++ b/swagger.yml @@ -339,7 +339,7 @@ paths: in: path required: true type: string - - name: path + - name: prefix in: query required: true type: string