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

STI association not accessible on certain reloaded Resources #90

Open
solnic opened this issue May 17, 2011 · 6 comments
Open

STI association not accessible on certain reloaded Resources #90

solnic opened this issue May 17, 2011 · 6 comments

Comments

@solnic
Copy link
Contributor

solnic commented May 17, 2011

This appears to be a hybrid of #960 and #1001. I don't understand it, but I can explain the behaviour we have seen.

In a GrandParent-Parent-Child relationship, when using a subclass of the Parent class, if you reload the Parent and try to save it implicitly via the GrandParent, inside a save call - and only inside a save call (we are using a hook) - you get the association Collection back instead of the Resource itself.

It can be fixed by explicitly accessing the failing Child before the save. (Although you must touch all children, and bizarrely you can't use #each to do so Equally bizarrely, it can also be fixed by moving the association definition to the Parent superclass.

Any ideas?

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}")

module DataMapper
  module Validate
    extend Chainable

    # NOTE: this is different to the current next branch, as of writing:
    # http://github.com/datamapper/dm-more/blob/0c58330f1ee3b27978fec1e3331c1b68417a738a/dm-validations/lib/dm-validations.rb
    def save(context = :default)
      return false unless save_people && (context.nil? || valid?(context))
      save_self && save_children
    end
  end

  module Resource
    def save_people
      parent_relationships.all? do |relationship|
        parent = relationship.get!(self)

        if parent.new? || parent.dirty?
          next unless parent.save_people && parent.save_self
        end

        relationship.set(self, parent)  # set the FK values
      end
    end
  end
end

class Farm
  include DataMapper::Resource
  after :save, :dm_bug_save_children
  property :id, Serial

  has n, :people

  def dm_bug_save_children
    people.each { |p| p.save }
  end
end

class Person
  include DataMapper::Resource

  property :id, Serial
  property :type, Discriminator

  # has 1, :cow  # Also fixes the first example
  belongs_to :farm
end

class Farmer < Person
  after :save, :save_cow

  has 1, :cow # Move to Person to fix

  def initialize(*)
    super
    self.cow = Cow.new
  end

  def save_cow
    cow.save
  end
end

class Cow
  include DataMapper::Resource

  property :id, Serial

  belongs_to :farmer, :child_key => [ :person_id ]
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
    Farm.auto_migrate!
    Person.auto_migrate!
    Cow.auto_migrate!
  end

  config.before(:each) do    
    DataMapper::Repository.context << repository(:default)
  end

  config.after(:each) do
    DataMapper::Repository.context.pop
  end
end

describe "STI GrandParent (1), Parent (n), Child (1)" do
  before(:each) do
    @farm = Farm.new
    @giles = Farmer.new
    @farm.people << @giles
  end

  # Fails with:
  # undefined method `cow' for #<DataMapper::Associations::OneToMany::Collection:0x17c65f4>
  it "can't save an unaccessed child after reloading" do
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      farm_reloaded.people << Farmer.new # Necessary to cause the breakage
      farm_reloaded.save
    end
  end

  it "fails if you have >= 2 resources in the association before the initial save" do
    @farm.people << Farmer.new
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      farm_reloaded.people << Farmer.new
      farm_reloaded.save
    end
  end

  it "can't be fixed by touching all the children using #each" do
    @farm.people << Farmer.new
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      farm_reloaded.people << Farmer.new
      farm_reloaded.people.each { |person| puts "MOO"; person.cow } # Fails here, doesn't fix it
      farm_reloaded.save
    end
  end

  it "can be fixed by touching all children using #[]" do
    @farm.people << Farmer.new
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      farm_reloaded.people << Farmer.new
      farm_reloaded.people[0].cow # Fixes it
      farm_reloaded.people[1].cow # Fixes it
      farm_reloaded.save
    end
  end

  it "can be fixed if you have one child and use #first" do
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      farm_reloaded.people << Farmer.new
      farm_reloaded.people.first.cow # Fixes it
      farm_reloaded.save
    end
  end

  it "works if you save a new object" do
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      @jones = Farmer.new
      farm_reloaded.people << @jones
      @jones.save
    end
  end

  it "works if you save a reloaded child" do
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      @jones = Farmer.new
      farm_reloaded.people << @jones
      @jones.save
      reload(@jones).save
    end
  end
