[S3] Support null version ids for buckets with versioning disabled

moto/s3/models.py

@@ -10,6 +10,7 @@ import random
 import string
 import tempfile
 import sys
+import uuid
 
 import six
 
@@ -35,7 +36,7 @@ class FakeDeleteMarker(BaseModel):
         self.key = key
         self.name = key.name
         self.last_modified = datetime.datetime.utcnow()
-        self._version_id = key.version_id + 1
+        self._version_id = str(uuid.uuid4())
 
     @property
     def last_modified_ISO8601(self):
@@ -115,15 +116,16 @@ class FakeKey(BaseModel):
         self.last_modified = datetime.datetime.utcnow()
         self._etag = None  # must recalculate etag
         if self._is_versioned:
-            self._version_id += 1
+            self._version_id = str(uuid.uuid4())
         else:
-            self._is_versioned = 0
+            self._version_id = None
 
     def restore(self, days):
         self._expiry = datetime.datetime.utcnow() + datetime.timedelta(days)
 
-    def increment_version(self):
+    def refresh_version(self):
-        self._version_id += 1
+        self._version_id = str(uuid.uuid4())
+        self.last_modified = datetime.datetime.utcnow()
 
     @property
     def etag(self):
@@ -716,17 +718,18 @@ class S3Backend(BaseBackend):
 
     def get_bucket_latest_versions(self, bucket_name):
         versions = self.get_bucket_versions(bucket_name)
-        maximum_version_per_key = {}
+        latest_modified_per_key = {}
         latest_versions = {}
 
         for version in versions:
             name = version.name
+            last_modified = version.last_modified
             version_id = version.version_id
-            maximum_version_per_key[name] = max(
+            latest_modified_per_key[name] = max(
-                version_id,
+                last_modified,
-                maximum_version_per_key.get(name, -1)
+                latest_modified_per_key.get(name, datetime.datetime.min)
             )
-            if version_id == maximum_version_per_key[name]:
+            if last_modified == latest_modified_per_key[name]:
                 latest_versions[name] = version_id
 
         return latest_versions
@@ -774,20 +777,19 @@ class S3Backend(BaseBackend):
 
         bucket = self.get_bucket(bucket_name)
 
-        old_key = bucket.keys.get(key_name, None)
-        if old_key is not None and bucket.is_versioned:
-            new_version_id = old_key._version_id + 1
-        else:
-            new_version_id = 0
-
         new_key = FakeKey(
             name=key_name,
             value=value,
             storage=storage,
             etag=etag,
             is_versioned=bucket.is_versioned,
-            version_id=new_version_id)
+            version_id=str(uuid.uuid4()) if bucket.is_versioned else None)
-        bucket.keys[key_name] = new_key
+
+        keys = [
+            key for key in bucket.keys.getlist(key_name, [])
+            if key.version_id != new_key.version_id
+        ] + [new_key]
+        bucket.keys.setlist(key_name, keys)
 
         return new_key
 
@@ -977,7 +979,7 @@ class S3Backend(BaseBackend):
 
         # By this point, the destination key must exist, or KeyError
         if dest_bucket.is_versioned:
-            dest_bucket.keys[dest_key_name].increment_version()
+            dest_bucket.keys[dest_key_name].refresh_version()
         if storage is not None:
             key.set_storage_class(storage)
         if acl is not None:

moto/s3/responses.py

