Page MenuHomePhabricator
Files F15471904

D9289.diff
No OneTemporary
Actions

D9289.diff
View Options

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
@@ -343,6 +343,7 @@
'DifferentialChangesetOneUpTestRenderer' => 'applications/differential/render/DifferentialChangesetOneUpTestRenderer.php',
'DifferentialChangesetParser' => 'applications/differential/parser/DifferentialChangesetParser.php',
'DifferentialChangesetParserTestCase' => 'applications/differential/parser/__tests__/DifferentialChangesetParserTestCase.php',
+ 'DifferentialChangesetQuery' => 'applications/differential/query/DifferentialChangesetQuery.php',
'DifferentialChangesetRenderer' => 'applications/differential/render/DifferentialChangesetRenderer.php',
'DifferentialChangesetTestRenderer' => 'applications/differential/render/DifferentialChangesetTestRenderer.php',
'DifferentialChangesetTwoUpRenderer' => 'applications/differential/render/DifferentialChangesetTwoUpRenderer.php',
@@ -3018,6 +3019,7 @@
'DifferentialChangesetOneUpRenderer' => 'DifferentialChangesetHTMLRenderer',
'DifferentialChangesetOneUpTestRenderer' => 'DifferentialChangesetTestRenderer',
'DifferentialChangesetParserTestCase' => 'PhabricatorTestCase',
+ 'DifferentialChangesetQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'DifferentialChangesetTestRenderer' => 'DifferentialChangesetRenderer',
'DifferentialChangesetTwoUpRenderer' => 'DifferentialChangesetHTMLRenderer',
'DifferentialChangesetTwoUpTestRenderer' => 'DifferentialChangesetTestRenderer',
diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php
--- a/src/applications/differential/controller/DifferentialChangesetViewController.php
+++ b/src/applications/differential/controller/DifferentialChangesetViewController.php
@@ -23,25 +23,33 @@
$id = (int)$id;
$vs = (int)$vs;
- $changeset = id(new DifferentialChangeset())->load($id);
- if (!$changeset) {
- return new Aphront404Response();
+ $load_ids = array($id);
+ if ($vs && ($vs != -1)) {
+ $load_ids[] = $vs;
}
- // TODO: (T603) Make Changeset policy-aware. For now, just fake it
- // by making sure we can see the diff.
- $diff = id(new DifferentialDiffQuery())
+ $changesets = id(new DifferentialChangesetQuery())
->setViewer($request->getUser())
- ->withIDs(array($changeset->getDiffID()))
- ->executeOne();
- if (!$diff) {
+ ->withIDs($load_ids)
+ ->needHunks(true)
+ ->execute();
+ $changesets = mpull($changesets, null, 'getID');
+
+ $changeset = idx($changesets, $id);
+ if (!$changeset) {
return new Aphront404Response();
}
+ $vs_changeset = null;
+ if ($vs && ($vs != -1)) {
+ $vs_changeset = idx($changesets, $vs);
+ if (!$vs_changeset) {
+ return new Aphront404Response();
+ }
+ }
$view = $request->getStr('view');
if ($view) {
- $changeset->attachHunks($changeset->loadHunks());
$phid = idx($changeset->getMetadata(), "$view:binary-phid");
if ($phid) {
return id(new AphrontRedirectResponse())->setURI("/file/info/$phid/");
@@ -50,12 +58,8 @@
case 'new':
return $this->buildRawFileResponse($changeset, $is_new = true);
case 'old':
- if ($vs && ($vs != -1)) {
- $vs_changeset = id(new DifferentialChangeset())->load($vs);
- if ($vs_changeset) {
- $vs_changeset->attachHunks($vs_changeset->loadHunks());
- return $this->buildRawFileResponse($vs_changeset, $is_new = true);
- }
+ if ($vs_changeset) {
+ return $this->buildRawFileResponse($vs_changeset, $is_new = true);
}
return $this->buildRawFileResponse($changeset, $is_new = false);
default:
@@ -63,13 +67,6 @@
}
}
- if ($vs && ($vs != -1)) {
- $vs_changeset = id(new DifferentialChangeset())->load($vs);
- if (!$vs_changeset) {
- return new Aphront404Response();
- }
- }
-
if (!$vs) {
$right = $changeset;
$left = null;
@@ -103,15 +100,6 @@
}
if ($left) {
- $left->attachHunks($left->loadHunks());
- }
-
- if ($right) {
- $right->attachHunks($right->loadHunks());
- }
-
- if ($left) {
-
$left_data = $left->makeNewFile();
if ($right) {
$right_data = $right->makeNewFile();
@@ -264,9 +252,30 @@
),
$detail->render()));
+ $crumbs = $this->buildApplicationCrumbs();
+
+ $revision_id = $changeset->getDiff()->getRevisionID();
+ if ($revision_id) {
+ $crumbs->addTextCrumb('D'.$revision_id, '/D'.$revision_id);
+ }
+
+ $diff_id = $changeset->getDiff()->getID();
+ if ($diff_id) {
+ $crumbs->addTextCrumb(
+ pht('Diff %d', $diff_id),
+ $this->getApplicationURI('diff/'.$diff_id));
+ }
+
+ $crumbs->addTextCrumb($changeset->getDisplayFilename());
+
+ $box = id(new PHUIObjectBoxView())
+ ->setHeaderText(pht('Standalone View'))
+ ->appendChild($panel);
+
return $this->buildApplicationPage(
array(
- $panel
+ $crumbs,
+ $box,
),
array(
'title' => pht('Changeset View'),
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -844,9 +844,11 @@
$viewer = $this->getRequest()->getUser();
- foreach ($changesets as $changeset) {
- $changeset->attachHunks($changeset->loadHunks());
- }
+ id(new DifferentialHunkQuery())
+ ->setViewer($viewer)
+ ->withChangesets($changesets)
+ ->needAttachToChangesets(true)
+ ->execute();
$diff = new DifferentialDiff();
$diff->attachChangesets($changesets);
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -1284,15 +1284,13 @@
$changeset_ids[$id] = $id;
}
- // TODO: We should write a proper Query class for this eventually.
- $changesets = id(new DifferentialChangeset())->loadAllWhere(
- 'id IN (%Ld)',
- $changeset_ids);
if ($show_context) {
$hunk_parser = new DifferentialHunkParser();
- foreach ($changesets as $changeset) {
- $changeset->attachHunks($changeset->loadHunks());
- }
+ $changesets = id(new DifferentialChangesetQuery())
+ ->setViewer($this->getActor())
+ ->withIDs($changeset_ids)
+ ->needHunks(true)
+ ->execute();
}
$inline_groups = DifferentialTransactionComment::sortAndGroupInlines(
diff --git a/src/applications/differential/query/DifferentialChangesetQuery.php b/src/applications/differential/query/DifferentialChangesetQuery.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/query/DifferentialChangesetQuery.php
@@ -0,0 +1,158 @@
+<?php
+
+final class DifferentialChangesetQuery
+ extends PhabricatorCursorPagedPolicyAwareQuery {
+
+ private $ids;
+ private $diffs;
+
+ private $needAttachToDiffs;
+ private $needHunks;
+
+ public function withIDs(array $ids) {
+ $this->ids = $ids;
+ return $this;
+ }
+
+ public function withDiffs(array $diffs) {
+ assert_instances_of($diffs, 'DifferentialDiff');
+ $this->diffs = $diffs;
+ return $this;
+ }
+
+ public function needAttachToDiffs($attach) {
+ $this->needAttachToDiffs = $attach;
+ return $this;
+ }
+
+ public function needHunks($need) {
+ $this->needHunks = $need;
+ return $this;
+ }
+
+ public function willExecute() {
+ // If we fail to load any changesets (which is possible in the case of an
+ // empty commit) we'll never call didFilterPage(). Attach empty changeset
+ // lists now so that we end up with the right result.
+ if ($this->needAttachToDiffs) {
+ foreach ($this->diffs as $diff) {
+ $diff->attachChangesets(array());
+ }
+ }
+ }
+
+ public function loadPage() {
+ $table = new DifferentialChangeset();
+ $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);
+ }
+
+ public function willFilterPage(array $changesets) {
+
+ // First, attach all the diffs we already have. We can just do this
+ // directly without worrying about querying for them. When we don't have
+ // a diff, record that we need to load it.
+ if ($this->diffs) {
+ $have_diffs = mpull($this->diffs, null, 'getID');
+ } else {
+ $have_diffs = array();
+ }
+
+ $must_load = array();
+ foreach ($changesets as $key => $changeset) {
+ $diff_id = $changeset->getDiffID();
+ if (isset($have_diffs[$diff_id])) {
+ $changeset->attachDiff($have_diffs[$diff_id]);
+ } else {
+ $must_load[$key] = $changeset;
+ }
+ }
+
+ // Load all the diffs we don't have.
+ $need_diff_ids = mpull($must_load, 'getDiffID');
+ $more_diffs = array();
+ if ($need_diff_ids) {
+ $more_diffs = id(new DifferentialDiffQuery())
+ ->setViewer($this->getViewer())
+ ->setParentQuery($this)
+ ->withIDs($need_diff_ids)
+ ->execute();
+ $more_diffs = mpull($more_diffs, null, 'getID');
+ }
+
+ // Attach the diffs we loaded.
+ foreach ($must_load as $key => $changeset) {
+ $diff_id = $changeset->getDiffID();
+ if (isset($more_diffs[$diff_id])) {
+ $changeset->attachDiff($more_diffs[$diff_id]);
+ } else {
+ // We didn't have the diff, and could not load it (it does not exist,
+ // or we can't see it), so filter this result out.
+ unset($changesets[$key]);
+ }
+ }
+
+ return $changesets;
+ }
+
+ public function didFilterPage(array $changesets) {
+ if ($this->needAttachToDiffs) {
+ $changeset_groups = mgroup($changesets, 'getDiffID');
+ foreach ($this->diffs as $diff) {
+ $diff_changesets = idx($changeset_groups, $diff->getID(), array());
+ $diff->attachChangesets($diff_changesets);
+ }
+ }
+
+ if ($this->needHunks) {
+ id(new DifferentialHunkQuery())
+ ->setViewer($this->getViewer())
+ ->setParentQuery($this)
+ ->withChangesets($changesets)
+ ->needAttachToChangesets(true)
+ ->execute();
+ }
+
+ return $changesets;
+ }
+
+ private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+ $where = array();
+
+ if ($this->diffs !== null) {
+ $where[] = qsprintf(
+ $conn_r,
+ 'diffID IN (%Ld)',
+ mpull($this->diffs, 'getID'));
+ }
+
+ if ($this->ids !== null) {
+ $where[] = qsprintf(
+ $conn_r,
+ 'id IN (%Ld)',
+ $this->ids);
+ }
+
+ $where[] = $this->buildPagingClause($conn_r);
+
+ return $this->formatWhereClause($where);
+ }
+
+ public function getQueryApplicationClass() {
+ return 'PhabricatorApplicationDifferential';
+ }
+
+ protected function getReversePaging() {
+ return true;
+ }
+
+}
diff --git a/src/applications/differential/query/DifferentialDiffQuery.php b/src/applications/differential/query/DifferentialDiffQuery.php
--- a/src/applications/differential/query/DifferentialDiffQuery.php
+++ b/src/applications/differential/query/DifferentialDiffQuery.php
@@ -88,26 +88,13 @@
}
private function loadChangesets(array $diffs) {
- $diff_ids = mpull($diffs, 'getID');
-
- $changesets = id(new DifferentialChangeset())->loadAllWhere(
- 'diffID IN (%Ld)',
- $diff_ids);
-
- if ($changesets) {
- id(new DifferentialHunkQuery())
- ->setViewer($this->getViewer())
- ->setParentQuery($this)
- ->withChangesets($changesets)
- ->needAttachToChangesets(true)
- ->execute();
- }
-
- $changeset_groups = mgroup($changesets, 'getDiffID');
- foreach ($diffs as $diff) {
- $diff_changesets = idx($changeset_groups, $diff->getID(), array());
- $diff->attachChangesets($diff_changesets);
- }
+ id(new DifferentialChangesetQuery())
+ ->setViewer($this->getViewer())
+ ->setParentQuery($this)
+ ->withDiffs($diffs)
+ ->needAttachToDiffs(true)
+ ->needHunks(true)
+ ->execute();
return $diffs;
}
diff --git a/src/applications/differential/storage/DifferentialChangeset.php b/src/applications/differential/storage/DifferentialChangeset.php
--- a/src/applications/differential/storage/DifferentialChangeset.php
+++ b/src/applications/differential/storage/DifferentialChangeset.php
@@ -62,15 +62,6 @@
return $this;
}
- public function loadHunks() {
- if (!$this->getID()) {
- return array();
- }
- return id(new DifferentialHunk())->loadAllWhere(
- 'changesetID = %d',
- $this->getID());
- }
-
public function save() {
$this->openTransaction();
$ret = parent::save();
@@ -84,9 +75,14 @@
public function delete() {
$this->openTransaction();
- foreach ($this->loadHunks() as $hunk) {
+
+ $hunks = id(new DifferentialHunk())->loadAllWhere(
+ 'changesetID = %d',
+ $this->getID());
+ foreach ($hunks as $hunk) {
$hunk->delete();
}
+
$this->unsavedHunks = array();
queryfx(
@@ -174,6 +170,15 @@
return false;
}
+ public function attachDiff(DifferentialDiff $diff) {
+ $this->diff = $diff;
+ return $this;
+ }
+
+ public function getDiff() {
+ return $this->assertAttached($this->diff);
+ }
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
@@ -185,13 +190,11 @@
}
public function getPolicy($capability) {
- // TODO: For now, these are never queried directly through the policy
- // framework. Fix that up.
- return PhabricatorPolicies::getMostOpenPolicy();
+ return $this->getDiff()->getPolicy($capability);
}
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
- return false;
+ return $this->getDiff()->hasAutomaticCapability($capability, $viewer);
}
public function describeAutomaticCapability($capability) {

File Metadata

Mime Type
text/plain
Expires
Sun, Apr 6, 9:20 AM (3 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7716500
Default Alt Text
D9289.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility