From 68fd15b6885c09210558433017007054c5894c58 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Thu, 20 Nov 2014 14:11:49 -0800 Subject: [PATCH] Miscellaneous storagedriver+ipc fixes Fixes/tests listing for keys beginning with "/" No longer extraneously wraps Closers in ioutil.NopClosers Uses omitempty for all ipc struct type fields --- storagedriver/filesystem/driver.go | 5 ++-- storagedriver/inmemory/driver.go | 7 ++++-- storagedriver/ipc/client.go | 2 +- storagedriver/ipc/ipc.go | 34 +++++++++++++------------- storagedriver/ipc/server.go | 2 +- storagedriver/testsuites/testsuites.go | 8 ++++-- 6 files changed, 33 insertions(+), 25 deletions(-) diff --git a/storagedriver/filesystem/driver.go b/storagedriver/filesystem/driver.go index eabb493d..a4b2e688 100644 --- a/storagedriver/filesystem/driver.go +++ b/storagedriver/filesystem/driver.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "os" "path" - "strings" "github.com/docker/docker-registry/storagedriver" "github.com/docker/docker-registry/storagedriver/factory" @@ -177,7 +176,9 @@ func (d *Driver) CurrentSize(subPath string) (uint64, error) { // List returns a list of the objects that are direct descendants of the given // path. func (d *Driver) List(subPath string) ([]string, error) { - subPath = strings.TrimRight(subPath, "/") + if subPath[len(subPath)-1] != '/' { + subPath += "/" + } fullPath := d.subPath(subPath) dir, err := os.Open(fullPath) diff --git a/storagedriver/inmemory/driver.go b/storagedriver/inmemory/driver.go index 8685eb25..0d28b2da 100644 --- a/storagedriver/inmemory/driver.go +++ b/storagedriver/inmemory/driver.go @@ -121,14 +121,17 @@ func (d *Driver) CurrentSize(path string) (uint64, error) { // List returns a list of the objects that are direct descendants of the given // path. func (d *Driver) List(path string) ([]string, error) { - subPathMatcher, err := regexp.Compile(fmt.Sprintf("^%s/[^/]+", path)) + if path[len(path)-1] != '/' { + path += "/" + } + subPathMatcher, err := regexp.Compile(fmt.Sprintf("^%s[^/]+", path)) if err != nil { return nil, err } d.mutex.RLock() defer d.mutex.RUnlock() - // we use map to collect uniq keys + // we use map to collect unique keys keySet := make(map[string]struct{}) for k := range d.storage { if key := subPathMatcher.FindString(k); key != "" { diff --git a/storagedriver/ipc/client.go b/storagedriver/ipc/client.go index 51b02b46..a0ff3788 100644 --- a/storagedriver/ipc/client.go +++ b/storagedriver/ipc/client.go @@ -267,7 +267,7 @@ func (driver *StorageDriverClient) WriteStream(path string, offset, size uint64, } receiver, remoteSender := libchan.Pipe() - params := map[string]interface{}{"Path": path, "Offset": offset, "Size": size, "Reader": ioutil.NopCloser(reader)} + params := map[string]interface{}{"Path": path, "Offset": offset, "Size": size, "Reader": reader} err := driver.sender.Send(&Request{Type: "WriteStream", Parameters: params, ResponseChannel: remoteSender}) if err != nil { return err diff --git a/storagedriver/ipc/ipc.go b/storagedriver/ipc/ipc.go index 182a1af6..82bdcbd7 100644 --- a/storagedriver/ipc/ipc.go +++ b/storagedriver/ipc/ipc.go @@ -31,9 +31,9 @@ func (e IncompatibleVersionError) Error() string { // Request defines a remote method call request // A return value struct is to be sent over the ResponseChannel type Request struct { - Type string - Parameters map[string]interface{} - ResponseChannel libchan.Sender + Type string `codec:",omitempty"` + Parameters map[string]interface{} `codec:",omitempty"` + ResponseChannel libchan.Sender `codec:",omitempty"` } // ResponseError is a serializable error type. @@ -41,9 +41,9 @@ type Request struct { // client side, falling back to using the Type and Message if this cannot be // done. type ResponseError struct { - Type string - Message string - Parameters map[string]interface{} + Type string `codec:",omitempty"` + Message string `codec:",omitempty"` + Parameters map[string]interface{} `codec:",omitempty"` } // WrapError wraps an error in a serializable struct containing the error's type @@ -108,39 +108,39 @@ func (err *ResponseError) Error() string { // VersionResponse is a response for a Version request type VersionResponse struct { - Version storagedriver.Version - Error *ResponseError + Version storagedriver.Version `codec:",omitempty"` + Error *ResponseError `codec:",omitempty"` } // ReadStreamResponse is a response for a ReadStream request type ReadStreamResponse struct { - Reader io.ReadCloser - Error *ResponseError + Reader io.ReadCloser `codec:",omitempty"` + Error *ResponseError `codec:",omitempty"` } // WriteStreamResponse is a response for a WriteStream request type WriteStreamResponse struct { - Error *ResponseError + Error *ResponseError `codec:",omitempty"` } // CurrentSizeResponse is a response for a CurrentSize request type CurrentSizeResponse struct { - Position uint64 - Error *ResponseError + Position uint64 `codec:",omitempty"` + Error *ResponseError `codec:",omitempty"` } // ListResponse is a response for a List request type ListResponse struct { - Keys []string - Error *ResponseError + Keys []string `codec:",omitempty"` + Error *ResponseError `codec:",omitempty"` } // MoveResponse is a response for a Move request type MoveResponse struct { - Error *ResponseError + Error *ResponseError `codec:",omitempty"` } // DeleteResponse is a response for a Delete request type DeleteResponse struct { - Error *ResponseError + Error *ResponseError `codec:",omitempty"` } diff --git a/storagedriver/ipc/server.go b/storagedriver/ipc/server.go index 71422f93..7d1876ca 100644 --- a/storagedriver/ipc/server.go +++ b/storagedriver/ipc/server.go @@ -106,7 +106,7 @@ func handleRequest(driver storagedriver.StorageDriver, request Request) { if err != nil { response = ReadStreamResponse{Error: WrapError(err)} } else { - response = ReadStreamResponse{Reader: ioutil.NopCloser(reader)} + response = ReadStreamResponse{Reader: reader} } err = request.ResponseChannel.Send(&response) if err != nil { diff --git a/storagedriver/testsuites/testsuites.go b/storagedriver/testsuites/testsuites.go index 45633d10..4c86b05a 100644 --- a/storagedriver/testsuites/testsuites.go +++ b/storagedriver/testsuites/testsuites.go @@ -253,7 +253,7 @@ func (suite *DriverSuite) TestReadNonexistentStream(c *check.C) { // TestList checks the returned list of keys after populating a directory tree func (suite *DriverSuite) TestList(c *check.C) { - rootDirectory := randomString(uint64(8 + rand.Intn(8))) + rootDirectory := "/" + randomString(uint64(8+rand.Intn(8))) defer suite.StorageDriver.Delete(rootDirectory) parentDirectory := rootDirectory + "/" + randomString(uint64(8+rand.Intn(8))) @@ -266,7 +266,11 @@ func (suite *DriverSuite) TestList(c *check.C) { } sort.Strings(childFiles) - keys, err := suite.StorageDriver.List(rootDirectory) + keys, err := suite.StorageDriver.List("/") + c.Assert(err, check.IsNil) + c.Assert(keys, check.DeepEquals, []string{rootDirectory}) + + keys, err = suite.StorageDriver.List(rootDirectory) c.Assert(err, check.IsNil) c.Assert(keys, check.DeepEquals, []string{parentDirectory})