Merge pull request #1338 from TOMToolkit/1185-sanitize-dataproduct-paths

Sanitize data product paths

Author: Fingel

tom_dataproducts/models.py

@@ -6,7 +6,7 @@
 from django.conf import settings
 from django.core.files import File
 from django.db import models
-from django.utils import timezone
+from django.utils import timezone, text
 from django.core.exceptions import ValidationError
 from fits2image.conversions import fits_to_jpg
 from PIL import Image
@@ -113,10 +113,13 @@ def data_product_path(instance, filename):
         return clazz(instance, filename)
     except AttributeError:
         # Uploads go to MEDIA_ROOT
+        # Slugify ensures strings are both FS and URL safe.
+        name = text.slugify(instance.target.name)
         if instance.observation_record is not None:
-            return f'{instance.target.name}/{instance.observation_record.facility}/{filename}'
+            facility = text.slugify(instance.observation_record.facility)
+            return f'{name}/{facility}/{filename}'
         else:
-            return f'{instance.target.name}/none/{filename}'
+            return f'{name}/none/{filename}'
 
 
 class DataProductGroup(models.Model):

tom_dataproducts/tests/tests.py

@@ -14,7 +14,7 @@
 from django.core.exceptions import ValidationError
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.urls import reverse
-from django.utils import timezone
+from django.utils import timezone, text
 from guardian.shortcuts import assign_perm
 import numpy as np
 from specutils import Spectrum1D
@@ -171,14 +171,22 @@ def test_no_path_overide(self):
         """Test that the default path is used if no overide is set"""
         filename = 'afile.fits'
         path = data_product_path(self.data_product, filename)
-        self.assertIn(f'FakeRoboticFacility/{filename}', path)
+        self.assertIn(f'{text.slugify(FakeRoboticFacility.name)}/{filename}', path)
 
     @override_settings(DATA_PRODUCT_PATH='tom_dataproducts.tests.tests.dp_path')
     def test_path_overide(self):
         """Test that the overide path is used if set"""
         path = data_product_path(self.data_product, 'afile.fits')
         self.assertIn('new_path/afile.fits', path)
 
+    def test_target_name_with_slashes(self):
+        """Test that a target with a name such as C/2021 G2 (ATLAS) doesn't create
+        mutiple directories in the filesystem. TOMToolkit/tom_base #1185"""
+        self.target.name = 'C/2021 G2 (ATLAS)'
+        self.target.save()
+        path = data_product_path(self.data_product, 'afile.fits')
+        self.assertEqual('c2021-g2-atlas/fakeroboticfacility/afile.fits', path)
+
 
 @override_settings(TOM_FACILITY_CLASSES=['tom_observations.tests.utils.FakeRoboticFacility'],
                    TARGET_PERMISSIONS_ONLY=False)
@@ -309,7 +317,9 @@ def test_upload_data_for_target(self, run_data_processor_mock):
             },
             follow=True
         )
-        self.assertContains(response, 'Successfully uploaded: {0}/none/afile.fits'.format(self.target.name))
+        self.assertContains(response, 'Successfully uploaded: {0}/none/afile.fits'.format(
+            text.slugify(self.target.name)
+        ))
 
     def test_upload_data_for_observation(self, run_data_processor_mock):
         response = self.client.post(
@@ -326,8 +336,8 @@ def test_upload_data_for_observation(self, run_data_processor_mock):
             follow=True
         )
         self.assertContains(response, 'Successfully uploaded: {0}/{1}/bfile.fits'.format(
-            self.target.name, FakeRoboticFacility.name)
-        )
+            text.slugify(self.target.name), text.slugify(FakeRoboticFacility.name)
+        ))
 
 
 class TestDeleteDataProducts(TestCase):