add more search tests

Fix bug with join query

See merge request saburly/senadibilal/reklamice/gem!3

lib/pg_searchable_regex.rb

@@ -47,7 +47,7 @@ module PgSearchable
     def ts_add_scope
       class_eval do
         scope ts_scope_method, ->(value) do
-          resulting_ids = ts_search(value).map(&:id)
+          resulting_ids = ts_search(value).rows.map { |row| row[0] }
           where(id: resulting_ids)
         end
       end
@@ -55,16 +55,16 @@ module PgSearchable
 
     def ts_search(value)
       return if @ts_search_fields.blank? || value.blank?
-      model = ancestors.first
       sql_query_object = TextToSqlQuery.new(
         value,
         @ts_search_fields,
         @default_fields,
         @ts_search_fields_mappings,
-        @ts_joins,
+        @ts_joins
-        model
       )
-      joins(sql_query_object.join_clause).where(sql_query_object.where_clause).distinct
+      sql_query = select(:id).distinct.joins(sql_query_object.join_clause).group(:id).having(sql_query_object.where_clause)
+      modified_sql_query = sql_query.to_sql.gsub('INNER', 'LEFT OUTER')
+      ActiveRecord::Base.connection.exec_query(modified_sql_query)
     end
 
     def should_update_cache_field?

lib/text_to_sql_query.rb

@@ -1,7 +1,7 @@
 require_relative 'parser'
 
 class TextToSqlQuery
-  def initialize(text, fields, default_fields, fields_mappings = {}, joins = [], model = nil)
+  def initialize(text, fields, default_fields, fields_mappings = {}, joins = [])
     @text = text.to_s.strip
     @fields = fields.map(&:to_sym)
 
@@ -21,7 +21,6 @@ class TextToSqlQuery
       @fields_mappings[field] = value if @fields_mappings[field]
     end
     @joins = joins
-    @model = model
   end
 
   def where_clause
@@ -31,20 +30,11 @@ class TextToSqlQuery
   end
 
   def join_clause
-    return if @joins.empty?
+    return nil if @joins.empty?
-
+    return *@joins
-    table_column_mappings
-    model_association_mappings
-
-    join_clause_part = ''
-    @joins.each do |join|
-      join_sql_part = generate_join_sql_part_for join
-      join_clause_part += join_sql_part
-    end
-    join_clause_part
   end
 
-  private
+private
 
   def generate_sql(tree)
     first_key = tree.keys.first
@@ -83,7 +73,7 @@ class TextToSqlQuery
       if mapping.nil?
         raise "Unknown field '#{first_key.to_s}'"
       else
-        ["CAST(#{mapping.to_s} AS TEXT) ILIKE ?", "%#{escaped_node_value}%"]
+        ["STRING_AGG(CAST(#{mapping.to_s} AS TEXT), '') ILIKE ?", "%#{escaped_node_value}%"]
       end
     end
   end
@@ -118,52 +108,7 @@ class TextToSqlQuery
     result.gsub!(/\_/, '\_')
     result.tr!('\\', '\\')
     result.gsub!(/%/, '\%')
+    result.downcase!
     result
   end
