OpenConceptLab/ocl_issues#1841 | not using any pattern for kwargs in …

…url | repo version can accept special characters | uri will be encoded
OpenConceptLab · May 21, 2024 · fdac087 · fdac087
1 parent ce51b0a
commit fdac087
Show file tree

Hide file tree

Showing 19 changed files with 525 additions and 424 deletions.
diff --git a/core/code_systems/urls.py b/core/code_systems/urls.py
@@ -1,16 +1,10 @@
-from django.urls import re_path
+from django.urls import path
 
-from core.common.constants import NAMESPACE_PATTERN
 from . import views
 
 urlpatterns = [
-    re_path(r'^$', views.CodeSystemListView.as_view(), name='code-system-list'),
-    re_path(r'^\$lookup/$', views.CodeSystemListLookupView.as_view(), name='code-system-list-lookup'),
-    re_path(r'^\$validate-code/$', views.CodeSystemValidateCodeView.as_view(),
-            name='code-system-validate-code'),
-    re_path(
-        fr"^(?P<source>{NAMESPACE_PATTERN})/$",
-        views.CodeSystemRetrieveUpdateView.as_view(),
-        name='code-system-detail'
-    )
+    path('', views.CodeSystemListView.as_view(), name='code-system-list'),
+    path('$lookup/', views.CodeSystemListLookupView.as_view(), name='code-system-list-lookup'),
+    path('$validate-code/', views.CodeSystemValidateCodeView.as_view(), name='code-system-validate-code'),
+    path("<str:source>/", views.CodeSystemRetrieveUpdateView.as_view(), name='code-system-detail')
 ]
diff --git a/core/collections/urls.py b/core/collections/urls.py
@@ -1,50 +1,49 @@
-from django.urls import re_path, path
+from django.urls import path
 
 from core.collections.feeds import CollectionFeed
