Skip to content
This repository has been archived by the owner on Dec 10, 2024. It is now read-only.

Fix ImportFile() to send a multipart form request #1215

Closed
wants to merge 2 commits into from
Closed

Fix ImportFile() to send a multipart form request #1215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49d8e56
Fix ImportFile() to send a multipart form request
dheitman42 Sep 11, 2021
d7ed40c
Tweak PR a bit
svanharmelen Oct 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions group_import_export.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,13 @@ type GroupImportFileOptions struct {
// https://docs.gitlab.com/ce/api/group_import_export.html#import-a-file
func (s *GroupImportExportService) ImportFile(opt *GroupImportFileOptions, options ...RequestOptionFunc) (*Response, error) {
// First check if we got all required options.
if opt.Name == nil || *opt.Name == "" {
if opt == nil || opt.Name == nil || *opt.Name == "" {
return nil, fmt.Errorf("Missing required option: Name")
}
if opt.Path == nil || *opt.Path == "" {
if opt == nil || opt.Path == nil || *opt.Path == "" {
return nil, fmt.Errorf("Missing required option: Path")
}
if opt.File == nil || *opt.File == "" {
if opt == nil || opt.File == nil || *opt.File == "" {
return nil, fmt.Errorf("Missing required option: File")
}

Expand Down
52 changes: 51 additions & 1 deletion project_import_export.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@ package gitlab
import (
"bytes"
"fmt"
"io"
"mime/multipart"
"net/http"
"os"
"time"

"github.com/google/go-querystring/query"
)

// ProjectImportExportService handles communication with the project
Expand Down Expand Up @@ -162,6 +167,7 @@ func (s *ProjectImportExportService) ExportDownload(pid interface{}, options ...
// https://docs.gitlab.com/ce/api/project_import_export.html#import-a-file
type ImportFileOptions struct {
Namespace *string `url:"namespace,omitempty" json:"namespace,omitempty"`
Name *string `url:"name,omitempty" json:"name,omitempty"`
File *string `url:"file,omitempty" json:"file,omitempty"`
Path *string `url:"path,omitempty" json:"path,omitempty"`
Overwrite *bool `url:"overwrite,omitempty" json:"overwrite,omitempty"`
Expand All @@ -173,10 +179,54 @@ type ImportFileOptions struct {
// GitLab API docs:
// https://docs.gitlab.com/ce/api/project_import_export.html#import-a-file
func (s *ProjectImportExportService) ImportFile(opt *ImportFileOptions, options ...RequestOptionFunc) (*ImportStatus, *Response, error) {
req, err := s.client.NewRequest(http.MethodPost, "projects/import", opt, options)
// First check if we got all required options.
if opt == nil || opt.File == nil || *opt.File == "" {
return nil, nil, fmt.Errorf("Missing required option: File")
}
if opt == nil || opt.Path == nil || *opt.Path == "" {
return nil, nil, fmt.Errorf("Missing required option: Path")
}

file, err := os.Open(*opt.File)
if err != nil {
return nil, nil, err
}
defer file.Close()

b := &bytes.Buffer{}
w := multipart.NewWriter(b)

fields, err := query.Values(opt)
if err != nil {
return nil, nil, err
}

for name := range fields {
if name == "file" {
part, err := w.CreateFormFile("file", file.Name())
if err != nil {
return nil, nil, err
}
if _, err = io.Copy(part, file); err != nil {
return nil, nil, err
}
} else {
if err = w.WriteField(name, fmt.Sprintf("%v", fields.Get(name))); err != nil {
return nil, nil, err
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No clue what you are trying to do here, but this doesn't look right to me. Can you please elaborate on this approach?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we're creating a multipart form, creating the file component with CreateFormFile(), copying the file contents to that part, then adding each of the fields from the ImportFileOptions struct as additional field data in the multipart form. That's the intent, anyway. I'll admit to being n00bish with golang, so maybe this isn't the proper way to do the struct-to-string-map conversion, but that's the basic idea.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno if that answered your question, happy to clarify further if need be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@svanharmelen this is how you do a multipart form upload: https://gist.github.com/mattetti/5914158/f4d1393d83ebedc682a3c8e7bdc6b49670083b84

In the api doc it is described as:

To upload a file from your file system, use the --form argument. This causes cURL to post data using the header Content-Type: multipart/form-data.

Copy link
Contributor

@zbindenren zbindenren Oct 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dheitman42 I tried out your pull request and I got an internal server error. I tested our server successfully with the following code though:

func TestHelloWorld(t *testing.T) {
	extraParams := map[string]string{
		"namespace": "go",
		"path":      "test",
	}

	request, err := newfileUploadRequest("https://gitlab.example.com/api/v4/projects/import", extraParams, "file", "/tmp/file.tar.gz")
	require.NoError(t, err)
	request.Header.Set("Authorization", "Bearer "+gitlabTkn)
	client := &http.Client{}
	resp, err := client.Do(request)
        require.NoError(t, err)
}


func newfileUploadRequest(uri string, params map[string]string, paramName, path string) (*http.Request, error) {
	file, err := os.Open(path)
	if err != nil {
		return nil, err
	}
	fileContents, err := ioutil.ReadAll(file)
	if err != nil {
		return nil, err
	}
	fi, err := file.Stat()
	if err != nil {
		return nil, err
	}
	file.Close()

	body := new(bytes.Buffer)
	writer := multipart.NewWriter(body)
	part, err := writer.CreateFormFile(paramName, fi.Name())
	if err != nil {
		return nil, err
	}

	if _, err := part.Write(fileContents); err != nil {
		return nil, err
	}

	for key, val := range params {
		_ = writer.WriteField(key, val)
	}
	err = writer.Close()
	if err != nil {
		return nil, err
	}

	req, err := http.NewRequest("POST", uri, body)
	if err != nil {
		return nil, err
	}
	req.Header.Set("Content-Type", writer.FormDataContentType())

	return req, nil
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, I'm using the exact code from my MR in production, just verified it's working. I'll peek at your version when I have a spare moment (so, probably don't hold your breath :D )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my latest commit to this PR:

By using query.Values we are using the URL tags and are omitting any empty values. This seems to be inline with what the API expects. Could you both this this version @dheitman42 and @zbindenren?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@svanharmelen I am still getting a http 500 for my test:

	is, _, err := dst.ProjectImportExport.ImportFile(&gitlab.ImportFileOptions{
		Namespace: gitlab.String("linux/go"),
		Path:      gitlab.String("flash"),
		File:      gitlab.String("flash.tar.gz"),
	})
	if err != nil {
		fmt.Println("error", err)
		fmt.Println("body", err)
	}
go test -count=1 -v -run 'TestHelloWorld$' ./.
=== RUN   TestHelloWorld
error POST https://gitlab.pnet.ch/api/v4/projects/import: 500 failed to parse unknown error format
body POST https://gitlab.pnet.ch/api/v4/projects/import: 500 failed to parse unknown error format
    main_test.go:82: 
                Error Trace:    main_test.go:82
                Error:          Received unexpected error:
                                POST https://gitlab.pnet.ch/api/v4/projects/import: 500 failed to parse unknown error format
                Test:           TestHelloWorld
--- FAIL: TestHelloWorld (12.38s)
FAIL
FAIL    it.pnet.ch/golang/gitmig        12.388s
FAIL


err = w.Close()
if err != nil {
return nil, nil, err
}

req, err := s.client.NewRequest(http.MethodPost, "projects/import", b, options)
if err != nil {
return nil, nil, err
}
req.Header.Set("Content-Type", w.FormDataContentType())

is := new(ImportStatus)
resp, err := s.client.Do(req, is)
Expand Down
57 changes: 0 additions & 57 deletions project_import_export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,63 +128,6 @@ func TestProjectImportExportService_ExportDownload(t *testing.T) {
require.Equal(t, http.StatusNotFound, resp.StatusCode)
}

func TestProjectImportExportService_ImportFile(t *testing.T) {
mux, server, client := setup(t)
defer teardown(server)

mux.HandleFunc("/api/v4/projects/import", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, http.MethodPost)
fmt.Fprintf(w, `
{
"id": 1,
"description": null,
"name": "api-project",
"name_with_namespace": "Administrator / api-project",
"path": "api-project",
"path_with_namespace": "root/api-project",
"import_status": "scheduled",
"correlation_id": "mezklWso3Za",
"failed_relations": []
}
`)
})

want := &ImportStatus{
ID: 1,
Description: "",
Name: "api-project",
NameWithNamespace: "Administrator / api-project",
Path: "api-project",
PathWithNamespace: "root/api-project",
ImportStatus: "scheduled",
}

es, resp, err := client.ProjectImportExport.ImportFile(nil, nil)
require.NoError(t, err)
require.NotNil(t, resp)
require.Equal(t, want, es)

es, resp, err = client.ProjectImportExport.ImportFile(nil, errorOption)
require.EqualError(t, err, "RequestOptionFunc returns an error")
require.Nil(t, resp)
require.Nil(t, es)
}

func TestProjectImportExportService_ImportFile_NotFound(t *testing.T) {
mux, server, client := setup(t)
defer teardown(server)

mux.HandleFunc("/api/v4/projects/import", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, http.MethodPost)
w.WriteHeader(http.StatusNotFound)
})

es, resp, err := client.ProjectImportExport.ImportFile(nil, nil)
require.Error(t, err)
require.Nil(t, es)
require.Equal(t, http.StatusNotFound, resp.StatusCode)
}

func TestProjectImportExportService_ImportStatus(t *testing.T) {
mux, server, client := setup(t)
defer teardown(server)
Expand Down