diff --git a/resources/sql/autopatches/20140325.push.1.event.sql b/resources/sql/autopatches/20140325.push.1.event.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140325.push.1.event.sql @@ -0,0 +1,15 @@ +CREATE TABLE {$NAMESPACE}_repository.repository_pushevent ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + phid VARCHAR(64) NOT NULL COLLATE utf8_bin, + repositoryPHID VARCHAR(64) NOT NULL COLLATE utf8_bin, + epoch INT UNSIGNED NOT NULL, + pusherPHID VARCHAR(64) NOT NULL COLLATE utf8_bin, + remoteAddress INT UNSIGNED, + remoteProtocol VARCHAR(32), + rejectCode INT UNSIGNED NOT NULL, + rejectDetails VARCHAR(64) COLLATE utf8_bin, + + UNIQUE KEY `key_phid` (phid), + KEY `key_repository` (repositoryPHID) + +) ENGINE=InnoDB, COLLATE=utf8_general_ci; diff --git a/resources/sql/autopatches/20140325.push.2.eventphid.sql b/resources/sql/autopatches/20140325.push.2.eventphid.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140325.push.2.eventphid.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_repository.repository_pushlog + ADD pushEventPHID VARCHAR(64) NOT NULL COLLATE utf8_bin AFTER epoch; + +ALTER TABLE {$NAMESPACE}_repository.repository_pushlog + ADD KEY `key_event` (pushEventPHID); diff --git a/resources/sql/autopatches/20140325.push.3.groups.php b/resources/sql/autopatches/20140325.push.3.groups.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140325.push.3.groups.php @@ -0,0 +1,43 @@ +<?php + +$conn_w = id(new PhabricatorRepository())->establishConnection('w'); + +echo "Adding transaction log event groups...\n"; + +$logs = queryfx_all( + $conn_w, + 'SELECT * FROM %T GROUP BY transactionKey ORDER BY id ASC', + 'repository_pushlog'); +foreach ($logs as $log) { + $id = $log['id']; + echo "Migrating log {$id}...\n"; + if ($log['pushEventPHID']) { + continue; + } + + $event_phid = id(new PhabricatorRepositoryPushEvent())->generatePHID(); + + queryfx( + $conn_w, + 'INSERT INTO %T (phid, repositoryPHID, epoch, pusherPHID, remoteAddress, + remoteProtocol, rejectCode, rejectDetails) + VALUES (%s, %s, %d, %s, %d, %s, %d, %s)', + 'repository_pushevent', + $event_phid, + $log['repositoryPHID'], + $log['epoch'], + $log['pusherPHID'], + $log['remoteAddress'], + $log['remoteProtocol'], + $log['rejectCode'], + $log['rejectDetails']); + + queryfx( + $conn_w, + 'UPDATE %T SET pushEventPHID = %s WHERE transactionKey = %s', + 'repository_pushlog', + $event_phid, + $log['transactionKey']); +} + +echo "Done.\n"; diff --git a/resources/sql/autopatches/20140325.push.4.prune.sql b/resources/sql/autopatches/20140325.push.4.prune.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140325.push.4.prune.sql @@ -0,0 +1,14 @@ +ALTER TABLE {$NAMESPACE}_repository.repository_pushlog + DROP remoteAddress; + +ALTER TABLE {$NAMESPACE}_repository.repository_pushlog + DROP remoteProtocol; + +ALTER TABLE {$NAMESPACE}_repository.repository_pushlog + DROP transactionKey; + +ALTER TABLE {$NAMESPACE}_repository.repository_pushlog + DROP rejectCode; + +ALTER TABLE {$NAMESPACE}_repository.repository_pushlog + DROP rejectDetails; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1958,11 +1958,14 @@ 'PhabricatorRepositoryPHIDTypeArcanistProject' => 'applications/repository/phid/PhabricatorRepositoryPHIDTypeArcanistProject.php', 'PhabricatorRepositoryPHIDTypeCommit' => 'applications/repository/phid/PhabricatorRepositoryPHIDTypeCommit.php', 'PhabricatorRepositoryPHIDTypeMirror' => 'applications/repository/phid/PhabricatorRepositoryPHIDTypeMirror.php', + 'PhabricatorRepositoryPHIDTypePushEvent' => 'applications/repository/phid/PhabricatorRepositoryPHIDTypePushEvent.php', 'PhabricatorRepositoryPHIDTypePushLog' => 'applications/repository/phid/PhabricatorRepositoryPHIDTypePushLog.php', 'PhabricatorRepositoryPHIDTypeRepository' => 'applications/repository/phid/PhabricatorRepositoryPHIDTypeRepository.php', 'PhabricatorRepositoryParsedChange' => 'applications/repository/data/PhabricatorRepositoryParsedChange.php', 'PhabricatorRepositoryPullEngine' => 'applications/repository/engine/PhabricatorRepositoryPullEngine.php', 'PhabricatorRepositoryPullLocalDaemon' => 'applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php', + 'PhabricatorRepositoryPushEvent' => 'applications/repository/storage/PhabricatorRepositoryPushEvent.php', + 'PhabricatorRepositoryPushEventQuery' => 'applications/repository/query/PhabricatorRepositoryPushEventQuery.php', 'PhabricatorRepositoryPushLog' => 'applications/repository/storage/PhabricatorRepositoryPushLog.php', 'PhabricatorRepositoryPushLogQuery' => 'applications/repository/query/PhabricatorRepositoryPushLogQuery.php', 'PhabricatorRepositoryPushLogSearchEngine' => 'applications/repository/query/PhabricatorRepositoryPushLogSearchEngine.php', @@ -4792,11 +4795,18 @@ 'PhabricatorRepositoryPHIDTypeArcanistProject' => 'PhabricatorPHIDType', 'PhabricatorRepositoryPHIDTypeCommit' => 'PhabricatorPHIDType', 'PhabricatorRepositoryPHIDTypeMirror' => 'PhabricatorPHIDType', + 'PhabricatorRepositoryPHIDTypePushEvent' => 'PhabricatorPHIDType', 'PhabricatorRepositoryPHIDTypePushLog' => 'PhabricatorPHIDType', 'PhabricatorRepositoryPHIDTypeRepository' => 'PhabricatorPHIDType', 'PhabricatorRepositoryParsedChange' => 'Phobject', 'PhabricatorRepositoryPullEngine' => 'PhabricatorRepositoryEngine', 'PhabricatorRepositoryPullLocalDaemon' => 'PhabricatorDaemon', + 'PhabricatorRepositoryPushEvent' => + array( + 0 => 'PhabricatorRepositoryDAO', + 1 => 'PhabricatorPolicyInterface', + ), + 'PhabricatorRepositoryPushEventQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorRepositoryPushLog' => array( 0 => 'PhabricatorRepositoryDAO', diff --git a/src/applications/diffusion/controller/DiffusionPushLogListController.php b/src/applications/diffusion/controller/DiffusionPushLogListController.php --- a/src/applications/diffusion/controller/DiffusionPushLogListController.php +++ b/src/applications/diffusion/controller/DiffusionPushLogListController.php @@ -52,7 +52,7 @@ // Reveal this if it's valid and the user can edit the repository. $remote_addr = '-'; if (isset($editable_repos[$log->getRepositoryPHID()])) { - $remote_long = $log->getRemoteAddress(); + $remote_long = $log->getPushEvent()->getRemoteAddress(); if ($remote_long) { $remote_addr = long2ip($remote_long); } @@ -60,6 +60,7 @@ $callsign = $log->getRepository()->getCallsign(); $rows[] = array( + $log->getPushEvent()->getID(), phutil_tag( 'a', array( @@ -68,7 +69,7 @@ $callsign), $this->getHandle($log->getPusherPHID())->renderLink(), $remote_addr, - $log->getRemoteProtocol(), + $log->getPushEvent()->getRemoteProtocol(), $log->getRefType(), $log->getRefName(), phutil_tag( @@ -86,7 +87,7 @@ // TODO: Make these human-readable. $log->getChangeFlags(), - $log->getRejectCode(), + $log->getPushEvent()->getRejectCode(), phabricator_datetime($log->getEpoch(), $viewer), ); } @@ -94,6 +95,7 @@ $table = id(new AphrontTableView($rows)) ->setHeaders( array( + pht('Push'), pht('Repository'), pht('Pusher'), pht('From'), @@ -113,6 +115,7 @@ '', '', '', + '', 'wide', 'n', 'n', diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -30,6 +30,8 @@ private $gitCommits = array(); private $heraldViewerProjects; + private $rejectCode = PhabricatorRepositoryPushLog::REJECT_BROKEN; + private $rejectDetails; /* -( Config )------------------------------------------------------------- */ @@ -62,14 +64,6 @@ return $remote_address; } - private function getTransactionKey() { - if (!$this->transactionKey) { - $entropy = Filesystem::readRandomBytes(64); - $this->transactionKey = PhabricatorHash::digestForIndex($entropy); - } - return $this->transactionKey; - } - public function setSubversionTransactionInfo($transaction, $repository) { $this->subversionTransaction = $transaction; $this->subversionRepository = $repository; @@ -137,10 +131,7 @@ } catch (DiffusionCommitHookRejectException $ex) { // If we're rejecting dangerous changes, flag everything that we've // seen as rejected so it's clear that none of it was accepted. - foreach ($all_updates as $update) { - $update->setRejectCode( - PhabricatorRepositoryPushLog::REJECT_DANGEROUS); - } + $this->rejectCode = PhabricatorRepositoryPushLog::REJECT_DANGEROUS; throw $ex; } @@ -156,9 +147,7 @@ // If we make it this far, we're accepting these changes. Mark all the // logs as accepted. - foreach ($all_updates as $update) { - $update->setRejectCode(PhabricatorRepositoryPushLog::REJECT_ACCEPT); - } + $this->rejectCode = PhabricatorRepositoryPushLog::REJECT_ACCEPT; } catch (Exception $ex) { // We'll throw this again in a minute, but we want to save all the logs // first. @@ -166,9 +155,18 @@ } // Save all the logs no matter what the outcome was. - foreach ($all_updates as $update) { - $update->save(); - } + $event = $this->newPushEvent(); + + $event->setRejectCode($this->rejectCode); + $event->setRejectDetails($this->rejectDetails); + + $event->openTransaction(); + $event->save(); + foreach ($all_updates as $update) { + $update->setPushEventPHID($event->getPHID()); + $update->save(); + } + $event->saveTransaction(); if ($caught) { throw $caught; @@ -296,10 +294,8 @@ } if ($blocking_effect) { - foreach ($all_updates as $update) { - $update->setRejectCode(PhabricatorRepositoryPushLog::REJECT_HERALD); - $update->setRejectDetails($blocking_effect->getRulePHID()); - } + $this->rejectCode = PhabricatorRepositoryPushLog::REJECT_HERALD; + $this->rejectDetails = $blocking_effect->getRulePHID(); $message = $blocking_effect->getTarget(); if (!strlen($message)) { @@ -596,12 +592,8 @@ continue; } - // Mark everything as rejected by this hook. - foreach ($updates as $update) { - $update->setRejectCode( - PhabricatorRepositoryPushLog::REJECT_EXTERNAL); - $update->setRejectDetails(basename($hook)); - } + $this->rejectCode = PhabricatorRepositoryPushLog::REJECT_EXTERNAL; + $this->rejectDetails = basename($hook); throw new DiffusionCommitHookRejectException( pht( @@ -983,24 +975,23 @@ private function newPushLog() { - // NOTE: By default, we create these with REJECT_BROKEN as the reject - // code. This indicates a broken hook, and covers the case where we - // encounter some unexpected exception and consequently reject the changes. - // NOTE: We generate PHIDs up front so the Herald transcripts can pick them // up. $phid = id(new PhabricatorRepositoryPushLog())->generatePHID(); return PhabricatorRepositoryPushLog::initializeNewLog($this->getViewer()) ->setPHID($phid) - ->attachRepository($this->getRepository()) ->setRepositoryPHID($this->getRepository()->getPHID()) - ->setEpoch(time()) + ->setEpoch(time()); + } + + private function newPushEvent() { + $viewer = $this->getViewer(); + return PhabricatorRepositoryPushEvent::initializeNewEvent($viewer) + ->setRepositoryPHID($this->getRepository()->getPHID()) ->setRemoteAddress($this->getRemoteAddressForLog()) ->setRemoteProtocol($this->getRemoteProtocol()) - ->setTransactionKey($this->getTransactionKey()) - ->setRejectCode(PhabricatorRepositoryPushLog::REJECT_BROKEN) - ->setRejectDetails(null); + ->setEpoch(time()); } public function loadChangesetsForCommit($identifier) { diff --git a/src/applications/repository/phid/PhabricatorRepositoryPHIDTypePushEvent.php b/src/applications/repository/phid/PhabricatorRepositoryPHIDTypePushEvent.php new file mode 100644 --- /dev/null +++ b/src/applications/repository/phid/PhabricatorRepositoryPHIDTypePushEvent.php @@ -0,0 +1,40 @@ +<?php + +final class PhabricatorRepositoryPHIDTypePushEvent + extends PhabricatorPHIDType { + + const TYPECONST = 'PSHE'; + + public function getTypeConstant() { + return self::TYPECONST; + } + + public function getTypeName() { + return pht('Push Event'); + } + + public function newObject() { + return new PhabricatorRepositoryPushEvent(); + } + + protected function buildQueryForObjects( + PhabricatorObjectQuery $query, + array $phids) { + + return id(new PhabricatorRepositoryPushEventQuery()) + ->withPHIDs($phids); + } + + public function loadHandles( + PhabricatorHandleQuery $query, + array $handles, + array $objects) { + + foreach ($handles as $phid => $handle) { + $event = $objects[$phid]; + + $handle->setName(pht('Push Event %d', $event->getID())); + } + } + +} diff --git a/src/applications/repository/query/PhabricatorRepositoryPushLogQuery.php b/src/applications/repository/query/PhabricatorRepositoryPushEventQuery.php copy from src/applications/repository/query/PhabricatorRepositoryPushLogQuery.php copy to src/applications/repository/query/PhabricatorRepositoryPushEventQuery.php --- a/src/applications/repository/query/PhabricatorRepositoryPushLogQuery.php +++ b/src/applications/repository/query/PhabricatorRepositoryPushEventQuery.php @@ -1,14 +1,12 @@ <?php -final class PhabricatorRepositoryPushLogQuery +final class PhabricatorRepositoryPushEventQuery extends PhabricatorCursorPagedPolicyAwareQuery { private $ids; private $phids; private $repositoryPHIDs; private $pusherPHIDs; - private $refTypes; - private $newRefs; public function withIDs(array $ids) { $this->ids = $ids; @@ -30,18 +28,8 @@ return $this; } - public function withRefTypes(array $ref_types) { - $this->refTypes = $ref_types; - return $this; - } - - public function withNewRefs(array $new_refs) { - $this->newRefs = $new_refs; - return $this; - } - protected function loadPage() { - $table = new PhabricatorRepositoryPushLog(); + $table = new PhabricatorRepositoryPushEvent(); $conn_r = $table->establishConnection('r'); $data = queryfx_all( @@ -55,28 +43,24 @@ return $table->loadAllFromArray($data); } - public function willFilterPage(array $logs) { - $repository_phids = mpull($logs, 'getRepositoryPHID'); - if ($repository_phids) { - $repositories = id(new PhabricatorRepositoryQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs($repository_phids) - ->execute(); - $repositories = mpull($repositories, null, 'getPHID'); - } else { - $repositories = array(); - } + public function willFilterPage(array $events) { + $repository_phids = mpull($events, 'getRepositoryPHID'); + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($repository_phids) + ->execute(); + $repositories = mpull($repositories, null, 'getPHID'); - foreach ($logs as $key => $log) { - $phid = $log->getRepositoryPHID(); + foreach ($events as $key => $event) { + $phid = $event->getRepositoryPHID(); if (empty($repositories[$phid])) { - unset($logs[$key]); + unset($events[$key]); continue; } - $log->attachRepository($repositories[$phid]); + $event->attachRepository($repositories[$phid]); } - return $logs; + return $events; } @@ -111,20 +95,6 @@ $this->pusherPHIDs); } - if ($this->refTypes) { - $where[] = qsprintf( - $conn_r, - 'refType IN (%Ls)', - $this->refTypes); - } - - if ($this->newRefs) { - $where[] = qsprintf( - $conn_r, - 'refNew IN (%Ls)', - $this->newRefs); - } - $where[] = $this->buildPagingClause($conn_r); return $this->formatWhereClause($where); diff --git a/src/applications/repository/query/PhabricatorRepositoryPushLogQuery.php b/src/applications/repository/query/PhabricatorRepositoryPushLogQuery.php --- a/src/applications/repository/query/PhabricatorRepositoryPushLogQuery.php +++ b/src/applications/repository/query/PhabricatorRepositoryPushLogQuery.php @@ -56,24 +56,20 @@ } public function willFilterPage(array $logs) { - $repository_phids = mpull($logs, 'getRepositoryPHID'); - if ($repository_phids) { - $repositories = id(new PhabricatorRepositoryQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs($repository_phids) - ->execute(); - $repositories = mpull($repositories, null, 'getPHID'); - } else { - $repositories = array(); - } + $event_phids = mpull($logs, 'getPushEventPHID'); + $events = id(new PhabricatorRepositoryPushEventQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($event_phids) + ->execute(); + $events = mpull($events, null, 'getPHID'); foreach ($logs as $key => $log) { - $phid = $log->getRepositoryPHID(); - if (empty($repositories[$phid])) { + $event = idx($events, $log->getPushEventPHID()); + if (!$event) { unset($logs[$key]); continue; } - $log->attachRepository($repositories[$phid]); + $log->attachPushEvent($event); } return $logs; diff --git a/src/applications/repository/storage/PhabricatorRepositoryPushEvent.php b/src/applications/repository/storage/PhabricatorRepositoryPushEvent.php new file mode 100644 --- /dev/null +++ b/src/applications/repository/storage/PhabricatorRepositoryPushEvent.php @@ -0,0 +1,71 @@ +<?php + +/** + * Groups a set of push logs corresponding to changes which were all pushed in + * the same transaction. + */ +final class PhabricatorRepositoryPushEvent + extends PhabricatorRepositoryDAO + implements PhabricatorPolicyInterface { + + protected $repositoryPHID; + protected $epoch; + protected $pusherPHID; + protected $remoteAddress; + protected $remoteProtocol; + protected $rejectCode; + protected $rejectDetails; + + private $repository = self::ATTACHABLE; + + public static function initializeNewEvent(PhabricatorUser $viewer) { + return id(new PhabricatorRepositoryPushEvent()) + ->setPusherPHID($viewer->getPHID()); + } + + public function getConfiguration() { + return array( + self::CONFIG_AUX_PHID => true, + self::CONFIG_TIMESTAMPS => false, + ) + parent::getConfiguration(); + } + + public function generatePHID() { + return PhabricatorPHID::generateNewPHID( + PhabricatorRepositoryPHIDTypePushEvent::TYPECONST); + } + + public function attachRepository(PhabricatorRepository $repository) { + $this->repository = $repository; + return $this; + } + + public function getRepository() { + return $this->assertAttached($this->repository); + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + public function getPolicy($capability) { + return $this->getRepository()->getPolicy($capability); + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return $this->getRepository()->hasAutomaticCapability($capability, $viewer); + } + + public function describeAutomaticCapability($capability) { + return pht( + "A repository's push events are visible to users who can see the ". + "repository."); + } + +} diff --git a/src/applications/repository/storage/PhabricatorRepositoryPushLog.php b/src/applications/repository/storage/PhabricatorRepositoryPushLog.php --- a/src/applications/repository/storage/PhabricatorRepositoryPushLog.php +++ b/src/applications/repository/storage/PhabricatorRepositoryPushLog.php @@ -33,9 +33,7 @@ protected $repositoryPHID; protected $epoch; protected $pusherPHID; - protected $remoteAddress; - protected $remoteProtocol; - protected $transactionKey; + protected $pushEventPHID; protected $refType; protected $refNameHash; protected $refNameRaw; @@ -44,11 +42,9 @@ protected $refNew; protected $mergeBase; protected $changeFlags; - protected $rejectCode; - protected $rejectDetails; private $dangerousChangeDescription = self::ATTACHABLE; - private $repository = self::ATTACHABLE; + private $pushEvent = self::ATTACHABLE; public static function initializeNewLog(PhabricatorUser $viewer) { return id(new PhabricatorRepositoryPushLog()) @@ -70,13 +66,17 @@ PhabricatorRepositoryPHIDTypePushLog::TYPECONST); } - public function attachRepository(PhabricatorRepository $repository) { - $this->repository = $repository; + public function getRepository() { + return $this->getPushEvent()->getRepository(); + } + + public function attachPushEvent(PhabricatorRepositoryPushEvent $push_event) { + $this->pushEvent = $push_event; return $this; } - public function getRepository() { - return $this->assertAttached($this->repository); + public function getPushEvent() { + return $this->assertAttached($this->pushEvent); } public function getRefName() { @@ -131,11 +131,11 @@ } public function getPolicy($capability) { - return $this->getRepository()->getPolicy($capability); + return $this->getPushEvent()->getPolicy($capability); } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { - return $this->getRepository()->hasAutomaticCapability($capability, $viewer); + return $this->getPushEvent()->hasAutomaticCapability($capability, $viewer); } public function describeAutomaticCapability($capability) {