#960 ✓resolved
Ashley Moran

After reconnecting with another repository block, saving parent resources no longer propagate to children

Reported by Ashley Moran | July 7th, 2009 @ 11:00 AM | in 0.10.2

This one came about because our specs were passing, but the Cucumber features driving the app were not. Merb wraps all requests in a repository block, which means the following simulates a 3-request process in the browser. The first example fails in 0.10. All of them pass in 0.9.11.

I consider this a serious bug, because it means any long-running process using DataMapper cannot be assumed to be saving resources.

require 'rubygems'
  
USE_DM_0_9 = false
 
if USE_DM_0_9
  DM_GEMS_VERSION   = "0.9.11"
  DO_GEMS_VERSION   = "0.9.12"
else
  DM_GEMS_VERSION   = "0.10.0"
  DO_GEMS_VERSION   = "0.10.0"
end
 
gem "data_objects",   DO_GEMS_VERSION
gem "do_sqlite3",     DO_GEMS_VERSION # If using another database, replace this
gem "dm-core",        DM_GEMS_VERSION   
gem "dm-types",       DM_GEMS_VERSION        
gem "dm-validations", DM_GEMS_VERSION  

require "data_objects"
require "dm-core"
require "dm-types"
require "dm-validations"

require 'spec'
 
SQLITE_FILE = File.join(`pwd`.chomp, "test.db")
 
DataMapper.setup(:default, "sqlite3:#{SQLITE_FILE}")
DataMapper.setup(:reloaded, "sqlite3:#{SQLITE_FILE}")
 
class Parent
  include DataMapper::Resource

  property :id, Serial
  
  has n, :children
end
 
class Child
  include DataMapper::Resource

  property :id, Serial
  property :name, String, :nullable => false
  
  belongs_to :parent
end
 
module IdentityMapHelper
  def reload(object)
    object.class.get(object.id)
  end
  
  def with_db_reconnect(&blk)
    original_repository = DataMapper::Repository.context.pop
    repository(:reloaded, &blk)
    DataMapper::Repository.context << original_repository
  end
end
 
Spec::Runner.configure do |config|
  include IdentityMapHelper
  
  config.before(:each) do
    DataMapper.auto_migrate!
  end
  
  config.before(:each) do    
    DataMapper::Repository.context << repository(:default)
  end
  
  config.after(:each) do
    DataMapper::Repository.context.pop
  end
end

describe "Child with unsaved parent" do
  before(:each) do
    @child = Child.new(:name => "Bart")
    @parent = Parent.new
    
    @parent.children << @child
  end
  
  it "should persist changes between reloads" do
    @parent.save
    @parent.children.first.name.should == "Bart"
    
    with_db_reconnect do
      parent_reloaded = reload(@parent)
      parent_reloaded.children.first.name = "Lisa"
      parent_reloaded.save
      parent_reloaded.children.first.name.should == "Lisa"
    end
    
    with_db_reconnect do
      parent_reloaded_again = reload(@parent)
      parent_reloaded_again.children.first.name.should == "Lisa"
    end
  end
  
  it "should persist changes between reloads if you explicitly save the child BEFORE the parent" do
    @parent.save
    @parent.children.first.name.should == "Bart"
    
    with_db_reconnect do
      parent_reloaded = reload(@parent)
      parent_reloaded.children.first.name = "Lisa"
      parent_reloaded.children.first.save
      parent_reloaded.save
    end
    
    with_db_reconnect do
      parent_reloaded_again = reload(@parent)
      parent_reloaded_again.children.first.name.should == "Lisa"
    end
  end
    
  it "should persist changes between reloads if you explicitly save the child AFTER the parent" do
    @parent.save
    @parent.children.first.name.should == "Bart"
    
    with_db_reconnect do
      parent_reloaded = reload(@parent)
      parent_reloaded.children.first.name = "Lisa"
      parent_reloaded.save
      parent_reloaded.children.first.save
    end
    
    with_db_reconnect do
      parent_reloaded_again = reload(@parent)
      parent_reloaded_again.children.first.name.should == "Lisa"
    end
  end
end

Comments and changes to this ticket

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) July 22nd, 2009 @ 01:25 PM

    • State changed from “new” to “unconfirmed”

    This wasn't overlooked. I just have 47 tickets assigned to me personally, 33 of those for the 0.10.0 milestone. I can only close maybe 10 easy ones a week or 3-4 hard ones a week, so it's going to be a while before I can work through all of them.

    If you want to help speed things up, the best thing would be to submit patches for some of the easier tickets I have assigned to me. Grant though, most of the time I accept a ticket when it requires in-depth knowledge of DM to fix, so some of them might be pretty difficult. The second best thing people can do is take on some of the tickets without any owner, either submitting patches, adding failing specs or just confirming that it is still failing with edge.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) July 22nd, 2009 @ 04:02 PM

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

    Dan Kubb (dkubb) October 4th, 2009 @ 09:33 PM

    • Tag cleared.
    • Milestone changed from “0.10.0” to “0.10.2”

    [project:id#20609 not-tagged:"0.10.0" milestone:id#51895 bulk edit command]

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) October 24th, 2009 @ 03:04 PM

    • Tag set to “regression”
  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) October 25th, 2009 @ 01:38 AM

    This bug is happening because calling Collection#first on an unloaded Collection will not lazy load the Collection, but will instead query for the Resource directly and return it to the caller. It doesn't add the resource to the children. That means that later on when the parent is saved and it asks the children to save nothing happens because the children don't contain the dirty resource.

    The solution to this will be to make sure that Collection#first prepends the resources it returns to the Collection.

    This same problem would also affect Collection#last, Collection#at and Collection#[] so the approach should work for all of them too.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) October 25th, 2009 @ 03:05 AM

    @Ashley: I've got a really preliminary patch to try to see if it resolves your problem which you'll find attached. It should be applied against dm-core in git.

    I was experimenting with the smallest change I could think of to make your specs pass, which they should with this, but I want to make sure it also fixes the real-world problem this ticket represents.

    Once you give me an indication of whether or not I am on the right track I'll start work on a set of specs, and refactor/refine this code a bit more, and then push it to edge.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) October 25th, 2009 @ 03:10 AM

    I should note that Collection#at and Collection#[] use Collection#first and Collection#last underneath, so in one instance the problem is resolved, but not in every case. When the offset is 0 or -1 (first and last) the problem is resolved, but will still occur when the offset is different (like, say, 1). A more general approach will be needed to solve the root cause of the problem, but I think this is on the right track.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) October 27th, 2009 @ 03:28 AM

    • State changed from “accepted” to “resolved”

    (from [6a6add48ca6f5dcf85af7ae5cf6358cb9f950657]) Ensure Collection#first and Collection#last always return the same instance

    • When Collection#first and Collection#last are called without any arguments the same Resource instance should be returned every time.

    [#960 state:resolved] http://github.com/datamapper/dm-core/commit/6a6add48ca6f5dcf85af7ae...

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