Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add types to service #8246

Merged
merged 6 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions updater/lib/dependabot/api_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def update_pull_request(dependency_change, base_commit_sha)
retry
end

sig { params(dependency_names: T.any(String, T::Array[String]), reason: Symbol).void }
sig { params(dependency_names: T.any(String, T::Array[String]), reason: T.any(String, Symbol)).void }
def close_pull_request(dependency_names, reason)
api_url = "#{base_url}/update_jobs/#{job_id}/close_pull_request"
body = { data: { "dependency-names": dependency_names, reason: reason } }
Expand All @@ -81,7 +81,7 @@ def close_pull_request(dependency_names, reason)
retry
end

sig { params(error_type: String, error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void }
sig { params(error_type: T.any(String, Symbol), error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void }
def record_update_job_error(error_type:, error_details:)
api_url = "#{base_url}/update_jobs/#{job_id}/record_update_job_error"
body = {
Expand All @@ -101,7 +101,7 @@ def record_update_job_error(error_type:, error_details:)
retry
end

sig { params(error_type: String, error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void }
sig { params(error_type: T.any(Symbol, String), error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void }
def record_update_job_unknown_error(error_type:, error_details:)
error_type = "unknown_error" if error_type.nil?

Expand Down
63 changes: 49 additions & 14 deletions updater/lib/dependabot/service.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
# typed: false
# typed: strict
# frozen_string_literal: true

require "raven"
require "terminal-table"
require "dependabot/api_client"
require "sorbet-runtime"

# This class provides an output adapter for the Dependabot Service which manages
# communication with the private API as well as consolidated error handling.
Expand All @@ -13,46 +14,61 @@
#
module Dependabot
class Service
extend T::Sig
extend Forwardable
attr_reader :pull_requests, :errors

sig { returns(T::Array[T::Array[String]]) }
attr_reader :pull_requests

sig { returns(T::Array[T::Array[T.untyped]]) }
attr_reader :errors

sig { params(client: Dependabot::ApiClient).void }
def initialize(client:)
@client = client
@pull_requests = []
@errors = []
@pull_requests = T.let([], T::Array[T.untyped])
@errors = T.let([], T::Array[T.untyped])
end

def_delegators :client,
:mark_job_as_processed,
:update_dependency_list,
:record_ecosystem_versions,
:increment_metric

sig { params(dependency_change: Dependabot::DependencyChange, base_commit_sha: String).void }
def create_pull_request(dependency_change, base_commit_sha)
client.create_pull_request(dependency_change, base_commit_sha)
@pull_requests << [dependency_change.humanized, :created]
pull_requests << [dependency_change.humanized, :created]
end

sig { params(dependency_change: Dependabot::DependencyChange, base_commit_sha: String).void }
def update_pull_request(dependency_change, base_commit_sha)
client.update_pull_request(dependency_change, base_commit_sha)
@pull_requests << [dependency_change.humanized, :updated]
pull_requests << [dependency_change.humanized, :updated]
end

sig { params(dependencies: T.any(String, T::Array[String]), reason: T.any(String, Symbol)).void }
def close_pull_request(dependencies, reason)
client.close_pull_request(dependencies, reason)
humanized_deps = dependencies.is_a?(String) ? dependencies : dependencies.join(",")
@pull_requests << [humanized_deps, "closed: #{reason}"]
pull_requests << [humanized_deps, "closed: #{reason}"]
end

sig do
params(error_type: T.any(String, Symbol), error_details: T.nilable(T::Hash[T.untyped, T.untyped]),
dependency: T.nilable(Dependabot::Dependency)).void
end
def record_update_job_error(error_type:, error_details:, dependency: nil)
@errors << [error_type.to_s, dependency]
errors << [error_type.to_s, dependency]
client.record_update_job_error(error_type: error_type, error_details: error_details)
end

sig { params(error_type: T.any(String, Symbol), error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void }
def record_update_job_unknown_error(error_type:, error_details:)
client.record_update_job_unknown_error(error_type: error_type, error_details: error_details)
end

sig { params(dependency_snapshot: Dependabot::DependencySnapshot).void }
def update_dependency_list(dependency_snapshot:)
dependency_payload = dependency_snapshot.dependencies.map do |dep|
{
Expand All @@ -71,8 +87,18 @@ def update_dependency_list(dependency_snapshot:)
#
# This should be called as an alternative/in addition to record_update_job_error
# for cases where an error could indicate a problem with the service.
sig do
params(
error: StandardError,
job: T.untyped,
dependency: T.nilable(Dependabot::Dependency),
dependency_group: T.nilable(Dependabot::DependencyGroup),
tags: T::Hash[String, T.untyped],
extra: T::Hash[String, T.untyped]
).void
end
def capture_exception(error:, job: nil, dependency: nil, dependency_group: nil, tags: {}, extra: {})
Raven.capture_exception(
T.unsafe(Raven).capture_exception(
error,
{
tags: tags.merge({
Expand All @@ -88,10 +114,12 @@ def capture_exception(error:, job: nil, dependency: nil, dependency_group: nil,
)
end

sig { returns(T::Boolean) }
def noop?
pull_requests.empty? && errors.empty?
end

sig { returns(T::Boolean) }
def failure?
errors.any?
end
Expand All @@ -106,6 +134,7 @@ def failure?
# | closed:dependency-removed | package-c |
# +----------------------------+-----------------------------------+
#
sig { returns(T.nilable(String)) }
def summary
return if noop?

Expand All @@ -120,17 +149,20 @@ def summary

private

sig { returns(Dependabot::ApiClient) }
attr_reader :client

sig { returns(T.nilable(Terminal::Table)) }
def pull_request_summary
return unless pull_requests.any?

Terminal::Table.new do |t|
T.unsafe(Terminal::Table).new do |t|
t.title = "Changes to Dependabot Pull Requests"
t.rows = pull_requests.map { |deps, action| [action, truncate(deps)] }
t.rows = pull_requests.map { |deps, action| [action, truncate(T.must(deps))] }
end
end

sig { returns(T.nilable(String)) }
def error_summary
return unless errors.any?

Expand All @@ -144,11 +176,12 @@ def error_summary
# +--------------------+
# | job_repo_not_found |
# +--------------------+
sig { returns(T.nilable(Terminal::Table)) }
def job_error_type_summary
job_error_types = errors.filter_map { |error_type, dependency| [error_type] if dependency.nil? }
return if job_error_types.none?

Terminal::Table.new do |t|
T.unsafe(Terminal::Table).new do |t|
t.title = "Errors"
t.rows = job_error_types
end
Expand All @@ -161,18 +194,20 @@ def job_error_type_summary
# +---------------------+---------------+
# | best_dependency_yay | unknown_error |
# +---------------------+---------------+
sig { returns(T.nilable(Terminal::Table)) }
def dependency_error_summary
dependency_errors = errors.filter_map do |error_type, dependency|
[dependency.name, error_type] unless dependency.nil?
end
return if dependency_errors.none?

Terminal::Table.new do |t|
T.unsafe(Terminal::Table).new do |t|
t.title = "Dependencies failed to update"
t.rows = dependency_errors
end
end

sig { params(string: String, max: Integer).returns(String) }
def truncate(string, max: 120)
snip = max - 3
string.length > max ? "#{string[0...snip]}..." : string
Expand Down
1 change: 1 addition & 0 deletions updater/spec/dependabot/file_fetcher_command_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
allow(api_client).to receive(:mark_job_as_processed)
allow(api_client).to receive(:record_update_job_error)
allow(api_client).to receive(:record_ecosystem_versions)
allow(api_client).to receive(:is_a?).with(Dependabot::ApiClient).and_return(true)

allow(Dependabot::Environment).to receive(:output_path).and_return(File.join(Dir.mktmpdir, "output.json"))
allow(Dependabot::Environment).to receive(:job_definition).and_return(job_definition)
Expand Down
22 changes: 12 additions & 10 deletions updater/spec/dependabot/integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,18 @@
end

let(:api_client) do
instance_double(Dependabot::ApiClient,
create_pull_request: nil,
update_pull_request: nil,
close_pull_request: nil,
mark_job_as_processed: nil,
update_dependency_list: nil,
record_update_job_error: nil,
record_update_job_unknown_error: nil,
record_ecosystem_versions: nil,
increment_metric: nil)
api_client = instance_double(Dependabot::ApiClient,
create_pull_request: nil,
update_pull_request: nil,
close_pull_request: nil,
mark_job_as_processed: nil,
update_dependency_list: nil,
record_update_job_error: nil,
record_update_job_unknown_error: nil,
record_ecosystem_versions: nil,
increment_metric: nil)
allow(api_client).to receive(:is_a?).with(Dependabot::ApiClient).and_return(true)
api_client
end
let(:file_fetcher) do
instance_double(Dependabot::FileFetchers::Base,
Expand Down
73 changes: 40 additions & 33 deletions updater/spec/dependabot/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@
let(:base_sha) { "mock-sha" }

let(:mock_client) do
instance_double(Dependabot::ApiClient, {
api_client = instance_double(Dependabot::ApiClient, {
create_pull_request: nil,
update_pull_request: nil,
close_pull_request: nil,
record_update_job_error: nil,
record_update_job_unknown_error: nil
})
allow(api_client).to receive(:is_a?).with(Dependabot::ApiClient).and_return(true)
api_client
end
subject(:service) { described_class.new(client: mock_client) }

Expand Down Expand Up @@ -269,7 +271,7 @@
end

it "extracts information from a dependency if provided" do
dependency = OpenStruct.new(name: "lodash")
dependency = Dependabot::Dependency.new(name: "lodash", requirements: [], package_manager: "npm_and_yarn")
service.capture_exception(error: error, dependency: dependency)

expect(Raven).to have_received(:capture_exception)
Expand All @@ -282,6 +284,7 @@

it "extracts information from a dependency_group if provided" do
dependency_group = OpenStruct.new(name: "all-the-things")
allow(dependency_group).to receive(:is_a?).with(Dependabot::DependencyGroup).and_return(true)
service.capture_exception(error: error, dependency_group: dependency_group)

expect(Raven).to have_received(:capture_exception)
Expand All @@ -295,37 +298,41 @@

describe "#update_dependency_list" do
let(:dependency_snapshot) do
instance_double(Dependabot::DependencySnapshot,
dependencies: [
Dependabot::Dependency.new(
name: "dummy-pkg-a",
package_manager: "bundler",
version: "2.0.0",
requirements: [
{ file: "Gemfile", requirement: "~> 2.0.0", groups: [:default], source: nil }
]
),
Dependabot::Dependency.new(
name: "dummy-pkg-b",
package_manager: "bundler",
version: "1.1.0",
requirements: [
{ file: "Gemfile", requirement: "~> 1.1.0", groups: [:default], source: nil }
]
)
],
dependency_files: [
Dependabot::DependencyFile.new(
name: "Gemfile",
content: fixture("bundler/original/Gemfile"),
directory: "/"
),
Dependabot::DependencyFile.new(
name: "Gemfile.lock",
content: fixture("bundler/original/Gemfile.lock"),
directory: "/"
)
])
dependency_snapshot = instance_double(Dependabot::DependencySnapshot,
dependencies: [
Dependabot::Dependency.new(
name: "dummy-pkg-a",
package_manager: "bundler",
version: "2.0.0",
requirements: [
{ file: "Gemfile", requirement: "~> 2.0.0", groups: [:default],
source: nil }
]
),
Dependabot::Dependency.new(
name: "dummy-pkg-b",
package_manager: "bundler",
version: "1.1.0",
requirements: [
{ file: "Gemfile", requirement: "~> 1.1.0", groups: [:default],
source: nil }
]
)
],
dependency_files: [
Dependabot::DependencyFile.new(
name: "Gemfile",
content: fixture("bundler/original/Gemfile"),
directory: "/"
),
Dependabot::DependencyFile.new(
name: "Gemfile.lock",
content: fixture("bundler/original/Gemfile.lock"),
directory: "/"
)
])
allow(dependency_snapshot).to receive(:is_a?).and_return(true)
dependency_snapshot
end

let(:expected_dependency_payload) do
Expand Down
2 changes: 2 additions & 0 deletions updater/spec/dependabot/updater_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2753,12 +2753,14 @@ def build_service
record_update_job_unknown_error: nil,
increment_metric: nil
)
allow(api_client).to receive(:is_a?).with(Dependabot::ApiClient).and_return(true)

service = Dependabot::Service.new(
client: api_client
)
allow(service).to receive(:record_update_job_error)
allow(service).to receive(:record_update_job_unknown_error)
allow(service).to receive(:is_a?).with(Dependabot::Service).and_return(true)

service
end
Expand Down