From 5821d2780579a35f06381abd1c08cc8f43c6e578 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Josef=20=C5=A0im=C3=A1nek?= <josef.simanek@gmail.com>
Date: Tue, 14 Nov 2023 00:19:05 +0100
Subject: [PATCH] Update with review suggestions.

---
 app/jobs/rstuf/check_job.rb       |  5 ++++-
 app/models/deletion.rb            |  4 ++--
 config/initializers/rstuf.rb      |  2 +-
 lib/rstuf/client.rb               |  1 +
 test/jobs/rstuf/check_job_test.rb | 12 +++++++++++-
 test/models/deletion_test.rb      |  2 +-
 test/models/pusher_test.rb        |  2 +-
 7 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/app/jobs/rstuf/check_job.rb b/app/jobs/rstuf/check_job.rb
index 5d58f2e4fa2..e410dd1c676 100644
--- a/app/jobs/rstuf/check_job.rb
+++ b/app/jobs/rstuf/check_job.rb
@@ -6,12 +6,15 @@ class Rstuf::CheckJob < Rstuf::ApplicationJob
   queue_with_priority PRIORITIES.fetch(:push)
 
   def perform(task_id)
-    case Rstuf::Client.task_status(task_id)
+    case status = Rstuf::Client.task_status(task_id)
     when "SUCCESS"
       # no-op, all good
     when "FAILURE"
       raise FailureException, "RSTUF job failed"
+    when "PENDING"
+      raise RetryException
     else
+      Rails.logger.info "RSTUF job returned unexpected state #{status}"
       raise RetryException
     end
   end
diff --git a/app/models/deletion.rb b/app/models/deletion.rb
index d1a28681c20..d5ad4474ef5 100644
--- a/app/models/deletion.rb
+++ b/app/models/deletion.rb
@@ -57,13 +57,13 @@ def expire_cache
   def remove_from_index
     version.update!(indexed: false, yanked_at: Time.now.utc)
     reindex
-    Rstuf::RemoveJob.perform_later(version: version)
+    Rstuf::RemoveJob.perform_later(version:)
   end
 
   def restore_to_index
     version.update!(indexed: true, yanked_at: nil, yanked_info_checksum: nil)
     reindex
-    Rstuf::AddJob.perform_later(version: version)
+    Rstuf::AddJob.perform_later(version:)
   end
 
   def reindex
diff --git a/config/initializers/rstuf.rb b/config/initializers/rstuf.rb
index 17037541bbb..768d2ccee9d 100644
--- a/config/initializers/rstuf.rb
+++ b/config/initializers/rstuf.rb
@@ -1,6 +1,6 @@
 require 'rstuf'
 
-if ENV['RSTUF_API_URL']
+if ENV['RSTUF_API_URL'].presence
   Rstuf.base_url = ENV['RSTUF_API_URL']
   Rstuf.enabled = true
   Rstuf.wait_for = 10.seconds
diff --git a/lib/rstuf/client.rb b/lib/rstuf/client.rb
index 3cb7e1657d8..01847e7324e 100644
--- a/lib/rstuf/client.rb
+++ b/lib/rstuf/client.rb
@@ -24,6 +24,7 @@ def self.connection
     Faraday.new(url: Rstuf.base_url) do |f|
       f.request :json
       f.response :json
+      f.response :logger, Rails.logger
     end
   end
 
diff --git a/test/jobs/rstuf/check_job_test.rb b/test/jobs/rstuf/check_job_test.rb
index dd019c771e4..d26f8ccbcf8 100644
--- a/test/jobs/rstuf/check_job_test.rb
+++ b/test/jobs/rstuf/check_job_test.rb
@@ -27,7 +27,7 @@ class Rstuf::CheckJobTest < ActiveJob::TestCase
     end
   end
 
-  test "perform raises a retry exception on retry state and retries" do
+  test "perform raises a retry exception on pending state and retries" do
     retry_response = { "data" => { "state" => "PENDING" } }
     stub_request(:get, "#{Rstuf.base_url}/api/v1/task/?task_id=#{@task_id}")
       .to_return(status: 200, body: retry_response.to_json, headers: { "Content-Type" => "application/json" })
@@ -37,6 +37,16 @@ class Rstuf::CheckJobTest < ActiveJob::TestCase
     end
   end
 
+  test "perform raises a retry exception on retry state and retries" do
+    retry_response = { "data" => { "state" => "UNKNOWN" } }
+    stub_request(:get, "#{Rstuf.base_url}/api/v1/task/?task_id=#{@task_id}")
+      .to_return(status: 200, body: retry_response.to_json, headers: { "Content-Type" => "application/json" })
+
+    assert_enqueued_with(job: Rstuf::CheckJob, args: [@task_id]) do
+      Rstuf::CheckJob.perform_now(@task_id)
+    end
+  end
+
   teardown do
     teardown_rstuf
   end
diff --git a/test/models/deletion_test.rb b/test/models/deletion_test.rb
index 52b45bf505d..7c78e3c4aeb 100644
--- a/test/models/deletion_test.rb
+++ b/test/models/deletion_test.rb
@@ -100,7 +100,7 @@ class DeletionTest < ActiveSupport::TestCase
     end
 
     should "enqueue rstuf removal" do
-      assert_enqueued_jobs 1, only: Rstuf::RemoveJob do
+      assert_enqueued_with(job: Rstuf::RemoveJob, args: [{ version: @version }]) do
         delete_gem
       end
     end
diff --git a/test/models/pusher_test.rb b/test/models/pusher_test.rb
index 4105afa1392..ba28bb2b90f 100644
--- a/test/models/pusher_test.rb
+++ b/test/models/pusher_test.rb
@@ -626,7 +626,7 @@ def two_cert_chain(signing_key:, root_not_before: Time.current, cert_not_before:
       end
 
       should "enqueue rstuf addition" do
-        assert_enqueued_jobs 1, only: Rstuf::AddJob do
+        assert_enqueued_with(job: Rstuf::AddJob, args: [{ version: @cutter.version }]) do
           @cutter.save
         end
       end