Cast Problems in Ruby 2.0 #242

Closed
dannluciano opened this Issue Feb 27, 2013 · 27 comments

Comments

Projects
None yet

DataMapper not convert string to int in Model.get("1").
This runned in ruby 1.9.3 but not in 2.0.0.

Owner

envygeeks commented Feb 27, 2013

Please provide more debugging information: errors, better examples, anything else that might help sorting it out.

Can confirm the very same problem: In Ruby 2.0.0 Model.get(id) workds only if id is a Number, not if it is a String.
Example:
MyModel.get 26
works fine, but
MyModel.get "26" returns nil.

The same problem can be seen when assigning strings to foreign keys. You can assign an Integer, and everything is fine. But if you assign a string, the foreign key is set to nil. E.G:

my_model_instance.parent_id = 13 #sets parent_id to 13
but
my_model_instance.parent_id = '13' #sets parent_id to 'nil'

Owner

dkubb commented Mar 1, 2013

@dannluciano if you run the specs for DataMapper under Ruby 2.0, do you see any failing specs that can help pinpoint the issue? We've run DM somewhat recently against Ruby 2.0 https://travis-ci.org/datamapper/dm-core and not found any issues, which is why I find this curious.

I try run spec in dm-core and I get this errors.

'DataMapper::Property::Decimal#typecast does not typecast non-numeric value "00.0"' FAILED

expected #String:70284319365080 => "00.0"
got #BigDecimal:70284307553040 => #BigDecimal:7fd8ae5eb620,'0.0',9(18)

Compared using equal?, which compares object identity,
but expected and actual are not the same object. Use
'actual.should == expected' if you don't care about
object identity in this example.

'DataMapper::Property::Float#typecast does not typecast non-numeric value "00.0"' FAILED

expected #String:70284317166840 => "00.0"
got #Float:-9223372036854775806 => 0.0

Compared using equal?, which compares object identity,
but expected and actual are not the same object. Use
'actual.should == expected' if you don't care about
object identity in this example.

'DataMapper::Property::Integer#typecast does not typecast non-numeric value "00.0"' FAILED

expected #String:70284315974140 => "00.0"
got #Fixnum:1 => 0

Compared using equal?, which compares object identity,
but expected and actual are not the same object. Use
'actual.should == expected' if you don't care about
object identity in this example.

12770 examples, 3 failures, 3496 pending

The complete log is there https://gist.github.com/dannluciano/5067665.

Thanks for your time.

Where the coercions is implemented in dm-core? By dm-core or https://github.com/solnic/virtus with https://github.com/solnic/coercible ?

I believe I tracked the issue. Ruby 2.0 changed the way it treats the visibility of protected methods, so calls to respond_to?(:typecast_to_primitive) are returning false in the DataMapper::Property#typecast method.

What surprises me is that the issue does not exists in the code hosted on Github. A second argument true is being passed to the mentioned respond_to? call, which instructs it to look for protected methods (https://github.com/datamapper/dm-core/blob/release-1.2/lib/dm-core/property.rb#L685), but the gem available in Rubygems does not have the argument. I guess that's why Travis didn't pointed anything.

Owner

envygeeks commented Mar 10, 2013

Maybe ping @dkubb and @solnic and see if they think everything is copacetic and ready to release a new DM.

Contributor

solnic commented Mar 10, 2013

We could port dm1 to use coercible

I believe backporting coercible would be great, but I think releasing the already fixed 1.2.1 version (release-1.2 branch) as a gem would solve the problem until then.

Contributor

solnic commented Mar 12, 2013

@samflores if all projects are ported to use travis and all specs passed then we can push 1.2.1

@dkubb are all projects ported to travis already?

Owner

dkubb commented Mar 12, 2013

@solnic no, a few people talked about assisting and we had a bit of initial activity towards it, but there are still a bunch of gems that need to be updated to work with travis.

Owner

envygeeks commented Mar 12, 2013

@dkubb @solnic I thought we got all but one or two?

Owner

dkubb commented Mar 13, 2013

@envygeeks no, about 80% of the dm-* gems listed under https://github.com/datamapper/ still haven't been touched.

Hi guys, any news on when you might be fixing/releasing this? Some users were scratching their heads when this happened on Padrino and then we found this issue :). Let me know if I can help in any way. Thanks!

Owner

envygeeks commented Mar 26, 2013

I'm waiting for @dkubb to get back to me with a list of repos that need travis so I can get them added and then they can do the rest, it's already fixed in head we just have to get the rest of the components on public testing so everything can be confirmed.

@envygeeks amazing! thanks!! :D will keep an eye on this then

Owner

mbj commented Mar 26, 2013

@envygeeks I think all repos under the datamapper organization, that have a release, need to be ported.

Contributor

solnic commented Mar 26, 2013

Here's a list of all gems that are part of the dm1 suite: https://gist.github.com/solnic/4999203

Owner

dkubb commented Mar 26, 2013

@envygeeks sorry, I didn't realize you wanted me to get you a list. I see @solnic has already created a list of gems that are part of DM1 for you.

Owner

snusnu commented Mar 26, 2013

@dkubb @solnic @mbj fyi, i've made @envygeeks an owner of the datamapper organization, so that he's not blocked during his travis spree

p referenced this issue in integrity/integrity Apr 11, 2013

Closed

Fix build under ruby 2.0.0 #240

p commented Apr 11, 2013

    # @api semipublic
    def typecast(value)
      if value.nil? || primitive?(value)
        value
      elsif respond_to?(:typecast_to_primitive)
        typecast_to_primitive(value)
      end
    end

If neither branch matches the return value is nil which surely cannot be right.

p referenced this issue in integrity/integrity Apr 11, 2013

Closed

Fix Integrity on ruby 2.0. #246

Owner

dkubb commented Apr 12, 2013

@p this has been fixed in the release-1.2 branch by using respond_to?(:typecast_to_primitive, true).

However, you do bring up a good point. In a custom property, it is possible that neither of the first two branches will match, and nil will be returned which is not what was intended originally. I'm committing a change that will handle the custom property side of things by simply passing-through the value when it is unable to typecast.

@dkubb dkubb added a commit that referenced this issue Apr 12, 2013

@dkubb dkubb Fix Property#typecast to pass-through values that cannot be handled
* This fixes a related problem to #242. Most of properties that ship
  with dm-core define #typecast_to_primitive, but custom properties
  may not and I believe we should pass-through the value when it
  cannot be handled.
9224447

Any idea when a new release with that fix will be made?

1syo referenced this issue in lokka/lokka Jun 6, 2013

Merged

Fix broken spec in ruby 2.0 #197

Contributor

ujifgc commented Jun 17, 2013

'dm-core' '1.2.1' at rubygems is fixed.

@rud rud added a commit to rud/integrity that referenced this issue Jan 18, 2015

@rud rud dm-core minor upgrade
Issue datamapper/dm-core#242 is fixed in 1.2.1
5f34c5f

rud referenced this issue in integrity/integrity Jan 18, 2015

Merged

dm-core minor upgrade to 1.2.1 #291

Owner

tpitale commented Feb 8, 2016

This issue appears to be resolved. Closing.

tpitale closed this Feb 8, 2016

@bhaak bhaak added a commit to junethack/Junethack that referenced this issue Mar 25, 2016

@bhaak bhaak Update data_mapper to 1.2.1
Fixes the issue that with Ruby >=2.0, data_mapper couldn't cast string to
integer properties.
datamapper/dm-core#242
59c996e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment