From 8ac73b2bf351fa06e7be0971c138406e8a3bee48 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Fri, 19 Dec 2014 14:54:15 -0800 Subject: [PATCH] Differential - tighten up access of Differential data from other applications Summary: Fixes T6790. Turn the old method into "new" (old signature) and "newEphemeral". Deploy "newEphemeral" as many places as possible; basically places we are not in the Differential application *and* have no intentions of ever saving the diff. These callsites are also all places we are just trying to get some changesets at the end of the day. Test Plan: set differential application policy to 'administrators only'. viewed a commit in diffusion and it worked without any errors! i'm just using my thinkin' noodle on the other code paths. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T6790 Differential Revision: https://secure.phabricator.com/D11020 --- .../differential/storage/DifferentialDiff.php | 14 ++++++++++++++ .../controller/DiffusionChangeController.php | 3 +-- .../controller/DiffusionDiffController.php | 3 +-- .../diffusion/data/DiffusionPathChange.php | 3 ++- .../diffusion/engine/DiffusionCommitHookEngine.php | 3 +-- .../herald/adapter/HeraldCommitAdapter.php | 3 +-- .../diff/PhabricatorDifferenceEngine.php | 3 +-- 7 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php index f055000fdb..1124582263 100644 --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/src/applications/differential/storage/DifferentialDiff.php @@ -159,6 +159,20 @@ final class DifferentialDiff assert_instances_of($changes, 'ArcanistDiffChange'); $diff = self::initializeNewDiff($actor); + return self::buildChangesetsFromRawChanges($diff, $changes); + } + + public static function newEphemeralFromRawChanges(array $changes) { + assert_instances_of($changes, 'ArcanistDiffChange'); + + $diff = id(new DifferentialDiff())->makeEphemeral(); + return self::buildChangesetsFromRawChanges($diff, $changes); + } + + private static function buildChangesetsFromRawChanges( + DifferentialDiff $diff, + array $changes) { + // There may not be any changes; initialize the changesets list so that // we don't throw later when accessing it. $diff->attachChangesets(array()); diff --git a/src/applications/diffusion/controller/DiffusionChangeController.php b/src/applications/diffusion/controller/DiffusionChangeController.php index ddfa283883..373209e645 100644 --- a/src/applications/diffusion/controller/DiffusionChangeController.php +++ b/src/applications/diffusion/controller/DiffusionChangeController.php @@ -22,8 +22,7 @@ final class DiffusionChangeController extends DiffusionController { $drequest->updateSymbolicCommit($data['effectiveCommit']); $raw_changes = ArcanistDiffChange::newFromConduit($data['changes']); - $diff = DifferentialDiff::newFromRawChanges( - $viewer, + $diff = DifferentialDiff::newEphemeralFromRawChanges( $raw_changes); $changesets = $diff->getChangesets(); $changeset = reset($changesets); diff --git a/src/applications/diffusion/controller/DiffusionDiffController.php b/src/applications/diffusion/controller/DiffusionDiffController.php index 81460ea67a..4f6e3a8067 100644 --- a/src/applications/diffusion/controller/DiffusionDiffController.php +++ b/src/applications/diffusion/controller/DiffusionDiffController.php @@ -54,8 +54,7 @@ final class DiffusionDiffController extends DiffusionController { )); $drequest->updateSymbolicCommit($data['effectiveCommit']); $raw_changes = ArcanistDiffChange::newFromConduit($data['changes']); - $diff = DifferentialDiff::newFromRawChanges( - $user, + $diff = DifferentialDiff::newEphemeralFromRawChanges( $raw_changes); $changesets = $diff->getChangesets(); $changeset = reset($changesets); diff --git a/src/applications/diffusion/data/DiffusionPathChange.php b/src/applications/diffusion/data/DiffusionPathChange.php index e4f6fa182a..b1d7286fdd 100644 --- a/src/applications/diffusion/data/DiffusionPathChange.php +++ b/src/applications/diffusion/data/DiffusionPathChange.php @@ -147,7 +147,8 @@ final class DiffusionPathChange { array $changes) { assert_instances_of($changes, 'DiffusionPathChange'); $arcanist_changes = self::convertToArcanistChanges($changes); - $diff = DifferentialDiff::newFromRawChanges($user, $arcanist_changes); + $diff = DifferentialDiff::newEphemeralFromRawChanges( + $arcanist_changes); return $diff->getChangesets(); } diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php index 9e7d5ccef9..8dd7ab81e0 100644 --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -1115,8 +1115,7 @@ final class DiffusionCommitHookEngine extends Phobject { $parser = new ArcanistDiffParser(); $changes = $parser->parseDiff($raw_diff); - $diff = DifferentialDiff::newFromRawChanges( - $this->getViewer(), + $diff = DifferentialDiff::newEphemeralFromRawChanges( $changes); return $diff->getChangesets(); } diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php index 35e93f5916..9b9c1f8ddd 100644 --- a/src/applications/herald/adapter/HeraldCommitAdapter.php +++ b/src/applications/herald/adapter/HeraldCommitAdapter.php @@ -346,8 +346,7 @@ final class HeraldCommitAdapter extends HeraldAdapter { $parser = new ArcanistDiffParser(); $changes = $parser->parseDiff($raw); - $diff = DifferentialDiff::newFromRawChanges( - PhabricatorUser::getOmnipotentUser(), + $diff = DifferentialDiff::newEphemeralFromRawChanges( $changes); return $diff; } diff --git a/src/infrastructure/diff/PhabricatorDifferenceEngine.php b/src/infrastructure/diff/PhabricatorDifferenceEngine.php index b7b22d7da0..6db867722f 100644 --- a/src/infrastructure/diff/PhabricatorDifferenceEngine.php +++ b/src/infrastructure/diff/PhabricatorDifferenceEngine.php @@ -163,8 +163,7 @@ final class PhabricatorDifferenceEngine { $diff = $this->generateRawDiffFromFileContent($old, $new); $changes = id(new ArcanistDiffParser())->parseDiff($diff); - $diff = DifferentialDiff::newFromRawChanges( - PhabricatorUser::getOmnipotentUser(), + $diff = DifferentialDiff::newEphemeralFromRawChanges( $changes); return head($diff->getChangesets()); }