S3File model: add version and change task-run to work-id

Author: Psykar

missioncontrol/home/migrations/0022_auto_20190306_0017.py

@@ -0,0 +1,33 @@
+# Generated by Django 2.1.4 on 2019-03-06 00:17
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('home', '0021_auto_20190304_0300'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='s3file',
+            name='version',
+            field=models.IntegerField(default=1),
+            preserve_default=False,
+        ),
+        migrations.AddField(
+            model_name='s3file',
+            name='work_id',
+            field=models.TextField(default=1),
+            preserve_default=False,
+        ),
+        migrations.RemoveField(
+            model_name='s3file',
+            name='task_run',
+        ),
+        migrations.AlterUniqueTogether(
+            name='s3file',
+            unique_together={('version', 'what', 'where', 'work_id')},
+        ),
+    ]

missioncontrol/home/migrations/0023_auto_20190306_0231.py

@@ -0,0 +1,22 @@
+# Generated by Django 2.1.4 on 2019-03-06 02:31
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('home', '0022_auto_20190306_0017'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='s3file',
+            options={'ordering': ('-version',)},
+        ),
+        migrations.AlterField(
+            model_name='s3file',
+            name='work_id',
+            field=models.TextField(blank=True, null=True),
+        ),
+    ]

missioncontrol/home/models.py

@@ -13,7 +13,7 @@
 from django.contrib.postgres.fields import JSONField, HStoreField
 from django.db import models
 from django import forms
-from django.db.models import Q
+from django.db.models import Q, Max
 from django.db.models.signals import pre_save
 from django.dispatch import receiver
 from django.utils import timezone, dateformat
@@ -448,11 +448,10 @@ def to_dict(self):
         return retval
 
     def _get_file_cid_if_exists(self, what):
-        try:
-            f = self.files.get(what=what)
-        except S3File.DoesNotExist:
-            return None
-        return f.cid
+        f = S3File.objects.filter(what=what, work_id=self.uuid).first()
+        if f:
+            return f.cid
+        return None
 
     @property
     def stdout(self):
@@ -486,22 +485,28 @@ class S3File(models.Model, Serializable):
                   'Can be blank if instantaneous file.',
         null=True, blank=True)
     created = ISODateTimeField(auto_now_add=True)
-    task_run = models.ForeignKey(TaskRun, to_field='uuid', related_name='files',
-                                 on_delete=models.PROTECT, null=True, blank=True)
+    work_id = models.TextField(null=True, blank=True)
+    version = models.IntegerField()
+
+    class Meta:
+        unique_together = ('version', 'what', 'where', 'work_id')
+        ordering = ('-version', )
 
     # s3://bucket/some_path
     @property
     def prefix(self):
         path = settings.FILE_STORAGE_PATH
         if path.startswith('s3://'):
             return '/'.join(path.split('/')[3:])
+        # TODO
         raise NotImplementedError("Not yet implemented non s3 paths")
 
     @property
     def bucket(self):
         path = settings.FILE_STORAGE_PATH
         if path.startswith('s3://'):
             return path.split('/')[2]
+        # TODO
         raise NotImplementedError("Not yet implemented non s3 paths")
 
     @property
@@ -529,6 +534,20 @@ def get_upload_url(self):
 
         return post
 
+    def save(self, *args, **kwargs):
+        # Set version if not given
+        if self.version is None:
+            prev_version = S3File.objects.filter(
+                what=self.what, where=self.where, work_id=self.work_id
+            ).aggregate(max_version=Max('version'))
+
+            if prev_version['max_version'] is None:
+                self.version = 1
+            else:
+                self.version = prev_version['max_version'] + 1
+
+        super().save(*args, **kwargs)
+
 
 
 class CachedAccess(models.Model):

missioncontrol/openapi/openapi.yaml

@@ -1125,9 +1125,9 @@ paths:
         schema:
           type: string
       - in: query
-        name: task_run
+        name: work_id
         description:
-            Only return files with this task_run uuid.
+            Only return files with this work_id uuid.
         schema:
           type: string
           format: uuid
@@ -1151,6 +1151,14 @@ paths:
           Only return files with this path name
         schema:
           type: string
+      - in: query
+        name: version
+        description:
+          Only return files with this version number
+          # TODO add a flag to return just the latest
+          # version numbers
+        schema:
+          type: string
       responses:
         200:
           description: A list of files
@@ -1542,15 +1550,14 @@ components:
           type: string
           format: date-time
           nullable: true
-        task_run:
+        work_id:
           type: string
           format: uuid
           nullable: true
-        post_url_fields:
-          "$ref": "#/components/schemas/PostUrlFields"
+        version:
+          type: integer
           readOnly: true
 
-
     PostUrlFields:
       properties:
         url:

missioncontrol/tests/conftest.py

@@ -140,7 +140,7 @@ def simple_file(some_hash):
         'what': 'stdout',
         'start': "2018-11-25T01:00:00.000000Z",
         'end': None,
