Fix publish

dandiapi/api/models/asset.py

@@ -148,41 +148,28 @@ def _populate_metadata(self):
             )
         return metadata
 
-    @classmethod
-    def published_asset(cls, asset: Asset):
-        """
-        Generate a published asset + metadata without saving it.
-
-        This is useful to validate asset metadata without saving it.
-        """
+    def published_metadata(self):
+        """Generate the metadata of this asset as if it were being published."""
         now = datetime.datetime.utcnow()
         # Inject the publishedBy and datePublished fields
         published_metadata, _ = AssetMetadata.objects.get_or_create(
             metadata={
-                **asset.metadata.metadata,
-                'publishedBy': asset.metadata.published_by(now),
+                **self.metadata.metadata,
+                'publishedBy': self.metadata.published_by(now),
                 'datePublished': now.isoformat(),
             },
         )
+        return published_metadata
 
-        # Create the published model
-        published_asset = Asset(
-            path=asset.path,
-            blob=asset.blob,
-            metadata=published_metadata,
-            # If we're publishing, just assume that the asset was valid
-            status=Asset.Status.VALID,
-            previous=asset,
-            published=True,
-        )
-
-        # Recompute the metadata
-        published_metadata, _ = AssetMetadata.objects.get_or_create(
-            metadata=published_asset._populate_metadata(),
-        )
-        published_asset.metadata = published_metadata
+    def publish(self):
+        """
+        Modify the metadata of this asset as if it were being published.
 
-        return published_asset
+        This is useful to validate asset metadata without saving it.
+        To actually publish this Asset, simply save() after calling publish().
+        """
+        self.metadata = self.published_metadata()
+        self.published = True
 
     def save(self, *args, **kwargs):
         metadata = self._populate_metadata()

dandiapi/api/tasks.py

@@ -93,8 +93,7 @@ def validate_asset_metadata(asset_id: int) -> None:
     asset.save()
 
     try:
-        publish_asset = Asset.published_asset(asset)
-        metadata = publish_asset.metadata.metadata
+        metadata = asset.published_metadata().metadata
         validate(metadata, schema_key='PublishedAsset')
     except Exception as e:
         logger.error('Error while validating asset %s', asset_id)

dandiapi/api/tests/conftest.py

