Add allow_nil option for IS NULL filtering on non-JSONB types

lib/procore-sift.rb

@@ -66,8 +66,8 @@ def sort_fields
   end
 
   class_methods do
-    def filter_on(parameter, type:, internal_name: parameter, default: nil, validate: nil, scope_params: [], tap: nil)
-      filters << Filter.new(parameter, type, internal_name, default, validate, scope_params, tap)
+    def filter_on(parameter, type:, internal_name: parameter, default: nil, validate: nil, scope_params: [], tap: nil, allow_nil: false)
+      filters << Filter.new(parameter, type, internal_name, default, validate, scope_params, tap, allow_nil: allow_nil)
     end
 
     def filters

lib/sift/filter.rb

@@ -4,8 +4,8 @@ module Sift
   class Filter
     attr_reader :parameter, :default, :custom_validate, :scope_params
 
-    def initialize(param, type, internal_name, default, custom_validate = nil, scope_params = [], tap = ->(value, _params) { value })
-      @parameter = Parameter.new(param, type, internal_name)
+    def initialize(param, type, internal_name, default, custom_validate = nil, scope_params = [], tap = ->(value, _params) { value }, allow_nil: false)
+      @parameter = Parameter.new(param, type, internal_name, allow_nil: allow_nil)
       @default = default
       @custom_validate = custom_validate
       @scope_params = scope_params
@@ -41,7 +41,7 @@ def validation_field
     end
 
     def type_validator
-      @type_validator ||= Sift::TypeValidator.new(param, type)
+      @type_validator ||= Sift::TypeValidator.new(param, type, allow_nil: parameter.allow_nil)
     end
 
     def type

lib/sift/parameter.rb

@@ -1,12 +1,13 @@
 module Sift
   # Value Object that wraps some handling of filter params
   class Parameter
-    attr_reader :param, :type, :internal_name
+    attr_reader :param, :type, :internal_name, :allow_nil
 
-    def initialize(param, type, internal_name = param)
+    def initialize(param, type, internal_name = param, allow_nil: false)
       @param = param
       @type = type
       @internal_name = internal_name
+      @allow_nil = allow_nil
     end
 
     def parse_options

lib/sift/type_validator.rb

@@ -17,9 +17,10 @@ class TypeValidator
                        :scope,
                        :jsonb].freeze
 
-    def initialize(param, type)
+    def initialize(param, type, allow_nil: false)
       @param = param
       @type = type
+      @allow_nil = allow_nil
     end
 
     attr_reader :param, :type
@@ -46,7 +47,7 @@ def valid_type?
     private
 
     def valid_int?
-      { valid_int: true }
+      @allow_nil ? { valid_int: { allow_null_token: true } } : { valid_int: true }
     end
   end
 end

lib/sift/validators/valid_int_validator.rb

@@ -1,24 +1,34 @@
 class ValidIntValidator < ActiveModel::EachValidator
+  NULL_TOKEN = "null"
+
   def validate_each(record, attribute, value)
     record.errors.add attribute, (options[:message] || "must be integer, array of integers, or range") unless
       valid_int?(value)
   end
 
   private
 
+  def allow_null_token?
+    options.fetch(:allow_null_token, false)
+  end
+
   def valid_int?(value)
-    integer_array?(value) || integer_or_range?(value)
+    (allow_null_token? && null_token?(value)) || integer_array?(value) || integer_or_range?(value)
   end
 
   def integer_array?(value)
     if value.is_a?(String)
       value = Sift::ValueParser.new(value: value).array_from_json
     end
 
-    value.is_a?(Array) && value.any? && value.all? { |v| integer_or_range?(v) }
+    value.is_a?(Array) && value.any? && value.all? { |v| integer_or_range?(v) || (allow_null_token? && null_token?(v)) }
   end
 
   def integer_or_range?(value)
     !!(/\A\d+(...\d+)?\z/ =~ value.to_s)
   end
+
+  def null_token?(value)
+    value.to_s.downcase == NULL_TOKEN
+  end
 end

lib/sift/where_handler.rb

@@ -1,19 +1,38 @@
 module Sift
   class WhereHandler
+    NULL_TOKEN = "null"
+
     def initialize(param)
       @param = param
     end
 
     def call(collection, value, _params, _scope_params)
       if @param.type == :jsonb
         apply_jsonb_conditions(collection, value)
+      elsif @param.allow_nil
+        apply_null_aware_conditions(collection, value)
       else
         collection.where(@param.internal_name => value)
       end
     end
 
     private
 
+    def apply_null_aware_conditions(collection, value)
+      values = Array(value)
+      has_null = values.any? { |v| v.to_s.downcase == NULL_TOKEN }
+      non_null_values = values.reject { |v| v.to_s.downcase == NULL_TOKEN }
+
+      if has_null && non_null_values.any?
+        collection.where(@param.internal_name => nil)
+                  .or(collection.where(@param.internal_name => non_null_values))
+      elsif has_null
+        collection.where(@param.internal_name => nil)
+      else
+        collection.where(@param.internal_name => value)
+      end
+    end
+
     def apply_jsonb_conditions(collection, value)
       return collection.where("#{@param.internal_name} @> ?", value.to_s) if value.is_a?(Array)
 

test/controller_test.rb

@@ -328,4 +328,70 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
     assert_mock class_mock
     assert_mock instance_mock
   end
+
+  test "it filters with null token on an allow_nil integer column" do
+    post_with_nil = Post.create!(priority: nil)
+    Post.create!(priority: 5)
+
+    get("/posts", params: { filters: { nullable_priority: "null" } })
+
+    json = JSON.parse(@response.body)
+    assert_equal 1, json.size
+    assert_equal post_with_nil.id, json.first["id"]
+  end
+
+  test "it filters with uppercase NULL token on an allow_nil integer column" do
+    post_with_nil = Post.create!(priority: nil)
+    Post.create!(priority: 5)
+
+    get("/posts", params: { filters: { nullable_priority: "NULL" } })
+
+    json = JSON.parse(@response.body)
+    assert_equal 1, json.size
+    assert_equal post_with_nil.id, json.first["id"]
+  end
+
+  test "it filters with null token combined with real values on an allow_nil integer column" do
+    post_with_nil = Post.create!(priority: nil)
+    post_with_five = Post.create!(priority: 5)
+    Post.create!(priority: 10)
+
+    get("/posts", params: { filters: { nullable_priority: ["null", post_with_five.priority] } })
+
+    json = JSON.parse(@response.body)
+    returned_ids = json.map { |p| p["id"] }.sort
+    assert_equal [post_with_nil.id, post_with_five.id].sort, returned_ids
+  end
+
+  test "it filters with null token on an allow_nil string column" do
+    Post.create!(title: "hello")
+    post_with_nil = Post.create!(title: nil)
+
+    get("/posts", params: { filters: { nullable_title: "null" } })
+
+    json = JSON.parse(@response.body)
+    assert_equal 1, json.size
+    assert_equal post_with_nil.id, json.first["id"]
+  end
+
+  test "it rejects null token on an integer column without allow_nil" do
+    Post.create!(priority: nil)
+
+    get("/posts", params: { filters: { priority: "null" } })
+
+    assert_equal "400", @response.code
+    json = JSON.parse(@response.body)
+    assert_equal({ "errors" => { "priority" => ["must be integer, array of integers, or range"] } }, json)
+  end
+
+  test "it filters without null token preserves original behavior" do
+    post = Post.create!(priority: 5)
+    Post.create!(priority: 10)
+
+    get("/posts", params: { filters: { priority: post.priority } })
+
+    json = JSON.parse(@response.body)
+    assert_equal 1, json.size
+    assert_equal post.id, json.first["id"]
+  end
 end

test/dummy/app/controllers/posts_controller.rb

@@ -17,6 +17,8 @@ class PostsController < ApplicationController
   filter_on :metadata, type: :jsonb
 
   filter_on :french_bread, type: :string, internal_name: :title
+  filter_on :nullable_priority, type: :int, internal_name: :priority, allow_nil: true
+  filter_on :nullable_title, type: :string, internal_name: :title, allow_nil: true
   filter_on :body2, type: :scope, internal_name: :body2, default: ->(c) { c.order(:body) }
 
   filter_on :expiration, type: :datetime, tap: ->(value, params) {