Repository: openwisp/openwisp-firmware-upgrader (language: python)

Issue:
[bug] Fix hardcoded storage class for private storage
This code makes it impossible to configure a different storage class (eg: object storage, Amazon S3) for the private storage files:

https://github.com/openwisp/openwisp-firmware-upgrader/blob/e1669998e9ad9b5ede5fc54c2be69cabc494cb40/openwisp_firmware_upgrader/base/models.py#L183-L185


Discussion / hints:

Fix patch:
diff --git a/.gitignore b/.gitignore
index 237877b3..fe98efe8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -47,8 +47,8 @@ coverage.xml
 # Django stuff:
 *.log
 tests/openwisp2/media/
-tests/openwisp2/firmware/
-!tests/openwisp2/firmware/fake-img*.bin
+tests/openwisp2/private/
+!tests/openwisp2/private/firmware/fake-img*.bin
 
 # Sphinx documentation
 docs/_build/
diff --git a/README.rst b/README.rst
index 7c9e3769..70e52045 100644
--- a/README.rst
+++ b/README.rst
@@ -805,6 +805,22 @@ If you need to use a `custom upgrader class <#writing-custom-firmware-upgrader-c
 you will need to use this setting to provide an entry with the class path of your upgrader
 as the value.
 
+``OPENWISP_FIRMWARE_PRIVATE_STORAGE_INSTANCE``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
++--------------+-------------------------------------------------------------------------------------+
+| **type**:    | ``str``                                                                             |
++--------------+-------------------------------------------------------------------------------------+
+| **default**: |  ``openwisp_firmware_upgrader.private_storage.storage.file_system_private_storage`` |
++--------------+-------------------------------------------------------------------------------------+
+
+Dotted path to an instance of any one of the storage classes in
+`private_storage <https://github.com/edoburu/django-private-storage#django-private-storage>`_.
+This instance is used to store firmware image files.
+
+By default, an instance of ``private_storage.storage.files.PrivateFileSystemStorage``
+is used.
+
 Extending openwisp-firmware-upgrader
 ------------------------------------
 
diff --git a/openwisp_firmware_upgrader/__init__.py b/openwisp_firmware_upgrader/__init__.py
index 1d6fdd28..4eeba62e 100644
--- a/openwisp_firmware_upgrader/__init__.py
+++ b/openwisp_firmware_upgrader/__init__.py
@@ -1,4 +1,4 @@
-VERSION = (1, 0, 0, 'final')
+VERSION = (1, 0, 1, 'alpha')
 __version__ = VERSION  # alias
 
 
diff --git a/openwisp_firmware_upgrader/base/models.py b/openwisp_firmware_upgrader/base/models.py
index 2bf54140..34d67499 100644
--- a/openwisp_firmware_upgrader/base/models.py
+++ b/openwisp_firmware_upgrader/base/models.py
@@ -2,7 +2,6 @@
 import os
 from decimal import Decimal
 from pathlib import Path
-from urllib.parse import urljoin
 
 import swapper
 from django.conf import settings
@@ -13,7 +12,6 @@
 from django.utils.module_loading import import_string
 from django.utils.translation import gettext_lazy as _
 from private_storage.fields import PrivateFileField
-from private_storage.storage.files import PrivateFileSystemStorage
 
 from openwisp_users.mixins import OrgMixin
 from openwisp_utils.base import TimeStampedEditableModel
@@ -30,7 +28,6 @@
     FIRMWARE_IMAGE_TYPE_CHOICES,
     REVERSE_FIRMWARE_IMAGE_MAP,
 )
-from ..settings import FIRMWARE_API_BASEURL, IMAGE_URL_PATH
 from ..swapper import get_model_name, load_model
 from ..tasks import (
     batch_upgrade_operation,
@@ -180,9 +177,7 @@ class AbstractFirmwareImage(TimeStampedEditableModel):
         'File',
         upload_to=get_build_directory,
         max_file_size=app_settings.MAX_FILE_SIZE,
-        storage=PrivateFileSystemStorage(
-            base_url=urljoin(FIRMWARE_API_BASEURL, IMAGE_URL_PATH)
-        ),
+        storage=app_settings.PRIVATE_STORAGE_INSTANCE,
     )
     type = models.CharField(
         blank=True,
diff --git a/openwisp_firmware_upgrader/private_storage/storage.py b/openwisp_firmware_upgrader/private_storage/storage.py
new file mode 100644
index 00000000..231af25f
--- /dev/null
+++ b/openwisp_firmware_upgrader/private_storage/storage.py
@@ -0,0 +1,9 @@
+from urllib.parse import urljoin
+
+from private_storage.storage.files import PrivateFileSystemStorage
+
+from ..settings import FIRMWARE_API_BASEURL, IMAGE_URL_PATH
+
+file_system_private_storage = PrivateFileSystemStorage(
+    base_url=urljoin(FIRMWARE_API_BASEURL, IMAGE_URL_PATH)
+)
diff --git a/openwisp_firmware_upgrader/settings.py b/openwisp_firmware_upgrader/settings.py
index 5c5c1eb9..a5c22533 100644
--- a/openwisp_firmware_upgrader/settings.py
+++ b/openwisp_firmware_upgrader/settings.py
@@ -1,4 +1,6 @@
 from django.conf import settings
+from django.core.exceptions import ImproperlyConfigured
+from django.utils.module_loading import import_string
 
 from openwisp_controller.connection import settings as conn_settings
 
@@ -28,3 +30,16 @@
 
 # Path of urls that need to be refered in migrations files.
 IMAGE_URL_PATH = 'firmware/'
+
+try:
+    PRIVATE_STORAGE_INSTANCE = import_string(
+        getattr(
+            settings,
+            'OPENWISP_FIRMWARE_PRIVATE_STORAGE_INSTANCE',
+            'openwisp_firmware_upgrader.private_storage.storage.file_system_private_storage',
+        )
+    )
+except ImportError:
+    raise ImproperlyConfigured(
+        'Failed to import FIRMWARE_UPGRADER_PRIVATE_STORAGE_INSTANCE'
+    )


Test patch:
diff --git a/tests/openwisp2/firmware/fake-img.bin b/tests/openwisp2/private/firmware/fake-img.bin
similarity index 100%
rename from tests/openwisp2/firmware/fake-img.bin
rename to tests/openwisp2/private/firmware/fake-img.bin
diff --git a/tests/openwisp2/firmware/fake-img2.bin b/tests/openwisp2/private/firmware/fake-img2.bin
similarity index 100%
rename from tests/openwisp2/firmware/fake-img2.bin
rename to tests/openwisp2/private/firmware/fake-img2.bin
diff --git a/tests/openwisp2/settings.py b/tests/openwisp2/settings.py
index dcd80564..1136fbd7 100644
--- a/tests/openwisp2/settings.py
+++ b/tests/openwisp2/settings.py
@@ -106,7 +106,7 @@
 MEDIA_URL = '/media/'
 MEDIA_ROOT = '{0}/media/'.format(BASE_DIR)
 
-PRIVATE_STORAGE_ROOT = '{0}/firmware/'.format(BASE_DIR)
+PRIVATE_STORAGE_ROOT = os.path.join(BASE_DIR, 'private', 'firmware')
 
 TEMPLATES = [
     {