Support postgres hstore/jsonb fields #58 (#60)

.github/workflows/test.yml

@@ -6,19 +6,19 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        ruby: ["2.6", "2.7", "3.0", "3.1"]
+        ruby: ["2.7", "3.0", "3.1", "3.2"]
         rails: ["rails5", "rails6", "rails7"]
         database: ["sqlite", "postgres", "mysql"]
         exclude:
           - ruby: "3.0"
             rails: "rails5"
           - ruby: "3.1"
             rails: "rails5"
-          - ruby: "2.6"
-            rails: "rails7"
+          - ruby: "3.2"
+            rails: "rails5"
     services:
       postgres:
-        image: postgres
+        image: styriadigital/postgres_hstore:10
         env:
           POSTGRES_USER: search_cop
           POSTGRES_PASSWORD: secret
@@ -44,8 +44,6 @@ jobs:
         env:
           DATABASE: ${{ matrix.database }}
         run: |
-          gem install bundler
           bundle config set --local gemfile "gemfiles/${{ matrix.rails }}.gemfile"
           bundle install
           bundle exec rake test
-          bundle exec rubocop

README.md

@@ -136,6 +136,30 @@ to prevent SQL injection. While we can never be 100% safe from security issues,
 SearchCop takes security issues seriously. Please report responsibly via
 security at flakks dot com in case you find any security related issues.
 
+## json/jsonb/hstore
+
+SearchCop supports json fields for MySQL, as well as json, jsonb and hstore
+fields for postgres. Currently, field values are always expected to be strings
+and no arrays are supported. You can specify json attributes via:
+
+```ruby
+search_scope :search do
+  attributes user_agent: "context->browser->user_agent"
+
+  # ...
+end
+```
+
+where `context` is a json/jsonb column which e.g. contains:
+
+```json
+{
+  "browser": {
+    "user_agent": "Firefox ..."
+  }
+}
+```
+
 ## Fulltext index capabilities
 
 By default, i.e. if you don't tell SearchCop about your fulltext indices,

docker-compose.yml

@@ -6,10 +6,11 @@ services:
     environment:
       - MYSQL_ALLOW_EMPTY_PASSWORD=yes
       - MYSQL_ROOT_PASSWORD=
+      - MYSQL_DATABASE=search_cop
     ports:
       - 3306:3306
   postgres:
-    image: postgres:9.6.6
+    image: styriadigital/postgres_hstore:10
     environment:
       POSTGRES_DB: search_cop
       POSTGRES_USER: search_cop

lib/search_cop/visitors.rb

@@ -1,3 +1,4 @@
 require "search_cop/visitors/visitor"
 require "search_cop/visitors/mysql"
 require "search_cop/visitors/postgres"
+require "search_cop/visitors/sqlite"

lib/search_cop/visitors/mysql.rb

@@ -3,6 +3,10 @@ module Visitors
     module Mysql
       # rubocop:disable Naming/MethodName
 
+      def visit_SearchCopGrammar_Attributes_Json(attribute)
+        "#{quote_table_name attribute.table_alias}.#{quote_column_name attribute.column_name}->#{quote "$.#{attribute.field_names.join(".")}"}"
+      end
+
       class FulltextQuery < Visitor
         def visit_SearchCopGrammar_Nodes_MatchesFulltextNot(node)
           node.right.split(/[\s+'"<>()~-]+/).collect { |word| "-#{word}" }.join(" ")
@@ -38,8 +42,8 @@ def visit_SearchCopGrammar_Attributes_Collection(node)
       def visit_SearchCopGrammar_Nodes_FulltextExpression(node)
         "MATCH(#{visit node.collection}) AGAINST(#{visit FulltextQuery.new(connection).visit(node.node)} IN BOOLEAN MODE)"
       end
-    end
 
-    # rubocop:enable Naming/MethodName
+      # rubocop:enable Naming/MethodName
+    end
   end
 end

lib/search_cop/visitors/postgres.rb

@@ -3,6 +3,22 @@ module Visitors
     module Postgres
       # rubocop:disable Naming/MethodName
 
+      def visit_SearchCopGrammar_Attributes_Json(attribute)
+        elements = ["#{quote_table_name attribute.table_alias}.#{quote_column_name attribute.column_name}", *attribute.field_names.map { |field_name| quote(field_name) }]
+
+        "#{elements[0...-1].join("->")}->>#{elements.last}"
+      end
+
+      def visit_SearchCopGrammar_Attributes_Jsonb(attribute)
+        elements = ["#{quote_table_name attribute.table_alias}.#{quote_column_name attribute.column_name}", *attribute.field_names.map { |field_name| quote(field_name) }]
+
+        "#{elements[0...-1].join("->")}->>#{elements.last}"
+      end
+
+      def visit_SearchCopGrammar_Attributes_Hstore(attribute)
+        "#{quote_table_name attribute.table_alias}.#{quote_column_name attribute.column_name}->#{quote attribute.field_names.first}"
+      end
+
       class FulltextQuery < Visitor
         def visit_SearchCopGrammar_Nodes_MatchesFulltextNot(node)
           "!'#{node.right.gsub(/[\s&|!:'"]+/, " ")}'"

