1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-27 07:50:57 +01:00

Move board relationships to dedicated storage

Summary:
Fixes T5476. Using edges to store which objects are on which board columns ends up being pretty awkward. In particular, it makes T4807 very difficult to implement.

Introduce a dedicated `BoardColumnPosition` storage.

This doesn't affect ordering rules (T4807) yet: boards are still arranged by priority. We just read which tasks are on which columns out of a new table.

Test Plan:
  - Migrated data, then viewed some boards. Saw exactly the same data.
  - Dragged tasks from column to column.
  - Created a task directly into a column.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5476

Differential Revision: https://secure.phabricator.com/D10160
This commit is contained in:
epriestley 2014-08-06 15:09:09 -07:00
parent 95ef72e4f9
commit 09868271bd
11 changed files with 472 additions and 124 deletions

View file

@ -0,0 +1,10 @@
CREATE TABLE {$NAMESPACE}_project.project_columnposition (
id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
boardPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
columnPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
sequence INT UNSIGNED NOT NULL,
UNIQUE KEY (boardPHID, columnPHID, objectPHID),
KEY (objectPHID, boardPHID),
KEY (boardPHID, columnPHID, sequence)
) ENGINE=InnoDB, COLLATE utf8_general_ci;

View file

@ -0,0 +1,53 @@
<?php
// Was PhabricatorEdgeConfig::TYPE_COLUMN_HAS_OBJECT
$type_has_object = 44;
$column = new PhabricatorProjectColumn();
$conn_w = $column->establishConnection('w');
$rows = queryfx_all(
$conn_w,
'SELECT src, dst FROM %T WHERE type = %d',
PhabricatorEdgeConfig::TABLE_NAME_EDGE,
$type_has_object);
$cols = array();
foreach ($rows as $row) {
$cols[$row['src']][] = $row['dst'];
}
$sql = array();
foreach ($cols as $col_phid => $obj_phids) {
echo "Migrating column '{$col_phid}'...\n";
$column = id(new PhabricatorProjectColumn())->loadOneWhere(
'phid = %s',
$col_phid);
if (!$column) {
echo "Column '{$col_phid}' does not exist.\n";
continue;
}
$sequence = 0;
foreach ($obj_phids as $obj_phid) {
$sql[] = qsprintf(
$conn_w,
'(%s, %s, %s, %d)',
$column->getProjectPHID(),
$column->getPHID(),
$obj_phid,
$sequence++);
}
}
echo "Inserting rows...\n";
foreach (PhabricatorLiskDAO::chunkSQL($sql) as $chunk) {
queryfx(
$conn_w,
'INSERT INTO %T (boardPHID, columnPHID, objectPHID, sequence)
VALUES %Q',
id(new PhabricatorProjectColumnPosition())->getTableName(),
$chunk);
}
echo "Done.\n";

View file

