Merge pull request #54 from ImperialCollegeLondon/fix/issue49

Fix to address issues with handling pathlib-based paths

django_drf_filepond/apps.py

@@ -14,9 +14,20 @@ class DjangoDrfFilepondConfig(AppConfig):
     verbose_name = 'FilePond Server-side API'
 
     def ready(self):
+        # Get BASE_DIR and process to ensure it works across platforms
+        # Handle py3.5 where pathlib exists but os.path.join can't accept a
+        # pathlib object (ensure we always pass a string to os.path.join)
+        BASE_DIR = local_settings.BASE_DIR
+        try:
+            from pathlib import Path
+            if isinstance(BASE_DIR, Path):
+                BASE_DIR = str(BASE_DIR)
+        except ImportError:
+            pass
+
         upload_tmp = getattr(
             local_settings, 'UPLOAD_TMP',
-            os.path.join(local_settings.BASE_DIR, 'filepond_uploads'))
+            os.path.join(BASE_DIR, 'filepond_uploads'))
 
         LOG.debug('Upload temp directory from top level settings: <%s>'
                   % (upload_tmp))

django_drf_filepond/drf_filepond_settings.py

@@ -21,7 +21,16 @@
 # installed app base directory to use as an alternative.
 BASE_DIR = os.path.dirname(django_drf_filepond.__file__)
 if hasattr(settings, 'BASE_DIR'):
+    # If BASE_DIR is set in the main settings, get it and process it to
+    # handle py3.5 where pathlib exists but os.path.join can't accept a
+    # pathlib object (ensure we always pass a string to os.path.join)
     BASE_DIR = settings.BASE_DIR
+    try:
+        from pathlib import Path
+        if isinstance(BASE_DIR, Path):
+            BASE_DIR = str(BASE_DIR)
+    except ImportError:
+        pass
 
 # The location where uploaded files are temporarily stored. At present,
 # this must be a subdirectory of settings.BASE_DIR

django_drf_filepond/models.py

@@ -19,9 +19,17 @@
 
 LOG = logging.getLogger(__name__)
 
+BASE_DIR = local_settings.BASE_DIR
+try:
+    from pathlib import Path
+    if isinstance(BASE_DIR, Path):
+        BASE_DIR = str(BASE_DIR)
+except ImportError:
+    pass
+
 FILEPOND_UPLOAD_TMP = getattr(
     local_settings, 'UPLOAD_TMP',
-    os.path.join(local_settings.BASE_DIR, 'filepond_uploads'))
+    os.path.join(BASE_DIR, 'filepond_uploads'))
 
 FILEPOND_FILE_STORE_PATH = getattr(local_settings, 'FILE_STORE_PATH', None)
 

django_drf_filepond/utils.py

@@ -1,4 +1,5 @@
 # A module containing some utility functions used by the views and uploaders
+import django_drf_filepond.drf_filepond_settings as local_settings
 from django.contrib.auth.models import AnonymousUser
 import shortuuid
 import six
@@ -18,3 +19,27 @@ def _get_user(request):
 def _get_file_id():
     file_id = shortuuid.uuid()
     return six.ensure_text(file_id)
+
+
+# Get the BASE_DIR variable from local_settings and process it to ensure that
+# it can be used in django_drf_filepond across Python 2.7, 3.5 and 3.6+.
+# Need to take into account that this may be a regular string or a
+# pathlib.Path object. django-drf-filepond expects to work with BASE_DIR as a
+# string so return a string regardless of the type of BASE_DIR. To maintain
+# suport for Python 2.7, need to handle the case where pathlib.Path doesn't
+# exist...
+def get_local_settings_base_dir():
+    base_dir = local_settings.BASE_DIR
+    return _process_base_dir(base_dir)
+
+
+# Process the provided BASE_DIR variable
+def _process_base_dir(base_dir):
+    try:
+        from pathlib import Path
+    except ImportError:
+        return base_dir
+
+    if isinstance(base_dir, Path):
+        return str(base_dir)
+    return base_dir

django_drf_filepond/views.py

