From adf74ffdb090e8915fd319e5da90b5766b2667d4 Mon Sep 17 00:00:00 2001 From: Brendan Ashworth Date: Sun, 6 Mar 2016 12:16:22 -0800 Subject: [PATCH] api: DRY code and add new test This commit makes code cleaner and reduces the repetitions in the code base. Specifically, it reduces the clutter in setObjectHeaders. It also merges encodeSuccessResponse and encodeErrorResponse together because they served no purpose differently. Finally, it adds a simple test for generateRequestID. --- api-headers.go | 39 ++++++++++++--------------------------- api-headers_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ api-response.go | 2 +- bucket-handlers.go | 12 ++++++------ object-handlers.go | 8 ++++---- 5 files changed, 63 insertions(+), 38 deletions(-) create mode 100644 api-headers_test.go diff --git a/api-headers.go b/api-headers.go index 03d2047cf..2e128f389 100644 --- a/api-headers.go +++ b/api-headers.go @@ -50,10 +50,9 @@ func setCommonHeaders(w http.ResponseWriter) { w.Header().Set("Accept-Ranges", "bytes") } -// Write error response headers -func encodeErrorResponse(response interface{}) []byte { +// Encodes the response headers into XML format. +func encodeResponse(response interface{}) []byte { var bytesBuffer bytes.Buffer - // write common headers bytesBuffer.WriteString(xml.Header) e := xml.NewEncoder(&bytesBuffer) e.Encode(response) @@ -63,40 +62,26 @@ func encodeErrorResponse(response interface{}) []byte { // Write object header func setObjectHeaders(w http.ResponseWriter, metadata fs.ObjectMetadata, contentRange *httpRange) { // set common headers - if contentRange != nil { - if contentRange.length > 0 { - w.Header().Set("Content-Length", strconv.FormatInt(contentRange.length, 10)) - setCommonHeaders(w) - } else { - w.Header().Set("Content-Length", strconv.FormatInt(metadata.Size, 10)) - setCommonHeaders(w) - } - } else { - w.Header().Set("Content-Length", strconv.FormatInt(metadata.Size, 10)) - setCommonHeaders(w) - } - // set object headers + setCommonHeaders(w) + + // set object-related metadata headers lastModified := metadata.LastModified.UTC().Format(http.TimeFormat) - // object related headers + w.Header().Set("Last-Modified", lastModified) + w.Header().Set("Content-Type", metadata.ContentType) if metadata.MD5 != "" { w.Header().Set("ETag", "\""+metadata.MD5+"\"") } - w.Header().Set("Last-Modified", lastModified) - // set content range + w.Header().Set("Content-Length", strconv.FormatInt(metadata.Size, 10)) + + // for providing ranged content if contentRange != nil { if contentRange.start > 0 || contentRange.length > 0 { + // override content-length + w.Header().Set("Content-Length", strconv.FormatInt(contentRange.length, 10)) w.Header().Set("Content-Range", contentRange.String()) w.WriteHeader(http.StatusPartialContent) } } } - -func encodeSuccessResponse(response interface{}) []byte { - var bytesBuffer bytes.Buffer - bytesBuffer.WriteString(xml.Header) - e := xml.NewEncoder(&bytesBuffer) - e.Encode(response) - return bytesBuffer.Bytes() -} diff --git a/api-headers_test.go b/api-headers_test.go new file mode 100644 index 000000000..847e70e9a --- /dev/null +++ b/api-headers_test.go @@ -0,0 +1,40 @@ +/* + * Minio Cloud Storage, (C) 2015, 2016 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package main + +import ( + "testing" +) + +func TestGenerateRequestID(t *testing.T) { + // Ensure that it returns an alphanumeric result of length 16. + var id []byte = generateRequestID() + + if len(id) != 16 { + t.Fail() + } + + var e rune + for _, char := range id { + e = rune(char) + + // Ensure that it is alphanumeric, in this case, between 0-9 and A-Z. + if !(('0' <= e && e <= '9') || ('A' <= e && e <= 'Z')) { + t.Fail() + } + } +} diff --git a/api-response.go b/api-response.go index 463917885..9114b550a 100644 --- a/api-response.go +++ b/api-response.go @@ -436,7 +436,7 @@ func writeErrorResponse(w http.ResponseWriter, req *http.Request, errorType int, error := getErrorCode(errorType) // generate error response errorResponse := getErrorResponse(error, resource) - encodedErrorResponse := encodeErrorResponse(errorResponse) + encodedErrorResponse := encodeResponse(errorResponse) // set common headers setCommonHeaders(w) // write Header diff --git a/bucket-handlers.go b/bucket-handlers.go index 15043fe01..3bcdb3f87 100644 --- a/bucket-handlers.go +++ b/bucket-handlers.go @@ -63,9 +63,9 @@ func (api storageAPI) GetBucketLocationHandler(w http.ResponseWriter, r *http.Re } // Generate response. - encodedSuccessResponse := encodeSuccessResponse(LocationResponse{}) + encodedSuccessResponse := encodeResponse(LocationResponse{}) if api.Region != "us-east-1" { - encodedSuccessResponse = encodeSuccessResponse(LocationResponse{ + encodedSuccessResponse = encodeResponse(LocationResponse{ Location: api.Region, }) } @@ -117,7 +117,7 @@ func (api storageAPI) ListMultipartUploadsHandler(w http.ResponseWriter, r *http } // generate response response := generateListMultipartUploadsResponse(bucket, resources) - encodedSuccessResponse := encodeSuccessResponse(response) + encodedSuccessResponse := encodeResponse(response) // write headers. setCommonHeaders(w) // write success response. @@ -160,7 +160,7 @@ func (api storageAPI) ListObjectsHandler(w http.ResponseWriter, r *http.Request) if err == nil { // generate response response := generateListObjectsResponse(bucket, prefix, marker, delimiter, maxkeys, listResp) - encodedSuccessResponse := encodeSuccessResponse(response) + encodedSuccessResponse := encodeResponse(response) // Write headers setCommonHeaders(w) // Write success response. @@ -201,7 +201,7 @@ func (api storageAPI) ListBucketsHandler(w http.ResponseWriter, r *http.Request) if err == nil { // generate response response := generateListBucketsResponse(buckets) - encodedSuccessResponse := encodeSuccessResponse(response) + encodedSuccessResponse := encodeResponse(response) // write headers setCommonHeaders(w) // write response @@ -475,7 +475,7 @@ func (api storageAPI) GetBucketACLHandler(w http.ResponseWriter, r *http.Request } // Generate response response := generateAccessControlPolicyResponse(bucketMetadata.ACL) - encodedSuccessResponse := encodeSuccessResponse(response) + encodedSuccessResponse := encodeResponse(response) // Write headers setCommonHeaders(w) // Write success response. diff --git a/object-handlers.go b/object-handlers.go index 3e6958f21..61257280d 100644 --- a/object-handlers.go +++ b/object-handlers.go @@ -392,7 +392,7 @@ func (api storageAPI) CopyObjectHandler(w http.ResponseWriter, r *http.Request) return } response := generateCopyObjectResponse(metadata.MD5, metadata.LastModified) - encodedSuccessResponse := encodeSuccessResponse(response) + encodedSuccessResponse := encodeResponse(response) // write headers setCommonHeaders(w) // write success response. @@ -534,7 +534,7 @@ func (api storageAPI) NewMultipartUploadHandler(w http.ResponseWriter, r *http.R } response := generateInitiateMultipartUploadResponse(bucket, object, uploadID) - encodedSuccessResponse := encodeSuccessResponse(response) + encodedSuccessResponse := encodeResponse(response) // write headers setCommonHeaders(w) // write success response. @@ -723,7 +723,7 @@ func (api storageAPI) ListObjectPartsHandler(w http.ResponseWriter, r *http.Requ return } response := generateListPartsResponse(objectResourcesMetadata) - encodedSuccessResponse := encodeSuccessResponse(response) + encodedSuccessResponse := encodeResponse(response) // write headers. setCommonHeaders(w) // write success response. @@ -802,7 +802,7 @@ func (api storageAPI) CompleteMultipartUploadHandler(w http.ResponseWriter, r *h location := getLocation(r) // Generate complete multipart response. response := generateCompleteMultpartUploadResponse(bucket, object, location, metadata.MD5) - encodedSuccessResponse := encodeSuccessResponse(response) + encodedSuccessResponse := encodeResponse(response) // write headers setCommonHeaders(w) // write success response.