feat

Author: awais786

smart_storages/s3_backend.py

@@ -4,39 +4,55 @@
 
 class BaseSpecialS3Storage(S3Boto3Storage):
     """
-    Generic reusable S3 storage backend.
-    Uses the bucket defined by a Django setting key.
+    Generic reusable S3 storage backend that provides a robust configuration
+    hierarchy for bucket name, region, and options.
     """
 
-    storage_key = None  # e.g. "import_export" or "COURSE_IMPORT_EXPORT_BUCKET"
+    storage_key = None  # e.g. "import_export" or "analytics"
 
     def __init__(self, **kwargs):
-        # Start with sane defaults
-        options = {"querystring_auth": True}
-        bucket_name = None
+        # 1. Validation: Ensure the subclass has set a storage_key
+        if self.storage_key is None:
+            raise NotImplementedError(
+                "Subclasses of BaseSpecialS3Storage must set a 'storage_key' class attribute."
+            )
+
+        # 2. Initialize Options: Start with project-wide/legacy defaults
         options = {
             "querystring_auth": getattr(settings, "AWS_QUERYSTRING_AUTH", True),
             "default_acl": getattr(settings, "AWS_DEFAULT_ACL", None),
             "object_parameters": getattr(settings, "AWS_S3_OBJECT_PARAMETERS", {}),
+            # General region default
+            "region_name": getattr(settings, "AWS_S3_REGION_NAME", None),
         }
 
-        # 1. Try STORAGES dict
-        if hasattr(settings, "STORAGES") and self.storage_key:
+        bucket_name = None
+
+        # 3. Hierarchy for Options and Bucket Name (Prioritizing STORAGES)
+        if hasattr(settings, "STORAGES"):
             storage_conf = settings.STORAGES.get(self.storage_key, {})
+
+            # Update base options with OPTIONS from STORAGES (includes region_name)
             options.update(storage_conf.get("OPTIONS", {}))
-            bucket_name = options.get("bucket_name", None)
 
-        # 2. Fallback to storage_key setting if bucket_name is None
-        if bucket_name is None and self.storage_key:
-            bucket_name = getattr(settings, self.storage_key, None)
+            # Check for bucket_name within the updated options
+            bucket_name = options.get("bucket_name", None)
 
-        # 3. Final fallback to AWS_STORAGE_BUCKET_NAME if still None
+        # 4. Fallback for Bucket Name (Directly to AWS_STORAGE_BUCKET_NAME)
+        # This is the single, final bucket name fallback.
         if bucket_name is None:
             bucket_name = getattr(settings, "AWS_STORAGE_BUCKET_NAME", None)
 
+        # 5. Secondary Fallback for Region (Explicitly check <STORAGE_KEY>_REGION)
+        # This remains useful for specific regional overrides not in STORAGES.
+        if options["region_name"] is None:
+            region_key = f"{self.storage_key.upper()}_REGION"
+            options["region_name"] = getattr(settings, region_key, None)
+
+        # 6. Final Assembly and Override
         options["bucket_name"] = bucket_name
 
-        # 4. Override with any kwargs
+        # Override any setting with explicit kwargs passed to the constructor
         options.update(kwargs)
 
         super().__init__(**options)

tests/settings.py

@@ -9,6 +9,7 @@
 AWS_STORAGE_BUCKET_NAME = "default-bucket"
 AWS_QUERYSTRING_AUTH = False
 AWS_DEFAULT_ACL = "private"
+AWS_S3_REGION_NAME = "us-east-1"  # Or any valid region name
 AWS_S3_OBJECT_PARAMETERS = {"CacheControl": "max-age=3600"}
 
 INSTALLED_APPS = ["django.contrib.auth", "django.contrib.contenttypes", "storages", "smart_storages"]

tests/test_storages.py

@@ -3,14 +3,21 @@
 from django.test import TestCase
 from django.conf import settings
 from storages.backends.s3boto3 import S3Boto3Storage
-
-from smart_storages.s3_backend import BaseSpecialS3Storage
+from smart_storages.s3_backend import BaseSpecialS3Storage  # Assuming this is the module path
 
 # Ensure Django settings are configured for standalone test runs
+# Assuming 'smart_storages.tests.settings' exists and configures AWS_STORAGE_BUCKET_NAME, STORAGES, etc.
 if not settings.configured:
     os.environ.setdefault("DJANGO_SETTINGS_MODULE", "smart_storages.tests.settings")
 
 
+# --- Helper Storage Subclasses ---
+
+class MissingKeyS3Storage(BaseSpecialS3Storage):
+    # Intentional: storage_key = None
+    pass
+
+
 class ImportExportS3Storage(BaseSpecialS3Storage):
     storage_key = "import_export"
 
