1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 00:32:42 +01:00

Add DifferentialDiffQuery and change most callsites

Summary:
Ref T603. This introduces a policy-aware DifferentialDiffQuery and converts most callsites.

I've left unusual callsites (mostly: hard to get the viewer, unusual query, queries related to active diffs) alone for now, so this isn't exhaustive but hits 60-80% of sites.

Test Plan: Created diff; created revision; viewed diffs and revisions; made additional conduit calls.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D6338
This commit is contained in:
epriestley 2013-07-01 12:38:42 -07:00
parent 328aa383e4
commit 90123dd739
14 changed files with 133 additions and 50 deletions

View file

@ -1,34 +0,0 @@
#!/usr/bin/env php
<?php
$root = dirname(dirname(dirname(__FILE__)));
require_once $root.'/scripts/__init_script__.php';
if ($argc != 3 || !is_numeric($argv[1]) || !is_numeric($argv[2])) {
echo "Usage: {$argv[0]} <diff_id_from> <diff_id_to>\n";
exit(1);
}
list(, $from, $to) = $argv;
for ($diff_id = $from; $diff_id <= $to; $diff_id++) {
echo "Processing $diff_id";
$diff = id(new DifferentialDiff())->load($diff_id);
if ($diff) {
$diff->attachChangesets($diff->loadChangesets());
$orig_copy = array();
foreach ($diff->getChangesets() as $i => $changeset) {
$orig_copy[$i] = idx((array)$changeset->getMetadata(), 'copy:lines');
$changeset->attachHunks($changeset->loadHunks());
}
$diff->detectCopiedCode();
foreach ($diff->getChangesets() as $i => $changeset) {
if (idx($changeset->getMetadata(), 'copy:lines') || $orig_copy[$i]) {
echo ".";
$changeset->save();
}
}
}
echo "\n";
}
echo "Done.\n";

View file

