Skip to content

Commit

Permalink
Correctly handle more than 2 hierarchy levels in the single table inh…
Browse files Browse the repository at this point in the history
…eritance plugin

Previously, Sequel did not handle more than 2 hierarchy levels in
the single table inheritance plugin.  If you were using more than
two levels, the documentation told you that you had to set up your
own custom filter.

With this change, you can now have as many class hierarchy levels
as you want, and Sequel will handle things automatically.

There shouldn't be any backwards compatibility problems, but this
does switch subclasses to use IN instead of = in their filter. That
may have performance implications, but any decent database is going
to treat an IN with one element the same as an =, so there should
not be any performance regression in the 2 level hierarchy case.
  • Loading branch information
jeremyevans committed May 25, 2010
1 parent 799a209 commit b65beb2
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 10 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG
@@ -1,5 +1,7 @@
=== HEAD

* Correctly handle more than 2 hierarchy levels in the single table inheritance plugin (jeremyevans)

* Allow using a custom column value<->class mapping to the single_table_inheritance plugin (jeremyevans, tmm1)

* Handle SQL::Identifiers in the schema_dumper extension (jeremyevans) (#304)
Expand Down
25 changes: 19 additions & 6 deletions lib/sequel/plugins/single_table_inheritance.rb
Expand Up @@ -20,11 +20,6 @@ module Plugins
# direct class references in the plugin class. You should specify subclasses
# in the plugin call using class name strings or symbols, see usage below.
#
# The filters and row_proc that sti_key sets up in subclasses may not work correctly if
# those subclasses have further subclasses. For those middle subclasses,
# you may need to call set_dataset manually with the correct filter and
# row_proc.
#
# Usage:
#
# # Use the default of storing the class name in the sti_key
Expand Down Expand Up @@ -56,6 +51,7 @@ module SingleTableInheritance
# Setup the necessary STI variables, see the module RDoc for SingleTableInheritance
def self.configure(model, key, opts={})
model.instance_eval do
@sti_key_array = nil
@sti_key = key
@sti_dataset = dataset
@sti_model_map = opts[:model_map] || lambda{|v| v if v && v != ''}
Expand Down Expand Up @@ -87,6 +83,10 @@ module ClassMethods
# The column name holding the STI key for this model
attr_reader :sti_key

# Array holding keys for all subclasses of this class, used for the
# dataset filter in subclasses. Nil in the main class.
attr_reader :sti_key_array

# A hash/proc with class keys and column value values, mapping
# the the class to a particular value given to the sti_key column.
# Used to set the column value when creating objects, and for the
Expand All @@ -105,9 +105,13 @@ def inherited(subclass)
sd = sti_dataset
skm = sti_key_map
smm = sti_model_map
subclass.set_dataset(sd.filter(SQL::QualifiedIdentifier.new(table_name, sk)=>skm[subclass]), :inherited=>true)
key = skm[subclass]
sti_subclass_added(key)
ska = [key]
subclass.set_dataset(sd.filter(SQL::QualifiedIdentifier.new(table_name, sk)=>ska), :inherited=>true)
subclass.instance_eval do
@sti_key = sk
@sti_key_array = ska
@sti_dataset = sd
@sti_key_map = skm
@sti_model_map = smm
Expand All @@ -121,6 +125,15 @@ def sti_load(r)
sti_class(sti_model_map[r[sti_key]]).load(r)
end

# Make sure that all subclasses of the parent class correctly include
# keys for all of their descendant classes.
def sti_subclass_added(key)
if sti_key_array
sti_key_array << key
superclass.sti_subclass_added(key)
end
end

private

# Return a class object. If a class is given, return it directly.
Expand Down
21 changes: 17 additions & 4 deletions spec/extensions/single_table_inheritance_spec.rb
Expand Up @@ -41,8 +41,8 @@ def @ds.fetch_rows(sql)
end
StiTest.all.collect{|x| x.class}.should == [StiTest, StiTestSub1, StiTestSub2]
StiTest.dataset.sql.should == "SELECT * FROM sti_tests"
StiTestSub1.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.blah = 'StiTestSub1')"
StiTestSub2.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.blah = 'StiTestSub2')"
StiTestSub1.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.blah IN ('StiTestSub1'))"
StiTestSub2.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.blah IN ('StiTestSub2'))"
end

it "should return rows with the correct class based on the polymorphic_key value" do
Expand Down Expand Up @@ -90,8 +90,21 @@ def @ds.fetch_rows(sql)

it "should add a filter to model datasets inside subclasses hook to only retreive objects with the matching key" do
StiTest.dataset.sql.should == "SELECT * FROM sti_tests"
StiTestSub1.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind = 'StiTestSub1')"
StiTestSub2.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind = 'StiTestSub2')"
StiTestSub1.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind IN ('StiTestSub1'))"
StiTestSub2.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind IN ('StiTestSub2'))"
end

it "should add a correct filter for multiple levels of subclasses" do
class ::StiTestSub1A < StiTestSub1; end
StiTestSub1.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind IN ('StiTestSub1', 'StiTestSub1A'))"
StiTestSub1A.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind IN ('StiTestSub1A'))"
class ::StiTestSub2A < StiTestSub2; end
StiTestSub2.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind IN ('StiTestSub2', 'StiTestSub2A'))"
StiTestSub2A.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind IN ('StiTestSub2A'))"
class ::StiTestSub1B < StiTestSub1A; end
StiTestSub1.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind IN ('StiTestSub1', 'StiTestSub1A', 'StiTestSub1B'))"
StiTestSub1A.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind IN ('StiTestSub1A', 'StiTestSub1B'))"
StiTestSub1B.dataset.sql.should == "SELECT * FROM sti_tests WHERE (sti_tests.kind IN ('StiTestSub1B'))"
end

context "with custom options" do
Expand Down

0 comments on commit b65beb2

Please sign in to comment.