@@ -19,35 +26,81 @@ class AnalyticsS3Storage(BaseSpecialS3Storage):
     storage_key = "analytics"
 
 
-class DefaultsS3Storage(AnalyticsS3Storage):
-    storage_key = "none"
-
+# --- Test Suite ---
 
 class TestSpecialStorages(TestCase):
+
+    # --- New Tests for Robustness ---
+
+    def test_missing_storage_key_raises_error(self):
+        """The base class must raise NotImplementedError if storage_key is not set."""
+        with self.assertRaises(NotImplementedError):
+            MissingKeyS3Storage()
+
+    def test_region_is_picked_up_from_storages_dict(self):
+        """The class should pick up the region name from the STORAGES dict."""
+        storage = AnalyticsS3Storage()
+        # Assuming settings.STORAGES['analytics']['OPTIONS']['region_name'] is set to 'eu-west-1'
+        self.assertEqual(storage.region_name, settings.STORAGES["analytics"]["OPTIONS"]["region_name"])
+        self.assertEqual(storage.bucket_name, settings.STORAGES["analytics"]["OPTIONS"]["bucket_name"])
+
+    def test_region_falls_back_to_global_setting(self):
+        """If not in STORAGES, region should fallback to AWS_S3_REGION_NAME."""
+        # For this test, we temporarily set the key to something not in STORAGES
+        # and assume AWS_S3_REGION_NAME is globally set (e.g., 'us-east-1').
+
+        # We'll use ImportExportS3Storage, but temporarily clear its region option for the test
+        temp_options = settings.STORAGES["import_export"]["OPTIONS"].copy()
+        temp_options.pop("region_name", None)
+        settings.STORAGES["import_export"]["OPTIONS"] = temp_options
+
+        storage = ImportExportS3Storage()
+        self.assertEqual(storage.region_name, settings.AWS_S3_REGION_NAME)
+
+        # Restore settings to avoid side effects
+        settings.STORAGES["import_export"]["OPTIONS"]["region_name"] = "us-east-1-specific"
+
+    # --- Modified/Existing Tests ---
+
     def test_base_storage_uses_default_bucket(self):
-        """Fallback to AWS_STORAGE_BUCKET_NAME if specific bucket setting is missing."""
-        storage = DefaultsS3Storage()
+        """Fallback to AWS_STORAGE_BUCKET_NAME if specific bucket is not in STORAGES."""
+
+        # To test the fallback properly, we use a key that we assume exists in STORAGES,
+        # but does *not* define a 'bucket_name' option.
+        class TempNoBucketStorage(BaseSpecialS3Storage):
+            storage_key = "default_fallback_test"
+
+        # Temporarily configure a non-existent storage key without a bucket_name
+        settings.STORAGES["default_fallback_test"] = {"BACKEND": "..."}
+
+        storage = TempNoBucketStorage()
+
         self.assertIsInstance(storage, S3Boto3Storage)
         self.assertEqual(storage.bucket_name, settings.AWS_STORAGE_BUCKET_NAME)
+        # Verify defaults from settings are applied
         self.assertFalse(storage.querystring_auth)
         self.assertEqual(storage.default_acl, "private")
         self.assertEqual(storage.object_parameters, {"CacheControl": "max-age=3600"})
+        # Clean up
+        del settings.STORAGES["default_fallback_test"]
 
     def test_custom_bucket_is_used(self):
         """The class should pick up its bucket name from the STORAGES dict."""
         storage = ImportExportS3Storage()
-        self.assertEqual(storage.bucket_name, "import_export")
+        self.assertEqual(storage.bucket_name, settings.STORAGES["import_export"]["OPTIONS"]["bucket_name"])
 
     def test_multiple_subclasses_can_have_different_buckets(self):
         """Each subclass can have its own bucket."""
         export_storage = ImportExportS3Storage()
         analytics_storage = AnalyticsS3Storage()
+
         self.assertNotEqual(export_storage.bucket_name, analytics_storage.bucket_name)
         self.assertEqual(export_storage.bucket_name, settings.STORAGES["import_export"]["OPTIONS"]["bucket_name"])
         self.assertEqual(analytics_storage.bucket_name, settings.STORAGES["analytics"]["OPTIONS"]["bucket_name"])
 
     def test_init_allows_overriding_default_kwargs(self):
         """User-provided kwargs override defaults."""
+        # The default for import_export should be True (assuming settings)
         storage = ImportExportS3Storage(querystring_auth=False)
         self.assertFalse(storage.querystring_auth)
         self.assertEqual(storage.bucket_name, settings.STORAGES["import_export"]["OPTIONS"]["bucket_name"])