From 48052d70ec065a3a8d9e6e121cab5ae857f8da1a Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 3 Jun 2006 00:01:08 +0000 Subject: to_xml fixes, features, and speedup. Closes #4989. git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4413 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 2 + activerecord/lib/active_record/base.rb | 210 ++++++++++++++++++--- activerecord/test/base_test.rb | 46 ++--- activerecord/test/fixtures/topics.yml | 4 +- activesupport/CHANGELOG | 2 + .../active_support/core_ext/array/conversions.rb | 12 +- .../active_support/core_ext/hash/conversions.rb | 57 ++++-- activesupport/test/core_ext/array_ext_test.rb | 20 ++ activesupport/test/core_ext/hash_ext_test.rb | 39 +++- 9 files changed, 320 insertions(+), 72 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 9f0e71aad4..fa55eb147d 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Allow models to override to_xml. #4989 [Blair Zajac ] + * PostgreSQL: don't ignore port when host is nil since it's often used to label the domain socket. #5247 [shimbo@is.naist.jp] * Records and arrays of records are bound as quoted ids. [Jeremy Kemper] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 735c8203aa..0a81681126 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1,3 +1,4 @@ +require 'base64' require 'yaml' require 'set' require 'active_record/deprecated_finders' @@ -84,7 +85,7 @@ module ActiveRecord #:nodoc: # The array form is to be used when the condition input is tainted and requires sanitization. The string form can # be used for statements that don't involve tainted data. Examples: # - # User < ActiveRecord::Base + # class User < ActiveRecord::Base # def self.authenticate_unsafely(user_name, password) # find(:first, :conditions => "user_name = '#{user_name}' AND password = '#{password}'") # end @@ -1024,7 +1025,7 @@ module ActiveRecord #:nodoc: safe_to_array(first) + safe_to_array(second) end - # Object#to_a is deprecated, though it does have the desired behaviour + # Object#to_a is deprecated, though it does have the desired behavior def safe_to_array(o) case o when NilClass @@ -1635,7 +1636,7 @@ module ActiveRecord #:nodoc: # Builds an XML document to represent the model. Some configuration is # availble through +options+, however more complicated cases should use - # Builder. + # override ActiveRecord's to_xml. # # By default the generated XML document will include the processing # instruction and all object's attributes. For example: @@ -1655,8 +1656,14 @@ module ActiveRecord #:nodoc: # 2004-04-15 # # - # This behaviour can be controlled with :only, :except, and :skip_instruct - # for instance: + # This behavior can be controlled with :only, :except, + # :skip_instruct, :skip_types and :dasherize. The :only and + # :except options are the same as for the #attributes method. + # The default is to dasherize all column names, to disable this, + # set :dasherize to false. To not have the column type included + # in the XML output, set :skip_types to false. + # + # For instance: # # topic.to_xml(:skip_instruct => true, :except => [ :id, :bonus_time, :written_on, :replies_count ]) # @@ -1697,49 +1704,198 @@ module ActiveRecord #:nodoc: # # To include any methods on the object(s) being called use :methods # - # firm.to_xml :methods => [ :calculated_earnings, :real_earnings ] + # firm.to_xml :methods => [ :calculated_earnings, :real_earnings ] # # # # ... normal attributes as shown above ... # 100000000000000000 # 5 # + # + # To call any Proc's on the object(s) use :procs. The Proc's + # are passed a modified version of the options hash that was + # given to #to_xml. + # + # proc = Proc.new { |options| options[:builder].tag!('abc', 'def') } + # firm.to_xml :procs => [ proc ] + # + # + # # ... normal attributes as shown above ... + # def + # + # + # You may override the to_xml method in your ActiveRecord::Base + # subclasses if you need to. The general form of doing this is + # + # class IHaveMyOwnXML < ActiveRecord::Base + # def to_xml(options = {}) + # options[:indent] ||= 2 + # xml = options[:builder] ||= Builder::XmlMarkup.new(:indent => options[:indent]) + # xml.instruct! unless options[:skip_instruct] + # xml.level_one do + # xml.tag!(:second_level, 'content') + # end + # end + # end def to_xml(options = {}) - options[:root] ||= self.class.to_s.underscore - options[:except] = Array(options[:except]) << self.class.inheritance_column unless options[:only] # skip type column - root_only_or_except = { :only => options[:only], :except => options[:except] } + options[:indent] ||= 2 + builder = options[:builder] ||= Builder::XmlMarkup.new(:indent => options[:indent]) - attributes_for_xml = attributes(root_only_or_except) - - if methods_to_call = options.delete(:methods) - [*methods_to_call].each do |meth| - attributes_for_xml[meth] = send(meth) - end + unless options[:skip_instruct] + builder.instruct! + options[:skip_instruct] = true end - - if include_associations = options.delete(:include) - include_has_options = include_associations.is_a?(Hash) - - for association in include_has_options ? include_associations.keys : Array(include_associations) - association_options = include_has_options ? include_associations[association] : root_only_or_except - case self.class.reflect_on_association(association).macro + root = (options[:root] || self.class.to_s.underscore).to_s + if dasherize = !options.has_key?(:dasherize) || options[:dasherize] + root = root.dasherize + end + + builder.tag!(root) do + # To replicate the behavior in ActiveRecord#attributes, + # :except takes precedence over :only. If :only is not set + # for a N level model but is set for the N+1 level models, + # then because :except is set to a default value, the second + # level model can have both :except and :only set. So if + # :only is set, always delete :except. + + a_names = self.attribute_names + if options[:only] + options.delete(:except) + a_names = a_names & Array(options[:only]).collect { |n| n.to_s } + else + options[:except] = Array(options[:except]) | Array(self.class.inheritance_column) + a_names = a_names - options[:except].collect { |n| n.to_s } + end + + columns_hash = self.class.columns_hash + a_names.each do |name| + + # To be consistent with the ActiveRecord instance being + # converted into a Hash using #attributes, convert SQL + # type names. Map the different "string" types all to + # "string", as the client of the XML does not care if a + # TEXT or VARCHAR column stores the value. + + type = columns_hash[name].type + case type + when :text + type = :string + when :time + type = :datetime + end + + attributes = options[:skip_types] ? { } : { :type => type } + + value = self.send(name) + if dasherize + name = name.dasherize + 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. + if value.nil? + attributes[:nil] = 'true' + builder.tag!(name, attributes) + else + value_needs_no_encoding = false + case type + when :binary + value = Base64.encode64(value) + attributes[:encoding] = 'base64' + value_needs_no_encoding = true + when :date + value = value.to_s(:db) + value_needs_no_encoding = true + when :datetime + value = value.xmlschema + value_needs_no_encoding = true + when :boolean, :float, :integer + value = value.to_s + value_needs_no_encoding = true + end + + if value_needs_no_encoding + builder.tag!(name, attributes) do + builder << value + end + else + builder.tag!(name, value, attributes) + end + end + end + + if methods_to_call = options.delete(:methods) + [ *methods_to_call ].each do |meth| + value = self.send(meth) + + tag = dasherize ? meth.to_s.dasherize : meth.to_s + + type_name = ActiveSupport::CoreExtensions::Hash::Conversions::XML_TYPE_NAMES[value.class] + + if formatter = ActiveSupport::CoreExtensions::Hash::Conversions::XML_FORMATTING[type_name] + value = formatter.call(value) + end + + if value.nil? + attributes = { :nil => true } + else + if !options[:skip_types] && !type_name.nil? + attributes = { :type => type_name } + else + attributes = { } + end + end + + builder.tag!(tag, value, attributes) + end + end + + if include_associations = options.delete(:include) + root_only_or_except = { :except => options[:except], + :only => options[:only] } + + include_has_options = include_associations.is_a?(Hash) + + for association in include_has_options ? include_associations.keys : Array(include_associations) + association_options = include_has_options ? include_associations[association] : root_only_or_except + + opts = options.merge(association_options) + + case self.class.reflect_on_association(association).macro when :has_many, :has_and_belongs_to_many records = send(association).to_a unless records.empty? - attributes_for_xml[association] = records.collect do |record| - record.attributes(association_options) + tag = records.first.class.to_s.underscore.pluralize + if dasherize + tag = tag.dasherize + end + builder.tag!(tag) do + records.each { |r| r.to_xml(opts) } end end when :has_one, :belongs_to if record = send(association) - attributes_for_xml[association] = record.attributes(association_options) + record.to_xml(opts.merge(:root => association)) end + end end end - end - attributes_for_xml.to_xml(options) + if procs = options.delete(:procs) + [ *procs ].each do |proc| + proc.call(options) + end + end + + end end private diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb index 6ea46d8e85..5a6870faa3 100755 --- a/activerecord/test/base_test.rb +++ b/activerecord/test/base_test.rb @@ -439,18 +439,18 @@ class BasicsTest < Test::Unit::TestCase def test_increment_counter Topic.increment_counter("replies_count", 1) - assert_equal 1, Topic.find(1).replies_count + assert_equal 2, Topic.find(1).replies_count Topic.increment_counter("replies_count", 1) - assert_equal 2, Topic.find(1).replies_count + assert_equal 3, Topic.find(1).replies_count end def test_decrement_counter Topic.decrement_counter("replies_count", 2) - assert_equal 1, Topic.find(2).replies_count + assert_equal -1, Topic.find(2).replies_count Topic.decrement_counter("replies_count", 2) - assert_equal 0, Topic.find(1).replies_count + assert_equal -2, Topic.find(2).replies_count end def test_update_all @@ -987,12 +987,12 @@ class BasicsTest < Test::Unit::TestCase end def test_increment_attribute - assert_equal 0, topics(:first).replies_count + assert_equal 1, topics(:first).replies_count topics(:first).increment! :replies_count - assert_equal 1, topics(:first, :reload).replies_count + assert_equal 2, topics(:first, :reload).replies_count topics(:first).increment(:replies_count).increment!(:replies_count) - assert_equal 3, topics(:first, :reload).replies_count + assert_equal 4, topics(:first, :reload).replies_count end def test_increment_nil_attribute @@ -1003,13 +1003,13 @@ class BasicsTest < Test::Unit::TestCase def test_decrement_attribute topics(:first).increment(:replies_count).increment!(:replies_count) - assert_equal 2, topics(:first).replies_count + assert_equal 3, topics(:first).replies_count topics(:first).decrement!(:replies_count) - assert_equal 1, topics(:first, :reload).replies_count + assert_equal 2, topics(:first, :reload).replies_count topics(:first).decrement(:replies_count).decrement!(:replies_count) - assert_equal -1, topics(:first, :reload).replies_count + assert_equal 0, topics(:first, :reload).replies_count end def test_toggle_attribute @@ -1217,14 +1217,14 @@ class BasicsTest < Test::Unit::TestCase written_on_in_current_timezone = topics(:first).written_on.xmlschema last_read_in_current_timezone = topics(:first).last_read.xmlschema assert_equal "", xml.first(7) - assert xml.include?(%(The First Topic)) - assert xml.include?(%(David)) + assert xml.include?(%(The First Topic)) + assert xml.include?(%(David)) assert xml.include?(%(1)) - assert xml.include?(%(0)) + assert xml.include?(%(1)) assert xml.include?(%(#{written_on_in_current_timezone})) - assert xml.include?(%(Have a nice day)) - assert xml.include?(%(david@loudthinking.com)) - assert xml.include?(%()) + assert xml.include?(%(Have a nice day)) + assert xml.include?(%(david@loudthinking.com)) + assert xml.match(%r{}) if current_adapter?(:SybaseAdapter) or current_adapter?(:SQLServerAdapter) assert xml.include?(%(#{last_read_in_current_timezone})) else @@ -1236,23 +1236,23 @@ class BasicsTest < Test::Unit::TestCase assert xml.include?(%(#{bonus_time_in_current_timezone})) end end - + def test_to_xml_skipping_attributes xml = topics(:first).to_xml(:indent => 0, :skip_instruct => true, :except => :title) assert_equal "", xml.first(7) - assert !xml.include?(%(The First Topic)) - assert xml.include?(%(David)) + assert !xml.include?(%(The First Topic)) + assert xml.include?(%(David)) xml = topics(:first).to_xml(:indent => 0, :skip_instruct => true, :except => [ :title, :author_name ]) - assert !xml.include?(%(The First Topic)) - assert !xml.include?(%(David)) + assert !xml.include?(%(The First Topic)) + assert !xml.include?(%(David)) end def test_to_xml_including_has_many_association xml = topics(:first).to_xml(:indent => 0, :skip_instruct => true, :include => :replies) assert_equal "", xml.first(7) assert xml.include?(%()) - assert xml.include?(%(The Second Topic's of the day)) + assert xml.include?(%(The Second Topic's of the day)) end def test_to_xml_including_belongs_to_association @@ -1277,7 +1277,7 @@ class BasicsTest < Test::Unit::TestCase ) assert_equal "", xml.first(6) - assert xml.include?(%(Summit)) + assert xml.include?(%(Summit)) assert xml.include?(%()) end diff --git a/activerecord/test/fixtures/topics.yml b/activerecord/test/fixtures/topics.yml index 810bbcf4e8..b1e5159be2 100644 --- a/activerecord/test/fixtures/topics.yml +++ b/activerecord/test/fixtures/topics.yml @@ -8,7 +8,7 @@ first: bonus_time: 2005-01-30t15:28:00.00+01:00 content: Have a nice day approved: false - replies_count: 0 + replies_count: 1 second: id: 2 @@ -17,6 +17,6 @@ second: written_on: 2003-07-15t15:28:00.00+01:00 content: Have a nice day approved: true - replies_count: 2 + replies_count: 0 parent_id: 1 type: Reply diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index c6fb0f82b5..a9fd1a902e 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* to_xml fixes, features, and speedup: introduce :dasherize option that converts updated_at to updated-at if true (the existing default); binary columns get encoding="base64" attribute; nil values get nil="true" attribute to distinguish empty values; add type information for float columns; allow arbitrarily deep :include; include SQL type information as the type attribute. #4989 [Blair Zajac ] + * Add OrderedHash#values. [Sam Stephenson] * Added Array#to_s(:db) that'll produce a comma-separated list of ids [DHH]. Example: diff --git a/activesupport/lib/active_support/core_ext/array/conversions.rb b/activesupport/lib/active_support/core_ext/array/conversions.rb index 35d010b164..2b42a55ab1 100644 --- a/activesupport/lib/active_support/core_ext/array/conversions.rb +++ b/activesupport/lib/active_support/core_ext/array/conversions.rb @@ -52,12 +52,20 @@ module ActiveSupport #:nodoc: options[:indent] ||= 2 options[:builder] ||= Builder::XmlMarkup.new(:indent => options[:indent]) - root = options.delete(:root) + root = options.delete(:root).to_s children = options.delete(:children) + if !options.has_key?(:dasherize) || options[:dasherize] + root = root.dasherize + end + options[:builder].instruct! unless options.delete(:skip_instruct) - options[:builder].tag!(root.to_s.dasherize) { each { |e| e.to_xml(options.merge({ :skip_instruct => true, :root => children })) } } + + opts = options.merge({ :skip_instruct => true, :root => children }) + + options[:builder].tag!(root) { each { |e| e.to_xml(opts) } } end + end end end diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index d556bab09b..be0012ad45 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -3,40 +3,69 @@ module ActiveSupport #:nodoc: module Hash #:nodoc: module Conversions XML_TYPE_NAMES = { - "Fixnum" => "integer", - "Date" => "date", - "Time" => "datetime", - "TrueClass" => "boolean", - "FalseClass" => "boolean" + ::Fixnum => "integer", + ::Float => "float", + ::Date => "date", + ::DateTime => "datetime", + ::Time => "datetime", + ::TrueClass => "boolean", + ::FalseClass => "boolean" } - + XML_FORMATTING = { "date" => Proc.new { |date| date.to_s(:db) }, "datetime" => Proc.new { |time| time.xmlschema } } - + def to_xml(options = {}) options[:indent] ||= 2 - options.reverse_merge!({ :builder => Builder::XmlMarkup.new(:indent => options[:indent]), :root => "hash" }) + options.reverse_merge!({ :builder => Builder::XmlMarkup.new(:indent => options[:indent]), + :root => "hash" }) options[:builder].instruct! unless options.delete(:skip_instruct) + dasherize = !options.has_key?(:dasherize) || options[:dasherize] + root = dasherize ? options[:root].to_s.dasherize : options[:root].to_s - options[:builder].__send__(options[:root].to_s.dasherize) do + options[:builder].__send__(root) do each do |key, value| case value when ::Hash value.to_xml(options.merge({ :root => key, :skip_instruct => true })) when ::Array value.to_xml(options.merge({ :root => key, :children => key.to_s.singularize, :skip_instruct => true})) + when ::Method, ::Proc + # If the Method or Proc takes two arguments, then + # pass the suggested child element name. This is + # used if the Method or Proc will be operating over + # multiple records and needs to create an containing + # element that will contain the objects being + # serialized. + if 1 == value.arity + value.call(options.merge({ :root => key, :skip_instruct => true })) + else + value.call(options.merge({ :root => key, :skip_instruct => true }), key.to_s.singularize) + end else - type_name = XML_TYPE_NAMES[value.class.to_s] + if value.respond_to?(:to_xml) + value.to_xml(options.merge({ :root => key, :skip_instruct => true })) + else + type_name = XML_TYPE_NAMES[value.class] + + key = dasherize ? key.to_s.dasherize : key.to_s - options[:builder].tag!(key.to_s.dasherize, - XML_FORMATTING[type_name] ? XML_FORMATTING[type_name].call(value) : value, - options[:skip_types] || value.nil? || type_name.nil? ? { } : { :type => type_name } - ) + attributes = options[:skip_types] || value.nil? || type_name.nil? ? { } : { :type => type_name } + if value.nil? + attributes[:nil] = true + end + + options[:builder].tag!(key, + XML_FORMATTING[type_name] ? XML_FORMATTING[type_name].call(value) : value, + attributes + ) + end end end end + end end end diff --git a/activesupport/test/core_ext/array_ext_test.rb b/activesupport/test/core_ext/array_ext_test.rb index 4cef0120d6..492e59bf21 100644 --- a/activesupport/test/core_ext/array_ext_test.rb +++ b/activesupport/test/core_ext/array_ext_test.rb @@ -136,4 +136,24 @@ class ArrayToXmlTests < Test::Unit::TestCase assert xml.include?(%(Evergreen)) assert xml.include?(%(Jason)) end + + def test_to_xml_with_dasherize_false + xml = [ + { :name => "David", :street_address => "Paulina" }, { :name => "Jason", :street_address => "Evergreen" } + ].to_xml(:skip_instruct => true, :skip_types => true, :indent => 0, :dasherize => false) + + assert_equal "", xml.first(17) + assert xml.include?(%(Paulina)) + assert xml.include?(%(Evergreen)) + end + + def test_to_xml_with_dasherize_true + xml = [ + { :name => "David", :street_address => "Paulina" }, { :name => "Jason", :street_address => "Evergreen" } + ].to_xml(:skip_instruct => true, :skip_types => true, :indent => 0, :dasherize => true) + + assert_equal "", xml.first(17) + assert xml.include?(%(Paulina)) + assert xml.include?(%(Evergreen)) + end end diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index a12299d32c..f8ed54c881 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -3,7 +3,6 @@ require File.dirname(__FILE__) + '/../../lib/active_support' class HashExtTest < Test::Unit::TestCase def setup - @strings = { 'a' => 1, 'b' => 2 } @symbols = { :a => 1, :b => 2 } @mixed = { :a => 1, 'b' => 2 } @@ -191,6 +190,17 @@ class HashExtTest < Test::Unit::TestCase end end +class IWriteMyOwnXML + def to_xml(options = {}) + options[:indent] ||= 2 + xml = options[:builder] ||= Builder::XmlMarkup.new(:indent => options[:indent]) + xml.instruct! unless options[:skip_instruct] + xml.level_one do + xml.tag!(:second_level, 'content') + end + end +end + class HashToXmlTest < Test::Unit::TestCase def setup @xml_options = { :root => :person, :skip_instruct => true, :indent => 0 } @@ -203,6 +213,20 @@ class HashToXmlTest < Test::Unit::TestCase assert xml.include?(%(David)) end + def test_one_level_dasherize_false + xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:dasherize => false)) + assert_equal "", xml.first(8) + assert xml.include?(%(Paulina)) + assert xml.include?(%(David)) + end + + def test_one_level_dasherize_true + xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:dasherize => true)) + assert_equal "", xml.first(8) + assert xml.include?(%(Paulina)) + assert xml.include?(%(David)) + end + def test_one_level_with_types xml = { :name => "David", :street => "Paulina", :age => 26, :moved_on => Date.new(2005, 11, 15) }.to_xml(@xml_options) assert_equal "", xml.first(8) @@ -217,7 +241,7 @@ class HashToXmlTest < Test::Unit::TestCase assert_equal "", xml.first(8) assert xml.include?(%(Paulina)) assert xml.include?(%(David)) - assert xml.include?(%()) + assert xml.include?(%()) end def test_one_level_with_skipping_types @@ -225,7 +249,7 @@ class HashToXmlTest < Test::Unit::TestCase assert_equal "", xml.first(8) assert xml.include?(%(Paulina)) assert xml.include?(%(David)) - assert xml.include?(%()) + assert xml.include?(%()) end def test_two_levels @@ -234,6 +258,13 @@ class HashToXmlTest < Test::Unit::TestCase assert xml.include?(%(
Paulina
)) assert xml.include?(%(David)) end + + def test_two_levels_with_second_level_overriding_to_xml + xml = { :name => "David", :address => { :street => "Paulina" }, :child => IWriteMyOwnXML.new }.to_xml(@xml_options) + assert_equal "", xml.first(8) + assert xml.include?(%(
Paulina
)) + assert xml.include?(%(content)) + end def test_two_levels_with_array xml = { :name => "David", :addresses => [{ :street => "Paulina" }, { :street => "Evergreen" }] }.to_xml(@xml_options) @@ -243,10 +274,10 @@ class HashToXmlTest < Test::Unit::TestCase assert xml.include?(%(
Evergreen
)) assert xml.include?(%(David)) end - def test_three_levels_with_array xml = { :name => "David", :addresses => [{ :streets => [ { :name => "Paulina" }, { :name => "Paulina" } ] } ] }.to_xml(@xml_options) assert xml.include?(%(
)) end + end -- cgit v1.2.3