Xml serialisation has one #7857

Closed
wants to merge 1,082 commits into
from

Conversation

Projects
None yet
@cliochris
Contributor

cliochris commented Oct 5, 2012

When serialising a class, specify the type of any singular associations, if necessary. Rails already correctly specifies the :type of any enumerable association (e.g. a has_many association), but made no attempt to do so for non-enumerables (e.g. a has_one association).
We must specify the :type of any polymorphic association. A has_one association to a class which uses single-table inheritance is an example of a polymorphic association.
Fixes #7471

kennyj and others added some commits Mar 3, 2012

Fix GH #3163. Should quote database on mysql/mysql2.
Conflicts:

	activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb
Change the string to use in test case.
Conflicts:

	activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb
	activerecord/test/cases/adapters/mysql2/schema_test.rb
Additional fix for CVE-2012-2661
While the patched PredicateBuilder in 3.1.5 prevents a user
from specifying a table name using the `table.column` format,
it doesn't protect against the nesting of hashes changing the
table context in the next call to build_from_hash. This fix
covers this case as well.
Fix the the backport of the object dup with the ruby 1.9.3p194.
At the end of initialize_dup was added the call to super if it exists,
so it also works with 1.8.7 where initialize_dup doesn't exist.
This issu was introduced with the pull request #6324
Merge pull request #6682 from acapilleri/dup_validation_fix_backport_…
…for_1_9_3

Dup validation fix backport for 1 9 3
Merge pull request #6676 from aurelian/master
Don't assign the attributes if the list is empty
Merge branch '3-2-stable-rel' into 3-2-stable
* 3-2-stable-rel:
  updating changelogs
  bumping version numbers
  updating changelogs with security fixes
  updating changelogs
  Array parameters should not contain nil values.
  Additional fix for CVE-2012-2661
@kennyj

This comment has been minimized.

Show comment Hide comment
@kennyj

kennyj Jun 13, 2012

Contributor

Hi @ernie @tenderlove
Please see #6718.

I guess specification's conflict has occurred.

Contributor

kennyj commented on cc2903d Jun 13, 2012

Hi @ernie @tenderlove
Please see #6718.

I guess specification's conflict has occurred.

This comment has been minimized.

Show comment Hide comment
@ernie

ernie Jun 13, 2012

Contributor

@kennyj posting response to that issue now. Short version -- code that treated nested hashes this was is wrong. Long version will be posted shortly.

Contributor

ernie replied Jun 13, 2012

@kennyj posting response to that issue now. Short version -- code that treated nested hashes this was is wrong. Long version will be posted shortly.

spastorino and others added some commits Jun 13, 2012

Deprecate update_attribute.
Historically, update_attribute and update_attributes are similar, but
with one big difference: update_attribute does not run validations.
These two methods are really easy to confuse given their similar
names. Therefore, update_attribute is being deprecated in favor of
update_column, and will be removed in Rails 4.

See the discussion on rails-core here:
https://groups.google.com/d/topic/rubyonrails-core/BWPUTK7WvYA/discussion
Remove unneded tests.
Before b081f6b, this test are
asserting that update_attribute does the dirty tracking. Since we
deprecated this method and update_column write in the database directly
this tests will always fail.
Merge pull request #6752 from steveklabnik/fix_5680
Respect absolute paths in compute_source_path.
Merge pull request #6764 from frodsan/patch-4
bump AS deprecation_horizon to 4.0
Deprecating composed_of in ActiveRecord
This feature adds a lot of complication to ActiveRecord for dubious
value. Let's talk about what it does currently:

class Customer < ActiveRecord::Base
  composed_of :balance, :class_name => "Money", :mapping => %w(balance
amount)
end

Instead, you can do something like this:

    def balance
      @balance ||= Money.new(value, currency)
    end

    def balance=(balance)
      self[:value] = balance.value
      self[:currency] = balance.currency
      @balance = balance
    end

Since that's fairly easy code to write, and doesn't need anything
extra from the framework, if you use composed_of today, you'll
have to add accessors/mutators like that.

This feature will be removed in Rails 4.
Merge pull request #6649 from route/logger_in_metal_3_2
Logger in metal backport for 3.2
Merge pull request #6758 from caironoleto/master
Fixing load config in some tasks
Conflicts:
	activerecord/lib/active_record/railties/databases.rake
Merge branch 'acapilleri-update_nested_attributes'
Closes #6675

