diff options
111 files changed, 1392 insertions, 637 deletions
@@ -45,7 +45,7 @@ end # Active Support. gem 'dalli', '>= 2.2.1' -gem 'listen', '~> 3.0.4' +gem 'listen', '~> 3.0.5' # Active Job. group :job do diff --git a/Gemfile.lock b/Gemfile.lock index fdd4946c4a..6c31a26a25 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -176,7 +176,7 @@ GEM builder (3.2.2) bunny (2.2.0) amq-protocol (>= 2.0.0) - byebug (6.0.2) + byebug (8.2.0) celluloid (0.17.2) celluloid-essentials celluloid-extras @@ -222,7 +222,7 @@ GEM kindlerb (0.1.1) mustache nokogiri - listen (3.0.4) + listen (3.0.5) rb-fsevent (>= 0.9.3) rb-inotify (>= 0.9) loofah (2.0.3) @@ -342,7 +342,7 @@ DEPENDENCIES jquery-rails! json kindlerb (= 0.1.1) - listen (~> 3.0.4) + listen (~> 3.0.5) mail! minitest (< 5.3.4) mocha (~> 0.14) @@ -46,18 +46,18 @@ and may also be used independently outside Rails. 1. Install Rails at the command prompt if you haven't yet: - gem install rails + $ gem install rails 2. At the command prompt, create a new Rails application: - rails new myapp + $ rails new myapp where "myapp" is the application name. 3. Change directory to `myapp` and start the web server: - cd myapp - rails server + $ cd myapp + $ rails server Run with `--help` or `-h` for options. diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index cbd7cec70f..0b12860619 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -464,30 +464,26 @@ module ActionMailer # Either a class, string or symbol can be passed in as the Observer. # If a string or symbol is passed in it will be camelized and constantized. def register_observer(observer) - delivery_observer = case observer - when String, Symbol - observer.to_s.camelize.constantize - else - observer - end - - Mail.register_observer(delivery_observer) + Mail.register_observer(observer_class_for(observer)) end # Register an Interceptor which will be called before mail is sent. # Either a class, string or symbol can be passed in as the Interceptor. # If a string or symbol is passed in it will be camelized and constantized. def register_interceptor(interceptor) - delivery_interceptor = case interceptor - when String, Symbol - interceptor.to_s.camelize.constantize - else - interceptor - end - - Mail.register_interceptor(delivery_interceptor) + Mail.register_interceptor(observer_class_for(interceptor)) end + def observer_class_for(value) # :nodoc: + case value + when String, Symbol + value.to_s.camelize.constantize + else + value + end + end + private :observer_class_for + # Returns the name of current mailer. This method is also being used as a path for a view lookup. # If this is an anonymous mailer, this method will return +anonymous+ instead. def mailer_name @@ -796,52 +792,40 @@ module ActionMailer # end # def mail(headers = {}, &block) - return @_message if @_mail_was_called && headers.blank? && !block - - m = @_message + return message if @_mail_was_called && headers.blank? && !block # At the beginning, do not consider class default for content_type content_type = headers[:content_type] - # Call all the procs (if any) - default_values = {} - self.class.default.each do |k,v| - default_values[k] = v.is_a?(Proc) ? instance_eval(&v) : v - end - - # Handle defaults - headers = headers.reverse_merge(default_values) - headers[:subject] ||= default_i18n_subject + headers = apply_defaults(headers) # Apply charset at the beginning so all fields are properly quoted - m.charset = charset = headers[:charset] + message.charset = charset = headers[:charset] # Set configure delivery behavior - wrap_delivery_behavior!(headers.delete(:delivery_method), headers.delete(:delivery_method_options)) + wrap_delivery_behavior!(headers[:delivery_method], headers[:delivery_method_options]) - # Assign all headers except parts_order, content_type, body, template_name, and template_path - assignable = headers.except(:parts_order, :content_type, :body, :template_name, :template_path) - assignable.each { |k, v| m[k] = v } + assign_headers_to_message(message, headers) # Render the templates and blocks responses = collect_responses(headers, &block) @_mail_was_called = true - create_parts_from_responses(m, responses) + create_parts_from_responses(message, responses) # Setup content type, reapply charset and handle parts order - m.content_type = set_content_type(m, content_type, headers[:content_type]) - m.charset = charset + message.content_type = set_content_type(message, content_type, headers[:content_type]) + message.charset = charset - if m.multipart? - m.body.set_sort_order(headers[:parts_order]) - m.body.sort_parts! + if message.multipart? + message.body.set_sort_order(headers[:parts_order]) + message.body.sort_parts! end - m + message end - protected + protected # Used by #mail to set the content type of the message. # @@ -879,36 +863,61 @@ module ActionMailer I18n.t(:subject, interpolations.merge(scope: [mailer_scope, action_name], default: action_name.humanize)) end - def collect_responses(headers) #:nodoc: - responses = [] + # Emails do not support relative path links. + def self.supports_path? + false + end + private + + def apply_defaults(headers) + default_values = self.class.default.map do |key, value| + [ + key, + value.is_a?(Proc) ? instance_eval(&value) : value + ] + end.to_h + + headers_with_defaults = headers.reverse_merge(default_values) + headers_with_defaults[:subject] ||= default_i18n_subject + headers_with_defaults + end + + def assign_headers_to_message(message, headers) + assignable = headers.except(:parts_order, :content_type, :body, :template_name, + :template_path, :delivery_method, :delivery_method_options) + assignable.each { |k, v| message[k] = v } + end + + def collect_responses(headers) if block_given? collector = ActionMailer::Collector.new(lookup_context) { render(action_name) } yield(collector) - responses = collector.responses + collector.responses elsif headers[:body] - responses << { + [{ body: headers.delete(:body), content_type: self.class.default[:content_type] || "text/plain" - } + }] else - templates_path = headers.delete(:template_path) || self.class.mailer_name - templates_name = headers.delete(:template_name) || action_name + collect_responses_from_templates(headers) + end + end - each_template(Array(templates_path), templates_name) do |template| - self.formats = template.formats + def collect_responses_from_templates(headers) + templates_path = headers[:template_path] || self.class.mailer_name + templates_name = headers[:template_name] || action_name - responses << { - body: render(template: template), - content_type: template.type.to_s - } - end + each_template(Array(templates_path), templates_name).map do |template| + self.formats = template.formats + { + body: render(template: template), + content_type: template.type.to_s + } end - - responses end - def each_template(paths, name, &block) #:nodoc: + def each_template(paths, name, &block) templates = lookup_context.find_all(name, paths) if templates.empty? raise ActionView::MissingTemplate.new(paths, name, paths, false, 'mailer') @@ -917,7 +926,7 @@ module ActionMailer end end - def create_parts_from_responses(m, responses) #:nodoc: + def create_parts_from_responses(m, responses) if responses.size == 1 && !m.has_attachments? responses[0].each { |k,v| m[k] = v } elsif responses.size > 1 && m.has_attachments? @@ -930,17 +939,12 @@ module ActionMailer end end - def insert_part(container, response, charset) #:nodoc: + def insert_part(container, response, charset) response[:charset] ||= charset part = Mail::Part.new(response) container.add_part(part) end - # Emails do not support relative path links. - def self.supports_path? - false - end - ActiveSupport.run_load_hooks(:action_mailer, self) end end diff --git a/actionmailer/lib/action_mailer/log_subscriber.rb b/actionmailer/lib/action_mailer/log_subscriber.rb index 7e9d916b66..2867bf90fb 100644 --- a/actionmailer/lib/action_mailer/log_subscriber.rb +++ b/actionmailer/lib/action_mailer/log_subscriber.rb @@ -8,7 +8,7 @@ module ActionMailer def deliver(event) info do recipients = Array(event.payload[:to]).join(', ') - "\nSent mail to #{recipients} (#{event.duration.round(1)}ms)" + "Sent mail to #{recipients} (#{event.duration.round(1)}ms)" end debug { event.payload[:mail] } @@ -16,7 +16,7 @@ module ActionMailer # An email was received. def receive(event) - info { "\nReceived mail (#{event.duration.round(1)}ms)" } + info { "Received mail (#{event.duration.round(1)}ms)" } debug { event.payload[:mail] } end @@ -25,7 +25,7 @@ module ActionMailer debug do mailer = event.payload[:mailer] action = event.payload[:action] - "\n#{mailer}##{action}: processed outbound mail in #{event.duration.round(1)}ms" + "#{mailer}##{action}: processed outbound mail in #{event.duration.round(1)}ms" end end diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 40f447ada7..cab7d85ee7 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,9 @@ +* In url_for, never append a question mark to the URL when the query string + is empty anyway. (It used to do that when called like `url_for(controller: + 'x', action: 'y', q: {})`.) + + *Paul Grayson* + * Catch invalid UTF-8 querystring values and respond with BadRequest Check querystring params for invalid UTF-8 characters, and raise an diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 64f6f7cf51..836ed892dc 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -77,6 +77,10 @@ module ActionController #:nodoc: config_accessor :log_warning_on_csrf_failure self.log_warning_on_csrf_failure = true + # Controls whether the Origin header is checked in addition to the CSRF token. + config_accessor :forgery_protection_origin_check + self.forgery_protection_origin_check = false + helper_method :form_authenticity_token helper_method :protect_against_forgery? end @@ -257,8 +261,19 @@ module ActionController #:nodoc: # * Does the X-CSRF-Token header match the form_authenticity_token def verified_request? !protect_against_forgery? || request.get? || request.head? || - valid_authenticity_token?(session, form_authenticity_param) || - valid_authenticity_token?(session, request.headers['X-CSRF-Token']) + (valid_request_origin? && any_authenticity_token_valid?) + end + + # Checks if any of the authenticity tokens from the request are valid. + def any_authenticity_token_valid? + request_authenticity_tokens.any? do |token| + valid_authenticity_token?(session, token) + end + end + + # Possible authenticity tokens sent in the request. + def request_authenticity_tokens + [form_authenticity_param, request.x_csrf_token] end # Sets the token value for the current session. @@ -336,5 +351,16 @@ module ActionController #:nodoc: def protect_against_forgery? allow_forgery_protection end + + # Checks if the request originated from the same origin by looking at the + # Origin header. + def valid_request_origin? + if forgery_protection_origin_check + # We accept blank origin headers because some user agents don't send it. + request.origin.nil? || request.origin == request.base_url + else + true + end + end end end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index ea61ad0c02..3280799647 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -36,8 +36,8 @@ module ActionDispatch HTTP_ACCEPT HTTP_ACCEPT_CHARSET HTTP_ACCEPT_ENCODING HTTP_ACCEPT_LANGUAGE HTTP_CACHE_CONTROL HTTP_FROM HTTP_NEGOTIATE HTTP_PRAGMA HTTP_CLIENT_IP - HTTP_X_FORWARDED_FOR HTTP_VERSION - HTTP_X_REQUEST_ID HTTP_X_FORWARDED_HOST + HTTP_X_FORWARDED_FOR HTTP_ORIGIN HTTP_VERSION + HTTP_X_CSRF_TOKEN HTTP_X_REQUEST_ID HTTP_X_FORWARDED_HOST SERVER_ADDR ].freeze @@ -306,10 +306,16 @@ module ActionDispatch end end - # Returns true if the request's content MIME type is - # +application/x-www-form-urlencoded+ or +multipart/form-data+. + # Determine whether the request body contains form-data by checking + # the request Content-Type for one of the media-types: + # "application/x-www-form-urlencoded" or "multipart/form-data". The + # list of form-data media types can be modified through the + # +FORM_DATA_MEDIA_TYPES+ array. + # + # A request body is not assumed to contain form-data when no + # Content-Type header is provided and the request_method is POST. def form_data? - FORM_DATA_MEDIA_TYPES.include?(content_mime_type.to_s) + FORM_DATA_MEDIA_TYPES.include?(media_type) end def body_stream #:nodoc: diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 92b10b6d3b..37f41ae988 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -81,7 +81,8 @@ module ActionDispatch def add_params(path, params) params = { params: params } unless params.is_a?(Hash) params.reject! { |_,v| v.to_param.nil? } - path << "?#{params.to_query}" unless params.empty? + query = params.to_query + path << "?#{query}" unless query.empty? end def add_anchor(path, anchor) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 2889acaeb8..65baf117ba 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -77,6 +77,12 @@ module ActionDispatch # # It can be read using the signed method `cookies.signed[:name]` # cookies.signed[:user_id] = current_user.id # + # # Sets an encrypted cookie value before sending it to the client which + # # prevent users from reading and tampering with its value. + # # The cookie is signed by your app's `secrets.secret_key_base` value. + # # It can be read using the encrypted method `cookies.encrypted[:name]` + # cookies.encrypted[:discount] = 45 + # # # Sets a "permanent" cookie (which expires in 20 years from now). # cookies.permanent[:login] = "XJ-122" # @@ -89,6 +95,7 @@ module ActionDispatch # cookies.size # => 2 # JSON.parse(cookies[:lat_lon]) # => [47.68, -122.37] # cookies.signed[:login] # => "XJ-122" + # cookies.encrypted[:discount] # => 45 # # Example for deleting: # diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index aee2334da9..31b75498b6 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -43,7 +43,7 @@ module ActionDispatch # Create a new +RemoteIp+ middleware instance. # - # The +check_ip_spoofing+ option is on by default. When on, an exception + # The +ip_spoofing_check+ option is on by default. When on, an exception # is raised if it looks like the client is trying to lie about its own IP # address. It makes sense to turn off this check on sites aimed at non-IP # clients (like WAP devices), or behind proxies that set headers in an @@ -57,9 +57,9 @@ module ActionDispatch # with your proxy servers after it. If your proxies aren't removed, pass # them in via the +custom_proxies+ parameter. That way, the middleware will # ignore those IP addresses, and return the one that you want. - def initialize(app, check_ip_spoofing = true, custom_proxies = nil) + def initialize(app, ip_spoofing_check = true, custom_proxies = nil) @app = app - @check_ip = check_ip_spoofing + @check_ip = ip_spoofing_check @proxies = if custom_proxies.blank? TRUSTED_PROXIES elsif custom_proxies.respond_to?(:any?) @@ -116,10 +116,18 @@ module ActionDispatch forwarded_ips = ips_from(@req.x_forwarded_for).reverse # +Client-Ip+ and +X-Forwarded-For+ should not, generally, both be set. - # If they are both set, it means that this request passed through two - # proxies with incompatible IP header conventions, and there is no way - # for us to determine which header is the right one after the fact. - # Since we have no idea, we give up and explode. + # If they are both set, it means that either: + # + # 1) This request passed through two proxies with incompatible IP header + # conventions. + # 2) The client passed one of +Client-Ip+ or +X-Forwarded-For+ + # (whichever the proxy servers weren't using) themselves. + # + # Either way, there is no way for us to determine which header is the + # right one after the fact. Since we have no idea, if we are concerned + # about IP spoofing we need to give up and explode. (If you're not + # concerned about IP spoofing you can turn the +ip_spoofing_check+ + # option off.) should_check_ip = @check_ip && client_ips.last && forwarded_ips.last if should_check_ip && !forwarded_ips.include?(client_ips.last) # We don't know which came from the proxy, and which from the user diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb index e7b913bbe4..e7b913bbe4 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.text.erb new file mode 100644 index 0000000000..23a9c7ba3f --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.text.erb @@ -0,0 +1,8 @@ +<% @source_extracts.first(3).each do |source_extract| %> +<% if source_extract[:code] %> +Extracted source (around line #<%= source_extract[:line_number] %>): + +<% source_extract[:code].each do |line, source| -%> +<%= line == source_extract[:line_number] ? "*#{line}" : "##{line}" -%> <%= source -%><% end -%> +<% end %> +<% end %> diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 7c0404ca62..18cd205bad 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -11,7 +11,7 @@ module ActionDispatch class Mapper URL_OPTIONS = [:protocol, :subdomain, :domain, :host, :port] - class Constraints < Endpoint #:nodoc: + class Constraints < Routing::Endpoint #:nodoc: attr_reader :app, :constraints SERVE = ->(app, req) { app.serve req } @@ -600,17 +600,20 @@ module ActionDispatch def mount(app, options = nil) if options path = options.delete(:at) - else - unless Hash === app - raise ArgumentError, "must be called with mount point" - end - + elsif Hash === app options = app app, path = options.find { |k, _| k.respond_to?(:call) } options.delete(app) if app end - raise "A rack application must be specified" unless path + raise ArgumentError, "A rack application must be specified" unless app.respond_to?(:call) + raise ArgumentError, <<-MSG.strip_heredoc unless path + Must be called with mount point + + mount SomeRackApp, at: "some_route" + or + mount(SomeRackApp => "some_route") + MSG rails_app = rails_app? app options[:as] ||= app_name(app, rails_app) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 5f54ea130b..c4228df925 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -371,10 +371,6 @@ module ActionDispatch end def eval_block(block) - if block.arity == 1 - raise "You are using the old router DSL which has been removed in Rails 3.1. " << - "Please check how to update your routes file at: http://www.engineyard.com/blog/2010/the-lowdown-on-routes-in-rails-3/" - end mapper = Mapper.new(self) if default_scope mapper.with_default_scope(default_scope, &block) diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 94ffbe3cd0..2a3704c300 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -304,6 +304,41 @@ module RequestForgeryProtectionTests assert_not_blocked { put :index } end + def test_should_allow_post_with_origin_checking_and_correct_origin + forgery_protection_origin_check do + session[:_csrf_token] = @token + @controller.stub :form_authenticity_token, @token do + assert_not_blocked do + @request.set_header 'HTTP_ORIGIN', 'http://test.host' + post :index, params: { custom_authenticity_token: @token } + end + end + end + end + + def test_should_allow_post_with_origin_checking_and_no_origin + forgery_protection_origin_check do + session[:_csrf_token] = @token + @controller.stub :form_authenticity_token, @token do + assert_not_blocked do + post :index, params: { custom_authenticity_token: @token } + end + end + end + end + + def test_should_block_post_with_origin_checking_and_wrong_origin + forgery_protection_origin_check do + session[:_csrf_token] = @token + @controller.stub :form_authenticity_token, @token do + assert_blocked do + @request.set_header 'HTTP_ORIGIN', 'http://bad.host' + post :index, params: { custom_authenticity_token: @token } + end + end + end + end + def test_should_warn_on_missing_csrf_token old_logger = ActionController::Base.logger logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new @@ -405,6 +440,16 @@ module RequestForgeryProtectionTests def assert_cross_origin_not_blocked assert_not_blocked { yield } end + + def forgery_protection_origin_check + old_setting = ActionController::Base.forgery_protection_origin_check + ActionController::Base.forgery_protection_origin_check = true + begin + yield + ensure + ActionController::Base.forgery_protection_origin_check = old_setting + end + end end # OK let's get our test on diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 4a2b02a003..a39fede5b9 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -289,12 +289,6 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal({:id=>"1", :filters=>"foo", :format=>"js"}, params) end - def test_draw_with_block_arity_one_raises - assert_raise(RuntimeError) do - rs.draw { |map| map.match '/:controller(/:action(/:id))' } - end - end - def test_specific_controller_action_failure rs.draw do mount lambda {} => "/foo" diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 8960156d00..30772bd9ed 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -166,6 +166,14 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_equal "text/plain", response.content_type assert_match(/RuntimeError\npuke/, body) + Rails.stub :root, Pathname.new('.') do + get "/", headers: xhr_request_env + + assert_response 500 + assert_match 'Extracted source (around line #', body + assert_select 'pre', { count: 0 }, body + end + get "/not_found", headers: xhr_request_env assert_response 404 assert_no_match(/<body>/, body) diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index e783df855e..df27e41997 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -158,7 +158,7 @@ module ActionDispatch assert_equal '/*path.:format', fakeset.asts.first.to_s end - def test_raising_helpful_error_on_invalid_arguments + def test_raising_error_when_path_is_not_passed fakeset = FakeSet.new mapper = Mapper.new fakeset app = lambda { |env| [200, {}, [""]] } @@ -166,6 +166,18 @@ module ActionDispatch mapper.mount app end end + + def test_raising_error_when_rack_app_is_not_passed + fakeset = FakeSet.new + mapper = Mapper.new fakeset + assert_raises ArgumentError do + mapper.mount 10, as: "exciting" + end + + assert_raises ArgumentError do + mapper.mount as: "exciting" + end + end end end end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 22240699d9..08c4554721 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -1212,3 +1212,23 @@ class RequestVariant < BaseRequestTest end end end + +class RequestFormData < BaseRequestTest + test 'media_type is from the FORM_DATA_MEDIA_TYPES array' do + assert stub_request('CONTENT_TYPE' => 'application/x-www-form-urlencoded').form_data? + assert stub_request('CONTENT_TYPE' => 'multipart/form-data').form_data? + end + + test 'media_type is not from the FORM_DATA_MEDIA_TYPES array' do + assert !stub_request('CONTENT_TYPE' => 'application/xml').form_data? + assert !stub_request('CONTENT_TYPE' => 'multipart/related').form_data? + end + + test 'no Content-Type header is provided and the request_method is POST' do + request = stub_request('REQUEST_METHOD' => 'POST') + + assert_equal '', request.media_type + assert_equal 'POST', request.request_method + assert !request.form_data? + end +end diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index fd4ede4d1b..8c9782bb90 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -129,6 +129,13 @@ module TestUrlGeneration ) end + test "generating URLS with empty querystring" do + assert_equal "/bars.json", bars_path( + a: {}, + format: 'json' + ) + end + end end diff --git a/actionview/lib/action_view/helpers/asset_tag_helper.rb b/actionview/lib/action_view/helpers/asset_tag_helper.rb index 2ffed6b395..91e934cd64 100644 --- a/actionview/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionview/lib/action_view/helpers/asset_tag_helper.rb @@ -205,6 +205,8 @@ module ActionView # # => <img alt="Icon" height="32" src="/icons/icon.gif" width="32" /> # image_tag("/icons/icon.gif", class: "menu_icon") # # => <img alt="Icon" class="menu_icon" src="/icons/icon.gif" /> + # image_tag("/icons/icon.gif", data: { title: 'Rails Application' }) + # # => <img data-title="Rails Application" src="/icons/icon.gif" /> def image_tag(source, options={}) options = options.symbolize_keys check_for_image_tag_errors(options) diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index dfb7d463b4..63a60542d4 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -46,9 +46,8 @@ module ActionView #:nodoc: class NullResolver < PathResolver def query(path, exts, formats) handler, format, variant = extract_handler_and_format_and_variant(path, formats) - [ActionView::Template.new("Template generated by Null Resolver", path, handler, :virtual_path => path, :format => format, :variant => variant)] + [ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, :virtual_path => path.virtual, :format => format, :variant => variant)] end end - end diff --git a/actionview/test/abstract_unit.rb b/actionview/test/abstract_unit.rb index 2354e91822..79173f730f 100644 --- a/actionview/test/abstract_unit.rb +++ b/actionview/test/abstract_unit.rb @@ -280,7 +280,6 @@ def jruby_skip(message = '') skip message if defined?(JRUBY_VERSION) end -require 'mocha/setup' # FIXME: stop using mocha class ActiveSupport::TestCase include ActiveSupport::Testing::MethodCallAssertions end diff --git a/actionview/test/template/asset_tag_helper_test.rb b/actionview/test/template/asset_tag_helper_test.rb index a63ac442cd..fe40010528 100644 --- a/actionview/test/template/asset_tag_helper_test.rb +++ b/actionview/test/template/asset_tag_helper_test.rb @@ -193,7 +193,8 @@ class AssetTagHelperTest < ActionView::TestCase %(image_tag("//www.rubyonrails.com/images/rails.png")) => %(<img alt="Rails" src="//www.rubyonrails.com/images/rails.png" />), %(image_tag("mouse.png", :alt => nil)) => %(<img src="/images/mouse.png" />), %(image_tag("data:image/gif;base64,R0lGODlhAQABAID/AMDAwAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==", :alt => nil)) => %(<img src="data:image/gif;base64,R0lGODlhAQABAID/AMDAwAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==" />), - %(image_tag("")) => %(<img src="" />) + %(image_tag("")) => %(<img src="" />), + %(image_tag("gold.png", data: { title: 'Rails Application' })) => %(<img data-title="Rails Application" src="/images/gold.png" alt="Gold" />) } FaviconLinkToTag = { diff --git a/actionview/test/template/date_helper_i18n_test.rb b/actionview/test/template/date_helper_i18n_test.rb index 21fca35185..52aef56a61 100644 --- a/actionview/test/template/date_helper_i18n_test.rb +++ b/actionview/test/template/date_helper_i18n_test.rb @@ -46,8 +46,9 @@ class DateHelperDistanceOfTimeInWordsI18nTests < ActiveSupport::TestCase end def test_time_ago_in_words_passes_locale - I18n.expects(:t).with(:less_than_x_minutes, :scope => :'datetime.distance_in_words', :count => 1, :locale => 'ru') - time_ago_in_words(15.seconds.ago, :locale => 'ru') + assert_called_with(I18n, :t, [:less_than_x_minutes, :scope => :'datetime.distance_in_words', :count => 1, :locale => 'ru']) do + time_ago_in_words(15.seconds.ago, :locale => 'ru') + end end def test_distance_of_time_pluralizations @@ -80,8 +81,9 @@ class DateHelperDistanceOfTimeInWordsI18nTests < ActiveSupport::TestCase options = { locale: 'en', scope: :'datetime.distance_in_words' }.merge!(expected_options) options[:count] = count if count - I18n.expects(:t).with(key, options) - distance_of_time_in_words(@from, to, passed_options.merge(locale: 'en')) + assert_called_with(I18n, :t, [key, options]) do + distance_of_time_in_words(@from, to, passed_options.merge(locale: 'en')) + end end end @@ -89,60 +91,74 @@ class DateHelperSelectTagsI18nTests < ActiveSupport::TestCase include ActionView::Helpers::DateHelper attr_reader :request - def setup - @prompt_defaults = {:year => 'Year', :month => 'Month', :day => 'Day', :hour => 'Hour', :minute => 'Minute', :second => 'Seconds'} - - I18n.stubs(:translate).with(:'date.month_names', :locale => 'en').returns Date::MONTHNAMES - end - # select_month def test_select_month_given_use_month_names_option_does_not_translate_monthnames - I18n.expects(:translate).never - select_month(8, :locale => 'en', :use_month_names => Date::MONTHNAMES) + assert_not_called(I18n, :translate) do + select_month(8, :locale => 'en', :use_month_names => Date::MONTHNAMES) + end end def test_select_month_translates_monthnames - I18n.expects(:translate).with(:'date.month_names', :locale => 'en').returns Date::MONTHNAMES - select_month(8, :locale => 'en') + assert_called_with(I18n, :translate, [:'date.month_names', :locale => 'en'], returns: Date::MONTHNAMES) do + select_month(8, :locale => 'en') + end end def test_select_month_given_use_short_month_option_translates_abbr_monthnames - I18n.expects(:translate).with(:'date.abbr_month_names', :locale => 'en').returns Date::ABBR_MONTHNAMES - select_month(8, :locale => 'en', :use_short_month => true) + assert_called_with(I18n, :translate, [:'date.abbr_month_names', :locale => 'en'], returns: Date::ABBR_MONTHNAMES) do + select_month(8, :locale => 'en', :use_short_month => true) + end end def test_date_or_time_select_translates_prompts - @prompt_defaults.each do |key, prompt| - I18n.expects(:translate).with(('datetime.prompts.' + key.to_s).to_sym, :locale => 'en').returns prompt + prompt_defaults = {:year => 'Year', :month => 'Month', :day => 'Day', :hour => 'Hour', :minute => 'Minute', :second => 'Seconds'} + defaults = {[:'date.order', :locale => 'en', :default => []] => %w(year month day)} + + prompt_defaults.each do |key, prompt| + defaults[[('datetime.prompts.' + key.to_s).to_sym, :locale => 'en']] = prompt + end + + prompts_check = -> (prompt, x) do + @prompt_called ||= 0 + + return_value = defaults[[prompt, x]] + @prompt_called += 1 if return_value.present? + + return_value end - I18n.expects(:translate).with(:'date.order', :locale => 'en', :default => []).returns %w(year month day) - datetime_select('post', 'updated_at', :locale => 'en', :include_seconds => true, :prompt => true) + I18n.stub(:translate, prompts_check) do + datetime_select('post', 'updated_at', :locale => 'en', :include_seconds => true, :prompt => true, :use_month_names => Date::MONTHNAMES) + end + assert_equal defaults.count, @prompt_called end # date_or_time_select def test_date_or_time_select_given_an_order_options_does_not_translate_order - I18n.expects(:translate).never - datetime_select('post', 'updated_at', :order => [:year, :month, :day], :locale => 'en') + assert_not_called(I18n, :translate) do + datetime_select('post', 'updated_at', :order => [:year, :month, :day], :locale => 'en', :use_month_names => Date::MONTHNAMES) + end end def test_date_or_time_select_given_no_order_options_translates_order - I18n.expects(:translate).with(:'date.order', :locale => 'en', :default => []).returns %w(year month day) - datetime_select('post', 'updated_at', :locale => 'en') + assert_called_with(I18n, :translate, [ [:'date.order', :locale => 'en', :default => []], [:"date.month_names", {:locale=>"en"}] ], returns: %w(year month day)) do + datetime_select('post', 'updated_at', :locale => 'en') + end end def test_date_or_time_select_given_invalid_order - I18n.expects(:translate).with(:'date.order', :locale => 'en', :default => []).returns %w(invalid month day) - - assert_raise StandardError do - datetime_select('post', 'updated_at', :locale => 'en') + assert_called_with(I18n, :translate, [:'date.order', :locale => 'en', :default => []], returns: %w(invalid month day)) do + assert_raise StandardError do + datetime_select('post', 'updated_at', :locale => 'en') + end end end def test_date_or_time_select_given_symbol_keys - I18n.expects(:translate).with(:'date.order', :locale => 'en', :default => []).returns [:year, :month, :day] - datetime_select('post', 'updated_at', :locale => 'en') + assert_called_with(I18n, :translate, [ [:'date.order', :locale => 'en', :default => []], [:"date.month_names", {:locale=>"en"}] ], returns: [:year, :month, :day]) do + datetime_select('post', 'updated_at', :locale => 'en') + end end end diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index d7daba8bf3..6b97cec34c 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -17,13 +17,37 @@ class FormOptionsHelperTest < ActionView::TestCase Album = Struct.new('Album', :id, :title, :genre) end - def setup - @fake_timezones = %w(A B C D E).map do |id| - tz = stub(:name => id, :to_s => id) - ActiveSupport::TimeZone.stubs(:[]).with(id).returns(tz) - tz + module FakeZones + FakeZone = Struct.new(:name) do + def to_s; name; end end - ActiveSupport::TimeZone.stubs(:all).returns(@fake_timezones) + + module ClassMethods + def [](id); fake_zones ? fake_zones[id] : super; end + def all; fake_zones ? fake_zones.values : super; end + def dummy; :test; end + end + + def self.prepended(base) + class << base + mattr_accessor(:fake_zones) + prepend ClassMethods + end + end + end + + ActiveSupport::TimeZone.prepend FakeZones + + setup do + ActiveSupport::TimeZone.fake_zones = %w(A B C D E).map do |id| + [ id, FakeZones::FakeZone.new(id) ] + end.to_h + + @fake_timezones = ActiveSupport::TimeZone.all + end + + teardown do + ActiveSupport::TimeZone.fake_zones = nil end def test_collection_options @@ -1163,8 +1187,8 @@ class FormOptionsHelperTest < ActionView::TestCase def test_time_zone_select_with_priority_zones_as_regexp @firm = Firm.new("D") - @fake_timezones.each_with_index do |tz, i| - tz.stubs(:=~).returns(i.zero? || i == 3) + @fake_timezones.each do |tz| + def tz.=~(re); %(A D).include?(name) end end html = time_zone_select("firm", "time_zone", /A|D/) @@ -1179,15 +1203,16 @@ class FormOptionsHelperTest < ActionView::TestCase html end - def test_time_zone_select_with_priority_zones_as_regexp_using_grep_finds_no_zones + def test_time_zone_select_with_priority_zones_is_not_implemented_with_grep @firm = Firm.new("D") - priority_zones = /A|D/ + # `time_zone_select` can't be written with `grep` because Active Support + # time zones don't support implicit string coercion with `to_str`. @fake_timezones.each do |tz| - priority_zones.stubs(:===).with(tz).raises(Exception) + def tz.===(zone); raise Exception; end end - html = time_zone_select("firm", "time_zone", priority_zones) + html = time_zone_select("firm", "time_zone", /A|D/) assert_dom_equal "<select id=\"firm_time_zone\" name=\"firm[time_zone]\">" + "<option value=\"\" disabled=\"disabled\">-------------</option>\n" + "<option value=\"A\">A</option>\n" + diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 7f4c84929f..4776c18b0b 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -12,13 +12,18 @@ class AVLogSubscriberTest < ActiveSupport::TestCase lookup_context = ActionView::LookupContext.new(view_paths, {}, ["test"]) renderer = ActionView::Renderer.new(lookup_context) @view = ActionView::Base.new(renderer, {}) - Rails.stubs(:root).returns(File.expand_path(FIXTURE_LOAD_PATH)) ActionView::LogSubscriber.attach_to :action_view + unless Rails.respond_to?(:root) + @defined_root = true + def Rails.root; :defined_root; end # Minitest `stub` expects the method to be defined. + end end def teardown super ActiveSupport::LogSubscriber.log_subscribers.clear + # We need to undef `root`, RenderTestCases don't want this to be defined + Rails.instance_eval { undef :root } if @defined_root end def set_logger(logger) @@ -26,66 +31,82 @@ class AVLogSubscriberTest < ActiveSupport::TestCase end def test_render_file_template - @view.render(:file => "test/hello_world") - wait + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + @view.render(:file => "test/hello_world") + wait - assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered test\/hello_world\.erb/, @logger.logged(:info).last) + assert_equal 1, @logger.logged(:info).size + assert_match(/Rendered test\/hello_world\.erb/, @logger.logged(:info).last) + end end def test_render_text_template - @view.render(:text => "TEXT") - wait + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + @view.render(:text => "TEXT") + wait - assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered text template/, @logger.logged(:info).last) + assert_equal 1, @logger.logged(:info).size + assert_match(/Rendered text template/, @logger.logged(:info).last) + end end def test_render_inline_template - @view.render(:inline => "<%= 'TEXT' %>") - wait + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + @view.render(:inline => "<%= 'TEXT' %>") + wait - assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered inline template/, @logger.logged(:info).last) + assert_equal 1, @logger.logged(:info).size + assert_match(/Rendered inline template/, @logger.logged(:info).last) + end end def test_render_partial_template - @view.render(:partial => "test/customer") - wait + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + @view.render(:partial => "test/customer") + wait - assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered test\/_customer.erb/, @logger.logged(:info).last) + assert_equal 1, @logger.logged(:info).size + assert_match(/Rendered test\/_customer.erb/, @logger.logged(:info).last) + end end def test_render_partial_with_implicit_path - @view.render(Customer.new("david"), :greeting => "hi") - wait + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + @view.render(Customer.new("david"), :greeting => "hi") + wait - assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered customers\/_customer\.html\.erb/, @logger.logged(:info).last) + assert_equal 1, @logger.logged(:info).size + assert_match(/Rendered customers\/_customer\.html\.erb/, @logger.logged(:info).last) + end end def test_render_collection_template - @view.render(:partial => "test/customer", :collection => [ Customer.new("david"), Customer.new("mary") ]) - wait + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + @view.render(:partial => "test/customer", :collection => [ Customer.new("david"), Customer.new("mary") ]) + wait - assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered test\/_customer.erb/, @logger.logged(:info).last) + assert_equal 1, @logger.logged(:info).size + assert_match(/Rendered test\/_customer.erb/, @logger.logged(:info).last) + end end def test_render_collection_with_implicit_path - @view.render([ Customer.new("david"), Customer.new("mary") ], :greeting => "hi") - wait + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + @view.render([ Customer.new("david"), Customer.new("mary") ], :greeting => "hi") + wait - assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered customers\/_customer\.html\.erb/, @logger.logged(:info).last) + assert_equal 1, @logger.logged(:info).size + assert_match(/Rendered customers\/_customer\.html\.erb/, @logger.logged(:info).last) + end end def test_render_collection_template_without_path - @view.render([ GoodCustomer.new("david"), Customer.new("mary") ], :greeting => "hi") - wait + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + @view.render([ GoodCustomer.new("david"), Customer.new("mary") ], :greeting => "hi") + wait - assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered collection/, @logger.logged(:info).last) + assert_equal 1, @logger.logged(:info).size + assert_match(/Rendered collection/, @logger.logged(:info).last) + end end end diff --git a/actionview/test/template/test_test.rb b/actionview/test/template/test_test.rb index 88bac85039..e1ff639979 100644 --- a/actionview/test/template/test_test.rb +++ b/actionview/test/template/test_test.rb @@ -41,12 +41,12 @@ class PeopleHelperTest < ActionView::TestCase extend ActiveModel::Naming def to_model; self; end def persisted?; true; end - def self.name; 'Mocha::Mock'; end + def self.name; 'Minitest::Mock'; end }.new "David" the_model = nil extend Module.new { - define_method(:mocha_mock_path) { |model, *args| + define_method(:minitest_mock_path) { |model, *args| the_model = model "/people/1" } diff --git a/actionview/test/template/url_helper_test.rb b/actionview/test/template/url_helper_test.rb index 48d0a9a47a..62fa75bc63 100644 --- a/actionview/test/template/url_helper_test.rb +++ b/actionview/test/template/url_helper_test.rb @@ -38,6 +38,10 @@ class UrlHelperTest < ActiveSupport::TestCase assert_equal "/?a=b&c=d", url_for(hash_for(a: :b, c: :d)) end + def test_url_for_does_not_include_empty_hashes + assert_equal "/", url_for(hash_for(a: {})) + end + def test_url_for_with_back referer = 'http://www.example.com/referer' @controller = Struct.new(:request).new(Struct.new(:env).new("HTTP_REFERER" => referer)) diff --git a/activejob/test/integration/queuing_test.rb b/activejob/test/integration/queuing_test.rb index e435ed4aa6..d8425c9706 100644 --- a/activejob/test/integration/queuing_test.rb +++ b/activejob/test/integration/queuing_test.rb @@ -78,7 +78,7 @@ class QueuingTest < ActiveSupport::TestCase TestJob.perform_later @id wait_for_jobs_to_finish_for(5.seconds) assert job_executed - assert_equal 'de', job_output + assert_equal 'de', job_executed_in_locale ensure I18n.available_locales = [:en] I18n.locale = :en diff --git a/activejob/test/support/integration/dummy_app_template.rb b/activejob/test/support/integration/dummy_app_template.rb index 0c062a025e..262ca72327 100644 --- a/activejob/test/support/integration/dummy_app_template.rb +++ b/activejob/test/support/integration/dummy_app_template.rb @@ -18,8 +18,11 @@ class TestJob < ActiveJob::Base queue_as :integration_tests def perform(x) - File.open(Rails.root.join("tmp/\#{x}"), "w+") do |f| - f.write I18n.locale + File.open(Rails.root.join("tmp/\#{x}"), "wb+") do |f| + f.write Marshal.dump({ + "locale" => I18n.locale.to_s || "en", + "executed_at" => Time.now.to_r + }) end end end diff --git a/activejob/test/support/integration/test_case_helpers.rb b/activejob/test/support/integration/test_case_helpers.rb index 8319d09520..9897f76fd0 100644 --- a/activejob/test/support/integration/test_case_helpers.rb +++ b/activejob/test/support/integration/test_case_helpers.rb @@ -42,15 +42,23 @@ module TestCaseHelpers end end + def job_file(id) + Dummy::Application.root.join("tmp/#{id}") + end + def job_executed(id=@id) - Dummy::Application.root.join("tmp/#{id}").exist? + job_file(id).exist? + end + + def job_data(id) + Marshal.load(File.binread(job_file(id))) end def job_executed_at(id=@id) - File.new(Dummy::Application.root.join("tmp/#{id}")).ctime + job_data(id)["executed_at"] end - def job_output - File.read Dummy::Application.root.join("tmp/#{@id}") + def job_executed_in_locale(id=@id) + job_data(id)["locale"] end end diff --git a/activemodel/lib/active_model/attribute_assignment.rb b/activemodel/lib/active_model/attribute_assignment.rb index 087d11f708..62014cd1cd 100644 --- a/activemodel/lib/active_model/attribute_assignment.rb +++ b/activemodel/lib/active_model/attribute_assignment.rb @@ -27,7 +27,7 @@ module ActiveModel if !new_attributes.respond_to?(:stringify_keys) raise ArgumentError, "When assigning attributes, you must pass a hash as an argument." end - return if new_attributes.blank? + return if new_attributes.nil? || new_attributes.empty? attributes = new_attributes.stringify_keys _assign_attributes(sanitize_for_mass_assignment(attributes)) diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index 0ab8df42f5..6e2e5afd1b 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -26,6 +26,10 @@ module ActiveModel # # define_attribute_methods :name # + # def initialize(name) + # @name = name + # end + # # def name # @name # end @@ -54,7 +58,7 @@ module ActiveModel # # A newly instantiated +Person+ object is unchanged: # - # person = Person.new + # person = Person.new("Uncle Bob") # person.changed? # => false # # Change the name: diff --git a/activemodel/test/cases/attribute_assignment_test.rb b/activemodel/test/cases/attribute_assignment_test.rb index 3336691841..287bea719c 100644 --- a/activemodel/test/cases/attribute_assignment_test.rb +++ b/activemodel/test/cases/attribute_assignment_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require "active_support/core_ext/hash/indifferent_access" require "active_support/hash_with_indifferent_access" class AttributeAssignmentTest < ActiveModel::TestCase @@ -23,13 +24,32 @@ class AttributeAssignmentTest < ActiveModel::TestCase class ErrorFromAttributeWriter < StandardError end - class ProtectedParams < ActiveSupport::HashWithIndifferentAccess + class ProtectedParams + attr_accessor :permitted + alias :permitted? :permitted + + delegate :keys, :key?, :has_key?, :empty?, to: :@parameters + + def initialize(attributes) + @parameters = attributes.with_indifferent_access + @permitted = false + end + def permit! @permitted = true + self + end + + def [](key) + @parameters[key] + end + + def to_h + @parameters end - def permitted? - @permitted ||= false + def stringify_keys + dup end def dup diff --git a/activemodel/test/cases/forbidden_attributes_protection_test.rb b/activemodel/test/cases/forbidden_attributes_protection_test.rb index 3cb204a2c5..d8d757f52a 100644 --- a/activemodel/test/cases/forbidden_attributes_protection_test.rb +++ b/activemodel/test/cases/forbidden_attributes_protection_test.rb @@ -2,12 +2,14 @@ require 'cases/helper' require 'active_support/core_ext/hash/indifferent_access' require 'models/account' -class ProtectedParams < ActiveSupport::HashWithIndifferentAccess +class ProtectedParams attr_accessor :permitted alias :permitted? :permitted + delegate :keys, :key?, :has_key?, :empty?, to: :@parameters + def initialize(attributes) - super(attributes) + @parameters = attributes @permitted = false end @@ -15,6 +17,10 @@ class ProtectedParams < ActiveSupport::HashWithIndifferentAccess @permitted = true self end + + def to_h + @parameters + end end class ActiveModelMassUpdateProtectionTest < ActiveSupport::TestCase diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 98c6384402..79faa9326d 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,19 @@ +* Fix `rake db:structure:dump` on Postgres when multiple schemas are used. + + Fixes #22346. + + *Nick Muerdter*, *ckoenig* + +* Add schema dumping support for PostgreSQL geometric data types. + + *Ryuta Kamizono* + +* Except keys of `build_record`'s argument from `create_scope` in `initialize_attributes`. + + Fixes #21893. + + *Yuichiro Kaneko* + * Deprecate `connection.tables` on the SQLite3 and MySQL adapters. Also deprecate passing arguments to `#tables`. And deprecate `table_exists?`. @@ -201,9 +217,9 @@ Example: - config.generators do |g| - g.orm :active_record, primary_key_type: :uuid - end + config.generators do |g| + g.orm :active_record, primary_key_type: :uuid + end *Jon McCartie* @@ -279,10 +295,10 @@ To load the fixtures file `accounts.yml` as the `User` model, use: - _fixture: - model_class: User - david: - name: David + _fixture: + model_class: User + david: + name: David Fixes #9516. diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index f32dddb8f0..473b80a658 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -414,12 +414,16 @@ module ActiveRecord def replace_on_target(record, index, skip_callbacks) callback(:before_add, record) unless skip_callbacks + + was_loaded = loaded? yield(record) if block_given? - if index - @target[index] = record - else - @target << record + unless !was_loaded && loaded? + if index + @target[index] = record + else + @target << record + end end callback(:after_add, record) unless skip_callbacks diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index abf0124562..159cbcb85a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -206,14 +206,13 @@ module ActiveRecord include ColumnMethods attr_accessor :indexes - attr_reader :name, :temporary, :options, :as, :foreign_keys, :native + attr_reader :name, :temporary, :options, :as, :foreign_keys - def initialize(types, name, temporary, options, as = nil) + def initialize(name, temporary, options, as = nil) @columns_hash = {} @indexes = {} @foreign_keys = {} @primary_keys = nil - @native = types @temporary = temporary @options = options @as = as @@ -362,11 +361,8 @@ module ActiveRecord def new_column_definition(name, type, options) # :nodoc: type = aliased_types(type.to_s, type) column = create_column_definition name, type - limit = options.fetch(:limit) do - native[type][:limit] if native[type].is_a?(Hash) - end - column.limit = limit + column.limit = options[:limit] column.precision = options[:precision] column.scale = options[:scale] column.default = options[:default] @@ -627,11 +623,6 @@ module ActiveRecord def foreign_key_exists?(*args) # :nodoc: @base.foreign_key_exists?(name, *args) end - - private - def native - @base.native_database_types - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 5cacf6eddc..b50d28862c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1168,7 +1168,7 @@ module ActiveRecord private def create_table_definition(name, temporary = false, options = nil, as = nil) - TableDefinition.new native_database_types, name, temporary, options, as + TableDefinition.new(name, temporary, options, as) end def create_alter_table(name) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 55910865e5..4b6912c616 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -95,14 +95,15 @@ module ActiveRecord attr_reader :prepared_statements - def initialize(connection, logger = nil, pool = nil) #:nodoc: + def initialize(connection, logger = nil, config = {}) # :nodoc: super() @connection = connection @owner = nil @instrumenter = ActiveSupport::Notifications.instrumenter @logger = logger - @pool = pool + @config = config + @pool = nil @schema_cache = SchemaCache.new self @visitor = nil @prepared_statements = false @@ -289,14 +290,14 @@ module ActiveRecord # locks # # Return true if we got the lock, otherwise false - def get_advisory_lock(key) # :nodoc: + def get_advisory_lock(lock_id) # :nodoc: end # This is meant to be implemented by the adapters that support advisory # locks. # # Return true if we released the lock, otherwise false - def release_advisory_lock(key) # :nodoc: + def release_advisory_lock(lock_id) # :nodoc: end # A list of extensions, to be filled in by adapters that support them. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index c75c9ab3b5..25ba42e5c9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -143,8 +143,7 @@ module ActiveRecord # FIXME: Make the first parameter more similar for the two adapters def initialize(connection, logger, connection_options, config) - super(connection, logger) - @connection_options, @config = connection_options, config + super(connection, logger, config) @quoted_column_names, @quoted_table_names = {}, {} @visitor = Arel::Visitors::MySQL.new self @@ -224,12 +223,12 @@ module ActiveRecord version >= '5.0.0' end - def get_advisory_lock(key, timeout = 0) # :nodoc: - select_value("SELECT GET_LOCK('#{key}', #{timeout});").to_s == '1' + def get_advisory_lock(lock_name, timeout = 0) # :nodoc: + select_value("SELECT GET_LOCK('#{lock_name}', #{timeout});").to_s == '1' end - def release_advisory_lock(key) # :nodoc: - select_value("SELECT RELEASE_LOCK('#{key}')").to_s == '1' + def release_advisory_lock(lock_name) # :nodoc: + select_value("SELECT RELEASE_LOCK('#{lock_name}')").to_s == '1' end def native_database_types @@ -588,10 +587,8 @@ module ActiveRecord sql = "SHOW FULL FIELDS FROM #{quote_table_name(table_name)}" execute_and_free(sql, 'SCHEMA') do |result| each_hash(result).map do |field| - field_name = set_field_encoding(field[:Field]) - sql_type = field[:Type] - type_metadata = fetch_type_metadata(sql_type, field[:Extra]) - new_column(field_name, field[:Default], type_metadata, field[:Null] == "YES", nil, field[:Collation]) + type_metadata = fetch_type_metadata(field[:Type], field[:Extra]) + new_column(field[:Field], field[:Default], type_metadata, field[:Null] == "YES", nil, field[:Collation]) end end end @@ -1041,7 +1038,7 @@ module ActiveRecord end def create_table_definition(name, temporary = false, options = nil, as = nil) # :nodoc: - MySQL::TableDefinition.new(native_database_types, name, temporary, options, as) + MySQL::TableDefinition.new(name, temporary, options, as) end def integer_to_sql(limit) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 773ecbe126..7ca597859d 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -16,8 +16,7 @@ module ActiveRecord end client = Mysql2::Client.new(config) - options = [config[:host], config[:username], config[:password], config[:database], config[:port], config[:socket], 0] - ConnectionAdapters::Mysql2Adapter.new(client, logger, options, config) + ConnectionAdapters::Mysql2Adapter.new(client, logger, nil, config) rescue Mysql2::Error => error if error.message.include?("Unknown database") raise ActiveRecord::NoDatabaseError @@ -185,10 +184,6 @@ module ActiveRecord def full_version @full_version ||= @connection.server_info[:version] end - - def set_field_encoding field_name - field_name - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 89d18ee14e..76f1b91e6b 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -82,6 +82,7 @@ module ActiveRecord super @statements = StatementPool.new(self.class.type_cast_config_to_integer(config.fetch(:statement_limit) { 1000 })) @client_encoding = nil + @connection_options = connection_options connect end @@ -104,6 +105,11 @@ module ActiveRecord end end + def new_column(field, default, sql_type_metadata = nil, null = true, default_function = nil, collation = nil) # :nodoc: + field = set_field_encoding(field) + super + end + def error_number(exception) # :nodoc: exception.errno if exception.respond_to?(:errno) end @@ -463,7 +469,7 @@ module ActiveRecord @full_version ||= @connection.server_info end - def set_field_encoding field_name + def set_field_encoding(field_name) field_name.force_encoding(client_encoding) if internal_enc = Encoding.default_internal field_name = field_name.encode!(internal_enc) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 3eefab8f44..aa43854d01 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -100,6 +100,12 @@ module ActiveRecord ltree: { name: "ltree" }, citext: { name: "citext" }, point: { name: "point" }, + line: { name: "line" }, + lseg: { name: "lseg" }, + box: { name: "box" }, + path: { name: "path" }, + polygon: { name: "polygon" }, + circle: { name: "circle" }, bit: { name: "bit" }, bit_varying: { name: "bit varying" }, money: { name: "money" }, @@ -186,7 +192,7 @@ module ActiveRecord # Initializes and connects a PostgreSQL adapter. def initialize(connection, logger, connection_parameters, config) - super(connection, logger) + super(connection, logger, config) @visitor = Arel::Visitors::PostgreSQL.new self if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @@ -196,7 +202,7 @@ module ActiveRecord @prepared_statements = false end - @connection_parameters, @config = connection_parameters, config + @connection_parameters = connection_parameters # @local_tz is initialized as nil to avoid warnings when connect tries to use it @local_tz = nil @@ -304,18 +310,18 @@ module ActiveRecord postgresql_version >= 90300 end - def get_advisory_lock(key) # :nodoc: - unless key.is_a?(Integer) && key.bit_length <= 63 - raise(ArgumentError, "Postgres requires advisory lock keys to be a signed 64 bit integer") + def get_advisory_lock(lock_id) # :nodoc: + unless lock_id.is_a?(Integer) && lock_id.bit_length <= 63 + raise(ArgumentError, "Postgres requires advisory lock ids to be a signed 64 bit integer") end - select_value("SELECT pg_try_advisory_lock(#{key});") + select_value("SELECT pg_try_advisory_lock(#{lock_id});") end - def release_advisory_lock(key) # :nodoc: - unless key.is_a?(Integer) && key.bit_length <= 63 - raise(ArgumentError, "Postgres requires advisory lock keys to be a signed 64 bit integer") + def release_advisory_lock(lock_id) # :nodoc: + unless lock_id.is_a?(Integer) && lock_id.bit_length <= 63 + raise(ArgumentError, "Postgres requires advisory lock ids to be a signed 64 bit integer") end - select_value("SELECT pg_advisory_unlock(#{key})") + select_value("SELECT pg_advisory_unlock(#{lock_id})") end def enable_extension(name) @@ -455,15 +461,15 @@ module ActiveRecord m.register_type 'macaddr', OID::SpecializedString.new(:macaddr) m.register_type 'citext', OID::SpecializedString.new(:citext) m.register_type 'ltree', OID::SpecializedString.new(:ltree) + m.register_type 'line', OID::SpecializedString.new(:line) + m.register_type 'lseg', OID::SpecializedString.new(:lseg) + m.register_type 'box', OID::SpecializedString.new(:box) + m.register_type 'path', OID::SpecializedString.new(:path) + m.register_type 'polygon', OID::SpecializedString.new(:polygon) + m.register_type 'circle', OID::SpecializedString.new(:circle) # FIXME: why are we keeping these types as strings? m.alias_type 'interval', 'varchar' - m.alias_type 'path', 'varchar' - m.alias_type 'line', 'varchar' - m.alias_type 'polygon', 'varchar' - m.alias_type 'circle', 'varchar' - m.alias_type 'lseg', 'varchar' - m.alias_type 'box', 'varchar' register_class_with_precision m, 'time', Type::Time register_class_with_precision m, 'timestamp', OID::DateTime @@ -734,7 +740,7 @@ module ActiveRecord end def create_table_definition(name, temporary = false, options = nil, as = nil) # :nodoc: - PostgreSQL::TableDefinition.new native_database_types, name, temporary, options, as + PostgreSQL::TableDefinition.new(name, temporary, options, as) end def can_perform_case_insensitive_comparison_for?(column) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 90df9b8825..72ca909b02 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -78,11 +78,10 @@ module ActiveRecord end def initialize(connection, logger, connection_options, config) - super(connection, logger) + super(connection, logger, config) @active = nil @statements = StatementPool.new(self.class.type_cast_config_to_integer(config.fetch(:statement_limit) { 1000 })) - @config = config @visitor = Arel::Visitors::SQLite.new self @quoted_column_names = {} diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 589c70db0d..8b719e0bcb 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -198,10 +198,11 @@ module ActiveRecord # If this is a StrongParameters hash, and access to inheritance_column is not permitted, # this will ignore the inheritance column and return nil def subclass_from_attributes?(attrs) - attribute_names.include?(inheritance_column) && attrs.is_a?(Hash) + attribute_names.include?(inheritance_column) && (attrs.is_a?(Hash) || attrs.respond_to?(:permitted?)) end def subclass_from_attributes(attrs) + attrs = attrs.to_h if attrs.respond_to?(:permitted?) subclass_name = attrs.with_indifferent_access[inheritance_column] if subclass_name.present? diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index ffd243a517..ca2537cdc3 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1202,17 +1202,17 @@ module ActiveRecord end def with_advisory_lock - key = generate_migrator_advisory_lock_key - got_lock = Base.connection.get_advisory_lock(key) + lock_id = generate_migrator_advisory_lock_id + got_lock = Base.connection.get_advisory_lock(lock_id) raise ConcurrentMigrationError unless got_lock load_migrated # reload schema_migrations to be sure it wasn't changed by another process before we got the lock yield ensure - Base.connection.release_advisory_lock(key) if got_lock + Base.connection.release_advisory_lock(lock_id) if got_lock end MIGRATOR_SALT = 2053462845 - def generate_migrator_advisory_lock_key + def generate_migrator_advisory_lock_id db_name_hash = Zlib.crc32(Base.connection.current_database) MIGRATOR_SALT * db_name_hash end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 94316d5249..46c6d8c293 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -358,6 +358,14 @@ module ActiveRecord # if the predicate returns +true+ the attribute will become +false+. This # method toggles directly the underlying value without calling any setter. # Returns +self+. + # + # Example: + # + # user = User.first + # user.banned? # => false + # user.toggle(:banned) + # user.banned? # => true + # def toggle(attribute) self[attribute] = !public_send("#{attribute}?") self diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 27de313d05..b1333f110c 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -36,13 +36,8 @@ module ActiveRecord # may vary depending on the klass of a relation, so we create a subclass of Relation # for each different klass, and the delegations are compiled into that subclass only. - BLACKLISTED_ARRAY_METHODS = [ - :compact!, :flatten!, :reject!, :reverse!, :rotate!, :map!, - :shuffle!, :slice!, :sort!, :sort_by!, :delete_if, - :keep_if, :pop, :shift, :delete_at, :select! - ].to_set # :nodoc: - - delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :join, to: :to_a + delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :join, + :[], :&, :|, :+, :-, :sample, :reverse, :compact, to: :to_a delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :columns_hash, :to => :klass @@ -114,21 +109,14 @@ module ActiveRecord def respond_to?(method, include_private = false) super || @klass.respond_to?(method, include_private) || - array_delegable?(method) || arel.respond_to?(method, include_private) end protected - def array_delegable?(method) - Array.method_defined?(method) && BLACKLISTED_ARRAY_METHODS.exclude?(method) - end - def method_missing(method, *args, &block) if @klass.respond_to?(method) scoping { @klass.public_send(method, *args, &block) } - elsif array_delegable?(method) - to_a.public_send(method, *args, &block) elsif arel.respond_to?(method) arel.public_send(method, *args, &block) else diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 2dc52982c9..dbecb842b5 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -13,6 +13,8 @@ module ActiveRecord # WhereChain objects act as placeholder for queries in which #where does not have any parameter. # In this case, #where must be chained with #not to return a new relation. class WhereChain + include ActiveModel::ForbiddenAttributesProtection + def initialize(scope) @scope = scope end @@ -41,6 +43,8 @@ module ActiveRecord # User.where.not(name: "Jon", role: "admin") # # SELECT * FROM users WHERE name != 'Jon' AND role != 'admin' def not(opts, *rest) + opts = sanitize_forbidden_attributes(opts) + where_clause = @scope.send(:where_clause_factory).build(opts, rest) @scope.references!(PredicateBuilder.references(opts)) if Hash === opts @@ -407,10 +411,30 @@ module ActiveRecord self end - # Performs a joins on +args+: + # Performs a joins on +args+. The given symbol(s) should match the name of + # the association(s). # # User.joins(:posts) - # # SELECT "users".* FROM "users" INNER JOIN "posts" ON "posts"."user_id" = "users"."id" + # # SELECT "users".* + # # FROM "users" + # # INNER JOIN "posts" ON "posts"."user_id" = "users"."id" + # + # Multiple joins: + # + # User.joins(:posts, :account) + # # SELECT "users".* + # # FROM "users" + # # INNER JOIN "posts" ON "posts"."user_id" = "users"."id" + # # INNER JOIN "accounts" ON "accounts"."id" = "users"."account_id" + # + # Nested joins: + # + # User.joins(posts: [:comments]) + # # SELECT "users".* + # # FROM "users" + # # INNER JOIN "posts" ON "posts"."user_id" = "users"."id" + # # INNER JOIN "comments" "comments_posts" + # # ON "comments_posts"."post_id" = "posts"."id" # # You can use strings in order to customize your joins: # diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 5c3318651a..67d7f83cb4 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -12,6 +12,7 @@ module ActiveRecord # Merges in the conditions from <tt>other</tt>, if <tt>other</tt> is an ActiveRecord::Relation. # Returns an array representing the intersection of the resulting records with <tt>other</tt>, if <tt>other</tt> is an array. + # # Post.where(published: true).joins(:comments).merge( Comment.where(spam: false) ) # # Performs a single join query with both where conditions. # @@ -37,11 +38,14 @@ module ActiveRecord end def merge!(other) # :nodoc: - if !other.is_a?(Relation) && other.respond_to?(:to_proc) + if other.is_a?(Hash) + Relation::HashMerger.new(self, other).merge + elsif other.is_a?(Relation) + Relation::Merger.new(self, other).merge + elsif other.respond_to?(:to_proc) instance_exec(&other) else - klass = other.is_a?(Hash) ? Relation::HashMerger : Relation::Merger - klass.new(self, other).merge + raise ArgumentError, "#{other.inspect} is not an ActiveRecord::Relation" end end diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb index cd7d949239..8b4874044c 100644 --- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb @@ -56,9 +56,9 @@ module ActiveRecord args = ['-s', '-x', '-O', '-f', filename] unless search_path.blank? - args << search_path.split(',').map do |part| + args += search_path.split(',').map do |part| "--schema=#{part.strip}" - end.join(' ') + end end args << configuration['database'] run_cmd('pg_dump', args, 'dumping') diff --git a/activerecord/test/cases/adapters/mysql/active_schema_test.rb b/activerecord/test/cases/adapters/mysql/active_schema_test.rb index 0b5c9e1798..8a7a0bb25d 100644 --- a/activerecord/test/cases/adapters/mysql/active_schema_test.rb +++ b/activerecord/test/cases/adapters/mysql/active_schema_test.rb @@ -16,8 +16,8 @@ class MysqlActiveSchemaTest < ActiveRecord::MysqlTestCase end def test_add_index - # add_index calls table_exists? and index_name_exists? which can't work since execute is stubbed - def (ActiveRecord::Base.connection).table_exists?(*); true; end + # add_index calls data_source_exists? and index_name_exists? which can't work since execute is stubbed + def (ActiveRecord::Base.connection).data_source_exists?(*); true; end def (ActiveRecord::Base.connection).index_name_exists?(*); false; end expected = "CREATE INDEX `index_people_on_last_name` ON `people` (`last_name`) " @@ -60,7 +60,7 @@ class MysqlActiveSchemaTest < ActiveRecord::MysqlTestCase end def test_index_in_create - def (ActiveRecord::Base.connection).table_exists?(*); false; end + def (ActiveRecord::Base.connection).data_source_exists?(*); false; end %w(SPATIAL FULLTEXT UNIQUE).each do |type| expected = "CREATE TABLE `people` (#{type} INDEX `index_people_on_last_name` (`last_name`) ) ENGINE=InnoDB" @@ -78,7 +78,7 @@ class MysqlActiveSchemaTest < ActiveRecord::MysqlTestCase end def test_index_in_bulk_change - def (ActiveRecord::Base.connection).table_exists?(*); true; end + def (ActiveRecord::Base.connection).data_source_exists?(*); true; end def (ActiveRecord::Base.connection).index_name_exists?(*); false; end %w(SPATIAL FULLTEXT UNIQUE).each do |type| @@ -152,7 +152,7 @@ class MysqlActiveSchemaTest < ActiveRecord::MysqlTestCase end def test_indexes_in_create - ActiveRecord::Base.connection.stubs(:table_exists?).with(:temp).returns(false) + ActiveRecord::Base.connection.stubs(:data_source_exists?).with(:temp).returns(false) ActiveRecord::Base.connection.stubs(:index_name_exists?).with(:index_temp_on_zip).returns(false) expected = "CREATE TEMPORARY TABLE `temp` ( INDEX `index_temp_on_zip` (`zip`) ) ENGINE=InnoDB AS SELECT id, name, zip FROM a_really_complicated_query" diff --git a/activerecord/test/cases/adapters/mysql/connection_test.rb b/activerecord/test/cases/adapters/mysql/connection_test.rb index 75653ee9af..390dd15b92 100644 --- a/activerecord/test/cases/adapters/mysql/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql/connection_test.rb @@ -171,31 +171,31 @@ class MysqlConnectionTest < ActiveRecord::MysqlTestCase end def test_get_and_release_advisory_lock - key = "test_key" + lock_name = "test_lock_name" - got_lock = @connection.get_advisory_lock(key) + got_lock = @connection.get_advisory_lock(lock_name) assert got_lock, "get_advisory_lock should have returned true but it didn't" - assert_equal test_lock_free(key), false, + assert_equal test_lock_free(lock_name), false, "expected the test advisory lock to be held but it wasn't" - released_lock = @connection.release_advisory_lock(key) + released_lock = @connection.release_advisory_lock(lock_name) assert released_lock, "expected release_advisory_lock to return true but it didn't" - assert test_lock_free(key), 'expected the test key to be available after releasing' + assert test_lock_free(lock_name), 'expected the test lock to be available after releasing' end def test_release_non_existent_advisory_lock - fake_key = "fake_key" - released_non_existent_lock = @connection.release_advisory_lock(fake_key) + lock_name = "fake_lock_name" + released_non_existent_lock = @connection.release_advisory_lock(lock_name) assert_equal released_non_existent_lock, false, 'expected release_advisory_lock to return false when there was no lock to release' end protected - def test_lock_free(key) - @connection.select_value("SELECT IS_FREE_LOCK('#{key}');") == '1' + def test_lock_free(lock_name) + @connection.select_value("SELECT IS_FREE_LOCK('#{lock_name}');") == '1' end private diff --git a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb index b97eb3e228..99f97c7914 100644 --- a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb +++ b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb @@ -16,7 +16,7 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase end def test_add_index - # add_index calls table_exists? and index_name_exists? which can't work since execute is stubbed + # add_index calls data_source_exists? and index_name_exists? which can't work since execute is stubbed def (ActiveRecord::Base.connection).data_source_exists?(*); true; end def (ActiveRecord::Base.connection).index_name_exists?(*); false; end diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb index 71c4028675..507d024bb6 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -133,30 +133,30 @@ class Mysql2ConnectionTest < ActiveRecord::Mysql2TestCase end def test_get_and_release_advisory_lock - key = "test_key" + lock_name = "test_lock_name" - got_lock = @connection.get_advisory_lock(key) + got_lock = @connection.get_advisory_lock(lock_name) assert got_lock, "get_advisory_lock should have returned true but it didn't" - assert_equal test_lock_free(key), false, + assert_equal test_lock_free(lock_name), false, "expected the test advisory lock to be held but it wasn't" - released_lock = @connection.release_advisory_lock(key) + released_lock = @connection.release_advisory_lock(lock_name) assert released_lock, "expected release_advisory_lock to return true but it didn't" - assert test_lock_free(key), 'expected the test key to be available after releasing' + assert test_lock_free(lock_name), 'expected the test lock to be available after releasing' end def test_release_non_existent_advisory_lock - fake_key = "fake_key" - released_non_existent_lock = @connection.release_advisory_lock(fake_key) + lock_name = "fake_lock_name" + released_non_existent_lock = @connection.release_advisory_lock(lock_name) assert_equal released_non_existent_lock, false, 'expected release_advisory_lock to return false when there was no lock to release' end protected - def test_lock_free(key) - @connection.select_value("SELECT IS_FREE_LOCK('#{key}');") == 1 + def test_lock_free(lock_name) + @connection.select_value("SELECT IS_FREE_LOCK('#{lock_name}');") == 1 end end diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb index 0ecac2cfa3..d559de3e28 100644 --- a/activerecord/test/cases/adapters/postgresql/connection_test.rb +++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb @@ -211,33 +211,33 @@ module ActiveRecord end def test_get_and_release_advisory_lock - key = 5295901941911233559 + lock_id = 5295901941911233559 list_advisory_locks = <<-SQL SELECT locktype, - (classid::bigint << 32) | objid::bigint AS lock_key + (classid::bigint << 32) | objid::bigint AS lock_id FROM pg_locks WHERE locktype = 'advisory' SQL - got_lock = @connection.get_advisory_lock(key) + got_lock = @connection.get_advisory_lock(lock_id) assert got_lock, "get_advisory_lock should have returned true but it didn't" - advisory_lock = @connection.query(list_advisory_locks).find {|l| l[1] == key} + advisory_lock = @connection.query(list_advisory_locks).find {|l| l[1] == lock_id} assert advisory_lock, - "expected to find an advisory lock with key #{key} but there wasn't one" + "expected to find an advisory lock with lock_id #{lock_id} but there wasn't one" - released_lock = @connection.release_advisory_lock(key) + released_lock = @connection.release_advisory_lock(lock_id) assert released_lock, "expected release_advisory_lock to return true but it didn't" - advisory_locks = @connection.query(list_advisory_locks).select {|l| l[1] == key} + advisory_locks = @connection.query(list_advisory_locks).select {|l| l[1] == lock_id} assert_empty advisory_locks, - "expected to have released advisory lock with key #{key} but it was still held" + "expected to have released advisory lock with lock_id #{lock_id} but it was still held" end def test_release_non_existent_advisory_lock - fake_key = 2940075057017742022 + fake_lock_id = 2940075057017742022 with_warning_suppression do - released_non_existent_lock = @connection.release_advisory_lock(fake_key) + released_non_existent_lock = @connection.release_advisory_lock(fake_lock_id) assert_equal released_non_existent_lock, false, 'expected release_advisory_lock to return false when there was no lock to release' end diff --git a/activerecord/test/cases/adapters/postgresql/geometric_test.rb b/activerecord/test/cases/adapters/postgresql/geometric_test.rb index 0baf985654..8d0c5bf23f 100644 --- a/activerecord/test/cases/adapters/postgresql/geometric_test.rb +++ b/activerecord/test/cases/adapters/postgresql/geometric_test.rb @@ -167,16 +167,18 @@ class PostgresqlPointTest < ActiveRecord::PostgreSQLTestCase end class PostgresqlGeometricTest < ActiveRecord::PostgreSQLTestCase + include SchemaDumpingHelper + class PostgresqlGeometric < ActiveRecord::Base; end setup do @connection = ActiveRecord::Base.connection @connection.create_table("postgresql_geometrics") do |t| - t.column :a_line_segment, :lseg - t.column :a_box, :box - t.column :a_path, :path - t.column :a_polygon, :polygon - t.column :a_circle, :circle + t.lseg :a_line_segment + t.box :a_box + t.path :a_path + t.polygon :a_polygon + t.circle :a_circle end end @@ -233,4 +235,142 @@ class PostgresqlGeometricTest < ActiveRecord::PostgreSQLTestCase objs = PostgresqlGeometric.find_by_sql "SELECT isclosed(a_path) FROM postgresql_geometrics ORDER BY id ASC" assert_equal [false, true], objs.map(&:isclosed) end + + def test_schema_dumping + output = dump_table_schema("postgresql_geometrics") + assert_match %r{t\.lseg\s+"a_line_segment"$}, output + assert_match %r{t\.box\s+"a_box"$}, output + assert_match %r{t\.path\s+"a_path"$}, output + assert_match %r{t\.polygon\s+"a_polygon"$}, output + assert_match %r{t\.circle\s+"a_circle"$}, output + end +end + +class PostgreSQLGeometricLineTest < ActiveRecord::PostgreSQLTestCase + include SchemaDumpingHelper + + class PostgresqlLine < ActiveRecord::Base; end + + setup do + unless ActiveRecord::Base.connection.send(:postgresql_version) >= 90400 + skip("line type is not fully implemented") + end + @connection = ActiveRecord::Base.connection + @connection.create_table("postgresql_lines") do |t| + t.line :a_line + end + end + + teardown do + @connection.drop_table 'postgresql_lines', if_exists: true + end + + def test_geometric_line_type + g = PostgresqlLine.new( + a_line: '{2.0, 3, 5.5}' + ) + g.save! + + h = PostgresqlLine.find(g.id) + assert_equal '{2,3,5.5}', h.a_line + end + + def test_alternative_format_line_type + g = PostgresqlLine.new( + a_line: '(2.0, 3), (4.0, 6.0)' + ) + g.save! + + h = PostgresqlLine.find(g.id) + assert_equal '{1.5,-1,0}', h.a_line + end + + def test_schema_dumping_for_line_type + output = dump_table_schema("postgresql_lines") + assert_match %r{t\.line\s+"a_line"$}, output + end +end + +class PostgreSQLGeometricTypesTest < ActiveRecord::PostgreSQLTestCase + attr_reader :connection, :table_name + + def setup + super + @connection = ActiveRecord::Base.connection + @table_name = :testings + end + + def test_creating_column_with_point_type + connection.create_table(table_name) do |t| + t.point :foo_point + end + + assert_column_exists(:foo_point) + assert_type_correct(:foo_point, :point) + end + + def test_creating_column_with_line_type + connection.create_table(table_name) do |t| + t.line :foo_line + end + + assert_column_exists(:foo_line) + assert_type_correct(:foo_line, :line) + end + + def test_creating_column_with_lseg_type + connection.create_table(table_name) do |t| + t.lseg :foo_lseg + end + + assert_column_exists(:foo_lseg) + assert_type_correct(:foo_lseg, :lseg) + end + + def test_creating_column_with_box_type + connection.create_table(table_name) do |t| + t.box :foo_box + end + + assert_column_exists(:foo_box) + assert_type_correct(:foo_box, :box) + end + + def test_creating_column_with_path_type + connection.create_table(table_name) do |t| + t.path :foo_path + end + + assert_column_exists(:foo_path) + assert_type_correct(:foo_path, :path) + end + + def test_creating_column_with_polygon_type + connection.create_table(table_name) do |t| + t.polygon :foo_polygon + end + + assert_column_exists(:foo_polygon) + assert_type_correct(:foo_polygon, :polygon) + end + + def test_creating_column_with_circle_type + connection.create_table(table_name) do |t| + t.circle :foo_circle + end + + assert_column_exists(:foo_circle) + assert_type_correct(:foo_circle, :circle) + end + + private + + def assert_column_exists(column_name) + assert connection.column_exists?(table_name, column_name) + end + + def assert_type_correct(column_name, type) + column = connection.columns(table_name).find { |c| c.name == column_name.to_s } + assert_equal type, column.type + end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 50ca6537cc..ad157582a4 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2348,6 +2348,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [first_bulb, second_bulb], car.bulbs end + test 'double insertion of new object to association when same association used in the after create callback of a new object' do + car = Car.create! + car.bulbs << TrickyBulb.new + assert_equal 1, car.bulbs.size + end + def test_association_force_reload_with_only_true_is_deprecated company = Company.find(1) diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 6686ce012d..91214da048 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -706,96 +706,13 @@ class FinderTest < ActiveRecord::TestCase assert Company.where(["name = :name", {name: "37signals' go'es agains"}]).first end - def test_bind_arity - assert_nothing_raised { bind '' } - assert_raise(ActiveRecord::PreparedStatementInvalid) { bind '', 1 } - - assert_raise(ActiveRecord::PreparedStatementInvalid) { bind '?' } - assert_nothing_raised { bind '?', 1 } - assert_raise(ActiveRecord::PreparedStatementInvalid) { bind '?', 1, 1 } - end - def test_named_bind_variables - assert_equal '1', bind(':a', :a => 1) # ' ruby-mode - assert_equal '1 1', bind(':a :a', :a => 1) # ' ruby-mode - - assert_nothing_raised { bind("'+00:00'", :foo => "bar") } - assert_kind_of Firm, Company.where(["name = :name", { name: "37signals" }]).first assert_nil Company.where(["name = :name", { name: "37signals!" }]).first assert_nil Company.where(["name = :name", { name: "37signals!' OR 1=1" }]).first assert_kind_of Time, Topic.where(["id = :id", { id: 1 }]).first.written_on end - def test_named_bind_arity - assert_nothing_raised { bind "name = :name", { name: "37signals" } } - assert_nothing_raised { bind "name = :name", { name: "37signals", id: 1 } } - assert_raise(ActiveRecord::PreparedStatementInvalid) { bind "name = :name", { id: 1 } } - end - - class SimpleEnumerable - include Enumerable - - def initialize(ary) - @ary = ary - end - - def each(&b) - @ary.each(&b) - end - end - - def test_bind_enumerable - quoted_abc = %(#{ActiveRecord::Base.connection.quote('a')},#{ActiveRecord::Base.connection.quote('b')},#{ActiveRecord::Base.connection.quote('c')}) - - assert_equal '1,2,3', bind('?', [1, 2, 3]) - assert_equal quoted_abc, bind('?', %w(a b c)) - - assert_equal '1,2,3', bind(':a', :a => [1, 2, 3]) - assert_equal quoted_abc, bind(':a', :a => %w(a b c)) # ' - - assert_equal '1,2,3', bind('?', SimpleEnumerable.new([1, 2, 3])) - assert_equal quoted_abc, bind('?', SimpleEnumerable.new(%w(a b c))) - - assert_equal '1,2,3', bind(':a', :a => SimpleEnumerable.new([1, 2, 3])) - assert_equal quoted_abc, bind(':a', :a => SimpleEnumerable.new(%w(a b c))) # ' - end - - def test_bind_empty_enumerable - quoted_nil = ActiveRecord::Base.connection.quote(nil) - assert_equal quoted_nil, bind('?', []) - assert_equal " in (#{quoted_nil})", bind(' in (?)', []) - assert_equal "foo in (#{quoted_nil})", bind('foo in (?)', []) - end - - def test_bind_empty_string - quoted_empty = ActiveRecord::Base.connection.quote('') - assert_equal quoted_empty, bind('?', '') - end - - def test_bind_chars - quoted_bambi = ActiveRecord::Base.connection.quote("Bambi") - quoted_bambi_and_thumper = ActiveRecord::Base.connection.quote("Bambi\nand\nThumper") - assert_equal "name=#{quoted_bambi}", bind('name=?', "Bambi") - assert_equal "name=#{quoted_bambi_and_thumper}", bind('name=?', "Bambi\nand\nThumper") - assert_equal "name=#{quoted_bambi}", bind('name=?', "Bambi".mb_chars) - assert_equal "name=#{quoted_bambi_and_thumper}", bind('name=?', "Bambi\nand\nThumper".mb_chars) - end - - def test_bind_record - o = Struct.new(:quoted_id).new(1) - assert_equal '1', bind('?', o) - - os = [o] * 3 - assert_equal '1,1,1', bind('?', os) - end - - def test_named_bind_with_postgresql_type_casts - l = Proc.new { bind(":a::integer '2009-01-01'::date", :a => '10') } - assert_nothing_raised(&l) - assert_equal "#{ActiveRecord::Base.connection.quote('10')}::integer '2009-01-01'::date", l.call - end - def test_string_sanitation assert_not_equal "'something ' 1=1'", ActiveRecord::Base.sanitize("something ' 1=1") assert_equal "'something; select table'", ActiveRecord::Base.sanitize("something; select table") @@ -1136,14 +1053,6 @@ class FinderTest < ActiveRecord::TestCase end protected - def bind(statement, *vars) - if vars.first.is_a?(Hash) - ActiveRecord::Base.send(:replace_named_bind_variables, statement, vars.first) - else - ActiveRecord::Base.send(:replace_bind_variables, statement, vars) - end - end - def table_with_custom_primary_key yield(Class.new(Toy) do def self.name diff --git a/activerecord/test/cases/forbidden_attributes_protection_test.rb b/activerecord/test/cases/forbidden_attributes_protection_test.rb index f4e7646f03..91921469b8 100644 --- a/activerecord/test/cases/forbidden_attributes_protection_test.rb +++ b/activerecord/test/cases/forbidden_attributes_protection_test.rb @@ -1,14 +1,20 @@ require 'cases/helper' require 'active_support/core_ext/hash/indifferent_access' -require 'models/person' + require 'models/company' +require 'models/person' +require 'models/ship' +require 'models/ship_part' +require 'models/treasure' -class ProtectedParams < ActiveSupport::HashWithIndifferentAccess +class ProtectedParams attr_accessor :permitted alias :permitted? :permitted + delegate :keys, :key?, :has_key?, :empty?, to: :@parameters + def initialize(attributes) - super(attributes) + @parameters = attributes.with_indifferent_access @permitted = false end @@ -17,6 +23,18 @@ class ProtectedParams < ActiveSupport::HashWithIndifferentAccess self end + def [](key) + @parameters[key] + end + + def to_h + @parameters + end + + def stringify_keys + dup + end + def dup super.tap do |duplicate| duplicate.instance_variable_set :@permitted, @permitted @@ -75,6 +93,13 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase end end + def test_create_with_works_with_permitted_params + params = ProtectedParams.new(first_name: 'Guille').permit! + + person = Person.create_with(params).create! + assert_equal 'Guille', person.first_name + end + def test_create_with_works_with_params_values params = ProtectedParams.new(first_name: 'Guille') @@ -90,10 +115,51 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase end end + def test_where_works_with_permitted_params + params = ProtectedParams.new(first_name: 'Guille').permit! + + person = Person.where(params).create! + assert_equal 'Guille', person.first_name + end + def test_where_works_with_params_values params = ProtectedParams.new(first_name: 'Guille') person = Person.where(first_name: params[:first_name]).create! assert_equal 'Guille', person.first_name end + + def test_where_not_checks_permitted + params = ProtectedParams.new(first_name: 'Guille', gender: 'm') + + assert_raises(ActiveModel::ForbiddenAttributesError) do + Person.where().not(params) + end + end + + def test_where_not_works_with_permitted_params + params = ProtectedParams.new(first_name: 'Guille').permit! + Person.create!(params) + assert_empty Person.where.not(params).select {|p| p.first_name == 'Guille' } + end + + def test_strong_params_style_objects_work_with_singular_associations + params = ProtectedParams.new( name: "Stern", ship_attributes: ProtectedParams.new(name: "The Black Rock").permit!).permit! + part = ShipPart.new(params) + + assert_equal "Stern", part.name + assert_equal "The Black Rock", part.ship.name + end + + def test_strong_params_style_objects_work_with_collection_associations + params = ProtectedParams.new( + trinkets_attributes: ProtectedParams.new( + "0" => ProtectedParams.new(name: "Necklace").permit!, + "1" => ProtectedParams.new(name: "Spoon").permit! ) ).permit! + part = ShipPart.new(params) + + assert_equal "Necklace", part.trinkets[0].name + assert_equal "Spoon", part.trinkets[1].name + end + end diff --git a/activerecord/test/cases/migration/postgresql_geometric_types_test.rb b/activerecord/test/cases/migration/postgresql_geometric_types_test.rb deleted file mode 100644 index e4772905bb..0000000000 --- a/activerecord/test/cases/migration/postgresql_geometric_types_test.rb +++ /dev/null @@ -1,93 +0,0 @@ -require 'cases/helper' - -module ActiveRecord - class Migration - class PostgreSQLGeometricTypesTest < ActiveRecord::TestCase - attr_reader :connection, :table_name - - def setup - super - @connection = ActiveRecord::Base.connection - @table_name = :testings - end - - if current_adapter?(:PostgreSQLAdapter) - def test_creating_column_with_point_type - connection.create_table(table_name) do |t| - t.point :foo_point - end - - assert_column_exists(:foo_point) - assert_type_correct(:foo_point, :point) - end - - def test_creating_column_with_line_type - connection.create_table(table_name) do |t| - t.line :foo_line - end - - assert_column_exists(:foo_line) - assert_type_correct(:foo_line, :line) - end - - def test_creating_column_with_lseg_type - connection.create_table(table_name) do |t| - t.lseg :foo_lseg - end - - assert_column_exists(:foo_lseg) - assert_type_correct(:foo_lseg, :lseg) - end - - def test_creating_column_with_box_type - connection.create_table(table_name) do |t| - t.box :foo_box - end - - assert_column_exists(:foo_box) - assert_type_correct(:foo_box, :box) - end - - def test_creating_column_with_path_type - connection.create_table(table_name) do |t| - t.path :foo_path - end - - assert_column_exists(:foo_path) - assert_type_correct(:foo_path, :path) - end - - def test_creating_column_with_polygon_type - connection.create_table(table_name) do |t| - t.polygon :foo_polygon - end - - assert_column_exists(:foo_polygon) - assert_type_correct(:foo_polygon, :polygon) - end - - def test_creating_column_with_circle_type - connection.create_table(table_name) do |t| - t.circle :foo_circle - end - - assert_column_exists(:foo_circle) - assert_type_correct(:foo_circle, :circle) - end - end - - private - def assert_column_exists(column_name) - columns = connection.columns(table_name) - assert columns.map(&:name).include?(column_name.to_s) - end - - def assert_type_correct(column_name, type) - columns = connection.columns(table_name) - column = columns.select{ |c| c.name == column_name.to_s }.first - assert_equal type.to_s, column.sql_type - end - - end - end -end
\ No newline at end of file diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 15a0e0516d..c3c204cf9f 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -524,33 +524,33 @@ class MigrationTest < ActiveRecord::TestCase end if ActiveRecord::Base.connection.supports_advisory_locks? - def test_migrator_generates_valid_lock_key + def test_migrator_generates_valid_lock_id migration = Class.new(ActiveRecord::Migration).new migrator = ActiveRecord::Migrator.new(:up, [migration], 100) - lock_key = migrator.send(:generate_migrator_advisory_lock_key) + lock_id = migrator.send(:generate_migrator_advisory_lock_id) - assert ActiveRecord::Base.connection.get_advisory_lock(lock_key), - "the Migrator should have generated a valid lock key, but it didn't" - assert ActiveRecord::Base.connection.release_advisory_lock(lock_key), - "the Migrator should have generated a valid lock key, but it didn't" + assert ActiveRecord::Base.connection.get_advisory_lock(lock_id), + "the Migrator should have generated a valid lock id, but it didn't" + assert ActiveRecord::Base.connection.release_advisory_lock(lock_id), + "the Migrator should have generated a valid lock id, but it didn't" end - def test_generate_migrator_advisory_lock_key + def test_generate_migrator_advisory_lock_id # It is important we are consistent with how we generate this so that # exclusive locking works across migrator versions migration = Class.new(ActiveRecord::Migration).new migrator = ActiveRecord::Migrator.new(:up, [migration], 100) - lock_key = migrator.send(:generate_migrator_advisory_lock_key) + lock_id = migrator.send(:generate_migrator_advisory_lock_id) current_database = ActiveRecord::Base.connection.current_database salt = ActiveRecord::Migrator::MIGRATOR_SALT - expected_key = Zlib.crc32(current_database) * salt + expected_id = Zlib.crc32(current_database) * salt - assert lock_key == expected_key, "expected lock key generated by the migrator to be #{expected_key}, but it was #{lock_key} instead" - assert lock_key.is_a?(Fixnum), "expected lock key to be a Fixnum, but it wasn't" - assert lock_key.bit_length <= 63, "lock key must be a signed integer of max 63 bits magnitude" + assert lock_id == expected_id, "expected lock id generated by the migrator to be #{expected_id}, but it was #{lock_id} instead" + assert lock_id.is_a?(Fixnum), "expected lock id to be a Fixnum, but it wasn't" + assert lock_id.bit_length <= 63, "lock id must be a signed integer of max 63 bits magnitude" end def test_migrator_one_up_with_unavailable_lock @@ -564,9 +564,9 @@ class MigrationTest < ActiveRecord::TestCase }.new migrator = ActiveRecord::Migrator.new(:up, [migration], 100) - lock_key = migrator.send(:generate_migrator_advisory_lock_key) + lock_id = migrator.send(:generate_migrator_advisory_lock_id) - with_another_process_holding_lock(lock_key) do + with_another_process_holding_lock(lock_id) do assert_raise(ActiveRecord::ConcurrentMigrationError) { migrator.migrate } end @@ -585,9 +585,9 @@ class MigrationTest < ActiveRecord::TestCase }.new migrator = ActiveRecord::Migrator.new(:up, [migration], 100) - lock_key = migrator.send(:generate_migrator_advisory_lock_key) + lock_id = migrator.send(:generate_migrator_advisory_lock_id) - with_another_process_holding_lock(lock_key) do + with_another_process_holding_lock(lock_id) do assert_raise(ActiveRecord::ConcurrentMigrationError) { migrator.run } end @@ -606,18 +606,18 @@ class MigrationTest < ActiveRecord::TestCase } end - def with_another_process_holding_lock(lock_key) + def with_another_process_holding_lock(lock_id) thread_lock = Concurrent::CountDownLatch.new test_terminated = Concurrent::CountDownLatch.new other_process = Thread.new do begin conn = ActiveRecord::Base.connection_pool.checkout - conn.get_advisory_lock(lock_key) + conn.get_advisory_lock(lock_id) thread_lock.count_down test_terminated.wait # hold the lock open until we tested everything ensure - conn.release_advisory_lock(lock_key) + conn.release_advisory_lock(lock_id) ActiveRecord::Base.connection_pool.checkin(conn) end end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 93cb631a04..0b700afcb4 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -1068,39 +1068,4 @@ class TestHasManyAutosaveAssociationWhichItselfHasAutosaveAssociations < ActiveR assert_not part.valid? assert_equal ["Ship name can't be blank"], part.errors.full_messages end - - class ProtectedParameters - def initialize(hash) - @hash = hash - end - - def permitted? - true - end - - def [](key) - @hash[key] - end - - def to_h - @hash - end - end - - test "strong params style objects can be assigned for singular associations" do - params = { name: "Stern", ship_attributes: - ProtectedParameters.new(name: "The Black Rock") } - part = ShipPart.new(params) - - assert_equal "Stern", part.name - assert_equal "The Black Rock", part.ship.name - end - - test "strong params style objects can be assigned for collection associations" do - params = { trinkets_attributes: ProtectedParameters.new("0" => ProtectedParameters.new(name: "Necklace"), "1" => ProtectedParameters.new(name: "Spoon")) } - part = ShipPart.new(params) - - assert_equal "Necklace", part.trinkets[0].name - assert_equal "Spoon", part.trinkets[1].name - end end diff --git a/activerecord/test/cases/relation/delegation_test.rb b/activerecord/test/cases/relation/delegation_test.rb index 989f4e1e5d..b4269bd56d 100644 --- a/activerecord/test/cases/relation/delegation_test.rb +++ b/activerecord/test/cases/relation/delegation_test.rb @@ -40,12 +40,6 @@ module ActiveRecord assert_respond_to target, method end end - - ActiveRecord::Delegation::BLACKLISTED_ARRAY_METHODS.each do |method| - define_method "test_#{method}_is_not_delegated_to_Array" do - assert_raises(NoMethodError) { call_method(target, method) } - end - end end class DelegationAssociationTest < DelegationTest diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 8794bc8043..f46d414b95 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -235,6 +235,13 @@ module ActiveRecord assert_equal 3, relation.where(id: post.id).pluck(:id).size end + def test_merge_raises_with_invalid_argument + assert_raises ArgumentError do + relation = Relation.new(FakeKlass, :b, nil) + relation.merge(true) + end + end + def test_respond_to_for_non_selected_element post = Post.select(:title).first assert_equal false, post.respond_to?(:body), "post should not respond_to?(:body) since invoking it raises exception" diff --git a/activerecord/test/cases/sanitize_test.rb b/activerecord/test/cases/sanitize_test.rb index 14e392ac30..07970fb1c1 100644 --- a/activerecord/test/cases/sanitize_test.rb +++ b/activerecord/test/cases/sanitize_test.rb @@ -69,4 +69,98 @@ class SanitizeTest < ActiveRecord::TestCase searchable_post.search("20% _reduction_!").to_a end end + + def test_bind_arity + assert_nothing_raised { bind '' } + assert_raise(ActiveRecord::PreparedStatementInvalid) { bind '', 1 } + + assert_raise(ActiveRecord::PreparedStatementInvalid) { bind '?' } + assert_nothing_raised { bind '?', 1 } + assert_raise(ActiveRecord::PreparedStatementInvalid) { bind '?', 1, 1 } + end + + def test_named_bind_variables + assert_equal '1', bind(':a', :a => 1) # ' ruby-mode + assert_equal '1 1', bind(':a :a', :a => 1) # ' ruby-mode + + assert_nothing_raised { bind("'+00:00'", :foo => "bar") } + end + + def test_named_bind_arity + assert_nothing_raised { bind "name = :name", { name: "37signals" } } + assert_nothing_raised { bind "name = :name", { name: "37signals", id: 1 } } + assert_raise(ActiveRecord::PreparedStatementInvalid) { bind "name = :name", { id: 1 } } + end + + class SimpleEnumerable + include Enumerable + + def initialize(ary) + @ary = ary + end + + def each(&b) + @ary.each(&b) + end + end + + def test_bind_enumerable + quoted_abc = %(#{ActiveRecord::Base.connection.quote('a')},#{ActiveRecord::Base.connection.quote('b')},#{ActiveRecord::Base.connection.quote('c')}) + + assert_equal '1,2,3', bind('?', [1, 2, 3]) + assert_equal quoted_abc, bind('?', %w(a b c)) + + assert_equal '1,2,3', bind(':a', :a => [1, 2, 3]) + assert_equal quoted_abc, bind(':a', :a => %w(a b c)) # ' + + assert_equal '1,2,3', bind('?', SimpleEnumerable.new([1, 2, 3])) + assert_equal quoted_abc, bind('?', SimpleEnumerable.new(%w(a b c))) + + assert_equal '1,2,3', bind(':a', :a => SimpleEnumerable.new([1, 2, 3])) + assert_equal quoted_abc, bind(':a', :a => SimpleEnumerable.new(%w(a b c))) # ' + end + + def test_bind_empty_enumerable + quoted_nil = ActiveRecord::Base.connection.quote(nil) + assert_equal quoted_nil, bind('?', []) + assert_equal " in (#{quoted_nil})", bind(' in (?)', []) + assert_equal "foo in (#{quoted_nil})", bind('foo in (?)', []) + end + + def test_bind_empty_string + quoted_empty = ActiveRecord::Base.connection.quote('') + assert_equal quoted_empty, bind('?', '') + end + + def test_bind_chars + quoted_bambi = ActiveRecord::Base.connection.quote("Bambi") + quoted_bambi_and_thumper = ActiveRecord::Base.connection.quote("Bambi\nand\nThumper") + assert_equal "name=#{quoted_bambi}", bind('name=?', "Bambi") + assert_equal "name=#{quoted_bambi_and_thumper}", bind('name=?', "Bambi\nand\nThumper") + assert_equal "name=#{quoted_bambi}", bind('name=?', "Bambi".mb_chars) + assert_equal "name=#{quoted_bambi_and_thumper}", bind('name=?', "Bambi\nand\nThumper".mb_chars) + end + + def test_bind_record + o = Struct.new(:quoted_id).new(1) + assert_equal '1', bind('?', o) + + os = [o] * 3 + assert_equal '1,1,1', bind('?', os) + end + + def test_named_bind_with_postgresql_type_casts + l = Proc.new { bind(":a::integer '2009-01-01'::date", :a => '10') } + assert_nothing_raised(&l) + assert_equal "#{ActiveRecord::Base.connection.quote('10')}::integer '2009-01-01'::date", l.call + end + + private + def bind(statement, *vars) + if vars.first.is_a?(Hash) + ActiveRecord::Base.send(:replace_named_bind_variables, statement, vars.first) + else + ActiveRecord::Base.send(:replace_bind_variables, statement, vars) + end + end end diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 2a2c2bc8d0..43f133b12d 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -353,6 +353,38 @@ class SchemaDumperTest < ActiveRecord::TestCase ActiveRecord::Base.table_name_suffix = ActiveRecord::Base.table_name_prefix = '' $stdout = original end + + def test_schema_dump_with_table_name_prefix_and_ignoring_tables + original, $stdout = $stdout, StringIO.new + + create_cat_migration = Class.new(ActiveRecord::Migration) do + def change + create_table("cats") do |t| + end + create_table("omg_cats") do |t| + end + end + end + + original_table_name_prefix = ActiveRecord::Base.table_name_prefix + original_schema_dumper_ignore_tables = ActiveRecord::SchemaDumper.ignore_tables + ActiveRecord::Base.table_name_prefix = 'omg_' + ActiveRecord::SchemaDumper.ignore_tables = ["cats"] + migration = create_cat_migration.new + migration.migrate(:up) + + stream = StringIO.new + output = ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, stream).string + + assert_match %r{create_table "omg_cats"}, output + refute_match %r{create_table "cats"}, output + ensure + migration.migrate(:down) + ActiveRecord::Base.table_name_prefix = original_table_name_prefix + ActiveRecord::SchemaDumper.ignore_tables = original_schema_dumper_ignore_tables + + $stdout = original + end end class SchemaDumperDefaultsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb index c31f94b2f2..ba53f340ae 100644 --- a/activerecord/test/cases/tasks/postgresql_rake_test.rb +++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb @@ -212,7 +212,7 @@ module ActiveRecord def test_structure_dump_with_schema_search_path @configuration['schema_search_path'] = 'foo,bar' - Kernel.expects(:system).with('pg_dump', '-s', '-x', '-O', '-f', @filename, '--schema=foo --schema=bar', 'my-app-db').returns(true) + Kernel.expects(:system).with('pg_dump', '-s', '-x', '-O', '-f', @filename, '--schema=foo', '--schema=bar', 'my-app-db').returns(true) ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename) end @@ -228,7 +228,7 @@ module ActiveRecord end def test_structure_dump_with_dump_schemas_string - Kernel.expects(:system).with("pg_dump", '-s', '-x', '-O', '-f', @filename, '--schema=foo --schema=bar', "my-app-db").returns(true) + Kernel.expects(:system).with("pg_dump", '-s', '-x', '-O', '-f', @filename, '--schema=foo', '--schema=bar', "my-app-db").returns(true) with_dump_schemas('foo,bar') do ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename) diff --git a/activerecord/test/models/bulb.rb b/activerecord/test/models/bulb.rb index c1e491e5c5..dc0296305a 100644 --- a/activerecord/test/models/bulb.rb +++ b/activerecord/test/models/bulb.rb @@ -50,3 +50,9 @@ class FailedBulb < Bulb throw(:abort) end end + +class TrickyBulb < Bulb + after_create do |record| + record.car.bulbs.to_a + end +end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 5a73ce9b28..d946fc1bbd 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,12 @@ +* `ActiveSupport::Cache::Store#namespaced_key`, + `ActiveSupport::Cache::MemCachedStore#escape_key`, and + `ActiveSupport::Cache::FileStore#key_file_path` + are deprecated and replaced with `normalize_key` that now calls `super`. + + `ActiveSupport::Cache::LocaleCache#set_cache_value` is deprecated and replaced with `write_cache_value`. + + *Michael Grosser* + * Implements an evented file system monitor to asynchronously detect changes in the application source code, routes, locales, etc. diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 402a35e198..5011014e96 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -8,6 +8,7 @@ require 'active_support/core_ext/numeric/bytes' require 'active_support/core_ext/numeric/time' require 'active_support/core_ext/object/to_param' require 'active_support/core_ext/string/inflections' +require 'active_support/core_ext/string/strip' module ActiveSupport # See ActiveSupport::Cache::Store for documentation. @@ -536,7 +537,14 @@ module ActiveSupport key = "#{prefix}:#{key}" if prefix key end - alias namespaced_key normalize_key + + def namespaced_key(*args) + ActiveSupport::Deprecation.warn(<<-MESSAGE.strip_heredoc) + `namespaced_key` is deprecated and will be removed from Rails 5.1. + Please use `normalize_key` which will return a fully resolved key. + MESSAGE + normalize_key(*args) + end def instrument(operation, key, options = nil) log { "Cache #{operation}: #{normalize_key(key, options)}#{options.blank? ? "" : " (#{options.inspect})"}" } diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 9488b95214..dff2443bc8 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -137,6 +137,14 @@ module ActiveSupport File.join(cache_path, DIR_FORMATTER % dir_1, DIR_FORMATTER % dir_2, *fname_paths) end + def key_file_path(key) + ActiveSupport::Deprecation.warn(<<-MESSAGE.strip_heredoc) + `key_file_path` is deprecated and will be removed from Rails 5.1. + Please use `normalize_key` which will return a fully resolved key or nothing. + MESSAGE + key + end + # Translate a file path into a key. def file_path_key(path) fname = path[cache_path.to_s.size..-1].split(File::SEPARATOR, 4).last diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index 5cf5318030..36f1ba2713 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -183,6 +183,14 @@ module ActiveSupport key end + def escape_key(key) + ActiveSupport::Deprecation.warn(<<-MESSAGE.strip_heredoc) + `escape_key` is deprecated and will be removed from Rails 5.1. + Please use `normalize_key` which will return a fully resolved key or nothing. + MESSAGE + key + end + def deserialize_entry(raw_value) if raw_value entry = Marshal.load(raw_value) rescue raw_value diff --git a/activesupport/lib/active_support/cache/strategy/local_cache.rb b/activesupport/lib/active_support/cache/strategy/local_cache.rb index 46d5c6879d..df38dbcf11 100644 --- a/activesupport/lib/active_support/cache/strategy/local_cache.rb +++ b/activesupport/lib/active_support/cache/strategy/local_cache.rb @@ -93,14 +93,14 @@ module ActiveSupport def increment(name, amount = 1, options = nil) # :nodoc: return super unless local_cache value = bypass_local_cache{super} - set_cache_value(name, value, options) + write_cache_value(name, value, options) value end def decrement(name, amount = 1, options = nil) # :nodoc: return super unless local_cache value = bypass_local_cache{super} - set_cache_value(name, value, options) + write_cache_value(name, value, options) value end @@ -123,7 +123,15 @@ module ActiveSupport super end - def set_cache_value(name, value, options) # :nodoc: + def set_cache_value(value, name, amount, options) # :nodoc: + ActiveSupport::Deprecation.warn(<<-MESSAGE.strip_heredoc) + `set_cache_value` is deprecated and will be removed from Rails 5.1. + Please use `write_cache_value` + MESSAGE + write_cache_value name, value, options + end + + def write_cache_value(name, value, options) # :nodoc: name = normalize_key(name, options) cache = local_cache cache.mute do diff --git a/activesupport/lib/active_support/core_ext/class/subclasses.rb b/activesupport/lib/active_support/core_ext/class/subclasses.rb index 3c4bfc5f1e..b0f9a8be34 100644 --- a/activesupport/lib/active_support/core_ext/class/subclasses.rb +++ b/activesupport/lib/active_support/core_ext/class/subclasses.rb @@ -3,7 +3,8 @@ require 'active_support/core_ext/module/reachable' class Class begin - ObjectSpace.each_object(Class.new) {} + # Test if this Ruby supports each_object against singleton_class + ObjectSpace.each_object(Numeric.singleton_class) {} def descendants # :nodoc: descendants = [] @@ -12,7 +13,7 @@ class Class end descendants end - rescue StandardError # JRuby + rescue StandardError # JRuby 9.0.4.0 and earlier def descendants # :nodoc: descendants = [] ObjectSpace.each_object(Class) do |k| diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index fc7531d088..8a74ad4d66 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -21,7 +21,7 @@ module Enumerable if block_given? map(&block).sum(identity) else - inject { |sum, element| sum + element } || identity + inject(:+) || identity end end diff --git a/activesupport/lib/active_support/file_evented_update_checker.rb b/activesupport/lib/active_support/file_evented_update_checker.rb index 638458c6ce..bb0f26f874 100644 --- a/activesupport/lib/active_support/file_evented_update_checker.rb +++ b/activesupport/lib/active_support/file_evented_update_checker.rb @@ -1,6 +1,7 @@ require 'listen' require 'set' require 'pathname' +require 'concurrent/atomic/atomic_boolean' module ActiveSupport class FileEventedUpdateChecker #:nodoc: all @@ -14,7 +15,7 @@ module ActiveSupport end @block = block - @updated = false + @updated = Concurrent::AtomicBoolean.new(false) @lcsp = @ph.longest_common_subpath(@dirs.keys) if (dtw = directories_to_watch).any? @@ -23,13 +24,12 @@ module ActiveSupport end def updated? - @updated + @updated.true? end def execute + @updated.make_false @block.call - ensure - @updated = false end def execute_if_updated @@ -43,7 +43,7 @@ module ActiveSupport def changed(modified, added, removed) unless updated? - @updated = (modified + added + removed).any? { |f| watching?(f) } + @updated.make_true if (modified + added + removed).any? { |f| watching?(f) } end end diff --git a/activesupport/lib/active_support/per_thread_registry.rb b/activesupport/lib/active_support/per_thread_registry.rb index 506dd950cb..a909a65bb6 100644 --- a/activesupport/lib/active_support/per_thread_registry.rb +++ b/activesupport/lib/active_support/per_thread_registry.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/module/delegation' + module ActiveSupport # This module is used to encapsulate access to thread local variables. # diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index 6404f65612..7ca3592520 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -86,7 +86,8 @@ module ActiveSupport "Paris" => "Europe/Paris", "Amsterdam" => "Europe/Amsterdam", "Berlin" => "Europe/Berlin", - "Bern" => "Europe/Berlin", + "Bern" => "Europe/Zurich", + "Zurich" => "Europe/Zurich", "Rome" => "Europe/Rome", "Stockholm" => "Europe/Stockholm", "Vienna" => "Europe/Vienna", diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 3f9682231f..7bef73136c 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -518,6 +518,12 @@ module CacheStoreBehavior ensure ActiveSupport::Notifications.unsubscribe "cache_read.active_support" end + + def test_can_call_deprecated_namesaced_key + assert_deprecated "`namespaced_key` is deprecated" do + @cache.send(:namespaced_key, 111, {}) + end + end end # https://rails.lighthouseapp.com/projects/8994/tickets/6225-memcachestore-cant-deal-with-umlauts-and-special-characters @@ -693,6 +699,15 @@ module LocalCacheBehavior app = @cache.middleware.new(app) app.call({}) end + + def test_can_call_deprecated_set_cache_value + @cache.with_local_cache do + assert_deprecated "`set_cache_value` is deprecated" do + @cache.send(:set_cache_value, 1, 'foo', :ignored, {}) + end + assert_equal 1, @cache.read('foo') + end + end end module AutoloadingCacheBehavior @@ -858,6 +873,12 @@ class FileStoreTest < ActiveSupport::TestCase @cache.write(1, nil) assert_equal false, @cache.write(1, "aaaaaaaaaa", unless_exist: true) end + + def test_can_call_deprecated_key_file_path + assert_deprecated "`key_file_path` is deprecated" do + assert_equal 111, @cache.send(:key_file_path, 111) + end + end end class MemoryStoreTest < ActiveSupport::TestCase @@ -1032,6 +1053,12 @@ class MemCacheStoreTest < ActiveSupport::TestCase value << 'bingo' assert_not_equal value, @cache.read('foo') end + + def test_can_call_deprecated_escape_key + assert_deprecated "`escape_key` is deprecated" do + assert_equal 111, @cache.send(:escape_key, 111) + end + end end class NullStoreTest < ActiveSupport::TestCase diff --git a/activesupport/test/file_evented_update_checker_test.rb b/activesupport/test/file_evented_update_checker_test.rb index 071449d399..ec3a7e28f3 100644 --- a/activesupport/test/file_evented_update_checker_test.rb +++ b/activesupport/test/file_evented_update_checker_test.rb @@ -5,6 +5,11 @@ require 'file_update_checker_shared_tests' class FileEventedUpdateCheckerTest < ActiveSupport::TestCase include FileUpdateCheckerSharedTests + def setup + skip if ENV['LISTEN'] == '0' + super + end + def new_checker(files = [], dirs = {}, &block) ActiveSupport::FileEventedUpdateChecker.new(files, dirs, &block).tap do wait diff --git a/activesupport/test/multibyte_conformance_test.rb b/activesupport/test/multibyte_conformance_test.rb index 2a885e32bf..d493a48fe4 100644 --- a/activesupport/test/multibyte_conformance_test.rb +++ b/activesupport/test/multibyte_conformance_test.rb @@ -104,11 +104,8 @@ class MultibyteConformanceTest < ActiveSupport::TestCase protected def each_line_of_norm_tests(&block) - lines = 0 - max_test_lines = 0 # Don't limit below 38, because that's the header of the testfile File.open(File.join(CACHE_DIR, UNIDATA_FILE), 'r') do | f | - until f.eof? || (max_test_lines > 38 and lines > max_test_lines) - lines += 1 + until f.eof? line = f.gets.chomp! next if (line.empty? || line =~ /^\#/) diff --git a/ci/travis.rb b/ci/travis.rb index 52fef05fbf..658d66c6b4 100755 --- a/ci/travis.rb +++ b/ci/travis.rb @@ -72,6 +72,10 @@ class Build key.join(':') end + def activesupport? + gem == 'activesupport' + end + def activerecord? gem == 'activerecord' end @@ -101,11 +105,22 @@ class Build tasks.each do |task| cmd = "bundle exec rake #{task}" puts "Running command: #{cmd}" - return false unless system(cmd) + return false unless system(env, cmd) end true end + def env + if activesupport? && !isolated? + # There is a known issue with the listen tests that casuses files to be + # incorrectly GC'ed even when they are still in-use. The current is to + # only run them in isolation to avoid randomly failing our test suite. + { 'LISTEN' => '0' } + else + {} + end + end + def run_bug_report_templates Dir.glob('bug_report_templates/*.rb').all? do |file| system(Gem.ruby, '-w', file) diff --git a/guides/rails_guides/generator.rb b/guides/rails_guides/generator.rb index b7a94f144c..7618fce2c8 100644 --- a/guides/rails_guides/generator.rb +++ b/guides/rails_guides/generator.rb @@ -162,7 +162,7 @@ module RailsGuides def select_only(guides) prefixes = ENV['ONLY'].split(",").map(&:strip) guides.select do |guide| - prefixes.any? { |p| guide.start_with?(p) || guide.start_with?("kindle") } + guide.start_with?('kindle'.freeze, *prefixes) end end diff --git a/guides/rails_guides/kindle.rb b/guides/rails_guides/kindle.rb index 32926622e3..081afcb09f 100644 --- a/guides/rails_guides/kindle.rb +++ b/guides/rails_guides/kindle.rb @@ -27,7 +27,7 @@ module Kindle generate_document_metadata(mobi_outfile) - puts "Creating MOBI document with kindlegen. This make take a while." + puts "Creating MOBI document with kindlegen. This may take a while." cmd = "kindlerb . > #{File.absolute_path logfile} 2>&1" puts cmd system(cmd) diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index fe42cec158..ec31385077 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -777,7 +777,36 @@ Topic.create(title: nil).valid? # => true As you've already seen, the `:message` option lets you specify the message that will be added to the `errors` collection when validation fails. When this option is not used, Active Record will use the respective default error message -for each validation helper. +for each validation helper. The `:message` option accepts a `String` or `Proc`. + +A `String` `:message` value can optionally contain any/all of `%{value}`, +`%{attribute}`, and `%{model}` which will be dynamically replaced when +validation fails. + +A `Proc` `:message` value is given two arguments: a message key for i18n, and +a hash with `:model`, `:attribute`, and `:value` key-value pairs. + +```ruby +class Person < ActiveRecord::Base + # Hard-coded message + validates :name, presence: { message: "must be given please" } + + # Message with dynamic attribute value. %{value} will be replaced with + # the actual value of the attribute. %{attribute} and %{model} also + # available. + validates :age, numericality: { message: "%{value} seems wrong" } + + # Proc + validates :username, + uniqueness: { + # key = "activerecord.errors.models.person.attributes.username.taken" + # data = { model: "Person", attribute: "Username", value: <username> } + message: ->(key, data) do + "#{data[:value]} taken! Try again #{Time.zone.tomorrow}" + end + } +end +``` ### `:on` diff --git a/guides/source/api_app.md b/guides/source/api_app.md index feaaff166a..fb3127555e 100644 --- a/guides/source/api_app.md +++ b/guides/source/api_app.md @@ -221,7 +221,7 @@ For instance, using the `stale?` method: ```ruby def show - @post = Post.find(params[:id]) + @post = Post.find(params[:id]) if stale?(last_modified: @post.updated_at) render json: @post @@ -240,7 +240,7 @@ cross-client caching in the call to `stale?`: ```ruby def show - @post = Post.find(params[:id]) + @post = Post.find(params[:id]) if stale?(last_modified: @post.updated_at, public: true) render json: @post diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 729d1cc661..a286a7c5d2 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -35,7 +35,7 @@ In general, the work of configuring Rails means configuring the components of Ra For example, the `config/application.rb` file includes this setting: ```ruby -config.autoload_paths += %W(#{config.root}/extras) +config.time_zone = 'Central Time (US & Canada)' ``` This is a setting for Rails itself. If you want to pass settings to individual Rails components, you can do so via the same `config` object in `config/application.rb`: @@ -345,6 +345,8 @@ The schema dumper adds one additional configuration option: * `config.action_controller.allow_forgery_protection` enables or disables CSRF protection. By default this is `false` in test mode and `true` in all other modes. +* `config.action_controller.forgery_protection_origin_check` configures whether the HTTP `Origin` header should be checked against the site's origin as an additional CSRF defense. + * `config.action_controller.relative_url_root` can be used to tell Rails that you are [deploying to a subdirectory](configuring.html#deploy-to-a-subdirectory-relative-url-root). The default is `ENV['RAILS_RELATIVE_URL_ROOT']`. * `config.action_controller.permit_all_parameters` sets all the parameters for mass assignment to be permitted by default. The default value is `false`. @@ -1043,9 +1045,9 @@ Below is a comprehensive list of all the initializers found in Rails in the orde * `action_mailer.compile_config_methods` Initializes methods for the config settings specified so that they are quicker to access. -* `set_load_path` This initializer runs before `bootstrap_hook`. Adds the `vendor`, `lib`, all directories of `app` and any paths specified by `config.load_paths` to `$LOAD_PATH`. +* `set_load_path` This initializer runs before `bootstrap_hook`. Adds paths specified by `config.load_paths` and all autoload paths to `$LOAD_PATH`. -* `set_autoload_paths` This initializer runs before `bootstrap_hook`. Adds all sub-directories of `app` and paths specified by `config.autoload_paths` to `ActiveSupport::Dependencies.autoload_paths`. +* `set_autoload_paths` This initializer runs before `bootstrap_hook`. Adds all sub-directories of `app` and paths specified by `config.autoload_paths`, `config.eager_load_paths` and `config.autoload_once_paths` to `ActiveSupport::Dependencies.autoload_paths`. * `add_routing_paths` Loads (by default) all `config/routes.rb` files (in the application and railties, including engines) and sets up the routes for the application. diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index 53c3cbf80b..5885eb6e1c 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -148,6 +148,42 @@ NOTE: To help our CI servers you should add [ci skip] to your documentation comm WARNING: Docrails has a very strict policy: no code can be touched whatsoever, no matter how trivial or small the change. Only RDoc and guides can be edited via docrails. Also, CHANGELOGs should never be edited in docrails. +Translating Rails Guides +------------------------ + +We are happy to have people volunteer to translate the Rails guides into their own language. +If you want to translate the Rails guides in your own language, follows these steps: + +* Fork the project (rails/rails). +* Add a source folder for your own language, for example: *guides/source/it-IT* for Italian. +* Copy the contents of *guides/source* into your own language directory and translate them. +* Do NOT translate the HTML files, as they are automatically generated. + +To generate the guides in HTML format cd into the *guides* direcotry then run (eg. for it-IT): + +```bash +$ bundle install +$ bundle exec rake guides:generate:html GUIDES_LANGUAGE=it-IT +``` + +This will generate the guides in an *output* directory. + +NOTE: The instructions are for Rails > 4. The Redcarpet Gem doesn't work with JRuby. + +Translation efforts we know about (various versions): + +* **Italian**: [https://github.com/rixlabs/docrails](https://github.com/rixlabs/docrails) +* **Spanish**: [http://wiki.github.com/gramos/docrails](http://wiki.github.com/gramos/docrails) +* **Polish**: [http://github.com/apohllo/docrails/tree/master](http://github.com/apohllo/docrails/tree/master) +* **French** : [http://github.com/railsfrance/docrails](http://github.com/railsfrance/docrails) +* **Czech** : [https://github.com/rubyonrails-cz/docrails/tree/czech](https://github.com/rubyonrails-cz/docrails/tree/czech) +* **Turkish** : [https://github.com/ujk/docrails/tree/master](https://github.com/ujk/docrails/tree/master) +* **Korean** : [https://github.com/rorlakr/rails-guides](https://github.com/rorlakr/rails-guides) +* **Simplified Chinese** : [https://github.com/ruby-china/guides](https://github.com/ruby-china/guides) +* **Traditional Chinese** : [https://github.com/docrails-tw/guides](https://github.com/docrails-tw/guides) +* **Russian** : [https://github.com/morsbox/rusrails](https://github.com/morsbox/rusrails) +* **Japanese** : [https://github.com/yasslab/railsguides.jp](https://github.com/yasslab/railsguides.jp) + Contributing to the Rails Code ------------------------------ diff --git a/guides/source/debugging_rails_applications.md b/guides/source/debugging_rails_applications.md index a05abb61d6..5424313b33 100644 --- a/guides/source/debugging_rails_applications.md +++ b/guides/source/debugging_rails_applications.md @@ -862,8 +862,8 @@ such as Valgrind. ### Valgrind -[Valgrind](http://valgrind.org/) is a Linux-only application for detecting -C-based memory leaks and race conditions. +[Valgrind](http://valgrind.org/) is an application for detecting C-based memory +leaks and race conditions. There are Valgrind tools that can automatically detect many memory management and threading bugs, and profile your programs in detail. For example, if a C diff --git a/guides/source/plugins.md b/guides/source/plugins.md index b94c26a1ae..922bbb4f73 100644 --- a/guides/source/plugins.md +++ b/guides/source/plugins.md @@ -294,7 +294,7 @@ Getting closer... Now we will implement the code of the `acts_as_yaffle` method module Yaffle module ActsAsYaffle - extend ActiveSupport::Concern + extend ActiveSupport::Concern included do end diff --git a/guides/source/routing.md b/guides/source/routing.md index 245689932b..fc756d00b3 100644 --- a/guides/source/routing.md +++ b/guides/source/routing.md @@ -142,10 +142,10 @@ Sometimes, you have a resource that clients always look up without referencing a get 'profile', to: 'users#show' ``` -Passing a `String` to `get` will expect a `controller#action` format, while passing a `Symbol` will map directly to an action: +Passing a `String` to `get` will expect a `controller#action` format, while passing a `Symbol` will map directly to an action but you must also specify the `controller:` to use: ```ruby -get 'profile', to: :show +get 'profile', to: :show, controller: 'users' ``` This resourceful route: diff --git a/guides/source/testing.md b/guides/source/testing.md index f63ea49955..3bfbf4f7ff 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -54,10 +54,12 @@ NOTE: Your tests are run under `RAILS_ENV=test`. ### Rails meets Minitest -If you remember when you used the `rails generate scaffold` command from the [Getting Started with Rails](getting_started.html) guide. We created our first resource among other things it created test stubs in the `test` directory: +If you remember when you used the `rails generate model` command from the +[Getting Started with Rails](getting_started.html) guide. We created our first +model among other things it created test stubs in the `test` directory: ```bash -$ bin/rails generate scaffold article title:string body:text +$ bin/rails generate model article title:string body:text ... create app/models/article.rb create test/models/article_test.rb @@ -649,14 +651,31 @@ You should test for things such as: * was the correct object stored in the response template? * was the appropriate message displayed to the user in the view? -Now that we have used Rails scaffold generator for our `Article` resource, it has already created the controller code and tests. You can take look at the file `articles_controller_test.rb` in the `test/controllers` directory. +The easiest way to see functional tests in action is to generate a controller +scaffold: -The following command will generate a controller test case with a filled up -test for each of the seven default actions. +```bash +$ bin/rails generate scaffold_controller article title:string body:test +... +create app/controllers/articles_controller.rb +... +invoke test_unit +create test/controllers/articles_controller_test.rb +... +``` + +This will generate the controller code and tests for an `Article` resource. +You can take look at the file `articles_controller_test.rb` in the `test/controllers` directory. + +If you already have a controller and just want to generate the test scaffold code for +each of the seven default actions, you can use the following command: ```bash $ bin/rails generate test_unit:scaffold article +... +invoke test_unit create test/controllers/articles_controller_test.rb +... ``` Let me take you through one such test, `test_should_get_index` from the file `articles_controller_test.rb`. @@ -1253,4 +1272,3 @@ assert_equal Date.new(2004, 10, 24), user.activation_date # The change was visib Please see [`ActiveSupport::TimeHelpers` API Documentation](http://api.rubyonrails.org/classes/ActiveSupport/Testing/TimeHelpers.html) for in-depth information about the available time helpers. - diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index aa12708669..d20ec75b61 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,15 @@ +* Allow use of minitest-rails gem with Rails test runner. + + Fixes #22455. + + *Chris Kottom* + +* Add `bin/test` script to rails plugin. + + `bin/test` can use the same API as `bin/rails test`. + + *Yuji Yaginuma* + * Make `static_index` part of the `config.public_file_server` config and call it `public_file_server.index_name`. diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index 2645102619..e3d79521e7 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -208,6 +208,7 @@ module Rails "#{test}:model", "#{test}:scaffold", "#{test}:view", + "#{test}:job", "#{template}:controller", "#{template}:scaffold", "#{template}:mailer", diff --git a/railties/lib/rails/generators/erb/scaffold/templates/_form.html.erb b/railties/lib/rails/generators/erb/scaffold/templates/_form.html.erb index d99b27cb2d..519b6c8603 100644 --- a/railties/lib/rails/generators/erb/scaffold/templates/_form.html.erb +++ b/railties/lib/rails/generators/erb/scaffold/templates/_form.html.erb @@ -14,15 +14,15 @@ <% attributes.each do |attribute| -%> <div class="field"> <% if attribute.password_digest? -%> - <%%= f.label :password %><br> + <%%= f.label :password %> <%%= f.password_field :password %> </div> <div class="field"> - <%%= f.label :password_confirmation %><br> + <%%= f.label :password_confirmation %> <%%= f.password_field :password_confirmation %> <% else -%> - <%%= f.label :<%= attribute.column_name %> %><br> + <%%= f.label :<%= attribute.column_name %> %> <%%= f.<%= attribute.field_type %> :<%= attribute.column_name %> %> <% end -%> </div> diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index 4f3f59cc22..a0880d5166 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -51,7 +51,7 @@ group :development do # Loading the listen gem enables an evented file system monitor. Check # https://github.com/guard/listen#listen-adapters if on Windows or *BSD. - # gem 'listen', '~> 3.0.4' + # gem 'listen', '~> 3.0.5' end <% end -%> diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/request_forgery_protection.rb b/railties/lib/rails/generators/rails/app/templates/config/initializers/request_forgery_protection.rb new file mode 100644 index 0000000000..3eab78a885 --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/request_forgery_protection.rb @@ -0,0 +1,4 @@ +# Be sure to restart your server when you modify this file. + +# Enable origin-checking CSRF mitigation. +Rails.application.config.action_controller.forgery_protection_origin_check = true diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index eeeef430bb..776019a6a0 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -148,9 +148,8 @@ task default: :test end def bin(force = false) - return unless engine? - - directory "bin", force: force do |content| + bin_file = engine? ? 'bin/rails.tt' : 'bin/test.tt' + template bin_file, force: force do |content| "#{shebang}\n" + content end chmod "bin", 0755, verbose: false diff --git a/railties/lib/rails/generators/rails/plugin/templates/%name%.gemspec b/railties/lib/rails/generators/rails/plugin/templates/%name%.gemspec index ff242adbab..3f5682b0c1 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/%name%.gemspec +++ b/railties/lib/rails/generators/rails/plugin/templates/%name%.gemspec @@ -15,9 +15,6 @@ Gem::Specification.new do |s| s.license = "MIT" s.files = Dir["{app,config,db,lib}/**/*", "MIT-LICENSE", "Rakefile", "README.rdoc"] -<% unless options.skip_test? -%> - s.test_files = Dir["test/**/*"] -<% end -%> <%= '# ' if options.dev? || options.edge? -%>s.add_dependency "rails", "~> <%= Rails::VERSION::STRING %>" <% unless options[:skip_active_record] -%> diff --git a/railties/lib/rails/generators/rails/plugin/templates/bin/test.tt b/railties/lib/rails/generators/rails/plugin/templates/bin/test.tt new file mode 100644 index 0000000000..62b94618fd --- /dev/null +++ b/railties/lib/rails/generators/rails/plugin/templates/bin/test.tt @@ -0,0 +1,8 @@ +$: << File.expand_path(File.expand_path('../../test', __FILE__)) + +require 'bundler/setup' +require 'rails/test_unit/minitest_plugin' + +Rails::TestUnitReporter.executable = 'bin/test' + +exit Minitest.run(ARGV) diff --git a/railties/lib/rails/generators/rails/plugin/templates/rails/routes.rb b/railties/lib/rails/generators/rails/plugin/templates/rails/routes.rb index 673de44108..694510edc0 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/rails/routes.rb +++ b/railties/lib/rails/generators/rails/plugin/templates/rails/routes.rb @@ -1,4 +1,3 @@ Rails.application.routes.draw do - mount <%= camelized_modules %>::Engine => "/<%= name %>" end diff --git a/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb b/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb index f315144723..a0b00fc5c5 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb +++ b/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb @@ -14,6 +14,10 @@ require "rails/test_help" # to be shown. Minitest.backtrace_filter = Minitest::BacktraceFilter.new +<% unless engine? -%> +Rails::TestUnitReporter.executable = 'bin/test' +<% end -%> + # Load fixtures from the engine if ActiveSupport::TestCase.respond_to?(:fixture_path=) ActiveSupport::TestCase.fixture_path = File.expand_path("../fixtures", __FILE__) diff --git a/railties/lib/rails/generators/rails/scaffold/templates/scaffold.css b/railties/lib/rails/generators/rails/scaffold/templates/scaffold.css index b7818883d1..79f8b7f96f 100644 --- a/railties/lib/rails/generators/rails/scaffold/templates/scaffold.css +++ b/railties/lib/rails/generators/rails/scaffold/templates/scaffold.css @@ -78,3 +78,7 @@ div.actions { font-size: 12px; list-style: square; } + +label { + display: block; +} diff --git a/railties/lib/rails/tasks/dev.rake b/railties/lib/rails/tasks/dev.rake index e949172d3f..4593100465 100644 --- a/railties/lib/rails/tasks/dev.rake +++ b/railties/lib/rails/tasks/dev.rake @@ -1,7 +1,6 @@ namespace :dev do + desc 'Toggle development mode caching on/off' task :cache do - desc 'Toggle development mode caching on/off' - if File.exist? 'tmp/caching-dev.txt' File.delete 'tmp/caching-dev.txt' puts 'Development mode is no longer being cached.' diff --git a/railties/lib/rails/test_unit/minitest_plugin.rb b/railties/lib/rails/test_unit/minitest_plugin.rb index d1ba35a5ec..4e1fb13009 100644 --- a/railties/lib/rails/test_unit/minitest_plugin.rb +++ b/railties/lib/rails/test_unit/minitest_plugin.rb @@ -14,15 +14,16 @@ module Minitest SummaryReporter.prepend AggregatedResultSuppresion def self.plugin_rails_options(opts, options) + executable = ::Rails::TestUnitReporter.executable opts.separator "" - opts.separator "Usage: bin/rails test [options] [files or directories]" + opts.separator "Usage: #{executable} [options] [files or directories]" opts.separator "You can run a single test by appending a line number to a filename:" opts.separator "" - opts.separator " bin/rails test test/models/user_test.rb:27" + opts.separator " #{executable} test/models/user_test.rb:27" opts.separator "" opts.separator "You can run multiple files and directories at the same time:" opts.separator "" - opts.separator " bin/rails test test/controllers test/integration/login_test.rb" + opts.separator " #{executable} test/controllers test/integration/login_test.rb" opts.separator "" opts.separator "By default test failures and errors are reported inline during a run." opts.separator "" diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 715debf344..67c744b529 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -65,8 +65,11 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_match(/require.+test\/dummy\/config\/environment/, content) assert_match(/ActiveRecord::Migrator\.migrations_paths.+test\/dummy\/db\/migrate/, content) assert_match(/Minitest\.backtrace_filter = Minitest::BacktraceFilter\.new/, content) + assert_match(/Rails::TestUnitReporter\.executable = 'bin\/test'/, content) end assert_file "test/bukkits_test.rb", /assert_kind_of Module, Bukkits/ + assert_file 'bin/test' + assert_no_file 'bin/rails' end def test_generating_test_files_in_full_mode @@ -223,7 +226,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase run_generator FileUtils.cd destination_root quietly { system 'bundle install' } - assert_match(/1 runs, 1 assertions, 0 failures, 0 errors/, `bundle exec rake test 2>&1`) + assert_match(/1 runs, 1 assertions, 0 failures, 0 errors/, `bin/test 2>&1`) end def test_ensure_that_tests_works_in_full_mode @@ -315,7 +318,9 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_match(/ActiveRecord::Migrator\.migrations_paths.+\.\.\/test\/dummy\/db\/migrate/, content) assert_match(/ActiveRecord::Migrator\.migrations_paths.+<<.+\.\.\/db\/migrate/, content) assert_match(/ActionDispatch::IntegrationTest\.fixture_path = ActiveSupport::TestCase\.fixture_pat/, content) + assert_no_match(/Rails::TestUnitReporter\.executable = 'bin\/test'/, content) end + assert_no_file 'bin/test' end def test_create_mountable_application_with_mountable_option_and_hypenated_name diff --git a/railties/test/generators/plugin_test_runner_test.rb b/railties/test/generators/plugin_test_runner_test.rb new file mode 100644 index 0000000000..0887afd0db --- /dev/null +++ b/railties/test/generators/plugin_test_runner_test.rb @@ -0,0 +1,123 @@ +require 'tmpdir' +require 'abstract_unit' + +class PluginTestRunnerTest < ActiveSupport::TestCase + def setup + @destination_root = Dir.mktmpdir('bukkits') + Dir.chdir(@destination_root) { `bundle exec rails plugin new bukkits --skip-bundle` } + plugin_file 'test/dummy/db/schema.rb', '' + end + + def teardown + FileUtils.rm_rf(@destination_root) + end + + def test_run_single_file + create_test_file 'foo' + create_test_file 'bar' + assert_match "1 runs, 1 assertions, 0 failures", run_test_command("test/foo_test.rb") + end + + def test_run_multiple_files + create_test_file 'foo' + create_test_file 'bar' + assert_match "2 runs, 2 assertions, 0 failures", run_test_command("test/foo_test.rb test/bar_test.rb") + end + + def test_mix_files_and_line_filters + create_test_file 'account' + plugin_file 'test/post_test.rb', <<-RUBY + require 'test_helper' + + class PostTest < ActiveSupport::TestCase + def test_post + puts 'PostTest' + assert true + end + + def test_line_filter_does_not_run_this + assert true + end + end + RUBY + + run_test_command('test/account_test.rb test/post_test.rb:4').tap do |output| + assert_match 'AccountTest', output + assert_match 'PostTest', output + assert_match '2 runs, 2 assertions', output + end + end + + def test_multiple_line_filters + create_test_file 'account' + create_test_file 'post' + + run_test_command('test/account_test.rb:4 test/post_test.rb:4').tap do |output| + assert_match 'AccountTest', output + assert_match 'PostTest', output + end + end + + def test_line_filter_without_line_runs_all_tests + create_test_file 'account' + + run_test_command('test/account_test.rb:').tap do |output| + assert_match 'AccountTest', output + end + end + + def test_output_inline_by_default + create_test_file 'post', pass: false + + output = run_test_command('test/post_test.rb') + assert_match %r{Running:\n\nPostTest\nF\n\nwups!\n\nbin/test #{plugin_path}/test/post_test.rb:6}, output + end + + def test_only_inline_failure_output + create_test_file 'post', pass: false + + output = run_test_command('test/post_test.rb') + assert_match %r{Finished in.*\n\n1 runs, 1 assertions}, output + end + + def test_fail_fast + create_test_file 'post', pass: false + + assert_match(/Interrupt/, + capture(:stderr) { run_test_command('test/post_test.rb --fail-fast') }) + end + + def test_raise_error_when_specified_file_does_not_exist + error = capture(:stderr) { run_test_command('test/not_exists.rb') } + assert_match(%r{cannot load such file.+test/not_exists\.rb}, error) + end + + private + def plugin_path + "#{@destination_root}/bukkits" + end + + def run_test_command(arguments) + Dir.chdir(plugin_path) { `bin/test #{arguments}` } + end + + def create_test_file(name, pass: true) + plugin_file "test/#{name}_test.rb", <<-RUBY + require 'test_helper' + + class #{name.camelize}Test < ActiveSupport::TestCase + def test_truth + puts "#{name.camelize}Test" + assert #{pass}, 'wups!' + end + end + RUBY + end + + def plugin_file(path, contents, mode: 'w') + FileUtils.mkdir_p File.dirname("#{plugin_path}/#{path}") + File.open("#{plugin_path}/#{path}", mode) do |f| + f.puts contents + end + end +end |