end

Created by Ashley Moran - 2009-08-05 11:13:13 UTC

Original Lighthouse ticket: http://datamapper.lighthouseapp.com/projects/20609/tickets/1002

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

According to our DataMapper bug canary specs, this is fixed in 0.10RC2.

by Ashley Moran

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

I made a mistake - our canary spec was wrong and didn’t actually replicate this bug. It’s still broken, but now fails with the error undefined method &rsquo;save&rsquo; for nil:NilClass.

You can no longer fix this by moving the association to the base class. You can still fix it by touching every element using #[]. (ie, our revised spec fails in the same places as the original.)

Note that our revised example has a workaround for bug #1041, but that shouldn’t affect #1002.

require &rsquo;rubygems&rsquo;

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

require &rsquo;spec&rsquo;

DataMapper.setup(:default, "sqlite3::memory:")
DataMapper.setup(:reloaded, "sqlite3::memory:")

class Farm
  include DataMapper::Resource
  after :save, :dm_bug_save_children
  property :id, Serial

  has n, :people

  def dm_bug_save_children
    people.each { |p| p.save }
  end
end

class Person
  include DataMapper::Resource

  property :id, Serial
  property :type, Discriminator

  # has n, :cows  # No longer fixes the first example
  belongs_to :farm
end

class Farmer < Person
  after :save, :save_cow

  # has 1, :cows # this is broken due to #1041
  has n, :cows

  def initialize(*)
    super
    cows.new
  end

  def cow
    cows[0]
  end

  def save_cow
    cow.save
  end
end

class Cow
  include DataMapper::Resource

  property :id, Serial

  belongs_to :farmer
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
    Farm.auto_migrate!
    Person.auto_migrate!
    Cow.auto_migrate!
  end

  config.before(:each) do    
    DataMapper::Repository.context << repository(:default)
  end

  config.after(:each) do
    DataMapper::Repository.context.pop
  end
end

describe "STI GrandParent (1), Parent (n), Child (1)" do
  before(:each) do
    @farm = Farm.new
    @giles = Farmer.new
    @farm.people << @giles
  end

  # Fails with:
  # undefined method `save&rsquo; for nil:NilClass
  it "can&rsquo;t save an unaccessed child after reloading" do
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      farm_reloaded.people << Farmer.new # Necessary to cause the breakage
      farm_reloaded.save
    end
  end

  it "fails if you have >= 2 resources in the association before the initial save" do
    @farm.people << Farmer.new
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      farm_reloaded.people << Farmer.new
      farm_reloaded.save
    end
  end

  it "can&rsquo;t be fixed by touching all the children using #each" do
    @farm.people << Farmer.new
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      farm_reloaded.people << Farmer.new
      farm_reloaded.people.each { |person| person.cow } # Fails here, doesn&rsquo;t fix it
      farm_reloaded.save
    end
  end

  it "can be fixed by touching all children using #[]" do
    @farm.people << Farmer.new
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      farm_reloaded.people << Farmer.new
      farm_reloaded.people[0].cow # Fixes it
      farm_reloaded.people[1].cow # Fixes it
      farm_reloaded.save
    end
  end

  it "can be fixed if you have one child and use #first" do
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      farm_reloaded.people << Farmer.new
      farm_reloaded.people.first.cow # Fixes it
      farm_reloaded.save
    end
  end

  it "works if you save a new object" do
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      @jones = Farmer.new
      farm_reloaded.people << @jones
      @jones.save
    end
  end

  it "works if you save a reloaded child" do
    @farm.save

    with_db_reconnect do
      farm_reloaded = reload(@farm)
      @jones = Farmer.new
      farm_reloaded.people << @jones
      @jones.save
      reload(@jones).save
    end
  end
end

by Ashley Moran

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

[bulk edit]

by Dan Kubb (dkubb)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

[bulk edit]

by Dan Kubb (dkubb)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

[bulk edit]

by Dan Kubb (dkubb)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

[bulk edit]

by Dan Kubb (dkubb)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant