TOOL-602 Removing curl command (#99)

* Removing curl command

* Comment out golint

* Comment out errcheck

* Comment out test

* Adding retry

* Removing error message printed when trying to close already closed file

* Test without content-length

* Test without content-length

* Test timeout:

* Test timeout

* Test timeout

* Add test for upload

* Undo bitrise.yml change

* Do not try to close file twice

* PR fixes

* PR fixes

uploaders/common.go

@@ -1,6 +1,7 @@
 package uploaders
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -11,7 +12,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/bitrise-io/go-utils/command"
 	"github.com/bitrise-io/go-utils/log"
 	"github.com/bitrise-io/go-utils/retry"
 	"github.com/bitrise-io/go-utils/urlutil"
@@ -110,32 +110,60 @@ func createArtifact(buildURL, token, artifactPth, artifactType string) (string,
 func uploadArtifact(uploadURL, artifactPth, contentType string) error {
 	log.Printf("uploading artifact")
 
-	data, err := os.Open(artifactPth)
-	if err != nil {
-		return fmt.Errorf("failed to open artifact, error: %s", err)
+	netClient := &http.Client{
+		Timeout: 10 * time.Minute,
 	}
-	defer func() {
-		if err := data.Close(); err != nil {
-			log.Errorf("Failed to close file, error: %s", err)
+
+	return retry.Times(3).Wait(5).Try(func(attempt uint) error {
+		file, err := os.Open(artifactPth)
+		if err != nil {
+			return fmt.Errorf("failed to open artifact, error: %s", err)
 		}
-	}()
+		defer func() {
+			if err := file.Close(); err != nil {
+				log.Warnf("failed to close file, error: %s", err)
+			}
+		}()
 
-	args := []string{"curl", "--fail", "--tlsv1", "--globoff"}
-	if contentType != "" {
-		args = append(args, "-H", fmt.Sprintf("Content-Type: %s", contentType))
-	}
-	args = append(args, "-T", artifactPth, "-X", "PUT", uploadURL)
+		request, err := http.NewRequest(http.MethodPut, uploadURL, ioutil.NopCloser(file))
+		if err != nil {
+			return fmt.Errorf("failed to create request, error: %s", err)
+		}
 
-	return retry.Times(3).Wait(5 * time.Second).Try(func(attempt uint) error {
-		if attempt > 0 {
-			log.Warnf("%d attempt failed", attempt)
+		request.Header.Add("Content-Type", contentType)
+
+		// Set Content Length manually (https://stackoverflow.com/a/39764726), as it is part of signature in signed URL
+		fileInfo, err := file.Stat()
+		if err != nil {
+			return fmt.Errorf("failed to get file info for %s, error: %s", artifactPth, err)
 		}
-		cmd, err := command.NewFromSlice(args)
+		request.ContentLength = fileInfo.Size()
+
+		ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
+		defer cancel()
+		request = request.WithContext(ctx)
+
+		resp, err := netClient.Do(request)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to upload artifact, error: %s", err)
 		}
-		cmd.SetStdout(os.Stdout).SetStderr(os.Stderr)
-		return cmd.Run()
+
+		defer func() {
+			if err := resp.Body.Close(); err != nil {
+				log.Errorf("Failed to close response body, error: %s", err)
+			}
+		}()
+
+		body, err := ioutil.ReadAll(resp.Body)
+		if err != nil {
+			return fmt.Errorf("failed to read response body, error: %s", err)
+		}
+
+		if resp.StatusCode != http.StatusOK {
+			return fmt.Errorf("non success status code: %d, headers: %s, body: %s", resp.StatusCode, resp.Header, body)
+		}
+
+		return nil
 	})
 }
 

uploaders/common_test.go

@@ -0,0 +1,90 @@
+package uploaders
+
+import (
+	"image"
+	"image/png"
+	"io/ioutil"
+	"math/rand"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"testing"
+)
+
+func Test_uploadArtifact(t *testing.T) {
+	const contentType = "image/png"
+
+	file, err := ioutil.TempFile("", "")
+	if err != nil {
+		t.Fatalf("setup: failed to create file, error: %s", err)
+	}
+	testFilePath, err := filepath.Abs(file.Name())
+	if err != nil {
+		t.Fatalf("setup: failed to get file path, error: %s", err)
+	}
+
+	img := image.NewRGBA(image.Rectangle{image.Point{0, 0}, image.Point{rand.Intn(1000) + 1, rand.Intn(1000) + 1}})
+	if err := png.Encode(file, img); err != nil {
+		t.Fatalf("setup: failed to write file, error: %s", err)
+	}
+
+	fileInfo, err := file.Stat()
+	if err != nil {
+		t.Fatalf("setup: failed to get file info, error: %s", err)
+	}
+	wantFileSize := fileInfo.Size()
+
+	if err := file.Close(); err != nil {
+		t.Errorf("setup: failed to close file")
+	}
+
+	storage := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPut {
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+
+		bytes, err := ioutil.ReadAll(r.Body)
+		if err != nil {
+			t.Errorf("httptest: failed to read request, error: %s", err)
+			return
+		}
+
+		if r.ContentLength != wantFileSize {
+			t.Errorf("httptest: Content-length got %d want %d", r.ContentLength, wantFileSize)
+		}
+
+		if r.Header.Get("Content-Type") != contentType {
+			t.Errorf("httptest: content type got: %s want: %s", r.Header.Get("Content-Type"), contentType)
+		}
+
+		if int64(len(bytes)) != wantFileSize {
+			w.WriteHeader(http.StatusInternalServerError)
+			return
+		}
+
+		w.WriteHeader(http.StatusOK)
+	}))
+
+	tests := []struct {
+		name        string
+		uploadURL   string
+		artifactPth string
+		contentType string
+		wantErr     bool
+	}{
+		{
+			name:        "Happy path",
+			uploadURL:   storage.URL,
+			artifactPth: testFilePath,
+			contentType: contentType,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := uploadArtifact(tt.uploadURL, tt.artifactPth, tt.contentType); (err != nil) != tt.wantErr {
+				t.Errorf("uploadArtifact() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}