Skip to content
This repository has been archived by the owner on Sep 2, 2019. It is now read-only.

Inheritance #26

Open
sfeu opened this issue Jan 31, 2012 · 5 comments
Open

Inheritance #26

sfeu opened this issue Jan 31, 2012 · 5 comments

Comments

@sfeu
Copy link

sfeu commented Jan 31, 2012

Hey,

i played around with the redis adapter, because i am thinking about changing from a tuplespace (rinda) to redis. But i have trouble with inheritance for queries - like described here: http://datamapper.org/docs/misc.html

So far as i understood the adapter code - I think that the redis adapter does not consider the InclusionComparison correctly in the perform_query function.

Can anyone help me to fix this for the redis adapter?

Attached the spec that shows the problem - it actually only returns Woman (First test) and Mother (second test) but not all of them, which i espected to happed for the first test.

require 'spec_helper'

describe DataMapper::Adapters::RedisAdapter do
  before(:all) do
    @adapter = DataMapper.setup(:default, {
        :adapter  => "redis",
        :db => 15
    })
    @repository = DataMapper.repository(@adapter.name)
    @redis = Redis.new(:db => 15)
  end
  describe 'Inheritance' do

    before :all do

      class Person
        include DataMapper::Resource

        property :name, String
        property :job,  String,        :length => 1..255
        property :type, Discriminator
        property :id, Serial
      end

      class Male   < Person; end
      class Father < Male;   end
      class Son    < Male;   end

      class Woman    < Person; end
      class Mother   < Woman;  end
      class Daughter < Woman;  end

      DataMapper.finalize
    end

    it 'should select all women, mothers, and daughters based on Woman query' do
      w = Woman.create(:name => "woman")
      m = Mother.create(:name => "mother")
      d = Daughter.create(:name => "daughter")

      r = Woman.all
      r.to_set.should == [w,m,d].to_set
      r.size.should == [w,m,d].size
    end

    it 'should select all women, mothers, and daughters based on Person query' do
      w = Woman.create(:name => "woman")
      m = Mother.create(:name => "mother")
      d = Daughter.create(:name => "daughter")
      p = Person.all
      p.to_set.should == [w,m,d].to_set
      p.size.should == [w,m,d].size
    end

    it 'should select all mothers' do
      w = Woman.create(:name => "woman")
      m = Mother.create(:name => "mother")
      d = Daughter.create(:name => "daughter")

      mo = Mother.all
      mo.to_set.should == [m].to_set
      mo.size.should == [m].size
    end

  end
  after(:each) do
    @redis.flushdb
  end
end
@sfeu
Copy link
Author

sfeu commented Feb 2, 2012

Hey,

have started to work on this issue. A first implementation and an improved spec can be found in this branch.

Its already working in most situations, but there is a problem which i think is related to the dm-core that prevents
correct results if one queries for Person.all (see second test on spec above).

https://github.com/sfeu/dm-redis-adapter/tree/inheritance_fixes

@whoahbot
Copy link
Owner

whoahbot commented Feb 4, 2012

Hello!

Catching up on issues this weekend. I'll have a look at your branch.

@sfeu
Copy link
Author

sfeu commented Feb 4, 2012

Hey,

would be great if you could take a look at it. Not sure if i did it correctly - but at least the specs continue to work ;-)

I asked for help regarding the remaining issue, but did not get an answer so far...

http://groups.google.com/group/datamapper/browse_thread/thread/3365c68cb0a3d170

@whoahbot
Copy link
Owner

whoahbot commented Feb 5, 2012

Hey sfeu,

I had a look at this and I wasn't able to get much further than you were. At this point it looks as though it's doing a find with no conditions, and there isn't a simple way to express that all subclasses of the base type should be included. I'll keep working on it after I finish and merge the refactoring branch.

I'll keep an eye on the mailing list as well.

Thanks!

whoahbot

@sfeu
Copy link
Author

sfeu commented Feb 6, 2012

Hey whoahbot,

i was able to fix this problem by collecting all descendants myself for this special case. It works but unfortunately i have broken one of your tests ( "should allow has n :through"), because for the inheritance i need unique serials over all models:

  • initialize_serial(resource, @redis.incr("#{redis_key_for(resource.model)}:serial"))
  • initialize_serial(resource, @redis.incr("#{resource.model.to_s.downcase}:#{redis_key_for(resource.model)}:serial"))

i am not sure how to proceed...

When i played around with it and tried to change the broken test to run with the in_memory adapter instead of the redis one it fails there as well...

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

2 participants