Merge pull request #3455 from huginn/liquid_output_agent_supports_etag

LiquidOutputAgent supports ETag

app/models/agents/liquid_output_agent.rb

@@ -102,6 +102,8 @@ module Agents
     form_configurable :mode, type: :array, values: ['Last event in', 'Merge events', 'Last X events']
     form_configurable :event_limit
 
+    before_save :update_last_modified_at, if: :options_changed?
+
     def working?
       last_receive_at && last_receive_at > options['expected_receive_period_in_days'].to_i.days.ago && !recent_error_logs?
     end
@@ -155,13 +157,18 @@ module Agents
             event.payload
           end
       end
+      update_last_modified_at
     end
 
-    def receive_web_request(params, method, format)
-      if valid_authentication?(params)
-        [liquified_content, 200, mime_type, interpolated['response_headers'].presence]
+    def receive_web_request(request)
+      if valid_authentication?(request.params)
+        if request.headers['If-None-Match'].presence&.include?(etag)
+          [nil, 304, {}]
+        else
+          [liquified_content, 200, mime_type, response_headers]
+        end
       else
-        [unauthorized_content(format), 401]
+        [unauthorized_content(request.format.to_s), 401]
       end
     end
 
@@ -204,13 +211,39 @@ module Agents
       end
     end
 
+    public def etag
+      memory['etag'] || '"0.000000000"'
+    end
+
+    def last_modified_at
+      memory['last_modified_at']&.to_time || Time.at(0)
+    end
+
+    def last_modified_at=(time)
+      memory['last_modified_at'] = time.iso8601(9)
+      memory['etag'] = time.strftime('"%s.%9N"')
+    end
+
+    def update_last_modified_at
+      self.last_modified_at = Time.now
+    end
+
+    def max_age
+      options['expected_receive_period_in_days'].to_i * 86400
+    end
+
+    def response_headers
+      {
+        'Last-Modified' => last_modified_at.httpdate,
+        'ETag' => etag,
+        'Cache-Control' => "max-age=#{max_age}",
+      }.update(interpolated['response_headers'].presence || {})
+    end
+
     def count_limit
-      limit = begin
-        Integer(options['event_limit'])
-      rescue StandardError
-        1000
-      end
-      limit <= 1000 ? limit : 1000
+      [Integer(options['event_limit']), 1000].min
+    rescue StandardError
+      1000
     end
 
     def date_limit

spec/models/agents/liquid_output_agent_spec.rb

@@ -4,15 +4,19 @@ require 'rails_helper'
 
 describe Agents::LiquidOutputAgent do
   let(:agent) do
-    _agent = Agents::LiquidOutputAgent.new(:name => 'My Data Output Agent')
-    _agent.options = _agent.default_options.merge('secret' => 'secret1', 'events_to_show' => 3)
-    _agent.options['secret'] = "a secret"
+    _agent = Agents::LiquidOutputAgent.new(name: 'My Data Output Agent')
+    _agent.options = _agent.default_options.merge(
+      'secret' => 'a secret1',
+      'events_to_show' => 3,
+    )
     _agent.user = users(:bob)
     _agent.sources << agents(:bob_website_agent)
     _agent.save!
     _agent
   end
 
+  let(:event_struct) { Struct.new(:payload) }
+
   describe "#working?" do
     it "checks if events have been received within expected receive period" do
       expect(agent).not_to be_working
@@ -86,19 +90,17 @@ describe Agents::LiquidOutputAgent do
   end
 
   describe "#receive?" do
-
     let(:key)   { SecureRandom.uuid }
     let(:value) { SecureRandom.uuid }
 
     let(:incoming_events) do
       last_payload = { key => value }
-      [Struct.new(:payload).new( { key => SecureRandom.uuid } ),
-       Struct.new(:payload).new( { key => SecureRandom.uuid } ),
-       Struct.new(:payload).new(last_payload)]
+      [event_struct.new( { key => SecureRandom.uuid } ),
+       event_struct.new( { key => SecureRandom.uuid } ),
+       event_struct.new(last_payload)]
     end
 
     describe "and the mode is last event in" do
