Page MenuHomePhabricator

D10035.id25277.diff
No OneTemporary

D10035.id25277.diff

diff --git a/resources/sql/autopatches/20140830.imagemacrousage.sql b/resources/sql/autopatches/20140830.imagemacrousage.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20140830.imagemacrousage.sql
@@ -0,0 +1,8 @@
+CREATE TABLE {$NAMESPACE}_file.file_imagemacrousage (
+ id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+ macroPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+ authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+ usagePHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+ count INT UNSIGNED NOT NULL
+
+) ENGINE=InnoDB, COLLATE utf8_general_ci;
diff --git a/resources/sql/autopatches/20140830.imagemacrousagebackfill.php b/resources/sql/autopatches/20140830.imagemacrousagebackfill.php
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20140830.imagemacrousagebackfill.php
@@ -0,0 +1,46 @@
+<?php
+$xaction_classes = id(new PhutilSymbolLoader())
+ ->setAncestorClass('PhabricatorApplicationTransactionComment')
+ ->loadObjects();
+
+$engine = PhabricatorMarkupEngine::getEngine('extract');
+$engine->setConfig('viewer', PhabricatorUser::getOmnipotentUser());
+PhabricatorEnv::setRequestBaseURI('/'); // Doesn't matter during backfill
+
+$macro_usage_table = new PhabricatorFileImageMacroUsage();
+$macro_usage_key = PhabricatorImageMacroRemarkupRule::KEY_MACRO_USAGE;
+
+/* If I uncomment this, I get the following error.
+
+Argument 1 passed to PhabricatorApplicationTransactionComment::getTableNameFromTransaction() must be an instance of PhabricatorApplicationTransaction, instance of HarbormasterBuildPlan given, called in /home/vagrant/src/phabricator/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php on line 52 and defined at [<phutil>/src/error/PhutilErrorHandler.php:200]
+ #0 PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phabricator>/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php:56]
+ #1 PhabricatorApplicationTransactionComment::getTableNameFromTransaction(HarbormasterBuildPlan) called at [<phabricator>/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php:52]
+ #2 PhabricatorApplicationTransactionComment::getTableName() called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:498]
+ #3 LiskDAO::loadRawDataWhere(string, integer, integer)
+ #4 call_user_func_array(array, array) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:447]
+ #5 LiskDAO::loadAllWhere(string, integer, integer) called at [<phabricator>/src/infrastructure/storage/lisk/LiskMigrationIterator.php:41]
+ #6 LiskMigrationIterator::loadPage() called at [<phutil>/src/utils/PhutilBufferedIterator.php:129]
+ #7 PhutilBufferedIterator::next() called at [<phutil>/src/utils/PhutilBufferedIterator.php:84]
+ #8 PhutilBufferedIterator::rewind() called at [<phabricator>/resources/sql/autopatches/20140830.imagemacrousagebackfill.php:16]
+ #9 require_once(string) called at [<phabricator>/src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php:196]
+ #10 PhabricatorStorageManagementAPI::applyPatchPHP(string) called at [<phabricator>/src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php:171]
+ #11 PhabricatorStorageManagementAPI::applyPatch(PhabricatorStoragePatch) called at [<phabricator>/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementUpgradeWorkflow.php:168]
+ #12 PhabricatorStorageManagementUpgradeWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:394]
+ #13 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:290]
+ #14 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/sql/manage_storage.php:163]
+
+Looks like some of the PhabricatorApplicationTransactionComment objects work, but some don't. Is this not something we can iterate over?
+*/
+$xaction_classes = array($xaction_classes['DifferentialTransactionComment']);
+
+foreach ($xaction_classes as $xaction_class) {
+ foreach (new LiskMigrationIterator($xaction_class) as $xaction) {
+ $authorPHID = $xaction->getAuthorPHID();
+ $usagePHID = $xaction->getPHID();
+
+ $block = $xaction->getContent();
+ $engine->markupText($block);
+ $macro_usage = $engine->getTextMetadata($macro_usage_key, array());
+ $macro_usage_table->updateUsage($authorPHID, $usagePHID, $macro_usage);
+ }
+}
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
@@ -1595,6 +1595,7 @@
'PhabricatorFileEditor' => 'applications/files/editor/PhabricatorFileEditor.php',
'PhabricatorFileFilePHIDType' => 'applications/files/phid/PhabricatorFileFilePHIDType.php',
'PhabricatorFileImageMacro' => 'applications/macro/storage/PhabricatorFileImageMacro.php',
+ 'PhabricatorFileImageMacroUsage' => 'applications/macro/storage/PhabricatorFileImageMacroUsage.php',
'PhabricatorFileInfoController' => 'applications/files/controller/PhabricatorFileInfoController.php',
'PhabricatorFileLinkListView' => 'view/layout/PhabricatorFileLinkListView.php',
'PhabricatorFileLinkView' => 'view/layout/PhabricatorFileLinkView.php',
@@ -4513,6 +4514,7 @@
'PhabricatorFlaggableInterface',
'PhabricatorPolicyInterface',
),
+ 'PhabricatorFileImageMacroUsage' => 'PhabricatorFileDAO',
'PhabricatorFileInfoController' => 'PhabricatorFileController',
'PhabricatorFileLinkListView' => 'AphrontView',
'PhabricatorFileLinkView' => 'AphrontView',
diff --git a/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php b/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php
--- a/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php
+++ b/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php
@@ -5,6 +5,7 @@
private $macros;
const KEY_RULE_MACRO = 'rule.macro';
+ const KEY_MACRO_USAGE = 'rule.macro_usage';
public function apply($text) {
return preg_replace_callback(
@@ -158,6 +159,17 @@
$engine->overwriteStoredText($spec['token'], $result);
}
+ $macro_usage_key = self::KEY_MACRO_USAGE;
+ $macro_usage = $engine->getTextMetadata($macro_usage_key, array());
+ foreach ($metadata as $spec) {
+ if (isset($macro_usage[$spec['phid']])) {
+ $macro_usage[$spec['phid']]++;
+ } else {
+ $macro_usage[$spec['phid']] = 1;
+ }
+ }
+ $engine->setTextMetadata($macro_usage_key, $macro_usage);
+
$engine->setTextMetadata($metadata_key, array());
}
diff --git a/src/applications/macro/query/PhabricatorMacroQuery.php b/src/applications/macro/query/PhabricatorMacroQuery.php
--- a/src/applications/macro/query/PhabricatorMacroQuery.php
+++ b/src/applications/macro/query/PhabricatorMacroQuery.php
@@ -3,9 +3,11 @@
final class PhabricatorMacroQuery
extends PhabricatorCursorPagedPolicyAwareQuery {
+ private $byPopularity;
private $ids;
private $phids;
private $authors;
+ private $abusers;
private $names;
private $nameLike;
private $dateCreatedAfter;
@@ -40,6 +42,11 @@
return $options;
}
+ public function withPopularitySort($use_popularity_sort) {
+ $this->byPopularity = $use_popularity_sort;
+ return $this;
+ }
+
public function withIDs(array $ids) {
$this->ids = $ids;
return $this;
@@ -55,6 +62,11 @@
return $this;
}
+ public function withAbuserPHIDs(array $abusers) {
+ $this->abusers = $abusers;
+ return $this;
+ }
+
public function withNameLike($name) {
$this->nameLike = $name;
return $this;
@@ -94,15 +106,77 @@
$macro_table = new PhabricatorFileImageMacro();
$conn = $macro_table->establishConnection('r');
- $rows = queryfx_all(
+ if ($this->byPopularity) {
+ $rows = queryfx_all(
+ $conn,
+ 'SELECT m.*, SUM(n.count) as popularity FROM %T m %Q %Q %Q %Q %Q %Q',
+ $macro_table->getTableName(),
+ $this->buildJoinClause($conn),
+ $this->buildWhereClause($conn),
+ $this->buildGroupClause($conn),
+ $this->buildHavingClause($conn),
+ $this->buildOrderClause($conn),
+ $this->buildLimitClause($conn));
+ } else {
+ $rows = queryfx_all(
+ $conn,
+ 'SELECT m.* FROM %T m %Q %Q %Q',
+ $macro_table->getTableName(),
+ $this->buildWhereClause($conn),
+ $this->buildOrderClause($conn),
+ $this->buildLimitClause($conn));
+ }
+
+ $macros = $macro_table->loadAllFromArray($rows);
+ if ($this->byPopularity) {
+ $macros_by_id = mpull($macros, null, 'getId');
+
+ $macro_usage_table = new PhabricatorFileImageMacroUsage();
+ $author_popularity_rows = queryfx_all(
+ $conn,
+ 'SELECT macroPHID, authorPHID, SUM(count) as authorUses FROM %T
+ GROUP BY macroPHID, authorPHID
+ ORDER BY macroPHID, authorUses',
+ $macro_usage_table->getTableName());
+ $by_macro = ipull($author_popularity_rows, null, 'macroPHID');
+
+ foreach ($rows as $row) {
+ $macro_id = $row['id'];
+ $macro_phid = $row['phid'];
+ $author_phid = isset($by_macro[$macro_phid]) ?
+ $by_macro[$macro_phid]['authorPHID'] : null;
+ $author_uses = isset($by_macro[$macro_phid]) ?
+ $by_macro[$macro_phid]['authorUses'] : null;
+ $macros_by_id[$macro_id]->setPopularity(
+ $row['popularity'] != null ? $row['popularity'] : 0,
+ $author_phid,
+ $author_uses);
+ }
+ }
+ return $macros;
+ }
+
+
+ private function buildHavingClause(AphrontDatabaseConnection $conn) {
+ $where = $this->buildPagingClause($conn);
+ if ($where != null) {
+ return 'HAVING '.$where;
+ }
+ return '';
+ }
+
+ private function buildJoinClause(AphrontDatabaseConnection $conn) {
+ assert($this->byPopularity);
+ $macro_usage_table = new PhabricatorFileImageMacroUsage();
+
+ $joins = array();
+ $joins[] = qsprintf(
$conn,
- 'SELECT m.* FROM %T m %Q %Q %Q',
- $macro_table->getTableName(),
- $this->buildWhereClause($conn),
- $this->buildOrderClause($conn),
- $this->buildLimitClause($conn));
+ 'LEFT JOIN %T n ON m.phid = n.macroPHID',
+ $macro_usage_table->getTableName());
+
+ return implode(' ', $joins);
- return $macro_table->loadAllFromArray($rows);
}
protected function buildWhereClause(AphrontDatabaseConnection $conn) {
@@ -129,6 +203,13 @@
$this->authors);
}
+ if ($this->abusers && $this->byPopularity) {
+ $where[] = qsprintf(
+ $conn,
+ 'n.authorPHID IN (%Ls)',
+ $this->abusers);
+ }
+
if ($this->nameLike) {
$where[] = qsprintf(
$conn,
@@ -197,11 +278,22 @@
}
}
- $where[] = $this->buildPagingClause($conn);
+ if (!$this->byPopularity) {
+ $where[] = $this->buildPagingClause($conn);
+ }
return $this->formatWhereClause($where);
}
+ private function buildGroupClause(AphrontDatabaseConnection $conn) {
+ $group = array();
+ $group[] = qsprintf(
+ $conn,
+ 'GROUP BY m.phid');
+
+ return implode(' ', $group);
+ }
+
protected function didFilterPage(array $macros) {
if ($this->needFiles) {
$file_phids = mpull($macros, 'getFilePHID');
@@ -226,7 +318,50 @@
}
protected function getPagingColumn() {
- return 'm.id';
+ $popularity_order = 'popularity '.
+ ($this->getBeforeID() ? 'ASC' : 'DESC').', m.id';
+ return $this->byPopularity ? $popularity_order : 'm.id';
+ }
+
+ protected function getPagingValue($macro) {
+ if ($this->byPopularity) {
+ $popularity = $macro->getPopularity();
+ return (string)$popularity['popularity'].','.(string)$macro->getId();
+ }
+ return PhabricatorCursorPagedPolicyAwareQuery::getPagingValue($macro);
+ }
+
+ protected function buildPagingClause(
+ AphrontDatabaseConnection $conn_r) {
+
+ if ($this->byPopularity) {
+ if ($this->getBeforeID()) {
+ $strs = explode(',', $this->getBeforeID());
+ $beforePopularity = (int)$strs[0];
+ $beforeId = (int)$strs[1];
+
+ return qsprintf(
+ $conn_r,
+ 'popularity > %d OR (popularity = %d AND m.id > %d)',
+ $beforePopularity,
+ $beforePopularity,
+ $beforeId);
+ } else if ($this->getAfterID()) {
+ $strs = explode(',', $this->getAfterID());
+ $afterPopularity = (int)$strs[0];
+ $afterId = (int)$strs[1];
+
+ return qsprintf(
+ $conn_r,
+ 'popularity < %d OR (popularity = %d AND m.id < %d)',
+ $afterPopularity,
+ $afterPopularity,
+ $afterId);
+ }
+
+ return null;
+ }
+ return PhabricatorCursorPagedPolicyAwareQuery::buildPagingClause($conn_r);
}
public function getQueryApplicationClass() {
diff --git a/src/applications/macro/query/PhabricatorMacroSearchEngine.php b/src/applications/macro/query/PhabricatorMacroSearchEngine.php
--- a/src/applications/macro/query/PhabricatorMacroSearchEngine.php
+++ b/src/applications/macro/query/PhabricatorMacroSearchEngine.php
@@ -17,6 +17,12 @@
'authorPHIDs',
$this->readUsersFromRequest($request, 'authors'));
+ $abuser_phids = $this->readUsersFromRequest($request, 'abusers');
+ $saved->setParameter(
+ 'abuserPHIDs',
+ $abuser_phids);
+ $saved->setParameter('popularitySort', !empty($abuser_phids));
+
$saved->setParameter('status', $request->getStr('status'));
$saved->setParameter('names', $request->getStrList('names'));
$saved->setParameter('nameLike', $request->getStr('nameLike'));
@@ -32,7 +38,9 @@
->needFiles(true)
->withIDs($saved->getParameter('ids', array()))
->withPHIDs($saved->getParameter('phids', array()))
- ->withAuthorPHIDs($saved->getParameter('authorPHIDs', array()));
+ ->withAuthorPHIDs($saved->getParameter('authorPHIDs', array()))
+ ->withAbuserPHIDs($saved->getParameter('abuserPHIDs', array()))
+ ->withPopularitySort($saved->getParameter('popularitySort', false));
$status = $saved->getParameter('status');
$options = PhabricatorMacroQuery::getStatusOptions();
@@ -80,6 +88,12 @@
->withPHIDs($phids)
->execute();
+ $abuser_phids = $saved_query->getParameter('abuserPHIDs', array());
+ $abuser_handles = id(new PhabricatorHandleQuery())
+ ->setViewer($this->requireViewer())
+ ->withPHIDs($abuser_phids)
+ ->execute();
+
$status = $saved_query->getParameter('status');
$names = implode(', ', $saved_query->getParameter('names', array()));
$like = $saved_query->getParameter('nameLike');
@@ -99,6 +113,12 @@
->setLabel(pht('Authors'))
->setValue($author_handles))
->appendChild(
+ id(new AphrontFormTokenizerControl())
+ ->setDatasource(new PhabricatorPeopleDatasource())
+ ->setName('abusers')
+ ->setLabel(pht('Abused By'))
+ ->setValue($abuser_handles))
+ ->appendChild(
id(new AphrontFormTextControl())
->setName('nameLike')
->setLabel(pht('Name Contains'))
@@ -131,11 +151,13 @@
public function getBuiltinQueryNames() {
$names = array(
'active' => pht('Active'),
- 'all' => pht('All'),
+ 'recent' => pht('Recent'),
+ 'popular' => pht('Popular'),
);
if ($this->requireViewer()->isLoggedIn()) {
$names['authored'] = pht('Authored');
+ $names['favorites'] = pht('Favorites');
}
return $names;
@@ -148,7 +170,7 @@
switch ($query_key) {
case 'active':
return $query;
- case 'all':
+ case 'recent':
return $query->setParameter(
'status',
PhabricatorMacroQuery::STATUS_ANY);
@@ -156,6 +178,25 @@
return $query->setParameter(
'authorPHIDs',
array($this->requireViewer()->getPHID()));
+ case 'popular':
+ return $query
+ ->setParameter(
+ 'status',
+ PhabricatorMacroQuery::STATUS_ANY)
+ ->setParameter(
+ 'popularitySort',
+ true);
+ case 'favorites':
+ return $query
+ ->setParameter(
+ 'status',
+ PhabricatorMacroQuery::STATUS_ANY)
+ ->setParameter(
+ 'popularitySort',
+ true)
+ ->setParameter(
+ 'abuserPHIDs',
+ array($this->requireViewer()->getPHID()));
}
return parent::buildSavedQueryFromBuiltin($query_key);
@@ -164,7 +205,9 @@
protected function getRequiredHandlePHIDsForResultList(
array $macros,
PhabricatorSavedQuery $query) {
- return mpull($macros, 'getAuthorPHID');
+ return array_merge(
+ mpull($macros, 'getAuthorPHID'),
+ ipull(mpull($macros, 'getPopularity'), 'top_abuser'));
}
protected function renderResultList(
@@ -208,6 +251,30 @@
pht('Created by %s', $author_handle->renderLink()));
}
+ $popularity = $macro->getPopularity();
+ if ($popularity != null) {
+ $popularizer_phid = $popularity['top_abuser'];
+
+ $item->appendChild(
+ phutil_tag(
+ 'div',
+ array(),
+ pht('Popularity %d', $popularity['popularity'])));
+
+ $message = $popularity['popularity'] == 0 ?
+ pht('Wow. This macro must suck.') :
+ pht('Abused most by %s (%d time%s)',
+ $handles[$popularizer_phid]->renderLink(),
+ $popularity['top_abuser_count'],
+ $popularity['top_abuser_count'] != 1 ? 's' : '');
+ $item->appendChild(
+ phutil_tag(
+ 'div',
+ array(),
+ $message));
+ }
+
+
$item->setURI($this->getApplicationURI('/view/'.$macro->getID().'/'));
$item->setDisabled($macro->getisDisabled());
$item->setHeader($macro->getName());
diff --git a/src/applications/macro/storage/PhabricatorFileImageMacro.php b/src/applications/macro/storage/PhabricatorFileImageMacro.php
--- a/src/applications/macro/storage/PhabricatorFileImageMacro.php
+++ b/src/applications/macro/storage/PhabricatorFileImageMacro.php
@@ -17,6 +17,9 @@
private $file = self::ATTACHABLE;
private $audio = self::ATTACHABLE;
+ private $popularity = null;
+ private $topAbuser = null;
+ private $topAbuserCount = null;
const AUDIO_BEHAVIOR_NONE = 'audio:none';
const AUDIO_BEHAVIOR_ONCE = 'audio:once';
@@ -59,6 +62,23 @@
return parent::save();
}
+ public function setPopularity($popularity, $top_abuser, $top_abuser_count) {
+ $this->popularity = $popularity;
+ $this->topAbuser = $top_abuser;
+ $this->topAbuserCount = $top_abuser_count;
+ }
+
+ public function getPopularity() {
+ if (is_null($this->popularity)) {
+ return null;
+ } else {
+ return array(
+ 'popularity' => $this->popularity,
+ 'top_abuser' => $this->topAbuser,
+ 'top_abuser_count' => $this->topAbuserCount);
+ }
+ }
+
/* -( PhabricatorApplicationTransactionInterface )------------------------- */
diff --git a/src/applications/macro/storage/PhabricatorFileImageMacroUsage.php b/src/applications/macro/storage/PhabricatorFileImageMacroUsage.php
new file mode 100644
--- /dev/null
+++ b/src/applications/macro/storage/PhabricatorFileImageMacroUsage.php
@@ -0,0 +1,32 @@
+<?php
+final class PhabricatorFileImageMacroUsage extends PhabricatorFileDAO {
+
+ /********************************/
+ /* Update our macro usage table */
+ /********************************/
+
+ public function updateUsage($author_phid, $usage_phid, $macros) {
+ $conn = $this->establishConnection('w');
+
+ // Append to our wonderful macrousage table
+ $values = array();
+ foreach ($macros as $macro_phid => $count) {
+ $values[] = qsprintf(
+ $conn,
+ '(%s, %s, %s, %d)',
+ $macro_phid,
+ $author_phid,
+ $usage_phid,
+ $count);
+ }
+ $values_clause = implode(',', $values);
+
+ queryfx(
+ $conn,
+ 'INSERT INTO %T
+ (macroPHID, authorPHID, usagePHID, count)
+ VALUES %Q',
+ $this->getTableName(),
+ $values_clause);
+ }
+}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -1166,6 +1166,19 @@
$blocks,
$engine);
+ $macro_usage_table = new PhabricatorFileImageMacroUsage();
+ $macro_usage_key = PhabricatorImageMacroRemarkupRule::KEY_MACRO_USAGE;
+ $authorPHID = $object->getAuthorPHID();
+ $usagePHID = $object->getPHID();
+
+ foreach ($blocks as $key => $xaction_blocks) {
+ foreach ($xaction_blocks as $block) {
+ $engine->markupText($block);
+ $macro_usage = $engine->getTextMetadata($macro_usage_key, array());
+ $macro_usage_table->updateUsage($authorPHID, $usagePHID, $macro_usage);
+ }
+ }
+
$mentioned_phids = array();
foreach ($blocks as $key => $xaction_blocks) {
foreach ($xaction_blocks as $block) {

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 18, 12:42 AM (1 w, 18 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7704080
Default Alt Text
D10035.id25277.diff (21 KB)

Event Timeline