diff options
20 files changed, 231 insertions, 72 deletions
diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 4f4cb96a74..aaed0d750f 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -42,20 +42,6 @@ module ActionDispatch attr_reader :cache_control, :etag alias :etag? :etag - def initialize(*) - super - - @cache_control = {} - @etag = self["ETag"] - - if cache_control = self["Cache-Control"] - cache_control.split(/,\s*/).each do |segment| - first, last = segment.split("=") - @cache_control[first.to_sym] = last || true - end - end - end - def last_modified if last = headers['Last-Modified'] Time.httpdate(last) @@ -77,6 +63,18 @@ module ActionDispatch private + def prepare_cache_control! + @cache_control = {} + @etag = self["ETag"] + + if cache_control = self["Cache-Control"] + cache_control.split(/,\s*/).each do |segment| + first, last = segment.split("=") + @cache_control[first.to_sym] = last || true + end + end + end + def handle_conditional_get! if etag? || last_modified? || !@cache_control.empty? set_conditional_cache_control! diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 1f4f3ac0da..3a6b1da4fd 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -56,26 +56,25 @@ module ActionDispatch # :nodoc: cattr_accessor(:default_charset) { "utf-8" } - module Setup - def initialize(status = 200, header = {}, body = []) - self.body, self.header, self.status = body, header, status + include Rack::Response::Helpers + include ActionDispatch::Http::Cache::Response - @sending_file = false - @blank = false + def initialize(status = 200, header = {}, body = []) + self.body, self.header, self.status = body, header, status - if content_type = self["Content-Type"] - type, charset = content_type.split(/;\s*charset=/) - @content_type = Mime::Type.lookup(type) - @charset = charset || "UTF-8" - end + @sending_file = false + @blank = false - yield self if block_given? + if content_type = self["Content-Type"] + type, charset = content_type.split(/;\s*charset=/) + @content_type = Mime::Type.lookup(type) + @charset = charset || "UTF-8" end - end - include Rack::Response::Helpers - include Setup - include ActionDispatch::Http::Cache::Response + prepare_cache_control! + + yield self if block_given? + end def status=(status) @status = Rack::Utils.status_code(status) @@ -116,9 +115,32 @@ module ActionDispatch # :nodoc: EMPTY = " " + class BodyBuster #:nodoc: + def initialize(response) + @response = response + @body = "" + end + + def bust(body) + body.call(@response, self) + body.close if body.respond_to?(:close) + @body + end + + def write(string) + @body << string.to_s + end + end + def body=(body) @blank = true if body == EMPTY + if body.respond_to?(:call) + ActiveSupport::Deprecation.warn "Setting a Proc or an object that responds to call " \ + "in response_body is no longer supported", caller + body = BodyBuster.new(self).bust(body) + end + # Explicitly check for strings. This is *wrong* theoretically # but if we don't check this, the performance on string bodies # is bad on Ruby 1.8 (because strings responds to each then). @@ -150,6 +172,10 @@ module ActionDispatch # :nodoc: headers['Location'] = url end + def close + @body.close if @body.respond_to?(:close) + end + def to_a assign_default_content_type_and_charset! handle_conditional_get! diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb index ead7feb091..3b5f4e694f 100644 --- a/actionpack/lib/action_view/helpers/capture_helper.rb +++ b/actionpack/lib/action_view/helpers/capture_helper.rb @@ -135,8 +135,12 @@ module ActionView # for elements that will be fragment cached. def content_for(name, content = nil, &block) content = capture(&block) if block_given? - result = @view_flow.append(name, content) if content - result unless content + if content + @view_flow.append(name, content) + nil + else + @view_flow.get(name) + end end # The same as +content_for+ but when used with streaming flushes diff --git a/actionpack/test/dispatch/response_body_is_proc_test.rb b/actionpack/test/dispatch/response_body_is_proc_test.rb new file mode 100644 index 0000000000..fd94832624 --- /dev/null +++ b/actionpack/test/dispatch/response_body_is_proc_test.rb @@ -0,0 +1,37 @@ +require 'abstract_unit' + +class ResponseBodyIsProcTest < ActionDispatch::IntegrationTest + class TestController < ActionController::Base + def test + self.response_body = proc { |response, output| + output.write 'Hello' + } + end + end + + def test_simple_get + with_test_route_set do + assert_deprecated do + get '/test' + end + assert_response :success + assert_equal 'Hello', response.body + end + end + + private + + def with_test_route_set(options = {}) + with_routing do |set| + set.draw do + match ':action', :to => ::ResponseBodyIsProcTest::TestController + end + + @app = self.class.build_app(set) do |middleware| + middleware.delete "ActionDispatch::ShowExceptions" + end + + yield + end + end +end diff --git a/actionpack/test/template/capture_helper_test.rb b/actionpack/test/template/capture_helper_test.rb index 592c7da060..a9157e711c 100644 --- a/actionpack/test/template/capture_helper_test.rb +++ b/actionpack/test/template/capture_helper_test.rb @@ -38,7 +38,15 @@ class CaptureHelperTest < ActionView::TestCase assert_equal '<em>bar</em>', string end - def test_content_for + def test_capture_used_for_read + content_for :foo, "foo" + assert_equal "foo", content_for(:foo) + + content_for(:bar){ "bar" } + assert_equal "bar", content_for(:bar) + end + + def test_content_for_question_mark assert ! content_for?(:title) content_for :title, 'title' assert content_for?(:title) @@ -49,14 +57,14 @@ class CaptureHelperTest < ActionView::TestCase assert !content_for?(:title) provide :title, "hi" assert content_for?(:title) - assert_equal "hi", @view_flow.get(:title) + assert_equal "hi", content_for(:title) provide :title, "<p>title</p>" - assert_equal "hi<p>title</p>", @view_flow.get(:title) + assert_equal "hi<p>title</p>", content_for(:title) @view_flow = ActionView::OutputFlow.new provide :title, "hi" provide :title, "<p>title</p>".html_safe - assert_equal "hi<p>title</p>", @view_flow.get(:title) + assert_equal "hi<p>title</p>", content_for(:title) end def test_with_output_buffer_swaps_the_output_buffer_given_no_argument diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb index d4295e6afe..19639b1363 100644 --- a/activemodel/lib/active_model/serializers/xml.rb +++ b/activemodel/lib/active_model/serializers/xml.rb @@ -33,6 +33,7 @@ module ActiveModel protected def compute_type + return if value.nil? type = ActiveSupport::XmlMini::TYPE_NAMES[value.class.name] type ||= :string if value.respond_to?(:to_str) type ||= :yaml diff --git a/activemodel/test/cases/serializers/xml_serialization_test.rb b/activemodel/test/cases/serializers/xml_serialization_test.rb index b6a2f88667..8f5c196850 100644 --- a/activemodel/test/cases/serializers/xml_serialization_test.rb +++ b/activemodel/test/cases/serializers/xml_serialization_test.rb @@ -92,6 +92,10 @@ class XmlSerializationTest < ActiveModel::TestCase test "should serialize string" do assert_match %r{<name>aaron stack</name>}, @contact.to_xml end + + test "should serialize nil" do + assert_match %r{<pseudonyms nil=\"true\"></pseudonyms>}, @contact.to_xml(:methods => :pseudonyms) + end test "should serialize integer" do assert_match %r{<age type="integer">25</age>}, @contact.to_xml diff --git a/activemodel/test/models/contact.rb b/activemodel/test/models/contact.rb index f4f3078473..7bfc542afb 100644 --- a/activemodel/test/models/contact.rb +++ b/activemodel/test/models/contact.rb @@ -16,6 +16,10 @@ class Contact options.each { |name, value| send("#{name}=", value) } end + def pseudonyms + nil + end + def persisted? id end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 6cdec8c487..85a4f47b7d 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -94,7 +94,14 @@ module ActiveRecord end def build(attributes = {}, options = {}, &block) - build_or_create(:build, attributes, options, &block) + if attributes.is_a?(Array) + attributes.collect { |attr| build(attr, options, &block) } + else + add_to_target(build_record(attributes, options)) do |record| + yield(record) if block_given? + set_owner_attributes(record) + end + end end def create(attributes = {}, options = {}, &block) @@ -102,7 +109,16 @@ module ActiveRecord raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" end - build_or_create(:create, attributes, options, &block) + if attributes.is_a?(Array) + attributes.collect { |attr| create(attr, options, &block) } + else + transaction do + add_to_target(build_record(attributes, options)) do |record| + yield(record) if block_given? + insert_record(record) + end + end + end end def create!(attrs = {}, options = {}, &block) @@ -337,20 +353,18 @@ module ActiveRecord end def add_to_target(record) - transaction do - callback(:before_add, record) - yield(record) if block_given? - - if options[:uniq] && index = @target.index(record) - @target[index] = record - else - @target << record - end + callback(:before_add, record) + yield(record) if block_given? - callback(:after_add, record) - set_inverse_instance(record) + if options[:uniq] && index = @target.index(record) + @target[index] = record + else + @target << record end + callback(:after_add, record) + set_inverse_instance(record) + record end @@ -403,26 +417,13 @@ module ActiveRecord end + existing end - def build_or_create(method, attributes, options) - records = Array.wrap(attributes).map do |attrs| - record = build_record(attrs, options) - - add_to_target(record) do - yield(record) if block_given? - insert_record(record) if method == :create - end - end - - attributes.is_a?(Array) ? records : records.first - end - # Do the relevant stuff to insert the given record into the association collection. def insert_record(record, validate = true) raise NotImplementedError end def build_record(attributes, options) - reflection.build_association(scoped.scope_for_create.merge(attributes), options) + reflection.build_association(scoped.scope_for_create.merge(attributes || {}), options) end def delete_or_destroy(records, method) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 67af21c9a0..d07f9365b3 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1688,6 +1688,8 @@ MSG # user.name # => "Josh" # user.is_admin? # => true def assign_attributes(new_attributes, options = {}) + return unless new_attributes + attributes = new_attributes.stringify_keys role = options[:as] || :default diff --git a/activerecord/lib/active_record/identity_map.rb b/activerecord/lib/active_record/identity_map.rb index cad3865f03..b15b5a8133 100644 --- a/activerecord/lib/active_record/identity_map.rb +++ b/activerecord/lib/active_record/identity_map.rb @@ -12,7 +12,34 @@ module ActiveRecord # In order to enable IdentityMap, set <tt>config.active_record.identity_map = true</tt> # in your <tt>config/application.rb</tt> file. # - # IdentityMap is disabled by default. + # IdentityMap is disabled by default and still in development (i.e. use it with care). + # + # == Associations + # + # Active Record Identity Map does not track associations yet. For example: + # + # comment = @post.comments.first + # comment.post = nil + # @post.comments.include?(comment) #=> true + # + # Ideally, the example above would return false, removing the comment object from the + # post association when the association is nullified. This may cause side effects, as + # in the situation below, if Identity Map is enabled: + # + # Post.has_many :comments, :dependent => :destroy + # + # comment = @post.comments.first + # comment.post = nil + # comment.save + # Post.destroy(@post.id) + # + # Without using Identity Map, the code above will destroy the @post object leaving + # the comment object intact. However, once we enable Identity Map, the post loaded + # by Post.destroy is exactly the same object as the object @post. As the object @post + # still has the comment object in @post.comments, once Identity Map is enabled, the + # comment object will be accidently removed. + # + # This inconsistency is meant to be fixed in future Rails releases. # module IdentityMap diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb index 7e77aefb21..98e21db908 100644 --- a/activerecord/lib/active_record/session_store.rb +++ b/activerecord/lib/active_record/session_store.rb @@ -83,6 +83,8 @@ module ActiveRecord cattr_accessor :data_column_name self.data_column_name = 'data' + attr_accessible :session_id, :data, :marshaled_data + before_save :marshal_data! before_save :raise_on_session_data_overflow! diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 247decc67b..dc2481456b 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -66,6 +66,35 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 'exotic', bulb.name end + def test_create_from_association_with_nil_values_should_work + car = Car.create(:name => 'honda') + + bulb = car.bulbs.new(nil) + assert_equal 'defaulty', bulb.name + + bulb = car.bulbs.build(nil) + assert_equal 'defaulty', bulb.name + + bulb = car.bulbs.create(nil) + assert_equal 'defaulty', bulb.name + end + + def test_create_from_association_set_owner_attributes_by_passing_protection + Bulb.attr_protected :car_id + car = Car.create(:name => 'honda') + + bulb = car.bulbs.new + assert_equal car.id, bulb.car_id + + bulb = car.bulbs.build + assert_equal car.id, bulb.car_id + + bulb = car.bulbs.create + assert_equal car.id, bulb.car_id + ensure + Bulb.attr_protected :id + end + # When creating objects on the association, we must not do it within a scope (even though it # would be convenient), because this would cause that scope to be applied to any callbacks etc. def test_build_and_create_should_not_happen_within_scope diff --git a/activerecord/test/cases/mass_assignment_security_test.rb b/activerecord/test/cases/mass_assignment_security_test.rb index c81015b7c2..062a642e50 100644 --- a/activerecord/test/cases/mass_assignment_security_test.rb +++ b/activerecord/test/cases/mass_assignment_security_test.rb @@ -87,6 +87,10 @@ class MassAssignmentSecurityTest < ActiveRecord::TestCase end end + def test_mass_assigning_does_not_choke_on_nil + Firm.new.assign_attributes(nil) + end + def test_assign_attributes_uses_default_role_when_no_role_is_provided p = LoosePerson.new p.assign_attributes(attributes_hash) diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index b066575af8..57d1441128 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -336,6 +336,10 @@ class PersistencesTest < ActiveRecord::TestCase assert !Topic.find(1).approved? end + def test_update_attribute_does_not_choke_on_nil + assert Topic.find(1).update_attributes(nil) + end + def test_update_attribute_for_readonly_attribute minivan = Minivan.find('m1') assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') } diff --git a/activerecord/test/cases/session_store/session_test.rb b/activerecord/test/cases/session_store/session_test.rb index cee5ddd003..669c0b7b4d 100644 --- a/activerecord/test/cases/session_store/session_test.rb +++ b/activerecord/test/cases/session_store/session_test.rb @@ -21,6 +21,12 @@ module ActiveRecord assert_equal 'sessions', Session.table_name end + def test_accessible_attributes + assert Session.accessible_attributes.include?(:session_id) + assert Session.accessible_attributes.include?(:data) + assert Session.accessible_attributes.include?(:marshaled_data) + end + def test_create_table! assert !Session.table_exists? Session.create_table! diff --git a/activerecord/test/cases/xml_serialization_test.rb b/activerecord/test/cases/xml_serialization_test.rb index a6074b23e7..756c8a32eb 100644 --- a/activerecord/test/cases/xml_serialization_test.rb +++ b/activerecord/test/cases/xml_serialization_test.rb @@ -143,10 +143,7 @@ class NilXmlSerializationTest < ActiveRecord::TestCase end def test_should_serialize_yaml - assert %r{<preferences(.*)></preferences>}.match(@xml) - attributes = $1 - assert_match %r{type="yaml"}, attributes - assert_match %r{nil="true"}, attributes + assert_match %r{<preferences nil=\"true\"></preferences>}, @xml end end diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt index 612c614f2e..19294b3478 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt +++ b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt @@ -1,5 +1,8 @@ -// FIXME: Tell people that this is a manifest file, real code should go into discrete files -// FIXME: Tell people how Sprockets and CoffeeScript works +// This is a manifest file that'll be compiled into including all the files listed below. +// Add new JavaScript/Coffee code in separate files in this directory and they'll automatically +// be included in the compiled file accessible from http://example.com/assets/application.js +// It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the +// the compiled file. // <% unless options[:skip_javascript] -%> //= require <%= options[:javascript] %> diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css b/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css index f4b082ccc0..fc25b5723f 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css +++ b/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css @@ -1,5 +1,7 @@ /* - * FIXME: Introduce SCSS & Sprockets + * This is a manifest file that'll automatically include all the stylesheets available in this directory + * and any sub-directories. You're free to add application-wide styles to this file and they'll appear at + * the top of the compiled file, but it's generally better to create a new file per style scope. *= require_self *= require_tree . */
\ No newline at end of file diff --git a/railties/lib/rails/generators/rails/assets/assets_generator.rb b/railties/lib/rails/generators/rails/assets/assets_generator.rb index 80beb7abfe..fb215f91af 100644 --- a/railties/lib/rails/generators/rails/assets/assets_generator.rb +++ b/railties/lib/rails/generators/rails/assets/assets_generator.rb @@ -1,10 +1,10 @@ module Rails module Generators class AssetsGenerator < NamedBase - class_option :javascripts, :type => :boolean, :desc => "Generate javascripts" + class_option :javascripts, :type => :boolean, :desc => "Generate JavaScripts" class_option :stylesheets, :type => :boolean, :desc => "Generate stylesheets" - class_option :javascript_engine, :desc => "Engine for javascripts" + class_option :javascript_engine, :desc => "Engine for JavaScripts" class_option :stylesheet_engine, :desc => "Engine for stylesheets" def create_javascript_files |