Merge pull request #2100 from gilbsgilbs/non-versioned-bucket-consitency

[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):
-        self._version_id += 1
+    def refresh_version(self):
+        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(
-                version_id,
-                maximum_version_per_key.get(name, -1)
+            latest_modified_per_key[name] = max(
+                last_modified,
+                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)
-        bucket.keys[key_name] = new_key
+            version_id=str(uuid.uuid4()) if bucket.is_versioned else None)
+
+        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>