-
-  def table_column_mappings
-    @table_column_mappings = {}
-    @fields_mappings.each_value do |table_with_column|
-      split_names = table_with_column.to_s.split '.'
-      table_name = split_names.first
-      column_name = split_names.second
-      @table_column_mappings[table_name] = [] if @table_column_mappings[table_name].nil?
-      @table_column_mappings[table_name] << column_name
-    end
-    @table_column_mappings
-  end
-
-  def model_association_mappings
-    @model_associations = {}
-    @model.reflect_on_all_associations.each do |association|
-      name = association.name
-
-      @model_associations[name] = {
-        option_as: association.options[:as] || name,
-        type: association.type
-      }
-    end
-    @model_associations
-  end
-
-  def generate_join_sql_part_for(join)
-    association_data = @model_associations[join]
-    join_table_name = join.to_s
-    raise "Join table #{join_table_name} has no association data" if association_data.nil?
-
-    select_sql_part = ''
-    columns_for_table = @table_column_mappings[join_table_name] || []
-
-    # TODO: Can be optimized - do not include columns that are not referenced in user query
-    columns_for_table.each do |column_name|
-      select_sql_part += "string_agg(#{column_name}, '') AS #{column_name}, "
-    end
-
-    option_as = association_data[:option_as]
-    type = association_data[:type]
-    model_name = @model.to_s
-    table_name = @model.table_name
-
-    "LEFT JOIN (SELECT #{option_as}_id, #{select_sql_part} #{type} FROM #{join_table_name} GROUP BY #{option_as}_id, #{type}) #{join_table_name} on #{join_table_name}.#{option_as}_id = #{table_name}.id AND #{join_table_name}.#{type} = '#{model_name}'"
-  end
 end

spec/lib/pg_searchable_new_spec.rb

@@ -158,12 +158,25 @@ describe PgSearchable do
         expect(DynamicModelWithTagValues.scope_search('tag:green or value:"not"')).to contain_exactly(record1, record2)
       end
 
