diff --git a/updater/lib/dependabot/service.rb b/updater/lib/dependabot/service.rb index 530144833f..d669aca9dd 100644 --- a/updater/lib/dependabot/service.rb +++ b/updater/lib/dependabot/service.rb @@ -8,6 +8,7 @@ require "dependabot/api_client" require "dependabot/errors" require "dependabot/opentelemetry" +require "dependabot/experiments" # This class provides an output adapter for the Dependabot Service which manages # communication with the private API as well as consolidated error handling. @@ -20,6 +21,21 @@ class Service extend T::Sig extend Forwardable + class InvalidUpdatedDependencies < DependabotError + extend T::Sig + + sig { params(deps_no_previous_version: T::Array[String], deps_no_change: T::Array[String]).void } + def initialize(deps_no_previous_version:, deps_no_change:) + msg = "" + if deps_no_previous_version.any? + msg += "Previous version was not provided for: '#{deps_no_previous_version.join(', ')}' " + end + msg += "No requirements change for: '#{deps_no_change.join(', ')}'" if deps_no_change.any? + + super(msg) + end + end + sig { returns(T::Array[T.untyped]) } attr_reader :pull_requests @@ -48,6 +64,10 @@ def wait_for_calls_to_finish sig { params(dependency_change: Dependabot::DependencyChange, base_commit_sha: String).void } def create_pull_request(dependency_change, base_commit_sha) + if Experiments.enabled?("dependency_change_validation") + check_dependencies_have_previous_version(dependency_change.updated_dependencies) + end + if Experiments.enabled?("threaded_metadata") @threads << Thread.new { client.create_pull_request(dependency_change, base_commit_sha) } else @@ -228,5 +248,23 @@ def truncate(string, max: 120) snip = max - 3 string.length > max ? "#{string[0...snip]}..." : string end + + sig { params(updated_dependencies: T::Array[Dependabot::Dependency]).void } + def check_dependencies_have_previous_version(updated_dependencies) + return if updated_dependencies.all? { |d| requirements_changed?(d) } + return if updated_dependencies.all?(&:previous_version) + + deps_no_previous_version = updated_dependencies.reject(&:previous_version) + deps_no_change = updated_dependencies.reject { |d| requirements_changed?(d) } + raise InvalidUpdatedDependencies.new( + deps_no_previous_version: deps_no_previous_version.map(&:name), + deps_no_change: deps_no_change.map(&:name) + ) + end + + sig { params(dependency: Dependabot::Dependency).returns(T::Boolean) } + def requirements_changed?(dependency) + (dependency.requirements - T.must(dependency.previous_requirements)).any? + end end end diff --git a/updater/spec/dependabot/service_spec.rb b/updater/spec/dependabot/service_spec.rb index a422e04c21..8b390c714f 100644 --- a/updater/spec/dependabot/service_spec.rb +++ b/updater/spec/dependabot/service_spec.rb @@ -10,6 +10,7 @@ require "dependabot/errors" require "dependabot/pull_request_creator" require "dependabot/service" +require "dependabot/experiments" RSpec.describe Dependabot::Service do subject(:service) { described_class.new(client: mock_client) } @@ -94,8 +95,6 @@ commit_message: "Commit message" ) ) - - service.create_pull_request(dependency_change, base_sha) end end @@ -221,15 +220,46 @@ describe "#create_pull_request" do include_context :a_pr_was_created + before do + Dependabot::Experiments.register("dependency_change_validation", true) + end + it "delegates to @client" do + service.create_pull_request(dependency_change, base_sha) + expect(mock_client) .to have_received(:create_pull_request).with(dependency_change, base_sha) end it "memoizes a shorthand summary of the PR" do + service.create_pull_request(dependency_change, base_sha) + expect(service.pull_requests) .to eql([["dependabot-fortran ( from 1.7.0 to 1.8.0 ), dependabot-pascal ( from 2.7.0 to 2.8.0 )", :created]]) end + + context "when the change is missing a previous version and there's no change in requirements" do + let(:dependencies) do + [ + Dependabot::Dependency.new( + name: "dependabot-fortran", + package_manager: "bundler", + version: "1.8.0", + requirements: [ + { file: "Gemfile", requirement: "~> 1.8.0", groups: [], source: nil } + ], + previous_requirements: [ + { file: "Gemfile", requirement: "~> 1.8.0", groups: [], source: nil } + ] + ) + ] + end + + it "raises an InvalidUpdatedDependencies error" do + expect { service.create_pull_request(dependency_change, base_sha) } + .to raise_error(Dependabot::Service::InvalidUpdatedDependencies) + end + end end describe "#update_pull_request" do @@ -488,6 +518,8 @@ include_context :a_pr_was_created it "includes the summary of the created PR" do + service.create_pull_request(dependency_change, base_sha) + expect(service.summary) .to include("created", "dependabot-fortran ( from 1.7.0 to 1.8.0 ), dependabot-pascal ( from 2.7.0 to 2.8.0 )") @@ -563,6 +595,10 @@ include_context :an_error_was_reported include_context :a_dependency_error_was_reported + before do + service.create_pull_request(dependency_change, base_sha) + end + it "includes the summary of the created PR" do expect(service.summary) .to include("created",