Conflicts:
	activerecord/lib/active_record/attribute_methods/dirty.rb
Merge pull request #6842 from ernie/handle-non-strings-in-grouped-cal…
…culations

Stop assuming strings for grouped calculations
Conflicts:
	activerecord/lib/active_record/relation/calculations.rb
Ensure Arel columns are typecasted properly when grouping with calcul…
…ation

Fix build issue with postgresql.

Conflicts:
	activerecord/lib/active_record/relation/calculations.rb
	activerecord/test/cases/calculations_test.rb
Merge pull request #6857 from rsutphin/as_core_ext_time_missing_require
Missing require breaks Time.=== when selectively loading ActiveSupport core_exts in 3.2.4+
Merge pull request #6900 from cbandy/issue-6898
Require URI in ConnectionSpecification
Conflicts:
	activerecord/lib/active_record/connection_adapters/connection_specification.rb

rafaelfranca and others added some commits Sep 20, 2012

Revert "backport fair connection pool 02b2335 to 3-2-stable"
This reverts commit 0693e07.

Revert "Cache columns metadata to avoid extra while testing"

This reverts commit a82f1e3.

Reason: This is causing failures in the postgresql build.
See http://travis-ci.org/#!/rails/rails/builds/2485584

Related with #7675
Revert "Respect `config.digest = false` for `asset_path`"
This reverts commit 1ac19c1.

Conflicts:
	actionpack/CHANGELOG.md

Reason: This is causing failures in the railties build.
See http://travis-ci.org/#!/rails/rails/jobs/2491787

Related with #7672
Revert "Revert "Respect `config.digest = false` for `asset_path`""
This reverts commit 54f5574.

Reason: the last commit fixed the failing case
Merge pull request #3544 from amatsuda/_field_changed
Rename field_changed? to _field_changed? so that users can create a field named field
Conflicts:

	activerecord/lib/active_record/core.rb
	activerecord/test/cases/dirty_test.rb
ConnectionPool accepts spec key 'checkout_timeout'
Backport of #6441 cb6f839 . Old 'wait_timeout' is still supported,
but conflicts with mysql2 using that spec key for different thing.
'checkout_timeout' can now be used taking precedence for ConnectionPool
over 'wait_timeout'.
Merge pull request #7684 from jrochkind/connection_pool_timeout_key_b…
…ackport

ConnectionPool accepts spec key 'checkout_timeout' (Backport)
Add logger.push_tags and .pop_tags to complement logger.tagged
Avoid memory leak from unflushed logs on other threads leaving tags behind.

Conflicts:
	activesupport/CHANGELOG.md
	activesupport/lib/active_support/tagged_logging.rb
	activesupport/test/tagged_logging_test.rb
Tune up Rails::Rack::Logger. Only put space between requests in devel…
…opment logs.

Conflicts:
	railties/test/application/rack/logger_test.rb
