You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by mf...@redhat.com on 2012/07/11 11:13:47 UTC

[PATCH core] Core: Fixed broken listing of supported collections due to bug in capability checking

From: Michal Fojtik <mf...@redhat.com>


Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/deltacloud/helpers/deltacloud_helper.rb |   13 +++++++++++++
 server/lib/deltacloud/server.rb                    |    2 +-
 server/views/api/show.html.haml                    |    3 +--
 server/views/api/show.xml.haml                     |    3 +--
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/server/lib/deltacloud/helpers/deltacloud_helper.rb b/server/lib/deltacloud/helpers/deltacloud_helper.rb
index de28590..cec22ac 100644
--- a/server/lib/deltacloud/helpers/deltacloud_helper.rb
+++ b/server/lib/deltacloud/helpers/deltacloud_helper.rb
@@ -18,6 +18,19 @@ module Deltacloud::Helpers
 
     require 'benchmark'
 
+    def supported_collections
+      Deltacloud::Collections.deltacloud_modules.each do |m|
+        m.collections.each do |c|
+          # Get the required capability for the :index operation (like 'realms' or 'instance_state_machine')
+          index_operation_capability = c.operation(:index).required_capability
+          # Then use this capability to check if the 'capability' lambda defined
+          # for the Sinatra::Base class evaluate to 'true'
+          next if m.settings.respond_to?(:capability) and !m.settings.capability(index_operation_capability)
+          yield c if block_given?
+        end
+      end
+    end
+
     def auth_feature_name
       return 'key' if driver.class.has_feature?(:instances, :authentication_key)
       return 'password' if driver.class.has_feature?(:instances, :authentication_password)
diff --git a/server/lib/deltacloud/server.rb b/server/lib/deltacloud/server.rb
index 10af202..d842c62 100644
--- a/server/lib/deltacloud/server.rb
+++ b/server/lib/deltacloud/server.rb
@@ -56,7 +56,7 @@ module Deltacloud
     end
 
     options Deltacloud.config[:deltacloud].root_url + '/?' do
-      headers 'Allow' => Deltacloud.collections.select { |c| driver.respond_to?(c.collection_name) }.map { |c| c.collection_name }.join(',')
+      headers 'Allow' => supported_collections { |c| c.collection_name }.join(',')
     end
 
     post Deltacloud.config[:deltacloud].root_url + '/?' do
diff --git a/server/views/api/show.html.haml b/server/views/api/show.html.haml
index ef666d8..d315ebb 100644
--- a/server/views/api/show.html.haml
+++ b/server/views/api/show.html.haml
@@ -3,8 +3,7 @@
 
 %div{ :'data-role' => :content, :'data-theme' => 'c'}
   %ul{ :'data-role' => :listview, :'data-inset' => 'true'}
-    - Deltacloud.collections.each do |c|
-      - next unless c.operation(:index).has_capability?
+    - supported_collections do |c|
       %li
         %a{ :href => api_url_for(c.collection_name),  :'data-icon' => "arrow-r"}=c.collection_name.to_s.gsub('_', ' ').titlecase
 
diff --git a/server/views/api/show.xml.haml b/server/views/api/show.xml.haml
index 4ba8168..73e7171 100644
--- a/server/views/api/show.xml.haml
+++ b/server/views/api/show.xml.haml
@@ -1,6 +1,5 @@
 %api{ :version => settings.version, :driver => driver_symbol, :provider => Thread.current[:provider] || ENV['API_PROVIDER'] }
-  - Deltacloud.collections.each do |c|
-    - next unless c.operation(:index).has_capability?
+  - supported_collections do |c|
     %link{ :rel => c.collection_name, :href => self.send(:"#{c.collection_name}_url")}
       - c.features.select { |f| driver.class.has_feature?(c.collection_name, f.name) }.each do |f|
         %feature{ :name => f.name }
-- 
1.7.7.5 (Apple Git-26)


Re: [PATCH core] Core: Fixed broken listing of supported collections due to bug in capability checking

Posted by "marios@redhat.com" <ma...@redhat.com>.
ACK -

