launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/gomaasapi/upload-files2 into lp:gomaasapi.

Commit message:
Add a method to upload files to the API.  Exercise the files API in the example package.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~rvb/gomaasapi/upload-files2/+merge/147920

The meat of this branch is the change to nonIdempotentRequest: it now accept a new parameter, 'files'.  If this parameter is non-nil, it will issue an multipart request with the given files instead of the standard post request.

I've also tested that this works against a real MAAS server (see the changes to live_example.go).  Try it out with:
(first, get a MAAS dev server started)
go run example/live_example.go <<EOF
your:api:key
http://0.0.0.0:5240/api/1.0/
EOF

-- 
https://code.launchpad.net/~rvb/gomaasapi/upload-files2/+merge/147920
Your team MAAS Maintainers is requested to review the proposed merge of lp:~rvb/gomaasapi/upload-files2 into lp:gomaasapi.

=== modified file 'client.go'
--- client.go	2013-02-11 12:15:14 +0000
+++ client.go	2013-02-12 14:00:27 +0000
@@ -4,9 +4,12 @@
 package gomaasapi
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
+	"io"
 	"io/ioutil"
+	"mime/multipart"
 	"net/http"
 	"net/url"
 	"strings"
@@ -65,30 +68,76 @@
 	return client.dispatchRequest(request)
 }
 
+// writeMultiPartFiles writes the given files as parts of a multipart message
+// using the given writer.
+func writeMultiPartFiles(writer *multipart.Writer, files map[string][]byte) {
+	for fileName, fileContent := range files {
+
+		fw, err := writer.CreateFormFile(fileName, fileName)
+		if err != nil {
+			panic(err)
+		}
+		io.Copy(fw, bytes.NewBuffer(fileContent))
+	}
+}
+
+// writeMultiPartParams writes the given parameters as parts of a multipart
+// message using the given writer.
+func writeMultiPartParams(writer *multipart.Writer, parameters url.Values) {
+	for key, values := range parameters {
+		for _, value := range values {
+			fw, err := writer.CreateFormField(key)
+			if err != nil {
+				panic(err)
+			}
+			buffer := bytes.NewBufferString(value)
+			io.Copy(fw, buffer)
+		}
+	}
+
+}
+
 // nonIdempotentRequest implements the common functionality of PUT and POST
 // requests (but not GET or DELETE requests).
