#1295 ✓resolved
dhf (at f8technologies)

DataObjects::IntegrityError not thrown when should be

Reported by dhf (at f8technologies) | May 28th, 2010 @ 08:40 PM | in 1.0.0

Upgraded to rc3 and I am now getting failures in my rspec tests. I have a record that contains this property: property :medrec, String, :unique_index => true, :required => true. This test was passing with rc2 but now fails with rc3: lambda{Patient.create!(:name => 'Test Patient')}.should raise_error(DataObjects::IntegrityError) I do have raise_on_save_failure = true. but have commented it out and still fails.

rspec test result: 'Simple should not allow empty medrec' FAILED
expected DataObjects::IntegrityError but nothing was raised
/Users/dhf/RubymineProjects/pn/spec/simple_spec.rb:11:

Comments and changes to this ticket

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 29th, 2010 @ 12:38 AM

    • Milestone set to “1.0.0”
    • State changed from “new” to “confirmed”
    • Assigned user set to “Dan Kubb (dkubb)”

    I can confirm this behaviour.

    The internals of the state machine that handles persistence changed a bit since rc2, where it now validates that the attributes are valid before attempting to persist the resource. This was always done when updating a resource, but not on creation.

    I think this is going to remain inside the system, since it ensures all datastores have consistent behavior -- some will explode when receiving bad data, others will silently accept the data. In both cases we want for Resource#save and Resource#save! to return false, not for one to return false, and the other to sometimes blow up.

    However, this did point to a bug in the system. Inside DM::Resource#save we always assumed that creation was successful and returned false, and this is clearly not always the case. What we need to do is similar to what #update does, and return true only if the resource is clean (meaning it was fully persisted).

    In addition to this change, I am going to make it so that the exception thrown by setting raise_on_save_failure to true has a reference to the resource, so that you can get at the errors. This should make it much easier to figure out what went wrong when persisting the object.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 29th, 2010 @ 12:39 AM

    • State changed from “confirmed” to “accepted”
  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 29th, 2010 @ 01:20 AM

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 29th, 2010 @ 01:20 AM

    (from [f34b5f8962469f18efe8e46806b563784309bfb3]) Ensure the SaveFailureError exception has a reference to the resource

    • When dm-validations is being used, this allows the exact errors to be retrieved from the invalid resource when the SaveFailureError exception is raised.

    [#1295] http://github.com/datamapper/dm-core/commit/f34b5f8962469f18efe8e46...

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 29th, 2010 @ 01:21 AM

    • State changed from “accepted” to “resolved”

    @dhf: This should now be resolved on edge dm-core. Can you give it a try and confirm it is working as you expected it?

    Marking this as resolved for now.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

People watching this ticket

Attachments

Tags

Referenced by

Pages