From dbab3cf1b48a76f02730270d35b56236d2dcbc11 Mon Sep 17 00:00:00 2001 From: Chance Downs Date: Mon, 6 Oct 2014 09:13:23 -0400 Subject: [PATCH 01/17] add bigint support for RealTime indices --- lib/thinking_sphinx/real_time/index.rb | 2 ++ .../real_time/index/template.rb | 2 +- spec/acceptance/big_integers_spec.rb | 32 +++++++++++++++---- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/thinking_sphinx/real_time/index.rb b/lib/thinking_sphinx/real_time/index.rb index 531652f5e..43ea17dfa 100644 --- a/lib/thinking_sphinx/real_time/index.rb +++ b/lib/thinking_sphinx/real_time/index.rb @@ -67,6 +67,8 @@ def collection_for(attribute) @rt_attr_timestamp when :float @rt_attr_float + when :bigint + attribute.multi? ? @rt_attr_multi_64 : @rt_attr_bigint else raise "Unknown attribute type '#{attribute.type}'" end diff --git a/lib/thinking_sphinx/real_time/index/template.rb b/lib/thinking_sphinx/real_time/index/template.rb index 170ec80f4..89d0de79b 100644 --- a/lib/thinking_sphinx/real_time/index/template.rb +++ b/lib/thinking_sphinx/real_time/index/template.rb @@ -8,7 +8,7 @@ def initialize(index) def apply add_field class_column, :sphinx_internal_class_name - add_attribute :id, :sphinx_internal_id, :integer + add_attribute :id, :sphinx_internal_id, :bigint add_attribute class_column, :sphinx_internal_class, :string, :facet => true add_attribute 0, :sphinx_deleted, :integer end diff --git a/spec/acceptance/big_integers_spec.rb b/spec/acceptance/big_integers_spec.rb index 259e887cd..845e120b6 100644 --- a/spec/acceptance/big_integers_spec.rb +++ b/spec/acceptance/big_integers_spec.rb @@ -12,8 +12,13 @@ indexes title } + real_time_index = ThinkingSphinx::RealTime::Index.new(:product) + real_time_index.definition_block = Proc.new { + indexes name + } + ThinkingSphinx::Configuration::ConsistentIds.new( - [small_index, large_index] + [small_index, large_index, real_time_index] ).reconcile large_index.sources.first.attributes.detect { |attribute| @@ -23,16 +28,31 @@ small_index.sources.first.attributes.detect { |attribute| attribute.name == 'sphinx_internal_id' }.type.should == :bigint + + real_time_index.attributes.detect { |attribute| + attribute.name == 'sphinx_internal_id' + }.type.should == :bigint end end describe '64 bit document ids', :live => true do - it 'handles large 32 bit integers with an offset multiplier' do - user = User.create! :name => 'Pat' - user.update_column :id, 980190962 + context 'with ActiveRecord' do + it 'handles large 32 bit integers with an offset multiplier' do + user = User.create! :name => 'Pat' + user.update_column :id, 980190962 + + index - index + expect(User.search('pat').to_a).to eq([user]) + end + end - expect(User.search('pat').to_a).to eq([user]) + context 'with Real-Time' do + it 'handles large 32 bit integers with an offset multiplier' do + product = Product.create! :name => "Widget" + product.update_attribute :id, 980190962 + expect(Product.search('widget').to_a).to eq([product]) + end end + end From 46e73dc9fb128603a8aa608d7f843ecca94b55fe Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Thu, 13 Nov 2014 23:15:07 +1100 Subject: [PATCH 02/17] Simplifying STI sql_query generation. Looks like ActiveRecord filters by the appropriate classes already - so let's not double-up (also, wasn't taking into account sub-subclasses). --- .../active_record/sql_builder.rb | 4 - .../active_record/sql_builder/statement.rb | 1 - spec/acceptance/searching_with_sti_spec.rb | 26 +++-- spec/internal/app/indices/bird_index.rb | 4 + .../active_record/sql_builder_spec.rb | 105 ------------------ 5 files changed, 23 insertions(+), 117 deletions(-) create mode 100644 spec/internal/app/indices/bird_index.rb diff --git a/lib/thinking_sphinx/active_record/sql_builder.rb b/lib/thinking_sphinx/active_record/sql_builder.rb index 277659473..f611bb477 100644 --- a/lib/thinking_sphinx/active_record/sql_builder.rb +++ b/lib/thinking_sphinx/active_record/sql_builder.rb @@ -100,10 +100,6 @@ def reversed_document_id "($id - #{source.offset}) / #{config.indices.count}" end - def inheritance_column_condition - "#{quoted_inheritance_column} = '#{model_name}'" - end - def range_condition condition = [] condition << "#{quoted_primary_key} BETWEEN $start AND $end" unless source.disable_range? diff --git a/lib/thinking_sphinx/active_record/sql_builder/statement.rb b/lib/thinking_sphinx/active_record/sql_builder/statement.rb index a9bcf7db5..0c468cfbd 100644 --- a/lib/thinking_sphinx/active_record/sql_builder/statement.rb +++ b/lib/thinking_sphinx/active_record/sql_builder/statement.rb @@ -129,7 +129,6 @@ def select_clause def where_clause(for_range = false) builder = SQLBuilder::ClauseBuilder.new(nil) - builder.add_clause inheritance_column_condition unless model.descends_from_active_record? builder.add_clause delta_processor.clause(source.delta?) if delta_processor builder.add_clause range_condition unless for_range builder.separated(' AND ') diff --git a/spec/acceptance/searching_with_sti_spec.rb b/spec/acceptance/searching_with_sti_spec.rb index 672cf0072..da4a98170 100644 --- a/spec/acceptance/searching_with_sti_spec.rb +++ b/spec/acceptance/searching_with_sti_spec.rb @@ -6,7 +6,7 @@ duck = Bird.create :name => 'Duck' index - Animal.search.to_a.should == [platypus, duck] + Animal.search(:indices => ['animal_core']).to_a.should == [platypus, duck] end it "limits results based on subclasses" do @@ -14,7 +14,7 @@ duck = Bird.create :name => 'Duck' index - Bird.search.to_a.should == [duck] + Bird.search(:indices => ['animal_core']).to_a.should == [duck] end it "returns results for deeper subclasses when searching on their parents" do @@ -23,7 +23,7 @@ emu = FlightlessBird.create :name => 'Emu' index - Bird.search.to_a.should == [duck, emu] + Bird.search(:indices => ['animal_core']).to_a.should == [duck, emu] end it "returns results for deeper subclasses" do @@ -32,7 +32,7 @@ emu = FlightlessBird.create :name => 'Emu' index - FlightlessBird.search.to_a.should == [emu] + FlightlessBird.search(:indices => ['animal_core']).to_a.should == [emu] end it "filters out sibling subclasses" do @@ -41,7 +41,7 @@ otter = Mammal.create :name => 'Otter' index - Bird.search.to_a.should == [duck] + Bird.search(:indices => ['animal_core']).to_a.should == [duck] end it "obeys :classes if supplied" do @@ -50,13 +50,25 @@ emu = FlightlessBird.create :name => 'Emu' index - Bird.search(nil, :skip_sti => true, :classes=>[Bird, FlightlessBird]).to_a.should == [duck, emu] + Bird.search( + :indices => ['animal_core'], + :skip_sti => true, + :classes => [Bird, FlightlessBird] + ).to_a.should == [duck, emu] end it 'finds root objects when type is blank' do animal = Animal.create :name => 'Animal', type: '' index - Animal.search.to_a.should == [animal] + Animal.search(:indices => ['animal_core']).to_a.should == [animal] + end + + it 'allows for indices on mid-hierarchy classes' do + duck = Bird.create :name => 'Duck' + emu = FlightlessBird.create :name => 'Emu' + index + + Bird.search(:indices => ['bird_core']).to_a.should == [duck, emu] end end diff --git a/spec/internal/app/indices/bird_index.rb b/spec/internal/app/indices/bird_index.rb new file mode 100644 index 000000000..6879ae1c3 --- /dev/null +++ b/spec/internal/app/indices/bird_index.rb @@ -0,0 +1,4 @@ +FlightlessBird +ThinkingSphinx::Index.define :bird, :with => :active_record do + indexes name +end diff --git a/spec/thinking_sphinx/active_record/sql_builder_spec.rb b/spec/thinking_sphinx/active_record/sql_builder_spec.rb index edfdc131c..c7f853b0f 100644 --- a/spec/thinking_sphinx/active_record/sql_builder_spec.rb +++ b/spec/thinking_sphinx/active_record/sql_builder_spec.rb @@ -192,27 +192,6 @@ model.stub! :store_full_sti_class => true end - it "limits results to just the model" do - relation.should_receive(:where) do |string| - string.should match(/`users`.`type` = 'User'/) - relation - end - - builder.sql_query - end - - it "uses the demodulised name if that's what is stored" do - model.stub! :store_full_sti_class => false - model.name.stub! :demodulize => 'U' - - relation.should_receive(:where) do |string| - string.should match(/`users`.`type` = 'U'/) - relation - end - - builder.sql_query - end - it "groups by the inheritance column" do relation.should_receive(:group) do |string| string.should match(/`users`.`type`/) @@ -228,15 +207,6 @@ model.stub :inheritance_column => 'custom_type' end - it "limits results on the right column" do - relation.should_receive(:where) do |string| - string.should match(/`users`.`custom_type` = 'User'/) - relation - end - - builder.sql_query - end - it "groups by the right column" do relation.should_receive(:group) do |string| string.should match(/`users`.`custom_type`/) @@ -452,27 +422,6 @@ model.stub! :store_full_sti_class => true end - it "limits results to just the model" do - relation.should_receive(:where) do |string| - string.should match(/"users"."type" = 'User'/) - relation - end - - builder.sql_query - end - - it "uses the demodulised name if that's what is stored" do - model.stub! :store_full_sti_class => false - model.name.stub! :demodulize => 'U' - - relation.should_receive(:where) do |string| - string.should match(/"users"."type" = 'U'/) - relation - end - - builder.sql_query - end - it "groups by the inheritance column" do relation.should_receive(:group) do |string| string.should match(/"users"."type"/) @@ -488,15 +437,6 @@ model.stub :inheritance_column => 'custom_type' end - it "limits results on the right column" do - relation.should_receive(:where) do |string| - string.should match(/"users"."custom_type" = 'User'/) - relation - end - - builder.sql_query - end - it "groups by the right column" do relation.should_receive(:group) do |string| string.should match(/"users"."custom_type"/) @@ -675,51 +615,6 @@ builder.sql_query_range end - context 'STI model' do - before :each do - model.column_names << 'type' - model.stub! :descends_from_active_record? => false - model.stub! :store_full_sti_class => true - end - - it "limits results to just the model" do - relation.should_receive(:where) do |string| - string.should match(/`users`.`type` = 'User'/) - relation - end - - builder.sql_query_range - end - - it "uses the demodulised name if that's what is stored" do - model.stub! :store_full_sti_class => false - model.name.stub! :demodulize => 'U' - - relation.should_receive(:where) do |string| - string.should match(/`users`.`type` = 'U'/) - relation - end - - builder.sql_query_range - end - - context 'with a custom inheritance column' do - before :each do - model.column_names << 'custom_type' - model.stub :inheritance_column => 'custom_type' - end - - it "limits results on the right column" do - relation.should_receive(:where) do |string| - string.should match(/`users`.`custom_type` = 'User'/) - relation - end - - builder.sql_query_range - end - end - end - context 'with a delta processor' do let(:processor) { double('processor') } From a24102d7e6dbf9a969bf03c382404722d8f61e0b Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Sat, 15 Nov 2014 21:06:59 +0800 Subject: [PATCH 03/17] Allow for custom IndexSet classes. --- .../callbacks/delete_callbacks.rb | 4 +- .../callbacks/delta_callbacks.rb | 2 +- lib/thinking_sphinx/configuration.rb | 9 ++-- lib/thinking_sphinx/facet_search.rb | 10 ++++- lib/thinking_sphinx/index_set.rb | 44 ++++++++++++++----- lib/thinking_sphinx/middlewares/sphinxql.rb | 4 +- .../callbacks/delta_callbacks_spec.rb | 8 ++-- spec/thinking_sphinx/configuration_spec.rb | 13 ------ spec/thinking_sphinx/facet_search_spec.rb | 3 +- spec/thinking_sphinx/index_set_spec.rb | 44 +++++++++++++------ .../middlewares/sphinxql_spec.rb | 10 +++-- 11 files changed, 97 insertions(+), 54 deletions(-) diff --git a/lib/thinking_sphinx/active_record/callbacks/delete_callbacks.rb b/lib/thinking_sphinx/active_record/callbacks/delete_callbacks.rb index c2df15c66..8d3c961e8 100644 --- a/lib/thinking_sphinx/active_record/callbacks/delete_callbacks.rb +++ b/lib/thinking_sphinx/active_record/callbacks/delete_callbacks.rb @@ -14,6 +14,8 @@ def after_destroy private def indices - ThinkingSphinx::IndexSet.new([instance.class], []).to_a + ThinkingSphinx::Configuration.instance.index_set_class.new( + :classes => [instance.class] + ).to_a end end diff --git a/lib/thinking_sphinx/active_record/callbacks/delta_callbacks.rb b/lib/thinking_sphinx/active_record/callbacks/delta_callbacks.rb index 6d9272976..1789b6bc2 100644 --- a/lib/thinking_sphinx/active_record/callbacks/delta_callbacks.rb +++ b/lib/thinking_sphinx/active_record/callbacks/delta_callbacks.rb @@ -42,7 +42,7 @@ def delta_indices? end def indices - @indices ||= ThinkingSphinx::IndexSet.new [instance.class], [] + @indices ||= config.index_set_class.new :classes => [instance.class] end def processors diff --git a/lib/thinking_sphinx/configuration.rb b/lib/thinking_sphinx/configuration.rb index e5b7e1d38..1dfdb16c1 100644 --- a/lib/thinking_sphinx/configuration.rb +++ b/lib/thinking_sphinx/configuration.rb @@ -3,7 +3,7 @@ class ThinkingSphinx::Configuration < Riddle::Configuration attr_accessor :configuration_file, :indices_location, :version attr_reader :index_paths - attr_writer :controller + attr_writer :controller, :index_set_class delegate :environment, :to => :framework @@ -56,9 +56,12 @@ def engine_indice_paths end end + def index_set_class + @index_set_class ||= ThinkingSphinx::IndexSet + end + def indices_for_references(*references) - preload_indices - indices.select { |index| references.include?(index.reference) } + index_set_class.new(:references => references).to_a end def next_offset(reference) diff --git a/lib/thinking_sphinx/facet_search.rb b/lib/thinking_sphinx/facet_search.rb index 5ba3d5caa..bb1170fa2 100644 --- a/lib/thinking_sphinx/facet_search.rb +++ b/lib/thinking_sphinx/facet_search.rb @@ -63,6 +63,10 @@ def to_hash private + def configuration + ThinkingSphinx::Configuration.instance + end + def facets @facets ||= properties.group_by(&:name).collect { |name, matches| ThinkingSphinx::Facet.new name, matches @@ -92,11 +96,13 @@ def facet_names(facets) end def indices - @indices ||= ThinkingSphinx::IndexSet.new options[:classes], options[:indices] + @indices ||= configuration.index_set_class.new( + options.slice(:classes, :indices) + ) end def max_matches - ThinkingSphinx::Configuration.instance.settings['max_matches'] || 1000 + configuration.settings['max_matches'] || 1000 end def limit diff --git a/lib/thinking_sphinx/index_set.rb b/lib/thinking_sphinx/index_set.rb index b65a93c82..185f49bef 100644 --- a/lib/thinking_sphinx/index_set.rb +++ b/lib/thinking_sphinx/index_set.rb @@ -3,9 +3,9 @@ class ThinkingSphinx::IndexSet delegate :each, :empty?, :to => :indices - def initialize(classes, index_names, configuration = nil) - @classes = classes || [] - @index_names = index_names + def initialize(options = {}, configuration = nil) + @options = options + @index_names = options[:indices] || [] @configuration = configuration || ThinkingSphinx::Configuration.instance end @@ -19,7 +19,20 @@ def to_a private - attr_reader :classes, :configuration, :index_names + attr_reader :configuration, :options + + def all_indices + configuration.preload_indices + configuration.indices + end + + def classes + options[:classes] || [] + end + + def classes_specified? + classes.any? || references_specified? + end def classes_and_ancestors @classes_and_ancestors ||= classes.collect { |model| @@ -31,21 +44,30 @@ def classes_and_ancestors }.flatten end - def indices - configuration.preload_indices + def index_names + options[:indices] || [] + end - return configuration.indices.select { |index| + def indices + return all_indices.select { |index| index_names.include?(index.name) - } if index_names && index_names.any? + } if index_names.any? - everything = classes.empty? ? configuration.indices : - configuration.indices_for_references(*references) + everything = classes_specified? ? indices_for_references : all_indices everything.reject &:distributed? end + def indices_for_references + all_indices.select { |index| references.include? index.reference } + end + def references - classes_and_ancestors.collect { |klass| + options[:references] || classes_and_ancestors.collect { |klass| klass.name.underscore.to_sym } end + + def references_specified? + options[:references] && options[:references].any? + end end diff --git a/lib/thinking_sphinx/middlewares/sphinxql.rb b/lib/thinking_sphinx/middlewares/sphinxql.rb index 275413d1e..21115e924 100644 --- a/lib/thinking_sphinx/middlewares/sphinxql.rb +++ b/lib/thinking_sphinx/middlewares/sphinxql.rb @@ -134,7 +134,9 @@ def index_options def indices @indices ||= begin - set = ThinkingSphinx::IndexSet.new classes, options[:indices] + set = configuration.index_set_class.new( + options.slice(:classes, :indices) + ) raise ThinkingSphinx::NoIndicesError if set.empty? set end diff --git a/spec/thinking_sphinx/active_record/callbacks/delta_callbacks_spec.rb b/spec/thinking_sphinx/active_record/callbacks/delta_callbacks_spec.rb index 948fc07c1..d5f50f080 100644 --- a/spec/thinking_sphinx/active_record/callbacks/delta_callbacks_spec.rb +++ b/spec/thinking_sphinx/active_record/callbacks/delta_callbacks_spec.rb @@ -46,7 +46,7 @@ } before :each do - ThinkingSphinx::IndexSet.stub :new => [index] + config.stub :index_set_class => double(:new => [index]) end context 'without delta indices' do @@ -72,7 +72,9 @@ before :each do ThinkingSphinx::Deltas.stub :suspended? => false - ThinkingSphinx::IndexSet.stub :new => [core_index, delta_index] + config.stub :index_set_class => double( + :new => [core_index, delta_index] + ) end it "only indexes delta indices" do @@ -127,7 +129,7 @@ } before :each do - ThinkingSphinx::IndexSet.stub :new => [index] + config.stub :index_set_class => double(:new => [index]) end it "sets delta to true if there are delta indices" do diff --git a/spec/thinking_sphinx/configuration_spec.rb b/spec/thinking_sphinx/configuration_spec.rb index ff4641c9e..4a190cab0 100644 --- a/spec/thinking_sphinx/configuration_spec.rb +++ b/spec/thinking_sphinx/configuration_spec.rb @@ -101,19 +101,6 @@ end end - describe '#indices_for_references' do - it "selects from the full index set those with matching references" do - config.preload_indices - config.indices.clear - - config.indices << double('index', :reference => :article) - config.indices << double('index', :reference => :book) - config.indices << double('index', :reference => :page) - - config.indices_for_references(:book, :article).length.should == 2 - end - end - describe '#indices_location' do it "stores index files in db/sphinx/ENVIRONMENT" do config.indices_location. diff --git a/spec/thinking_sphinx/facet_search_spec.rb b/spec/thinking_sphinx/facet_search_spec.rb index a9fb90843..bab2f32c3 100644 --- a/spec/thinking_sphinx/facet_search_spec.rb +++ b/spec/thinking_sphinx/facet_search_spec.rb @@ -10,10 +10,9 @@ :multi? => false) } let(:property_b) { double('property', :name => 'category_id', :multi? => false) } - let(:configuration) { double 'configuration', :settings => {} } + let(:configuration) { double 'configuration', :settings => {}, :index_set_class => double(:new => index_set) } before :each do - stub_const 'ThinkingSphinx::IndexSet', double(:new => index_set) stub_const 'ThinkingSphinx::BatchedSearch', double(:new => batch) stub_const 'ThinkingSphinx::Search', DumbSearch stub_const 'ThinkingSphinx::Middlewares::RAW_ONLY', double diff --git a/spec/thinking_sphinx/index_set_spec.rb b/spec/thinking_sphinx/index_set_spec.rb index bb4a5071d..5c58a7cfd 100644 --- a/spec/thinking_sphinx/index_set_spec.rb +++ b/spec/thinking_sphinx/index_set_spec.rb @@ -5,13 +5,11 @@ module ThinkingSphinx; end require 'thinking_sphinx/index_set' describe ThinkingSphinx::IndexSet do - let(:set) { ThinkingSphinx::IndexSet.new classes, indices, - configuration } - let(:classes) { [] } - let(:indices) { [] } + let(:set) { ThinkingSphinx::IndexSet.new options, configuration } let(:configuration) { double('configuration', :preload_indices => true, :indices => []) } let(:ar_base) { double('ActiveRecord::Base') } + let(:options) { {} } before :each do stub_const 'ActiveRecord::Base', ar_base @@ -43,21 +41,29 @@ def class_double(name, *superclasses) end it "uses indices for the given classes" do - classes << class_double('Article') + configuration.indices.replace [ + double(:reference => :article, :distributed? => false), + double(:reference => :opinion_article, :distributed? => false), + double(:reference => :page, :distributed? => false) + ] - configuration.should_receive(:indices_for_references).with(:article). - and_return([]) + options[:classes] = [class_double('Article')] - set.to_a + set.to_a.length.should == 1 end it "requests indices for any superclasses" do - classes << class_double('OpinionArticle', class_double('Article')) + configuration.indices.replace [ + double(:reference => :article, :distributed? => false), + double(:reference => :opinion_article, :distributed? => false), + double(:reference => :page, :distributed? => false) + ] - configuration.should_receive(:indices_for_references). - with(:opinion_article, :article).and_return([]) + options[:classes] = [ + class_double('OpinionArticle', class_double('Article')) + ] - set.to_a + set.to_a.length.should == 2 end it "uses named indices if names are provided" do @@ -65,9 +71,21 @@ def class_double(name, *superclasses) user_core = double('index', :name => 'user_core') configuration.indices.replace [article_core, user_core] - indices << 'article_core' + options[:indices] = ['article_core'] set.to_a.should == [article_core] end + + it "selects from the full index set those with matching references" do + configuration.indices.replace [ + double('index', :reference => :article, :distributed? => false), + double('index', :reference => :book, :distributed? => false), + double('index', :reference => :page, :distributed? => false) + ] + + options[:references] = [:book, :article] + + set.to_a.length.should == 2 + end end end diff --git a/spec/thinking_sphinx/middlewares/sphinxql_spec.rb b/spec/thinking_sphinx/middlewares/sphinxql_spec.rb index e147c9ff1..1c448c426 100644 --- a/spec/thinking_sphinx/middlewares/sphinxql_spec.rb +++ b/spec/thinking_sphinx/middlewares/sphinxql_spec.rb @@ -28,12 +28,13 @@ class SphinxQLSubclass let(:sphinx_sql) { double('sphinx_sql', :from => true, :offset => true, :limit => true, :where => true, :matching => true, :values => true) } let(:query) { double('query') } - let(:configuration) { double('configuration', :settings => {}) } + let(:configuration) { double('configuration', :settings => {}, + index_set_class: set_class) } + let(:set_class) { double(:new => index_set) } before :each do stub_const 'Riddle::Query::Select', double(:new => sphinx_sql) stub_const 'ThinkingSphinx::Search::Query', double(:new => query) - stub_const 'ThinkingSphinx::IndexSet', double(:new => index_set) context.stub :search => search, :configuration => configuration end @@ -58,8 +59,9 @@ class SphinxQLSubclass search.options[:indices] = ['user_core'] index_set.first.stub :reference => :user - ThinkingSphinx::IndexSet.should_receive(:new). - with([klass], ['user_core']).and_return(index_set) + set_class.should_receive(:new). + with(:classes => [klass], :indices => ['user_core']). + and_return(index_set) middleware.call [context] end From f4c34b083ca90a54115272ec53972701cf1c890a Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Sat, 15 Nov 2014 21:07:42 +0800 Subject: [PATCH 04/17] Don't cache index sets on a class/callback combination. --- .../real_time/callbacks/real_time_callbacks.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/thinking_sphinx/real_time/callbacks/real_time_callbacks.rb b/lib/thinking_sphinx/real_time/callbacks/real_time_callbacks.rb index 578651ffd..8030cba0e 100644 --- a/lib/thinking_sphinx/real_time/callbacks/real_time_callbacks.rb +++ b/lib/thinking_sphinx/real_time/callbacks/real_time_callbacks.rb @@ -27,7 +27,7 @@ def configuration end def indices - @indices ||= configuration.indices_for_references reference + configuration.indices_for_references reference end def objects_for(instance) @@ -45,8 +45,6 @@ def real_time_indices? end def real_time_indices - @real_time_indices ||= indices.select { |index| - index.is_a? ThinkingSphinx::RealTime::Index - } + indices.select { |index| index.is_a? ThinkingSphinx::RealTime::Index } end end From 43110fdcf9e8598422b91d87bf2faf931b64df91 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Sat, 15 Nov 2014 21:09:16 +0800 Subject: [PATCH 05/17] Allow customisable offset references. This allows records in different PostgreSQL schemas (e.g. using the Apartment gem) with the same primary key to be treated as different Sphinx records. --- lib/thinking_sphinx/core/index.rb | 2 +- spec/acceptance/big_integers_spec.rb | 5 +- spec/acceptance/facets_spec.rb | 3 +- spec/acceptance/real_time_updates_spec.rb | 8 ++++ .../acceptance/remove_deleted_records_spec.rb | 6 ++- .../searching_across_schemas_spec.rb | 38 +++++++++++++++ spec/internal/app/indices/product_index.rb | 21 +++++++++ spec/support/multi_schema.rb | 46 +++++++++++++++++++ 8 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 spec/acceptance/searching_across_schemas_spec.rb create mode 100644 spec/support/multi_schema.rb diff --git a/lib/thinking_sphinx/core/index.rb b/lib/thinking_sphinx/core/index.rb index b045655cb..0a9d1ad22 100644 --- a/lib/thinking_sphinx/core/index.rb +++ b/lib/thinking_sphinx/core/index.rb @@ -12,7 +12,7 @@ def initialize(reference, options = {}) @docinfo = :extern @charset_type = 'utf-8' @options = options - @offset = config.next_offset(reference) + @offset = config.next_offset(options[:offset_as] || reference) @type = 'plain' super "#{options[:name] || reference.to_s.gsub('/', '_')}_#{name_suffix}" diff --git a/spec/acceptance/big_integers_spec.rb b/spec/acceptance/big_integers_spec.rb index 845e120b6..2106aaa98 100644 --- a/spec/acceptance/big_integers_spec.rb +++ b/spec/acceptance/big_integers_spec.rb @@ -51,8 +51,9 @@ it 'handles large 32 bit integers with an offset multiplier' do product = Product.create! :name => "Widget" product.update_attribute :id, 980190962 - expect(Product.search('widget').to_a).to eq([product]) + expect( + Product.search('widget', :indices => ['product_core']).to_a + ).to eq([product]) end end - end diff --git a/spec/acceptance/facets_spec.rb b/spec/acceptance/facets_spec.rb index 577c9e0f9..5c49467c5 100644 --- a/spec/acceptance/facets_spec.rb +++ b/spec/acceptance/facets_spec.rb @@ -25,14 +25,13 @@ Tee.create! Tee.create! City.create! - Product.create! Article.create! index article_count = ENV['SPHINX_VERSION'].try(:[], /2.0.\d/) ? 2 : 1 ThinkingSphinx.facets.to_hash[:class].should == { - 'Tee' => 2, 'City' => 1, 'Product' => 1, 'Article' => article_count + 'Tee' => 2, 'City' => 1, 'Article' => article_count } end diff --git a/spec/acceptance/real_time_updates_spec.rb b/spec/acceptance/real_time_updates_spec.rb index a20c8bfc5..57d3d273a 100644 --- a/spec/acceptance/real_time_updates_spec.rb +++ b/spec/acceptance/real_time_updates_spec.rb @@ -6,4 +6,12 @@ Product.search.first.should == product end + + it "handles attributes for sortable fields accordingly" do + product = Product.create! :name => 'Red Fish' + product.update_attributes :name => 'Blue Fish' + + Product.search('blue fish', :indices => ['product_core']).to_a. + should == [product] + end end diff --git a/spec/acceptance/remove_deleted_records_spec.rb b/spec/acceptance/remove_deleted_records_spec.rb index 4ed2116b2..1101afd50 100644 --- a/spec/acceptance/remove_deleted_records_spec.rb +++ b/spec/acceptance/remove_deleted_records_spec.rb @@ -24,11 +24,13 @@ it "removes records from real-time index results" do product = Product.create! :name => 'Shiny' - Product.search('Shiny').to_a.should == [product] + Product.search('Shiny', :indices => ['product_core']).to_a. + should == [product] product.destroy - Product.search_for_ids('Shiny').should be_empty + Product.search_for_ids('Shiny', :indices => ['product_core']). + should be_empty end it "deletes STI child classes from parent indices" do diff --git a/spec/acceptance/searching_across_schemas_spec.rb b/spec/acceptance/searching_across_schemas_spec.rb new file mode 100644 index 000000000..b50daed6b --- /dev/null +++ b/spec/acceptance/searching_across_schemas_spec.rb @@ -0,0 +1,38 @@ +require 'acceptance/spec_helper' + +multi_schema = MultiSchema.new + +describe 'Searching across PostgreSQL schemas', :live => true do + before :each do + ThinkingSphinx::Configuration.instance.index_set_class = + MultiSchema::IndexSet + end + + after :each do + ThinkingSphinx::Configuration.instance.index_set_class = nil + multi_schema.switch :public + end + + it 'can distinguish between objects with the same primary key' do + multi_schema.switch :public + jekyll = Product.create name: 'Doctor Jekyll' + Product.search('Jekyll', :retry_stale => false).to_a.should == [jekyll] + Product.search(:retry_stale => false).to_a.should == [jekyll] + + multi_schema.switch :thinking_sphinx + hyde = Product.create name: 'Mister Hyde' + Product.search('Jekyll', :retry_stale => false).to_a.should == [] + Product.search('Hyde', :retry_stale => false).to_a.should == [hyde] + Product.search(:retry_stale => false).to_a.should == [hyde] + + multi_schema.switch :public + Product.search('Jekyll', :retry_stale => false).to_a.should == [jekyll] + Product.search(:retry_stale => false).to_a.should == [jekyll] + Product.search('Hyde', :retry_stale => false).to_a.should == [] + + Product.search( + :middleware => ThinkingSphinx::Middlewares::RAW_ONLY, + :indices => ['product_core', 'product_two_core'] + ).to_a.length.should == 2 + end +end if multi_schema.active? diff --git a/spec/internal/app/indices/product_index.rb b/spec/internal/app/indices/product_index.rb index 49abf8f32..9cbef0d94 100644 --- a/spec/internal/app/indices/product_index.rb +++ b/spec/internal/app/indices/product_index.rb @@ -1,5 +1,26 @@ +multi_schema = MultiSchema.new + ThinkingSphinx::Index.define :product, :with => :real_time do indexes name, :sortable => true has category_ids, :type => :integer, :multi => true end + +if multi_schema.active? + multi_schema.create 'thinking_sphinx' + + ThinkingSphinx::Index.define(:product, + :name => :product_two, :offset_as => :product_two, :with => :real_time + ) do + indexes name, prefixes: true + + set_property min_prefix_len: 1, dict: :keywords + + scope do + multi_schema.switch :thinking_sphinx + User + end + end + + multi_schema.switch :public +end diff --git a/spec/support/multi_schema.rb b/spec/support/multi_schema.rb new file mode 100644 index 000000000..d5cf6b6bb --- /dev/null +++ b/spec/support/multi_schema.rb @@ -0,0 +1,46 @@ +class MultiSchema + def active? + ENV['DATABASE'] == 'postgresql' + end + + def create(schema_name) + unless connection.schema_exists? schema_name + connection.execute %Q{CREATE SCHEMA "#{schema_name}"} + end + + switch schema_name + silence_stream(STDOUT) { load Rails.root.join('db', 'schema.rb') } + end + + def current + connection.schema_search_path + end + + def switch(schema_name) + connection.schema_search_path = %Q{"#{schema_name}"} + connection.clear_query_cache + end + + private + + def connection + ActiveRecord::Base.connection + end + + class IndexSet < ThinkingSphinx::IndexSet + private + + def indices + return super if index_names.any? + + prefixed = !multi_schema.current.include?('public') + super.select { |index| + prefixed ? index.name[/_two_core$/] : index.name[/_two_core$/].nil? + } + end + + def multi_schema + @multi_schema ||= MultiSchema.new + end + end +end From 87928c9298111818c9dd42e8c7faebfc8ae41c0b Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Wed, 24 Dec 2014 00:30:51 +1100 Subject: [PATCH 06/17] Switch from ActiveSupport's on_load to railtie initializer. This resolves the insistent warning about errors in transactions, covered in #867. --- lib/thinking_sphinx/railtie.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/thinking_sphinx/railtie.rb b/lib/thinking_sphinx/railtie.rb index ab77e7e78..1d8a17aa8 100644 --- a/lib/thinking_sphinx/railtie.rb +++ b/lib/thinking_sphinx/railtie.rb @@ -1,6 +1,8 @@ class ThinkingSphinx::Railtie < Rails::Railtie - ActiveSupport.on_load :active_record do - include ThinkingSphinx::ActiveRecord::Base + initializer 'thinking_sphinx.initialisation' do + if defined?(ActiveRecord::Base) + ActiveRecord::Base.send :include, ThinkingSphinx::ActiveRecord::Base + end end rake_tasks do From 4b4d7ba5154eb831f0b046471c7c013b3fc9e7aa Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Mon, 29 Dec 2014 23:13:03 +1100 Subject: [PATCH 07/17] Updating Rails versions, adding 4.2 --- Appraisals | 10 +++++++--- gemfiles/rails_3_2.gemfile | 2 +- gemfiles/rails_4_0.gemfile | 2 +- gemfiles/rails_4_1.gemfile | 2 +- gemfiles/rails_4_2.gemfile | 11 +++++++++++ 5 files changed, 21 insertions(+), 6 deletions(-) create mode 100644 gemfiles/rails_4_2.gemfile diff --git a/Appraisals b/Appraisals index 334806fe2..36aa693ad 100644 --- a/Appraisals +++ b/Appraisals @@ -1,11 +1,15 @@ appraise 'rails_3_2' do - gem 'rails', '~> 3.2.0' + gem 'rails', '~> 3.2.21' end appraise 'rails_4_0' do - gem 'rails', '~> 4.0.2' + gem 'rails', '~> 4.0.12' end appraise 'rails_4_1' do - gem 'rails', '~> 4.1.0.beta1' + gem 'rails', '~> 4.1.8' +end + +appraise 'rails_4_2' do + gem 'rails', '~> 4.2.0' end diff --git a/gemfiles/rails_3_2.gemfile b/gemfiles/rails_3_2.gemfile index 38a85ca09..6e8bfa3bf 100644 --- a/gemfiles/rails_3_2.gemfile +++ b/gemfiles/rails_3_2.gemfile @@ -6,6 +6,6 @@ gem "mysql2", "~> 0.3.12b4", :platform=>:ruby gem "pg", "~> 0.16.0", :platform=>:ruby gem "activerecord-jdbcmysql-adapter", "~> 1.3.4", :platform=>:jruby gem "activerecord-jdbcpostgresql-adapter", "~> 1.3.4", :platform=>:jruby -gem "rails", "~> 3.2.0" +gem "rails", "~> 3.2.21" gemspec :path=>"../" \ No newline at end of file diff --git a/gemfiles/rails_4_0.gemfile b/gemfiles/rails_4_0.gemfile index d9aade516..7b8dfd189 100644 --- a/gemfiles/rails_4_0.gemfile +++ b/gemfiles/rails_4_0.gemfile @@ -6,6 +6,6 @@ gem "mysql2", "~> 0.3.12b4", :platform=>:ruby gem "pg", "~> 0.16.0", :platform=>:ruby gem "activerecord-jdbcmysql-adapter", "~> 1.3.4", :platform=>:jruby gem "activerecord-jdbcpostgresql-adapter", "~> 1.3.4", :platform=>:jruby -gem "rails", "~> 4.0.2" +gem "rails", "~> 4.0.12" gemspec :path=>"../" \ No newline at end of file diff --git a/gemfiles/rails_4_1.gemfile b/gemfiles/rails_4_1.gemfile index fa19233fd..f79b9af5d 100644 --- a/gemfiles/rails_4_1.gemfile +++ b/gemfiles/rails_4_1.gemfile @@ -6,6 +6,6 @@ gem "mysql2", "~> 0.3.12b4", :platform=>:ruby gem "pg", "~> 0.16.0", :platform=>:ruby gem "activerecord-jdbcmysql-adapter", "~> 1.3.4", :platform=>:jruby gem "activerecord-jdbcpostgresql-adapter", "~> 1.3.4", :platform=>:jruby -gem "rails", "~> 4.1.0.beta1" +gem "rails", "~> 4.1.8" gemspec :path=>"../" \ No newline at end of file diff --git a/gemfiles/rails_4_2.gemfile b/gemfiles/rails_4_2.gemfile new file mode 100644 index 000000000..7bf8eb808 --- /dev/null +++ b/gemfiles/rails_4_2.gemfile @@ -0,0 +1,11 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "mysql2", "~> 0.3.12b4", :platform=>:ruby +gem "pg", "~> 0.16.0", :platform=>:ruby +gem "activerecord-jdbcmysql-adapter", "~> 1.3.4", :platform=>:jruby +gem "activerecord-jdbcpostgresql-adapter", "~> 1.3.4", :platform=>:jruby +gem "rails", "~> 4.2.0" + +gemspec :path=>"../" \ No newline at end of file From fc65a952fac8dc5211d31dc4566ae1a1758170e5 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Mon, 29 Dec 2014 23:13:32 +1100 Subject: [PATCH 08/17] Add Rails 4.1 and 4.2 to Travis test matrix. --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 9dcafb1ee..fa3722781 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,3 +19,5 @@ env: gemfile: - gemfiles/rails_3_2.gemfile - gemfiles/rails_4_0.gemfile + - gemfiles/rails_4_1.gemfile + - gemfiles/rails_4_2.gemfile From 45a0696311c3d8c17b23084cb4792585babc188c Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Mon, 29 Dec 2014 23:14:35 +1100 Subject: [PATCH 09/17] Add required timestamps null option for Rails 4.2. --- spec/internal/db/schema.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/internal/db/schema.rb b/spec/internal/db/schema.rb index 767bf71ba..62e8936d6 100644 --- a/spec/internal/db/schema.rb +++ b/spec/internal/db/schema.rb @@ -1,7 +1,7 @@ ActiveRecord::Schema.define do create_table(:admin_people, :force => true) do |t| t.string :name - t.timestamps + t.timestamps null: false end create_table(:animals, :force => true) do |t| @@ -14,7 +14,7 @@ t.text :content t.boolean :published t.integer :user_id - t.timestamps + t.timestamps null: false end create_table(:manufacturers, :force => true) do |t| @@ -33,7 +33,7 @@ t.string :blurb_file t.boolean :delta, :default => true, :null => false t.string :type, :default => 'Book', :null => false - t.timestamps + t.timestamps null: false end create_table(:books_genres, :force => true, :id => false) do |t| @@ -58,7 +58,7 @@ create_table(:colours, :force => true) do |t| t.string :name - t.timestamps + t.timestamps null: false end create_table(:events, :force => true) do |t| @@ -77,27 +77,27 @@ create_table(:taggings, :force => true) do |t| t.integer :tag_id t.integer :article_id - t.timestamps + t.timestamps null: false end create_table(:tags, :force => true) do |t| t.string :name - t.timestamps + t.timestamps null: false end create_table(:tees, :force => true) do |t| t.integer :colour_id - t.timestamps + t.timestamps null: false end create_table(:tweets, :force => true, :id => false) do |t| t.column :id, :bigint, :null => false t.string :text - t.timestamps + t.timestamps null: false end create_table(:users, :force => true) do |t| t.string :name - t.timestamps + t.timestamps null: false end end From a5cf43d8ee95f25e989e131f73f69fe9475667e3 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Mon, 29 Dec 2014 23:14:58 +1100 Subject: [PATCH 10/17] Load multi-schema before TS, Combustion. --- spec/spec_helper.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 888e5dda7..00ddd84e0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,11 +3,12 @@ Bundler.require :default, :development +root = File.expand_path File.dirname(__FILE__) +require "#{root}/support/multi_schema" require 'thinking_sphinx/railtie' Combustion.initialize! :active_record -root = File.expand_path File.dirname(__FILE__) Dir["#{root}/support/**/*.rb"].each { |file| require file } RSpec.configure do |config| From 4821647d4b9dd4d9e0d254ed647585b8c2db32bd Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Mon, 29 Dec 2014 23:16:35 +1100 Subject: [PATCH 11/17] Can't rely on ActiveRecord::Base.reflections Rails 4.2 has switched the hash keys to strings. reflect_on_association accepts both string or key parameter. --- lib/thinking_sphinx/active_record/polymorpher.rb | 6 +++--- lib/thinking_sphinx/active_record/property_query.rb | 2 +- lib/thinking_sphinx/active_record/simple_many_query.rb | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/thinking_sphinx/active_record/polymorpher.rb b/lib/thinking_sphinx/active_record/polymorpher.rb index 04a6a82d1..240a1b5b9 100644 --- a/lib/thinking_sphinx/active_record/polymorpher.rb +++ b/lib/thinking_sphinx/active_record/polymorpher.rb @@ -14,7 +14,7 @@ def morph! def append_reflections mappings.each do |class_name, name| - next if klass.reflections[name] + next if klass.reflect_on_association(name) reflection = clone_with name, class_name if ActiveRecord::Reflection.respond_to?(:add_reflection) @@ -51,12 +51,12 @@ def morph_properties end def reflection - @reflection ||= klass.reflections[column.__name] + @reflection ||= klass.reflect_on_association column.__name end def klass @klass ||= column.__stack.inject(source.model) { |parent, key| - parent.reflections[key].klass + parent.reflect_on_association(key).klass } end end diff --git a/lib/thinking_sphinx/active_record/property_query.rb b/lib/thinking_sphinx/active_record/property_query.rb index 060c0562c..df2ca96c8 100644 --- a/lib/thinking_sphinx/active_record/property_query.rb +++ b/lib/thinking_sphinx/active_record/property_query.rb @@ -110,7 +110,7 @@ def reflections base = source.model column.__stack.collect { |key| - reflection = base.reflections[key] + reflection = base.reflect_on_association key base = reflection.klass extend_reflection reflection diff --git a/lib/thinking_sphinx/active_record/simple_many_query.rb b/lib/thinking_sphinx/active_record/simple_many_query.rb index 13959bd61..943694bde 100644 --- a/lib/thinking_sphinx/active_record/simple_many_query.rb +++ b/lib/thinking_sphinx/active_record/simple_many_query.rb @@ -8,7 +8,7 @@ def to_s private def reflection - @reflection ||= source.model.reflections[column.__stack.first] + @reflection ||= source.model.reflect_on_association column.__stack.first end def quoted_foreign_key From c22d11b5a18e66a60342c86209fd857d3d093f00 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Mon, 29 Dec 2014 23:17:56 +1100 Subject: [PATCH 12/17] Workaround for association reflection class changes. Now no longer using a subclass of AssociationReflection - we weren't using anything within the class anymore anyway. So, that's good. Supporting three different approaches for generating associations isn't quite so fun though. --- lib/thinking_sphinx/active_record.rb | 2 +- .../active_record/filter_reflection.rb | 75 ++++++++ .../active_record/filtered_reflection.rb | 75 -------- .../active_record/polymorpher.rb | 2 +- .../active_record/filter_reflection_spec.rb | 172 ++++++++++++++++++ .../active_record/filtered_reflection_spec.rb | 141 -------------- .../active_record/polymorpher_spec.rb | 29 +-- 7 files changed, 267 insertions(+), 229 deletions(-) create mode 100644 lib/thinking_sphinx/active_record/filter_reflection.rb delete mode 100644 lib/thinking_sphinx/active_record/filtered_reflection.rb create mode 100644 spec/thinking_sphinx/active_record/filter_reflection_spec.rb delete mode 100644 spec/thinking_sphinx/active_record/filtered_reflection_spec.rb diff --git a/lib/thinking_sphinx/active_record.rb b/lib/thinking_sphinx/active_record.rb index 285d4024b..c78f668ed 100644 --- a/lib/thinking_sphinx/active_record.rb +++ b/lib/thinking_sphinx/active_record.rb @@ -14,7 +14,7 @@ module Callbacks; end require 'thinking_sphinx/active_record/column_sql_presenter' require 'thinking_sphinx/active_record/database_adapters' require 'thinking_sphinx/active_record/field' -require 'thinking_sphinx/active_record/filtered_reflection' +require 'thinking_sphinx/active_record/filter_reflection' require 'thinking_sphinx/active_record/index' require 'thinking_sphinx/active_record/interpreter' require 'thinking_sphinx/active_record/join_association' diff --git a/lib/thinking_sphinx/active_record/filter_reflection.rb b/lib/thinking_sphinx/active_record/filter_reflection.rb new file mode 100644 index 000000000..d2dff4845 --- /dev/null +++ b/lib/thinking_sphinx/active_record/filter_reflection.rb @@ -0,0 +1,75 @@ +class ThinkingSphinx::ActiveRecord::FilterReflection + attr_reader :reflection, :class_name + + delegate :foreign_type, :active_record, :to => :reflection + + def self.call(reflection, name, class_name) + filter = new(reflection, class_name) + klass = reflection.class + + if defined?(ActiveRecord::Reflection::MacroReflection) + klass.new name, filter.scope, filter.options, reflection.active_record + elsif reflection.respond_to?(:scope) + klass.new reflection.macro, name, filter.scope, filter.options, + reflection.active_record + else + klass.new reflection.macro, name, filter.options, + reflection.active_record + end + end + + def initialize(reflection, class_name) + @reflection, @class_name = reflection, class_name + @options = reflection.options.clone + end + + def options + @options.delete :polymorphic + @options[:class_name] = class_name + @options[:foreign_key] ||= "#{reflection.name}_id" + @options[:foreign_type] = reflection.foreign_type + + if reflection.respond_to?(:scope) + @options[:sphinx_internal_filtered] = true + return @options + end + + case @options[:conditions] + when nil + @options[:conditions] = condition + when Array + @options[:conditions] << condition + when Hash + @options[:conditions].merge!(reflection.foreign_type => @options[:class_name]) + else + @options[:conditions] << " AND #{condition}" + end + + @options + end + + def scope + if ::Joiner::Joins.instance_methods.include?(:join_association_class) + return nil + end + + lambda { |association| + reflection = association.reflection + klass = reflection.class_name.constantize + where( + association.parent.aliased_table_name.to_sym => + {reflection.foreign_type => klass.base_class.name} + ) + } + end + + private + + def condition + "::ts_join_alias::.#{quoted_foreign_type} = '#{class_name}'" + end + + def quoted_foreign_type + active_record.connection.quote_column_name foreign_type + end +end diff --git a/lib/thinking_sphinx/active_record/filtered_reflection.rb b/lib/thinking_sphinx/active_record/filtered_reflection.rb deleted file mode 100644 index 3d7a608d4..000000000 --- a/lib/thinking_sphinx/active_record/filtered_reflection.rb +++ /dev/null @@ -1,75 +0,0 @@ -class ThinkingSphinx::ActiveRecord::FilteredReflection < - ActiveRecord::Reflection::AssociationReflection - - class Filter - attr_reader :reflection, :class_name - - delegate :foreign_type, :active_record, :to => :reflection - - def initialize(reflection, class_name) - @reflection, @class_name = reflection, class_name - @options = reflection.options.clone - end - - def options - @options.delete :polymorphic - @options[:class_name] = class_name - @options[:foreign_key] ||= "#{reflection.name}_id" - @options[:foreign_type] = reflection.foreign_type - - if reflection.respond_to?(:scope) - @options[:sphinx_internal_filtered] = true - return @options - end - - case @options[:conditions] - when nil - @options[:conditions] = condition - when Array - @options[:conditions] << condition - when Hash - @options[:conditions].merge!(reflection.foreign_type => @options[:class_name]) - else - @options[:conditions] << " AND #{condition}" - end - - @options - end - - def scope - if ::Joiner::Joins.instance_methods.include?(:join_association_class) - return nil - end - - lambda { |association| - reflection = association.reflection - klass = reflection.class_name.constantize - where( - association.parent.aliased_table_name.to_sym => - {reflection.foreign_type => klass.base_class.name} - ) - } - end - - private - - def condition - "::ts_join_alias::.#{quoted_foreign_type} = '#{class_name}'" - end - - def quoted_foreign_type - active_record.connection.quote_column_name foreign_type - end - end - - def self.clone_with_filter(reflection, name, class_name) - filter = Filter.new(reflection, class_name) - - if reflection.respond_to?(:scope) - new reflection.macro, name, filter.scope, filter.options, - reflection.active_record - else - new reflection.macro, name, filter.options, reflection.active_record - end - end -end diff --git a/lib/thinking_sphinx/active_record/polymorpher.rb b/lib/thinking_sphinx/active_record/polymorpher.rb index 240a1b5b9..92c061c6c 100644 --- a/lib/thinking_sphinx/active_record/polymorpher.rb +++ b/lib/thinking_sphinx/active_record/polymorpher.rb @@ -26,7 +26,7 @@ def append_reflections end def clone_with(name, class_name) - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( + ThinkingSphinx::ActiveRecord::FilterReflection.call( reflection, name, class_name ) end diff --git a/spec/thinking_sphinx/active_record/filter_reflection_spec.rb b/spec/thinking_sphinx/active_record/filter_reflection_spec.rb new file mode 100644 index 000000000..0bef35593 --- /dev/null +++ b/spec/thinking_sphinx/active_record/filter_reflection_spec.rb @@ -0,0 +1,172 @@ +require 'spec_helper' + +describe ThinkingSphinx::ActiveRecord::FilterReflection do + describe '.call' do + let(:reflection) { double('Reflection', :macro => :has_some, + :options => options, :active_record => double, :name => 'baz', + :foreign_type => :foo_type, :class => reflection_klass) } + let(:options) { {:polymorphic => true} } + let(:filtered_reflection) { double 'filtered reflection' } + let(:reflection_klass) { double :new => filtered_reflection } + + before :each do + reflection.active_record.stub_chain(:connection, :quote_column_name). + and_return('"foo_type"') + end + + it "uses the existing reflection's macro" do + reflection_klass.should_receive(:new). + with(:has_some, anything, anything, anything) + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end unless defined?(ActiveRecord::Reflection::MacroReflection) + + it "uses the supplied name" do + if defined?(ActiveRecord::Reflection::MacroReflection) + reflection_klass.should_receive(:new). + with('foo_bar', anything, anything, anything) + else + reflection_klass.should_receive(:new). + with(anything, 'foo_bar', anything, anything) + end + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end + + it "uses the existing reflection's parent" do + if defined?(ActiveRecord::Reflection::MacroReflection) + reflection_klass.should_receive(:new). + with(anything, anything, anything, reflection.active_record) + else + reflection_klass.should_receive(:new). + with(anything, anything, anything, reflection.active_record) + end + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end + + it "removes the polymorphic setting from the options" do + if defined?(ActiveRecord::Reflection::MacroReflection) + reflection_klass.should_receive(:new) do |name, scope, options, parent| + options[:polymorphic].should be_nil + end + else + reflection_klass.should_receive(:new) do |macro, name, options, parent| + options[:polymorphic].should be_nil + end + end + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end + + it "adds the class name option" do + if defined?(ActiveRecord::Reflection::MacroReflection) + reflection_klass.should_receive(:new) do |name, scope, options, parent| + options[:class_name].should == 'Bar' + end + else + reflection_klass.should_receive(:new) do |macro, name, options, parent| + options[:class_name].should == 'Bar' + end + end + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end + + it "sets the foreign key if necessary" do + if defined?(ActiveRecord::Reflection::MacroReflection) + reflection_klass.should_receive(:new) do |name, scope, options, parent| + options[:foreign_key].should == 'baz_id' + end + else + reflection_klass.should_receive(:new) do |macro, name, options, parent| + options[:foreign_key].should == 'baz_id' + end + end + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end + + it "respects supplied foreign keys" do + options[:foreign_key] = 'qux_id' + + if defined?(ActiveRecord::Reflection::MacroReflection) + reflection_klass.should_receive(:new) do |name, scope, options, parent| + options[:foreign_key].should == 'qux_id' + end + else + reflection_klass.should_receive(:new) do |macro, name, options, parent| + options[:foreign_key].should == 'qux_id' + end + end + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end + + it "sets conditions if there are none" do + reflection_klass.should_receive(:new) do |macro, name, options, parent| + options[:conditions].should == "::ts_join_alias::.\"foo_type\" = 'Bar'" + end + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end unless defined?(ActiveRecord::Reflection::MacroReflection) + + it "appends to the conditions array" do + options[:conditions] = ['existing'] + + reflection_klass.should_receive(:new) do |macro, name, options, parent| + options[:conditions].should == ['existing', "::ts_join_alias::.\"foo_type\" = 'Bar'"] + end + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end unless defined?(ActiveRecord::Reflection::MacroReflection) + + it "extends the conditions hash" do + options[:conditions] = {:x => :y} + + reflection_klass.should_receive(:new) do |macro, name, options, parent| + options[:conditions].should == {:x => :y, :foo_type => 'Bar'} + end + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end unless defined?(ActiveRecord::Reflection::MacroReflection) + + it "appends to the conditions string" do + options[:conditions] = 'existing' + + reflection_klass.should_receive(:new) do |macro, name, options, parent| + options[:conditions].should == "existing AND ::ts_join_alias::.\"foo_type\" = 'Bar'" + end + + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ) + end unless defined?(ActiveRecord::Reflection::MacroReflection) + + it "returns the new reflection" do + ThinkingSphinx::ActiveRecord::FilterReflection.call( + reflection, 'foo_bar', 'Bar' + ).should == filtered_reflection + end + end +end diff --git a/spec/thinking_sphinx/active_record/filtered_reflection_spec.rb b/spec/thinking_sphinx/active_record/filtered_reflection_spec.rb deleted file mode 100644 index 7f0e14413..000000000 --- a/spec/thinking_sphinx/active_record/filtered_reflection_spec.rb +++ /dev/null @@ -1,141 +0,0 @@ -require 'spec_helper' - -describe ThinkingSphinx::ActiveRecord::FilteredReflection do - describe '.clone_with_filter' do - let(:reflection) { double('Reflection', :macro => :has_some, - :options => options, :active_record => double, :name => 'baz', - :foreign_type => :foo_type) } - let(:options) { {:polymorphic => true} } - let(:filtered_reflection) { double } - - before :each do - ThinkingSphinx::ActiveRecord::FilteredReflection.stub( - :new => filtered_reflection - ) - - reflection.active_record.stub_chain(:connection, :quote_column_name). - and_return('"foo_type"') - end - - it "uses the existing reflection's macro" do - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new). - with(:has_some, anything, anything, anything) - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "uses the supplied name" do - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new). - with(anything, 'foo_bar', anything, anything) - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "uses the existing reflection's parent" do - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new). - with(anything, anything, anything, reflection.active_record) - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "removes the polymorphic setting from the options" do - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new) do |macro, name, options, parent| - options[:polymorphic].should be_nil - end - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "adds the class name option" do - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new) do |macro, name, options, parent| - options[:class_name].should == 'Bar' - end - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "sets the foreign key if necessary" do - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new) do |macro, name, options, parent| - options[:foreign_key].should == 'baz_id' - end - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "respects supplied foreign keys" do - options[:foreign_key] = 'qux_id' - - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new) do |macro, name, options, parent| - options[:foreign_key].should == 'qux_id' - end - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "sets conditions if there are none" do - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new) do |macro, name, options, parent| - options[:conditions].should == "::ts_join_alias::.\"foo_type\" = 'Bar'" - end - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "appends to the conditions array" do - options[:conditions] = ['existing'] - - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new) do |macro, name, options, parent| - options[:conditions].should == ['existing', "::ts_join_alias::.\"foo_type\" = 'Bar'"] - end - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "extends the conditions hash" do - options[:conditions] = {:x => :y} - - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new) do |macro, name, options, parent| - options[:conditions].should == {:x => :y, :foo_type => 'Bar'} - end - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "appends to the conditions string" do - options[:conditions] = 'existing' - - ThinkingSphinx::ActiveRecord::FilteredReflection.should_receive(:new) do |macro, name, options, parent| - options[:conditions].should == "existing AND ::ts_join_alias::.\"foo_type\" = 'Bar'" - end - - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ) - end - - it "returns the new reflection" do - ThinkingSphinx::ActiveRecord::FilteredReflection.clone_with_filter( - reflection, 'foo_bar', 'Bar' - ).should == filtered_reflection - end - end -end diff --git a/spec/thinking_sphinx/active_record/polymorpher_spec.rb b/spec/thinking_sphinx/active_record/polymorpher_spec.rb index 9ed78a6a7..646c1c0ef 100644 --- a/spec/thinking_sphinx/active_record/polymorpher_spec.rb +++ b/spec/thinking_sphinx/active_record/polymorpher_spec.rb @@ -10,9 +10,12 @@ let(:class_names) { %w( Article Animal ) } let(:field) { double :rebase => true } let(:attribute) { double :rebase => true } - let(:outer) { double :reflections => {:a => double(:klass => inner)} } - let(:inner) { double :reflections => {:b => double(:klass => model)} } - let(:model) { double 'Model', :reflections => {:foo => reflection} } + let(:outer) { double( + :reflect_on_association => double(:klass => inner)) } + let(:inner) { double( + :reflect_on_association => double(:klass => model)) } + let(:model) { double 'Model', :reflections => {}, + :reflect_on_association => reflection } let(:reflection) { double 'Polymorphic Reflection' } describe '#morph!' do @@ -20,25 +23,29 @@ let(:animal_reflection) { double 'Animal Reflection' } before :each do - ThinkingSphinx::ActiveRecord::FilteredReflection. - stub(:clone_with_filter). + ThinkingSphinx::ActiveRecord::FilterReflection. + stub(:call). and_return(article_reflection, animal_reflection) + model.stub(:reflect_on_association) do |name| + name == :foo ? reflection : nil + end + if ActiveRecord::Reflection.respond_to?(:add_reflection) ActiveRecord::Reflection.stub :add_reflection end end it "creates a new reflection for each class" do - ThinkingSphinx::ActiveRecord::FilteredReflection. - unstub :clone_with_filter + ThinkingSphinx::ActiveRecord::FilterReflection. + unstub :call - ThinkingSphinx::ActiveRecord::FilteredReflection. - should_receive(:clone_with_filter). + ThinkingSphinx::ActiveRecord::FilterReflection. + should_receive(:call). with(reflection, :foo_article, 'Article'). and_return(article_reflection) - ThinkingSphinx::ActiveRecord::FilteredReflection. - should_receive(:clone_with_filter). + ThinkingSphinx::ActiveRecord::FilterReflection. + should_receive(:call). with(reflection, :foo_animal, 'Animal'). and_return(animal_reflection) From 8673ab5a796cc956dba6ce01bdf99cc60dbd8088 Mon Sep 17 00:00:00 2001 From: Bryan Ricker Date: Wed, 31 Dec 2014 20:24:01 -0800 Subject: [PATCH 13/17] Convert raw results to array --- lib/thinking_sphinx/middlewares/inquirer.rb | 2 +- .../middlewares/inquirer_spec.rb | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/thinking_sphinx/middlewares/inquirer.rb b/lib/thinking_sphinx/middlewares/inquirer.rb index 73fb807c0..75bd45dad 100644 --- a/lib/thinking_sphinx/middlewares/inquirer.rb +++ b/lib/thinking_sphinx/middlewares/inquirer.rb @@ -44,7 +44,7 @@ def initialize(context) end def call(raw_results, meta_results) - context[:results] = raw_results + context[:results] = raw_results.to_a context[:raw] = raw_results context[:meta] = meta_results.inject({}) { |hash, row| hash[row['Variable_name']] = row['Value'] diff --git a/spec/thinking_sphinx/middlewares/inquirer_spec.rb b/spec/thinking_sphinx/middlewares/inquirer_spec.rb index 6777800a0..4867f0f33 100644 --- a/spec/thinking_sphinx/middlewares/inquirer_spec.rb +++ b/spec/thinking_sphinx/middlewares/inquirer_spec.rb @@ -47,5 +47,24 @@ module Middlewares; end context[:results].should == [:raw] end + + context "with mysql2 result" do + class FakeResult + include Enumerable + def each; [{"fake" => "value"}].each { |m| yield m }; end + end + + let(:batch_inquirer) { double('batcher', :append_query => true, + :results => [ + FakeResult.new, [{'Variable_name' => 'meta', 'Value' => 'value'}] + ]) + } + + it "converts the results into an array" do + middleware.call [context] + + context[:results].should be_a Array + end + end end end From 13ffd27caf2e9caf94dade9f960b038fee9df7ed Mon Sep 17 00:00:00 2001 From: Andrew Cone Date: Wed, 14 Jan 2015 18:12:04 -0800 Subject: [PATCH 14/17] proposed close to #878 --- lib/thinking_sphinx.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/thinking_sphinx.rb b/lib/thinking_sphinx.rb index 60366a6ef..c219fca57 100644 --- a/lib/thinking_sphinx.rb +++ b/lib/thinking_sphinx.rb @@ -77,4 +77,4 @@ module Subscribers; end require 'thinking_sphinx/logger' require 'thinking_sphinx/real_time' -require 'thinking_sphinx/railtie' if defined?(Rails) +require 'thinking_sphinx/railtie' if defined?(Rails::Railtie) From de070904a8d1a5958be9d5d6ad162419cc443690 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Sun, 18 Jan 2015 14:48:08 +1100 Subject: [PATCH 15/17] Log excerpts/snippets queries. --- lib/thinking_sphinx/excerpter.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/thinking_sphinx/excerpter.rb b/lib/thinking_sphinx/excerpter.rb index 89ace7856..65fc66505 100644 --- a/lib/thinking_sphinx/excerpter.rb +++ b/lib/thinking_sphinx/excerpter.rb @@ -15,7 +15,10 @@ def initialize(index, words, options = {}) def excerpt!(text) result = ThinkingSphinx::Connection.take do |connection| - connection.execute(statement_for(text)).first['snippet'] + query = statement_for text + ThinkingSphinx::Logger.log :query, query do + connection.execute(query).first['snippet'] + end end encoded? ? result : ThinkingSphinx::UTF8.encode(result) From 4285f5a93a265a6b014c002f141354451977b0b8 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Wed, 21 Jan 2015 19:17:38 +1100 Subject: [PATCH 16/17] Updating history --- HISTORY | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/HISTORY b/HISTORY index 3fe62ab7a..227a7ceb3 100644 --- a/HISTORY +++ b/HISTORY @@ -1,3 +1,15 @@ +Latest: +* [CHANGE] Log excerpt SphinxQL queries just like the search queries. +* [CHANGE] Load Railtie if Rails::Railtie is defined, instead of just Rails (Andrew Cone). +* [CHANGE] Convert raw Sphinx results to an array when querying (Bryan Ricker). +* [FIX] Generate de-polymorphised associations properly for Rails 4.2 +* [FIX] Use reflect_on_association instead of reflections, to stick to the public ActiveRecord::Base API. +* [FIX] Don't load ActiveRecord early - fixes a warning in Rails 4.2. +* [FEATURE] Allow for custom offset references with the :offset_as option - thus one model across many schemas with Apartment can be treated differently. +* [FEATURE] Allow for custom IndexSet classes. +* [FIX] Don't double-up on STI filtering, already handled by Rails. +* [CHANGE] Add bigint support for real-time indices, and use bigints for the sphinx_internal_id attribute (mapped to model primary keys) (Chance Downs). + 2014-11-04: 3.1.2 * [CHANGE] regenerate task now only deletes index files for real-time indices. * [CHANGE] Raise an exception when a populated search query is modified (as it can't be requeried). From 3a1355bb6a2858e028f73a8259505004451aceff Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Wed, 21 Jan 2015 19:27:11 +1100 Subject: [PATCH 17/17] 3.1.3 --- HISTORY | 2 +- README.textile | 8 +++++--- thinking-sphinx.gemspec | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/HISTORY b/HISTORY index 227a7ceb3..e1d552b9e 100644 --- a/HISTORY +++ b/HISTORY @@ -1,4 +1,4 @@ -Latest: +2015-01-21: 3.1.3 * [CHANGE] Log excerpt SphinxQL queries just like the search queries. * [CHANGE] Load Railtie if Rails::Railtie is defined, instead of just Rails (Andrew Cone). * [CHANGE] Convert raw Sphinx results to an array when querying (Bryan Ricker). diff --git a/README.textile b/README.textile index 386cbdef8..e2a8c16bd 100644 --- a/README.textile +++ b/README.textile @@ -1,11 +1,13 @@ h1. Thinking Sphinx -Thinking Sphinx is a library for connecting ActiveRecord to the Sphinx full-text search tool, and integrates closely with Rails (but also works with other Ruby web frameworks). The current release is v3.1.1. +Thinking Sphinx is a library for connecting ActiveRecord to the Sphinx full-text search tool, and integrates closely with Rails (but also works with other Ruby web frameworks). The current release is v3.1.3. h2. Upgrading Please refer to the release notes for any changes you need to make when upgrading: +* "v3.1.3":https://github.com/pat/thinking-sphinx/releases/tag/v3.1.3 +* "v3.1.2":https://github.com/pat/thinking-sphinx/releases/tag/v3.1.2 * "v3.1.1":https://github.com/pat/thinking-sphinx/releases/tag/v3.1.1 * "v3.1.0":https://github.com/pat/thinking-sphinx/releases/tag/v3.1.0 * "v3.0.6":https://github.com/pat/thinking-sphinx/releases/tag/v3.0.6 @@ -18,7 +20,7 @@ It's a gem, so install it like you would any other gem. You will also need to sp 
gem 'mysql2',          '~> 0.3.13', :platform => :ruby
 gem 'jdbc-mysql',      '~> 5.1.28', :platform => :jruby
