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

Pull request updater for azure client #3153

Merged
merged 5 commits into from
Mar 3, 2021
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
22 changes: 22 additions & 0 deletions common/lib/dependabot/clients/azure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,28 @@ def create_pull_request(pr_name, source_branch, target_branch,
"/_apis/git/repositories/" + source.unscoped_repo +
"/pullrequests?api-version=5.0", content.to_json)
end

def pull_request(pull_request_id)
response = get(source.api_endpoint +
source.organization + "/" + source.project +
"/_apis/git/pullrequests/" + pull_request_id)

JSON.parse(response.body)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking for now, but we might want to handle non-200 responses here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we have some sort of handling in the get method of the azure client. Can you specify the scenarios that we might want to cover here?

end

def update_ref(branch_name, old_commit, new_commit)
content = [
{
name: "refs/heads/" + branch_name,
oldObjectId: old_commit,
newObjectId: new_commit
}
]

post(source.api_endpoint + source.organization + "/" + source.project +
"/_apis/git/repositories/" + source.unscoped_repo +
"/refs?api-version=5.0", content.to_json)
end
# rubocop:enable Metrics/ParameterLists

def get(url)
Expand Down
12 changes: 12 additions & 0 deletions common/lib/dependabot/pull_request_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def update
case source.provider
when "github" then github_updater.update
when "gitlab" then gitlab_updater.update
when "azure" then azure_updater.update
else raise "Unsupported provider #{source.provider}"
end
end
Expand Down Expand Up @@ -56,5 +57,16 @@ def gitlab_updater
pull_request_number: pull_request_number
)
end

def azure_updater
Azure.new(
source: source,
base_commit: base_commit,
old_commit: old_commit,
files: files,
credentials: credentials,
pull_request_number: pull_request_number
)
end
end
end
112 changes: 112 additions & 0 deletions common/lib/dependabot/pull_request_updater/azure.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# frozen_string_literal: true

require "dependabot/clients/azure"
require "securerandom"

module Dependabot
class PullRequestUpdater
class Azure
OBJECT_ID_FOR_BRANCH_DELETE = "0000000000000000000000000000000000000000"

attr_reader :source, :files, :base_commit, :old_commit, :credentials,
:pull_request_number

def initialize(source:, files:, base_commit:, old_commit:,
credentials:, pull_request_number:)
@source = source
@files = files
@base_commit = base_commit
@old_commit = old_commit
@credentials = credentials
@pull_request_number = pull_request_number
end

def update
return unless pull_request_exists? && source_branch_exists?

update_source_branch
end

private

def azure_client_for_source
@azure_client_for_source ||=
Dependabot::Clients::Azure.for_source(
source: source,
credentials: credentials
)
end

def pull_request_exists?
pull_request
rescue Dependabot::Clients::Azure::NotFound
false
end

def source_branch_exists?
azure_client_for_source.branch(source_branch_name)
rescue Dependabot::Clients::Azure::NotFound
false
end

# Currently the PR diff in ADO shows difference in commits instead of actual diff in files.
# This workaround is done to get the target branch commit history on the source branch alongwith file changes
def update_source_branch
# 1) Push the file changes to a newly created temporary branch (from base commit)
new_commit = create_temp_branch
# 2) Update PR source branch to point to the temp branch head commit.
update_branch(source_branch_name, old_source_branch_commit, new_commit)
# 3) Delete temp branch
update_branch(temp_branch_name, new_commit, OBJECT_ID_FOR_BRANCH_DELETE)
end

def pull_request
@pull_request ||=
azure_client_for_source.pull_request(pull_request_number.to_s)
end

def source_branch_name
@source_branch_name ||= pull_request&.fetch("sourceRefName")&.gsub("refs/heads/", "")
end

def create_temp_branch
response = azure_client_for_source.create_commit(
temp_branch_name,
base_commit,
commit_message,
files,
nil
)

JSON.parse(response.body).fetch("refUpdates").first.fetch("newObjectId")
end

def temp_branch_name
@temp_branch_name ||=
"#{source_branch_name}-temp-#{SecureRandom.uuid[0..6]}"
end

def update_branch(branch_name, old_commit, new_commit)
azure_client_for_source.update_ref(
branch_name,
old_commit,
new_commit
)
end

# For updating source branch, we require the latest commit for the source branch.
def commit_being_updated
@commit_being_updated ||=
azure_client_for_source.commits(source_branch_name).first
end

def old_source_branch_commit
commit_being_updated.fetch("commitId")
end

def commit_message
commit_being_updated.fetch("comment")
end
end
end
end
71 changes: 71 additions & 0 deletions common/spec/dependabot/clients/azure_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,77 @@
end
end

describe "#pull_request" do
subject { client.pull_request(pull_request_id) }

let(:pull_request_id) { "1" }
let(:pull_request_url) { base_url + "/_apis/git/pullrequests/#{pull_request_id}" }

context "when response is 200" do
response_body = fixture("azure", "pull_request_details.json")

before do
stub_request(:get, pull_request_url).
with(basic_auth: [username, password]).
to_return(status: 200, body: response_body)
end

specify { expect { subject }.to_not raise_error }

it { is_expected.to eq(JSON.parse(response_body)) }
end

context "when response is 404" do
before do
stub_request(:get, pull_request_url).
with(basic_auth: [username, password]).
to_return(status: 404)
end

it "raises a helpful error" do
expect { subject }.to raise_error(Dependabot::Clients::Azure::NotFound)
end
end
end

describe "#update_ref" do
subject(:update_ref) do
client.update_ref(
branch,
old_commit_id,
new_commit_id
)
end

let(:old_commit_id) { "oldcommitsha" }
let(:new_commit_id) { "newcommitsha" }
let(:update_ref_url) { repo_url + "/refs?api-version=5.0" }

it "sends update branch request with old and new commit id" do
stub_request(:post, update_ref_url).
with(basic_auth: [username, password]).
to_return(status: 200)

update_ref

expect(WebMock).
to(
have_requested(:post, update_ref_url).
with do |req|
json_body = JSON.parse(req.body)
expect(json_body.count).to eq(1)
ref_update_details = json_body.first
expect(ref_update_details.fetch("name")).
to eq("refs/heads/#{branch}")
expect(ref_update_details.fetch("oldObjectId")).
to eq(old_commit_id)
expect(ref_update_details.fetch("newObjectId")).
to eq(new_commit_id)
end
)
end
end

describe "#get" do
context "Using auth headers" do
token = ":test_token"
Expand Down
Loading