Merge pull request #33 from procore/support_json_array

Support json array

Author: amayer171

.rubocop.yml

@@ -373,7 +373,7 @@ Style/TrailingCommaInArguments:
     - no_comma
   Enabled: true
 
-Style/TrailingCommaInLiteral:
+Style/TrailingCommaInArrayLiteral:
   Description: 'Checks for trailing comma in array and hash literals.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-trailing-array-commas'
   EnforcedStyleForMultiline: comma

README.md

@@ -109,6 +109,26 @@ class PostsController < ApplicationController
 end
 ```
 
+### Filter on Ranges
+Some parameter types support ranges. Ranges are expected to be a string with the bounding values separated by `...`
+
+For example `?filters[price]=3...50` would return records with a price between 3 and 50.
+
+The following types support ranges
+* int
+* decimal
+* boolean
+* date
+* time
+* datetime
+
+### Filter on JSON Array
+`int` type filters support sending the values as an array in the URL Query parameters. For example `?filters[id]=[1,2]`. This is a way to keep payloads smaller for GET requests. When URI encoded this will become `filters%5Bid%5D=%5B1,2%5D` which is much smaller the standard format of `filters%5Bid%5D%5B%5D=1&&filters%5Bid%5D%5B%5D=2`.
+
+On the server side, the params will be received as:
+`"filters"=>{"id"=>"[1,2]"}` compared to the standard format of
+`"filters"=>{"id"=>["1", "2"]}`.
+
 ### Sort Types
 Every sort must have a type, so that Brita knows what to do with it. The current valid sort types are:
 * int - Sort on an integer column
@@ -189,7 +209,11 @@ $ gem install brita
 
 ## Contributing
 
-Contribution directions go here.
+Running tests:
+```bash
+$ bundle exec rake test
+```
+Bash shell is recommend if tests are not working in your shell of choice.
 
 ## License
 

lib/brita.rb

@@ -5,6 +5,7 @@
 require "brita/subset_comparator"
 require "brita/type_validator"
 require "brita/parameter"
+require "brita/value_parser"
 require "brita/scope_handler"
 require "brita/where_handler"
 require "brita/validators/valid_int_validator"

lib/brita/filter.rb

@@ -51,6 +51,10 @@ def param
 
     private
 
+    def parameterize(value)
+      ValueParser.new(value: value, options: parameter.parse_options).parse
+    end
+
     def not_processable?(value)
       value.nil? && default.nil?
     end
@@ -65,20 +69,6 @@ def mapped_scope_params(params)
       end
     end
 
-    def parameterize(value)
-      if supports_ranges? && value.to_s.include?("...")
-        Range.new(*value.split("..."))
-      elsif type == :boolean
-        if Rails.version.starts_with?("5")
-          ActiveRecord::Type::Boolean.new.cast(value)
-        else
-          ActiveRecord::Type::Boolean.new.type_cast_from_user(value)
-        end
-      else
-        value
-      end
-    end
-
     def valid_scope_params?(scope_params)
       scope_params.is_a?(Array) && scope_params.all? { |symbol| symbol.is_a?(Symbol) }
     end

lib/brita/parameter.rb

@@ -9,8 +9,12 @@ def initialize(param, type, internal_name = param)
       @internal_name = internal_name
     end
 
-    def supports_ranges?
-      ![:string, :text, :scope].include?(type)
+    def parse_options
+      {
+        supports_boolean: supports_boolean?,
+        supports_ranges: supports_ranges?,
+        supports_json: supports_json?
+      }
     end
 
     def handler
@@ -20,5 +24,19 @@ def handler
         WhereHandler.new(self)
       end
     end
+
+    private
+
+    def supports_ranges?
+      ![:string, :text, :scope].include?(type)
+    end
+
+    def supports_json?
+      type == :int
+    end
+
+    def supports_boolean?
+      type == :boolean
+    end
   end
 end

lib/brita/validators/valid_int_validator.rb

@@ -11,6 +11,10 @@ def valid_int?(value)
   end
 
   def integer_array?(value)
+    if value.is_a?(String)
+      value = Brita::ValueParser.new(value: value).array_from_json
+    end
+
     value.is_a?(Array) && value.any? && value.all? { |v| integer_or_range?(v) }
   end
 

lib/brita/value_parser.rb

@@ -0,0 +1,61 @@
+module Brita
+  class ValueParser
+    attr_accessor :value, :supports_boolean, :supports_json, :supports_ranges
+    def initialize(value:, options: {})
+      @value = value
+      @supports_boolean = options.fetch(:supports_boolean, false)
+      @supports_ranges = options.fetch(:supports_ranges, false)
+      @supports_json = options.fetch(:supports_json, false)
+    end
+
+    def parse
+      @_result ||=
+        if parse_as_range?
+          range_value
+        elsif parse_as_boolean?
+          boolean_value
+        elsif parse_as_json?
+          array_from_json
+        else
+          value
+        end
+    end
+
+    def array_from_json
+      result = JSON.parse(value)
+      if result.is_a?(Array)
+        result
+      else
+        value
+      end
+    rescue JSON::ParserError
+      value
+    end
+
+    private
+
+    def parse_as_range?
+      supports_ranges && value.to_s.include?("...")
+    end
+
+    def range_value
+      Range.new(*value.split("..."))
+    end
+
+    def parse_as_json?
+      supports_json && value.is_a?(String)
+    end
+
+    def parse_as_boolean?
+      supports_boolean
+    end
+
+    def boolean_value
+      if Rails.version.starts_with?("5")
+        ActiveRecord::Type::Boolean.new.cast(value)
+      else
+        ActiveRecord::Type::Boolean.new.type_cast_from_user(value)
+      end
+    end
+  end
+end

test/controller_test.rb

@@ -22,6 +22,19 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
     assert_equal post.id, json.first["id"]
   end
 
+  test "it filters on id by value for a JSON string array" do
+    post1 = Post.create!
+    post2 = Post.create!
+    post3 = Post.create!
+
+    get("/posts", params: { filters: { id: "[#{post1.id},#{post2.id}]" } })
+
+    json = JSON.parse(@response.body)
+    assert_equal 2, json.size
+    ids_array = json.map { |json_hash| json_hash["id"] }
+    assert_equal [post1.id, post2.id], ids_array
+  end
+
   test "it filters on decimals" do
     post = Post.create!(rating: 1.25)
     Post.create!

test/type_validator_test.rb

@@ -33,4 +33,11 @@ class TypeValidatorTest < ActiveSupport::TestCase
 
     assert_equal expected_validation, validator.validate
   end
+
+  test "it accepts a json array for type int" do
+    validator = Brita::TypeValidator.new("[1,10]", :int)
+    expected_validation = { valid_int: true }
+
+    assert_equal expected_validation, validator.validate
+  end
 end

test/value_parser_test.rb

@@ -0,0 +1,77 @@
+require "test_helper"
+
+class FilterTest < ActiveSupport::TestCase
+  test "returns the original value without options" do
+    parser = Brita::ValueParser.new(value: "hi")
+
+    assert_equal "hi", parser.parse
+  end
+
+  test "With options an array of integers results in an array of integers" do
+    options = {
+      supports_ranges: true,
+      supports_json: true
+    }
+    parser = Brita::ValueParser.new(value: [1,2,3])
+
+    assert_equal [1,2,3], parser.parse
+  end
+
+  test "With options a json string array of integers results in an array of integers" do
+    options = {
+      supports_ranges: true,
+      supports_json: true
+    }
+    parser = Brita::ValueParser.new(value: "[1,2,3]", options: options)
+
+    assert_equal [1,2,3], parser.parse
+  end
+
+  test "with invalid json returns original value" do
+    options = {
+      supports_ranges: true,
+      supports_json: true
+    }
+    parser = Brita::ValueParser.new(value: "[1,2,3", options: options)
+
+    assert_equal "[1,2,3", parser.parse
+  end
+
+  test "JSON parsing only supports arrays" do
+    options = {
+      supports_json: true
+    }
+    json_string = "{\"a\":4}"
+    parser = Brita::ValueParser.new(value: json_string, options: options)
+
+    assert_equal json_string, parser.parse
+  end
+
+  test "With options a range string of integers results in a range" do
+    options = {
+      supports_ranges: true,
+      supports_json: true
+    }
+    parser = Brita::ValueParser.new(value: "1...3", options: options)
+
+    assert_instance_of Range, parser.parse
+  end
+
+  test "parses true from 1" do
+    options = {
+      supports_boolean: true
+    }
+    parser = Brita::ValueParser.new(value: 1, options: options)
+
+    assert_equal true, parser.parse
+  end
+
+  test "parses false from 0" do
+    options = {
+      supports_boolean: true
+    }
+    parser = Brita::ValueParser.new(value: 0, options: options)
+
+    assert_equal false, parser.parse
+  end
+end