Allow filter conditionals to be defined as symbols too

lib/rackstash/filter.rb

@@ -67,17 +67,21 @@ module Rackstash
       #   first use the filter {.registry} to find the matching class. With that
       #   we then create a filter object as before. When giving an object which
       #   responds to `call` already (e.g. a `Proc`) we return it unchanged,
-      #   ignoring any additional passed `args`.
-      # @param only_if [#call, nil] An optional condition defining whether the
-      #   filter should be applied, usually given as a `Proc` object. Before
-      #   evaluating the newly created filter object, we first call the given
-      #   proc with the event as its argument. The filter is applied only if the
-      #   proc returns a truethy value.
-      # @param not_if [#call, nil] An optional condition defining whether the
-      #   filter should not be applied, usually given as a `Proc` object. Before
-      #   evaluating the newly created filter object, we first call the given
-      #   proc with the event as its argument. The filter is not applied if the
-      #   proc returns a truethy value.
+      #   ignoring any additional passed `args`, `kwargs`, or condition.
+      # @param only_if [#call, Symbol, nil] An optional condition defining
+      #   whether the filter should be applied,  defined as a `Proc` (or another
+      #   object responding to `call`) or a Symbol. Before evaluating the newly
+      #   created filter object, we first call the given proc or we call the
+      #   method identified by the given Symbol on the filter object, each time
+      #   giving the `event` Hash as its argument. The filter is only applied
+      #   if the proc or filter method returns a truethy value.
+      # @param not_if [#call, Symbol, nil] An optional condition defining
+      #   whether the filter should not be applied, defined as a `Proc` (or
+      #   another object responding to `call`) or a Symbol. Before evaluating
+      #   the newly created filter object, we first call the given proc or we
+      #   call the method identified by the given Symbol on the filter object,
+      #   each time giving the `event` Hash as its argument. The filter is not
+      #   applied if the proc or filter method returns a truethy value.
       # @param args [Array] an optional list of arguments which is passed to the
       #   initializer for the new filter object.
       # @param kwargs [Hash] an optional list of keyword arguments which are
@@ -88,30 +92,49 @@ module Rackstash
       #   for the specified class name
       # @return [Object] a new filter object
       def build(filter_spec, *args, only_if: nil, not_if: nil, **kwargs, &block)
-        case filter_spec
-        when ->(filter) { filter.respond_to?(:call) }
-          filter_spec
-        else
-          args << kwargs unless kwargs.empty?
+        # TODO: warn if args, kwargs, only_if, not_if were given here
+        #       since they are ignored
+        return filter_spec if filter_spec.respond_to?(:call)
 
-          filter = registry.fetch(filter_spec).new(*args, &block)
-          conditional_filter(filter, only_if: only_if, not_if: not_if)
-        end
+        filter_class = registry.fetch(filter_spec)
+        filter = filter_class.new(*args, **kwargs, &block)
+
+        conditional_filter(filter, only_if: only_if, not_if: not_if)
       end
 
       private
 
       def conditional_filter(filter, only_if: nil, not_if: nil)
-        return filter if only_if.nil? && not_if.nil?
+        if only_if.nil?
+          # Empty conditional, do nothing
+        elsif only_if.is_a?(Symbol)
+          apply_condition(filter) { |event| filter.send(only_if, event) }
+        elsif only_if.respond_to?(:call)
+          apply_condition(filter) { |event| only_if.call(event) }
+        else
+          raise TypeError, 'Invalid only_if filter'
+        end
 
-        conditional = Module.new do
+        if not_if.nil?
+          # Empty conditional, do nothing
+        elsif not_if.is_a?(Symbol)
+          apply_condition(filter) { |event| !filter.send(not_if, event) }
+        elsif not_if.respond_to?(:call)
+          apply_condition(filter) { |event| !not_if.call(event) }
+        else
+          raise TypeError, 'Invalid not_if filter'
+        end
+
+        filter
+      end
+
+      def apply_condition(filter)
+        mod = Module.new do
           define_method(:call) do |event|
-            return event if only_if && !only_if.call(event)
-            return event if not_if && not_if.call(event)
-            super(event)
+            yield(event) ? super(event) : event
           end
         end
-        filter.extend(conditional)
+        filter.extend(mod)
       end
     end
   end

spec/rackstash/filter_spec.rb

