From eb7a6e83a3551ea9af6847eb182247140c039f16 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Mon, 8 Jan 2024 21:14:33 -0800 Subject: [PATCH] Strong type `Dependabot::RegistryClient` --- common/lib/dependabot/registry_client.rb | 30 ++++++++++++++++--- .../spec/dependabot/registry_client_spec.rb | 4 +-- sorbet/rbi/shims/excon.rbi | 10 +++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/common/lib/dependabot/registry_client.rb b/common/lib/dependabot/registry_client.rb index 4e32c30e598..b303e4808e9 100644 --- a/common/lib/dependabot/registry_client.rb +++ b/common/lib/dependabot/registry_client.rb @@ -1,6 +1,7 @@ -# typed: true +# typed: strong # frozen_string_literal: true +require "sorbet-runtime" require "dependabot/shared_helpers" # This class provides a thin wrapper around our normal usage of Excon as a simple HTTP client in order to @@ -11,10 +12,20 @@ # read-timeouts as some jobs tend to be sensitive to exceeding our overall 45 minute timeout. module Dependabot class RegistryClient - @cached_errors = {} + extend T::Sig + @cached_errors = T.let({}, T::Hash[T.nilable(String), Excon::Error::Timeout]) + + sig do + params( + url: String, + headers: T::Hash[Symbol, T.untyped], + options: T::Hash[Symbol, T.untyped] + ) + .returns(Excon::Response) + end def self.get(url:, headers: {}, options: {}) - raise cached_error_for(url) if cached_error_for(url) + raise T.must(cached_error_for(url)) if cached_error_for(url) Excon.get( url, @@ -26,8 +37,16 @@ def self.get(url:, headers: {}, options: {}) raise e end + sig do + params( + url: String, + headers: T::Hash[Symbol, T.untyped], + options: T::Hash[Symbol, T.untyped] + ) + .returns(Excon::Response) + end def self.head(url:, headers: {}, options: {}) - raise cached_error_for(url) if cached_error_for(url) + raise T.must(cached_error_for(url)) if cached_error_for(url) Excon.head( url, @@ -39,15 +58,18 @@ def self.head(url:, headers: {}, options: {}) raise e end + sig { void } def self.clear_cache! @cached_errors = {} end + sig { params(url: String, error: Excon::Error::Timeout).void } private_class_method def self.cache_error(url, error) host = URI(url).host @cached_errors[host] = error end + sig { params(url: String).returns(T.nilable(Excon::Error::Timeout)) } private_class_method def self.cached_error_for(url) host = URI(url).host @cached_errors.fetch(host, nil) diff --git a/common/spec/dependabot/registry_client_spec.rb b/common/spec/dependabot/registry_client_spec.rb index 0201f18d651..96313050c11 100644 --- a/common/spec/dependabot/registry_client_spec.rb +++ b/common/spec/dependabot/registry_client_spec.rb @@ -14,8 +14,8 @@ end before do - allow(Excon).to receive(:get) - allow(Excon).to receive(:head) + allow(Excon).to receive(:get).and_return(Excon::Response.new) + allow(Excon).to receive(:head).and_return(Excon::Response.new) end describe "delegation to Excon" do diff --git a/sorbet/rbi/shims/excon.rbi b/sorbet/rbi/shims/excon.rbi index 10f1bea8a5c..0ae8321a985 100644 --- a/sorbet/rbi/shims/excon.rbi +++ b/sorbet/rbi/shims/excon.rbi @@ -12,6 +12,16 @@ module Excon .returns(Excon::Response) end def get(url, params = T.unsafe(nil), &block); end + + sig do + params( + url: String, + params: T.untyped, + block: T.untyped + ) + .returns(Excon::Response) + end + def head(url, params = T.unsafe(nil), &block); end end class Response