Update the PR

Author: svanharmelen

generic_packages.go

@@ -23,31 +23,32 @@ import (
 	"net/http"
 )
 
-// GenericPackageStatusValue represents a GitLab Package Status.
-type GenericPackageStatusValue string
-
-// These constants represent all valid package statuses.
-const (
-	Default GenericPackageStatusValue = "default"
-	Hidden  GenericPackageStatusValue = "hidden"
-)
-
-// GenericPackagesService handles communication with the packages related methods
-// of the GitLab API.
+// GenericPackagesService handles communication with the packages related
+// methods of the GitLab API.
 //
-// GitLab docs: https://docs.gitlab.com/ee/user/packages/generic_packages/index.html
+// GitLab docs:
+// https://docs.gitlab.com/ee/user/packages/generic_packages/index.html
 type GenericPackagesService struct {
 	client *Client
 }
 
-// DownloadPackageFile allows you to download the package file.
+// PublishPackageFileOptions represents the available PublishPackageFile()
+// options.
 //
 // GitLab docs:
 // https://docs.gitlab.com/ee/user/packages/generic_packages/index.html#download-package-file
-func (s *GenericPackagesService) DownloadPackageFile(pid interface{}, packageName, packageVersion, fileName string, options ...RequestOptionFunc) ([]byte, *Response, error) {
+type PublishPackageFileOptions struct {
+	Status *GenericPackageStatusValue `url:"status,omitempty" json:"status,omitempty"`
+}
+
+// PublishPackageFile uploads a file to a project's package registry.
+//
+// GitLab docs:
+// https://docs.gitlab.com/ee/user/packages/generic_packages/index.html#download-package-file
+func (s *GenericPackagesService) PublishPackageFile(pid interface{}, packageName, packageVersion, fileName string, content io.Reader, opt *PublishPackageFileOptions, options ...RequestOptionFunc) (*Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 	u := fmt.Sprintf(
 		"projects/%s/packages/generic/%s/%s/%s",
@@ -57,37 +58,29 @@ func (s *GenericPackagesService) DownloadPackageFile(pid interface{}, packageNam
 		pathEscape(fileName),
 	)
 
-	req, err := s.client.NewRequest(http.MethodGet, u, nil, options)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	var f bytes.Buffer
-	resp, err := s.client.Do(req, &f)
+	// We need to create the request as a GET request to make sure the options
+	// are set correctly. After the request is created we will overwrite both
+	// the method and the body.
+	req, err := s.client.NewRequest(http.MethodGet, u, opt, options)
 	if err != nil {
-		return nil, resp, err
+		return nil, err
 	}
 
-	return f.Bytes(), resp, err
-}
+	// Overwrite the method and body.
+	req.Method = http.MethodPut
+	req.SetBody(content)
 
-// PublishPackageFileOptions represents the available PublishPackageFile() options.
-//
-// GitLab docs:
-// https://docs.gitlab.com/ee/user/packages/generic_packages/index.html#download-package-file
-type PublishPackageFileOptions struct {
-	Status *GenericPackageStatusValue `url:"status,omitempty" json:"status,omitempty"`
+	return s.client.Do(req, nil)
 }
 
-// PublishPackageFile uploads a file to a project's Package Registry.
-// Returns the package URL, the response body, the Response, and any error.
+// DownloadPackageFile allows you to download the package file.
 //
 // GitLab docs:
 // https://docs.gitlab.com/ee/user/packages/generic_packages/index.html#download-package-file
-func (s *GenericPackagesService) PublishPackageFile(pid interface{}, packageName, packageVersion, fileName string, content io.ReadCloser, opt *PublishPackageFileOptions, options ...RequestOptionFunc) (string, []byte, *Response, error) {
+func (s *GenericPackagesService) DownloadPackageFile(pid interface{}, packageName, packageVersion, fileName string, options ...RequestOptionFunc) ([]byte, *Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
-		return "", nil, nil, err
+		return nil, nil, err
 	}
 	u := fmt.Sprintf(
 		"projects/%s/packages/generic/%s/%s/%s",
@@ -97,22 +90,16 @@ func (s *GenericPackagesService) PublishPackageFile(pid interface{}, packageName
 		pathEscape(fileName),
 	)
 
-	// This is currently the only way to use a PUT request to upload a non-JSON file.
-	// We invoke NewRequest with MethodGet so the body is not marshalled to JSON;
-	// WithUploadFile modifies the request to MethodPut
-	options = append(options, WithUploadFile(content))
-	req, err := s.client.NewRequest(http.MethodGet, u, opt, options)
+	req, err := s.client.NewRequest(http.MethodGet, u, nil, options)
 	if err != nil {
-		return "", nil, nil, err
+		return nil, nil, err
 	}
 
 	var f bytes.Buffer
 	resp, err := s.client.Do(req, &f)
 	if err != nil {
-		return "", nil, resp, err
+		return nil, resp, err
 	}
 
-	// ${CI_API_V4_URL}/projects/${CI_PROJECT_ID}/packages/generic/my_package/0.0.1/file.txt'
-	downloadURL := fmt.Sprintf("%s%s", s.client.BaseURL(), u)
-	return downloadURL, f.Bytes(), resp, err
+	return f.Bytes(), resp, err
 }

generic_packages_test.go

@@ -17,37 +17,13 @@
 package gitlab
 
 import (
-	"encoding/json"
 	"fmt"
-	"io"
 	"net/http"
 	"reflect"
 	"strings"
 	"testing"
 )
 
-func TestDownloadPackageFile(t *testing.T) {
-	mux, server, client := setup(t)
-	defer teardown(server)
-
-	mux.HandleFunc("/api/v4/projects/1234/packages/generic/foo/0.1.2/bar-baz.txt", func(w http.ResponseWriter, r *http.Request) {
-		testMethod(t, r, http.MethodGet)
-		fmt.Fprint(w, strings.TrimSpace(`
-		bar = baz
-	`))
-	})
-
-	packageBytes, _, err := client.GenericPackages.DownloadPackageFile(1234, "foo", "0.1.2", "bar-baz.txt")
-	if err != nil {
-		t.Errorf("GenericPackages.DownloadPackageFile returned error: %v", err)
-	}
-
-	want := []byte("bar = baz")
-	if !reflect.DeepEqual(want, packageBytes) {
-		t.Errorf("GenericPackages.DownloadPackageFile returned %+v, want %+v", packageBytes, want)
-	}
-}
-
 func TestPublishPackageFile(t *testing.T) {
 	mux, server, client := setup(t)
 	defer teardown(server)
@@ -61,23 +37,30 @@ func TestPublishPackageFile(t *testing.T) {
 	`)
 	})
 
-	url, result, _, err := client.GenericPackages.PublishPackageFile(1234, "foo", "0.1.2", "bar-baz.txt", io.NopCloser(strings.NewReader("bar = baz")), &PublishPackageFileOptions{})
+	_, err := client.GenericPackages.PublishPackageFile(1234, "foo", "0.1.2", "bar-baz.txt", strings.NewReader("bar = baz"), &PublishPackageFileOptions{})
 	if err != nil {
 		t.Errorf("GenericPackages.PublishPackageFile returned error: %v", err)
 	}
+}
 
-	goldenURL := client.BaseURL().String() + "projects/1234/packages/generic/foo/0%2E1%2E2/bar-baz%2Etxt"
-	if url != goldenURL {
-		t.Errorf("GenericPackages.PublishPackageFile URL was %+v, want %+v", url, goldenURL)
-	}
+func TestDownloadPackageFile(t *testing.T) {
+	mux, server, client := setup(t)
+	defer teardown(server)
+
+	mux.HandleFunc("/api/v4/projects/1234/packages/generic/foo/0.1.2/bar-baz.txt", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, http.MethodGet)
+		fmt.Fprint(w, strings.TrimSpace(`
+		bar = baz
+	`))
+	})
 
-	body := map[string]interface{}{}
-	if err := json.Unmarshal(result, &body); err != nil {
-		t.Errorf("Error decoding body: %v", err)
+	packageBytes, _, err := client.GenericPackages.DownloadPackageFile(1234, "foo", "0.1.2", "bar-baz.txt")
+	if err != nil {
+		t.Errorf("GenericPackages.DownloadPackageFile returned error: %v", err)
 	}
 
-	want := map[string]interface{}{"message": "201 Created"}
-	if !reflect.DeepEqual(body, want) {
-		t.Errorf("GenericPackages.PublishPackageFile response code was %+v, want %+v", body, want)
+	want := []byte("bar = baz")
+	if !reflect.DeepEqual(want, packageBytes) {
+		t.Errorf("GenericPackages.DownloadPackageFile returned %+v, want %+v", packageBytes, want)
 	}
 }

request_options.go

@@ -18,8 +18,6 @@ package gitlab
 
 import (
 	"context"
-	"io"
-	"net/http"
 
 	retryablehttp "github.com/hashicorp/go-retryablehttp"
 )
@@ -61,11 +59,3 @@ func WithToken(authType AuthType, token string) RequestOptionFunc {
 		return nil
 	}
 }
-
-func WithUploadFile(content io.ReadCloser) RequestOptionFunc {
-	return func(req *retryablehttp.Request) error {
-		req.Body = content
-		req.Method = http.MethodPut
-		return nil
-	}
-}

types.go

@@ -199,6 +199,23 @@ func FileAction(v FileActionValue) *FileActionValue {
 	return p
 }
 
+// GenericPackageStatusValue represents a GitLab Package Status.
+type GenericPackageStatusValue string
+
+// The available generic package statuses.
+const (
+	PackageDefault GenericPackageStatusValue = "default"
+	PackageHidden  GenericPackageStatusValue = "hidden"
+)
+
+// GenericPackageStatus is a helper routine that allocates a new GenericPackageStatusValue
+// value to store v and returns a pointer to it.
+func GenericPackageStatus(v GenericPackageStatusValue) *GenericPackageStatusValue {
+	p := new(GenericPackageStatusValue)
+	*p = v
+	return p
+}
+
 // ISOTime represents an ISO 8601 formatted date
 type ISOTime time.Time