On Aug 31, 2012, at 2:48 PM, [email protected] wrote: Just more clarification why this is needed:
If you try to run Deltacloud in OpenShift, our current config.ru will use 'mount' in Passenger. Without Sinatra 'url()' helper all links are broken (they does not have /api prefix). This patch should fix that as well. -- Michal > From: Michal Fojtik <[email protected]> > > * Updated url_for helper code to latest version from github[1] > * Improved handling of matrix parameters > * Added 'url()' Sinatra helper to produce valid links when Deltacloud > is 'mounted' in another Rack application. > > Signed-off-by: Michal fojtik <[email protected]> > --- > server/Rakefile | 1 + > server/lib/cimi/helpers.rb | 2 +- > server/lib/deltacloud/helpers.rb | 2 +- > server/lib/deltacloud/helpers/url_helper.rb | 183 ++++++++++++-------- > .../tests/helpers/sinatra/url_for_helper_test.rb | 69 ++++++++ > 5 files changed, 183 insertions(+), 74 deletions(-) > create mode 100644 server/tests/helpers/sinatra/url_for_helper_test.rb > > diff --git a/server/Rakefile b/server/Rakefile > index 80d45d3..c7cad4b 100644 > --- a/server/Rakefile > +++ b/server/Rakefile > @@ -165,6 +165,7 @@ namespace :test do > t.test_files = FileList[ > 'tests/helpers/core_ext/*test.rb', # Deltacloud extensions > (core_ext) and other helpers > 'tests/helpers/rack/*test.rb', # Rack extensions Deltacloud > use > + 'tests/helpers/sinatra/*test.rb', # Sinatra helpers and > extensions Deltacloud use > 'tests/drivers/base/*test.rb', # Deltacloud drivers API tests > 'tests/drivers/models/*test.rb', # Deltacloud models tests > 'tests/deltacloud/*test.rb', # Deltacloud internal API > tests > diff --git a/server/lib/cimi/helpers.rb b/server/lib/cimi/helpers.rb > index 2dba3be..93910e4 100644 > --- a/server/lib/cimi/helpers.rb > +++ b/server/lib/cimi/helpers.rb > @@ -50,12 +50,12 @@ module CIMI::Collections > > helpers Deltacloud::Helpers::Drivers > helpers Sinatra::AuthHelper > - helpers Sinatra::UrlForHelper > helpers Rack::RespondTo::Helpers > helpers Deltacloud::Helpers::Application > helpers CIMIHelper > > register Rack::RespondTo > + register Sinatra::UrlFor > > enable :xhtml > enable :dump_errors > diff --git a/server/lib/deltacloud/helpers.rb > b/server/lib/deltacloud/helpers.rb > index 6566c59..9ce14bf 100644 > --- a/server/lib/deltacloud/helpers.rb > +++ b/server/lib/deltacloud/helpers.rb > @@ -29,11 +29,11 @@ module Deltacloud::Collections > > helpers Deltacloud::Helpers::Drivers > helpers Sinatra::AuthHelper > - helpers Sinatra::UrlForHelper > helpers Rack::RespondTo::Helpers > helpers Deltacloud::Helpers::Application > > register Rack::RespondTo > + register Sinatra::UrlFor > > enable :xhtml > enable :dump_errors > diff --git a/server/lib/deltacloud/helpers/url_helper.rb > b/server/lib/deltacloud/helpers/url_helper.rb > index 1b084d4..ff5c910 100644 > --- a/server/lib/deltacloud/helpers/url_helper.rb > +++ b/server/lib/deltacloud/helpers/url_helper.rb > @@ -25,91 +25,130 @@ > # USE OR OTHER DEALINGS IN THE SOFTWARE. > > module Sinatra > - module UrlForHelper > + module UrlFor > > require 'uri' > > - def method_missing(name, *args) > - if name.to_s =~ /^([\w\_]+)_url$/ > - if args.size > 0 > - t = $1 > - if t.match(/^(stop|reboot|start|attach|detach)_/) > - action = $1 > - api_url_for(t.pluralize.split('_').last + '/' + args.first.to_s > + '/' + action, :full) > - elsif t.match(/^(destroy|update)_/) > - api_url_for(t.pluralize.split('_').last + '/' + args.first.to_s, > :full) > - else > - api_url_for(t.pluralize, :full) + '/' + "#{args.first}" > + # FIXME: > + # This is the list of 'allowable' actions. Everything else is considered > + # as a collection by the UrlFor: > + # > + ACTIONS = [ > + :start, :stop, :new, :run, :reboot, :attach, :detach, :register, > :unregister > + ] > + > + def self.valid_action?(action) > + ACTIONS.include?(action.to_sym) > + end > + > + def self.registered(app) > + app.helpers UrlFor::Helper > + app.class_eval do > + def method_missing(name, *args) > + action_list = Sinatra::UrlFor::ACTIONS.join('|') > + return super unless name =~ %r[^((#{action_list})_)?([\w_]+)_url$] > + action, resource, options = $2, $3, (args.first || {}) > + > + options = { :id => options } unless options.is_a? Hash > + > + # instances_url => /instances > + # instances_url( :format => :xml) => /instances?format=xml > + # > + if (action.nil? or (action && action=='create')) and options.empty? > + return url_for('/%s' % resource, options.reject { |k, v| k == > :id }) > end > - else > - api_url_for($1, :full) > + > + # instance_url(:id => 'i-111') => /instances/i-111 > + # instance_url(:id => 'i-123', :format => 'xml') => > /instances/i-123?format=xml > + if action.nil? and options.has_key?(:id) > + raise TypeError.new("The :id parameter cannot be nil > (/#{resource}/nil)") if options[:id].nil? > + return url_for('/' + [resource, options[:id]].compact.join('/'), > + options.reject { |k, v| k == :id }) > + end > + > + # stop_instance_url(:id => 'i-1234') => /instances/i-123/stop > + # > + # NOTE: The resource must be in singular! > + # > + if action and options.has_key?(:id) > + action = nil if ['create', 'destroy'].include? action > + return url_for('/' + [resource.pluralize, options[:id], > action].compact.join('/'), > + options.reject { |k, v| k == :id }) > + end > + > + # create_image_url => /images > + # > + if action and action == :create > + return url_for('/' + resource.pluralize, options.reject { |k,v| > l == :id }) > + end > + > + super > end > - else > - super > end > end > > - def api_url_for(url_fragment, mode=:path_only) > - matrix_params = '' > - if request.params['api'] > - matrix_params += ";provider=%s" % request.params['api']['provider'] > if request.params['api']['provider'] > - matrix_params += ";driver=%s" % request.params['api']['driver'] if > request.params['api']['driver'] > - end > - url_fragment = "/#{url_fragment}" unless url_fragment =~ /^\// # There > is no need to prefix URI with '/' > - if mode == :path_only > - url_for "#{settings.root_url}#{matrix_params}#{url_fragment}", mode > - else > - url_for "#{matrix_params}#{url_fragment}", :full > - end > - end > + module Helper > > - # Construct a link to +url_fragment+, which should be given relative to > - # the base of this Sinatra app. The mode should be either > - # <code>:path_only</code>, which will generate an absolute path within > - # the current domain (the default), or <code>:full</code>, which will > - # include the site name and port number. (The latter is typically > - # necessary for links in RSS feeds.) Example usage: > - # > - # url_for "/" # Returns "/myapp/" > - # url_for "/foo" # Returns "/myapp/foo" > - # url_for "/foo", :full # Returns "http://example.com/myapp/foo"; > - #-- > - # See README.rdoc for a list of some of the people who helped me clean > - # up earlier versions of this code. > - def url_for url_fragment, mode=:path_only > - case mode > - when :path_only > - base = request.script_name.empty? ? > Deltacloud.default_frontend.root_url : request.script_name > - when :full > - scheme = request.scheme > - port = request.port > - request_host = request.host > - if request.env['HTTP_X_FORWARDED_FOR'] > - scheme = request.env['HTTP_X_FORWARDED_SCHEME'] || scheme > - port = request.env['HTTP_X_FORWARDED_PORT'] > - request_host = request.env['HTTP_X_FORWARDED_HOST'] > + # Code originaly copied from https://github.com/emk/sinatra-url-for > + # Copyright 2009 Eric Kidd > + def url_for(url_fragment, mode=nil, options = nil) > + > + if mode.is_a? Hash > + options = mode > + mode = nil > + end > + > + mode ||= :full > + > + mode = mode.to_sym unless mode.is_a? Symbol > + optstring = nil > + > + # Deal with matrix parameters > + # > + if request.params['api'] and request.params['api'].is_a? Hash > + matrix_params = '' > + if request.params['api']['driver'] > + matrix_params += ';driver=%s' % request.params['api']['driver'] > + end > + if request.params['api']['provider'] > + matrix_params += ';provider=%s' % > request.params['api']['provider'] > + end > + end > + > + if options.is_a? Hash > + optstring = '?' + options.map do |k,v| > + next if k.nil? > + "#{k}=#{URI.escape(v.to_s, /[^#{URI::PATTERN::UNRESERVED}]/)}" > + end.compact.join('&') if !options.empty? > end > - if (port.nil? || port == "" || > - (scheme == 'http' && port.to_s == '80') || > - (scheme == 'https' && port.to_s == '443')) > - port = "" > + > + case mode > + when :path_only > + base = url request.script_name > + when :full > + scheme = request.scheme > + if (scheme == 'http' && request.port == 80 || > + scheme == 'https' && request.port == 443) > + port = "" > + else > + port = ":#{request.port}" > + end > + if matrix_params > + uri_components = url_fragment.split('/') > + if uri_components.size>1 > + uri_components[1] = uri_components[1] + matrix_params > + url_fragment = uri_components.join('/') > + end > + end > + base = url(request.script_name).gsub(/\/$/, '') > else > - port = ":#{port}" > + raise TypeError, "Unknown url_for mode #{mode.inspect}" > end > - base = > "#{scheme}://#{request_host}#{port}#{request.script_name.empty? ? > settings.config.root_url : request.script_name}" > - else > - raise TypeError, "Unknown url_for mode #{mode}" > - end > - uri_parser = URI.const_defined?(:Parser) ? URI::Parser.new : URI > - url_escape = uri_parser.escape(url_fragment) > - # Don't add the base fragment if url_for gets called more than once > - # per url or the url_fragment passed in is an absolute url > - if url_escape.match(/^#{base}/) or url_escape.match(/^http/) > - url_escape > - else > - "#{base}#{url_escape}" > + "#{base}#{url_fragment}#{optstring}" > end > + > + alias_method :api_url_for, :url_for > end > - end > > + end > end > diff --git a/server/tests/helpers/sinatra/url_for_helper_test.rb > b/server/tests/helpers/sinatra/url_for_helper_test.rb > new file mode 100644 > index 0000000..159f657 > --- /dev/null > +++ b/server/tests/helpers/sinatra/url_for_helper_test.rb > @@ -0,0 +1,69 @@ > +require 'rubygems' > +require 'require_relative' if RUBY_VERSION < '1.9' > + > +require_relative '../../test_helper.rb' > +require_relative '../rack/common.rb' > +require_relative '../../../lib/deltacloud/helpers/url_helper.rb' > + > +class TestURLApp < Sinatra::Base > + register Sinatra::UrlFor > + use Rack::MatrixParams > + > + get '/api/:id' do > + case params[:id] > + when '1' then tests_url > + when '2' then tests_url(:id => '1') > + when '3' then stop_test_url(:id => '1') > + when '4' then tests_url(:id => '1', :format => :xml) > + when '5' then tests_url(:id => nil) > + end > + end > +end > + > +describe Sinatra::UrlFor do > + > + before do > + def app; TestURLApp; end > + end > + > + it 'should return /tests for for test_url' do > + must_return_for '1', 'http://example.org/tests' > + end > + > + it 'should return /tests/1 for test_url(:id => 1)' do > + must_return_for '2', 'http://example.org/tests/1' > + end > + > + it 'should return /tests/1/stop for stop_test_url(:id => 1)' do > + must_return_for '3', 'http://example.org/tests/1/stop' > + end > + > + it 'should return /tests/1?format=xml for stop_test_url(:id => 1, :format > => :xml)' do > + must_return_for '4', 'http://example.org/tests/1?format=xml' > + end > + > + it 'should preserve the matrix parameters in URL' do > + get '/api;driver=mock/1' > + response_body.must_equal 'http://example.org/tests;driver=mock' > + get '/api;driver=mock/2' > + response_body.must_equal 'http://example.org/tests;driver=mock/1' > + get '/api;driver=mock/3' > + response_body.must_equal 'http://example.org/tests;driver=mock/1/stop' > + get '/api;driver=mock/4' > + response_body.must_equal > 'http://example.org/tests;driver=mock/1?format=xml' > + end > + > + it 'should raise exception when :id is nil for test_url(:id => nil)' do > + lambda { get '/api/5' }.must_raise TypeError > + end > + > + private > + > + def must_return_for(url, value) > + get '/api/%s' % url > + status.must_equal 200 > + response_body.wont_be_empty > + response_body.must_equal value > + end > + > +end > -- > 1.7.10.2 > Michal Fojtik http://deltacloud.org [email protected]