@@ -1303,7 +1303,7 @@ S3_BUCKET_GET_VERSIONS = """<?xml version="1.0" encoding="UTF-8"?>
     {% for key in key_list %}
     <Version>
         <Key>{{ key.name }}</Key>
-        <VersionId>{{ key.version_id }}</VersionId>
+        <VersionId>{% if key.version_id is none %}null{% else %}{{ key.version_id }}{% endif %}</VersionId>
         <IsLatest>{% if latest_versions[key.name] == key.version_id %}true{% else %}false{% endif %}</IsLatest>
         <LastModified>{{ key.last_modified_ISO8601 }}</LastModified>
         <ETag>{{ key.etag }}</ETag>

tests/test_s3/test_s3.py

@@ -444,7 +444,12 @@ def test_copy_key_with_version():
     key.set_contents_from_string("some value")
     key.set_contents_from_string("another value")
 
-    bucket.copy_key('new-key', 'foobar', 'the-key', src_version_id='0')
+    key = [
+        key.version_id
+        for key in bucket.get_all_versions()
+        if not key.is_latest
+    ][0]
+    bucket.copy_key('new-key', 'foobar', 'the-key', src_version_id=key)
 
     bucket.get_key(
         "the-key").get_contents_as_string().should.equal(b"another value")
@@ -818,16 +823,19 @@ def test_key_version():
     bucket = conn.create_bucket('foobar')
     bucket.configure_versioning(versioning=True)
 
+    versions = []
+
     key = Key(bucket)
     key.key = 'the-key'
     key.version_id.should.be.none
     key.set_contents_from_string('some string')
-    key.version_id.should.equal('0')
+    versions.append(key.version_id)
     key.set_contents_from_string('some string')
-    key.version_id.should.equal('1')
+    versions.append(key.version_id)
+    set(versions).should.have.length_of(2)
 
     key = bucket.get_key('the-key')
-    key.version_id.should.equal('1')
+    key.version_id.should.equal(versions[-1])
 
 
 @mock_s3_deprecated
@@ -836,23 +844,25 @@ def test_list_versions():
     bucket = conn.create_bucket('foobar')
     bucket.configure_versioning(versioning=True)
 
+    key_versions = []
+
     key = Key(bucket, 'the-key')
     key.version_id.should.be.none
     key.set_contents_from_string("Version 1")
-    key.version_id.should.equal('0')
+    key_versions.append(key.version_id)
     key.set_contents_from_string("Version 2")
-    key.version_id.should.equal('1')
+    key_versions.append(key.version_id)
+    key_versions.should.have.length_of(2)
 
     versions = list(bucket.list_versions())
-
     versions.should.have.length_of(2)
 
     versions[0].name.should.equal('the-key')
-    versions[0].version_id.should.equal('0')
+    versions[0].version_id.should.equal(key_versions[0])
     versions[0].get_contents_as_string().should.equal(b"Version 1")
 
     versions[1].name.should.equal('the-key')
-    versions[1].version_id.should.equal('1')
+    versions[1].version_id.should.equal(key_versions[1])
     versions[1].get_contents_as_string().should.equal(b"Version 2")
 
     key = Key(bucket, 'the2-key')
@@ -1483,16 +1493,22 @@ def test_boto3_head_object_with_versioning():
     s3.Object('blah', 'hello.txt').put(Body=old_content)
     s3.Object('blah', 'hello.txt').put(Body=new_content)
 
+    versions = list(s3.Bucket('blah').object_versions.all())
+    latest = list(filter(lambda item: item.is_latest, versions))[0]
+    oldest = list(filter(lambda item: not item.is_latest, versions))[0]
+
     head_object = s3.Object('blah', 'hello.txt').meta.client.head_object(
         Bucket='blah', Key='hello.txt')
-    head_object['VersionId'].should.equal('1')
+    head_object['VersionId'].should.equal(latest.id)
     head_object['ContentLength'].should.equal(len(new_content))
 
     old_head_object = s3.Object('blah', 'hello.txt').meta.client.head_object(
-        Bucket='blah', Key='hello.txt', VersionId='0')
+        Bucket='blah', Key='hello.txt', VersionId=oldest.id)
-    old_head_object['VersionId'].should.equal('0')
+    old_head_object['VersionId'].should.equal(oldest.id)
     old_head_object['ContentLength'].should.equal(len(old_content))
 
+    old_head_object['VersionId'].should_not.equal(head_object['VersionId'])
+
 
 @mock_s3
 def test_boto3_copy_object_with_versioning():
@@ -1507,9 +1523,6 @@ def test_boto3_copy_object_with_versioning():
     obj1_version = client.get_object(Bucket='blah', Key='test1')['VersionId']
     obj2_version = client.get_object(Bucket='blah', Key='test2')['VersionId']
 
-    # Versions should be the same
-    obj1_version.should.equal(obj2_version)
-
     client.copy_object(CopySource={'Bucket': 'blah', 'Key': 'test1'}, Bucket='blah', Key='test2')
     obj2_version_new = client.get_object(Bucket='blah', Key='test2')['VersionId']
 
@@ -2507,6 +2520,75 @@ def test_boto3_list_object_versions():
     response['Body'].read().should.equal(items[-1])
 
 
+@mock_s3
+def test_boto3_list_object_versions_with_versioning_disabled():
+    s3 = boto3.client('s3', region_name='us-east-1')
+    bucket_name = 'mybucket'
+    key = 'key-with-versions'
+    s3.create_bucket(Bucket=bucket_name)
+    items = (six.b('v1'), six.b('v2'))
+    for body in items:
+        s3.put_object(
+            Bucket=bucket_name,
+            Key=key,
+            Body=body
+        )
+    response = s3.list_object_versions(
+        Bucket=bucket_name
+    )
+
+    # One object version should be returned
+    len(response['Versions']).should.equal(1)
+    response['Versions'][0]['Key'].should.equal(key)
+
+    # The version id should be the string null
+    response['Versions'][0]['VersionId'].should.equal('null')
+
+    # Test latest object version is returned
+    response = s3.get_object(Bucket=bucket_name, Key=key)
+    response['Body'].read().should.equal(items[-1])
+
+
+@mock_s3
+def test_boto3_list_object_versions_with_versioning_enabled_late():
+    s3 = boto3.client('s3', region_name='us-east-1')
+    bucket_name = 'mybucket'
+    key = 'key-with-versions'
+    s3.create_bucket(Bucket=bucket_name)
+    items = (six.b('v1'), six.b('v2'))
+    s3.put_object(
+        Bucket=bucket_name,
+        Key=key,
+        Body=six.b('v1')
+    )
+    s3.put_bucket_versioning(
+        Bucket=bucket_name,
+        VersioningConfiguration={
+            'Status': 'Enabled'
+        }
+    )
+    s3.put_object(
+        Bucket=bucket_name,
+        Key=key,
+        Body=six.b('v2')
+    )
+    response = s3.list_object_versions(
+        Bucket=bucket_name
+    )
+
+    # Two object versions should be returned
+    len(response['Versions']).should.equal(2)
+    keys = set([item['Key'] for item in response['Versions']])
+    keys.should.equal({key})
+
+    # There should still be a null version id.
+    versionsId = set([item['VersionId'] for item in response['Versions']])
+    versionsId.should.contain('null')
+
+    # Test latest object version is returned
+    response = s3.get_object(Bucket=bucket_name, Key=key)
+    response['Body'].read().should.equal(items[-1])
+
 @mock_s3
 def test_boto3_bad_prefix_list_object_versions():
     s3 = boto3.client('s3', region_name='us-east-1')
@@ -2563,18 +2645,25 @@ def test_boto3_delete_markers():
             Bucket=bucket_name,
             Key=key
         )
-        e.response['Error']['Code'].should.equal('404')
+    e.exception.response['Error']['Code'].should.equal('NoSuchKey')
+
+    response = s3.list_object_versions(
+        Bucket=bucket_name
+    )
+    response['Versions'].should.have.length_of(2)
+    response['DeleteMarkers'].should.have.length_of(1)
 
     s3.delete_object(
         Bucket=bucket_name,
         Key=key,
-        VersionId='2'
+        VersionId=response['DeleteMarkers'][0]['VersionId']
     )
     response = s3.get_object(
         Bucket=bucket_name,
         Key=key
     )
     response['Body'].read().should.equal(items[-1])
+
     response = s3.list_object_versions(
         Bucket=bucket_name
     )
@@ -2583,10 +2672,8 @@ def test_boto3_delete_markers():
     # We've asserted there is only 2 records so one is newest, one is oldest
     latest = list(filter(lambda item: item['IsLatest'], response['Versions']))[0]
     oldest = list(filter(lambda item: not item['IsLatest'], response['Versions']))[0]
-
     # Double check ordering of version ID's
-    latest['VersionId'].should.equal('1')
+    latest['VersionId'].should_not.equal(oldest['VersionId'])
-    oldest['VersionId'].should.equal('0')
 
     # Double check the name is still unicode
     latest['Key'].should.equal('key-with-versions-and-unicode-ó')
@@ -2631,12 +2718,12 @@ def test_boto3_multiple_delete_markers():
     s3.delete_object(
         Bucket=bucket_name,
         Key=key,
-        VersionId='2'
+        VersionId=response['DeleteMarkers'][0]['VersionId']
     )
     s3.delete_object(
         Bucket=bucket_name,
         Key=key,
-        VersionId='3'
+        VersionId=response['DeleteMarkers'][1]['VersionId']
     )
 
     response = s3.get_object(
@@ -2652,8 +2739,7 @@ def test_boto3_multiple_delete_markers():
     oldest = list(filter(lambda item: not item['IsLatest'], response['Versions']))[0]
 
     # Double check ordering of version ID's
-    latest['VersionId'].should.equal('1')
+    latest['VersionId'].should_not.equal(oldest['VersionId'])
-    oldest['VersionId'].should.equal('0')
 
     # Double check the name is still unicode
     latest['Key'].should.equal('key-with-versions-and-unicode-ó')