-from core.common.constants import NAMESPACE_PATTERN
 from core.concepts.views import ConceptCascadeView
 from . import views
 
 urlpatterns = [
-    re_path(r'^$', views.CollectionListView.as_view(), name='collection-list'),
-    re_path(
-        fr"^(?P<collection>{NAMESPACE_PATTERN})/$",
+    path('', views.CollectionListView.as_view(), name='collection-list'),
+    path(
+        "<str:collection>/",
         views.CollectionRetrieveUpdateDestroyView.as_view(),
         name='collection-detail'
     ),
-    re_path(
-        r'^(?P<collection>' + NAMESPACE_PATTERN + ')/client-configs/$',
+    path(
+        '<str:collection>/client-configs/',
         views.CollectionClientConfigsView.as_view(),
         name='collection-client-configs'
     ),
-    re_path(
-        fr"^(?P<collection>{NAMESPACE_PATTERN})/summary/$",
+    path(
+        "<str:collection>/summary/",
         views.CollectionSummaryView.as_view(),
         name='collection-summary'
     ),
-    re_path(
-        fr"^(?P<collection>{NAMESPACE_PATTERN})/logo/$",
+    path(
+        "<str:collection>/logo/",
         views.CollectionLogoView.as_view(),
         name='collection-logo'
     ),
-    re_path(
-        fr'^(?P<collection>{NAMESPACE_PATTERN})/versions/$',
+    path(
+        '<str:collection>/versions/',
         views.CollectionVersionListView.as_view(),
         name='collection-version-list'
     ),
-    re_path(fr'^(?P<collection>{NAMESPACE_PATTERN})/concepts/atom/$', CollectionFeed()),
-    re_path(
-        fr'^(?P<collection>{NAMESPACE_PATTERN})/latest/$',
+    path('<str:collection>/concepts/atom/', CollectionFeed()),
+    path(
+        '<str:collection>/latest/',
         views.CollectionLatestVersionRetrieveUpdateView.as_view(),
         name='collectionversion-latest-detail'
     ),
-    re_path(
-        fr'^(?P<collection>{NAMESPACE_PATTERN})/latest/summary/$',
+    path(
+        '<str:collection>/latest/summary/',
         views.CollectionLatestVersionSummaryView.as_view(),
         name='collectionversion-latest-summary'
     ),
-    re_path(
-        fr'^(?P<collection>{NAMESPACE_PATTERN})/latest/export/$',
+    path(
+        '<str:collection>/latest/export/',
         views.CollectionVersionExportView.as_view(),
         name='collectionversion-latest-export-detail'
     ),
@@ -73,8 +72,8 @@
         views.CollectionVersionConceptRetrieveView.as_view(),
         name='concept-detail'
     ),
-    re_path(
-        fr"^(?P<collection>{NAMESPACE_PATTERN})/concepts/",
+    path(
+        "<str:collection>/concepts/",
         views.CollectionVersionConceptsView.as_view(),
         name='concept-list'
     ),
@@ -88,54 +87,53 @@
         views.CollectionVersionMappingRetrieveView.as_view(),
         name='mapping-detail'
     ),
-    re_path(
-        fr"^(?P<collection>{NAMESPACE_PATTERN})/mappings/",
+    path(
+        "<str:collection>/mappings/",
         views.CollectionVersionMappingsView.as_view(),
         name='mapping-list'
     ),
-    re_path(
-        fr'^(?P<collection>{NAMESPACE_PATTERN})/references/$',
+    path(
+        '<str:collection>/references/',
         views.CollectionReferencesView.as_view(),
         name='collection-references'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/references/(?P<reference>{pattern})/$'.format(pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/references/<str:reference>/',
         views.CollectionReferenceView.as_view(),
         name='collection-reference'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/references/(?P<reference>{pattern})/concepts/$'.format(pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/references/<str:reference>/concepts/',
         views.CollectionReferenceConceptsView.as_view(),
         name='collection-reference-concepts-list'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/references/(?P<reference>{pattern})/mappings/$'.format(pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/references/<str:reference>/mappings/',
         views.CollectionReferenceMappingsView.as_view(),
         name='collection-reference-mappings-list'
     ),
-    re_path(
-        fr"^(?P<collection>{NAMESPACE_PATTERN})/extras/$",
+    path(
+        "<str:collection>/extras/",
         views.CollectionExtrasView.as_view(),
         name='collection-extras'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/$'.format(pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/',
         views.CollectionVersionRetrieveUpdateDestroyView.as_view(),
         name='collection-version-detail'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/summary/$'.format(pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/summary/',
         views.CollectionVersionSummaryView.as_view(),
         name='collection-version-summary'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/expansions/$'.format(pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/expansions/',
         views.CollectionVersionExpansionsView.as_view(),
         name='expansion-list'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/expansions/(?P<expansion>{pattern})/$'.format(
-            pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/expansions/<str:expansion>/',
         views.CollectionVersionExpansionView.as_view(),
         name='collection-version-expansion-detail'
     ),
@@ -184,29 +182,27 @@
         views.CollectionVersionExpansionMappingRetrieveView.as_view(),
         name='mapping-detail'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/expansions/(?P<expansion>{pattern})/concepts/$'.format(
-            pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/expansions/<str:expansion>/concepts/',
         views.CollectionVersionExpansionConceptsView.as_view(),
         name='collection-version-expansion-concepts'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/expansions/(?P<expansion>{pattern})/mappings/$'.format(
-            pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/expansions/<str:expansion>/mappings/',
         views.CollectionVersionExpansionMappingsView.as_view(),
         name='collection-version-expansion-mappings'
     ),
-    re_path(
-        r"^(?P<collection>{pattern})/extras/(?P<extra>{pattern})/$".format(pattern=NAMESPACE_PATTERN),
+    path(
+        "<str:collection>/extras/<str:extra>/",
         views.CollectionExtraRetrieveUpdateDestroyView.as_view(),
         name='collection-extra'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/export/$'.format(pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/export/',
         views.CollectionVersionExportView.as_view(), name='collectionversion-export'
     ),
-    re_path(
-        r"^(?P<collection>{pattern})/(?P<version>{pattern})/extras/$".format(pattern=NAMESPACE_PATTERN),
+    path(
+        "<str:collection>/<str:version>/extras/",
         views.CollectionVersionExtrasView.as_view(),
         name='collectionversion-extras'
     ),
@@ -235,10 +231,8 @@
         views.CollectionVersionConceptRetrieveView.as_view(),
         name='concept-detail'
     ),
-    re_path(
-        r"^(?P<collection>{pattern})/(?P<version>{pattern})/concepts/".format(
-            pattern=NAMESPACE_PATTERN
-        ),
+    path(
+        "<str:collection>/<str:version>/concepts/",
         views.CollectionVersionConceptsView.as_view(),
         name='concept-list'
     ),
@@ -252,44 +246,38 @@
         views.CollectionVersionMappingRetrieveView.as_view(),
         name='mapping-detail'
     ),
-    re_path(
-        r"^(?P<collection>{pattern})/(?P<version>{pattern})/mappings/".format(
-            pattern=NAMESPACE_PATTERN
-        ),
+    path(
+        "<str:collection>/<str:version>/mappings/",
         views.CollectionVersionMappingsView.as_view(),
         name='mapping-list'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/references/(?P<reference>{pattern})/$'.format(
-            pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/references/<str:reference>/',
         views.CollectionReferenceView.as_view(),
         name='collectionversion-reference'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/references/(?P<reference>{pattern})/resolve/$'.format(
-            pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/references/<str:reference>/resolve/',
         views.CollectionReferenceResolveView.as_view(),
         name='collectionversion-reference-resolve'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/references/(?P<reference>{pattern})/concepts/$'.format(
-            pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/references/<str:reference>/concepts/',
         views.CollectionReferenceConceptsView.as_view(),
         name='collectionversion-reference-concepts-list'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/references/(?P<reference>{pattern})/mappings/$'.format(
-            pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/references/<str:reference>/mappings/',
         views.CollectionReferenceMappingsView.as_view(),
         name='collectionversion-reference-mappings-list'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/references/$'.format(pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/references/',
         views.CollectionVersionReferencesView.as_view(),
         name='collectionversion-references'
     ),
-    re_path(
-        r'^(?P<collection>{pattern})/(?P<version>{pattern})/processing/$'.format(pattern=NAMESPACE_PATTERN),
+    path(
+        '<str:collection>/<str:version>/processing/',
         views.CollectionVersionProcessingView.as_view(),
         name='collectionversion-processing'
     ),

diff --git a/core/common/mixins.py b/core/common/mixins.py
@@ -24,7 +24,7 @@
 from .checksums import ChecksumModel, Checksum
 from .utils import write_csv_to_s3, get_csv_from_s3, get_query_params_from_url_string, compact_dict_by_values, \
     to_owner_uri, parse_updated_since_param, get_export_service, to_int, get_truthy_values, generate_temp_version, \
-    canonical_url_to_url_and_version
+    canonical_url_to_url_and_version, decode_string
 from ..concepts.constants import PERSIST_CLONE_ERROR
 from ..toggles.models import Toggle
 
@@ -451,7 +451,7 @@ def find_repo_by_canonical_url(self, canonical_url):
         if not queryset.exists():
             queryset = self.collection_set.filter(canonical_url=url)
         if version:
-            queryset = queryset.filter(version=version)
+            queryset = queryset.filter(version=decode_string(version))
         return queryset.first()
 
     @staticmethod

diff --git a/core/common/models.py b/core/common/models.py
@@ -20,7 +20,7 @@
     delete_s3_objects
 from core.common.utils import reverse_resource, reverse_resource_version, parse_updated_since_param, drop_version, \
     to_parent_uri, is_canonical_uri, get_export_service, from_string_to_date, get_truthy_values, \
-    canonical_url_to_url_and_version, get_current_authorized_user
+    canonical_url_to_url_and_version, get_current_authorized_user, encode_string, decode_string
 from core.common.utils import to_owner_uri
 from core.settings import DEFAULT_LOCALE
 from .checksums import ChecksumModel
@@ -111,6 +111,13 @@ def undelete(self):
             self.is_active = True
             self.save()
 
+    @staticmethod
+    def get_encoded_str_variations(value):
+        return [
+            value, encode_string(value, safe=' '), encode_string(value, safe='+'),
+            encode_string(value, safe='+%'), encode_string(value, safe='% +'),
+            decode_string(value), decode_string(value, False)
+        ]
     @property
     def is_versioned(self):
         return False
@@ -174,14 +181,14 @@ def toggle_indexing(state=True):
         settings.ES_SYNC = state
 
     @staticmethod
-    def get_exact_or_criteria(attr, values):
+    def get_exact_or_criteria(attr, values, decode=False):
         criteria = Q()
 
         if isinstance(values, str):
             values = values.split(',')
 
         for value in values:
-            criteria = criteria | Q(**{f'{attr}': value})
+            criteria = criteria | Q(**{f'{attr}': decode_string(value) if decode else value})
 
         return criteria
 
@@ -495,7 +502,7 @@ def get_base_queryset(cls, params):
         if org:
             queryset = queryset.filter(cls.get_exact_or_criteria('organization__mnemonic', org))
         if version:
-            queryset = queryset.filter(cls.get_exact_or_criteria('version', version))
+            queryset = queryset.filter(cls.get_exact_or_criteria('version', version, True))
         if is_latest:
             queryset = queryset.filter(is_latest_version=True)
         if updated_since:
@@ -984,7 +991,7 @@ def resolve_reference_expression(cls, url, namespace=None, version=None):
     def resolve_repo(cls, instance, version, is_canonical, resolution_url):
         if instance:
             if version:
-                instance = instance.versions.filter(version=version).first()
+                instance = instance.versions.filter(version=decode_string(version)).first()
             elif instance.is_head:
                 instance = instance.get_latest_released_version() or instance
 

diff --git a/core/common/utils.py b/core/common/utils.py
@@ -106,7 +106,10 @@ def reverse_resource(resource, viewname, args=None, kwargs=None, **extra):
             if head:
                 kwargs.update({head.get_url_kwarg(): head.mnemonic})
 
-            kwargs.update({parent.get_url_kwarg(): parent.version})
+            version = parent.version if is_url_encoded_string(
+                parent.version) else encode_string(parent.version, safe=' ')
+
+            kwargs.update({parent.get_url_kwarg(): version})
             parent_resource_url_kwarg = parent.get_resource_url_kwarg()
             if parent_resource_url_kwarg not in kwargs:
                 kwargs.update({parent_resource_url_kwarg: parent.mnemonic})
@@ -137,8 +140,8 @@ def reverse_resource_version(resource, viewname, args=None, kwargs=None, **extra
     kwargs = kwargs or {}
     if head:
         kwargs.update({head.get_url_kwarg(): head.mnemonic})
-
-    kwargs.update({resource.get_url_kwarg(): resource.version})
+    version = resource.version if is_url_encoded_string(resource.version) else encode_string(resource.version, safe=' ')
+    kwargs.update({resource.get_url_kwarg(): version})
     resource_url_kwarg = resource.get_resource_url_kwarg()
 
     if resource_url_kwarg not in kwargs: