From d60d7edce462f4602bfc9996689087a235b034c9 Mon Sep 17 00:00:00 2001 From: Sam Goldstein Date: Wed, 8 Jul 2009 11:45:26 -0700 Subject: Make it so AR attributes which conflict with object-private methods (e.g. system) don't 'randomly' cause NoMethodErrors Previously if you called this attribute before others, you'd get exceptions. But if it was the second-or-subsequent attribute you retrieved you'd get the correct behaviour. Signed-off-by: Michael Koziarski [#2808 state:committed] --- activerecord/lib/active_record/attribute_methods.rb | 13 +++++++++---- activerecord/test/cases/attribute_methods_test.rb | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 15358979c2..ecd2d57a5a 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -133,6 +133,7 @@ module ActiveRecord end private + # Suffixes a, ?, c become regexp /(a|\?|c)$/ def rebuild_attribute_method_regexp suffixes = attribute_method_suffixes.map { |s| Regexp.escape(s) } @@ -238,19 +239,17 @@ module ActiveRecord def method_missing(method_id, *args, &block) method_name = method_id.to_s - if self.class.private_method_defined?(method_name) - raise NoMethodError.new("Attempt to call private method", method_name, args) - end - # If we haven't generated any methods yet, generate them, then # see if we've created the method we're looking for. if !self.class.generated_methods? self.class.define_attribute_methods + guard_private_attribute_method!(method_name, args) if self.class.generated_methods.include?(method_name) return self.send(method_id, *args, &block) end end + guard_private_attribute_method!(method_name, args) if self.class.primary_key.to_s == method_name id elsif md = self.class.match_attribute_method?(method_name) @@ -371,6 +370,12 @@ module ActiveRecord end private + # prevent method_missing from calling private methods with #send + def guard_private_attribute_method!(method_name, args) + if self.class.private_method_defined?(method_name) + raise NoMethodError.new("Attempt to call private method", method_name, args) + end + end def missing_attribute(attr_name, stack) raise ActiveRecord::MissingAttributeError, "missing attribute: #{attr_name}", stack diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 17ed302465..183be1e2f9 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -277,6 +277,22 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::UnknownAttributeError) { @target.new.attributes = { :title => "Ants in pants" } } end + def test_read_attribute_overwrites_private_method_not_considered_implemented + # simulate a model with a db column that shares its name an inherited + # private method (e.g. Object#system) + # + Object.class_eval do + private + def title; "private!"; end + end + assert !@target.instance_method_already_implemented?(:title) + topic = @target.new + assert_equal nil, topic.title + + Object.send(:undef_method, :title) # remove test method from object + end + + private def time_related_columns_on_topic Topic.columns.select{|c| [:time, :date, :datetime, :timestamp].include?(c.type)}.map(&:name) -- cgit v1.2.3 From 01d92021e69f54def1ec8103b2b99f907dd88ec4 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 10 Jul 2009 11:34:39 -0500 Subject: Switch scaffolding templates to HTML 5 doctype --- railties/html/404.html | 33 ++++++------ railties/html/422.html | 33 ++++++------ railties/html/500.html | 31 +++++------ railties/html/index.html | 61 +++++++++++----------- .../components/scaffold/templates/layout.html.erb | 8 ++- .../scaffold/templates/view_index.html.erb | 3 ++ 6 files changed, 80 insertions(+), 89 deletions(-) diff --git a/railties/html/404.html b/railties/html/404.html index eff660b90c..88ee108e90 100644 --- a/railties/html/404.html +++ b/railties/html/404.html @@ -1,23 +1,20 @@ - - - - + + The page you were looking for doesn't exist (404) - + @@ -27,4 +24,4 @@

You may have mistyped the address or the page may have moved.

- \ No newline at end of file + diff --git a/railties/html/422.html b/railties/html/422.html index b54e4a3cad..9c3c96670b 100644 --- a/railties/html/422.html +++ b/railties/html/422.html @@ -1,23 +1,20 @@ - - - - + + The change you wanted was rejected (422) - + @@ -27,4 +24,4 @@

Maybe you tried to change something you didn't have access to.

- \ No newline at end of file + diff --git a/railties/html/500.html b/railties/html/500.html index ec3bbf02c4..f71c86e652 100644 --- a/railties/html/500.html +++ b/railties/html/500.html @@ -1,23 +1,20 @@ - - - - + + We're sorry, but something went wrong (500) - + diff --git a/railties/html/index.html b/railties/html/index.html index cd337dc74c..ff2dfd3193 100644 --- a/railties/html/index.html +++ b/railties/html/index.html @@ -1,5 +1,4 @@ - + @@ -14,20 +13,20 @@ font-size: 13px; color: #333; } - + h1 { font-size: 28px; color: #000; } - + a {color: #03c} a:hover { background-color: #03c; color: white; text-decoration: none; } - - + + #page { background-color: #f0f0f0; width: 750px; @@ -35,7 +34,7 @@ margin-left: auto; margin-right: auto; } - + #content { float: left; background-color: white; @@ -44,7 +43,7 @@ padding: 25px; width: 500px; } - + #sidebar { float: right; width: 175px; @@ -53,7 +52,7 @@ #footer { clear: both; } - + #header, #about, #getting-started { padding-left: 75px; @@ -73,14 +72,14 @@ font-weight: normal; font-size: 16px; } - - + + #about h3 { margin: 0; margin-bottom: 10px; font-size: 14px; } - + #about-content { background-color: #ffd; border: 1px solid #fc0; @@ -113,8 +112,8 @@ margin: 0; padding: 10px; } - - + + #getting-started { border-top: 1px solid #ccc; margin-top: 25px; @@ -150,8 +149,8 @@ color: #555; font-size: 13px; } - - + + #search { margin: 0; padding-top: 10px; @@ -163,8 +162,8 @@ margin: 2px; } #search-text {width: 170px} - - + + #sidebar ul { margin-left: 0; padding-left: 0; @@ -181,7 +180,7 @@ #sidebar ul.links li { margin-bottom: 5px; } - + @@ -194,11 +193,11 @@ onComplete: function() {new Effect.BlindDown('about-content', {duration: 0.25})} }); } else { - new Effect[Element.visible('about-content') ? + new Effect[Element.visible('about-content') ? 'BlindUp' : 'BlindDown']('about-content', {duration: 0.25}); } } - + window.onload = function() { $('search-text').value = ''; $('search').onsubmit = function() { @@ -218,7 +217,7 @@ the Rails site - +

  • Join the community

      @@ -227,7 +226,7 @@
    • Wiki
    • - +

  • Browse the documentation

      @@ -250,17 +249,17 @@

      About your application’s environment

      - +

      Getting started

      Here’s how to get rolling:

      - -
        + +
        1. -

          Use script/generate to create your models and controllers

          +

          Use script/generate to create your models and controllers

          To see all available options, run it without parameters.

          2. - +

        2. Set up a default route and remove or rename this file

          Routes are set up in config/routes.rb.

          @@ -268,13 +267,13 @@

        3. Create your database

          -

          Run rake db:migrate to create your database. If you're not using SQLite (the default), edit config/database.yml with your username and password.

          +

          Run rake db:migrate to create your database. If you're not using SQLite (the default), edit config/database.yml with your username and password.

      - + - \ No newline at end of file + diff --git a/railties/lib/rails_generator/generators/components/scaffold/templates/layout.html.erb b/railties/lib/rails_generator/generators/components/scaffold/templates/layout.html.erb index ebc97f8130..aacfbe4a8f 100644 --- a/railties/lib/rails_generator/generators/components/scaffold/templates/layout.html.erb +++ b/railties/lib/rails_generator/generators/components/scaffold/templates/layout.html.erb @@ -1,9 +1,7 @@ - - - + + - + <%= controller_class_name %>: <%%= controller.action_name %> <%%= stylesheet_link_tag 'scaffold' %> diff --git a/railties/lib/rails_generator/generators/components/scaffold/templates/view_index.html.erb b/railties/lib/rails_generator/generators/components/scaffold/templates/view_index.html.erb index 2e603d5b4a..69beb388db 100644 --- a/railties/lib/rails_generator/generators/components/scaffold/templates/view_index.html.erb +++ b/railties/lib/rails_generator/generators/components/scaffold/templates/view_index.html.erb @@ -5,6 +5,9 @@ <% for attribute in attributes -%> <%= attribute.column.human_name %> <% end -%> + + + <%% @<%= plural_name %>.each do |<%= singular_name %>| %> -- cgit v1.2.3 From e83a05af076637cc78f0408f5810d5f8f965e10c Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 11 Jul 2009 16:20:58 -0500 Subject: Integrate AMo XML serializer into AR --- activemodel/lib/active_model/serializers/xml.rb | 49 +++++---- .../active_record/serializers/xml_serializer.rb | 113 +-------------------- 2 files changed, 31 insertions(+), 131 deletions(-) diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb index d187859b44..7cdd281223 100644 --- a/activemodel/lib/active_model/serializers/xml.rb +++ b/activemodel/lib/active_model/serializers/xml.rb @@ -17,6 +17,15 @@ module ActiveModel @value = compute_value end + # There is a significant speed improvement if the value + # does not need to be escaped, as tag! escapes all values + # to ensure that valid XML is generated. For known binary + # values, it is at least an order of magnitude faster to + # Base64 encode binary values and directly put them in the + # output XML than to pass the original value or the Base64 + # encoded value to the tag! method. It definitely makes + # no sense to Base64 encode the value and then give it to + # tag!, since that just adds additional overhead. def needs_encoding? ![ :binary, :date, :datetime, :boolean, :float, :integer ].include?(type) end @@ -105,28 +114,6 @@ module ActiveModel end end - def add_attributes - (serializable_attributes + serializable_method_attributes).each do |attribute| - add_tag(attribute) - end - end - - def add_procs - if procs = options.delete(:procs) - [ *procs ].each do |proc| - proc.call(options) - end - end - end - - def add_tag(attribute) - builder.tag!( - reformat_name(attribute.name), - attribute.value.to_s, - attribute.decorations(!options[:skip_types]) - ) - end - def serialize args = [root] @@ -152,6 +139,24 @@ module ActiveModel name = name.camelize if camelize? dasherize? ? name.dasherize : name end + + def add_attributes + (serializable_attributes + serializable_method_attributes).each do |attribute| + builder.tag!( + reformat_name(attribute.name), + attribute.value.to_s, + attribute.decorations(!options[:skip_types]) + ) + end + end + + def add_procs + if procs = options.delete(:procs) + [ *procs ].each do |proc| + proc.call(options) + end + end + end end def to_xml(options = {}, &block) diff --git a/activerecord/lib/active_record/serializers/xml_serializer.rb b/activerecord/lib/active_record/serializers/xml_serializer.rb index c3811caa53..253fa03785 100644 --- a/activerecord/lib/active_record/serializers/xml_serializer.rb +++ b/activerecord/lib/active_record/serializers/xml_serializer.rb @@ -164,42 +164,9 @@ module ActiveRecord #:nodoc: end end - class XmlSerializer < ActiveModel::Serializer #:nodoc: + class XmlSerializer < ActiveModel::Serializers::Xml::Serializer #:nodoc: include Serialization::RecordSerializer - def builder - @builder ||= begin - require 'builder' unless defined? ::Builder - options[:indent] ||= 2 - builder = options[:builder] ||= ::Builder::XmlMarkup.new(:indent => options[:indent]) - - unless options[:skip_instruct] - builder.instruct! - options[:skip_instruct] = true - end - - builder - end - end - - def root - root = (options[:root] || @serializable.class.to_s.underscore).to_s - reformat_name(root) - end - - def dasherize? - !options.has_key?(:dasherize) || options[:dasherize] - end - - def camelize? - options.has_key?(:camelize) && options[:camelize] - end - - def reformat_name(name) - name = name.camelize if camelize? - dasherize? ? name.dasherize : name - end - def serializable_attributes serializable_attribute_names.collect { |name| Attribute.new(name, @serializable) } end @@ -211,28 +178,6 @@ module ActiveRecord #:nodoc: end end - def add_attributes - (serializable_attributes + serializable_method_attributes).each do |attribute| - add_tag(attribute) - end - end - - def add_procs - if procs = options.delete(:procs) - [ *procs ].each do |proc| - proc.call(options) - end - end - end - - def add_tag(attribute) - builder.tag!( - reformat_name(attribute.name), - attribute.value.to_s, - attribute.decorations(!options[:skip_types]) - ) - end - def add_associations(association, records, opts) if records.is_a?(Enumerable) tag = reformat_name(association.to_s) @@ -282,50 +227,10 @@ module ActiveRecord #:nodoc: end end - class Attribute #:nodoc: - attr_reader :name, :value, :type - - def initialize(name, record) - @name, @record = name, record - - @type = compute_type - @value = compute_value - end - - # There is a significant speed improvement if the value - # does not need to be escaped, as tag! escapes all values - # to ensure that valid XML is generated. For known binary - # values, it is at least an order of magnitude faster to - # Base64 encode binary values and directly put them in the - # output XML than to pass the original value or the Base64 - # encoded value to the tag! method. It definitely makes - # no sense to Base64 encode the value and then give it to - # tag!, since that just adds additional overhead. - def needs_encoding? - ![ :binary, :date, :datetime, :boolean, :float, :integer ].include?(type) - end - - def decorations(include_types = true) - decorations = {} - - if type == :binary - decorations[:encoding] = 'base64' - end - - if include_types && type != :string - decorations[:type] = type - end - - if value.nil? - decorations[:nil] = true - end - - decorations - end - + class Attribute < ActiveModel::Serializers::Xml::Serializer::Attribute #:nodoc: protected def compute_type - type = @record.class.serialized_attributes.has_key?(name) ? :yaml : @record.class.columns_hash[name].type + type = @serializable.class.serialized_attributes.has_key?(name) ? :yaml : @serializable.class.columns_hash[name].type case type when :text @@ -336,22 +241,12 @@ module ActiveRecord #:nodoc: type end end - - def compute_value - value = @record.send(name) - - if formatter = Hash::XML_FORMATTING[type.to_s] - value ? formatter.call(value) : nil - else - value - end - end end class MethodAttribute < Attribute #:nodoc: protected def compute_type - Hash::XML_TYPE_NAMES[@record.send(name).class.name] || :string + Hash::XML_TYPE_NAMES[@serializable.send(name).class.name] || :string end end end -- cgit v1.2.3 From c863388039ff6e97447be86184d41fbf30f0f389 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 11 Jul 2009 16:59:11 -0500 Subject: Add observing hooks to ARes --- activeresource/lib/active_resource/base.rb | 11 +++++-- activeresource/test/observing_test.rb | 53 ++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 activeresource/test/observing_test.rb diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index f919f911e4..88a431a6d9 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -804,7 +804,8 @@ module ActiveResource # my_company.size = 10 # my_company.save # sends PUT /companies/1 (update) def save - new? ? create : update + notify(:before_save) + (new? ? create : update).tap { notify(:after_save) } end # Deletes the resource from the remote service. @@ -820,7 +821,8 @@ module ActiveResource # new_person.destroy # Person.find(new_id) # 404 (Resource Not Found) def destroy - connection.delete(element_path, self.class.headers) + notify(:before_destroy) + connection.delete(element_path, self.class.headers).tap { notify(:after_destroy) } end # Evaluates to true if this resource is not new? and is @@ -995,16 +997,20 @@ module ActiveResource # Update the resource on the remote service. def update + notify(:before_update) connection.put(element_path(prefix_options), encode, self.class.headers).tap do |response| load_attributes_from_response(response) + notify(:after_update) end end # Create (i.e., \save to the remote service) the \new resource. def create + notify(:before_create) connection.post(collection_path, encode, self.class.headers).tap do |response| self.id = id_from_response(response) load_attributes_from_response(response) + notify(:after_create) end end @@ -1088,5 +1094,6 @@ module ActiveResource class Base extend ActiveModel::Naming include CustomMethods, Validations + include ActiveModel::Observing end end diff --git a/activeresource/test/observing_test.rb b/activeresource/test/observing_test.rb new file mode 100644 index 0000000000..334b256772 --- /dev/null +++ b/activeresource/test/observing_test.rb @@ -0,0 +1,53 @@ +require 'abstract_unit' + +class ObservingTest < Test::Unit::TestCase + cattr_accessor :history + + class PersonObserver < ActiveModel::Observer + observe :person + + %w( after_create after_destroy after_save after_update + before_create before_destroy before_save before_update).each do |method| + define_method(method) { log method } + end + + private + def log(method) + (ObservingTest.history ||= []) << method.to_sym + end + end + + def setup + @matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person') + + ActiveResource::HttpMock.respond_to do |mock| + mock.get "/people/1.xml", {}, @matz + mock.post "/people.xml", {}, @matz, 201, 'Location' => '/people/1.xml' + mock.put "/people/1.xml", {}, nil, 204 + mock.delete "/people/1.xml", {}, nil, 200 + end + + PersonObserver.instance + end + + def teardown + self.history = nil + end + + def test_create_fires_save_and_create_notifications + rick = Person.create(:name => 'Rick') + assert_equal [:before_save, :before_create, :after_create, :after_save], self.history + end + + def test_update_fires_save_and_update_notifications + person = Person.find(1) + person.save + assert_equal [:before_save, :before_update, :after_update, :after_save], self.history + end + + def test_destroy_fires_destroy_notifications + person = Person.find(1) + person.destroy + assert_equal [:before_destroy, :after_destroy], self.history + end +end -- cgit v1.2.3