Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Correctly LEFT JOIN associations.

Merge :include into :fields, no need for them to be separate.

[git-p4: depot-paths = "//src/Sphincter/dev/": change = 3494]
  • Loading branch information...
commit e1a6d05bb068e8f9bbc11a3498f097c7841bd99e 1 parent 2eb1c4c
@drbrain drbrain authored
View
2  lib/sphincter.rb
@@ -92,7 +92,7 @@ module Sphincter
##
# This is the version of Sphincter you are using.
- VERSION = '1.0.0'
+ VERSION = '1.1.0'
##
# Sphincter error base class.
View
55 lib/sphincter/configure.rb
@@ -50,12 +50,18 @@
module Sphincter::Configure
+ ##
+ # A class for building sphinx.conf source/index sections.
+
class Index
attr_reader :source_conf
attr_reader :name
+ ##
+ # Creates a new Index for +klass+ and +options+.
+
def initialize(klass, options)
@fields = []
@where = []
@@ -68,12 +74,11 @@ def initialize(klass, options)
@klass = klass
@table = @klass.table_name
@conn = @klass.connection
- @tables = [@table]
+ @tables = @table.dup
defaults = {
:conditions => [],
:fields => [],
- :include => [],
:name => @table,
}
@@ -82,6 +87,10 @@ def initialize(klass, options)
@name = @options[:name] || @table
end
+ ##
+ # Adds plain field +field+ to the index from class +klass+ using
+ # +as_table+ as the table name.
+
def add_field(field, klass = @klass, as_table = nil)
table = klass.table_name
quoted_field = @conn.quote_column_name field
@@ -106,9 +115,10 @@ def add_field(field, klass = @klass, as_table = nil)
"#{expr} AS #{as_name}"
end
- def add_include(assoc_include)
- as_name, as_field = assoc_include.split '.', 2
+ ##
+ # Includes field +as_field+ from association +as_name+ in the index.
+ def add_include(as_name, as_field)
as_assoc = @klass.reflect_on_all_associations.find do |assoc|
assoc.name == as_name.intern
end
@@ -126,10 +136,17 @@ def add_include(assoc_include)
case as_assoc.macro
when :belongs_to then
- @tables << as_table
@fields << add_field(as_field, as_klass, as_table)
- @where << "#{@table}.#{as_assoc_key} = #{as_table}.#{as_klass_key}"
+ @tables << " LEFT JOIN #{as_table} ON" \
+ " #{@table}.#{as_assoc_key} = #{as_table}.#{as_klass_key}"
+
when :has_many then
+ if as_assoc.options.include? :through then
+ raise Sphincter::Error,
+ "unsupported macro has_many :through for \"#{as_name}\" " \
+ "in #{klass.name}.add_index"
+ end
+
as_pkey = @conn.quote_column_name as_klass.primary_key.to_s
as_fkey = @conn.quote_column_name as_assoc.primary_key_name.to_s
@@ -138,9 +155,21 @@ def add_include(assoc_include)
field = @conn.quote_column_name as_field
- @tables << as_table
@fields << "GROUP_CONCAT(#{as_table}.#{field} SEPARATOR ' ') AS #{as_name}"
- @where << "#{@table}.#{as_klass_key} = #{as_table}.#{as_assoc_key}"
+
+ if as_assoc.options.include? :as then
+ poly_name = as_assoc.options[:as]
+ id_col = @conn.quote_column_name "#{poly_name}_id"
+ type_col = @conn.quote_column_name "#{poly_name}_type"
+
+ @tables << " LEFT JOIN #{as_table} ON"\
+ " #{@table}.#{as_klass_key} = #{as_table}.#{id_col} AND" \
+ " #{@conn.quote @klass.name} = #{as_table}.#{type_col}"
+ else
+ @tables << " LEFT JOIN #{as_table} ON" \
+ " #{@table}.#{as_klass_key} = #{as_table}.#{as_assoc_key}"
+ end
+
@group = true
else
raise Sphincter::Error,
@@ -160,8 +189,12 @@ def configure
@fields << "#{index_id} AS sphincter_index_id"
@fields << "'#{@klass.name}' AS sphincter_klass"
- @options[:fields].each do |field| @fields << add_field(field) end
- @options[:include].each do |as_include| add_include as_include end
+ @options[:fields].each do |field|
+ case field
+ when /\./ then add_include(*field.split('.', 2))
+ else @fields << add_field(field)
+ end
+ end
@fields = @fields.join ', '
@@ -170,7 +203,7 @@ def configure
@where.push(*@options[:conditions])
@where = @where.compact.join ' AND '
- query = "SELECT #{@fields} FROM #{@tables.join ', '} WHERE #{@where}"
+ query = "SELECT #{@fields} FROM #{@tables} WHERE #{@where}"
query << " GROUP BY #{@table}.#{pk}" if @group
@source_conf['sql_query'] = query
View
19 lib/sphincter/search.rb
@@ -45,9 +45,8 @@ def self.indexes
#
# :name:: Name of index. Defaults to ActiveRecord::Base::table_name.
# :fields:: Array of fields to index. Foreign key columns for belongs_to
- # associations are automatically added.
- # :include:: Array of "association.column" for fields from associations to
- # include in the index.
+ # associations are automatically added. Fields from associations
+ # may be included by using "association.field".
# :conditions:: Array of SQL conditions that will be ANDed together to
# predicate inclusion in the search index.
#
@@ -58,10 +57,20 @@ def self.indexes
# belongs_to :blog
# has_many :comments
#
- # add_index :fields => %w[title body],
- # :include => %w[user.name comments.body],
+ # add_index :fields => %w[title body user.name, comments.body],
# :conditions => ['published = 1']
# end
+ #
+ # When including fields from associations, MySQL's GROUP_CONCAT() function
+ # is used. By default this will create a string up to 1024 characters long.
+ # A larger string can be used by changing the value of MySQL's
+ # group_concat_max_len variable. To do this, add the following to your
+ # sphincter.RAILS_ENV.yml files:
+ #
+ # mysql:
+ # sql_query_pre:
+ # - SET NAMES utf8
+ # - SET SESSION group_concat_max_len = VALUE
def add_index(options = {})
options[:fields] ||= []
View
22 test/sphincter_test_case.rb
@@ -5,11 +5,12 @@
$TESTING = true
class String
- def constantize()
+ def constantize
case self
when /belongs_to/i then SphincterTestCase::BelongsTo
when /many/i then SphincterTestCase::HasMany
- else raise "missing klass for #{self}"
+ when /poly/i then SphincterTestCase::Poly
+ else raise "missing klass for #{self} in #constantize"
end
end
end
@@ -101,11 +102,11 @@ class Reflection
attr_accessor :klass
attr_reader :macro, :options, :name
- def initialize(macro, name)
+ def initialize(macro, name, options = {})
@klass = Model
@macro = macro
@name = name.intern
- @options = {}
+ @options = options
end
def class_name() @name.to_s.sub(/s$/, '').capitalize end
@@ -115,7 +116,8 @@ def primary_key_name() "#{@name}_id" end
class Model
@reflections = [Reflection.new(:belongs_to, 'belongs_to'),
- Reflection.new(:has_many, 'manys')]
+ Reflection.new(:has_many, 'manys'),
+ Reflection.new(:has_many, 'polys', :as => :polyable)]
class << self; attr_accessor :reflections; end
@@ -126,6 +128,7 @@ def self.columns_hash
'boolean' => Column.new(:boolean),
'date' => Column.new(:date),
'datetime' => Column.new(:datetime),
+ 'float' => Column.new(:float),
'integer' => Column.new(:integer),
'string' => Column.new(:string),
'text' => Column.new(:text),
@@ -165,6 +168,15 @@ def self.table_name() 'has_manys' end
def id() 84 end
end
+ class Poly < Model
+ @reflections = [Reflection.new(:belongs_to, 'polyable',
+ :polymorphic => true)]
+
+ def self.table_name() 'polys' end
+
+ def id() 126 end
+ end
+
class Model
extend Sphincter::Search
end
View
58 test/test_sphincter_configure.rb
@@ -128,7 +128,9 @@ def test_self_get_conf_from
end
def test_self_get_sources
- Sphincter::Search.indexes[Model] << { :fields => %w[text] }
+ Sphincter::Search.indexes[Model] << {
+ :fields => %w[text belongs_to.string]
+ }
expected = {
"models" => {
@@ -142,9 +144,11 @@ def test_self_get_sources
"SELECT (models.`id` * 1 + 0) AS `id`, " \
"0 AS sphincter_index_id, " \
"'Model' AS sphincter_klass, "\
- "models.`text` AS `text` " \
- "FROM models WHERE models.`id` >= $start AND " \
- "models.`id` <= $end"
+ "models.`text` AS `text`, " \
+ "belongs_tos.`string` AS `belongs_tos_string` "\
+ "FROM models LEFT JOIN belongs_tos ON " \
+ "models.`belongs_to_id` = belongs_tos.`id` " \
+ "WHERE models.`id` >= $start AND models.`id` <= $end"
}
}
@@ -319,33 +323,59 @@ def test_self_add_field
def test_self_add_field_unknown
e = assert_raise Sphincter::Error do
- @index.add_field 'other'
+ @index.add_field 'float'
end
- assert_equal 'unknown column type NilClass', e.message
+ assert_equal 'unknown column type float', e.message
end
def test_add_include_belongs_to
- @index.add_include 'belongs_to.string'
+ @index.add_include 'belongs_to', 'string'
assert_equal ["belongs_tos.`string` AS `belongs_tos_string`"], @index.fields
- assert_equal %w[models belongs_tos], @index.tables
- assert_equal ["models.`belongs_to_id` = belongs_tos.`id`"], @index.where
+
+ tables = "models LEFT JOIN belongs_tos ON " \
+ "models.`belongs_to_id` = belongs_tos.`id`"
+ assert_equal tables, @index.tables
+
+ assert_equal [], @index.where
+
assert_equal false, @index.group
end
def test_add_include_has_many
- @index.add_include 'manys.string'
+ @index.add_include 'manys', 'string'
+
+ fields = [
+ "GROUP_CONCAT(has_manys.`string` SEPARATOR ' ') AS `has_manys_string`"
+ ]
+ assert_equal fields, @index.fields
+
+ tables = "models LEFT JOIN has_manys ON models.`id` = has_manys.`manys_id`"
+ assert_equal tables, @index.tables
+
+ assert_equal [], @index.where
+ assert_equal true, @index.group
+ end
+
+ def test_add_include_has_many_polymorphic
+ @index.add_include 'polys', 'string'
+
+ fields = ["GROUP_CONCAT(polys.`string` SEPARATOR ' ') AS `polys_string`"]
+ assert_equal fields, @index.fields
+
+ tables = "models LEFT JOIN polys ON " \
+ "models.`id` = polys.`polyable_id` AND " \
+ "'Model' = polys.`polyable_type`"
+ assert_equal tables, @index.tables
- assert_equal ["GROUP_CONCAT(has_manys.`string` SEPARATOR ' ') AS `has_manys_string`"], @index.fields
- assert_equal %w[models has_manys], @index.tables
- assert_equal ["models.`id` = has_manys.`manys_id`"], @index.where
+ assert_equal [], @index.where
assert_equal true, @index.group
end
def test_add_include_nonexistent_association
e = assert_raise Sphincter::Error do
- @index.add_include 'nonexistent.string'
+ @index.add_include 'nonexistent', 'string'
end
assert_equal "could not find association \"nonexistent\" in Model",
Please sign in to comment.
Something went wrong with that request. Please try again.