From 4f5f5fb5c832ba122b71dae53a03867c9469cea0 Mon Sep 17 00:00:00 2001
From: Denis Kirillov <d.kirillov@yadro.com>
Date: Tue, 17 Oct 2023 11:20:37 +0300
Subject: [PATCH] [#222] Fix marshaling errors in `DeleteObjects` method

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
---
 CHANGELOG.md               |  1 +
 api/handler/delete.go      |  8 +++---
 api/handler/delete_test.go | 54 +++++++++++++++++++++++++++++++++-----
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f4e5bad..8f7f34c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,7 @@ This document outlines major changes between releases.
 - Fix url escaping (#188)
 - Use correct keys in `list-multipart-uploads` response (#185)
 - Fix parsing `key-marker` for object list versions (#243)
+- Fix marshaling errors in `DeleteObjects` method (#222)
 
 ### Added
 - Add `trace_id` value into log record when tracing is enabled (#142)
diff --git a/api/handler/delete.go b/api/handler/delete.go
index 3224e6e..326b7ad 100644
--- a/api/handler/delete.go
+++ b/api/handler/delete.go
@@ -46,10 +46,10 @@ type DeletedObject struct {
 
 // DeleteError structure.
 type DeleteError struct {
-	Code      string
-	Message   string
-	Key       string
-	VersionID string `xml:"versionId,omitempty"`
+	Code      string `xml:"Code,omitempty"`
+	Message   string `xml:"Message,omitempty"`
+	Key       string `xml:"Key,omitempty"`
+	VersionID string `xml:"VersionId,omitempty"`
 }
 
 // DeleteObjectsResponse container for multiple object deletes.
diff --git a/api/handler/delete_test.go b/api/handler/delete_test.go
index bae3005..bef7b31 100644
--- a/api/handler/delete_test.go
+++ b/api/handler/delete_test.go
@@ -2,6 +2,7 @@ package handler
 
 import (
 	"bytes"
+	"encoding/xml"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -12,6 +13,9 @@ import (
 	apiErrors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors"
 	apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status"
 	oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/private/protocol/xml/xmlutil"
+	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/stretchr/testify/require"
 )
 
@@ -80,6 +84,38 @@ func TestDeleteBucketOnNotFoundError(t *testing.T) {
 	deleteBucket(t, hc, bktName, http.StatusNoContent)
 }
 
+func TestDeleteObjectsError(t *testing.T) {
+	hc := prepareHandlerContext(t)
+
+	bktName, objName := "bucket-for-removal", "object-to-delete"
+	bktInfo := createTestBucket(hc, bktName)
+	putBucketVersioning(t, hc, bktName, true)
+
+	putObject(hc, bktName, objName)
+
+	nodeVersion, err := hc.tree.GetLatestVersion(hc.context, bktInfo, objName)
+	require.NoError(t, err)
+	var addr oid.Address
+	addr.SetContainer(bktInfo.CID)
+	addr.SetObject(nodeVersion.OID)
+
+	expectedError := apiErrors.GetAPIError(apiErrors.ErrAccessDenied)
+	hc.tp.SetObjectError(addr, expectedError)
+
+	w := deleteObjectsBase(hc, bktName, [][2]string{{objName, nodeVersion.OID.EncodeToString()}})
+
+	res := &s3.DeleteObjectsOutput{}
+	err = xmlutil.UnmarshalXML(res, xml.NewDecoder(w.Result().Body), "")
+	require.NoError(t, err)
+
+	require.ElementsMatch(t, []*s3.Error{{
+		Code:      aws.String(expectedError.Code),
+		Key:       aws.String(objName),
+		Message:   aws.String(expectedError.Error()),
+		VersionId: aws.String(nodeVersion.OID.EncodeToString()),
+	}}, res.Errors)
+}
+
 func TestDeleteObject(t *testing.T) {
 	tc := prepareHandlerContext(t)
 
@@ -408,6 +444,14 @@ func deleteObject(t *testing.T, tc *handlerContext, bktName, objName, version st
 }
 
 func deleteObjects(t *testing.T, tc *handlerContext, bktName string, objVersions [][2]string) *DeleteObjectsResponse {
+	w := deleteObjectsBase(tc, bktName, objVersions)
+
+	res := &DeleteObjectsResponse{}
+	parseTestResponse(t, w, res)
+	return res
+}
+
+func deleteObjectsBase(hc *handlerContext, bktName string, objVersions [][2]string) *httptest.ResponseRecorder {
 	req := &DeleteObjectsRequest{}
 	for _, version := range objVersions {
 		req.Objects = append(req.Objects, ObjectIdentifier{
@@ -416,14 +460,12 @@ func deleteObjects(t *testing.T, tc *handlerContext, bktName string, objVersions
 		})
 	}
 
-	w, r := prepareTestRequest(tc, bktName, "", req)
+	w, r := prepareTestRequest(hc, bktName, "", req)
 	r.Header.Set(api.ContentMD5, "")
-	tc.Handler().DeleteMultipleObjectsHandler(w, r)
-	assertStatus(t, w, http.StatusOK)
+	hc.Handler().DeleteMultipleObjectsHandler(w, r)
+	assertStatus(hc.t, w, http.StatusOK)
 
-	res := &DeleteObjectsResponse{}
-	parseTestResponse(t, w, res)
-	return res
+	return w
 }
 
 func deleteBucket(t *testing.T, tc *handlerContext, bktName string, code int) {