From 47691cc5081b145b81df6e1fd4806b78752c0541 Mon Sep 17 00:00:00 2001 From: Adam Williams Date: Tue, 29 Dec 2009 15:26:12 -0500 Subject: [PATCH] Fixed bug where sti finder names would not be generated correctly --- VERSION.yml | 2 +- dataset.gemspec | 7 +-- lib/dataset.rb | 1 + lib/dataset/database/base.rb | 15 ++++++- lib/dataset/record/heirarchy.rb | 65 +++++++++++++++++++++++++++ lib/dataset/record/meta.rb | 50 +++------------------ lib/dataset/session_binding.rb | 36 ++++++++------- spec/dataset/record/heirarchy_spec.rb | 14 ++++++ spec/dataset/record/meta_spec.rb | 14 ------ spec/dataset/session_binding_spec.rb | 5 +++ 10 files changed, 130 insertions(+), 79 deletions(-) create mode 100644 lib/dataset/record/heirarchy.rb create mode 100644 spec/dataset/record/heirarchy_spec.rb delete mode 100644 spec/dataset/record/meta_spec.rb diff --git a/VERSION.yml b/VERSION.yml index d7462e6..b7daf47 100644 --- a/VERSION.yml +++ b/VERSION.yml @@ -1,4 +1,4 @@ --- -:patch: 1 +:patch: 2 :major: 1 :minor: 3 diff --git a/dataset.gemspec b/dataset.gemspec index caededb..138288a 100644 --- a/dataset.gemspec +++ b/dataset.gemspec @@ -5,11 +5,11 @@ Gem::Specification.new do |s| s.name = %q{dataset} - s.version = "1.3.1" + s.version = "1.3.2" s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version= s.authors = ["Adam Williams"] - s.date = %q{2009-12-22} + s.date = %q{2009-12-29} s.description = %q{A simple API for creating and finding sets of data in your database, built on ActiveRecord.} s.email = %q{adam@thewilliams.ws} s.extra_rdoc_files = [ @@ -37,6 +37,7 @@ Gem::Specification.new do |s| "lib/dataset/instance_methods.rb", "lib/dataset/load.rb", "lib/dataset/record/fixture.rb", + "lib/dataset/record/heirarchy.rb", "lib/dataset/record/meta.rb", "lib/dataset/record/model.rb", "lib/dataset/resolver.rb", @@ -54,7 +55,7 @@ Gem::Specification.new do |s| s.test_files = [ "spec/dataset/cucumber_spec.rb", "spec/dataset/database/base_spec.rb", - "spec/dataset/record/meta_spec.rb", + "spec/dataset/record/heirarchy_spec.rb", "spec/dataset/resolver_spec.rb", "spec/dataset/rspec_spec.rb", "spec/dataset/session_binding_spec.rb", diff --git a/lib/dataset.rb b/lib/dataset.rb index 1aa0188..00d1e43 100644 --- a/lib/dataset.rb +++ b/lib/dataset.rb @@ -13,6 +13,7 @@ require 'dataset/resolver' require 'dataset/session' require 'dataset/session_binding' +require 'dataset/record/heirarchy' require 'dataset/record/meta' require 'dataset/record/fixture' require 'dataset/record/model' diff --git a/lib/dataset/database/base.rb b/lib/dataset/database/base.rb index 7396a13..313515c 100644 --- a/lib/dataset/database/base.rb +++ b/lib/dataset/database/base.rb @@ -17,14 +17,27 @@ def clear end end + def record_heirarchy(record_class) + base_class = record_class.base_class + record_heirarchies[base_class] ||= Dataset::Record::Heirarchy.new(base_class) + end + def record_meta(record_class) - record_metas[record_class] ||= Dataset::Record::Meta.new(record_class) + record_metas[record_class] ||= begin + heirarchy = record_heirarchy(record_class) + heirarchy.update(record_class) + Dataset::Record::Meta.new(heirarchy, record_class) + end end protected def record_metas @record_metas ||= Hash.new end + + def record_heirarchies + @record_heirarchies ||= Hash.new + end end end end \ No newline at end of file diff --git a/lib/dataset/record/heirarchy.rb b/lib/dataset/record/heirarchy.rb new file mode 100644 index 0000000..c38a3df --- /dev/null +++ b/lib/dataset/record/heirarchy.rb @@ -0,0 +1,65 @@ +module Dataset + module Record # :nodoc: + + class Heirarchy # :nodoc: + attr_reader :base_class, :class_name, :columns, :table_name + + delegate :inheritance_column, :to => :base_class + + def initialize(base_class) + @base_class = base_class + @class_name = base_class.name + @table_name = base_class.table_name + @columns = base_class.columns + end + + def id_cache_key + @id_cache_key ||= table_name + end + + def id_finder_names + @id_finder_names ||= [id_finder_name(base_class)] + end + + def model_finder_names + @model_finder_names ||= [model_finder_name(base_class)] + end + + def to_s + "#" + end + + def update(record_class) + record_class.ancestors.each do |c| + finder_name = model_finder_name(c) + unless model_finder_names.include?(finder_name) + model_finder_names << finder_name + id_finder_names << id_finder_name(c) + end + end + end + + def finder_name(klass) + klass.name.underscore.gsub('/', '_').sub(/^(\w)_/, '\1').gsub(/_(\w)_/, '_\1') + end + + def id_finder_name(klass) + "#{finder_name(klass)}_id".to_sym + end + + def model_finder_name(klass) + finder_name(klass).pluralize.to_sym + end + + def timestamp_columns + @timestamp_columns ||= begin + timestamps = %w(created_at created_on updated_at updated_on) + columns.select do |column| + timestamps.include?(column.name) + end + end + end + end + + end +end diff --git a/lib/dataset/record/meta.rb b/lib/dataset/record/meta.rb index fbf73a2..fbb31a1 100644 --- a/lib/dataset/record/meta.rb +++ b/lib/dataset/record/meta.rb @@ -4,62 +4,26 @@ module Record # :nodoc: # A mechanism to cache information about an ActiveRecord class to speed # things up a bit for insertions, finds, and method generation. class Meta # :nodoc: - attr_reader :class_name, :columns, :record_class, :table_name + attr_reader :heirarchy, :class_name, :record_class # Provides information necessary to insert STI classes correctly for # later reading. - delegate :name, :inheritance_column, :sti_name, :to => :record_class + delegate :name, :sti_name, :to => :record_class + delegate :inheritance_column, :table_name, :timestamp_columns, :to => :heirarchy - def initialize(record_class) - @record_class = record_class - @class_name = record_class.name - @table_name = record_class.table_name - @columns = record_class.columns - end - - def id_cache_key - @id_cache_key ||= table_name + def initialize(heirarchy, record_class) + @heirarchy = heirarchy + @record_class = record_class + @class_name = record_class.name end def inheriting_record? !record_class.descends_from_active_record? end - def timestamp_columns - @timestamp_columns ||= begin - timestamps = %w(created_at created_on updated_at updated_on) - columns.select do |column| - timestamps.include?(column.name) - end - end - end - - def id_finder_names - @id_finder_names ||= begin - names = descendants.collect {|c| finder_name c} - names.uniq.collect {|n| "#{n}_id".to_sym} - end - end - - def model_finder_names - @record_finder_names ||= descendants.collect {|c| finder_name(c).pluralize.to_sym}.uniq - end - def to_s "#" end - - def descendants - if record_class.respond_to?(:self_and_descendents_from_active_record) - record_class.self_and_descendents_from_active_record - else - record_class.self_and_descendants_from_active_record - end - end - - def finder_name(klass) - klass.name.underscore.gsub('/', '_').sub(/^(\w)_/, '\1').gsub(/_(\w)_/, '_\1') - end end end diff --git a/lib/dataset/session_binding.rb b/lib/dataset/session_binding.rb index efa9a04..3126256 100644 --- a/lib/dataset/session_binding.rb +++ b/lib/dataset/session_binding.rb @@ -5,8 +5,8 @@ module Dataset # raised. # class RecordNotFound < StandardError - def initialize(record_type, symbolic_name) - super "There is no '#{record_type.name}' found for the symbolic name ':#{symbolic_name}'." + def initialize(record_heirarchy, symbolic_name) + super "There is no '#{record_heirarchy.base_class.name}' found for the symbolic name ':#{symbolic_name}'." end end @@ -41,12 +41,12 @@ def initialize(record_type, symbolic_name) # people(:bobby) OR users(:bobby) # module ModelFinders - def create_finder(record_meta) # :nodoc: + def create_finders(record_meta) # :nodoc: @finders_generated ||= [] + heirarchy_finders_hash = record_meta.heirarchy.model_finder_names.join('').hash + return if @finders_generated.include?(heirarchy_finders_hash) - return if @finders_generated.include?(record_meta) - - record_meta.model_finder_names.each do |finder_name| + record_meta.heirarchy.model_finder_names.each do |finder_name| unless instance_methods.include?(finder_name) define_method finder_name do |*symbolic_names| names = Array(symbolic_names) @@ -58,7 +58,7 @@ def create_finder(record_meta) # :nodoc: end end - record_meta.id_finder_names.each do |finder_name| + record_meta.heirarchy.id_finder_names.each do |finder_name| unless instance_methods.include?(finder_name) define_method finder_name do |*symbolic_names| names = Array(symbolic_names) @@ -70,7 +70,7 @@ def create_finder(record_meta) # :nodoc: end end - @finders_generated << record_meta + @finders_generated << heirarchy_finders_hash end end @@ -207,23 +207,25 @@ def create_record(record_type, *args) def find_id(record_type_or_meta, symbolic_name) record_meta = record_meta_for_type(record_type_or_meta) - if local_id = @id_cache[record_meta.id_cache_key][symbolic_name] + heirarchy = record_meta.heirarchy + if local_id = @id_cache[heirarchy.id_cache_key][symbolic_name] local_id elsif !parent_binding.nil? parent_binding.find_id record_meta, symbolic_name else - raise RecordNotFound.new(record_meta, symbolic_name) + raise RecordNotFound.new(heirarchy, symbolic_name) end end def find_model(record_type_or_meta, symbolic_name) record_meta = record_meta_for_type(record_type_or_meta) - if local_id = @id_cache[record_meta.id_cache_key][symbolic_name] - record_meta.record_class.find local_id + heirarchy = record_meta.heirarchy + if local_id = @id_cache[heirarchy.id_cache_key][symbolic_name] + heirarchy.base_class.find local_id elsif !parent_binding.nil? parent_binding.find_model record_meta, symbolic_name else - raise RecordNotFound.new(record_meta, symbolic_name) + raise RecordNotFound.new(heirarchy, symbolic_name) end end @@ -235,8 +237,8 @@ def install_block_variables(target) def name_model(record, symbolic_name) record_meta = database.record_meta(record.class) - @model_finders.create_finder(record_meta) - @id_cache[record_meta.id_cache_key][symbolic_name] = record.id + @model_finders.create_finders(record_meta) + @id_cache[record_meta.heirarchy.id_cache_key][symbolic_name] = record.id record end @@ -258,10 +260,10 @@ def insert(dataset_record_class, record_type, *args) record = dataset_record_class.new(record_meta, attributes, symbolic_name, self) return_value = nil - @model_finders.create_finder(record_meta) + @model_finders.create_finders(record_meta) ActiveRecord::Base.silence do return_value = record.create - @id_cache[record_meta.id_cache_key][symbolic_name] = record.id + @id_cache[record_meta.heirarchy.id_cache_key][symbolic_name] = record.id end return_value end diff --git a/spec/dataset/record/heirarchy_spec.rb b/spec/dataset/record/heirarchy_spec.rb new file mode 100644 index 0000000..b362958 --- /dev/null +++ b/spec/dataset/record/heirarchy_spec.rb @@ -0,0 +1,14 @@ +require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper') + +class MThingy + class NThingy + end +end + +describe Dataset::Record::Heirarchy, 'finder name' do + it 'should collapse single character followed by underscore to just the single character' do + @heirarchy = Dataset::Record::Heirarchy.new(Place) + @heirarchy.finder_name(MThingy).should == 'mthingy' + @heirarchy.finder_name(MThingy::NThingy).should == 'mthingy_nthingy' + end +end \ No newline at end of file diff --git a/spec/dataset/record/meta_spec.rb b/spec/dataset/record/meta_spec.rb deleted file mode 100644 index ff94188..0000000 --- a/spec/dataset/record/meta_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper') - -class MThingy - class NThingy - end -end - -describe Dataset::Record::Meta, 'finder name' do - it 'should collapse single character followed by underscore to just the single character' do - @meta = Dataset::Record::Meta.new(Place) - @meta.finder_name(MThingy).should == 'mthingy' - @meta.finder_name(MThingy::NThingy).should == 'mthingy_nthingy' - end -end \ No newline at end of file diff --git a/spec/dataset/session_binding_spec.rb b/spec/dataset/session_binding_spec.rb index b2259d7..036bf98 100644 --- a/spec/dataset/session_binding_spec.rb +++ b/spec/dataset/session_binding_spec.rb @@ -142,18 +142,23 @@ describe 'name_model' do before do + @place = Place.create! + @binding.name_model(@place, :myplace) @state = State.create!(:name => 'NC') @binding.name_model(@state, :mystate) end it 'should allow assigning a name to a model for later lookup' do + @binding.find_model(Place, :myplace).should == @place @binding.find_model(State, :mystate).should == @state end it 'should allow finding STI' do @context = Object.new @context.extend @binding.model_finders + @context.places(:myplace).should == @place @context.places(:mystate).should == @state + @context.states(:mystate).should == @state end end