Permalink
Browse files

Cache one_to_one associations using an object instead of an array

This is nearing the end of the one_to_one commits, but we still have
a couple to go.  Because one_to_one associations were originally
just one_to_many associations, they were cached as an array (with
a single object in it).  This commit switches that to use the
same type of caching at many_to_one associations, were nil is
a cached negative lookup, and where a non-nil value is the cached
lookup.

While here, add a limit(1) to the many_to_one association (in two
places, one of which will be removed soon).

There is a slight breakage of backwards compatibility in regards
to *_to_one associations with a nil :key option still addining a
limit(1), but that will be fixed soon.
  • Loading branch information...
1 parent ed948a1 commit f692c2d095119c7909d550afe638a7c9fa5fac40 @jeremyevans committed Mar 18, 2010
@@ -110,6 +110,7 @@ def reciprocal
keys = self[:keys]
associated_class.all_association_reflections.each do |assoc_reflect|
if r_types.include?(assoc_reflect[:type]) && assoc_reflect[:keys] == keys && assoc_reflect.associated_class == self[:model]
+ self[:reciprocal_type] = assoc_reflect[:type]
return self[:reciprocal] = assoc_reflect[:name]
end
end
@@ -201,19 +202,30 @@ def primary_keys
self[:primary_keys] ||= Array(primary_key)
end
alias associated_object_keys primary_keys
+
+ # True only if the reciprocal is a one_to_many association.
+ def reciprocal_array?
+ !set_reciprocal_to_self?
+ end
# Whether this association returns an array of objects instead of a single object,
# false for a many_to_one association.
def returns_array?
false
end
+
+ # True only if the reciprocal is a one_to_one association.
+ def set_reciprocal_to_self?
+ reciprocal
+ self[:reciprocal_type] == :one_to_one
+ end
private
# The reciprocal type of a many_to_one association is either
# a one_to_many or a one_to_one association.
def reciprocal_type
- [:one_to_many, :one_to_one]
+ self[:reciprocal_type] ||= [:one_to_many, :one_to_one]
end
end
@@ -269,6 +281,11 @@ def reciprocal_type
class OneToOneAssociationReflection < OneToManyAssociationReflection
ASSOCIATION_TYPES[:one_to_one] = self
+
+ # one_to_one associations return a single object, not an array
+ def returns_array?
+ false
+ end
end
class ManyToManyAssociationReflection < AssociationReflection
@@ -786,6 +803,7 @@ def def_many_to_one(opts)
name = opts[:name]
model = self
opts[:key] = opts.default_key unless opts.include?(:key)
+ opts[:limit] = opts.fetch(:limit, 1)
key = opts[:key]
cks = opts[:keys] = Array(opts[:key])
raise(Error, 'mismatched number of composite keys') if opts[:primary_key] && cks.length != Array(opts[:primary_key]).length
@@ -875,7 +893,6 @@ def def_one_to_many(opts)
end
def_association_dataset_methods(opts)
- association_module_def(opts.association_method){|*reload| load_associated_objects(opts, reload[0]).last} if opts[:one_to_one]
ck_nil_hash ={}
cks.each{|k| ck_nil_hash[k] = nil}
@@ -949,6 +966,7 @@ def _dataset(opts)
end
ds = ds.order(*opts[:order]) if opts[:order]
ds = ds.limit(*opts[:limit]) if opts[:limit]
+ ds = ds.limit(1) unless opts.returns_array?
ds = ds.eager(*opts[:eager]) if opts[:eager]
ds = ds.distinct if opts[:distinct]
ds = ds.eager_graph(opts[:eager_graph]) if opts[:eager_graph] && opts.eager_graph_lazy_dataset?
@@ -961,10 +979,8 @@ def _load_associated_objects(opts)
if opts.returns_array?
opts.can_have_associated_objects?(self) ? send(opts.dataset_method).all : []
else
- if !opts[:key]
+ if opts.can_have_associated_objects?(self)
send(opts.dataset_method).all.first
- elsif opts.can_have_associated_objects?(self)
- send(opts.dataset_method).first
end
end
end
@@ -1018,7 +1034,13 @@ def load_associated_objects(opts, reload=false)
else
objs = _load_associated_objects(opts)
run_association_callbacks(opts, :after_load, objs)
- objs.each{|o| add_reciprocal_object(opts, o)} if opts.set_reciprocal_to_self?
+ if opts.set_reciprocal_to_self?
+ if opts.returns_array?
+ objs.each{|o| add_reciprocal_object(opts, o)}
+ elsif objs
+ add_reciprocal_object(opts, objs)
+ end
+ end
associations[name] = objs
end
end
@@ -1104,11 +1126,11 @@ def set_associated_object(opts, o)
def set_one_to_one_associated_object(opts, o)
raise(Error, "object #{inspect} does not have a primary key") unless pk
run_association_callbacks(opts, :before_set, o)
- if arr = associations[opts[:name]] and a = arr.last
+ if a = associations[opts[:name]]
remove_reciprocal_object(opts, a)
end
send(opts._setter_method, o)
- associations[opts[:name]] = [o]
+ associations[opts[:name]] = o
add_reciprocal_object(opts, o) if o
run_association_callbacks(opts, :after_set, o)
o
@@ -28,7 +28,7 @@ def ds1.fetch_rows(s)
specify "should allow destroying associated many_to_one associated object" do
@Album.add_association_dependencies :artist=>:destroy
@Album.load(:id=>1, :name=>'Al', :artist_id=>2).destroy
- MODEL_DB.sqls.should == ['DELETE FROM albums WHERE (id = 1)', 'SELECT * FROM artists WHERE (artists.id = 2)', 'DELETE FROM artists WHERE (id = 2)']
+ MODEL_DB.sqls.should == ['DELETE FROM albums WHERE (id = 1)', 'SELECT * FROM artists WHERE (artists.id = 2) LIMIT 1', 'DELETE FROM artists WHERE (id = 2)']
end
specify "should allow deleting associated many_to_one associated object" do
@@ -86,14 +86,14 @@ def ds1.fetch_rows(s)
specify "should allow specifying association dependencies in the plugin call" do
@Album.plugin :association_dependencies, :artist=>:destroy
@Album.load(:id=>1, :name=>'Al', :artist_id=>2).destroy
- MODEL_DB.sqls.should == ['DELETE FROM albums WHERE (id = 1)', 'SELECT * FROM artists WHERE (artists.id = 2)', 'DELETE FROM artists WHERE (id = 2)']
+ MODEL_DB.sqls.should == ['DELETE FROM albums WHERE (id = 1)', 'SELECT * FROM artists WHERE (artists.id = 2) LIMIT 1', 'DELETE FROM artists WHERE (id = 2)']
end
specify "should work with subclasses" do
c = Class.new(@Album)
c.add_association_dependencies :artist=>:destroy
c.load(:id=>1, :name=>'Al', :artist_id=>2).destroy
- MODEL_DB.sqls.should == ['DELETE FROM albums WHERE (id = 1)', 'SELECT * FROM artists WHERE (artists.id = 2)', 'DELETE FROM artists WHERE (id = 2)']
+ MODEL_DB.sqls.should == ['DELETE FROM albums WHERE (id = 1)', 'SELECT * FROM artists WHERE (artists.id = 2) LIMIT 1', 'DELETE FROM artists WHERE (id = 2)']
MODEL_DB.reset
@Album.load(:id=>1, :name=>'Al', :artist_id=>2).destroy
@@ -103,6 +103,6 @@ def ds1.fetch_rows(s)
@Album.add_association_dependencies :artist=>:destroy
c2 = Class.new(@Album)
c2.load(:id=>1, :name=>'Al', :artist_id=>2).destroy
- MODEL_DB.sqls.should == ['DELETE FROM albums WHERE (id = 1)', 'SELECT * FROM artists WHERE (artists.id = 2)', 'DELETE FROM artists WHERE (id = 2)']
+ MODEL_DB.sqls.should == ['DELETE FROM albums WHERE (id = 1)', 'SELECT * FROM artists WHERE (artists.id = 2) LIMIT 1', 'DELETE FROM artists WHERE (id = 2)']
end
end
@@ -383,17 +383,12 @@ def ds.fetch_rows(sql, &block)
end
it "should have many_to_one setter deal with a one_to_one reciprocal" do
- @c2.many_to_one :parent, :class => @c2
+ @c2.many_to_one :parent, :class => @c2, :key=>:parent_id
@c2.one_to_one :child, :class => @c2, :key=>:parent_id
- ds = @c2.dataset
- def ds.fetch_rows(sql, &block)
- MODEL_DB.sqls << sql
- end
d = @c2.new(:id => 1)
e = @c2.new(:id => 2)
- MODEL_DB.sqls.should == []
- e.associations[:child] = []
+ e.associations[:child] = nil
d.parent = e
e.child.should == d
d.parent = nil
@@ -404,7 +399,7 @@ def ds.fetch_rows(sql, &block)
f = @c2.new(:id => 3)
d.parent = nil
e.child.should == nil
- e.associations[:child] = [f]
+ e.associations[:child] = f
d.parent = e
e.child.should == d
end
@@ -447,7 +442,7 @@ def ds.fetch_rows(sql, &block)
end
p = @c2.new(:parent_id=>2)
fgp = p.first_grand_parent
- MODEL_DB.sqls.should == ["SELECT nodes.id, nodes.parent_id, nodes.par_parent_id, nodes.blah, children.id AS children_id, children.parent_id AS children_parent_id, children.par_parent_id AS children_par_parent_id, children.blah AS children_blah FROM nodes LEFT OUTER JOIN nodes AS children ON (children.parent_id = nodes.id) WHERE (children_id = 2)"]
+ MODEL_DB.sqls.should == ["SELECT nodes.id, nodes.parent_id, nodes.par_parent_id, nodes.blah, children.id AS children_id, children.parent_id AS children_parent_id, children.par_parent_id AS children_par_parent_id, children.blah AS children_blah FROM nodes LEFT OUTER JOIN nodes AS children ON (children.parent_id = nodes.id) WHERE (children_id = 2) LIMIT 1"]
fgp.values.should == {:id=>1, :parent_id=>0, :par_parent_id=>3, :blah=>4}
fgp.children.first.values.should == {:id=>2, :parent_id=>1, :par_parent_id=>5, :blah=>6}
end
@@ -860,7 +855,7 @@ def ds.fetch_rows(sql, &block); MODEL_DB.sqls << sql; yield({:id=>234}) end
MODEL_DB.sqls.should == ["SELECT * FROM nodes WHERE (nodes.node_id = 1) LIMIT 1"]
d.parent
MODEL_DB.sqls.should == ["SELECT * FROM nodes WHERE (nodes.node_id = 1) LIMIT 1"]
- d.associations[:parent].should == [e]
+ d.associations[:parent].should == e
end
it "should set cached instance variable when assigned" do
@@ -871,14 +866,14 @@ def ds.fetch_rows(sql, &block); MODEL_DB.sqls << sql; yield({:id=>234}) end
e = @c2.load(:id => 234)
d.parent = e
f = d.parent
- d.associations[:parent].should == [e]
+ d.associations[:parent].should == e
e.should == f
end
it "should use cached instance variable if available" do
@c2.one_to_one :parent, :class => @c2
d = @c2.load(:id => 1, :parent_id => 234)
- d.associations[:parent] = [42]
+ d.associations[:parent] = 42
d.parent.should == 42
MODEL_DB.sqls.should == []
end
@@ -981,11 +976,11 @@ def blahr(x)
it "should support after_load association callback" do
h = []
- @c2.one_to_one :parent, :class => @c2, :after_load=>[proc{|x,y| h << [x.pk, y.first.pk]}, :al]
+ @c2.one_to_one :parent, :class => @c2, :after_load=>[proc{|x,y| h << [x.pk, y.pk]}, :al]
@c2.class_eval do
@@blah = h
def al(v)
- @@blah << v.first.pk
+ @@blah << v.pk
end
def @dataset.fetch_rows(sql)
yield({:id=>20})
@@ -1009,7 +1004,7 @@ def @dataset.fetch_rows(sql)
proc{p.parent = c}.should raise_error(Sequel::Error)
p.parent.should == nil
- p.associations[:parent] = [c]
+ p.associations[:parent] = c
p.parent.should == c
proc{p.parent = nil}.should raise_error(Sequel::Error)
end

0 comments on commit f692c2d

Please sign in to comment.