lib/search_cop/visitors/sqlite.rb

@@ -0,0 +1,13 @@
+module SearchCop
+  module Visitors
+    module Sqlite
+      # rubocop:disable Naming/MethodName
+
+      def visit_SearchCopGrammar_Attributes_Json(attribute)
+        "json_extract(#{quote_table_name attribute.table_alias}.#{quote_column_name attribute.column_name}, #{quote "$.#{attribute.field_names.join(".")}"})"
+      end
+
+      # rubocop:enable Naming/MethodName
+    end
+  end
+end

lib/search_cop/visitors/visitor.rb

@@ -10,6 +10,7 @@ def initialize(connection)
 
         extend(SearchCop::Visitors::Mysql) if @connection.adapter_name =~ /mysql/i
         extend(SearchCop::Visitors::Postgres) if @connection.adapter_name =~ /postgres|postgis/i
+        extend(SearchCop::Visitors::Sqlite) if @connection.adapter_name =~ /sqlite/i
       end
 
       def visit(visit_node = node)

lib/search_cop_grammar/attributes.rb

@@ -88,12 +88,13 @@ def alias_for(name)
       def attribute_for(attribute_definition)
         query_info.references.push attribute_definition
 
-        table, column = attribute_definition.split(".")
+        table, column_with_fields = attribute_definition.split(".")
+        column, *fields = column_with_fields.split("->")
         klass = klass_for(table)
 
         raise(SearchCop::UnknownAttribute, "Unknown attribute #{attribute_definition}") unless klass.columns_hash[column]
 
-        Attributes.const_get(klass.columns_hash[column].type.to_s.classify).new(klass, alias_for(table), column, options)
+        Attributes.const_get(klass.columns_hash[column].type.to_s.classify).new(klass, alias_for(table), column, fields, options)
       end
 
       def generator_for(name)
@@ -110,13 +111,14 @@ def generators
     end
 
     class Base
-      attr_reader :attribute, :table_alias, :column_name, :options
+      attr_reader :attribute, :table_alias, :column_name, :field_names, :options
 
-      def initialize(klass, table_alias, column_name, options = {})
+      def initialize(klass, table_alias, column_name, field_names, options = {})
         @attribute = klass.arel_table.alias(table_alias)[column_name]
         @klass = klass
         @table_alias = table_alias
         @column_name = column_name
+        @field_names = field_names
         @options = (options || {})
       end
 
@@ -179,6 +181,9 @@ def matches(value)
     end
 
     class Text < String; end
+    class Jsonb < String; end
+    class Json < String; end
+    class Hstore < String; end
 
     class WithoutMatches < Base
       def matches(value)

test/string_test.rb

@@ -139,4 +139,45 @@ def test_less_or_greater
     assert_includes Product.search("title <= 'Title B'"), product
     refute_includes Product.search("title <= 'Title A'"), product
   end
+
+  def test_jsonb
+    return if DATABASE != "postgres"
+
+    product = create(:product, jsonb: { name: "expected" })
+
+    assert_includes Product.search("jsonb_name: expected"), product
+    refute_includes Product.search("jsonb_name: rejected"), product
+  end
+
+  def test_nested_jsonb
+    return if DATABASE != "postgres"
+
+    product = create(:product, nested_jsonb: { nested: { name: "expected" } })
+
+    assert_includes Product.search("nested_jsonb_name: expected"), product
+    refute_includes Product.search("nested_jsonb_name: rejected"), product
+  end
+
+  def test_json
+    product = create(:product, json: { name: "expected" })
+
+    assert_includes Product.search("json_name: expected"), product
+    refute_includes Product.search("json_name: rejected"), product
+  end
+
+  def test_nested_json
+    product = create(:product, nested_json: { nested: { name: "expected" } })
+
+    assert_includes Product.search("nested_json_name: expected"), product
+    refute_includes Product.search("nested_json_name: rejected"), product
+  end
+
+  def test_hstore
+    return if DATABASE != "postgres"
+
+    product = create(:product, hstore: { name: "expected" })
+
+    assert_includes Product.search("hstore_name: expected"), product
+    refute_includes Product.search("hstore_name: rejected"), product
+  end
 end

