diff options
74 files changed, 657 insertions, 358 deletions
diff --git a/.travis.yml b/.travis.yml index 596611ecf4..953df9e7e5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,6 +2,7 @@ script: 'ci/travis.rb' rvm: - 1.8.7 - 1.9.2 + - 1.9.3 env: - "GEM=railties" - "GEM=ap,am,amo,ares,as" @@ -11,7 +11,12 @@ gem "jquery-rails" # it being automatically loaded by sprockets gem "uglifier", ">= 1.0.0", :require => false -gem "rake", ">= 0.8.7" +# Temp fix until rake 0.9.3 is out +if RUBY_VERSION >= "1.9.3" + gem "rake", "0.9.3.beta.1" +else + gem "rake", ">= 0.8.7" +end gem "mocha", ">= 0.9.8" group :doc do diff --git a/README.rdoc b/README.rdoc index 5d1e3f5c5d..1e78799e83 100644 --- a/README.rdoc +++ b/README.rdoc @@ -18,7 +18,7 @@ you to present the data from database rows as objects and embellish these data o with business logic methods. Although most Rails models are backed by a database, models can also be ordinary Ruby classes, or Ruby classes that implement a set of interfaces as provided by the ActiveModel module. You can read more about Active Record in its -{README}[link:blob/master/activerecord/README.rdoc]. +{README}[link:/rails/rails/blob/master/activerecord/README.rdoc]. The Controller layer is responsible for handling incoming HTTP requests and providing a suitable response. Usually this means returning HTML, but Rails controllers can also @@ -29,7 +29,7 @@ In Rails, the Controller and View layers are handled together by Action Pack. These two layers are bundled in a single package due to their heavy interdependence. This is unlike the relationship between Active Record and Action Pack which are independent. Each of these packages can be used independently outside of Rails. You -can read more about Action Pack in its {README}[link:blob/master/actionpack/README.rdoc]. +can read more about Action Pack in its {README}[link:/rails/rails/blob/master/actionpack/README.rdoc]. == Getting Started @@ -74,7 +74,7 @@ RDoc::Task.new do |rdoc| # since no autolinking happens there and RDoc displays the backslash # otherwise. rdoc_main.gsub!(/^(?=\S).*?\b(?=Rails)\b/) { "#$&\\" } - rdoc_main.gsub!(%r{link:blob/master/(\w+)/README\.rdoc}, "link:files/\\1/README_rdoc.html") + rdoc_main.gsub!(%r{link:/rails/rails/blob/master/(\w+)/README\.rdoc}, "link:files/\\1/README_rdoc.html") File.open(RDOC_MAIN, 'w') do |f| f.write(rdoc_main) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 3b323b3899..b1286d04cc 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -30,6 +30,8 @@ *Rails 3.1.0 (unreleased)* +* x_sendfile_header now defaults to nil and config/environments/production.rb doesn't set a particular value for it. This allows servers to set it through X-Sendfile-Type. [Santiago Pastorino] + * The submit form helper does not generate an id "object_name_id" anymore. [fbrusatti] * Make sure respond_with with :js tries to render a template in all cases [José Valim] diff --git a/actionpack/lib/abstract_controller/view_paths.rb b/actionpack/lib/abstract_controller/view_paths.rb index 6b7aae8c74..e8394447a7 100644 --- a/actionpack/lib/abstract_controller/view_paths.rb +++ b/actionpack/lib/abstract_controller/view_paths.rb @@ -1,3 +1,5 @@ +require 'action_view/base' + module AbstractController module ViewPaths extend ActiveSupport::Concern @@ -63,7 +65,7 @@ module AbstractController # the default view path. You may also provide a custom view path # (see ActionView::PathSet for more information) def append_view_path(path) - self.view_paths = view_paths.dup + Array(path) + self._view_paths = view_paths + Array(path) end # Prepend a path to the list of view paths for this controller. @@ -73,7 +75,7 @@ module AbstractController # the default view path. You may also provide a custom view path # (see ActionView::PathSet for more information) def prepend_view_path(path) - self.view_paths = Array(path) + view_paths.dup + self._view_paths = ActionView::PathSet.new(Array(path) + view_paths) end # A list of all of the default view paths for this controller. @@ -87,8 +89,7 @@ module AbstractController # * <tt>paths</tt> - If a PathSet is provided, use that; # otherwise, process the parameter into a PathSet. def view_paths=(paths) - self._view_paths = ActionView::Base.process_view_paths(paths) - self._view_paths.freeze + self._view_paths = ActionView::PathSet.new(Array.wrap(paths)) end end end diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index 0785fe9679..5e077dd7bd 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -17,7 +17,7 @@ module ActionController #:nodoc: protected # Sends the file. This uses a server-appropriate method (such as X-Sendfile) # via the Rack::Sendfile middleware. The header to use is set via - # config.action_dispatch.x_sendfile_header, and defaults to "X-Sendfile". + # config.action_dispatch.x_sendfile_header. # Your server can also configure this for you by setting the X-Sendfile-Type header. # # Be careful to sanitize the path parameter if it is coming from a web diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index f51cc3711b..fbda1f8442 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -4,7 +4,7 @@ require "rails" module ActionDispatch class Railtie < Rails::Railtie config.action_dispatch = ActiveSupport::OrderedOptions.new - config.action_dispatch.x_sendfile_header = "" + config.action_dispatch.x_sendfile_header = nil config.action_dispatch.ip_spoofing_check = true config.action_dispatch.show_exceptions = true config.action_dispatch.best_standards_support = true diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 480144fe9d..de14113c51 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -140,7 +140,7 @@ module ActionDispatch when String options when nil, Hash - _routes.url_for((options || {}).reverse_merge!(url_options).symbolize_keys) + _routes.url_for((options || {}).reverse_merge(url_options).symbolize_keys) else polymorphic_url(options) end diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 36a0a20066..43d67f2032 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -4,6 +4,7 @@ require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/array/wrap' require 'active_support/ordered_options' require 'action_view/log_subscriber' +require 'active_support/core_ext/module/deprecation' module ActionView #:nodoc: # = Action View Base @@ -161,6 +162,7 @@ module ActionView #:nodoc: value.is_a?(PathSet) ? value.dup : ActionView::PathSet.new(Array.wrap(value)) end + deprecate :process_view_paths def xss_safe? #:nodoc: true diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index 2bbe0c175f..9ed4611123 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -56,8 +56,8 @@ module ActionView # form_tag('http://far.away.com/form', :authenticity_token => "cf50faa3fe97702ca1ae") # # form with custom authenticity token # - def form_tag(url_for_options = {}, options = {}, *parameters_for_url, &block) - html_options = html_options_for_form(url_for_options, options, *parameters_for_url) + def form_tag(url_for_options = {}, options = {}, &block) + html_options = html_options_for_form(url_for_options, options) if block_given? form_tag_in_block(html_options, &block) else @@ -604,12 +604,12 @@ module ActionView end private - def html_options_for_form(url_for_options, options, *parameters_for_url) + def html_options_for_form(url_for_options, options) options.stringify_keys.tap do |html_options| html_options["enctype"] = "multipart/form-data" if html_options.delete("multipart") # The following URL is unescaped, this is just a hash of options, and it is the # responsibility of the caller to escape all the values. - html_options["action"] = url_for(url_for_options, *parameters_for_url) + html_options["action"] = url_for(url_for_options) html_options["accept-charset"] = "UTF-8" html_options["data-remote"] = true if html_options.delete("remote") html_options["authenticity_token"] = html_options.delete("authenticity_token") if html_options.has_key?("authenticity_token") diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index 560df15e82..9ec410ac2b 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -78,7 +78,7 @@ module ActionView # Whenever setting view paths, makes a copy so we can manipulate then in # instance objects as we wish. def view_paths=(paths) - @view_paths = ActionView::Base.process_view_paths(paths) + @view_paths = ActionView::PathSet.new(Array.wrap(paths)) end def find(name, prefixes = [], partial = false, keys = []) diff --git a/actionpack/lib/action_view/path_set.rb b/actionpack/lib/action_view/path_set.rb index e0cb5d6a70..bbb1af8154 100644 --- a/actionpack/lib/action_view/path_set.rb +++ b/actionpack/lib/action_view/path_set.rb @@ -1,11 +1,55 @@ module ActionView #:nodoc: # = Action View PathSet - class PathSet < Array #:nodoc: - %w(initialize << concat insert push unshift).each do |method| + class PathSet #:nodoc: + include Enumerable + + attr_reader :paths + + def initialize(paths = []) + @paths = typecast paths + end + + def initialize_copy(other) + @paths = other.paths.dup + self + end + + def [](i) + paths[i] + end + + def to_ary + paths.dup + end + + def include?(item) + paths.include? item + end + + def pop + paths.pop + end + + def size + paths.size + end + + def each(&block) + paths.each(&block) + end + + def compact + PathSet.new paths.compact + end + + def +(array) + PathSet.new(paths + array) + end + + %w(<< concat push insert unshift).each do |method| class_eval <<-METHOD, __FILE__, __LINE__ + 1 def #{method}(*args) - super - typecast! + paths.#{method}(*typecast(args)) end METHOD end @@ -17,7 +61,7 @@ module ActionView #:nodoc: def find_all(path, prefixes = [], *args) prefixes = [prefixes] if String === prefixes prefixes.each do |prefix| - each do |resolver| + paths.each do |resolver| templates = resolver.find_all(path, prefix, *args) return templates unless templates.empty? end @@ -25,17 +69,20 @@ module ActionView #:nodoc: [] end - def exists?(*args) - find_all(*args).any? + def exists?(path, prefixes, *args) + find_all(path, prefixes, *args).any? end - protected + private - def typecast! - each_with_index do |path, i| - path = path.to_s if path.is_a?(Pathname) - next unless path.is_a?(String) - self[i] = OptimizedFileSystemResolver.new(path) + def typecast(paths) + paths.map do |path| + case path + when Pathname, String + OptimizedFileSystemResolver.new path.to_s + else + path + end end end end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 5f7fe81bd5..7abaa07bc7 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -1,5 +1,6 @@ require "pathname" require "active_support/core_ext/class" +require "active_support/core_ext/io" require "action_view/template" module ActionView @@ -68,7 +69,7 @@ module ActionView # before returning it. def cached(key, path_info, details, locals) #:nodoc: name, prefix, partial = path_info - locals = sort_locals(locals) + locals = locals.map { |x| x.to_s }.sort! if key && caching? @cached[key][name][prefix][partial][locals] ||= decorate(yield, path_info, details, locals) @@ -97,18 +98,6 @@ module ActionView t.virtual_path ||= (cached ||= build_path(*path_info)) end end - - if :symbol.respond_to?("<=>") - def sort_locals(locals) #:nodoc: - locals.sort.freeze - end - else - def sort_locals(locals) #:nodoc: - locals = locals.map{ |l| l.to_s } - locals.sort! - locals.freeze - end - end end # An abstract class that implements a Resolver with path semantics. @@ -130,20 +119,24 @@ module ActionView def query(path, details, formats) query = build_query(path, details) - templates = [] - sanitizer = Hash.new { |h,k| h[k] = Dir["#{File.dirname(k)}/*"] } - Dir[query].each do |p| - next if File.directory?(p) || !sanitizer[p].include?(p) + # deals with case-insensitive file systems. + sanitizer = Hash.new { |h,dir| h[dir] = Dir["#{dir}/*"] } - handler, format = extract_handler_and_format(p, formats) - contents = File.open(p, "rb") { |io| io.read } + template_paths = Dir[query].reject { |filename| + File.directory?(filename) || + !sanitizer[File.dirname(filename)].include?(filename) + } - templates << Template.new(contents, File.expand_path(p), handler, - :virtual_path => path.virtual, :format => format, :updated_at => mtime(p)) - end + template_paths.map { |template| + handler, format = extract_handler_and_format(template, formats) + contents = File.binread template - templates + Template.new(contents, File.expand_path(template), handler, + :virtual_path => path.virtual, + :format => format, + :updated_at => mtime(template)) + } end # Helper for building query glob string based on resolver's pattern. @@ -235,15 +228,10 @@ module ActionView class OptimizedFileSystemResolver < FileSystemResolver #:nodoc: def build_query(path, details) exts = EXTENSIONS.map { |ext| details[ext] } - query = File.join(@path, path) - - exts.each do |ext| - query << "{" - ext.compact.each { |e| query << ".#{e}," } - query << "}" - end - query + File.join(@path, path) + exts.map { |ext| + "{#{ext.compact.uniq.map { |e| ".#{e}," }.join}}" + }.join end end diff --git a/actionpack/lib/sprockets/assets.rake b/actionpack/lib/sprockets/assets.rake index 50278cffcd..01132d218f 100644 --- a/actionpack/lib/sprockets/assets.rake +++ b/actionpack/lib/sprockets/assets.rake @@ -1,7 +1,7 @@ namespace :assets do - # Ensures the RAILS_GROUPS environment variable is set task :ensure_env do ENV["RAILS_GROUPS"] ||= "assets" + ENV["RAILS_ENV"] ||= "production" end desc "Compile all the assets named in config.assets.precompile" @@ -16,7 +16,7 @@ namespace :assets do end desc "Remove compiled assets" - task :clean => :environment do + task :clean => [:environment, 'tmp:cache:clear'] do assets = Rails.application.config.assets public_asset_path = Rails.public_path + assets.prefix rm_rf public_asset_path, :secure => true diff --git a/actionpack/lib/sprockets/railtie.rb b/actionpack/lib/sprockets/railtie.rb index c8438e6043..0a2c8c1ea3 100644 --- a/actionpack/lib/sprockets/railtie.rb +++ b/actionpack/lib/sprockets/railtie.rb @@ -20,6 +20,7 @@ module Sprockets app.assets = Sprockets::Environment.new(app.root.to_s) do |env| env.static_root = File.join(app.root.join('public'), config.assets.prefix) env.logger = ::Rails.logger + env.version = ::Rails.env + "#{'-' + config.assets.version if config.assets.version.present?}" if config.assets.cache_store != false env.cache = ActiveSupport::Cache.lookup_store(config.assets.cache_store) || ::Rails.cache diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index b693fbec2b..5bf68decca 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -1664,114 +1664,6 @@ class RackMountIntegrationTests < ActiveSupport::TestCase assert_raise(ActionController::RoutingError) { @routes.recognize_path('/none', :method => :get) } end - def test_generate - assert_equal '/admin/users', url_for(@routes, { :use_route => 'admin_users' }) - assert_equal '/admin/users', url_for(@routes, { :controller => 'admin/users' }) - assert_equal '/admin/users', url_for(@routes, { :controller => 'admin/users', :action => 'index' }) - assert_equal '/admin/users', url_for(@routes, { :action => 'index' }, { :controller => 'admin/users' }) - assert_equal '/admin/users', url_for(@routes, { :controller => 'users', :action => 'index' }, { :controller => 'admin/accounts' }) - assert_equal '/people', url_for(@routes, { :controller => '/people', :action => 'index' }, { :controller => 'admin/accounts' }) - - assert_equal '/admin/posts', url_for(@routes, { :controller => 'admin/posts' }) - assert_equal '/admin/posts/new', url_for(@routes, { :controller => 'admin/posts', :action => 'new' }) - - assert_equal '/blog/2009', url_for(@routes, { :controller => 'posts', :action => 'show_date', :year => 2009 }) - assert_equal '/blog/2009/1', url_for(@routes, { :controller => 'posts', :action => 'show_date', :year => 2009, :month => 1 }) - assert_equal '/blog/2009/1/1', url_for(@routes, { :controller => 'posts', :action => 'show_date', :year => 2009, :month => 1, :day => 1 }) - - assert_equal '/archive/2010', url_for(@routes, { :controller => 'archive', :action => 'index', :year => '2010' }) - assert_equal '/archive', url_for(@routes, { :controller => 'archive', :action => 'index' }) - assert_equal '/archive?year=january', url_for(@routes, { :controller => 'archive', :action => 'index', :year => 'january' }) - - assert_equal '/people', url_for(@routes, { :controller => 'people', :action => 'index' }) - assert_equal '/people', url_for(@routes, { :action => 'index' }, { :controller => 'people' }) - assert_equal '/people', url_for(@routes, { :action => 'index' }, { :controller => 'people', :action => 'show', :id => '1' }) - assert_equal '/people', url_for(@routes, { :controller => 'people', :action => 'index' }, { :controller => 'people', :action => 'show', :id => '1' }) - assert_equal '/people', url_for(@routes, {}, { :controller => 'people', :action => 'index' }) - assert_equal '/people/1', url_for(@routes, { :controller => 'people', :action => 'show' }, { :controller => 'people', :action => 'show', :id => '1' }) - assert_equal '/people/new', url_for(@routes, { :use_route => 'new_person' }) - assert_equal '/people/new', url_for(@routes, { :controller => 'people', :action => 'new' }) - assert_equal '/people/1', url_for(@routes, { :use_route => 'person', :id => '1' }) - assert_equal '/people/1', url_for(@routes, { :controller => 'people', :action => 'show', :id => '1' }) - assert_equal '/people/1.xml', url_for(@routes, { :controller => 'people', :action => 'show', :id => '1', :format => 'xml' }) - assert_equal '/people/1', url_for(@routes, { :controller => 'people', :action => 'show', :id => 1 }) - assert_equal '/people/1', url_for(@routes, { :controller => 'people', :action => 'show', :id => Model.new('1') }) - assert_equal '/people/1', url_for(@routes, { :action => 'show', :id => '1' }, { :controller => 'people', :action => 'index' }) - assert_equal '/people/1', url_for(@routes, { :action => 'show', :id => 1 }, { :controller => 'people', :action => 'show', :id => '1' }) - assert_equal '/people', url_for(@routes, { :controller => 'people', :action => 'index' }, { :controller => 'people', :action => 'show', :id => '1' }) - assert_equal '/people/1', url_for(@routes, {}, { :controller => 'people', :action => 'show', :id => '1' }) - assert_equal '/people/1', url_for(@routes, { :controller => 'people', :action => 'show' }, { :controller => 'people', :action => 'index', :id => '1' }) - assert_equal '/people/1/edit', url_for(@routes, { :controller => 'people', :action => 'edit', :id => '1' }) - assert_equal '/people/1/edit.xml', url_for(@routes, { :controller => 'people', :action => 'edit', :id => '1', :format => 'xml' }) - assert_equal '/people/1/edit', url_for(@routes, { :use_route => 'edit_person', :id => '1' }) - assert_equal '/people/1?legacy=true', url_for(@routes, { :controller => 'people', :action => 'show', :id => '1', :legacy => 'true' }) - assert_equal '/people?legacy=true', url_for(@routes, { :controller => 'people', :action => 'index', :legacy => 'true' }) - - assert_equal '/id_default/2', url_for(@routes, { :controller => 'foo', :action => 'id_default', :id => '2' }) - assert_equal '/id_default', url_for(@routes, { :controller => 'foo', :action => 'id_default', :id => '1' }) - assert_equal '/id_default', url_for(@routes, { :controller => 'foo', :action => 'id_default', :id => 1 }) - assert_equal '/id_default', url_for(@routes, { :controller => 'foo', :action => 'id_default' }) - assert_equal '/optional/bar', url_for(@routes, { :controller => 'posts', :action => 'index', :optional => 'bar' }) - assert_equal '/posts', url_for(@routes, { :controller => 'posts', :action => 'index' }) - - assert_equal '/project', url_for(@routes, { :controller => 'project', :action => 'index' }) - assert_equal '/projects/1', url_for(@routes, { :controller => 'project', :action => 'index', :project_id => '1' }) - assert_equal '/projects/1', url_for(@routes, { :controller => 'project', :action => 'index'}, {:project_id => '1' }) - assert_raise(ActionController::RoutingError) { url_for(@routes, { :use_route => 'project', :controller => 'project', :action => 'index' }) } - assert_equal '/projects/1', url_for(@routes, { :use_route => 'project', :controller => 'project', :action => 'index', :project_id => '1' }) - assert_equal '/projects/1', url_for(@routes, { :use_route => 'project', :controller => 'project', :action => 'index' }, { :project_id => '1' }) - - assert_equal '/clients', url_for(@routes, { :controller => 'projects', :action => 'index' }) - assert_equal '/clients?project_id=1', url_for(@routes, { :controller => 'projects', :action => 'index', :project_id => '1' }) - assert_equal '/clients', url_for(@routes, { :controller => 'projects', :action => 'index' }, { :project_id => '1' }) - assert_equal '/clients', url_for(@routes, { :action => 'index' }, { :controller => 'projects', :action => 'index', :project_id => '1' }) - - assert_equal '/comment/20', url_for(@routes, { :id => 20 }, { :controller => 'comments', :action => 'show' }) - assert_equal '/comment/20', url_for(@routes, { :controller => 'comments', :id => 20, :action => 'show' }) - assert_equal '/comments/boo', url_for(@routes, { :controller => 'comments', :action => 'boo' }) - - assert_equal '/ws/posts/show/1', url_for(@routes, { :controller => 'posts', :action => 'show', :id => '1', :ws => true }) - assert_equal '/ws/posts', url_for(@routes, { :controller => 'posts', :action => 'index', :ws => true }) - - assert_equal '/account', url_for(@routes, { :controller => 'account', :action => 'subscription' }) - assert_equal '/account/billing', url_for(@routes, { :controller => 'account', :action => 'billing' }) - - assert_equal '/pages/1/notes/show/1', url_for(@routes, { :page_id => '1', :controller => 'notes', :action => 'show', :id => '1' }) - assert_equal '/pages/1/notes/list', url_for(@routes, { :page_id => '1', :controller => 'notes', :action => 'list' }) - assert_equal '/pages/1/notes', url_for(@routes, { :page_id => '1', :controller => 'notes', :action => 'index' }) - assert_equal '/pages/1/notes', url_for(@routes, { :page_id => '1', :controller => 'notes' }) - assert_equal '/notes', url_for(@routes, { :page_id => nil, :controller => 'notes' }) - assert_equal '/notes', url_for(@routes, { :controller => 'notes' }) - assert_equal '/notes/print', url_for(@routes, { :controller => 'notes', :action => 'print' }) - assert_equal '/notes/print', url_for(@routes, {}, { :controller => 'notes', :action => 'print' }) - - assert_equal '/notes/index/1', url_for(@routes, { :controller => 'notes' }, { :controller => 'notes', :id => '1' }) - assert_equal '/notes/index/1', url_for(@routes, { :controller => 'notes' }, { :controller => 'notes', :id => '1', :foo => 'bar' }) - assert_equal '/notes/index/1', url_for(@routes, { :controller => 'notes' }, { :controller => 'notes', :id => '1' }) - assert_equal '/notes/index/1', url_for(@routes, { :action => 'index' }, { :controller => 'notes', :id => '1' }) - assert_equal '/notes/index/1', url_for(@routes, {}, { :controller => 'notes', :id => '1' }) - assert_equal '/notes/show/1', url_for(@routes, {}, { :controller => 'notes', :action => 'show', :id => '1' }) - assert_equal '/notes/index/1', url_for(@routes, { :controller => 'notes', :id => '1' }, { :foo => 'bar' }) - assert_equal '/posts', url_for(@routes, { :controller => 'posts' }, { :controller => 'notes', :action => 'show', :id => '1' }) - assert_equal '/notes/list', url_for(@routes, { :action => 'list' }, { :controller => 'notes', :action => 'show', :id => '1' }) - - assert_equal '/posts/ping', url_for(@routes, { :controller => 'posts', :action => 'ping' }) - assert_equal '/posts/show/1', url_for(@routes, { :controller => 'posts', :action => 'show', :id => '1' }) - assert_equal '/posts', url_for(@routes, { :controller => 'posts' }) - assert_equal '/posts', url_for(@routes, { :controller => 'posts', :action => 'index' }) - assert_equal '/posts', url_for(@routes, { :controller => 'posts' }, { :controller => 'posts', :action => 'index' }) - assert_equal '/posts/create', url_for(@routes, { :action => 'create' }, { :controller => 'posts' }) - assert_equal '/posts?foo=bar', url_for(@routes, { :controller => 'posts', :foo => 'bar' }) - assert_equal '/posts?foo%5B%5D=bar&foo%5B%5D=baz', url_for(@routes, { :controller => 'posts', :foo => ['bar', 'baz'] }) - assert_equal '/posts?page=2', url_for(@routes, { :controller => 'posts', :page => 2 }) - assert_equal '/posts?q%5Bfoo%5D%5Ba%5D=b', url_for(@routes, { :controller => 'posts', :q => { :foo => { :a => 'b'}} }) - - assert_equal '/news.rss', url_for(@routes, { :controller => 'news', :action => 'index', :format => 'rss' }) - - - assert_raise(ActionController::RoutingError) { url_for(@routes, { :action => 'index' }) } - end - def test_generate_extras assert_equal ['/people', []], @routes.generate_extras(:controller => 'people') assert_equal ['/people', [:foo]], @routes.generate_extras(:controller => 'people', :foo => 'bar') diff --git a/actionpack/test/controller/url_for_integration_test.rb b/actionpack/test/controller/url_for_integration_test.rb new file mode 100644 index 0000000000..7b734ff0fb --- /dev/null +++ b/actionpack/test/controller/url_for_integration_test.rb @@ -0,0 +1,183 @@ +# encoding: utf-8 +require 'abstract_unit' +require 'controller/fake_controllers' +require 'active_support/core_ext/object/with_options' + +module RoutingTestHelpers + def url_for(set, options, recall = nil) + set.send(:url_for, options.merge(:only_path => true, :_path_segments => recall)) + end +end + +module ActionPack + class URLForIntegrationTest < ActiveSupport::TestCase + include RoutingTestHelpers + + Model = Struct.new(:to_param) + + Mapping = lambda { + namespace :admin do + resources :users, :posts + end + + namespace 'api' do + root :to => 'users#index' + end + + match '/blog(/:year(/:month(/:day)))' => 'posts#show_date', + :constraints => { + :year => /(19|20)\d\d/, + :month => /[01]?\d/, + :day => /[0-3]?\d/ + }, + :day => nil, + :month => nil + + match 'archive/:year', :controller => 'archive', :action => 'index', + :defaults => { :year => nil }, + :constraints => { :year => /\d{4}/ }, + :as => "blog" + + resources :people + #match 'legacy/people' => "people#index", :legacy => "true" + + match 'symbols', :controller => :symbols, :action => :show, :name => :as_symbol + match 'id_default(/:id)' => "foo#id_default", :id => 1 + match 'get_or_post' => "foo#get_or_post", :via => [:get, :post] + match 'optional/:optional' => "posts#index" + match 'projects/:project_id' => "project#index", :as => "project" + match 'clients' => "projects#index" + + match 'ignorecase/geocode/:postalcode' => 'geocode#show', :postalcode => /hx\d\d-\d[a-z]{2}/i + match 'extended/geocode/:postalcode' => 'geocode#show',:constraints => { + :postalcode => /# Postcode format + \d{5} #Prefix + (-\d{4})? #Suffix + /x + }, :as => "geocode" + + match 'news(.:format)' => "news#index" + + match 'comment/:id(/:action)' => "comments#show" + match 'ws/:controller(/:action(/:id))', :ws => true + match 'account(/:action)' => "account#subscription" + match 'pages/:page_id/:controller(/:action(/:id))' + match ':controller/ping', :action => 'ping' + match ':controller(/:action(/:id))(.:format)' + root :to => "news#index" + } + + def setup + @routes = ActionDispatch::Routing::RouteSet.new + @routes.draw(&Mapping) + end + + [ + ['/admin/users',[ { :use_route => 'admin_users' }]], + ['/admin/users',[ { :controller => 'admin/users' }]], + ['/admin/users',[ { :controller => 'admin/users', :action => 'index' }]], + ['/admin/users',[ { :action => 'index' }, { :controller => 'admin/users' }]], + ['/admin/users',[ { :controller => 'users', :action => 'index' }, { :controller => 'admin/accounts' }]], + ['/people',[ { :controller => '/people', :action => 'index' }, { :controller => 'admin/accounts' }]], + + ['/admin/posts',[ { :controller => 'admin/posts' }]], + ['/admin/posts/new',[ { :controller => 'admin/posts', :action => 'new' }]], + + ['/blog/2009',[ { :controller => 'posts', :action => 'show_date', :year => 2009 }]], + ['/blog/2009/1',[ { :controller => 'posts', :action => 'show_date', :year => 2009, :month => 1 }]], + ['/blog/2009/1/1',[ { :controller => 'posts', :action => 'show_date', :year => 2009, :month => 1, :day => 1 }]], + + ['/archive/2010',[ { :controller => 'archive', :action => 'index', :year => '2010' }]], + ['/archive',[ { :controller => 'archive', :action => 'index' }]], + ['/archive?year=january',[ { :controller => 'archive', :action => 'index', :year => 'january' }]], + + ['/people',[ { :controller => 'people', :action => 'index' }]], + ['/people',[ { :action => 'index' }, { :controller => 'people' }]], + ['/people',[ { :action => 'index' }, { :controller => 'people', :action => 'show', :id => '1' }]], + ['/people',[ { :controller => 'people', :action => 'index' }, { :controller => 'people', :action => 'show', :id => '1' }]], + ['/people',[ {}, { :controller => 'people', :action => 'index' }]], + ['/people/1',[ { :controller => 'people', :action => 'show' }, { :controller => 'people', :action => 'show', :id => '1' }]], + ['/people/new',[ { :use_route => 'new_person' }]], + ['/people/new',[ { :controller => 'people', :action => 'new' }]], + ['/people/1',[ { :use_route => 'person', :id => '1' }]], + ['/people/1',[ { :controller => 'people', :action => 'show', :id => '1' }]], + ['/people/1.xml',[ { :controller => 'people', :action => 'show', :id => '1', :format => 'xml' }]], + ['/people/1',[ { :controller => 'people', :action => 'show', :id => 1 }]], + ['/people/1',[ { :controller => 'people', :action => 'show', :id => Model.new('1') }]], + ['/people/1',[ { :action => 'show', :id => '1' }, { :controller => 'people', :action => 'index' }]], + ['/people/1',[ { :action => 'show', :id => 1 }, { :controller => 'people', :action => 'show', :id => '1' }]], + ['/people',[ { :controller => 'people', :action => 'index' }, { :controller => 'people', :action => 'show', :id => '1' }]], + ['/people/1',[ {}, { :controller => 'people', :action => 'show', :id => '1' }]], + ['/people/1',[ { :controller => 'people', :action => 'show' }, { :controller => 'people', :action => 'index', :id => '1' }]], + ['/people/1/edit',[ { :controller => 'people', :action => 'edit', :id => '1' }]], + ['/people/1/edit.xml',[ { :controller => 'people', :action => 'edit', :id => '1', :format => 'xml' }]], + ['/people/1/edit',[ { :use_route => 'edit_person', :id => '1' }]], + ['/people/1?legacy=true',[ { :controller => 'people', :action => 'show', :id => '1', :legacy => 'true' }]], + ['/people?legacy=true',[ { :controller => 'people', :action => 'index', :legacy => 'true' }]], + + ['/id_default/2',[ { :controller => 'foo', :action => 'id_default', :id => '2' }]], + ['/id_default',[ { :controller => 'foo', :action => 'id_default', :id => '1' }]], + ['/id_default',[ { :controller => 'foo', :action => 'id_default', :id => 1 }]], + ['/id_default',[ { :controller => 'foo', :action => 'id_default' }]], + ['/optional/bar',[ { :controller => 'posts', :action => 'index', :optional => 'bar' }]], + ['/posts',[ { :controller => 'posts', :action => 'index' }]], + + ['/project',[ { :controller => 'project', :action => 'index' }]], + ['/projects/1',[ { :controller => 'project', :action => 'index', :project_id => '1' }]], + ['/projects/1',[ { :controller => 'project', :action => 'index'}, {:project_id => '1' }]], + ['/projects/1',[ { :use_route => 'project', :controller => 'project', :action => 'index', :project_id => '1' }]], + ['/projects/1',[ { :use_route => 'project', :controller => 'project', :action => 'index' }, { :project_id => '1' }]], + + ['/clients',[ { :controller => 'projects', :action => 'index' }]], + ['/clients?project_id=1',[ { :controller => 'projects', :action => 'index', :project_id => '1' }]], + ['/clients',[ { :controller => 'projects', :action => 'index' }, { :project_id => '1' }]], + ['/clients',[ { :action => 'index' }, { :controller => 'projects', :action => 'index', :project_id => '1' }]], + + ['/comment/20',[ { :id => 20 }, { :controller => 'comments', :action => 'show' }]], + ['/comment/20',[ { :controller => 'comments', :id => 20, :action => 'show' }]], + ['/comments/boo',[ { :controller => 'comments', :action => 'boo' }]], + + ['/ws/posts/show/1',[ { :controller => 'posts', :action => 'show', :id => '1', :ws => true }]], + ['/ws/posts',[ { :controller => 'posts', :action => 'index', :ws => true }]], + + ['/account',[ { :controller => 'account', :action => 'subscription' }]], + ['/account/billing',[ { :controller => 'account', :action => 'billing' }]], + + ['/pages/1/notes/show/1',[ { :page_id => '1', :controller => 'notes', :action => 'show', :id => '1' }]], + ['/pages/1/notes/list',[ { :page_id => '1', :controller => 'notes', :action => 'list' }]], + ['/pages/1/notes',[ { :page_id => '1', :controller => 'notes', :action => 'index' }]], + ['/pages/1/notes',[ { :page_id => '1', :controller => 'notes' }]], + ['/notes',[ { :page_id => nil, :controller => 'notes' }]], + ['/notes',[ { :controller => 'notes' }]], + ['/notes/print',[ { :controller => 'notes', :action => 'print' }]], + ['/notes/print',[ {}, { :controller => 'notes', :action => 'print' }]], + + ['/notes/index/1',[ { :controller => 'notes' }, { :controller => 'notes', :id => '1' }]], + ['/notes/index/1',[ { :controller => 'notes' }, { :controller => 'notes', :id => '1', :foo => 'bar' }]], + ['/notes/index/1',[ { :controller => 'notes' }, { :controller => 'notes', :id => '1' }]], + ['/notes/index/1',[ { :action => 'index' }, { :controller => 'notes', :id => '1' }]], + ['/notes/index/1',[ {}, { :controller => 'notes', :id => '1' }]], + ['/notes/show/1',[ {}, { :controller => 'notes', :action => 'show', :id => '1' }]], + ['/notes/index/1',[ { :controller => 'notes', :id => '1' }, { :foo => 'bar' }]], + ['/posts',[ { :controller => 'posts' }, { :controller => 'notes', :action => 'show', :id => '1' }]], + ['/notes/list',[ { :action => 'list' }, { :controller => 'notes', :action => 'show', :id => '1' }]], + + ['/posts/ping',[ { :controller => 'posts', :action => 'ping' }]], + ['/posts/show/1',[ { :controller => 'posts', :action => 'show', :id => '1' }]], + ['/posts',[ { :controller => 'posts' }]], + ['/posts',[ { :controller => 'posts', :action => 'index' }]], + ['/posts',[ { :controller => 'posts' }, { :controller => 'posts', :action => 'index' }]], + ['/posts/create',[ { :action => 'create' }, { :controller => 'posts' }]], + ['/posts?foo=bar',[ { :controller => 'posts', :foo => 'bar' }]], + ['/posts?foo%5B%5D=bar&foo%5B%5D=baz', [{ :controller => 'posts', :foo => ['bar', 'baz'] }]], + ['/posts?page=2', [{ :controller => 'posts', :page => 2 }]], + ['/posts?q%5Bfoo%5D%5Ba%5D=b', [{ :controller => 'posts', :q => { :foo => { :a => 'b'}} }]], + + ['/news.rss', [{ :controller => 'news', :action => 'index', :format => 'rss' }]], + ].each_with_index do |(url, params), i| + define_method("test_#{url.gsub(/\W/, '_')}_#{i}") do + assert_equal url, url_for(@routes, *params), params.inspect + end + end + end +end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 1938348375..9685b24c1c 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -851,6 +851,18 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + # tests the use of dup in url_for + def test_url_for_with_no_side_effects + # without dup, additional (and possibly unwanted) values will be present in the options (eg. :host) + original_options = {:controller => 'projects', :action => 'status'} + options = original_options.dup + + url_for options + + # verify that the options passed in have not changed from the original ones + assert_equal original_options, options + end + def test_projects_status with_test_routes do assert_equal '/projects/status', url_for(:controller => 'projects', :action => 'status', :only_path => true) diff --git a/actionpack/test/template/compiled_templates_test.rb b/actionpack/test/template/compiled_templates_test.rb index 3f31edd5ce..8be0f452fb 100644 --- a/actionpack/test/template/compiled_templates_test.rb +++ b/actionpack/test/template/compiled_templates_test.rb @@ -42,7 +42,7 @@ class CompiledTemplatesTest < Test::Unit::TestCase def render_without_cache(*args) path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH) - view_paths = ActionView::Base.process_view_paths(path) + view_paths = ActionView::PathSet.new([path]) ActionView::Base.new(view_paths, {}).render(*args) end diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 6f02f8662d..8a582030f6 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -380,7 +380,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase # is not eager loaded def setup path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH) - view_paths = ActionView::Base.process_view_paths(path) + view_paths = ActionView::PathSet.new([path]) assert_equal ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH), view_paths.first setup_view(view_paths) end diff --git a/actionpack/test/template/sprockets_helper_test.rb b/actionpack/test/template/sprockets_helper_test.rb index f4b5344d63..dfa635335e 100644 --- a/actionpack/test/template/sprockets_helper_test.rb +++ b/actionpack/test/template/sprockets_helper_test.rb @@ -205,4 +205,17 @@ class SprocketsHelperTest < ActionView::TestCase stubs(:asset_environment).returns(assets) assert_match %r{/assets/style-[0-9a-f]+.css}, asset_path("style", "css") end + + test "alternate hash based on environment" do + assets = Sprockets::Environment.new + assets.version = 'development' + assets.append_path(FIXTURES.join("sprockets/alternate/stylesheets")) + stubs(:asset_environment).returns(assets) + dev_path = asset_path("style", "css") + + assets.version = 'production' + prod_path = asset_path("style", "css") + + assert_not_equal prod_path, dev_path + end end diff --git a/activemodel/lib/active_model/observing.rb b/activemodel/lib/active_model/observing.rb index d48f2e8a1f..7a910d18e7 100644 --- a/activemodel/lib/active_model/observing.rb +++ b/activemodel/lib/active_model/observing.rb @@ -226,10 +226,10 @@ module ActiveModel # Send observed_method(object) if the method exists and # the observer is enabled for the given object's class. - def update(observed_method, object) #:nodoc: + def update(observed_method, object, &block) #:nodoc: return unless respond_to?(observed_method) return if disabled_for?(object) - send(observed_method, object) + send(observed_method, object, &block) end # Special method sent by the observed class when it is inherited. diff --git a/activemodel/test/cases/observing_test.rb b/activemodel/test/cases/observing_test.rb index 99b1f407ae..f6ec24ae57 100644 --- a/activemodel/test/cases/observing_test.rb +++ b/activemodel/test/cases/observing_test.rb @@ -17,6 +17,10 @@ class FooObserver < ActiveModel::Observer def on_spec(record) stub.event_with(record) if stub end + + def around_save(record) + yield :in_around_save + end end class Foo @@ -133,4 +137,12 @@ class ObserverTest < ActiveModel::TestCase foo = Foo.new Foo.send(:notify_observers, :whatever, foo) end + + test "update passes a block on to the observer" do + yielded_value = nil + FooObserver.instance.update(:around_save, Foo.new) do |val| + yielded_value = val + end + assert_equal :in_around_save, yielded_value + end end diff --git a/activerecord/activerecord.gemspec b/activerecord/activerecord.gemspec index 8ac4d9a225..c91a6ccd35 100644 --- a/activerecord/activerecord.gemspec +++ b/activerecord/activerecord.gemspec @@ -21,6 +21,6 @@ Gem::Specification.new do |s| s.add_dependency('activesupport', version) s.add_dependency('activemodel', version) - s.add_dependency('arel', '~> 2.1.3') + s.add_dependency('arel', '~> 2.2.0') s.add_dependency('tzinfo', '~> 0.3.29') end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index f7ce70db1a..1f917f58f2 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -26,7 +26,7 @@ module ActiveRecord join_table[reflection.association_foreign_key] => record.id ) - owner.connection.insert stmt.to_sql + owner.connection.insert stmt end record @@ -46,7 +46,7 @@ module ActiveRecord stmt = relation.where(relation[reflection.foreign_key].eq(owner.id). and(relation[reflection.association_foreign_key].in(records.map { |x| x.id }.compact)) ).compile_delete - owner.connection.delete stmt.to_sql + owner.connection.delete stmt end end diff --git a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb index 24be279449..b77b667219 100644 --- a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb @@ -13,7 +13,7 @@ module ActiveRecord # access the aliased column on the join table def records_for(ids) scope = super - klass.connection.select_all(scope.arel.to_sql, 'SQL', scope.bind_values) + klass.connection.select_all(scope.arel, 'SQL', scope.bind_values) end def owner_key_name diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index 8bd898d126..ed71b5e7d4 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -3,11 +3,10 @@ module ActiveRecord module PrimaryKey extend ActiveSupport::Concern - # Returns this record's primary key value wrapped in an Array or nil if - # the record is not persisted? or has just been destroyed. + # Returns this record's primary key value wrapped in an Array if one is available def to_key key = send(self.class.primary_key) - persisted? && key ? [key] : nil + [key] if key end module ClassMethods diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index b3a0267276..59977280b3 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -941,17 +941,6 @@ module ActiveRecord #:nodoc: self.current_scope = nil end - # Specifies how the record is loaded by +Marshal+. - # - # +_load+ sets an instance variable for each key in the hash it takes as input. - # Override this method if you require more complex marshalling. - def _load(data) - record = allocate - record.init_with(Marshal.load(data)) - record - end - - # Finder methods must instantiate through this method to work with the # single-table inheritance model that makes it possible to create # objects of different types from the same table. @@ -1271,27 +1260,43 @@ MSG self.default_scopes = default_scopes + [scope] end - # The @ignore_default_scope flag is used to prevent an infinite recursion situation where - # a default scope references a scope which has a default scope which references a scope... def build_default_scope #:nodoc: - return if defined?(@ignore_default_scope) && @ignore_default_scope - @ignore_default_scope = true - if method(:default_scope).owner != Base.singleton_class - default_scope + evaluate_default_scope { default_scope } elsif default_scopes.any? - default_scopes.inject(relation) do |default_scope, scope| - if scope.is_a?(Hash) - default_scope.apply_finder_options(scope) - elsif !scope.is_a?(Relation) && scope.respond_to?(:call) - default_scope.merge(scope.call) - else - default_scope.merge(scope) + evaluate_default_scope do + default_scopes.inject(relation) do |default_scope, scope| + if scope.is_a?(Hash) + default_scope.apply_finder_options(scope) + elsif !scope.is_a?(Relation) && scope.respond_to?(:call) + default_scope.merge(scope.call) + else + default_scope.merge(scope) + end end end end - ensure - @ignore_default_scope = false + end + + def ignore_default_scope? #:nodoc: + Thread.current["#{self}_ignore_default_scope"] + end + + def ignore_default_scope=(ignore) #:nodoc: + Thread.current["#{self}_ignore_default_scope"] = ignore + end + + # The ignore_default_scope flag is used to prevent an infinite recursion situation where + # a default scope references a scope which has a default scope which references a scope... + def evaluate_default_scope + return if ignore_default_scope? + + begin + self.ignore_default_scope = true + yield + ensure + self.ignore_default_scope = false + end end # Returns the class type of the record using the current module as a prefix. So descendants of @@ -1413,9 +1418,8 @@ MSG attrs = expand_hash_conditions_for_aggregates(attrs) table = Arel::Table.new(table_name).alias(default_table_name) - viz = Arel::Visitors.for(arel_engine) PredicateBuilder.build_from_hash(arel_engine, attrs, table).map { |b| - viz.accept b + connection.visitor.accept b }.join(' AND ') end alias_method :sanitize_sql_hash, :sanitize_sql_hash_for_conditions @@ -1593,16 +1597,6 @@ MSG self end - # Specifies how the record is dumped by +Marshal+. - # - # +_dump+ emits a marshalled hash which has been passed to +encode_with+. Override this - # method if you require more complex marshalling. - def _dump(level) - dump = {} - encode_with(dump) - Marshal.dump(dump) - end - # Returns a String, which Action Pack uses for constructing an URL to this # object. The default implementation returns this record's id as a String, # or nil if this record's unsaved. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index ddfdb05297..61994d4a47 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -82,10 +82,11 @@ module ActiveRecord # default max pool size to 5 @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5 - @connections = [] - @checked_out = [] + @connections = [] + @checked_out = [] @automatic_reconnect = true - @tables = {} + @tables = {} + @visitor = nil @columns = Hash.new do |h, table_name| h[table_name] = with_connection do |conn| @@ -298,8 +299,18 @@ module ActiveRecord :connected?, :disconnect!, :with => :@connection_mutex private + def new_connection - ActiveRecord::Base.send(spec.adapter_method, spec.config) + connection = ActiveRecord::Base.send(spec.adapter_method, spec.config) + + # TODO: This is a bit icky, and in the long term we may want to change the method + # signature for connections. Also, if we switch to have one visitor per + # connection (and therefore per thread), we can get rid of the thread-local + # variable in Arel::Visitors::ToSql. + @visitor ||= connection.class.visitor_for(self) + connection.visitor = @visitor + + connection end def current_connection_id #:nodoc: 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 777ef15dfc..2ae655e68d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -1,30 +1,39 @@ module ActiveRecord module ConnectionAdapters # :nodoc: module DatabaseStatements + # Converts an arel AST to SQL + def to_sql(arel) + if arel.respond_to?(:ast) + visitor.accept(arel.ast) + else + arel + end + end + # Returns an array of record hashes with the column names as keys and # column values as values. - def select_all(sql, name = nil, binds = []) - select(sql, name, binds) + def select_all(arel, name = nil, binds = []) + select(to_sql(arel), name, binds) end # Returns a record hash with the column names as keys and column values # as values. - def select_one(sql, name = nil) - result = select_all(sql, name) + def select_one(arel, name = nil) + result = select_all(arel, name) result.first if result end # Returns a single value from a record - def select_value(sql, name = nil) - if result = select_one(sql, name) + def select_value(arel, name = nil) + if result = select_one(arel, name) result.values.first end end # Returns an array of the values of the first column in a select: # select_values("SELECT id FROM companies LIMIT 3") => [1,2,3] - def select_values(sql, name = nil) - result = select_rows(sql, name) + def select_values(arel, name = nil) + result = select_rows(to_sql(arel), name) result.map { |v| v[0] } end @@ -74,20 +83,20 @@ module ActiveRecord # # If the next id was calculated in advance (as in Oracle), it should be # passed in as +id_value+. - def insert(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil, binds = []) - sql, binds = sql_for_insert(sql, pk, id_value, sequence_name, binds) + def insert(arel, name = nil, pk = nil, id_value = nil, sequence_name = nil, binds = []) + sql, binds = sql_for_insert(to_sql(arel), pk, id_value, sequence_name, binds) value = exec_insert(sql, name, binds) id_value || last_inserted_id(value) end # Executes the update statement and returns the number of rows affected. - def update(sql, name = nil, binds = []) - exec_update(sql, name, binds) + def update(arel, name = nil, binds = []) + exec_update(to_sql(arel), name, binds) end # Executes the delete statement and returns the number of rows affected. - def delete(sql, name = nil, binds = []) - exec_delete(sql, name, binds) + def delete(arel, name = nil, binds = []) + exec_delete(to_sql(arel), name, binds) end # Checks whether there is currently no transaction active. This is done diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 093c30aa42..27ff13ad89 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -55,9 +55,10 @@ module ActiveRecord @query_cache.clear end - def select_all(sql, name = nil, binds = []) + def select_all(arel, name = nil, binds = []) if @query_cache_enabled - cache_sql(sql, binds) { super } + sql = to_sql(arel) + cache_sql(sql, binds) { super(sql, name, binds) } else super end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index bde31d1cda..077cf7df1b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -2,6 +2,7 @@ require 'date' require 'bigdecimal' require 'bigdecimal/util' require 'active_support/core_ext/benchmark' +require 'active_support/deprecation' # TODO: Autoload these files require 'active_record/connection_adapters/column' @@ -38,6 +39,8 @@ module ActiveRecord define_callbacks :checkout, :checkin + attr_accessor :visitor + def initialize(connection, logger = nil) #:nodoc: @active = nil @connection, @logger = connection, logger @@ -45,6 +48,25 @@ module ActiveRecord @query_cache = Hash.new { |h,sql| h[sql] = {} } @open_transactions = 0 @instrumenter = ActiveSupport::Notifications.instrumenter + @visitor = nil + end + + # Returns a visitor instance for this adaptor, which conforms to the Arel::ToSql interface + def self.visitor_for(pool) # :nodoc: + adapter = pool.spec.config[:adapter] + + if Arel::Visitors::VISITORS[adapter] + ActiveSupport::Deprecation.warn( + "Arel::Visitors::VISITORS is deprecated and will be removed. Database adapters " \ + "should define a visitor_for method which returns the appropriate visitor for " \ + "the database. For example, MysqlAdapter.visitor_for(pool) returns " \ + "Arel::Visitors::MySQL.new(pool)." + ) + + Arel::Visitors::VISITORS[adapter].new(pool) + else + Arel::Visitors::ToSql.new(pool) + end end # Returns the human-readable name of the adapter. Use mixed case - one diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index f9602bbe77..18fdfa29ec 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -129,6 +129,10 @@ module ActiveRecord configure_connection end + def self.visitor_for(pool) # :nodoc: + Arel::Visitors::MySQL.new(pool) + end + def adapter_name ADAPTER_NAME end diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 9e6cb13cca..14b950dbb0 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -192,6 +192,10 @@ module ActiveRecord connect end + def self.visitor_for(pool) # :nodoc: + Arel::Visitors::MySQL.new(pool) + end + def adapter_name #:nodoc: ADAPTER_NAME end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index aefe69f8ed..45c13bdcd6 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -265,6 +265,10 @@ module ActiveRecord @local_tz = execute('SHOW TIME ZONE', 'SCHEMA').first["TimeZone"] end + def self.visitor_for(pool) # :nodoc: + Arel::Visitors::PostgreSQL.new(pool) + end + # Clears the prepared statements cache. def clear_cache! @statements.each_value do |value| diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index c3a7b039ff..0a0da0b5d3 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -1,4 +1,6 @@ require 'active_record/connection_adapters/sqlite_adapter' + +gem 'sqlite3', '~> 1.3.4' require 'sqlite3' module ActiveRecord diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index e2a0f63393..486efc5ba0 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -53,6 +53,10 @@ module ActiveRecord @config = config end + def self.visitor_for(pool) # :nodoc: + Arel::Visitors::SQLite.new(pool) + end + def adapter_name #:nodoc: 'SQLite' end @@ -238,15 +242,15 @@ module ActiveRecord end def begin_db_transaction #:nodoc: - @connection.transaction + log('begin transaction',nil) { @connection.transaction } end def commit_db_transaction #:nodoc: - @connection.commit + log('commit transaction',nil) { @connection.commit } end def rollback_db_transaction #:nodoc: - @connection.rollback + log('rollback transaction',nil) { @connection.rollback } end # SCHEMA STATEMENTS ======================================== diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 4d387565d9..3c7defedac 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -33,7 +33,7 @@ module ActiveRecord stmt = unscoped.where(arel_table[primary_key].eq(object.id)).arel.compile_update({ arel_table[counter_name] => object.send(association).count }) - connection.update stmt.to_sql + connection.update stmt end return true end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 6cfce6e573..d9ad7e4132 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -70,7 +70,7 @@ module ActiveRecord # If the locking column has no default value set, # start the lock version at zero. Note we can't use - # <tt>locking_enabled?</tt> at this point as + # <tt>locking_enabled?</tt> at this point as # <tt>@attributes</tt> may not have been initialized yet. if result.key?(self.class.locking_column) && lock_optimistically @@ -100,7 +100,7 @@ module ActiveRecord ) ).arel.compile_update(arel_attributes_values(false, false, attribute_names)) - affected_rows = connection.update stmt.to_sql + affected_rows = connection.update stmt unless affected_rows == 1 raise ActiveRecord::StaleObjectError, "Attempted to update a stale object: #{self.class.name}" diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index fa1b303fc7..7166f1b82a 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -563,7 +563,7 @@ module ActiveRecord def get_all_versions table = Arel::Table.new(schema_migrations_table_name) - Base.connection.select_values(table.project(table['version']).to_sql).map{ |v| v.to_i }.sort + Base.connection.select_values(table.project(table['version'])).map{ |v| v.to_i }.sort end def current_version @@ -720,11 +720,11 @@ module ActiveRecord if down? @migrated_versions.delete(version) stmt = table.where(table["version"].eq(version.to_s)).compile_delete - Base.connection.delete stmt.to_sql + Base.connection.delete stmt else @migrated_versions.push(version).sort! stmt = table.compile_insert table["version"] => version.to_s - Base.connection.insert stmt.to_sql + Base.connection.insert stmt end end diff --git a/activerecord/lib/active_record/observer.rb b/activerecord/lib/active_record/observer.rb index 5a5351b517..fdf17c003c 100644 --- a/activerecord/lib/active_record/observer.rb +++ b/activerecord/lib/active_record/observer.rb @@ -111,7 +111,7 @@ module ActiveRecord callback_meth = :"_notify_#{observer_name}_for_#{callback}" unless klass.respond_to?(callback_meth) klass.send(:define_method, callback_meth) do |&block| - observer.send(callback, self, &block) + observer.update(callback, self, &block) end klass.send(callback, callback_meth) end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index ebda3875cd..2dac9ea0fb 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -304,7 +304,7 @@ module ActiveRecord return 0 if attributes_with_values.empty? klass = self.class stmt = klass.unscoped.where(klass.arel_table[klass.primary_key].eq(id)).arel.compile_update(attributes_with_values) - klass.connection.update stmt.to_sql + klass.connection.update stmt end # Creates a record with values matching those of the instance attributes diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index fff0ad1b83..7e59eb4584 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -68,7 +68,7 @@ module ActiveRecord end conn.insert( - im.to_sql, + im, 'SQL', primary_key, primary_key_value, @@ -108,10 +108,10 @@ module ActiveRecord if default_scoped.equal?(self) @records = if @readonly_value.nil? && !@klass.locking_enabled? - eager_loading? ? find_with_associations : @klass.find_by_sql(arel.to_sql, @bind_values) + eager_loading? ? find_with_associations : @klass.find_by_sql(arel, @bind_values) else IdentityMap.without do - eager_loading? ? find_with_associations : @klass.find_by_sql(arel.to_sql, @bind_values) + eager_loading? ? find_with_associations : @klass.find_by_sql(arel, @bind_values) end end @@ -224,7 +224,7 @@ module ActiveRecord stmt.order(*arel.orders) stmt.key = table[primary_key] - @klass.connection.update stmt.to_sql, 'SQL', bind_values + @klass.connection.update stmt, 'SQL', bind_values end end @@ -341,8 +341,7 @@ module ActiveRecord where(conditions).delete_all else statement = arel.compile_delete - affected = @klass.connection.delete( - statement.to_sql, 'SQL', bind_values) + affected = @klass.connection.delete(statement, 'SQL', bind_values) reset affected @@ -388,7 +387,7 @@ module ActiveRecord end def to_sql - @to_sql ||= arel.to_sql + @to_sql ||= klass.connection.to_sql(arel) end def where_values_hash diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 9a7ff87e88..af86771d2d 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -223,7 +223,7 @@ module ActiveRecord query_builder = relation.arel end - type_cast_calculated_value(@klass.connection.select_value(query_builder.to_sql), column_for(column_name), operation) + type_cast_calculated_value(@klass.connection.select_value(query_builder), column_for(column_name), operation) end def execute_grouped_calculation(operation, column_name, distinct) #:nodoc: @@ -259,7 +259,7 @@ module ActiveRecord relation = except(:group).group(group.join(',')) relation.select_values = select_values - calculated_data = @klass.connection.select_all(relation.to_sql) + calculated_data = @klass.connection.select_all(relation) if association key_ids = calculated_data.collect { |row| row[group_aliases.first] } diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 8cef4e5554..73368aed18 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -193,8 +193,8 @@ module ActiveRecord else relation = relation.where(table[primary_key].eq(id)) if id end - - connection.select_value(relation.to_sql, "#{name} Exists") ? true : false + + connection.select_value(relation, "#{name} Exists") ? true : false end protected @@ -202,7 +202,7 @@ module ActiveRecord def find_with_associations join_dependency = construct_join_dependency_for_association_find relation = construct_relation_for_association_find(join_dependency) - rows = connection.select_all(relation.to_sql, 'SQL', relation.bind_values) + rows = connection.select_all(relation, 'SQL', relation.bind_values) join_dependency.instantiate(rows) rescue ThrowResult [] diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 3942e7bb41..94497e37c7 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -151,7 +151,20 @@ class AdapterTest < ActiveRecord::TestCase else @connection.execute "INSERT INTO fk_test_has_fk (fk_id) VALUES (0)" end + # should deleted created record as otherwise disable_referential_integrity will try to enable contraints after executed block + # and will fail (at least on Oracle) + @connection.execute "DELETE FROM fk_test_has_fk" end end end + + def test_deprecated_visitor_for + visitor_klass = Class.new(Arel::Visitors::ToSql) + Arel::Visitors::VISITORS['fuuu'] = visitor_klass + pool = stub(:spec => stub(:config => { :adapter => 'fuuu' })) + visitor = assert_deprecated { + ActiveRecord::ConnectionAdapters::AbstractAdapter.visitor_for(pool) + } + assert visitor.is_a?(visitor_klass) + end end diff --git a/activerecord/test/cases/associations/extension_test.rb b/activerecord/test/cases/associations/extension_test.rb index 24830a661a..490fc5177e 100644 --- a/activerecord/test/cases/associations/extension_test.rb +++ b/activerecord/test/cases/associations/extension_test.rb @@ -39,7 +39,9 @@ class AssociationsExtensionsTest < ActiveRecord::TestCase david = developers(:david) assert_equal projects(:action_controller), david.projects.find_most_recent - david = Marshal.load(Marshal.dump(david)) + marshalled = Marshal.dump(david) + david = Marshal.load(marshalled) + assert_equal projects(:action_controller), david.projects.find_most_recent end @@ -47,7 +49,9 @@ class AssociationsExtensionsTest < ActiveRecord::TestCase david = developers(:david) assert_equal projects(:action_controller), david.projects_extended_by_name.find_most_recent - david = Marshal.load(Marshal.dump(david)) + marshalled = Marshal.dump(david) + david = Marshal.load(marshalled) + assert_equal projects(:action_controller), david.projects_extended_by_name.find_most_recent end diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index dd450a2a8e..80c6e41169 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -247,7 +247,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase def test_has_many_through_has_and_belongs_to_many_with_has_many_source_reflection_preload_via_joins assert_includes_and_joins_equal( - Category.where('comments.id' => comments(:more_greetings).id).order('comments.id'), + Category.where('comments.id' => comments(:more_greetings).id).order('categories.id'), [categories(:general), categories(:technology)], :post_comments ) end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 12101c1683..b8ebabfe70 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -144,25 +144,6 @@ 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 table_exists?(name); false; 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, @@ -1827,11 +1808,29 @@ class BasicsTest < ActiveRecord::TestCase def test_marshal_round_trip expected = posts(:welcome) - actual = Marshal.load(Marshal.dump(expected)) + marshalled = Marshal.dump(expected) + actual = Marshal.load(marshalled) assert_equal expected.attributes, actual.attributes end + def test_marshal_new_record_round_trip + marshalled = Marshal.dump(Post.new) + post = Marshal.load(marshalled) + + assert post.new_record?, "should be a new record" + end + + def test_marshalling_with_associations + post = Post.new + post.comments.build + + marshalled = Marshal.dump(post) + post = Marshal.load(marshalled) + + assert_equal 1, post.comments.length + end + def test_attribute_names assert_equal ["id", "type", "ruby_type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id"], Company.attribute_names diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 42f98b3d42..c38814713a 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -171,7 +171,7 @@ class CalculationsTest < ActiveRecord::TestCase end def test_should_group_by_summed_field_having_condition_from_select - c = Account.select("MIN(credit_limit) AS min_credit_limit").group(:firm_id).having("min_credit_limit > 50").sum(:credit_limit) + c = Account.select("MIN(credit_limit) AS min_credit_limit").group(:firm_id).having("MIN(credit_limit) > 50").sum(:credit_limit) assert_nil c[1] assert_equal 60, c[2] assert_equal 53, c[9] diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index f92f4e62c5..8a0f453127 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -135,6 +135,10 @@ module ActiveRecord pool.with_connection end end + + def test_pool_sets_connection_visitor + assert @pool.connection.visitor.is_a?(Arel::Visitors::ToSql) + end end end end diff --git a/activerecord/test/cases/lifecycle_test.rb b/activerecord/test/cases/lifecycle_test.rb index 643e949087..75e5dfa49b 100644 --- a/activerecord/test/cases/lifecycle_test.rb +++ b/activerecord/test/cases/lifecycle_test.rb @@ -231,6 +231,18 @@ class LifecycleTest < ActiveRecord::TestCase assert_not_nil observer.topic_ids.last end + test "able to disable observers" do + observer = DeveloperObserver.instance # activate + observer.calls.clear + + ActiveRecord::Base.observers.disable DeveloperObserver do + Developer.create! :name => 'Ancestor', :salary => 100000 + SpecialDeveloper.create! :name => 'Descendent', :salary => 100000 + end + + assert_equal [], observer.calls + end + def test_observer_is_called_once observer = DeveloperObserver.instance # activate observer.calls.clear diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index a0cb5dbdc5..0ab4f30363 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -14,7 +14,7 @@ class MethodScopingTest < ActiveRecord::TestCase def test_set_conditions Developer.send(:with_scope, :find => { :conditions => 'just a test...' }) do - assert_match '(just a test...)', Developer.scoped.arel.to_sql + assert_match '(just a test...)', Developer.scoped.to_sql end end @@ -274,7 +274,7 @@ class NestedScopingTest < ActiveRecord::TestCase Developer.send(:with_scope, :find => { :conditions => 'salary = 80000' }) do Developer.send(:with_scope, :find => { :limit => 10 }) do devs = Developer.scoped - assert_match '(salary = 80000)', devs.arel.to_sql + assert_match '(salary = 80000)', devs.to_sql assert_equal 10, devs.taken end end @@ -308,7 +308,7 @@ class NestedScopingTest < ActiveRecord::TestCase Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do Developer.send(:with_scope, :find => { :conditions => 'salary = 80000' }) do devs = Developer.scoped - assert_match "(name = 'David') AND (salary = 80000)", devs.arel.to_sql + assert_match "(name = 'David') AND (salary = 80000)", devs.to_sql assert_equal(1, Developer.count) end Developer.send(:with_scope, :find => { :conditions => "name = 'Maiha'" }) do @@ -321,7 +321,7 @@ class NestedScopingTest < ActiveRecord::TestCase Developer.send(:with_scope, :find => { :conditions => 'salary = 80000', :limit => 10 }) do Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do devs = Developer.scoped - assert_match "(salary = 80000) AND (name = 'David')", devs.arel.to_sql + assert_match "(salary = 80000) AND (name = 'David')", devs.to_sql assert_equal 10, devs.taken end end diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 7e3da145e5..05a41d8a0a 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -26,7 +26,7 @@ class PrimaryKeysTest < ActiveRecord::TestCase def test_to_key_with_primary_key_after_destroy topic = Topic.find(1) topic.destroy - assert_equal nil, topic.to_key + assert_equal [1], topic.to_key end def test_integer_key diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index f2d177d834..1e2093273e 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -170,7 +170,7 @@ class NestedRelationScopingTest < ActiveRecord::TestCase Developer.where('salary = 80000').scoping do Developer.limit(10).scoping do devs = Developer.scoped - assert_match '(salary = 80000)', devs.arel.to_sql + assert_match '(salary = 80000)', devs.to_sql assert_equal 10, devs.taken end end @@ -524,4 +524,22 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal 1, DeveloperWithIncludes.where(:audit_logs => { :message => 'foo' }).count end + + def test_default_scope_is_threadsafe + if in_memory_db? + skip "in memory db can't share a db between threads" + end + + threads = [] + assert_not_equal 1, ThreadsafeDeveloper.unscoped.count + + threads << Thread.new do + Thread.current[:long_default_scope] = true + assert_equal 1, ThreadsafeDeveloper.all.count + end + threads << Thread.new do + assert_equal 1, ThreadsafeDeveloper.all.count + end + threads.each(&:join) + end end diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index f182a7fa97..4dc9fff9fd 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -227,3 +227,12 @@ class EagerDeveloperWithCallableDefaultScope < ActiveRecord::Base default_scope OpenStruct.new(:call => includes(:projects)) end + +class ThreadsafeDeveloper < ActiveRecord::Base + self.table_name = 'developers' + + def self.default_scope + sleep 0.05 if Thread.current[:long_default_scope] + limit(1) + end +end diff --git a/activesupport/lib/active_support/core_ext/io.rb b/activesupport/lib/active_support/core_ext/io.rb new file mode 100644 index 0000000000..75f1055191 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/io.rb @@ -0,0 +1,15 @@ +if RUBY_VERSION < '1.9.2' + +# :stopdoc: +class IO + def self.binread(name, length = nil, offset = nil) + return File.read name unless length || offset + File.open(name, 'rb') { |f| + f.seek offset if offset + f.read length + } + end +end +# :startdoc: + +end diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index 149b849b12..bf7e009290 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -106,26 +106,32 @@ class Module unless options.is_a?(Hash) && to = options[:to] raise ArgumentError, "Delegation needs a target. Supply an options hash with a :to key as the last argument (e.g. delegate :hello, :to => :greeter)." end + prefix, to, allow_nil = options[:prefix], options[:to], options[:allow_nil] - if options[:prefix] == true && options[:to].to_s =~ /^[^a-z_]/ + if prefix == true && to.to_s =~ /^[^a-z_]/ raise ArgumentError, "Can only automatically set the delegation prefix when delegating to a method." end - prefix = options[:prefix] ? "#{options[:prefix] == true ? to : options[:prefix]}_" : '' + method_prefix = + if prefix + "#{prefix == true ? to : prefix}_" + else + '' + end file, line = caller.first.split(':', 2) line = line.to_i methods.each do |method| on_nil = - if options[:allow_nil] + if allow_nil 'return' else - %(raise "#{self}##{prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}") + %(raise "#{self}##{method_prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}") end module_eval(<<-EOS, file, line - 5) - def #{prefix}#{method}(*args, &block) # def customer_name(*args, &block) + def #{method_prefix}#{method}(*args, &block) # def customer_name(*args, &block) #{to}.__send__(#{method.inspect}, *args, &block) # client.__send__(:name, *args, &block) rescue NoMethodError # rescue NoMethodError if #{to}.nil? # if client.nil? diff --git a/activesupport/lib/active_support/core_ext/time/calculations.rb b/activesupport/lib/active_support/core_ext/time/calculations.rb index c5fbbcf890..a15a06d0e4 100644 --- a/activesupport/lib/active_support/core_ext/time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/time/calculations.rb @@ -1,5 +1,6 @@ require 'active_support/duration' require 'active_support/core_ext/time/zones' +require 'active_support/core_ext/time/conversions' class Time COMMON_YEAR_DAYS_IN_MONTH = [nil, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31] diff --git a/activesupport/lib/active_support/gzip.rb b/activesupport/lib/active_support/gzip.rb index 62f9c9aa2e..9651f02c73 100644 --- a/activesupport/lib/active_support/gzip.rb +++ b/activesupport/lib/active_support/gzip.rb @@ -1,5 +1,6 @@ require 'zlib' require 'stringio' +require 'active_support/core_ext/string/encoding' module ActiveSupport # A convenient wrapper for the zlib standard library that allows compression/decompression of strings with gzip. diff --git a/activesupport/lib/active_support/inflections.rb b/activesupport/lib/active_support/inflections.rb index 06ceccdb22..daf2a1e1d9 100644 --- a/activesupport/lib/active_support/inflections.rb +++ b/activesupport/lib/active_support/inflections.rb @@ -54,6 +54,7 @@ module ActiveSupport inflect.irregular('sex', 'sexes') inflect.irregular('move', 'moves') inflect.irregular('cow', 'kine') + inflect.irregular('zombie', 'zombies') inflect.uncountable(%w(equipment information rice money species series fish sheep jeans)) end diff --git a/activesupport/test/core_ext/io_test.rb b/activesupport/test/core_ext/io_test.rb new file mode 100644 index 0000000000..b9abf685da --- /dev/null +++ b/activesupport/test/core_ext/io_test.rb @@ -0,0 +1,23 @@ +require 'abstract_unit' + +require 'active_support/core_ext/io' + +class IOTest < Test::Unit::TestCase + def test_binread_one_arg + assert_equal File.read(__FILE__), IO.binread(__FILE__) + end + + def test_binread_two_args + assert_equal File.read(__FILE__).bytes.first(10).pack('C*'), + IO.binread(__FILE__, 10) + end + + def test_binread_three_args + actual = IO.binread(__FILE__, 5, 10) + expected = File.open(__FILE__, 'rb') { |f| + f.seek 10 + f.read 5 + } + assert_equal expected, actual + end +end diff --git a/railties/guides/rails_guides/generator.rb b/railties/guides/rails_guides/generator.rb index d304512ff7..4682ead66e 100644 --- a/railties/guides/rails_guides/generator.rb +++ b/railties/guides/rails_guides/generator.rb @@ -199,50 +199,10 @@ module RailsGuides end def textile(body, lite_mode=false) - # If the issue with notextile is fixed just remove the wrapper. - with_workaround_for_notextile(body) do |body| - t = RedCloth.new(body) - t.hard_breaks = false - t.lite_mode = lite_mode - t.to_html(:notestuff, :plusplus, :code) - end - end - - # For some reason the notextile tag does not always turn off textile. See - # LH ticket of the security guide (#7). As a temporary workaround we deal - # with code blocks by hand. - def with_workaround_for_notextile(body) - code_blocks = [] - - body.gsub!(%r{<(yaml|shell|ruby|erb|html|sql|plain)>(.*?)</\1>}m) do |m| - brush = case $1 - when 'ruby', 'sql', 'plain' - $1 - when 'erb' - 'ruby; html-script: true' - when 'html' - 'xml' # html is understood, but there are .xml rules in the CSS - else - 'plain' - end - - code_blocks.push(<<HTML) -<notextile> -<div class="code_container"> -<pre class="brush: #{brush}; gutter: false; toolbar: false"> -#{ERB::Util.h($2).strip} -</pre> -</div> -</notextile> -HTML - "\ndirty_workaround_for_notextile_#{code_blocks.size - 1}\n" - end - - body = yield body - - body.gsub(%r{<p>dirty_workaround_for_notextile_(\d+)</p>}) do |_| - code_blocks[$1.to_i] - end + t = RedCloth.new(body) + t.hard_breaks = false + t.lite_mode = lite_mode + t.to_html(:notestuff, :plusplus, :code) end def warn_about_broken_links(html) diff --git a/railties/guides/rails_guides/textile_extensions.rb b/railties/guides/rails_guides/textile_extensions.rb index b3e0e32357..4677fae504 100644 --- a/railties/guides/rails_guides/textile_extensions.rb +++ b/railties/guides/rails_guides/textile_extensions.rb @@ -33,11 +33,30 @@ module RailsGuides body.gsub!('<plus>', '+') end + def brush_for(code_type) + case code_type + when 'ruby', 'sql', 'plain' + code_type + when 'erb' + 'ruby; html-script: true' + when 'html' + 'xml' # html is understood, but there are .xml rules in the CSS + else + 'plain' + end + end + def code(body) body.gsub!(%r{<(yaml|shell|ruby|erb|html|sql|plain)>(.*?)</\1>}m) do |m| - es = ERB::Util.h($2) - css_class = $1.in?(['erb', 'shell']) ? 'html' : $1 - %{<notextile><div class="code_container"><code class="#{css_class}">#{es}</code></div></notextile>} + <<HTML +<notextile> +<div class="code_container"> +<pre class="brush: #{brush_for($1)}; gutter: false; toolbar: false"> +#{ERB::Util.h($2).strip} +</pre> +</div> +</notextile> +HTML end end end diff --git a/railties/guides/source/i18n.textile b/railties/guides/source/i18n.textile index 0c8e4e974d..5a6343472c 100644 --- a/railties/guides/source/i18n.textile +++ b/railties/guides/source/i18n.textile @@ -1,4 +1,4 @@ -lh2. Rails Internationalization (I18n) API +h2. Rails Internationalization (I18n) API The Ruby I18n (shorthand for _internationalization_) gem which is shipped with Ruby on Rails (starting from Rails 2.2) provides an easy-to-use and extensible framework for *translating your application to a single custom language* other than English or for *providing multi-language support* in your application. diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 975e159999..cd850b6a75 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -37,6 +37,7 @@ module Rails @assets.paths = [] @assets.precompile = [ /\w+\.(?!js|css).+/, /application.(css|js)$/ ] @assets.prefix = "/assets" + @assets.version = '' @assets.cache_store = [ :file_store, "#{root}/tmp/cache/assets/" ] @assets.js_compressor = nil diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt index 06ed890e05..de56d47688 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt @@ -15,8 +15,8 @@ config.assets.compress = true # Specifies the header that your server uses for sending files - # (comment out if your front-end server doesn't support this) - config.action_dispatch.x_sendfile_header = "X-Sendfile" # Use 'X-Accel-Redirect' for nginx + # config.action_dispatch.x_sendfile_header = "X-Sendfile" # for apache + # config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for nginx # Force all access to the app over SSL, use Strict-Transport-Security, and use secure cookies. # config.force_ssl = true diff --git a/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb b/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb index c46422437d..4baa2110e7 100644 --- a/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb +++ b/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb @@ -11,8 +11,6 @@ module Rails def app if mountable? directory "app" - template "app/views/layouts/application.html.erb.tt", - "app/views/layouts/#{name}/application.html.erb" empty_directory_with_gitkeep "app/assets/images/#{name}" elsif full? empty_directory_with_gitkeep "app/models" diff --git a/railties/lib/rails/generators/rails/plugin_new/templates/app/views/layouts/application.html.erb.tt b/railties/lib/rails/generators/rails/plugin_new/templates/app/views/layouts/%name%/application.html.erb.tt index 01550dec2f..01550dec2f 100644 --- a/railties/lib/rails/generators/rails/plugin_new/templates/app/views/layouts/application.html.erb.tt +++ b/railties/lib/rails/generators/rails/plugin_new/templates/app/views/layouts/%name%/application.html.erb.tt diff --git a/railties/lib/rails/tasks/documentation.rake b/railties/lib/rails/tasks/documentation.rake index 5d613e0698..ca8875ad9b 100644 --- a/railties/lib/rails/tasks/documentation.rake +++ b/railties/lib/rails/tasks/documentation.rake @@ -8,6 +8,8 @@ end # Monkey-patch to remove redoc'ing and clobber descriptions to cut down on rake -T noise class RDocTaskWithoutDescriptions < RDoc::Task + include ::Rake::DSL + def define task rdoc_task_name diff --git a/railties/test/application/assets_test.rb b/railties/test/application/assets_test.rb index 802b737483..38dd3f5a3f 100644 --- a/railties/test/application/assets_test.rb +++ b/railties/test/application/assets_test.rb @@ -80,7 +80,7 @@ module ApplicationTests Dir.chdir(app_path){ `bundle exec rake assets:clean` } end - files = Dir["#{app_path}/public/assets/**/*"] + files = Dir["#{app_path}/public/assets/**/*", "#{app_path}/tmp/cache/*"] assert_equal 0, files.length, "Expected no assets, but found #{files.join(', ')}" end diff --git a/railties/test/application/middleware/sendfile_test.rb b/railties/test/application/middleware/sendfile_test.rb index c7a1c573f9..d2ad2668bb 100644 --- a/railties/test/application/middleware/sendfile_test.rb +++ b/railties/test/application/middleware/sendfile_test.rb @@ -27,11 +27,12 @@ module ApplicationTests end # x_sendfile_header middleware - test "config.action_dispatch.x_sendfile_header defaults to ''" do + test "config.action_dispatch.x_sendfile_header defaults to nil" do make_basic_app simple_controller get "/" + assert !last_response.headers["X-Sendfile"] assert_equal File.read(__FILE__), last_response.body end diff --git a/tasks/release.rb b/tasks/release.rb index 01950b227d..2422efa786 100644 --- a/tasks/release.rb +++ b/tasks/release.rb @@ -4,11 +4,11 @@ root = File.expand_path('../../', __FILE__) version = File.read("#{root}/RAILS_VERSION").strip tag = "v#{version}" -directory "dist" +directory "pkg" (FRAMEWORKS + ['rails']).each do |framework| namespace framework do - gem = "dist/#{framework}-#{version}.gem" + gem = "pkg/#{framework}-#{version}.gem" gemspec = "#{framework}.gemspec" task :clean do @@ -41,10 +41,10 @@ directory "dist" File.open(file, 'w') { |f| f.write ruby } end - task gem => %w(update_version_rb dist) do + task gem => %w(update_version_rb pkg) do cmd = "" cmd << "cd #{framework} && " unless framework == "rails" - cmd << "gem build #{gemspec} && mv #{framework}-#{version}.gem #{root}/dist/" + cmd << "gem build #{gemspec} && mv #{framework}-#{version}.gem #{root}/pkg/" sh cmd end @@ -104,14 +104,14 @@ namespace :all do end task :commit do - File.open('dist/commit_message.txt', 'w') do |f| + File.open('pkg/commit_message.txt', 'w') do |f| f.puts "# Preparing for #{version} release\n" f.puts f.puts "# UNCOMMENT THE LINE ABOVE TO APPROVE THIS COMMIT" end - sh "git add . && git commit --verbose --template=dist/commit_message.txt" - rm_f "dist/commit_message.txt" + sh "git add . && git commit --verbose --template=pkg/commit_message.txt" + rm_f "pkg/commit_message.txt" end task :tag do |