Merge pull request fog#732 from nirvdrum/s3_test_fixes

[aws|storage|test] Make sure tests pass with both mocking enabled and disabled.

lib/fog/aws/requests/storage/copy_object.rb

@@ -48,7 +48,7 @@ class Mock # :nodoc:all
         def copy_object(source_bucket_name, source_object_name, target_bucket_name, target_object_name, options = {})
           response = Excon::Response.new
           source_bucket = self.data[:buckets][source_bucket_name]
-          source_object = source_bucket && source_bucket[:objects][source_object_name]
+          source_object = source_bucket && source_bucket[:objects][source_object_name] && source_bucket[:objects][source_object_name].first
           target_bucket = self.data[:buckets][target_bucket_name]
 
           acl = options['x-amz-acl'] || 'private'

lib/fog/aws/requests/storage/delete_object.rb

@@ -61,6 +61,7 @@ def delete_object(bucket_name, object_name, options = {})
                 else
                   response.status = 400
                   response.body = invalid_version_id_payload(version_id)
+                  raise(Excon::Errors.status_error({:expects => 200}, response))
                 end
               else
                 delete_marker = {
@@ -72,11 +73,11 @@ def delete_object(bucket_name, object_name, options = {})
 
                 # When versioning is suspended, a delete marker is placed if the last object ID is not the value 'null',
                 # otherwise the last object is replaced.
-                if bucket[:versioning] == 'Suspended' && bucket[:objects][object_name].last['VersionId'] == 'null'
-                  bucket[:objects][object_name].pop
+                if bucket[:versioning] == 'Suspended' && bucket[:objects][object_name].first['VersionId'] == 'null'
+                  bucket[:objects][object_name].shift
                 end
 
-                bucket[:objects][object_name] << delete_marker
+                bucket[:objects][object_name].unshift(delete_marker)
 
                 response.headers['x-amz-delete-marker'] = 'true'
                 response.headers['x-amz-version-id'] = delete_marker['VersionId']
@@ -85,6 +86,7 @@ def delete_object(bucket_name, object_name, options = {})
               if version_id && version_id != 'null'
                 response.status = 400
                 response.body = invalid_version_id_payload(version_id)
+                raise(Excon::Errors.status_error({:expects => 200}, response))
               else
                 bucket[:objects].delete(object_name)
 

lib/fog/aws/requests/storage/get_bucket_object_versions.rb

@@ -97,7 +97,10 @@ def get_bucket_object_versions(bucket_name, options = {})
           # Valid case.
           # TODO: (nirvdrum 12/15/11) It's not clear to me how to actually use version-id-marker, so I didn't implement it below.
           elsif bucket = self.data[:buckets][bucket_name]
-            contents = bucket[:objects].values.flatten.sort {|x,y| x['Key'] <=> y['Key']}.reject do |object|
+            # We need to order results by S3 key, but since our data store is key => [versions], we want to ensure the integrity
+            # of the versions as well.  So, sort the keys, then fetch the versions, and then combine them all as a sorted list by
+            # flattening the results.
+            contents = bucket[:objects].keys.sort.collect { |key| bucket[:objects][key] }.flatten.reject do |object|
                 (prefix      && object['Key'][0...prefix.length] != prefix) ||
                 (key_marker  && object['Key'] <= key_marker) ||
                 (delimiter   && object['Key'][(prefix ? prefix.length : 0)..-1].include?(delimiter) \
@@ -116,7 +119,7 @@ def get_bucket_object_versions(bucket_name, options = {})
                 data[tag_name].merge!({
                   'LastModified' => Time.parse(object['Last-Modified']),
                   'Owner'        => bucket['Owner'],
-                  'IsLatest'     => object == bucket[:objects][object['Key']].last
+                  'IsLatest'     => object == bucket[:objects][object['Key']].first
                 })
 
                 data[tag_name]['Size'] = object['Content-Length'].to_i if tag_name == 'Version'
@@ -144,15 +147,18 @@ def get_bucket_object_versions(bucket_name, options = {})
 
           # Missing bucket case.
           else
-            response.status = 403
+            response.status = 404
             response.body = {
               'Error' => {
-                'Code' => 'AccessDenied',
-                'Message' => 'AccessDenied',
+                'Code' => 'NoSuchBucket',
+                'Message' => 'The specified bucket does not exist',
+                'BucketName' => bucket_name,
                 'RequestId' => Fog::Mock.random_hex(16),
                 'HostId' => Fog::Mock.random_base64(65)
               }
             }
+
+            raise(Excon::Errors.status_error({:expects => 200}, response))
           end
           response
         end

lib/fog/aws/requests/storage/get_bucket_versioning.rb

@@ -60,6 +60,8 @@ def get_bucket_versioning(bucket_name)
                 'HostId' => Fog::Mock.random_base64(65)
               }
             }
+
+            raise(Excon::Errors.status_error({:expects => 200}, response))
           end
 
           response

