Perform initial sync of subjects using an installation token (#1135)

* Perform initial sync of subjects using an installation token

Fixes #1125

There was an interesting race condition where the GitHub App was 
installed on a repository and the background job fired before any users 
had authorized the GitHub App oauth.

This meant there wasn't an app_token to download the subjects for 
existing notifications. 

Instead we kick off a sync of all possible subjects for a repository 
using an installation token which doesn't require any users to have 
authorized the GitHub App

* Fix tests
This commit is contained in:
Andrew Nesbitt 2018-11-12 12:31:41 +00:00 коммит произвёл GitHub
Родитель 1aa7a493b7
Коммит 1673a99d52
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
9 изменённых файлов: 56 добавлений и 1 удалений

Просмотреть файл

@ -21,7 +21,7 @@ class AppInstallation < ApplicationRecord
app_installation_id: self.id app_installation_id: self.id
}) })
repository.notifications.includes(:user, :subject, :repository).find_each{|n| n.update_subject(true) } repository.sync_subjects
end end
end end

Просмотреть файл

@ -36,4 +36,22 @@ class Repository < ApplicationRecord
last_synced_at: Time.current last_synced_at: Time.current
}) })
end end
def sync_subjects
UpdateRepoSubjectsWorker.perform_async_if_configured(self.id)
end
def sync_subjects_in_foreground
subject_urls = notifications.subjectable.distinct.pluck(:subject_url)
client = app_installation.github_client
subject_urls.each do |subject_url|
begin
remote_subject = client.get(subject_url)
SyncSubjectWorker.perform_async_if_configured(remote_subject.to_h)
rescue Octokit::ClientError => e
Rails.logger.warn("\n\n\033[32m[#{Time.current}] WARNING -- #{e.message}\033[0m\n\n")
nil
end
end
end
end end

Просмотреть файл

@ -0,0 +1,10 @@
# frozen_string_literal: true
class UpdateRepoSubjectsWorker
include Sidekiq::Worker
sidekiq_options queue: :sync_repos, unique: :until_and_while_executing
def perform(repository_id)
Repository.find_by_id(repository_id)&.sync_subjects_in_foreground
end
end

Просмотреть файл

@ -4,3 +4,4 @@
- [sync_notifications, 2] - [sync_notifications, 2]
- [marketplace, 2] - [marketplace, 2]
- [sync_subjects, 1] - [sync_subjects, 1]
- [sync_repos, 1]

Просмотреть файл

@ -19,12 +19,14 @@ class HooksControllerTest < ActionController::TestCase
end end
test 'installation_repositories webhook payload' do test 'installation_repositories webhook payload' do
stub_access_tokens_request
app = create(:app_installation, github_id: 293804) app = create(:app_installation, github_id: 293804)
send_webhook 'installation_repositories' send_webhook 'installation_repositories'
assert_equal Repository.count, 1 assert_equal Repository.count, 1
end end
test 'installation webhook payload' do test 'installation webhook payload' do
stub_access_tokens_request
send_webhook 'installation' send_webhook 'installation'
assert_equal AppInstallation.count, 1 assert_equal AppInstallation.count, 1
end end

Просмотреть файл

@ -8,6 +8,7 @@ class NotificationsControllerTest < ActionDispatch::IntegrationTest
stub_fetch_subject_enabled(value: false) stub_fetch_subject_enabled(value: false)
stub_notifications_request stub_notifications_request
stub_repository_request stub_repository_request
stub_comments_requests
@user = create(:user) @user = create(:user)
end end

4
test/fixtures/files/access_token.json поставляемый Normal file
Просмотреть файл

@ -0,0 +1,4 @@
{
"token": "v1.1f699f1069f60xxx",
"expires_at": "2016-07-11T22:14:10Z"
}

1
test/fixtures/files/comments.json поставляемый Normal file
Просмотреть файл

@ -0,0 +1 @@
[]

Просмотреть файл

@ -47,6 +47,8 @@ module StubHelper
.to_return({ status: 200, body: file_fixture('subject_56.json'), headers: headers }) .to_return({ status: 200, body: file_fixture('subject_56.json'), headers: headers })
stub_request(:get, 'https://api.github.com/repos/octobox/octobox/issues/57') stub_request(:get, 'https://api.github.com/repos/octobox/octobox/issues/57')
.to_return({ status: 200, body: file_fixture('subject_57.json'), headers: headers }) .to_return({ status: 200, body: file_fixture('subject_57.json'), headers: headers })
stub_request(:get, 'https://api.github.com/repos/octobox/octobox/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e')
.to_return({ status: 200, body: file_fixture('commit_no_author.json'), headers: { 'Content-Type' => 'application/json' } })
end end
url ||= %r{https://api.github.com/notifications} url ||= %r{https://api.github.com/notifications}
@ -56,7 +58,23 @@ module StubHelper
stub_request(method, url).to_return(response) stub_request(method, url).to_return(response)
end end
def stub_comments_requests(url: nil, body: nil, method: :get, extra_headers: {})
headers = { 'Content-Type' => 'application/json' }.merge(extra_headers)
stub_request(:get, /\/comments\z/)
.to_return({ status: 200, body: file_fixture('comments.json'), headers: headers })
end
def stub_access_tokens_request(url: nil, body: nil, method: :get, extra_headers: {})
Octobox.stubs(:generate_jwt).returns('MIIEpAIBAAKCAQEA8PcoKAOyTG0rl9PUfdgey3smnkF2U0')
headers = { 'Content-Type' => 'application/json' }.merge(extra_headers)
stub_request(:post, /https:\/\/api\.github\.com\/app\/installations\/\d+\/access_tokens\z/)
.to_return({ status: 200, body: file_fixture('access_token.json'), headers: headers })
end
def stub_repository_request(url: nil, body: nil, method: :get, extra_headers: {}) def stub_repository_request(url: nil, body: nil, method: :get, extra_headers: {})
stub_comments_requests
headers = { 'Content-Type' => 'application/json' }.merge(extra_headers) headers = { 'Content-Type' => 'application/json' }.merge(extra_headers)
stub_request(:get, /https:\/\/api.github.com\/repos\/octobox\/octobox\z/) stub_request(:get, /https:\/\/api.github.com\/repos\/octobox\/octobox\z/)