Page MenuHomePhabricator

D8455.diff
No OneTemporary

D8455.diff

diff --git a/src/applications/differential/editor/DifferentialRevisionEditor.php b/src/applications/differential/editor/DifferentialRevisionEditor.php
--- a/src/applications/differential/editor/DifferentialRevisionEditor.php
+++ b/src/applications/differential/editor/DifferentialRevisionEditor.php
@@ -18,75 +18,12 @@
private $auxiliaryFields = array();
private $contentSource;
private $isCreate;
- private $aphrontRequestForEventDispatch;
-
-
- public function setAphrontRequestForEventDispatch(AphrontRequest $request) {
- $this->aphrontRequestForEventDispatch = $request;
- return $this;
- }
-
- public function getAphrontRequestForEventDispatch() {
- return $this->aphrontRequestForEventDispatch;
- }
public function __construct(DifferentialRevision $revision) {
$this->revision = $revision;
$this->isCreate = !($revision->getID());
}
- public static function newRevisionFromConduitWithDiff(
- array $fields,
- DifferentialDiff $diff,
- PhabricatorUser $actor) {
-
- $revision = DifferentialRevision::initializeNewRevision($actor);
- $revision->setPHID($revision->generatePHID());
-
- $editor = new DifferentialRevisionEditor($revision);
- $editor->setActor($actor);
- $editor->addDiff($diff, null);
- $editor->copyFieldsFromConduit($fields);
-
- $editor->save();
-
- return $revision;
- }
-
- public function copyFieldsFromConduit(array $fields) {
-
- $actor = $this->getActor();
- $revision = $this->revision;
- $revision->loadRelationships();
-
- $all_fields = DifferentialFieldSelector::newSelector()
- ->getFieldSpecifications();
-
- $aux_fields = array();
- foreach ($all_fields as $aux_field) {
- $aux_field->setRevision($revision);
- $aux_field->setDiff($this->diff);
- $aux_field->setUser($actor);
- if ($aux_field->shouldAppearOnCommitMessage()) {
- $aux_fields[$aux_field->getCommitMessageKey()] = $aux_field;
- }
- }
-
- foreach ($fields as $field => $value) {
- if (empty($aux_fields[$field])) {
- throw new Exception(
- "Parsed commit message contains unrecognized field '{$field}'.");
- }
- $aux_fields[$field]->setValueFromParsedCommitMessage($value);
- }
-
- foreach ($aux_fields as $aux_field) {
- $aux_field->validateField();
- }
-
- $this->setAuxiliaryFields($all_fields);
- }
-
public function setAuxiliaryFields(array $auxiliary_fields) {
assert_instances_of($auxiliary_fields, 'DifferentialFieldSpecification');
$this->auxiliaryFields = $auxiliary_fields;
@@ -399,9 +336,6 @@
$diff->setDescription(preg_replace('/\n.*/s', '', $this->getComments()));
$diff->save();
- $this->updateAffectedPathTable($revision, $diff, $changesets);
- $this->updateRevisionHashTable($revision, $diff);
-
// An updated diff should require review, as long as it's not closed
// or accepted. The "accepted" status is "sticky" to encourage courtesy
// re-diffs after someone accepts with minor changes/suggestions.
@@ -686,177 +620,7 @@
}
}
- /**
- * Update the table which links Differential revisions to paths they affect,
- * so Diffusion can efficiently find pending revisions for a given file.
- */
- private function updateAffectedPathTable(
- DifferentialRevision $revision,
- DifferentialDiff $diff,
- array $changesets) {
- assert_instances_of($changesets, 'DifferentialChangeset');
-
- $project = $diff->loadArcanistProject();
- if (!$project) {
- // Probably an old revision from before projects.
- return;
- }
-
- $repository = $project->loadRepository();
- if (!$repository) {
- // Probably no project <-> repository link, or the repository where the
- // project lives is untracked.
- return;
- }
-
- $path_prefix = null;
-
- $local_root = $diff->getSourceControlPath();
- if ($local_root) {
- // We're in a working copy which supports subdirectory checkouts (e.g.,
- // SVN) so we need to figure out what prefix we should add to each path
- // (e.g., trunk/projects/example/) to get the absolute path from the
- // root of the repository. DVCS systems like Git and Mercurial are not
- // affected.
-
- // Normalize both paths and check if the repository root is a prefix of
- // the local root. If so, throw it away. Note that this correctly handles
- // the case where the remote path is "/".
- $local_root = id(new PhutilURI($local_root))->getPath();
- $local_root = rtrim($local_root, '/');
-
- $repo_root = id(new PhutilURI($repository->getRemoteURI()))->getPath();
- $repo_root = rtrim($repo_root, '/');
-
- if (!strncmp($repo_root, $local_root, strlen($repo_root))) {
- $path_prefix = substr($local_root, strlen($repo_root));
- }
- }
-
- $paths = array();
- foreach ($changesets as $changeset) {
- $paths[] = $path_prefix.'/'.$changeset->getFilename();
- }
-
- // Mark this as also touching all parent paths, so you can see all pending
- // changes to any file within a directory.
- $all_paths = array();
- foreach ($paths as $local) {
- foreach (DiffusionPathIDQuery::expandPathToRoot($local) as $path) {
- $all_paths[$path] = true;
- }
- }
- $all_paths = array_keys($all_paths);
-
- $path_ids =
- PhabricatorRepositoryCommitChangeParserWorker::lookupOrCreatePaths(
- $all_paths);
-
- $table = new DifferentialAffectedPath();
- $conn_w = $table->establishConnection('w');
-
- $sql = array();
- foreach ($path_ids as $path_id) {
- $sql[] = qsprintf(
- $conn_w,
- '(%d, %d, %d, %d)',
- $repository->getID(),
- $path_id,
- time(),
- $revision->getID());
- }
-
- queryfx(
- $conn_w,
- 'DELETE FROM %T WHERE revisionID = %d',
- $table->getTableName(),
- $revision->getID());
- foreach (array_chunk($sql, 256) as $chunk) {
- queryfx(
- $conn_w,
- 'INSERT INTO %T (repositoryID, pathID, epoch, revisionID) VALUES %Q',
- $table->getTableName(),
- implode(', ', $chunk));
- }
- }
-
-
- /**
- * Update the table connecting revisions to DVCS local hashes, so we can
- * identify revisions by commit/tree hashes.
- */
- private function updateRevisionHashTable(
- DifferentialRevision $revision,
- DifferentialDiff $diff) {
-
- $vcs = $diff->getSourceControlSystem();
- if ($vcs == DifferentialRevisionControlSystem::SVN) {
- // Subversion has no local commit or tree hash information, so we don't
- // have to do anything.
- return;
- }
- $property = id(new DifferentialDiffProperty())->loadOneWhere(
- 'diffID = %d AND name = %s',
- $diff->getID(),
- 'local:commits');
- if (!$property) {
- return;
- }
-
- $hashes = array();
-
- $data = $property->getData();
- switch ($vcs) {
- case DifferentialRevisionControlSystem::GIT:
- foreach ($data as $commit) {
- $hashes[] = array(
- ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT,
- $commit['commit'],
- );
- $hashes[] = array(
- ArcanistDifferentialRevisionHash::HASH_GIT_TREE,
- $commit['tree'],
- );
- }
- break;
- case DifferentialRevisionControlSystem::MERCURIAL:
- foreach ($data as $commit) {
- $hashes[] = array(
- ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT,
- $commit['rev'],
- );
- }
- break;
- }
-
- $conn_w = $revision->establishConnection('w');
-
- $sql = array();
- foreach ($hashes as $info) {
- list($type, $hash) = $info;
- $sql[] = qsprintf(
- $conn_w,
- '(%d, %s, %s)',
- $revision->getID(),
- $type,
- $hash);
- }
-
- queryfx(
- $conn_w,
- 'DELETE FROM %T WHERE revisionID = %d',
- ArcanistDifferentialRevisionHash::TABLE_NAME,
- $revision->getID());
-
- if ($sql) {
- queryfx(
- $conn_w,
- 'INSERT INTO %T (revisionID, type, hash) VALUES %Q',
- ArcanistDifferentialRevisionHash::TABLE_NAME,
- implode(', ', $sql));
- }
- }
/**
* Try to move a revision to "accepted". We look for:
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
@@ -276,27 +276,25 @@
$maniphest = 'PhabricatorApplicationManiphest';
if (PhabricatorApplication::isClassInstalled($maniphest)) {
$diff = $this->loadDiff($xaction->getNewValue());
- if ($diff) {
- $branch = $diff->getBranch();
-
- // No "$", to allow for branches like T123_demo.
- $match = null;
- if (preg_match('/^T(\d+)/i', $branch, $match)) {
- $task_id = $match[1];
- $tasks = id(new ManiphestTaskQuery())
- ->setViewer($this->getActor())
- ->withIDs(array($task_id))
- ->execute();
- if ($tasks) {
- $task = head($tasks);
- $task_phid = $task->getPHID();
-
- $results[] = id(new DifferentialTransaction())
- ->setTransactionType($type_edge)
- ->setMetadataValue('edge:type', $edge_ref_task)
- ->setIgnoreOnNoEffect(true)
- ->setNewValue(array('+' => array($task_phid => $task_phid)));
- }
+ $branch = $diff->getBranch();
+
+ // No "$", to allow for branches like T123_demo.
+ $match = null;
+ if (preg_match('/^T(\d+)/i', $branch, $match)) {
+ $task_id = $match[1];
+ $tasks = id(new ManiphestTaskQuery())
+ ->setViewer($this->getActor())
+ ->withIDs(array($task_id))
+ ->execute();
+ if ($tasks) {
+ $task = head($tasks);
+ $task_phid = $task->getPHID();
+
+ $results[] = id(new DifferentialTransaction())
+ ->setTransactionType($type_edge)
+ ->setMetadataValue('edge:type', $edge_ref_task)
+ ->setIgnoreOnNoEffect(true)
+ ->setNewValue(array('+' => array($task_phid => $task_phid)));
}
}
}
@@ -506,6 +504,20 @@
PhabricatorLiskDAO $object,
array $xactions) {
+ foreach ($xactions as $xaction) {
+ switch ($xaction->getTransactionType()) {
+ case DifferentialTransaction::TYPE_UPDATE:
+ $diff = $this->loadDiff($xaction->getNewValue(), true);
+
+ // Update these denormalized index tables when we attach a new
+ // diff to a revision.
+
+ $this->updateRevisionHashTable($object, $diff);
+ $this->updateAffectedPathTable($object, $diff);
+ break;
+ }
+ }
+
$status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
$status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
@@ -1167,11 +1179,16 @@
return implode("\n", $result);
}
- private function loadDiff($phid) {
- return id(new DifferentialDiffQuery())
+ private function loadDiff($phid, $need_changesets = false) {
+ $query = id(new DifferentialDiffQuery())
->withPHIDs(array($phid))
- ->setViewer($this->getActor())
- ->executeOne();
+ ->setViewer($this->getActor());
+
+ if ($need_changesets) {
+ $query->needChangesets(true);
+ }
+
+ return $query->executeOne();
}
/* -( Herald Integration )------------------------------------------------- */
@@ -1300,4 +1317,178 @@
return $xactions;
}
+ /**
+ * Update the table which links Differential revisions to paths they affect,
+ * so Diffusion can efficiently find pending revisions for a given file.
+ */
+ private function updateAffectedPathTable(
+ DifferentialRevision $revision,
+ DifferentialDiff $diff) {
+
+ $changesets = $diff->getChangesets();
+
+ // TODO: This all needs to be modernized.
+
+ $project = $diff->loadArcanistProject();
+ if (!$project) {
+ // Probably an old revision from before projects.
+ return;
+ }
+
+ $repository = $project->loadRepository();
+ if (!$repository) {
+ // Probably no project <-> repository link, or the repository where the
+ // project lives is untracked.
+ return;
+ }
+
+ $path_prefix = null;
+
+ $local_root = $diff->getSourceControlPath();
+ if ($local_root) {
+ // We're in a working copy which supports subdirectory checkouts (e.g.,
+ // SVN) so we need to figure out what prefix we should add to each path
+ // (e.g., trunk/projects/example/) to get the absolute path from the
+ // root of the repository. DVCS systems like Git and Mercurial are not
+ // affected.
+
+ // Normalize both paths and check if the repository root is a prefix of
+ // the local root. If so, throw it away. Note that this correctly handles
+ // the case where the remote path is "/".
+ $local_root = id(new PhutilURI($local_root))->getPath();
+ $local_root = rtrim($local_root, '/');
+
+ $repo_root = id(new PhutilURI($repository->getRemoteURI()))->getPath();
+ $repo_root = rtrim($repo_root, '/');
+
+ if (!strncmp($repo_root, $local_root, strlen($repo_root))) {
+ $path_prefix = substr($local_root, strlen($repo_root));
+ }
+ }
+
+ $paths = array();
+ foreach ($changesets as $changeset) {
+ $paths[] = $path_prefix.'/'.$changeset->getFilename();
+ }
+
+ // Mark this as also touching all parent paths, so you can see all pending
+ // changes to any file within a directory.
+ $all_paths = array();
+ foreach ($paths as $local) {
+ foreach (DiffusionPathIDQuery::expandPathToRoot($local) as $path) {
+ $all_paths[$path] = true;
+ }
+ }
+ $all_paths = array_keys($all_paths);
+
+ $path_ids =
+ PhabricatorRepositoryCommitChangeParserWorker::lookupOrCreatePaths(
+ $all_paths);
+
+ $table = new DifferentialAffectedPath();
+ $conn_w = $table->establishConnection('w');
+
+ $sql = array();
+ foreach ($path_ids as $path_id) {
+ $sql[] = qsprintf(
+ $conn_w,
+ '(%d, %d, %d, %d)',
+ $repository->getID(),
+ $path_id,
+ time(),
+ $revision->getID());
+ }
+
+ queryfx(
+ $conn_w,
+ 'DELETE FROM %T WHERE revisionID = %d',
+ $table->getTableName(),
+ $revision->getID());
+ foreach (array_chunk($sql, 256) as $chunk) {
+ queryfx(
+ $conn_w,
+ 'INSERT INTO %T (repositoryID, pathID, epoch, revisionID) VALUES %Q',
+ $table->getTableName(),
+ implode(', ', $chunk));
+ }
+ }
+
+
+ /**
+ * Update the table connecting revisions to DVCS local hashes, so we can
+ * identify revisions by commit/tree hashes.
+ */
+ private function updateRevisionHashTable(
+ DifferentialRevision $revision,
+ DifferentialDiff $diff) {
+
+ $vcs = $diff->getSourceControlSystem();
+ if ($vcs == DifferentialRevisionControlSystem::SVN) {
+ // Subversion has no local commit or tree hash information, so we don't
+ // have to do anything.
+ return;
+ }
+
+ $property = id(new DifferentialDiffProperty())->loadOneWhere(
+ 'diffID = %d AND name = %s',
+ $diff->getID(),
+ 'local:commits');
+ if (!$property) {
+ return;
+ }
+
+ $hashes = array();
+
+ $data = $property->getData();
+ switch ($vcs) {
+ case DifferentialRevisionControlSystem::GIT:
+ foreach ($data as $commit) {
+ $hashes[] = array(
+ ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT,
+ $commit['commit'],
+ );
+ $hashes[] = array(
+ ArcanistDifferentialRevisionHash::HASH_GIT_TREE,
+ $commit['tree'],
+ );
+ }
+ break;
+ case DifferentialRevisionControlSystem::MERCURIAL:
+ foreach ($data as $commit) {
+ $hashes[] = array(
+ ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT,
+ $commit['rev'],
+ );
+ }
+ break;
+ }
+
+ $conn_w = $revision->establishConnection('w');
+
+ $sql = array();
+ foreach ($hashes as $info) {
+ list($type, $hash) = $info;
+ $sql[] = qsprintf(
+ $conn_w,
+ '(%d, %s, %s)',
+ $revision->getID(),
+ $type,
+ $hash);
+ }
+
+ queryfx(
+ $conn_w,
+ 'DELETE FROM %T WHERE revisionID = %d',
+ ArcanistDifferentialRevisionHash::TABLE_NAME,
+ $revision->getID());
+
+ if ($sql) {
+ queryfx(
+ $conn_w,
+ 'INSERT INTO %T (revisionID, type, hash) VALUES %Q',
+ ArcanistDifferentialRevisionHash::TABLE_NAME,
+ implode(', ', $sql));
+ }
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Sat, Jun 8, 5:39 AM (2 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6287960
Default Alt Text
D8455.diff (16 KB)

Event Timeline