@ -345,6 +345,7 @@ phutil_register_library_map(array(
'DifferentialDiffContentMail' => 'applications/differential/mail/DifferentialDiffContentMail.php', 'DifferentialDiffContentMail' => 'applications/differential/mail/DifferentialDiffContentMail.php',
'DifferentialDiffCreateController' => 'applications/differential/controller/DifferentialDiffCreateController.php', 'DifferentialDiffCreateController' => 'applications/differential/controller/DifferentialDiffCreateController.php',
'DifferentialDiffProperty' => 'applications/differential/storage/DifferentialDiffProperty.php', 'DifferentialDiffProperty' => 'applications/differential/storage/DifferentialDiffProperty.php',
'DifferentialDiffQuery' => 'applications/differential/query/DifferentialDiffQuery.php',
'DifferentialDiffTableOfContentsView' => 'applications/differential/view/DifferentialDiffTableOfContentsView.php', 'DifferentialDiffTableOfContentsView' => 'applications/differential/view/DifferentialDiffTableOfContentsView.php',
'DifferentialDiffTestCase' => 'applications/differential/storage/__tests__/DifferentialDiffTestCase.php', 'DifferentialDiffTestCase' => 'applications/differential/storage/__tests__/DifferentialDiffTestCase.php',
'DifferentialDiffViewController' => 'applications/differential/controller/DifferentialDiffViewController.php', 'DifferentialDiffViewController' => 'applications/differential/controller/DifferentialDiffViewController.php',
@ -2245,10 +2246,15 @@ phutil_register_library_map(array(
'DifferentialDefaultFieldSelector' => 'DifferentialFieldSelector', 'DifferentialDefaultFieldSelector' => 'DifferentialFieldSelector',
'DifferentialDependenciesFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialDependenciesFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialDependsOnFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialDependsOnFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialDiff' => 'DifferentialDAO', 'DifferentialDiff' =>
array(
0 => 'DifferentialDAO',
1 => 'PhabricatorPolicyInterface',
),
'DifferentialDiffContentMail' => 'DifferentialMail', 'DifferentialDiffContentMail' => 'DifferentialMail',
'DifferentialDiffCreateController' => 'DifferentialController', 'DifferentialDiffCreateController' => 'DifferentialController',
'DifferentialDiffProperty' => 'DifferentialDAO', 'DifferentialDiffProperty' => 'DifferentialDAO',
'DifferentialDiffQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'DifferentialDiffTableOfContentsView' => 'AphrontView', 'DifferentialDiffTableOfContentsView' => 'AphrontView',
'DifferentialDiffTestCase' => 'ArcanistPhutilTestCase', 'DifferentialDiffTestCase' => 'ArcanistPhutilTestCase',
'DifferentialDiffViewController' => 'DifferentialController', 'DifferentialDiffViewController' => 'DifferentialController',

View file

@ -33,7 +33,10 @@ final class ConduitAPI_differential_createrevision_Method
protected function execute(ConduitAPIRequest $request) { protected function execute(ConduitAPIRequest $request) {
$fields = $request->getValue('fields'); $fields = $request->getValue('fields');
$diff = id(new DifferentialDiff())->load($request->getValue('diffid')); $diff = id(new DifferentialDiffQuery())
->setViewer($request->getUser())
->withIDs(array($request->getValue('diffid')))
->executeOne();
if (!$diff) { if (!$diff) {
throw new ConduitException('ERR_BAD_DIFF'); throw new ConduitException('ERR_BAD_DIFF');
} }

View file

@ -35,7 +35,10 @@ final class ConduitAPI_differential_finishpostponedlinters_Method
$diff_id = $request->getValue('diffID'); $diff_id = $request->getValue('diffID');
$linter_map = $request->getValue('linters'); $linter_map = $request->getValue('linters');
$diff = id(new DifferentialDiff())->load($diff_id); $diff = id(new DifferentialDiffQuery())
->setViewer($request->getUser())
->withIDs(array($diff_id))
->executeOne();
if (!$diff) { if (!$diff) {
throw new ConduitException('ERR-BAD-DIFF'); throw new ConduitException('ERR-BAD-DIFF');
} }

View file

@ -32,9 +32,10 @@ final class ConduitAPI_differential_getalldiffs_Method
return $results; return $results;
} }
$diffs = id(new DifferentialDiff())->loadAllWhere( $diffs = id(new DifferentialDiffQuery())
'revisionID IN (%Ld)', ->setViewer($request->getUser())
$revision_ids); ->withRevisionIDs($revision_ids)
->execute();
foreach ($diffs as $diff) { foreach ($diffs as $diff) {
$results[] = array( $results[] = array(

View file

@ -30,7 +30,10 @@ final class ConduitAPI_differential_getcommitpaths_Method
protected function execute(ConduitAPIRequest $request) { protected function execute(ConduitAPIRequest $request) {
$id = $request->getValue('revision_id'); $id = $request->getValue('revision_id');
$revision = id(new DifferentialRevision())->load($id); $revision = id(new DifferentialRevisionQuery())
->setViewer($request->getUser())
->withIDs(array($id))
->executeOne();
if (!$revision) { if (!$revision) {
throw new ConduitException('ERR_NOT_FOUND'); throw new ConduitException('ERR_NOT_FOUND');
} }

View file

@ -46,7 +46,10 @@ final class ConduitAPI_differential_getdiff_Method extends ConduitAPIMethod {
} else { } else {
$diff_id = $request->getValue('diff_id'); $diff_id = $request->getValue('diff_id');
if ($diff_id) { if ($diff_id) {
$diff = id(new DifferentialDiff())->load($diff_id); $diff = id(new DifferentialDiffQuery())
->setViewer($request->getUser())
->withIDs(array($diff_id))
->executeOne();
} }
} }

View file

@ -33,12 +33,18 @@ final class ConduitAPI_differential_updaterevision_Method
} }
protected function execute(ConduitAPIRequest $request) { protected function execute(ConduitAPIRequest $request) {
$diff = id(new DifferentialDiff())->load($request->getValue('diffid')); $diff = id(new DifferentialDiffQuery())
->setViewer($request->getUser())
->withIDs(array($request->getValue('diffid')))
->executeOne();
if (!$diff) { if (!$diff) {
throw new ConduitException('ERR_BAD_DIFF'); throw new ConduitException('ERR_BAD_DIFF');
} }
$revision = id(new DifferentialRevision())->load($request->getValue('id')); $revision = id(new DifferentialRevisionQuery())
->setViewer($request->getUser())
->withIDs(array($request->getValue('id')))
->executeOne();
if (!$revision) { if (!$revision) {
throw new ConduitException('ERR_BAD_REVISION'); throw new ConduitException('ERR_BAD_REVISION');
} }

View file

@ -56,7 +56,11 @@ final class ConduitAPI_differential_updateunitresults_Method
throw new ConduitException('ERR_NO_RESULTS'); throw new ConduitException('ERR_NO_RESULTS');
} }
$diff = id(new DifferentialDiff())->load($diff_id); $diff = id(new DifferentialDiffQuery())
->setViewer($request->getUser())
->withIDs(array($diff_id))
->executeOne();
$unit_results = $diff_property->getData(); $unit_results = $diff_property->getData();
$postponed_count = 0; $postponed_count = 0;
$unit_status = null; $unit_status = null;

View file

@ -12,7 +12,10 @@ final class DifferentialDiffViewController extends DifferentialController {
$request = $this->getRequest(); $request = $this->getRequest();
$viewer = $request->getUser(); $viewer = $request->getUser();
$diff = id(new DifferentialDiff())->load($this->id); $diff = id(new DifferentialDiffQuery())
->setViewer($viewer)
->withIDs(array($this->id))
->executeOne();
if (!$diff) { if (!$diff) {
return new Aphront404Response(); return new Aphront404Response();
} }

View file

@ -9,15 +9,18 @@ final class DifferentialRevisionEditController extends DifferentialController {
} }
public function processRequest() { public function processRequest() {
$request = $this->getRequest(); $request = $this->getRequest();
$viewer = $request->getUser();
if (!$this->id) { if (!$this->id) {
$this->id = $request->getInt('revisionID'); $this->id = $request->getInt('revisionID');
} }
if ($this->id) { if ($this->id) {
$revision = id(new DifferentialRevision())->load($this->id); $revision = id(new DifferentialRevisionQuery())
->setViewer($viewer)
->withIDs(array($this->id))
->executeOne();
if (!$revision) { if (!$revision) {
return new Aphront404Response(); return new Aphront404Response();
} }
@ -30,7 +33,10 @@ final class DifferentialRevisionEditController extends DifferentialController {
$diff_id = $request->getInt('diffID'); $diff_id = $request->getInt('diffID');
if ($diff_id) { if ($diff_id) {
$diff = id(new DifferentialDiff())->load($diff_id); $diff = id(new DifferentialDiffQuery())
->setViewer($viewer)
->withIDs(array($diff_id))
->executeOne();
if (!$diff) { if (!$diff) {
return new Aphront404Response(); return new Aphront404Response();
} }

View file

@ -0,0 +1,55 @@
<?php
final class DifferentialDiffQuery
extends PhabricatorCursorPagedPolicyAwareQuery {
private $ids;
private $revisionIDs;
public function withIDs(array $ids) {
$this->ids = $ids;
return $this;
}
public function withRevisionIDs(array $revision_ids) {
$this->revisionIDs = $revision_ids;
return $this;
}
public function loadPage() {
$table = new DifferentialDiff();
$conn_r = $table->establishConnection('r');
$data = queryfx_all(
$conn_r,
'SELECT * FROM %T %Q %Q %Q',
$table->getTableName(),
$this->buildWhereClause($conn_r),
$this->buildOrderClause($conn_r),
$this->buildLimitClause($conn_r));
return $table->loadAllFromArray($data);
}
private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
$where = array();
if ($this->ids) {
$where[] = qsprintf(
$conn_r,
'id IN (%Ld)',
$this->ids);
}
if ($this->revisionIDs) {
$where[] = qsprintf(
$conn_r,
'revisionID IN (%Ld)',
$this->revisionIDs);
}
$where[] = $this->buildPagingClause($conn_r);
return $this->formatWhereClause($where);
}
}

View file

@ -413,6 +413,10 @@ final class DifferentialRevisionQuery {
return $revisions; return $revisions;
} }
public function executeOne() {
return head($this->execute());
}
/** /**
* Determine if we should execute an optimized, fast-path query to fetch * Determine if we should execute an optimized, fast-path query to fetch

View file

@ -1,6 +1,8 @@
<?php <?php
final class DifferentialDiff extends DifferentialDAO { final class DifferentialDiff
extends DifferentialDAO
implements PhabricatorPolicyInterface {
protected $revisionID; protected $revisionID;
protected $authorPHID; protected $authorPHID;
@ -258,4 +260,22 @@ final class DifferentialDiff extends DifferentialDAO {
return $dict; return $dict;
} }
/* -( PhabricatorPolicyInterface )----------------------------------------- */
public function getCapabilities() {
return array(
PhabricatorPolicyCapability::CAN_VIEW,
);
}
public function getPolicy($capability) {
return PhabricatorPolicies::POLICY_USER;
}
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
return false;
}
} }