Merge pull request #499 from dandi/fix-next-published-version

Fix next published version

dandiapi/api/migrations/0001_initial.py

@@ -224,7 +224,7 @@ class Migration(migrations.Migration):
                 (
                     'version',
                     models.CharField(
-                        default=dandiapi.api.models.version._get_default_version,
+                        default='draft',
                         max_length=13,
                         validators=[
                             django.core.validators.RegexValidator('^(0\\.\\d{6}\\.\\d{4})|draft$')

dandiapi/api/migrations/0017_alter_version_version.py

@@ -0,0 +1,22 @@
+# Generated by Django 3.2 on 2021-08-27 23:34
+
+import django.core.validators
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0016_remove_metadata_tables'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='version',
+            name='version',
+            field=models.CharField(
+                max_length=13,
+                validators=[django.core.validators.RegexValidator('^(0\\.\\d{6}\\.\\d{4})|draft$')],
+            ),
+        ),
+    ]

dandiapi/api/models/version.py

@@ -25,11 +25,6 @@
 logger = logging.getLogger(__name__)
 
 
-def _get_default_version() -> str:
-    # This cannot be a lambda, as migrations cannot serialize those
-    return Version.make_version()
-
-
 class Version(PublishableMetadataMixin, TimeStampedModel):
     VERSION_REGEX = r'(0\.\d{6}\.\d{4})|draft'
 
@@ -46,7 +41,6 @@ class Status(models.TextChoices):
     version = models.CharField(
         max_length=13,
         validators=[RegexValidator(f'^{VERSION_REGEX}$')],
-        default=_get_default_version,
     )  # TODO: rename this?
     doi = models.CharField(max_length=64, null=True, blank=True)
     """Track the validation status of this version, without considering assets"""
@@ -131,14 +125,12 @@ def datetime_to_version(time: datetime.datetime) -> str:
         return time.strftime('0.%y%m%d.%H%M')
 
     @classmethod
-    def make_version(cls, dandiset: Dandiset = None) -> str:
-        versions: models.Manager = dandiset.versions if dandiset else cls.objects
-
+    def next_published_version(cls, dandiset: Dandiset) -> str:
         time = datetime.datetime.now(datetime.timezone.utc)
         # increment time until there are no collisions
         while True:
             version = cls.datetime_to_version(time)
-            collision = versions.filter(version=version).exists()
+            collision = dandiset.versions.filter(version=version).exists()
             if not collision:
                 break
             time += datetime.timedelta(minutes=1)
@@ -158,6 +150,7 @@ def publish_version(self):
             name=self.name,
             metadata=self.metadata,
             status=Version.Status.VALID,
+            version=Version.next_published_version(self.dandiset),
         )
 
         now = datetime.datetime.now(datetime.timezone.utc)

dandiapi/api/tests/factories.py

@@ -51,6 +51,10 @@ class Meta:
     dandiset = factory.SubFactory(DandisetFactory)
     name = factory.Faker('sentence')
 
+    @factory.lazy_attribute
+    def version(self):
+        return Version.next_published_version(self.dandiset)
+
     @factory.lazy_attribute
     def metadata(self):
         metadata = {

dandiapi/api/tests/test_version.py

@@ -12,25 +12,38 @@
 
 
 @pytest.mark.django_db
-def test_version_make_version_nosave(dandiset):
+def test_version_next_published_version_nosave(dandiset):
     # Without saving, the output should be reproducible
-    version_str_1 = Version.make_version(dandiset)
-    version_str_2 = Version.make_version(dandiset)
+    version_str_1 = Version.next_published_version(dandiset)
+    version_str_2 = Version.next_published_version(dandiset)
     assert version_str_1 == version_str_2
     assert version_str_1 == VERSION_ID_RE
 
 
 @pytest.mark.django_db
-def test_version_make_version_save(mocker, dandiset, published_version_factory):
+def test_version_next_published_version_save(mocker, dandiset, published_version_factory):
     # Given an existing version at the current time, a different one should be allocated
-    make_version_spy = mocker.spy(Version, 'make_version')
+    next_published_version_spy = mocker.spy(Version, 'next_published_version')
     version_1 = published_version_factory(dandiset=dandiset)
-    make_version_spy.assert_called_once()
+    next_published_version_spy.assert_called_once()
 
-    version_str_2 = Version.make_version(dandiset)
+    version_str_2 = Version.next_published_version(dandiset)
     assert version_1.version != version_str_2
 
 
+@pytest.mark.django_db
+def test_version_next_published_version_simultaneous_save(
+    dandiset_factory,
+    published_version_factory,
+):
+    dandiset_1 = dandiset_factory()
+    dandiset_2 = dandiset_factory()
+    version_1 = published_version_factory(dandiset=dandiset_1)
+    version_2 = published_version_factory(dandiset=dandiset_2)
+    # Different dandisets published at the same time should have the same version string
+    assert version_1.version == version_2.version
+
+
 @pytest.mark.django_db
 def test_draft_version_metadata_computed(draft_version: Version):
     original_metadata = {'schemaVersion': settings.DANDI_SCHEMA_VERSION}