You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by lu...@redhat.com on 2011/01/22 01:02:56 UTC
[PATCH 5/5] rabbit: remove any static dependencies on the driver
From: David Lutterkort <lu...@redhat.com>
With driver switching, we only have a driver for the duration of the
request. That means the request handlers that rabbit generates can not
depend on the features of a prticular driver at generation time. Instead,
those dependencies need to be taken into account when a request is
processed.
In particular, we need to generate handlers for all collections and their
operations, even if some drivers do not support them; at request time, we
check if the selected driver supports the collection/operation and generate
a 404 if it does not.
We also generate the complete list of parameters for an operation only at
request time (via effective_params); this is the list of parameters against
which we validate.
---
server/lib/deltacloud/base_driver/features.rb | 20 ++++++---
server/lib/deltacloud/validation.rb | 6 +-
server/lib/sinatra/rabbit.rb | 56 +++++++++++++++---------
server/server.rb | 4 ++
server/tests/common.rb | 12 ++++-
server/tests/drivers/mock/api_test.rb | 30 +++++++++++---
server/tests/rabbit_test.rb | 36 ++++++++++++++++
7 files changed, 125 insertions(+), 39 deletions(-)
create mode 100644 server/tests/rabbit_test.rb
diff --git a/server/lib/deltacloud/base_driver/features.rb b/server/lib/deltacloud/base_driver/features.rb
index f20f635..4264b6d 100644
--- a/server/lib/deltacloud/base_driver/features.rb
+++ b/server/lib/deltacloud/base_driver/features.rb
@@ -39,13 +39,19 @@ module Deltacloud
@description
end
- # Add a new operation or modify an existing one through BLOCK
+ # Add/modify an operation or look up an existing one. If +block+ is
+ # provided, create a new operation if none exists with name
+ # +name+. Evaluate the +block+ against this instance. If no +block+
+ # is provided, look up the operation with name +name+
def operation(name, &block)
- unless op = @operations.find { |op| op.name == name }
- op = Operation.new(name, &block)
- @operations << op
- else
- op.instance_eval(&block) if block_given?
+ op = @operations.find { |op| op.name == name }
+ if block_given?
+ if op.nil?
+ op = Operation.new(name, &block)
+ @operations << op
+ else
+ op.instance_eval(&block)
+ end
end
op
end
@@ -122,7 +128,7 @@ module Deltacloud
f.operations.detect { |o| o.name == operation }
end
end
-
+
#
# Declaration of optional features
#
diff --git a/server/lib/deltacloud/validation.rb b/server/lib/deltacloud/validation.rb
index 18e71cc..73aa64f 100644
--- a/server/lib/deltacloud/validation.rb
+++ b/server/lib/deltacloud/validation.rb
@@ -51,7 +51,7 @@ module Deltacloud::Validation
end
def param(*args)
- raise DuplicateParamException if params[args[0]]
+ raise "Duplicate param #{args[0]} #{params.inspect} #{self.class.name}" if params[args[0]]
p = Param.new(args)
params[p.name] = p
end
@@ -74,8 +74,8 @@ module Deltacloud::Validation
params.each_value { |p| yield p }
end
- def validate(values)
- each_param do |p|
+ def validate(all_params, values)
+ all_params.each_value do |p|
if p.required? and not values[p.name]
raise Failure.new(p, "Required parameter #{p.name} not found")
end
diff --git a/server/lib/sinatra/rabbit.rb b/server/lib/sinatra/rabbit.rb
index e843366..a2b1c3d 100644
--- a/server/lib/sinatra/rabbit.rb
+++ b/server/lib/sinatra/rabbit.rb
@@ -10,9 +10,10 @@ module Sinatra
class DuplicateParamException < Exception; end
class DuplicateOperationException < Exception; end
class DuplicateCollectionException < Exception; end
+ class UnsupportedCollectionException < Exception; end
class Operation
- attr_reader :name, :method
+ attr_reader :name, :method, :collection
include ::Deltacloud::BackendCapability
include ::Deltacloud::Validation
@@ -60,8 +61,9 @@ module Sinatra
def control(&block)
op = self
@control = Proc.new do
- op.check_capability(Deltacloud::driver)
- op.validate(params)
+ op.collection.check_supported(driver)
+ op.check_capability(driver)
+ op.validate(op.effective_params(driver), params)
instance_eval(&block)
end
end
@@ -96,6 +98,24 @@ module Sinatra
end
end
+ # Return a hash of all params, the params statically defined for this
+ # operation plus the params defined by any features in the +driver+
+ # that might modify this operation
+ def effective_params(driver)
+ driver.features(@collection.name).collect do |f|
+ f.decl.operation(@name)
+ end.flatten.select { |op| op }.inject(params.dup) do |result, fop|
+ fop.params.each_key do |k|
+ if result.has_key?(k)
+ raise DuplicateParamException, "Parameter '#{k}' for operation #{fop.name} in collection #{@collection.name}"
+ else
+ result[k] = fop.params[k]
+ end
+ end
+ result
+ end
+ end
+
private
def gen_route(name)
route_url = path
@@ -133,9 +153,12 @@ module Sinatra
end
def generate_documentation
- coll, oper, features = self, @operations, Deltacloud::driver.features(name)
+ coll = self
::Sinatra::Application.get("/api/docs/#{@name}") do
- @collection, @operations, @features = coll, oper, features
+ coll.check_supported(driver)
+ @collection = coll
+ @operations = coll.operations
+ @features = driver.features(coll.name)
respond_to do |format|
format.html { haml :'docs/collection' }
format.xml { haml :'docs/collection' }
@@ -178,19 +201,10 @@ module Sinatra
end
end
- def add_feature_params(features)
- features.each do |f|
- f.operations.each do |fop|
- if cop = operations[fop.name]
- fop.params.each_key do |k|
- if cop.params.has_key?(k)
- raise DuplicateParamException, "Parameter '#{k}' for operation #{fop.name} defined by collection #{@name} and by feature #{f.name}"
- else
- cop.params[k] = fop.params[k]
- end
- end
- end
- end
+ def check_supported(driver)
+ unless driver.has_collection?(@name)
+ raise UnsupportedCollectionException,
+ "Collection #{@name} not supported by this driver"
end
end
end
@@ -206,9 +220,7 @@ module Sinatra
# operation on this collection.
def collection(name, &block)
raise DuplicateCollectionException if collections[name]
- return unless Deltacloud::driver.has_collection?(name.to_sym)
collections[name] = Collection.new(name, &block)
- collections[name].add_feature_params(Deltacloud::driver.features(name))
collections[name].generate
end
@@ -229,7 +241,9 @@ module Sinatra
end
def entry_points
- collections.values.inject([]) do |m, coll|
+ collections.values.select { |coll|
+ driver.has_collection?(coll.name)
+ }.inject([]) do |m, coll|
url = url_for coll.operations[:index].path, :full
m << [ coll.name, url ]
end
diff --git a/server/server.rb b/server/server.rb
index 8c3b72c..6987815 100644
--- a/server/server.rb
+++ b/server/server.rb
@@ -66,6 +66,10 @@ error Deltacloud::BackendError do
report_error(500, "backend_error")
end
+error Sinatra::Rabbit::UnsupportedCollectionException do
+ report_error(404, "not_found")
+end
+
Sinatra::Application.register Sinatra::RespondTo
# Redirect to /api
diff --git a/server/tests/common.rb b/server/tests/common.rb
index 3751aaf..6efe089 100644
--- a/server/tests/common.rb
+++ b/server/tests/common.rb
@@ -73,7 +73,7 @@ module DeltacloudTestCommon
puts "[401] Authentication required to get #{uri}" if last_response.status == 401
if last_response.status == 200
@xml_response = false
- @xml_response = Nokogiri::XML(last_response.body)
+ @xml_response = Nokogiri::XML(last_response.body)
end
end
@@ -91,7 +91,15 @@ module DeltacloudTestCommon
$created_instances << id
end
+ def with_provider(new_provider, &block)
+ old_provider = ENV["API_PROVIDER"]
+ begin
+ ENV["API_PROVIDER"] = new_provider
+ yield
+ ensure
+ ENV["API_PROVIDER"] = old_provider
+ end
+ end
end
include DeltacloudTestCommon
-
diff --git a/server/tests/drivers/mock/api_test.rb b/server/tests/drivers/mock/api_test.rb
index 7dae8d1..c0cda07 100644
--- a/server/tests/drivers/mock/api_test.rb
+++ b/server/tests/drivers/mock/api_test.rb
@@ -35,17 +35,35 @@ module DeltacloudUnitTest
end
def test_it_switches_drivers
- begin
- ENV.delete("API_PROVIDER")
+ with_provider("") do
do_xml_request '/api'
(last_xml_response/"api/link[rel = 'instances']").first.should_not == nil
+ end
- # Switch to storage-only mock driver
- ENV["API_PROVIDER"] = "storage"
+ # Switch to storage-only mock driver
+ with_provider("storage") do
do_xml_request '/api'
(last_xml_response/"api/link[rel = 'instances']").first.should == nil
- ensure
- ENV.delete("API_PROVIDER")
+ end
+ end
+
+ def test_it_handles_unsupported_collections
+ do_xml_request '/api/no_such_collection'
+ last_response.status.should == 404
+
+ with_provider("storage") do
+ do_xml_request '/api/instances'
+ last_response.status.should == 404
+ end
+ end
+
+ def test_it_allows_accessing_docs
+ do_request '/api/docs/instances'
+ last_response.status.should == 200
+
+ with_provider("storage") do
+ do_request '/api/docs/instances'
+ last_response.status.should == 404
end
end
end
diff --git a/server/tests/rabbit_test.rb b/server/tests/rabbit_test.rb
new file mode 100644
index 0000000..265175a
--- /dev/null
+++ b/server/tests/rabbit_test.rb
@@ -0,0 +1,36 @@
+$:.unshift File.join(File.dirname(__FILE__), '..', '..', '..')
+require 'tests/common'
+
+require 'drivers'
+require 'deltacloud/drivers/mock/mock_driver'
+
+module DeltacloudUnitTest
+ class ApiTest < Test::Unit::TestCase
+ include Rack::Test::Methods
+
+ def setup
+ @app ||= Sinatra::Application
+ @driver ||= Deltacloud::Drivers::Mock::MockDriver.new
+ end
+
+ def teardown
+ @app = nil
+ @driver = nil
+ end
+
+ def test_params
+ op = @app.collections[:instances].operations[:create]
+ op.params.keys =~ [:realm_id, :image_id, :hwp_id]
+ end
+
+ def test_effective_params
+ features = @driver.features(:instances).collect { |f| f.name }
+ features.should =~ [:hardware_profiles, :user_name, :authentication_key]
+
+ op = @app.collections[:instances].operations[:create]
+ op.effective_params(@driver).keys.should =~ [:image_id, :hwp_memory, :hwp_id, :keyname, :name, :hwp_storage, :realm_id]
+
+ op.params.keys =~ [:realm_id, :image_id, :hwp_id]
+ end
+ end
+end
--
1.7.3.4