From 3cf52f81144941686c71ab4fefdeec4d34965327 Mon Sep 17 00:00:00 2001
From: Chris Lu <chris.lu@gmail.com>
Date: Wed, 8 Oct 2025 21:44:41 -0700
Subject: [PATCH] Address second round of Gemini Code Assist review comments

Fixed all issues identified in the second review:

1. MEDIUM: Added constants for hardcoded values
   - Defined defaultBlockSize (4 MB) and defaultConcurrency (16)
   - Applied to WriteFile UploadStream options
   - Improves maintainability and readability

2. MEDIUM: Made DeleteFile idempotent
   - Now returns nil (no error) if blob doesn't exist
   - Uses bloberror.HasCode(err, bloberror.BlobNotFound)
   - Consistent with idempotent operation expectations

3. Fixed TestToMetadata test failures
   - Test was using lowercase 'x-amz-meta-' but constant is 'X-Amz-Meta-'
   - Updated test to use s3_constants.AmzUserMetaPrefix
   - All tests now pass

Changes:
- Added import: github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror
- Added constants: defaultBlockSize, defaultConcurrency
- Updated WriteFile to use constants
- Updated DeleteFile to be idempotent
- Fixed test to use correct S3 metadata prefix constant

All tests pass. Build succeeds. Code follows Azure SDK best practices.
---
 weed/remote_storage/azure/azure_storage_client.go  | 14 ++++++++++++--
 .../azure/azure_storage_client_test.go             | 11 ++++++-----
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/weed/remote_storage/azure/azure_storage_client.go b/weed/remote_storage/azure/azure_storage_client.go
index 4fa3a8388..b657b5777 100644
--- a/weed/remote_storage/azure/azure_storage_client.go
+++ b/weed/remote_storage/azure/azure_storage_client.go
@@ -11,6 +11,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob"
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/blob"
+	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror"
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/blockblob"
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/container"
 	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
@@ -20,6 +21,11 @@ import (
 	"github.com/seaweedfs/seaweedfs/weed/util"
 )
 
+const (
+	defaultBlockSize   = 4 * 1024 * 1024
+	defaultConcurrency = 16
+)
+
 func init() {
 	remote_storage.RemoteStorageClientMakers["azure"] = new(azureRemoteStorageMaker)
 }
@@ -165,8 +171,8 @@ func (az *azureRemoteStorageClient) WriteFile(loc *remote_pb.RemoteStorageLocati
 	}
 
 	_, err = blobClient.UploadStream(context.Background(), reader, &blockblob.UploadStreamOptions{
-		BlockSize:   4 * 1024 * 1024,
-		Concurrency: 16,
+		BlockSize:   defaultBlockSize,
+		Concurrency: defaultConcurrency,
 		HTTPHeaders: httpHeaders,
 		Metadata:    metadata,
 	})
@@ -238,6 +244,10 @@ func (az *azureRemoteStorageClient) DeleteFile(loc *remote_pb.RemoteStorageLocat
 		DeleteSnapshots: to.Ptr(blob.DeleteSnapshotsOptionTypeInclude),
 	})
 	if err != nil {
+		// Make delete idempotent - don't return error if blob doesn't exist
+		if bloberror.HasCode(err, bloberror.BlobNotFound) {
+			return nil
+		}
 		return fmt.Errorf("azure delete %s%s: %v", loc.Bucket, loc.Path, err)
 	}
 	return
diff --git a/weed/remote_storage/azure/azure_storage_client_test.go b/weed/remote_storage/azure/azure_storage_client_test.go
index e893453c8..b0cd8f513 100644
--- a/weed/remote_storage/azure/azure_storage_client_test.go
+++ b/weed/remote_storage/azure/azure_storage_client_test.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
 	"github.com/seaweedfs/seaweedfs/weed/pb/remote_pb"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
 )
 
 // TestAzureStorageClientBasic tests basic Azure storage client operations
@@ -192,8 +193,8 @@ func TestToMetadata(t *testing.T) {
 		{
 			name: "basic metadata",
 			input: map[string][]byte{
-				"x-amz-meta-key1": []byte("value1"),
-				"x-amz-meta-key2": []byte("value2"),
+				s3_constants.AmzUserMetaPrefix + "key1": []byte("value1"),
+				s3_constants.AmzUserMetaPrefix + "key2": []byte("value2"),
 			},
 			expected: map[string]*string{
 				"key1": stringPtr("value1"),
@@ -203,7 +204,7 @@ func TestToMetadata(t *testing.T) {
 		{
 			name: "metadata with dashes",
 			input: map[string][]byte{
-				"x-amz-meta-content-type": []byte("text/plain"),
+				s3_constants.AmzUserMetaPrefix + "content-type": []byte("text/plain"),
 			},
 			expected: map[string]*string{
 				"content_type": stringPtr("text/plain"),
@@ -212,8 +213,8 @@ func TestToMetadata(t *testing.T) {
 		{
 			name: "non-metadata keys ignored",
 			input: map[string][]byte{
-				"some-other-key":      []byte("ignored"),
-				"x-amz-meta-included": []byte("included"),
+				"some-other-key":                           []byte("ignored"),
+				s3_constants.AmzUserMetaPrefix + "included": []byte("included"),
 			},
 			expected: map[string]*string{
 				"included": stringPtr("included"),