Merge pull request #1368 from dandi/1109-conflicting-asset-paths

dandiapi/api/asset_paths.py

@@ -42,20 +42,45 @@ def get_root_paths(version: Version) -> QuerySet[AssetPath]:
     )
 
 
-def get_path_children(path: AssetPath) -> QuerySet[AssetPath]:
-    """Get all direct children from an existing path."""
-    qs = AssetPath.objects.select_related(
-        'asset',
-        'asset__blob',
-        'asset__embargoed_blob',
-        'asset__zarr',
+def get_path_children(path: AssetPath, depth: int | None = 1) -> QuerySet[AssetPath]:
+    """
+    Get all children from an existing path.
+
+    By default, returns only the direct children.
+    If depth is `None`, all children will be returned, regardless of depth.
+    """
+    relation_qs = AssetPathRelation.objects.filter(parent=path).exclude(child=path)
+    if depth is not None:
+        relation_qs = relation_qs.filter(depth=depth)
+
+    path_ids = relation_qs.values_list('child', flat=True).distinct()
+    return (
+        AssetPath.objects.select_related(
+            'asset',
+            'asset__blob',
+            'asset__embargoed_blob',
+            'asset__zarr',
+        )
+        .filter(id__in=path_ids)
+        .order_by('path')
     )
-    path_ids = (
-        AssetPathRelation.objects.filter(parent=path, depth=1)
-        .values_list('child', flat=True)
-        .distinct()
+
+
+def get_conflicting_paths(path: str, version: Version) -> list[str]:
+    """Return a list of existing paths that conflict with the given path."""
+    # Check that this path isn't already occupied by a "folder"
+    # Database constraints ensure this queryset could return at most 1 result
+    folder = AssetPath.objects.filter(version=version, path=path, asset__isnull=True).first()
+    if folder:
+        return list(get_path_children(folder, depth=None).values_list('path', flat=True))
+
+    # Check that this path doesn't conflict with any existing "files"
+    nodepaths = extract_paths(path)[:-1]
+    return list(
+        AssetPath.objects.filter(
+            version=version, path__in=nodepaths, asset__isnull=False
+        ).values_list('path', flat=True)
     )
-    return qs.filter(id__in=path_ids).order_by('path')
 
 
 def search_asset_paths(query: str, version: Version) -> QuerySet[AssetPath] | None:

dandiapi/api/services/asset/__init__.py

@@ -1,10 +1,16 @@
 from django.db import transaction
 
-from dandiapi.api.asset_paths import add_asset_paths, delete_asset_paths, update_asset_paths
+from dandiapi.api.asset_paths import (
+    add_asset_paths,
+    delete_asset_paths,
+    get_conflicting_paths,
+    update_asset_paths,
+)
 from dandiapi.api.models.asset import Asset, AssetBlob, EmbargoedAssetBlob
 from dandiapi.api.models.version import Version
 from dandiapi.api.services.asset.exceptions import (
     AssetAlreadyExists,
+    AssetPathConflict,
     DandisetOwnerRequired,
     DraftDandisetNotModifiable,
     ZarrArchiveBelongsToDifferentDandiset,
@@ -112,9 +118,15 @@ def add_asset_to_version(
         raise DraftDandisetNotModifiable()
 
     # Check if there are already any assets with the same path
-    if version.assets.filter(path=metadata['path']).exists():
+    path = metadata['path']
+    if version.assets.filter(path=path).exists():
         raise AssetAlreadyExists()
 
+    # Check if there are any assets that conflict with this path
+    conflicts = get_conflicting_paths(path, version)
+    if conflicts:
+        raise AssetPathConflict(new_path=path, existing_paths=conflicts)
+
     # Ensure zarr archive doesn't already belong to a dandiset
     if zarr_archive and zarr_archive.dandiset != version.dandiset:
         raise ZarrArchiveBelongsToDifferentDandiset()
@@ -128,7 +140,7 @@ def add_asset_to_version(
 
     with transaction.atomic():
         asset = _create_asset(
-            path=metadata['path'],
+            path=path,
             asset_blob=asset_blob,
             embargoed_asset_blob=embargoed_asset_blob,
             zarr_archive=zarr_archive,

dandiapi/api/services/asset/exceptions.py

@@ -17,6 +17,14 @@ class AssetAlreadyExists(DandiException):
     message = 'An asset with that path already exists'
 
 
+class AssetPathConflict(DandiException):
+    http_status_code = status.HTTP_409_CONFLICT
+
+    def __init__(self, new_path: str, existing_paths: list[str]) -> None:
+        message = f'Path of new asset "{new_path}" conflicts with existing assets: {existing_paths}'
+        super().__init__(message)
+
+
 class ZarrArchiveBelongsToDifferentDandiset(DandiException):
     http_status_code = status.HTTP_400_BAD_REQUEST
     message = 'The zarr archive belongs to a different dandiset'

dandiapi/api/tests/test_asset.py

@@ -13,6 +13,8 @@
 from dandiapi.api.models import Asset, AssetBlob, EmbargoedAssetBlob, Version
 from dandiapi.api.models.asset_paths import AssetPath
 from dandiapi.api.models.dandiset import Dandiset
+from dandiapi.api.services.asset import add_asset_to_version
+from dandiapi.api.services.asset.exceptions import AssetPathConflict
 from dandiapi.zarr.tasks import ingest_zarr_archive
 
 from .fuzzy import HTTP_URL_RE, TIMESTAMP_RE, URN_RE, UTC_ISO_TIMESTAMP_RE, UUID_RE
@@ -526,6 +528,47 @@ def test_asset_create_path_validation(
     assert resp.status_code == expected_status_code, resp.data
 
 
+@pytest.mark.django_db
+def test_asset_create_conflicting_path(api_client, user, draft_version, asset_blob):
+    assign_perm('owner', user, draft_version.dandiset)
+    api_client.force_authenticate(user=user)
+
+    # Add first asset
+    add_asset_to_version(
+        user=user,
+        version=draft_version,
+        asset_blob=asset_blob,
+        metadata={
+            'path': 'foo/bar.txt',
+            'schemaVersion': settings.DANDI_SCHEMA_VERSION,
+        },
+    )
+
+    # Add an asset that has a path which fully contains that of the first asset
+    with pytest.raises(AssetPathConflict):
+        add_asset_to_version(
+            user=user,
+            version=draft_version,
+            asset_blob=asset_blob,
+            metadata={
+                'path': 'foo/bar.txt/baz.txt',
+                'schemaVersion': settings.DANDI_SCHEMA_VERSION,
+            },
+        )
+
+    # Add an asset that's path is fully contained by the first asset
+    with pytest.raises(AssetPathConflict):
+        add_asset_to_version(
+            user=user,
+            version=draft_version,
+            asset_blob=asset_blob,
+            metadata={
+                'path': 'foo',
+                'schemaVersion': settings.DANDI_SCHEMA_VERSION,
+            },
+        )
+
+
 @pytest.mark.django_db
 def test_asset_create_embargo(api_client, user, draft_version, embargoed_asset_blob):
     assign_perm('owner', user, draft_version.dandiset)