I tried with sinatra-rabbit 1.0.6/7/8 against mock driver - just GET
/api - seems ok now - no explosions or showing of collections that
shouldn't be there.




On 11/07/12 12:13, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> 
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
>  server/lib/deltacloud/helpers/deltacloud_helper.rb |   13 +++++++++++++
>  server/lib/deltacloud/server.rb                    |    2 +-
>  server/views/api/show.html.haml                    |    3 +--
>  server/views/api/show.xml.haml                     |    3 +--
>  4 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/server/lib/deltacloud/helpers/deltacloud_helper.rb b/server/lib/deltacloud/helpers/deltacloud_helper.rb
> index de28590..cec22ac 100644
> --- a/server/lib/deltacloud/helpers/deltacloud_helper.rb
> +++ b/server/lib/deltacloud/helpers/deltacloud_helper.rb
> @@ -18,6 +18,19 @@ module Deltacloud::Helpers
>  
>      require 'benchmark'
>  
> +    def supported_collections
> +      Deltacloud::Collections.deltacloud_modules.each do |m|
> +        m.collections.each do |c|
> +          # Get the required capability for the :index operation (like 'realms' or 'instance_state_machine')
> +          index_operation_capability = c.operation(:index).required_capability
> +          # Then use this capability to check if the 'capability' lambda defined
> +          # for the Sinatra::Base class evaluate to 'true'
> +          next if m.settings.respond_to?(:capability) and !m.settings.capability(index_operation_capability)
> +          yield c if block_given?
> +        end
> +      end
> +    end
> +
>      def auth_feature_name
>        return 'key' if driver.class.has_feature?(:instances, :authentication_key)
>        return 'password' if driver.class.has_feature?(:instances, :authentication_password)
> diff --git a/server/lib/deltacloud/server.rb b/server/lib/deltacloud/server.rb
> index 10af202..d842c62 100644
> --- a/server/lib/deltacloud/server.rb
> +++ b/server/lib/deltacloud/server.rb
> @@ -56,7 +56,7 @@ module Deltacloud
>      end
>  
>      options Deltacloud.config[:deltacloud].root_url + '/?' do
> -      headers 'Allow' => Deltacloud.collections.select { |c| driver.respond_to?(c.collection_name) }.map { |c| c.collection_name }.join(',')
> +      headers 'Allow' => supported_collections { |c| c.collection_name }.join(',')
>      end
>  
>      post Deltacloud.config[:deltacloud].root_url + '/?' do
> diff --git a/server/views/api/show.html.haml b/server/views/api/show.html.haml
> index ef666d8..d315ebb 100644
> --- a/server/views/api/show.html.haml
> +++ b/server/views/api/show.html.haml
> @@ -3,8 +3,7 @@
>  
>  %div{ :'data-role' => :content, :'data-theme' => 'c'}
>    %ul{ :'data-role' => :listview, :'data-inset' => 'true'}
> -    - Deltacloud.collections.each do |c|
> -      - next unless c.operation(:index).has_capability?
> +    - supported_collections do |c|
>        %li
>          %a{ :href => api_url_for(c.collection_name),  :'data-icon' => "arrow-r"}=c.collection_name.to_s.gsub('_', ' ').titlecase
>  
> diff --git a/server/views/api/show.xml.haml b/server/views/api/show.xml.haml
> index 4ba8168..73e7171 100644
> --- a/server/views/api/show.xml.haml
> +++ b/server/views/api/show.xml.haml
> @@ -1,6 +1,5 @@
>  %api{ :version => settings.version, :driver => driver_symbol, :provider => Thread.current[:provider] || ENV['API_PROVIDER'] }
> -  - Deltacloud.collections.each do |c|
> -    - next unless c.operation(:index).has_capability?
> +  - supported_collections do |c|
>      %link{ :rel => c.collection_name, :href => self.send(:"#{c.collection_name}_url")}
>        - c.features.select { |f| driver.class.has_feature?(c.collection_name, f.name) }.each do |f|
>          %feature{ :name => f.name }
>