Fixes: active_job/railtie, re-enqueue webhook that errored out (#18)

Two small improvements to Munster:

- Webhook processor should require `active_job/railtie`, not just
`active_job`. It require GlobalID to work and that get's required only
with railtie.
- We're adding a state transition from `error` to `received`. System
will enqueue webhook processing, if this transition was executed.

lib/munster/base_handler.rb

@@ -16,6 +16,16 @@ def handle(action_dispatch_request)
       webhook = Munster::ReceivedWebhook.new(request: action_dispatch_request, handler_event_id: handler_event_id, handler_module_name: handler_module_name)
       webhook.save!
 
+      enqueue(webhook)
+    rescue ActiveRecord::RecordNotUnique # Webhook deduplicated
+      Rails.logger.info { "#{inspect} Webhook #{handler_event_id} is a duplicate delivery and will not be stored." }
+    end
+
+    # Enqueues the processing job to process webhook asynchronously. The job class could be configured.
+    #
+    # @param webhook [Munster::ReceivedWebhook]
+    # @return [void]
+    def enqueue(webhook)
       # The configured job class can be a class name or a module, to support lazy loading
       job_class_or_module_name = Munster.configuration.processing_job_class
       job_class = if job_class_or_module_name.respond_to?(:perform_later)
@@ -25,8 +35,6 @@ def handle(action_dispatch_request)
       end
 
       job_class.perform_later(webhook)
-    rescue ActiveRecord::RecordNotUnique # Webhook deduplicated
-      Rails.logger.info { "#{inspect} Webhook #{handler_event_id} is a duplicate delivery and will not be stored." }
     end
 
     # This is the heart of your webhook processing. Override this method and define your processing inside of it.

lib/munster/jobs/processing_job.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require "active_job" if defined?(Rails)
+require "active_job/railtie"
 
 module Munster
   class ProcessingJob < ActiveJob::Base

lib/munster/models/received_webhook.rb

@@ -15,6 +15,11 @@ class ReceivedWebhook < ActiveRecord::Base
       s.permit_transition(:processing, :skipped)
       s.permit_transition(:processing, :processed)
       s.permit_transition(:processing, :error)
+      s.permit_transition(:error, :received)
+
+      s.after_committed_transition_to(:received) do |webhook|
+        webhook.handler.enqueue(webhook)
+      end
     end
 
     # Store the pertinent data from an ActionDispatch::Request into the webhook.

test/munster_test.rb

@@ -4,6 +4,8 @@
 require_relative "test_app"
 
 class TestMunster < ActionDispatch::IntegrationTest
+  teardown { Munster::ReceivedWebhook.delete_all }
+
   def test_that_it_has_a_version_number
     refute_nil ::Munster::VERSION
   end
@@ -37,9 +39,18 @@ def self.xtest(msg)
     test(msg) { skip }
   end
 
-  test "accepts a webhook, stores and processes it" do
-    Munster::ReceivedWebhook.delete_all
+  test "ensure webhook is processed only once during creation" do
+    tf = Tempfile.new
+    body = {isValid: true, outputToFilename: tf.path}
+    body_json = body.to_json
 
+    assert_enqueued_jobs 1, only: Munster::ProcessingJob do
+      post "/munster/test", params: body_json, headers: {"CONTENT_TYPE" => "application/json"}
+      assert_response 200
+    end
+  end
+
+  test "accepts a webhook, stores and processes it" do
     tf = Tempfile.new
     body = {isValid: true, outputToFilename: tf.path}
     body_json = body.to_json
@@ -61,8 +72,6 @@ def self.xtest(msg)
   end
 
   test "accepts a webhook but does not process it if it is invalid" do
-    Munster::ReceivedWebhook.delete_all
-
     tf = Tempfile.new
     body = {isValid: false, outputToFilename: tf.path}
     body_json = body.to_json
@@ -85,8 +94,6 @@ def self.xtest(msg)
   end
 
   test "marks a webhook as errored if it raises during processing" do
-    Munster::ReceivedWebhook.delete_all
-
     tf = Tempfile.new
     body = {isValid: true, raiseDuringProcessing: true, outputToFilename: tf.path}
     body_json = body.to_json
@@ -109,8 +116,6 @@ def self.xtest(msg)
   end
 
   test "does not accept a test payload that is larger than the configured maximum size" do
-    Munster::ReceivedWebhook.delete_all
-
     oversize = Munster.configuration.request_body_size_limit + 1
     utf8_junk = Base64.strict_encode64(Random.bytes(oversize))
     body = {isValid: true, filler: utf8_junk, raiseDuringProcessing: false, outputToFilename: "/tmp/nothing"}
@@ -121,8 +126,6 @@ def self.xtest(msg)
   end
 
   test "does not try to process a webhook if it is not in `received' state" do
-    Munster::ReceivedWebhook.delete_all
-
     tf = Tempfile.new
     body = {isValid: true, raiseDuringProcessing: true, outputToFilename: tf.path}
     body_json = body.to_json
@@ -193,4 +196,19 @@ def self.xtest(msg)
     assert_equal 14, received_webhook.request.params["number_of_dependents"]
     assert_equal "123", received_webhook.request.params["user_id"]
   end
+
+  test "erroneous webhook could be processed again" do
+    webhook = Munster::ReceivedWebhook.create(
+      handler_event_id: "test",
+      handler_module_name: "WebhookTestHandler",
+      status: "error",
+      body: {isValid: true}.to_json
+    )
+
+    assert_enqueued_jobs 1, only: Munster::ProcessingJob do
+      webhook.received!
+
+      assert_equal "received", webhook.status
+    end
+  end
 end

test/test_app.rb

@@ -3,7 +3,6 @@
 require "active_record"
 require "action_pack"
 require "action_controller"
-require "active_job/railtie"
 require "rails"
 
 database = "development.sqlite3"