From 4630eea10ab8b9938a9bd94eed70f58dbbca2ac2 Mon Sep 17 00:00:00 2001
From: chrislu <chris.lu@gmail.com>
Date: Mon, 14 Jul 2025 22:14:53 -0700
Subject: [PATCH] address comments

---
 weed/s3api/s3api_bucket_config.go        | 36 ++++++++++++++++++++++--
 weed/s3api/s3api_bucket_cors_handlers.go | 28 +++---------------
 2 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/weed/s3api/s3api_bucket_config.go b/weed/s3api/s3api_bucket_config.go
index 57fcafb2e..8cbb190af 100644
--- a/weed/s3api/s3api_bucket_config.go
+++ b/weed/s3api/s3api_bucket_config.go
@@ -314,18 +314,50 @@ func (s3a *S3ApiServer) getCORSConfiguration(bucket string) (*cors.CORSConfigura
 	return config.CORS, s3err.ErrNone
 }
 
+// getCORSStorage returns a CORS storage instance for persistent operations
+func (s3a *S3ApiServer) getCORSStorage() *cors.Storage {
+	entryGetter := &S3EntryGetter{server: s3a}
+	return cors.NewStorage(s3a, entryGetter, s3a.option.BucketsPath)
+}
+
 // updateCORSConfiguration updates CORS configuration and invalidates cache
 func (s3a *S3ApiServer) updateCORSConfiguration(bucket string, corsConfig *cors.CORSConfiguration) s3err.ErrorCode {
-	return s3a.updateBucketConfig(bucket, func(config *BucketConfig) error {
+	// Update in-memory cache
+	errCode := s3a.updateBucketConfig(bucket, func(config *BucketConfig) error {
 		config.CORS = corsConfig
 		return nil
 	})
+	if errCode != s3err.ErrNone {
+		return errCode
+	}
+
+	// Persist to .s3metadata file
+	storage := s3a.getCORSStorage()
+	if err := storage.Store(bucket, corsConfig); err != nil {
+		glog.Errorf("updateCORSConfiguration: failed to persist CORS config to metadata for bucket %s: %v", bucket, err)
+		return s3err.ErrInternalError
+	}
+
+	return s3err.ErrNone
 }
 
 // removeCORSConfiguration removes CORS configuration and invalidates cache
 func (s3a *S3ApiServer) removeCORSConfiguration(bucket string) s3err.ErrorCode {
-	return s3a.updateBucketConfig(bucket, func(config *BucketConfig) error {
+	// Remove from in-memory cache
+	errCode := s3a.updateBucketConfig(bucket, func(config *BucketConfig) error {
 		config.CORS = nil
 		return nil
 	})
+	if errCode != s3err.ErrNone {
+		return errCode
+	}
+
+	// Remove from .s3metadata file
+	storage := s3a.getCORSStorage()
+	if err := storage.Delete(bucket); err != nil {
+		glog.Errorf("removeCORSConfiguration: failed to remove CORS config from metadata for bucket %s: %v", bucket, err)
+		return s3err.ErrInternalError
+	}
+
+	return s3err.ErrNone
 }
diff --git a/weed/s3api/s3api_bucket_cors_handlers.go b/weed/s3api/s3api_bucket_cors_handlers.go
index 9a4cb8d42..e46021d7e 100644
--- a/weed/s3api/s3api_bucket_cors_handlers.go
+++ b/weed/s3api/s3api_bucket_cors_handlers.go
@@ -38,12 +38,6 @@ func (g *S3CORSConfigGetter) GetCORSConfiguration(bucket string) (*cors.CORSConf
 	return g.server.getCORSConfiguration(bucket)
 }
 
-// getCORSStorage returns a CORS storage instance
-func (s3a *S3ApiServer) getCORSStorage() *cors.Storage {
-	entryGetter := &S3EntryGetter{server: s3a}
-	return cors.NewStorage(s3a, entryGetter, s3a.option.BucketsPath)
-}
-
 // getCORSMiddleware returns a CORS middleware instance with caching
 func (s3a *S3ApiServer) getCORSMiddleware() *cors.Middleware {
 	storage := s3a.getCORSStorage()
@@ -111,20 +105,13 @@ func (s3a *S3ApiServer) PutBucketCorsHandler(w http.ResponseWriter, r *http.Requ
 	}
 
 	// Store CORS configuration and update cache
+	// This handles both cache update and persistent storage through the unified bucket config system
 	if err := s3a.updateCORSConfiguration(bucket, &config); err != s3err.ErrNone {
 		glog.Errorf("Failed to update CORS configuration: %v", err)
 		s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)
 		return
 	}
 
-	// Also update the .s3metadata file directly
-	storage := s3a.getCORSStorage()
-	if err := storage.Store(bucket, &config); err != nil {
-		glog.Errorf("Failed to store CORS configuration in metadata: %v", err)
-		s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)
-		return
-	}
-
 	// Return success
 	writeSuccessResponseEmpty(w, r)
 }
@@ -140,17 +127,10 @@ func (s3a *S3ApiServer) DeleteBucketCorsHandler(w http.ResponseWriter, r *http.R
 		return
 	}
 
-	// Remove CORS configuration from cache
+	// Remove CORS configuration from cache and persistent storage
+	// This handles both cache invalidation and persistent storage cleanup through the unified bucket config system
 	if err := s3a.removeCORSConfiguration(bucket); err != s3err.ErrNone {
-		glog.Errorf("Failed to remove CORS configuration from cache: %v", err)
-		s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)
-		return
-	}
-
-	// Also remove from .s3metadata file
-	storage := s3a.getCORSStorage()
-	if err := storage.Delete(bucket); err != nil {
-		glog.Errorf("Failed to delete CORS configuration from metadata: %v", err)
+		glog.Errorf("Failed to remove CORS configuration: %v", err)
 		s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)
 		return
 	}