From 7f011ae6422f0fb676739786fe0fd4f73d1b207e Mon Sep 17 00:00:00 2001
From: Aiden Keating <akeating@redhat.com>
Date: Tue, 22 Jan 2019 23:10:51 +0000
Subject: [PATCH] Allow media type to be specified in release asset upload

Update UploadOptions with a MediaType field. If the MediaType is
provided to UploadReleaseAsset it will take precedence over the type
retrieved from looking at the file extension. The complete order of
precedence is:

- MediaType in UploadOptions
- media type from file extension
- default media type, application/octet-stream

Update unit tests to test the order of precedence, default value and
that the Content-Type header is actually set in the upload request.
---
 github/github.go              |  3 +-
 github/repos_releases.go      |  4 ++
 github/repos_releases_test.go | 79 +++++++++++++++++++++++++----------
 3 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/github/github.go b/github/github.go
index abed36dd9bd..4b4768f6946 100644
--- a/github/github.go
+++ b/github/github.go
@@ -191,7 +191,8 @@ type ListOptions struct {
 
 // UploadOptions specifies the parameters to methods that support uploads.
 type UploadOptions struct {
-	Name string `url:"name,omitempty"`
+	Name      string `url:"name,omitempty"`
+	MediaType string `url:"-"`
 }
 
 // RawType represents type of raw format of a request instead of JSON.
diff --git a/github/repos_releases.go b/github/repos_releases.go
index bf58743ebdc..5c0a1cea84f 100644
--- a/github/repos_releases.go
+++ b/github/repos_releases.go
@@ -356,6 +356,10 @@ func (s *RepositoriesService) UploadReleaseAsset(ctx context.Context, owner, rep
 	}
 
 	mediaType := mime.TypeByExtension(filepath.Ext(file.Name()))
+	if opt.MediaType != "" {
+		mediaType = opt.MediaType
+	}
+
 	req, err := s.client.NewUploadRequest(u, file, stat.Size(), mediaType)
 	if err != nil {
 		return nil, nil, err
diff --git a/github/repos_releases_test.go b/github/repos_releases_test.go
index d9a9332419a..971b79c75fa 100644
--- a/github/repos_releases_test.go
+++ b/github/repos_releases_test.go
@@ -354,32 +354,65 @@ func TestRepositoriesService_DeleteReleaseAsset(t *testing.T) {
 }
 
 func TestRepositoriesService_UploadReleaseAsset(t *testing.T) {
+	uploadTests := []struct {
+		uploadOpts        *UploadOptions
+		fileName          string
+		expectedMediaType string
+	}{
+		// No file extension and no explicit media type.
+		{
+			&UploadOptions{Name: "n"},
+			"upload",
+			defaultMediaType,
+		},
+		// File extension and no explicit media type.
+		{
+			&UploadOptions{Name: "n"},
+			"upload.txt",
+			"text/plain; charset=utf-8",
+		},
+		// No file extension and explicit media type.
+		{
+			&UploadOptions{Name: "n", MediaType: "image/png"},
+			"upload",
+			"image/png",
+		},
+		// File extension and explicit media type.
+		{
+			&UploadOptions{Name: "n", MediaType: "image/png"},
+			"upload.png",
+			"image/png",
+		},
+	}
+
 	client, mux, _, teardown := setup()
 	defer teardown()
 
-	mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) {
-		testMethod(t, r, "POST")
-		testHeader(t, r, "Content-Type", "text/plain; charset=utf-8")
-		testHeader(t, r, "Content-Length", "12")
-		testFormValues(t, r, values{"name": "n"})
-		testBody(t, r, "Upload me !\n")
-
-		fmt.Fprintf(w, `{"id":1}`)
-	})
-
-	file, dir, err := openTestFile("upload.txt", "Upload me !\n")
-	if err != nil {
-		t.Fatalf("Unable to create temp file: %v", err)
-	}
-	defer os.RemoveAll(dir)
+	for key, test := range uploadTests {
+		releaseEndpoint := fmt.Sprintf("/repos/o/r/releases/%d/assets", key)
+		mux.HandleFunc(releaseEndpoint, func(w http.ResponseWriter, r *http.Request) {
+			testMethod(t, r, "POST")
+			testHeader(t, r, "Content-Type", test.expectedMediaType)
+			testHeader(t, r, "Content-Length", "12")
+			testFormValues(t, r, values{"name": "n"})
+			testBody(t, r, "Upload me !\n")
+
+			fmt.Fprintf(w, `{"id":1}`)
+		})
+
+		file, dir, err := openTestFile(test.fileName, "Upload me !\n")
+		if err != nil {
+			t.Fatalf("Unable to create temp file: %v", err)
+		}
+		defer os.RemoveAll(dir)
 
-	opt := &UploadOptions{Name: "n"}
-	asset, _, err := client.Repositories.UploadReleaseAsset(context.Background(), "o", "r", 1, opt, file)
-	if err != nil {
-		t.Errorf("Repositories.UploadReleaseAssert returned error: %v", err)
-	}
-	want := &ReleaseAsset{ID: Int64(1)}
-	if !reflect.DeepEqual(asset, want) {
-		t.Errorf("Repositories.UploadReleaseAssert returned %+v, want %+v", asset, want)
+		asset, _, err := client.Repositories.UploadReleaseAsset(context.Background(), "o", "r", int64(key), test.uploadOpts, file)
+		if err != nil {
+			t.Errorf("Repositories.UploadReleaseAssert returned error: %v", err)
+		}
+		want := &ReleaseAsset{ID: Int64(1)}
+		if !reflect.DeepEqual(asset, want) {
+			t.Errorf("Repositories.UploadReleaseAssert returned %+v, want %+v", asset, want)
+		}
 	}
 }