Skip to content

OpenConceptLab/ocl_issues#2477 | repo external exports #881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
snyaggarwal wants to merge 1 commit into
base: master
Choose a base branch
from
Open

OpenConceptLab/ocl_issues#2477 | repo external exports #881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions core/collections/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
from core.client_configs.serializers import ClientConfigSerializer
from core.collections.models import Collection, CollectionReference, Expansion
from core.common.constants import HEAD, DEFAULT_ACCESS_TYPE, NAMESPACE_REGEX, ACCESS_TYPE_CHOICES, INCLUDE_SUMMARY, \
INCLUDE_CLIENT_CONFIGS, INVALID_EXPANSION_URL, INCLUDE_STATES, INCLUDE_TASKS, INCLUDE_RESOLVED_REPO_VERSIONS
INCLUDE_CLIENT_CONFIGS, INVALID_EXPANSION_URL, INCLUDE_STATES, INCLUDE_TASKS, INCLUDE_RESOLVED_REPO_VERSIONS, \
INCLUDE_EXTERNAL_EXPORTS
from core.common.serializers import AbstractRepoResourcesSerializer, AbstractResourceSerializer
from core.common.utils import get_truthy_values
from core.orgs.models import Organization
Expand Down Expand Up @@ -106,19 +107,42 @@ class CollectionVersionListSerializer(ModelSerializer):
autoexpand = BooleanField(source='should_auto_expand')
expansion_url = CharField(source='expansion_uri', read_only=True)
checksums = SerializerMethodField()
external_exports = SerializerMethodField()

class Meta:
model = Collection
fields = (
'type', 'short_code', 'name', 'url', 'canonical_url', 'owner', 'owner_type', 'owner_url', 'version',
'created_at', 'id', 'collection_type', 'updated_at', 'released', 'retired', 'version_url',
'previous_version_url', 'autoexpand', 'expansion_url', 'checksums'
'previous_version_url', 'autoexpand', 'expansion_url', 'checksums', 'external_exports'
)

def __init__(self, *args, **kwargs):
params = get(kwargs, 'context.request.query_params')

self.query_params = {}
if params:
self.query_params = params if isinstance(params, dict) else params.dict()
self.include_external_exports = self.query_params.get(INCLUDE_EXTERNAL_EXPORTS) in TRUTHY

try:
if not self.include_external_exports:
self.fields.pop('external_exports', None)
except: # pylint: disable=bare-except
pass

super().__init__(*args, **kwargs)

@staticmethod
def get_checksums(obj):
return obj.get_all_checksums()

@staticmethod
def get_external_exports(obj):
from core.repos.serializers import RepoExternalExportSerializer
queryset = obj.external_exports.filter()
return RepoExternalExportSerializer(queryset, many=True).data


class CollectionCreateOrUpdateSerializer(ModelSerializer):
canonical_url = CharField(allow_blank=True, allow_null=True, required=False)
Expand Down Expand Up @@ -442,6 +466,7 @@ class CollectionVersionDetailSerializer(CollectionCreateOrUpdateSerializer, Abst
expansion_url = CharField(source='expansion_uri', allow_null=True, allow_blank=True)
states = SerializerMethodField()
tasks = SerializerMethodField()
external_exports = SerializerMethodField()

class Meta:
model = Collection
Expand All @@ -454,7 +479,7 @@ class Meta:
'version', 'concepts_url', 'mappings_url', 'expansions_url', 'is_processing', 'released', 'retired',
'canonical_url', 'identifier', 'publisher', 'contact', 'jurisdiction', 'purpose', 'copyright', 'meta',
'immutable', 'revision_date', 'summary', 'text', 'experimental', 'locked_date',
'autoexpand', 'expansion_url', 'checksums', 'states', 'tasks'
'autoexpand', 'expansion_url', 'checksums', 'states', 'tasks', 'external_exports'
) + AbstractRepoResourcesSerializer.Meta.fields

