Merge pull request #217 from agworld/feature/deny_empty

Add option to deny empty filter values.

Author: richmolj

lib/graphiti/errors.rb

@@ -143,8 +143,13 @@ def initialize(resource, filter, value)
       def message
         allow = @filter.values[0][:allow]
         deny = @filter.values[0][:deny]
+        value_string = if @value == "(empty)"
+          "empty value"
+        else
+          "value #{@value.inspect}"
+        end
         msg = <<-MSG
-          #{@resource.class.name}: tried to filter on #{@filter.keys[0].inspect}, but passed invalid value #{@value.inspect}.
+          #{@resource.class.name}: tried to filter on #{@filter.keys[0].inspect}, but passed invalid #{value_string}.
         MSG
         msg << "\nAllowlist: #{allow.inspect}" if allow
         msg << "\nDenylist: #{deny.inspect}" if deny

lib/graphiti/resource/configuration.rb

@@ -82,7 +82,8 @@ class << self
           :attributes_schema_by_default,
           :relationships_readable_by_default,
           :relationships_writable_by_default,
-          :filters_accept_nil_by_default
+          :filters_accept_nil_by_default,
+          :filters_deny_empty_by_default
 
         class << self
           prepend Overrides
@@ -104,6 +105,7 @@ def self.inherited(klass)
           default(klass, :relationships_readable_by_default, true)
           default(klass, :relationships_writable_by_default, true)
           default(klass, :filters_accept_nil_by_default, false)
+          default(klass, :filters_deny_empty_by_default, false)
 
           unless klass.config[:attributes][:id]
             klass.attribute :id, :integer_id

lib/graphiti/resource/dsl.rb

@@ -32,7 +32,8 @@ def filter(name, *args, &blk)
               dependencies: opts[:dependent],
               required: required,
               operators: operators.to_hash,
-              allow_nil: opts.fetch(:allow_nil, filters_accept_nil_by_default)
+              allow_nil: opts.fetch(:allow_nil, filters_accept_nil_by_default),
+              deny_empty: opts.fetch(:deny_empty, filters_deny_empty_by_default)
             }
           elsif (type = args[0])
             attribute name, type, only: [:filterable], allow: opts[:allow]

lib/graphiti/scoping/filter.rb

@@ -54,6 +54,8 @@ def each_filter
           unless type[:canonical_name] == :hash || !value.is_a?(String)
             value = parse_string_value(filter.values[0], value)
           end
+
+          check_deny_empty_filters!(resource, filter, value)
           value = parse_string_null(filter.values[0], value)
           validate_singular(resource, filter, value)
           value = coerce_types(filter.values[0], param_name.to_sym, value)
@@ -200,5 +202,13 @@ def parse_string_null(filter, value)
 
       value
     end
+
+    def check_deny_empty_filters!(resource, filter, value)
+      return unless filter.values[0][:deny_empty]
+
+      if value.nil? || value.empty? || value == "null"
+        raise Errors::InvalidFilterValue.new(resource, filter, "(empty)")
+      end
+    end
   end
 end

spec/filtering_spec.rb

@@ -313,6 +313,38 @@ def self.name
     end
   end
 
+  context "when passed an empty value when deny_empty is true" do
+    before do
+      resource.filter :first_name, deny_empty: true
+      employee2.update_attributes(first_name: value)
+      params[:filter] = {first_name: "null"}
+    end
+
+    context 'via explicit string value "null"' do
+      let(:value) { "null" }
+
+      it "raises an invalid filter value error " do
+        expect { records.map(&:id) }.to raise_error(Graphiti::Errors::InvalidFilterValue)
+      end
+    end
+
+    context "via empty value" do
+      let(:value) { "" }
+
+      it "raises an invalid filter value error " do
+        expect { records.map(&:id) }.to raise_error(Graphiti::Errors::InvalidFilterValue)
+      end
+    end
+
+    context "via empty array" do
+      let(:value) { "[]" }
+
+      it "raises an invalid filter value error " do
+        expect { records.map(&:id) }.to raise_error(Graphiti::Errors::InvalidFilterValue)
+      end
+    end
+  end
+
   context "when passed comma, but filter marked single: true" do
     before do
       resource.filter :first_name, single: true

spec/resource_spec.rb

@@ -24,6 +24,8 @@
         expect(klass.attributes_schema_by_default).to eq(true)
         expect(klass.relationships_readable_by_default).to eq(true)
         expect(klass.relationships_writable_by_default).to eq(true)
+        expect(klass.filters_accept_nil_by_default).to eq(false)
+        expect(klass.filters_deny_empty_by_default).to eq(false)
       end
 
       it "does not have serializer, type, or model" do
@@ -68,6 +70,7 @@
         expect(klass.relationships_readable_by_default).to eq(true)
         expect(klass.relationships_writable_by_default).to eq(true)
         expect(klass.filters_accept_nil_by_default).to eq(false)
+        expect(klass.filters_deny_empty_by_default).to eq(false)
       end
 
       context "when rails" do
@@ -153,7 +156,8 @@ def self.name
             self.attributes_schema_by_default = false
             self.relationships_readable_by_default = false
             self.relationships_writable_by_default = false
-            self.filters_accept_nil_by_default = false
+            self.filters_accept_nil_by_default = true
+            self.filters_deny_empty_by_default = true
           end
         end
 
@@ -168,7 +172,8 @@ def self.name
           expect(klass.attributes_schema_by_default).to eq(false)
           expect(klass.relationships_readable_by_default).to eq(false)
           expect(klass.relationships_writable_by_default).to eq(false)
-          expect(klass.filters_accept_nil_by_default).to eq(false)
+          expect(klass.filters_accept_nil_by_default).to eq(true)
+          expect(klass.filters_deny_empty_by_default).to eq(true)
         end
       end
 
@@ -310,7 +315,8 @@ class TestResourceOverrideSerializer < PORO::ApplicationSerializer
             self.attributes_schema_by_default = false
             self.relationships_readable_by_default = false
             self.relationships_writable_by_default = false
-            self.filters_accept_nil_by_default = false
+            self.filters_accept_nil_by_default = true
+            self.filters_deny_empty_by_default = true
           end
         end
 
@@ -325,7 +331,8 @@ class TestResourceOverrideSerializer < PORO::ApplicationSerializer
           expect(klass2.attributes_schema_by_default).to eq(false)
           expect(klass2.relationships_readable_by_default).to eq(false)
           expect(klass2.relationships_writable_by_default).to eq(false)
-          expect(klass2.filters_accept_nil_by_default).to eq(false)
+          expect(klass2.filters_accept_nil_by_default).to eq(true)
+          expect(klass2.filters_deny_empty_by_default).to eq(true)
         end
       end