Merge pull request #1187 from stevvooe/check-storage-drivers-list-path-not-found

[WIP] registry/storage/driver: checking that non-existent path returns PathNotFoundError

registry/storage/driver/filesystem/driver.go

@@ -184,9 +184,6 @@ func (d *driver) Stat(ctx context.Context, subPath string) (storagedriver.FileIn
 // List returns a list of the objects that are direct descendants of the given
 // path.
 func (d *driver) List(ctx context.Context, subPath string) ([]string, error) {
-	if subPath[len(subPath)-1] != '/' {
-		subPath += "/"
-	}
 	fullPath := d.fullPath(subPath)
 
 	dir, err := os.Open(fullPath)

registry/storage/driver/oss/oss.go

@@ -651,8 +651,9 @@ func (d *driver) Stat(ctx context.Context, path string) (storagedriver.FileInfo,
 }
 
 // List returns a list of the objects that are direct descendants of the given path.
-func (d *driver) List(ctx context.Context, path string) ([]string, error) {
+func (d *driver) List(ctx context.Context, opath string) ([]string, error) {
-	if path != "/" && path[len(path)-1] != '/' {
+	path := opath
+	if path != "/" && opath[len(path)-1] != '/' {
 		path = path + "/"
 	}
 
@@ -666,7 +667,7 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) {
 
 	listResponse, err := d.Bucket.List(d.ossPath(path), "/", "", listMax)
 	if err != nil {
-		return nil, err
+		return nil, parseError(opath, err)
 	}
 
 	files := []string{}
@@ -691,6 +692,14 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) {
 		}
 	}
 
+	if opath != "/" {
+		if len(files) == 0 && len(directories) == 0 {
+			// Treat empty response as missing directory, since we don't actually
+			// have directories in s3.
+			return nil, storagedriver.PathNotFoundError{Path: opath}
+		}
+	}
+
 	return append(files, directories...), nil
 }
 

registry/storage/driver/rados/rados.go

@@ -404,7 +404,7 @@ func (d *driver) List(ctx context.Context, dirPath string) ([]string, error) {
 	files, err := d.listDirectoryOid(dirPath)
 
 	if err != nil {
-		return nil, err
+		return nil, storagedriver.PathNotFoundError{Path: dirPath}
 	}
 
 	keys := make([]string, 0, len(files))

registry/storage/driver/s3/s3.go

@@ -667,7 +667,8 @@ func (d *driver) Stat(ctx context.Context, path string) (storagedriver.FileInfo,
 }
 
 // List returns a list of the objects that are direct descendants of the given path.
-func (d *driver) List(ctx context.Context, path string) ([]string, error) {
+func (d *driver) List(ctx context.Context, opath string) ([]string, error) {
+	path := opath
 	if path != "/" && path[len(path)-1] != '/' {
 		path = path + "/"
 	}
@@ -682,7 +683,7 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) {
 
 	listResponse, err := d.Bucket.List(d.s3Path(path), "/", "", listMax)
 	if err != nil {
-		return nil, err
+		return nil, parseError(opath, err)
 	}
 
 	files := []string{}
@@ -707,6 +708,14 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) {
 		}
 	}
 
+	if opath != "/" {
+		if len(files) == 0 && len(directories) == 0 {
+			// Treat empty response as missing directory, since we don't actually
+			// have directories in s3.
+			return nil, storagedriver.PathNotFoundError{Path: opath}
+		}
+	}
+
 	return append(files, directories...), nil
 }
 

registry/storage/driver/storagedriver.go

@@ -97,7 +97,7 @@ type ErrUnsupportedMethod struct {
 }
 
 func (err ErrUnsupportedMethod) Error() string {
-	return fmt.Sprintf("[%s] unsupported method", err.DriverName)
+	return fmt.Sprintf("%s: unsupported method", err.DriverName)
 }
 
 // PathNotFoundError is returned when operating on a nonexistent path.
@@ -107,7 +107,7 @@ type PathNotFoundError struct {
 }
 
 func (err PathNotFoundError) Error() string {
-	return fmt.Sprintf("[%s] Path not found: %s", err.DriverName, err.Path)
+	return fmt.Sprintf("%s: Path not found: %s", err.DriverName, err.Path)
 }
 
 // InvalidPathError is returned when the provided path is malformed.
@@ -117,7 +117,7 @@ type InvalidPathError struct {
 }
 
 func (err InvalidPathError) Error() string {
-	return fmt.Sprintf("[%s] Invalid path: %s", err.DriverName, err.Path)
+	return fmt.Sprintf("%s: invalid path: %s", err.DriverName, err.Path)
 }
 
 // InvalidOffsetError is returned when attempting to read or write from an
@@ -129,7 +129,7 @@ type InvalidOffsetError struct {
 }
 
 func (err InvalidOffsetError) Error() string {
-	return fmt.Sprintf("[%s] Invalid offset: %d for path: %s", err.DriverName, err.Offset, err.Path)
+	return fmt.Sprintf("%s: invalid offset: %d for path: %s", err.DriverName, err.Offset, err.Path)
 }
 
 // Error is a catch-all error type which captures an error string and
@@ -140,5 +140,5 @@ type Error struct {
 }
 
 func (err Error) Error() string {
-	return fmt.Sprintf("[%s] %s", err.DriverName, err.Enclosed)
+	return fmt.Sprintf("%s: %s", err.DriverName, err.Enclosed)
 }

registry/storage/driver/swift/swift.go

@@ -589,7 +589,7 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) {
 		files = append(files, strings.TrimPrefix(strings.TrimSuffix(obj.Name, "/"), d.swiftPath("/")))
 	}
 
-	if err == swift.ContainerNotFound {
+	if err == swift.ContainerNotFound || (len(objects) == 0 && path != "/") {
 		return files, storagedriver.PathNotFoundError{Path: path}
 	}
 	return files, err

registry/storage/driver/testsuites/testsuites.go

@@ -472,6 +472,13 @@ func (suite *DriverSuite) TestList(c *check.C) {
 	rootDirectory := "/" + randomFilename(int64(8+rand.Intn(8)))
 	defer suite.StorageDriver.Delete(suite.ctx, rootDirectory)
 
+	doesnotexist := path.Join(rootDirectory, "nonexistent")
+	_, err := suite.StorageDriver.List(suite.ctx, doesnotexist)
+	c.Assert(err, check.Equals, storagedriver.PathNotFoundError{
+		Path:       doesnotexist,
+		DriverName: suite.StorageDriver.Name(),
+	})
+
 	parentDirectory := rootDirectory + "/" + randomFilename(int64(8+rand.Intn(8)))
 	childFiles := make([]string, 50)
 	for i := 0; i < len(childFiles); i++ {