def __init__(self, *args, **kwargs):
Expand All @@ -467,6 +492,7 @@ def __init__(self, *args, **kwargs):
self.include_summary = self.query_params.get(INCLUDE_SUMMARY) in TRUTHY
self.include_states = self.query_params.get(INCLUDE_STATES) in TRUTHY
self.include_tasks = self.query_params.get(INCLUDE_TASKS) in TRUTHY
self.include_external_exports = self.query_params.get(INCLUDE_EXTERNAL_EXPORTS) in TRUTHY

try:
if not self.include_summary:
Expand All @@ -475,6 +501,8 @@ def __init__(self, *args, **kwargs):
self.fields.pop('states', None)
if not self.include_tasks:
self.fields.pop('tasks', None)
if not self.include_external_exports:
self.fields.pop('external_exports', None)
except: # pylint: disable=bare-except
pass

Expand Down Expand Up @@ -508,6 +536,12 @@ def get_tasks(self, obj):
def get_autoexpand(obj):
return obj.should_auto_expand

@staticmethod
def get_external_exports(obj):
from core.repos.serializers import RepoExternalExportSerializer
queryset = obj.external_exports.filter()
return RepoExternalExportSerializer(queryset, many=True).data


class CollectionReferenceSerializer(ModelSerializer):
reference_type = CharField(read_only=True)
Expand Down
9 changes: 9 additions & 0 deletions core/collections/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@
views.CollectionVersionExportView.as_view(),
name='collectionversion-latest-export-detail'
),
path(
'<str:collection>/latest/export/<str:external_export_key>/',
views.CollectionVersionExternalExportView.as_view(),
name='collectionversion-latest-external-export-detail'
),
path(
"<str:collection>/concepts/<str:concept>/mappings/",
views.CollectionVersionConceptMappingsView.as_view(),
Expand Down Expand Up @@ -215,6 +220,10 @@
'<str:collection>/<str:version>/export/',
views.CollectionVersionExportView.as_view(), name='collectionversion-export'
),
path(
'<str:collection>/<str:version>/export/<str:external_export_key>/',
views.CollectionVersionExternalExportView.as_view(), name='collectionversion-external-export'
),
path(
"<str:collection>/<str:version>/extras/",
views.CollectionVersionExtrasView.as_view(),
Expand Down
5 changes: 5 additions & 0 deletions core/collections/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
ConceptDictionaryCreateMixin, ListWithHeadersMixin, ConceptDictionaryUpdateMixin,
ConceptContainerExportMixin,
ConceptContainerProcessingMixin)
from core.repos.mixins import RepoExternalExportMixin
from core.common.permissions import (
CanViewConceptDictionary, CanEditConceptDictionary, HasAccessToVersionedObject,
CanViewConceptDictionaryVersion
Expand Down Expand Up @@ -1184,6 +1185,10 @@ def handle_export_version(self):
return status.HTTP_409_CONFLICT


class CollectionVersionExternalExportView(RepoExternalExportMixin, CollectionVersionBaseView):
permission_classes = (CanViewConceptDictionary, IsAuthenticated)


class CollectionSummaryView(SummaryMixin, CollectionBaseView, RetrieveAPIView):
serializer_class = CollectionSummaryDetailSerializer
permission_classes = (CanViewConceptDictionary,)
Expand Down
1 change: 1 addition & 0 deletions core/common/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
INCLUDE_CREATOR_PINS = 'includeCreatorPins'
INCLUDE_HIERARCHY_ROOT = 'includeHierarchyRoot'
INCLUDE_SUMMARY = 'includeSummary'
INCLUDE_EXTERNAL_EXPORTS = 'includeExternalExports'
INCLUDE_LOGS = 'includeLogs'
INCLUDE_STATES = 'includeStates'
INCLUDE_TASKS = 'includeTasks'
Expand Down
12 changes: 12 additions & 0 deletions core/common/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,9 @@ class ConceptContainerModel(VersionedModel, ChecksumModel):
'url_registry.URLRegistry', object_id_field='repo_id', content_type_field='repo_type'
)
followers = GenericRelation('users.Follow', object_id_field='following_id', content_type_field='following_type')
external_exports = GenericRelation(
'repos.RepoExternalExport', object_id_field='resource_id', content_type_field='resource_type'
)