Don't paramify ActionDispatch::Http::UploadedFile in tests
To test uploading a file without using fixture_file_upload, a posted
ActionDispatch::Http::UploadedFile should not be paramified (just like
Rack::Test::UploadedFile).
(Rack::Test::UploadedFile and ActionDispatch::Http::UploadedFile don't
share the same API, tempfile is not accessible on
Rack::Test::UploadedFile as discussed in
rack-test/rack-test#30)
Merge pull request #7786 from yabawock/3-2-stable
Backport "Don't paramify ActionDispatch::Http::UploadedFile in tests"
Merge pull request #7659 from HugoLnx/template_error_no_matches_rebased
REBASED: fixing assert_template bug when template matches expected, but not ends with
Conflicts:
	actionpack/CHANGELOG.md
	actionpack/lib/action_controller/test_case.rb
Merge pull request #7802 from steveklabnik/issue_7799
Fix reference to code sample in Getting Started.
Asset manifest includes aliases for foo.js -> foo/index.js and vice v…
…ersa. Bump Sprockets requirements from 2.1+ to 2.2+ and let it answer "should we compile this asset?" for us.
Merge pull request #6450 from iHiD/resource_generator_routes_master
Master branch: Fixed generated whitespace in routes when using namespaced resource.

Merge pull request #7811 from iHiD/resource_generator_routes_master

Fix the build (Broken scaffold routes test)
Merge pull request #7789 from senny/7777_resource_functions_modify_op…
…tions

resource and resources do no longer modify passed options
Merge pull request #7797 from senny/7459_prefix_tempalte_assertion_va…
…riables

prefix TemplateAssertions ivars.

Closes #7459
Conflicts:
	actionpack/lib/action_controller/test_case.rb
	actionpack/lib/action_view/test_case.rb
@Mange

This comment has been minimized.

Show comment Hide comment
@Mange

Mange Oct 2, 2012

Thanks a lot!

Mange commented on 19987b6 Oct 2, 2012

Thanks a lot!

This comment has been minimized.

Show comment Hide comment
@rwz

rwz Oct 2, 2012

Contributor

I can't believe this was so damn easy. I really tried looking into it before and it seemed much more complicated back then :)

Contributor

rwz replied Oct 2, 2012

I can't believe this was so damn easy. I really tried looking into it before and it seemed much more complicated back then :)

This comment has been minimized.

Show comment Hide comment
@route

route Oct 2, 2012

Contributor

@jeremy ❤️

Contributor

route replied Oct 2, 2012

@jeremy ❤️

This comment has been minimized.

Show comment Hide comment
@ndbroadbent

ndbroadbent Oct 6, 2012

Contributor

This commit is adding /index.* aliases for every asset in my manifest.yml, including images. I don't think this is necessary, so it can be limited by checking if the index file exists with something like:

aliased_path = aliased_path_for asset.logical_path
if File.exists? "#{target}/#{aliased_path}"
  manifest[aliased_path] = digest_path
end
Contributor

ndbroadbent replied Oct 6, 2012

This commit is adding /index.* aliases for every asset in my manifest.yml, including images. I don't think this is necessary, so it can be limited by checking if the index file exists with something like:

aliased_path = aliased_path_for asset.logical_path
if File.exists? "#{target}/#{aliased_path}"
  manifest[aliased_path] = digest_path
end

This comment has been minimized.

Show comment Hide comment
@jeremy

jeremy Oct 7, 2012

Member

Sounds good @ndbroadbent - giving it a shot?

Could move the file existence check into aliased_path_for:

    def aliased_path_for(logical_path)
      if File.basename(logical_path).start_with?('index')
        logical_path.sub(/\/index([^\/]+)$/, '\1')
      else
        aliased_path = logical_path.sub(/\.([^\/]+)$/, '/index.\1')
        aliased_path if File.exist?("#{target}/#{aliased_path}")
      end
    end

Then skip absent aliases:

          if aliased_path = aliased_path_for(asset.logical_path)
            manifest[aliased_path] = digest_path
          end
Member

jeremy replied Oct 7, 2012

Sounds good @ndbroadbent - giving it a shot?

Could move the file existence check into aliased_path_for:

    def aliased_path_for(logical_path)
      if File.basename(logical_path).start_with?('index')
        logical_path.sub(/\/index([^\/]+)$/, '\1')
      else
        aliased_path = logical_path.sub(/\.([^\/]+)$/, '/index.\1')
        aliased_path if File.exist?("#{target}/#{aliased_path}")
      end
    end

Then skip absent aliases:

          if aliased_path = aliased_path_for(asset.logical_path)
            manifest[aliased_path] = digest_path
          end

This comment has been minimized.

Show comment Hide comment
@jeremy

jeremy Oct 7, 2012

Member

@rwz Simple result... of a long evening full of facepalms :)

I tried a few different approaches, but our hands are tied by asset digests which leave us without a useful sprockets environment. So the only avenue left is to use the manifest. And sprockets itself doesn't normalize logical paths for us, or even express a preference for one form over another -- foo.js and foo/index.js are peers in its eyes, yet #each_logical_path yields foo/index.js. Argh!

Member

jeremy replied Oct 7, 2012

@rwz Simple result... of a long evening full of facepalms :)

I tried a few different approaches, but our hands are tied by asset digests which leave us without a useful sprockets environment. So the only avenue left is to use the manifest. And sprockets itself doesn't normalize logical paths for us, or even express a preference for one form over another -- foo.js and foo/index.js are peers in its eyes, yet #each_logical_path yields foo/index.js. Argh!

This comment has been minimized.

Show comment Hide comment
@ndbroadbent

ndbroadbent Oct 7, 2012

Contributor

@jeremy, that looks great, thanks!

While we're on the topic of assets in Rails 3.2, I was wondering if you might be interested in taking a look at my pull request to improve asset compilation: #7866

