Test for upload_subfolder path separators

Renamed test paths for uniqueness.
Show which files do exist in the folder.

private_storage/fields.py

@@ -5,6 +5,7 @@ import datetime
 import logging
 import os
 import posixpath
+import warnings
 
 import django
 from django.core.exceptions import ValidationError
@@ -77,13 +78,17 @@ class PrivateFileField(models.FileField):
         # Add our custom subdir function.
         upload_subfolder = self.upload_subfolder
         if upload_subfolder:
+            # Should return a list, so joining can be done in a storage-specific manner.
             extra_dirs = upload_subfolder(instance)
+
+            # Avoid mistakes by developers, no "s/u/b/p/a/t/h/"
             if isinstance(extra_dirs, string_types):
-                # Avoid mistakes by developers, no "s/u/b/p/a/t/h/"
-                path_parts.append(self.storage.get_valid_name(extra_dirs))
-            else:
-                # Support list, so joining can be done in a storage-specific manner.
-                path_parts.extend([self.storage.get_valid_name(dir) for dir in extra_dirs])
+                warnings.warn("{}.{}.upload_subfolder should return a list"
+                              " to avoid path-separator issues.".format(
+                    instance.__class__.__name__, self.name), UserWarning)
+                extra_dirs = os.path.split(extra_dirs)
+
+            path_parts.extend([self.storage.get_valid_name(dir) for dir in extra_dirs])
 
         path_parts.append(self._get_clean_filename(filename))
         if django.VERSION >= (1, 10):

private_storage/tests/models.py

+import os
+
 from django.db import models
 from django.utils.text import slugify
 
@@ -9,7 +11,14 @@ class SimpleDossier(models.Model):
 
 
 class UploadToDossier(models.Model):
-    file = PrivateFileField(upload_to='dossier2')
+    file = PrivateFileField(upload_to='UploadToDossier')
+
+
+class UploadToCallableDossier(models.Model):
+    def upload_to(self, filename):
+        return 'UploadToCallableDossier/test/' + filename
+
+    file = PrivateFileField(upload_to=upload_to)
 
 
 class CustomerDossier(models.Model):
@@ -19,4 +28,14 @@ class CustomerDossier(models.Model):
         return [slugify(self.customer)]
 
     customer = models.CharField(max_length=100)
-    file = PrivateFileField(upload_to='dossier3', upload_subfolder=upload_subfolder)
+    file = PrivateFileField(upload_to='CustomerDossier', upload_subfolder=upload_subfolder)
+
+
+class CustomerDossierJoin(models.Model):
+
+    def upload_subfolder2(self):
+        # Slightly incorrect usage by developers, joining locally.
+        return os.path.join(slugify(self.customer), 'sub2')
+
+    customer = models.CharField(max_length=100)
+    file = PrivateFileField(upload_to='CustomerDossierJoin', upload_subfolder=upload_subfolder2)

private_storage/tests/test_models.py

 from django.core.files.uploadedfile import SimpleUploadedFile
 
-from private_storage.tests.models import CustomerDossier, SimpleDossier, UploadToDossier
+from private_storage.tests.models import CustomerDossier, CustomerDossierJoin, SimpleDossier, \
+    UploadToCallableDossier, UploadToDossier
 from private_storage.tests.utils import PrivateFileTestCase
 
 
@@ -12,8 +13,16 @@ class ModelTests(PrivateFileTestCase):
 
     def test_upload_to(self):
         UploadToDossier.objects.create(file=SimpleUploadedFile('test2.txt', b'test2'))
-        self.assertExists('dossier2', 'test2.txt')
+        self.assertExists('UploadToDossier', 'test2.txt')
+
+    def test_upload_to_callable(self):
+        UploadToCallableDossier.objects.create(file=SimpleUploadedFile('test7.txt', b'test7'))
+        self.assertExists('UploadToCallableDossier', 'test', 'test7.txt')
 
     def test_upload_subfolder(self):
         obj = CustomerDossier.objects.create(customer='cust1', file=SimpleUploadedFile('test3.txt', b'test3'))
-        self.assertExists('dossier3', 'cust1', 'test3.txt')
+        self.assertExists('CustomerDossier', 'cust1', 'test3.txt')
+
+    def test_upload_subfolder_join(self):
+        obj = CustomerDossierJoin.objects.create(customer='cust4', file=SimpleUploadedFile('test6.txt', b'test6'))
+        self.assertExists('CustomerDossierJoin', 'cust4', 'sub2', 'test6.txt')

private_storage/tests/test_views.py

@@ -47,7 +47,7 @@ class ViewTests(PrivateFileTestCase):
             customer='cust2',
             file=SimpleUploadedFile('test5.txt', b'test5')
         )
-        self.assertExists('dossier3', 'cust2', 'test5.txt')
+        self.assertExists('CustomerDossier', 'cust2', 'test5.txt')
 
         request = RequestFactory().get('/cust1/file/')
         request.user = User.objects.create_superuser('admin', 'admin@example.com', 'admin')
@@ -61,7 +61,7 @@ class ViewTests(PrivateFileTestCase):
 
         response = view(
             request,
-            path='dossier3/cust2/test5.txt'
+            path='CustomerDossier/cust2/test5.txt'
         )
         self.assertEqual(list(response.streaming_content), [b'test5'])
         self.assertEqual(response['Content-Type'], 'text/plain')

private_storage/tests/utils.py

@@ -21,4 +21,11 @@ class PrivateFileTestCase(TestCase):
         """
         path = os.path.join(settings.PRIVATE_STORAGE_ROOT, *parts)
         if not os.path.exists(path):
-            raise self.failureException("Path {} does not exist".format(path))
+            all_files = []
+            for root, dirs, files in os.walk(settings.PRIVATE_STORAGE_ROOT):
+                all_files.extend([os.path.join(root, file) for file in files])
+            all_files.sort()
+            raise self.failureException("Path {} does not exist, found:\n{}".format(
+                path,
+                "\n".join(all_files)
+            ))