@ -1932,6 +1932,8 @@ phutil_register_library_map(array(
'PhabricatorProjectColumn' => 'applications/project/storage/PhabricatorProjectColumn.php',
'PhabricatorProjectColumnDetailController' => 'applications/project/controller/PhabricatorProjectColumnDetailController.php',
'PhabricatorProjectColumnPHIDType' => 'applications/project/phid/PhabricatorProjectColumnPHIDType.php',
'PhabricatorProjectColumnPosition' => 'applications/project/storage/PhabricatorProjectColumnPosition.php',
'PhabricatorProjectColumnPositionQuery' => 'applications/project/query/PhabricatorProjectColumnPositionQuery.php',
'PhabricatorProjectColumnQuery' => 'applications/project/query/PhabricatorProjectColumnQuery.php',
'PhabricatorProjectColumnTransaction' => 'applications/project/storage/PhabricatorProjectColumnTransaction.php',
'PhabricatorProjectColumnTransactionEditor' => 'applications/project/editor/PhabricatorProjectColumnTransactionEditor.php',
@ -4764,6 +4766,11 @@ phutil_register_library_map(array(
),
'PhabricatorProjectColumnDetailController' => 'PhabricatorProjectBoardController',
'PhabricatorProjectColumnPHIDType' => 'PhabricatorPHIDType',
'PhabricatorProjectColumnPosition' => array(
'PhabricatorProjectDAO',
'PhabricatorPolicyInterface',
),
'PhabricatorProjectColumnPositionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorProjectColumnQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorProjectColumnTransaction' => 'PhabricatorApplicationTransaction',
'PhabricatorProjectColumnTransactionEditor' => 'PhabricatorApplicationTransactionEditor',

View file

@ -534,27 +534,13 @@ final class ManiphestTaskDetailController extends ManiphestController {
if ($project_phids) {
require_celerity_resource('maniphest-task-summary-css');
// If we end up with real-world projects with many hundreds of columns, it
// might be better to just load all the edges, then load those columns and
// work backward that way, or denormalize this data more.
$columns = id(new PhabricatorProjectColumnQuery())
$positions = id(new PhabricatorProjectColumnPositionQuery())
->setViewer($viewer)
->withProjectPHIDs($project_phids)
->withBoardPHIDs($project_phids)
->withObjectPHIDs(array($task->getPHID()))
->needColumns(true)
->execute();
$columns = mpull($columns, null, 'getPHID');
$column_edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_COLUMN;
$all_column_phids = array_keys($columns);
$column_edge_query = id(new PhabricatorEdgeQuery())
->withSourcePHIDs(array($task->getPHID()))
->withEdgeTypes(array($column_edge_type))
->withDestinationPHIDs($all_column_phids);
$column_edge_query->execute();
$in_column_phids = array_fuse($column_edge_query->getDestinationPHIDs());
$column_groups = mgroup($columns, 'getProjectPHID');
$positions = mpull($positions, null, 'getBoardPHID');
$project_handles = array();
$project_annotations = array();
@ -562,9 +548,10 @@ final class ManiphestTaskDetailController extends ManiphestController {
$handle = $this->getHandle($project_phid);
$project_handles[] = $handle;
$columns = idx($column_groups, $project_phid, array());
$column = head(array_intersect_key($columns, $in_column_phids));
if ($column) {
$position = idx($positions, $project_phid);
if ($position) {
$column = $position->getColumn();
$column_name = pht('(%s)', $column->getDisplayName());
$column_link = phutil_tag(
'a',

View file

@ -339,6 +339,7 @@ final class ManiphestTaskEditController extends ManiphestController {
if ($parent_task) {
// TODO: This should be transactional now.
id(new PhabricatorEdgeEditor())
->addEdge(
$parent_task->getPHID(),
@ -364,47 +365,26 @@ final class ManiphestTaskEditController extends ManiphestController {
->setOwner($owner)
->setCanEdit(true)
->getItem();
$column_phid = $request->getStr('columnPHID');
$column = id(new PhabricatorProjectColumnQuery())
->setViewer($user)
->withPHIDs(array($column_phid))
->withPHIDs(array($request->getStr('columnPHID')))
->executeOne();
if ($column->isDefaultColumn()) {
$column_tasks = array();
$potential_col_tasks = id(new ManiphestTaskQuery())
->setViewer($user)
->withAllProjects(array($column->getProjectPHID()))
->execute();
$potential_col_tasks = mpull(
$potential_col_tasks,
null,
'getPHID');
$potential_task_phids = array_keys($potential_col_tasks);
if ($potential_task_phids) {
$edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_COLUMN;
$edge_query = id(new PhabricatorEdgeQuery())
->withSourcePHIDs($potential_task_phids)
->withEdgeTypes(array($edge_type));
$edges = $edge_query->execute();
foreach ($potential_col_tasks as $task_phid => $curr_task) {
$curr_column_phids = $edges[$task_phid][$edge_type];
$curr_column_phid = head_key($curr_column_phids);
if (!$curr_column_phid ||
$curr_column_phid == $column_phid) {
$column_tasks[] = $curr_task;
}
}
}
} else {
$column_task_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
$column_phid,
PhabricatorEdgeConfig::TYPE_COLUMN_HAS_OBJECT);
$column_tasks = id(new ManiphestTaskQuery())
->setViewer($user)
->withPHIDs($column_task_phids)
->execute();
if (!$column) {
return new Aphront404Response();
}
$positions = id(new PhabricatorProjectColumnPositionQuery())
->setViewer($user)
->withColumns(array($column))
->execute();
$task_phids = mpull($positions, 'getObjectPHID');
$column_tasks = id(new ManiphestTaskQuery())
->setViewer($user)
->withPHIDs($task_phids)
->execute();
$sort_map = mpull(
$column_tasks,
'getPrioritySortVector',

View file

@ -184,43 +184,45 @@ final class ManiphestTransactionEditor
switch ($xaction->getTransactionType()) {
case ManiphestTransaction::TYPE_PROJECT_COLUMN:
$new = $xaction->getNewValue();
$old = $xaction->getOldValue();
$src = $object->getPHID();
$dst = head($new['columnPHIDs']);
$edges = $old['columnPHIDs'];
$edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_COLUMN;
// NOTE: Normally, we expect only one edge to exist, but this works in
// a general way so it will repair any stray edges.
$remove = array();
$edge_missing = true;
foreach ($edges as $phid) {
if ($phid == $dst) {
$edge_missing = false;
} else {
$remove[] = $phid;
$board_phid = idx($xaction->getNewValue(), 'projectPHID');
if (!$board_phid) {
throw new Exception(
pht("Expected 'projectPHID' in column transaction."));
}
$new_phids = idx($xaction->getNewValue(), 'columnPHIDs', array());
if (count($new_phids) !== 1) {
throw new Exception(
pht("Expected exactly one 'columnPHIDs' in column transaction."));
}
$positions = id(new PhabricatorProjectColumnPositionQuery())
->setViewer($this->requireActor())
->withObjectPHIDs(array($object->getPHID()))
->withBoardPHIDs(array($board_phid))
->execute();
// Remove all existing column positions on the board.
foreach ($positions as $position) {
if (!$position->getID()) {
// This is an ephemeral position, so don't try to destroy it.
continue;
}
$position->delete();
}
$add = array();
if ($edge_missing) {
$add[] = $dst;
}
// Add the new column position.
// This should never happen because of the code in
// transactionHasEffect, but keep it for maximum conservativeness
if (!$add && !$remove) {
return;
foreach ($new_phids as $phid) {
id(new PhabricatorProjectColumnPosition())
->setBoardPHID($board_phid)
->setColumnPHID($phid)
->setObjectPHID($object->getPHID())
// TODO: Do real sequence stuff.
->setSequence(0)
->save();
}
$editor = new PhabricatorEdgeEditor();
foreach ($add as $phid) {
$editor->addEdge($src, $edge_type, $phid);
}
foreach ($remove as $phid) {
$editor->removeEdge($src, $edge_type, $phid);
}
$editor->save();
break;
default:
break;

View file

@ -135,26 +135,28 @@ final class PhabricatorProjectBoardViewController
->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY)
->setViewer($viewer)
->execute();
$tasks = mpull($tasks, null, 'getPHID');
$task_phids = array_keys($tasks);
if ($task_phids) {
$edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_COLUMN;
$edge_query = id(new PhabricatorEdgeQuery())
->withSourcePHIDs($task_phids)
->withEdgeTypes(array($edge_type))
->withDestinationPHIDs(mpull($columns, 'getPHID'));
$edge_query->execute();
if ($tasks) {
$positions = id(new PhabricatorProjectColumnPositionQuery())
->setViewer($viewer)
->withObjectPHIDs(mpull($tasks, 'getPHID'))
->withColumns($columns)
->execute();
$positions = mpull($positions, null, 'getObjectPHID');
} else {
$positions = array();
}
$task_map = array();
$default_phid = $columns[0]->getPHID();
foreach ($tasks as $task) {
$task_phid = $task->getPHID();
$column_phids = $edge_query->getDestinationPHIDs(array($task_phid));
$column_phid = head($column_phids);
$column_phid = null;
if (isset($positions[$task_phid])) {
$column_phid = $positions[$task_phid]->getColumnPHID();
}
$column_phid = nonempty($column_phid, $default_phid);
$task_map[$column_phid][] = $task_phid;

View file

@ -57,27 +57,26 @@ final class PhabricatorProjectMoveController
return new Aphront404Response();
}
$positions = id(new PhabricatorProjectColumnPositionQuery())
->setViewer($viewer)
->withColumns($columns)
->withObjectPHIDs(array($object_phid))
->execute();
$xactions = array();
$edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_COLUMN;
$query = id(new PhabricatorEdgeQuery())
->withSourcePHIDs(array($object->getPHID()))
->withEdgeTypes(array($edge_type))
->withDestinationPHIDs(array_keys($columns));
$query->execute();
$edge_phids = $query->getDestinationPHIDs();
$xactions[] = id(new ManiphestTransaction())
->setTransactionType(ManiphestTransaction::TYPE_PROJECT_COLUMN)
->setNewValue(array(
'columnPHIDs' => array($column->getPHID()),
'projectPHID' => $column->getProjectPHID()))
->setOldValue(array(
'columnPHIDs' => $edge_phids,
'projectPHID' => $column->getProjectPHID()));
->setNewValue(
array(
'columnPHIDs' => array($column->getPHID()),
'projectPHID' => $column->getProjectPHID(),
))
->setOldValue(
array(
'columnPHIDs' => mpull($positions, 'getColumnPHID'),
'projectPHID' => $column->getProjectPHID(),
));
$task_phids = array();
if ($after_phid) {
@ -86,6 +85,7 @@ final class PhabricatorProjectMoveController
if ($before_phid) {
$task_phids[] = $before_phid;
}
if ($task_phids) {
$tasks = id(new ManiphestTaskQuery())
->setViewer($viewer)

View file

@ -0,0 +1,258 @@
<?php
final class PhabricatorProjectColumnPositionQuery
extends PhabricatorCursorPagedPolicyAwareQuery {
private $ids;
private $boardPHIDs;
private $objectPHIDs;
private $columns;
private $needColumns;
public function withIDs(array $ids) {
$this->ids = $ids;
return $this;
}
public function withBoardPHIDs(array $board_phids) {
$this->boardPHIDs = $board_phids;
return $this;
}
public function withObjectPHIDs(array $object_phids) {
$this->objectPHIDs = $object_phids;
return $this;
}
/**
* Find objects in specific columns.
*
* NOTE: Using this method activates logic which constructs virtual
* column positions for objects not in any column, if you pass a default
* column. Normally these results are not returned.
*
* @param list<PhabricatorProjectColumn> Columns to look for objects in.
* @return this
*/
public function withColumns(array $columns) {
assert_instances_of($columns, 'PhabricatorProjectColumn');
$this->columns = $columns;
return $this;
}
public function needColumns($need_columns) {
$this->needColumns = true;
return $this;
}
// NOTE: For now, boards are always attached to projects. However, they might
// not be in the future. This generalization just anticipates a future where
// we let other types of objects (like users) have boards, or let boards
// contain other types of objects.
private function newPositionObject() {
return new PhabricatorProjectColumnPosition();
}
private function newColumnQuery() {
return new PhabricatorProjectColumnQuery();
}
private function getBoardMembershipEdgeTypes() {
return array(
PhabricatorProjectProjectHasObjectEdgeType::EDGECONST,
);
}
private function getBoardMembershipPHIDTypes() {
return array(
ManiphestTaskPHIDType::TYPECONST,
);
}
protected function loadPage() {
$table = $this->newPositionObject();
$conn_r = $table->establishConnection('r');
// We're going to find results by combining two queries: one query finds
// objects on a board column, while the other query finds objects not on
// any board column and virtually puts them on the default column.
$unions = array();
// First, find all the stuff that's actually on a column.
$unions[] = qsprintf(
$conn_r,
'SELECT * FROM %T %Q',
$table->getTableName(),
$this->buildWhereClause($conn_r));
// If we have a default column, find all the stuff that's not in any
// column and put it in the default column.
$must_type_filter = false;
if ($this->columns) {
$default_map = array();
foreach ($this->columns as $column) {
if ($column->isDefaultColumn()) {
$default_map[$column->getProjectPHID()] = $column->getPHID();
}
}
if ($default_map) {
$where = array();
// Find the edges attached to the boards we have default columns for.
$where[] = qsprintf(
$conn_r,
'e.src IN (%Ls)',
array_keys($default_map));
// Find only edges which describe a board relationship.
$where[] = qsprintf(
$conn_r,
'e.type IN (%Ld)',
$this->getBoardMembershipEdgeTypes());
if ($this->boardPHIDs !== null) {
// This should normally be redundant, but construct it anyway if
// the caller has told us to.
$where[] = qsprintf(
$conn_r,
'e.src IN (%Ls)',
$this->boardPHIDs);
}
if ($this->objectPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
'e.dst IN (%Ls)',
$this->objectPHIDs);
}
$where[] = qsprintf(
$conn_r,
'p.id IS NULL');
$where = $this->formatWhereClause($where);
$unions[] = qsprintf(
$conn_r,
'SELECT NULL id, e.src boardPHID, NULL columnPHID, e.dst objectPHID,
0 sequence
FROM %T e LEFT JOIN %T p
ON e.src = p.boardPHID AND e.dst = p.objectPHID
%Q',
PhabricatorEdgeConfig::TABLE_NAME_EDGE,
$table->getTableName(),
$where);
$must_type_filter = true;
}
}
$data = queryfx_all(
$conn_r,
'%Q %Q %Q',
implode(' UNION ALL ', $unions),
$this->buildOrderClause($conn_r),
$this->buildLimitClause($conn_r));
// If we've picked up objects not in any column, we need to filter out any
// matched objects which have the wrong edge type.
if ($must_type_filter) {
$allowed_types = array_fuse($this->getBoardMembershipPHIDTypes());
foreach ($data as $id => $row) {
if ($row['columnPHID'] === null) {
$object_phid = $row['objectPHID'];
if (empty($allowed_types[phid_get_type($object_phid)])) {
unset($data[$id]);
}
}
}
}
$positions = $table->loadAllFromArray($data);
foreach ($positions as $position) {
if ($position->getColumnPHID() === null) {
$position->makeEphemeral();
$column_phid = idx($default_map, $position->getBoardPHID());
$position->setColumnPHID($column_phid);
}
}
return $positions;
}
protected function willFilterPage(array $page) {
if ($this->needColumns) {
$column_phids = mpull($page, 'getColumnPHID');
$columns = $this->newColumnQuery()
->setParentQuery($this)
->setViewer($this->getViewer())
->withPHIDs($column_phids)
->execute();
$columns = mpull($columns, null, 'getPHID');
foreach ($page as $key => $position) {
$column = idx($columns, $position->getColumnPHID());
if (!$column) {
unset($page[$key]);
continue;
}
$position->attachColumn($column);
}
}
return $page;
}
private function buildWhereClause($conn_r) {
$where = array();
if ($this->ids !== null) {
$where[] = qsprintf(
$conn_r,
'id IN (%Ld)',
$this->ids);
}
if ($this->boardPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
'boardPHID IN (%Ls)',
$this->boardPHIDs);
}
if ($this->objectPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
'objectPHID IN (%Ls)',
$this->objectPHIDs);
}
if ($this->columns !== null) {
$where[] = qsprintf(
$conn_r,
'columnPHID IN (%Ls)',
mpull($this->columns, 'getPHID'));
}
// NOTE: Explicitly not building the paging clause here, since it won't
// work with the UNION.
return $this->formatWhereClause($where);
}
public function getQueryApplicationClass() {
return 'PhabricatorProjectApplication';
}
}

View file

@ -0,0 +1,51 @@
<?php
final class PhabricatorProjectColumnPosition extends PhabricatorProjectDAO
implements PhabricatorPolicyInterface {
protected $boardPHID;
protected $columnPHID;
protected $objectPHID;
protected $sequence;
private $column = self::ATTACHABLE;
public function getConfiguration() {
return array(
self::CONFIG_TIMESTAMPS => false,
) + parent::getConfiguration();
}
public function getColumn() {
return $this->assertAttached($this->column);
}
public function attachColumn(PhabricatorProjectColumn $column) {
$this->column = $column;
return $this;
}
/* -( PhabricatorPolicyInterface )----------------------------------------- */
public function getCapabilities() {
return array(
PhabricatorPolicyCapability::CAN_VIEW,
);
}
public function getPolicy($capability) {
switch ($capability) {
case PhabricatorPolicyCapability::CAN_VIEW:
return PhabricatorPolicies::getMostOpenPolicy();
}
}
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
return false;
}
public function describeAutomaticCapability($capability) {
return null;
}
}

View file

@ -57,9 +57,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
const TYPE_OBJECT_USES_CREDENTIAL = 39;
const TYPE_CREDENTIAL_USED_BY_OBJECT = 40;
const TYPE_OBJECT_HAS_COLUMN = 43;
const TYPE_COLUMN_HAS_OBJECT = 44;
const TYPE_DASHBOARD_HAS_PANEL = 45;
const TYPE_PANEL_HAS_DASHBOARD = 46;
@ -105,6 +102,10 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
range(1, 50),
array(9000),
range(80000, 80005));
$exclude[] = 43; // Was TYPE_OBJECT_HAS_COLUMN
$exclude[] = 44; // Was TYPE_COLUMN_HAS_OBJECT
$consts = array_diff($consts, $exclude);
$map = array();
@ -190,9 +191,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
self::TYPE_OBJECT_USES_CREDENTIAL => self::TYPE_CREDENTIAL_USED_BY_OBJECT,
self::TYPE_CREDENTIAL_USED_BY_OBJECT => self::TYPE_OBJECT_USES_CREDENTIAL,
self::TYPE_OBJECT_HAS_COLUMN => self::TYPE_COLUMN_HAS_OBJECT,
self::TYPE_COLUMN_HAS_OBJECT => self::TYPE_OBJECT_HAS_COLUMN,
self::TYPE_PANEL_HAS_DASHBOARD => self::TYPE_DASHBOARD_HAS_PANEL,
self::TYPE_DASHBOARD_HAS_PANEL => self::TYPE_PANEL_HAS_DASHBOARD,