incubator-deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lut...@redhat.com
Subject [PATCH 5/5] rabbit: remove any static dependencies on the driver
Date Sat, 22 Jan 2011 00:02:56 GMT
From: David Lutterkort <lutter@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


Mime
View raw message