@@ -30,7 +30,8 @@
 from rest_framework.response import Response
 from rest_framework.views import APIView
 from django_drf_filepond.uploaders import FilepondFileUploader
-from django_drf_filepond.utils import _get_file_id, _get_user
+from django_drf_filepond.utils import _get_file_id, _get_user,\
+    get_local_settings_base_dir
 
 LOG = logging.getLogger(__name__)
 
@@ -104,8 +105,9 @@ def post(self, request):
         # TODO: Check whether this is necessary - maybe add a security
         # parameter that can be disabled to turn off this check if the
         # developer wishes?
-        if ((not (storage.location).startswith(local_settings.BASE_DIR)) and
-                (local_settings.BASE_DIR !=
+        LOCAL_BASE_DIR = get_local_settings_base_dir()
+        if ((not (storage.location).startswith(LOCAL_BASE_DIR)) and
+                (LOCAL_BASE_DIR !=
                  os.path.dirname(django_drf_filepond.__file__))):
             if not local_settings.ALLOW_EXTERNAL_UPLOAD_DIR:
                 return Response('The file upload path settings are not '

setup.py

@@ -5,7 +5,7 @@
 
 setup(
     name="django-drf-filepond",
-    version="0.3.0",
+    version="0.3.1",
     description="Filepond server app for Django REST Framework",
     long_description=long_description,
     long_description_content_type='text/markdown',
@@ -26,9 +26,11 @@
         "Django>=2.2;python_version>='3.6'",
         "djangorestframework==3.9.3;python_version=='2.7'",
         "djangorestframework>=3.9.3;python_version>='3.5'",
-        "shortuuid>=0.5.0",
+        "shortuuid==0.5.0;python_version=='2.7'",
+        "shortuuid>=0.5.0;python_version>='3.5'",
         "requests>=2.20.1",
-        "django-storages>=1.8",
+        "django-storages==1.9.1;python_version=='2.7'",
+        "django-storages>=1.9.1;python_version>='3.5'",
         "six>=1.14.0"
     ],
     tests_require=[

tests/settings.py

@@ -14,6 +14,12 @@
 
 # Build paths inside the project like this: os.path.join(BASE_DIR, ...)
 BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+# For testing with pathlib.Path based BASE_DIR
+# from pathlib import Path
+# BASE_DIR = Path(__file__).resolve().parent.parent
+
+# Get a string representation of BASE_DIR in case it's provided as a Path
+BASE_DIR_STR = str(BASE_DIR)
 
 # Quick-start development settings - unsuitable for production
 # See https://docs.djangoproject.com/en/2.1/howto/deployment/checklist/
@@ -76,7 +82,7 @@
 DATABASES = {
     'default': {
         'ENGINE': 'django.db.backends.sqlite3',
-        'NAME': os.path.join(BASE_DIR, 'filepond_tests.db'),
+        'NAME': os.path.join(BASE_DIR_STR, 'filepond_tests.db'),
     }
 }
 
@@ -126,7 +132,7 @@
 # The URL base used for the URL import
 URL_BASE = r'^fp/'
 
-DJANGO_DRF_FILEPOND_FILE_STORE_PATH = os.path.join(BASE_DIR, 'filestore')
+DJANGO_DRF_FILEPOND_FILE_STORE_PATH = os.path.join(BASE_DIR_STR, 'filestore')
 
 LOGGING = {
     'version': 1,

tests/test_process_view.py

@@ -26,6 +26,8 @@
     from mock import patch, MagicMock, ANY
 
 LOG = logging.getLogger(__name__)
+
+
 #
 # New tests for checking file storage outside of BASE_DIR (see #18)
 #
@@ -44,8 +46,10 @@
 # test_new_chunked_upload_request: Check that a new chunked upload request
 #    results in handle_upload being called on the chunked uploader class.
 #
-
-
+# UPDATE: June 2021:
+# test_process_data_BASE_DIR_pathlib: Tests the upload process when BASE_DIR
+#    is set as a pathlib.Path object as it is by default in more recent Django
+#    versions - at present, django-drf-filepond uses regular strings for paths
 class ProcessTestCase(TestCase):
 
     def setUp(self):
@@ -57,6 +61,31 @@ def setUp(self):
     def test_process_data(self):
         self._process_data()
 
+    def test_process_data_BASE_DIR_pathlib(self):
+        # In recent Django versions, BASE_DIR is set by default to
+        # Path(__file__).resolve().parent.parent when creating a new project
+        # using django-admin. Older versions of Django, in use when
+        # django-drf-filepond was originally created used regular strings
+        # for paths. Need to be able to handle both.
+        # Set modified BASE_DIR using context manager - on older Python
+        # versions that don't have pathlib support, fall back to strings
+        OLD_BASE_DIR = drf_filepond_settings.BASE_DIR
+        try:
+            from pathlib import Path
+            NEW_BASE_DIR = Path(__file__).resolve().parent.parent
+            LOG.debug('PATHLIB TEST: Old BASE_DIR: %s   NEW_BASE_DIR: %s' %
+                      (repr(drf_filepond_settings.BASE_DIR),
+                       repr(NEW_BASE_DIR)))
+            drf_filepond_settings.BASE_DIR = NEW_BASE_DIR
+        except ImportError:
+            LOG.debug('NO PATHLIB SUPPORT FOR PATHLIB TEST. '
+                      'FALLING BACK TO USING REGULAR STRING PATHS...')
+
+        try:
+            self._process_data()
+        finally:
+            drf_filepond_settings.BASE_DIR = OLD_BASE_DIR
+
     def test_UPLOAD_TMP_not_set(self):
         upload_tmp = drf_filepond_settings.UPLOAD_TMP
         delattr(drf_filepond_settings, 'UPLOAD_TMP')

tests/test_utils.py

@@ -7,7 +7,9 @@
 import shortuuid
 from six import text_type
 
-from django_drf_filepond.utils import _get_user, _get_file_id
+import django_drf_filepond.drf_filepond_settings as local_settings
+from django_drf_filepond.utils import _get_user, _get_file_id, \
+    get_local_settings_base_dir
 
 
 # Python 2/3 support
@@ -27,6 +29,15 @@
 # test_get_file_id: Test that get_file_id returns an ID that corresponds to
 #    the 22-character specification.
 #
+# test_get_base_dir_with_str: Test that when the local settings BASE_DIR
+#    is a string, a string is returned.
+#
+# test_get_base_dir_with_path: Test that when the local settings BASE_DIR
+#    is a Path object, a string is returned.
+#
+# test_get_base_dir_join_path: Test that when the local settings BASE_DIR
+#    is a Path object, a string is returned.
+#
 class UtilsTestCase(TestCase):
 
     def test_get_user_regular(self):
@@ -48,3 +59,35 @@ def test_get_file_id(self):
         id_format = re.compile('^([%s]){22}$' % (shortuuid.get_alphabet()))
         self.assertRegex(fid, id_format, ('The generated ID does not match '
                                           'the defined ID format.'))
+
+    def test_get_base_dir_with_str(self):
+        test_dir_name = '/tmp/testdir'
+        old_base_dir = local_settings.BASE_DIR
+        try:
+            local_settings.BASE_DIR = test_dir_name
+            bd = get_local_settings_base_dir()
+            self.assertIsInstance(
+                bd, str, 'The base directory is not a string.')
+            self.assertEqual(
+                bd, test_dir_name, 'The test directory name doesn\'t match.')
+        finally:
+            local_settings.BASE_DIR = old_base_dir
+
+    def test_get_base_dir_with_path(self):
+        try:
+            from pathlib import Path
+            test_dir_name = Path('/tmp/testdir')
+        except ImportError:
+            test_dir_name = '/tmp/testdir'
+
+        old_base_dir = local_settings.BASE_DIR
+        try:
+            local_settings.BASE_DIR = test_dir_name
+            bd = get_local_settings_base_dir()
+            self.assertIsInstance(
+                bd, str, 'The base directory is not a string.')
+            self.assertEqual(
+                bd, str(test_dir_name),
+                'The test directory name doesn\'t match.')
+        finally:
+            local_settings.BASE_DIR = old_base_dir