Page MenuHomePhabricator
Differential D8775 Diff 20825 src/applications/doorkeeper/bridge/DoorkeeperBridgeGitHub.php

Changeset View

Standalone View

View Options

src/applications/doorkeeper/bridge/DoorkeeperBridgeGitHub.php

  • This file was added.
<?php
final class DoorkeeperBridgeGitHub extends DoorkeeperBridge {
const APPTYPE_GITHUB = 'github';
const APPDOMAIN_GITHUB ='github.com';
const OBJTYPE_PULL_REQUEST = 'github:pull-request';
public function canPullRef(DoorkeeperObjectRef $ref) {
if ($ref->getApplicationType() != self::APPTYPE_GITHUB) {
return false;
}
$types = array(
self::OBJTYPE_PULL_REQUEST => true,
);
return isset($types[$ref->getObjectType()]);
}
public function pullRefs(array $refs) {
$id_map = mpull($refs, 'getObjectID', 'getObjectKey');
// Object IDs for pull requests are formatted as:
//
// user:repo:pullid
//
// since we need both the user and repo name to retrieve
// data about a pull request.
$data_map = array();
foreach ($id_map as $key => $id) {
$split = explode(':', $id);
$user = str_replace('/', '', $split[0]);
$repo = str_replace('/', '', $split[1]);
$pull_id = (int)$split[2];
$data_map[$key] = array(
'id' => $id,
'user' => $user,
'repo' => $repo,
'pullID' => $pull_id,
'getURI' =>
'https://api.github.com/repos/'.$user.'/'.$repo.'/pulls/'.$pull_id);
}
// Query the GitHub public API, retrieving all of the information.
$futures = array();
$user_agent =
'Phabricator Instance at '.
PhabricatorEnv::getProductionURI('/');
foreach ($data_map as $key => $data) {
$futures[$key] = id(new HTTPSFuture($data['getURI']))
->setMethod('GET')
->setTimeout(30)
->addHeader('User-Agent', $user_agent);
}
$results = array();
$failed = array();
foreach (Futures($futures) as $key => $future) {
try {
$response = $future->resolvex();
$results[$key] = json_decode($response[0], true);
} catch (Exception $ex) {
if (($ex instanceof HTTPFutureResponseStatus) &&
($ex->getStatusCode() == 404)) {
// This indicates that the object has been deleted (or never existed,
// or isn't visible to the current user) but it's a successful sync of
// an object which isn't visible.
} else {
// This is something else, so consider it a synchronization failure.
phlog($ex);
$failed[$key] = $ex;
}
}
}
// Set all the associated data.
foreach ($refs as $ref) {
$did_fail = idx($failed, $ref->getObjectKey());
if ($did_fail) {
// Set at least the key to identify, normally the
// name is the name of the PR.
$ref->setAttribute('name', 'GitHub PR '.$ref->getObjectID());
$ref->setSyncFailed(true);
continue;
}
$result = idx($results, $ref->getObjectKey());
if (!$result) {
continue;
}
$ref->setIsVisible(true);
$ref->setAttribute('name', $result['title']);
// TODO: Does the data belong on the ref attributes or the
hach-queAuthorUnsubmitted
Not Done
Inline Actions

I'm not sure what the best option is here, both references and objects seem to accept arbitrary data.

At the moment it doesn't matter too much (the synchronisation is from GitHub into Phabricator), but when we have a worker pushing Phabricator comments back to the GitHub PR it will be important.

hach-que: I'm not sure what the best option is here, both references and objects seem to accept arbitrary…
// object properties??
$obj = $ref->getExternalObject();
$obj->setObjectURI($result['url']);
$obj->setProperty('name', $result['title']);
$obj->setProperty('diffURL', $result['diff_url']);
$obj->setProperty('state', $result['state']);
$obj->setProperty('userLogin', $result['user']['login']);
$obj->setProperty('body', $result['body']);
$obj->setProperty('headGitURL', $result['head']['repo']['git_url']);
$obj->setProperty('headGitSHA', $result['head']['sha']);
$obj->setProperty('headLabel', $result['head']['label']);
$obj->setProperty('headOwnerLogin', $result['head']['owner']['login']);
$obj->setProperty('baseGitURL', $result['base']['repo']['git_url']);
$obj->setProperty('baseGitSHA', $result['base']['sha']);
$obj->setProperty('baseLabel', $result['base']['label']);
$obj->setProperty('baseOwnerLogin', $result['base']['owner']['login']);
$obj->setProperty('merged', $result['merged']);
$existing_revision_id = $obj->getProperty('revisionID');
$revision_id = $this->syncRevisionForExternalObject(
$obj,
$existing_revision_id);
$obj->setProperty('revisionID', $revision_id);
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
$obj->save();
unset($unguarded);
}
}
private function syncRevisionForExternalObject(
DoorkeeperExternalObject $pr,
$existing_revision_id) {
$author_user = $this->lookupPhabricatorUserByConnectedAccount(
$pr->getProperty('userLogin'));
if ($existing_revision_id === null) {
// There is no existing revision, so we need to create one.
$revision = DifferentialRevision::initializeNewRevision($author_user);
$revision->attachReviewerStatus(array());
} else {
$revision = id(new DifferentialRevisionQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withIDs(array($existing_revision_id))
->needReviewerStatus(true)
->needActiveDiffs(true)
->executeOne();
}
$revision->setAuthorPHID($author_user->getPHID());
$revision->setTitle($pr->getProperty('name'));
$revision->setSummary($pr->getProperty('body'));
// Download diff.
$user_agent =
'Phabricator Instance at '.
PhabricatorEnv::getProductionURI('/');
$response = id(new HTTPSFuture($pr->getProperty('diffURL')))
->setMethod('GET')
->setTimeout(30)
->addHeader('User-Agent', $user_agent)
->resolvex();
$raw_diff = $response[0];
// Copied from Conduit code? Should we just make
// the Conduit call here?
$parser = new ArcanistDiffParser();
$changes = $parser->parseDiff($raw_diff);
$diff = DifferentialDiff::newFromRawChanges($changes);
$diff->setLintStatus(DifferentialLintStatus::LINT_SKIP);
$diff->setUnitStatus(DifferentialUnitStatus::UNIT_SKIP);
$diff->setDescription('Received from GitHub service hook');
$diff->setAuthorPHID($author_user->getPHID());
$diff->setCreationMethod('github');
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
$diff->save();
unset($unguarded);
// Attach diff.
$xactions = array();
$xactions[] = id(new DifferentialTransaction())
->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
->setNewValue($diff->getPHID());
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
$editor = id(new DifferentialTransactionEditor())
->setActor($author_user)
->setContentSource(
PhabricatorContentSource::newForSource('github', array()))
->setContinueOnNoEffect(true)
->applyTransactions($revision, $xactions);
unset($unguarded);
return $revision->getID();
}
private function lookupPhabricatorUserByConnectedAccount($github_account) {
// TODO
hach-queAuthorUnsubmitted
Not Done
Inline Actions

So here we should map the associated GitHub account name back to a Phabricator account with it connected if possible. There's a few problems, namely:

  • This is a public API, someone could potentially forge a request and act as another user. This is really bad. I believe if we have a configuration option which sets a secret token for this API, with the secret token then also set as a parameter on the GitHub side, then we might be able to mitigate this somewhat (although anyone setting up the web hook could still forge requests).
  • Anyone on GitHub can create pull requests, even users without Phabricator accounts. Either we need to use a service account, or auto-create an account that represents an external account on GitHub.
hach-que: So here we should map the associated GitHub account name back to a Phabricator account with it…
return id(new PhabricatorPeopleQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withUsernames(array('github'))
->executeOne();
}
}
Phabricator · Built by Phacility