@@ -12,10 +12,11 @@
 
 from .factories import (
     AssetBlobFactory,
-    AssetFactory,
     AssetMetadataFactory,
     DandisetFactory,
+    DraftAssetFactory,
     DraftVersionFactory,
+    PublishedAssetFactory,
     PublishedVersionFactory,
     SocialAccountFactory,
     UploadFactory,
@@ -28,7 +29,8 @@
     import mypy_boto3_s3 as s3
 
 
-register(AssetFactory)
+register(PublishedAssetFactory, _name='published_asset')
+register(DraftAssetFactory, _name='draft_asset')
 register(AssetBlobFactory)
 register(AssetMetadataFactory)
 register(DandisetFactory)
@@ -42,6 +44,16 @@
 register(VersionMetadataFactory)
 
 
+@pytest.fixture(params=[DraftAssetFactory, PublishedAssetFactory], ids=['draft', 'published'])
+def asset_factory(request):
+    return request.param
+
+
+@pytest.fixture
+def asset(asset_factory):
+    return asset_factory()
+
+
 @pytest.fixture(params=[DraftVersionFactory, PublishedVersionFactory], ids=['draft', 'published'])
 def version(request):
     return request.param()

dandiapi/api/tests/factories.py

@@ -137,7 +137,7 @@ def metadata(self):
         return metadata
 
 
-class AssetFactory(factory.django.DjangoModelFactory):
+class DraftAssetFactory(factory.django.DjangoModelFactory):
     class Meta:
         model = Asset
 
@@ -146,6 +146,15 @@ class Meta:
     blob = factory.SubFactory(AssetBlobFactory)
 
 
+class PublishedAssetFactory(DraftAssetFactory):
+    @classmethod
+    def _create(cls, *args, **kwargs):
+        asset: Asset = super()._create(*args, **kwargs)
+        asset.publish()
+        asset.save()
+        return asset
+
+
 class UploadFactory(factory.django.DjangoModelFactory):
     class Meta:
         model = Upload

dandiapi/api/tests/test_asset.py

@@ -96,14 +96,20 @@ def test_asset_s3_url(asset_blob):
 
 
 @pytest.mark.django_db
-def test_version_published_asset(asset):
-    published_asset = Asset.published_asset(asset)
-    published_asset.save()
+def test_publish_asset(draft_asset: Asset):
+    draft_asset_id = draft_asset.asset_id
+    draft_blob = draft_asset.blob
+    draft_metadata = draft_asset.metadata.metadata
+    draft_asset.publish()
+    draft_asset.save()
 
-    assert published_asset.blob == asset.blob
+    # draft_asset has been published, so it is now published_asset
+    published_asset = draft_asset
+
+    assert published_asset.blob == draft_blob
     assert published_asset.metadata.metadata == {
-        **asset.metadata.metadata,
-        'id': f'dandiasset:{published_asset.asset_id}',
+        **draft_metadata,
+        'id': f'dandiasset:{draft_asset_id}',
         'publishedBy': {
             'id': URN_RE,
             'name': 'DANDI publish',
@@ -122,7 +128,7 @@ def test_version_published_asset(asset):
             'schemaKey': 'PublishActivity',
         },
         'datePublished': TIMESTAMP_RE,
-        'identifier': str(published_asset.asset_id),
+        'identifier': str(draft_asset_id),
         'contentUrl': [HTTP_URL_RE, HTTP_URL_RE],
     }
 
@@ -149,11 +155,11 @@ def test_asset_total_size(draft_version_factory, asset_factory, asset_blob_facto
 
 
 @pytest.mark.django_db
-def test_asset_populate_metadata(asset_factory, asset_metadata):
+def test_asset_populate_metadata(draft_asset_factory, asset_metadata):
     raw_metadata = asset_metadata.metadata
 
     # This should trigger _populate_metadata to inject all the computed metadata fields
-    asset = asset_factory(metadata=asset_metadata)
+    asset = draft_asset_factory(metadata=asset_metadata)
 
     download_url = 'https://api.dandiarchive.org' + reverse(
         'asset-direct-download',

dandiapi/api/tests/test_version.py

@@ -612,10 +612,10 @@ def test_version_rest_publish(api_client, user: User, draft_version: Version, as
     assert published_version
     assert draft_version.dandiset.versions.count() == 2
 
-    published_asset = published_version.assets.get()
+    published_asset: Asset = published_version.assets.get()
     assert published_asset.published
-    # The blob should be the same between the two assets
-    assert asset.blob == published_asset.blob
+    # The asset should be the same after publishing
+    assert asset.asset_id == published_asset.asset_id
 
     assert published_version.metadata.metadata == {
         **draft_version.metadata.metadata,
@@ -674,6 +674,79 @@ def test_version_rest_publish(api_client, user: User, draft_version: Version, as
     assert not draft_version.valid
 
 
+@pytest.mark.django_db
+def test_version_rest_publish_assets(
+    api_client,
+    user: User,
+    draft_version: Version,
+    draft_asset_factory,
+    published_asset_factory,
+):
+    assign_perm('owner', user, draft_version.dandiset)
+    api_client.force_authenticate(user=user)
+
+    old_draft_asset: Asset = draft_asset_factory()
+    old_published_asset: Asset = published_asset_factory()
+    old_published_asset.publish()
+    old_published_asset.save()
+    assert not old_draft_asset.published
+    assert old_published_asset.published
+    draft_version.assets.add(old_draft_asset)
+    draft_version.assets.add(old_published_asset)
+
+    # Validate the metadata to mark the assets and version as `VALID`
+    tasks.validate_asset_metadata(old_draft_asset.id)
+    tasks.validate_asset_metadata(old_published_asset.id)
+    tasks.validate_version_metadata(draft_version.id)
+    draft_version.refresh_from_db()
+    assert draft_version.valid
+
+    resp = api_client.post(
+        f'/api/dandisets/{draft_version.dandiset.identifier}'
+        f'/versions/{draft_version.version}/publish/'
+    )
+    assert resp.status_code == 200
+    published_version = Version.objects.get(version=resp.data['version'])
+
+    assert published_version.assets.count() == 2
+    new_draft_asset: Asset = published_version.assets.get(asset_id=old_draft_asset.asset_id)
+    new_published_asset: Asset = published_version.assets.get(asset_id=old_published_asset.asset_id)
+
+    # The former draft asset should have been modified into a published asset
+    assert new_draft_asset.published
+    assert new_draft_asset.asset_id == old_draft_asset.asset_id
+    assert new_draft_asset.path == old_draft_asset.path
+    assert new_draft_asset.blob == old_draft_asset.blob
+    assert new_draft_asset.metadata.metadata == {
+        **old_draft_asset.metadata.metadata,
+        'datePublished': TIMESTAMP_RE,
+        'publishedBy': {
+            'id': URN_RE,
+            'name': 'DANDI publish',
+            'startDate': TIMESTAMP_RE,
+            # TODO endDate needs to be defined before publish is complete
+            'endDate': TIMESTAMP_RE,
+            'wasAssociatedWith': [
+                {
+                    'id': URN_RE,
+                    'identifier': 'RRID:SCR_017571',
+                    'name': 'DANDI API',
+                    'version': '0.1.0',
+                    'schemaKey': 'Software',
+                }
+            ],
+            'schemaKey': 'PublishActivity',
+        },
+    }
+
+    # The published_asset should be completely unchanged
+    assert new_published_asset.published
+    assert new_published_asset.asset_id == old_published_asset.asset_id
+    assert new_published_asset.path == old_published_asset.path
+    assert new_published_asset.blob == old_published_asset.blob
+    assert new_published_asset.metadata.metadata == old_published_asset.metadata.metadata
+
+
 @pytest.mark.django_db
 def test_version_rest_publish_not_an_owner(api_client, user, version, asset):
     api_client.force_authenticate(user=user)

dandiapi/api/views/version.py

@@ -141,14 +141,12 @@ def publish(self, request, **kwargs):
         from dandiapi.api.models import Asset
 
         draft_assets = old_version.assets.filter(published=False).all()
-        new_published_assets = [Asset.published_asset(draft_asset) for draft_asset in draft_assets]
-        Asset.objects.bulk_create(new_published_assets)
+        for draft_asset in draft_assets:
+            draft_asset.publish()
+        Asset.objects.bulk_update(draft_assets, ['metadata', 'published'])
 
         AssetVersions.objects.bulk_create(
-            [
-                AssetVersions(asset_id=asset.id, version_id=new_version.id)
-                for asset in new_published_assets
-            ]
+            [AssetVersions(asset_id=asset.id, version_id=new_version.id) for asset in draft_assets]
         )
 
         # Save again to recompute metadata, specifically assetsSummary