Skip to content
This repository was archived by the owner on Apr 7, 2022. It is now read-only.

Commit 2ef5b53

Browse files
author
epriestley
committed
Move Drydock logs to PHIDs and increased structure
Summary: Ref T9252. Several general changes here: - Moves logs to use PHIDs instead of IDs. This generally improves flexibility (for example, it's a lot easier to render handles). - Adds `blueprintPHID` to logs. Although you can usually figure this out from the leasePHID or resourcePHID, it lets us query relevant logs on Blueprint views. - Instead of making logs a top-level object, make them strictly a sub-object of Blueprints, Resources and Leases. So you go Drydock > Lease > Logs, etc., to get to logs. - I might restore the "everything" view eventually, but it doesn't interact well with policies and I'm not sure it's very useful. A policy-violating `bin/drydock log` might be cleaner. - Policy-wise, we always show you that logs exist, we just don't show you log content if it's about something you can't see. This is similar to seeing restricted handles in other applications. - Instead of just having a message, give logs "type" + "data". This will let logs be more structured and translatable. This is similar to recent changes to Herald which seem to have worked well. Test Plan: Added some placeholder log writes, viewed those logs in the UI. {F855199} Reviewers: chad Reviewed By: chad Maniphest Tasks: T9252 Differential Revision: https://secure.phabricator.com/D14196
1 parent 98006f2 commit 2ef5b53

14 files changed

+484
-248
lines changed

‎resources/sql/autopatches/20150930.drydock.log.1.sql‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
TRUNCATE {$NAMESPACE}_drydock.drydock_log;
2+
3+
ALTER TABLE {$NAMESPACE}_drydock.drydock_log
4+
DROP resourceID;
5+
6+
ALTER TABLE {$NAMESPACE}_drydock.drydock_log
7+
DROP leaseID;
8+
9+
ALTER TABLE {$NAMESPACE}_drydock.drydock_log
10+
DROP message;
11+
12+
ALTER TABLE {$NAMESPACE}_drydock.drydock_log
13+
ADD blueprintPHID VARBINARY(64);
14+
15+
ALTER TABLE {$NAMESPACE}_drydock.drydock_log
16+
ADD resourcePHID VARBINARY(64);
17+
18+
ALTER TABLE {$NAMESPACE}_drydock.drydock_log
19+
ADD leasePHID VARBINARY(64);
20+
21+
ALTER TABLE {$NAMESPACE}_drydock.drydock_log
22+
ADD type VARCHAR(64) NOT NULL COLLATE {$COLLATE_TEXT};
23+
24+
ALTER TABLE {$NAMESPACE}_drydock.drydock_log
25+
ADD data LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};