-        'task_run': None,
+        'work_id': None,
         'path': '/some/path/for/files',
         'where': 'somewhere hidden',
     }
@@ -160,4 +160,8 @@ def yet_another_uuid():
 
 @pytest.fixture
 def some_hash():
+    return hashlib.blake2b(uuid4().bytes).hexdigest()
+
+@pytest.fixture
+def another_hash():
     return hashlib.blake2b(uuid4().bytes).hexdigest()

missioncontrol/tests/test_files.py

@@ -24,11 +24,10 @@ def test_file_put_get(boto3_mock, test_client, some_hash, simple_file):
 
     assert response.status_code == 200, response.get_data()
     expected = simple_file.copy()
-    expected['post_url_fields'] = post_values
-
 
     formatter = dateformat.DateFormat(created)
     expected['created'] = formatter.format(settings.DATETIME_FORMAT)
+    expected['version'] = 1
     assert response.json == expected
 
     try:
@@ -70,7 +69,7 @@ def test_file_search_empty(test_client, some_uuid):
             'what': 'a thing',
             'cid': 'nope',
             'where': 'nowhere',
-            'task_run': some_uuid,
+            'work_id': some_uuid,
             'range_start': "2018-11-25T00:00:00.000000Z",
             'range_end': "2018-11-25T00:00:00.000000Z",
         })
@@ -121,7 +120,7 @@ def create_asset(asset_type, asset):
         json=simple_task_run)
     assert response.status_code == 201, response.get_data()
 
-    simple_file['task_run'] = some_uuid
+    simple_file['work_id'] = some_uuid
     created = timezone.now()
     with patch('django.utils.timezone.now', return_value=created):
         response = test_client.put(f'/api/v0/files/{some_hash}/', json=simple_file)
@@ -130,6 +129,7 @@ def create_asset(asset_type, asset):
     expected = simple_file.copy()
     formatter = dateformat.DateFormat(created)
     expected['created'] = formatter.format(settings.DATETIME_FORMAT)
+    expected['version'] = 1
 
     search_query = simple_file.copy()
     search_query['range_start'] = search_query.pop('start')
@@ -142,7 +142,7 @@ def create_asset(asset_type, asset):
 
     # Make sure changing some of the searches *DON'T* find it.
     simple_query_false = simple_file.copy()
-    simple_query_false['task_run'] = uuid.uuid4()
+    simple_query_false['work_id'] = uuid.uuid4()
     simple_query_false['range_start'] = simple_query_false.pop('start')
     response = test_client.get(f'/api/v0/files/', query_string=simple_query_false)
     assert response.status_code == 200, response.get_data()
@@ -159,4 +159,26 @@ def create_asset(asset_type, asset):
     expected['stdout'] = simple_file['cid']
     expected['stderr'] = None
 
-    assert response.json == expected
+    assert response.json == expected
+
+
+@pytest.mark.django_db
+def test_version_increment(test_client, simple_file, some_hash, another_hash):
+    created = timezone.now()
+    with patch('django.utils.timezone.now', return_value=created):
+        response = test_client.put(f'/api/v0/files/{some_hash}/', json=simple_file)
+    assert response.status_code == 201, response.get_data()
+    result1 = response.json
+
+    simple_file['cid'] = another_hash
+    with patch('django.utils.timezone.now', return_value=created):
+        response = test_client.put(f'/api/v0/files/{another_hash}/', json=simple_file)
+    assert response.status_code == 201, response.get_data()
+    result2 = response.json
+
+    assert result1.pop('version') == 1
+    assert result2.pop('version') == 2
+
+    assert result1.pop('cid') == some_hash
+    assert result2.pop('cid') == another_hash
+    assert result1 == result2

missioncontrol/v0/files.py

@@ -1,9 +1,10 @@
 import requests
 
+from connexion.exceptions import ProblemException
 from django.conf import settings
 from django.db.models import Q
 
-from home.models import S3File, TaskRun
+from home.models import S3File
 
 def search(**kwargs):
     # remove some unused ones
@@ -31,19 +32,16 @@ def get_data(cid):
 def get(cid):
     obj = S3File.objects.get(cid=cid)
     retval = obj.to_dict()
-    retval['post_url_fields'] = obj.get_upload_url()
     return retval
 
 def put(cid, file_body):
-    task_run_uuid = file_body.pop('task_run', None)
-    task_run = None
-    if task_run_uuid:
-        # See if it exists
-        task_run = TaskRun.objects.get(uuid=task_run_uuid)
-        file_body['task_run'] = task_run
-
+    body_cid = file_body.pop('cid', None)
+    if body_cid is not None and cid != body_cid:
+        raise ProblemException(
+            status=409,
+            title='Conflict',
+            detail='cid in url does not match body',
+        )
     obj, created = S3File.objects.update_or_create(cid=cid, defaults=file_body)
-
     retval = obj.to_dict()
-    retval['post_url_fields'] = obj.get_upload_url()
     return retval, 201 if created else 200