class Meta:
abstract = True
Expand Down Expand Up @@ -1002,6 +1005,11 @@ def get_version_export_path(self, suffix='*'):

return path

def get_external_export_path(self, key, filename):

@filiperochalopes filiperochalopes Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple sanitization using .replace() for slashes might be insufficient and risky.

  1. Path Traversal: It doesn't handle '..' sequences, which can lead to unpredictable path resolution.
  2. OS Compatibility: It preserves characters like ':', '*', '<', '>', which are invalid on Windows systems.
  3. Storage/URL Issues: Spaces and special characters are kept, which often break S3 signed URLs and Content-Disposition headers during download.

Recommendation: Use django.utils.text.get_valid_filename(filename) for a robust, industry-standard solution.

from django.utils.text import get_valid_filename

def get_external_export_path(self, key, filename):
    base_path = self.get_version_export_path(suffix=None).rstrip(".")
    safe_filename = get_valid_filename(filename)
    return f"{base_path}/external/{key}_{safe_filename}"

base_path = self.get_version_export_path(suffix=None).rstrip('.')
safe_filename = filename.replace('/', '_').replace('\\', '_')
return f"{base_path}/external/{key}_{safe_filename}"

def get_export_path(self):
if self.is_head:
return self.version_export_path
Expand Down Expand Up @@ -1287,6 +1295,10 @@ def get_tasks(self):
'exported': export_task,
}

def upload_external_export(self, key, file, user, description=None):
from core.repos.models import RepoExternalExport
return RepoExternalExport.upsert(self, key, file, user, description)


class CelerySignalProcessor(RealTimeSignalProcessor):
def handle_save(self, sender, instance, **kwargs):
Expand Down
126 changes: 126 additions & 0 deletions core/integration_tests/tests_collections.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import zipfile

