fix: nil must be handled differently in a sequence sub query (#9)

* fix: nil should be handled different in a sequence sub query

* chore: prefer not to use locale for logs and exception

* chore: ready for 0.2.2

CHANGELOG.md

@@ -10,6 +10,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Move translation from wallaby-core to here
 
+## [0.2.2](https://github.com/wallaby-rails/wallaby-active_record/releases/tag/0.2.2) - 2020-03-19
+
+### Changed
+
+- fix: nil must be handled differently in a sequence sub query ([#9](https://github.com/wallaby-rails/wallaby-active_record/pull/9))
+- fix: check database and table's existence before getting the metadata. ([#8](https://github.com/wallaby-rails/wallaby-active_record/pull/8))
+- chore: use simplecov 0.17 for codeclimate report ([#7](https://github.com/wallaby-rails/wallaby-active_record/pull/7))
+
 ## [0.2.1](https://github.com/wallaby-rails/wallaby-active_record/releases/tag/0.2.1) - 2020-02-17
 
 ### Changed

lib/adapters/wallaby/active_record/model_pagination_provider.rb

@@ -12,11 +12,7 @@ def paginatable?
           # kaminari
           @collection.respond_to?(:total_count) || \
           @collection.respond_to?(:total_entries) # will_paginate
-        unless paginatable
-          Rails.logger.warn I18n.t(
-            'errors.activerecord.paginatable', collection: @collection.inspect
-          )
-        end
+        Rails.logger.warn "#{@collection.inspect} is not paginatable.\nfrom #{__FILE__}:#{__LINE__}" unless paginatable
 
         paginatable
       end

lib/adapters/wallaby/active_record/model_service_provider/querier.rb

@@ -13,9 +13,9 @@ def initialize(model_decorator)
           @model_class = @model_decorator.model_class
         end
 
-        # Pull out the query expression string from the parameter `q`,
-        # use parser to understand the expression, then use transformer to run
-        # SQL arel query.
+        # Extract the filter and query information
+        # from parameters `filter` and `q` respectively,
+        # and execute query based on the information.
         # @param params [ActionController::Parameters]
         # @return [ActiveRecord::Relation]
         def search(params)
@@ -28,40 +28,21 @@ def search(params)
 
         private
 
-        # @see Wallaby::Parser
-        def parser
-          @parser ||= Parser.new
-        end
-
-        # @see Wallaby::ActiveRecord::ModelServiceProvider::Querier::Transformer
-        def transformer
-          @transformer ||= Transformer.new
-        end
-
         # @return [Arel::Table] arel table
         def table
           @model_class.arel_table
         end
 
         # @param params [ActionController::Parameters]
-        # @return [Array<String, Array, Array>] a list of object for other
-        #   method to use.
+        # @return [Array<String, Array, Array>] filter_name, keywords, field_queries
         def extract(params)
-          expressions = to_expressions params
+          expressions = Transformer.new.transform params[:q]
           keywords = expressions.select { |v| v.is_a? String }
-          field_queries = expressions.select { |v| v.is_a? Hash }
+          field_queries = expressions.select { |v| v.is_a? Wrapper }
           filter_name = params[:filter]
           [filter_name, keywords, field_queries]
         end
 
-        # @param params [ActionController::Parameters]
-        # @return [Array] a list of transformed operations
-        def to_expressions(params)
-          parsed = parser.parse(params[:q] || EMPTY_STRING)
-          converted = transformer.apply parsed
-          converted.is_a?(Array) ? converted : [converted]
-        end
-
         # Use the filter name to find out the scope in the following precedents:
         # - scope from metadata
         # - defined scope from the model
@@ -81,25 +62,23 @@ def filtered_by(filter_name)
         end
 
         # Find out the scope for given filter
-        # - from metadata
+        # - from filter metadata
         # - filter name itself
         # @param filter_name [String] filter name
         # @return [String]
         def find_scope(filter_name)
-          filter = @model_decorator.filters[filter_name] || {}
-          filter[:scope] || filter_name
+          @model_decorator.filters[filter_name].try(:[], :scope) || filter_name
         end
 
-        # Unscoped query
-        # @return [ActiveRecord::Relation]
+        # @return [ActiveRecord::Relation] Unscoped query
         def unscoped
           @model_class.where nil
         end
 
-        # Search text for the text columns that appear in `index_field_names`
+        # Search text for the text columns (see {}) in `index_field_names`
         # @param keywords [String] keywords
         # @param query [ActiveRecord::Relation, nil]
-        # @return [ActiveRecord::Relation]
+        # @return [ActiveRecord::Relation, nil]
         def text_search(keywords, query = nil)
           return query unless keywords_check? keywords
 
@@ -121,9 +100,9 @@ def text_search(keywords, query = nil)
         def field_search(field_queries, query)
           return query unless field_check? field_queries
 
-          field_queries.each do |exp|
-            sub_query = table[exp[:left]].try(exp[:op], exp[:right])
-            query = query.try(:and, sub_query) || sub_query
+          field_queries.each do |exps|
+            sub_queries = build_sub_queries_with exps
+            query = query.try(:and, sub_queries) || sub_queries
           end
           query
         end
@@ -140,31 +119,39 @@ def text_fields
         end
 
         # @param keywords [Array<String>] a list of keywords
-        # @return [Boolean] false when keywords are empty
-        #   true when text fields for query exist
-        #   otherwise, raise exception
+        # @return [false] when keywords are empty
+        # @return [true] when text fields for query exist
+        # @raise [Wallaby::UnprocessableEntity] if no text columns can be used for text search
         def keywords_check?(keywords)
           return false if keywords.blank?
           return true if text_fields.present?
 
-          message = I18n.t 'errors.unprocessable_entity.keyword_search'
-          raise UnprocessableEntity, message
+          raise UnprocessableEntity, 'Unable to perform keyword search when no text fields can be used for this.'
         end
 
         # @param field_queries [Array]
-        # @return [Boolean] false when field queries are blank
-        #   true when the fields used are valid (exist in `fields`)
-        #   otherwise, raise exception
+        # @return [false] when field queries are blank
+        # @return [true] when field queries are blank
+        # @raise [Wallaby::UnprocessableEntity] if invalid fields are entered
         def field_check?(field_queries)
           return false if field_queries.blank?
 
           fields = field_queries.map { |exp| exp[:left] }
           invalid_fields = fields - @model_decorator.fields.keys
           return true if invalid_fields.blank?
 
-          message = I18n.t 'errors.unprocessable_entity.field_colon_search',
-                           invalid_fields: invalid_fields.to_sentence
-          raise UnprocessableEntity, message
+          raise UnprocessableEntity, "Unable to perform field colon search for #{invalid_fields.to_sentence}"
+        end
+
+        # @param exps [Wallaby::ActiveRecord::ModelServiceProvider::Querier::Wrapper]
+        # @return [ActiveRecord::Relation] sub queries connected using OR
+        def build_sub_queries_with(exps)
+          query = nil
+          exps.each do |exp|
+            sub = table[exp[:left]].try(exp[:op], exp[:right])
+            query = query.try(:or, sub) || sub
+          end
+          query
         end
       end
     end

lib/adapters/wallaby/active_record/model_service_provider/querier/transformer.rb

@@ -61,7 +61,7 @@ class Transformer < Parslet::Transform
               when ':^', ':!^' then "#{right}%"
               when ':$', ':!$' then "%#{right}"
               end
-            { left: lefted, op: operator, right: convert || right }
+            Wrapper.new [{ left: lefted, op: operator, right: convert || right }]
           end
 
           # For operators that have multiple items
@@ -70,9 +70,22 @@ class Transformer < Parslet::Transform
             operator = SEQUENCE_OPERATORS[oped]
             next unless operator
 
+            exps = Wrapper.new
             lefted = left.try :to_str
-            convert = Range.new right.try(:first), right.try(:last) if %w(:() :!()).include?(oped)
-            { left: lefted, op: operator, right: convert || right }
+            if right.include? nil
+              nil_operator = SIMPLE_OPERATORS[oped]
+              next unless nil_operator
+
+              exps.push left: lefted, op: nil_operator, right: right.delete(nil)
+            end
+            convert = Range.new right.try(:first), right.try(:second) if %w(:() :!()).include?(oped)
+            exps.push left: lefted, op: operator, right: convert || right
+            exps
+          end
+
+          def transform(query_string)
+            result = apply Parser.new.parse(query_string || EMPTY_STRING)
+            result.is_a?(Array) ? result : [result]
           end
         end
       end

lib/adapters/wallaby/active_record/model_service_provider/querier/wrapper.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module Wallaby
+  class ActiveRecord
+    class ModelServiceProvider
+      class Querier
+        # Build up query using the results
+        class Wrapper
+          attr_reader :list
+          delegate :push, :each, to: :list
+
+          def initialize(list = [])
+            @list = list
+          end
+
+          def [](key)
+            list.last[key]
+          end
+        end
+      end
+    end
+  end
+end

lib/adapters/wallaby/active_record/pundit_provider.rb

@@ -11,7 +11,7 @@ class PunditProvider < PunditAuthorizationProvider
       def accessible_for(_action, scope)
         Pundit.policy_scope! user, scope
       rescue Pundit::NotDefinedError
-        Rails.logger.warn I18n.t('errors.pundit.not_found.scope_policy', scope: scope)
+        Rails.logger.warn "Cannot find scope policy for #{scope.inspect}.\nfrom #{__FILE__}:#{__LINE__}"
         scope
       end
     end

lib/wallaby/active_record.rb

@@ -28,6 +28,7 @@
 require 'adapters/wallaby/active_record/model_service_provider/querier'
 require 'adapters/wallaby/active_record/model_service_provider/querier/escaper'
 require 'adapters/wallaby/active_record/model_service_provider/querier/transformer'
+require 'adapters/wallaby/active_record/model_service_provider/querier/wrapper'
 require 'adapters/wallaby/active_record/model_service_provider/validator'
 # ModelServiceProvider: end
 

lib/wallaby/active_record/version.rb

@@ -2,6 +2,6 @@
 
 module Wallaby
   module ActiveRecordGem
-    VERSION = '0.2.1' # :nodoc:
+    VERSION = '0.2.2' # :nodoc:
   end
 end

spec/adapters/wallaby/active_record/model_service_provider/querier/transformer_spec.rb

@@ -17,33 +17,33 @@
 
   describe 'simple colon_queries' do
     it 'transforms' do
-      expect(subject.apply(left: 'field', op: ':', right: { string: 'key' })).to eq(left: 'field', op: :eq, right: 'key')
-      expect(subject.apply(left: 'field', op: ':', right: [{ string: 'key1' }, { string: 'key2' }])).to eq(left: 'field', op: :in, right: %w(key1 key2))
+      expect(subject.apply(left: 'field', op: ':', right: { string: 'key' }).list).to eq([{ left: 'field', op: :eq, right: 'key' }])
+      expect(subject.apply(left: 'field', op: ':', right: [{ string: 'key1' }, { string: 'key2' }]).list).to eq([{ left: 'field', op: :in, right: %w(key1 key2) }])
     end
 
     describe 'general colon_queries' do
       it 'transforms' do
-        expect(subject.apply(left: 'field', op: ':', right: { string: 'key' })).to eq(left: 'field', op: :eq, right: 'key')
-        expect(subject.apply(left: 'field', op: ':=', right: { string: 'key' })).to eq(left: 'field', op: :eq, right: 'key')
-        expect(subject.apply(left: 'field', op: ':!', right: { string: 'key' })).to eq(left: 'field', op: :not_eq, right: 'key')
-        expect(subject.apply(left: 'field', op: ':!=', right: { string: 'key' })).to eq(left: 'field', op: :not_eq, right: 'key')
-        expect(subject.apply(left: 'field', op: ':~', right: { string: 'key' })).to eq(left: 'field', op: :matches, right: '%key%')
-        expect(subject.apply(left: 'field', op: ':^', right: { string: 'key' })).to eq(left: 'field', op: :matches, right: 'key%')
-        expect(subject.apply(left: 'field', op: ':$', right: { string: 'key' })).to eq(left: 'field', op: :matches, right: '%key')
-        expect(subject.apply(left: 'field', op: ':!~', right: { string: 'key' })).to eq(left: 'field', op: :does_not_match, right: '%key%')
-        expect(subject.apply(left: 'field', op: ':!^', right: { string: 'key' })).to eq(left: 'field', op: :does_not_match, right: 'key%')
-        expect(subject.apply(left: 'field', op: ':!$', right: { string: 'key' })).to eq(left: 'field', op: :does_not_match, right: '%key')
-        expect(subject.apply(left: 'field', op: ':>', right: { string: 'key' })).to eq(left: 'field', op: :gt, right: 'key')
-        expect(subject.apply(left: 'field', op: ':>=', right: { string: 'key' })).to eq(left: 'field', op: :gteq, right: 'key')
-        expect(subject.apply(left: 'field', op: ':<', right: { string: 'key' })).to eq(left: 'field', op: :lt, right: 'key')
-        expect(subject.apply(left: 'field', op: ':<=', right: { string: 'key' })).to eq(left: 'field', op: :lteq, right: 'key')
-        expect(subject.apply(left: 'field', op: ':', right: [{ string: 'key1' }, { string: 'key2' }])).to eq(left: 'field', op: :in, right: %w(key1 key2))
-        expect(subject.apply(left: 'field', op: ':=', right: [{ string: 'key1' }, { string: 'key2' }])).to eq(left: 'field', op: :in, right: %w(key1 key2))
-        expect(subject.apply(left: 'field', op: ':!', right: [{ string: 'key1' }, { string: 'key2' }])).to eq(left: 'field', op: :not_in, right: %w(key1 key2))
-        expect(subject.apply(left: 'field', op: ':!=', right: [{ string: 'key1' }, { string: 'key2' }])).to eq(left: 'field', op: :not_in, right: %w(key1 key2))
-        expect(subject.apply(left: 'field', op: ':<>', right: [{ string: 'key1' }, { string: 'key2' }])).to eq(left: 'field', op: :not_in, right: %w(key1 key2))
-        expect(subject.apply(left: 'field', op: ':()', right: [{ string: 'key1' }, { string: 'key2' }])).to eq(left: 'field', op: :between, right: 'key1'..'key2')
-        expect(subject.apply(left: 'field', op: ':!()', right: [{ string: 'key1' }, { string: 'key2' }])).to eq(left: 'field', op: :not_between, right: 'key1'..'key2')
+        expect(subject.apply(left: 'field', op: ':', right: { string: 'key' }).list).to eq([{ left: 'field', op: :eq, right: 'key' }])
+        expect(subject.apply(left: 'field', op: ':=', right: { string: 'key' }).list).to eq([{ left: 'field', op: :eq, right: 'key' }])
+        expect(subject.apply(left: 'field', op: ':!', right: { string: 'key' }).list).to eq([{ left: 'field', op: :not_eq, right: 'key' }])
+        expect(subject.apply(left: 'field', op: ':!=', right: { string: 'key' }).list).to eq([{ left: 'field', op: :not_eq, right: 'key' }])
+        expect(subject.apply(left: 'field', op: ':~', right: { string: 'key' }).list).to eq([{ left: 'field', op: :matches, right: '%key%' }])
+        expect(subject.apply(left: 'field', op: ':^', right: { string: 'key' }).list).to eq([{ left: 'field', op: :matches, right: 'key%' }])
+        expect(subject.apply(left: 'field', op: ':$', right: { string: 'key' }).list).to eq([{ left: 'field', op: :matches, right: '%key' }])
+        expect(subject.apply(left: 'field', op: ':!~', right: { string: 'key' }).list).to eq([{ left: 'field', op: :does_not_match, right: '%key%' }])
+        expect(subject.apply(left: 'field', op: ':!^', right: { string: 'key' }).list).to eq([{ left: 'field', op: :does_not_match, right: 'key%' }])
+        expect(subject.apply(left: 'field', op: ':!$', right: { string: 'key' }).list).to eq([{ left: 'field', op: :does_not_match, right: '%key' }])
+        expect(subject.apply(left: 'field', op: ':>', right: { string: 'key' }).list).to eq([{ left: 'field', op: :gt, right: 'key' }])
+        expect(subject.apply(left: 'field', op: ':>=', right: { string: 'key' }).list).to eq([{ left: 'field', op: :gteq, right: 'key' }])
+        expect(subject.apply(left: 'field', op: ':<', right: { string: 'key' }).list).to eq([{ left: 'field', op: :lt, right: 'key' }])
+        expect(subject.apply(left: 'field', op: ':<=', right: { string: 'key' }).list).to eq([{ left: 'field', op: :lteq, right: 'key' }])
+        expect(subject.apply(left: 'field', op: ':', right: [{ string: 'key1' }, { string: 'key2' }]).list).to eq([{ left: 'field', op: :in, right: %w(key1 key2) }])
+        expect(subject.apply(left: 'field', op: ':=', right: [{ string: 'key1' }, { string: 'key2' }]).list).to eq([{ left: 'field', op: :in, right: %w(key1 key2) }])
+        expect(subject.apply(left: 'field', op: ':!', right: [{ string: 'key1' }, { string: 'key2' }]).list).to eq([{ left: 'field', op: :not_in, right: %w(key1 key2) }])
+        expect(subject.apply(left: 'field', op: ':!=', right: [{ string: 'key1' }, { string: 'key2' }]).list).to eq([{ left: 'field', op: :not_in, right: %w(key1 key2) }])
+        expect(subject.apply(left: 'field', op: ':<>', right: [{ string: 'key1' }, { string: 'key2' }]).list).to eq([{ left: 'field', op: :not_in, right: %w(key1 key2) }])
+        expect(subject.apply(left: 'field', op: ':()', right: [{ string: 'key1' }, { string: 'key2' }]).list).to eq([{ left: 'field', op: :between, right: 'key1'..'key2' }])
+        expect(subject.apply(left: 'field', op: ':!()', right: [{ string: 'key1' }, { string: 'key2' }]).list).to eq([{ left: 'field', op: :not_between, right: 'key1'..'key2' }])
       end
     end
 

spec/adapters/wallaby/active_record/model_service_provider/querier_spec.rb

@@ -183,7 +183,7 @@
               expect(subject.search(parameters(q: keyword)).to_sql).to eq 'SELECT "all_postgres_types".* FROM "all_postgres_types" WHERE "all_postgres_types"."integer" IN (1, 2)'
 
               keyword = 'integer:=1,nil'
-              expect(subject.search(parameters(q: keyword)).to_sql).to eq 'SELECT "all_postgres_types".* FROM "all_postgres_types" WHERE "all_postgres_types"."integer" IN (1, NULL)'
+              expect(subject.search(parameters(q: keyword)).to_sql).to eq 'SELECT "all_postgres_types".* FROM "all_postgres_types" WHERE ("all_postgres_types"."integer" IS NULL OR "all_postgres_types"."integer" IN (1))'
             end
           end