You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by ma...@redhat.com on 2011/01/25 21:04:35 UTC
[PATCH] Adds ability to specify arbitrary user metadata (KEY-VALUE) for S3, Cloudfiles and Azure, during blob creation
From: marios <ma...@redhat.com>
---
server/lib/deltacloud/base_driver/base_driver.rb | 4 +-
.../lib/deltacloud/drivers/azure/azure_driver.rb | 30 +++++++++------
server/lib/deltacloud/drivers/ec2/ec2_driver.rb | 23 +++++++-----
server/lib/deltacloud/drivers/mock/mock_driver.rb | 10 +++---
.../drivers/rackspace/rackspace_driver.rb | 37 ++++++++++++-------
server/lib/deltacloud/helpers/blob_stream.rb | 17 +++++++++
server/lib/deltacloud/models/blob.rb | 1 +
server/public/javascripts/application.js | 35 ++++++++++++++++++
server/server.rb | 18 +++++++++-
server/views/blobs/new.html.haml | 18 +++++++++-
server/views/blobs/show.html.haml | 3 ++
server/views/blobs/show.xml.haml | 4 ++-
12 files changed, 155 insertions(+), 45 deletions(-)
diff --git a/server/lib/deltacloud/base_driver/base_driver.rb b/server/lib/deltacloud/base_driver/base_driver.rb
index bf06e56..2442385 100644
--- a/server/lib/deltacloud/base_driver/base_driver.rb
+++ b/server/lib/deltacloud/base_driver/base_driver.rb
@@ -186,12 +186,12 @@ module Deltacloud
storage_snapshots(credentials, opts).first if has_capability?(:storage_snapshots)
end
- def bucket(credentials, opts = nil)
+ def bucket(credentials, opts = {})
#list of objects within bucket
buckets(credentials, opts).first if has_capability?(:buckets)
end
- def blob(credentials, opts = nil)
+ def blob(credentials, opts = {})
blobs(credentials, opts).first if has_capability?(:blobs)
end
diff --git a/server/lib/deltacloud/drivers/azure/azure_driver.rb b/server/lib/deltacloud/drivers/azure/azure_driver.rb
index abbd353..8d7b1e6 100644
--- a/server/lib/deltacloud/drivers/azure/azure_driver.rb
+++ b/server/lib/deltacloud/drivers/azure/azure_driver.rb
@@ -32,7 +32,7 @@ class AzureDriver < Deltacloud::BaseDriver
#--
# Buckets
#--
- def buckets(credentials, opts)
+ def buckets(credentials, opts={})
buckets = []
azure_connect(credentials)
safely do
@@ -46,7 +46,7 @@ class AzureDriver < Deltacloud::BaseDriver
#--
# Create bucket
#--
- def create_bucket(credentials, name, opts)
+ def create_bucket(credentials, name, opts={})
#for whatever reason, bucket names MUST be lowercase...
#http://msdn.microsoft.com/en-us/library/dd135715.aspx
name.downcase!
@@ -62,7 +62,7 @@ class AzureDriver < Deltacloud::BaseDriver
#--
# Delete bucket
#--
- def delete_bucket(credentials, name, opts)
+ def delete_bucket(credentials, name, opts={})
azure_connect(credentials)
safely do
WAZ::Blobs::Container.find(name).destroy!
@@ -72,7 +72,7 @@ class AzureDriver < Deltacloud::BaseDriver
#--
# Blobs
#--
- def blobs(credentials, opts)
+ def blobs(credentials, opts={})
blob_list = []
azure_connect(credentials)
safely do
@@ -85,7 +85,7 @@ class AzureDriver < Deltacloud::BaseDriver
blob_list
end
- def blob_data(credentials, bucket_id, blob_id, opts)
+ def blob_data(credentials, bucket_id, blob_id, opts={})
azure_connect(credentials)
# WAZ get blob data methods cant accept blocks for 'streaming'... FIXME
yield WAZ::Blobs::Container.find(bucket_id)[blob_id].value
@@ -94,23 +94,28 @@ class AzureDriver < Deltacloud::BaseDriver
#--
# Create Blob
#--
- def create_blob(credentials, bucket_id, blob_id, blob_data, opts=nil)
+ def create_blob(credentials, bucket_id, blob_id, blob_data, opts={})
azure_connect(credentials)
- #get a handle to the bucket in order to put there
- the_bucket = WAZ::Blobs::Container.find(bucket_id)
- the_bucket.store(blob_id, blob_data[:tempfile], blob_data[:type])
+ #insert azure-specific header for user metadata ... x-ms-meta-kEY = VALUE
+ opts.gsub_keys("HTTP_X_Deltacloud_Blobmeta_", "x-ms-meta-")
+ safely do
+ #get a handle to the bucket in order to put there
+ the_bucket = WAZ::Blobs::Container.find(bucket_id)
+ the_bucket.store(blob_id, blob_data[:tempfile], blob_data[:type], opts)
+ end
Blob.new( { :id => blob_id,
:bucket => bucket_id,
:content_lengh => blob_data[:tempfile].length,
:content_type => blob_data[:type],
- :last_modified => ''
+ :last_modified => '',
+ :user_metadata => opts.select{|k,v| k.match(/^x-ms-meta-/)}
} )
end
#--
# Delete Blob
#--
- def delete_blob(credentials, bucket_id, blob_id, opts=nil)
+ def delete_blob(credentials, bucket_id, blob_id, opts={})
azure_connect(credentials)
#get a handle to bucket and blob, and destroy!
the_bucket = WAZ::Blobs::Container.find(bucket_id)
@@ -146,7 +151,8 @@ class AzureDriver < Deltacloud::BaseDriver
:bucket => bucket,
:content_length => waz_blob.metadata[:content_length],
:content_type => waz_blob.metadata[:content_type],
- :last_modified => waz_blob.metadata[:last_modified]
+ :last_modified => waz_blob.metadata[:last_modified],
+ :user_metadata => waz_blob.metadata.select{|k,v| k.to_s.match(/^x_ms_meta_/)}
})
end
diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
index 7a4b394..c9e5319 100644
--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
@@ -305,7 +305,7 @@ module Deltacloud
end
- def buckets(credentials, opts)
+ def buckets(credentials, opts={})
buckets = []
safely do
s3_client = new_client(credentials, :s3)
@@ -338,7 +338,7 @@ module Deltacloud
end
end
- def blobs(credentials, opts = nil)
+ def blobs(credentials, opts = {})
s3_client = new_client(credentials, :s3)
blobs = []
safely do
@@ -354,25 +354,29 @@ module Deltacloud
#--
# Create Blob
#--
- def create_blob(credentials, bucket_id, blob_id, data = nil, opts = nil)
+ def create_blob(credentials, bucket_id, blob_id, data = nil, opts = {})
s3_client = new_client(credentials, :s3)
#data is a construct with the temporary file created by server @.tempfile
#also file[:type] will give us the content-type
res = nil
# File stream needs to be reopened in binary mode for whatever reason
file = File::open(data[:tempfile].path, 'rb')
+ #insert ec2-specific header for user metadata ... x-amz-meta-KEY = VALUE
+ opts.gsub_keys('HTTP_X_Deltacloud_Blobmeta_', 'x-amz-meta-')
+ opts["Content-Type"] = data[:type]
safely do
res = s3_client.interface.put(bucket_id,
blob_id,
file,
- {"Content-Type" => data[:type]})
+ opts)
end
#create a new Blob object and return that
Blob.new( { :id => blob_id,
:bucket => bucket_id,
:content_length => data[:tempfile].length,
:content_type => data[:type],
- :last_modified => ''
+ :last_modified => '',
+ :user_metadata => opts.select{|k,v| k.match(/^x-amz-meta-/)}
}
)
end
@@ -380,7 +384,7 @@ module Deltacloud
#--
# Delete Blob
#--
- def delete_blob(credentials, bucket_id, blob_id, opts=nil)
+ def delete_blob(credentials, bucket_id, blob_id, opts={})
s3_client = new_client(credentials, :s3)
safely do
s3_client.interface.delete(bucket_id, blob_id)
@@ -388,7 +392,7 @@ module Deltacloud
end
- def blob_data(credentials, bucket_id, blob_id, opts)
+ def blob_data(credentials, bucket_id, blob_id, opts={})
s3_client = new_client(credentials, :s3)
safely do
s3_client.interface.get(bucket_id, blob_id) do |chunk|
@@ -503,7 +507,7 @@ module Deltacloud
'us-east-1' => 'elasticloadbalancing.us-east-1.amazonaws.com',
'us-west-1' => 'elasticloadbalancing.us-west-1.amazonaws.com'
},
-
+
's3' => {
'ap-southeast-1' => 's3.amazonaws.com',
'eu-west-1' => 's3.amazonaws.com',
@@ -568,7 +572,8 @@ module Deltacloud
:bucket => s3_object.bucket.name.to_s,
:content_length => s3_object.headers['content-length'],
:content_type => s3_object.headers['content-type'],
- :last_modified => s3_object.last_modified
+ :last_modified => s3_object.last_modified,
+ :user_metadata => s3_object.meta_headers
)
end
diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb b/server/lib/deltacloud/drivers/mock/mock_driver.rb
index 0f91355..e21988e 100644
--- a/server/lib/deltacloud/drivers/mock/mock_driver.rb
+++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb
@@ -318,7 +318,7 @@ class MockDriver < Deltacloud::BaseDriver
#--
# Buckets
#--
- def buckets(credentials, opts=nil)
+ def buckets(credentials, opts={})
check_credentials(credentials)
buckets=[]
Dir[ "#{@storage_root}/buckets/*.yml" ].each do |bucket_file|
@@ -334,7 +334,7 @@ class MockDriver < Deltacloud::BaseDriver
#--
# Create bucket
#--
- def create_bucket(credentials, name, opts=nil)
+ def create_bucket(credentials, name, opts={})
check_credentials(credentials)
bucket = {
:name=>name,
@@ -348,7 +348,7 @@ class MockDriver < Deltacloud::BaseDriver
#--
# Delete bucket
#--
- def delete_bucket(credentials, name, opts=nil)
+ def delete_bucket(credentials, name, opts={})
bucket = bucket(credentials, {:id => name})
unless (bucket.size == "0")
raise Deltacloud::BackendError.new(403, self.class.to_s, "bucket-not-empty", "delete operation not valid for non-empty bucket")
@@ -361,7 +361,7 @@ class MockDriver < Deltacloud::BaseDriver
#--
# Blobs
#--
- def blobs(credentials, opts = nil)
+ def blobs(credentials, opts = {})
check_credentials(credentials)
blobs=[]
Dir[ "#{@storage_root}/buckets/blobs/*.yml" ].each do |blob_file|
@@ -377,7 +377,7 @@ class MockDriver < Deltacloud::BaseDriver
#--
# Blob content
#--
- def blob_data(credentials, bucket_id, blob_id, opts = nil)
+ def blob_data(credentials, bucket_id, blob_id, opts = {})
check_credentials(credentials)
blob=nil
Dir[ "#{@storage_root}/buckets/blobs/*.yml" ].each do |blob_file|
diff --git a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
index e40fb9c..8e6612b 100644
--- a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
+++ b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
@@ -166,7 +166,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
#--
# Buckets
#--
- def buckets(credentials, opts)
+ def buckets(credentials, opts = {})
bucket_list = []
cf = cloudfiles_client(credentials)
safely do
@@ -182,7 +182,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
#--
# Create Bucket
#--
- def create_bucket(credentials, name, opts)
+ def create_bucket(credentials, name, opts = {})
bucket = nil
cf = cloudfiles_client(credentials)
safely do
@@ -195,7 +195,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
#--
# Delete Bucket
#--
- def delete_bucket(credentials, name, opts)
+ def delete_bucket(credentials, name, opts = {})
cf = cloudfiles_client(credentials)
safely do
cf.delete_container(name)
@@ -205,7 +205,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
#--
# Blobs
#--
- def blobs(credentials, opts)
+ def blobs(credentials, opts = {})
cf = cloudfiles_client(credentials)
blobs = []
safely do
@@ -221,7 +221,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
#-
# Blob data
#-
- def blob_data(credentials, bucket_id, blob_id, opts)
+ def blob_data(credentials, bucket_id, blob_id, opts = {})
cf = cloudfiles_client(credentials)
cf.container(bucket_id).object(blob_id).data_stream do |chunk|
yield chunk
@@ -231,18 +231,26 @@ class RackspaceDriver < Deltacloud::BaseDriver
#--
# Create Blob
#--
- def create_blob(credentials, bucket_id, blob_id, blob_data, opts=nil)
+ def create_blob(credentials, bucket_id, blob_id, blob_data, opts={})
cf = cloudfiles_client(credentials)
- #must first create the object using cloudfiles_client.create_object
- #then can write using object.write(data)
- object = cf.container(bucket_id).create_object(blob_id)
- #blob_data is a construct with data in .tempfile and content-type in {:type}
- res = object.write(blob_data[:tempfile], {'Content-Type' => blob_data[:type]})
+ user_meta = opts.select{|k,v| k.match(/^META-/)}
+ #insert ec2-specific header for user metadata ... X-Object-Meta-KEY = VALUE
+ opts.gsub_keys("HTTP_X_Deltacloud_Blobmeta_", "X-Object-Meta-")
+ opts['Content-Type'] = blob_data[:type]
+ object = nil
+ safely do
+ #must first create the object using cloudfiles_client.create_object
+ #then can write using object.write(data)
+ object = cf.container(bucket_id).create_object(blob_id)
+ #blob_data is a construct with data in .tempfile and content-type in {:type}
+ res = object.write(blob_data[:tempfile], opts)
+ end
Blob.new( { :id => object.name,
:bucket => object.container.name,
:content_length => blob_data[:tempfile].length,
:content_type => blob_data[:type],
- :last_modified => ''
+ :last_modified => '',
+ :user_metadata => opts.select{|k,v| k.match(/^X-Object-Meta-/)}
}
)
end
@@ -250,7 +258,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
#--
# Delete Blob
#--
- def delete_blob(credentials, bucket_id, blob_id, opts=nil)
+ def delete_blob(credentials, bucket_id, blob_id, opts={})
cf = cloudfiles_client(credentials)
cf.container(bucket_id).delete_object(blob_id)
end
@@ -292,7 +300,8 @@ private
:bucket => cf_object.container.name,
:content_length => cf_object.bytes,
:content_type => cf_object.content_type,
- :last_modified => cf_object.last_modified
+ :last_modified => cf_object.last_modified,
+ :user_metadata => cf_object.metadata
})
end
diff --git a/server/lib/deltacloud/helpers/blob_stream.rb b/server/lib/deltacloud/helpers/blob_stream.rb
index a99e6c2..0b21088 100644
--- a/server/lib/deltacloud/helpers/blob_stream.rb
+++ b/server/lib/deltacloud/helpers/blob_stream.rb
@@ -62,3 +62,20 @@ rescue LoadError => e
end
end
end
+
+class Hash
+
+ def gsub_keys(pattern, replacement)
+ remove = []
+ self.each_key do |key|
+ if key.to_s.match(pattern)
+ new_key = key.gsub(pattern, replacement)
+ self[new_key] = self[key]
+ remove << key
+ end #key.match
+ end # each_key do
+ #remove the original keys
+ self.delete_if{|k,v| remove.include?(k)}
+ end #def
+
+end #class
diff --git a/server/lib/deltacloud/models/blob.rb b/server/lib/deltacloud/models/blob.rb
index dfa67fe..0bf473b 100644
--- a/server/lib/deltacloud/models/blob.rb
+++ b/server/lib/deltacloud/models/blob.rb
@@ -24,5 +24,6 @@ class Blob < BaseModel
attr_accessor :content_type
attr_accessor :last_modified
attr_accessor :content
+ attr_accessor :user_metadata
end
diff --git a/server/public/javascripts/application.js b/server/public/javascripts/application.js
index 5ee0f7f..95c9bc2 100644
--- a/server/public/javascripts/application.js
+++ b/server/public/javascripts/application.js
@@ -16,3 +16,38 @@ $(document).ready(function() {
}
})
+
+function more_fields()
+{
+ //increment the hidden input that captures how many meta_data are passed
+ var meta_params = document.getElementsByName('meta_params')
+ current_number_params = eval(meta_params[0].value)+1
+ meta_params[0].value = current_number_params
+ var new_meta = document.getElementById('metadata_holder').cloneNode(true);
+ new_meta.id = 'metadata_holder' + current_number_params;
+ new_meta.style.display = 'block';
+ var nodes = new_meta.childNodes;
+ for (var i=0;i < nodes.length;i++) {
+ var theName = nodes[i].name;
+ if (theName)
+ nodes[i].name = theName + current_number_params;
+ }
+ var insertHere = document.getElementById('metadata_holder');
+ insertHere.parentNode.insertBefore(new_meta,insertHere);
+}
+
+function less_fields()
+{
+ var meta_params = document.getElementsByName('meta_params')
+ current_val = eval(meta_params[0].value)
+ if (current_val == 0)
+ {
+ return;
+ }
+ else
+ {
+ var theDiv = document.getElementById('metadata_holder'+current_val)
+ theDiv.parentNode.removeChild(theDiv)
+ meta_params[0].value = eval(current_val)-1
+ }
+}
diff --git a/server/server.rb b/server/server.rb
index e32e2a4..344360f 100644
--- a/server/server.rb
+++ b/server/server.rb
@@ -594,7 +594,23 @@ post '/api/buckets/:bucket' do
bucket_id = params[:bucket]
blob_id = params['blob_id']
blob_data = params['blob_data']
- @blob = driver.create_blob(credentials, bucket_id, blob_id, blob_data )
+ user_meta = {}
+#first try get blob_metadata from params (i.e., passed by http form post, e.g. browser)
+ max = params[:meta_params]
+ if(max)
+ (1..max.to_i).each do |i|
+ key = params[:"meta_name#{i}"]
+ key = "HTTP_X_Deltacloud_Blobmeta_#{key}"
+ value = params[:"meta_value#{i}"]
+ user_meta[key] = value
+ end #max.each do
+ else #can try to get blob_metadata from http headers
+ meta_array = request.env.select{|k,v| k.match(/^HTTP[-_]X[-_]Deltacloud[-_]Blobmeta[-_]/i)}
+ meta_array.each do |k,v|
+ user_meta[k] = v
+ end
+ end #end if
+ @blob = driver.create_blob(credentials, bucket_id, blob_id, blob_data, user_meta)
respond_to do |format|
format.html { haml :"blobs/show"}
format.xml { haml :"blobs/show" }
diff --git a/server/views/blobs/new.html.haml b/server/views/blobs/new.html.haml
index feb94e5..a9817d5 100644
--- a/server/views/blobs/new.html.haml
+++ b/server/views/blobs/new.html.haml
@@ -4,7 +4,23 @@
%label
Blob Name:
%input{ :name => 'blob_id', :size => 512}/
+ %label
Blob Data:
- %input{ :type => "file", :name => 'blob_data', :size => 50}/
%br
+ %input{ :type => "file", :name => 'blob_data', :size => 50}/
+ %br
+ %br
+ %input{ :type => "hidden", :name => "meta_params", :value => "0"}
+ %a{ :href => "javascript:;", :onclick => "more_fields();"} Add Metadata
+ %div{ :id => "metadata_holder", :style => "display: none;"}
+ %label
+ Metadata Name:
+ %input{ :type => "text", :name => "meta_name", :value => ""}/
+ %label
+ Metadata Value:
+ %input{ :type => "text", :name => "meta_value", :value => ""}/
+ %br
+ %a{ :href => "javascript:;", :onclick => "less_fields();"} Less Metadata
+ %br
+ %br
%input{ :type => :submit, :name => "commit", :value => "create"}/
diff --git a/server/views/blobs/show.html.haml b/server/views/blobs/show.html.haml
index e5c2cee..29025c0 100644
--- a/server/views/blobs/show.html.haml
+++ b/server/views/blobs/show.html.haml
@@ -19,6 +19,9 @@
%dt Content
%dd
=link_to 'Blob content', bucket_url(@blob.bucket) + '/' + @blob.id + '/content'
+ %dt User_Metadata
+ %dd
+ = @blob.user_metadata.inspect
%dt Delete this Blob
%dd
%form{ :action => bucket_url(@blob.bucket) + '/' + @blob.id , :method => :post }
diff --git a/server/views/blobs/show.xml.haml b/server/views/blobs/show.xml.haml
index bc499ca..1b9ea55 100644
--- a/server/views/blobs/show.xml.haml
+++ b/server/views/blobs/show.xml.haml
@@ -1,7 +1,9 @@
!!! XML
%blob{:href => bucket_url(@blob.bucket) + '/' + @blob.id, :id => @blob.id}
- - @blob.attributes.select{ |attr| attr!=:id}.each do |attribute|
+ - @blob.attributes.select{ |attr| (attr!=:id && attr!=:user_metadata) }.each do |attribute|
- unless attribute == :content
- haml_tag(attribute, :<) do
- haml_concat @blob.send(attribute)
+ - @blob.user_metadata.each do |k, v|
+ %user_metadata{:name => k, :value => v}
%content{:href => bucket_url(@blob.bucket) + '/' + @blob.id + '/content'}
--
1.7.3.4
Re: [PATCH] Adds ability to specify arbitrary user metadata (KEY-VALUE)
for S3, Cloudfiles and Azure, during blob creation
Posted by "marios@redhat.com" <ma...@redhat.com>.
Hi David,
many thanks for taking the time again, I'll fix the nits you mention and
send out a final patch to make sure everyone is happy.
I cannot address 'hash.select' returning a multi-dimensional array
rather than a hash - this is a ruby version issue. Prior to 1.9 an array
is returned, but since 1.9 a hash is returned (current rubydoc
http://www.ruby-doc.org/core/classes/Hash.html#M000750). We manipulate
how this is given to the client using the haml though so I just
considered it to be an 'internal' issue (i mean the haml converts the
'multi-array' to a hash),
marios
On 28/01/11 03:04, David Lutterkort wrote:
> On Tue, 2011-01-25 at 22:04 +0200, marios@redhat.com wrote:
>> From: marios<ma...@redhat.com>
>
> ACK, with a few nits.
>
>> diff --git a/server/lib/deltacloud/drivers/azure/azure_driver.rb b/server/lib/deltacloud/drivers/azure/azure_driver.rb
>> index abbd353..8d7b1e6 100644
>> --- a/server/lib/deltacloud/drivers/azure/azure_driver.rb
>> +++ b/server/lib/deltacloud/drivers/azure/azure_driver.rb
>> @@ -94,23 +94,28 @@ class AzureDriver< Deltacloud::BaseDriver
>> #--
>> # Create Blob
>> #--
>> - def create_blob(credentials, bucket_id, blob_id, blob_data, opts=nil)
>> + def create_blob(credentials, bucket_id, blob_id, blob_data, opts={})
>> azure_connect(credentials)
>> - #get a handle to the bucket in order to put there
>> - the_bucket = WAZ::Blobs::Container.find(bucket_id)
>> - the_bucket.store(blob_id, blob_data[:tempfile], blob_data[:type])
>> + #insert azure-specific header for user metadata ... x-ms-meta-kEY = VALUE
>> + opts.gsub_keys("HTTP_X_Deltacloud_Blobmeta_", "x-ms-meta-")
>> + safely do
>> + #get a handle to the bucket in order to put there
>> + the_bucket = WAZ::Blobs::Container.find(bucket_id)
>> + the_bucket.store(blob_id, blob_data[:tempfile], blob_data[:type], opts)
>> + end
>> Blob.new( { :id => blob_id,
>> :bucket => bucket_id,
>> :content_lengh => blob_data[:tempfile].length,
>> :content_type => blob_data[:type],
>> - :last_modified => ''
>> + :last_modified => '',
>> + :user_metadata => opts.select{|k,v| k.match(/^x-ms-meta-/)}
>
> This sets user_metadata to an array of pairs, i.e. sth like [[:a, 1],
> [:b, 2]] instead of a hash. Also, the key names will still have the
> x-ms-meta prefix - shouldn't that be stripped ?
>
>> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>> index 7a4b394..c9e5319 100644
>> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>> @@ -354,25 +354,29 @@ module Deltacloud
>> #--
>> # Create Blob
>> #--
>> - def create_blob(credentials, bucket_id, blob_id, data = nil, opts = nil)
>> + def create_blob(credentials, bucket_id, blob_id, data = nil, opts = {})
>> s3_client = new_client(credentials, :s3)
>> #data is a construct with the temporary file created by server @.tempfile
>> #also file[:type] will give us the content-type
>> res = nil
>> # File stream needs to be reopened in binary mode for whatever reason
>> file = File::open(data[:tempfile].path, 'rb')
>> + #insert ec2-specific header for user metadata ... x-amz-meta-KEY = VALUE
>> + opts.gsub_keys('HTTP_X_Deltacloud_Blobmeta_', 'x-amz-meta-')
>> + opts["Content-Type"] = data[:type]
>> safely do
>> res = s3_client.interface.put(bucket_id,
>> blob_id,
>> file,
>> - {"Content-Type" => data[:type]})
>> + opts)
>> end
>> #create a new Blob object and return that
>> Blob.new( { :id => blob_id,
>> :bucket => bucket_id,
>> :content_length => data[:tempfile].length,
>> :content_type => data[:type],
>> - :last_modified => ''
>> + :last_modified => '',
>> + :user_metadata => opts.select{|k,v| k.match(/^x-amz-meta-/)}
>
> Same comment as for Azure.
>
>> diff --git a/server/views/blobs/new.html.haml b/server/views/blobs/new.html.haml
>> index feb94e5..a9817d5 100644
>> --- a/server/views/blobs/new.html.haml
>> +++ b/server/views/blobs/new.html.haml
>> @@ -4,7 +4,23 @@
>> %label
>> Blob Name:
>> %input{ :name => 'blob_id', :size => 512}/
>> + %label
>> Blob Data:
>> - %input{ :type => "file", :name => 'blob_data', :size => 50}/
>> %br
>> + %input{ :type => "file", :name => 'blob_data', :size => 50}/
>> + %br
>> + %br
>> + %input{ :type => "hidden", :name => "meta_params", :value => "0"}
>> + %a{ :href => "javascript:;", :onclick => "more_fields();"} Add Metadata
>> + %div{ :id => "metadata_holder", :style => "display: none;"}
>> + %label
>> + Metadata Name:
>
> Minor nit: shouldn't we call that a metadata key ? Seems that's the
> general nomenclature.
>
>> diff --git a/server/views/blobs/show.html.haml b/server/views/blobs/show.html.haml
>> index e5c2cee..29025c0 100644
>> --- a/server/views/blobs/show.html.haml
>> +++ b/server/views/blobs/show.html.haml
>> @@ -19,6 +19,9 @@
>> %dt Content
>> %dd
>> =link_to 'Blob content', bucket_url(@blob.bucket) + '/' + @blob.id + '/content'
>> + %dt User_Metadata
>> + %dd
>> + = @blob.user_metadata.inspect
>
> Another nit: maybe we should format this more friendly than with
> inspect ;)
>
>> diff --git a/server/views/blobs/show.xml.haml b/server/views/blobs/show.xml.haml
>> index bc499ca..1b9ea55 100644
>> --- a/server/views/blobs/show.xml.haml
>> +++ b/server/views/blobs/show.xml.haml
>> @@ -1,7 +1,9 @@
>> !!! XML
>> %blob{:href => bucket_url(@blob.bucket) + '/' + @blob.id, :id => @blob.id}
>> - - @blob.attributes.select{ |attr| attr!=:id}.each do |attribute|
>> + - @blob.attributes.select{ |attr| (attr!=:id&& attr!=:user_metadata) }.each do |attribute|
>> - unless attribute == :content
>> - haml_tag(attribute, :<) do
>> - haml_concat @blob.send(attribute)
>> + - @blob.user_metadata.each do |k, v|
>> + %user_metadata{:name => k, :value => v}
>
> Do we need to worry about the size of v and possibly that it might
> contain illegal characters here ? Might be safer to make the value the
> content of the tag, possibly in a CDATA section.
>
> Also, I'd prefer if the metadata is inside a container tag, like
>
> <user_metadata>
> <entry key="artist">John Lennon</entry>
> ...
> <user_metadata>
>
> David
>
Re: [PATCH] Adds ability to specify arbitrary user metadata
(KEY-VALUE) for S3, Cloudfiles and Azure, during blob creation
Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2011-01-25 at 22:04 +0200, marios@redhat.com wrote:
> From: marios <ma...@redhat.com>
ACK, with a few nits.
> diff --git a/server/lib/deltacloud/drivers/azure/azure_driver.rb b/server/lib/deltacloud/drivers/azure/azure_driver.rb
> index abbd353..8d7b1e6 100644
> --- a/server/lib/deltacloud/drivers/azure/azure_driver.rb
> +++ b/server/lib/deltacloud/drivers/azure/azure_driver.rb
> @@ -94,23 +94,28 @@ class AzureDriver < Deltacloud::BaseDriver
> #--
> # Create Blob
> #--
> - def create_blob(credentials, bucket_id, blob_id, blob_data, opts=nil)
> + def create_blob(credentials, bucket_id, blob_id, blob_data, opts={})
> azure_connect(credentials)
> - #get a handle to the bucket in order to put there
> - the_bucket = WAZ::Blobs::Container.find(bucket_id)
> - the_bucket.store(blob_id, blob_data[:tempfile], blob_data[:type])
> + #insert azure-specific header for user metadata ... x-ms-meta-kEY = VALUE
> + opts.gsub_keys("HTTP_X_Deltacloud_Blobmeta_", "x-ms-meta-")
> + safely do
> + #get a handle to the bucket in order to put there
> + the_bucket = WAZ::Blobs::Container.find(bucket_id)
> + the_bucket.store(blob_id, blob_data[:tempfile], blob_data[:type], opts)
> + end
> Blob.new( { :id => blob_id,
> :bucket => bucket_id,
> :content_lengh => blob_data[:tempfile].length,
> :content_type => blob_data[:type],
> - :last_modified => ''
> + :last_modified => '',
> + :user_metadata => opts.select{|k,v| k.match(/^x-ms-meta-/)}
This sets user_metadata to an array of pairs, i.e. sth like [[:a, 1],
[:b, 2]] instead of a hash. Also, the key names will still have the
x-ms-meta prefix - shouldn't that be stripped ?
> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> index 7a4b394..c9e5319 100644
> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> @@ -354,25 +354,29 @@ module Deltacloud
> #--
> # Create Blob
> #--
> - def create_blob(credentials, bucket_id, blob_id, data = nil, opts = nil)
> + def create_blob(credentials, bucket_id, blob_id, data = nil, opts = {})
> s3_client = new_client(credentials, :s3)
> #data is a construct with the temporary file created by server @.tempfile
> #also file[:type] will give us the content-type
> res = nil
> # File stream needs to be reopened in binary mode for whatever reason
> file = File::open(data[:tempfile].path, 'rb')
> + #insert ec2-specific header for user metadata ... x-amz-meta-KEY = VALUE
> + opts.gsub_keys('HTTP_X_Deltacloud_Blobmeta_', 'x-amz-meta-')
> + opts["Content-Type"] = data[:type]
> safely do
> res = s3_client.interface.put(bucket_id,
> blob_id,
> file,
> - {"Content-Type" => data[:type]})
> + opts)
> end
> #create a new Blob object and return that
> Blob.new( { :id => blob_id,
> :bucket => bucket_id,
> :content_length => data[:tempfile].length,
> :content_type => data[:type],
> - :last_modified => ''
> + :last_modified => '',
> + :user_metadata => opts.select{|k,v| k.match(/^x-amz-meta-/)}
Same comment as for Azure.
> diff --git a/server/views/blobs/new.html.haml b/server/views/blobs/new.html.haml
> index feb94e5..a9817d5 100644
> --- a/server/views/blobs/new.html.haml
> +++ b/server/views/blobs/new.html.haml
> @@ -4,7 +4,23 @@
> %label
> Blob Name:
> %input{ :name => 'blob_id', :size => 512}/
> + %label
> Blob Data:
> - %input{ :type => "file", :name => 'blob_data', :size => 50}/
> %br
> + %input{ :type => "file", :name => 'blob_data', :size => 50}/
> + %br
> + %br
> + %input{ :type => "hidden", :name => "meta_params", :value => "0"}
> + %a{ :href => "javascript:;", :onclick => "more_fields();"} Add Metadata
> + %div{ :id => "metadata_holder", :style => "display: none;"}
> + %label
> + Metadata Name:
Minor nit: shouldn't we call that a metadata key ? Seems that's the
general nomenclature.
> diff --git a/server/views/blobs/show.html.haml b/server/views/blobs/show.html.haml
> index e5c2cee..29025c0 100644
> --- a/server/views/blobs/show.html.haml
> +++ b/server/views/blobs/show.html.haml
> @@ -19,6 +19,9 @@
> %dt Content
> %dd
> =link_to 'Blob content', bucket_url(@blob.bucket) + '/' + @blob.id + '/content'
> + %dt User_Metadata
> + %dd
> + = @blob.user_metadata.inspect
Another nit: maybe we should format this more friendly than with
inspect ;)
> diff --git a/server/views/blobs/show.xml.haml b/server/views/blobs/show.xml.haml
> index bc499ca..1b9ea55 100644
> --- a/server/views/blobs/show.xml.haml
> +++ b/server/views/blobs/show.xml.haml
> @@ -1,7 +1,9 @@
> !!! XML
> %blob{:href => bucket_url(@blob.bucket) + '/' + @blob.id, :id => @blob.id}
> - - @blob.attributes.select{ |attr| attr!=:id}.each do |attribute|
> + - @blob.attributes.select{ |attr| (attr!=:id && attr!=:user_metadata) }.each do |attribute|
> - unless attribute == :content
> - haml_tag(attribute, :<) do
> - haml_concat @blob.send(attribute)
> + - @blob.user_metadata.each do |k, v|
> + %user_metadata{:name => k, :value => v}
Do we need to worry about the size of v and possibly that it might
contain illegal characters here ? Might be safer to make the value the
content of the tag, possibly in a CDATA section.
Also, I'd prefer if the metadata is inside a container tag, like
<user_metadata>
<entry key="artist">John Lennon</entry>
...
<user_metadata>
David