From b65beb2d24fba9bcc4496df12e59c0fb07f90461 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Tue, 25 May 2010 09:35:41 -0700 Subject: [PATCH] Correctly handle more than 2 hierarchy levels in the single table inheritance 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. --- CHANGELOG | 2 ++ .../plugins/single_table_inheritance.rb | 25 ++++++++++++++----- .../single_table_inheritance_spec.rb | 21 +++++++++++++--- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index d6188fcb0c..a59e4b8ad4 100644 --- a/CHANGELOG +++ b/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) diff --git a/lib/sequel/plugins/single_table_inheritance.rb b/lib/sequel/plugins/single_table_inheritance.rb index 6c180ba996..21ff219cb5 100644 --- a/lib/sequel/plugins/single_table_inheritance.rb +++ b/lib/sequel/plugins/single_table_inheritance.rb @@ -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 @@ -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 != ''} @@ -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 @@ -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 @@ -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. diff --git a/spec/extensions/single_table_inheritance_spec.rb b/spec/extensions/single_table_inheritance_spec.rb index 05c5ddb2ef..28f199458c 100644 --- a/spec/extensions/single_table_inheritance_spec.rb +++ b/spec/extensions/single_table_inheritance_spec.rb @@ -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 @@ -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