@@ -11,42 +11,83 @@ require 'securerandom'
 require 'rackstash/filter'
 
 RSpec.describe Rackstash::Filter do
-  let(:registry) { Rackstash::ClassRegistry.new('filter') }
-
   let(:filter_class) {
     Class.new do
+      attr_reader :args, :kwargs
+
+      def initialize(*args, **kwargs)
+        @args = args
+        @kwargs = kwargs
+
+        @called = false
+        @called_with_if = false
+        @called_with_unless = false
+      end
+
+      def if(event)
+        @called_with_if = true
+        @kwargs[:if]
+      end
+
+      def unless(event)
+        @called_with_unless = true
+        @kwargs[:unless]
+      end
+
       def call(_event)
+        @called = true
         'filtered'
       end
+
+      %i[called called_with_if called_with_unless].each do |m|
+        define_method(:"#{m}?") { instance_variable_get("@#{m}") }
+      end
     end
   }
   let(:filter_name) { :"filter_class_#{SecureRandom.hex(6)}" }
 
-  describe '.build' do
-    before do
-      allow(described_class).to receive(:registry).and_return(registry)
-      described_class.register(filter_class, filter_name)
+  around(:each) do |example|
+    original_filters = described_class.registry.to_h
+    described_class.registry.clear
+    example.run
+    original_filters.each do |name, registered_clas|
+      described_class.registry[name] = registered_clas
     end
+  end
 
+  before(:each) do
+    described_class.register(filter_class, filter_name)
+  end
+
+  describe '.build' do
     it 'builds a filter from a class' do
-      args = ['arg1', foo: 'bar']
-      expect(filter_class).to receive(:new).with(*args)
+      args = ['arg1']
+      kwargs = { foo: 'bar' }
 
-      described_class.build(filter_class, *args)
+      filter = described_class.build(filter_class, *args, **kwargs)
+      expect(filter).to be_a filter_class
+      expect(filter.args).to eq args
+      expect(filter.kwargs).to eq kwargs
     end
 
     it 'builds a filter from a Symbol' do
-      args = ['arg1', foo: 'bar']
-      expect(filter_class).to receive(:new).with(*args)
+      args = ['arg1']
+      kwargs = { foo: 'bar' }
 
-      described_class.build(filter_name.to_sym, *args)
+      filter = described_class.build(filter_name.to_sym, *args, **kwargs)
+      expect(filter).to be_a filter_class
+      expect(filter.args).to eq args
+      expect(filter.kwargs).to eq kwargs
     end
 
     it 'builds a filter from a String' do
-      args = ['arg1', foo: 'bar']
-      expect(filter_class).to receive(:new).with(*args)
+      args = ['arg1']
+      kwargs = { foo: 'bar' }
 
-      described_class.build(filter_name.to_s, *args)
+      filter = described_class.build(filter_name.to_s, *args, **kwargs)
+      expect(filter).to be_a filter_class
+      expect(filter.args).to eq args
+      expect(filter.kwargs).to eq kwargs
     end
 
     it 'returns an existing filter' do
@@ -60,30 +101,60 @@ RSpec.describe Rackstash::Filter do
       let(:event) { Object.new }
 
       it 'applies the only_if conditional for new filters' do
-        only_if = -> {}
+        only_if = ->(_event) { false }
         filter = described_class.build(filter_name, only_if: only_if)
+        expect(filter.call(event)).to equal event
+        expect(filter).not_to be_called
 
-        expect(only_if).to receive(:call).and_return false
-        expect { filter.call({}) }.not_to raise_error
+        only_if = ->(_event) { true }
+        filter = described_class.build(filter_name, only_if: only_if)
+        expect(filter.call(event)).to eql 'filtered'
+        expect(filter).to be_called
+      end
+
+      it 'applies the only_if filter conditional for new filters' do
+        filter = described_class.build(filter_name, only_if: :if, if: false)
+        filter.call(event)
+        expect(filter).to be_called_with_if
+        expect(filter).not_to be_called
+
+        filter = described_class.build(filter_name, only_if: :if, if: true)
+        expect(filter.call(event)).to eql 'filtered'
+        expect(filter).to be_called
+        expect(filter).to be_called_with_if
       end
 
       it 'applies the not_if conditional for new filters' do
-        not_if = -> {}
+        not_if = ->(_event) { true }
         filter = described_class.build(filter_name, not_if: not_if)
-
-        expect(not_if).to receive(:call).and_return true
         expect(filter.call(event)).to equal event