-
       before { agent.options['mode'] = 'Last event in' }
 
       it "stores the last event in memory" do
@@ -114,11 +116,9 @@ describe Agents::LiquidOutputAgent do
           expect(agent.memory['last_event'][key]).to equal(value)
         end
       end
-
     end
 
     describe "but the mode is merge" do
-
       let(:second_key)   { SecureRandom.uuid }
       let(:second_value) { SecureRandom.uuid }
 
@@ -126,8 +126,8 @@ describe Agents::LiquidOutputAgent do
 
       let(:incoming_events) do
         last_payload = { key => value }
-        [Struct.new(:payload).new( { key => SecureRandom.uuid, second_key => second_value } ),
-         Struct.new(:payload).new(last_payload)]
+        [event_struct.new( { key => SecureRandom.uuid, second_key => second_value } ),
+         event_struct.new(last_payload)]
       end
 
       it "should merge all of the events passed to it" do
@@ -137,7 +137,6 @@ describe Agents::LiquidOutputAgent do
       end
 
       describe "but the casing on the mode is wrong" do
-
         before { agent.options['mode'] = 'MERGE EVENTS' }
 
         it "should merge all of the events passed to it" do
@@ -145,27 +144,23 @@ describe Agents::LiquidOutputAgent do
           expect(agent.memory['last_event'][key]).to equal(value)
           expect(agent.memory['last_event'][second_key]).to equal(second_value)
         end
-
       end
-
     end
 
     describe "but the mode is anything else" do
-
       before { agent.options['mode'] = SecureRandom.uuid }
 
       let(:incoming_events) do
         last_payload = { key => value }
-        [Struct.new(:payload).new(last_payload)]
+        [event_struct.new(last_payload)]
       end
 
-      it "should do nothing" do
-        agent.receive incoming_events
-        expect(agent.memory.keys.count).to equal(0)
+      it "should update nothing" do
+        expect {
+          agent.receive incoming_events
+        }.not_to change { agent.reload.memory&.fetch("last_event", nil) }
       end
-
     end
-
   end
 
   describe "#count_limit" do
@@ -209,13 +204,19 @@ describe Agents::LiquidOutputAgent do
   end
 
   describe "#receive_web_request?" do
-
     let(:secret) { SecureRandom.uuid }
 
+    let(:headers) { {} }
     let(:params) { { 'secret' => secret } }
-
-    let(:method) { nil }
-    let(:format) { nil }
+    let(:format) { :html }
+    let(:request) {
+      instance_double(
+        ActionDispatch::Request,
+        headers:,
+        params:,
+        format:,
+      )
+    }
 
     let(:mime_type) { SecureRandom.uuid }
     let(:content) { "The key is {{#{key}}}." }
@@ -228,27 +229,107 @@ describe Agents::LiquidOutputAgent do
       agent.options['mime_type'] = mime_type
       agent.options['content'] = content
       agent.memory['last_event'] = { key => value }
+      agent.save!
       agents(:bob_website_agent).events.destroy_all
     end
 
     it 'should respond with custom response header if configured with `response_headers` option' do
-      agent.options['response_headers'] = {"X-My-Custom-Header" => 'hello'}
-      result = agent.receive_web_request params, method, format
-      expect(result).to eq(["The key is #{value}.", 200, mime_type, {"X-My-Custom-Header" => "hello"}])
+      agent.options['response_headers'] = { "X-My-Custom-Header" => 'hello' }
+      result = agent.receive_web_request request
+      expect(result).to match([
+        "The key is #{value}.",
+        200,
+        mime_type,
+        a_hash_including(
+          "Cache-Control" => a_kind_of(String),
+          "ETag" => a_kind_of(String),
+          "Last-Modified" => a_kind_of(String),
+          "X-My-Custom-Header" => "hello"
+        )
+      ])
     end
 
     it 'should allow the usage custom liquid tags' do
       agent.options['content'] = "{% credential aws_secret %}"
