From e6a9db56a96a9ac2aec134ab5084381899b03e35 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 22 Jan 2018 20:09:52 -0800 Subject: [PATCH] Add a basic view for repository pull logs Summary: Depends on D18912. Ref T13046. Add a UI to browse the existing pull log table. The actual log still has some significant flaws, but get the basics working. Test Plan: {F5391909} Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13046 Differential Revision: https://secure.phabricator.com/D18914 --- src/__phutil_library_map__.php | 14 ++- .../PhabricatorDiffusionApplication.php | 3 + ...troller.php => DiffusionLogController.php} | 2 +- .../DiffusionPullLogListController.php | 12 ++ .../DiffusionPushEventViewController.php | 6 +- .../DiffusionPushLogListController.php | 7 +- .../DiffusionRepositoryController.php | 11 +- .../query/DiffusionPullLogSearchEngine.php | 85 +++++++++++++ .../view/DiffusionPullLogListView.php | 115 ++++++++++++++++++ .../PhabricatorRepositoryPullEventQuery.php | 30 +++-- .../PhabricatorRepositoryPullEvent.php | 44 ++++++- 11 files changed, 304 insertions(+), 25 deletions(-) rename src/applications/diffusion/controller/{DiffusionPushLogController.php => DiffusionLogController.php} (54%) create mode 100644 src/applications/diffusion/controller/DiffusionPullLogListController.php create mode 100644 src/applications/diffusion/query/DiffusionPullLogSearchEngine.php create mode 100644 src/applications/diffusion/view/DiffusionPullLogListView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 7caf7a6dc2..2324ac0205 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -764,6 +764,7 @@ phutil_register_library_map(array( 'DiffusionLintCountQuery' => 'applications/diffusion/query/DiffusionLintCountQuery.php', 'DiffusionLintSaveRunner' => 'applications/diffusion/DiffusionLintSaveRunner.php', 'DiffusionLocalRepositoryFilter' => 'applications/diffusion/data/DiffusionLocalRepositoryFilter.php', + 'DiffusionLogController' => 'applications/diffusion/controller/DiffusionLogController.php', 'DiffusionLookSoonConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionLookSoonConduitAPIMethod.php', 'DiffusionLowLevelCommitFieldsQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php', 'DiffusionLowLevelCommitQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php', @@ -831,9 +832,11 @@ phutil_register_library_map(array( 'DiffusionPreCommitRefTypeHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitRefTypeHeraldField.php', 'DiffusionPreCommitUsesGitLFSHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitUsesGitLFSHeraldField.php', 'DiffusionPullEventGarbageCollector' => 'applications/diffusion/garbagecollector/DiffusionPullEventGarbageCollector.php', + 'DiffusionPullLogListController' => 'applications/diffusion/controller/DiffusionPullLogListController.php', + 'DiffusionPullLogListView' => 'applications/diffusion/view/DiffusionPullLogListView.php', + 'DiffusionPullLogSearchEngine' => 'applications/diffusion/query/DiffusionPullLogSearchEngine.php', 'DiffusionPushCapability' => 'applications/diffusion/capability/DiffusionPushCapability.php', 'DiffusionPushEventViewController' => 'applications/diffusion/controller/DiffusionPushEventViewController.php', - 'DiffusionPushLogController' => 'applications/diffusion/controller/DiffusionPushLogController.php', 'DiffusionPushLogListController' => 'applications/diffusion/controller/DiffusionPushLogListController.php', 'DiffusionPushLogListView' => 'applications/diffusion/view/DiffusionPushLogListView.php', 'DiffusionPythonExternalSymbolsSource' => 'applications/diffusion/symbol/DiffusionPythonExternalSymbolsSource.php', @@ -5864,6 +5867,7 @@ phutil_register_library_map(array( 'DiffusionLintCountQuery' => 'PhabricatorQuery', 'DiffusionLintSaveRunner' => 'Phobject', 'DiffusionLocalRepositoryFilter' => 'Phobject', + 'DiffusionLogController' => 'DiffusionController', 'DiffusionLookSoonConduitAPIMethod' => 'DiffusionConduitAPIMethod', 'DiffusionLowLevelCommitFieldsQuery' => 'DiffusionLowLevelQuery', 'DiffusionLowLevelCommitQuery' => 'DiffusionLowLevelQuery', @@ -5931,10 +5935,12 @@ phutil_register_library_map(array( 'DiffusionPreCommitRefTypeHeraldField' => 'DiffusionPreCommitRefHeraldField', 'DiffusionPreCommitUsesGitLFSHeraldField' => 'DiffusionPreCommitContentHeraldField', 'DiffusionPullEventGarbageCollector' => 'PhabricatorGarbageCollector', + 'DiffusionPullLogListController' => 'DiffusionLogController', + 'DiffusionPullLogListView' => 'AphrontView', + 'DiffusionPullLogSearchEngine' => 'PhabricatorApplicationSearchEngine', 'DiffusionPushCapability' => 'PhabricatorPolicyCapability', - 'DiffusionPushEventViewController' => 'DiffusionPushLogController', - 'DiffusionPushLogController' => 'DiffusionController', - 'DiffusionPushLogListController' => 'DiffusionPushLogController', + 'DiffusionPushEventViewController' => 'DiffusionLogController', + 'DiffusionPushLogListController' => 'DiffusionLogController', 'DiffusionPushLogListView' => 'AphrontView', 'DiffusionPythonExternalSymbolsSource' => 'DiffusionExternalSymbolsSource', 'DiffusionQuery' => 'PhabricatorQuery', diff --git a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php index 5797b8ba7c..d932149a75 100644 --- a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php +++ b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php @@ -124,6 +124,9 @@ final class PhabricatorDiffusionApplication extends PhabricatorApplication { '(?:query/(?P[^/]+)/)?' => 'DiffusionPushLogListController', 'view/(?P\d+)/' => 'DiffusionPushEventViewController', ), + 'pulllog/' => array( + $this->getQueryRoutePattern() => 'DiffusionPullLogListController', + ), '(?P[A-Z]+)' => $repository_routes, '(?P[1-9]\d*)' => $repository_routes, diff --git a/src/applications/diffusion/controller/DiffusionPushLogController.php b/src/applications/diffusion/controller/DiffusionLogController.php similarity index 54% rename from src/applications/diffusion/controller/DiffusionPushLogController.php rename to src/applications/diffusion/controller/DiffusionLogController.php index f974cc91cd..9e5a4eaa3d 100644 --- a/src/applications/diffusion/controller/DiffusionPushLogController.php +++ b/src/applications/diffusion/controller/DiffusionLogController.php @@ -1,6 +1,6 @@ setController($this) + ->buildResponse(); + } + +} diff --git a/src/applications/diffusion/controller/DiffusionPushEventViewController.php b/src/applications/diffusion/controller/DiffusionPushEventViewController.php index c5eb6368b4..21718c256d 100644 --- a/src/applications/diffusion/controller/DiffusionPushEventViewController.php +++ b/src/applications/diffusion/controller/DiffusionPushEventViewController.php @@ -1,11 +1,7 @@ getViewer(); diff --git a/src/applications/diffusion/controller/DiffusionPushLogListController.php b/src/applications/diffusion/controller/DiffusionPushLogListController.php index 5b58881470..658e21674d 100644 --- a/src/applications/diffusion/controller/DiffusionPushLogListController.php +++ b/src/applications/diffusion/controller/DiffusionPushLogListController.php @@ -1,10 +1,7 @@ isHosted()) { $push_uri = $this->getApplicationURI( - 'pushlog/?repositories='.$repository->getMonogram()); + 'pushlog/?repositories='.$repository->getPHID()); $action_view->addAction( id(new PhabricatorActionView()) @@ -374,6 +374,15 @@ final class DiffusionRepositoryController extends DiffusionController { ->setHref($push_uri)); } + $pull_uri = $this->getApplicationURI( + 'pulllog/?repositories='.$repository->getPHID()); + + $action_view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('View Pull Logs')) + ->setIcon('fa-list-alt') + ->setHref($pull_uri)); + return $action_view; } diff --git a/src/applications/diffusion/query/DiffusionPullLogSearchEngine.php b/src/applications/diffusion/query/DiffusionPullLogSearchEngine.php new file mode 100644 index 0000000000..85a540f6f5 --- /dev/null +++ b/src/applications/diffusion/query/DiffusionPullLogSearchEngine.php @@ -0,0 +1,85 @@ +newQuery(); + + if ($map['repositoryPHIDs']) { + $query->withRepositoryPHIDs($map['repositoryPHIDs']); + } + + if ($map['pullerPHIDs']) { + $query->withPullerPHIDs($map['pullerPHIDs']); + } + + return $query; + } + + protected function buildCustomSearchFields() { + return array( + id(new PhabricatorSearchDatasourceField()) + ->setDatasource(new DiffusionRepositoryDatasource()) + ->setKey('repositoryPHIDs') + ->setAliases(array('repository', 'repositories', 'repositoryPHID')) + ->setLabel(pht('Repositories')) + ->setDescription( + pht('Search for pull logs for specific repositories.')), + id(new PhabricatorUsersSearchField()) + ->setKey('pullerPHIDs') + ->setAliases(array('puller', 'pullers', 'pullerPHID')) + ->setLabel(pht('Pullers')) + ->setDescription( + pht('Search for pull logs by specific users.')), + ); + } + + protected function getURI($path) { + return '/diffusion/pulllog/'.$path; + } + + protected function getBuiltinQueryNames() { + return array( + 'all' => pht('All Pull Logs'), + ); + } + + public function buildSavedQueryFromBuiltin($query_key) { + $query = $this->newSavedQuery(); + $query->setQueryKey($query_key); + + switch ($query_key) { + case 'all': + return $query; + } + + return parent::buildSavedQueryFromBuiltin($query_key); + } + + protected function renderResultList( + array $logs, + PhabricatorSavedQuery $query, + array $handles) { + + $table = id(new DiffusionPullLogListView()) + ->setViewer($this->requireViewer()) + ->setLogs($logs); + + return id(new PhabricatorApplicationSearchResultView()) + ->setTable($table); + } + +} diff --git a/src/applications/diffusion/view/DiffusionPullLogListView.php b/src/applications/diffusion/view/DiffusionPullLogListView.php new file mode 100644 index 0000000000..7babb9c584 --- /dev/null +++ b/src/applications/diffusion/view/DiffusionPullLogListView.php @@ -0,0 +1,115 @@ +logs = $logs; + return $this; + } + + public function render() { + $events = $this->logs; + $viewer = $this->getViewer(); + + $handle_phids = array(); + foreach ($events as $event) { + if ($event->getPullerPHID()) { + $handle_phids[] = $event->getPullerPHID(); + } + } + $handles = $viewer->loadHandles($handle_phids); + + // Figure out which repositories are editable. We only let you see remote + // IPs if you have edit capability on a repository. + $editable_repos = array(); + if ($events) { + $editable_repos = id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->withPHIDs(mpull($events, 'getRepositoryPHID')) + ->execute(); + $editable_repos = mpull($editable_repos, null, 'getPHID'); + } + + $rows = array(); + $any_host = false; + foreach ($events as $event) { + if ($event->getRepositoryPHID()) { + $repository = $event->getRepository(); + } else { + $repository = null; + } + + // Reveal this if it's valid and the user can edit the repository. For + // invalid requests you currently have to go fishing in the database. + $remote_address = '-'; + if ($repository) { + if (isset($editable_repos[$event->getRepositoryPHID()])) { + $remote_address = $event->getRemoteAddress(); + } + } + + $event_id = $event->getID(); + + $repository_link = null; + if ($repository) { + $repository_link = phutil_tag( + 'a', + array( + 'href' => $repository->getURI(), + ), + $repository->getDisplayName()); + } + + $puller_link = null; + if ($event->getPullerPHID()) { + $puller_link = $viewer->renderHandle($event->getPullerPHID()); + } + + $rows[] = array( + $event_id, + $repository_link, + $puller_link, + $remote_address, + $event->getRemoteProtocolDisplayName(), + $event->newResultIcon(), + $event->getResultCode(), + phabricator_datetime($event->getEpoch(), $viewer), + ); + } + + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Pull'), + pht('Repository'), + pht('Puller'), + pht('From'), + pht('Via'), + null, + pht('Error'), + pht('Date'), + )) + ->setColumnClasses( + array( + 'n', + '', + '', + 'n', + 'wide', + '', + 'n', + 'right', + )); + + return $table; + } + +} diff --git a/src/applications/repository/query/PhabricatorRepositoryPullEventQuery.php b/src/applications/repository/query/PhabricatorRepositoryPullEventQuery.php index 1063cdb48c..af60ee0383 100644 --- a/src/applications/repository/query/PhabricatorRepositoryPullEventQuery.php +++ b/src/applications/repository/query/PhabricatorRepositoryPullEventQuery.php @@ -37,19 +37,35 @@ final class PhabricatorRepositoryPullEventQuery } protected function willFilterPage(array $events) { + // If a pull targets an invalid repository or fails before authenticating, + // it may not have an associated repository. + $repository_phids = mpull($events, 'getRepositoryPHID'); - $repositories = id(new PhabricatorRepositoryQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs($repository_phids) - ->execute(); - $repositories = mpull($repositories, null, 'getPHID'); + $repository_phids = array_filter($repository_phids); + + if ($repository_phids) { + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($repository_phids) + ->execute(); + $repositories = mpull($repositories, null, 'getPHID'); + } else { + $repositories = array(); + } foreach ($events as $key => $event) { $phid = $event->getRepositoryPHID(); - if (empty($repositories[$phid])) { - unset($events[$key]); + if (!$phid) { + $event->attachRepository(null); continue; } + + if (empty($repositories[$phid])) { + unset($events[$key]); + $this->didRejectResult($event); + continue; + } + $event->attachRepository($repositories[$phid]); } diff --git a/src/applications/repository/storage/PhabricatorRepositoryPullEvent.php b/src/applications/repository/storage/PhabricatorRepositoryPullEvent.php index c1227402d7..1b095ce8fd 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryPullEvent.php +++ b/src/applications/repository/storage/PhabricatorRepositoryPullEvent.php @@ -51,7 +51,7 @@ final class PhabricatorRepositoryPullEvent PhabricatorRepositoryPullEventPHIDType::TYPECONST); } - public function attachRepository(PhabricatorRepository $repository) { + public function attachRepository(PhabricatorRepository $repository = null) { $this->repository = $repository; return $this; } @@ -60,6 +60,38 @@ final class PhabricatorRepositoryPullEvent return $this->assertAttached($this->repository); } + public function getRemoteProtocolDisplayName() { + $map = array( + 'ssh' => pht('SSH'), + 'http' => pht('HTTP'), + ); + + $protocol = $this->getRemoteProtocol(); + + return idx($map, $protocol, $protocol); + } + + public function newResultIcon() { + $icon = new PHUIIconView(); + $type = $this->getResultType(); + + switch ($type) { + case 'wild': + $icon + ->setIcon('fa-question indigo') + ->setTooltip(pht('Unknown ("%s")', $type)); + break; + case 'pull': + $icon + ->setIcon('fa-download green') + ->setTooltip(pht('Pull')); + break; + } + + return $icon; + } + + /* -( PhabricatorPolicyInterface )----------------------------------------- */ @@ -71,10 +103,18 @@ final class PhabricatorRepositoryPullEvent } public function getPolicy($capability) { - return $this->getRepository()->getPolicy($capability); + if ($this->getRepository()) { + return $this->getRepository()->getPolicy($capability); + } + + return PhabricatorPolicies::POLICY_ADMIN; } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + if (!$this->getRepository()) { + return false; + } + return $this->getRepository()->hasAutomaticCapability($capability, $viewer); }