Close http connection in perform method of Request class (#6889)

HTTP connections must be explicitly closed in many cases, and letting
perform method close connections makes its callers less redundant and
prevent them from forgetting to close connections.

3 files changed, 19 insertions, 31 deletions

diff --git a/app/workers/activitypub/delivery_worker.rb b/app/workers/activitypub/delivery_worker.rb
index 4763856ac..e6cfd0d07 100644
--- a/app/workers/activitypub/delivery_worker.rb
+++ b/app/workers/activitypub/delivery_worker.rb
@@ -12,11 +12,10 @@ class ActivityPub::DeliveryWorker
     @source_account = Account.find(source_account_id)
     @inbox_url      = inbox_url
 
-    perform_request
+    perform_request do |response|
+      raise Mastodon::UnexpectedResponseError, response unless response_successful? response
+    end
 
-    raise Mastodon::UnexpectedResponseError, @response unless response_successful?
-
-    @response.connection&.close
     failure_tracker.track_success!
   rescue => e
     failure_tracker.track_failure!
@@ -31,12 +30,12 @@ class ActivityPub::DeliveryWorker
     request.add_headers(HEADERS)
   end
 
-  def perform_request
-    @response = build_request.perform
+  def perform_request(&block)
+    build_request.perform(&block)
   end
 
-  def response_successful?
-    @response.code > 199 && @response.code < 300
+  def response_successful?(response)
+    response.code > 199 && response.code < 300
   end
 
   def failure_tracker
diff --git a/app/workers/pubsubhubbub/confirmation_worker.rb b/app/workers/pubsubhubbub/confirmation_worker.rb
index e1ccfb99c..cc2d1225b 100644
--- a/app/workers/pubsubhubbub/confirmation_worker.rb
+++ b/app/workers/pubsubhubbub/confirmation_worker.rb
@@ -21,8 +21,8 @@ class Pubsubhubbub::ConfirmationWorker
   def process_confirmation
     prepare_subscription
 
-    confirm_callback
-    logger.debug "Confirming PuSH subscription for #{subscription.callback_url} with challenge #{challenge}: #{callback_response_body}"
+    callback_get_with_params
+    logger.debug "Confirming PuSH subscription for #{subscription.callback_url} with challenge #{challenge}: #{@callback_response_body}"
 
     update_subscription
   end
@@ -44,7 +44,7 @@ class Pubsubhubbub::ConfirmationWorker
   end
 
   def response_matches_challenge?
-    callback_response_body == challenge
+    @callback_response_body == challenge
   end
 
   def subscribing?
@@ -55,16 +55,10 @@ class Pubsubhubbub::ConfirmationWorker
     mode == 'unsubscribe'
   end
 
-  def confirm_callback
-    @_confirm_callback ||= callback_get_with_params
-  end
-
   def callback_get_with_params
-    Request.new(:get, subscription.callback_url, params: callback_params).perform
-  end
-
-  def callback_response_body
-    confirm_callback.body.to_s
+    Request.new(:get, subscription.callback_url, params: callback_params).perform do |response|
+      @callback_response_body = response.body.to_s
+    end
   end
 
   def callback_params
diff --git a/app/workers/pubsubhubbub/delivery_worker.rb b/app/workers/pubsubhubbub/delivery_worker.rb
index a9174edd2..619bfa48a 100644
--- a/app/workers/pubsubhubbub/delivery_worker.rb
+++ b/app/workers/pubsubhubbub/delivery_worker.rb
@@ -23,22 +23,17 @@ class Pubsubhubbub::DeliveryWorker
   private
 
   def process_delivery
-    payload_delivery
+    callback_post_payload do |payload_delivery|
+      raise Mastodon::UnexpectedResponseError, payload_delivery unless response_successful? payload_delivery
+    end
 
-    raise Mastodon::UnexpectedResponseError, payload_delivery unless response_successful?
-
-    payload_delivery.connection&.close
     subscription.touch(:last_successful_delivery_at)
   end
 
-  def payload_delivery
-    @_payload_delivery ||= callback_post_payload
-  end
-
-  def callback_post_payload
+  def callback_post_payload(&block)
     request = Request.new(:post, subscription.callback_url, body: payload)
     request.add_headers(headers)
-    request.perform
+    request.perform(&block)
   end
 
   def blocked_domain?
@@ -80,7 +75,7 @@ class Pubsubhubbub::DeliveryWorker
     OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), subscription.secret, payload)
   end
 
-  def response_successful?
+  def response_successful?(payload_delivery)
     payload_delivery.code > 199 && payload_delivery.code < 300
   end
 end