lib/fog/aws/requests/storage/get_object.rb

@@ -76,7 +76,7 @@ def get_object(bucket_name, object_name, options = {}, &block)
           if (bucket = self.data[:buckets][bucket_name])
             object = nil
             if bucket[:objects].has_key?(object_name)
-              object = version_id ? bucket[:objects][object_name].find { |object| object['VersionId'] == version_id} : bucket[:objects][object_name].last
+              object = version_id ? bucket[:objects][object_name].find { |object| object['VersionId'] == version_id} : bucket[:objects][object_name].first
             end
 
             if (object && !object[:delete_marker])
@@ -123,6 +123,8 @@ def get_object(bucket_name, object_name, options = {}, &block)
                   'HostId' => Fog::Mock.random_base64(65)
                 }
               }
+
+              raise(Excon::Errors.status_error({:expects => 200}, response))
             else
               response.status = 404
               response.body = "...<Code>NoSuchKey<\/Code>..."

lib/fog/aws/requests/storage/put_bucket_versioning.rb

@@ -51,17 +51,22 @@ def put_bucket_versioning(bucket_name, status)
                   'HostId' => Fog::Mock.random_base64(65)
                 }
               }
+
+              raise(Excon::Errors.status_error({:expects => 200}, response))
             end
           else
-            response.status = 403
+            response.status = 404
             response.body = {
               'Error' => {
-                'Code' => 'AccessDenied',
-                'Message' => 'AccessDenied',
+                'Code' => 'NoSuchBucket',
+                'Message' => 'The specified bucket does not exist',
+                'BucketName' => bucket_name,
                 'RequestId' => Fog::Mock.random_hex(16),
                 'HostId' => Fog::Mock.random_base64(65)
               }
             }
+
+            raise(Excon::Errors.status_error({:expects => 200}, response))
           end
 
           response

lib/fog/aws/requests/storage/put_bucket_website.rb

@@ -52,7 +52,7 @@ def put_bucket_website(bucket_name, suffix, options = {})
           if self.data[:buckets][bucket_name]
             response.status = 200
           else
-            response.status = 403
+            response.status = 404
             raise(Excon::Errors.status_error({:expects => 200}, response))
           end
 

lib/fog/aws/requests/storage/put_object.rb

@@ -86,11 +86,11 @@ def put_object(bucket_name, object_name, data, options = {})
 
               # When versioning is suspended, putting an object will create a new 'null' version if the latest version
               # is a value other than 'null', otherwise it will replace the latest version.
-              if bucket[:versioning] == 'Suspended' && bucket[:objects][object_name].last['VersionId'] == 'null'
-                bucket[:objects][object_name].pop
+              if bucket[:versioning] == 'Suspended' && bucket[:objects][object_name].first['VersionId'] == 'null'
+                bucket[:objects][object_name].shift
               end
 
-              bucket[:objects][object_name] << object
+              bucket[:objects][object_name].unshift(object)
             else
               bucket[:objects][object_name] = [object]
             end

tests/aws/models/storage/version_tests.rb

@@ -29,10 +29,10 @@
         @version_instance.delete_marker
       end
 
-      tests("#delete_marker should be true if the version isn't a DeleteMarker'").returns(true) do
+      tests("#delete_marker should be true if the version is a DeleteMarker'").returns(true) do
         @instance.destroy
 
-        @instance.versions.all.last.delete_marker
+        @instance.versions.all.first.delete_marker
       end
     end
 

tests/aws/models/storage/versions_tests.rb

@@ -15,17 +15,24 @@
 
     versions = []
     versions << @instance.connection.put_object(@instance.key, 'one', 'abcde').headers['x-amz-version-id']
+
+    puts versions.first
+
     versions << @instance.connection.put_object(@instance.key, 'one', '32423').headers['x-amz-version-id']
     versions << @instance.connection.delete_object(@instance.key, 'one').headers['x-amz-version-id']
+    versions.reverse!
+
+    puts versions.first
+
     versions << @instance.connection.put_object(@instance.key, 'two', 'aoeu').headers['x-amz-version-id']
 
     tests('#versions') do
       tests('#versions.size includes versions (including DeleteMarkers) for all keys').returns(4) do
-        @instance.versions.size
+        @instance.versions.all.size
       end
 
       tests('#versions returns the correct versions').returns(versions) do
-        @instance.versions.collect(&:version)
+        @instance.versions.all.collect(&:version)
       end
     end
 
@@ -35,11 +42,11 @@
       end
 
       tests("#all for file returns only versions for that file").returns(1) do
-        @instance.files.get('two').versions.collect(&:version).size
+        @instance.files.get('two').versions.all.collect(&:version).size
       end
 
       tests("#all for file returns only versions for that file").returns(versions.last) do
-        @instance.files.get('two').versions.collect(&:version).first
+        @instance.files.get('two').versions.all.collect(&:version).first
       end
     end