Permalink
Browse files

Merge pull request #732 from nirvdrum/s3_test_fixes

[aws|storage|test] Make sure tests pass with both mocking enabled and disabled.
  • Loading branch information...
2 parents 7c62e44 + aff7038 commit 5eba7505a6978cab275ae43e16173645fd37990c @geemus geemus committed Feb 6, 2012
View
2 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'
View
8 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)
View
16 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
View
2 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
View
4 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>..."
View
11 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
View
2 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
View
6 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
View
4 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
View
15 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

0 comments on commit 5eba750

Please sign in to comment.