test/test_helper.rb

@@ -49,9 +49,13 @@ class Product < ActiveRecord::Base
     end
 
     if DATABASE == "postgres"
+      attributes nested_jsonb_name: "nested_jsonb->nested->name", jsonb_name: "jsonb->name", hstore_name: "hstore->name"
+
       options :title, dictionary: "english"
     end
 
+    attributes nested_json_name: "nested_json->nested->name", json_name: "json->name"
+
     generator :custom_eq do |column_name, raw_value|
       "#{column_name} = #{quote raw_value}"
     end
@@ -128,6 +132,15 @@ class AvailableProduct < Product
   t.boolean :available
   t.string :brand
   t.string :notice
+
+  if DATABASE == "postgres"
+    t.jsonb :jsonb
+    t.jsonb :nested_jsonb
+    t.hstore :hstore
+  end
+
+  t.json :json
+  t.json :nested_json
 end
 
 ActiveRecord::Base.connection.create_table :posts do |t|

test/visitor_test.rb

@@ -2,62 +2,62 @@
 
 class VisitorTest < SearchCop::TestCase
   def test_and
-    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").gt(0).and(SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").lt(2))
+    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).gt(0).and(SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).lt(2))
 
     assert_equal "(#{quote_table_name "products"}.#{quote_column_name "stock"} > 0 AND #{quote_table_name "products"}.#{quote_column_name "stock"} < 2)", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)
   end
 
   def test_or
-    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").gt(0).or(SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").lt(2))
+    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).gt(0).or(SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).lt(2))
 
     assert_equal "(#{quote_table_name "products"}.#{quote_column_name "stock"} > 0 OR #{quote_table_name "products"}.#{quote_column_name "stock"} < 2)", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)
   end
 
   def test_greater_than
-    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").gt(1)
+    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).gt(1)
 
     assert_equal "#{quote_table_name "products"}.#{quote_column_name "stock"} > 1", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)
   end
 
   def test_greater_than_or_equal
-    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").gteq(1)
+    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).gteq(1)
 
     assert_equal "#{quote_table_name "products"}.#{quote_column_name "stock"} >= 1", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)
   end
 
   def test_less_than
-    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").lt(1)
+    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).lt(1)
 
     assert_equal "#{quote_table_name "products"}.#{quote_column_name "stock"} < 1", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)
   end
 
   def test_less_than_or_equal
-    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").lteq(1)
+    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).lteq(1)
 
     assert_equal "#{quote_table_name "products"}.#{quote_column_name "stock"} <= 1", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)
   end
 
   def test_equality
-    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").eq(1)
+    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).eq(1)
 
     assert_equal "#{quote_table_name "products"}.#{quote_column_name "stock"} = 1", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)
   end
 
   def test_not_equal
-    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").not_eq(1)
+    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).not_eq(1)
 
     assert_equal "#{quote_table_name "products"}.#{quote_column_name "stock"} != 1", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)
   end
 
   def test_matches
-    node = SearchCopGrammar::Attributes::String.new(Product, "products", "notice").matches("Notice")
+    node = SearchCopGrammar::Attributes::String.new(Product, "products", "notice", nil).matches("Notice")
 
     assert_equal("(#{quote_table_name "products"}.#{quote_column_name "notice"} IS NOT NULL AND #{quote_table_name "products"}.#{quote_column_name "notice"} LIKE #{quote "%Notice%"} ESCAPE #{quote "\\"})", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)) if ENV["DATABASE"] != "postgres"
     assert_equal("(#{quote_table_name "products"}.#{quote_column_name "notice"} IS NOT NULL AND #{quote_table_name "products"}.#{quote_column_name "notice"} ILIKE #{quote "%Notice%"} ESCAPE #{quote "\\"})", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)) if ENV["DATABASE"] == "postgres"
   end
 
   def test_not
-    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock").eq(1).not
+    node = SearchCopGrammar::Attributes::Integer.new(Product, "products", "stock", nil).eq(1).not
 
     assert_equal "NOT (#{quote_table_name "products"}.#{quote_column_name "stock"} = 1)", SearchCop::Visitors::Visitor.new(ActiveRecord::Base.connection).visit(node)
   end