+        expect(filter).not_to be_called
+
+        not_if = ->(_event) { false }
+        filter = described_class.build(filter_name, not_if: not_if)
+        expect(filter.call(event)).to eql 'filtered'
+        expect(filter).to be_called
+      end
+
+      it 'applies the not_if filter conditional for new filters' do
+        filter = described_class.build(filter_name, not_if: :unless, unless: true)
+        expect(filter.call(event)).to equal event
+        expect(filter).not_to be_called
+        expect(filter).to be_called_with_unless
+
+        filter = described_class.build(filter_name, not_if: :unless, unless: false)
+        expect(filter.call(event)).to eql 'filtered'
+        expect(filter).to be_called
+        expect(filter).to be_called_with_unless
       end
 
       it 'applies both conditionals for new filters' do
-        only_if = -> {}
-        not_if = -> {}
-
+        only_if = ->(_event) { true }
+        not_if = ->(_event) { false }
         filter = described_class.build(filter_name, only_if: only_if, not_if: not_if)
 
-        expect(only_if).to receive(:call).and_return true
-        expect(not_if).to receive(:call).and_return false
         expect(filter.call(event)).to eql 'filtered'
+        expect(filter).to be_called
       end
 
       it 'keeps the class hierarchy unchanged' do
@@ -94,7 +165,7 @@ RSpec.describe Rackstash::Filter do
 
       it 'ignores the conditional for existing filters' do
         filter = filter_class.new
-        only_if = -> {}
+        only_if = ->(_event) {}
 
         expect(described_class.build(filter, only_if: only_if))
           .to equal filter
@@ -105,31 +176,27 @@ RSpec.describe Rackstash::Filter do
       end
 
       it 'passes keyword arguments to the initializer' do
-        filter_class.class_eval do
-          def initialize(mandatory:)
-            @mandatory = mandatory
-          end
+        filter = described_class.build(filter_name, 'foo', only_if: -> {}, argument: 'bar')
+        expect(filter.args).to eq ['foo']
+        expect(filter.kwargs).to eq argument: 'bar'
+      end
 
-          attr_reader :mandatory
-        end
+      it 'expects callable objects' do
+        object = false
 
-        filter = described_class.build(filter_name, only_if: -> {}, mandatory: 'foo')
-        expect(filter.mandatory).to eql 'foo'
+        expect { described_class.build(filter_name, only_if: object) }
+          .to raise_error(TypeError, 'Invalid only_if filter')
+        expect { described_class.build(filter_name, not_if: object) }
+          .to raise_error(TypeError, 'Invalid not_if filter')
       end
     end
 
     context 'without conditionals' do
-      it 'passes keyword arguments to the initializer' do
-        filter_class.class_eval do
-          def initialize(mandatory:)
-            @mandatory = mandatory
-          end
+      it 'passes arguments to the initializer' do
+        filter = described_class.build(filter_name, 'foo', argument: 'bar')
 
-          attr_reader :mandatory
-        end
-
-        filter = described_class.build(filter_name, mandatory: 'foo')
-        expect(filter.mandatory).to eql 'foo'
+        expect(filter.args).to eq ['foo']
+        expect(filter.kwargs).to eq argument: 'bar'
       end
     end
 
@@ -158,29 +225,17 @@ RSpec.describe Rackstash::Filter do
   end
 
   describe '.register' do
-    let(:filter_class) {
-      Class.new do
-        def call
-        end
-      end
-    }
-
     it 'registers a filter class' do
-      expect(described_class.registry).to receive(:[]=).with(:foo, filter_class).ordered
-      expect(described_class.registry).to receive(:[]=).with(:bar, filter_class).ordered
-
       described_class.register(filter_class, :foo, :bar)
+      expect(described_class.registry.fetch(:foo)).to equal filter_class
+      expect(described_class.registry.fetch(:bar)).to equal filter_class
     end
 
     it 'rejects invalid classes' do
-      expect(described_class.registry).not_to receive(:[]=)
-
       expect { described_class.register(:not_a_class, :foo) }.to raise_error TypeError
       expect { described_class.register(Class.new, :foo) }.to raise_error TypeError
-    end
 
-    it 'rejects invalid names' do
-      expect { described_class.register(filter_class, 123) }.to raise_error TypeError
+      expect(described_class.registry[:foo]).to be_nil
     end
   end
 end