From 688d55488ccebd2e3a97a0e4570c3bc8bbdceb8f Mon Sep 17 00:00:00 2001 From: shichanglin5 Date: Sat, 14 May 2022 10:40:29 +0800 Subject: [PATCH] test(s3api_object_copy_handlers_test.go): some unit tests have been added to the processMetadata & processMetadataBytes methods of s3api_object_copy_handlers.go --- weed/s3api/s3api_object_copy_handlers.go | 37 +- weed/s3api/s3api_object_copy_handlers_test.go | 426 ++++++++++++++++++ 2 files changed, 441 insertions(+), 22 deletions(-) create mode 100644 weed/s3api/s3api_object_copy_handlers_test.go diff --git a/weed/s3api/s3api_object_copy_handlers.go b/weed/s3api/s3api_object_copy_handlers.go index ff1329ba4..c44ca7ddf 100644 --- a/weed/s3api/s3api_object_copy_handlers.go +++ b/weed/s3api/s3api_object_copy_handlers.go @@ -16,6 +16,11 @@ import ( "github.com/chrislusf/seaweedfs/weed/util" ) +const ( + DirectiveCopy = "COPY" + DirectiveReplace = "REPLACE" +) + func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request) { dstBucket, dstObject := xhttp.GetBucketAndObject(r) @@ -31,7 +36,7 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request glog.V(3).Infof("CopyObjectHandler %s %s => %s %s", srcBucket, srcObject, dstBucket, dstObject) - replaceMeta, replaceTagging := replaceDirective(r) + replaceMeta, replaceTagging := replaceDirective(r.Header) if (srcBucket == dstBucket && srcObject == dstObject || cpSrcPath == "") && (replaceMeta || replaceTagging) { fullPath := util.FullPath(fmt.Sprintf("%s/%s%s", s3a.option.BucketsPath, dstBucket, dstObject)) @@ -190,8 +195,8 @@ func (s3a *S3ApiServer) CopyObjectPartHandler(w http.ResponseWriter, r *http.Req } -func replaceDirective(r *http.Request) (replaceMeta, replaceTagging bool) { - return r.Header.Get(headers.AmzUserMetaDirective) == "REPLACE", r.Header.Get(headers.AmzObjectTaggingDirective) == "REPLACE" +func replaceDirective(reqHeader http.Header) (replaceMeta, replaceTagging bool) { + return reqHeader.Get(headers.AmzUserMetaDirective) == DirectiveReplace, reqHeader.Get(headers.AmzObjectTaggingDirective) == DirectiveReplace } func processMetadata(reqHeader, existing http.Header, replaceMeta, replaceTagging bool, getTags func(parentDirectoryPath string, entryName string) (tags map[string]string, err error), dir, name string) (err error) { @@ -214,19 +219,7 @@ func processMetadata(reqHeader, existing http.Header, replaceMeta, replaceTaggin } } - if replaceTagging { - if tags := reqHeader.Get(xhttp.AmzObjectTagging); tags != "" { - for _, v := range strings.Split(tags, "&") { - tag := strings.Split(v, "=") - if len(tag) == 2 { - reqHeader[xhttp.AmzObjectTagging+"-"+tag[0]] = []string{tag[1]} - } else if len(tag) == 1 { - reqHeader[xhttp.AmzObjectTagging+"-"+tag[0]] = []string{} - } - } - } - delete(reqHeader, xhttp.AmzObjectTagging) - } else { + if !replaceTagging { for header, _ := range reqHeader { if strings.HasPrefix(header, xhttp.AmzObjectTagging) { delete(reqHeader, header) @@ -269,12 +262,6 @@ func processMetadataBytes(reqHeader http.Header, existing map[string][]byte, rep } if replaceMeta { - for k, v := range existing { - if strings.HasPrefix(k, xhttp.AmzUserMetaPrefix) { - metadata[k] = v - } - } - } else { for header, values := range reqHeader { if strings.HasPrefix(header, xhttp.AmzUserMetaPrefix) { for _, value := range values { @@ -282,6 +269,12 @@ func processMetadataBytes(reqHeader http.Header, existing map[string][]byte, rep } } } + } else { + for k, v := range existing { + if strings.HasPrefix(k, xhttp.AmzUserMetaPrefix) { + metadata[k] = v + } + } } if replaceTagging { diff --git a/weed/s3api/s3api_object_copy_handlers_test.go b/weed/s3api/s3api_object_copy_handlers_test.go new file mode 100644 index 000000000..d2c8e488b --- /dev/null +++ b/weed/s3api/s3api_object_copy_handlers_test.go @@ -0,0 +1,426 @@ +package s3api + +import ( + "fmt" + headers "github.com/chrislusf/seaweedfs/weed/s3api/http" + "net/http" + "reflect" + "sort" + "strings" + "testing" +) + +type H map[string]string + +func (h H) String() string { + pairs := make([]string, 0, len(h)) + for k, v := range h { + pairs = append(pairs, fmt.Sprintf("%s : %s", k, v)) + } + sort.Strings(pairs) + join := strings.Join(pairs, "\n") + return "\n" + join + "\n" +} + +var processMetadataTestCases = []struct { + caseId int + request H + existing H + getTags H + want H +}{ + { + 201, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-Type": "existing", + }, + H{ + "A": "B", + "a": "b", + "type": "existing", + }, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging": "A=B&a=b&type=existing", + }, + }, + { + 202, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzUserMetaDirective: DirectiveReplace, + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-Type": "existing", + }, + H{ + "A": "B", + "a": "b", + "type": "existing", + }, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=existing", + headers.AmzUserMetaDirective: DirectiveReplace, + }, + }, + + { + 203, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-Type": "existing", + }, + H{ + "A": "B", + "a": "b", + "type": "existing", + }, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + }, + + { + 204, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-Type": "existing", + }, + H{ + "A": "B", + "a": "b", + "type": "existing", + }, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + }, + + { + 205, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + H{}, + H{}, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + }, + + { + 206, + H{ + "User-Agent": "firefox", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-Type": "existing", + }, + H{ + "A": "B", + "a": "b", + "type": "existing", + }, + H{ + "User-Agent": "firefox", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + }, + + { + 207, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-Type": "existing", + }, + H{ + "A": "B", + "a": "b", + "type": "existing", + }, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + }, +} +var processMetadataBytesTestCases = []struct { + caseId int + request H + existing H + want H +}{ + { + 101, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-type": "existing", + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-type": "existing", + }, + }, + + { + 102, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzUserMetaDirective: DirectiveReplace, + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-type": "existing", + }, + H{ + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-type": "existing", + }, + }, + + { + 103, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-type": "existing", + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-type": "request", + }, + }, + + { + 104, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-type": "existing", + }, + H{ + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-type": "request", + }, + }, + + { + 105, + H{ + "User-Agent": "firefox", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + H{ + "X-Amz-Meta-My-Meta": "existing", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-type": "existing", + }, + H{}, + }, + + { + 107, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request", + headers.AmzUserMetaDirective: DirectiveReplace, + headers.AmzObjectTaggingDirective: DirectiveReplace, + }, + H{}, + H{ + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging-A": "B", + "X-Amz-Tagging-a": "b", + "X-Amz-Tagging-type": "request", + }, + }, +} + +func TestProcessMetadata(t *testing.T) { + for _, tc := range processMetadataTestCases { + reqHeader := transferHToHeader(tc.request) + existing := transferHToHeader(tc.existing) + replaceMeta, replaceTagging := replaceDirective(reqHeader) + + err := processMetadata(reqHeader, existing, replaceMeta, replaceTagging, func(_ string, _ string) (tags map[string]string, err error) { + return tc.getTags, nil + }, "", "") + if err != nil { + t.Error(err) + } + + result := transferHeaderToH(reqHeader) + fmtTagging(result, tc.want) + + if !reflect.DeepEqual(result, tc.want) { + t.Error(fmt.Errorf("\n### CaseID: %d ###"+ + "\nRequest:%v"+ + "\nExisting:%v"+ + "\nGetTags:%v"+ + "\nWant:%v"+ + "\nActual:%v", + tc.caseId, tc.request, tc.existing, tc.getTags, tc.want, result)) + } + } +} + +func TestProcessMetadataBytes(t *testing.T) { + for _, tc := range processMetadataBytesTestCases { + reqHeader := transferHToHeader(tc.request) + existing := transferHToBytesArr(tc.existing) + replaceMeta, replaceTagging := replaceDirective(reqHeader) + extends := processMetadataBytes(reqHeader, existing, replaceMeta, replaceTagging) + + result := transferBytesArrToH(extends) + fmtTagging(result, tc.want) + + if !reflect.DeepEqual(result, tc.want) { + t.Error(fmt.Errorf("\n### CaseID: %d ###"+ + "\nRequest:%v"+ + "\nExisting:%v"+ + "\nWant:%v"+ + "\nActual:%v", + tc.caseId, tc.request, tc.existing, tc.want, result)) + } + } +} + +func fmtTagging(maps ...map[string]string) { + for _, m := range maps { + if tagging := m[headers.AmzObjectTagging]; len(tagging) > 0 { + split := strings.Split(tagging, "&") + sort.Strings(split) + m[headers.AmzObjectTagging] = strings.Join(split, "&") + } + } +} + +func transferHToHeader(data map[string]string) http.Header { + header := http.Header{} + for k, v := range data { + header.Add(k, v) + } + return header +} + +func transferHToBytesArr(data map[string]string) map[string][]byte { + m := make(map[string][]byte, len(data)) + for k, v := range data { + m[k] = []byte(v) + } + return m +} + +func transferBytesArrToH(data map[string][]byte) H { + m := make(map[string]string, len(data)) + for k, v := range data { + m[k] = string(v) + } + return m +} + +func transferHeaderToH(data map[string][]string) H { + m := make(map[string]string, len(data)) + for k, v := range data { + m[k] = v[len(v)-1] + } + return m +}