+      it 'can search for search term containing INNER word' do
+        records = DynamicModelWithTagValues.create [{ name: 'inner', value: 'amazing' },
+                                                    { name: 'new record', value: 'INNER' }]
+
+        Tag.create [{ taggable: records[0], value: 'red', custom_attribute: 'rose' },
+                    { taggable: records[1], value: 'INNER', custom_attribute: 'garden' }]
+
+        expect(DynamicModelWithTagValues.scope_search('name:inner')).to contain_exactly(records[0])
+        expect(DynamicModelWithTagValues.scope_search('value:INNER')).to contain_exactly(records[1])
+        expect(DynamicModelWithTagValues.scope_search('tag:inner')).to contain_exactly(records[1])
+        expect(DynamicModelWithTagValues.scope_search('tag:INNER')).to contain_exactly(records[1])
+      end
+
       describe 'searching in model with has_many association' do
         before do
           records = DynamicModelWithTagValues.create [{ name: 'something', value: 'amazing' },
-                                                                        { name: 'new record', value: 'not so amazing' },
+                                                      { name: 'new record', value: 'not so amazing' },
-                                                                        { name: 'last one', value: 'no value' },
+                                                      { name: 'last one', value: 'no value' },
-                                                                        { name: 'really last one', value: 'no value' }]
+                                                      { name: 'really last one', value: 'no value' }]
 
           Tag.create [{ taggable: records[0], value: 'red', custom_attribute: 'rose' },
                       { taggable: records[0], value: 'green', custom_attribute: 'garden' },
@@ -242,9 +255,9 @@ describe PgSearchable do
       describe 'searching in model with multiple has_many associations' do
         before do
           records = DynamicModelWithTagAndCategories.create [{ name: 'something', value: 'amazing' },
-                                                                        { name: 'new record', value: 'not so amazing' },
+                                                             { name: 'new record', value: 'not so amazing' },
-                                                                        { name: 'last one', value: 'no value' },
+                                                             { name: 'last one', value: 'no value' },
-                                                                        { name: 'really last one', value: 'no value' }]
+                                                             { name: 'really last one', value: 'no value' }]
 
           Tag.create [{ taggable: records[0], value: 'red', custom_attribute: 'rose' },
                       { taggable: records[0], value: 'green', custom_attribute: 'garden' },
@@ -285,6 +298,72 @@ describe PgSearchable do
           expect(DynamicModelWithTagAndCategories.scope_search('(tag:- and not tag:12) or (value:"amazing" and not value:"not") or (category:unknown or category:math)')).to contain_exactly(records[0], records[2], records[3])
         end
       end
+
+      describe 'joins with through association' do
+        let(:players) { Player.all }
+
+        before do
+          players = Player.create([{ name: 'first', value: 'downtown' },
+                                   { name: 'redliner', value: 'uptown' },
+                                   { name: 'flying', value: 'eagle' },
+                                   { name: 'blue', value: 'marine' }])
+
+
+          ptags = Ptag.create [{ value: 'e-ink' },
+                               { value: 'black&white' },
+                               { value: 'rich' },
+                               { value: 'grayscale' },
+                               { value: 'LED' }]
+
+          players[0].ptags << ptags[0..1]
+          players[1].ptags << ptags[0..2]
+          players[2].ptags << ptags[2..3]
+          players[3].ptags << ptags[4]
+        end
+
+        it 'can search in default column of model with through association' do
+          expect(Player.scope_search('l and not y and not blue')).to contain_exactly(players[1])
+        end
+
+        it 'can search in referenced column associated with through relation - simple search' do
+          expect(Player.scope_search('tag:e-ink')).to contain_exactly(players[0], players[1])
+        end
+
+        it 'can search in referenced column associated with through relaction - simple search with OR operator' do
+          expect(Player.scope_search('tag:e-ink OR tag:rich')).to contain_exactly(players[0], players[1], players[2])
+        end
+
+        it 'can search in referenced column associated with through relaction - simple search with AND operator' do
+          expect(Player.scope_search('tag:e-ink AND tag:rich')).to contain_exactly(players[1])
+        end
+
+        it 'can search in referenced column associated with through relaction - simple search with NOT operator' do
+          expect(Player.scope_search('NOT tag:e-ink')).to contain_exactly(players[2], players[3])
+        end
+
+        it 'can search in referenced column associated with through relaction - mixed search terms with OR' do
+          expect(Player.scope_search('tag:e-ink OR blue')).to contain_exactly(players[0], players[1], players[3])
+        end
+
+        it 'can search in referenced column associated with through relaction - mixed search terms with AND' do
+          expect(Player.scope_search('tag:e-ink AND first')).to contain_exactly(players[0])
+        end
+
+        it 'can search in referenced column associated with through relaction - mixed search terms with OR NOT' do
+          expect(Player.scope_search('tag:led OR NOT first')).to contain_exactly(players[1], players[2], players[3])
+        end
+
+        it 'can search in referenced column associated with through relaction - mixed search terms with AND NOT' do
+          expect(Player.scope_search('tag:e-ink AND NOT first')).to contain_exactly(players[1])
+        end
+
+        it 'can search in referenced column and in model columns - complex search' do
+          expect(Player.scope_search('(tag:e-ink AND tag:rich) OR (blue or value:eagle)')).to contain_exactly(players[1], players[2], players[3])
+          expect(Player.scope_search('tag:e-ink AND tag:rich AND NOT tag:black')).to be_empty
+          expect(Player.scope_search('name:l AND (value:uptown or value:marine) AND (tag:e-ink and tag:rich)')).to contain_exactly(players[1])
+          expect(Player.scope_search('name:l AND (value:uptown or value:marine) AND (tag:e-ink and tag:led)')).to be_empty
+        end
+      end
     end
   end
 end

spec/lib/text_to_sql_query_spec.rb

@@ -16,7 +16,7 @@ describe TextToSqlQuery do
     it { expect(described_class.new('search-term', [:'players.name', :'players.tags', 'players.id'], [:'players.name', :'players.tags', :'players.id']).where_clause).to eq(['(CAST(players.name AS TEXT) ILIKE ? OR CAST(players.tags AS TEXT) ILIKE ? OR CAST(players.id AS TEXT) ILIKE ?)', '%search-term%', '%search-term%', '%search-term%']) }
 
     # tests simple search term with column name and without quotes
-    it { expect(described_class.new('name:ab', [:"players.name"], :"players.name").where_clause).to eq(['CAST(players.name AS TEXT) ILIKE ?', '%ab%']) }
+    it { expect(described_class.new('name:ab', [:"players.name"], :"players.name").where_clause).to eq(["STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ?", '%ab%']) }
 
     # tests simple search term with unknown column name and without quotes
     it { expect{described_class.new('unknown:ab', [:"players.name"], :"players.name").where_clause}.to raise_error(RuntimeError, "Unknown field 'unknown'") }
@@ -25,7 +25,7 @@ describe TextToSqlQuery do
     it { expect(described_class.new('"ab"', [:"players.name", :"players.device_id"],  :"players.device_id").where_clause).to eq(["(CAST(players.device_id AS TEXT) ILIKE ?)", "%ab%"]) }
 
     # tests simple search term with column name and with quotes
-    it { expect(described_class.new('tags:"ab"', [:"players.name", :"players.tags"],  :"players.device_id").where_clause).to eq(["CAST(players.tags AS TEXT) ILIKE ?", "%ab%"]) }
+    it { expect(described_class.new('tags:"ab"', [:"players.name", :"players.tags"],  :"players.device_id").where_clause).to eq(["STRING_AGG(CAST(players.tags AS TEXT), '') ILIKE ?", "%ab%"]) }
 
     # tests search without operators
     it { expect(described_class.new('123 456', [:"players.name", :"players.device_id"],  :"players.device_id").where_clause).to eq(["((CAST(players.device_id AS TEXT) ILIKE ?) OR (CAST(players.device_id AS TEXT) ILIKE ?))", "%123%", "%456%"]) }
@@ -40,34 +40,34 @@ describe TextToSqlQuery do
     it { expect(described_class.new('not 23', [:"players.name", :"players.device_id"],  :"players.device_id").where_clause).to eq(["NOT (CAST(players.device_id AS TEXT) ILIKE ?)", "%23%"]) }
 
     # tests search with NOT operator on non-default column
-    it { expect(described_class.new('not value:23', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(["NOT CAST(players.value AS TEXT) ILIKE ?", "%23%"]) }
+    it { expect(described_class.new('not value:23', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(["NOT STRING_AGG(CAST(players.value AS TEXT), '') ILIKE ?", "%23%"]) }
 
     # tests search with mixed logical operators
-    it { expect(described_class.new('name:ab and not value:hf-1', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(['(CAST(players.name AS TEXT) ILIKE ? AND NOT CAST(players.value AS TEXT) ILIKE ?)', '%ab%', '%hf-1%']) }
+    it { expect(described_class.new('name:ab and not value:hf-1', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(["(STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ? AND NOT STRING_AGG(CAST(players.value AS TEXT), '') ILIKE ?)", '%ab%', '%hf-1%']) }
 
     # tests search with mixed logical operators without NOT'
-    it { expect(described_class.new('name:a and name:b or name:c', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(['((CAST(players.name AS TEXT) ILIKE ? AND CAST(players.name AS TEXT) ILIKE ?) OR CAST(players.name AS TEXT) ILIKE ?)', '%a%', '%b%', '%c%']) }
+    it { expect(described_class.new('name:a and name:b or name:c', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(["((STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ? AND STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ?) OR STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ?)", '%a%', '%b%', '%c%']) }
 
     # tests search with brackets in expression
-    it { expect(described_class.new('name:a and (name:b or name:c)', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(['(CAST(players.name AS TEXT) ILIKE ? AND (CAST(players.name AS TEXT) ILIKE ? OR CAST(players.name AS TEXT) ILIKE ?))', '%a%', '%b%', '%c%']) }
+    it { expect(described_class.new('name:a and (name:b or name:c)', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(["(STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ? AND (STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ? OR STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ?))", '%a%', '%b%', '%c%']) }
 
     # tests search with brackets in expression and with NOT operator
-    it { expect(described_class.new('name:a and not (name:b or name:c)', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(['(CAST(players.name AS TEXT) ILIKE ? AND NOT (CAST(players.name AS TEXT) ILIKE ? OR CAST(players.name AS TEXT) ILIKE ?))', '%a%', '%b%', '%c%']) }
+    it { expect(described_class.new('name:a and not (name:b or name:c)', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(["(STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ? AND NOT (STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ? OR STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ?))", '%a%', '%b%', '%c%']) }
 
     # tests search with special characters in search term
-    it { expect(described_class.new('name:"%a_\"', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(['CAST(players.name AS TEXT) ILIKE ?', '%\%a\_\\%']) }
+    it { expect(described_class.new('name:"%a_\"', [:"players.name", :"players.value"],  :"players.device_id").where_clause).to eq(["STRING_AGG(CAST(players.name AS TEXT), '') ILIKE ?", '%\%a\_\\%']) }
 
     # tests search with field mappings
-    it { expect(described_class.new('tags:h1-r', [:'players.name', :'players.value', :'players.device_id'],  :"players.device_id", { tags: "tags.value" }).where_clause).to eq(['CAST(tags.value AS TEXT) ILIKE ?', '%h1-r%']) }
+    it { expect(described_class.new('tags:h1-r', [:'players.name', :'players.value', :'players.device_id'],  :"players.device_id", { tags: "tags.value" }).where_clause).to eq(["STRING_AGG(CAST(tags.value AS TEXT), '') ILIKE ?", '%h1-r%']) }
 
     # tests search with field mappings when fields array has same mapping
-    it { expect(described_class.new('tags:h1-r', [:'players.name', :'players.tags', :'players.device_id'],  :"players.device_id", { tags: "tags.value" }).where_clause).to eq(["CAST(tags.value AS TEXT) ILIKE ?", "%h1-r%"]) }
+    it { expect(described_class.new('tags:h1-r', [:'players.name', :'players.tags', :'players.device_id'],  :"players.device_id", { tags: "tags.value" }).where_clause).to eq(["STRING_AGG(CAST(tags.value AS TEXT), '') ILIKE ?", "%h1-r%"]) }
 
     # tests complex query
-    it { expect(described_class.new('(device_id:"with space" tags:mta no-quotes-id-123) or "id with quotes-5"  and   ( ("id with q 10" or "id with q 20") and ("id with Q 30" "id with Q 40") and not id-without-Q-50)', [:'players.name', :'players.value', :'players.device_id'],  :"players.device_id", { tags: 'tags.name' }).where_clause).to eq(['((CAST(players.device_id AS TEXT) ILIKE ? OR (CAST(tags.name AS TEXT) ILIKE ? OR (CAST(players.device_id AS TEXT) ILIKE ?))) OR ((CAST(players.device_id AS TEXT) ILIKE ?) AND ((((CAST(players.device_id AS TEXT) ILIKE ?) OR (CAST(players.device_id AS TEXT) ILIKE ?)) AND ((CAST(players.device_id AS TEXT) ILIKE ?) OR (CAST(players.device_id AS TEXT) ILIKE ?))) AND NOT (CAST(players.device_id AS TEXT) ILIKE ?))))', '%with space%', '%mta%', '%no-quotes-id-123%', '%id with quotes-5%', '%id with q 10%', '%id with q 20%', '%id with Q 30%', '%id with Q 40%', '%id-without-Q-50%']) }
+    it { expect(described_class.new('(device_id:"with space" tags:mta no-quotes-id-123) or "id with quotes-5"  and   ( ("id with q 10" or "id with q 20") and ("id with Q 30" "id with Q 40") and not id-without-Q-50)', [:'players.name', :'players.value', :'players.device_id'],  :"players.device_id", { tags: 'tags.name' }).where_clause).to eq(["((STRING_AGG(CAST(players.device_id AS TEXT), '') ILIKE ? OR (STRING_AGG(CAST(tags.name AS TEXT), '') ILIKE ? OR (CAST(players.device_id AS TEXT) ILIKE ?))) OR ((CAST(players.device_id AS TEXT) ILIKE ?) AND ((((CAST(players.device_id AS TEXT) ILIKE ?) OR (CAST(players.device_id AS TEXT) ILIKE ?)) AND ((CAST(players.device_id AS TEXT) ILIKE ?) OR (CAST(players.device_id AS TEXT) ILIKE ?))) AND NOT (CAST(players.device_id AS TEXT) ILIKE ?))))", '%with space%', '%mta%', '%no-quotes-id-123%', '%id with quotes-5%', '%id with q 10%', '%id with q 20%', '%id with q 30%', '%id with q 40%', '%id-without-q-50%']) }
 
     # tests mixed query with and without column names and with multiple default columns
-    it { expect(described_class.new('("green hunter" and device_id:100) or ("blue bird" and not device_id:200)', [:'players.name', :'players.value', :'players.device_id'],  [:"players.name", :"players.tags"]).where_clause).to eq(['(((CAST(players.name AS TEXT) ILIKE ? OR CAST(players.tags AS TEXT) ILIKE ?) AND CAST(players.device_id AS TEXT) ILIKE ?) OR ((CAST(players.name AS TEXT) ILIKE ? OR CAST(players.tags AS TEXT) ILIKE ?) AND NOT CAST(players.device_id AS TEXT) ILIKE ?))', '%green hunter%', '%green hunter%', '%100%', '%blue bird%', '%blue bird%', '%200%']) }
+    it { expect(described_class.new('("green hunter" and device_id:100) or ("blue bird" and not device_id:200)', [:'players.name', :'players.value', :'players.device_id'],  [:"players.name", :"players.tags"]).where_clause).to eq(["(((CAST(players.name AS TEXT) ILIKE ? OR CAST(players.tags AS TEXT) ILIKE ?) AND STRING_AGG(CAST(players.device_id AS TEXT), '') ILIKE ?) OR ((CAST(players.name AS TEXT) ILIKE ? OR CAST(players.tags AS TEXT) ILIKE ?) AND NOT STRING_AGG(CAST(players.device_id AS TEXT), '') ILIKE ?))", '%green hunter%', '%green hunter%', '%100%', '%blue bird%', '%blue bird%', '%200%']) }
 
     # tests query with multiple search terms with mixed and-or-not after dash and underscore
     it { expect(described_class.new('123-and-456 -or-2 -not_not_1', [:'players.title', :'players.tag', :'players.device_id'], :'players.device_id').where_clause).to eq(['((CAST(players.device_id AS TEXT) ILIKE ?) OR ((CAST(players.device_id AS TEXT) ILIKE ?) OR (CAST(players.device_id AS TEXT) ILIKE ?)))', '%123-and-456%', '%-or-2%', '%-not\_not\_1%'])}

spec/schema.rb

@@ -44,4 +44,19 @@ ActiveRecord::Schema.define do
     t.string :name
     t.timestamps null: false
   end
+
+  create_table :players, force: true do |t|
+    t.string :name
+    t.string :value
+    t.timestamps null: false
+  end
+
+  create_table :ptags, force: true do |t|
+    t.string :value
+  end
+
+  create_table :taggings, force: true do |t|
+    t.belongs_to :player
+    t.belongs_to :ptag
+  end
 end

spec/support/models.rb

@@ -129,3 +129,21 @@ class Category < ActiveRecord::Base
   has_many :tags
   belongs_to :categoriable, polymorphic: true
 end
+
+class Player < ActiveRecord::Base
+  include PgSearchable
+  pg_search fields: %i[players.id players.name players.value], default_fields: [:name], fields_mappings: { tag: 'ptags.value' }, joins: [:ptags]
+  has_many :taggings
+  has_many :ptags, through: :taggings
+end
+
+class Ptag < ActiveRecord::Base
+  self.table_name = :ptags
+  has_many :taggings
+  has_many :players, through: :taggings
+end
+
+class Tagging < ActiveRecord::Base
+  belongs_to :player
+  belongs_to :ptag
+end