diff options
52 files changed, 362 insertions, 252 deletions
diff --git a/.travis.yml b/.travis.yml index 5d4a9e9c67..46be91d18e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,8 +13,6 @@ before_script: - bundle update cache: bundler env: - global: - - JRUBY_OPTS='-J-Xmx1024M' matrix: - "GEM=railties" - "GEM=ap" diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a3b4f6e989..4f95a9bab9 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,11 @@ +* Catch invalid UTF-8 querystring values and respond with BadRequest + + Check querystring params for invalid UTF-8 characters, and raise an + ActionController::BadRequest error if present. Previously these strings + would typically trigger errors further down the stack. + + *Grey Baker* + * Parse RSS/ATOM responses as XML, not HTML. *Alexander Kaupanin* diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 58df5c539e..6e346fadfe 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -91,11 +91,11 @@ module ActionController #:nodoc: # and accept Rails' defaults, life will be much easier. # # If you need to use a MIME type which isn't supported by default, you can register your own handlers in - # config/initializers/mime_types.rb as follows. + # +config/initializers/mime_types.rb+ as follows. # # Mime::Type.register "image/jpg", :jpg # - # Respond to also allows you to specify a common block for different formats by using any: + # Respond to also allows you to specify a common block for different formats by using +any+: # # def index # @people = Person.all @@ -151,7 +151,7 @@ module ActionController #:nodoc: # format.html.none { render "trash" } # end # - # Variants also support common `any`/`all` block that formats have. + # Variants also support common +any+/+all+ block that formats have. # # It works for both inline: # @@ -174,7 +174,7 @@ module ActionController #:nodoc: # request.variant = [:tablet, :phone] # # which will work similarly to formats and MIME types negotiation. If there will be no - # :tablet variant declared, :phone variant will be picked: + # +:tablet+ variant declared, +:phone+ variant will be picked: # # respond_to do |format| # format.html.none diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index c6ab4dbc9a..35e3ac304f 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -338,7 +338,10 @@ module ActionDispatch # Override Rack's GET method to support indifferent access def GET fetch_header("action_dispatch.request.query_parameters") do |k| - set_header k, Request::Utils.normalize_encode_params(super || {}) + rack_query_params = super || {} + # Check for non UTF-8 parameter values, which would cause errors later + Request::Utils.check_param_encoding(rack_query_params) + set_header k, Request::Utils.normalize_encode_params(rack_query_params) end rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e raise ActionController::BadRequest.new("Invalid query parameters: #{e.message}", e) diff --git a/actionpack/lib/action_dispatch/request/utils.rb b/actionpack/lib/action_dispatch/request/utils.rb index a8151a8224..bb3df3c311 100644 --- a/actionpack/lib/action_dispatch/request/utils.rb +++ b/actionpack/lib/action_dispatch/request/utils.rb @@ -13,6 +13,21 @@ module ActionDispatch end end + def self.check_param_encoding(params) + case params + when Array + params.each { |element| check_param_encoding(element) } + when Hash + params.each_value { |value| check_param_encoding(value) } + when String + unless params.valid_encoding? + # Raise Rack::Utils::InvalidParameterError for consistency with Rack. + # ActionDispatch::Request#GET will re-raise as a BadRequest error. + raise Rack::Utils::InvalidParameterError, "Non UTF-8 value: #{params}" + end + end + end + class ParamEncoder # :nodoc: # Convert nested Hash to HashWithIndifferentAccess. # diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 339e2b7c4a..5f54ea130b 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -1,5 +1,4 @@ require 'action_dispatch/journey' -require 'forwardable' require 'active_support/concern' require 'active_support/core_ext/object/to_query' require 'active_support/core_ext/hash/slice' diff --git a/actionpack/lib/action_dispatch/testing/assertions.rb b/actionpack/lib/action_dispatch/testing/assertions.rb index 715d94d406..fae266273e 100644 --- a/actionpack/lib/action_dispatch/testing/assertions.rb +++ b/actionpack/lib/action_dispatch/testing/assertions.rb @@ -12,7 +12,7 @@ module ActionDispatch include Rails::Dom::Testing::Assertions def html_document - @html_document ||= if @response.content_type.to_s =~ /xml$/ + @html_document ||= if @response.content_type.to_s =~ /xml\z/ Nokogiri::XML::Document.parse(@response.body) else Nokogiri::HTML::Document.parse(@response.body) diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 99f6f540a8..d0a1d1285f 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -463,7 +463,7 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest def test_get_xml_rss_atom %w[ application/xml application/rss+xml application/atom+xml ].each do |mime_string| with_test_route_set do - get "/get", {}, {"HTTP_ACCEPT" => mime_string} + get "/get", headers: {"HTTP_ACCEPT" => mime_string} assert_equal 200, status assert_equal "OK", status_message assert_response 200 diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 4d1c23cbee..4224ac2a1b 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -388,8 +388,14 @@ module ActionController end def test_exception_callback_when_committed + current_threads = Thread.list + capture_log_output do |output| get :exception_with_callback, format: 'text/event-stream' + + # Wait on the execution of all threads + (Thread.list - current_threads).each(&:join) + assert_equal %(data: "500 Internal Server Error"\n\n), response.body assert_match 'An exception occurred...', output.rewind && output.read assert_stream_closed diff --git a/actionpack/test/controller/parameters/mutators_test.rb b/actionpack/test/controller/parameters/mutators_test.rb index 6c57c4caeb..744d8664be 100644 --- a/actionpack/test/controller/parameters/mutators_test.rb +++ b/actionpack/test/controller/parameters/mutators_test.rb @@ -62,15 +62,11 @@ class ParametersMutatorsTest < ActiveSupport::TestCase end test "select! retains permitted status" do - jruby_skip "https://github.com/jruby/jruby/issues/3137" - @params.permit! assert @params.select! { |k| k != "person" }.permitted? end test "select! retains unpermitted status" do - jruby_skip "https://github.com/jruby/jruby/issues/3137" - assert_not @params.select! { |k| k != "person" }.permitted? end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 82c7ebf568..256ebf6a07 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -629,13 +629,13 @@ class HttpCacheForeverTest < ActionController::TestCase def test_cache_with_public get :cache_me_forever, params: {public: true} - assert_equal "max-age=#{100.years.to_i}, public", @response.headers["Cache-Control"] + assert_equal "max-age=#{100.years}, public", @response.headers["Cache-Control"] assert_not_nil @response.etag end def test_cache_with_private get :cache_me_forever - assert_equal "max-age=#{100.years.to_i}, private", @response.headers["Cache-Control"] + assert_equal "max-age=#{100.years}, private", @response.headers["Cache-Control"] assert_not_nil @response.etag assert_response :success end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index dfedc8ae25..e9896a71f4 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -977,13 +977,17 @@ class RequestParameters < BaseRequestTest test "parameters not accessible after rack parse error of invalid UTF8 character" do request = stub_request("QUERY_STRING" => "foo%81E=1") + assert_raises(ActionController::BadRequest) { request.parameters } + end - 2.times do - assert_raises(ActionController::BadRequest) do - # rack will raise a Rack::Utils::InvalidParameterError when parsing this query string - request.parameters - end - end + test "parameters containing an invalid UTF8 character" do + request = stub_request("QUERY_STRING" => "foo=%81E") + assert_raises(ActionController::BadRequest) { request.parameters } + end + + test "parameters containing a deeply nested invalid UTF8 character" do + request = stub_request("QUERY_STRING" => "foo[bar]=%81E") + assert_raises(ActionController::BadRequest) { request.parameters } end test "parameters not accessible after rack parse error 1" do diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index cbb12a2209..15dd702161 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -93,13 +93,13 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest assert_kind_of AbstractController::ActionNotFound, env["action_dispatch.exception"] assert_equal "/404", env["PATH_INFO"] assert_equal "/not_found_original_exception", env["action_dispatch.original_path"] - [404, { "Content-Type" => "text/plain" }, ["YOU FAILED BRO"]] + [404, { "Content-Type" => "text/plain" }, ["YOU FAILED"]] end @app = ActionDispatch::ShowExceptions.new(Boomer.new, exceptions_app) get "/not_found_original_exception", headers: { 'action_dispatch.show_exceptions' => true } assert_response 404 - assert_equal "YOU FAILED BRO", body + assert_equal "YOU FAILED", body end test "returns an empty response if custom exceptions app returns X-Cascade pass" do diff --git a/actionview/test/fixtures/digestor/comments/_comment.html.erb b/actionview/test/fixtures/digestor/comments/_comment.html.erb index f172e749da..a8fa21f644 100644 --- a/actionview/test/fixtures/digestor/comments/_comment.html.erb +++ b/actionview/test/fixtures/digestor/comments/_comment.html.erb @@ -1 +1 @@ -Great story, bro! +Great story! diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c2d78bd7bf..64532e90c7 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -2,10 +2,18 @@ *Jay Hayes* +* Queries such as `Computer.joins(:monitor).group(:status).count` will now be + interpreted as `Computer.joins(:monitor).group('computers.status').count` + so that when `Computer` and `Monitor` have both `status` columns we don't + have conflicts in projection. + + *Rafael Sales* + * Add ability to default to `uuid` as primary key when generating database migrations - Set `Rails.application.config.active_record.primary_key = :uuid` - or `config.active_record.primary_key = :uuid` in config/application.rb + config.generators do |g| + g.orm :active_record, primary_key_type: :uuid + end *Jon McCartie* diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 2abc5fa9d5..d4f079e346 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -508,7 +508,7 @@ module ActiveRecord # to customize them. For example, to add a condition: # # class Blog < ActiveRecord::Base - # has_many :published_posts, -> { where published: true }, class_name: 'Post' + # has_many :published_posts, -> { where(published: true) }, class_name: 'Post' # end # # Inside the <tt>-> { ... }</tt> block you can use all of the usual Relation methods. @@ -520,7 +520,7 @@ module ActiveRecord # events that occur on the user's birthday: # # class User < ActiveRecord::Base - # has_many :birthday_events, ->(user) { where starts_on: user.birthday }, class_name: 'Event' + # has_many :birthday_events, ->(user) { where(starts_on: user.birthday) }, class_name: 'Event' # end # # Note: Joining, eager loading and preloading of these associations is not fully possible. @@ -895,7 +895,7 @@ module ActiveRecord # In this case it is usually more natural to include an association which has conditions defined on it: # # class Post < ActiveRecord::Base - # has_many :approved_comments, -> { where approved: true }, class_name: 'Comment' + # has_many :approved_comments, -> { where(approved: true) }, class_name: 'Comment' # end # # Post.includes(:approved_comments) @@ -1345,10 +1345,10 @@ module ActiveRecord # association objects. # # Option examples: - # has_many :comments, -> { order "posted_on" } - # has_many :comments, -> { includes :author } + # has_many :comments, -> { order("posted_on") } + # has_many :comments, -> { includes(:author) } # has_many :people, -> { where(deleted: false).order("name") }, class_name: "Person" - # has_many :tracks, -> { order "position" }, dependent: :destroy + # has_many :tracks, -> { order("position") }, dependent: :destroy # has_many :comments, dependent: :nullify # has_many :tags, as: :taggable # has_many :reports, -> { readonly } @@ -1476,12 +1476,12 @@ module ActiveRecord # has_one :credit_card, dependent: :destroy # destroys the associated credit card # has_one :credit_card, dependent: :nullify # updates the associated records foreign # # key value to NULL rather than destroying it - # has_one :last_comment, -> { order 'posted_on' }, class_name: "Comment" - # has_one :project_manager, -> { where role: 'project_manager' }, class_name: "Person" + # has_one :last_comment, -> { order('posted_on') }, class_name: "Comment" + # has_one :project_manager, -> { where(role: 'project_manager') }, class_name: "Person" # has_one :attachment, as: :attachable # has_one :boss, -> { readonly } # has_one :club, through: :membership - # has_one :primary_address, -> { where primary: true }, through: :addressables, source: :addressable + # has_one :primary_address, -> { where(primary: true) }, through: :addressables, source: :addressable # has_one :credit_card, required: true def has_one(name, scope = nil, options = {}) reflection = Builder::HasOne.build(self, name, scope, options) @@ -1721,7 +1721,7 @@ module ActiveRecord # query when you access the associated collection. # # Scope examples: - # has_and_belongs_to_many :projects, -> { includes :milestones, :manager } + # has_and_belongs_to_many :projects, -> { includes(:milestones, :manager) } # has_and_belongs_to_many :categories, ->(category) { # where("default_category = ?", category.name) # } @@ -1774,7 +1774,7 @@ module ActiveRecord # # Option examples: # has_and_belongs_to_many :projects - # has_and_belongs_to_many :projects, -> { includes :milestones, :manager } + # has_and_belongs_to_many :projects, -> { includes(:milestones, :manager) } # has_and_belongs_to_many :nations, class_name: "Country" # has_and_belongs_to_many :categories, join_table: "prods_cats" # has_and_belongs_to_many :categories, -> { readonly } diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 3992a240b9..7ac99bc5c6 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -54,6 +54,8 @@ module ActiveRecord autoload :BelongsTo, 'active_record/associations/preloader/belongs_to' end + NULL_RELATION = Struct.new(:values, :where_clause, :joins_values).new({}, Relation::WhereClause.empty, []) + # Eager loads the named associations for the given Active Record record(s). # # In this description, 'association name' shall refer to the name passed @@ -88,9 +90,6 @@ module ActiveRecord # [ :books, :author ] # { author: :avatar } # [ :books, { author: :avatar } ] - - NULL_RELATION = Struct.new(:values, :where_clause, :joins_values).new({}, Relation::WhereClause.empty, []) - def preload(records, associations, preload_scope = nil) records = Array.wrap(records).compact.uniq associations = Array.wrap(associations) diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 7a5a8f8ae6..29dd0643d6 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -12,7 +12,6 @@ module ActiveRecord @preload_scope = preload_scope @model = owners.first && owners.first.class @scope = nil - @owners_by_key = nil @preloaded_records = [] end @@ -56,18 +55,6 @@ module ActiveRecord raise NotImplementedError end - def owners_by_key - @owners_by_key ||= if key_conversion_required? - owners.group_by do |owner| - owner[owner_key_name].to_s - end - else - owners.group_by do |owner| - owner[owner_key_name] - end - end - end - def options reflection.options end @@ -75,32 +62,33 @@ module ActiveRecord private def associated_records_by_owner(preloader) - owners_map = owners_by_key - owner_keys = owners_map.keys.compact - - # Each record may have multiple owners, and vice-versa - records_by_owner = owners.each_with_object({}) do |owner,h| - h[owner] = [] + records = load_records + owners.each_with_object({}) do |owner, result| + result[owner] = records[convert_key(owner[owner_key_name])] || [] end + end - if owner_keys.any? - # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) - # Make several smaller queries if necessary or make one query if the adapter supports it - sliced = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) - - records = load_slices sliced - records.each do |record, owner_key| - owners_map[owner_key].each do |owner| - records_by_owner[owner] << record - end + def owner_keys + unless defined?(@owner_keys) + @owner_keys = owners.map do |owner| + owner[owner_key_name] end + @owner_keys.uniq! + @owner_keys.compact! end - - records_by_owner + @owner_keys end def key_conversion_required? - association_key_type != owner_key_type + @key_conversion_required ||= association_key_type != owner_key_type + end + + def convert_key(key) + if key_conversion_required? + key.to_s + else + key + end end def association_key_type @@ -111,17 +99,17 @@ module ActiveRecord @model.type_for_attribute(owner_key_name.to_s).type end - def load_slices(slices) - @preloaded_records = slices.flat_map { |slice| + def load_records + return {} if owner_keys.empty? + # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) + # Make several smaller queries if necessary or make one query if the adapter supports it + slices = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) + @preloaded_records = slices.flat_map do |slice| records_for(slice) - } - - @preloaded_records.map { |record| - key = record[association_key_name] - key = key.to_s if key_conversion_required? - - [record, key] - } + end + @preloaded_records.group_by do |record| + convert_key(record[association_key_name]) + end end def reflection_scope 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 bceda5abd9..e2ef56798b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -210,7 +210,7 @@ module ActiveRecord # An array of ColumnDefinition objects, representing the column changes # that have been defined. attr_accessor :indexes - attr_reader :name, :temporary, :options, :as, :foreign_keys + attr_reader :name, :temporary, :options, :as, :foreign_keys, :native def initialize(types, name, temporary, options, as = nil) @columns_hash = {} @@ -238,86 +238,19 @@ module ActiveRecord end # Instantiates a new column for the table. - # The +type+ parameter is normally one of the migrations native types, - # which is one of the following: - # <tt>:primary_key</tt>, <tt>:string</tt>, <tt>:text</tt>, - # <tt>:integer</tt>, <tt>:bigint</tt>, <tt>:float</tt>, <tt>:decimal</tt>, - # <tt>:datetime</tt>, <tt>:time</tt>, <tt>:date</tt>, - # <tt>:binary</tt>, <tt>:boolean</tt>. - # - # You may use a type not in this list as long as it is supported by your - # database (for example, "polygon" in MySQL), but this will not be database - # agnostic and should usually be avoided. - # - # Available options are (none of these exists by default): - # * <tt>:limit</tt> - - # Requests a maximum column length. This is number of characters for a <tt>:string</tt> column - # and number of bytes for <tt>:text</tt>, <tt>:binary</tt> and <tt>:integer</tt> columns. - # * <tt>:default</tt> - - # The column's default value. Use nil for NULL. - # * <tt>:null</tt> - - # Allows or disallows +NULL+ values in the column. This option could - # have been named <tt>:null_allowed</tt>. - # * <tt>:precision</tt> - - # Specifies the precision for a <tt>:decimal</tt> column. - # * <tt>:scale</tt> - - # Specifies the scale for a <tt>:decimal</tt> column. + # See {connection.add_column}[rdoc-ref:ConnectionAdapters::SchemaStatements#add_column] + # for available options. + # + # Additional options are: # * <tt>:index</tt> - # Create an index for the column. Can be either <tt>true</tt> or an options hash. # - # Note: The precision is the total number of significant digits - # and the scale is the number of digits that can be stored following - # the decimal point. For example, the number 123.45 has a precision of 5 - # and a scale of 2. A decimal with a precision of 5 and a scale of 2 can - # range from -999.99 to 999.99. - # - # Please be aware of different RDBMS implementations behavior with - # <tt>:decimal</tt> columns: - # * The SQL standard says the default scale should be 0, <tt>:scale</tt> <= - # <tt>:precision</tt>, and makes no comments about the requirements of - # <tt>:precision</tt>. - # * MySQL: <tt>:precision</tt> [1..63], <tt>:scale</tt> [0..30]. - # Default is (10,0). - # * PostgreSQL: <tt>:precision</tt> [1..infinity], - # <tt>:scale</tt> [0..infinity]. No default. - # * SQLite2: Any <tt>:precision</tt> and <tt>:scale</tt> may be used. - # Internal storage as strings. No default. - # * SQLite3: No restrictions on <tt>:precision</tt> and <tt>:scale</tt>, - # but the maximum supported <tt>:precision</tt> is 16. No default. - # * Oracle: <tt>:precision</tt> [1..38], <tt>:scale</tt> [-84..127]. - # Default is (38,0). - # * DB2: <tt>:precision</tt> [1..63], <tt>:scale</tt> [0..62]. - # Default unknown. - # * SqlServer?: <tt>:precision</tt> [1..38], <tt>:scale</tt> [0..38]. - # Default (38,0). - # # This method returns <tt>self</tt>. # # == Examples - # # Assuming +td+ is an instance of TableDefinition - # td.column(:granted, :boolean) - # # granted BOOLEAN - # - # td.column(:picture, :binary, limit: 2.megabytes) - # # => picture BLOB(2097152) - # - # td.column(:sales_stage, :string, limit: 20, default: 'new', null: false) - # # => sales_stage VARCHAR(20) DEFAULT 'new' NOT NULL - # - # td.column(:bill_gates_money, :decimal, precision: 15, scale: 2) - # # => bill_gates_money DECIMAL(15,2) # - # td.column(:sensor_reading, :decimal, precision: 30, scale: 20) - # # => sensor_reading DECIMAL(30,20) - # - # # While <tt>:scale</tt> defaults to zero on most databases, it - # # probably wouldn't hurt to include it. - # td.column(:huge_integer, :decimal, precision: 30) - # # => huge_integer DECIMAL(30) - # - # # Defines a column with a database-specific type. - # td.column(:foo, 'polygon') - # # => foo polygon + # # Assuming +td+ is an instance of TableDefinition + # td.column(:granted, :boolean, index: true) # # == Short-hand examples # @@ -455,10 +388,6 @@ module ActiveRecord ColumnDefinition.new name, type end - def native - @native - end - def aliased_types(name, fallback) 'timestamp' == name ? :datetime : fallback 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 d3fbc18f97..d5f8dbc8fc 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -446,12 +446,81 @@ module ActiveRecord execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}" end - # Adds a new column to the named table. - # See TableDefinition#column for details of the options you can use. - # - # Note: Not all options will be available, generally this command should - # ignore most of them. In favor of doing a low-level call to simply - # create a column. + # Add a new +type+ column named +column_name+ to +table_name+. + # + # The +type+ parameter is normally one of the migrations native types, + # which is one of the following: + # <tt>:primary_key</tt>, <tt>:string</tt>, <tt>:text</tt>, + # <tt>:integer</tt>, <tt>:bigint</tt>, <tt>:float</tt>, <tt>:decimal</tt>, + # <tt>:datetime</tt>, <tt>:time</tt>, <tt>:date</tt>, + # <tt>:binary</tt>, <tt>:boolean</tt>. + # + # You may use a type not in this list as long as it is supported by your + # database (for example, "polygon" in MySQL), but this will not be database + # agnostic and should usually be avoided. + # + # Available options are (none of these exists by default): + # * <tt>:limit</tt> - + # Requests a maximum column length. This is number of characters for a <tt>:string</tt> column + # and number of bytes for <tt>:text</tt>, <tt>:binary</tt> and <tt>:integer</tt> columns. + # * <tt>:default</tt> - + # The column's default value. Use nil for NULL. + # * <tt>:null</tt> - + # Allows or disallows +NULL+ values in the column. This option could + # have been named <tt>:null_allowed</tt>. + # * <tt>:precision</tt> - + # Specifies the precision for a <tt>:decimal</tt> column. + # * <tt>:scale</tt> - + # Specifies the scale for a <tt>:decimal</tt> column. + # + # Note: The precision is the total number of significant digits + # and the scale is the number of digits that can be stored following + # the decimal point. For example, the number 123.45 has a precision of 5 + # and a scale of 2. A decimal with a precision of 5 and a scale of 2 can + # range from -999.99 to 999.99. + # + # Please be aware of different RDBMS implementations behavior with + # <tt>:decimal</tt> columns: + # * The SQL standard says the default scale should be 0, <tt>:scale</tt> <= + # <tt>:precision</tt>, and makes no comments about the requirements of + # <tt>:precision</tt>. + # * MySQL: <tt>:precision</tt> [1..63], <tt>:scale</tt> [0..30]. + # Default is (10,0). + # * PostgreSQL: <tt>:precision</tt> [1..infinity], + # <tt>:scale</tt> [0..infinity]. No default. + # * SQLite2: Any <tt>:precision</tt> and <tt>:scale</tt> may be used. + # Internal storage as strings. No default. + # * SQLite3: No restrictions on <tt>:precision</tt> and <tt>:scale</tt>, + # but the maximum supported <tt>:precision</tt> is 16. No default. + # * Oracle: <tt>:precision</tt> [1..38], <tt>:scale</tt> [-84..127]. + # Default is (38,0). + # * DB2: <tt>:precision</tt> [1..63], <tt>:scale</tt> [0..62]. + # Default unknown. + # * SqlServer?: <tt>:precision</tt> [1..38], <tt>:scale</tt> [0..38]. + # Default (38,0). + # + # == Examples + # + # add_column(:users, :picture, :binary, limit: 2.megabytes) + # # ALTER TABLE "users" ADD "picture" blob(2097152) + # + # add_column(:articles, :status, :string, limit: 20, default: 'draft', null: false) + # # ALTER TABLE "articles" ADD "status" varchar(20) DEFAULT 'draft' NOT NULL + # + # add_column(:answers, :bill_gates_money, :decimal, precision: 15, scale: 2) + # # ALTER TABLE "answers" ADD "bill_gates_money" decimal(15,2) + # + # add_column(:measurements, :sensor_reading, :decimal, precision: 30, scale: 20) + # # ALTER TABLE "measurements" ADD "sensor_reading" decimal(30,20) + # + # # While :scale defaults to zero on most databases, it + # # probably wouldn't hurt to include it. + # add_column(:measurements, :huge_integer, :decimal, precision: 30) + # # ALTER TABLE "measurements" ADD "huge_integer" decimal(30) + # + # # Defines a column with a database-specific type. + # add_column(:shapes, :triangle, 'polygon') + # # ALTER TABLE "shapes" ADD "triangle" polygon def add_column(table_name, column_name, type, options = {}) at = create_alter_table table_name at.add_column(column_name, type, options) 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 59137fb842..251acf1c83 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -482,16 +482,11 @@ module ActiveRecord show_variable 'collation_database' end - def tables(name = nil, database = nil, like = nil) #:nodoc: - database ||= current_database - + def tables(name = nil) # :nodoc: sql = "SELECT table_name FROM information_schema.tables " - sql << "WHERE table_schema = #{quote(database)}" - sql << " AND table_name = #{quote(like)}" if like + sql << "WHERE table_schema = #{quote(@config[:database])}" - execute_and_free(sql, 'SCHEMA') do |result| - result.collect(&:first) - end + select_values(sql, 'SCHEMA') end alias data_sources tables diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 765cdf90d2..fddb318553 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -383,7 +383,7 @@ module ActiveRecord type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } log(sql, name, binds) do - if binds.empty? || !cache_stmt + if !cache_stmt stmt = @connection.prepare(sql) else cache = @statements[sql] ||= { @@ -399,7 +399,7 @@ module ActiveRecord # place when an error occurs. To support older MySQL versions, we # need to close the statement and delete the statement from the # cache. - if binds.empty? || !cache_stmt + if !cache_stmt stmt.close else @statements.delete sql @@ -417,7 +417,7 @@ module ActiveRecord affected_rows = stmt.affected_rows stmt.free_result - stmt.close if binds.empty? + stmt.close if !cache_stmt [result_set, affected_rows] end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 5fb34dbfac..9028c1fcb9 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -311,22 +311,18 @@ module ActiveRecord # SCHEMA STATEMENTS ======================================== - def tables(name = nil, table_name = nil) #:nodoc: - sql = <<-SQL - SELECT name - FROM sqlite_master - WHERE (type = 'table' OR type = 'view') AND NOT name = 'sqlite_sequence' - SQL - sql << " AND name = #{quote_table_name(table_name)}" if table_name - - exec_query(sql, 'SCHEMA').map do |row| - row['name'] - end + def tables(name = nil) # :nodoc: + select_values("SELECT name FROM sqlite_master WHERE type IN ('table','view') AND name <> 'sqlite_sequence'", 'SCHEMA') end alias data_sources tables def table_exists?(table_name) - table_name && tables(nil, table_name).any? + return false unless table_name.present? + + sql = "SELECT name FROM sqlite_master WHERE type IN ('table','view') AND name <> 'sqlite_sequence'" + sql << " AND name = #{quote(table_name)}" + + select_values(sql, 'SCHEMA').any? end alias data_source_exists? table_exists? diff --git a/activerecord/lib/active_record/explain_subscriber.rb b/activerecord/lib/active_record/explain_subscriber.rb index 9adabd7819..90bcf5a205 100644 --- a/activerecord/lib/active_record/explain_subscriber.rb +++ b/activerecord/lib/active_record/explain_subscriber.rb @@ -14,7 +14,7 @@ module ActiveRecord end # SCHEMA queries cannot be EXPLAINed, also we do not want to run EXPLAIN on - # our own EXPLAINs now matter how loopingly beautiful that would be. + # our own EXPLAINs no matter how loopingly beautiful that would be. # # On the other hand, we want to monitor the performance of our real database # queries, not the performance of the access to the query cache. diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 8b9367acc7..f45844a9ea 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -275,15 +275,10 @@ module ActiveRecord else group_fields = group_attrs end + group_fields = arel_columns(group_fields) - group_aliases = group_fields.map { |field| - column_alias_for(field) - } - group_columns = group_aliases.zip(group_fields).map { |aliaz,field| - [aliaz, field] - } - - group = group_fields + group_aliases = group_fields.map { |field| column_alias_for(field) } + group_columns = group_aliases.zip(group_fields) if operation == 'count' && column_name == :all aggregate_alias = 'count_all' @@ -299,7 +294,7 @@ module ActiveRecord ] select_values += select_values unless having_clause.empty? - select_values.concat arel_columns(group_fields).zip(group_aliases).map { |field,aliaz| + select_values.concat group_columns.map { |aliaz, field| if field.respond_to?(:as) field.as(aliaz) else @@ -308,7 +303,7 @@ module ActiveRecord } relation = except(:group) - relation.group_values = group + relation.group_values = group_fields relation.select_values = select_values calculated_data = @klass.connection.select_all(relation, nil, relation.bound_attributes) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 55fd0e0b52..f5afc1000d 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -375,7 +375,7 @@ module ActiveRecord # # This means it can be used in association definitions: # - # has_many :comments, -> { unscope where: :trashed } + # has_many :comments, -> { unscope(where: :trashed) } # def unscope(*args) check_if_method_has_arguments!(:unscope, args) diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index 17bc066e4d..cdcb73382f 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -17,7 +17,7 @@ module ActiveRecord # # class Post < ActiveRecord::Base # def self.default_scope - # where published: true + # where(published: true) # end # end # diff --git a/activerecord/lib/rails/generators/active_record/migration.rb b/activerecord/lib/rails/generators/active_record/migration.rb index b7418cf42f..c2b2209638 100644 --- a/activerecord/lib/rails/generators/active_record/migration.rb +++ b/activerecord/lib/rails/generators/active_record/migration.rb @@ -13,6 +13,13 @@ module ActiveRecord ActiveRecord::Migration.next_migration_number(next_migration_number) end end + + private + + def primary_key_type + key_type = options[:primary_key_type] + ", id: :#{key_type}" if key_type + end end end end diff --git a/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb b/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb index 0d57de4d65..4e5872b585 100644 --- a/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb +++ b/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb @@ -5,6 +5,8 @@ module ActiveRecord class MigrationGenerator < Base # :nodoc: argument :attributes, :type => :array, :default => [], :banner => "field[:type][:index] field[:type][:index]" + class_option :primary_key_type, type: :string, desc: "The type for primary key" + def create_migration_file set_local_assigns! validate_file_name! diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb index 4a7deb3c75..fadab2a1e6 100644 --- a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb +++ b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb @@ -1,6 +1,6 @@ class <%= migration_class_name %> < ActiveRecord::Migration def change - create_table :<%= table_name %><%= id_kind %> do |t| + create_table :<%= table_name %><%= primary_key_type %> do |t| <% attributes.each do |attribute| -%> <% if attribute.password_digest? -%> t.string :password_digest<%= attribute.inject_options %> diff --git a/activerecord/lib/rails/generators/active_record/model/model_generator.rb b/activerecord/lib/rails/generators/active_record/model/model_generator.rb index 7e8d68ce69..395951ac9d 100644 --- a/activerecord/lib/rails/generators/active_record/model/model_generator.rb +++ b/activerecord/lib/rails/generators/active_record/model/model_generator.rb @@ -7,14 +7,13 @@ module ActiveRecord check_class_collision - class_option :migration, :type => :boolean - class_option :timestamps, :type => :boolean - class_option :parent, :type => :string, :desc => "The parent class for the generated model" - class_option :indexes, :type => :boolean, :default => true, :desc => "Add indexes for references and belongs_to columns" + class_option :migration, type: :boolean + class_option :timestamps, type: :boolean + class_option :parent, type: :string, desc: "The parent class for the generated model" + class_option :indexes, type: :boolean, default: true, desc: "Add indexes for references and belongs_to columns" + class_option :primary_key_type, type: :string, desc: "The type for primary key" - # creates the migration file for the model. - def create_migration_file return unless options[:migration] && options[:parent].nil? attributes.each { |a| a.attr_options.delete(:index) if a.reference? && !a.has_index? } if options[:indexes] == false diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 77d99bc116..640df31e2e 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -294,7 +294,7 @@ module ActiveRecord def test_tables_logs_name sql = <<-SQL SELECT name FROM sqlite_master - WHERE (type = 'table' OR type = 'view') AND NOT name = 'sqlite_sequence' + WHERE type IN ('table','view') AND name <> 'sqlite_sequence' SQL assert_logged [[sql.squish, 'SCHEMA', []]] do @conn.tables('hello') @@ -313,8 +313,7 @@ module ActiveRecord with_example_table do sql = <<-SQL SELECT name FROM sqlite_master - WHERE (type = 'table' OR type = 'view') - AND NOT name = 'sqlite_sequence' AND name = \"ex\" + WHERE type IN ('table','view') AND name <> 'sqlite_sequence' AND name = 'ex' SQL assert_logged [[sql.squish, 'SCHEMA', []]] do assert @conn.table_exists?('ex') diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index ece4dab539..57d1c8feda 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -83,10 +83,10 @@ class AutomaticInverseFindingTests < ActiveRecord::TestCase assert_equal rating.comment, comment, "The Rating's comment should be the original Comment" - rating.comment.body = "Brogramming is the act of programming, like a bro." + rating.comment.body = "Fennec foxes are the smallest of the foxes." assert_equal rating.comment.body, comment.body, "Changing the Comment's body on the association should change the original Comment's body" - comment.body = "Broseiden is the king of the sea of bros." + comment.body = "Kittens are adorable." assert_equal comment.body, rating.comment.body, "Changing the original Comment's body should change the Comment's body on the association" end @@ -97,10 +97,10 @@ class AutomaticInverseFindingTests < ActiveRecord::TestCase assert_equal rating.comment, comment, "The Rating's comment should be the original Comment" - rating.comment.body = "Brogramming is the act of programming, like a bro." + rating.comment.body = "Fennec foxes are the smallest of the foxes." assert_equal rating.comment.body, comment.body, "Changing the Comment's body on the association should change the original Comment's body" - comment.body = "Broseiden is the king of the sea of bros." + comment.body = "Kittens are adorable." assert_equal comment.body, rating.comment.body, "Changing the original Comment's body should change the Comment's body on the association" end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index a0e4b4971b..4a0e6f497f 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -455,6 +455,11 @@ class CalculationsTest < ActiveRecord::TestCase [1,6,2,9].each { |firm_id| assert c.keys.include?(firm_id) } end + def test_should_count_field_of_root_table_with_conflicting_group_by_column + assert_equal({ 1 => 1 }, Firm.joins(:accounts).group(:firm_id).count) + assert_equal({ 1 => 1 }, Firm.joins(:accounts).group('accounts.firm_id').count) + end + def test_count_with_no_parameters_isnt_deprecated assert_not_deprecated { Account.count } end diff --git a/activerecord/test/cases/test_case.rb b/activerecord/test/cases/test_case.rb index 6020a3c832..47e664f4e7 100644 --- a/activerecord/test/cases/test_case.rb +++ b/activerecord/test/cases/test_case.rb @@ -103,7 +103,7 @@ module ActiveRecord # ignored SQL, or better yet, use a different notification for the queries # instead examining the SQL content. oracle_ignored = [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from all_triggers/im, /^\s*select .* from all_constraints/im, /^\s*select .* from all_tab_cols/im] - mysql_ignored = [/^SHOW FULL TABLES/i, /^SHOW FULL FIELDS/, /^SHOW CREATE TABLE /i, /^SHOW VARIABLES /, /^SELECT DATABASE\(\) as db$/, /^\s*SELECT (?:column_name|table_name)\b.*\bFROM information_schema\.(?:key_column_usage|tables)\b/im] + mysql_ignored = [/^SHOW FULL TABLES/i, /^SHOW FULL FIELDS/, /^SHOW CREATE TABLE /i, /^SHOW VARIABLES /, /^\s*SELECT (?:column_name|table_name)\b.*\bFROM information_schema\.(?:key_column_usage|tables)\b/im] postgresql_ignored = [/^\s*select\b.*\bfrom\b.*pg_namespace\b/im, /^\s*select tablename\b.*from pg_tables\b/im, /^\s*select\b.*\battname\b.*\bfrom\b.*\bpg_attribute\b/im, /^SHOW search_path/i] sqlite3_ignored = [/^\s*SELECT name\b.*\bFROM sqlite_master/im, /^\s*SELECT sql\b.*\bFROM sqlite_master/im] diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index a96b8ef0f2..1dcd9fc21e 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -10,7 +10,6 @@ class Company < AbstractCompany has_one :dummy_account, :foreign_key => "firm_id", :class_name => "Account" has_many :contracts has_many :developers, :through => :contracts - has_many :accounts scope :of_first_firm, lambda { joins(:account => :firm). diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index fcbb3ea372..3705fc57fc 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,19 @@ +* Change Integer#year to return a Fixnum instead of a Float to improve + consistency. + + Integer#years returned a Float while the rest of the accompanying methods + (days, weeks, months, etc.) return a Fixnum. + + Before: + + 1.year # => 31557600.0 + + After: + + 1.year # => 31557600 + + *Konstantinos Rousis* + * Handle invalid UTF-8 strings when HTML escaping Use `ActiveSupport::Multibyte::Unicode.tidy_bytes` to handle invalid UTF-8 diff --git a/activesupport/lib/active_support/core_ext/class/attribute.rb b/activesupport/lib/active_support/core_ext/class/attribute.rb index f2b7bb3ef1..802d988af2 100644 --- a/activesupport/lib/active_support/core_ext/class/attribute.rb +++ b/activesupport/lib/active_support/core_ext/class/attribute.rb @@ -75,11 +75,15 @@ class Class instance_predicate = options.fetch(:instance_predicate, true) attrs.each do |name| + remove_possible_singleton_method(name) define_singleton_method(name) { nil } + + remove_possible_singleton_method("#{name}?") define_singleton_method("#{name}?") { !!public_send(name) } if instance_predicate ivar = "@#{name}" + remove_possible_singleton_method("#{name}=") define_singleton_method("#{name}=") do |val| singleton_class.class_eval do remove_possible_method(name) @@ -110,10 +114,15 @@ class Class self.class.public_send name end end + + remove_possible_method "#{name}?" define_method("#{name}?") { !!public_send(name) } if instance_predicate end - attr_writer name if instance_writer + if instance_writer + remove_possible_method "#{name}=" + attr_writer name + end end end end diff --git a/activesupport/lib/active_support/core_ext/integer/time.rb b/activesupport/lib/active_support/core_ext/integer/time.rb index f0b7382ef3..87185b024f 100644 --- a/activesupport/lib/active_support/core_ext/integer/time.rb +++ b/activesupport/lib/active_support/core_ext/integer/time.rb @@ -23,7 +23,7 @@ class Integer alias :month :months def years - ActiveSupport::Duration.new(self * 365.25.days, [[:years, self]]) + ActiveSupport::Duration.new(self * 365.25.days.to_i, [[:years, self]]) end alias :year :years end diff --git a/activesupport/lib/active_support/core_ext/module/anonymous.rb b/activesupport/lib/active_support/core_ext/module/anonymous.rb index 0ecc67a855..510c9a5430 100644 --- a/activesupport/lib/active_support/core_ext/module/anonymous.rb +++ b/activesupport/lib/active_support/core_ext/module/anonymous.rb @@ -7,7 +7,7 @@ class Module # m = Module.new # m.name # => nil # - # +anonymous?+ method returns true if module does not have a name: + # +anonymous?+ method returns true if module does not have a name, false otherwise: # # Module.new.anonymous? # => true # @@ -18,8 +18,10 @@ class Module # via the +module+ or +class+ keyword or by an explicit assignment: # # m = Module.new # creates an anonymous module - # M = m # => m gets a name here as a side-effect + # m.anonymous? # => true + # M = m # m gets a name here as a side-effect # m.name # => "M" + # m.anonymous? # => false def anonymous? name.nil? end diff --git a/activesupport/lib/active_support/core_ext/module/remove_method.rb b/activesupport/lib/active_support/core_ext/module/remove_method.rb index 52632d2c6b..d5ec16d68a 100644 --- a/activesupport/lib/active_support/core_ext/module/remove_method.rb +++ b/activesupport/lib/active_support/core_ext/module/remove_method.rb @@ -6,10 +6,30 @@ class Module end end + # Removes the named singleton method, if it exists. + def remove_possible_singleton_method(method) + singleton_class.instance_eval do + remove_possible_method(method) + end + end + # Replaces the existing method definition, if there is one, with the passed # block as its body. def redefine_method(method, &block) + visibility = method_visibility(method) remove_possible_method(method) define_method(method, &block) + send(visibility, method) + end + + def method_visibility(method) # :nodoc: + case + when private_method_defined?(method) + :private + when protected_method_defined?(method) + :protected + else + :public + end end end diff --git a/activesupport/test/core_ext/module/remove_method_test.rb b/activesupport/test/core_ext/module/remove_method_test.rb index 4657f0c175..0d684dc70e 100644 --- a/activesupport/test/core_ext/module/remove_method_test.rb +++ b/activesupport/test/core_ext/module/remove_method_test.rb @@ -6,24 +6,54 @@ module RemoveMethodTests def do_something return 1 end - + + def do_something_protected + return 1 + end + protected :do_something_protected + + def do_something_private + return 1 + end + private :do_something_private + + class << self + def do_something_else + return 2 + end + end end end class RemoveMethodTest < ActiveSupport::TestCase - + def test_remove_method_from_an_object RemoveMethodTests::A.class_eval{ self.remove_possible_method(:do_something) } assert !RemoveMethodTests::A.new.respond_to?(:do_something) end - + + def test_remove_singleton_method_from_an_object + RemoveMethodTests::A.class_eval{ + self.remove_possible_singleton_method(:do_something_else) + } + assert !RemoveMethodTests::A.respond_to?(:do_something_else) + end + def test_redefine_method_in_an_object RemoveMethodTests::A.class_eval{ self.redefine_method(:do_something) { return 100 } + self.redefine_method(:do_something_protected) { return 100 } + self.redefine_method(:do_something_private) { return 100 } } assert_equal 100, RemoveMethodTests::A.new.do_something + assert_equal 100, RemoveMethodTests::A.new.send(:do_something_protected) + assert_equal 100, RemoveMethodTests::A.new.send(:do_something_private) + + assert RemoveMethodTests::A.public_method_defined? :do_something + assert RemoveMethodTests::A.protected_method_defined? :do_something_protected + assert RemoveMethodTests::A.private_method_defined? :do_something_private end -end
\ No newline at end of file +end diff --git a/activesupport/test/tagged_logging_test.rb b/activesupport/test/tagged_logging_test.rb index 03a63e94e8..917fa46c96 100644 --- a/activesupport/test/tagged_logging_test.rb +++ b/activesupport/test/tagged_logging_test.rb @@ -72,11 +72,11 @@ class TaggedLoggingTest < ActiveSupport::TestCase test "keeps each tag in their own thread" do @logger.tagged("BCX") do Thread.new do - @logger.tagged("OMG") { @logger.info "Cool story bro" } + @logger.tagged("OMG") { @logger.info "Cool story" } end.join @logger.info "Funky time" end - assert_equal "[OMG] Cool story bro\n[BCX] Funky time\n", @output.string + assert_equal "[OMG] Cool story\n[BCX] Funky time\n", @output.string end test "keeps each tag in their own instance" do @@ -84,11 +84,11 @@ class TaggedLoggingTest < ActiveSupport::TestCase @other_logger = ActiveSupport::TaggedLogging.new(MyLogger.new(@other_output)) @logger.tagged("OMG") do @other_logger.tagged("BCX") do - @logger.info "Cool story bro" + @logger.info "Cool story" @other_logger.info "Funky time" end end - assert_equal "[OMG] Cool story bro\n", @output.string + assert_equal "[OMG] Cool story\n", @output.string assert_equal "[BCX] Funky time\n", @other_output.string end @@ -97,11 +97,11 @@ class TaggedLoggingTest < ActiveSupport::TestCase Thread.new do @logger.tagged("OMG") do @logger.flush - @logger.info "Cool story bro" + @logger.info "Cool story" end end.join end - assert_equal "[FLUSHED]\nCool story bro\n", @output.string + assert_equal "[FLUSHED]\nCool story\n", @output.string end test "mixed levels of tagging" do diff --git a/guides/source/api_documentation_guidelines.md b/guides/source/api_documentation_guidelines.md index 526bf768cc..73e62eb6d9 100644 --- a/guides/source/api_documentation_guidelines.md +++ b/guides/source/api_documentation_guidelines.md @@ -84,10 +84,11 @@ English Please use American English (*color*, *center*, *modularize*, etc). See [a list of American and British English spelling differences here](http://en.wikipedia.org/wiki/American_and_British_English_spelling_differences). -Comma -------- +Oxford Comma +------------ -Please use the Oxford comma (*red, white, and blue* style). See [the detail of Oxford comma](http://en.wikipedia.org/wiki/Serial_comma). +Please use the [Oxford comma](http://en.wikipedia.org/wiki/Serial_comma) +("red, white, and blue", instead of "red, white and blue"). Example Code ------------ diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index 7b8d2d3aef..41881abb62 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -662,7 +662,7 @@ generates something like this: rel="stylesheet" /> ``` -Note: with the Asset Pipeline the :cache and :concat options aren't used +NOTE: with the Asset Pipeline the `:cache` and `:concat` options aren't used anymore, delete these options from the `javascript_include_tag` and `stylesheet_link_tag`. diff --git a/railties/lib/rails/engine/configuration.rb b/railties/lib/rails/engine/configuration.rb index b4ddee3b1b..8cadbc3ddd 100644 --- a/railties/lib/rails/engine/configuration.rb +++ b/railties/lib/rails/engine/configuration.rb @@ -26,7 +26,7 @@ module Rails # # config.generators.colorize_logging = false # - def generators #:nodoc: + def generators @generators ||= Rails::Configuration::Generators.new yield(@generators) if block_given? @generators diff --git a/railties/lib/rails/generators/migration.rb b/railties/lib/rails/generators/migration.rb index a755471a4b..87f2e1d42b 100644 --- a/railties/lib/rails/generators/migration.rb +++ b/railties/lib/rails/generators/migration.rb @@ -30,11 +30,6 @@ module Rails end end - def id_kind - kind = Rails.application.config.active_record.primary_key rescue nil - ", id: :#{kind}" if kind - end - def create_migration(destination, data, config = {}, &block) action Rails::Generators::Actions::CreateMigration.new(self, destination, block || data.to_s, config) end diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index c65b8b84be..eeeef430bb 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -117,7 +117,7 @@ task default: :test remove_file "Gemfile" remove_file "lib/tasks" remove_file "public/robots.txt" - remove_file "README" + remove_file "README.md" remove_file "test" remove_file "vendor" end diff --git a/railties/test/application/middleware/exceptions_test.rb b/railties/test/application/middleware/exceptions_test.rb index 4906f9a1e8..7b4babb13b 100644 --- a/railties/test/application/middleware/exceptions_test.rb +++ b/railties/test/application/middleware/exceptions_test.rb @@ -48,7 +48,7 @@ module ApplicationTests test "uses custom exceptions app" do add_to_config <<-RUBY config.exceptions_app = lambda do |env| - [404, { "Content-Type" => "text/plain" }, ["YOU FAILED BRO"]] + [404, { "Content-Type" => "text/plain" }, ["YOU FAILED"]] end RUBY @@ -56,7 +56,7 @@ module ApplicationTests get "/foo" assert_equal 404, last_response.status - assert_equal "YOU FAILED BRO", last_response.body + assert_equal "YOU FAILED", last_response.body end test "url generation error when action_dispatch.show_exceptions is set raises an exception" do @@ -67,7 +67,7 @@ module ApplicationTests end end RUBY - + app.config.action_dispatch.show_exceptions = true get '/foo' diff --git a/railties/test/generators/migration_generator_test.rb b/railties/test/generators/migration_generator_test.rb index a7f807b747..199743a396 100644 --- a/railties/test/generators/migration_generator_test.rb +++ b/railties/test/generators/migration_generator_test.rb @@ -222,16 +222,12 @@ class MigrationGeneratorTest < Rails::Generators::TestCase end def test_add_uuid_to_create_table_migration - previous_value = Rails.application.config.active_record.primary_key - Rails.application.config.active_record.primary_key = :uuid - run_generator ["create_books"] + run_generator ["create_books", "--primary_key_type=uuid"] assert_migration "db/migrate/create_books.rb" do |content| assert_method :change, content do |change| assert_match(/create_table :books, id: :uuid/, change) end end - - Rails.application.config.active_record.primary_key = previous_value end def test_should_create_empty_migrations_if_name_not_start_with_add_or_remove_or_create diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index abd3ff50a4..64b9a480f3 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -374,6 +374,15 @@ class ModelGeneratorTest < Rails::Generators::TestCase end end + def test_add_uuid_to_create_table_migration + run_generator ["account", "--primary_key_type=uuid"] + assert_migration "db/migrate/create_accounts.rb" do |content| + assert_method :change, content do |change| + assert_match(/create_table :accounts, id: :uuid/, change) + end + end + end + def test_required_belongs_to_adds_required_association run_generator ["account", "supplier:references{required}"] diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index a19a29056d..715debf344 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -440,6 +440,19 @@ class PluginGeneratorTest < Rails::Generators::TestCase end end + def test_unnecessary_files_are_not_generated_in_dummy_application + run_generator + assert_no_file 'test/dummy/.gitignore' + assert_no_file 'test/dummy/db/seeds.rb' + assert_no_file 'test/dummy/Gemfile' + assert_no_file 'test/dummy/public/robots.txt' + assert_no_file 'test/dummy/README.md' + assert_no_directory 'test/dummy/lib/tasks' + assert_no_directory 'test/dummy/doc' + assert_no_directory 'test/dummy/test' + assert_no_directory 'test/dummy/vendor' + end + def test_skipping_test_files run_generator [destination_root, "--skip-test"] assert_no_file "test" |