Jeroen T. Vermeulen has proposed merging lp:~jtv/gomaasapi/keep-file-content
into lp:gomaasapi.
Commit message:
Fix small bug in test service, where the "list" operation accidentally deleted
files' contents.
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~jtv/gomaasapi/keep-file-content/+merge/150747
Map objects have pass-by-reference semantics internally. So where the test
service delete()d the "content" entry from a file's attributes, it did so on
the original file. And so this branch creates a copy first.
I also extracted some code because Go's "sweet spot" for function length seems
to be very low, at least in terms of functionality provided. In line count
it's probably not particularly high.
Jeroen
--
https://code.launchpad.net/~jtv/gomaasapi/keep-file-content/+merge/150747
Your team MAAS Maintainers is requested to review the proposed merge of
lp:~jtv/gomaasapi/keep-file-content into lp:gomaasapi.
=== modified file 'testservice.go'
--- testservice.go 2013-02-20 08:35:19 +0000
+++ testservice.go 2013-02-27 09:33:28 +0000
@@ -288,28 +288,48 @@
}
+// listFilenames returns the names of those uploaded files whose names start
+// with the given prefix, sorted lexicographically.
+func listFilenames(server *TestServer, prefix string) []string {
+ var filenames = make([]string, 0)
+ for filename, _ := range server.files {
+ if strings.HasPrefix(filename, prefix) {
+ filenames = append(filenames, filename)
+ }
+ }
+ sort.Strings(filenames)
+ return filenames
+}
+
+// stripFileContent copies a map of attributes representing an uploaded file,
+// but with the "content" attribute removed.
+func stripContent(original map[string]JSONObject) map[string]JSONObject {
+ newMap := make(map[string]JSONObject, len(original)-1)
+ for key, value := range original {
+ if key != "content" {
+ newMap[key] = value
+ }
+ }
+ return newMap
+}
+
// fileListingHandler handles requests for '/api/<version>/files/?op=list'.
func fileListingHandler(server *TestServer, w http.ResponseWriter, r *http.Request) {
values, _ := url.ParseQuery(r.URL.RawQuery)
prefix := values.Get("prefix")
- var convertedFiles = []map[string]JSONObject{}
- // Create slice of selected filenames.
- var filenames = []string{}
- for filename, _ := range server.files {
- if strings.Index(filename, prefix) == 0 {
- filenames = append(filenames, filename)
- }
- }
- // Sort filenames.
- sort.Strings(filenames)
+ filenames := listFilenames(server, prefix)
+
// Build a sorted list of the files as map[string]JSONObject objects.
+ convertedFiles := make([]map[string]JSONObject, 0)
for _, filename := range filenames {
- file, _ := server.files[filename]
- fileMap := file.GetMap()
- delete(fileMap, "content")
+ // The "content" attribute is not in the listing.
+ fileMap := stripContent(server.files[filename].GetMap())
convertedFiles = append(convertedFiles, fileMap)
}
- res, _ := json.Marshal(convertedFiles)
+ res, err := json.Marshal(convertedFiles)
+ if err != nil {
+ panic(err)
+ }
w.WriteHeader(http.StatusOK)
fmt.Fprint(w, string(res))
}
=== modified file 'testservice_test.go'
--- testservice_test.go 2013-02-14 16:40:47 +0000
+++ testservice_test.go 2013-02-27 09:33:28 +0000
@@ -207,7 +207,7 @@
c.Check(content, DeepEquals, fileContent)
}
-func (suite *TestServerSuite) TestHandlesListReturnedSortedFilenames(c *C) {
+func (suite *TestServerSuite) TestHandlesListReturnsSortedFilenames(c *C) {
fileName1 := "filename1"
suite.server.NewFile(fileName1, []byte("test file content"))
fileName2 := "filename2"
@@ -218,15 +218,16 @@
c.Check(err, IsNil)
c.Check(resp.StatusCode, Equals, http.StatusOK)
content, err := ioutil.ReadAll(resp.Body)
- c.Check(err, IsNil)
+ c.Assert(err, IsNil)
var files []map[string]string
err = json.Unmarshal(content, &files)
+ c.Assert(err, IsNil)
c.Check(len(files), Equals, 2)
c.Check(files[0]["filename"], Equals, fileName1)
c.Check(files[1]["filename"], Equals, fileName2)
}
-func (suite *TestServerSuite) TestHandlesListReturnedFilteredFiles(c *C) {
+func (suite *TestServerSuite) TestHandlesListFiltersFiles(c *C) {
fileName1 := "filename1"
suite.server.NewFile(fileName1, []byte("test file content"))
fileName2 := "prefixFilename"
@@ -238,13 +239,41 @@
c.Check(err, IsNil)
c.Check(resp.StatusCode, Equals, http.StatusOK)
content, err := ioutil.ReadAll(resp.Body)
- c.Check(err, IsNil)
+ c.Assert(err, IsNil)
var files []map[string]string
err = json.Unmarshal(content, &files)
+ c.Assert(err, IsNil)
c.Check(len(files), Equals, 1)
c.Check(files[0]["filename"], Equals, fileName2)
}
+func (suite *TestServerSuite) TestHandlesListOmitsContent(c *C) {
+ const filename = "myfile"
+ fileContent := []byte("test file content")
+ suite.server.NewFile(filename, fileContent)
+ getURI := fmt.Sprintf("/api/%s/files/?op=list", suite.server.version)
+
+ resp, err := http.Get(suite.server.Server.URL + getURI)
+ c.Assert(err, IsNil)
+
+ content, err := ioutil.ReadAll(resp.Body)
+ c.Assert(err, IsNil)
+ var files []map[string]string
+ err = json.Unmarshal(content, &files)
+
+ // The resulting dict does not have a "content" entry.
+ file := files[0]
+ _, ok := file["content"]
+ c.Check(ok, Equals, false)
+
+ // But the original as stored in the test service still has it.
+ contentAfter, err := suite.server.files[filename].GetField("content")
+ c.Assert(err, IsNil)
+ bytes, err := base64.StdEncoding.DecodeString(contentAfter)
+ c.Assert(err, IsNil)
+ c.Check(string(bytes), Equals, string(fileContent))
+}
+
func (suite *TestServerSuite) TestDeleteFile(c *C) {
fileName1 := "filename1"
suite.server.NewFile(fileName1, []byte("test file content"))
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
