From c26d10563eaa29961ae895a9fbe3afae7d24a9b1 Mon Sep 17 00:00:00 2001 From: Pete Deffendol Date: Sat, 3 May 2008 21:41:10 -0600 Subject: PostgreSQL: update rake tasks to use full settings from database.yml Signed-off-by: Michael Koziarski --- railties/lib/tasks/databases.rake | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/railties/lib/tasks/databases.rake b/railties/lib/tasks/databases.rake index 20fdcce205..63f71448f8 100644 --- a/railties/lib/tasks/databases.rake +++ b/railties/lib/tasks/databases.rake @@ -46,7 +46,7 @@ namespace :db do @encoding = config[:encoding] || ENV['CHARSET'] || 'utf8' begin ActiveRecord::Base.establish_connection(config.merge('database' => 'template1')) - ActiveRecord::Base.connection.create_database(config['database'], :encoding => @encoding) + ActiveRecord::Base.connection.create_database(config['database'], config.merge('encoding' => @encoding)) ActiveRecord::Base.establish_connection(config) rescue $stderr.puts $!, *($!.backtrace) @@ -314,14 +314,9 @@ namespace :db do ActiveRecord::Base.establish_connection(:test) ActiveRecord::Base.connection.recreate_database(abcs["test"]["database"]) when "postgresql" - ENV['PGHOST'] = abcs["test"]["host"] if abcs["test"]["host"] - ENV['PGPORT'] = abcs["test"]["port"].to_s if abcs["test"]["port"] - ENV['PGPASSWORD'] = abcs["test"]["password"].to_s if abcs["test"]["password"] - enc_option = "-E #{abcs["test"]["encoding"]}" if abcs["test"]["encoding"] - ActiveRecord::Base.clear_active_connections! - `dropdb -U "#{abcs["test"]["username"]}" #{abcs["test"]["database"]}` - `createdb #{enc_option} -U "#{abcs["test"]["username"]}" #{abcs["test"]["database"]}` + drop_database(abcs['test']) + create_database(abcs['test']) when "sqlite","sqlite3" dbfile = abcs["test"]["database"] || abcs["test"]["dbfile"] File.delete(dbfile) if File.exist?(dbfile) -- cgit v1.2.3 From fbebdb0c091c37b0bc75ab774d187d8bc8795bd2 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Fri, 2 May 2008 00:00:42 +0100 Subject: Ensure correct record is returned when preloading has_one where more than one row exists Signed-off-by: Michael Koziarski [#73 state:closed] --- activerecord/lib/active_record/association_preload.rb | 7 ++++++- activerecord/test/cases/associations/eager_test.rb | 4 ++++ activerecord/test/models/post.rb | 2 ++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 3e7c787dee..da4ebdef51 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -65,7 +65,13 @@ module ActiveRecord end def set_association_single_records(id_to_record_map, reflection_name, associated_records, key) + seen_keys = {} associated_records.each do |associated_record| + #this is a has_one or belongs_to: there should only be one record. + #Unfortunately we can't (in portable way) ask the database for 'all records where foo_id in (x,y,z), but please + # only one row per distinct foo_id' so this where we enforce that + next if seen_keys[associated_record[key].to_s] + seen_keys[associated_record[key].to_s] = true mapped_records = id_to_record_map[associated_record[key].to_s] mapped_records.each do |mapped_record| mapped_record.send("set_#{reflection_name}_target", associated_record) @@ -122,7 +128,6 @@ module ActiveRecord else records.each {|record| record.send("set_#{reflection.name}_target", nil)} - set_association_single_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), reflection.primary_key_name) end end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 546ed80894..67b57ceb42 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -29,6 +29,10 @@ class EagerAssociationTest < ActiveRecord::TestCase post = Post.find(:first, :include => :comments, :conditions => "posts.title = 'Welcome to the weblog'") assert_equal 2, post.comments.size assert post.comments.include?(comments(:greetings)) + + posts = Post.find(:all, :include => :last_comment) + post = posts.find { |p| p.id == 1 } + assert_equal Post.find(1).last_comment, post.last_comment end def test_loading_conditions_with_or diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 22c5a645b8..d9101706b5 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -9,6 +9,8 @@ class Post < ActiveRecord::Base belongs_to :author_with_posts, :class_name => "Author", :foreign_key => :author_id, :include => :posts + has_one :last_comment, :class_name => 'Comment', :order => 'id desc' + has_many :comments, :order => "body" do def find_most_recent find(:first, :order => "id DESC") -- cgit v1.2.3 From 8ded457b1b31b157d6fe89b553749579e5ac4a27 Mon Sep 17 00:00:00 2001 From: John Devine Date: Sat, 3 May 2008 22:49:18 -0500 Subject: Added logic to associations.rb to make sure select_for_limited_ids includes joins that are needed to reach tables listed in the :order or :conditions options if they are not joined directly to the main active_record table. Signed-off-by: Michael Koziarski [#109 state:resolved] --- activerecord/lib/active_record/associations.rb | 25 ++++++++++++++++++++++++- activerecord/test/cases/finder_test.rb | 9 +++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) mode change 100755 => 100644 activerecord/lib/active_record/associations.rb diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb old mode 100755 new mode 100644 index 7d27b0607a..0809b271d7 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1446,6 +1446,12 @@ module ActiveRecord tables_from_conditions = conditions_tables(options) tables_from_order = order_tables(options) all_tables = tables_from_conditions + tables_from_order + distinct_join_associations = all_tables.uniq.map{|table| + join_dependency.joins_for_table_name(table) + }.flatten.compact.uniq + + + is_distinct = !options[:joins].blank? || include_eager_conditions?(options, tables_from_conditions) || include_eager_order?(options, tables_from_order) sql = "SELECT " @@ -1457,7 +1463,7 @@ module ActiveRecord sql << " FROM #{connection.quote_table_name table_name} " if is_distinct - sql << join_dependency.join_associations.reject{ |ja| !all_tables.include?(ja.table_name) }.collect(&:association_join).join + sql << distinct_join_associations.collect(&:association_join).join add_joins!(sql, options, scope) end @@ -1617,6 +1623,23 @@ module ActiveRecord end end + def join_for_table_name(table_name) + @joins.select{|j|j.aliased_table_name == table_name.gsub(/^\"(.*)\"$/){$1} }.first rescue nil + end + + def joins_for_table_name(table_name) + join = join_for_table_name(table_name) + result = nil + if join && join.is_a?(JoinAssociation) + result = [join] + if join.parent && join.parent.is_a?(JoinAssociation) + result = joins_for_table_name(join.parent.aliased_table_name) + + result + end + end + result + end + protected def build(associations, parent = nil) parent ||= @joins.last diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index b7f87fe6e8..2acfe9b387 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -8,6 +8,7 @@ require 'models/entrant' require 'models/developer' require 'models/post' require 'models/customer' +require 'models/job' class FinderTest < ActiveRecord::TestCase fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers @@ -857,6 +858,14 @@ class FinderTest < ActiveRecord::TestCase Company.connection.select_rows("SELECT id, name FROM companies WHERE id IN (1,2,3) ORDER BY id").map! {|i| i.map! {|j| j.to_s unless j.nil?}} end + def test_find_with_order_on_included_associations_with_construct_finder_sql_for_association_limiting_and_is_distinct + assert_equal 2, Post.find(:all,:include=>{:authors=>:author_address},:order=>' author_addresses.id DESC ', :limit=>2).size + + assert_equal 3, Post.find(:all,:include=>{:author=>:author_address,:authors=>:author_address}, + :order=>' author_addresses_authors.id DESC ', :limit=>3).size + end + + protected def bind(statement, *vars) if vars.first.is_a?(Hash) -- cgit v1.2.3 From 2c39836dc3c06813fce031d1bb390149b53ebd1c Mon Sep 17 00:00:00 2001 From: Marcos Arias Date: Mon, 5 May 2008 09:53:30 +0200 Subject: Refactored and fixed Resources.map_member_actions to make use of custom ActionController::Base.resources_path_names when the option :path_names is not directly specified. Added a specific test for this functionality and fixed assert_restful_routes_for test helper to make use of ActionController::Base.resources_path_names instead of just "new" or "edit". Signed-off-by: Michael Koziarski [#111 state:resolved] --- actionpack/lib/action_controller/resources.rb | 8 +++---- actionpack/test/controller/resources_test.rb | 30 +++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/actionpack/lib/action_controller/resources.rb b/actionpack/lib/action_controller/resources.rb index 0f0fa27d74..c5e7dd7359 100644 --- a/actionpack/lib/action_controller/resources.rb +++ b/actionpack/lib/action_controller/resources.rb @@ -524,11 +524,9 @@ module ActionController resource.member_methods.each do |method, actions| actions.each do |action| action_options = action_options_for(action, resource, method) - action_path = action - if resource.options[:path_names] - action_path = resource.options[:path_names][action] - action_path ||= Base.resources_path_names[action] || action - end + + action_path = resource.options[:path_names][action] if resource.options[:path_names].is_a?(Hash) + action_path ||= Base.resources_path_names[action] || action map.named_route("#{action}_#{resource.name_prefix}#{resource.singular}", "#{resource.member_path}#{resource.action_separator}#{action_path}", action_options) map.named_route("formatted_#{action}_#{resource.name_prefix}#{resource.singular}", "#{resource.member_path}#{resource.action_separator}#{action_path}.:format",action_options) diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index b138cee29f..0d089d0f23 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -226,6 +226,28 @@ class ResourcesTest < Test::Unit::TestCase end end + def test_member_when_changed_default_restful_actions_and_path_names_not_specified + default_path_names = ActionController::Base.resources_path_names + ActionController::Base.resources_path_names = {:new => 'nuevo', :edit => 'editar'} + + with_restful_routing :messages do + new_options = { :action => 'new', :controller => 'messages' } + new_path = "/messages/nuevo" + edit_options = { :action => 'edit', :id => '1', :controller => 'messages' } + edit_path = "/messages/1/editar" + + assert_restful_routes_for :messages do |options| + assert_recognizes(options.merge(new_options), :path => new_path, :method => :get) + end + + assert_restful_routes_for :messages do |options| + assert_recognizes(options.merge(edit_options), :path => edit_path, :method => :get) + end + end + ensure + ActionController::Base.resources_path_names = default_path_names + end + def test_with_two_member_actions_with_same_method [:put, :post].each do |method| with_restful_routing :messages, :member => { :mark => method, :unmark => method } do @@ -691,11 +713,11 @@ class ResourcesTest < Test::Unit::TestCase options[:options] ||= {} options[:options][:controller] = options[:controller] || controller_name.to_s - new_action = "new" - edit_action = "edit" + new_action = ActionController::Base.resources_path_names[:new] || "new" + edit_action = ActionController::Base.resources_path_names[:edit] || "edit" if options[:path_names] - new_action = options[:path_names][:new] || "new" - edit_action = options[:path_names][:edit] || "edit" + new_action = options[:path_names][:new] if options[:path_names][:new] + edit_action = options[:path_names][:edit] if options[:path_names][:edit] end collection_path = "/#{options[:path_prefix]}#{options[:as] || controller_name}" -- cgit v1.2.3 From a53331a161f72b25f6e9c860db43acaf23250f68 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Tue, 6 May 2008 11:52:44 +0100 Subject: Add class to deprecate instance variables Add ActiveSupport::Deprecation::DeprecatedInstanceVariable class to deprecate instance variables of primitive types such as stings. --- activesupport/lib/active_support/deprecation.rb | 14 ++++++++++++++ activesupport/test/deprecation_test.rb | 10 ++++++++++ 2 files changed, 24 insertions(+) diff --git a/activesupport/lib/active_support/deprecation.rb b/activesupport/lib/active_support/deprecation.rb index c9f1884da3..7613652c71 100644 --- a/activesupport/lib/active_support/deprecation.rb +++ b/activesupport/lib/active_support/deprecation.rb @@ -175,6 +175,20 @@ module ActiveSupport ActiveSupport::Deprecation.warn("#{@var} is deprecated! Call #{@method}.#{called} instead of #{@var}.#{called}. Args: #{args.inspect}", callstack) end end + + class DeprecatedInstanceVariable < Delegator #:nodoc: + def initialize(value, method) + super(value) + @method = method + @value = value + end + + def __getobj__ + ActiveSupport::Deprecation.warn("Instance variable @#{@method} is deprecated! Call instance method #{@method} instead.") + @value + end + end + end end diff --git a/activesupport/test/deprecation_test.rb b/activesupport/test/deprecation_test.rb index ebfa405947..11357e250f 100644 --- a/activesupport/test/deprecation_test.rb +++ b/activesupport/test/deprecation_test.rb @@ -149,3 +149,13 @@ class DeprecationTest < Test::Unit::TestCase assert_nil @last_message end end + +class DeprecatedIvarTest < Test::Unit::TestCase + + def test_deprecated_ivar + @action = ActiveSupport::Deprecation::DeprecatedInstanceVariable.new("fubar", :foobar) + + assert_deprecated(/Instance variable @foobar is deprecated! Call instance method foobar instead/) { assert_equal "fubar", @action } + end + +end -- cgit v1.2.3 From e520fd5db7cb839b862c03647effee50f9223d98 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Tue, 6 May 2008 12:02:24 +0100 Subject: Delegate action_name to controller inside views. --- actionpack/lib/action_view/base.rb | 2 +- actionpack/test/controller/new_render_test.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index a6da81de07..d32c6fbe35 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -185,7 +185,7 @@ module ActionView #:nodoc: attr_internal :request delegate :request_forgery_protection_token, :template, :params, :session, :cookies, :response, :headers, - :flash, :logger, :to => :controller + :flash, :logger, :action_name, :to => :controller module CompiledTemplates #:nodoc: # holds compiled template code diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb index 8e39057f55..6e2c6d90c6 100644 --- a/actionpack/test/controller/new_render_test.rb +++ b/actionpack/test/controller/new_render_test.rb @@ -246,6 +246,10 @@ class NewRenderTestController < ActionController::Base def accessing_logger_in_template render :inline => "<%= logger.class %>" end + + def accessing_action_name_in_template + render :inline => "<%= action_name %>" + end def accessing_params_in_template_with_layout render :layout => nil, :inline => "Hello: <%= params[:name] %>" @@ -545,6 +549,11 @@ class NewRenderTest < Test::Unit::TestCase get :accessing_logger_in_template assert_equal "Logger", @response.body end + + def test_access_to_action_name_in_view + get :accessing_action_name_in_template + assert_equal "accessing_action_name_in_template", @response.body + end def test_render_xml get :render_xml_hello -- cgit v1.2.3