Pass the source file information to the Move API of the driver

Signed-off-by: Tejaswini Duggaraju <naduggar@microsoft.com>

Format the file using go fmt

registry/storage/blobwriter.go

@@ -321,7 +321,8 @@ func (bw *blobWriter) moveBlob(ctx context.Context, desc distribution.Descriptor
 	// the size here and write a zero-length file to blobPath if this is the
 	// case. For the most part, this should only ever happen with zero-length
 	// blobs.
-	if _, err := bw.blobStore.driver.Stat(ctx, bw.path); err != nil {
+	sourceFileInfo, err := bw.blobStore.driver.Stat(ctx, bw.path)
+	if err != nil {
 		switch err := err.(type) {
 		case storagedriver.PathNotFoundError:
 			// HACK(stevvooe): This is slightly dangerous: if we verify above,
@@ -340,11 +341,16 @@ func (bw *blobWriter) moveBlob(ctx context.Context, desc distribution.Descriptor
 		default:
 			return err // unrelated error
 		}
+	} else {
+		// We will make sure that the size on the blob store matches with the local commited file.
+		if sourceFileInfo.Size() != desc.Size {
+			return distribution.ErrBlobInvalidLength
+		}
 	}
 
 	// TODO(stevvooe): We should also write the mediatype when executing this move.
 
-	return bw.blobStore.driver.Move(ctx, bw.path, blobPath)
+	return bw.blobStore.driver.Move(ctx, bw.path, blobPath, sourceFileInfo)
 }
 
 // removeResources should clean up all resources associated with the upload

registry/storage/driver/azure/azure.go

@@ -291,11 +291,18 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) {
 
 // Move moves an object stored at sourcePath to destPath, removing the original
 // object.
-func (d *driver) Move(ctx context.Context, sourcePath string, destPath string) error {
+func (d *driver) Move(ctx context.Context, sourcePath string, destPath string, sourceFileInfo storagedriver.FileInfo) error {
 	srcBlobRef := d.client.GetContainerReference(d.container).GetBlobReference(sourcePath)
 	sourceBlobURL := srcBlobRef.GetURL()
 	destBlobRef := d.client.GetContainerReference(d.container).GetBlobReference(destPath)
-	err := destBlobRef.Copy(sourceBlobURL, nil)
+	options := &azure.CopyOptions{}
+	// sourceFileInfo can be nil if the source path is not found. If the file info is available
+	// we will ensure it is unmodified during the move operation.
+	if sourceFileInfo != nil {
+		lastModTime := sourceFileInfo.ModTime()
+		options.Source.IfUnmodifiedSince = &lastModTime
+	}
+	err := destBlobRef.Copy(sourceBlobURL, options)
 	if err != nil {
 		if is404(err) {
 			return storagedriver.PathNotFoundError{Path: sourcePath}

registry/storage/driver/base/base.go

@@ -181,7 +181,7 @@ func (base *Base) List(ctx context.Context, path string) ([]string, error) {
 }
 
 // Move wraps Move of underlying storage driver.
-func (base *Base) Move(ctx context.Context, sourcePath string, destPath string) error {
+func (base *Base) Move(ctx context.Context, sourcePath string, destPath string, sourceFileInfo storagedriver.FileInfo) error {
 	ctx, done := dcontext.WithTrace(ctx)
 	defer done("%s.Move(%q, %q", base.Name(), sourcePath, destPath)
 
@@ -192,7 +192,7 @@ func (base *Base) Move(ctx context.Context, sourcePath string, destPath string)
 	}
 
 	start := time.Now()
-	err := base.setDriverName(base.StorageDriver.Move(ctx, sourcePath, destPath))
+	err := base.setDriverName(base.StorageDriver.Move(ctx, sourcePath, destPath, sourceFileInfo))
 	storageAction.WithValues(base.Name(), "Move").UpdateSince(start)
 	return err
 }

registry/storage/driver/base/regulator.go

@@ -157,11 +157,11 @@ func (r *regulator) List(ctx context.Context, path string) ([]string, error) {
 // original object.
 // Note: This may be no more efficient than a copy followed by a delete for
 // many implementations.
-func (r *regulator) Move(ctx context.Context, sourcePath string, destPath string) error {
+func (r *regulator) Move(ctx context.Context, sourcePath string, destPath string, sourceFileInfo storagedriver.FileInfo) error {
 	r.enter()
 	defer r.exit()
 
-	return r.StorageDriver.Move(ctx, sourcePath, destPath)
+	return r.StorageDriver.Move(ctx, sourcePath, destPath, sourceFileInfo)
 }
 
 // Delete recursively deletes all objects stored at "path" and its subpaths.

registry/storage/driver/filesystem/driver.go

@@ -252,7 +252,7 @@ func (d *driver) List(ctx context.Context, subPath string) ([]string, error) {
 
 // Move moves an object stored at sourcePath to destPath, removing the original
 // object.
-func (d *driver) Move(ctx context.Context, sourcePath string, destPath string) error {
+func (d *driver) Move(ctx context.Context, sourcePath string, destPath string, sourceFileInfo storagedriver.FileInfo) error {
 	source := d.fullPath(sourcePath)
 	dest := d.fullPath(destPath)
 

registry/storage/driver/gcs/gcs_test.go

@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"os"
 	"testing"
+	"time"
 
 	dcontext "github.com/docker/distribution/context"
 	storagedriver "github.com/docker/distribution/registry/storage/driver"
@@ -292,6 +293,13 @@ func TestMoveDirectory(t *testing.T) {
 
 	ctx := dcontext.Background()
 	contents := []byte("contents")
+	sourcePath := "/parent/dir"
+	sourceFileInfo := storagedriver.FileInfoInternal{FileInfoFields: storagedriver.FileInfoFields{
+		Path:    sourcePath,
+		Size:    int64(len(contents)),
+		ModTime: time.Now(),
+		IsDir:   false,
+	}}
 	// Create a regular file.
 	err = driver.PutContent(ctx, "/parent/dir/foo", contents)
 	if err != nil {
@@ -304,7 +312,7 @@ func TestMoveDirectory(t *testing.T) {
 		}
 	}()
 
-	err = driver.Move(ctx, "/parent/dir", "/parent/other")
+	err = driver.Move(ctx, sourcePath, "/parent/other", sourceFileInfo)
 	if err == nil {
 		t.Fatalf("Moving directory /parent/dir /parent/other should have return a non-nil error\n")
 	}

registry/storage/driver/inmemory/driver.go

@@ -207,7 +207,7 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) {
 
 // Move moves an object stored at sourcePath to destPath, removing the original
 // object.
-func (d *driver) Move(ctx context.Context, sourcePath string, destPath string) error {
+func (d *driver) Move(ctx context.Context, sourcePath string, destPath string, sourceFileInfo storagedriver.FileInfo) error {
 	d.mutex.Lock()
 	defer d.mutex.Unlock()
 

registry/storage/driver/oss/oss.go

@@ -407,7 +407,7 @@ const maxConcurrency = 10
 
 // Move moves an object stored at sourcePath to destPath, removing the original
 // object.
-func (d *driver) Move(ctx context.Context, sourcePath string, destPath string) error {
+func (d *driver) Move(ctx context.Context, sourcePath string, destPath string, sourceFileInfo storagedriver.FileInfo) error {
 	logrus.Infof("Move from %s to %s", d.ossPath(sourcePath), d.ossPath(destPath))
 	err := d.Bucket.CopyLargeFileInParallel(d.ossPath(sourcePath), d.ossPath(destPath),
 		d.getContentType(),

registry/storage/driver/s3-aws/s3.go

@@ -696,7 +696,7 @@ func (d *driver) List(ctx context.Context, opath string) ([]string, error) {
 
 // Move moves an object stored at sourcePath to destPath, removing the original
 // object.
-func (d *driver) Move(ctx context.Context, sourcePath string, destPath string) error {
+func (d *driver) Move(ctx context.Context, sourcePath string, destPath string, sourceFileInfo storagedriver.FileInfo) error {
 	/* This is terrible, but aws doesn't have an actual move. */
 	if err := d.copy(ctx, sourcePath, destPath); err != nil {
 		return err

registry/storage/driver/s3-aws/s3_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"strconv"
 	"testing"
+	"time"
 
 	"gopkg.in/check.v1"
 
@@ -303,7 +304,14 @@ func TestMoveWithMultipartCopy(t *testing.T) {
 		t.Fatalf("unexpected error creating content: %v", err)
 	}
 
-	err = d.Move(ctx, sourcePath, destPath)
+	sourceFileInfo := storagedriver.FileInfoInternal{FileInfoFields: storagedriver.FileInfoFields{
+		Path:    sourcePath,
+		Size:    int64(len(contents)),
+		ModTime: time.Now(),
+		IsDir:   false,
+	}}
+
+	err = d.Move(ctx, sourcePath, destPath, sourceFileInfo)
 	if err != nil {
 		t.Fatalf("unexpected error moving file: %v", err)
 	}

registry/storage/driver/storagedriver.go

@@ -73,7 +73,10 @@ type StorageDriver interface {
 	// original object.
 	// Note: This may be no more efficient than a copy followed by a delete for
 	// many implementations.
-	Move(ctx context.Context, sourcePath string, destPath string) error
+	// We will pass along the source file info so the driver can verify the
+	// parity of the moved object.
+	// Note: the source file info can be nil in the case the source path is not found.
+	Move(ctx context.Context, sourcePath string, destPath string, sourceFileInfo FileInfo) error
 
 	// Delete recursively deletes all objects stored at "path" and its subpaths.
 	Delete(ctx context.Context, path string) error

registry/storage/driver/swift/swift.go

@@ -498,7 +498,7 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) {
 
 // Move moves an object stored at sourcePath to destPath, removing the original
 // object.
-func (d *driver) Move(ctx context.Context, sourcePath string, destPath string) error {
+func (d *driver) Move(ctx context.Context, sourcePath string, destPath string, sourceFileInfo storagedriver.FileInfo) error {
 	_, headers, err := d.Conn.Object(d.Container, d.swiftPath(sourcePath))
 	if err == nil {
 		if manifest, ok := headers["X-Object-Manifest"]; ok {

registry/storage/driver/testsuites/testsuites.go

@@ -514,6 +514,12 @@ func (suite *DriverSuite) TestMove(c *check.C) {
 	contents := randomContents(32)
 	sourcePath := randomPath(32)
 	destPath := randomPath(32)
+	sourceFileInfo := storagedriver.FileInfoInternal{FileInfoFields: storagedriver.FileInfoFields{
+		Path:    sourcePath,
+		Size:    int64(len(contents)),
+		ModTime: time.Now(),
+		IsDir:   false,
+	}}
 
 	defer suite.deletePath(c, firstPart(sourcePath))
 	defer suite.deletePath(c, firstPart(destPath))
@@ -521,7 +527,7 @@ func (suite *DriverSuite) TestMove(c *check.C) {
 	err := suite.StorageDriver.PutContent(suite.ctx, sourcePath, contents)
 	c.Assert(err, check.IsNil)
 
-	err = suite.StorageDriver.Move(suite.ctx, sourcePath, destPath)
+	err = suite.StorageDriver.Move(suite.ctx, sourcePath, destPath, sourceFileInfo)
 	c.Assert(err, check.IsNil)
 
 	received, err := suite.StorageDriver.GetContent(suite.ctx, destPath)
@@ -541,6 +547,12 @@ func (suite *DriverSuite) TestMoveOverwrite(c *check.C) {
 	destPath := randomPath(32)
 	sourceContents := randomContents(32)
 	destContents := randomContents(64)
+	sourceFileInfo := storagedriver.FileInfoInternal{FileInfoFields: storagedriver.FileInfoFields{
+		Path:    sourcePath,
+		Size:    int64(len(sourceContents)),
+		ModTime: time.Now(),
+		IsDir:   false,
+	}}
 
 	defer suite.deletePath(c, firstPart(sourcePath))
 	defer suite.deletePath(c, firstPart(destPath))
@@ -551,7 +563,7 @@ func (suite *DriverSuite) TestMoveOverwrite(c *check.C) {
 	err = suite.StorageDriver.PutContent(suite.ctx, destPath, destContents)
 	c.Assert(err, check.IsNil)
 
-	err = suite.StorageDriver.Move(suite.ctx, sourcePath, destPath)
+	err = suite.StorageDriver.Move(suite.ctx, sourcePath, destPath, sourceFileInfo)
 	c.Assert(err, check.IsNil)
 
 	received, err := suite.StorageDriver.GetContent(suite.ctx, destPath)
@@ -576,7 +588,7 @@ func (suite *DriverSuite) TestMoveNonexistent(c *check.C) {
 	err := suite.StorageDriver.PutContent(suite.ctx, destPath, contents)
 	c.Assert(err, check.IsNil)
 
-	err = suite.StorageDriver.Move(suite.ctx, sourcePath, destPath)
+	err = suite.StorageDriver.Move(suite.ctx, sourcePath, destPath, nil)
 	c.Assert(err, check.NotNil)
 	c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{})
 	c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true)
@@ -596,7 +608,7 @@ func (suite *DriverSuite) TestMoveInvalid(c *check.C) {
 	defer suite.deletePath(c, "/notadir")
 
 	// Now try to move a non-existent file under it.
-	err = suite.StorageDriver.Move(suite.ctx, "/notadir/foo", "/notadir/bar")
+	err = suite.StorageDriver.Move(suite.ctx, "/notadir/foo", "/notadir/bar", nil)
 	c.Assert(err, check.NotNil) // non-nil error
 }