-gem 'thinking-sphinx', '~> 3.1.1'
+gem 'thinking-sphinx', '~> 3.1.3' The MySQL gems mentioned are required for connecting to Sphinx, so please include it even when you're using PostgreSQL for your database. @@ -75,4 +77,4 @@ You can then run the unit tests with @rake spec:unit@, the acceptance tests with h2. Licence -Copyright (c) 2007-2014, Thinking Sphinx is developed and maintained by Pat Allan, and is released under the open MIT Licence. Many thanks to "all who have contributed patches":https://github.com/pat/thinking-sphinx/contributors. +Copyright (c) 2007-2015, Thinking Sphinx is developed and maintained by Pat Allan, and is released under the open MIT Licence. Many thanks to "all who have contributed patches":https://github.com/pat/thinking-sphinx/contributors. diff --git a/thinking-sphinx.gemspec b/thinking-sphinx.gemspec index e87468440..06a43ec62 100644 --- a/thinking-sphinx.gemspec +++ b/thinking-sphinx.gemspec @@ -3,7 +3,7 @@ $:.push File.expand_path('../lib', __FILE__) Gem::Specification.new do |s| s.name = 'thinking-sphinx' - s.version = '3.1.2' + s.version = '3.1.3' s.platform = Gem::Platform::RUBY s.authors = ["Pat Allan"] s.email = ["pat@freelancing-gods.com"]