Skip to content
This repository has been archived by the owner on Apr 17, 2018. It is now read-only.

DirtyMinder should not extend NilClass #71

Closed
wants to merge 3 commits into from

Conversation

blt04
Copy link

@blt04 blt04 commented May 22, 2014

Can PR #50 be back ported to the release-1.2 branch? When using DataMapper with Passenger, Passenger sometimes freezes the nil value (see https://code.google.com/p/phusion-passenger/issues/detail?id=1093). When DirtyMinder tries to track a frozen nil value, an error can occur:

/Users/bturner/.rvm/gems/ruby-2.1.2@ghnproxy/gems/dm-types-1.2.2/lib/dm-types/support/dirty_minder.rb:144:in `track': can't modify frozen object (RuntimeError)
    from /Users/bturner/.rvm/gems/ruby-2.1.2@ghnproxy/gems/dm-types-1.2.2/lib/dm-types/support/dirty_minder.rb:161:in `hook_value'
    from /Users/bturner/.rvm/gems/ruby-2.1.2@ghnproxy/gems/dm-types-1.2.2/lib/dm-types/support/dirty_minder.rb:151:in `set!'
    from /Users/bturner/.rvm/gems/ruby-2.1.2@ghnproxy/gems/dm-core-1.2.1/lib/dm-core/property.rb:625:in `set'
    from /Users/bturner/.rvm/gems/ruby-2.1.2@ghnproxy/gems/dm-core-1.2.1/lib/dm-core/resource/persistence_state.rb:22:in `set'
    from /Users/bturner/.rvm/gems/ruby-2.1.2@ghnproxy/gems/dm-core-1.2.1/lib/dm-core/resource/persistence_state/transient.rb:14:in `set'
    from /Users/bturner/.rvm/gems/ruby-2.1.2@ghnproxy/gems/dm-core-1.2.1/lib/dm-core/model/property.rb:238:in `headers='
    from test.rb:19:in `<main>'

PR #50 limited extending classes with Hooker to only certain classes. It would be awesome to back port this fix into 1.2.x.

This PR simply cherry-picks mbj's fixes to release-1.2.

Markus Schirp added 3 commits May 21, 2014 21:00
The fix excludes Strings from be extended by the Hooker.

Cherry-picked from: 68562ae
Cherry-picked from: 3c795ed
@tpitale
Copy link
Member

tpitale commented Mar 9, 2015

I've run the specs locally on branch release-1.2. I think this is safe to merge, ignoring Travis-CI failure.

@tpitale
Copy link
Member

tpitale commented Mar 9, 2015

I've merged this in my own fork into release-1.2 and deployed my app. That's working fine, but obviously does not test the range of rubies and datastores that Travis is.

@dkubb dkubb closed this Mar 28, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants