From 27bda808320212d1b91ab1b51c58ede2074596d3 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Thu, 16 Feb 2023 11:35:13 -0500 Subject: [PATCH] Handle path conflict when creating leaf asset path --- dandiapi/api/asset_paths.py | 21 ++++++++++++++++++--- dandiapi/api/tests/test_asset_paths.py | 22 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/dandiapi/api/asset_paths.py b/dandiapi/api/asset_paths.py index b1e5de005..20e940a0a 100644 --- a/dandiapi/api/asset_paths.py +++ b/dandiapi/api/asset_paths.py @@ -2,7 +2,7 @@ import os -from django.db import transaction +from django.db import IntegrityError, transaction from django.db.models import Count, F, QuerySet, Sum from django.db.models.functions import Coalesce from tqdm import tqdm @@ -101,8 +101,23 @@ def search_asset_paths(query: str, version: Version) -> QuerySet[AssetPath] | No def insert_asset_paths(asset: Asset, version: Version): """Add all intermediate paths from an asset and link them together.""" - # Get or create leaf path - leaf, created = AssetPath.objects.get_or_create(path=asset.path, asset=asset, version=version) + try: + # Get or create leaf path + leaf, created = AssetPath.objects.get_or_create( + path=asset.path, asset=asset, version=version + ) + except IntegrityError as e: + from dandiapi.api.services.asset.exceptions import AssetAlreadyExists + + # If there are simultaneous requests to create the same asset, this check constraint can + # fail, and should be handled directly, rather than be allowed to bubble up + if 'unique-version-path' in str(e): + raise AssetAlreadyExists() + + # Re-raise original exception otherwise + raise + + # If the asset was not created, return early, as the work is already done if not created: return leaf diff --git a/dandiapi/api/tests/test_asset_paths.py b/dandiapi/api/tests/test_asset_paths.py index bc1e33285..e0155f9a8 100644 --- a/dandiapi/api/tests/test_asset_paths.py +++ b/dandiapi/api/tests/test_asset_paths.py @@ -11,6 +11,7 @@ ) from dandiapi.api.models import Asset, AssetPath, Version from dandiapi.api.models.asset_paths import AssetPathRelation +from dandiapi.api.services.asset.exceptions import AssetAlreadyExists from dandiapi.api.tasks import publish_dandiset_task @@ -96,6 +97,27 @@ def test_asset_path_add_asset_idempotent(draft_version_factory, asset_factory): assert path.aggregate_size == asset.size +@pytest.mark.django_db +def test_asset_path_add_asset_conflicting_path(draft_version_factory, asset_factory): + # Create asset with version + asset1: Asset = asset_factory() + asset2: Asset = asset_factory(path=asset1.path) + version: Version = draft_version_factory() + version.assets.add(asset1) + version.assets.add(asset2) + + # Add asset1 paths + add_asset_paths(asset1, version) + assert version.asset_paths.filter(asset__isnull=False).count() == 1 + + # Ensure that adding asset2 raises the correct exception + with pytest.raises(AssetAlreadyExists): + add_asset_paths(asset2, version) + + # Ensure that there no new asset paths created + assert version.asset_paths.filter(asset__isnull=False).count() == 1 + + @pytest.mark.django_db def test_asset_path_add_version_asset_paths(draft_version_factory, asset_factory): # Create asset with version