Repository: C2FO/vfs
Pull request #121 (2022-06-30 19:15:55)

Add DeleteAllVersions option support for azure to remove all versions of a file
Added support for delete all versions in Azure.

Approach:
DeleteAllVersions 
- deletes the blob (this moves the latest version to previous versions)
- then loops through all the versions of the blob and deletes each version
   - if blobs have soft deletion enabled, this will mark each version as deleted and Azure will permanently delete them as per soft delete policy
   - if blobs don't have soft deletion enabled, this will immediately deletes all the versions

codecov[bot]: # [Codecov](https://codecov.io/gh/C2FO/vfs/pull/121?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO) Report
> Merging [#121](https://codecov.io/gh/C2FO/vfs/pull/121?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO) (bdb39b8) into [master](https://codecov.io/gh/C2FO/vfs/commit/888c53e5add9e07b77eeb586c2e94bc8d4a8936a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO) (888c53e) will **decrease** coverage by `0.64%`.
> The diff coverage is `22.64%`.

```diff
@@            Coverage Diff             @@
##           master     #121      +/-   ##
==========================================
- Coverage   68.80%   68.16%   -0.65%     
==========================================
  Files          34       34              
  Lines        3783     3835      +52     
==========================================
+ Hits         2603     2614      +11     
- Misses        928      969      +41     
  Partials      252      252              
```


| [Impacted Files](https://codecov.io/gh/C2FO/vfs/pull/121?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO) | Coverage Δ | |
|---|---|---|
| [backend/azure/client.go](https://codecov.io/gh/C2FO/vfs/pull/121/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO#diff-YmFja2VuZC9henVyZS9jbGllbnQuZ28=) | `3.24% <0.00%> (-1.14%)` | :arrow_down: |
| [backend/azure/file.go](https://codecov.io/gh/C2FO/vfs/pull/121/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO#diff-YmFja2VuZC9henVyZS9maWxlLmdv) | `51.17% <90.90%> (+1.91%)` | :arrow_up: |
| [backend/azure/mock\_client.go](https://codecov.io/gh/C2FO/vfs/pull/121/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO#diff-YmFja2VuZC9henVyZS9tb2NrX2NsaWVudC5nbw==) | `74.28% <100.00%> (+1.55%)` | :arrow_up: |
| [backend/os/location.go](https://codecov.io/gh/C2FO/vfs/pull/121/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO#diff-YmFja2VuZC9vcy9sb2NhdGlvbi5nbw==) | `61.16% <0.00%> (ø)` | |
| [backend/sftp/options.go](https://codecov.io/gh/C2FO/vfs/pull/121/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO#diff-YmFja2VuZC9zZnRwL29wdGlvbnMuZ28=) | `82.23% <0.00%> (ø)` | |
| [backend/sftp/location.go](https://codecov.io/gh/C2FO/vfs/pull/121/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO#diff-YmFja2VuZC9zZnRwL2xvY2F0aW9uLmdv) | `84.89% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/C2FO/vfs/pull/121?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/C2FO/vfs/pull/121?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO). Last update [888c53e...bdb39b8](https://codecov.io/gh/C2FO/vfs/pull/121?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO).


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index def55d7..98037ed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 ## [Unreleased]
 
+### Added
+- Add DeleteAllVersions option support for Azure to remove all versions of a file
+
 ### Fixed
 - Fixed golangci-lint issues
 
diff --git a/backend/azure/client.go b/backend/azure/client.go
index 4f29595..e32debd 100644
--- a/backend/azure/client.go
+++ b/backend/azure/client.go
@@ -41,6 +41,9 @@ type Client interface {
 
 	// Delete should delete the file specified by the parameter file.
 	Delete(file vfs.File) error
+
+	// DeleteAllVersions should delete all versions of the file specified by the parameter file.
+	DeleteAllVersions(file vfs.File) error
 }
 
 // DefaultClient is the main implementation that actually makes the calls to Azure Blob Storage
@@ -215,3 +218,54 @@ func (a *DefaultClient) Delete(file vfs.File) error {
 	_, err = blobURL.Delete(context.Background(), azblob.DeleteSnapshotsOptionNone, azblob.BlobAccessConditions{})
 	return err
 }
+
+// DeleteAllVersions deletes all the versions of the given file from Azure Blob Storage.
+// First the file blob is deleted, then each version of the blob is deleted.
+// If soft deletion is enabled for blobs in the storage account, each version will be marked for deletion and will be
+// permanently deleted by Azure as per the soft deletion policy.
+func (a *DefaultClient) DeleteAllVersions(file vfs.File) error {
+	URL, err := url.Parse(file.Location().(*Location).ContainerURL())
+	if err != nil {
+		return err
+	}
+
+	containerURL := azblob.NewContainerURL(*URL, a.pipeline)
+	blobURL := containerURL.NewBlockBlobURL(utils.RemoveLeadingSlash(file.Path()))
+
+	versions, err := a.getBlobVersions(containerURL, utils.RemoveLeadingSlash(file.Location().Path()))
+	if err != nil {
+		return err
+	}
+
+	for _, version := range versions {
+		// Delete a specific version
+		_, err = blobURL.WithVersionID(*version).Delete(context.Background(), azblob.DeleteSnapshotsOptionNone, azblob.BlobAccessConditions{})
+		if err != nil {
+			return err
+		}
+	}
+
+	return err
+}
+
+func (a *DefaultClient) getBlobVersions(containerURL azblob.ContainerURL, blobName string) ([]*string, error) {
+	ctx := context.Background()
+	var versions []*string
+	for marker := (azblob.Marker{}); marker.NotDone(); {
+		listBlob, err := containerURL.ListBlobsFlatSegment(ctx, marker,
+			azblob.ListBlobsSegmentOptions{Prefix: blobName, Details: azblob.BlobListingDetails{Versions: true}})
+		if err != nil {
+			return nil, err
+		}
+
+		marker = listBlob.NextMarker
+
+		for i := range listBlob.Segment.BlobItems {
+			blobItem := listBlob.Segment.BlobItems[i]
+			if blobItem.VersionID != nil {
+				versions = append(versions, blobItem.VersionID)
+			}
+		}
+	}
+	return versions, nil
+}
diff --git a/backend/azure/file.go b/backend/azure/file.go
index 06b1986..6317cf0 100644
--- a/backend/azure/file.go
+++ b/backend/azure/file.go
@@ -14,6 +14,7 @@ import (
 	"github.com/c2fo/vfs/v6"
 	"github.com/c2fo/vfs/v6/backend"
 	"github.com/c2fo/vfs/v6/options"
+	"github.com/c2fo/vfs/v6/options/delete"
 	"github.com/c2fo/vfs/v6/utils"
 )
 
@@ -193,6 +194,9 @@ func (f *File) MoveToFile(file vfs.File) error {
 }
 
 // Delete deletes the file.
+// If DeleteAllVersions option is provided, each version of the file is deleted. NOTE: if soft deletion is enabled,
+// it will mark all versions as soft deleted, and they will be removed by Azure as per soft deletion policy.
+// Returns any error returned by the API.
 func (f *File) Delete(opts ...options.DeleteOption) error {
 	if err := f.Close(); err != nil {
 		return err
@@ -202,7 +206,25 @@ func (f *File) Delete(opts ...options.DeleteOption) error {
 	if err != nil {
 		return err
 	}
-	return client.Delete(f)
+
+	var deleteAllVersions bool
+	for _, o := range opts {
+		switch o.(type) {
+		case delete.DeleteAllVersions:
+			deleteAllVersions = true
+		default:
+		}
+	}
+
+	if err := client.Delete(f); err != nil {
+		return err
+	}
+
+	if deleteAllVersions {
+		return client.DeleteAllVersions(f)
+	}
+
+	return err
 }
 
 // LastModified returns the last modified time as a time.Time
diff --git a/backend/azure/mock_client.go b/backend/azure/mock_client.go
index 926a36e..55e1be8 100644
--- a/backend/azure/mock_client.go
+++ b/backend/azure/mock_client.go
@@ -61,6 +61,11 @@ func (a *MockAzureClient) Delete(file vfs.File) error {
 	return a.ExpectedError
 }
 
+// DeleteAllVersions returns the value of ExpectedError
+func (a *MockAzureClient) DeleteAllVersions(file vfs.File) error {
+	return a.ExpectedError
+}
+
 // MockStorageError is a mock for the azblob.StorageError interface
 type MockStorageError struct {
 	azblob.ResponseError
diff --git a/backend/gs/file.go b/backend/gs/file.go
index 9b7de0a..58fbd94 100644
--- a/backend/gs/file.go
+++ b/backend/gs/file.go
@@ -226,6 +226,15 @@ func (f *File) Delete(opts ...options.DeleteOption) error {
 		}
 	}
 
+	handle, err := f.getObjectHandle()
+	if err != nil {
+		return err
+	}
+	err = handle.Delete(f.fileSystem.ctx)
+	if err != nil {
+		return err
+	}
+
 	if deleteAllVersions {
 		handles, err := f.getObjectGenerationHandles()
 		if err != nil {
@@ -237,14 +246,8 @@ func (f *File) Delete(opts ...options.DeleteOption) error {
 				return err
 			}
 		}
-		return nil
-	} else {
-		handle, err := f.getObjectHandle()
-		if err != nil {
-			return err
-		}
-		return handle.Delete(f.fileSystem.ctx)
 	}
+	return nil
 }
 
 // Touch creates a zero-length file on the vfs.File if no File exists.  Update File's last modified timestamp.
diff --git a/backend/s3/file.go b/backend/s3/file.go
index 61a7098..85df458 100644
--- a/backend/s3/file.go
+++ b/backend/s3/file.go
@@ -205,6 +205,14 @@ func (f *File) Delete(opts ...options.DeleteOption) error {
 		}
 	}
 
+	_, err = client.DeleteObject(&s3.DeleteObjectInput{
+		Key:    &f.key,
+		Bucket: &f.bucket,
+	})
+	if err != nil {
+		return err
+	}
+
 	if deleteAllVersions {
 		objectVersions, err := f.getAllObjectVersions(client)
 		if err != nil {
@@ -220,11 +228,6 @@ func (f *File) Delete(opts ...options.DeleteOption) error {
 				return err
 			}
 		}
-	} else {
-		_, err = client.DeleteObject(&s3.DeleteObjectInput{
-			Key:    &f.key,
-			Bucket: &f.bucket,
-		})
 	}
 
 	return err
diff --git a/docs/azure.md b/docs/azure.md
index 9f7d774..87cbee6 100644
--- a/docs/azure.md
+++ b/docs/azure.md
@@ -194,6 +194,15 @@ func (a *DefaultClient) Delete(file vfs.File) error
 ```
 Delete deletes the given file from Azure Blob Storage.
 
+#### func (*DefaultClient) DeleteAllVersions
+
+```go
+func (a *DefaultClient) Delete(file vfs.File) error
+```
+
+Deletes the file blob using Azure's delete blob api, then each version of the blob is deleted using Azure's delete api. NOTE that if soft deletion is enabled for the blobs in the storage account, each version will be marked as deleted and will get permanently deleted by Azure as per the soft deletion policy. Returns any error returned by the API.
+
+
 #### func (*DefaultClient) Download
 
 ```go
@@ -287,9 +296,9 @@ be done to the new file.
 #### func (*File) Delete
 
 ```go
-func (f *File) Delete() error
+func (f *File) Delete(opts ...options.DeleteOption) error
 ```
-Delete deletes the file.
+Deletes the file using Azure's delete blob api. If opts is of type DeleteAllVersions, after deleting the blob, each version of the blob is deleted using Azure's delete api. NOTE that if soft deletion is enabled for the blobs, each version will be marked as deleted and will get permanently deleted by Azure as per the soft deletion policy. Returns any error returned by the API.
 
 #### func (*File) Exists
 


Test patch:
diff --git a/backend/azure/client_integration_test.go b/backend/azure/client_integration_test.go
index a803e0d..372f4d3 100644
--- a/backend/azure/client_integration_test.go
+++ b/backend/azure/client_integration_test.go
@@ -137,6 +137,35 @@ func (s *ClientIntegrationTestSuite) TestAllTheThings_FileWithPath() {
 	s.Equal("foo/bar/test.txt", list[0])
 }
 
+func (s *ClientIntegrationTestSuite) TestDeleteAllVersions() {
+	fs := NewFileSystem()
+	f, err := fs.NewFile("test-container", "/test.txt")
+	s.NoError(err)
+	client, err := fs.Client()
+	s.NoError(err, "Env variables (AZURE_STORAGE_ACCOUNT, AZURE_STORAGE_ACCESS_KEY) should contain valid azure account credentials")
+
+	// Create the new file
+	err = client.Upload(f, strings.NewReader("Hello!"))
+	s.NoError(err, "The file should be successfully uploaded to azure")
+
+	// Recreate the file
+	err = client.Upload(f, strings.NewReader("Hello world!"))
+	s.NoError(err, "The file should be successfully uploaded to azure")
+
+	// make sure it exists
+	_, err = client.Properties(f.Location().URI(), f.Name())
+	s.NoError(err, "If the file exists no error should be returned")
+
+	// delete it
+	err = client.DeleteAllVersions(f)
+	s.NoError(err, "if the file versions were deleted no error should be returned")
+
+	// make sure the file doesn't exist
+	exists, err := f.Exists()
+	s.NoError(err, "no error should be returned on exists check")
+	s.Equal(false, exists)
+}
+
 func (s *ClientIntegrationTestSuite) TestProperties() {
 	fs := NewFileSystem()
 	f, err := fs.NewFile("test-container", "/foo/bar/test.txt")
diff --git a/backend/azure/file_test.go b/backend/azure/file_test.go
index 7b22cc9..7f480fd 100644
--- a/backend/azure/file_test.go
+++ b/backend/azure/file_test.go
@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/suite"
 
 	"github.com/c2fo/vfs/v6"
+	"github.com/c2fo/vfs/v6/options/delete"
 	"github.com/c2fo/vfs/v6/utils"
 )
 
@@ -188,7 +189,26 @@ func (s *FileTestSuite) TestDelete() {
 	s.NoError(f.Delete(), "The delete should succeed so there should be no error")
 }
 
-func (s *FileTestSuite) TestDelete_NonExistantFile() {
+func (s *FileTestSuite) TestDeleteWithDeleteAllVersionsOption() {
+	client := MockAzureClient{}
+	fs := NewFileSystem().WithClient(&client)
+
+	f, err := fs.NewFile("test-container", "/foo.txt")
+	s.NoError(err, "The path is valid so no error should be returned")
+	s.NoError(f.Delete(delete.WithDeleteAllVersions()), "The delete should succeed so there should be no error")
+}
+
+func (s *FileTestSuite) TestDeleteWithDeleteAllVersionsOption_Error() {
+	client := MockAzureClient{ExpectedError: errors.New("i always error")}
+	fs := NewFileSystem().WithClient(&client)
+
+	f, err := fs.NewFile("test-container", "/foo.txt")
+	s.NoError(err, "The path is valid so no error should be returned")
+	err = f.Delete(delete.WithDeleteAllVersions())
+	s.Error(err, "If the file does not exist we get an error")
+}
+
+func (s *FileTestSuite) TestDelete_NonExistentFile() {
 	client := MockAzureClient{ExpectedError: errors.New("i always error")}
 	fs := NewFileSystem().WithClient(&client)
 
diff --git a/backend/s3/file_test.go b/backend/s3/file_test.go
index 16d8e2d..af9af09 100644
--- a/backend/s3/file_test.go
+++ b/backend/s3/file_test.go
@@ -34,6 +34,8 @@ var (
 	fs             FileSystem
 	testFile       vfs.File
 	defaultOptions Options
+	testFileName   string
+	bucket         string
 )
 
 func (ts *fileTestSuite) SetupTest() {
@@ -41,7 +43,9 @@ func (ts *fileTestSuite) SetupTest() {
 	s3apiMock = &mocks.S3API{}
 	defaultOptions = Options{AccessKeyID: "abc"}
 	fs = FileSystem{client: s3apiMock, options: defaultOptions}
-	testFile, err = fs.NewFile("bucket", "/some/path/to/file.txt")
+	testFileName = "/some/path/to/file.txt"
+	bucket = "bucket"
+	testFile, err = fs.NewFile(bucket, testFileName)
 	getDownloader = func(client s3iface.S3API, partSize int64) Downloader {
 		return mocks.S3MockDownloader{}
 	}
@@ -573,7 +577,7 @@ func (ts *fileTestSuite) TestDeleteWithDeleteAllVersionsOption() {
 	err := testFile.Delete(delete.WithDeleteAllVersions())
 	ts.Nil(err, "Successful delete should not return an error.")
 	s3apiMock.AssertExpectations(ts.T())
-	s3apiMock.AssertNumberOfCalls(ts.T(), "DeleteObject", 2)
+	s3apiMock.AssertNumberOfCalls(ts.T(), "DeleteObject", 3)
 }
 
 func (ts *fileTestSuite) TestDeleteWithDeleteAllVersionsOptionError() {
@@ -586,12 +590,14 @@ func (ts *fileTestSuite) TestDeleteWithDeleteAllVersionsOptionError() {
 		Versions: versions,
 	}
 	s3apiMock.On("ListObjectVersions", mock.AnythingOfType("*s3.ListObjectVersionsInput")).Return(&versOutput, nil)
-	s3apiMock.On("DeleteObject", mock.AnythingOfType("*s3.DeleteObjectInput")).Return(nil, errors.New("something went wrong"))
+	s3apiMock.On("DeleteObject", &s3.DeleteObjectInput{Key: &testFileName, Bucket: &bucket}).Return(&s3.DeleteObjectOutput{}, nil)
+	s3apiMock.On("DeleteObject", &s3.DeleteObjectInput{Key: &testFileName, Bucket: &bucket, VersionId: &verIds[0]}).
+		Return(nil, errors.New("something went wrong"))
 
 	err := testFile.Delete(delete.WithDeleteAllVersions())
 	ts.NotNil(err, "Delete should return an error if s3 api had error.")
 	s3apiMock.AssertExpectations(ts.T())
-	s3apiMock.AssertNumberOfCalls(ts.T(), "DeleteObject", 1)
+	s3apiMock.AssertNumberOfCalls(ts.T(), "DeleteObject", 2)
 }
 
 func (ts *fileTestSuite) TestLastModified() {
diff --git a/backend/s3/location_test.go b/backend/s3/location_test.go
index 9ddf2d5..46e99e8 100644
--- a/backend/s3/location_test.go
+++ b/backend/s3/location_test.go
@@ -317,7 +317,7 @@ func (lt *locationTestSuite) TestDeleteFileWithDeleteAllVersionsOption() {
 	err = loc.DeleteFile("filename.txt", delete.WithDeleteAllVersions())
 	lt.Nil(err, "Successful delete should not return an error.")
 	lt.s3apiMock.AssertExpectations(lt.T())
-	lt.s3apiMock.AssertNumberOfCalls(lt.T(), "DeleteObject", 2)
+	lt.s3apiMock.AssertNumberOfCalls(lt.T(), "DeleteObject", 3)
 }
 
 func TestLocation(t *testing.T) {