from celery_once import AlreadyQueued
from django.core.files.uploadedfile import SimpleUploadedFile
from mock import patch, Mock, ANY
from mock.mock import PropertyMock
from rest_framework.exceptions import ErrorDetail
Expand Down Expand Up @@ -2831,6 +2832,131 @@ def test_delete_204_head(self, s3_remove_mock, has_export_mock, export_path_mock
s3_remove_mock.assert_called_once_with('head/export/path')


class CollectionVersionExternalExportViewTest(OCLAPITestCase):
def setUp(self):
super().setUp()
self.admin = UserProfile.objects.get(username='ocladmin')
self.admin_token = self.admin.get_token()
self.user = UserProfileFactory(username='username')
self.token = self.user.get_token()
self.collection_v1 = UserCollectionFactory(version='v1', mnemonic='coll', user=self.user)

def test_get_404_unknown_key(self):
response = self.client.get(
self.collection_v1.uri + 'export/openmrs23-sql/',
HTTP_AUTHORIZATION='Token ' + self.token,
format='json'
)

self.assertEqual(response.status_code, 404)

@patch('core.services.storages.cloud.aws.S3.remove')
@patch('core.services.storages.cloud.aws.S3.upload')
def test_post_201_create_then_get_302_and_delete_204(self, s3_upload_mock, s3_remove_mock):
uploaded_file = SimpleUploadedFile('openmrs23.sql.zip', b'content', content_type='application/zip')

response = self.client.post(
self.collection_v1.uri + 'export/openmrs23-sql/',
{'file': uploaded_file},
HTTP_AUTHORIZATION='Token ' + self.token,
)

self.assertEqual(response.status_code, 201)
self.assertEqual(response.data['key'], 'openmrs23-sql')
self.assertEqual(response.data['url'], self.collection_v1.uri + 'export/openmrs23-sql/')
s3_upload_mock.assert_called_once()

from core.repos.models import RepoExternalExport
instance = RepoExternalExport.objects.get(key='openmrs23-sql')

with patch('core.services.storages.cloud.aws.S3.url_for') as s3_url_for_mock:
s3_url_for_mock.return_value = 'https://signed.example/openmrs23.sql.zip'
response = self.client.get(
self.collection_v1.uri + 'export/openmrs23-sql/',
HTTP_AUTHORIZATION='Token ' + self.token,
format='json'
)

self.assertEqual(response.status_code, 302)
self.assertEqual(response['Location'], 'https://signed.example/openmrs23.sql.zip')

response = self.client.delete(
self.collection_v1.uri + 'export/openmrs23-sql/',
HTTP_AUTHORIZATION='Token ' + self.token,
format='json'
)

self.assertEqual(response.status_code, 204)
s3_remove_mock.assert_called_once_with(instance.file_path)
self.assertFalse(RepoExternalExport.objects.filter(key='openmrs23-sql').exists())

def test_post_400_no_file(self):
response = self.client.post(
self.collection_v1.uri + 'export/openmrs23-sql/',
{},
HTTP_AUTHORIZATION='Token ' + self.token,
)

self.assertEqual(response.status_code, 400)

def test_post_403_non_admin(self):
random_user = UserProfileFactory()
uploaded_file = SimpleUploadedFile('openmrs23.sql.zip', b'content', content_type='application/zip')

response = self.client.post(
self.collection_v1.uri + 'export/openmrs23-sql/',
{'file': uploaded_file},
HTTP_AUTHORIZATION='Token ' + random_user.get_token(),
)

self.assertEqual(response.status_code, 403)

@patch('core.services.storages.cloud.aws.S3.upload')
def test_export_serializer_includes_external_exports(self, s3_upload_mock): # pylint: disable=unused-argument
uploaded_file = SimpleUploadedFile('openmrs23.sql.zip', b'content', content_type='application/zip')
self.client.post(
self.collection_v1.uri + 'export/openmrs23-sql/',
{'file': uploaded_file},
HTTP_AUTHORIZATION='Token ' + self.token,
)

self.collection_v1.refresh_from_db()
external_exports = CollectionVersionExportSerializer(self.collection_v1).data['external_exports']

self.assertEqual(len(external_exports), 1)
self.assertEqual(external_exports[0]['key'], 'openmrs23-sql')
self.assertEqual(external_exports[0]['url'], self.collection_v1.uri + 'export/openmrs23-sql/')

def test_delete_404_unknown_key(self):
response = self.client.delete(
self.collection_v1.uri + 'export/openmrs23-sql/',
HTTP_AUTHORIZATION='Token ' + self.token,
format='json'
)

self.assertEqual(response.status_code, 404)

@patch('core.services.storages.cloud.aws.S3.remove')
@patch('core.services.storages.cloud.aws.S3.upload')
def test_delete_403_non_admin(self, s3_upload_mock, s3_remove_mock): # pylint: disable=unused-argument
uploaded_file = SimpleUploadedFile('openmrs23.sql.zip', b'content', content_type='application/zip')
self.client.post(
self.collection_v1.uri + 'export/openmrs23-sql/',
{'file': uploaded_file},
HTTP_AUTHORIZATION='Token ' + self.token,
)

random_user = UserProfileFactory()
response = self.client.delete(
self.collection_v1.uri + 'export/openmrs23-sql/',
HTTP_AUTHORIZATION='Token ' + random_user.get_token(),
format='json'
)

self.assertEqual(response.status_code, 403)
s3_remove_mock.assert_not_called()


class CollectionVersionListViewTest(OCLAPITestCase):
def setUp(self):
super().setUp()
Expand Down
Loading