[ftp] Conform to BaseStorage interface (#1423)

CHANGELOG.rst

@@ -29,6 +29,11 @@ Dropbox
 
 - Add support for Python 3.12 (`#1421`_)
 
+FTP
+---
+
+- Conform to ``BaseStorage`` interface (`#1423`_)
+
 .. _#1399: https://github.com/jschneier/django-storages/pull/1399
 .. _#1381: https://github.com/jschneier/django-storages/pull/1381
 .. _#1402: https://github.com/jschneier/django-storages/pull/1402
@@ -38,6 +43,7 @@ Dropbox
 .. _#1418: https://github.com/jschneier/django-storages/pull/1418
 .. _#1347: https://github.com/jschneier/django-storages/pull/1347
 .. _#1421: https://github.com/jschneier/django-storages/pull/1421
+.. _#1423: https://github.com/jschneier/django-storages/pull/1423
 
 
 1.14.3 (2024-05-04)

docs/backends/ftp.rst

@@ -5,25 +5,53 @@ FTP
 
 This implementation was done preliminary for upload files in admin to remote FTP location and read them back on site by HTTP. It was tested mostly in this configuration, so read/write using FTPStorageFile class may break.
 
+Configuration & Settings
+------------------------
+
+Django 4.2 changed the way file storage objects are configured. In particular, it made it easier to independently configure
+storage backends and add additional ones. To configure multiple storage objects pre Django 4.2 required subclassing the backend
+because the settings were global, now you pass them under the key ``OPTIONS``. For example, to use FTP to save media files on
+Django >= 4.2 you'd define::
+
+
+  STORAGES = {
+      "default": {
+          "BACKEND": "storages.backends.ftp.FTPStorage",
+          "OPTIONS": {
+            ...your_options_here
+          },
+      },
+  }
+
+On Django < 4.2 you'd instead define::
+
+    DEFAULT_FILE_STORAGE = "storages.backends.ftp.FTPStorage"
+
+To use FTP to store static files via ``collectstatic`` on Django >= 4.2 you'd include the ``staticfiles`` key (at the same level as
+``default``) in the ``STORAGES`` dictionary while on Django < 4.2 you'd instead define::
+
+    STATICFILES_STORAGE = "storages.backends.ftp.FTPStorage"
+
+The settings documented in the following sections include both the key for ``OPTIONS`` (and subclassing) as
+well as the global value. Given the significant improvements provided by the new API, migration is strongly encouraged.
+
 Settings
---------
+~~~~~~~~
+
+``location`` or ``FTP_STORAGE_LOCATION``
+
+  **Required**
 
-To use FtpStorage set::
+  Format as a url like ``"{scheme}://{user}:{passwd}@{host}:{port}/"``. Supports both FTP and FTPS connections via scheme.
 
-    # django < 4.2
-    DEFAULT_FILE_STORAGE = 'storages.backends.ftp.FTPStorage'
+``encoding`` or ``FTP_STORAGE_ENCODING``
 
-    # django >= 4.2
-    STORAGES = {"default": {"BACKEND": "storages.backends.ftp.FTPStorage"}}
+  default: ``latin-1``
 
-``FTP_STORAGE_LOCATION``
-    URL of the server that holds the files. Example ``'ftp://<user>:<pass>@<host>:<port>'``
+  File encoding.
 
-``BASE_URL``
-    URL that serves the files stored at this location. Defaults to the value of your ``MEDIA_URL`` setting.
+``base_url`` or ``BASE_URL``
 
-Optional parameters
-~~~~~~~~~~~~~~~~~~~
+  default: ``settings.MEDIA_URL``
 
-``ENCODING``
-    File encoding. Example ``'utf-8'``. Default value ``'latin-1'``
+  Serving base of files.

storages/backends/ftp.py

@@ -24,9 +24,9 @@
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.core.files.base import File
-from django.core.files.storage import Storage
 from django.utils.deconstruct import deconstructible
 
+from storages.base import BaseStorage
 from storages.utils import setting
 
 
@@ -35,24 +35,26 @@ class FTPStorageException(Exception):
 
 
 @deconstructible
-class FTPStorage(Storage):
+class FTPStorage(BaseStorage):
     """FTP Storage class for Django pluggable storage system."""
 
-    def __init__(self, location=None, base_url=None, encoding=None):
-        location = location or setting("FTP_STORAGE_LOCATION")
-        if location is None:
+    def __init__(self, **settings):
+        super().__init__(**settings)
+        if self.location is None:
             raise ImproperlyConfigured(
-                "You must set a location at "
-                "instanciation or at "
-                " settings.FTP_STORAGE_LOCATION'."
+                "You must set a location at instantiation "
+                "or at settings.FTP_STORAGE_LOCATION."
             )
-        self.location = location
-        self.encoding = encoding or setting("FTP_STORAGE_ENCODING") or "latin-1"
-        base_url = base_url or setting("BASE_URL") or settings.MEDIA_URL
-        self._config = self._decode_location(location)
-        self._base_url = base_url
+        self._config = self._decode_location(self.location)
         self._connection = None
 
+    def get_default_settings(self):
+        return {
+            "location": setting("FTP_STORAGE_LOCATION"),
+            "encoding": setting("FTP_STORAGE_ENCODING", "latin-1"),
+            "base_url": setting("BASE_URL", settings.MEDIA_URL),
+        }
+
     def _decode_location(self, location):
         """Return splitted configuration data from location."""
         splitted_url = re.search(
@@ -232,9 +234,9 @@ def size(self, name):
             return 0
 
     def url(self, name):
-        if self._base_url is None:
+        if self.base_url is None:
             raise ValueError("This file is not accessible via a URL.")
-        return urllib.parse.urljoin(self._base_url, name).replace("\\", "/")
+        return urllib.parse.urljoin(self.base_url, name).replace("\\", "/")
 
 
 class FTPStorageFile(File):

tests/test_ftp.py

@@ -4,6 +4,7 @@
 from django.core.exceptions import ImproperlyConfigured
 from django.core.files.base import File
 from django.test import TestCase
+from django.test import override_settings
 
 from storages.backends import ftp
 
@@ -80,6 +81,16 @@ def test_decode_location_error(self):
     def test_decode_location_urlchars_password(self):
         self.storage._decode_location(geturl(pwd="b#r"))
 
+    @override_settings(FTP_STORAGE_LOCATION=URL)
+    def test_override_settings(self):
+        storage = ftp.FTPStorage()
+        self.assertEqual(storage.encoding, "latin-1")
+        with override_settings(FTP_STORAGE_ENCODING="utf-8"):
+            storage = ftp.FTPStorage()
+            self.assertEqual(storage.encoding, "utf-8")
+        storage = ftp.FTPStorage(encoding="utf-8")
+        self.assertEqual(storage.encoding, "utf-8")
+
     @patch("ftplib.FTP")
     def test_start_connection(self, mock_ftp):
         self.storage._start_connection()
@@ -208,7 +219,7 @@ def test_size_error(self, mock_ftp):
 
     def test_url(self):
         with self.assertRaises(ValueError):
-            self.storage._base_url = None
+            self.storage.base_url = None
             self.storage.url("foo")
         self.storage = ftp.FTPStorage(location=URL, base_url="http://foo.bar/")
         self.assertEqual("http://foo.bar/foo", self.storage.url("foo"))