Page MenuHomePhabricator

D10875.id.diff
No OneTemporary

D10875.id.diff

diff --git a/resources/sql/autopatches/20141119.differential.diff.policy.sql b/resources/sql/autopatches/20141119.differential.diff.policy.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20141119.differential.diff.policy.sql
@@ -0,0 +1,5 @@
+ALTER TABLE {$NAMESPACE}_differential.differential_diff
+ ADD viewPolicy VARBINARY(64) NOT NULL;
+
+UPDATE {$NAMESPACE}_differential.differential_diff
+ SET viewPolicy = 'users' WHERE viewPolicy = '';
diff --git a/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php b/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php
--- a/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php
+++ b/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php
@@ -35,7 +35,9 @@
$parser = new ArcanistDiffParser();
$changes = $parser->parseDiff($data);
- $diff = DifferentialDiff::newFromRawChanges($changes);
+ $diff = DifferentialDiff::newFromRawChanges(
+ PhabricatorUser::getOmnipotentUser(),
+ $changes);
if (count($diff->getChangesets()) !== 1) {
throw new Exception("Expected one changeset: {$file}");
}
diff --git a/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php
--- a/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php
@@ -69,7 +69,7 @@
$changes[] = ArcanistDiffChange::newFromDictionary($dict);
}
- $diff = DifferentialDiff::newFromRawChanges($changes);
+ $diff = DifferentialDiff::newFromRawChanges($viewer, $changes);
// TODO: Remove repository UUID eventually; for now continue writing
// the UUID. Note that we'll overwrite it below if we identify a
diff --git a/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php
--- a/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php
@@ -15,6 +15,7 @@
return array(
'diff' => 'required string',
'repositoryPHID' => 'optional string',
+ 'viewPolicy' => 'optional string',
);
}
@@ -45,7 +46,7 @@
$parser = new ArcanistDiffParser();
$changes = $parser->parseDiff($raw_diff);
- $diff = DifferentialDiff::newFromRawChanges($changes);
+ $diff = DifferentialDiff::newFromRawChanges($viewer, $changes);
$diff_data_dict = array(
'creationMethod' => 'web',
@@ -58,6 +59,12 @@
->setTransactionType(DifferentialDiffTransaction::TYPE_DIFF_CREATE)
->setNewValue($diff_data_dict),);
+ if ($request->getValue('viewPolicy')) {
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
+ ->setNewValue($request->getValue('viewPolicy'));
+ }
+
id(new DifferentialDiffEditor())
->setActor($viewer)
->setContentSourceFromConduitRequest($request)
diff --git a/src/applications/differential/controller/DifferentialDiffCreateController.php b/src/applications/differential/controller/DifferentialDiffCreateController.php
--- a/src/applications/differential/controller/DifferentialDiffCreateController.php
+++ b/src/applications/differential/controller/DifferentialDiffCreateController.php
@@ -5,8 +5,11 @@
public function processRequest() {
$request = $this->getRequest();
+ $viewer = $request->getUser();
$diff = null;
+ // This object is just for policy stuff
+ $diff_object = DifferentialDiff::initializeNewDiff($viewer);
$repository_phid = null;
$repository_value = array();
$errors = array();
@@ -41,8 +44,9 @@
'differential.createrawdiff',
array(
'diff' => $diff,
- 'repositoryPHID' => $repository_phid,));
- $call->setUser($request->getUser());
+ 'repositoryPHID' => $repository_phid,
+ 'viewPolicy' => $request->getStr('viewPolicy'),));
+ $call->setUser($viewer);
$result = $call->execute();
$path = id(new PhutilURI($result['uri']))->getPath();
return id(new AphrontRedirectResponse())->setURI($path);
@@ -68,10 +72,15 @@
$repository_value = $this->loadViewerHandles(array($repository_phid));
}
+ $policies = id(new PhabricatorPolicyQuery())
+ ->setViewer($viewer)
+ ->setObject($diff_object)
+ ->execute();
+
$form
->setAction('/differential/diff/create/')
->setEncType('multipart/form-data')
- ->setUser($request->getUser())
+ ->setUser($viewer)
->appendInstructions(
pht(
'The best way to create a Differential diff is by using %s, but you '.
@@ -101,6 +110,13 @@
->setValue($repository_value)
->setLimit(1))
->appendChild(
+ id(new AphrontFormPolicyControl())
+ ->setUser($viewer)
+ ->setName('viewPolicy')
+ ->setPolicyObject($diff_object)
+ ->setPolicies($policies)
+ ->setCapability(PhabricatorPolicyCapability::CAN_VIEW))
+ ->appendChild(
id(new AphrontFormSubmitControl())
->addCancelButton($cancel_uri)
->setValue(pht('Create Diff')));
diff --git a/src/applications/differential/controller/DifferentialRevisionEditController.php b/src/applications/differential/controller/DifferentialRevisionEditController.php
--- a/src/applications/differential/controller/DifferentialRevisionEditController.php
+++ b/src/applications/differential/controller/DifferentialRevisionEditController.php
@@ -79,6 +79,13 @@
if ($repository_field) {
$repository_field->setValue($request->getStr($repo_key));
}
+ $view_policy_key = id(new DifferentialViewPolicyField())->getFieldKey();
+ $view_policy_field = idx(
+ $field_list->getFields(),
+ $view_policy_key);
+ if ($view_policy_field) {
+ $view_policy_field->setValue($diff->getViewPolicy());
+ }
}
$validation_exception = null;
diff --git a/src/applications/differential/editor/DifferentialDiffEditor.php b/src/applications/differential/editor/DifferentialDiffEditor.php
--- a/src/applications/differential/editor/DifferentialDiffEditor.php
+++ b/src/applications/differential/editor/DifferentialDiffEditor.php
@@ -22,6 +22,7 @@
public function getTransactionTypes() {
$types = parent::getTransactionTypes();
+ $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
$types[] = DifferentialDiffTransaction::TYPE_DIFF_CREATE;
return $types;
@@ -61,6 +62,9 @@
$dict = $this->diffDataDict;
$this->updateDiffFromDict($object, $dict);
return;
+ case PhabricatorTransactions::TYPE_VIEW_POLICY:
+ $object->setViewPolicy($xaction->getNewValue());
+ return;
}
return parent::applyCustomInternalTransaction($object, $xaction);
@@ -72,6 +76,7 @@
switch ($xaction->getTransactionType()) {
case DifferentialDiffTransaction::TYPE_DIFF_CREATE:
+ case PhabricatorTransactions::TYPE_VIEW_POLICY:
return;
}
diff --git a/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php b/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php
--- a/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php
+++ b/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php
@@ -40,7 +40,9 @@
throw new Exception("Expected 1 changeset for '{$name}'!");
}
- $diff = DifferentialDiff::newFromRawChanges($changes);
+ $diff = DifferentialDiff::newFromRawChanges(
+ PhabricatorUser::getOmnipotentUser(),
+ $changes);
return head($diff->getChangesets())->getHunks();
}
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
@@ -62,7 +62,6 @@
foreach ($diffs as $key => $diff) {
if (!$diff->getRevisionID()) {
- $diff->attachRevision(null);
continue;
}
diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php
--- a/src/applications/differential/storage/DifferentialDiff.php
+++ b/src/applications/differential/storage/DifferentialDiff.php
@@ -33,6 +33,8 @@
protected $description;
+ protected $viewPolicy;
+
private $unsavedChangesets = array();
private $changesets = self::ATTACHABLE;
private $arcanistProject = self::ATTACHABLE;
@@ -136,10 +138,27 @@
return $ret;
}
- public static function newFromRawChanges(array $changes) {
+ public static function initializeNewDiff(PhabricatorUser $actor) {
+ $app = id(new PhabricatorApplicationQuery())
+ ->setViewer($actor)
+ ->withClasses(array('PhabricatorDifferentialApplication'))
+ ->executeOne();
+ $view_policy = $app->getPolicy(
+ DifferentialDefaultViewCapability::CAPABILITY);
+
+ $diff = id(new DifferentialDiff())
+ ->setViewPolicy($view_policy);
+
+ return $diff;
+ }
+
+ public static function newFromRawChanges(
+ PhabricatorUser $actor,
+ array $changes) {
+
assert_instances_of($changes, 'ArcanistDiffChange');
- $diff = new DifferentialDiff();
+ $diff = self::initializeNewDiff($actor);
// There may not be any changes; initialize the changesets list so that
// we don't throw later when accessing it.
$diff->attachChangesets(array());
@@ -289,6 +308,10 @@
return $changes;
}
+ public function hasRevision() {
+ return $this->revision !== self::ATTACHABLE;
+ }
+
public function getRevision() {
return $this->assertAttached($this->revision);
}
@@ -318,27 +341,27 @@
}
public function getPolicy($capability) {
- if ($this->getRevision()) {
+ if ($this->hasRevision()) {
return $this->getRevision()->getPolicy($capability);
}
- return PhabricatorPolicies::POLICY_USER;
+ return $this->viewPolicy;
}
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
- if ($this->getRevision()) {
+ if ($this->hasRevision()) {
return $this->getRevision()->hasAutomaticCapability($capability, $viewer);
}
- return false;
+ return ($this->getAuthorPHID() == $viewer->getPhid());
}
public function describeAutomaticCapability($capability) {
- if ($this->getRevision()) {
+ if ($this->hasRevision()) {
return pht(
'This diff is attached to a revision, and inherits its policies.');
}
- return null;
+ return pht('The author of a diff can see it.');
}
diff --git a/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php b/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php
--- a/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php
+++ b/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php
@@ -7,6 +7,7 @@
$parser = new ArcanistDiffParser();
$diff = DifferentialDiff::newFromRawChanges(
+ PhabricatorUser::getOmnipotentUser(),
$parser->parseDiff(Filesystem::readFile($root.'lint_engine.diff')));
$copies = idx(head($diff->getChangesets())->getMetadata(), 'copy:lines');
@@ -46,7 +47,9 @@
{$oblock}
EODIFF;
- $diff = DifferentialDiff::newFromRawChanges($parser->parseDiff($raw_diff));
+ $diff = DifferentialDiff::newFromRawChanges(
+ PhabricatorUser::getOmnipotentUser(),
+ $parser->parseDiff($raw_diff));
$this->assertTrue(true);
}
diff --git a/src/applications/diffusion/controller/DiffusionChangeController.php b/src/applications/diffusion/controller/DiffusionChangeController.php
--- a/src/applications/diffusion/controller/DiffusionChangeController.php
+++ b/src/applications/diffusion/controller/DiffusionChangeController.php
@@ -22,7 +22,9 @@
$drequest->updateSymbolicCommit($data['effectiveCommit']);
$raw_changes = ArcanistDiffChange::newFromConduit($data['changes']);
- $diff = DifferentialDiff::newFromRawChanges($raw_changes);
+ $diff = DifferentialDiff::newFromRawChanges(
+ $viewer,
+ $raw_changes);
$changesets = $diff->getChangesets();
$changeset = reset($changesets);
diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -276,6 +276,7 @@
$content[] = $change_panel;
$changesets = DiffusionPathChange::convertToDifferentialChangesets(
+ $user,
$changes);
$vcs = $repository->getVersionControlSystem();
diff --git a/src/applications/diffusion/controller/DiffusionDiffController.php b/src/applications/diffusion/controller/DiffusionDiffController.php
--- a/src/applications/diffusion/controller/DiffusionDiffController.php
+++ b/src/applications/diffusion/controller/DiffusionDiffController.php
@@ -54,7 +54,9 @@
));
$drequest->updateSymbolicCommit($data['effectiveCommit']);
$raw_changes = ArcanistDiffChange::newFromConduit($data['changes']);
- $diff = DifferentialDiff::newFromRawChanges($raw_changes);
+ $diff = DifferentialDiff::newFromRawChanges(
+ $user,
+ $raw_changes);
$changesets = $diff->getChangesets();
$changeset = reset($changesets);
diff --git a/src/applications/diffusion/data/DiffusionPathChange.php b/src/applications/diffusion/data/DiffusionPathChange.php
--- a/src/applications/diffusion/data/DiffusionPathChange.php
+++ b/src/applications/diffusion/data/DiffusionPathChange.php
@@ -142,10 +142,12 @@
return array_select_keys($result, $direct);
}
- final public static function convertToDifferentialChangesets(array $changes) {
+ final public static function convertToDifferentialChangesets(
+ PhabricatorUser $user,
+ array $changes) {
assert_instances_of($changes, 'DiffusionPathChange');
$arcanist_changes = self::convertToArcanistChanges($changes);
- $diff = DifferentialDiff::newFromRawChanges($arcanist_changes);
+ $diff = DifferentialDiff::newFromRawChanges($user, $arcanist_changes);
return $diff->getChangesets();
}
diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
--- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
@@ -1115,7 +1115,9 @@
$parser = new ArcanistDiffParser();
$changes = $parser->parseDiff($raw_diff);
- $diff = DifferentialDiff::newFromRawChanges($changes);
+ $diff = DifferentialDiff::newFromRawChanges(
+ $this->getViewer(),
+ $changes);
return $diff->getChangesets();
}
diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php
--- a/src/applications/herald/adapter/HeraldCommitAdapter.php
+++ b/src/applications/herald/adapter/HeraldCommitAdapter.php
@@ -346,7 +346,9 @@
$parser = new ArcanistDiffParser();
$changes = $parser->parseDiff($raw);
- $diff = DifferentialDiff::newFromRawChanges($changes);
+ $diff = DifferentialDiff::newFromRawChanges(
+ PhabricatorUser::getOmnipotentUser(),
+ $changes);
return $diff;
}
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -265,7 +265,7 @@
$changes = array();
}
- $diff = DifferentialDiff::newFromRawChanges($changes)
+ $diff = DifferentialDiff::newFromRawChanges($viewer, $changes)
->setRepositoryPHID($this->repository->getPHID())
->setAuthorPHID($actor_phid)
->setCreationMethod('commit')
diff --git a/src/infrastructure/diff/PhabricatorDifferenceEngine.php b/src/infrastructure/diff/PhabricatorDifferenceEngine.php
--- a/src/infrastructure/diff/PhabricatorDifferenceEngine.php
+++ b/src/infrastructure/diff/PhabricatorDifferenceEngine.php
@@ -163,7 +163,9 @@
$diff = $this->generateRawDiffFromFileContent($old, $new);
$changes = id(new ArcanistDiffParser())->parseDiff($diff);
- $diff = DifferentialDiff::newFromRawChanges($changes);
+ $diff = DifferentialDiff::newFromRawChanges(
+ PhabricatorUser::getOmnipotentUser(),
+ $changes);
return head($diff->getChangesets());
}

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 2, 7:19 AM (2 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6730409
Default Alt Text
D10875.id.diff (16 KB)

Event Timeline