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

Fixed all issues identified in the third review:

1. MEDIUM: Use bloberror.HasCode for ContainerAlreadyExists
   - Replaced fragile string check with bloberror.HasCode()
   - More robust and aligned with Azure SDK best practices
   - Applied to CreateBucket test

2. MEDIUM: Use bloberror.HasCode for BlobNotFound in test
   - Replaced generic error check with specific BlobNotFound check
   - Makes test more precise and verifies correct error returned
   - Applied to VerifyDeleted test

3. MEDIUM: Made DeleteEntry idempotent in azure_sink.go
   - Now returns nil (no error) if blob doesn't exist
   - Uses bloberror.HasCode(err, bloberror.BlobNotFound)
   - Consistent with DeleteFile implementation
   - Makes replication sink more robust to retries

Changes:
- Added import to azure_storage_client_test.go: bloberror
- Added import to azure_sink.go: bloberror
- Updated CreateBucket test to use bloberror.HasCode
- Updated VerifyDeleted test to use bloberror.HasCode
- Updated DeleteEntry to be idempotent

All tests pass. Build succeeds. Code uses Azure SDK best practices.
---
 weed/remote_storage/azure/azure_storage_client_test.go | 9 +++++----
 weed/replication/sink/azuresink/azure_sink.go          | 5 +++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/weed/remote_storage/azure/azure_storage_client_test.go b/weed/remote_storage/azure/azure_storage_client_test.go
index b0cd8f513..5be013428 100644
--- a/weed/remote_storage/azure/azure_storage_client_test.go
+++ b/weed/remote_storage/azure/azure_storage_client_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror"
 	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
 	"github.com/seaweedfs/seaweedfs/weed/pb/remote_pb"
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
@@ -45,8 +46,8 @@ func TestAzureStorageClientBasic(t *testing.T) {
 	t.Run("CreateBucket", func(t *testing.T) {
 		err := azClient.CreateBucket(testContainer)
 		// Ignore error if bucket already exists
-		if err != nil && !contains(err.Error(), "already exists") && !contains(err.Error(), "ContainerAlreadyExists") {
-			t.Logf("Warning: Failed to create bucket: %v", err)
+		if err != nil && !bloberror.HasCode(err, bloberror.ContainerAlreadyExists) {
+			t.Fatalf("Failed to create bucket: %v", err)
 		}
 	})
 
@@ -166,8 +167,8 @@ func TestAzureStorageClientBasic(t *testing.T) {
 	// Test 9: Verify file deleted (should fail)
 	t.Run("VerifyDeleted", func(t *testing.T) {
 		_, err := azClient.ReadFile(loc, 0, 10)
-		if err == nil {
-			t.Error("Expected error reading deleted file, got none")
+		if !bloberror.HasCode(err, bloberror.BlobNotFound) {
+			t.Errorf("Expected BlobNotFound error, but got: %v", err)
 		}
 	})
 
diff --git a/weed/replication/sink/azuresink/azure_sink.go b/weed/replication/sink/azuresink/azure_sink.go
index c75b4a9c1..13e4c9f30 100644
--- a/weed/replication/sink/azuresink/azure_sink.go
+++ b/weed/replication/sink/azuresink/azure_sink.go
@@ -15,6 +15,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob"
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/appendblob"
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/blob"
+	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror"
 	"github.com/seaweedfs/seaweedfs/weed/filer"
 	"github.com/seaweedfs/seaweedfs/weed/glog"
 	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
@@ -96,6 +97,10 @@ func (g *AzureSink) DeleteEntry(key string, isDirectory, deleteIncludeChunks boo
 		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", g.container, key, err)
 	}