Skip to content

Commit

Permalink
Find the token that can reach the repo
Browse files Browse the repository at this point in the history
Also, rescue `Octokit::Unauthorized` when checking if config is
reachable, because if the token gets removed from GitHub's side then
we'll encounter the unauthorized error (i.e. 401 - Bad credentials).

In `GithubApi` make `client` method private, since we shouldn't be
calling it directly -- defeats the purpose of our wrapper.
  • Loading branch information
Greg Lazarev committed Jun 24, 2017
1 parent 9c0b112 commit a58acfc
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 15 deletions.
15 changes: 13 additions & 2 deletions app/models/user_token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,23 @@ def token
end

def user
@user ||= users_with_token.sample || user_with_default_token
@_user ||= users_with_token.shuffle.detect(-> { hound_user }) do |user|
can_reach_repository?(user)
end
end

private

def user_with_default_token
def can_reach_repository?(user)
if GithubApi.new(user.token).repository?(repo.name)
true
else
repo.remove_membership(user)
false
end
end

def hound_user
User.new(token: Hound::GITHUB_TOKEN)
end

Expand Down
2 changes: 1 addition & 1 deletion app/services/build_owner_hound_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def user_token

def config_repo_reachable?
!!github.repo(config_repo.name)
rescue Octokit::InvalidRepository, Octokit::NotFound
rescue Octokit::InvalidRepository, Octokit::NotFound, Octokit::Unauthorized
false
end

Expand Down
14 changes: 10 additions & 4 deletions lib/github_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ def initialize(token)
@file_cache = {}
end

def client
@client ||= Octokit::Client.new(access_token: token, auto_paginate: true)
end

def scopes
client.scopes(token).join(",")
end
Expand Down Expand Up @@ -129,8 +125,18 @@ def remove_collaborator(repo_name, username)
)
end

def repository?(repo_name)
client.repository?(repo_name)
rescue Octokit::Unauthorized
false
end

private

def client
@_client ||= Octokit::Client.new(access_token: token, auto_paginate: true)
end

def create_status(repo:, sha:, state:, description:, target_url: nil)
client.create_status(
repo,
Expand Down
54 changes: 46 additions & 8 deletions spec/models/user_token_spec.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,45 @@
require "octokit/error"
require "lib/github_api"
require "app/models/user_token"

describe UserToken do
describe "#token" do
context "when user with a token is found" do
it "returns user's token" do
token = "foo_bar_token"
user = instance_double("User", token: token)
repo = instance_double("Repo", users_with_token: [user])
user_token = UserToken.new(repo)
context "and the token can reach the repo" do
it "returns user's token" do
token = "foo_bar_token"
user = instance_double("User", token: token)
repo = instance_double(
"Repo",
users_with_token: [user],
name: "foo/bar",
)
user_token = UserToken.new(repo)
stub_github(user.token)

expect(user_token.token).to eq token
end
end

expect(user_token.token).to eq token
context "and the token cannot reach the repo" do
it "removes that user from repo and returns user with good token" do
user_with_bad_token = instance_double("User", token: "abc123")
user_with_good_token = instance_double("User", token: "def456")
repo = instance_double(
"Repo",
name: "thoughtbot/guides",
remove_membership: nil,
users_with_token:
double(shuffle: [user_with_bad_token, user_with_good_token]),
)
user_token = UserToken.new(repo)
stub_github("abc123", repository?: false)
stub_github("def456", repository?: true)

expect(user_token.token).to eq user_with_good_token.token
expect(repo).to have_received(:remove_membership).
with(user_with_bad_token)
end
end
end

Expand All @@ -27,11 +57,19 @@

describe "#user" do
it "returns the user that bears the current token" do
user = instance_double("User")
repo = instance_double("Repo", users_with_token: [user])
user = instance_double("User", token: "abcd1")
repo = instance_double("Repo", users_with_token: [user], name: "foo/bar")
user_token = UserToken.new(repo)
stub_github(user.token)

expect(user_token.user).to eq user
end
end

def stub_github(token, options = {})
default_options = { repository?: true }
github_stub = instance_double("GithubApi", default_options.merge(options))
allow(GithubApi).to receive(:new).with(token).and_return(github_stub)
github_stub
end
end
1 change: 1 addition & 0 deletions spec/services/build_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ def stubbed_github_api
create_pending_status: nil,
create_success_status: nil,
create_error_status: nil,
repository?: true,
)
allow(GithubApi).to receive(:new).and_return(github_api)

Expand Down

0 comments on commit a58acfc

Please sign in to comment.