-      result = agent.receive_web_request params, method, format
-      expect(result).to eq(["1111111111-bob", 200, mime_type, nil])
+      result = agent.receive_web_request request
+      expect(result).to match([
+        "1111111111-bob",
+        200,
+        mime_type,
+        a_hash_including(
+          "Cache-Control" => a_kind_of(String),
+          "ETag" => a_kind_of(String),
+          "Last-Modified" => a_kind_of(String),
+        )
+      ])
     end
 
-    describe "and the mode is last event in" do
+    describe "when requested with or without the If-None-Match header" do
+      let(:now) { Time.now }
+
+      it "should conditionally return 304 responses whenever ETag matches" do
+        travel_to now
+
+        allow(agent).to receive(:liquified_content).and_call_original
+
+        result = agent.receive_web_request request
+        expect(result).to eq([
+          "The key is #{value}.",
+          200,
+          mime_type,
+          {
+            'Cache-Control' => "max-age=#{agent.options['expected_receive_period_in_days'].to_i * 86400}",
+            'ETag' => agent.etag,
+            'Last-Modified' => agent.memory['last_modified_at'].to_time.httpdate,
+          }
+        ])
+        expect(agent).to have_received(:liquified_content).once
+
+        travel_to now + 1
+        request.headers['If-None-Match'] = agent.etag
+        result = agent.receive_web_request request
+        expect(result).to eq([nil, 304, {}])
+        expect(agent).to have_received(:liquified_content).once
+
+        # Receiving an event will update the ETag and Last-Modified-At
+        event = agents(:bob_website_agent).events.create!(payload: { key => 'latest' })
+        AgentReceiveJob.perform_now agent.id, [event.id]
+        agent.reload
+
+        travel_to now + 2
+        result = agent.receive_web_request request
+        expect(result).to eq(["The key is latest.", 200, mime_type, {
+          'Cache-Control' => "max-age=#{agent.options['expected_receive_period_in_days'].to_i * 86400}",
+          'ETag' => agent.etag,
+          'Last-Modified' => agent.last_receive_at.httpdate,
+        }])
+        expect(agent).to have_received(:liquified_content).twice
+
+        travel_to now + 3
+        request.headers['If-None-Match'] = agent.etag
+        result = agent.receive_web_request request
+        expect(result).to eq([nil, 304, {}])
+        expect(agent).to have_received(:liquified_content).twice
+
+        # Changing options will update the ETag and Last-Modified-At
+        agent.update!(options: agent.options.merge('content' => "The key is now {{#{key}}}."))
+        agent.reload
+
+        result = agent.receive_web_request request
+        expect(result).to eq(["The key is now latest.", 200, mime_type, {
+          'Cache-Control' => "max-age=#{agent.options['expected_receive_period_in_days'].to_i * 86400}",
+          'ETag' => agent.etag,
+          'Last-Modified' => (now + 3).httpdate,
+        }])
+        expect(agent).to have_received(:liquified_content).exactly(3).times
+      end
+    end
 
+    describe "and the mode is last event in" do
       before { agent.options['mode'] = 'Last event in' }
 
       it "should render the results as a liquid template from the last event in" do
-        result = agent.receive_web_request params, method, format
+        result = agent.receive_web_request request
 
         expect(result[0]).to eq("The key is #{value}.")
         expect(result[1]).to eq(200)
@@ -259,32 +340,24 @@ describe Agents::LiquidOutputAgent do
         before { agent.options['mode'] = 'last event in' }
 
         it "should render the results as a liquid template from the last event in" do
-          result = agent.receive_web_request params, method, format
+          result = agent.receive_web_request request
 
-          expect(result[0]).to eq("The key is #{value}.")
-          expect(result[1]).to eq(200)
-          expect(result[2]).to eq(mime_type)
+          expect(result).to match(["The key is #{value}.", 200, mime_type, a_kind_of(Hash)])
         end
       end
-
     end
 
     describe "and the mode is merge events" do
-
       before { agent.options['mode'] = 'Merge events' }
 
       it "should render the results as a liquid template from the last event in" do
-        result = agent.receive_web_request params, method, format
+        result = agent.receive_web_request request
 
