create bytes decoder (#328)

Author: informalict

Makefile

@@ -146,7 +146,7 @@ run-tests: run-unit-tests run-tests-single run-tests-resilientsingle run-tests-c
 # The below rule exists only for backward compatibility.
 run-tests-http: run-unit-tests
 
-run-unit-tests:
+run-unit-tests: run-v2-unit-tests
 	@$(DOCKER_CMD) \
 		--rm \
 		-v "${ROOTDIR}":/usr/code \
@@ -155,6 +155,15 @@ run-unit-tests:
 		$(GOIMAGE) \
 		go test $(TESTOPTIONS) $(REPOPATH)/http $(REPOPATH)/agency
 
+run-v2-unit-tests:
+	@$(DOCKER_CMD) \
+		--rm \
+		-v "${ROOTDIR}"/v2:/usr/code \
+		-e CGO_ENABLED=$(CGO_ENABLED) \
+		-w /usr/code/ \
+		$(GOIMAGE) \
+		go test $(TESTOPTIONS) $(REPOPATH)/v2/connection
+
 # Single server tests 
 run-tests-single: run-tests-single-json run-tests-single-vpack run-tests-single-vst-1.0 $(VST11_SINGLE_TESTS)
 

v2/connection/connection_http_bytes.go

@@ -0,0 +1,29 @@
+//
+// DISCLAIMER
+//
+// Copyright 2021 ArangoDB GmbH, Cologne, Germany
+//
+// 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.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+// Author Tomasz Mielech
+//
+
+package connection
+
+const (
+	PlainText              = "text/plain"
+	ApplicationOctetStream = "application/octet-stream"
+	ApplicationZip         = "application/zip"
+)

v2/connection/connection_http_internal.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2020 ArangoDB GmbH, Cologne, Germany
+// Copyright 2020-2021 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
 // Copyright holder is ArangoDB GmbH, Cologne, Germany
 //
 // Author Adam Janikowski
+// Author Tomasz Mielech
 //
 
 package connection
@@ -102,22 +103,21 @@ func (j *httpConnection) SetAuthentication(a Authentication) error {
 	return nil
 }
 
-func (j httpConnection) Decoder(content string) Decoder {
-	switch content {
-	case ApplicationVPack:
-		return getVPackDecoder()
-	case ApplicationJSON:
-		return getJsonDecoder()
-	default:
-		switch j.contentType {
-		case ApplicationVPack:
-			return getVPackDecoder()
-		case ApplicationJSON:
-			return getJsonDecoder()
-		default:
-			return getJsonDecoder()
-		}
+// Decoder returns the decoder according to the response content type or HTTP connection request content type.
+// If the content type is unknown then it returns default JSON decoder.
+func (j httpConnection) Decoder(contentType string) Decoder {
+	// First try to get decoder by the content type of the response.
+	if decoder := getDecoderByContentType(contentType); decoder != nil {
+		return decoder
+	}
+
+	// Next try to get decoder by the content type of the HTTP connection.
+	if decoder := getDecoderByContentType(j.contentType); decoder != nil {
+		return decoder
 	}
+
+	// Return the default decoder.
+	return getJsonDecoder()
 }
 
 func (j httpConnection) DoWithReader(ctx context.Context, request Request) (Response, io.ReadCloser, error) {
@@ -184,17 +184,16 @@ func (j httpConnection) doWithOutput(ctx context.Context, request *httpRequest,
 		return nil, err
 	}
 
-	if output != nil {
-		defer dropBodyData(body) // In case if there is data drop it all
+	// The body should be closed at the end of the function.
+	defer dropBodyData(body)
 
+	if output != nil {
+		// The output should be stored in the output variable.
 		if err = j.Decoder(resp.Content()).Decode(body, output); err != nil {
 			if err != io.EOF {
 				return nil, errors.WithStack(err)
 			}
 		}
-	} else {
-		// We still need to read data from request, but we can do this in background and ignore output
-		defer dropBodyData(body)
 	}
 
 	return resp, nil
@@ -222,6 +221,7 @@ func (j httpConnection) do(ctx context.Context, req *httpRequest) (*httpResponse
 		ctx = context.Background()
 	}
 
+	var bodyReader io.Reader
 	if req.Method() == http.MethodPost || req.Method() == http.MethodPut || req.Method() == http.MethodPatch {
 		decoder := j.Decoder(j.contentType)
 		if !j.streamSender {
@@ -230,12 +230,7 @@ func (j httpConnection) do(ctx context.Context, req *httpRequest) (*httpResponse
 				return nil, nil, err
 			}
 
-			r, err := req.asRequest(ctx, b)
-			if err != nil {
-				return nil, nil, errors.WithStack(err)
-			}
-
-			httpReq = r
+			bodyReader = b
 		} else {
 			reader, writer := io.Pipe()
 			go func() {
@@ -245,21 +240,16 @@ func (j httpConnection) do(ctx context.Context, req *httpRequest) (*httpResponse
 				}
 			}()
 
-			r, err := req.asRequest(ctx, reader)
-			if err != nil {
-				return nil, nil, errors.WithStack(err)
-			}
-
-			httpReq = r
+			bodyReader = reader
 		}
-	} else {
-		r, err := req.asRequest(ctx, nil)
-		if err != nil {
-			return nil, nil, errors.WithStack(err)
-		}
-		httpReq = r
 	}
 
+	r, err := req.asRequest(ctx, bodyReader)
+	if err != nil {
+		return nil, nil, errors.WithStack(err)
+	}
+	httpReq = r
+
 	resp, err := j.client.Do(httpReq)
 	if err != nil {
 		log.Debugf("(%s) Request failed: %s", id, err.Error())
@@ -276,3 +266,18 @@ func (j httpConnection) do(ctx context.Context, req *httpRequest) (*httpResponse
 
 	return &httpResponse{response: resp, request: req}, nil, nil
 }
+
+// getDecoderByContentType returns the decoder according to the content type.
+// If content type is unknown then nil is returned.
+func getDecoderByContentType(contentType string) Decoder {
+	switch contentType {
+	case ApplicationVPack:
+		return getVPackDecoder()
+	case ApplicationJSON:
+		return getJsonDecoder()
+	case PlainText, ApplicationOctetStream, ApplicationZip:
+		return getBytesDecoder()
+	default:
+		return nil
+	}
+}

v2/connection/connection_http_internal_test.go

@@ -0,0 +1,81 @@
+//
+// DISCLAIMER
+//
+// Copyright 2021 ArangoDB GmbH, Cologne, Germany
+//
+// 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.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+// Author Tomasz Mielech
+//
+
+package connection
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_httpConnection_Decoder(t *testing.T) {
+	tests := map[string]struct {
+		contentType string
+		conn        httpConnection
+		wantDecoder Decoder
+	}{
+		"JSON response decoder": {
+			contentType: ApplicationJSON,
+			wantDecoder: getJsonDecoder(),
+		},
+		"VPack response decoder": {
+			contentType: ApplicationVPack,
+			wantDecoder: getVPackDecoder(),
+		},
+		"Bytes response decoder": {
+			contentType: PlainText,
+			wantDecoder: getBytesDecoder(),
+		},
+		"JSON HTTP connection decoder": {
+			conn: httpConnection{
+				contentType: ApplicationJSON,
+			},
+			wantDecoder: getJsonDecoder(),
+		},
+		"VPack HTTP connection decoder": {
+			conn: httpConnection{
+				contentType: ApplicationVPack,
+			},
+			wantDecoder: getVPackDecoder(),
+		},
+		"Bytes HTTP connection decoder": {
+			conn: httpConnection{
+				contentType: PlainText,
+			},
+			wantDecoder: getBytesDecoder(),
+		},
+		"default decoder": {
+			wantDecoder: getJsonDecoder(),
+		},
+	}
+
+	for testName, test := range tests {
+		t.Run(testName, func(t *testing.T) {
+			decoder := test.conn.Decoder(test.contentType)
+
+			require.NotNil(t, decoder)
+			assert.Equal(t, test.wantDecoder, decoder)
+		})
+	}
+}

v2/connection/connection_http_response.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2020 ArangoDB GmbH, Cologne, Germany
+// Copyright 2020-2021 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -18,12 +18,14 @@
 // Copyright holder is ArangoDB GmbH, Cologne, Germany
 //
 // Author Adam Janikowski
+// Author Tomasz Mielech
 //
 
 package connection
 
 import (
 	"net/http"
+	"strings"
 )
 
 type httpResponse struct {
@@ -44,5 +46,11 @@ func (j httpResponse) Code() int {
 }
 
 func (j httpResponse) Content() string {
-	return j.response.Header.Get(ContentType)
+	value := strings.Split(j.response.Header.Get(ContentType), ";")
+	if len(value) > 0 {
+		// The header can be returned with arguments, e.g.: "Content-Type: text/html; charset=UTF-8".
+		return value[0]
+	}
+
+	return ""
 }

v2/connection/connection_http_response_test.go

@@ -0,0 +1,76 @@
+//
+// DISCLAIMER
+//
+// Copyright 2021 ArangoDB GmbH, Cologne, Germany
+//
+// 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.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+// Author Tomasz Mielech
+//
+
+package connection
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_httpResponse_Content(t *testing.T) {
+
+	t.Run("pure content type", func(t *testing.T) {
+		j := httpResponse{
+			response: &http.Response{
+				Header: map[string][]string{},
+			},
+		}
+		j.response.Header.Set(ContentType, "application/json")
+
+		assert.Equal(t, "application/json", j.Content())
+	})
+
+	t.Run("content type with the arguments", func(t *testing.T) {
+		j := httpResponse{
+			response: &http.Response{
+				Header: map[string][]string{},
+			},
+		}
+		j.response.Header.Set(ContentType, "text/plain; charset=UTF-8")
+
+		assert.Equal(t, "text/plain", j.Content())
+	})
+
+	t.Run("empty content type", func(t *testing.T) {
+		j := httpResponse{
+			response: &http.Response{
+				Header: map[string][]string{},
+			},
+		}
+		j.response.Header.Set(ContentType, "")
+
+		assert.Equal(t, "", j.Content())
+	})
+
+	t.Run("content type header does not exist", func(t *testing.T) {
+		j := httpResponse{
+			response: &http.Response{
+				Header: map[string][]string{},
+			},
+		}
+
+		assert.Equal(t, "", j.Content())
+	})
+}