@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Policy - do proper policy queries when updating owners packages in commit workers
Summary: Ref T7094. This makes the underlying class take a $user parameter, and then the worker just hands it an omnipotent user. Said underyling class is the benefactor of a small re-factor, dropping one query per-use, though the single query that now remains is policy-based so maybe its a wash or even worse. Still, gotta love one less query.
Test Plan:
a little tricky to test so some extra thought instead
basic acceptance test with `bin/repository reparse --change rValidHashHere` -- it worked!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7094
Differential Revision: https://secure.phabricator.com/D11653
+22
-10
+19
-9
src/applications/repository/worker/commitchangeparser/PhabricatorOwnersPackagePathValidator.php
+19
-9
src/applications/repository/worker/commitchangeparser/PhabricatorOwnersPackagePathValidator.php
···
8
8
* paths.
9
9
*/
10
10
public static function updateOwnersPackagePaths(
11
-
PhabricatorRepositoryCommit $commit) {
12
-
$changes = self::loadDiffusionChangesForCommit($commit);
11
+
PhabricatorRepositoryCommit $commit,
12
+
PhabricatorUser $actor) {
13
+
14
+
$repository = id(new PhabricatorRepositoryQuery())
15
+
->setViewer($actor)
16
+
->withIDs(array($commit->getRepositoryID()))
17
+
->executeOne();
18
+
if (!$repository) {
19
+
return;
20
+
}
21
+
$changes = self::loadDiffusionChangesForCommit(
22
+
$repository,
23
+
$commit,
24
+
$actor);
13
25
14
26
if (!$changes) {
15
27
return;
16
28
}
17
29
18
-
// TODO: (T603) This should be policy-aware.
19
-
$repository =
20
-
id(new PhabricatorRepository())->load($commit->getRepositoryID());
21
30
$move_map = array();
22
31
foreach ($changes as $change) {
23
32
if ($change->getChangeType() == DifferentialChangeType::TYPE_MOVE_HERE) {
···
82
91
}
83
92
}
84
93
85
-
private static function loadDiffusionChangesForCommit($commit) {
86
-
$repository =
87
-
id(new PhabricatorRepository())->load($commit->getRepositoryID());
94
+
private static function loadDiffusionChangesForCommit(
95
+
PhabricatorRepository $repository,
96
+
PhabricatorRepositoryCommit $commit,
97
+
PhabricatorUser $actor) {
88
98
$data = array(
89
-
'user' => PhabricatorUser::getOmnipotentUser(),
99
+
'user' => $actor,
90
100
'repository' => $repository,
91
101
'commit' => $commit->getCommitIdentifier(),
92
102
);
+3
-1
src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
+3
-1
src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
···
96
96
id(new PhabricatorSearchIndexer())
97
97
->queueDocumentForIndexing($commit->getPHID());
98
98
99
-
PhabricatorOwnersPackagePathValidator::updateOwnersPackagePaths($commit);
99
+
PhabricatorOwnersPackagePathValidator::updateOwnersPackagePaths(
100
+
$commit,
101
+
PhabricatorUser::getOmnipotentUser());
100
102
if ($this->shouldQueueFollowupTasks()) {
101
103
$this->queueTask(
102
104
'PhabricatorRepositoryCommitOwnersWorker',