OpenConceptLab/ocl_issues#2864 | Name export downloads with full repo coordinates

core/common/mixins.py

@@ -1058,7 +1058,9 @@ def get(self, request, *args, **kwargs):  # pylint: disable=unused-argument
 
         if version.has_export():
             export_path = version.get_export_path()
-            export_url = get_export_service().url_for(export_path)
+            download_name = version.get_export_download_name(export_path)
+            logger.info('Export %s downloads as %s', export_path, download_name)
+            export_url = get_export_service().url_for(export_path, filename=download_name)
             if export_url:
                 return redirect(export_url)
             logger.error(

core/common/models.py

@@ -1,4 +1,5 @@
 import logging
+import re
 import time
 
 from celery.result import AsyncResult
@@ -1400,6 +1401,30 @@ def get_export_path(self):
         service = get_export_service()
         return service.get_last_key_from_path(self.get_version_export_path(suffix=None)) or self.version_export_path
 
+    def get_export_download_name(self, export_path):
+        """
+        Name a client saves the export under, set on the signed URL:
+        [owner-type]_[owner]_[repo-type]_[repo]_[repo-version]_[expansion]_[lastUpdated].zip
+        lastUpdated is read from the storage key, so the name matches the export's content. A key without one
+        right after this version's prefix gets no name (None), and the download keeps its storage name. Characters
+        that aren't safe in a filename become '-'.
+        """
+        # current (2026-09-30_123456) or legacy (20260930123456) key timestamp, anchored to the prefix because a key
+        # listed under it can be another version's (1.8.24.1's under 1.8.24's)
+        match = re.fullmatch(
+            re.escape(self.get_version_export_path(suffix=None)) + r'(\d{4})-?(\d{2})-?(\d{2})_?(\d{6})\.zip',
+            export_path)
+        if not match:
+            return None
+
+        parts = (get(self, 'expansion_uri') or self.uri).strip('/').split('/')
+        # the uri percent-encodes the version, HEAD's uri has none and an expansion uri follows it with 'expansions',
+        # so the version comes from the model
+        parts[4:6] = [self.version]
+        year, month, day, time_of_day = match.groups()
+        parts.append(f'{year}-{month}-{day}_{time_of_day}')
+        return '_'.join(re.sub(r'[^A-Za-z0-9._@-]+', '-', part) for part in parts) + '.zip'
+
     def has_export(self):
         service = get_export_service()
         if self.is_head:

core/integration_tests/tests_collections.py

@@ -2700,7 +2700,10 @@ def test_get_302_version(self, s3_has_path_mock, s3_get_last_key_from_path_mock,
         self.assertEqual(response['Location'], 'https://signed.example/coll-v1.zip')
         s3_has_path_mock.assert_called_once_with("users/username/username_coll_v1.")
         s3_get_last_key_from_path_mock.assert_called_once_with("users/username/username_coll_v1.")
-        s3_url_for_mock.assert_called_once_with(f'users/username/username_coll_v1.{self.v1_updated_at}.zip')
+        s3_url_for_mock.assert_called_once_with(
+            f'users/username/username_coll_v1.{self.v1_updated_at}.zip',
+            filename=f'users_username_collections_coll_v1_{self.v1_updated_at}.zip'
+        )
 
     @patch('core.services.storages.cloud.aws.S3.url_for')
     @patch('core.services.storages.cloud.aws.S3.exists')
@@ -2717,7 +2720,10 @@ def test_get_302_head(self, s3_exists_mock, s3_url_for_mock):
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response['Location'], 'https://signed.example/coll-head.zip')
         s3_exists_mock.assert_called_once_with(f"users/username/username_coll_vHEAD.{self.HEAD_updated_at}.zip")
-        s3_url_for_mock.assert_called_once_with(f"users/username/username_coll_vHEAD.{self.HEAD_updated_at}.zip")
+        s3_url_for_mock.assert_called_once_with(
+            f"users/username/username_coll_vHEAD.{self.HEAD_updated_at}.zip",
+            filename=f"users_username_collections_coll_HEAD_{self.HEAD_updated_at}.zip"
+        )
 
     @patch('core.services.storages.cloud.aws.S3.url_for')
     @patch('core.services.storages.cloud.aws.S3.exists')
@@ -2734,7 +2740,10 @@ def test_get_500_head_when_signed_url_generation_fails(self, s3_exists_mock, s3_
         self.assertEqual(response.status_code, 500)
         self.assertEqual(response.data, {'detail': 'Export exists but could not generate a download URL.'})
         s3_exists_mock.assert_called_once_with(f"users/username/username_coll_vHEAD.{self.HEAD_updated_at}.zip")
-        s3_url_for_mock.assert_called_once_with(f"users/username/username_coll_vHEAD.{self.HEAD_updated_at}.zip")
+        s3_url_for_mock.assert_called_once_with(
+            f"users/username/username_coll_vHEAD.{self.HEAD_updated_at}.zip",
+            filename=f"users_username_collections_coll_HEAD_{self.HEAD_updated_at}.zip"
+        )
 
     def test_get_405(self):
         random_user = UserProfileFactory()

core/integration_tests/tests_sources.py

@@ -1169,7 +1169,10 @@ def test_get_302_head(self, s3_exists_mock, s3_url_for_mock):
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response['Location'], 'https://signed.example/head.zip')
         s3_exists_mock.assert_called_once_with(f"users/username/username_source1_vHEAD.{self.HEAD_updated_at}.zip")
-        s3_url_for_mock.assert_called_once_with(f"users/username/username_source1_vHEAD.{self.HEAD_updated_at}.zip")
+        s3_url_for_mock.assert_called_once_with(
+            f"users/username/username_source1_vHEAD.{self.HEAD_updated_at}.zip",
+            filename=f"users_username_sources_source1_HEAD_{self.HEAD_updated_at}.zip"
+        )
 
     @patch('core.services.storages.cloud.aws.S3.url_for')
     @patch('core.services.storages.cloud.aws.S3.get_last_key_from_path')
@@ -1179,17 +1182,26 @@ def test_get_302_version(self, s3_has_path_mock, s3_get_last_key_from_path_mock,
         s3_has_path_mock.return_value = True
         s3_get_last_key_from_path_mock.return_value = f'users/username/username_source1_v1.{self.v1_updated_at}.zip'
 
-        response = self.client.get(
-            self.source_v1.uri + 'export/',
-            HTTP_AUTHORIZATION='Token ' + self.token,
-            format='json'
-        )
+        with self.assertLogs('oclapi', level='INFO') as logs:
+            response = self.client.get(
+                self.source_v1.uri + 'export/',
+                HTTP_AUTHORIZATION='Token ' + self.token,
+                format='json'
+            )
 
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response['Location'], 'https://signed.example/v1.zip')
         s3_has_path_mock.assert_called_once_with("users/username/username_source1_v1.")
         s3_get_last_key_from_path_mock.assert_called_once_with("users/username/username_source1_v1.")
-        s3_url_for_mock.assert_called_once_with(f'users/username/username_source1_v1.{self.v1_updated_at}.zip')
+        s3_url_for_mock.assert_called_once_with(
+            f'users/username/username_source1_v1.{self.v1_updated_at}.zip',
+            filename=f'users_username_sources_source1_v1_{self.v1_updated_at}.zip'
+        )
+        self.assertIn(
+            f'INFO:oclapi:Export users/username/username_source1_v1.{self.v1_updated_at}.zip downloads as '
+            f'users_username_sources_source1_v1_{self.v1_updated_at}.zip',
+            logs.output
+        )
 
     @patch('core.services.storages.cloud.aws.S3.url_for')
     @patch('core.services.storages.cloud.aws.S3.exists')
@@ -1206,7 +1218,10 @@ def test_get_500_head_when_signed_url_generation_fails(self, s3_exists_mock, s3_
         self.assertEqual(response.status_code, 500)
         self.assertEqual(response.data, {'detail': 'Export exists but could not generate a download URL.'})
         s3_exists_mock.assert_called_once_with(f"users/username/username_source1_vHEAD.{self.HEAD_updated_at}.zip")
-        s3_url_for_mock.assert_called_once_with(f"users/username/username_source1_vHEAD.{self.HEAD_updated_at}.zip")
+        s3_url_for_mock.assert_called_once_with(
+            f"users/username/username_source1_vHEAD.{self.HEAD_updated_at}.zip",
+            filename=f"users_username_sources_source1_HEAD_{self.HEAD_updated_at}.zip"
+        )
 
     @patch('core.sources.models.Source.is_exporting', new_callable=PropertyMock)
     @patch('core.services.storages.cloud.aws.S3.exists')

core/repos/tests.py

@@ -7,7 +7,7 @@
 
 from core.collections.documents import CollectionDocument
 from core.collections.models import Collection
-from core.collections.tests.factories import OrganizationCollectionFactory, UserCollectionFactory
+from core.collections.tests.factories import OrganizationCollectionFactory, UserCollectionFactory, ExpansionFactory
 from core.common.constants import HEAD
 from core.common.tests import OCLAPITestCase, OCLTestCase
 from core.orgs.tests.factories import OrganizationFactory
@@ -115,6 +115,108 @@ def test_get_external_export_path_normalizes_invalid_filename_characters(self):
         )
 
 
+class RepoExportDownloadNameTest(OCLTestCase):
+    def test_org_source_version(self):
+        source_v1 = OrganizationSourceFactory(
+            organization=OrganizationFactory(mnemonic='PIH'), mnemonic='PIH', version='1.8.24')
+        export_path = source_v1.get_version_export_path(suffix='2026-09-30_123456.zip')
+
+        self.assertEqual(export_path, 'orgs/PIH/PIH_PIH_v1.8.24.2026-09-30_123456.zip')
+        self.assertEqual(
+            source_v1.get_export_download_name(export_path), 'orgs_PIH_sources_PIH_1.8.24_2026-09-30_123456.zip')
+
+    def test_user_collection_version_with_expansion(self):
+        collection_v1 = UserCollectionFactory(
+            user=UserProfileFactory(username='jdoe'), mnemonic='Concepts', version='1.0')
+        expansion = ExpansionFactory(collection_version=collection_v1, mnemonic='autoexpand-1.0')
+        collection_v1.expansion_uri = expansion.uri
+        collection_v1.save()
+        export_path = collection_v1.get_version_export_path(suffix='2026-09-30_123456.zip')
+
+        self.assertEqual(
+            collection_v1.get_export_download_name(export_path),
+            'users_jdoe_collections_Concepts_1.0_autoexpand-1.0_2026-09-30_123456.zip'
+        )
+
+    def test_source_head(self):
+        source = OrganizationSourceFactory(organization=OrganizationFactory(mnemonic='PIH'), mnemonic='PIH')
+        last_update = source.last_child_update.strftime('%Y-%m-%d_%H%M%S')
+
+        self.assertEqual(
+            source.get_export_download_name(source.version_export_path),
+            f'orgs_PIH_sources_PIH_HEAD_{last_update}.zip'
+        )
+
+    def test_collection_head_with_expansion(self):
+        collection = OrganizationCollectionFactory(
+            organization=OrganizationFactory(mnemonic='PIH'), mnemonic='PIHEMR_Concepts')
+        expansion = ExpansionFactory(collection_version=collection, mnemonic='autoexpand-HEAD')
+        collection.expansion_uri = expansion.uri
+        collection.save()
+        last_update = collection.last_child_update.strftime('%Y-%m-%d_%H%M%S')
+
+        self.assertEqual(
+            collection.get_export_download_name(collection.version_export_path),
+            f'orgs_PIH_collections_PIHEMR_Concepts_HEAD_autoexpand-HEAD_{last_update}.zip'
+        )
+
+    def test_legacy_key_timestamp(self):
+        source_v1 = OrganizationSourceFactory(
+            organization=OrganizationFactory(mnemonic='CIEL'), mnemonic='CIEL', version='v2016-08-22')
+
+        self.assertEqual(
+            source_v1.get_export_download_name('orgs/CIEL/CIEL_CIEL_v2016-08-22.20150516122820.zip'),
+            'orgs_CIEL_sources_CIEL_v2016-08-22_2015-05-16_122820.zip'
+        )
+
+    def test_key_without_timestamp_gets_no_name(self):
+        source_v1 = OrganizationSourceFactory(
+            organization=OrganizationFactory(mnemonic='PIH'), mnemonic='PIH', version='1.8.24')
+
+        self.assertIsNone(source_v1.get_export_download_name('orgs/PIH/PIH_PIH_v1.8.24.zip'))
+
+    def test_date_like_version(self):
+        source_v1 = OrganizationSourceFactory(
+            organization=OrganizationFactory(mnemonic='PIH'), mnemonic='PIH', version='1.20260930123456')
+
+        self.assertEqual(
+            source_v1.get_export_download_name('orgs/PIH/PIH_PIH_v1.20260930123456.2026-10-01_090000.zip'),
+            'orgs_PIH_sources_PIH_1.20260930123456_2026-10-01_090000.zip'
+        )
+        self.assertIsNone(source_v1.get_export_download_name('orgs/PIH/PIH_PIH_v1.20260930123456.zip'))
+        self.assertIsNone(source_v1.get_export_download_name('orgs/PIH/PIH_PIH_v1.20260930123456.1.zip'))
+
+    def test_key_of_another_version_gets_no_name(self):
+        source_v1 = OrganizationSourceFactory(
+            organization=OrganizationFactory(mnemonic='PIH'), mnemonic='PIH', version='1.8.24')
+
+        # get_export_path() lists keys by prefix, so 1.8.24's lookup can return 1.8.24.1's export
+        self.assertIsNone(source_v1.get_export_download_name('orgs/PIH/PIH_PIH_v1.8.24.1.2026-10-01_090000.zip'))
+
+    def test_mnemonics_named_expansions(self):
+        collection_v1 = OrganizationCollectionFactory(
+            organization=OrganizationFactory(mnemonic='expansions'), mnemonic='expansions', version='1.0')
+        expansion = ExpansionFactory(collection_version=collection_v1, mnemonic='expansions')
+        collection_v1.expansion_uri = expansion.uri
+        collection_v1.save()
+        export_path = collection_v1.get_version_export_path(suffix='2026-09-30_123456.zip')
+
+        self.assertEqual(
+            collection_v1.get_export_download_name(export_path),
+            'orgs_expansions_collections_expansions_1.0_expansions_2026-09-30_123456.zip'
+        )
+
+    def test_unsafe_version_characters_become_hyphens(self):
+        source_v1 = OrganizationSourceFactory(
+            organization=OrganizationFactory(mnemonic='PIH'), mnemonic='PIH', version='1.0 beta: [draft] "x";100%')
+        export_path = source_v1.get_version_export_path(suffix='2026-09-30_123456.zip')
+
+        self.assertEqual(
+            source_v1.get_export_download_name(export_path),
+            'orgs_PIH_sources_PIH_1.0-beta-draft-x-100-_2026-09-30_123456.zip'
+        )
+
+
 class UserOrganizationRepoListViewTest(OCLAPITestCase):
     def test_get(self):
         CollectionDocument._index.delete()  # pylint: disable=protected-access

core/services/storages/cloud/aws.py

@@ -5,6 +5,7 @@
 from botocore.exceptions import ClientError, NoCredentialsError, WaiterError
 from django.conf import settings
 from django.core.files.base import ContentFile
+from django.utils.http import content_disposition_header
 from pydash import get
 
 from core.services.storages.cloud.core import CloudStorageServiceInterface
@@ -64,8 +65,9 @@ def upload_base64(  # pylint: disable=too-many-arguments,inconsistent-return-sta
 
         return file_name_with_ext
 
-    def url_for(self, file_path):
-        return self._generate_signed_url(self.GET, file_path) if file_path else None
+    def url_for(self, file_path, filename=None):
+        metadata = {'ResponseContentDisposition': content_disposition_header(True, filename)} if filename else None
+        return self._generate_signed_url(self.GET, file_path, metadata) if file_path else None
 
     def public_url_for(self, file_path):
         url = f"http://{settings.AWS_STORAGE_BUCKET_NAME}.s3.amazonaws.com/{file_path}"

core/services/storages/cloud/azure.py

@@ -19,7 +19,8 @@ def __init__(self):
     def public_url_for(self, file_path):
         return f"https://{self.account_name}.blob.core.windows.net/{self.container_name}/{file_path}"
 
-    def url_for(self, file_path):
+    def url_for(self, file_path, filename=None):  # pylint: disable=unused-argument
+        # an unsigned blob URL can't carry a download name, so filename is ignored
         return self.public_url_for(file_path)
 
     def exists(self, key):

core/services/storages/cloud/core.py

@@ -21,9 +21,9 @@ def upload_base64(self, doc_base64, file_name, append_extension=True, public_rea
         Uploads base64 file content to file_name
         """
 
-    def url_for(self, file_path):
+    def url_for(self, file_path, filename=None):
         """
-        Returns signed url for file_path
+        Returns signed url for file_path; filename, if given, is the download name the response carries
         """
 
     def public_url_for(self, file_path):

core/services/storages/cloud/minio.py

@@ -2,6 +2,7 @@
 import mimetypes
 from io import BytesIO
 
+from django.utils.http import content_disposition_header
 from minio import Minio, S3Error
 from minio.deleteobjects import DeleteObject
 from pydash import get
@@ -85,15 +86,19 @@ def upload_base64(self, doc_base64, file_name, append_extension=True, public_rea
         except S3Error as e:
             raise Exception(f"Could not upload base64 file {file_name_with_ext} to MinIO. Error: {e}") from e  # pylint: disable=broad-exception-raised
 
-    def url_for(self, file_path):
+    def url_for(self, file_path, filename=None):
         """
         Generates a presigned URL for the given file using the external client so the
         signature is computed with the publicly accessible host (MINIO_EXTERNAL_ENDPOINT).
         Falls back to the internal client when no external endpoint is configured.
+        filename, if given, is the download name the response carries.
         """
+        response_headers = {
+            'response-content-disposition': content_disposition_header(True, filename)
+        } if filename else None
         try:
             return self._presign_client.get_presigned_url(
-                method='GET', bucket_name=self.bucket_name, object_name=file_path
+                method='GET', bucket_name=self.bucket_name, object_name=file_path, response_headers=response_headers
             ) if file_path else None
         except S3Error as e:
             raise Exception(f"Could not generate presigned URL for file {file_path}. Error: {e}") from e  # pylint: disable=broad-exception-raised

core/services/storages/cloud/tests.py

@@ -189,6 +189,17 @@ def test_url_for(self):
             'X-Amz-Expires=' in _url
         )
 
+    @mock_aws
+    def test_url_for_with_filename(self):
+        _url = S3().url_for('some/path', filename='orgs_PIH_sources_PIH_1.8.24_2026-09-30_123456.zip')
+
+        self.assertIn(
+            'response-content-disposition=attachment%3B%20filename%3D%22'
+            'orgs_PIH_sources_PIH_1.8.24_2026-09-30_123456.zip%22',
+            _url
+        )
+        self.assertIn('&X-Amz-Signature=', _url)
+
     def test_public_url_for(self):
         self.assertEqual(
             S3().public_url_for('some/path').replace('https://', 'http://'),
@@ -451,7 +462,23 @@ def test_url_for(self, mock_minio_client):
         # Assert that the URL is generated correctly
         self.assertEqual(url, "http://mock_url")
         mock_client.get_presigned_url.assert_called_once_with(method='GET', bucket_name="bucket",
-                                                              object_name=file_key)
+                                                              object_name=file_key, response_headers=None)
+
+    @patch("core.services.storages.cloud.minio.Minio")
+    def test_url_for_with_filename(self, mock_minio_client):
+        mock_client = mock_minio_client.return_value
+        mock_client.get_presigned_url = MagicMock(return_value="http://mock_url")
+
+        url = MinIO().url_for('test-file.zip', filename='orgs_PIH_sources_PIH_1.8.24_2026-09-30_123456.zip')
+
+        self.assertEqual(url, "http://mock_url")
+        mock_client.get_presigned_url.assert_called_once_with(
+            method='GET', bucket_name="bucket", object_name='test-file.zip',
+            response_headers={
+                'response-content-disposition':
+                    'attachment; filename="orgs_PIH_sources_PIH_1.8.24_2026-09-30_123456.zip"'
+            }
+        )
 
     @patch("core.services.storages.cloud.minio.Minio", Mock())
     def test_public_url_for(self):