-        expect(result[0]).to eq("The key is #{value}.")
-        expect(result[1]).to eq(200)
-        expect(result[2]).to eq(mime_type)
+        expect(result).to match(["The key is #{value}.", 200, mime_type, a_kind_of(Hash)])
       end
-
     end
 
     describe "and the mode is last X events" do
-
       before do
         agent.options['mode'] = 'Last X events'
 
@@ -315,7 +388,7 @@ EOF
 
       it "should render the results as a liquid template from the last event in, limiting to 2" do
         agent.options['event_limit'] = 2
-        result = agent.receive_web_request params, method, format
+        result = agent.receive_web_request request
 
         expect(result[0]).to eq <<EOF
 <table>
@@ -336,7 +409,7 @@ EOF
 
       it "should render the results as a liquid template from the last event in, limiting to 1" do
         agent.options['event_limit'] = 1
-        result = agent.receive_web_request params, method, format
+        result = agent.receive_web_request request
 
         expect(result[0]).to eq <<EOF
 <table>
@@ -352,7 +425,7 @@ EOF
 
       it "should render the results as a liquid template from the last event in, allowing no limit" do
         agent.options['event_limit'] = ''
-        result = agent.receive_web_request params, method, format
+        result = agent.receive_web_request request
 
         expect(result[0]).to eq <<EOF
 <table>
@@ -377,13 +450,12 @@ EOF
       end
 
       it "should allow the limiting by time, as well" do
-
         one_event = agent.received_events.select { |x| x.payload['name'] == 'John Galt' }.first
         one_event.created_at = 2.days.ago
         one_event.save!
 
         agent.options['event_limit'] = '1 day'
-        result = agent.receive_web_request params, method, format
+        result = agent.receive_web_request request
 
         expect(result[0]).to eq <<EOF
 <table>
@@ -409,7 +481,7 @@ EOF
         one_event.save!
 
         agent.options['event_limit'] = '1 DaY'
-        result = agent.receive_web_request params, method, format
+        result = agent.receive_web_request request
 
         expect(result[0]).to eq <<EOF
 <table>
@@ -430,14 +502,14 @@ EOF
 
       it "it should continue to work when the event limit is wrong" do
         agent.options['event_limit'] = 'five days'
-        result = agent.receive_web_request params, method, format
+        result = agent.receive_web_request request
 
         expect(result[0]).to include("Howard Roark")
         expect(result[0]).to include("Dagny Taggart")
         expect(result[0]).to include("John Galt")
 
         agent.options['event_limit'] = '5 quibblequarks'
-        result = agent.receive_web_request params, method, format
+        result = agent.receive_web_request request
 
         expect(result[0]).to include("Howard Roark")
         expect(result[0]).to include("Dagny Taggart")
@@ -445,35 +517,36 @@ EOF
       end
 
       describe "but the mode was set to last X events with the wrong casing" do
-
         before { agent.options['mode'] = 'LAST X EVENTS' }
 
         it "should still work as last x events" do
-          result = agent.receive_web_request params, method, format
+          result = agent.receive_web_request request
           expect(result[0]).to include("Howard Roark")
           expect(result[0]).to include("Dagny Taggart")
           expect(result[0]).to include("John Galt")
         end
-
       end
-
     end
 
     describe "but the secret provided does not match" do
       before { params['secret'] = SecureRandom.uuid }
 
       it "should return a 401 response" do
-        result = agent.receive_web_request params, method, format
+        result = agent.receive_web_request request
 
         expect(result[0]).to eq("Not Authorized")
         expect(result[1]).to eq(401)
       end
 
-      it "should return a 401 json response if the format is json" do
-        result = agent.receive_web_request params, method, 'json'
+      context 'if the format is json' do
+        let(:format) { :json }
 
-        expect(result[0][:error]).to eq("Not Authorized")
-        expect(result[1]).to eq(401)
+        it "should return a 401 json response " do
+          result = agent.receive_web_request request
+
+          expect(result[0][:error]).to eq("Not Authorized")
+          expect(result[1]).to eq(401)
+        end
       end
     end
   end