D20308.id48505.diff
No OneTemporary
Actions

Size

20 KB

Referenced Files

None

Subscribers

None

D20308.id48505.diff
View Options

	diff --git a/resources/sql/autopatches/20190322.triggers.01.usage.sql b/resources/sql/autopatches/20190322.triggers.01.usage.sql
	new file mode 100644
	--- /dev/null
	+++ b/resources/sql/autopatches/20190322.triggers.01.usage.sql
	@@ -0,0 +1,8 @@
	+CREATE TABLE {$NAMESPACE}_project.project_triggerusage (
	+ id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
	+ triggerPHID VARBINARY(64) NOT NULL,
	+ examplePHID VARBINARY(64),
	+ columnCount INT UNSIGNED NOT NULL,
	+ activeColumnCount INT UNSIGNED NOT NULL,
	+ UNIQUE KEY `key_trigger` (triggerPHID)
	+) ENGINE=InnoDB DEFAULT CHARSET={$CHARSET} COLLATE {$COLLATE_TEXT};
	diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
	--- a/src/__phutil_library_map__.php
	+++ b/src/__phutil_library_map__.php
	@@ -4193,6 +4193,8 @@
	'PhabricatorProjectTriggerTransactionQuery' => 'applications/project/query/PhabricatorProjectTriggerTransactionQuery.php',
	'PhabricatorProjectTriggerTransactionType' => 'applications/project/xaction/trigger/PhabricatorProjectTriggerTransactionType.php',
	'PhabricatorProjectTriggerUnknownRule' => 'applications/project/trigger/PhabricatorProjectTriggerUnknownRule.php',
	+ 'PhabricatorProjectTriggerUsage' => 'applications/project/storage/PhabricatorProjectTriggerUsage.php',
	+ 'PhabricatorProjectTriggerUsageIndexEngineExtension' => 'applications/project/engineextension/PhabricatorProjectTriggerUsageIndexEngineExtension.php',
	'PhabricatorProjectTriggerViewController' => 'applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php',
	'PhabricatorProjectTypeTransaction' => 'applications/project/xaction/PhabricatorProjectTypeTransaction.php',
	'PhabricatorProjectUIEventListener' => 'applications/project/events/PhabricatorProjectUIEventListener.php',
	@@ -10307,6 +10309,7 @@
	'PhabricatorProjectDAO',
	'PhabricatorApplicationTransactionInterface',
	'PhabricatorPolicyInterface',
	+ 'PhabricatorIndexableInterface',
	'PhabricatorDestructibleInterface',
	),
	'PhabricatorProjectTriggerController' => 'PhabricatorProjectController',
	@@ -10328,6 +10331,8 @@
	'PhabricatorProjectTriggerTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
	'PhabricatorProjectTriggerTransactionType' => 'PhabricatorModularTransactionType',
	'PhabricatorProjectTriggerUnknownRule' => 'PhabricatorProjectTriggerRule',
	+ 'PhabricatorProjectTriggerUsage' => 'PhabricatorProjectDAO',
	+ 'PhabricatorProjectTriggerUsageIndexEngineExtension' => 'PhabricatorIndexEngineExtension',
	'PhabricatorProjectTriggerViewController' => 'PhabricatorProjectTriggerController',
	'PhabricatorProjectTypeTransaction' => 'PhabricatorProjectTransactionType',
	'PhabricatorProjectUIEventListener' => 'PhabricatorEventListener',
	diff --git a/src/applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php b/src/applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php
	--- a/src/applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php
	+++ b/src/applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php
	@@ -71,17 +71,23 @@
	// so we load only the columns they can actually see, but have a list of
	// all the impacted column PHIDs.

	+ // (We're also exposing the status of columns the user might not be able
	+ // to see. This technically violates policy, but the trigger usage table
	+ // hints at it anyway and it seems unlikely to ever have any security
	+ // impact, but is useful in assessing whether a trigger is really in use
	+ // or not.)
	+
	$omnipotent_viewer = PhabricatorUser::getOmnipotentUser();
	$all_columns = id(new PhabricatorProjectColumnQuery())
	->setViewer($omnipotent_viewer)
	->withTriggerPHIDs(array($trigger->getPHID()))
	->execute();
	- $column_phids = mpull($all_columns, 'getPHID');
	+ $column_map = mpull($all_columns, 'getStatus', 'getPHID');

	- if ($column_phids) {
	+ if ($column_map) {
	$visible_columns = id(new PhabricatorProjectColumnQuery())
	->setViewer($viewer)
	- ->withPHIDs($column_phids)
	+ ->withPHIDs(array_keys($column_map))
	->execute();
	$visible_columns = mpull($visible_columns, null, 'getPHID');
	} else {
	@@ -89,7 +95,7 @@
	}

	$rows = array();
	- foreach ($column_phids as $column_phid) {
	+ foreach ($column_map as $column_phid => $column_status) {
	$column = idx($visible_columns, $column_phid);

	if ($column) {
	@@ -113,7 +119,18 @@
	$column_name = phutil_tag('em', array(), pht('Restricted Column'));
	}

	+ if ($column_status == PhabricatorProjectColumn::STATUS_ACTIVE) {
	+ $status_icon = id(new PHUIIconView())
	+ ->setIcon('fa-columns', 'blue')
	+ ->setTooltip(pht('Active Column'));
	+ } else {
	+ $status_icon = id(new PHUIIconView())
	+ ->setIcon('fa-eye-slash', 'grey')
	+ ->setTooltip(pht('Hidden Column'));
	+ }
	+
	$rows[] = array(
	+ $status_icon,
	$project_name,
	$column_name,
	);
	@@ -123,11 +140,13 @@
	->setNoDataString(pht('This trigger is not used by any columns.'))
	->setHeaders(
	array(
	+ null,
	pht('Project'),
	pht('Column'),
	))
	->setColumnClasses(
	array(
	+ null,
	null,
	'wide pri',
	));
	diff --git a/src/applications/project/editor/PhabricatorProjectTriggerEditor.php b/src/applications/project/editor/PhabricatorProjectTriggerEditor.php
	--- a/src/applications/project/editor/PhabricatorProjectTriggerEditor.php
	+++ b/src/applications/project/editor/PhabricatorProjectTriggerEditor.php
	@@ -27,4 +27,8 @@
	return $types;
	}

	+ protected function supportsSearch() {
	+ return true;
	+ }
	+
	}
	diff --git a/src/applications/project/engineextension/PhabricatorProjectTriggerUsageIndexEngineExtension.php b/src/applications/project/engineextension/PhabricatorProjectTriggerUsageIndexEngineExtension.php
	new file mode 100644
	--- /dev/null
	+++ b/src/applications/project/engineextension/PhabricatorProjectTriggerUsageIndexEngineExtension.php
	@@ -0,0 +1,69 @@
	+<?php
	+
	+final class PhabricatorProjectTriggerUsageIndexEngineExtension
	+ extends PhabricatorIndexEngineExtension {
	+
	+ const EXTENSIONKEY = 'trigger.usage';
	+
	+ public function getExtensionName() {
	+ return pht('Trigger Usage');
	+ }
	+
	+ public function shouldIndexObject($object) {
	+ if (!($object instanceof PhabricatorProjectTrigger)) {
	+ return false;
	+ }
	+
	+ return true;
	+ }
	+
	+ public function indexObject(
	+ PhabricatorIndexEngine $engine,
	+ $object) {
	+
	+ $usage_table = new PhabricatorProjectTriggerUsage();
	+ $column_table = new PhabricatorProjectColumn();
	+
	+ $conn_w = $object->establishConnection('w');
	+
	+ $active_statuses = array(
	+ PhabricatorProjectColumn::STATUS_ACTIVE,
	+ );
	+
	+ // Select summary information to populate the usage index. When picking
	+ // an "examplePHID", we try to pick an active column.
	+ $row = queryfx_one(
	+ $conn_w,
	+ 'SELECT phid, COUNT(*) N, SUM(IF(status IN (%Ls), 1, 0)) M FROM %R
	+ WHERE triggerPHID = %s
	+ ORDER BY IF(status IN (%Ls), 1, 0) DESC, id ASC',
	+ $active_statuses,
	+ $column_table,
	+ $object->getPHID(),
	+ $active_statuses);
	+ if ($row) {
	+ $example_phid = $row['phid'];
	+ $column_count = $row['N'];
	+ $active_count = $row['M'];
	+ } else {
	+ $example_phid = null;
	+ $column_count = 0;
	+ $active_count = 0;
	+ }
	+
	+ queryfx(
	+ $conn_w,
	+ 'INSERT INTO %R (triggerPHID, examplePHID, columnCount, activeColumnCount)
	+ VALUES (%s, %ns, %d, %d)
	+ ON DUPLICATE KEY UPDATE
	+ examplePHID = VALUES(examplePHID),
	+ columnCount = VALUES(columnCount),
	+ activeColumnCount = VALUES(activeColumnCount)',
	+ $usage_table,
	+ $object->getPHID(),
	+ $example_phid,
	+ $column_count,
	+ $active_count);
	+ }
	+
	+}
	diff --git a/src/applications/project/query/PhabricatorProjectTriggerQuery.php b/src/applications/project/query/PhabricatorProjectTriggerQuery.php
	--- a/src/applications/project/query/PhabricatorProjectTriggerQuery.php
	+++ b/src/applications/project/query/PhabricatorProjectTriggerQuery.php
	@@ -5,6 +5,10 @@

	private $ids;
	private $phids;
	+ private $activeColumnMin;
	+ private $activeColumnMax;
	+
	+ private $needUsage;

	public function withIDs(array $ids) {
	$this->ids = $ids;
	@@ -16,6 +20,17 @@
	return $this;
	}

	+ public function needUsage($need_usage) {
	+ $this->needUsage = $need_usage;
	+ return $this;
	+ }
	+
	+ public function withActiveColumnCountBetween($min, $max) {
	+ $this->activeColumnMin = $min;
	+ $this->activeColumnMax = $max;
	+ return $this;
	+ }
	+
	public function newResultObject() {
	return new PhabricatorProjectTrigger();
	}
	@@ -30,22 +45,91 @@
	if ($this->ids !== null) {
	$where[] = qsprintf(
	$conn,
	- 'id IN (%Ld)',
	+ 'trigger.id IN (%Ld)',
	$this->ids);
	}

	if ($this->phids !== null) {
	$where[] = qsprintf(
	$conn,
	- 'phid IN (%Ls)',
	+ 'trigger.phid IN (%Ls)',
	$this->phids);
	}

	+ if ($this->activeColumnMin !== null) {
	+ $where[] = qsprintf(
	+ $conn,
	+ 'trigger_usage.activeColumnCount >= %d',
	+ $this->activeColumnMin);
	+ }
	+
	+ if ($this->activeColumnMax !== null) {
	+ $where[] = qsprintf(
	+ $conn,
	+ 'trigger_usage.activeColumnCount <= %d',
	+ $this->activeColumnMax);
	+ }
	+
	return $where;
	}

	+ protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
	+ $joins = parent::buildJoinClauseParts($conn);
	+
	+ if ($this->shouldJoinUsageTable()) {
	+ $joins[] = qsprintf(
	+ $conn,
	+ 'JOIN %R trigger_usage ON trigger.phid = trigger_usage.triggerPHID',
	+ new PhabricatorProjectTriggerUsage());
	+ }
	+
	+ return $joins;
	+ }
	+
	+ private function shouldJoinUsageTable() {
	+ if ($this->activeColumnMin !== null) {
	+ return true;
	+ }
	+
	+ if ($this->activeColumnMax !== null) {
	+ return true;
	+ }
	+
	+ return false;
	+ }
	+
	+ protected function didFilterPage(array $triggers) {
	+ if ($this->needUsage) {
	+ $usage_map = id(new PhabricatorProjectTriggerUsage())->loadAllWhere(
	+ 'triggerPHID IN (%Ls)',
	+ mpull($triggers, 'getPHID'));
	+ $usage_map = mpull($usage_map, null, 'getTriggerPHID');
	+
	+ foreach ($triggers as $trigger) {
	+ $trigger_phid = $trigger->getPHID();
	+
	+ $usage = idx($usage_map, $trigger_phid);
	+ if (!$usage) {
	+ $usage = id(new PhabricatorProjectTriggerUsage())
	+ ->setTriggerPHID($trigger_phid)
	+ ->setExamplePHID(null)
	+ ->setColumnCount(0)
	+ ->setActiveColumnCount(0);
	+ }
	+
	+ $trigger->attachUsage($usage);
	+ }
	+ }
	+
	+ return $triggers;
	+ }
	+
	public function getQueryApplicationClass() {
	return 'PhabricatorProjectApplication';
	}

	+ protected function getPrimaryTableAlias() {
	+ return 'trigger';
	+ }
	+
	}
	diff --git a/src/applications/project/query/PhabricatorProjectTriggerSearchEngine.php b/src/applications/project/query/PhabricatorProjectTriggerSearchEngine.php
	--- a/src/applications/project/query/PhabricatorProjectTriggerSearchEngine.php
	+++ b/src/applications/project/query/PhabricatorProjectTriggerSearchEngine.php
	@@ -12,16 +12,33 @@
	}

	public function newQuery() {
	- return new PhabricatorProjectTriggerQuery();
	+ return id(new PhabricatorProjectTriggerQuery())
	+ ->needUsage(true);
	}

	protected function buildCustomSearchFields() {
	- return array();
	+ return array(
	+ id(new PhabricatorSearchThreeStateField())
	+ ->setLabel(pht('Active'))
	+ ->setKey('isActive')
	+ ->setOptions(
	+ pht('(Show All)'),
	+ pht('Show Only Active Triggers'),
	+ pht('Show Only Inactive Triggers')),
	+ );
	}

	protected function buildQueryFromParameters(array $map) {
	$query = $this->newQuery();

	+ if ($map['isActive'] !== null) {
	+ if ($map['isActive']) {
	+ $query->withActiveColumnCountBetween(1, null);
	+ } else {
	+ $query->withActiveColumnCountBetween(null, 0);
	+ }
	+ }
	+
	return $query;
	}

	@@ -32,7 +49,8 @@
	protected function getBuiltinQueryNames() {
	$names = array();

	- $names['all'] = pht('All');
	+ $names['active'] = pht('Active Triggers');
	+ $names['all'] = pht('All Triggers');

	return $names;
	}
	@@ -42,6 +60,8 @@
	$query->setQueryKey($query_key);

	switch ($query_key) {
	+ case 'active':
	+ return $query->setParameter('isActive', true);
	case 'all':
	return $query;
	}
	@@ -56,13 +76,73 @@
	assert_instances_of($triggers, 'PhabricatorProjectTrigger');
	$viewer = $this->requireViewer();

	+ $example_phids = array();
	+ foreach ($triggers as $trigger) {
	+ $example_phid = $trigger->getUsage()->getExamplePHID();
	+ if ($example_phid) {
	+ $example_phids[] = $example_phid;
	+ }
	+ }
	+
	+ $handles = $viewer->loadHandles($example_phids);
	+
	$list = id(new PHUIObjectItemListView())
	->setViewer($viewer);
	foreach ($triggers as $trigger) {
	+ $usage = $trigger->getUsage();
	+
	+ $column_handle = null;
	+ $have_column = false;
	+ $example_phid = $usage->getExamplePHID();
	+ if ($example_phid) {
	+ $column_handle = $handles[$example_phid];
	+ if ($column_handle->isComplete()) {
	+ if (!$column_handle->getPolicyFiltered()) {
	+ $have_column = true;
	+ }
	+ }
	+ }
	+
	+ $column_count = $usage->getColumnCount();
	+ $active_count = $usage->getActiveColumnCount();
	+
	+ if ($have_column) {
	+ if ($active_count > 1) {
	+ $usage_description = pht(
	+ 'Used on %s and %s other active column(s).',
	+ $column_handle->renderLink(),
	+ new PhutilNumber($active_count - 1));
	+ } else if ($column_count > 1) {
	+ $usage_description = pht(
	+ 'Used on %s and %s other column(s).',
	+ $column_handle->renderLink(),
	+ new PhutilNumber($column_count - 1));
	+ } else {
	+ $usage_description = pht(
	+ 'Used on %s.',
	+ $column_handle->renderLink());
	+ }
	+ } else {
	+ if ($active_count) {
	+ $usage_description = pht(
	+ 'Used on %s active column(s).',
	+ new PhutilNumber($active_count));
	+ } else if ($column_count) {
	+ $usage_description = pht(
	+ 'Used on %s column(s).',
	+ new PhutilNumber($column_count));
	+ } else {
	+ $usage_description = pht(
	+ 'Unused trigger.');
	+ }
	+ }
	+
	$item = id(new PHUIObjectItemView())
	->setObjectName($trigger->getObjectName())
	->setHeader($trigger->getDisplayName())
	- ->setHref($trigger->getURI());
	+ ->setHref($trigger->getURI())
	+ ->addAttribute($usage_description)
	+ ->setDisabled(!$active_count);

	$list->addItem($item);
	}
	diff --git a/src/applications/project/storage/PhabricatorProjectTrigger.php b/src/applications/project/storage/PhabricatorProjectTrigger.php
	--- a/src/applications/project/storage/PhabricatorProjectTrigger.php
	+++ b/src/applications/project/storage/PhabricatorProjectTrigger.php
	@@ -5,6 +5,7 @@
	implements
	PhabricatorApplicationTransactionInterface,
	PhabricatorPolicyInterface,
	+ PhabricatorIndexableInterface,
	PhabricatorDestructibleInterface {

	protected $name;
	@@ -12,6 +13,7 @@
	protected $editPolicy;

	private $triggerRules;
	+ private $usage = self::ATTACHABLE;

	public static function initializeNewTrigger() {
	$default_edit = PhabricatorPolicies::POLICY_USER;
	@@ -257,6 +259,15 @@
	return $sounds;
	}

	+ public function getUsage() {
	+ return $this->assertAttached($this->usage);
	+ }
	+
	+ public function attachUsage(PhabricatorProjectTriggerUsage $usage) {
	+ $this->usage = $usage;
	+ return $this;
	+ }
	+

	/* -( PhabricatorApplicationTransactionInterface )------------------------- */

	@@ -310,6 +321,13 @@
	new PhabricatorProjectColumn(),
	$this->getPHID());

	+ // Remove the usage index row for this trigger, if one exists.
	+ queryfx(
	+ $conn,
	+ 'DELETE FROM %R WHERE triggerPHID = %s',
	+ new PhabricatorProjectTriggerUsage(),
	+ $this->getPHID());
	+
	$this->delete();

	$this->saveTransaction();
	diff --git a/src/applications/project/storage/PhabricatorProjectTriggerUsage.php b/src/applications/project/storage/PhabricatorProjectTriggerUsage.php
	new file mode 100644
	--- /dev/null
	+++ b/src/applications/project/storage/PhabricatorProjectTriggerUsage.php
	@@ -0,0 +1,28 @@
	+<?php
	+
	+final class PhabricatorProjectTriggerUsage
	+ extends PhabricatorProjectDAO {
	+
	+ protected $triggerPHID;
	+ protected $examplePHID;
	+ protected $columnCount;
	+ protected $activeColumnCount;
	+
	+ protected function getConfiguration() {
	+ return array(
	+ self::CONFIG_TIMESTAMPS => false,
	+ self::CONFIG_COLUMN_SCHEMA => array(
	+ 'examplePHID' => 'phid?',
	+ 'columnCount' => 'uint32',
	+ 'activeColumnCount' => 'uint32',
	+ ),
	+ self::CONFIG_KEY_SCHEMA => array(
	+ 'key_trigger' => array(
	+ 'columns' => array('triggerPHID'),
	+ 'unique' => true,
	+ ),
	+ ),
	+ ) + parent::getConfiguration();
	+ }
	+
	+}
	diff --git a/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php b/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php
	--- a/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php
	+++ b/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php
	@@ -13,6 +13,15 @@
	$object->setStatus($value);
	}

	+ public function applyExternalEffects($object, $value) {
	+ // Update the trigger usage index, which cares about whether columns are
	+ // active or not.
	+ $trigger_phid = $object->getTriggerPHID();
	+ if ($trigger_phid) {
	+ PhabricatorSearchWorker::queueDocumentForIndexing($trigger_phid);
	+ }
	+ }
	+
	public function getTitle() {
	$new = $this->getNewValue();

	diff --git a/src/applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php b/src/applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php
	--- a/src/applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php
	+++ b/src/applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php
	@@ -13,6 +13,25 @@
	$object->setTriggerPHID($value);
	}

	+ public function applyExternalEffects($object, $value) {
	+ // After we change the trigger attached to a column, update the search
	+ // indexes for the old and new triggers so we update the usage index.
	+ $old = $this->getOldValue();
	+ $new = $this->getNewValue();
	+
	+ $column_phids = array();
	+ if ($old) {
	+ $column_phids[] = $old;
	+ }
	+ if ($new) {
	+ $column_phids[] = $new;
	+ }
	+
	+ foreach ($column_phids as $phid) {
	+ PhabricatorSearchWorker::queueDocumentForIndexing($phid);
	+ }
	+ }
	+
	public function getTitle() {
	$old = $this->getOldValue();
	$new = $this->getNewValue();
	diff --git a/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php b/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php
	--- a/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php
	+++ b/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php
	@@ -136,7 +136,13 @@

	if ($track_skips) {
	$new_versions = $this->loadIndexVersions($phid);
	- if ($old_versions !== $new_versions) {
	+
	+ if (!$old_versions && !$new_versions) {
	+ // If the document doesn't use an index version, both the lists
	+ // of versions will be empty. We still rebuild the index in this
	+ // case.
	+ $count_updated++;
	+ } else if ($old_versions !== $new_versions) {
	$count_updated++;
	} else {
	$count_skipped++;

File Metadata

Mime Type: text/plain
Expires: Fri, Oct 18, 2:04 PM (9 h, 41 m)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 6713567
Default Alt Text: D20308.id48505.diff (20 KB)

D20308.id48505.diffNo OneTemporaryActions

D20308.id48505.diffView Options

File Metadata

Event Timeline

D20308.id48505.diff
No OneTemporary
Actions

D20308.id48505.diff
View Options