Page MenuHomePhabricator

Remove "data" from edge infrastructure
Open, LowPublic


See T13051. There, edge transaction storage was simplified from { ... a huge dictionary ... } to [ ... a list of PHIDs ... ].

A related issue is that edges currently store optional "data", in the edgedata table. This was once used to store things like reviewer state in Differential.

However, it proved to not really be a great fit for much of anything and it now appears to have only one callsite, here:

if (!empty($edge['data']['gone'])) {
  return phutil_tag(
    pht('Asana Task Deleted'));

My recollection is that the case this deals with is:

  • Asana/Differential integration is enabled.
  • Differential publishes an Asana task for a review.
  • Someone manually deletes (or hides with policy controls?) the task in Asana.

Some possible fixes:

  • Just remove this entirely and accept that we'll show a bad link which 404s in this fairly extreme edge case.
  • Use a separate edge type (AsanaTaskIsGone) instead of edge data on the primary edge.
  • Store the data on the Doorkeeper ref instead.

Once this is fixed in some way, we should have no remaining readers or writers for edge data. Then we can:

  • Get rid of a bunch of edge data logic in EdgeQuery and EdgeEditor.
  • Drop all the edge data tables, schemata specs, the dataID column, etc.
  • Simplify all the pre-write logic in TransactionEditor.

A major goal would be to get rid of this junk from T13051, although a significant amount of pre-write logic currently depends on the older, richer datastructure:

// TODO: This is a transitional hack to let us migrate edge
// transactions to a more efficient storage format. For now, we're
// going to write a new slim format to the database but keep the old
// bulky format on the objects so we don't have to upgrade all the
// edit logic to the new format yet. See T13051.

$edge_type = PhabricatorTransactions::TYPE_EDGE;
if ($xaction->getTransactionType() == $edge_type) {
  $bulky_old = $xaction->getOldValue();
  $bulky_new = $xaction->getNewValue();

  $record = PhabricatorEdgeChangeRecord::newFromTransaction($xaction);
  $slim_old = $record->getModernOldEdgeTransactionData();
  $slim_new = $record->getModernNewEdgeTransactionData();


} else {