Contributor

ndbroadbent replied Oct 7, 2012

@jeremy, that looks great, thanks!

While we're on the topic of assets in Rails 3.2, I was wondering if you might be interested in taking a look at my pull request to improve asset compilation: #7866

rafaelfranca and others added some commits Oct 2, 2012

Merge pull request #7822 from lulalala/reset-counter-cache-for-has-ma…
…ny-through

Fix reset_counters crashing on has_many :through associations.
Conflicts:
	activerecord/CHANGELOG.md
	activerecord/lib/active_record/counter_cache.rb
Refactor
Conflicts:
	activerecord/lib/active_record/counter_cache.rb
Merge pull request #7836 from lihanli/error-msg-update
Update supported ruby versions error message in ruby_version_check.rb
Merge pull request #6978 from frodsan/count_nosql_unsaved_parent
Count returns 0 without querying if parent is not saved
When serialising a class, specify the type of any singular associatio…
…ns, if

necessary. Rails already correctly specifies the :type of any enumerable
association (e.g. a has_many association), but made no attempt to do so for
non-enumerables (e.g. a has_one association).
We must specify the :type of any polymorphic association. A has_one
association to a class which uses single-table inheritance is an example of
a polymorphic association.
Fixes #7471

@cliochris cliochris closed this Oct 5, 2012

@ndbroadbent

This comment has been minimized.

Show comment Hide comment
@ndbroadbent

ndbroadbent Oct 6, 2012

Contributor

Sorry, should have added my comment here as a line note: 19987b6#commitcomment-1956232

Sorry, should have added my comment here as a line note: 19987b6#commitcomment-1956232

@pixeltrix

This comment has been minimized.

Show comment Hide comment
@pixeltrix

pixeltrix Nov 2, 2012

Member

I think this needs to be reverted as it may break quite a few apps. The shorthand syntax has historically ignored everything in the scope so changing it in a patch release is probably too risky. We definitely need to sort out the shorthand syntax for 4.0 though - I'll drop into Campfire later to discuss what needs fixing.

Member

pixeltrix commented on 61d5d2d Nov 2, 2012

I think this needs to be reverted as it may break quite a few apps. The shorthand syntax has historically ignored everything in the scope so changing it in a patch release is probably too risky. We definitely need to sort out the shorthand syntax for 4.0 though - I'll drop into Campfire later to discuss what needs fixing.

This comment has been minimized.

Show comment Hide comment
@spastorino

spastorino Nov 2, 2012

Member

@pixeltrix yes I was waiting the release because of this guy issue. @rafaelfranca was trying to fix it in his branch https://github.com/rafaelfranca/rails/tree/route_fix Please let's discuss in CF so we can release the final version properly ❤️

Member

spastorino replied Nov 2, 2012

@pixeltrix yes I was waiting the release because of this guy issue. @rafaelfranca was trying to fix it in his branch https://github.com/rafaelfranca/rails/tree/route_fix Please let's discuss in CF so we can release the final version properly ❤️

@jmrepetti

This comment has been minimized.

Show comment Hide comment
@jmrepetti

jmrepetti Nov 6, 2015

I had to patch this line like this in Rails 3.2.22:

old == 0 && value.is_a?(String) && value.present? && value != '0' && !(value =~ /^0\.0*$/)

because I have problems when value is '0.0'

for example:

i = Item.find(277)
i.buffer
=> #<BigDecimal:7f80becd9510,'0.0',9(18)>
i.buffer.to_s
=> "0.0"
i.buffer = '0.0'
=> "0.0"
i.changed?
=> true

Probably I have to cast the input before, not sure. I'm receiving the value '0.0' from a web form. I know Rails 4 doesn't have this problem but I can't switch to Rails 4 now.

I had to patch this line like this in Rails 3.2.22:

old == 0 && value.is_a?(String) && value.present? && value != '0' && !(value =~ /^0\.0*$/)

because I have problems when value is '0.0'

for example:

i = Item.find(277)
i.buffer
=> #<BigDecimal:7f80becd9510,'0.0',9(18)>
i.buffer.to_s
=> "0.0"
i.buffer = '0.0'
=> "0.0"
i.changed?
=> true

Probably I have to cast the input before, not sure. I'm receiving the value '0.0' from a web form. I know Rails 4 doesn't have this problem but I can't switch to Rails 4 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment