From c55eac1c9e822f38e45e7d3d55082ca833a5f790 Mon Sep 17 00:00:00 2001 From: Stephen Blackstone Date: Wed, 8 Jun 2016 11:20:57 -0400 Subject: [PATCH 1/5] Fix route discovery with api! for Rails 5 Since rails deprecated some behavior that the old code depended on, we just change it to match what its been doing under the hood all along and avoid having to do a depth first search at the same time. --- lib/apipie/application.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/apipie/application.rb b/lib/apipie/application.rb index e59044f14..516093c6c 100644 --- a/lib/apipie/application.rb +++ b/lib/apipie/application.rb @@ -54,14 +54,9 @@ def rails_routes(route_set = nil) # the app might be nested when using contraints, namespaces etc. # this method does in depth search for the route controller def route_app_controller(app, route, visited_apps = []) - visited_apps << app - if app.respond_to?(:controller) - return app.controller(route.defaults) - elsif app.respond_to?(:app) && !visited_apps.include?(app.app) - return route_app_controller(app.app, route, visited_apps) + if route.defaults[:controller] + (route.defaults[:controller].camelize + "Controller").constantize end - rescue ActionController::RoutingError - # some errors in the routes will not stop us here: just ignoring end def routes_for_action(controller, method, args) From 5c90ed85584d8492d9e351dd9c85d371e628a22d Mon Sep 17 00:00:00 2001 From: Amin Ariana Date: Thu, 16 Feb 2017 11:07:12 -0800 Subject: [PATCH 2/5] Test against the newest ruby and rails versions --- .travis.yml | 10 ++++++++++ Gemfile.rails41 | 2 +- Gemfile.rails50 | 6 ++++++ spec/dummy/config/environments/development.rb | 3 +++ spec/dummy/config/environments/production.rb | 3 +++ spec/dummy/config/environments/test.rb | 3 +++ 6 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 Gemfile.rails50 diff --git a/.travis.yml b/.travis.yml index c0b78f0fe..06b8077ee 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,8 +5,18 @@ rvm: - 2.0.0 - 2.1.7 - 2.2.3 + - 2.3.3 gemfile: - Gemfile.rails32 - Gemfile.rails40 - Gemfile.rails41 - Gemfile.rails42 + - Gemfile.rails50 # Min ruby 2.2.2 +matrix: + exclude: + - rvm: 1.9.3 + gemfile: Gemfile.rails50 + - rvm: 2.0.0 + gemfile: Gemfile.rails50 + - rvm: 2.1.7 + gemfile: Gemfile.rails50 \ No newline at end of file diff --git a/Gemfile.rails41 b/Gemfile.rails41 index 0a7a2f156..1ea07cd22 100644 --- a/Gemfile.rails41 +++ b/Gemfile.rails41 @@ -3,4 +3,4 @@ source "https://rubygems.org" gemspec gem 'rails', '~> 4.1.0' -gem 'mime-types', '~> 2.99.3' \ No newline at end of file +gem 'mime-types', '~> 2.99.3' diff --git a/Gemfile.rails50 b/Gemfile.rails50 new file mode 100644 index 000000000..7fb13befd --- /dev/null +++ b/Gemfile.rails50 @@ -0,0 +1,6 @@ +source "https://rubygems.org" + +gemspec + +gem 'rails', '~> 5.0' +gem 'mime-types', '~> 2.99.3' diff --git a/spec/dummy/config/environments/development.rb b/spec/dummy/config/environments/development.rb index 4a797a03c..0be8bfa65 100644 --- a/spec/dummy/config/environments/development.rb +++ b/spec/dummy/config/environments/development.rb @@ -21,5 +21,8 @@ # Only use best-standards-support built into browsers config.action_dispatch.best_standards_support = :builtin + + # Do not eager load code on boot. (Rails 5) + config.eager_load = false end diff --git a/spec/dummy/config/environments/production.rb b/spec/dummy/config/environments/production.rb index cbdb27757..1ac38aea8 100644 --- a/spec/dummy/config/environments/production.rb +++ b/spec/dummy/config/environments/production.rb @@ -46,4 +46,7 @@ # Send deprecation notices to registered listeners config.active_support.deprecation = :notify + + # Eager load code on boot (Rails 5) + config.eager_load = true end diff --git a/spec/dummy/config/environments/test.rb b/spec/dummy/config/environments/test.rb index 768d8f044..5d38a3157 100644 --- a/spec/dummy/config/environments/test.rb +++ b/spec/dummy/config/environments/test.rb @@ -32,4 +32,7 @@ # Print deprecation notices to the stderr config.active_support.deprecation = :stderr + + # Do not eager load code on boot. (Rails 5) + config.eager_load = false end From 4de1df3135e336c5edbffda5d958e5108d428684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Ne=C4=8Das?= Date: Fri, 17 Feb 2017 00:16:19 +0100 Subject: [PATCH 3/5] Fix tests for rails 5 --- Gemfile.rails50 | 1 + lib/apipie/param_description.rb | 12 ++ spec/controllers/apipies_controller_spec.rb | 24 ++-- spec/controllers/concerns_controller_spec.rb | 2 +- spec/controllers/users_controller_spec.rb | 109 +++++++++---------- spec/dummy/config/routes.rb | 2 +- spec/spec_helper.rb | 27 +++++ 7 files changed, 107 insertions(+), 70 deletions(-) diff --git a/Gemfile.rails50 b/Gemfile.rails50 index 7fb13befd..18e224f14 100644 --- a/Gemfile.rails50 +++ b/Gemfile.rails50 @@ -4,3 +4,4 @@ gemspec gem 'rails', '~> 5.0' gem 'mime-types', '~> 2.99.3' +gem 'rails-controller-testing' diff --git a/lib/apipie/param_description.rb b/lib/apipie/param_description.rb index 95e6392fd..6c3780ed1 100644 --- a/lib/apipie/param_description.rb +++ b/lib/apipie/param_description.rb @@ -72,9 +72,20 @@ def from_concern? method_description.from_concern? || @from_concern end + def normalized_value(value) + if value.is_a?(ActionController::Parameters) && !value.is_a?(Hash) + value.to_unsafe_hash + elsif value.is_a? Array + value.map { |v| normalized_value (v) } + else + value + end + end + def validate(value) return true if @allow_nil && value.nil? return true if @allow_blank && value.blank? + value = normalized_value(value) if (!@allow_nil && value.nil?) || !@validator.valid?(value) error = @validator.error error = ParamError.new(error) unless error.is_a? StandardError @@ -83,6 +94,7 @@ def validate(value) end def process_value(value) + value = normalized_value(value) if @validator.respond_to?(:process_value) @validator.process_value(value) else diff --git a/spec/controllers/apipies_controller_spec.rb b/spec/controllers/apipies_controller_spec.rb index 0bbfcb1e2..867bf0a8d 100644 --- a/spec/controllers/apipies_controller_spec.rb +++ b/spec/controllers/apipies_controller_spec.rb @@ -12,37 +12,37 @@ end it "succeeds on version details" do - get :index, :version => "2.0" + get :index, :params => { :version => "2.0" } assert_response :success end it "returns not_found on wrong version" do - get :index, :version => "wrong_version" + get :index, :params => { :version => "wrong_version" } assert_response :not_found end it "succeeds on resource details" do - get :index, :version => "2.0", :resource => "architectures" + get :index, :params => { :version => "2.0", :resource => "architectures" } assert_response :success end it "returns not_found on wrong resource" do - get :index, :version => "2.0", :resource => "wrong_resource" + get :index, :params => { :version => "2.0", :resource => "wrong_resource" } assert_response :not_found end it "succeeds on method details" do - get :index, :version => "2.0", :resource => "architectures", :method => "index" + get :index, :params => { :version => "2.0", :resource => "architectures", :method => "index" } assert_response :success end it "returns not_found on wrong method" do - get :index, :version => "2.0", :resource => "architectures", :method => "wrong_method" + get :index, :params => { :version => "2.0", :resource => "architectures", :method => "wrong_method" } assert_response :not_found end @@ -215,17 +215,17 @@ it "uses the file in cache dir instead of generating the content on runtime" do get :index expect(response.body).to eq("apidoc.html cache v1") - get :index, :version => 'v1' + get :index, :params => { :version => 'v1' } expect(response.body).to eq("apidoc.html cache v1") - get :index, :version => 'v2' + get :index, :params => { :version => 'v2' } expect(response.body).to eq("apidoc.html cache v2") - get :index, :version => 'v1', :format => "html" + get :index, :params => { :version => 'v1', :format => "html" } expect(response.body).to eq("apidoc.html cache v1") - get :index, :version => 'v1', :format => "json" + get :index, :params => { :version => 'v1', :format => "json" } expect(response.body).to eq("apidoc.json cache") - get :index, :version => 'v1', :format => "html", :resource => "resource" + get :index, :params => { :version => 'v1', :format => "html", :resource => "resource" } expect(response.body).to eq("resource.html cache") - get :index, :version => 'v1', :format => "html", :resource => "resource", :method => "method" + get :index, :params => { :version => 'v1', :format => "html", :resource => "resource", :method => "method" } expect(response.body).to eq("method.html cache") end diff --git a/spec/controllers/concerns_controller_spec.rb b/spec/controllers/concerns_controller_spec.rb index a80d5c5fd..c1f5b7b33 100644 --- a/spec/controllers/concerns_controller_spec.rb +++ b/spec/controllers/concerns_controller_spec.rb @@ -8,7 +8,7 @@ end it "should reply to valid request" do - get :show, :id => '5', :session => "secret_hash" + get :show, :params => { :id => '5' }, :session => { :user_id => "secret_hash" } assert_response :success end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 9e5e9c5db..239a278d9 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -66,7 +66,7 @@ def compare_hashes(h1, h2) end it "should reply to valid request" do - get :show, :id => '5', :session => "secret_hash" + get :show, :params => { :id => '5', :session => "secret_hash" } assert_response :success end @@ -92,17 +92,17 @@ def reload_controllers end it "should reply to valid request" do - expect { get :show, :id => 5, :session => "secret_hash" }.not_to raise_error + expect { get :show, :params => { :id => 5, :session => "secret_hash" }}.not_to raise_error assert_response :success end it "should fail if required parameter is missing" do - expect { get :show, :id => 5 }.to raise_error(Apipie::ParamMissing, /session_parameter_is_required/) + expect { get :show, :params => { :id => 5 }}.to raise_error(Apipie::ParamMissing, /session_parameter_is_required/) end it "should pass if required parameter has wrong type" do - expect { get :show, :id => 5, :session => "secret_hash" }.not_to raise_error - expect { get :show, :id => "ten", :session => "secret_hash" }.not_to raise_error + expect { get :show, :params => { :id => 5 , :session => "secret_hash" }}.not_to raise_error + expect { get :show, :params => { :id => "ten" , :session => "secret_hash" }}.not_to raise_error end end @@ -115,12 +115,12 @@ def reload_controllers end it "should reply to valid request" do - expect { get :show, :id => 5, :session => "secret_hash" }.not_to raise_error + expect { get :show, :params => { :id => 5, :session => 'secret_hash' }}.not_to raise_error assert_response :success end it "should fail if extra parameter is passed in" do - expect { get :show, :id => 5, :session => "secret_hash", :badparam => 'badfoo' }.to raise_error(Apipie::UnknownParam, /\bbadparam\b/) + expect { get :show, :params => { :id => 5 , :badparam => 'badfoo', :session => "secret_hash" }}.to raise_error(Apipie::UnknownParam, /\bbadparam\b/) end end @@ -132,81 +132,75 @@ def reload_controllers end it "should reply to valid request" do - get :show, :id => '5', :session => "secret_hash" + get :show, :params => { :id => '5', :session => "secret_hash" } assert_response :success end it "should work with nil value for a required hash param" do expect { - get :show, :id => '5', :session => "secret_hash", :hash_param => {:dummy_hash => nil} + get :show, :params => { :id => '5', :session => "secret_hash", :hash_param => {:dummy_hash => nil} } }.to raise_error(Apipie::ParamInvalid, /dummy_hash/) assert_response :success end it "should fail if required parameter is missing" do - expect { get :show, :id => 5 }.to raise_error(Apipie::ParamMissing, /session_parameter_is_required/) + expect { get :show, :params => { :id => 5 }}.to raise_error(Apipie::ParamMissing, /session_parameter_is_required/) end it "should work with custom Type validator" do expect { get :show, - :id => "not a number", - :session => "secret_hash" + :params => { :id => "not a number", :session => "secret_hash" } }.to raise_error(Apipie::ParamError, /id/) # old-style error rather than ParamInvalid end it "should work with Regexp validator" do - get :show, - :id => 5, - :session => "secret_hash", - :regexp_param => "24 years" + get :show, :params => { :id => 5, :session => "secret_hash", :regexp_param => "24 years" } assert_response :success expect { - get :show, - :id => 5, - :session => "secret_hash", - :regexp_param => "ten years" + get :show, :params => { :id => 5, + :session => "secret_hash", + :regexp_param => "ten years" } }.to raise_error(Apipie::ParamInvalid, /regexp_param/) end it "should work with Array validator" do - get :show, :id => 5, :session => "secret_hash", :array_param => "one" - assert_response :success - get :show, :id => 5, :session => "secret_hash", :array_param => "two" + get :show, :params => { :id => 5, :session => "secret_hash", :array_param => "one" } assert_response :success - get :show, :id => 5, :session => "secret_hash", :array_param => '1' + get :show, :params => { :id => 5, :session => "secret_hash", :array_param => "two" } assert_response :success - get :show, :id => 5, :session => "secret_hash", :boolean_param => false + get :show, :params => { :id => 5, :session => "secret_hash", :array_param => '1' } assert_response :success expect { - get :show, - :id => 5, - :session => "secret_hash", - :array_param => "blabla" + get :show, :params => { :id => 5, + :session => "secret_hash", + :array_param => "blabla" } }.to raise_error(Apipie::ParamInvalid, /array_param/) expect { - get :show, - :id => 5, - :session => "secret_hash", - :array_param => 3 + get :show, :params => { + :id => 5, + :session => "secret_hash", + :array_param => 3 } }.to raise_error(Apipie::ParamInvalid, /array_param/) end it "should work with Proc validator" do expect { get :show, - :id => 5, - :session => "secret_hash", - :proc_param => "asdgsag" + :params => { + :id => 5, + :session => "secret_hash", + :proc_param => "asdgsag" } }.to raise_error(Apipie::ParamInvalid, /proc_param/) get :show, - :id => 5, - :session => "secret_hash", - :proc_param => "param value" + :params => { + :id => 5, + :session => "secret_hash", + :proc_param => "param value"} assert_response :success end @@ -262,7 +256,7 @@ def reload_controllers :pass => "12345", :membership => "standard", }, - :facts => nil + :facts => { :test => 'test' } assert_response :success end @@ -305,21 +299,21 @@ def reload_controllers it "should raise an error" do expect{ put :update, - { - :id => 5, - :user => { - :name => "root", - :pass => "12345" - }, - :comments => [ - { - :comment => 'comment1' + :params => { + :id => 5, + :user => { + :name => "root", + :pass => "12345" }, - { - :comment => {:bad_input => 5} - } - ] - } + :comments => [ + { + :comment => {:bad_input => 4} + }, + { + :comment => {:bad_input => 5} + } + ] + } }.to raise_error(Apipie::ParamInvalid) end end @@ -708,13 +702,16 @@ class IgnoredController < ApplicationController; end describe "Parameter processing / extraction" do before do + Apipie.configuration.validate = true Apipie.configuration.process_params = true + controllers_dirname = File.expand_path('../dummy/app/controllers', File.dirname(__FILE__)) + Dir.glob("#{controllers_dirname}/**/*") { |file| load(file) if File.file?(file) } end it "process correctly the parameters" do - post :create, {:user => {:name => 'dummy', :pass => 'dummy', :membership => 'standard'}, :facts => nil} + post :create, :user => {:name => 'dummy', :pass => 'dummy', :membership => 'standard' }, :facts => {:test => 'test'} - expect(assigns(:api_params).with_indifferent_access).to eq({:user => {:name=>"dummy", :pass=>"dummy", :membership=>"standard"}, :facts => nil}.with_indifferent_access) + expect(assigns(:api_params).with_indifferent_access).to eq({:user => {:name=>"dummy", :pass=>"dummy", :membership=>"standard"}, :facts => {:test => 'test'}}.with_indifferent_access) end it "ignore not described parameters" do diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index 2d3d48b01..d63cf976b 100644 --- a/spec/dummy/config/routes.rb +++ b/spec/dummy/config/routes.rb @@ -10,7 +10,7 @@ end resources :concerns, :only => [:index, :show] namespace :files do - get '/*file_path', to: :download, format: false + get '/*file_path', format: false, :action => 'download' end resources :twitter_example do collection do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9938dd5ad..5c4424cf0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -9,6 +9,30 @@ require 'apipie-rails' +module Rails4Compatibility + module Testing + def process(*args) + compatible_request(*args) { |*new_args| super(*new_args) } + end + + def compatible_request(method, action, hash = {}) + if hash.is_a?(Hash) + if Gem::Version.new(Rails.version) < Gem::Version.new('5.0.0') + hash = hash.dup + hash.merge!(hash.delete(:params) || {}) + elsif hash.key?(:params) + hash = { :params => hash } + end + end + if hash.empty? + yield method, action + else + yield method, action, hash + end + end + end +end + # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. Dir[File.expand_path("../support/**/*.rb", __FILE__)].each {|f| require f} @@ -41,3 +65,6 @@ # end config.infer_spec_type_from_file_location! end + +require 'action_controller/test_case.rb' +ActionController::TestCase::Behavior.prepend(Rails4Compatibility::Testing) From e64ad164bd88351088fc43ec212c77f11e9ced1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Ne=C4=8Das?= Date: Fri, 17 Feb 2017 00:27:48 +0100 Subject: [PATCH 4/5] Remove support for rails 3.2 It's not in our power to keep compatibility with ralis 3, 4 and 5 anymore. 3.2 needs to go out --- .travis.yml | 3 +-- Gemfile | 8 +------- Gemfile.rails32 | 6 ------ apipie-rails.gemspec | 2 +- 4 files changed, 3 insertions(+), 16 deletions(-) mode change 100644 => 120000 Gemfile delete mode 100644 Gemfile.rails32 diff --git a/.travis.yml b/.travis.yml index 06b8077ee..6bfc3a4a7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,6 @@ rvm: - 2.2.3 - 2.3.3 gemfile: - - Gemfile.rails32 - Gemfile.rails40 - Gemfile.rails41 - Gemfile.rails42 @@ -19,4 +18,4 @@ matrix: - rvm: 2.0.0 gemfile: Gemfile.rails50 - rvm: 2.1.7 - gemfile: Gemfile.rails50 \ No newline at end of file + gemfile: Gemfile.rails50 diff --git a/Gemfile b/Gemfile deleted file mode 100644 index c74bc4694..000000000 --- a/Gemfile +++ /dev/null @@ -1,7 +0,0 @@ -source "https://rubygems.org" - -gemspec - -# load local gemfile -local_gemfile = File.join(File.dirname(__FILE__), 'Gemfile.local') -self.instance_eval(Bundler.read_file(local_gemfile)) if File.exist?(local_gemfile) diff --git a/Gemfile b/Gemfile new file mode 120000 index 000000000..faa0ba8bc --- /dev/null +++ b/Gemfile @@ -0,0 +1 @@ +Gemfile.rails50 \ No newline at end of file diff --git a/Gemfile.rails32 b/Gemfile.rails32 deleted file mode 100644 index 808c331ef..000000000 --- a/Gemfile.rails32 +++ /dev/null @@ -1,6 +0,0 @@ -source "https://rubygems.org" - -gemspec - -gem 'rails', '~> 3.2.0' -gem 'test-unit', '~> 3.0' diff --git a/apipie-rails.gemspec b/apipie-rails.gemspec index 57ce5c1d6..4940f0e67 100644 --- a/apipie-rails.gemspec +++ b/apipie-rails.gemspec @@ -16,7 +16,7 @@ Gem::Specification.new do |s| s.test_files = `git ls-files -- {test,spec,features}/*`.split("\n") s.require_paths = ["lib"] - s.add_development_dependency "rails", ">= 3.0.20" + s.add_dependency "rails", ">= 4.0" s.add_dependency 'json' s.add_development_dependency "rspec-rails", "~> 3.0" s.add_development_dependency "sqlite3" From f187614d1abae6040f6fe2339600e86e3c8d3a27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Ne=C4=8Das?= Date: Fri, 17 Feb 2017 00:36:27 +0100 Subject: [PATCH 5/5] Keep compatibility with ruby 1.9.3 and 2.0.0 --- spec/spec_helper.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5c4424cf0..36e1c85a6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -11,8 +11,12 @@ module Rails4Compatibility module Testing - def process(*args) - compatible_request(*args) { |*new_args| super(*new_args) } + def process_with_rails4_compat(*args) + compatible_request(*args) { |*new_args| process_without_rails4_compat(*new_args) } + end + + def self.included(base) + base.alias_method_chain :process, :rails4_compat end def compatible_request(method, action, hash = {}) @@ -67,4 +71,5 @@ def compatible_request(method, action, hash = {}) end require 'action_controller/test_case.rb' -ActionController::TestCase::Behavior.prepend(Rails4Compatibility::Testing) +# TODO: replace with prepend once we deprecate ruby 2.0.0 +ActionController::TestCase::Behavior.send(:include, Rails4Compatibility::Testing)