diff options
Diffstat (limited to 'activerecord')
33 files changed, 350 insertions, 224 deletions
diff --git a/activerecord/activerecord.gemspec b/activerecord/activerecord.gemspec index 1387d6e288..b8a980b1fb 100644 --- a/activerecord/activerecord.gemspec +++ b/activerecord/activerecord.gemspec @@ -23,6 +23,6 @@ Gem::Specification.new do |s| s.add_dependency('activesupport', version) s.add_dependency('activemodel', version) - s.add_dependency('arel', '~> 1.0.1') + s.add_dependency('arel', '~> 2.0.0') s.add_dependency('tzinfo', '~> 0.3.23') end diff --git a/activerecord/examples/performance.rb b/activerecord/examples/performance.rb index ccd60c6c69..c4ce361b5d 100644 --- a/activerecord/examples/performance.rb +++ b/activerecord/examples/performance.rb @@ -88,7 +88,7 @@ else ) end - mysqldump_bin = %w[mysqldump mysqldump5].select { |bin| `which #{bin}`.length > 0 } + mysqldump_bin = %w[mysqldump mysqldump5].detect { |bin| `which #{bin}`.length > 0 } `#{mysqldump_bin} -u #{conn[:username]} #{"-p#{conn[:password]}" unless conn[:password].blank?} #{conn[:database]} exhibits users > #{sqlfile}` end diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 715c868598..e6b367790b 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -321,14 +321,14 @@ module ActiveRecord klasses_and_ids[reflection.klass.name] = id_map unless id_map.empty? end - klasses_and_ids.each do |klass_name, id_map| + klasses_and_ids.each do |klass_name, _id_map| klass = klass_name.constantize table_name = klass.quoted_table_name primary_key = reflection.options[:primary_key] || klass.primary_key column_type = klass.columns.detect{|c| c.name == primary_key}.type - ids = id_map.keys.map do |id| + ids = _id_map.keys.map do |id| if column_type == :integer id.to_i elsif column_type == :float @@ -343,7 +343,7 @@ module ActiveRecord associated_records = klass.unscoped.where([conditions, ids]).apply_finder_options(options.slice(:include, :select, :joins, :order)).to_a - set_association_single_records(id_map, reflection.name, associated_records, primary_key) + set_association_single_records(_id_map, reflection.name, associated_records, primary_key) end end diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index 38454ec242..e429806b0c 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -39,7 +39,7 @@ module ActiveRecord def set_inverse_instance(record, instance) return if record.nil? || !we_can_set_the_inverse_on_this?(record) inverse_relationship = @reflection.polymorphic_inverse_of(record.class) - unless inverse_relationship.nil? + if inverse_relationship record.send(:"set_#{inverse_relationship.name}_target", instance) end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index f5291b180e..2157a0aded 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1014,8 +1014,9 @@ module ActiveRecord #:nodoc: end def all_attributes_exists?(attribute_names) - attribute_names = expand_attribute_names_for_aggregates(attribute_names) - attribute_names.all? { |name| column_methods_hash.include?(name.to_sym) } + expand_attribute_names_for_aggregates(attribute_names).all? { |name| + column_methods_hash.include?(name.to_sym) + } end protected @@ -1383,10 +1384,7 @@ MSG ensure_proper_type - if scope = self.class.send(:current_scoped_methods) - create_with = scope.scope_for_create - create_with.each { |att,value| self.send("#{att}=", value) } if create_with - end + populate_with_current_scope_attributes self.attributes = attributes unless attributes.nil? result = yield self if block_given? @@ -1415,10 +1413,7 @@ MSG @new_record = true ensure_proper_type - if scope = self.class.send(:current_scoped_methods) - create_with = scope.scope_for_create - create_with.each { |att,value| self.send("#{att}=", value) } if create_with - end + populate_with_current_scope_attributes end # Returns a String, which Action Pack uses for constructing an URL to this @@ -1807,6 +1802,13 @@ MSG return string unless string.is_a?(String) && string =~ /^---/ YAML::load(string) rescue string end + + def populate_with_current_scope_attributes + if scope = self.class.send(:current_scoped_methods) + create_with = scope.scope_for_create + create_with.each { |att,value| self.respond_to?(:"#{att}=") && self.send("#{att}=", value) } if create_with + end + end end Base.class_eval do diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 25432e9985..646a78622c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -35,7 +35,7 @@ module ActiveRecord undef_method :select_rows # Executes the SQL statement in the context of this connection. - def execute(sql, name = nil, skip_logging = false) + def execute(sql, name = nil) end undef_method :execute 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 84fc4c03f9..6480aeb171 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -318,21 +318,13 @@ module ActiveRecord @base = base end - #Handles non supported datatypes - e.g. XML - def method_missing(symbol, *args) - if symbol.to_s == 'xml' - xml_column_fallback(args) - else - super - end - end + def xml(*args) + raise NotImplementedError unless %w{ + sqlite mysql mysql2 + }.include? @base.adapter_name.downcase - def xml_column_fallback(*args) - case @base.adapter_name.downcase - when 'sqlite', 'mysql' - options = args.extract_options! - column(args[0], :text, options) - end + options = args.extract_options! + column(args[0], :text, options) end # Appends a primary key definition to the table definition. 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 310423bb20..4e770c37da 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -151,10 +151,10 @@ module ActiveRecord # # See also TableDefinition#column for details on how to create columns. def create_table(table_name, options = {}) - table_definition = TableDefinition.new(self) - table_definition.primary_key(options[:primary_key] || Base.get_primary_key(table_name.to_s.singularize)) unless options[:id] == false + td = table_definition + td.primary_key(options[:primary_key] || Base.get_primary_key(table_name.to_s.singularize)) unless options[:id] == false - yield table_definition if block_given? + yield td if block_given? if options[:force] && table_exists?(table_name) drop_table(table_name, options) @@ -162,7 +162,7 @@ module ActiveRecord create_sql = "CREATE#{' TEMPORARY' if options[:temporary]} TABLE " create_sql << "#{quote_table_name(table_name)} (" - create_sql << table_definition.to_sql + create_sql << td.to_sql create_sql << ") #{options[:options]}" execute create_sql end @@ -327,14 +327,12 @@ module ActiveRecord # # Note: SQLite doesn't support index length def add_index(table_name, column_name, options = {}) - options[:name] = options[:name].to_s if options.key?(:name) - column_names = Array.wrap(column_name) index_name = index_name(table_name, :column => column_names) if Hash === options # legacy support, since this param was a string index_type = options[:unique] ? "UNIQUE" : "" - index_name = options[:name] || index_name + index_name = options[:name].to_s if options.key?(:name) else index_type = options end @@ -404,6 +402,7 @@ module ActiveRecord # as there's no way to determine the correct answer in that case. def index_name_exists?(table_name, index_name, default) return default unless respond_to?(:indexes) + index_name = index_name.to_s indexes(table_name).detect { |i| i.name == index_name } end @@ -535,6 +534,11 @@ module ActiveRecord def options_include_default?(options) options.include?(:default) && !(options[:null] == false && options[:default].nil?) end + + private + def table_definition + TableDefinition.new(self) + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0a2bacdb84..194842a9a0 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -1,20 +1,19 @@ require 'active_record/connection_adapters/abstract_adapter' require 'active_support/core_ext/kernel/requires' require 'active_support/core_ext/object/blank' +require 'pg' module ActiveRecord class Base # Establishes a connection to the database that's used by all Active Record objects def self.postgresql_connection(config) # :nodoc: - require 'pg' - config = config.symbolize_keys host = config[:host] port = config[:port] || 5432 username = config[:username].to_s if config[:username] password = config[:password].to_s if config[:password] - if config.has_key?(:database) + if config.key?(:database) database = config[:database] else raise ArgumentError, "No database specified. Missing argument: database." @@ -27,12 +26,6 @@ module ActiveRecord end module ConnectionAdapters - class TableDefinition - def xml(*args) - options = args.extract_options! - column(args[0], 'xml', options) - end - end # PostgreSQL-specific extensions to column definitions in a table. class PostgreSQLColumn < Column #:nodoc: # Instantiates a new PostgreSQL column definition in a table. @@ -170,9 +163,7 @@ module ActiveRecord end end end - end - module ConnectionAdapters # The PostgreSQL adapter works both with the native C (http://ruby.scripting.ca/postgres/) and the pure # Ruby (available both as gem and from http://rubyforge.org/frs/?group_id=234&release_id=1944) drivers. # @@ -192,10 +183,17 @@ module ActiveRecord # * <tt>:allow_concurrency</tt> - If true, use async query methods so Ruby threads don't deadlock; # otherwise, use blocking query methods. class PostgreSQLAdapter < AbstractAdapter - ADAPTER_NAME = 'PostgreSQL'.freeze + class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition + def xml(*args) + options = args.extract_options! + column(args[0], 'xml', options) + end + end + + ADAPTER_NAME = 'PostgreSQL' NATIVE_DATABASE_TYPES = { - :primary_key => "serial primary key".freeze, + :primary_key => "serial primary key", :string => { :name => "character varying", :limit => 255 }, :text => { :name => "text" }, :integer => { :name => "integer" }, @@ -317,19 +315,22 @@ module ActiveRecord def quote(value, column = nil) #:nodoc: return super unless column - if value.kind_of?(String) && column.type == :binary - "'#{escape_bytea(value)}'" - elsif value.kind_of?(String) && column.sql_type == 'xml' - "xml '#{quote_string(value)}'" - elsif value.kind_of?(Numeric) && column.sql_type == 'money' + case value + when Numeric + return super unless column.sql_type == 'money' # Not truly string input, so doesn't require (or allow) escape string syntax. "'#{value}'" - elsif value.kind_of?(String) && column.sql_type =~ /^bit/ - case value - when /^[01]*$/ - "B'#{value}'" # Bit-string notation - when /^[0-9A-F]*$/i - "X'#{value}'" # Hexadecimal notation + when String + case column.sql_type + when 'bytea' then "'#{escape_bytea(value)}'" + when 'xml' then "xml '#{quote_string(value)}'" + when /^bit/ + case value + when /^[01]*$/ then "B'#{value}'" # Bit-string notation + when /^[0-9A-F]*$/i then "X'#{value}'" # Hexadecimal notation + end + else + super end else super @@ -1024,6 +1025,10 @@ module ActiveRecord [match_data[1], (rest.length > 0 ? rest : nil)] end end + + def table_definition + TableDefinition.new(self) + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index e5e92f2b1c..5ca1923d89 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -1,4 +1,5 @@ require 'active_record/connection_adapters/sqlite_adapter' +require 'sqlite3' module ActiveRecord class Base @@ -20,16 +21,12 @@ module ActiveRecord raise ArgumentError, 'adapter name should be "sqlite3"' end - unless self.class.const_defined?(:SQLite3) - require_library_or_gem(config[:adapter]) - end - db = SQLite3::Database.new( config[:database], :results_as_hash => true ) - db.busy_timeout(config[:timeout]) unless config[:timeout].nil? + db.busy_timeout(config[:timeout]) if config[:timeout] ConnectionAdapters::SQLite3Adapter.new(db, logger, config) end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 0571e0cd14..c0cc7ba20d 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -360,8 +360,8 @@ module ActiveRecord end def copy_table_contents(from, to, columns, rename = {}) #:nodoc: - column_mappings = Hash[*columns.map {|name| [name, name]}.flatten] - rename.inject(column_mappings) {|map, a| map[a.last] = a.first; map} + column_mappings = Hash[columns.map {|name| [name, name]}] + rename.each { |a| column_mappings[a.last] = a.first } from_columns = columns(from).collect {|col| col.name} columns = columns.find_all{|col| from_columns.include?(column_mappings[col])} quoted_columns = columns.map { |col| quote_column_name(col) } * ',' diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index 533bc331ae..b309df9b1b 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -6,40 +6,43 @@ module ActiveRecord # class DynamicFinderMatch def self.match(method) - df_match = self.new(method) - df_match.finder ? df_match : nil - end - - def initialize(method) - @finder = :first - @bang = false - @instantiator = nil + finder = :first + bang = false + instantiator = nil case method.to_s - when /^find_(all_by|last_by|by)_([_a-zA-Z]\w*)$/ - @finder = :last if $1 == 'last_by' - @finder = :all if $1 == 'all_by' + when /^find_(all_|last_)?by_([_a-zA-Z]\w*)$/ + finder = :last if $1 == 'last_' + finder = :all if $1 == 'all_' names = $2 when /^find_by_([_a-zA-Z]\w*)\!$/ - @bang = true + bang = true names = $1 when /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ - @instantiator = $1 == 'initialize' ? :new : :create + instantiator = $1 == 'initialize' ? :new : :create names = $2 else - @finder = nil + return nil end - @attribute_names = names && names.split('_and_') + + new(finder, instantiator, bang, names.split('_and_')) + end + + def initialize(finder, instantiator, bang, attribute_names) + @finder = finder + @instantiator = instantiator + @bang = bang + @attribute_names = attribute_names end attr_reader :finder, :attribute_names, :instantiator def finder? - !@finder.nil? && @instantiator.nil? + @finder && !@instantiator end def instantiator? - @finder == :first && !@instantiator.nil? + @finder == :first && @instantiator end def creator? diff --git a/activerecord/lib/active_record/dynamic_scope_match.rb b/activerecord/lib/active_record/dynamic_scope_match.rb index 61c3ea0e7f..c832e927d6 100644 --- a/activerecord/lib/active_record/dynamic_scope_match.rb +++ b/activerecord/lib/active_record/dynamic_scope_match.rb @@ -8,25 +8,16 @@ module ActiveRecord # scope except that it's dynamic. class DynamicScopeMatch def self.match(method) - ds_match = self.new(method) - ds_match.scope ? ds_match : nil + return unless method.to_s =~ /^scoped_by_([_a-zA-Z]\w*)$/ + new(true, $1 && $1.split('_and_')) end - def initialize(method) - @scope = true - case method.to_s - when /^scoped_by_([_a-zA-Z]\w*)$/ - names = $1 - else - @scope = nil - end - @attribute_names = names && names.split('_and_') + def initialize(scope, attribute_names) + @scope = scope + @attribute_names = attribute_names end attr_reader :scope, :attribute_names - - def scope? - !@scope.nil? - end + alias :scope? :scope end end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 826031a3e3..6fb723f2f5 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -704,11 +704,9 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) end def read_yaml_fixture_files - yaml_string = "" - Dir["#{@fixture_path}/**/*.yml"].select { |f| test(?f, f) }.each do |subfixture_path| - yaml_string << IO.read(subfixture_path) - end - yaml_string << IO.read(yaml_file_path) + yaml_string = (Dir["#{@fixture_path}/**/*.yml"].select { |f| + File.file?(f) + } + [yaml_file_path]).map { |file_path| IO.read(file_path) }.join if yaml = parse_yaml_string(yaml_string) # If the file is an ordered map, extract its children. diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 7372ab3278..bdd940f3ee 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -377,7 +377,12 @@ module ActiveRecord end if attributes_collection.is_a? Hash - attributes_collection = attributes_collection.sort_by { |index, _| index.to_i }.map { |_, attributes| attributes } + keys = attributes_collection.keys + attributes_collection = if keys.include?('id') || keys.include?(:id) + Array.wrap(attributes_collection) + else + attributes_collection.sort_by { |i, _| i.to_i }.map { |_, attributes| attributes } + end end association = send(association_name) diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 94dda4e413..868fd6c3ff 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -13,7 +13,7 @@ module ActiveRecord class Railtie < Rails::Railtie config.active_record = ActiveSupport::OrderedOptions.new - config.generators.orm :active_record, :migration => true, + config.app_generators.orm :active_record, :migration => true, :timestamps => true config.app_middleware.insert_after "::ActionDispatch::Callbacks", diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 12bfe3c738..58c705c8b2 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -108,7 +108,7 @@ namespace :db do end end when 'postgresql' - @encoding = config[:encoding] || ENV['CHARSET'] || 'utf8' + @encoding = config['encoding'] || ENV['CHARSET'] || 'utf8' begin ActiveRecord::Base.establish_connection(config.merge('database' => 'postgres', 'schema_search_path' => 'public')) ActiveRecord::Base.connection.create_database(config['database'], config.merge('encoding' => @encoding)) @@ -390,7 +390,7 @@ namespace :db do db_string = firebird_db_string(abcs[Rails.env]) sh "isql -a #{db_string} > #{Rails.root}/db/#{Rails.env}_structure.sql" else - raise "Task not supported by '#{abcs["test"]["adapter"]}'" + raise "Task not supported by '#{abcs[Rails.env]["adapter"]}'" end if ActiveRecord::Base.connection.supports_migrations? diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 478f1e8ef1..04ba5b291e 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -326,7 +326,11 @@ module ActiveRecord def scope_for_create @scope_for_create ||= begin - @create_with_value || where_values_hash + if @create_with_value + @create_with_value.reverse_merge(where_values_hash) + else + where_values_hash + end end end @@ -358,15 +362,6 @@ module ActiveRecord scoping { @klass.send(method, *args, &block) } elsif arel.respond_to?(method) arel.send(method, *args, &block) - elsif match = DynamicFinderMatch.match(method) - attributes = match.attribute_names - super unless @klass.send(:all_attributes_exists?, attributes) - - if match.finder? - find_by_attributes(match, attributes, *args) - elsif match.instantiator? - find_or_instantiator_by_attributes(match, attributes, *args, &block) - end else super end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 12a2c6aec3..03862c78e4 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -191,7 +191,11 @@ module ActiveRecord end # Postgresql doesn't like ORDER BY when there are no GROUP BY - relation = except(:order).select(operation == 'count' ? column.count(distinct) : column.send(operation)) + relation = except(:order) + select_value = operation == 'count' ? column.count(distinct) : column.send(operation) + + relation.select_values = [select_value] + type_cast_calculated_value(@klass.connection.select_value(relation.to_sql), column_for(column_name), operation) end @@ -208,21 +212,22 @@ module ActiveRecord aggregate_alias = column_alias_for(operation, column_name) select_statement = if operation == 'count' && column_name == :all - "COUNT(*) AS count_all" + ["COUNT(*) AS count_all"] else - Arel::Attribute.new(@klass.unscoped.table, column_name).send(operation).as(aggregate_alias).to_sql + [Arel::Attribute.new(@klass.unscoped.table, column_name).send(operation).as(aggregate_alias)] end - select_statement << ", #{group_field} AS #{group_alias}" + select_statement << "#{group_field} AS #{group_alias}" - relation = except(:group).select(select_statement).group(group) + relation = except(:group).group(group) + relation.select_values = select_statement calculated_data = @klass.connection.select_all(relation.to_sql) if association key_ids = calculated_data.collect { |row| row[group_alias] } key_records = association.klass.base_class.find(key_ids) - key_records = key_records.inject({}) { |hsh, r| hsh.merge(r.id => r) } + key_records = Hash[key_records.map { |r| [r.id, r] }] end ActiveSupport::OrderedHash[calculated_data.map do |row| diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6a33edeb97..2e0a2effc2 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -36,7 +36,7 @@ module ActiveRecord to_a.select {|*block_args| value.call(*block_args) } else relation = clone - relation.select_values += [value] + relation.select_values += Array.wrap(value) relation end end @@ -135,14 +135,13 @@ module ActiveRecord end def reverse_order - order_clause = arel.order_clauses.join(', ') - relation = except(:order) + order_clause = arel.order_clauses - order = order_clause.blank? ? + order = order_clause.empty? ? "#{@klass.table_name}.#{@klass.primary_key} DESC" : - reverse_sql_order(order_clause) + reverse_sql_order(order_clause).join(', ') - relation.order(Arel::SqlLiteral.new(order)) + except(:order).order(Arel::SqlLiteral.new(order)) end def arel @@ -150,7 +149,7 @@ module ActiveRecord end def custom_join_sql(*joins) - arel = table + arel = table.select_manager joins.each do |join| next if join.blank? @@ -158,16 +157,13 @@ module ActiveRecord @implicit_readonly = true case join - when Hash, Array, Symbol - if array_of_strings?(join) - join_string = join.join(' ') - arel = arel.join(Arel::SqlLiteral.new(join_string)) - end + when Array + join = Arel.sql(join.join(' ')) if array_of_strings?(join) when String - arel = arel.join(Arel::SqlLiteral.new(join)) - else - arel = arel.join(join) + join = Arel.sql(join) end + + arel.join(join) end arel.joins(arel) @@ -179,13 +175,8 @@ module ActiveRecord arel = build_joins(arel, @joins_values) unless @joins_values.empty? (@where_values - ['']).uniq.each do |where| - case where - when Arel::SqlLiteral - arel = arel.where(where) - else - sql = where.is_a?(String) ? where : where.to_sql - arel = arel.where(Arel::SqlLiteral.new("(#{sql})")) - end + where = Arel.sql(where) if String === where + arel = arel.where(Arel::Nodes::Grouping.new(where)) end arel = arel.having(*@having_values.uniq.reject{|h| h.blank?}) unless @having_values.empty? @@ -260,16 +251,7 @@ module ActiveRecord def build_select(arel, selects) unless selects.empty? @implicit_readonly = false - # TODO: fix this ugly hack, we should refactor the callers to get an Arel compatible array. - # Before this change we were passing to Arel the last element only, and Arel is capable of handling an array - case select = selects.last - when Arel::Expression, Arel::SqlLiteral - arel.project(select) - when /^COUNT\(/ - arel.project(Arel::SqlLiteral.new(select)) - else - arel.project(*selects) - end + arel.project(*selects) else arel.project(Arel::SqlLiteral.new(@klass.quoted_table_name + '.*')) end @@ -283,15 +265,9 @@ module ActiveRecord end def reverse_sql_order(order_query) - order_query.split(',').each { |s| - if s.match(/\s(asc|ASC)$/) - s.gsub!(/\s(asc|ASC)$/, ' DESC') - elsif s.match(/\s(desc|DESC)$/) - s.gsub!(/\s(desc|DESC)$/, ' ASC') - else - s.concat(' DESC') - end - }.join(',') + order_query.join(', ').split(',').collect do |s| + s.gsub!(/\sasc\Z/i, ' DESC') || s.gsub!(/\sdesc\Z/i, ' ASC') || s.concat(' DESC') + end end def array_of_strings?(o) diff --git a/activerecord/lib/active_record/serialization.rb b/activerecord/lib/active_record/serialization.rb index ad3f7afd6f..398eb1534a 100644 --- a/activerecord/lib/active_record/serialization.rb +++ b/activerecord/lib/active_record/serialization.rb @@ -45,7 +45,7 @@ module ActiveRecord #:nodoc: send(association) end - unless records.nil? + if records association_options = include_has_options ? include_associations[association] : base_only_or_except opts = options.merge(association_options) yield(association, records, opts) diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 742513230e..cbaa4990f7 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -474,4 +474,9 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase Author.belongs_to :special_author_address, :dependent => :restrict end end + + def test_attributes_are_being_set_when_initialized_from_belongs_to_association_with_where_clause + new_firm = accounts(:signals37).build_firm(:name => 'Apple') + assert_equal new_firm.name, "Apple" + end end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 2bdf9d8971..c0be7dfdcc 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -848,4 +848,14 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_queries(0) { david.projects.columns; david.projects.columns } end + def test_attributes_are_being_set_when_initialized_from_habm_association_with_where_clause + new_developer = projects(:action_controller).developers.where(:name => "Marcelo").build + assert_equal new_developer.name, "Marcelo" + end + + def test_attributes_are_being_set_when_initialized_from_habm_association_with_multiple_where_clauses + new_developer = projects(:action_controller).developers.where(:name => "Marcelo").where(:salary => 90_000).build + assert_equal new_developer.name, "Marcelo" + assert_equal new_developer.salary, 90_000 + 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 efabf74e13..720b7fc386 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1255,4 +1255,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end EOF end + + def test_attributes_are_being_set_when_initialized_from_has_many_association_with_where_clause + new_comment = posts(:welcome).comments.where(:body => "Some content").build + assert_equal new_comment.body, "Some content" + end + + def test_attributes_are_being_set_when_initialized_from_has_many_association_with_multiple_where_clauses + new_comment = posts(:welcome).comments.where(:body => "Some content").where(:type => 'SpecialComment').build + assert_equal new_comment.body, "Some content" + assert_equal new_comment.type, "SpecialComment" + assert_equal new_comment.post_id, posts(:welcome).id + end end diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 45f8bd64eb..0dac633852 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -421,4 +421,18 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_raises(ActiveRecord::RecordNotFound) {company.developer_ids= ids} end + def test_build_a_model_from_hm_through_association_with_where_clause + assert_nothing_raised { books(:awdr).subscribers.where(:nick => "marklazz").build } + end + + def test_attributes_are_being_set_when_initialized_from_hm_through_association_with_where_clause + new_subscriber = books(:awdr).subscribers.where(:nick => "marklazz").build + assert_equal new_subscriber.nick, "marklazz" + end + + def test_attributes_are_being_set_when_initialized_from_hm_through_association_with_multiple_where_clauses + new_subscriber = books(:awdr).subscribers.where(:nick => "marklazz").where(:name => 'Marcelo Giorgi').build + assert_equal new_subscriber.nick, "marklazz" + assert_equal new_subscriber.name, "Marcelo Giorgi" + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index e959ed46cc..b522be3fe0 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -326,4 +326,9 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert !account.new_record? assert_equal 500, account.credit_limit end + + def test_attributes_are_being_set_when_initialized_from_has_one_association_with_where_clause + new_account = companies(:first_firm).build_account(:firm_name => 'Account') + assert_equal new_account.firm_name, "Account" + end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index d87f259f4b..16fd9a7465 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -69,6 +69,24 @@ class BasicsTest < ActiveRecord::TestCase end end + def test_use_table_engine_for_quoting_where + relation = Topic.where(Topic.arel_table[:id].eq(1)) + engine = relation.table.engine + + fakepool = Class.new(Struct.new(:spec)) { + def with_connection; yield self; end + def connection_pool; self; end + def quote_table_name(*args); raise "lol quote_table_name"; end + } + + relation.table.engine = fakepool.new(engine.connection_pool.spec) + + error = assert_raises(RuntimeError) { relation.to_a } + assert_match('lol', error.message) + ensure + relation.table.engine = engine + end + def test_preserving_time_objects assert_kind_of( Time, Topic.find(1).bonus_time, @@ -366,6 +384,10 @@ class BasicsTest < ActiveRecord::TestCase assert_equal Topic.find(1), Topic.find(2).topic end + def test_find_by_slug + assert_equal Topic.find('1-meowmeow'), Topic.find(1) + end + def test_equality_of_new_records assert_not_equal Topic.new, Topic.new end diff --git a/activerecord/test/cases/dynamic_finder_match_test.rb b/activerecord/test/cases/dynamic_finder_match_test.rb new file mode 100644 index 0000000000..e576870317 --- /dev/null +++ b/activerecord/test/cases/dynamic_finder_match_test.rb @@ -0,0 +1,98 @@ +require "cases/helper" + +module ActiveRecord + class DynamicFinderMatchTest < ActiveRecord::TestCase + def test_find_or_create_by + match = DynamicFinderMatch.match("find_or_create_by_age_and_sex_and_location") + assert_not_nil match + assert !match.finder? + assert match.instantiator? + assert_equal :first, match.finder + assert_equal :create, match.instantiator + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_or_initialize_by + match = DynamicFinderMatch.match("find_or_initialize_by_age_and_sex_and_location") + assert_not_nil match + assert !match.finder? + assert match.instantiator? + assert_equal :first, match.finder + assert_equal :new, match.instantiator + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_no_match + assert_nil DynamicFinderMatch.match("not_a_finder") + end + + def find_by_bang + match = DynamicFinderMatch.match("find_by_age_and_sex_and_location!") + assert_not_nil match + assert match.finder? + assert match.bang? + assert_equal :first, match.finder + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_by + match = DynamicFinderMatch.match("find_by_age_and_sex_and_location") + assert_not_nil match + assert match.finder? + assert_equal :first, match.finder + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_by_with_symbol + m = DynamicFinderMatch.match(:find_by_foo) + assert_equal :first, m.finder + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_all_by_with_symbol + m = DynamicFinderMatch.match(:find_all_by_foo) + assert_equal :all, m.finder + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_all_by + match = DynamicFinderMatch.match("find_all_by_age_and_sex_and_location") + assert_not_nil match + assert match.finder? + assert_equal :all, match.finder + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_last_by + m = DynamicFinderMatch.match(:find_last_by_foo) + assert_equal :last, m.finder + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_by! + m = DynamicFinderMatch.match(:find_by_foo!) + assert_equal :first, m.finder + assert m.bang?, 'should be banging' + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_or_create + m = DynamicFinderMatch.match(:find_or_create_by_foo) + assert_equal :first, m.finder + assert_equal %w{ foo }, m.attribute_names + assert_equal :create, m.instantiator + end + + def test_find_or_initialize + m = DynamicFinderMatch.match(:find_or_initialize_by_foo) + assert_equal :first, m.finder + assert_equal %w{ foo }, m.attribute_names + assert_equal :new, m.instantiator + end + + def test_garbage + assert !DynamicFinderMatch.match(:fooo), 'should be false' + assert !DynamicFinderMatch.match(:find_by), 'should be false' + end + end +end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 4f3e43d77d..26b5096255 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -11,57 +11,6 @@ require 'models/project' require 'models/developer' require 'models/customer' -class DynamicFinderMatchTest < ActiveRecord::TestCase - def test_find_no_match - assert_nil ActiveRecord::DynamicFinderMatch.match("not_a_finder") - end - - def test_find_by - match = ActiveRecord::DynamicFinderMatch.match("find_by_age_and_sex_and_location") - assert_not_nil match - assert match.finder? - assert_equal :first, match.finder - assert_equal %w(age sex location), match.attribute_names - end - - def find_by_bang - match = ActiveRecord::DynamicFinderMatch.match("find_by_age_and_sex_and_location!") - assert_not_nil match - assert match.finder? - assert match.bang? - assert_equal :first, match.finder - assert_equal %w(age sex location), match.attribute_names - end - - def test_find_all_by - match = ActiveRecord::DynamicFinderMatch.match("find_all_by_age_and_sex_and_location") - assert_not_nil match - assert match.finder? - assert_equal :all, match.finder - assert_equal %w(age sex location), match.attribute_names - end - - def test_find_or_initialize_by - match = ActiveRecord::DynamicFinderMatch.match("find_or_initialize_by_age_and_sex_and_location") - assert_not_nil match - assert !match.finder? - assert match.instantiator? - assert_equal :first, match.finder - assert_equal :new, match.instantiator - assert_equal %w(age sex location), match.attribute_names - end - - def test_find_or_create_by - match = ActiveRecord::DynamicFinderMatch.match("find_or_create_by_age_and_sex_and_location") - assert_not_nil match - assert !match.finder? - assert match.instantiator? - assert_equal :first, match.finder - assert_equal :create, match.instantiator - assert_equal %w(age sex location), match.attribute_names - end -end - class FinderTest < ActiveRecord::TestCase fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers, :categories, :categorizations diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index ffe16ffdfa..f3d3d62830 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -219,7 +219,7 @@ class MethodScopingTest < ActiveRecord::TestCase new_comment = nil VerySpecialComment.send(:with_scope, :create => { :post_id => 1 }) do - assert_equal({:post_id => 1}, VerySpecialComment.scoped.send(:scope_for_create)) + assert_equal({:post_id => 1, :type => 'VerySpecialComment' }, VerySpecialComment.scoped.send(:scope_for_create)) new_comment = VerySpecialComment.create :body => "Wonderful world" end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index bcae46c7e8..6e8ee95613 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -91,7 +91,7 @@ if ActiveRecord::Base.connection.supports_migrations? # Oracle adapter is shortening index name when just column list is given unless current_adapter?(:OracleAdapter) assert_nothing_raised { Person.connection.add_index("people", ["last_name", "first_name"]) } - assert_nothing_raised { Person.connection.remove_index("people", :name => "index_people_on_last_name_and_first_name") } + assert_nothing_raised { Person.connection.remove_index("people", :name => :index_people_on_last_name_and_first_name) } assert_nothing_raised { Person.connection.add_index("people", ["last_name", "first_name"]) } assert_nothing_raised { Person.connection.remove_index("people", "last_name_and_first_name") } end @@ -124,6 +124,13 @@ if ActiveRecord::Base.connection.supports_migrations? end end + def test_index_symbol_names + assert_nothing_raised { Person.connection.add_index :people, :primary_contact_id, :name => :symbol_index_name } + assert Person.connection.index_exists?(:people, :primary_contact_id, :name => :symbol_index_name) + assert_nothing_raised { Person.connection.remove_index :people, :name => :symbol_index_name } + assert !Person.connection.index_exists?(:people, :primary_contact_id, :name => :symbol_index_name) + end + def test_add_index_length_limit good_index_name = 'x' * Person.connection.index_name_length too_long_index_name = good_index_name + 'x' @@ -1581,13 +1588,23 @@ if ActiveRecord::Base.connection.supports_migrations? end end - if current_adapter?(:PostgreSQLAdapter) + if current_adapter?(:PostgreSQLAdapter) || current_adapter?(:SQLiteAdapter) || current_adapter?(:MysqlAdapter) || current_adapter?(:Mysql2Adapter) def test_xml_creates_xml_column + type = current_adapter?(:PostgreSQLAdapter) ? 'xml' : :text + with_new_table do |t| - t.expects(:column).with(:data, 'xml', {}) + t.expects(:column).with(:data, type, {}) t.xml :data end end + else + def test_xml_creates_xml_column + with_new_table do |t| + assert_raises(NotImplementedError) do + t.xml :data + end + end + end end protected diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 62e073ba8c..8382ca048b 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -115,7 +115,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase assert_difference('Ship.count') { pirate.save! } end - def test_reject_if_with_a_proc_which_returns_true_always + def test_reject_if_with_a_proc_which_returns_true_always_for_has_one Pirate.accepts_nested_attributes_for :ship, :reject_if => proc {|attributes| true } pirate = Pirate.new(:catchphrase => "Stop wastin' me time") ship = pirate.create_ship(:name => 's1') @@ -123,6 +123,22 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase assert_equal 's1', ship.reload.name end + def test_reject_if_with_a_proc_which_returns_true_always_for_has_many + Man.accepts_nested_attributes_for :interests, :reject_if => proc {|attributes| true } + man = Man.create(:name => "John") + interest = man.interests.create(:topic => 'photography') + man.update_attributes({:interests_attributes => { :topic => 'gardening', :id => interest.id } }) + assert_equal 'photography', interest.reload.topic + end + + def test_has_many_association_updating_a_single_record + Man.accepts_nested_attributes_for(:interests) + man = Man.create(:name => 'John') + interest = man.interests.create(:topic => 'photography') + man.update_attributes({:interests_attributes => {:topic => 'gardening', :id => interest.id}}) + assert_equal 'gardening', interest.reload.topic + end + end class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 3bc3671b77..d642aeed8b 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -32,6 +32,11 @@ class RelationTest < ActiveRecord::TestCase assert_equal 5, Post.where(:id => post_authors).size end + def test_dynamic_finder + x = Post.where('author_id = ?', 1) + assert x.klass.respond_to?(:find_by_id), '@klass should handle dynamic finders' + end + def test_multivalue_where posts = Post.where('author_id = ? AND id = ?', 1, 1) assert_equal 1, posts.to_a.size |