‎src/applications/drydock/application/PhabricatorDrydockApplication.php‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,37 +47,40 @@ public function getRoutes() {
4747
return array(
4848
'/drydock/' => array(
4949
'' => 'DrydockConsoleController',
50-
'blueprint/' => array(
50+
'(?P<type>blueprint)/' => array(
5151
'(?:query/(?P<queryKey>[^/]+)/)?' => 'DrydockBlueprintListController',
5252
'(?P<id>[1-9]\d*)/' => array(
5353
'' => 'DrydockBlueprintViewController',
5454
'(?P<action>disable|enable)/' =>
5555
'DrydockBlueprintDisableController',
5656
'resources/(?:query/(?P<queryKey>[^/]+)/)?' =>
5757
'DrydockResourceListController',
58+
'logs/(?:query/(?P<queryKey>[^/]+)/)?' =>
59+
'DrydockLogListController',
5860
),
5961
'create/' => 'DrydockBlueprintCreateController',
6062
'edit/(?:(?P<id>[1-9]\d*)/)?' => 'DrydockBlueprintEditController',
6163
),
62-
'resource/' => array(
64+
'(?P<type>resource)/' => array(
6365
'(?:query/(?P<queryKey>[^/]+)/)?' => 'DrydockResourceListController',
6466
'(?P<id>[1-9]\d*)/' => array(
6567
'' => 'DrydockResourceViewController',
6668
'release/' => 'DrydockResourceReleaseController',
6769
'leases/(?:query/(?P<queryKey>[^/]+)/)?' =>
6870
'DrydockLeaseListController',
71+
'logs/(?:query/(?P<queryKey>[^/]+)/)?' =>
72+
'DrydockLogListController',
6973
),
7074
),
71-
'lease/' => array(
75+
'(?P<type>lease)/' => array(
7276
'(?:query/(?P<queryKey>[^/]+)/)?' => 'DrydockLeaseListController',
7377
'(?P<id>[1-9]\d*)/' => array(
7478
'' => 'DrydockLeaseViewController',
7579
'release/' => 'DrydockLeaseReleaseController',
80+
'logs/(?:query/(?P<queryKey>[^/]+)/)?' =>
81+
'DrydockLogListController',
7682
),
7783
),
78-
'log/' => array(
79-
'(?:query/(?P<queryKey>[^/]+)/)?' => 'DrydockLogListController',
80-
),
8184
),
8285
);
8386
}

‎src/applications/drydock/blueprint/DrydockBlueprintImplementation.php‎

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -250,46 +250,6 @@ abstract public function getInterface(
250250
/* -( Logging )------------------------------------------------------------ */
251251

252252

253-
/**
254-
* @task log
255-
*/
256-
protected function logException(Exception $ex) {
257-
$this->log($ex->getMessage());
258-
}
259-
260-
261-
/**
262-
* @task log
263-
*/
264-
protected function log($message) {
265-
self::writeLog(null, null, $message);
266-
}
267-
268-
269-
/**
270-
* @task log
271-
*/
272-
public static function writeLog(
273-
DrydockResource $resource = null,
274-
DrydockLease $lease = null,
275-
$message = null) {
276-
277-
$log = id(new DrydockLog())
278-
->setEpoch(time())
279-
->setMessage($message);
280-
281-
if ($resource) {
282-
$log->setResourceID($resource->getID());
283-
}
284-
285-
if ($lease) {
286-
$log->setLeaseID($lease->getID());
287-
}
288-
289-
$log->save();
290-
}
291-
292-
293253
public static function getAllBlueprintImplementations() {
294254
return id(new PhutilClassMapQuery())
295255
->setAncestorClass(__CLASS__)

‎src/applications/drydock/controller/DrydockBlueprintViewController.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,19 @@ public function handleRequest(AphrontRequest $request) {
5656
new DrydockBlueprintTransactionQuery());
5757
$timeline->setShouldTerminate(true);
5858

59+
$log_query = id(new DrydockLogQuery())
60+
->withBlueprintPHIDs(array($blueprint->getPHID()));
61+
62+
$log_box = $this->buildLogBox(
63+
$log_query,
64+
$this->getApplicationURI("blueprint/{$id}/logs/query/all/"));
65+
5966
return $this->buildApplicationPage(
6067
array(
6168
$crumbs,
6269
$object_box,
6370
$resource_box,
71+
$log_box,
6472
$timeline,
6573
),
6674
array(

‎src/applications/drydock/controller/DrydockController.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,31 @@ protected function buildCommandsTab($target_phid) {
8585
->addRawContent($table);
8686
}
8787

88+
protected function buildLogBox(DrydockLogQuery $query, $all_uri) {
89+
$viewer = $this->getViewer();
90+
91+
$logs = $query
92+
->setViewer($viewer)
93+
->setLimit(100)
94+
->execute();
95+
96+
$log_table = id(new DrydockLogListView())
97+
->setUser($viewer)
98+
->setLogs($logs)
99+
->render();
100+
101+
$log_header = id(new PHUIHeaderView())
102+
->setHeader(pht('Logs'))
103+
->addActionLink(
104+
id(new PHUIButtonView())
105+
->setTag('a')
106+
->setHref($all_uri)
107+
->setIconFont('fa-search')
108+
->setText(pht('View All Logs')));
109+
110+
return id(new PHUIObjectBoxView())
111+
->setHeader($log_header)
112+
->setTable($log_table);
113+
}
114+
88115
}

‎src/applications/drydock/controller/DrydockLeaseViewController.php‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ public function handleRequest(AphrontRequest $request) {
1515
return new Aphront404Response();
1616
}
1717

18-
$lease_uri = $this->getApplicationURI('lease/'.$lease->getID().'/');
18+
$id = $lease->getID();
19+
$lease_uri = $this->getApplicationURI("lease/{$id}/");
1920

2021
$title = pht('Lease %d', $lease->getID());
2122

@@ -29,20 +30,12 @@ public function handleRequest(AphrontRequest $request) {
2930
$actions = $this->buildActionListView($lease);
3031
$properties = $this->buildPropertyListView($lease, $actions);
3132

32-
$pager = new PHUIPagerView();
33-
$pager->setURI(new PhutilURI($lease_uri), 'offset');
34-
$pager->setOffset($request->getInt('offset'));
33+
$log_query = id(new DrydockLogQuery())
34+
->withLeasePHIDs(array($lease->getPHID()));
3535

36-
$logs = id(new DrydockLogQuery())
37-
->setViewer($viewer)
38-
->withLeaseIDs(array($lease->getID()))
39-
->executeWithOffsetPager($pager);
40-
41-
$log_table = id(new DrydockLogListView())
42-
->setUser($viewer)
43-
->setLogs($logs)
44-
->render();
45-
$log_table->appendChild($pager);
36+
$log_box = $this->buildLogBox(
37+
$log_query,
38+
$this->getApplicationURI("lease/{$id}/logs/query/all/"));
4639

4740
$crumbs = $this->buildApplicationCrumbs();
4841
$crumbs->addTextCrumb($title, $lease_uri);
@@ -56,10 +49,6 @@ public function handleRequest(AphrontRequest $request) {
5649
->addPropertyList($locks, pht('Slot Locks'))
5750
->addPropertyList($commands, pht('Commands'));
5851

59-
$log_box = id(new PHUIObjectBoxView())
60-
->setHeaderText(pht('Lease Logs'))
61-
->setTable($log_table);
62-
6352
return $this->buildApplicationPage(
6453
array(
6554
$crumbs,

‎src/applications/drydock/controller/DrydockLogController.php‎

Lines changed: 98 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,60 @@
33
abstract class DrydockLogController
44
extends DrydockController {
55

6+
private $blueprint;
7+
private $resource;
8+
private $lease;
9+
10+
public function setBlueprint(DrydockBlueprint $blueprint) {
11+
$this->blueprint = $blueprint;
12+
return $this;
13+
}
14+
15+
public function getBlueprint() {
16+
return $this->blueprint;
17+
}
18+
19+
public function setResource(DrydockResource $resource) {
20+
$this->resource = $resource;
21+
return $this;
22+
}
23+
24+
public function getResource() {
25+
return $this->resource;
26+
}
27+
28+
public function setLease(DrydockLease $lease) {
29+
$this->lease = $lease;
30+
return $this;
31+
}
32+
33+
public function getLease() {
34+
return $this->lease;
35+
}
36+
637
public function buildSideNavView() {
738
$nav = new AphrontSideNavFilterView();
839
$nav->setBaseURI(new PhutilURI($this->getApplicationURI()));
940

10-
id(new DrydockLogSearchEngine())
11-
->setViewer($this->getRequest()->getUser())
12-
->addNavigationItems($nav->getMenu());
41+
$engine = id(new DrydockLogSearchEngine())
42+
->setViewer($this->getRequest()->getUser());
43+
44+
$blueprint = $this->getBlueprint();
45+
if ($blueprint) {
46+
$engine->setBlueprint($blueprint);
47+
}
48+
49+
$resource = $this->getResource();
50+
if ($resource) {
51+
$engine->setResource($resource);
52+
}
53+
54+
$lease = $this->getLease();
55+
if ($lease) {
56+
$engine->setLease($lease);
57+
}
58+
59+
$engine->addNavigationItems($nav->getMenu());
1360

1461
$nav->selectFilter(null);
1562

@@ -18,9 +65,54 @@ public function buildSideNavView() {
1865

1966
protected function buildApplicationCrumbs() {
2067
$crumbs = parent::buildApplicationCrumbs();
21-
$crumbs->addTextCrumb(
22-
pht('Logs'),
23-
$this->getApplicationURI('log/'));
68+
69+
$blueprint = $this->getBlueprint();
70+
$resource = $this->getResource();
71+
$lease = $this->getLease();
72+
if ($blueprint) {
73+
$id = $blueprint->getID();
74+
75+
$crumbs->addTextCrumb(
76+
pht('Blueprints'),
77+
$this->getApplicationURI('blueprint/'));
78+
79+
$crumbs->addTextCrumb(
80+
$blueprint->getBlueprintName(),
81+
$this->getApplicationURI("blueprint/{$id}/"));
82+
83+
$crumbs->addTextCrumb(
84+
pht('Logs'),
85+
$this->getApplicationURI("blueprint/{$id}/logs/"));
86+
} else if ($resource) {
87+
$id = $resource->getID();
88+
89+
$crumbs->addTextCrumb(
90+
pht('Resources'),
91+
$this->getApplicationURI('resource/'));
92+
93+
$crumbs->addTextCrumb(
94+
$resource->getName(),
95+
$this->getApplicationURI("resource/{$id}/"));
96+
97+
$crumbs->addTextCrumb(
98+
pht('Logs'),
99+
$this->getApplicationURI("resource/{$id}/logs/"));
100+
} else if ($lease) {
101+
$id = $lease->getID();
102+
103+
$crumbs->addTextCrumb(
104+
pht('Leases'),
105+
$this->getApplicationURI('lease/'));
106+
107+
$crumbs->addTextCrumb(
108+
$lease->getLeaseName(),
109+
$this->getApplicationURI("lease/{$id}/"));
110+
111+
$crumbs->addTextCrumb(
112+
pht('Logs'),
113+
$this->getApplicationURI("lease/{$id}/logs/"));
114+
}
115+
24116
return $crumbs;
25117
}
26118

‎src/applications/drydock/controller/DrydockLogListController.php‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,53 @@ public function shouldAllowPublic() {
88

99
public function handleRequest(AphrontRequest $request) {
1010
$viewer = $request->getViewer();
11-
$querykey = $request->getURIData('queryKey');
11+
$engine = new DrydockLogSearchEngine();
12+
13+
$id = $request->getURIData('id');
14+
$type = $request->getURIData('type');
15+
switch ($type) {
16+
case 'blueprint':
17+
$blueprint = id(new DrydockBlueprintQuery())
18+
->setViewer($viewer)
19+
->withIDs(array($id))
20+
->executeOne();
21+
if (!$blueprint) {
22+
return new Aphront404Response();
23+
}
24+
$engine->setBlueprint($blueprint);
25+
$this->setBlueprint($blueprint);
26+
break;
27+
case 'resource':
28+
$resource = id(new DrydockResourceQuery())
29+
->setViewer($viewer)
30+
->withIDs(array($id))
31+
->executeOne();
32+
if (!$resource) {
33+
return new Aphront404Response();
34+
}
35+
$engine->setResource($resource);
36+
$this->setResource($resource);
37+
break;
38+
case 'lease':
39+
$lease = id(new DrydockLeaseQuery())
40+
->setViewer($viewer)
41+
->withIDs(array($id))
42+
->executeOne();
43+
if (!$lease) {
44+
return new Aphront404Response();
45+
}
46+
$engine->setLease($lease);
47+
$this->setLease($lease);
48+
break;
49+
default:
50+
return new Aphront404Response();
51+
}
52+
53+
$query_key = $request->getURIData('queryKey');
1254

1355
$controller = id(new PhabricatorApplicationSearchController())
14-
->setQueryKey($querykey)
15-
->setSearchEngine(new DrydockLogSearchEngine())
56+
->setQueryKey($query_key)
57+
->setSearchEngine($engine)
1658
->setNavigation($this->buildSideNavView());
1759

1860
return $this->delegateToController($controller);

0 commit comments

Comments
 (0)