-func (client Client) nonIdempotentRequest(method string, uri *url.URL, parameters url.Values) ([]byte, error) {
-	url := client.GetURL(uri)
-	request, err := http.NewRequest(method, url.String(), strings.NewReader(string(parameters.Encode())))
-	if err != nil {
-		return nil, err
+func (client Client) nonIdempotentRequest(method string, uri *url.URL, parameters url.Values, files map[string][]byte) ([]byte, error) {
+	var request *http.Request
+	var err error
+	if files != nil {
+		// files is not nil, create a multipart request.
+		buf := new(bytes.Buffer)
+		writer := multipart.NewWriter(buf)
+		writeMultiPartFiles(writer, files)
+		writeMultiPartParams(writer, parameters)
+		writer.Close()
+		url := client.GetURL(uri)
+		request, err = http.NewRequest(method, url.String(), buf)
+		if err != nil {
+			return nil, err
+		}
+		request.Header.Set("Content-Type", writer.FormDataContentType())
+	} else {
+		url := client.GetURL(uri)
+		request, err = http.NewRequest(method, url.String(), strings.NewReader(string(parameters.Encode())))
+		if err != nil {
+			return nil, err
+		}
+		request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 	}
-	request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 	return client.dispatchRequest(request)
 }
 
 // Post performs an HTTP "POST" to the API.  This may be either an API method
 // invocation (if you pass its name in "operation") or plain resource
 // retrieval (if you leave "operation" blank).
-func (client Client) Post(uri *url.URL, operation string, parameters url.Values) ([]byte, error) {
+func (client Client) Post(uri *url.URL, operation string, parameters url.Values, files map[string][]byte) ([]byte, error) {
 	queryParams := url.Values{"op": {operation}}
 	uri.RawQuery = queryParams.Encode()
-	return client.nonIdempotentRequest("POST", uri, parameters)
+	return client.nonIdempotentRequest("POST", uri, parameters, files)
 }
 
 // Put updates an object on the API, using an HTTP "PUT" request.
 func (client Client) Put(uri *url.URL, parameters url.Values) ([]byte, error) {
-	return client.nonIdempotentRequest("PUT", uri, parameters)
+	return client.nonIdempotentRequest("PUT", uri, parameters, nil)
 }
 
 // Delete deletes an object on the API, using an HTTP "DELETE" request.

=== modified file 'client_test.go'
--- client_test.go	2013-02-05 14:39:14 +0000
+++ client_test.go	2013-02-12 14:00:27 +0000
@@ -4,6 +4,8 @@
 package gomaasapi
 
 import (
+	"bytes"
+	"io/ioutil"
 	. "launchpad.net/gocheck"
 	"net/http"
 	"net/url"
@@ -72,7 +74,7 @@
 	c.Check(string(result), Equals, expectedResult)
 }
 
-func (suite *ClientSuite) TestClientPostSendsRequest(c *C) {
+func (suite *ClientSuite) TestClientPostSendsRequestWithParams(c *C) {
 	URI, _ := url.Parse("/some/url")
 	expectedResult := "expected:result"
 	fullURI := URI.String() + "?op=list"
@@ -81,11 +83,40 @@
 	defer server.Close()
 	client, _ := NewAnonymousClient(server.URL)
 
-	result, err := client.Post(URI, "list", params)
-
-	c.Check(err, IsNil)
-	c.Check(string(result), Equals, expectedResult)
-	c.Check(*server.requestContent, Equals, "test=123")
+	result, err := client.Post(URI, "list", params, nil)
+
+	c.Check(err, IsNil)
+	c.Check(string(result), Equals, expectedResult)
+	postedValues, err := url.ParseQuery(*server.requestContent)
+	c.Check(err, IsNil)
+	expectedPostedValues, _ := url.ParseQuery("test=123")
+	c.Check(postedValues, DeepEquals, expectedPostedValues)
+}
+
+func (suite *ClientSuite) TestClientPostSendsMultipartRequest(c *C) {
+	URI, _ := url.Parse("/some/url")
+	expectedResult := "expected:result"
+	fullURI := URI.String() + "?op=add"
+	server := newSingleServingServer(fullURI, expectedResult, http.StatusOK)
+	defer server.Close()
+	client, _ := NewAnonymousClient(server.URL)
+	fileContent := []byte("content")
+	files := map[string][]byte{"testfile": fileContent}
+
+	result, err := client.Post(URI, "add", url.Values{}, files)
+
+	c.Check(err, IsNil)
+	c.Check(string(result), Equals, expectedResult)
+	// Recreate the request from server.requestContent to use the parsing
+	// utility from the http package (http.Request.FormFile).
+	request, err := http.NewRequest("POST", fullURI, bytes.NewBufferString(*server.requestContent))
+	c.Assert(err, IsNil)
+	request.Header.Set("Content-Type", server.requestHeader.Get("Content-Type"))
+
+	file, _, err := request.FormFile("testfile")
+	c.Check(err, IsNil)
+	receivedFileContent, err := ioutil.ReadAll(file)
+	c.Check(receivedFileContent, DeepEquals, fileContent)
 }
 
 func (suite *ClientSuite) TestClientPutSendsRequest(c *C) {

=== modified file 'example/live_example.go'
--- example/live_example.go	2013-02-12 05:42:32 +0000
+++ example/live_example.go	2013-02-12 14:00:27 +0000
@@ -10,6 +10,7 @@
 
 import (
 	"fmt"
+	"bytes"
 	"launchpad.net/gomaasapi"
 	"net/url"
 )
@@ -39,11 +40,49 @@
 
 func main() {
 	getParams()
+
+	// Create API server endpoint.
 	authClient, err := gomaasapi.NewAuthenticatedClient(apiURL, apiKey)
 	checkError(err)
-
 	maas := gomaasapi.NewMAAS(*authClient)
 
+	// Exercice the API.
+	ManipulateNodes(maas)
+	ManipulateFiles(maas)
+
+	fmt.Println("All done.")
+}
+
+// ManipulateFiles exercices the /api/1.0/files/ API endpoint.  Most precisely,
+// it uploads a files and then fetches it, making sure the received content
+// is the same as the one that was sent.
+func ManipulateFiles(maas gomaasapi.MAASObject) {
+	files := maas.GetSubObject("files")
+	fileContent := []byte("test file content")
+	filesToUpload := map[string][]byte{"file": fileContent}
+
+	// Upload a file.
+	fmt.Println("Uploading a file...")
+	_, err := files.CallPostFiles("add", url.Values{"filename": {"filename"}}, filesToUpload)
+	checkError(err)
+	fmt.Println("File sent.")
+
+	// Fetch the file.
+	fmt.Println("Fetching the file...")
+	fileResult, err := files.CallGet("get", url.Values{"filename": {"filename2"}})
+	checkError(err)
+	receivedFileContent, err := fileResult.GetBytes()
+	checkError(err)
+	if bytes.Compare(receivedFileContent, fileContent) != 0 {
+		panic("Received content differs from the content sent!")
+	}
+	fmt.Println("Got file.")
+}
+
+// ManipulateFiles exercices the /api/1.0/nodes/ API endpoint.  Most precisely,
+// it lists the existing nodes, creates a new node, updates it and then
+// deletes it.
+func ManipulateNodes(maas gomaasapi.MAASObject) {
 	nodeListing := maas.GetSubObject("nodes")
 
 	// List nodes.
@@ -99,6 +138,4 @@
 	listNodes3, err := listNodeObjects3.GetArray()
 	checkError(err)
 	fmt.Printf("We've got %v nodes\n", len(listNodes3))
-
-	fmt.Println("All done.")
 }

=== modified file 'maasobject.go'
--- maasobject.go	2013-02-12 07:17:17 +0000
+++ maasobject.go	2013-02-12 14:00:27 +0000
@@ -144,7 +144,7 @@
 // in "params."  It returns the object's new value as received from the API.
 func (obj MAASObject) Post(params url.Values) (JSONObject, error) {
 	uri := obj.URI()
-	result, err := obj.client.Post(uri, "", params)
+	result, err := obj.client.Post(uri, "", params, nil)
 	if err != nil {
 		return JSONObject{}, err
 	}
@@ -184,8 +184,15 @@
 
 // CallPost invokes a non-idempotent API method on this object.
 func (obj MAASObject) CallPost(operation string, params url.Values) (JSONObject, error) {
+	return obj.CallPostFiles(operation, params, nil)
+}
+
+// CallPostFiles invokes a non-idempotent API method on this object.  It is
+// similar to CallPost but has an extra parameter, 'files', which should
+// contain the files that will be uploaded to the API.
+func (obj MAASObject) CallPostFiles(operation string, params url.Values, files map[string][]byte) (JSONObject, error) {
 	uri := obj.URI()
-	result, err := obj.client.Post(uri, operation, params)
+	result, err := obj.client.Post(uri, operation, params, files)
 	if err != nil {
 		return JSONObject{}, err
 	}