From 6a3eb19876213c70689ff498e88d502c4db42015 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Wed, 29 Jun 2011 19:45:12 -0700 Subject: [PATCH 01/34] Provide a basic update script for Phabricator Summary: This isn't completely cross-system compatible but it's definitely better than nothing. Test Plan: Pushed secure.phabricator.com a bunch of times. Reviewed By: moskov Reviewers: moskov CC: aran, moskov Differential Revision: 558 --- scripts/install/update_phabricator.sh | 62 +++++++++++++++++++++++++++ src/docs/installation_guide.diviner | 7 +++ 2 files changed, 69 insertions(+) create mode 100755 scripts/install/update_phabricator.sh diff --git a/scripts/install/update_phabricator.sh b/scripts/install/update_phabricator.sh new file mode 100755 index 0000000000..207650d108 --- /dev/null +++ b/scripts/install/update_phabricator.sh @@ -0,0 +1,62 @@ +#!/bin/sh + +set -e +set -x + +# This is an example script for updating Phabricator, similar to the one used to +# update <https://secure.phabricator.com/>. It might not work perfectly on your +# system, but hopefully it should be easy to adapt. + +# NOTE: This script assumes you are running it from a directory which contains +# arcanist/, libphutil/, phabricator/, and possibly diviner/. + +ROOT=`pwd` # You can hard-code the path here instead. + + +### UPDATE WORKING COPIES ###################################################### + +if [ -e $ROOT/diviner ] +then + (cd $ROOT/diviner && git pull) +fi + +(cd $ROOT/libphutil && git pull) +(cd $ROOT/arcanist && git pull) +(cd $ROOT/phabricator && git pull && git submodule update --init) + + +### GENERATE DOCUMENTATION ##################################################### + +# This generates documentation if you have diviner/ checked out. You generally +# don't need to do this unless you're contributing to Phabricator and want to +# preview some of the amazing documentation you've just written. +if [ -e $ROOT/diviner ] +then + (cd $ROOT/diviner && $ROOT/diviner/bin/diviner .) + (cd $ROOT/libphutil && $ROOT/diviner/bin/diviner .) + (cd $ROOT/arcanist && $ROOT/diviner/bin/diviner .) + (cd $ROOT/phabricator && $ROOT/diviner/bin/diviner .) +fi + +### CYCLE APACHE AND DAEMONS ################################################### + +# Stop Apache. Depening on what system you're running, you may need to use +# 'apachectl' or something else to cycle apache. +sudo /etc/init.d/httpd stop + +# Stop daemons. +$ROOT/phabricator/bin/phd stop + +# Upgrade the database schema. +$ROOT/phabricator/scripts/sql/upgrade_schema.php -f + +# Restart apache. +sudo /etc/init.d/httpd start + +# Restart daemons. Customize this to start whatever daemons you're running on +# your system. + +# $ROOT/phabricator/bin/phd repository-launch-master +# $ROOT/phabricator/bin/phd launch metamta +# $ROOT/phabricator/bin/phd launch 4 taskmaster +# $ROOT/phabricator/bin/phd launch ircbot /config/bot.json diff --git a/src/docs/installation_guide.diviner b/src/docs/installation_guide.diviner index a10ebe062b..449ea71bd2 100644 --- a/src/docs/installation_guide.diviner +++ b/src/docs/installation_guide.diviner @@ -102,6 +102,13 @@ You can find installation instructions for xhprof here: You only need to install the PHP extension, not any of the library. += Updating Phabricator = + +Since Phabricator is under active development, you should update frequently. +You can use a script similar to this one to automate the process: + + http://phabricator.com/rsrc/install/update_phabricator.sh + = Next Steps = Continue by: From de0c89261e9ea789af9c54b72d01437cb043c467 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Wed, 29 Jun 2011 16:16:33 -0700 Subject: [PATCH 02/34] Allow Maniphest tasks to be filtered by Project Summary: Major things taking place here: - A new table for storing <task, project> relationships. - Moved all task query logic into a dedicated class. - Added a "projects" filter to the UI. I was originally going to try to drive this off the main search index but the perf benefits of a custom schema make an overwhelming argument in favor of doing it this way. Test Plan: Filtered tasks by author and owner and zero, one, and more than one project. Exercised all the group/sort options. Ran the index script over my 100k task corpus. Edited task-project membership and verified the index updated. Reviewed By: cadamo Reviewers: gc3, jungejason, cadamo, tuomaspelkonen, aran CC: aran, cadamo, epriestley Differential Revision: 556 --- CHANGELOG | 5 + resources/sql/patches/051.projectfilter.sql | 6 + scripts/search/reindex_maniphest.php | 32 ++ src/__phutil_library_map__.php | 3 + .../tasklist/ManiphestTaskListController.php | 222 +++++------ .../controller/tasklist/__init__.php | 5 +- .../maniphest/query/ManiphestTaskQuery.php | 351 ++++++++++++++++++ src/applications/maniphest/query/__init__.php | 18 + .../maniphest/storage/task/ManiphestTask.php | 19 +- .../maniphest/storage/task/__init__.php | 1 + .../taskproject/ManiphestTaskProject.php | 67 ++++ .../storage/taskproject/__init__.php | 14 + 12 files changed, 610 insertions(+), 133 deletions(-) create mode 100644 resources/sql/patches/051.projectfilter.sql create mode 100755 scripts/search/reindex_maniphest.php create mode 100644 src/applications/maniphest/query/ManiphestTaskQuery.php create mode 100644 src/applications/maniphest/query/__init__.php create mode 100644 src/applications/maniphest/storage/taskproject/ManiphestTaskProject.php create mode 100644 src/applications/maniphest/storage/taskproject/__init__.php diff --git a/CHANGELOG b/CHANGELOG index 09b5bdd0a9..171beb5130 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,11 @@ This is not a complete list of changes, just of API or workflow changes that may break existing installs. Newer changes are listed at the top. If you pull new changes and things stop working, check here first! +June 29 2011 - Maniphest project indexes + Old Maniphest tasks will not appear in project filter views until you run + "scripts/search/reindex_maniphest.php" to build indexes. New tasks will have + their indexes built automatically. + May 31 2011 - Javelin submodule moved The externals/javelin submodule location has moved. If you have an older checkout of Phabricator, you may need to edit .git/config to point at diff --git a/resources/sql/patches/051.projectfilter.sql b/resources/sql/patches/051.projectfilter.sql new file mode 100644 index 0000000000..42554d01a7 --- /dev/null +++ b/resources/sql/patches/051.projectfilter.sql @@ -0,0 +1,6 @@ +CREATE TABLE phabricator_maniphest.maniphest_taskproject ( + taskPHID varchar(64) BINARY NOT NULL, + projectPHID varchar(64) BINARY NOT NULL, + PRIMARY KEY (projectPHID, taskPHID), + UNIQUE KEY (taskPHID, projectPHID) +); diff --git a/scripts/search/reindex_maniphest.php b/scripts/search/reindex_maniphest.php new file mode 100755 index 0000000000..62550d56d2 --- /dev/null +++ b/scripts/search/reindex_maniphest.php @@ -0,0 +1,32 @@ +#!/usr/bin/env php +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +$root = dirname(dirname(dirname(__FILE__))); +require_once $root.'/scripts/__init_script__.php'; +require_once $root.'/scripts/__init_env__.php'; + +ini_set('memory_limit', -1); +$tasks = id(new ManiphestTask())->loadAll(); +echo "Updating relationships for ".count($tasks)." tasks"; +foreach ($tasks as $task) { + ManiphestTaskProject::updateTaskProjects($task); + echo '.'; +} +echo "\nDone.\n"; + diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 271fcc39f5..d050187ca1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -272,6 +272,8 @@ phutil_register_library_map(array( 'ManiphestTaskListView' => 'applications/maniphest/view/tasklist', 'ManiphestTaskOwner' => 'applications/maniphest/constants/owner', 'ManiphestTaskPriority' => 'applications/maniphest/constants/priority', + 'ManiphestTaskProject' => 'applications/maniphest/storage/taskproject', + 'ManiphestTaskQuery' => 'applications/maniphest/query', 'ManiphestTaskStatus' => 'applications/maniphest/constants/status', 'ManiphestTaskSummaryView' => 'applications/maniphest/view/tasksummary', 'ManiphestTransaction' => 'applications/maniphest/storage/transaction', @@ -765,6 +767,7 @@ phutil_register_library_map(array( 'ManiphestTaskEditController' => 'ManiphestController', 'ManiphestTaskListController' => 'ManiphestController', 'ManiphestTaskListView' => 'AphrontView', + 'ManiphestTaskProject' => 'ManiphestDAO', 'ManiphestTaskSummaryView' => 'AphrontView', 'ManiphestTransaction' => 'ManiphestDAO', 'ManiphestTransactionDetailView' => 'AphrontView', diff --git a/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php b/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php index 72250e2d1a..0c0dee7d1f 100644 --- a/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php +++ b/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php @@ -33,23 +33,30 @@ class ManiphestTaskListController extends ManiphestController { $uri = $request->getRequestURI(); if ($request->isFormPost()) { - $phid_arr = $request->getArr('view_user'); - $view_target = head($phid_arr); - return id(new AphrontRedirectResponse()) - ->setURI($request->getRequestURI()->alter('phid', $view_target)); - } + // Redirect to GET so URIs can be copy/pasted. + $user_phids = $request->getArr('set_users'); + $proj_phids = $request->getArr('set_projects'); + $user_phids = implode(',', $user_phids); + $proj_phids = implode(',', $proj_phids); + $user_phids = nonempty($user_phids, null); + $proj_phids = nonempty($proj_phids, null); + + $uri = $request->getRequestURI() + ->alter('users', $user_phids) + ->alter('projects', $proj_phids); + + return id(new AphrontRedirectResponse())->setURI($uri); + } $views = array( 'User Tasks', 'action' => 'Assigned', 'created' => 'Created', 'triage' => 'Need Triage', -// 'touched' => 'Touched', '<hr />', 'All Tasks', 'alltriage' => 'Need Triage', - 'unassigned' => 'Unassigned', 'all' => 'All Tasks', ); @@ -77,7 +84,7 @@ class ManiphestTaskListController extends ManiphestController { phutil_render_tag( 'a', array( - 'href' => $uri, + 'href' => $uri->alter('page', null), 'class' => ($this->view == $view) ? 'aphront-side-nav-selected' : null, @@ -90,13 +97,26 @@ class ManiphestTaskListController extends ManiphestController { list($grouping, $group_links) = $this->renderGroupLinks(); list($order, $order_links) = $this->renderOrderLinks(); - $view_phid = nonempty($request->getStr('phid'), $user->getPHID()); + $user_phids = $request->getStr('users'); + if (strlen($user_phids)) { + $user_phids = explode(',', $user_phids); + } else { + $user_phids = array($user->getPHID()); + } + + $project_phids = $request->getStr('projects'); + if (strlen($project_phids)) { + $project_phids = explode(',', $project_phids); + } else { + $project_phids = array(); + } $page = $request->getInt('page'); $page_size = self::DEFAULT_PAGE_SIZE; list($tasks, $handles, $total_count) = $this->loadTasks( - $view_phid, + $user_phids, + $project_phids, array( 'status' => $status_map, 'group' => $grouping, @@ -105,24 +125,34 @@ class ManiphestTaskListController extends ManiphestController { 'limit' => $page_size, )); - $form = id(new AphrontFormView()) - ->setUser($user); + ->setUser($user) + ->setAction($request->getRequestURI()); if (isset($has_filter[$this->view])) { + $tokens = array(); + foreach ($user_phids as $phid) { + $tokens[$phid] = $handles[$phid]->getFullName(); + } $form->appendChild( id(new AphrontFormTokenizerControl()) - ->setLimit(1) ->setDatasource('/typeahead/common/searchowner/') - ->setName('view_user') - ->setLabel('View User') - ->setCaption('Use "upforgrabs" to find unassigned tasks.') - ->setValue( - array( - $view_phid => $handles[$view_phid]->getFullName(), - ))); + ->setName('set_users') + ->setLabel('Users') + ->setValue($tokens)); } + $tokens = array(); + foreach ($project_phids as $phid) { + $tokens[$phid] = $handles[$phid]->getFullName(); + } + $form->appendChild( + id(new AphrontFormTokenizerControl()) + ->setDatasource('/typeahead/common/projects/') + ->setName('set_projects') + ->setLabel('Projects') + ->setValue($tokens)); + $form ->appendChild( id(new AphrontFormToggleButtonsControl()) @@ -137,6 +167,10 @@ class ManiphestTaskListController extends ManiphestController { ->setLabel('Order') ->setValue($order_links)); + $form->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue('Filter Tasks')); + $filter = new AphrontListFilterView(); $filter->addButton( phutil_render_tag( @@ -209,141 +243,71 @@ class ManiphestTaskListController extends ManiphestController { )); } - private function loadTasks($view_phid, array $dict) { - $phids = array($view_phid); + private function loadTasks( + array $user_phids, + array $project_phids, + array $dict) { - $include_upforgrabs = false; - foreach ($phids as $key => $phid) { - if ($phid == ManiphestTaskOwner::OWNER_UP_FOR_GRABS) { - unset($phids[$key]); - $include_upforgrabs = true; - } - } - - $task = new ManiphestTask(); - - $argv = array(); + $query = new ManiphestTaskQuery(); + $query->withProjects($project_phids); $status = $dict['status']; if (!empty($status['open']) && !empty($status['closed'])) { - $status_clause = '1 = 1'; + $query->withStatus(ManiphestTaskQuery::STATUS_ANY); } else if (!empty($status['open'])) { - $status_clause = 'status = %d'; - $argv[] = 0; + $query->withStatus(ManiphestTaskQuery::STATUS_OPEN); } else { - $status_clause = 'status > %d'; - $argv[] = 0; + $query->withStatus(ManiphestTaskQuery::STATUS_CLOSED); } - $extra_clause = '1 = 1'; switch ($this->view) { case 'action': - $parts = array(); - if ($phids) { - $parts[] = 'ownerPHID in (%Ls)'; - $argv[] = $phids; - } - if ($include_upforgrabs) { - $parts[] = 'ownerPHID IS NULL'; - } - $extra_clause = '('.implode(' OR ', $parts).')'; + $query->withOwners($user_phids); break; case 'created': - $parts = array(); - if ($phids) { - $parts[] = 'authorPHID in (%Ls)'; - $argv[] = $phids; - } - if ($include_upforgrabs) { - // This should be impossible since every task is supposed to have a - // valid author, but we might as well run the query. - $parts[] = 'authorPHID IS NULL'; - } - $extra_clause = '('.implode(' OR ', $parts).')'; + $query->withAuthors($user_phids); break; case 'triage': - $parts = array(); - if ($phids) { - $parts[] = 'ownerPHID in (%Ls)'; - $argv[] = $phids; - } - if ($include_upforgrabs) { - $parts[] = 'ownerPHID IS NULL'; - } - $extra_clause = '('.implode(' OR ', $parts).') AND priority = %d'; - $argv[] = ManiphestTaskPriority::PRIORITY_TRIAGE; + $query->withOwners($user_phids); + $query->withPriority(ManiphestTaskPriority::PRIORITY_TRIAGE); break; case 'alltriage': - $extra_clause = 'priority = %d'; - $argv[] = ManiphestTaskPriority::PRIORITY_TRIAGE; - break; - case 'unassigned': - $extra_clause = 'ownerPHID is NULL'; + $query->withPriority(ManiphestTaskPriority::PRIORITY_TRIAGE); break; case 'all': break; } - $order = array(); - switch ($dict['group']) { - case 'priority': - $order[] = 'priority'; - break; - case 'owner': - $order[] = 'ownerOrdering'; - break; - case 'status': - $order[] = 'status'; - break; - } + $order_map = array( + 'priority' => ManiphestTaskQuery::ORDER_PRIORITY, + 'created' => ManiphestTaskQuery::ORDER_CREATED, + ); + $query->setOrderBy( + idx( + $order_map, + $dict['order'], + ManiphestTaskQuery::ORDER_MODIFIED)); - switch ($dict['order']) { - case 'priority': - $order[] = 'priority'; - $order[] = 'dateModified'; - break; - case 'created': - $order[] = 'id'; - break; - default: - $order[] = 'dateModified'; - break; - } + $group_map = array( + 'priority' => ManiphestTaskQuery::GROUP_PRIORITY, + 'owner' => ManiphestTaskQuery::GROUP_OWNER, + 'status' => ManiphestTaskQuery::GROUP_STATUS, + ); + $query->setGroupBy( + idx( + $group_map, + $dict['group'], + ManiphestTaskQuery::GROUP_NONE)); - $order = array_unique($order); + $query->setCalculateRows(true); + $query->setLimit($dict['limit']); + $query->setOffset($dict['offset']); - foreach ($order as $k => $column) { - switch ($column) { - case 'ownerOrdering': - $order[$k] = "{$column} ASC"; - break; - default: - $order[$k] = "{$column} DESC"; - break; - } - } - - $order = implode(', ', $order); - - $offset = (int)idx($dict, 'offset', 0); - $limit = (int)idx($dict, 'limit', self::DEFAULT_PAGE_SIZE); - - $sql = "SELECT SQL_CALC_FOUND_ROWS * FROM %T WHERE ". - "({$status_clause}) AND ({$extra_clause}) ORDER BY {$order} ". - "LIMIT {$offset}, {$limit}"; - - array_unshift($argv, $task->getTableName()); - - $conn = $task->establishConnection('r'); - $data = vqueryfx_all($conn, $sql, $argv); - - $total_row_count = queryfx_one($conn, 'SELECT FOUND_ROWS() N'); - $total_row_count = $total_row_count['N']; - - $data = $task->loadAllFromArray($data); + $data = $query->execute(); + $total_row_count = $query->getRowCount(); $handle_phids = mpull($data, 'getOwnerPHID'); - $handle_phids[] = $view_phid; + $handle_phids = array_merge($handle_phids, $project_phids, $user_phids); $handles = id(new PhabricatorObjectHandleData($handle_phids)) ->loadHandles(); diff --git a/src/applications/maniphest/controller/tasklist/__init__.php b/src/applications/maniphest/controller/tasklist/__init__.php index 16e6296085..28c7ec5a4c 100644 --- a/src/applications/maniphest/controller/tasklist/__init__.php +++ b/src/applications/maniphest/controller/tasklist/__init__.php @@ -7,17 +7,16 @@ phutil_require_module('phabricator', 'aphront/response/redirect'); -phutil_require_module('phabricator', 'applications/maniphest/constants/owner'); phutil_require_module('phabricator', 'applications/maniphest/constants/priority'); phutil_require_module('phabricator', 'applications/maniphest/constants/status'); phutil_require_module('phabricator', 'applications/maniphest/controller/base'); -phutil_require_module('phabricator', 'applications/maniphest/storage/task'); +phutil_require_module('phabricator', 'applications/maniphest/query'); phutil_require_module('phabricator', 'applications/maniphest/view/tasklist'); phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); -phutil_require_module('phabricator', 'storage/queryfx'); phutil_require_module('phabricator', 'view/control/pager'); phutil_require_module('phabricator', 'view/form/base'); +phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/form/control/togglebuttons'); phutil_require_module('phabricator', 'view/form/control/tokenizer'); phutil_require_module('phabricator', 'view/layout/listfilter'); diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php new file mode 100644 index 0000000000..d7f874f1eb --- /dev/null +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -0,0 +1,351 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Query tasks by specific criteria. This class uses the higher-performance + * but less-general Maniphest indexes to satisfy queries. + */ +final class ManiphestTaskQuery { + + private $authorPHIDs = array(); + private $ownerPHIDs = array(); + private $includeUnowned = null; + private $projectPHIDs = array(); + + private $status = 'status-any'; + const STATUS_ANY = 'status-any'; + const STATUS_OPEN = 'status-open'; + const STATUS_CLOSED = 'status-closed'; + + private $priority = null; + + private $groupBy = 'group-none'; + const GROUP_NONE = 'group-none'; + const GROUP_PRIORITY = 'group-priority'; + const GROUP_OWNER = 'group-owner'; + const GROUP_STATUS = 'group-status'; + + private $orderBy = 'order-modified'; + const ORDER_PRIORITY = 'order-priority'; + const ORDER_CREATED = 'order-created'; + const ORDER_MODIFIED = 'order-modified'; + + private $limit = null; + const DEFAULT_PAGE_SIZE = 1000; + + private $offset = 0; + private $calculateRows = false; + + private $rowCount = null; + + + public function withAuthors(array $authors) { + $this->authorPHIDs = $authors; + return $this; + } + + public function withOwners(array $owners) { + $this->includeUnowned = false; + foreach ($owners as $k => $phid) { + if ($phid == ManiphestTaskOwner::OWNER_UP_FOR_GRABS) { + $this->includeUnowned = true; + unset($owners[$k]); + break; + } + } + $this->ownerPHIDs = $owners; + return $this; + } + + public function withProjects(array $projects) { + $this->projectPHIDs = $projects; + return $this; + } + + public function withStatus($status) { + $this->status = $status; + return $this; + } + + public function withPriority($priority) { + $this->priority = $priority; + return $this; + } + + public function setGroupBy($group) { + $this->groupBy = $group; + return $this; + } + + public function setOrderBy($order) { + $this->orderBy = $order; + return $this; + } + + public function setLimit($limit) { + $this->limit = $limit; + return $this; + } + + public function setOffset($offset) { + $this->offset = $offset; + return $this; + } + + public function setCalculateRows($calculate_rows) { + $this->calculateRows = $calculate_rows; + return $this; + } + + public function getRowCount() { + if ($this->rowCount === null) { + throw new Exception( + "You must execute a query with setCalculateRows() before you can ". + "retrieve a row count."); + } + return $this->rowCount; + } + + public function execute() { + + $task_dao = new ManiphestTask(); + $conn = $task_dao->establishConnection('r'); + + if ($this->calculateRows) { + $calc = 'SQL_CALC_FOUND_ROWS'; + } else { + $calc = ''; + } + + $where = array(); + $where[] = $this->buildStatusWhereClause($conn); + $where[] = $this->buildPriorityWhereClause($conn); + $where[] = $this->buildAuthorWhereClause($conn); + $where[] = $this->buildOwnerWhereClause($conn); + $where[] = $this->buildProjectWhereClause($conn); + + $where = array_filter($where); + if ($where) { + $where = 'WHERE ('.implode(') AND (', $where).')'; + } else { + $where = ''; + } + + $join = array(); + $join[] = $this->buildProjectJoinClause($conn); + + $join = array_filter($join); + if ($join) { + $join = implode(' ', $join); + } else { + $join = ''; + } + + $having = ''; + $count = ''; + $group = ''; + if (count($this->projectPHIDs) > 1) { + + // If we're searching for more than one project: + // - We'll get multiple rows for tasks when they join the project table + // multiple times. We use GROUP BY to make them distinct again. + // - We want to treat the query as an intersection query, not a union + // query. We sum the project count and require it be the same as the + // number of projects we're searching for. + + $group = 'GROUP BY task.id'; + $count = ', COUNT(1) projectCount'; + $having = qsprintf( + $conn, + 'HAVING projectCount = %d', + count($this->projectPHIDs)); + } + + $order = $this->buildOrderClause($conn); + + $offset = (int)nonempty($this->offset, 0); + $limit = (int)nonempty($this->limit, self::DEFAULT_PAGE_SIZE); + + $data = queryfx_all( + $conn, + 'SELECT %Q * %Q FROM %T task %Q %Q %Q %Q %Q LIMIT %d, %d', + $calc, + $count, + $task_dao->getTableName(), + $join, + $where, + $group, + $having, + $order, + $offset, + $limit); + + if ($this->calculateRows) { + $count = queryfx_one( + $conn, + 'SELECT FOUND_ROWS() N'); + $this->rowCount = $count['N']; + } else { + $this->rowCount = null; + } + + return $task_dao->loadAllFromArray($data); + } + + private function buildStatusWhereClause($conn) { + switch ($this->status) { + case self::STATUS_ANY: + return null; + case self::STATUS_OPEN: + return 'status = 0'; + case self::STATUS_CLOSED: + return 'status > 0'; + default: + throw new Exception("Unknown status query '{$this->status}'!"); + } + } + + private function buildPriorityWhereClause($conn) { + if ($this->priority === null) { + return null; + } + + return qsprintf( + $conn, + 'priority = %d', + $this->priority); + } + + private function buildAuthorWhereClause($conn) { + if (!$this->authorPHIDs) { + return null; + } + + return qsprintf( + $conn, + 'authorPHID in (%Ls)', + $this->authorPHIDs); + } + + private function buildOwnerWhereClause($conn) { + if (!$this->ownerPHIDs) { + if ($this->includeUnowned === null) { + return null; + } else if ($this->includeUnowned) { + return qsprintf( + $conn, + 'ownerPHID IS NULL'); + } else { + return qsprintf( + $conn, + 'ownerPHID IS NOT NULL'); + } + } + + if ($this->includeUnowned) { + return qsprintf( + $conn, + 'ownerPHID IN (%Ls) OR ownerPHID IS NULL', + $this->ownerPHIDs); + } else { + return qsprintf( + $conn, + 'ownerPHID IN (%Ls)', + $this->ownerPHIDs); + } + } + + private function buildProjectWhereClause($conn) { + if (!$this->projectPHIDs) { + return null; + } + + return qsprintf( + $conn, + 'project.projectPHID IN (%Ls)', + $this->projectPHIDs); + } + + private function buildProjectJoinClause($conn) { + if (!$this->projectPHIDs) { + return null; + } + + $project_dao = new ManiphestTaskProject(); + return qsprintf( + $conn, + 'JOIN %T project ON project.taskPHID = task.phid', + $project_dao->getTableName()); + } + + private function buildOrderClause($conn) { + $order = array(); + + switch ($this->groupBy) { + case self::GROUP_NONE: + break; + case self::GROUP_PRIORITY: + $order[] = 'priority'; + break; + case self::GROUP_OWNER: + $order[] = 'ownerOrdering'; + break; + case self::GROUP_STATUS: + $order[] = 'status'; + break; + default: + throw new Exception("Unknown group query '{$this->groupBy}'!"); + } + + switch ($this->orderBy) { + case self::ORDER_PRIORITY: + $order[] = 'priority'; + $order[] = 'dateModified'; + break; + case self::ORDER_CREATED: + $order[] = 'id'; + break; + case self::ORDER_MODIFIED: + $order[] = 'dateModified'; + break; + default: + throw new Exception("Unknown order query '{$this->orderBy}'!"); + } + + $order = array_unique($order); + + if (empty($order)) { + return null; + } + + foreach ($order as $k => $column) { + switch ($column) { + case 'ownerOrdering': + $order[$k] = "task.{$column} ASC"; + break; + default: + $order[$k] = "task.{$column} DESC"; + break; + } + } + + return 'ORDER BY '.implode(', ', $order); + } + + +} diff --git a/src/applications/maniphest/query/__init__.php b/src/applications/maniphest/query/__init__.php new file mode 100644 index 0000000000..19b4f9fc8c --- /dev/null +++ b/src/applications/maniphest/query/__init__.php @@ -0,0 +1,18 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/maniphest/constants/owner'); +phutil_require_module('phabricator', 'applications/maniphest/storage/task'); +phutil_require_module('phabricator', 'applications/maniphest/storage/taskproject'); +phutil_require_module('phabricator', 'storage/qsprintf'); +phutil_require_module('phabricator', 'storage/queryfx'); + +phutil_require_module('phutil', 'utils'); + + +phutil_require_source('ManiphestTaskQuery.php'); diff --git a/src/applications/maniphest/storage/task/ManiphestTask.php b/src/applications/maniphest/storage/task/ManiphestTask.php index 717e4e0692..5bf378b49c 100644 --- a/src/applications/maniphest/storage/task/ManiphestTask.php +++ b/src/applications/maniphest/storage/task/ManiphestTask.php @@ -33,6 +33,7 @@ class ManiphestTask extends ManiphestDAO { protected $attached = array(); protected $projectPHIDs = array(); + private $projectsNeedUpdate; protected $ownerOrdering; @@ -60,11 +61,27 @@ class ManiphestTask extends ManiphestDAO { return nonempty($this->ccPHIDs, array()); } + public function setProjectPHIDs(array $phids) { + $this->projectPHIDs = $phids; + $this->projectsNeedUpdate = true; + return $this; + } + public function save() { if (!$this->mailKey) { $this->mailKey = sha1(Filesystem::readRandomBytes(20)); } - return parent::save(); + + $result = parent::save(); + + if ($this->projectsNeedUpdate) { + // If we've changed the project PHIDs for this task, update the link + // table. + ManiphestTaskProject::updateTaskProjects($this); + $this->projectsNeedUpdate = false; + } + + return $result; } } diff --git a/src/applications/maniphest/storage/task/__init__.php b/src/applications/maniphest/storage/task/__init__.php index 22a5346f8c..14ba412113 100644 --- a/src/applications/maniphest/storage/task/__init__.php +++ b/src/applications/maniphest/storage/task/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/maniphest/storage/base'); +phutil_require_module('phabricator', 'applications/maniphest/storage/taskproject'); phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); diff --git a/src/applications/maniphest/storage/taskproject/ManiphestTaskProject.php b/src/applications/maniphest/storage/taskproject/ManiphestTaskProject.php new file mode 100644 index 0000000000..687694934e --- /dev/null +++ b/src/applications/maniphest/storage/taskproject/ManiphestTaskProject.php @@ -0,0 +1,67 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This is a DAO for the Task -> Project table, which denormalizes the + * relationship between tasks and projects into a link table so it can be + * efficiently queried. This table is not authoritative; the projectPHIDs field + * of ManiphestTask is. The rows in this table are regenerated when transactions + * are applied to tasks which affected their associated projects. + * + * @group maniphest + */ +final class ManiphestTaskProject extends ManiphestDAO { + + protected $taskPHID; + protected $projectPHID; + + public function getConfiguration() { + return array( + self::CONFIG_IDS => self::IDS_MANUAL, + self::CONFIG_TIMESTAMPS => false, + ); + } + + public static function updateTaskProjects(ManiphestTask $task) { + $dao = new ManiphestTaskProject(); + $conn = $dao->establishConnection('w'); + + $sql = array(); + foreach ($task->getProjectPHIDs() as $project_phid) { + $sql[] = qsprintf( + $conn, + '(%s, %s)', + $task->getPHID(), + $project_phid); + } + + queryfx( + $conn, + 'DELETE FROM %T WHERE taskPHID = %s', + $dao->getTableName(), + $task->getPHID()); + if ($sql) { + queryfx( + $conn, + 'INSERT INTO %T (taskPHID, projectPHID) VALUES %Q', + $dao->getTableName(), + implode(', ', $sql)); + } + } + +} diff --git a/src/applications/maniphest/storage/taskproject/__init__.php b/src/applications/maniphest/storage/taskproject/__init__.php new file mode 100644 index 0000000000..8a3c5b26af --- /dev/null +++ b/src/applications/maniphest/storage/taskproject/__init__.php @@ -0,0 +1,14 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/maniphest/storage/base'); +phutil_require_module('phabricator', 'storage/qsprintf'); +phutil_require_module('phabricator', 'storage/queryfx'); + + +phutil_require_source('ManiphestTaskProject.php'); From a46ae11030fbd815cd95f279392c2d6074ddad06 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Thu, 30 Jun 2011 12:14:11 -0700 Subject: [PATCH 03/34] Restore heavy arrow to signify linebreak. --- .../parser/changeset/DifferentialChangesetParser.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php index fb6c5befa1..772a00cadf 100644 --- a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php @@ -700,7 +700,7 @@ class DifferentialChangesetParser { foreach ($vector as $ii => $char) { $result[] = $char; if (isset($break_here[$ii])) { - $result[] = "<span class=\"over-the-line\">!</span><br />"; + $result[] = "<span class=\"over-the-line\">\xE2\xAC\x85</span><br />"; } } From f6c3a902b52f07fa52205658b52c8c44eab51b69 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Thu, 30 Jun 2011 12:10:28 -0700 Subject: [PATCH 04/34] Install "mysql-server" in the RHEL-derivatives script Summary: We don't currently install this component, but should. Test Plan: iiam Reviewed By: codeblock Reviewers: codeblock CC: aran, codeblock Differential Revision: 560 --- scripts/install/install_rhel-derivs.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/install/install_rhel-derivs.sh b/scripts/install/install_rhel-derivs.sh index 34bcf3b132..712f7a0a27 100755 --- a/scripts/install/install_rhel-derivs.sh +++ b/scripts/install/install_rhel-derivs.sh @@ -56,7 +56,7 @@ then echo "ERROR: You must be able to sudo to run this script, or run it as root."; exit 1 fi - + fi if [[ $RHEL_MAJOR_VER == 5 ]] @@ -65,10 +65,10 @@ then # (it tries to pull in php 5.1 stuff) ... echo "Adding EPEL repo, for git." $SUDO rpm -Uvh http://download.fedora.redhat.com/pub/epel/5/i386/epel-release-5-4.noarch.rpm - YUMCOMMAND="$SUDO yum install httpd git php53 php53-cli php53-mysql php53-process php53-devel php53-gd gcc wget make pcre-devel" + YUMCOMMAND="$SUDO yum install httpd git php53 php53-cli php53-mysql php53-process php53-devel php53-gd gcc wget make pcre-devel mysql-server" else # RHEL 6+ defaults with php 5.3 - YUMCOMMAND="$SUDO yum install httpd git php php-cli php-mysql php-process php-devel php-gd php-pecl-apc php-pecl-json" + YUMCOMMAND="$SUDO yum install httpd git php php-cli php-mysql php-process php-devel php-gd php-pecl-apc php-pecl-json mysql-server" fi echo "Dropping to yum to install dependencies..." From 553ef6f58764ae036abeb3e2bc81f448fc68f2ed Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Thu, 30 Jun 2011 12:32:30 -0700 Subject: [PATCH 05/34] When sending Differential comment emails, include added CCs and reviewers explicitly Summary: You currently have to click through to figure out who got added. Test Plan: - Made a comment which added CCs. - Made a comment which added reviewers. - Made a comment which added nothing. - Made a comment which added CCs and reviewers. Reviewed By: tomo Reviewers: tomo, jungejason, tuomaspelkonen, aran CC: aran, tomo Differential Revision: 562 --- .../mail/comment/DifferentialCommentMail.php | 34 +++++++++++++++++++ .../differential/mail/comment/__init__.php | 1 + 2 files changed, 35 insertions(+) diff --git a/src/applications/differential/mail/comment/DifferentialCommentMail.php b/src/applications/differential/mail/comment/DifferentialCommentMail.php index 060264b166..fb5695dee0 100644 --- a/src/applications/differential/mail/comment/DifferentialCommentMail.php +++ b/src/applications/differential/mail/comment/DifferentialCommentMail.php @@ -71,6 +71,32 @@ class DifferentialCommentMail extends DifferentialMail { $body = array(); $body[] = "{$actor} has {$verb} the revision \"{$name}\"."; + + // If the commented added reviewers or CCs, list them explicitly. + $meta = $comment->getMetadata(); + $m_reviewers = idx( + $meta, + DifferentialComment::METADATA_ADDED_REVIEWERS, + array()); + $m_cc = idx( + $meta, + DifferentialComment::METADATA_ADDED_CCS, + array()); + $load = array_merge($m_reviewers, $m_cc); + if ($load) { + $handles = id(new PhabricatorObjectHandleData($load))->loadHandles(); + if ($m_reviewers) { + $body[] = 'Added Reviewers: '.$this->renderHandleList( + $handles, + $m_reviewers); + } + if ($m_cc) { + $body[] = 'Added CCs: '.$this->renderHandleList( + $handles, + $m_cc); + } + } + $body[] = null; $content = $comment->getContent(); @@ -134,4 +160,12 @@ class DifferentialCommentMail extends DifferentialMail { return implode("\n", $body); } + + private function renderHandleList(array $handles, array $phids) { + $names = array(); + foreach ($phids as $phid) { + $names[] = $handles[$phid]->getName(); + } + return implode(', ', $names); + } } diff --git a/src/applications/differential/mail/comment/__init__.php b/src/applications/differential/mail/comment/__init__.php index f6f1281398..51b6e24688 100644 --- a/src/applications/differential/mail/comment/__init__.php +++ b/src/applications/differential/mail/comment/__init__.php @@ -9,6 +9,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/action'); phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/mail/base'); +phutil_require_module('phabricator', 'applications/differential/storage/comment'); phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'infrastructure/env'); From 2b8116d7ae0a6954e66db9f2117d85d26ebf7701 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Thu, 30 Jun 2011 13:43:41 -0700 Subject: [PATCH 06/34] Fix error cues on "Edit Revision" interface Summary: The field hints on this interface don't behave correctly. Particularly, when you add yourself as a reviewer you aren't pointed at the issue. Test Plan: Edited a revision and tried to save invalid changes, including self-reviewership. Reviewers: moskov, jungejason, tuomaspelkonen, aran CC: Differential Revision: 565 --- .../revisionedit/DifferentialRevisionEditController.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php b/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php index 3e1ce67956..7308e07a27 100644 --- a/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php +++ b/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php @@ -57,6 +57,7 @@ class DifferentialRevisionEditController extends DifferentialController { $e_title = true; $e_testplan = true; + $e_reviewers = null; $errors = array(); $revision->loadRelationships(); @@ -71,17 +72,22 @@ class DifferentialRevisionEditController extends DifferentialController { if (!strlen(trim($revision->getTitle()))) { $errors[] = 'You must provide a title.'; $e_title = 'Required'; + } else { + $e_title = null; } if (!strlen(trim($revision->getTestPlan()))) { $errors[] = 'You must provide a test plan.'; $e_testplan = 'Required'; + } else { + $e_testplan = null; } $user_phid = $request->getUser()->getPHID(); if (in_array($user_phid, $request->getArr('reviewers'))) { $errors[] = 'You may not review your own revision.'; + $e_reviewers = 'Invalid'; } if (!$errors) { @@ -172,6 +178,7 @@ class DifferentialRevisionEditController extends DifferentialController { ->setLabel('Reviewers') ->setName('reviewers') ->setDatasource('/typeahead/common/users/') + ->setError($e_reviewers) ->setValue($reviewer_map)) ->appendChild( id(new AphrontFormTokenizerControl()) From d92f303e0cdbd78f62fe7f0adfa494068ccd08b1 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Thu, 30 Jun 2011 13:58:28 -0700 Subject: [PATCH 07/34] Use authoritative user identity for revision author Summary: We added a proper way to get the authoritative user a while ago, this method just never got switched to use it. Test Plan: Created a revision locally, was recognized as the revision author. Reviewed By: jungejason Reviewers: gc3, jungejason, tuomaspelkonen, aran CC: aran, jungejason Differential Revision: 566 --- .../ConduitAPI_differential_createrevision_Method.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/applications/conduit/method/differential/createrevision/ConduitAPI_differential_createrevision_Method.php b/src/applications/conduit/method/differential/createrevision/ConduitAPI_differential_createrevision_Method.php index f9f246448b..29a9cf4f5c 100644 --- a/src/applications/conduit/method/differential/createrevision/ConduitAPI_differential_createrevision_Method.php +++ b/src/applications/conduit/method/differential/createrevision/ConduitAPI_differential_createrevision_Method.php @@ -26,7 +26,6 @@ class ConduitAPI_differential_createrevision_Method extends ConduitAPIMethod { return array( 'diffid' => 'required diffid', 'fields' => 'required dict', - 'user' => 'required guid', ); } @@ -51,7 +50,7 @@ class ConduitAPI_differential_createrevision_Method extends ConduitAPIMethod { $revision = DifferentialRevisionEditor::newRevisionFromConduitWithDiff( $fields, $diff, - $request->getValue('user')); // TODO: Should be authoritative + $request->getUser()->getPHID()); return array( 'revisionid' => $revision->getID(), From af107cff6539aaefa4117bcef58064b3d65480a9 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Thu, 30 Jun 2011 18:10:26 -0700 Subject: [PATCH 08/34] Explicitly list reviewers on reviewrequest email Summary: This used to be in the subject but there was a bunch of churn and now it's nowhere. Test Plan: Created, updated, and added CCs to a diff. Reviewed By: moskov Reviewers: moskov, avitaloliver, jungejason, tuomaspelkonen, aran CC: aran, moskov Differential Revision: 567 --- .../differential/mail/base/DifferentialMail.php | 8 ++++++++ .../mail/ccwelcome/DifferentialCCWelcomeMail.php | 1 + .../differential/mail/comment/DifferentialCommentMail.php | 8 -------- .../differential/mail/newdiff/DifferentialNewDiffMail.php | 1 + .../mail/reviewrequest/DifferentialReviewRequestMail.php | 6 ++++++ .../differential/mail/reviewrequest/__init__.php | 3 +++ 6 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/applications/differential/mail/base/DifferentialMail.php b/src/applications/differential/mail/base/DifferentialMail.php index 8ea77d456e..3b0b67c917 100644 --- a/src/applications/differential/mail/base/DifferentialMail.php +++ b/src/applications/differential/mail/base/DifferentialMail.php @@ -319,4 +319,12 @@ EOTEXT; return $this->heraldTranscriptURI; } + protected function renderHandleList(array $handles, array $phids) { + $names = array(); + foreach ($phids as $phid) { + $names[] = $handles[$phid]->getName(); + } + return implode(', ', $names); + } + } diff --git a/src/applications/differential/mail/ccwelcome/DifferentialCCWelcomeMail.php b/src/applications/differential/mail/ccwelcome/DifferentialCCWelcomeMail.php index 635f10c803..8d4b9ba00e 100644 --- a/src/applications/differential/mail/ccwelcome/DifferentialCCWelcomeMail.php +++ b/src/applications/differential/mail/ccwelcome/DifferentialCCWelcomeMail.php @@ -30,6 +30,7 @@ class DifferentialCCWelcomeMail extends DifferentialReviewRequestMail { $body = array(); $body[] = "{$actor} added you to the CC list for the revision \"{$name}\"."; + $body[] = $this->renderReviewersLine(); $body[] = null; $body[] = $this->renderReviewRequestBody(); diff --git a/src/applications/differential/mail/comment/DifferentialCommentMail.php b/src/applications/differential/mail/comment/DifferentialCommentMail.php index fb5695dee0..417f1e147b 100644 --- a/src/applications/differential/mail/comment/DifferentialCommentMail.php +++ b/src/applications/differential/mail/comment/DifferentialCommentMail.php @@ -160,12 +160,4 @@ class DifferentialCommentMail extends DifferentialMail { return implode("\n", $body); } - - private function renderHandleList(array $handles, array $phids) { - $names = array(); - foreach ($phids as $phid) { - $names[] = $handles[$phid]->getName(); - } - return implode(', ', $names); - } } diff --git a/src/applications/differential/mail/newdiff/DifferentialNewDiffMail.php b/src/applications/differential/mail/newdiff/DifferentialNewDiffMail.php index 9b18f6fd67..6082be50fb 100644 --- a/src/applications/differential/mail/newdiff/DifferentialNewDiffMail.php +++ b/src/applications/differential/mail/newdiff/DifferentialNewDiffMail.php @@ -58,6 +58,7 @@ class DifferentialNewDiffMail extends DifferentialReviewRequestMail { } else { $body[] = "{$actor} updated the revision \"{$name}\"."; } + $body[] = $this->renderReviewersLine(); $body[] = null; $body[] = $this->renderReviewRequestBody(); diff --git a/src/applications/differential/mail/reviewrequest/DifferentialReviewRequestMail.php b/src/applications/differential/mail/reviewrequest/DifferentialReviewRequestMail.php index 0fc013363c..95ad604a5b 100644 --- a/src/applications/differential/mail/reviewrequest/DifferentialReviewRequestMail.php +++ b/src/applications/differential/mail/reviewrequest/DifferentialReviewRequestMail.php @@ -39,6 +39,12 @@ abstract class DifferentialReviewRequestMail extends DifferentialMail { $this->setChangesets($changesets); } + protected function renderReviewersLine() { + $reviewers = $this->getRevision()->getReviewers(); + $handles = id(new PhabricatorObjectHandleData($reviewers))->loadHandles(); + return 'Reviewers: '.$this->renderHandleList($handles, $reviewers); + } + protected function renderReviewRequestBody() { $revision = $this->getRevision(); diff --git a/src/applications/differential/mail/reviewrequest/__init__.php b/src/applications/differential/mail/reviewrequest/__init__.php index 2411be0156..2189b1648a 100644 --- a/src/applications/differential/mail/reviewrequest/__init__.php +++ b/src/applications/differential/mail/reviewrequest/__init__.php @@ -7,6 +7,9 @@ phutil_require_module('phabricator', 'applications/differential/mail/base'); +phutil_require_module('phabricator', 'applications/phid/handle/data'); + +phutil_require_module('phutil', 'utils'); phutil_require_source('DifferentialReviewRequestMail.php'); From cd47271cf5e7a5fe3b94d0ed994e5f7b90e76d46 Mon Sep 17 00:00:00 2001 From: Jason Ge <jungejason@fb.com> Date: Fri, 1 Jul 2011 19:02:27 -0700 Subject: [PATCH 09/34] Handle the case when a repository was deleted Summary: when a repository was deleted, PhabricatorObjectHandleData::loadHandles() is throwing exception because it assumes that the repository for the commit exists. Test Plan: try an revision whose repo was deleted and it renders. Reviewed By: epriestley Reviewers: epriestley, andrewjcg CC: aran, epriestley Differential Revision: 576 --- .../data/PhabricatorObjectHandleData.php | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php index 9e1f4c9c65..06b99cccb3 100644 --- a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php @@ -210,14 +210,22 @@ class PhabricatorObjectHandleData { $repository = $repositories[$repository_ids[$phid]]; $commit_identifier = $commit->getCommitIdentifier(); - $vcs = $repository->getVersionControlSystem(); - if ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_GIT) { - $short_identifier = substr($commit_identifier, 0, 16); + // In case where the repository for the commit was deleted, + // we don't have have info about the repository anymore. + if ($repository) { + $vcs = $repository->getVersionControlSystem(); + if ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_GIT) { + $short_identifier = substr($commit_identifier, 0, 16); + } else { + $short_identifier = $commit_identifier; + } + + $handle->setName('r'.$callsign.$short_identifier); } else { - $short_identifier = $commit_identifier; + + $handle->setName('Commit '.'r'.$callsign.$commit_identifier); } - $handle->setName('r'.$callsign.$short_identifier); $handle->setURI('/r'.$callsign.$commit_identifier); $handle->setFullName('r'.$callsign.$commit_identifier); $handle->setTimestamp($commit->getEpoch()); From 39adae9aa85edf86f8b4f7cdad9291d484fe5499 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Sun, 3 Jul 2011 06:07:50 -0700 Subject: [PATCH 10/34] Prevent Phabot from spinning out of control Summary: When the remote closes the connection, phabot goes into a busy loop because of PHP's "nothing should ever be an error" semantics. Instead, detect connection termination. Test Plan: Disabled the "PONG" response in the protocol handler and let freenode disconnect phabot. It spun out of control before, now it detects the issue and exits to await automatic restart. Reviewed By: jungejason Reviewers: codeblock, jungejason, aran, tuomaspelkonen CC: aran, jungejason Differential Revision: 582 --- src/infrastructure/daemon/irc/bot/PhabricatorIRCBot.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/infrastructure/daemon/irc/bot/PhabricatorIRCBot.php b/src/infrastructure/daemon/irc/bot/PhabricatorIRCBot.php index 6cc6ccaaf4..fcd4a16900 100644 --- a/src/infrastructure/daemon/irc/bot/PhabricatorIRCBot.php +++ b/src/infrastructure/daemon/irc/bot/PhabricatorIRCBot.php @@ -130,6 +130,14 @@ final class PhabricatorIRCBot extends PhabricatorDaemon { } if ($read) { + // Test for connection termination; in PHP, fread() off a nonblocking, + // closed socket is empty string. + if (feof($this->socket)) { + // This indicates the connection was terminated on the other side, + // just exit via exception and let the overseer restart us after a + // delay so we can reconnect. + throw new Exception("Remote host closed connection."); + } do { $data = fread($this->socket, 4096); if ($data === false) { From a15f07cc33d43b2df2b6805003a1fb6f702984e1 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Thu, 30 Jun 2011 13:01:35 -0700 Subject: [PATCH 11/34] Allow Phabricator to be configured to use a public Reply-To address Summary: We already support this (and Facebook uses it) but it is difficult to configure and you have to write a bunch of code. Instead, provide a simple flag. See the documentation changes for details, but when this flag is enabled we send one email with a reply-to like "D2+public+23hf91fh19fh@phabricator.example.com". Anyone can reply to this, and we figure out who they are based on their "From" address instead of a unique hash. This is less secure, but a reasonable tradeoff in many cases. This also has the advantage over a naive implementation of at least doing object hash validation. @jungejason: I don't think this affects Facebook's implementation but this is an area where we've had problems in the past, so watch out for it when you deploy. Also note that you must set "metamta.public-replies" to true since Maniphest now looks for that key specifically before going into public reply mode; it no longer just tests for a public reply address being generateable (since it can always generate one now). Test Plan: Swapped my local install in and out of public reply mode and commented on objects. Got expected email behavior. Replied to public and private email addresses. Attacked public addresses by using them when the install was configured to disallow them and by altering the hash and the from address. All this stuff was rejected. Reviewed By: jungejason Reviewers: moskov, jungejason, tuomaspelkonen, aran CC: aran, epriestley, moskov, jungejason Differential Revision: 563 --- conf/default.conf.php | 11 +++++ .../replyhandler/DifferentialReplyHandler.php | 4 ++ .../replyhandler/ManiphestReplyHandler.php | 4 ++ .../base/PhabricatorMailReplyHandler.php | 28 ++++++++++- .../metamta/replyhandler/base/__init__.php | 1 + .../PhabricatorMetaMTAReceivedMail.php | 48 ++++++++++++++++--- .../configuring_inbound_email.diviner | 13 ++++- 7 files changed, 98 insertions(+), 11 deletions(-) diff --git a/conf/default.conf.php b/conf/default.conf.php index a90b9738a8..fcd09e4898 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -190,6 +190,17 @@ return array( // Prefix prepended to mail sent by Differential. 'metamta.differential.subject-prefix' => '[Differential]', + // By default, Phabricator generates unique reply-to addresses and sends a + // separate email to each recipient when you enable reply handling. This is + // more secure than using "From" to establish user identity, but can mean + // users may receive multiple emails when they are on mailing lists. Instead, + // you can use a single, non-unique reply to address and authenticate users + // based on the "From" address by setting this to 'true'. This trades away + // a little bit of security for convenience, but it's reasonable in many + // installs. Object interactions are still protected using hashes in the + // single public email address, so objects can not be replied to blindly. + 'metamta.public-replies' => false, + // -- Auth ------------------------------------------------------------------ // diff --git a/src/applications/differential/replyhandler/DifferentialReplyHandler.php b/src/applications/differential/replyhandler/DifferentialReplyHandler.php index ae7be54e1b..516ebd453c 100644 --- a/src/applications/differential/replyhandler/DifferentialReplyHandler.php +++ b/src/applications/differential/replyhandler/DifferentialReplyHandler.php @@ -31,6 +31,10 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler { return $this->getDefaultPrivateReplyHandlerEmailAddress($handle, 'D'); } + public function getPublicReplyHandlerEmailAddress() { + return $this->getDefaultPublicReplyHandlerEmailAddress('D'); + } + public function getReplyHandlerDomain() { return PhabricatorEnv::getEnvConfig( 'metamta.differential.reply-handler-domain'); diff --git a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php b/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php index b86f5685f7..c3f8cfe1c9 100644 --- a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php +++ b/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php @@ -29,6 +29,10 @@ class ManiphestReplyHandler extends PhabricatorMailReplyHandler { return $this->getDefaultPrivateReplyHandlerEmailAddress($handle, 'T'); } + public function getPublicReplyHandlerEmailAddress() { + return $this->getDefaultPublicReplyHandlerEmailAddress('T'); + } + public function getReplyHandlerDomain() { return PhabricatorEnv::getEnvConfig( 'metamta.maniphest.reply-handler-domain'); diff --git a/src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php index f8ab23fa3a..8b2d932bb4 100644 --- a/src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php +++ b/src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php @@ -48,12 +48,20 @@ abstract class PhabricatorMailReplyHandler { abstract public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail); public function supportsPrivateReplies() { - return (bool)$this->getReplyHandlerDomain(); + return (bool)$this->getReplyHandlerDomain() && + !$this->supportsPublicReplies(); + } + + public function supportsPublicReplies() { + if (!PhabricatorEnv::getEnvConfig('metamta.public-replies')) { + return false; + } + return (bool)$this->getPublicReplyHandlerEmailAddress(); } final public function supportsReplies() { return $this->supportsPrivateReplies() || - (bool)$this->getPublicReplyHandlerEmailAddress(); + $this->supportsPublicReplies(); } public function getPublicReplyHandlerEmailAddress() { @@ -145,6 +153,22 @@ abstract class PhabricatorMailReplyHandler { return implode(', ', $list); } + protected function getDefaultPublicReplyHandlerEmailAddress($prefix) { + + $receiver = $this->getMailReceiver(); + $receiver_id = $receiver->getID(); + $domain = $this->getReplyHandlerDomain(); + + // We compute a hash using the object's own PHID to prevent an attacker + // from blindly interacting with objects that they haven't ever received + // mail about by just sending to D1@, D2@, etc... + $hash = PhabricatorMetaMTAReceivedMail::computeMailHash( + $receiver->getMailKey(), + $receiver->getPHID()); + + return "{$prefix}{$receiver_id}+public+{$hash}@{$domain}"; + } + protected function getDefaultPrivateReplyHandlerEmailAddress( PhabricatorObjectHandle $handle, $prefix) { diff --git a/src/applications/metamta/replyhandler/base/__init__.php b/src/applications/metamta/replyhandler/base/__init__.php index 5ff3e91560..145c5f6fd3 100644 --- a/src/applications/metamta/replyhandler/base/__init__.php +++ b/src/applications/metamta/replyhandler/base/__init__.php @@ -8,6 +8,7 @@ phutil_require_module('phabricator', 'applications/metamta/storage/receivedmail'); phutil_require_module('phabricator', 'applications/phid/constants'); +phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php index cdd978feef..1ddc36c6e5 100644 --- a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php @@ -58,7 +58,7 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { // "some display name" <D1+xyz+asdf@example.com> $matches = null; $ok = preg_match( - '/(?:^|<)((?:D|T)\d+)\+(\d+)\+([a-f0-9]{16})@/U', + '/(?:^|<)((?:D|T)\d+)\+([\w]+)\+([a-f0-9]{16})@/U', $to, $matches); @@ -70,9 +70,35 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { $user_id = $matches[2]; $hash = $matches[3]; - $user = id(new PhabricatorUser())->load($user_id); - if (!$user) { - return $this->setMessage("Invalid user '{$user_id}'")->save(); + if ($user_id == 'public') { + if (!PhabricatorEnv::getEnvConfig('metamta.public-replies')) { + return $this->setMessage("Public replies not enabled.")->save(); + } + + // Strip the email address out of the 'from' if necessary, since it might + // have some formatting like '"Abraham Lincoln" <alincoln@logcab.in>'. + $from = idx($this->headers, 'from'); + $matches = null; + $ok = preg_match('/<(.*)>/', $from, $matches); + if ($ok) { + $from = $matches[1]; + } + + $user = id(new PhabricatorUser())->loadOneWhere( + 'email = %s', + $from); + if (!$user) { + return $this->setMessage("Invalid public user '{$from}'.")->save(); + } + + $use_user_hash = false; + } else { + $user = id(new PhabricatorUser())->load($user_id); + if (!$user) { + return $this->setMessage("Invalid private user '{$user_id}'.")->save(); + } + + $use_user_hash = true; } if ($user->getIsDisabled()) { @@ -88,9 +114,17 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { $this->setRelatedPHID($receiver->getPHID()); - $expect_hash = self::computeMailHash( - $receiver->getMailKey(), - $user->getPHID()); + if ($use_user_hash) { + // This is a private reply-to address, check that the user hash is + // correct. + $check_phid = $user->getPHID(); + } else { + // This is a public reply-to address, check that the object hash is + // correct. + $check_phid = $receiver->getPHID(); + } + + $expect_hash = self::computeMailHash($receiver->getMailKey(), $check_phid); if ($expect_hash != $hash) { return $this->setMessage("Invalid mail hash!")->save(); } diff --git a/src/docs/configuration/configuring_inbound_email.diviner b/src/docs/configuration/configuring_inbound_email.diviner index ebc524b738..981b6a9512 100644 --- a/src/docs/configuration/configuring_inbound_email.diviner +++ b/src/docs/configuration/configuring_inbound_email.diviner @@ -65,6 +65,15 @@ over). If you leak a bunch of reply-to addresses by accident, you can change ##phabricator.mail-key## in your configuration to invalidate all the old hashes. +You can also set ##metamta.public-replies##, which will change how Phabricator +delivers email. Instead of sending each recipient a unique mail with a personal +reply-to address, it will send a single email to everyone with a public reply-to +address. This decreases security because anyone who can spoof a "From" address +can act as another user, but increases convenience if you use mailing lists and, +practically, is a reasonable setting for many installs. The reply-to address +will still contain a hash unique to the object it represents, so users who have +not received an email about an object can not blindly interact with it. + NOTE: Phabricator does not currently attempt to verify "From" addresses because this is technically complex, seems unreasonably difficult in the general case, and no installs have had a need for it yet. If you have a specific case where a @@ -190,11 +199,11 @@ content of the email to Phabricator: import logging, subprocess from lamson.routing import route, stateless from lamson import view - + PHABRICATOR_ROOT = "/path/to/phabricator" PHABRICATOR_ENV = "custom/myconf" LOGGING_ENABLED = True - + @route("(address)@(host)", address=".+") @stateless def START(message, address=None, host=None): From fc28ab06c222fff7280c2ff0e7bc808ed294d406 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Sun, 3 Jul 2011 22:43:37 -0700 Subject: [PATCH 12/34] Bring Javelin external up to HEAD for autocomplete. --- externals/javelin | 2 +- src/__celerity_resource_map__.php | 176 ++++++++++++++++-------------- 2 files changed, 96 insertions(+), 82 deletions(-) diff --git a/externals/javelin b/externals/javelin index c727216edf..081b5658d0 160000 --- a/externals/javelin +++ b/externals/javelin @@ -1 +1 @@ -Subproject commit c727216edfb2527d14e1047656986f0cb8f694f9 +Subproject commit 081b5658d0f176fbbcd251c570cb62e7eab81af0 diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index c36196692c..b9fe71f51e 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -285,17 +285,18 @@ celerity_register_resource_map(array( ), 0 => array( - 'uri' => '/res/39de799e/rsrc/js/javelin/docs/Base.js', + 'uri' => '/res/1da00bfe/rsrc/js/javelin/lib/__tests__/URI.js', 'type' => 'js', 'requires' => array( - 0 => 'javelin-install', + 0 => 'javelin-uri', + 1 => 'javelin-php-serializer', ), - 'disk' => '/rsrc/js/javelin/docs/Base.js', + 'disk' => '/rsrc/js/javelin/lib/__tests__/URI.js', ), 'javelin-behavior' => array( - 'uri' => '/res/3c772c64/rsrc/js/javelin/lib/behavior.js', + 'uri' => '/res/b28adfa1/rsrc/js/javelin/lib/behavior.js', 'type' => 'js', 'requires' => array( @@ -665,7 +666,7 @@ celerity_register_resource_map(array( ), 'javelin-dom' => array( - 'uri' => '/res/43e9e2de/rsrc/js/javelin/lib/DOM.js', + 'uri' => '/res/34e7f2b5/rsrc/js/javelin/lib/DOM.js', 'type' => 'js', 'requires' => array( @@ -689,7 +690,7 @@ celerity_register_resource_map(array( ), 'javelin-install' => array( - 'uri' => '/res/f4d0e147/rsrc/js/javelin/core/install.js', + 'uri' => '/res/b6692f42/rsrc/js/javelin/core/install.js', 'type' => 'js', 'requires' => array( @@ -700,7 +701,7 @@ celerity_register_resource_map(array( ), 'javelin-json' => array( - 'uri' => '/res/1c4e3f6a/rsrc/js/javelin/lib/JSON.js', + 'uri' => '/res/f23dbfbd/rsrc/js/javelin/lib/JSON.js', 'type' => 'js', 'requires' => array( @@ -711,7 +712,7 @@ celerity_register_resource_map(array( ), 'javelin-magical-init' => array( - 'uri' => '/res/92e7f37e/rsrc/js/javelin/core/init.js', + 'uri' => '/res/6a069362/rsrc/js/javelin/core/init.js', 'type' => 'js', 'requires' => array( @@ -732,7 +733,7 @@ celerity_register_resource_map(array( ), 'javelin-request' => array( - 'uri' => '/res/1ed0d596/rsrc/js/javelin/lib/Request.js', + 'uri' => '/res/786bfe6f/rsrc/js/javelin/lib/Request.js', 'type' => 'js', 'requires' => array( @@ -740,12 +741,13 @@ celerity_register_resource_map(array( 1 => 'javelin-stratcom', 2 => 'javelin-util', 3 => 'javelin-behavior', + 4 => 'javelin-json', ), 'disk' => '/rsrc/js/javelin/lib/Request.js', ), 'javelin-stratcom' => array( - 'uri' => '/res/9e7eb62b/rsrc/js/javelin/core/Stratcom.js', + 'uri' => '/res/d6cf4631/rsrc/js/javelin/core/Stratcom.js', 'type' => 'js', 'requires' => array( @@ -758,7 +760,7 @@ celerity_register_resource_map(array( ), 'javelin-tokenizer' => array( - 'uri' => '/res/83787676/rsrc/js/javelin/lib/control/tokenizer/Tokenizer.js', + 'uri' => '/res/c2dbba74/rsrc/js/javelin/lib/control/tokenizer/Tokenizer.js', 'type' => 'js', 'requires' => array( @@ -771,7 +773,7 @@ celerity_register_resource_map(array( ), 'javelin-typeahead' => array( - 'uri' => '/res/ae18ee16/rsrc/js/javelin/lib/control/typeahead/Typeahead.js', + 'uri' => '/res/354d32f1/rsrc/js/javelin/lib/control/typeahead/Typeahead.js', 'type' => 'js', 'requires' => array( @@ -782,6 +784,18 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/javelin/lib/control/typeahead/Typeahead.js', ), + 'javelin-typeahead-composite-source' => + array( + 'uri' => '/res/9873d21c/rsrc/js/javelin/lib/control/typeahead/source/TypeaheadCompositeSource.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-install', + 1 => 'javelin-typeahead-source', + 2 => 'javelin-util', + ), + 'disk' => '/rsrc/js/javelin/lib/control/typeahead/source/TypeaheadCompositeSource.js', + ), 'javelin-typeahead-normalizer' => array( 'uri' => '/res/a5d60e3c/rsrc/js/javelin/lib/control/typeahead/normalizer/TypeaheadNormalizer.js', @@ -794,7 +808,7 @@ celerity_register_resource_map(array( ), 'javelin-typeahead-ondemand-source' => array( - 'uri' => '/res/0015bbf5/rsrc/js/javelin/lib/control/typeahead/source/TypeaheadOnDemandSource.js', + 'uri' => '/res/b2e527e4/rsrc/js/javelin/lib/control/typeahead/source/TypeaheadOnDemandSource.js', 'type' => 'js', 'requires' => array( @@ -808,7 +822,7 @@ celerity_register_resource_map(array( ), 'javelin-typeahead-preloaded-source' => array( - 'uri' => '/res/863a173c/rsrc/js/javelin/lib/control/typeahead/source/TypeaheadPreloadedSource.js', + 'uri' => '/res/f3c1e7c7/rsrc/js/javelin/lib/control/typeahead/source/TypeaheadPreloadedSource.js', 'type' => 'js', 'requires' => array( @@ -822,7 +836,7 @@ celerity_register_resource_map(array( ), 'javelin-typeahead-source' => array( - 'uri' => '/res/58518dde/rsrc/js/javelin/lib/control/typeahead/source/TypeaheadSource.js', + 'uri' => '/res/8a3aa262/rsrc/js/javelin/lib/control/typeahead/source/TypeaheadSource.js', 'type' => 'js', 'requires' => array( @@ -835,7 +849,7 @@ celerity_register_resource_map(array( ), 'javelin-uri' => array( - 'uri' => '/res/70c9d32b/rsrc/js/javelin/lib/URI.js', + 'uri' => '/res/1c0ead43/rsrc/js/javelin/lib/URI.js', 'type' => 'js', 'requires' => array( @@ -847,7 +861,7 @@ celerity_register_resource_map(array( ), 'javelin-util' => array( - 'uri' => '/res/be43fdba/rsrc/js/javelin/core/util.js', + 'uri' => '/res/725cc90d/rsrc/js/javelin/core/util.js', 'type' => 'js', 'requires' => array( @@ -856,7 +870,7 @@ celerity_register_resource_map(array( ), 'javelin-vector' => array( - 'uri' => '/res/cd4721c4/rsrc/js/javelin/lib/Vector.js', + 'uri' => '/res/155b931a/rsrc/js/javelin/lib/Vector.js', 'type' => 'js', 'requires' => array( @@ -867,7 +881,7 @@ celerity_register_resource_map(array( ), 'javelin-workflow' => array( - 'uri' => '/res/28a267b3/rsrc/js/javelin/lib/Workflow.js', + 'uri' => '/res/91a04640/rsrc/js/javelin/lib/Workflow.js', 'type' => 'js', 'requires' => array( @@ -1066,7 +1080,7 @@ celerity_register_resource_map(array( ), 'phabricator-search-results-css' => array( - 'uri' => '/res/9a9eeaf2/rsrc/css/application/search/search-results.css', + 'uri' => '/res/f8a86e27/rsrc/css/application/search/search-results.css', 'type' => 'css', 'requires' => array( @@ -1125,7 +1139,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/03ef179e/diffusion.pkg.css', 'type' => 'css', ), - '2892314d' => + '25f94e94' => array ( 'name' => 'typeahead.pkg.js', 'symbols' => @@ -1138,7 +1152,26 @@ celerity_register_resource_map(array( 5 => 'javelin-tokenizer', 6 => 'javelin-behavior-aphront-basic-tokenizer', ), - 'uri' => '/res/pkg/2892314d/typeahead.pkg.js', + 'uri' => '/res/pkg/25f94e94/typeahead.pkg.js', + 'type' => 'js', + ), + '6c5acfa3' => + array ( + 'name' => 'javelin.pkg.js', + 'symbols' => + array ( + 0 => 'javelin-util', + 1 => 'javelin-install', + 2 => 'javelin-event', + 3 => 'javelin-stratcom', + 4 => 'javelin-behavior', + 5 => 'javelin-request', + 6 => 'javelin-vector', + 7 => 'javelin-dom', + 8 => 'javelin-json', + 9 => 'javelin-uri', + ), + 'uri' => '/res/pkg/6c5acfa3/javelin.pkg.js', 'type' => 'js', ), 55967526 => @@ -1182,40 +1215,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/8e9024dc/core.pkg.css', 'type' => 'css', ), - 'da416e1c' => - array ( - 'name' => 'differential.pkg.js', - 'symbols' => - array ( - 0 => 'javelin-behavior-differential-feedback-preview', - 1 => 'javelin-behavior-differential-edit-inline-comments', - 2 => 'javelin-behavior-differential-populate', - 3 => 'javelin-behavior-differential-show-more', - 4 => 'javelin-behavior-differential-diff-radios', - ), - 'uri' => '/res/pkg/da416e1c/differential.pkg.js', - 'type' => 'js', - ), - 'db95a6d0' => - array ( - 'name' => 'javelin.pkg.js', - 'symbols' => - array ( - 0 => 'javelin-util', - 1 => 'javelin-install', - 2 => 'javelin-event', - 3 => 'javelin-stratcom', - 4 => 'javelin-behavior', - 5 => 'javelin-request', - 6 => 'javelin-vector', - 7 => 'javelin-dom', - 8 => 'javelin-json', - 9 => 'javelin-uri', - ), - 'uri' => '/res/pkg/db95a6d0/javelin.pkg.js', - 'type' => 'js', - ), - 'f1d27e2a' => + '973a2445' => array ( 'name' => 'workflow.pkg.js', 'symbols' => @@ -1228,7 +1228,21 @@ celerity_register_resource_map(array( 5 => 'phabricator-keyboard-shortcut', 6 => 'javelin-behavior-phabricator-keyboard-shortcuts', ), - 'uri' => '/res/pkg/f1d27e2a/workflow.pkg.js', + 'uri' => '/res/pkg/973a2445/workflow.pkg.js', + 'type' => 'js', + ), + 'da416e1c' => + array ( + 'name' => 'differential.pkg.js', + 'symbols' => + array ( + 0 => 'javelin-behavior-differential-feedback-preview', + 1 => 'javelin-behavior-differential-edit-inline-comments', + 2 => 'javelin-behavior-differential-populate', + 3 => 'javelin-behavior-differential-show-more', + 4 => 'javelin-behavior-differential-diff-radios', + ), + 'uri' => '/res/pkg/da416e1c/differential.pkg.js', 'type' => 'js', ), ), @@ -1252,38 +1266,38 @@ celerity_register_resource_map(array( 'differential-revision-history-css' => '55967526', 'differential-table-of-contents-css' => '55967526', 'diffusion-commit-view-css' => '03ef179e', - 'javelin-behavior' => 'db95a6d0', - 'javelin-behavior-aphront-basic-tokenizer' => '2892314d', - 'javelin-behavior-aphront-form-disable-on-submit' => 'f1d27e2a', + 'javelin-behavior' => '6c5acfa3', + 'javelin-behavior-aphront-basic-tokenizer' => '25f94e94', + 'javelin-behavior-aphront-form-disable-on-submit' => '973a2445', 'javelin-behavior-differential-diff-radios' => 'da416e1c', 'javelin-behavior-differential-edit-inline-comments' => 'da416e1c', 'javelin-behavior-differential-feedback-preview' => 'da416e1c', 'javelin-behavior-differential-populate' => 'da416e1c', 'javelin-behavior-differential-show-more' => 'da416e1c', - 'javelin-behavior-phabricator-keyboard-shortcuts' => 'f1d27e2a', - 'javelin-behavior-workflow' => 'f1d27e2a', - 'javelin-dom' => 'db95a6d0', - 'javelin-event' => 'db95a6d0', - 'javelin-install' => 'db95a6d0', - 'javelin-json' => 'db95a6d0', - 'javelin-mask' => 'f1d27e2a', - 'javelin-request' => 'db95a6d0', - 'javelin-stratcom' => 'db95a6d0', - 'javelin-tokenizer' => '2892314d', - 'javelin-typeahead' => '2892314d', - 'javelin-typeahead-normalizer' => '2892314d', - 'javelin-typeahead-ondemand-source' => '2892314d', - 'javelin-typeahead-preloaded-source' => '2892314d', - 'javelin-typeahead-source' => '2892314d', - 'javelin-uri' => 'db95a6d0', - 'javelin-util' => 'db95a6d0', - 'javelin-vector' => 'db95a6d0', - 'javelin-workflow' => 'f1d27e2a', + 'javelin-behavior-phabricator-keyboard-shortcuts' => '973a2445', + 'javelin-behavior-workflow' => '973a2445', + 'javelin-dom' => '6c5acfa3', + 'javelin-event' => '6c5acfa3', + 'javelin-install' => '6c5acfa3', + 'javelin-json' => '6c5acfa3', + 'javelin-mask' => '973a2445', + 'javelin-request' => '6c5acfa3', + 'javelin-stratcom' => '6c5acfa3', + 'javelin-tokenizer' => '25f94e94', + 'javelin-typeahead' => '25f94e94', + 'javelin-typeahead-normalizer' => '25f94e94', + 'javelin-typeahead-ondemand-source' => '25f94e94', + 'javelin-typeahead-preloaded-source' => '25f94e94', + 'javelin-typeahead-source' => '25f94e94', + 'javelin-uri' => '6c5acfa3', + 'javelin-util' => '6c5acfa3', + 'javelin-vector' => '6c5acfa3', + 'javelin-workflow' => '973a2445', 'phabricator-core-buttons-css' => '8e9024dc', 'phabricator-core-css' => '8e9024dc', 'phabricator-directory-css' => '8e9024dc', - 'phabricator-keyboard-shortcut' => 'f1d27e2a', - 'phabricator-keyboard-shortcut-manager' => 'f1d27e2a', + 'phabricator-keyboard-shortcut' => '973a2445', + 'phabricator-keyboard-shortcut-manager' => '973a2445', 'phabricator-remarkup-css' => '8e9024dc', 'phabricator-standard-page-view' => '8e9024dc', 'syntax-highlighting-css' => '8e9024dc', From 9454060c297069552eff1fa84a44319e469d927d Mon Sep 17 00:00:00 2001 From: Ricky Elrod <ricky@elrod.me> Date: Sun, 3 Jul 2011 18:29:58 -0400 Subject: [PATCH 13/34] Add a syntax highlight dropdown, if pygments is enabled. Summary: - Add a default list of supported languages to default.conf.php and make the initial/default value customizable. - Store a '' in the database to infer the language from the filename/title. Test Plan: Tested in my sandbox with pygments enabled and disabled and various combinations of filename/extension/dropdown selection. Reviewers: epriestley CC: Differential Revision: 587 --- conf/default.conf.php | 45 +++++++++++++++---- resources/sql/patches/052.pastelanguage.sql | 2 + .../data/commitmessage/__init__.php | 1 - .../PhabricatorPasteCreateController.php | 32 +++++++++++++ .../paste/controller/create/__init__.php | 2 + .../view/PhabricatorPasteViewController.php | 2 +- .../paste/storage/paste/PhabricatorPaste.php | 1 + 7 files changed, 75 insertions(+), 10 deletions(-) create mode 100644 resources/sql/patches/052.pastelanguage.sql diff --git a/conf/default.conf.php b/conf/default.conf.php index fcd09e4898..da5d7a0c2b 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -305,14 +305,6 @@ return array( // behalf, silencing the warning. 'phabricator.timezone' => null, - - // Phabricator can highlight PHP by default, but if you want syntax - // highlighting for other languages you should install the python package - // 'Pygments', make sure the 'pygmentize' script is available in the - // $PATH of the webserver, and then enable this. - 'pygments.enabled' => false, - - // -- Files ----------------------------------------------------------------- // // Lists which uploaded file types may be viewed in the browser. If a file @@ -416,4 +408,41 @@ return array( // settings are the defaults.) 'celerity.force-disk-reads' => false, + // -- Pygments ------------------------------------------------------------ // + // Phabricator can highlight PHP by default, but if you want syntax + // highlighting for other languages you should install the python package + // 'Pygments', make sure the 'pygmentize' script is available in the + // $PATH of the webserver, and then enable this. + 'pygments.enabled' => false, + + // In places that we display a dropdown to syntax-highlight code, + // this is where that list is defined. + // Syntax is 'lexer-name' => 'Display Name', + 'pygments.dropdown-choices' => array( + 'apacheconf' => 'Apache Configuration', + 'bash' => 'Bash Scripting', + 'brainfuck' => 'Brainf*ck', + 'c' => 'C', + 'cpp' => 'C++', + 'css' => 'CSS', + 'diff' => 'Diff', + 'django' => 'Django Templating', + 'erb' => 'Embedded Ruby/ERB', + 'erlang' => 'Erlang', + 'html' => 'HTML', + 'infer' => 'Infer from title (extension)', + 'java' => 'Java', + 'js' => 'Javascript', + 'mysql' => 'MySQL', + 'perl' => 'Perl', + 'php' => 'PHP', + 'text' => 'Plain Text', + 'python' => 'Python', + // TODO: 'remarkup' => 'Remarkup', + 'ruby' => 'Ruby', + 'xml' => 'XML', + ), + + 'pygments.dropdown-default' => 'infer', + ); diff --git a/resources/sql/patches/052.pastelanguage.sql b/resources/sql/patches/052.pastelanguage.sql new file mode 100644 index 0000000000..47f09e12c2 --- /dev/null +++ b/resources/sql/patches/052.pastelanguage.sql @@ -0,0 +1,2 @@ +ALTER TABLE phabricator_pastebin.pastebin_paste + ADD COLUMN language VARCHAR(64) NOT NULL; \ No newline at end of file diff --git a/src/applications/differential/data/commitmessage/__init__.php b/src/applications/differential/data/commitmessage/__init__.php index 4234c8c8aa..c6ce1f81a8 100644 --- a/src/applications/differential/data/commitmessage/__init__.php +++ b/src/applications/differential/data/commitmessage/__init__.php @@ -11,7 +11,6 @@ phutil_require_module('phabricator', 'applications/differential/storage/comment' phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'infrastructure/env'); -phutil_require_module('phutil', 'symbols'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/paste/controller/create/PhabricatorPasteCreateController.php b/src/applications/paste/controller/create/PhabricatorPasteCreateController.php index 3f2f241d79..e56b400e05 100644 --- a/src/applications/paste/controller/create/PhabricatorPasteCreateController.php +++ b/src/applications/paste/controller/create/PhabricatorPasteCreateController.php @@ -31,6 +31,16 @@ class PhabricatorPasteCreateController extends PhabricatorPasteController { if ($request->isFormPost()) { $errors = array(); $title = $request->getStr('title'); + + $language = $request->getStr('language'); + if ($language == 'infer') { + // If it's infer, store an empty string. Otherwise, store the + // language name. We do this so we can refer to 'infer' elsewhere + // in the code (such as default value) while retaining backwards + // compatibility with old posts with no language stored. + $language = ''; + } + $text = $request->getStr('text'); if (!strlen($text)) { @@ -41,6 +51,7 @@ class PhabricatorPasteCreateController extends PhabricatorPasteController { } $paste->setTitle($title); + $paste->setLanguage($language); if (!$errors) { $paste_file = PhabricatorFile::newFromFileData( @@ -76,6 +87,26 @@ class PhabricatorPasteCreateController extends PhabricatorPasteController { } $form = new AphrontFormView(); + + // If we're coming back from an error and the language was already defined, + // use that. Otherwise, ask the config for the default. + if ($paste->getLanguage()) { + $language_default = $paste->getLanguage(); + } else { + $language_default = PhabricatorEnv::getEnvConfig( + 'pygments.dropdown-default'); + } + + $available_languages = PhabricatorEnv::getEnvConfig( + 'pygments.dropdown-choices'); + asort($available_languages); + + $language_select = id(new AphrontFormSelectControl()) + ->setLabel('Language') + ->setName('language') + ->setValue($language_default) + ->setOptions($available_languages); + $form ->setUser($user) ->setAction($request->getRequestURI()->getPath()) @@ -84,6 +115,7 @@ class PhabricatorPasteCreateController extends PhabricatorPasteController { ->setLabel('Title') ->setValue($paste->getTitle()) ->setName('title')) + ->appendChild($language_select) ->appendChild( id(new AphrontFormTextAreaControl()) ->setLabel('Text') diff --git a/src/applications/paste/controller/create/__init__.php b/src/applications/paste/controller/create/__init__.php index f6151ac905..d0215f70f2 100644 --- a/src/applications/paste/controller/create/__init__.php +++ b/src/applications/paste/controller/create/__init__.php @@ -10,7 +10,9 @@ phutil_require_module('phabricator', 'aphront/response/redirect'); phutil_require_module('phabricator', 'applications/files/storage/file'); phutil_require_module('phabricator', 'applications/paste/controller/base'); phutil_require_module('phabricator', 'applications/paste/storage/paste'); +phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'view/form/base'); +phutil_require_module('phabricator', 'view/form/control/select'); phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/form/control/text'); phutil_require_module('phabricator', 'view/form/control/textarea'); diff --git a/src/applications/paste/controller/view/PhabricatorPasteViewController.php b/src/applications/paste/controller/view/PhabricatorPasteViewController.php index 850fb7842d..776eacea5f 100644 --- a/src/applications/paste/controller/view/PhabricatorPasteViewController.php +++ b/src/applications/paste/controller/view/PhabricatorPasteViewController.php @@ -96,7 +96,7 @@ class PhabricatorPasteViewController extends PhabricatorPasteController { $text_list = explode( "\n", $highlightEngine->highlightSource( - $paste->getTitle(), + nonempty($paste->getLanguage(), $paste->getTitle()), $file->loadFileData())); $rows = $this->buildDisplayRows($text_list); diff --git a/src/applications/paste/storage/paste/PhabricatorPaste.php b/src/applications/paste/storage/paste/PhabricatorPaste.php index 37c1564f83..4edeea81ea 100644 --- a/src/applications/paste/storage/paste/PhabricatorPaste.php +++ b/src/applications/paste/storage/paste/PhabricatorPaste.php @@ -22,6 +22,7 @@ class PhabricatorPaste extends PhabricatorPasteDAO { protected $title; protected $authorPHID; protected $filePHID; + protected $language; public function getConfiguration() { return array( From f2cedd81087bb3566a3bf57a397864c12e8490f1 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Mon, 4 Jul 2011 11:22:42 -0700 Subject: [PATCH 14/34] Provide basic Celerity documentation. --- .../flavortext/soon_static_resources.diviner | 5 +- src/docs/technical/celerity.diviner | 65 +++++++++++++++++++ .../celerity/api/CelerityAPI.php | 22 ++----- src/infrastructure/celerity/api/__init__.php | 1 + src/infrastructure/celerity/api/utils.php | 59 +++++++++++++++++ .../controller/CelerityResourceController.php | 7 ++ .../celerity/map/CelerityResourceMap.php | 14 ++-- src/infrastructure/celerity/map/__init__.php | 1 + src/infrastructure/celerity/map/utils.php | 29 +++++++++ .../CelerityStaticResourceResponse.php | 7 ++ 10 files changed, 188 insertions(+), 22 deletions(-) create mode 100644 src/docs/technical/celerity.diviner create mode 100644 src/infrastructure/celerity/api/utils.php create mode 100644 src/infrastructure/celerity/map/utils.php diff --git a/src/docs/flavortext/soon_static_resources.diviner b/src/docs/flavortext/soon_static_resources.diviner index 7b8139bc73..1a663fea26 100644 --- a/src/docs/flavortext/soon_static_resources.diviner +++ b/src/docs/flavortext/soon_static_resources.diviner @@ -1,10 +1,13 @@ @title Things You Should Do Soon: Static Resources @group flavortext - Over time, you'll write more JS and CSS and eventually need to put systems in place to manage it. +This is part of @{article:Things You Should Do Soon}, which describes +architectural problems in web applications which you should begin to consider +before you encounter them. + = Manage Dependencies Automatically = The naive way to add static resources to a page is to include them at the top diff --git a/src/docs/technical/celerity.diviner b/src/docs/technical/celerity.diviner new file mode 100644 index 0000000000..9c707565ce --- /dev/null +++ b/src/docs/technical/celerity.diviner @@ -0,0 +1,65 @@ +@title Celerity Technical Documentation +@group celerity + +Technical overview of the Celerity system. + += Overview = + +Celerity is a static resource (CSS and JS) management system, which handles: + + - Keeping track of which resources a page needs. + - Generating URIs for the browser to access resources. + - Managing dependencies between resources. + - Packaging resources into fewer HTTP requests for performance. + - Preprocessing resources (e.g., stripping comments and whitespace). + - Delivering resources and managing resource cache lifetimes. + - Interfacing with the client to manage resources. + +Celerity is an outgrowth of the //Haste// system at Facebook. You can find more +information about Celerity here: + + - @{article:Things You Should Do Soon: Static Resources} describes the history + and context of the system and the problems it solves. + - @{article:Adding New CSS and JS} provides a developer guide to using + Celerity. + += Class Relationships = + +Celerity's primary API is @{function:require_celerity_resource}, which marks a +resource for inclusion when a response is rendered (e.g., when the HTML page is +generated, or when the response to an Ajax request is built). For instance, if +you use a CSS class like "widget-view", you must ensure the appropriate CSS is +included by calling ##require_celerity_resource('widget-view-css')## (or +similar), at your use site. + +This function uses @{class:CelerityAPI} to access the active +@{class:CelerityStaticResourceResponse} and tells it that it needs to include +the resource later, when the response actually gets built. (This layer of +indirection provides future-proofing against certain complex situations Facebook +eventually encountered). + +When the time comes to render the response, the page renderer uses +@{class:CelerityAPI} to access the active +@{class:CelerityStaticResourceResponse} and requests that it render out +appropriate references to CSS and JS resources. It uses +@{class:CelerityResourceMap} to determine the dependencies for the requested +resources (so you only have to explicitly include what you're actually using, +and not all of its dependencies) and any packaging rules (so it may be able to +generate fewer resource requests, improving performance). It then generates +##<script />## and ##<link />## references to these resources. + +These references point at ##/res/## URIs, which are handled by +@{class:CelerityResourceController}. It responds to these requests and delivers +the relevant resources and packages, managing cache lifetimes and handling any +neessary preprocessing. It uses @{class:CelerityResourceMap} to locate resources +and read packaging rules. + +The dependency and packaging maps are generated by +##scripts/celerity_mapper.php##, which updates +##src/__celerity_resource_map__.php##. This file is automatically included and +just calls @{function:celerity_register_resource_map} with a large blob of +static data to populate @{class:CelerityResourceMap}. + +@{class:CelerityStaticResourceResponse} also manages some Javelin information, +and @{function:celerity_generate_unique_node_id} uses this metadata to provide +a better uniqueness guarantee when generating unique node IDs. diff --git a/src/infrastructure/celerity/api/CelerityAPI.php b/src/infrastructure/celerity/api/CelerityAPI.php index bf9064f44b..16f7b640ba 100644 --- a/src/infrastructure/celerity/api/CelerityAPI.php +++ b/src/infrastructure/celerity/api/CelerityAPI.php @@ -16,6 +16,12 @@ * limitations under the License. */ +/** + * Indirection layer which provisions for a terrifying future where we need to + * build multiple resource responses per page. + * + * @group celerity + */ final class CelerityAPI { private static $response; @@ -27,18 +33,4 @@ final class CelerityAPI { return self::$response; } -} - -function require_celerity_resource($symbol) { - $response = CelerityAPI::getStaticResourceResponse(); - $response->requireResource($symbol); -} - -function celerity_generate_unique_node_id() { - static $uniq = 0; - $response = CelerityAPI::getStaticResourceResponse(); - $block = $response->getMetadataBlock(); - - return 'UQ'.$block.'_'.($uniq++); -} - +} \ No newline at end of file diff --git a/src/infrastructure/celerity/api/__init__.php b/src/infrastructure/celerity/api/__init__.php index 762576517c..144af866b1 100644 --- a/src/infrastructure/celerity/api/__init__.php +++ b/src/infrastructure/celerity/api/__init__.php @@ -10,3 +10,4 @@ phutil_require_module('phabricator', 'infrastructure/celerity/response'); phutil_require_source('CelerityAPI.php'); +phutil_require_source('utils.php'); diff --git a/src/infrastructure/celerity/api/utils.php b/src/infrastructure/celerity/api/utils.php new file mode 100644 index 0000000000..ed82dbc9ce --- /dev/null +++ b/src/infrastructure/celerity/api/utils.php @@ -0,0 +1,59 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +/** + * Include a CSS or JS static resource by name. This function records a + * dependency for the current page, so when a response is generated it can be + * included. You can call this method from any context, and it is recommended + * you invoke it as close to the actual dependency as possible so that page + * dependencies are minimized. + * + * For more information, see @{article:Adding New CSS and JS}. + * + * @param string Name of the celerity module to include. This is whatever you + * annotated as "@provides" in the file. + * @return void + * + * @group celerity + */ +function require_celerity_resource($symbol) { + $response = CelerityAPI::getStaticResourceResponse(); + $response->requireResource($symbol); +} + + +/** + * Generate a node ID which is guaranteed to be unique for the current page, + * even across Ajax requests. You should use this method to generate IDs for + * nodes which require a uniqueness guarantee. + * + * @return string A string appropriate for use as an 'id' attribute on a DOM + * node. It is guaranteed to be unique for the current page, even + * if the current request is a subsequent Ajax request. + * + * @group celerity + */ +function celerity_generate_unique_node_id() { + static $uniq = 0; + $response = CelerityAPI::getStaticResourceResponse(); + $block = $response->getMetadataBlock(); + + return 'UQ'.$block.'_'.($uniq++); +} + diff --git a/src/infrastructure/celerity/controller/CelerityResourceController.php b/src/infrastructure/celerity/controller/CelerityResourceController.php index 2b09c7899e..0fe43c64ee 100644 --- a/src/infrastructure/celerity/controller/CelerityResourceController.php +++ b/src/infrastructure/celerity/controller/CelerityResourceController.php @@ -16,6 +16,13 @@ * limitations under the License. */ +/** + * Delivers CSS and JS resources to the browser. This controller handles all + * ##/res/## requests, and manages caching, package construction, and resource + * preprocessing. + * + * @group celerity + */ class CelerityResourceController extends AphrontController { private $path; diff --git a/src/infrastructure/celerity/map/CelerityResourceMap.php b/src/infrastructure/celerity/map/CelerityResourceMap.php index 2dc0be4465..59fff93f34 100644 --- a/src/infrastructure/celerity/map/CelerityResourceMap.php +++ b/src/infrastructure/celerity/map/CelerityResourceMap.php @@ -16,6 +16,14 @@ * limitations under the License. */ +/** + * Interface to the static resource map, which is a graph of available + * resources, resource dependencies, and packaging information. You generally do + * not need to invoke it directly; instead, you call higher-level Celerity APIs + * and it uses the resource map to satisfy your requests. + * + * @group celerity + */ final class CelerityResourceMap { private static $instance; @@ -124,9 +132,3 @@ final class CelerityResourceMap { } } - -function celerity_register_resource_map(array $map, array $package_map) { - $instance = CelerityResourceMap::getInstance(); - $instance->setResourceMap($map); - $instance->setPackageMap($package_map); -} diff --git a/src/infrastructure/celerity/map/__init__.php b/src/infrastructure/celerity/map/__init__.php index d34da6d1be..889b007a93 100644 --- a/src/infrastructure/celerity/map/__init__.php +++ b/src/infrastructure/celerity/map/__init__.php @@ -11,3 +11,4 @@ phutil_require_module('phutil', 'utils'); phutil_require_source('CelerityResourceMap.php'); +phutil_require_source('utils.php'); diff --git a/src/infrastructure/celerity/map/utils.php b/src/infrastructure/celerity/map/utils.php new file mode 100644 index 0000000000..93c5e05653 --- /dev/null +++ b/src/infrastructure/celerity/map/utils.php @@ -0,0 +1,29 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Registers a resource map for Celerity. This is glue code between the Celerity + * mapper script and @{class:CelerityResourceMap}. + * + * @group celerity + */ +function celerity_register_resource_map(array $map, array $package_map) { + $instance = CelerityResourceMap::getInstance(); + $instance->setResourceMap($map); + $instance->setPackageMap($package_map); +} diff --git a/src/infrastructure/celerity/response/CelerityStaticResourceResponse.php b/src/infrastructure/celerity/response/CelerityStaticResourceResponse.php index 8ef6be945c..4331df11cc 100644 --- a/src/infrastructure/celerity/response/CelerityStaticResourceResponse.php +++ b/src/infrastructure/celerity/response/CelerityStaticResourceResponse.php @@ -16,6 +16,13 @@ * limitations under the License. */ +/** + * Tracks and resolves dependencies the page declares with + * @{function:require_celerity_resource}, and then builds appropriate HTML or + * Ajax responses. + * + * @group celerity + */ final class CelerityStaticResourceResponse { private $symbols = array(); From a5e22e87e2a856df5799fe1732b4c75e052333d2 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Mon, 4 Jul 2011 12:03:36 -0700 Subject: [PATCH 15/34] Provide basic Conduit documentation. --- .divinerconfig | 1 + .../api/PhabricatorConduitAPIController.php | 3 + .../base/PhabricatorConduitController.php | 3 + .../PhabricatorConduitConsoleController.php | 3 + .../log/PhabricatorConduitLogController.php | 3 + .../PhabricatorConduitTokenController.php | 3 + .../conduit/method/base/ConduitAPIMethod.php | 3 + .../ConduitAPI_conduit_connect_Method.php | 3 + ...nduitAPI_conduit_getcertificate_Method.php | 3 + .../ping/ConduitAPI_conduit_ping_Method.php | 3 + .../ConduitAPI_daemon_launched_Method.php | 3 + .../log/ConduitAPI_daemon_log_Method.php | 3 + ...duitAPI_differential_creatediff_Method.php | 3 + ...API_differential_createrevision_Method.php | 3 + .../ConduitAPI_differential_find_Method.php | 3 + ...uitAPI_differential_getalldiffs_Method.php | 3 + ...I_differential_getcommitmessage_Method.php | 3 + ...API_differential_getcommitpaths_Method.php | 3 + ...ConduitAPI_differential_getdiff_Method.php | 3 + ...uitAPI_differential_getrevision_Method.php | 3 + ...ifferential_getrevisionfeedback_Method.php | 3 + ...tAPI_differential_markcommitted_Method.php | 3 + ...differential_parsecommitmessage_Method.php | 3 + ...PI_differential_setdiffproperty_Method.php | 3 + ...API_differential_updaterevision_Method.php | 3 + ...rential_updatetaskrevisionassoc_Method.php | 4 ++ ..._differential_updateunitresults_Method.php | 3 + ...ConduitAPI_diffusion_getcommits_Method.php | 3 + ...iffusion_getrecentcommitsbypath_Method.php | 3 + .../ConduitAPI_file_download_Method.php | 3 + .../upload/ConduitAPI_file_upload_Method.php | 3 + .../info/ConduitAPI_maniphest_info_Method.php | 3 + .../info/ConduitAPI_paste_info_Method.php | 3 + .../ConduitAPI_path_getowners_Method.php | 3 + .../user/find/ConduitAPI_user_find_Method.php | 3 + .../whoami/ConduitAPI_user_whoami_Method.php | 3 + .../protocol/exception/ConduitException.php | 3 + .../protocol/request/ConduitAPIRequest.php | 3 + .../storage/base/PhabricatorConduitDAO.php | 3 + .../PhabricatorConduitConnectionLog.php | 3 + .../PhabricatorConduitMethodCallLog.php | 3 + .../PhabricatorConduitCertificateToken.php | 3 + src/docs/technical/conduit.diviner | 57 +++++++++++++++++++ src/storage/queryfx/queryfx.php | 3 + 44 files changed, 185 insertions(+) create mode 100644 src/docs/technical/conduit.diviner diff --git a/.divinerconfig b/.divinerconfig index e20237a757..415bf4e95f 100644 --- a/.divinerconfig +++ b/.divinerconfig @@ -12,6 +12,7 @@ "diffusion" : "Diffusion (Repository Browser)", "maniphest" : "Maniphest (Task Tracking)", "herald" : "Herald (Notifications)", + "conduit" : "Conduit (Phabricator HTTP API)", "celerity" : "Celerity (CSS/JS Management)", "aphront" : "Aphront (Web Stack)", "console" : "DarkConsole (Debugging Console)", diff --git a/src/applications/conduit/controller/api/PhabricatorConduitAPIController.php b/src/applications/conduit/controller/api/PhabricatorConduitAPIController.php index 35a10d9d4b..cad7fa289c 100644 --- a/src/applications/conduit/controller/api/PhabricatorConduitAPIController.php +++ b/src/applications/conduit/controller/api/PhabricatorConduitAPIController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class PhabricatorConduitAPIController extends PhabricatorConduitController { diff --git a/src/applications/conduit/controller/base/PhabricatorConduitController.php b/src/applications/conduit/controller/base/PhabricatorConduitController.php index 2af8cdbd54..3bf54fa81f 100644 --- a/src/applications/conduit/controller/base/PhabricatorConduitController.php +++ b/src/applications/conduit/controller/base/PhabricatorConduitController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ abstract class PhabricatorConduitController extends PhabricatorController { public function buildStandardPageResponse($view, array $data) { diff --git a/src/applications/conduit/controller/console/PhabricatorConduitConsoleController.php b/src/applications/conduit/controller/console/PhabricatorConduitConsoleController.php index 4ffb6087fb..d0a2608b59 100644 --- a/src/applications/conduit/controller/console/PhabricatorConduitConsoleController.php +++ b/src/applications/conduit/controller/console/PhabricatorConduitConsoleController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class PhabricatorConduitConsoleController extends PhabricatorConduitController { diff --git a/src/applications/conduit/controller/log/PhabricatorConduitLogController.php b/src/applications/conduit/controller/log/PhabricatorConduitLogController.php index 0b9433cb11..74206aa9aa 100644 --- a/src/applications/conduit/controller/log/PhabricatorConduitLogController.php +++ b/src/applications/conduit/controller/log/PhabricatorConduitLogController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class PhabricatorConduitLogController extends PhabricatorConduitController { public function processRequest() { diff --git a/src/applications/conduit/controller/token/PhabricatorConduitTokenController.php b/src/applications/conduit/controller/token/PhabricatorConduitTokenController.php index 4fe6a55942..d340af76fd 100644 --- a/src/applications/conduit/controller/token/PhabricatorConduitTokenController.php +++ b/src/applications/conduit/controller/token/PhabricatorConduitTokenController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class PhabricatorConduitTokenController extends PhabricatorConduitController { public function processRequest() { diff --git a/src/applications/conduit/method/base/ConduitAPIMethod.php b/src/applications/conduit/method/base/ConduitAPIMethod.php index 5f38e4220f..7f04f9cb69 100644 --- a/src/applications/conduit/method/base/ConduitAPIMethod.php +++ b/src/applications/conduit/method/base/ConduitAPIMethod.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ abstract class ConduitAPIMethod { abstract public function getMethodDescription(); diff --git a/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php b/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php index b6723c7938..6637c8a2f4 100644 --- a/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php +++ b/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_conduit_connect_Method extends ConduitAPIMethod { public function shouldRequireAuthentication() { diff --git a/src/applications/conduit/method/conduit/getcertificate/ConduitAPI_conduit_getcertificate_Method.php b/src/applications/conduit/method/conduit/getcertificate/ConduitAPI_conduit_getcertificate_Method.php index 7c4101169e..f7f90d5938 100644 --- a/src/applications/conduit/method/conduit/getcertificate/ConduitAPI_conduit_getcertificate_Method.php +++ b/src/applications/conduit/method/conduit/getcertificate/ConduitAPI_conduit_getcertificate_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_conduit_getcertificate_Method extends ConduitAPIMethod { public function shouldRequireAuthentication() { diff --git a/src/applications/conduit/method/conduit/ping/ConduitAPI_conduit_ping_Method.php b/src/applications/conduit/method/conduit/ping/ConduitAPI_conduit_ping_Method.php index 137348ea21..f3c33b6963 100644 --- a/src/applications/conduit/method/conduit/ping/ConduitAPI_conduit_ping_Method.php +++ b/src/applications/conduit/method/conduit/ping/ConduitAPI_conduit_ping_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_conduit_ping_Method extends ConduitAPIMethod { public function shouldRequireAuthentication() { diff --git a/src/applications/conduit/method/daemon/launched/ConduitAPI_daemon_launched_Method.php b/src/applications/conduit/method/daemon/launched/ConduitAPI_daemon_launched_Method.php index fb27edbfd4..6c7b3e02e2 100644 --- a/src/applications/conduit/method/daemon/launched/ConduitAPI_daemon_launched_Method.php +++ b/src/applications/conduit/method/daemon/launched/ConduitAPI_daemon_launched_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_daemon_launched_Method extends ConduitAPIMethod { public function shouldRequireAuthentication() { diff --git a/src/applications/conduit/method/daemon/log/ConduitAPI_daemon_log_Method.php b/src/applications/conduit/method/daemon/log/ConduitAPI_daemon_log_Method.php index 4847e4b260..f7ba004d09 100644 --- a/src/applications/conduit/method/daemon/log/ConduitAPI_daemon_log_Method.php +++ b/src/applications/conduit/method/daemon/log/ConduitAPI_daemon_log_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_daemon_log_Method extends ConduitAPIMethod { public function shouldRequireAuthentication() { diff --git a/src/applications/conduit/method/differential/creatediff/ConduitAPI_differential_creatediff_Method.php b/src/applications/conduit/method/differential/creatediff/ConduitAPI_differential_creatediff_Method.php index 324cb40d03..52342b534c 100644 --- a/src/applications/conduit/method/differential/creatediff/ConduitAPI_differential_creatediff_Method.php +++ b/src/applications/conduit/method/differential/creatediff/ConduitAPI_differential_creatediff_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/createrevision/ConduitAPI_differential_createrevision_Method.php b/src/applications/conduit/method/differential/createrevision/ConduitAPI_differential_createrevision_Method.php index 29a9cf4f5c..dd4ff2e4f2 100644 --- a/src/applications/conduit/method/differential/createrevision/ConduitAPI_differential_createrevision_Method.php +++ b/src/applications/conduit/method/differential/createrevision/ConduitAPI_differential_createrevision_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_createrevision_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/find/ConduitAPI_differential_find_Method.php b/src/applications/conduit/method/differential/find/ConduitAPI_differential_find_Method.php index 9983150496..a6084762c4 100644 --- a/src/applications/conduit/method/differential/find/ConduitAPI_differential_find_Method.php +++ b/src/applications/conduit/method/differential/find/ConduitAPI_differential_find_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_find_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/getalldiffs/ConduitAPI_differential_getalldiffs_Method.php b/src/applications/conduit/method/differential/getalldiffs/ConduitAPI_differential_getalldiffs_Method.php index 93eb01d22b..527f0c146b 100644 --- a/src/applications/conduit/method/differential/getalldiffs/ConduitAPI_differential_getalldiffs_Method.php +++ b/src/applications/conduit/method/differential/getalldiffs/ConduitAPI_differential_getalldiffs_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_getalldiffs_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/getcommitmessage/ConduitAPI_differential_getcommitmessage_Method.php b/src/applications/conduit/method/differential/getcommitmessage/ConduitAPI_differential_getcommitmessage_Method.php index a61e5db669..b5b9b0c9c5 100644 --- a/src/applications/conduit/method/differential/getcommitmessage/ConduitAPI_differential_getcommitmessage_Method.php +++ b/src/applications/conduit/method/differential/getcommitmessage/ConduitAPI_differential_getcommitmessage_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_getcommitmessage_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/getcommitpaths/ConduitAPI_differential_getcommitpaths_Method.php b/src/applications/conduit/method/differential/getcommitpaths/ConduitAPI_differential_getcommitpaths_Method.php index 8511b8fd54..cf39f4de71 100644 --- a/src/applications/conduit/method/differential/getcommitpaths/ConduitAPI_differential_getcommitpaths_Method.php +++ b/src/applications/conduit/method/differential/getcommitpaths/ConduitAPI_differential_getcommitpaths_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_getcommitpaths_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/getdiff/ConduitAPI_differential_getdiff_Method.php b/src/applications/conduit/method/differential/getdiff/ConduitAPI_differential_getdiff_Method.php index 48428e18a2..f49ab8db04 100644 --- a/src/applications/conduit/method/differential/getdiff/ConduitAPI_differential_getdiff_Method.php +++ b/src/applications/conduit/method/differential/getdiff/ConduitAPI_differential_getdiff_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_getdiff_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/getrevision/ConduitAPI_differential_getrevision_Method.php b/src/applications/conduit/method/differential/getrevision/ConduitAPI_differential_getrevision_Method.php index be5b6a016f..a633d3e6d1 100644 --- a/src/applications/conduit/method/differential/getrevision/ConduitAPI_differential_getrevision_Method.php +++ b/src/applications/conduit/method/differential/getrevision/ConduitAPI_differential_getrevision_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_getrevision_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/getrevisionfeedback/ConduitAPI_differential_getrevisionfeedback_Method.php b/src/applications/conduit/method/differential/getrevisionfeedback/ConduitAPI_differential_getrevisionfeedback_Method.php index fcd351d309..253d1b8c90 100644 --- a/src/applications/conduit/method/differential/getrevisionfeedback/ConduitAPI_differential_getrevisionfeedback_Method.php +++ b/src/applications/conduit/method/differential/getrevisionfeedback/ConduitAPI_differential_getrevisionfeedback_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_getrevisionfeedback_Method extends ConduitAPIMethod { diff --git a/src/applications/conduit/method/differential/markcommitted/ConduitAPI_differential_markcommitted_Method.php b/src/applications/conduit/method/differential/markcommitted/ConduitAPI_differential_markcommitted_Method.php index ccc959a173..d06253b663 100644 --- a/src/applications/conduit/method/differential/markcommitted/ConduitAPI_differential_markcommitted_Method.php +++ b/src/applications/conduit/method/differential/markcommitted/ConduitAPI_differential_markcommitted_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_markcommitted_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/parsecommitmessage/ConduitAPI_differential_parsecommitmessage_Method.php b/src/applications/conduit/method/differential/parsecommitmessage/ConduitAPI_differential_parsecommitmessage_Method.php index 2e4058a4c6..b3feedabbb 100644 --- a/src/applications/conduit/method/differential/parsecommitmessage/ConduitAPI_differential_parsecommitmessage_Method.php +++ b/src/applications/conduit/method/differential/parsecommitmessage/ConduitAPI_differential_parsecommitmessage_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_parsecommitmessage_Method extends ConduitAPIMethod { diff --git a/src/applications/conduit/method/differential/setdiffproperty/ConduitAPI_differential_setdiffproperty_Method.php b/src/applications/conduit/method/differential/setdiffproperty/ConduitAPI_differential_setdiffproperty_Method.php index 07afed39d6..44e1840227 100644 --- a/src/applications/conduit/method/differential/setdiffproperty/ConduitAPI_differential_setdiffproperty_Method.php +++ b/src/applications/conduit/method/differential/setdiffproperty/ConduitAPI_differential_setdiffproperty_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_setdiffproperty_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/updaterevision/ConduitAPI_differential_updaterevision_Method.php b/src/applications/conduit/method/differential/updaterevision/ConduitAPI_differential_updaterevision_Method.php index 7acc5824c9..633761b4ed 100644 --- a/src/applications/conduit/method/differential/updaterevision/ConduitAPI_differential_updaterevision_Method.php +++ b/src/applications/conduit/method/differential/updaterevision/ConduitAPI_differential_updaterevision_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_updaterevision_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/differential/updatetaskrevisionassoc/ConduitAPI_differential_updatetaskrevisionassoc_Method.php b/src/applications/conduit/method/differential/updatetaskrevisionassoc/ConduitAPI_differential_updatetaskrevisionassoc_Method.php index 45a3a6829b..9bc2f54712 100644 --- a/src/applications/conduit/method/differential/updatetaskrevisionassoc/ConduitAPI_differential_updatetaskrevisionassoc_Method.php +++ b/src/applications/conduit/method/differential/updatetaskrevisionassoc/ConduitAPI_differential_updatetaskrevisionassoc_Method.php @@ -16,8 +16,12 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_updatetaskrevisionassoc_Method extends ConduitAPIMethod { + public function getMethodDescription() { return "Given a task together with its original and new associated ". "revisions, update the revisions for their attached_tasks."; diff --git a/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php b/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php index 97eb8b3d84..5c2b498022 100644 --- a/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php +++ b/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_differential_updateunitresults_Method extends ConduitAPIMethod { diff --git a/src/applications/conduit/method/diffusion/getcommits/ConduitAPI_diffusion_getcommits_Method.php b/src/applications/conduit/method/diffusion/getcommits/ConduitAPI_diffusion_getcommits_Method.php index 29800e95de..e78db350eb 100644 --- a/src/applications/conduit/method/diffusion/getcommits/ConduitAPI_diffusion_getcommits_Method.php +++ b/src/applications/conduit/method/diffusion/getcommits/ConduitAPI_diffusion_getcommits_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_diffusion_getcommits_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/diffusion/getrecentcommitsbypath/ConduitAPI_diffusion_getrecentcommitsbypath_Method.php b/src/applications/conduit/method/diffusion/getrecentcommitsbypath/ConduitAPI_diffusion_getrecentcommitsbypath_Method.php index 0bafc2aaa2..d5f9bb8378 100644 --- a/src/applications/conduit/method/diffusion/getrecentcommitsbypath/ConduitAPI_diffusion_getrecentcommitsbypath_Method.php +++ b/src/applications/conduit/method/diffusion/getrecentcommitsbypath/ConduitAPI_diffusion_getrecentcommitsbypath_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_diffusion_getrecentcommitsbypath_Method extends ConduitAPIMethod { diff --git a/src/applications/conduit/method/file/download/ConduitAPI_file_download_Method.php b/src/applications/conduit/method/file/download/ConduitAPI_file_download_Method.php index 989b26c941..14d8be1f83 100644 --- a/src/applications/conduit/method/file/download/ConduitAPI_file_download_Method.php +++ b/src/applications/conduit/method/file/download/ConduitAPI_file_download_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_file_download_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/file/upload/ConduitAPI_file_upload_Method.php b/src/applications/conduit/method/file/upload/ConduitAPI_file_upload_Method.php index 308852418a..4dd4897277 100644 --- a/src/applications/conduit/method/file/upload/ConduitAPI_file_upload_Method.php +++ b/src/applications/conduit/method/file/upload/ConduitAPI_file_upload_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_file_upload_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/maniphest/info/ConduitAPI_maniphest_info_Method.php b/src/applications/conduit/method/maniphest/info/ConduitAPI_maniphest_info_Method.php index 0abd32faa3..d6f15cc48d 100644 --- a/src/applications/conduit/method/maniphest/info/ConduitAPI_maniphest_info_Method.php +++ b/src/applications/conduit/method/maniphest/info/ConduitAPI_maniphest_info_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_maniphest_info_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/paste/info/ConduitAPI_paste_info_Method.php b/src/applications/conduit/method/paste/info/ConduitAPI_paste_info_Method.php index 45cbde1882..fea93f258b 100644 --- a/src/applications/conduit/method/paste/info/ConduitAPI_paste_info_Method.php +++ b/src/applications/conduit/method/paste/info/ConduitAPI_paste_info_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_paste_info_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/path/getowners/ConduitAPI_path_getowners_Method.php b/src/applications/conduit/method/path/getowners/ConduitAPI_path_getowners_Method.php index be28ce67b9..98130633d1 100644 --- a/src/applications/conduit/method/path/getowners/ConduitAPI_path_getowners_Method.php +++ b/src/applications/conduit/method/path/getowners/ConduitAPI_path_getowners_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_path_getowners_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/user/find/ConduitAPI_user_find_Method.php b/src/applications/conduit/method/user/find/ConduitAPI_user_find_Method.php index 4659e0d570..06b98f7294 100644 --- a/src/applications/conduit/method/user/find/ConduitAPI_user_find_Method.php +++ b/src/applications/conduit/method/user/find/ConduitAPI_user_find_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_user_find_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/method/user/whoami/ConduitAPI_user_whoami_Method.php b/src/applications/conduit/method/user/whoami/ConduitAPI_user_whoami_Method.php index f2feb82856..904b867fa3 100644 --- a/src/applications/conduit/method/user/whoami/ConduitAPI_user_whoami_Method.php +++ b/src/applications/conduit/method/user/whoami/ConduitAPI_user_whoami_Method.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPI_user_whoami_Method extends ConduitAPIMethod { public function getMethodDescription() { diff --git a/src/applications/conduit/protocol/exception/ConduitException.php b/src/applications/conduit/protocol/exception/ConduitException.php index 069d0a8ab8..a168a77ad4 100644 --- a/src/applications/conduit/protocol/exception/ConduitException.php +++ b/src/applications/conduit/protocol/exception/ConduitException.php @@ -16,5 +16,8 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitException extends Exception { } diff --git a/src/applications/conduit/protocol/request/ConduitAPIRequest.php b/src/applications/conduit/protocol/request/ConduitAPIRequest.php index 1393f35908..d22099f3e0 100644 --- a/src/applications/conduit/protocol/request/ConduitAPIRequest.php +++ b/src/applications/conduit/protocol/request/ConduitAPIRequest.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class ConduitAPIRequest { protected $params; diff --git a/src/applications/conduit/storage/base/PhabricatorConduitDAO.php b/src/applications/conduit/storage/base/PhabricatorConduitDAO.php index 15b79793ff..117059f8ae 100644 --- a/src/applications/conduit/storage/base/PhabricatorConduitDAO.php +++ b/src/applications/conduit/storage/base/PhabricatorConduitDAO.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ abstract class PhabricatorConduitDAO extends PhabricatorLiskDAO { public function getApplicationName() { diff --git a/src/applications/conduit/storage/connectionlog/PhabricatorConduitConnectionLog.php b/src/applications/conduit/storage/connectionlog/PhabricatorConduitConnectionLog.php index e1de62be0c..deb4462966 100644 --- a/src/applications/conduit/storage/connectionlog/PhabricatorConduitConnectionLog.php +++ b/src/applications/conduit/storage/connectionlog/PhabricatorConduitConnectionLog.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class PhabricatorConduitConnectionLog extends PhabricatorConduitDAO { protected $client; diff --git a/src/applications/conduit/storage/methodcalllog/PhabricatorConduitMethodCallLog.php b/src/applications/conduit/storage/methodcalllog/PhabricatorConduitMethodCallLog.php index 7a79d47ade..a44aa9c522 100644 --- a/src/applications/conduit/storage/methodcalllog/PhabricatorConduitMethodCallLog.php +++ b/src/applications/conduit/storage/methodcalllog/PhabricatorConduitMethodCallLog.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class PhabricatorConduitMethodCallLog extends PhabricatorConduitDAO { protected $connectionID; diff --git a/src/applications/conduit/storage/token/PhabricatorConduitCertificateToken.php b/src/applications/conduit/storage/token/PhabricatorConduitCertificateToken.php index 7cdc414313..f8578a3e79 100644 --- a/src/applications/conduit/storage/token/PhabricatorConduitCertificateToken.php +++ b/src/applications/conduit/storage/token/PhabricatorConduitCertificateToken.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group conduit + */ class PhabricatorConduitCertificateToken extends PhabricatorConduitDAO { protected $userPHID; diff --git a/src/docs/technical/conduit.diviner b/src/docs/technical/conduit.diviner new file mode 100644 index 0000000000..a5c3523bb3 --- /dev/null +++ b/src/docs/technical/conduit.diviner @@ -0,0 +1,57 @@ +@title Conduit Technical Documentation +@group conduit + +Technical overview of the Conduit API. + += Overview = + +Conduit is an informal mechanism for transferring ad-hoc JSON blobs around on +the internet. + +Theoretically, it provides an API to Phabricator so external scripts (including +scripts written in other languages) can interface with the applications in the +Phabricator suite. It technically does this, sort of, but it is unstable and +incomplete so you should keep your expectations very low if you choose to build +things on top of it. + +NOTE: Hopefully, this should improve over time, but making Conduit more robust +isn't currently a major project priority because there isn't much demand for it +outside of internal scripts. If you want to use Conduit to build things on top +of Phabricator, let us know so we can adjust priorities. + +Conduit provides an authenticated HTTP API for Phabricator. It is informal and +extremely simple: you post a JSON blob and you get a JSON blob back. You can +access Conduit in PHP with @{class@libphutil:ConduitClient}, or in any language +by executing ##arc call-conduit method## (see ##arc help call-conduit## for +more information). You can see and test available methods at ##/conduit/## in +the web interface. + +Arcanist is implemented using Conduit, and @{class:PhabricatorIRCBot} is +intended as a practical example of how to write a program which interfaces with +Phabricator over Conduit. + += Class Relationships = + +The primary Conduit workflow is exposed at ##/api/##, which routes to +@{class:PhabricatorConduitAPIController}. This controller builds a +@{class:ConduitAPIRequest} representing authentication information and POST +parameters, instantiates an appropriate subclass of @{class:ConduitAPIMethod}, +and passes the request to it. Subclasses of @{class:ConduitAPIMethod} implement +the actual methods which Conduit exposes. + +Conduit calls which fail throw @{class:ConduitException}, which the controller +handles. + +There is a web interface for viewing and testing Conduit called the "Conduit +Console", implemented by @{class:PhabricatorConduitConsoleController} at +##/conduit/##. + +A log of connections and calls is stored by +@{class:PhabriatorConduitConnectionLog} and +@{class:PhabricatorConduitMethodCallLog}, and can be accessed on the web via +@{class:PhabricatorConduitLogController} at ##/conduit/log/##. + +Conduit provides a token-based handshake mechanism used by +##arc install-certificate## at ##/conduit/token/##, implemented by +@{class:PhabricatorConduitTokenController} which stores generated tokens using +@{class:PhabricatorConduitCertificateToken}. \ No newline at end of file diff --git a/src/storage/queryfx/queryfx.php b/src/storage/queryfx/queryfx.php index 6195d9c88e..39fd5be254 100644 --- a/src/storage/queryfx/queryfx.php +++ b/src/storage/queryfx/queryfx.php @@ -57,6 +57,9 @@ function queryfx_one($conn, $sql/*, ... */) { return null; } +/** + * @group storage + */ function vqueryfx_all($conn, $sql, array $argv) { array_unshift($argv, $conn, $sql); call_user_func_array('queryfx', $argv); From 78c695bad23aa4bda9ce66fc77c1b2475ce133ed Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Mon, 4 Jul 2011 13:04:22 -0700 Subject: [PATCH 16/34] Slightly improve Maniphest documentation. --- src/__phutil_library_map__.php | 15 ++++++++---- .../constants/base/ManiphestConstants.php | 24 +++++++++++++++++++ .../maniphest/constants/base/__init__.php | 10 ++++++++ .../constants/owner/ManiphestTaskOwner.php | 5 +++- .../maniphest/constants/owner/__init__.php | 2 ++ .../priority/ManiphestTaskPriority.php | 5 +++- .../maniphest/constants/priority/__init__.php | 2 ++ .../constants/status/ManiphestTaskStatus.php | 5 +++- .../maniphest/constants/status/__init__.php | 2 ++ .../ManiphestTransactionType.php | 5 +++- .../constants/transactiontype/__init__.php | 2 ++ .../controller/base/ManiphestController.php | 3 +++ ...niphestTaskDescriptionChangeController.php | 3 +++ .../ManiphestTaskDetailController.php | 3 +++ .../taskedit/ManiphestTaskEditController.php | 3 +++ .../tasklist/ManiphestTaskListController.php | 3 +++ .../ManiphestTransactionPreviewController.php | 3 +++ .../ManiphestTransactionSaveController.php | 3 +++ .../ManiphestTransactionEditor.php | 3 +++ .../maniphest/query/ManiphestTaskQuery.php | 2 ++ .../replyhandler/ManiphestReplyHandler.php | 3 +++ .../maniphest/storage/base/ManiphestDAO.php | 3 +++ .../maniphest/storage/task/ManiphestTask.php | 3 +++ .../transaction/ManiphestTransaction.php | 3 +++ .../maniphest/view/base/ManiphestView.php | 24 +++++++++++++++++++ .../maniphest/view/base/__init__.php | 12 ++++++++++ .../view/tasklist/ManiphestTaskListView.php | 5 +++- .../maniphest/view/tasklist/__init__.php | 2 +- .../tasksummary/ManiphestTaskSummaryView.php | 5 +++- .../maniphest/view/tasksummary/__init__.php | 2 +- .../ManiphestTransactionDetailView.php | 5 +++- .../view/transactiondetail/__init__.php | 2 +- .../ManiphestTransactionListView.php | 5 +++- .../view/transactionlist/__init__.php | 2 +- src/docs/userguide/remarkup.diviner | 2 +- 35 files changed, 164 insertions(+), 17 deletions(-) create mode 100644 src/applications/maniphest/constants/base/ManiphestConstants.php create mode 100644 src/applications/maniphest/constants/base/__init__.php create mode 100644 src/applications/maniphest/view/base/ManiphestView.php create mode 100644 src/applications/maniphest/view/base/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d050187ca1..af7dba56bd 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -261,6 +261,7 @@ phutil_register_library_map(array( 'LiskIsolationTestCase' => 'storage/lisk/dao/__tests__', 'LiskIsolationTestDAO' => 'storage/lisk/dao/__tests__', 'LiskIsolationTestDAOException' => 'storage/lisk/dao/__tests__', + 'ManiphestConstants' => 'applications/maniphest/constants/base', 'ManiphestController' => 'applications/maniphest/controller/base', 'ManiphestDAO' => 'applications/maniphest/storage/base', 'ManiphestReplyHandler' => 'applications/maniphest/replyhandler', @@ -283,6 +284,7 @@ phutil_register_library_map(array( 'ManiphestTransactionPreviewController' => 'applications/maniphest/controller/transactionpreview', 'ManiphestTransactionSaveController' => 'applications/maniphest/controller/transactionsave', 'ManiphestTransactionType' => 'applications/maniphest/constants/transactiontype', + 'ManiphestView' => 'applications/maniphest/view/base', 'Phabricator404Controller' => 'applications/base/controller/404', 'PhabricatorAuthController' => 'applications/auth/controller/base', 'PhabricatorConduitAPIController' => 'applications/conduit/controller/api', @@ -766,14 +768,19 @@ phutil_register_library_map(array( 'ManiphestTaskDetailController' => 'ManiphestController', 'ManiphestTaskEditController' => 'ManiphestController', 'ManiphestTaskListController' => 'ManiphestController', - 'ManiphestTaskListView' => 'AphrontView', + 'ManiphestTaskListView' => 'ManiphestView', + 'ManiphestTaskOwner' => 'ManiphestConstants', + 'ManiphestTaskPriority' => 'ManiphestConstants', 'ManiphestTaskProject' => 'ManiphestDAO', - 'ManiphestTaskSummaryView' => 'AphrontView', + 'ManiphestTaskStatus' => 'ManiphestConstants', + 'ManiphestTaskSummaryView' => 'ManiphestView', 'ManiphestTransaction' => 'ManiphestDAO', - 'ManiphestTransactionDetailView' => 'AphrontView', - 'ManiphestTransactionListView' => 'AphrontView', + 'ManiphestTransactionDetailView' => 'ManiphestView', + 'ManiphestTransactionListView' => 'ManiphestView', 'ManiphestTransactionPreviewController' => 'ManiphestController', 'ManiphestTransactionSaveController' => 'ManiphestController', + 'ManiphestTransactionType' => 'ManiphestConstants', + 'ManiphestView' => 'AphrontView', 'Phabricator404Controller' => 'PhabricatorController', 'PhabricatorAuthController' => 'PhabricatorController', 'PhabricatorConduitAPIController' => 'PhabricatorConduitController', diff --git a/src/applications/maniphest/constants/base/ManiphestConstants.php b/src/applications/maniphest/constants/base/ManiphestConstants.php new file mode 100644 index 0000000000..7e12478621 --- /dev/null +++ b/src/applications/maniphest/constants/base/ManiphestConstants.php @@ -0,0 +1,24 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @group maniphest + */ +class ManiphestConstants { + +} diff --git a/src/applications/maniphest/constants/base/__init__.php b/src/applications/maniphest/constants/base/__init__.php new file mode 100644 index 0000000000..b216214d94 --- /dev/null +++ b/src/applications/maniphest/constants/base/__init__.php @@ -0,0 +1,10 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + + +phutil_require_source('ManiphestConstants.php'); diff --git a/src/applications/maniphest/constants/owner/ManiphestTaskOwner.php b/src/applications/maniphest/constants/owner/ManiphestTaskOwner.php index 6a6ff59b4c..7b820a8f74 100644 --- a/src/applications/maniphest/constants/owner/ManiphestTaskOwner.php +++ b/src/applications/maniphest/constants/owner/ManiphestTaskOwner.php @@ -16,7 +16,10 @@ * limitations under the License. */ -final class ManiphestTaskOwner { +/** + * @group maniphest + */ +final class ManiphestTaskOwner extends ManiphestConstants { const OWNER_UP_FOR_GRABS = 'PHID-!!!!-UP-FOR-GRABS'; diff --git a/src/applications/maniphest/constants/owner/__init__.php b/src/applications/maniphest/constants/owner/__init__.php index ac8bfb239d..4b587ae3d0 100644 --- a/src/applications/maniphest/constants/owner/__init__.php +++ b/src/applications/maniphest/constants/owner/__init__.php @@ -6,5 +6,7 @@ +phutil_require_module('phabricator', 'applications/maniphest/constants/base'); + phutil_require_source('ManiphestTaskOwner.php'); diff --git a/src/applications/maniphest/constants/priority/ManiphestTaskPriority.php b/src/applications/maniphest/constants/priority/ManiphestTaskPriority.php index fa52e3d9d6..afa75e3eb4 100644 --- a/src/applications/maniphest/constants/priority/ManiphestTaskPriority.php +++ b/src/applications/maniphest/constants/priority/ManiphestTaskPriority.php @@ -16,7 +16,10 @@ * limitations under the License. */ -final class ManiphestTaskPriority { +/** + * @group maniphest + */ +final class ManiphestTaskPriority extends ManiphestConstants { const PRIORITY_UNBREAK_NOW = 100; const PRIORITY_TRIAGE = 90; diff --git a/src/applications/maniphest/constants/priority/__init__.php b/src/applications/maniphest/constants/priority/__init__.php index 791832bac4..9f3dcb8c8f 100644 --- a/src/applications/maniphest/constants/priority/__init__.php +++ b/src/applications/maniphest/constants/priority/__init__.php @@ -6,6 +6,8 @@ +phutil_require_module('phabricator', 'applications/maniphest/constants/base'); + phutil_require_module('phutil', 'utils'); diff --git a/src/applications/maniphest/constants/status/ManiphestTaskStatus.php b/src/applications/maniphest/constants/status/ManiphestTaskStatus.php index 79611fe72c..813db81e94 100644 --- a/src/applications/maniphest/constants/status/ManiphestTaskStatus.php +++ b/src/applications/maniphest/constants/status/ManiphestTaskStatus.php @@ -16,7 +16,10 @@ * limitations under the License. */ -final class ManiphestTaskStatus { +/** + * @group maniphest + */ +final class ManiphestTaskStatus extends ManiphestConstants { const STATUS_OPEN = 0; const STATUS_CLOSED_RESOLVED = 1; diff --git a/src/applications/maniphest/constants/status/__init__.php b/src/applications/maniphest/constants/status/__init__.php index 0b7212d908..bd5a95071e 100644 --- a/src/applications/maniphest/constants/status/__init__.php +++ b/src/applications/maniphest/constants/status/__init__.php @@ -6,6 +6,8 @@ +phutil_require_module('phabricator', 'applications/maniphest/constants/base'); + phutil_require_module('phutil', 'utils'); diff --git a/src/applications/maniphest/constants/transactiontype/ManiphestTransactionType.php b/src/applications/maniphest/constants/transactiontype/ManiphestTransactionType.php index acf035dac6..01706429c9 100644 --- a/src/applications/maniphest/constants/transactiontype/ManiphestTransactionType.php +++ b/src/applications/maniphest/constants/transactiontype/ManiphestTransactionType.php @@ -16,7 +16,10 @@ * limitations under the License. */ -final class ManiphestTransactionType { +/** + * @group maniphest + */ +final class ManiphestTransactionType extends ManiphestConstants { const TYPE_NONE = 'comment'; const TYPE_STATUS = 'status'; diff --git a/src/applications/maniphest/constants/transactiontype/__init__.php b/src/applications/maniphest/constants/transactiontype/__init__.php index 2d1616cc7d..7beed47c4c 100644 --- a/src/applications/maniphest/constants/transactiontype/__init__.php +++ b/src/applications/maniphest/constants/transactiontype/__init__.php @@ -6,5 +6,7 @@ +phutil_require_module('phabricator', 'applications/maniphest/constants/base'); + phutil_require_source('ManiphestTransactionType.php'); diff --git a/src/applications/maniphest/controller/base/ManiphestController.php b/src/applications/maniphest/controller/base/ManiphestController.php index 6c4cc39668..70067624a4 100644 --- a/src/applications/maniphest/controller/base/ManiphestController.php +++ b/src/applications/maniphest/controller/base/ManiphestController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ abstract class ManiphestController extends PhabricatorController { public function buildStandardPageResponse($view, array $data) { diff --git a/src/applications/maniphest/controller/descriptionchange/ManiphestTaskDescriptionChangeController.php b/src/applications/maniphest/controller/descriptionchange/ManiphestTaskDescriptionChangeController.php index 4c8f5004a1..d1b73fdba6 100644 --- a/src/applications/maniphest/controller/descriptionchange/ManiphestTaskDescriptionChangeController.php +++ b/src/applications/maniphest/controller/descriptionchange/ManiphestTaskDescriptionChangeController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestTaskDescriptionChangeController extends ManiphestController { private $transactionID; diff --git a/src/applications/maniphest/controller/taskdetail/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/taskdetail/ManiphestTaskDetailController.php index d1c432e82e..99eb7408f9 100644 --- a/src/applications/maniphest/controller/taskdetail/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/taskdetail/ManiphestTaskDetailController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestTaskDetailController extends ManiphestController { private $id; diff --git a/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php b/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php index 47e474c453..9f5f278935 100644 --- a/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestTaskEditController extends ManiphestController { private $id; diff --git a/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php b/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php index 0c0dee7d1f..b57e8c55e2 100644 --- a/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php +++ b/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestTaskListController extends ManiphestController { const DEFAULT_PAGE_SIZE = 1000; diff --git a/src/applications/maniphest/controller/transactionpreview/ManiphestTransactionPreviewController.php b/src/applications/maniphest/controller/transactionpreview/ManiphestTransactionPreviewController.php index 6d63ef761e..a830d5f0cc 100644 --- a/src/applications/maniphest/controller/transactionpreview/ManiphestTransactionPreviewController.php +++ b/src/applications/maniphest/controller/transactionpreview/ManiphestTransactionPreviewController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestTransactionPreviewController extends ManiphestController { private $id; diff --git a/src/applications/maniphest/controller/transactionsave/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/transactionsave/ManiphestTransactionSaveController.php index 9284a8d027..99e95f6088 100644 --- a/src/applications/maniphest/controller/transactionsave/ManiphestTransactionSaveController.php +++ b/src/applications/maniphest/controller/transactionsave/ManiphestTransactionSaveController.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestTransactionSaveController extends ManiphestController { public function processRequest() { diff --git a/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php index 37acc5d26d..825d4936d5 100644 --- a/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestTransactionEditor { private $parentMessageID; diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index d7f874f1eb..1db3f02539 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -19,6 +19,8 @@ /** * Query tasks by specific criteria. This class uses the higher-performance * but less-general Maniphest indexes to satisfy queries. + * + * @group maniphest */ final class ManiphestTaskQuery { diff --git a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php b/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php index c3f8cfe1c9..0e0297c197 100644 --- a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php +++ b/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestReplyHandler extends PhabricatorMailReplyHandler { public function validateMailReceiver($mail_receiver) { diff --git a/src/applications/maniphest/storage/base/ManiphestDAO.php b/src/applications/maniphest/storage/base/ManiphestDAO.php index 632fa9a0a7..c7df0c59b0 100644 --- a/src/applications/maniphest/storage/base/ManiphestDAO.php +++ b/src/applications/maniphest/storage/base/ManiphestDAO.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestDAO extends PhabricatorLiskDAO { public function getApplicationName() { diff --git a/src/applications/maniphest/storage/task/ManiphestTask.php b/src/applications/maniphest/storage/task/ManiphestTask.php index 5bf378b49c..d3ff64b223 100644 --- a/src/applications/maniphest/storage/task/ManiphestTask.php +++ b/src/applications/maniphest/storage/task/ManiphestTask.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestTask extends ManiphestDAO { protected $phid; diff --git a/src/applications/maniphest/storage/transaction/ManiphestTransaction.php b/src/applications/maniphest/storage/transaction/ManiphestTransaction.php index ba7e5bf2ec..153e46f375 100644 --- a/src/applications/maniphest/storage/transaction/ManiphestTransaction.php +++ b/src/applications/maniphest/storage/transaction/ManiphestTransaction.php @@ -16,6 +16,9 @@ * limitations under the License. */ +/** + * @group maniphest + */ class ManiphestTransaction extends ManiphestDAO { protected $taskID; diff --git a/src/applications/maniphest/view/base/ManiphestView.php b/src/applications/maniphest/view/base/ManiphestView.php new file mode 100644 index 0000000000..6a8144c9c0 --- /dev/null +++ b/src/applications/maniphest/view/base/ManiphestView.php @@ -0,0 +1,24 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @group maniphest + */ +abstract class ManiphestView extends AphrontView { + +} diff --git a/src/applications/maniphest/view/base/__init__.php b/src/applications/maniphest/view/base/__init__.php new file mode 100644 index 0000000000..8ac251e590 --- /dev/null +++ b/src/applications/maniphest/view/base/__init__.php @@ -0,0 +1,12 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'view/base'); + + +phutil_require_source('ManiphestView.php'); diff --git a/src/applications/maniphest/view/tasklist/ManiphestTaskListView.php b/src/applications/maniphest/view/tasklist/ManiphestTaskListView.php index 0e01aefb96..5a51f7b7f1 100644 --- a/src/applications/maniphest/view/tasklist/ManiphestTaskListView.php +++ b/src/applications/maniphest/view/tasklist/ManiphestTaskListView.php @@ -16,7 +16,10 @@ * limitations under the License. */ -class ManiphestTaskListView extends AphrontView { +/** + * @group maniphest + */ +class ManiphestTaskListView extends ManiphestView { private $tasks; private $handles; diff --git a/src/applications/maniphest/view/tasklist/__init__.php b/src/applications/maniphest/view/tasklist/__init__.php index 5a35a49ef4..be0ea53278 100644 --- a/src/applications/maniphest/view/tasklist/__init__.php +++ b/src/applications/maniphest/view/tasklist/__init__.php @@ -6,8 +6,8 @@ +phutil_require_module('phabricator', 'applications/maniphest/view/base'); phutil_require_module('phabricator', 'applications/maniphest/view/tasksummary'); -phutil_require_module('phabricator', 'view/base'); phutil_require_source('ManiphestTaskListView.php'); diff --git a/src/applications/maniphest/view/tasksummary/ManiphestTaskSummaryView.php b/src/applications/maniphest/view/tasksummary/ManiphestTaskSummaryView.php index 7cad3d02a8..a43dd698d0 100644 --- a/src/applications/maniphest/view/tasksummary/ManiphestTaskSummaryView.php +++ b/src/applications/maniphest/view/tasksummary/ManiphestTaskSummaryView.php @@ -16,7 +16,10 @@ * limitations under the License. */ -class ManiphestTaskSummaryView extends AphrontView { +/** + * @group maniphest + */ +class ManiphestTaskSummaryView extends ManiphestView { private $task; private $handles; diff --git a/src/applications/maniphest/view/tasksummary/__init__.php b/src/applications/maniphest/view/tasksummary/__init__.php index 8ac09920ba..8e43605347 100644 --- a/src/applications/maniphest/view/tasksummary/__init__.php +++ b/src/applications/maniphest/view/tasksummary/__init__.php @@ -8,8 +8,8 @@ phutil_require_module('phabricator', 'applications/maniphest/constants/priority'); phutil_require_module('phabricator', 'applications/maniphest/constants/status'); +phutil_require_module('phabricator', 'applications/maniphest/view/base'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); -phutil_require_module('phabricator', 'view/base'); phutil_require_module('phabricator', 'view/utils'); phutil_require_module('phutil', 'markup'); diff --git a/src/applications/maniphest/view/transactiondetail/ManiphestTransactionDetailView.php b/src/applications/maniphest/view/transactiondetail/ManiphestTransactionDetailView.php index 243ee7eae9..dc2e2a3cc0 100644 --- a/src/applications/maniphest/view/transactiondetail/ManiphestTransactionDetailView.php +++ b/src/applications/maniphest/view/transactiondetail/ManiphestTransactionDetailView.php @@ -16,7 +16,10 @@ * limitations under the License. */ -class ManiphestTransactionDetailView extends AphrontView { +/** + * @group maniphest + */ +class ManiphestTransactionDetailView extends ManiphestView { private $transactions; private $handles; diff --git a/src/applications/maniphest/view/transactiondetail/__init__.php b/src/applications/maniphest/view/transactiondetail/__init__.php index 600b258bce..bbb604b5bc 100644 --- a/src/applications/maniphest/view/transactiondetail/__init__.php +++ b/src/applications/maniphest/view/transactiondetail/__init__.php @@ -9,12 +9,12 @@ phutil_require_module('phabricator', 'applications/maniphest/constants/priority'); phutil_require_module('phabricator', 'applications/maniphest/constants/status'); phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype'); +phutil_require_module('phabricator', 'applications/maniphest/view/base'); phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'infrastructure/javelin/markup'); -phutil_require_module('phabricator', 'view/base'); phutil_require_module('phabricator', 'view/utils'); phutil_require_module('phutil', 'markup'); diff --git a/src/applications/maniphest/view/transactionlist/ManiphestTransactionListView.php b/src/applications/maniphest/view/transactionlist/ManiphestTransactionListView.php index f2a33a1235..466c62df05 100644 --- a/src/applications/maniphest/view/transactionlist/ManiphestTransactionListView.php +++ b/src/applications/maniphest/view/transactionlist/ManiphestTransactionListView.php @@ -16,7 +16,10 @@ * limitations under the License. */ -class ManiphestTransactionListView extends AphrontView { +/** + * @group maniphest + */ +class ManiphestTransactionListView extends ManiphestView { private $transactions; private $handles; diff --git a/src/applications/maniphest/view/transactionlist/__init__.php b/src/applications/maniphest/view/transactionlist/__init__.php index 8c06249c94..6611e4539a 100644 --- a/src/applications/maniphest/view/transactionlist/__init__.php +++ b/src/applications/maniphest/view/transactionlist/__init__.php @@ -6,8 +6,8 @@ +phutil_require_module('phabricator', 'applications/maniphest/view/base'); phutil_require_module('phabricator', 'applications/maniphest/view/transactiondetail'); -phutil_require_module('phabricator', 'view/base'); phutil_require_source('ManiphestTransactionListView.php'); diff --git a/src/docs/userguide/remarkup.diviner b/src/docs/userguide/remarkup.diviner index 7d338a8ef1..cc0ae5c59f 100644 --- a/src/docs/userguide/remarkup.diviner +++ b/src/docs/userguide/remarkup.diviner @@ -1,7 +1,7 @@ @title Remarkup Reference @group userguide -Explains how to make bold text, etc. This makes your words louder so you can win +Explains how to make bold text; this makes your words louder so you can win arguments. = Overview = From eeb8d10f42ee28430e4544670b639429c6684557 Mon Sep 17 00:00:00 2001 From: Ricky Elrod <codeblock@eighthbit.net> Date: Tue, 5 Jul 2011 08:49:32 -0700 Subject: [PATCH 17/34] Edited src/infrastructure/daemon/irc/handler/objectname/PhabricatorIRCObjectNameHandler.php via GitHub --- .../irc/handler/objectname/PhabricatorIRCObjectNameHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/infrastructure/daemon/irc/handler/objectname/PhabricatorIRCObjectNameHandler.php b/src/infrastructure/daemon/irc/handler/objectname/PhabricatorIRCObjectNameHandler.php index 4b8e45bd95..c4126da658 100644 --- a/src/infrastructure/daemon/irc/handler/objectname/PhabricatorIRCObjectNameHandler.php +++ b/src/infrastructure/daemon/irc/handler/objectname/PhabricatorIRCObjectNameHandler.php @@ -122,7 +122,7 @@ class PhabricatorIRCObjectNameHandler extends PhabricatorIRCHandler { // since we (ideally) want to keep the bot to Conduit calls...and // not call to Phabricator-specific stuff (like actually loading // the User object and fetching his/her username.) - $output[$paste['phid']] = 'P'.$paste['id'].': '.$paste['uri']; + $output[$paste['phid']] = 'P'.$paste['id'].': '.$paste['uri'].' - '.$paste['title']; } } From 6eed2ad2bbed9264fed66ef69f4414e53132c402 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Tue, 28 Jun 2011 16:21:06 -0700 Subject: [PATCH 18/34] Deal more gracefully with unusual display changesets Summary: These came up in dealing with the diff produced by T271. When a file is unmodified, don't try to use the "ignore all whitespace" algorithm on it. Also, detect "changed only by adding or removing trailing whitespace" vs "this file was not modified" correctly. Test Plan: Viewed the diff that came out of running 'arc diff' on my T271 mess. Reviewed By: tuomaspelkonen Reviewers: alex, jungejason, tuomaspelkonen, aran CC: aran, tuomaspelkonen Differential Revision: 548 --- .../changeset/DifferentialChangesetParser.php | 56 ++++++++++++++----- 1 file changed, 42 insertions(+), 14 deletions(-) diff --git a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php index 772a00cadf..6a15fb9a52 100644 --- a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php @@ -341,6 +341,21 @@ class DifferentialChangesetParser { switch ($this->whitespaceMode) { case self::WHITESPACE_IGNORE_TRAILING: if (rtrim($o_desc['text']) == rtrim($n_desc['text'])) { + if ($o_desc['type']) { + // If we're converting this into an unchanged line because of + // a trailing whitespace difference, mark it as a whitespace + // change so we can show "This file was modified only by + // adding or removing trailing whitespace." instead of + // "This file was not modified.". + $whitelines = true; + } + $similar = true; + } + break; + default: + // In this case, the lines are similar if there is no change type + // (that is, just trust the diff algorithm). + if (!$o_desc['type']) { $similar = true; } break; @@ -349,7 +364,6 @@ class DifferentialChangesetParser { $o_desc['type'] = null; $n_desc['type'] = null; $skip_intra[count($old)] = true; - $whitelines = true; } else { $changed = true; } @@ -744,24 +758,38 @@ class DifferentialChangesetParser { $changeset->getFileType() == DifferentialChangeType::FILE_SYMLINK) { if ($skip_cache || !$this->loadCache()) { + $ignore_all = ($this->whitespaceMode == self::WHITESPACE_IGNORE_ALL); + // The "ignore all whitespace" algorithm depends on rediffing the // files, and we currently need complete representations of both // files to do anything reasonable. If we only have parts of the files, // don't use the "ignore all" algorithm. - $can_use_ignore_all = true; - $hunks = $changeset->getHunks(); - if (count($hunks) !== 1) { - $can_use_ignore_all = false; - } else { - $first_hunk = reset($hunks); - if ($first_hunk->getOldOffset() != 1 || - $first_hunk->getNewOffset() != 1) { - $can_use_ignore_all = false; + if ($ignore_all) { + $hunks = $changeset->getHunks(); + if (count($hunks) !== 1) { + $ignore_all = false; + } else { + $first_hunk = reset($hunks); + if ($first_hunk->getOldOffset() != 1 || + $first_hunk->getNewOffset() != 1) { + $ignore_all = false; + } } } - if ($this->whitespaceMode == self::WHITESPACE_IGNORE_ALL && - $can_use_ignore_all) { + if ($ignore_all) { + $old_file = $changeset->makeOldFile(); + $new_file = $changeset->makeNewFile(); + if ($old_file == $new_file) { + // If the old and new files are exactly identical, the synthetic + // diff below will give us nonsense and whitespace modes are + // irrelevant anyway. This occurs when you, e.g., copy a file onto + // itself in Subversion (see T271). + $ignore_all = false; + } + } + + if ($ignore_all) { // Huge mess. Generate a "-bw" (ignore all whitespace changes) diff, // parse it out, and then play a shell game with the parsed format @@ -773,8 +801,8 @@ class DifferentialChangesetParser { $old_tmp = new TempFile(); $new_tmp = new TempFile(); - Filesystem::writeFile($old_tmp, $changeset->makeOldFile()); - Filesystem::writeFile($new_tmp, $changeset->makeNewFile()); + Filesystem::writeFile($old_tmp, $old_file); + Filesystem::writeFile($new_tmp, $new_file); list($err, $diff) = exec_manual( 'diff -bw -U65535 %s %s ', $old_tmp, From f61a02e3425c780f0da531c7a65883606f9be934 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Sun, 3 Jul 2011 12:24:56 -0700 Subject: [PATCH 19/34] Use phutil_utf8_shorten() in PhabricatorProjectListController Summary: This is a little cleaner and more general than textWrap(). See also D559. Test Plan: Loaded project list page, edited a project description to have >100 characters of text, reloaded list page, it was correctly shortened. Reviewed By: tuomaspelkonen Reviewers: cadamo, jungejason, aran, tuomaspelkonen CC: aran, tuomaspelkonen Differential Revision: 584 --- .../list/PhabricatorProjectListController.php | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/applications/project/controller/list/PhabricatorProjectListController.php b/src/applications/project/controller/list/PhabricatorProjectListController.php index ba825511ee..759b8c1c61 100644 --- a/src/applications/project/controller/list/PhabricatorProjectListController.php +++ b/src/applications/project/controller/list/PhabricatorProjectListController.php @@ -74,7 +74,7 @@ class PhabricatorProjectListController $blurb = nonempty( $profile->getBlurb(), 'Oops!, nothing is known about this elusive project.'); - $blurb = $this->textWrap($blurb, $columns = 100); + $blurb = phutil_utf8_shorten($blurb, $columns = 100); $rows[] = array( phutil_escape_html($project->getName()), @@ -129,18 +129,4 @@ class PhabricatorProjectListController 'title' => 'Projects', )); } - - private function textWrap($text, $length) { - if (strlen($text) <= $length) { - return $text; - } else { - // TODO: perhaps this could be improved, adding the ability to get the - // last letter and suppress it, if it is one of [(,:; ,etc. - // making "blurb" looks a little bit better. :) - $wrapped = wordwrap($text, $length, '__#END#__'); - $end_position = strpos($wrapped, '__#END#__'); - $wrapped = substr($text, 0, $end_position).'...'; - return $wrapped; - } - } } From c9b7cffa4f8dddad6635f666dcf5fe23e4d6c381 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Sun, 3 Jul 2011 12:36:43 -0700 Subject: [PATCH 20/34] Correctly localize times in the user list Summary: We currently show a user's signup time in //their// local time, not the viewer's local time. Oops! Test Plan: Looked at user list. Reviewed By: tuomaspelkonen Reviewers: toulouse, jungejason, tuomaspelkonen, aran CC: aran, epriestley, tuomaspelkonen Differential Revision: 585 --- .../controller/list/PhabricatorPeopleListController.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/applications/people/controller/list/PhabricatorPeopleListController.php b/src/applications/people/controller/list/PhabricatorPeopleListController.php index e5f2d6b81e..991b5fff40 100644 --- a/src/applications/people/controller/list/PhabricatorPeopleListController.php +++ b/src/applications/people/controller/list/PhabricatorPeopleListController.php @@ -20,7 +20,8 @@ class PhabricatorPeopleListController extends PhabricatorPeopleController { public function processRequest() { $request = $this->getRequest(); - $is_admin = $request->getUser()->getIsAdmin(); + $viewer = $request->getUser(); + $is_admin = $viewer->getIsAdmin(); $user = new PhabricatorUser(); @@ -53,8 +54,8 @@ class PhabricatorPeopleListController extends PhabricatorPeopleController { } $rows[] = array( - phabricator_date($user->getDateCreated(), $user), - phabricator_time($user->getDateCreated(), $user), + phabricator_date($user->getDateCreated(), $viewer), + phabricator_time($user->getDateCreated(), $viewer), phutil_render_tag( 'a', array( From 51de554238801a6b6969186ba27e4873ea0cd54b Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Tue, 5 Jul 2011 07:21:04 -0700 Subject: [PATCH 21/34] Validate the provided "host" key for certain Conduit methods Summary: This allows us to detect a mismatched client and server hostname. See D591. Test Plan: See D591. Reviewed By: tuomaspelkonen Reviewers: jungejason, llorca, tuomaspelkonen, aran CC: aran, tuomaspelkonen Differential Revision: 592 --- .../conduit/method/base/ConduitAPIMethod.php | 25 +++++++++++++++++++ .../conduit/method/base/__init__.php | 3 +++ .../ConduitAPI_conduit_connect_Method.php | 3 +++ ...nduitAPI_conduit_getcertificate_Method.php | 2 ++ 4 files changed, 33 insertions(+) diff --git a/src/applications/conduit/method/base/ConduitAPIMethod.php b/src/applications/conduit/method/base/ConduitAPIMethod.php index 7f04f9cb69..4cf59a2148 100644 --- a/src/applications/conduit/method/base/ConduitAPIMethod.php +++ b/src/applications/conduit/method/base/ConduitAPIMethod.php @@ -66,4 +66,29 @@ abstract class ConduitAPIMethod { return str_replace('_', '.', $method_fragment); } + protected function validateHost($host) { + if (!$host) { + // If the client doesn't send a host key, don't complain. We should in + // the future, but this change isn't severe enough to bump the protocol + // version. + + // TODO: Remove this once the protocol version gets bumped past 2 (i.e., + // require the host key be present and valid). + return; + } + + $host = new PhutilURI($host); + $host->setPath('/'); + $host = (string)$host; + + $self = PhabricatorEnv::getProductionURI('/'); + if ($self !== $host) { + throw new Exception( + "Your client is connecting to this install as '{$host}', but it is ". + "configured as '{$self}'. The client and server must use the exact ". + "same URI to identify the install. Edit your .arcconfig or ". + "phabricator/conf so they agree on the URI for the install."); + } + } + } diff --git a/src/applications/conduit/method/base/__init__.php b/src/applications/conduit/method/base/__init__.php index f7268ea664..50c97a4d58 100644 --- a/src/applications/conduit/method/base/__init__.php +++ b/src/applications/conduit/method/base/__init__.php @@ -6,6 +6,9 @@ +phutil_require_module('phabricator', 'infrastructure/env'); + +phutil_require_module('phutil', 'parser/uri'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php b/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php index 6637c8a2f4..6e3e04d813 100644 --- a/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php +++ b/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php @@ -37,6 +37,7 @@ class ConduitAPI_conduit_connect_Method extends ConduitAPIMethod { 'user' => 'optional string', 'authToken' => 'optional int', 'authSignature' => 'optional string', + 'host' => 'required string', ); } @@ -70,6 +71,8 @@ class ConduitAPI_conduit_connect_Method extends ConduitAPIMethod { protected function execute(ConduitAPIRequest $request) { + $this->validateHost($request->getValue('host')); + $client = $request->getValue('client'); $client_version = (int)$request->getValue('clientVersion'); $client_description = (string)$request->getValue('clientDescription'); diff --git a/src/applications/conduit/method/conduit/getcertificate/ConduitAPI_conduit_getcertificate_Method.php b/src/applications/conduit/method/conduit/getcertificate/ConduitAPI_conduit_getcertificate_Method.php index f7f90d5938..4f24a08563 100644 --- a/src/applications/conduit/method/conduit/getcertificate/ConduitAPI_conduit_getcertificate_Method.php +++ b/src/applications/conduit/method/conduit/getcertificate/ConduitAPI_conduit_getcertificate_Method.php @@ -32,6 +32,7 @@ class ConduitAPI_conduit_getcertificate_Method extends ConduitAPIMethod { public function defineParamTypes() { return array( 'token' => 'required string', + 'host' => 'required string', ); } @@ -49,6 +50,7 @@ class ConduitAPI_conduit_getcertificate_Method extends ConduitAPIMethod { } protected function execute(ConduitAPIRequest $request) { + $this->validateHost($request->getValue('host')); $failed_attempts = PhabricatorUserLog::loadRecentEventsFromThisIP( PhabricatorUserLog::ACTION_CONDUIT_CERTIFICATE_FAILURE, From 4ef918e213ed4c349b236e28bc371a16b5457d99 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Sun, 3 Jul 2011 09:47:31 -0700 Subject: [PATCH 22/34] Add a garbage collector daemon Summary: Phabricator generates a bunch of data that we don't need to keep around forever, add a GC daemon to get rid of it with some basic configuration options. This needs a couple more diffs to get some of the details but I think this is a reasonable start. I also fixed a couple of UI things related to this, e.g. the daemon logs page going crazy when a daemon gets stuck in a loop and dumps tons of data to stdout. Test Plan: - Ran gc daemon in 'phd debug' mode and saw it delete stuff, then sleep once it had cleaned everything up. - Mucked around with TTLs and verified they work correctly. - Viewed gc'd transcripts in the web interface and made sure they displayed okay. - Viewed daemon logs before/after garbage collection. - Running some run-at / run-for tests now, I'll update if the daemon doesn't shut off in ~10-15 minutes. :P Reviewed By: tuomaspelkonen Reviewers: jungejason, tuomaspelkonen, aran CC: aran, tuomaspelkonen, epriestley Differential Revision: 583 --- conf/default.conf.php | 31 ++++ scripts/install/update_phabricator.sh | 1 + src/__phutil_library_map__.php | 2 + .../PhabricatorDaemonLogEventsView.php | 37 ++++- .../daemon/view/daemonlogevents/__init__.php | 1 + .../transcript/HeraldTranscriptController.php | 81 ++++------ .../herald/controller/transcript/__init__.php | 1 + .../configuration/managing_daemons.diviner | 1 + .../PhabricatorGarbageCollectorDaemon.php | 139 ++++++++++++++++++ .../daemon/garbagecollector/__init__.php | 16 ++ 10 files changed, 259 insertions(+), 51 deletions(-) create mode 100644 src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php create mode 100644 src/infrastructure/daemon/garbagecollector/__init__.php diff --git a/conf/default.conf.php b/conf/default.conf.php index da5d7a0c2b..7472c1ecb0 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -375,6 +375,37 @@ return array( // silly (but sort of awesome). 'remarkup.enable-embedded-youtube' => false, + +// -- Garbage Collection ---------------------------------------------------- // + + // Phabricator generates various logs and caches in the database which can + // be garbage collected after a while to make the total data size more + // manageable. To run garbage collection, launch a + // PhabricatorGarbageCollector daemon. + + // Since the GC daemon can issue large writes and table scans, you may want to + // run it only during off hours or make sure it is scheduled so it doesn't + // overlap with backups. This determines when the daemon can start running + // each day. + 'gcdaemon.run-at' => '12 AM', + + // How many seconds after 'gcdaemon.run-at' the daemon may collect garbage + // for. By default it runs continuously, but you can set it to run for a + // limited period of time. For instance, if you do backups at 3 AM, you might + // run garbage collection for an hour beforehand. This is not a high-precision + // limit so you may want to leave some room for the GC to actually stop, and + // if you set it to something like 3 seconds you're on your own. + 'gcdaemon.run-for' => 24 * 60 * 60, + + // These 'ttl' keys configure how much old data the GC daemon keeps around. + // Objects older than the ttl will be collected. Set any value to 0 to store + // data indefinitely. + + 'gcdaemon.ttl.herald-transcripts' => 30 * (24 * 60 * 60), + 'gcdaemon.ttl.daemon-logs' => 7 * (24 * 60 * 60), + 'gcdaemon.ttl.differential-render-cache' => 7 * (24 * 60 * 60), + + // -- Customization --------------------------------------------------------- // // Paths to additional phutil libraries to load. diff --git a/scripts/install/update_phabricator.sh b/scripts/install/update_phabricator.sh index 207650d108..ebde400efe 100755 --- a/scripts/install/update_phabricator.sh +++ b/scripts/install/update_phabricator.sh @@ -58,5 +58,6 @@ sudo /etc/init.d/httpd start # $ROOT/phabricator/bin/phd repository-launch-master # $ROOT/phabricator/bin/phd launch metamta +# $ROOT/phabricator/bin/phd launch garbagecollector # $ROOT/phabricator/bin/phd launch 4 taskmaster # $ROOT/phabricator/bin/phd launch ircbot /config/bot.json diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index af7dba56bd..f38452557b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -352,6 +352,7 @@ phutil_register_library_map(array( 'PhabricatorFileURI' => 'applications/files/uri', 'PhabricatorFileUploadController' => 'applications/files/controller/upload', 'PhabricatorFileViewController' => 'applications/files/controller/view', + 'PhabricatorGarbageCollectorDaemon' => 'infrastructure/daemon/garbagecollector', 'PhabricatorGoodForNothingWorker' => 'infrastructure/daemon/workers/worker/goodfornothing', 'PhabricatorHandleObjectSelectorDataView' => 'applications/phid/handle/view/selector', 'PhabricatorHelpController' => 'applications/help/controller/base', @@ -844,6 +845,7 @@ phutil_register_library_map(array( 'PhabricatorFileTransformController' => 'PhabricatorFileController', 'PhabricatorFileUploadController' => 'PhabricatorFileController', 'PhabricatorFileViewController' => 'PhabricatorFileController', + 'PhabricatorGarbageCollectorDaemon' => 'PhabricatorDaemon', 'PhabricatorGoodForNothingWorker' => 'PhabricatorWorker', 'PhabricatorHelpController' => 'PhabricatorController', 'PhabricatorHelpKeyboardShortcutController' => 'PhabricatorHelpController', diff --git a/src/applications/daemon/view/daemonlogevents/PhabricatorDaemonLogEventsView.php b/src/applications/daemon/view/daemonlogevents/PhabricatorDaemonLogEventsView.php index dd09781683..aaebcbec0c 100644 --- a/src/applications/daemon/view/daemonlogevents/PhabricatorDaemonLogEventsView.php +++ b/src/applications/daemon/view/daemonlogevents/PhabricatorDaemonLogEventsView.php @@ -43,11 +43,46 @@ final class PhabricatorDaemonLogEventsView extends AphrontView { } foreach ($this->events as $event) { + + // Limit display log size. If a daemon gets stuck in an output loop this + // page can be like >100MB if we don't truncate stuff. Try to do cheap + // line-based truncation first, and fall back to expensive UTF-8 character + // truncation if that doesn't get things short enough. + + $message = $event->getMessage(); + + $more_lines = null; + $more_chars = null; + $line_limit = 12; + if (substr_count($message, "\n") > $line_limit) { + $message = explode("\n", $message); + $more_lines = count($message) - $line_limit; + $message = array_slice($message, 0, $line_limit); + $message = implode("\n", $message); + } + + $char_limit = 8192; + if (strlen($message) > $char_limit) { + $message = phutil_utf8v($message); + $more_chars = count($message) - $char_limit; + $message = array_slice($message, 0, $char_limit); + $message = implode('', $message); + } + + $more = null; + if ($more_chars) { + $more = number_format($more_chars); + $more = "\n<... {$more} more characters ...>"; + } else if ($more_lines) { + $more = number_format($more_lines); + $more = "\n<... {$more} more lines ...>"; + } + $row = array( phutil_escape_html($event->getLogType()), phabricator_date($event->getEpoch(), $this->user), phabricator_time($event->getEpoch(), $this->user), - str_replace("\n", '<br />', phutil_escape_html($event->getMessage())), + str_replace("\n", '<br />', phutil_escape_html($message.$more)), ); if ($this->combinedLog) { diff --git a/src/applications/daemon/view/daemonlogevents/__init__.php b/src/applications/daemon/view/daemonlogevents/__init__.php index dd4e977647..04be32163d 100644 --- a/src/applications/daemon/view/daemonlogevents/__init__.php +++ b/src/applications/daemon/view/daemonlogevents/__init__.php @@ -11,6 +11,7 @@ phutil_require_module('phabricator', 'view/control/table'); phutil_require_module('phabricator', 'view/utils'); phutil_require_module('phutil', 'markup'); +phutil_require_module('phutil', 'utils'); phutil_require_source('PhabricatorDaemonLogEventsView.php'); diff --git a/src/applications/herald/controller/transcript/HeraldTranscriptController.php b/src/applications/herald/controller/transcript/HeraldTranscriptController.php index ced383d98d..bf9f0fcd5d 100644 --- a/src/applications/herald/controller/transcript/HeraldTranscriptController.php +++ b/src/applications/herald/controller/transcript/HeraldTranscriptController.php @@ -42,40 +42,45 @@ class HeraldTranscriptController extends HeraldController { throw new Exception('Uknown transcript!'); } - $field_names = HeraldFieldConfig::getFieldMap(); - $condition_names = HeraldConditionConfig::getConditionMap(); - $action_names = HeraldActionConfig::getActionMap(); - require_celerity_resource('herald-test-css'); - $filter = $this->getFilterPHIDs(); - $this->filterTranscript($xscript, $filter); - $phids = array_merge($filter, $this->getTranscriptPHIDs($xscript)); - $phids = array_unique($phids); - $phids = array_filter($phids); - - $handles = id(new PhabricatorObjectHandleData($phids)) - ->loadHandles(); - $this->handles = $handles; - - $object_xscript = $xscript->getObjectTranscript(); - $nav = $this->buildSideNav(); - $apply_xscript_panel = $this->buildApplyTranscriptPanel( - $xscript); - $nav->appendChild($apply_xscript_panel); + $object_xscript = $xscript->getObjectTranscript(); + if (!$object_xscript) { + $notice = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setTitle('Old Transcript') + ->appendChild( + '<p>Details of this transcript have been garbage collected.</p>'); + $nav->appendChild($notice); + } else { + $filter = $this->getFilterPHIDs(); + $this->filterTranscript($xscript, $filter); + $phids = array_merge($filter, $this->getTranscriptPHIDs($xscript)); + $phids = array_unique($phids); + $phids = array_filter($phids); - $action_xscript_panel = $this->buildActionTranscriptPanel( - $xscript); - $nav->appendChild($action_xscript_panel); + $handles = id(new PhabricatorObjectHandleData($phids)) + ->loadHandles(); + $this->handles = $handles; - $object_xscript_panel = $this->buildObjectTranscriptPanel( - $xscript); - $nav->appendChild($object_xscript_panel); + $apply_xscript_panel = $this->buildApplyTranscriptPanel( + $xscript); + $nav->appendChild($apply_xscript_panel); + + $action_xscript_panel = $this->buildActionTranscriptPanel( + $xscript); + $nav->appendChild($action_xscript_panel); + + $object_xscript_panel = $this->buildObjectTranscriptPanel( + $xscript); + $nav->appendChild($object_xscript_panel); + } /* + TODO $notice = null; if ($xscript->getDryRun()) { @@ -84,30 +89,6 @@ class HeraldTranscriptController extends HeraldController { This was a dry run to test Herald rules, no actions were executed. </tools:notice>; } - - if (!$object_xscript) { - $notice = - <x:frag> - <tools:notice title="Old Transcript"> - Details of this transcript have been discarded. Full transcripts - are retained for 30 days. - </tools:notice> - {$notice} - </x:frag>; - } - - - return - <herald:standard-page title="Transcript"> - <div style="padding: 1em;"> - <tools:side-nav items={$this->renderNavItems()}> - {$notice} - {$apply_xscript_markup} - {$rule_table} - {$object_xscript_table} - </tools:side-nav> - </div> - </herald:standard-page>; */ return $this->buildStandardPageResponse( @@ -264,7 +245,7 @@ class HeraldTranscriptController extends HeraldController { foreach ($xscript->getApplyTranscripts() as $id => $apply_xscript) { $rule_id = $apply_xscript->getRuleID(); if ($filter_owned) { - if (!$rule_xscripts[$rule_id]) { + if (empty($rule_xscripts[$rule_id])) { // No associated rule so you can't own this effect. continue; } diff --git a/src/applications/herald/controller/transcript/__init__.php b/src/applications/herald/controller/transcript/__init__.php index 81bfe3385f..3208b3b1e0 100644 --- a/src/applications/herald/controller/transcript/__init__.php +++ b/src/applications/herald/controller/transcript/__init__.php @@ -14,6 +14,7 @@ phutil_require_module('phabricator', 'applications/herald/storage/transcript/bas phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'view/control/table'); +phutil_require_module('phabricator', 'view/form/error'); phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phabricator', 'view/layout/sidenav'); diff --git a/src/docs/configuration/managing_daemons.diviner b/src/docs/configuration/managing_daemons.diviner index 92c7d2ae78..fdc7dfe9ed 100644 --- a/src/docs/configuration/managing_daemons.diviner +++ b/src/docs/configuration/managing_daemons.diviner @@ -70,3 +70,4 @@ You can get a list of launchable daemons with **phd list**: - **PhabricatorTaskmasterDaemon** runs a generic task queue; and - **PhabricatorRepository** daemons track repositories, descriptions are available in the @{article:Diffusion User Guide}. + - **PhabricatorGarbageCollectorDaemon** cleans up old logs and caches. diff --git a/src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php b/src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php new file mode 100644 index 0000000000..757d6084e9 --- /dev/null +++ b/src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php @@ -0,0 +1,139 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Collects old logs and caches to reduce the amount of data stored in the + * database. + * + * @group daemon + */ +class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon { + + public function run() { + + // Keep track of when we start and stop the GC so we can emit useful log + // messages. + $just_ran = false; + + do { + $run_at = PhabricatorEnv::getEnvConfig('gcdaemon.run-at'); + $run_for = PhabricatorEnv::getEnvConfig('gcdaemon.run-for'); + + // Just use the default timezone, we don't need to get fancy and try + // to localize this. + $start = strtotime($run_at); + if ($start === false) { + throw new Exception( + "Configuration 'gcdaemon.run-at' could not be parsed: '{$run_at}'."); + } + + $now = time(); + + if ($now < $start || $now > ($start + $run_for)) { + if ($just_ran) { + echo "Stopped garbage collector.\n"; + $just_ran = false; + } + // The configuration says we can't collect garbage right now, so + // just sleep until we can. + $this->sleep(300); + continue; + } + + if (!$just_ran) { + echo "Started garbage collector.\n"; + $just_ran = true; + } + + $n_herald = $this->collectHeraldTranscripts(); + $n_daemon = $this->collectDaemonLogs(); + $n_render = $this->collectRenderCaches(); + + $collected = array( + 'Herald Transcript' => $n_herald, + 'Daemon Log' => $n_daemon, + 'Render Cache' => $n_render, + ); + $collected = array_filter($collected); + + foreach ($collected as $thing => $count) { + $count = number_format($count); + echo "Garbage collected {$count} '{$thing}' objects.\n"; + } + + $total = array_sum($collected); + if ($total < 100) { + // We didn't max out any of the GCs so we're basically caught up. Ease + // off the GC loop so we don't keep doing table scans just to delete + // a handful of rows. + $this->sleep(300); + } else { + $this->stillWorking(); + } + } while (true); + + } + + private function collectHeraldTranscripts() { + $ttl = PhabricatorEnv::getEnvConfig('gcdaemon.ttl.herald-transcripts'); + if ($ttl <= 0) { + return 0; + } + + $table = new HeraldTranscript(); + $conn_w = $table->establishConnection('w'); + + queryfx( + $conn_w, + 'UPDATE %T SET + objectTranscript = "", + ruleTranscripts = "", + conditionTranscripts = "", + applyTranscripts = "" + WHERE `time` < %d AND objectTranscript != "" + LIMIT 100', + $table->getTableName(), + time() - $ttl); + + return $conn_w->getAffectedRows(); + } + + private function collectDaemonLogs() { + $ttl = PhabricatorEnv::getEnvConfig('gcdaemon.ttl.daemon-logs'); + if ($ttl <= 0) { + return 0; + } + + $table = new PhabricatorDaemonLogEvent(); + $conn_w = $table->establishConnection('w'); + + queryfx( + $conn_w, + 'DELETE FROM %T WHERE epoch < %d LIMIT 100', + $table->getTableName(), + time() - $ttl); + + return $conn_w->getAffectedRows(); + } + + private function collectRenderCaches() { + // TODO: Implement this, no epoch column on the table right now. + return 0; + } + +} diff --git a/src/infrastructure/daemon/garbagecollector/__init__.php b/src/infrastructure/daemon/garbagecollector/__init__.php new file mode 100644 index 0000000000..f3de769a1d --- /dev/null +++ b/src/infrastructure/daemon/garbagecollector/__init__.php @@ -0,0 +1,16 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/herald/storage/transcript/base'); +phutil_require_module('phabricator', 'infrastructure/daemon/base'); +phutil_require_module('phabricator', 'infrastructure/daemon/storage/event'); +phutil_require_module('phabricator', 'infrastructure/env'); +phutil_require_module('phabricator', 'storage/queryfx'); + + +phutil_require_source('PhabricatorGarbageCollectorDaemon.php'); From b548b5166be36991e829b4e94b35431a46029840 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Tue, 5 Jul 2011 14:07:14 -0700 Subject: [PATCH 23/34] Update externals/javelin to HEAD for the JX.Vector.getPos() fix. --- externals/javelin | 2 +- src/__celerity_resource_map__.php | 26 +++++++++++++------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/externals/javelin b/externals/javelin index 081b5658d0..c6bf95cd19 160000 --- a/externals/javelin +++ b/externals/javelin @@ -1 +1 @@ -Subproject commit 081b5658d0f176fbbcd251c570cb62e7eab81af0 +Subproject commit c6bf95cd19da7179c10220a858f4662139b6c3a0 diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index b9fe71f51e..6067d6abb1 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -870,7 +870,7 @@ celerity_register_resource_map(array( ), 'javelin-vector' => array( - 'uri' => '/res/155b931a/rsrc/js/javelin/lib/Vector.js', + 'uri' => '/res/50535cb8/rsrc/js/javelin/lib/Vector.js', 'type' => 'js', 'requires' => array( @@ -1155,7 +1155,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/25f94e94/typeahead.pkg.js', 'type' => 'js', ), - '6c5acfa3' => + '307df223' => array ( 'name' => 'javelin.pkg.js', 'symbols' => @@ -1171,7 +1171,7 @@ celerity_register_resource_map(array( 8 => 'javelin-json', 9 => 'javelin-uri', ), - 'uri' => '/res/pkg/6c5acfa3/javelin.pkg.js', + 'uri' => '/res/pkg/307df223/javelin.pkg.js', 'type' => 'js', ), 55967526 => @@ -1266,7 +1266,7 @@ celerity_register_resource_map(array( 'differential-revision-history-css' => '55967526', 'differential-table-of-contents-css' => '55967526', 'diffusion-commit-view-css' => '03ef179e', - 'javelin-behavior' => '6c5acfa3', + 'javelin-behavior' => '307df223', 'javelin-behavior-aphront-basic-tokenizer' => '25f94e94', 'javelin-behavior-aphront-form-disable-on-submit' => '973a2445', 'javelin-behavior-differential-diff-radios' => 'da416e1c', @@ -1276,22 +1276,22 @@ celerity_register_resource_map(array( 'javelin-behavior-differential-show-more' => 'da416e1c', 'javelin-behavior-phabricator-keyboard-shortcuts' => '973a2445', 'javelin-behavior-workflow' => '973a2445', - 'javelin-dom' => '6c5acfa3', - 'javelin-event' => '6c5acfa3', - 'javelin-install' => '6c5acfa3', - 'javelin-json' => '6c5acfa3', + 'javelin-dom' => '307df223', + 'javelin-event' => '307df223', + 'javelin-install' => '307df223', + 'javelin-json' => '307df223', 'javelin-mask' => '973a2445', - 'javelin-request' => '6c5acfa3', - 'javelin-stratcom' => '6c5acfa3', + 'javelin-request' => '307df223', + 'javelin-stratcom' => '307df223', 'javelin-tokenizer' => '25f94e94', 'javelin-typeahead' => '25f94e94', 'javelin-typeahead-normalizer' => '25f94e94', 'javelin-typeahead-ondemand-source' => '25f94e94', 'javelin-typeahead-preloaded-source' => '25f94e94', 'javelin-typeahead-source' => '25f94e94', - 'javelin-uri' => '6c5acfa3', - 'javelin-util' => '6c5acfa3', - 'javelin-vector' => '6c5acfa3', + 'javelin-uri' => '307df223', + 'javelin-util' => '307df223', + 'javelin-vector' => '307df223', 'javelin-workflow' => '973a2445', 'phabricator-core-buttons-css' => '8e9024dc', 'phabricator-core-css' => '8e9024dc', From 652baee54c89a9694feb987ca9443366e7479ce0 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Tue, 5 Jul 2011 14:17:38 -0700 Subject: [PATCH 24/34] D510 renamed this method to stringify. --- webroot/rsrc/js/application/core/behavior-keyboard-shortcuts.js | 2 +- webroot/rsrc/js/application/herald/HeraldRuleEditor.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/webroot/rsrc/js/application/core/behavior-keyboard-shortcuts.js b/webroot/rsrc/js/application/core/behavior-keyboard-shortcuts.js index 98a79d88f1..f01a382933 100644 --- a/webroot/rsrc/js/application/core/behavior-keyboard-shortcuts.js +++ b/webroot/rsrc/js/application/core/behavior-keyboard-shortcuts.js @@ -20,7 +20,7 @@ JX.behavior('phabricator-keyboard-shortcuts', function(config) { return; } var desc = manager.getShortcutDescriptions(); - var data = {keys : JX.JSON.serialize(desc)}; + var data = {keys : JX.JSON.stringify(desc)}; workflow = new JX.Workflow(config.helpURI, data) .setCloseHandler(function() { workflow = null; diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js index 5f14d0748e..c058352310 100644 --- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js +++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js @@ -115,7 +115,7 @@ JX.install('HeraldRuleEditor', { this._config.actions[k][1] = this._getActionTarget(k); } - rule.value = JX.JSON.serialize({ + rule.value = JX.JSON.stringify({ conditions: this._config.conditions, actions: this._config.actions }); From cdd25f427a15098d225f73ef9f931d971197d64c Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Tue, 5 Jul 2011 14:30:20 -0700 Subject: [PATCH 25/34] Update celerity map. --- src/__celerity_resource_map__.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 6067d6abb1..74fb164df7 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -258,7 +258,7 @@ celerity_register_resource_map(array( ), 'herald-rule-editor' => array( - 'uri' => '/res/402e94d2/rsrc/js/application/herald/HeraldRuleEditor.js', + 'uri' => '/res/ba957508/rsrc/js/application/herald/HeraldRuleEditor.js', 'type' => 'js', 'requires' => array( @@ -614,7 +614,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-phabricator-keyboard-shortcuts' => array( - 'uri' => '/res/7aed0604/rsrc/js/application/core/behavior-keyboard-shortcuts.js', + 'uri' => '/res/94b009e2/rsrc/js/application/core/behavior-keyboard-shortcuts.js', 'type' => 'js', 'requires' => array( @@ -1215,7 +1215,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/8e9024dc/core.pkg.css', 'type' => 'css', ), - '973a2445' => + 'd0713563' => array ( 'name' => 'workflow.pkg.js', 'symbols' => @@ -1228,7 +1228,7 @@ celerity_register_resource_map(array( 5 => 'phabricator-keyboard-shortcut', 6 => 'javelin-behavior-phabricator-keyboard-shortcuts', ), - 'uri' => '/res/pkg/973a2445/workflow.pkg.js', + 'uri' => '/res/pkg/d0713563/workflow.pkg.js', 'type' => 'js', ), 'da416e1c' => @@ -1268,19 +1268,19 @@ celerity_register_resource_map(array( 'diffusion-commit-view-css' => '03ef179e', 'javelin-behavior' => '307df223', 'javelin-behavior-aphront-basic-tokenizer' => '25f94e94', - 'javelin-behavior-aphront-form-disable-on-submit' => '973a2445', + 'javelin-behavior-aphront-form-disable-on-submit' => 'd0713563', 'javelin-behavior-differential-diff-radios' => 'da416e1c', 'javelin-behavior-differential-edit-inline-comments' => 'da416e1c', 'javelin-behavior-differential-feedback-preview' => 'da416e1c', 'javelin-behavior-differential-populate' => 'da416e1c', 'javelin-behavior-differential-show-more' => 'da416e1c', - 'javelin-behavior-phabricator-keyboard-shortcuts' => '973a2445', - 'javelin-behavior-workflow' => '973a2445', + 'javelin-behavior-phabricator-keyboard-shortcuts' => 'd0713563', + 'javelin-behavior-workflow' => 'd0713563', 'javelin-dom' => '307df223', 'javelin-event' => '307df223', 'javelin-install' => '307df223', 'javelin-json' => '307df223', - 'javelin-mask' => '973a2445', + 'javelin-mask' => 'd0713563', 'javelin-request' => '307df223', 'javelin-stratcom' => '307df223', 'javelin-tokenizer' => '25f94e94', @@ -1292,12 +1292,12 @@ celerity_register_resource_map(array( 'javelin-uri' => '307df223', 'javelin-util' => '307df223', 'javelin-vector' => '307df223', - 'javelin-workflow' => '973a2445', + 'javelin-workflow' => 'd0713563', 'phabricator-core-buttons-css' => '8e9024dc', 'phabricator-core-css' => '8e9024dc', 'phabricator-directory-css' => '8e9024dc', - 'phabricator-keyboard-shortcut' => '973a2445', - 'phabricator-keyboard-shortcut-manager' => '973a2445', + 'phabricator-keyboard-shortcut' => 'd0713563', + 'phabricator-keyboard-shortcut-manager' => 'd0713563', 'phabricator-remarkup-css' => '8e9024dc', 'phabricator-standard-page-view' => '8e9024dc', 'syntax-highlighting-css' => '8e9024dc', From f9599f44997bbd520e80229780fef1ff997b333c Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Mon, 4 Jul 2011 09:45:42 -0700 Subject: [PATCH 26/34] Allow configuration of a task-creation email address Summary: This lets you configure an email address which will create tasks when emails are sent to it. It's pretty basic but should get us most of the way there. Test Plan: Configured an address and created a task via email. Replied to a task via email to check that I didn't break that. Reviewed By: tuomaspelkonen Reviewers: davidreuss, jungejason, tuomaspelkonen, aran CC: aran, epriestley, tuomaspelkonen Differential Revision: 590 --- conf/default.conf.php | 10 ++ .../taskedit/ManiphestTaskEditController.php | 9 ++ .../controller/taskedit/__init__.php | 1 + .../replyhandler/ManiphestReplyHandler.php | 109 +++++++++++------- .../PhabricatorMetaMTAReceivedMail.php | 77 ++++++++++--- .../metamta/storage/receivedmail/__init__.php | 2 + .../configuring_inbound_email.diviner | 11 +- 7 files changed, 160 insertions(+), 59 deletions(-) diff --git a/conf/default.conf.php b/conf/default.conf.php index 7472c1ecb0..00adb39311 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -201,6 +201,16 @@ return array( // single public email address, so objects can not be replied to blindly. 'metamta.public-replies' => false, + // You can configure an email address like "bugs@phabricator.example.com" + // which will automatically create Maniphest tasks when users send email + // to it. This relies on the "From" address to authenticate users, so it is + // is not completely secure. To set this up, enter a complete email + // address like "bugs@phabricator.example.com" and then configure mail to + // that address so it routed to Phabricator (if you've already configured + // reply handlers, you're probably already done). See "Configuring Inbound + // Email" in the documentation for more information. + 'metamta.maniphest.public-create-email' => null, + // -- Auth ------------------------------------------------------------------ // diff --git a/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php b/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php index 9f5f278935..e529d4f60b 100644 --- a/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php @@ -285,11 +285,20 @@ class ManiphestTaskEditController extends ManiphestController { } } + $email_create = PhabricatorEnv::getEnvConfig( + 'metamta.maniphest.public-create-email'); + $email_hint = null; + if (!$task->getID() && $email_create) { + $email_hint = 'You can also create tasks by sending an email to: '. + '<tt>'.phutil_escape_html($email_create).'</tt>'; + } + $form ->appendChild( id(new AphrontFormTextAreaControl()) ->setLabel('Description') ->setName('description') + ->setCaption($email_hint) ->setValue($task->getDescription())) ->appendChild( id(new AphrontFormSubmitControl()) diff --git a/src/applications/maniphest/controller/taskedit/__init__.php b/src/applications/maniphest/controller/taskedit/__init__.php index d524917ca9..a12c754262 100644 --- a/src/applications/maniphest/controller/taskedit/__init__.php +++ b/src/applications/maniphest/controller/taskedit/__init__.php @@ -19,6 +19,7 @@ phutil_require_module('phabricator', 'applications/maniphest/storage/transaction phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); +phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/form/base'); diff --git a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php b/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php index 0e0297c197..374da6c953 100644 --- a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php +++ b/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php @@ -52,31 +52,85 @@ class ManiphestReplyHandler extends PhabricatorMailReplyHandler { public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { + // NOTE: We'll drop in here on both the "reply to a task" and "create a + // new task" workflows! Make sure you test both if you make changes! + $task = $this->getMailReceiver(); + + $is_new_task = !$task->getID(); + $user = $this->getActor(); $body = $mail->getCleanTextBody(); $body = trim($body); - $lines = explode("\n", trim($body)); - $first_line = head($lines); + $xactions = array(); - $command = null; - $matches = null; - if (preg_match('/^!(\w+)/', $first_line, $matches)) { - $lines = array_slice($lines, 1); - $body = implode("\n", $lines); - $body = trim($body); + $template = new ManiphestTransaction(); + $template->setAuthorPHID($user->getPHID()); - $command = $matches[1]; + if ($is_new_task) { + // If this is a new task, create a "User created this task." transaction + // and then set the title and description. + $xaction = clone $template; + $xaction->setTransactionType(ManiphestTransactionType::TYPE_STATUS); + $xaction->setNewValue(ManiphestTaskStatus::STATUS_OPEN); + $xactions[] = $xaction; + + $task->setAuthorPHID($user->getPHID()); + $task->setTitle(nonempty($mail->getSubject(), 'Untitled Task')); + $task->setDescription($body); + + } else { + $lines = explode("\n", trim($body)); + $first_line = head($lines); + + $command = null; + $matches = null; + if (preg_match('/^!(\w+)/', $first_line, $matches)) { + $lines = array_slice($lines, 1); + $body = implode("\n", $lines); + $body = trim($body); + + $command = $matches[1]; + } + + $ttype = ManiphestTransactionType::TYPE_NONE; + $new_value = null; + switch ($command) { + case 'close': + $ttype = ManiphestTransactionType::TYPE_STATUS; + $new_value = ManiphestTaskStatus::STATUS_CLOSED_RESOLVED; + break; + case 'claim': + $ttype = ManiphestTransactionType::TYPE_OWNER; + $new_value = $user->getPHID(); + break; + case 'unsubscribe': + $ttype = ManiphestTransactionType::TYPE_CCS; + $ccs = $task->getCCPHIDs(); + foreach ($ccs as $k => $phid) { + if ($phid == $user->getPHID()) { + unset($ccs[$k]); + } + } + $new_value = array_values($ccs); + break; + } + + $xaction = clone $template; + $xaction->setTransactionType($ttype); + $xaction->setNewValue($new_value); + $xaction->setComments($body); + + $xactions[] = $xaction; } - $xactions = array(); + // TODO: We should look at CCs on the mail and add them as CCs. $files = $mail->getAttachments(); if ($files) { - $file_xaction = new ManiphestTransaction(); - $file_xaction->setAuthorPHID($user->getPHID()); + $file_xaction = clone $template; $file_xaction->setTransactionType(ManiphestTransactionType::TYPE_ATTACH); $phid_type = PhabricatorPHIDConstants::PHID_TYPE_FILE; @@ -89,37 +143,6 @@ class ManiphestReplyHandler extends PhabricatorMailReplyHandler { $xactions[] = $file_xaction; } - $ttype = ManiphestTransactionType::TYPE_NONE; - $new_value = null; - switch ($command) { - case 'close': - $ttype = ManiphestTransactionType::TYPE_STATUS; - $new_value = ManiphestTaskStatus::STATUS_CLOSED_RESOLVED; - break; - case 'claim': - $ttype = ManiphestTransactionType::TYPE_OWNER; - $new_value = $user->getPHID(); - break; - case 'unsubscribe': - $ttype = ManiphestTransactionType::TYPE_CCS; - $ccs = $task->getCCPHIDs(); - foreach ($ccs as $k => $phid) { - if ($phid == $user->getPHID()) { - unset($ccs[$k]); - } - } - $new_value = array_values($ccs); - break; - } - - $xaction = new ManiphestTransaction(); - $xaction->setAuthorPHID($user->getPHID()); - $xaction->setTransactionType($ttype); - $xaction->setNewValue($new_value); - $xaction->setComments($body); - - $xactions[] = $xaction; - $editor = new ManiphestTransactionEditor(); $editor->setParentMessageID($mail->getMessageID()); $editor->applyTransactions($task, $xactions); diff --git a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php index 1ddc36c6e5..bbeca66523 100644 --- a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php @@ -50,15 +50,50 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return idx($this->headers, 'message-id'); } + public function getSubject() { + return idx($this->headers, 'subject'); + } + public function processReceivedMail() { $to = idx($this->headers, 'to'); + $to = $this->getRawEmailAddress($to); - // Accept a match either at the beginning of the address or after an open - // angle bracket, as in: - // "some display name" <D1+xyz+asdf@example.com> + $from = idx($this->headers, 'from'); + + $create_task = PhabricatorEnv::getEnvConfig( + 'metamta.maniphest.public-create-email'); + + if ($create_task && $to == $create_task) { + $user = $this->lookupPublicUser(); + if (!$user) { + // TODO: We should probably bounce these since from the user's + // perspective their email vanishes into a black hole. + return $this->setMessage("Invalid public user '{$from}'.")->save(); + } + + $this->setAuthorPHID($user->getPHID()); + + $receiver = new ManiphestTask(); + $receiver->setAuthorPHID($user->getPHID()); + $receiver->setPriority(ManiphestTaskPriority::PRIORITY_TRIAGE); + + $editor = new ManiphestTransactionEditor(); + $handler = $editor->buildReplyHandler($receiver); + + $handler->setActor($user); + $handler->receiveEmail($this); + + $this->setRelatedPHID($receiver->getPHID()); + $this->setMessage('OK'); + + return $this->save(); + } + + // We've already stripped this, so look for an object address which has + // a format like: D291+291+b0a41ca848d66dcc@example.com $matches = null; $ok = preg_match( - '/(?:^|<)((?:D|T)\d+)\+([\w]+)\+([a-f0-9]{16})@/U', + '/^((?:D|T)\d+)\+([\w]+)\+([a-f0-9]{16})@/U', $to, $matches); @@ -75,18 +110,8 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return $this->setMessage("Public replies not enabled.")->save(); } - // Strip the email address out of the 'from' if necessary, since it might - // have some formatting like '"Abraham Lincoln" <alincoln@logcab.in>'. - $from = idx($this->headers, 'from'); - $matches = null; - $ok = preg_match('/<(.*)>/', $from, $matches); - if ($ok) { - $from = $matches[1]; - } + $user = $this->lookupPublicUser(); - $user = id(new PhabricatorUser())->loadOneWhere( - 'email = %s', - $from); if (!$user) { return $this->setMessage("Invalid public user '{$from}'.")->save(); } @@ -181,5 +206,27 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return substr($hash, 0, 16); } + /** + * Strip an email address down to the actual user@domain.tld part if + * necessary, since sometimes it will have formatting like + * '"Abraham Lincoln" <alincoln@logcab.in>'. + */ + private function getRawEmailAddress($address) { + $matches = null; + $ok = preg_match('/<(.*)>/', $address, $matches); + if ($ok) { + $address = $matches[1]; + } + return $address; + } + + private function lookupPublicUser() { + $from = idx($this->headers, 'from'); + $from = $this->getRawEmailAddress($from); + + return id(new PhabricatorUser())->loadOneWhere( + 'email = %s', + $from); + } } diff --git a/src/applications/metamta/storage/receivedmail/__init__.php b/src/applications/metamta/storage/receivedmail/__init__.php index afd38aa144..6254939d3f 100644 --- a/src/applications/metamta/storage/receivedmail/__init__.php +++ b/src/applications/metamta/storage/receivedmail/__init__.php @@ -7,7 +7,9 @@ phutil_require_module('phabricator', 'applications/differential/mail/base'); +phutil_require_module('phabricator', 'applications/maniphest/constants/priority'); phutil_require_module('phabricator', 'applications/maniphest/editor/transaction'); +phutil_require_module('phabricator', 'applications/maniphest/storage/task'); phutil_require_module('phabricator', 'applications/metamta/parser'); phutil_require_module('phabricator', 'applications/metamta/storage/base'); phutil_require_module('phabricator', 'applications/people/storage/user'); diff --git a/src/docs/configuration/configuring_inbound_email.diviner b/src/docs/configuration/configuring_inbound_email.diviner index 981b6a9512..e19f8885c9 100644 --- a/src/docs/configuration/configuring_inbound_email.diviner +++ b/src/docs/configuration/configuring_inbound_email.diviner @@ -2,7 +2,8 @@ @group config This document contains instructions for configuring inbound email, so users -may update Differential and Maniphest by replying to messages. +may update Differential and Maniphest by replying to messages and create +Maniphest tasks via email. = Preamble = @@ -42,6 +43,11 @@ configured correctly, according to the instructions below -- will parse incoming email and allow users to interact with Maniphest tasks and Differential revisions over email. +You can also set up a task creation email address, like ##bugs@example.com##, +which will create a Maniphest task out of any email which is set to it. To do +this, set ##metamta.maniphest.public-create-email## in your configuration. This +has some mild security implications, see below. + = Security = The email reply channel is "somewhat" authenticated. Each reply-to address is @@ -74,6 +80,9 @@ practically, is a reasonable setting for many installs. The reply-to address will still contain a hash unique to the object it represents, so users who have not received an email about an object can not blindly interact with it. +If you enable ##metamta.maniphest.public-create-email##, that address also uses +the weaker "From" authentication mechanism. + NOTE: Phabricator does not currently attempt to verify "From" addresses because this is technically complex, seems unreasonably difficult in the general case, and no installs have had a need for it yet. If you have a specific case where a From f123abcc5a89eefa7f4e2b98b6320a6daaaf2c61 Mon Sep 17 00:00:00 2001 From: tuomaspelkonen <tuomaspelkonen@fb.com> Date: Wed, 6 Jul 2011 08:42:11 -0700 Subject: [PATCH 27/34] Prevent using deleted repositories when displaying commits for a revision. Summary: We deleted a repository (I don't remember which one). Some differential revisions where committed to this repository, and opening these revisions show a blank page, because the repository_id is not in the database anymore. This causes the 'ERROR 8: Undefined index: 12 at [phabricator/src/applications/phid/handle/data/PhabricatorObjectHandleData.php:209] ' in our log. Test Plan: Opened a revision which where committed to multiple repositories including the deleted one. Made sure that the page was rendered correctly and there were no messages in the error log. Reviewed By: epriestley Reviewers: epriestley, jungejason CC: aran, epriestley Differential Revision: 598 --- .../phid/handle/data/PhabricatorObjectHandleData.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php index 06b99cccb3..f4fb0059d2 100644 --- a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php @@ -202,7 +202,8 @@ class PhabricatorObjectHandleData { $handle = new PhabricatorObjectHandle(); $handle->setPHID($phid); $handle->setType($type); - if (empty($commits[$phid])) { + if (empty($commits[$phid]) || + !isset($callsigns[$repository_ids[$phid]])) { $handle->setName('Unknown Commit'); } else { $commit = $commits[$phid]; From 70cd8b1b346f74c20f662a8ff83c1726bc7c7845 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Wed, 6 Jul 2011 11:10:40 -0700 Subject: [PATCH 28/34] Update Phabricator for split syntax highlighting API Summary: I'll clean some of this stuff up in a followup too, but update the callers to use the new explicit filename-based API. Test Plan: Looked at paste, Diffusion and Differential. Reviewed By: tuomaspelkonen Reviewers: tuomaspelkonen, codeblock, jungejason, aran CC: aran, tuomaspelkonen Differential Revision: 600 --- .../changeset/DifferentialChangesetParser.php | 8 ++------ .../file/DiffusionBrowseFileController.php | 11 +++++++---- .../view/PhabricatorPasteViewController.php | 16 ++++++++++++---- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php index 6a15fb9a52..edc6153d08 100644 --- a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php @@ -27,7 +27,6 @@ class DifferentialChangesetParser { protected $parsedHunk = false; protected $filename = null; - protected $filetype = null; protected $missingOld = array(); protected $missingNew = array(); @@ -161,10 +160,7 @@ class DifferentialChangesetParser { public function setFilename($filename) { $this->filename = $filename; - if (strpos($filename, '.', 1) !== false) { - $parts = explode('.', $filename); - $this->filetype = end($parts); - } + return $this; } public function setHandles(array $handles) { @@ -723,7 +719,7 @@ class DifferentialChangesetParser { protected function getHighlightFuture($corpus) { return $this->highlightEngine->getHighlightFuture( - $this->filetype, + $this->highlightEngine->getLanguageFromFilename($this->filename), $corpus); } diff --git a/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php index c0b3402547..3bf7e7ee3b 100644 --- a/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php @@ -179,13 +179,16 @@ class DiffusionBrowseFileController extends DiffusionController { list($text_list, $rev_list, $blame_dict) = $file_query->getBlameData(); - $highlightEngine = new PhutilDefaultSyntaxHighlighterEngine(); - $highlightEngine->setConfig( + $highlight_engine = new PhutilDefaultSyntaxHighlighterEngine(); + $highlight_engine->setConfig( 'pygments.enabled', PhabricatorEnv::getEnvConfig('pygments.enabled')); - $text_list = explode("\n", $highlightEngine->highlightSource($path, - implode("\n", $text_list))); + $text_list = explode( + "\n", + $highlight_engine->highlightSource( + $highlight_engine->getLanguageFromFilename($path), + implode("\n", $text_list))); $rows = $this->buildDisplayRows($text_list, $rev_list, $blame_dict, $needs_blame, $drequest, $file_query, $selected); diff --git a/src/applications/paste/controller/view/PhabricatorPasteViewController.php b/src/applications/paste/controller/view/PhabricatorPasteViewController.php index 776eacea5f..2809395853 100644 --- a/src/applications/paste/controller/view/PhabricatorPasteViewController.php +++ b/src/applications/paste/controller/view/PhabricatorPasteViewController.php @@ -89,14 +89,22 @@ class PhabricatorPasteViewController extends PhabricatorPasteController { require_celerity_resource('diffusion-source-css'); require_celerity_resource('syntax-highlighting-css'); - $highlightEngine = new PhutilDefaultSyntaxHighlighterEngine(); - $highlightEngine->setConfig( + $highlight_engine = new PhutilDefaultSyntaxHighlighterEngine(); + $highlight_engine->setConfig( 'pygments.enabled', PhabricatorEnv::getEnvConfig('pygments.enabled')); + + $language = $paste->getLanguage(); + if (empty($language)) { + $language = $highlight_engine->getLanguageFromFilename( + $paste->getTitle()); + } + $text_list = explode( - "\n", $highlightEngine->highlightSource( - nonempty($paste->getLanguage(), $paste->getTitle()), + "\n", + $highlight_engine->highlightSource( + $language, $file->loadFileData())); $rows = $this->buildDisplayRows($text_list); From 85b34c23f9e9da92277f99d6a1904671da8607fb Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Wed, 6 Jul 2011 12:12:17 -0700 Subject: [PATCH 29/34] Clean up Phabricator interface to syntax highlighting Summary: Reduce the amount of code duplication here and allow for an override configuration on the filename.map stuff. Test Plan: Checked paste, diffusion and differential syntax highlighting and everything appeared reasonable. Reviewed By: codeblock Reviewers: tuomaspelkonen, codeblock, jungejason, aran CC: aran, codeblock, epriestley Differential Revision: 601 --- conf/default.conf.php | 19 +++++++ src/__phutil_library_map__.php | 1 + .../changeset/DifferentialChangesetParser.php | 5 +- .../parser/changeset/__init__.php | 3 +- .../file/DiffusionBrowseFileController.php | 15 ++---- .../diffusion/controller/file/__init__.php | 3 +- .../syntax/PhabricatorSyntaxHighlighter.php | 51 +++++++++++++++++++ src/applications/markup/syntax/__init__.php | 14 +++++ .../view/PhabricatorPasteViewController.php | 22 ++++---- .../paste/controller/view/__init__.php | 3 +- 10 files changed, 103 insertions(+), 33 deletions(-) create mode 100644 src/applications/markup/syntax/PhabricatorSyntaxHighlighter.php create mode 100644 src/applications/markup/syntax/__init__.php diff --git a/conf/default.conf.php b/conf/default.conf.php index 00adb39311..ee80b79c30 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -486,4 +486,23 @@ return array( 'pygments.dropdown-default' => 'infer', + // This is an override list of regular expressions which allows you to choose + // what language files are highlighted as. If your projects have certain rules + // about filenames or use unusual or ambiguous language extensions, you can + // create a mapping here. This is an ordered dictionary of regular expressions + // which will be tested against the filename. They should map to either an + // explicit language as a string value, or a numeric index into the captured + // groups as an integer. + 'syntax.filemap' => array( + // Example: Treat all '*.xyz' files as PHP. + // '@\\.xyz$@' => 'php', + + // Example: Treat 'httpd.conf' as 'apacheconf'. + // '@/httpd\\.conf$@' => 'apacheconf', + + // Example: Treat all '*.x.bak' file as '.x'. NOTE: we map to capturing + // group 1 by specifying the mapping as "1". + // '@\\.([^.]+)\\.bak$@' => 1, + ), + ); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f38452557b..f2019e64c1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -511,6 +511,7 @@ phutil_register_library_map(array( 'PhabricatorSetup' => 'infrastructure/setup', 'PhabricatorStandardPageView' => 'view/page/standard', 'PhabricatorStatusController' => 'applications/status/base', + 'PhabricatorSyntaxHighlighter' => 'applications/markup/syntax', 'PhabricatorTaskmasterDaemon' => 'infrastructure/daemon/workers/taskmaster', 'PhabricatorTestCase' => 'infrastructure/testing/testcase', 'PhabricatorTimelineCursor' => 'infrastructure/daemon/timeline/storage/cursor', diff --git a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php index edc6153d08..c43aaf9ec7 100644 --- a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php @@ -873,10 +873,7 @@ EOSYNTHETIC; $this->isTopLevel = (($range_start === null) && ($range_len === null)); - $this->highlightEngine = new PhutilDefaultSyntaxHighlighterEngine(); - $this->highlightEngine->setConfig( - 'pygments.enabled', - PhabricatorEnv::getEnvConfig('pygments.enabled')); + $this->highlightEngine = PhabricatorSyntaxHighlighter::newEngine(); $this->tryCacheStuff(); diff --git a/src/applications/differential/parser/changeset/__init__.php b/src/applications/differential/parser/changeset/__init__.php index 34285ac975..7ca3982ed4 100644 --- a/src/applications/differential/parser/changeset/__init__.php +++ b/src/applications/differential/parser/changeset/__init__.php @@ -14,7 +14,7 @@ phutil_require_module('phabricator', 'applications/differential/storage/changese phutil_require_module('phabricator', 'applications/differential/storage/diff'); phutil_require_module('phabricator', 'applications/differential/view/inlinecomment'); phutil_require_module('phabricator', 'applications/files/uri'); -phutil_require_module('phabricator', 'infrastructure/env'); +phutil_require_module('phabricator', 'applications/markup/syntax'); phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'storage/queryfx'); @@ -24,7 +24,6 @@ phutil_require_module('phutil', 'filesystem/tempfile'); phutil_require_module('phutil', 'future'); phutil_require_module('phutil', 'future/exec'); phutil_require_module('phutil', 'markup'); -phutil_require_module('phutil', 'markup/syntax/engine/default'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php index 3bf7e7ee3b..eac0ce27ff 100644 --- a/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php @@ -179,16 +179,11 @@ class DiffusionBrowseFileController extends DiffusionController { list($text_list, $rev_list, $blame_dict) = $file_query->getBlameData(); - $highlight_engine = new PhutilDefaultSyntaxHighlighterEngine(); - $highlight_engine->setConfig( - 'pygments.enabled', - PhabricatorEnv::getEnvConfig('pygments.enabled')); - - $text_list = explode( - "\n", - $highlight_engine->highlightSource( - $highlight_engine->getLanguageFromFilename($path), - implode("\n", $text_list))); + $text_list = implode("\n", $text_list); + $text_list = PhabricatorSyntaxHighlighter::highlightWithFilename( + $path, + $text_list); + $text_list = explode("\n", $text_list); $rows = $this->buildDisplayRows($text_list, $rev_list, $blame_dict, $needs_blame, $drequest, $file_query, $selected); diff --git a/src/applications/diffusion/controller/file/__init__.php b/src/applications/diffusion/controller/file/__init__.php index 8b61c73968..7bba4cebcd 100644 --- a/src/applications/diffusion/controller/file/__init__.php +++ b/src/applications/diffusion/controller/file/__init__.php @@ -8,13 +8,12 @@ phutil_require_module('phabricator', 'applications/diffusion/controller/base'); phutil_require_module('phabricator', 'applications/diffusion/query/filecontent/base'); +phutil_require_module('phabricator', 'applications/markup/syntax'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); -phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phutil', 'markup'); -phutil_require_module('phutil', 'markup/syntax/engine/default'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/markup/syntax/PhabricatorSyntaxHighlighter.php b/src/applications/markup/syntax/PhabricatorSyntaxHighlighter.php new file mode 100644 index 0000000000..52597ec333 --- /dev/null +++ b/src/applications/markup/syntax/PhabricatorSyntaxHighlighter.php @@ -0,0 +1,51 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @group markup + */ +final class PhabricatorSyntaxHighlighter { + + public static function newEngine() { + $engine = new PhutilDefaultSyntaxHighlighterEngine(); + + $config = array( + 'pygments.enabled' => PhabricatorEnv::getEnvConfig('pygments.enabled'), + 'filename.map' => PhabricatorEnv::getEnvConfig('syntax.filemap'), + ); + + foreach ($config as $key => $value) { + $engine->setConfig($key, $value); + } + + return $engine; + } + + public static function highlightWithFilename($filename, $source) { + $engine = self::newEngine(); + $language = $engine->getLanguageFromFilename($filename); + return $engine->getHighlightFuture($language, $source)->resolve(); + } + + public static function highlightWithLanguage($language, $source) { + $engine = self::newEngine(); + return $engine->getHighlightFuture($language, $source)->resolve(); + } + + +} diff --git a/src/applications/markup/syntax/__init__.php b/src/applications/markup/syntax/__init__.php new file mode 100644 index 0000000000..230b375ab6 --- /dev/null +++ b/src/applications/markup/syntax/__init__.php @@ -0,0 +1,14 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'infrastructure/env'); + +phutil_require_module('phutil', 'markup/syntax/engine/default'); + + +phutil_require_source('PhabricatorSyntaxHighlighter.php'); diff --git a/src/applications/paste/controller/view/PhabricatorPasteViewController.php b/src/applications/paste/controller/view/PhabricatorPasteViewController.php index 2809395853..90816f9de7 100644 --- a/src/applications/paste/controller/view/PhabricatorPasteViewController.php +++ b/src/applications/paste/controller/view/PhabricatorPasteViewController.php @@ -89,23 +89,19 @@ class PhabricatorPasteViewController extends PhabricatorPasteController { require_celerity_resource('diffusion-source-css'); require_celerity_resource('syntax-highlighting-css'); - $highlight_engine = new PhutilDefaultSyntaxHighlighterEngine(); - $highlight_engine->setConfig( - 'pygments.enabled', - PhabricatorEnv::getEnvConfig('pygments.enabled')); - - $language = $paste->getLanguage(); + $source = $file->loadFileData(); if (empty($language)) { - $language = $highlight_engine->getLanguageFromFilename( - $paste->getTitle()); + $source = PhabricatorSyntaxHighlighter::highlightWithFilename( + $paste->getTitle(), + $source); + } else { + $source = PhabricatorSyntaxHighlighter::highlightWithLanguage( + $language, + $source); } - $text_list = explode( - "\n", - $highlight_engine->highlightSource( - $language, - $file->loadFileData())); + $text_list = explode("\n", $source); $rows = $this->buildDisplayRows($text_list); diff --git a/src/applications/paste/controller/view/__init__.php b/src/applications/paste/controller/view/__init__.php index fc6f4b8546..527fc99e51 100644 --- a/src/applications/paste/controller/view/__init__.php +++ b/src/applications/paste/controller/view/__init__.php @@ -10,14 +10,13 @@ phutil_require_module('phabricator', 'aphront/response/400'); phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'applications/files/storage/file'); phutil_require_module('phabricator', 'applications/files/uri'); +phutil_require_module('phabricator', 'applications/markup/syntax'); phutil_require_module('phabricator', 'applications/paste/controller/base'); phutil_require_module('phabricator', 'applications/paste/storage/paste'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); -phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phutil', 'markup'); -phutil_require_module('phutil', 'markup/syntax/engine/default'); phutil_require_module('phutil', 'utils'); From ece9d792b22368dbf132eb14a66077ecf38dcf36 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Tue, 5 Jul 2011 08:35:18 -0700 Subject: [PATCH 30/34] Build basic infrastructure for an activity feed Summary: This defines an extremely basic version of an activity feed, like Facebook's news feed. It doesn't do much of interest yet. Test Plan: Published some feed stories: https://secure.phabricator.com/file/view/PHID-FILE-5061aa72105bbdc05b21/ Reviewed By: tuomaspelkonen Reviewers: jungejason, tuomaspelkonen, aran Commenters: codeblock, jungejason CC: aran, epriestley, codeblock, tuomaspelkonen, jungejason Differential Revision: 593 --- resources/sql/patches/053.feed.sql | 21 ++++ src/__phutil_library_map__.php | 21 ++++ ...AphrontDefaultApplicationConfiguration.php | 4 + .../ConduitAPI_feed_publish_Method.php | 65 ++++++++++ .../conduit/method/feed/publish/__init__.php | 13 ++ .../base/PhabricatorFeedController.php | 34 ++++++ .../feed/controller/base/__init__.php | 15 +++ .../PhabricatorFeedStreamController.php | 37 ++++++ .../feed/controller/stream/__init__.php | 13 ++ .../PhabricatorFeedStoryPublisher.php | 113 ++++++++++++++++++ src/applications/feed/publisher/__init__.php | 18 +++ .../feed/query/PhabricatorFeedQuery.php | 95 +++++++++++++++ src/applications/feed/query/__init__.php | 17 +++ .../feed/storage/base/PhabricatorFeedDAO.php | 25 ++++ .../feed/storage/base/__init__.php | 12 ++ .../story/PhabricatorFeedStoryData.php | 42 +++++++ .../feed/storage/story/__init__.php | 14 +++ .../PhabricatorFeedStoryReference.php | 31 +++++ .../feed/storage/storyreference/__init__.php | 12 ++ .../feed/story/base/PhabricatorFeedStory.php | 29 +++++ src/applications/feed/story/base/__init__.php | 10 ++ .../unknown/PhabricatorFeedStoryUnknown.php | 25 ++++ .../feed/story/unknown/__init__.php | 13 ++ .../feed/view/base/PhabricatorFeedView.php | 21 ++++ src/applications/feed/view/base/__init__.php | 12 ++ .../view/story/PhabricatorFeedStoryView.php | 32 +++++ src/applications/feed/view/story/__init__.php | 14 +++ .../constants/PhabricatorPHIDConstants.php | 3 + 28 files changed, 761 insertions(+) create mode 100644 resources/sql/patches/053.feed.sql create mode 100644 src/applications/conduit/method/feed/publish/ConduitAPI_feed_publish_Method.php create mode 100644 src/applications/conduit/method/feed/publish/__init__.php create mode 100644 src/applications/feed/controller/base/PhabricatorFeedController.php create mode 100644 src/applications/feed/controller/base/__init__.php create mode 100644 src/applications/feed/controller/stream/PhabricatorFeedStreamController.php create mode 100644 src/applications/feed/controller/stream/__init__.php create mode 100644 src/applications/feed/publisher/PhabricatorFeedStoryPublisher.php create mode 100644 src/applications/feed/publisher/__init__.php create mode 100644 src/applications/feed/query/PhabricatorFeedQuery.php create mode 100644 src/applications/feed/query/__init__.php create mode 100644 src/applications/feed/storage/base/PhabricatorFeedDAO.php create mode 100644 src/applications/feed/storage/base/__init__.php create mode 100644 src/applications/feed/storage/story/PhabricatorFeedStoryData.php create mode 100644 src/applications/feed/storage/story/__init__.php create mode 100644 src/applications/feed/storage/storyreference/PhabricatorFeedStoryReference.php create mode 100644 src/applications/feed/storage/storyreference/__init__.php create mode 100644 src/applications/feed/story/base/PhabricatorFeedStory.php create mode 100644 src/applications/feed/story/base/__init__.php create mode 100644 src/applications/feed/story/unknown/PhabricatorFeedStoryUnknown.php create mode 100644 src/applications/feed/story/unknown/__init__.php create mode 100644 src/applications/feed/view/base/PhabricatorFeedView.php create mode 100644 src/applications/feed/view/base/__init__.php create mode 100644 src/applications/feed/view/story/PhabricatorFeedStoryView.php create mode 100644 src/applications/feed/view/story/__init__.php diff --git a/resources/sql/patches/053.feed.sql b/resources/sql/patches/053.feed.sql new file mode 100644 index 0000000000..17dd8b2da7 --- /dev/null +++ b/resources/sql/patches/053.feed.sql @@ -0,0 +1,21 @@ +CREATE DATABASE phabricator_feed; + +CREATE TABLE phabricator_feed.feed_storydata ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + phid VARCHAR(64) BINARY NOT NULL, + UNIQUE KEY (phid), + chronologicalKey BIGINT UNSIGNED NOT NULL, + UNIQUE KEY (chronologicalKey), + storyType varchar(64) NOT NULL, + storyData LONGBLOB NOT NULL, + authorPHID varchar(64) BINARY NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL +); + +CREATE TABLE phabricator_feed.feed_storyreference ( + objectPHID varchar(64) BINARY NOT NULL, + chronologicalKey BIGINT UNSIGNED NOT NULL, + UNIQUE KEY (objectPHID, chronologicalKey), + KEY (chronologicalKey) +); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f2019e64c1..4ce8f7c0ef 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -103,6 +103,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_updateunitresults_Method' => 'applications/conduit/method/differential/updateunitresults', 'ConduitAPI_diffusion_getcommits_Method' => 'applications/conduit/method/diffusion/getcommits', 'ConduitAPI_diffusion_getrecentcommitsbypath_Method' => 'applications/conduit/method/diffusion/getrecentcommitsbypath', + 'ConduitAPI_feed_publish_Method' => 'applications/conduit/method/feed/publish', 'ConduitAPI_file_download_Method' => 'applications/conduit/method/file/download', 'ConduitAPI_file_upload_Method' => 'applications/conduit/method/file/upload', 'ConduitAPI_maniphest_info_Method' => 'applications/conduit/method/maniphest/info', @@ -336,6 +337,17 @@ phutil_register_library_map(array( 'PhabricatorEmailLoginController' => 'applications/auth/controller/email', 'PhabricatorEmailTokenController' => 'applications/auth/controller/emailtoken', 'PhabricatorEnv' => 'infrastructure/env', + 'PhabricatorFeedController' => 'applications/feed/controller/base', + 'PhabricatorFeedDAO' => 'applications/feed/storage/base', + 'PhabricatorFeedQuery' => 'applications/feed/query', + 'PhabricatorFeedStory' => 'applications/feed/story/base', + 'PhabricatorFeedStoryData' => 'applications/feed/storage/story', + 'PhabricatorFeedStoryPublisher' => 'applications/feed/publisher', + 'PhabricatorFeedStoryReference' => 'applications/feed/storage/storyreference', + 'PhabricatorFeedStoryUnknown' => 'applications/feed/story/unknown', + 'PhabricatorFeedStoryView' => 'applications/feed/view/story', + 'PhabricatorFeedStreamController' => 'applications/feed/controller/stream', + 'PhabricatorFeedView' => 'applications/feed/view/base', 'PhabricatorFile' => 'applications/files/storage/file', 'PhabricatorFileController' => 'applications/files/controller/base', 'PhabricatorFileDAO' => 'applications/files/storage/base', @@ -662,6 +674,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_updateunitresults_Method' => 'ConduitAPIMethod', 'ConduitAPI_diffusion_getcommits_Method' => 'ConduitAPIMethod', 'ConduitAPI_diffusion_getrecentcommitsbypath_Method' => 'ConduitAPIMethod', + 'ConduitAPI_feed_publish_Method' => 'ConduitAPIMethod', 'ConduitAPI_file_download_Method' => 'ConduitAPIMethod', 'ConduitAPI_file_upload_Method' => 'ConduitAPIMethod', 'ConduitAPI_maniphest_info_Method' => 'ConduitAPIMethod', @@ -831,6 +844,14 @@ phutil_register_library_map(array( 'PhabricatorEditPreferencesController' => 'PhabricatorPreferencesController', 'PhabricatorEmailLoginController' => 'PhabricatorAuthController', 'PhabricatorEmailTokenController' => 'PhabricatorAuthController', + 'PhabricatorFeedController' => 'PhabricatorController', + 'PhabricatorFeedDAO' => 'PhabricatorLiskDAO', + 'PhabricatorFeedStoryData' => 'PhabricatorFeedDAO', + 'PhabricatorFeedStoryReference' => 'PhabricatorFeedDAO', + 'PhabricatorFeedStoryUnknown' => 'PhabricatorFeedStory', + 'PhabricatorFeedStoryView' => 'PhabricatorFeedView', + 'PhabricatorFeedStreamController' => 'PhabricatorFeedController', + 'PhabricatorFeedView' => 'AphrontView', 'PhabricatorFile' => 'PhabricatorFileDAO', 'PhabricatorFileController' => 'PhabricatorController', 'PhabricatorFileDAO' => 'PhabricatorLiskDAO', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index a76b31a783..5670f85abd 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -330,6 +330,10 @@ class AphrontDefaultApplicationConfiguration 'delete/(?P<id>\d+)/$' => 'PhabricatorCountdownDeleteController' ), + + '/feed/' => array( + '$' => 'PhabricatorFeedStreamController', + ), ); } diff --git a/src/applications/conduit/method/feed/publish/ConduitAPI_feed_publish_Method.php b/src/applications/conduit/method/feed/publish/ConduitAPI_feed_publish_Method.php new file mode 100644 index 0000000000..832350646e --- /dev/null +++ b/src/applications/conduit/method/feed/publish/ConduitAPI_feed_publish_Method.php @@ -0,0 +1,65 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @group conduit + */ +class ConduitAPI_feed_publish_Method extends ConduitAPIMethod { + + public function getMethodDescription() { + return "(UNSTABLE!!!) Publish a story to the feed."; + } + + public function defineParamTypes() { + return array( + 'type' => 'required string', + 'data' => 'required dict', + 'time' => 'optional int', + ); + } + + public function defineErrorTypes() { + return array( + ); + } + + public function defineReturnType() { + return 'nonempty phid'; + } + + protected function execute(ConduitAPIRequest $request) { + $type = $request->getValue('type'); + $data = $request->getValue('data'); + $time = $request->getValue('time'); + + $author_phid = $request->getUser()->getPHID(); + $phids = array($author_phid); + + $publisher = new PhabricatorFeedStoryPublisher(); + $publisher->setStoryType($type); + $publisher->setStoryData($data); + $publisher->setStoryTime($time); + $publisher->setRelatedPHIDs($phids); + $publisher->setStoryAuthorPHID($author_phid); + + $data = $publisher->publish(); + + return $data->getPHID(); + } + +} diff --git a/src/applications/conduit/method/feed/publish/__init__.php b/src/applications/conduit/method/feed/publish/__init__.php new file mode 100644 index 0000000000..06f341680e --- /dev/null +++ b/src/applications/conduit/method/feed/publish/__init__.php @@ -0,0 +1,13 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/conduit/method/base'); +phutil_require_module('phabricator', 'applications/feed/publisher'); + + +phutil_require_source('ConduitAPI_feed_publish_Method.php'); diff --git a/src/applications/feed/controller/base/PhabricatorFeedController.php b/src/applications/feed/controller/base/PhabricatorFeedController.php new file mode 100644 index 0000000000..3d81bbfd69 --- /dev/null +++ b/src/applications/feed/controller/base/PhabricatorFeedController.php @@ -0,0 +1,34 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +abstract class PhabricatorFeedController extends PhabricatorController { + + public function buildStandardPageResponse($view, array $data) { + $page = $this->buildStandardPageView(); + + $page->setApplicationName('Feed'); + $page->setBaseURI('/feed/'); + $page->setTitle(idx($data, 'title')); + $page->setGlyph("\xE2\x88\x9E"); + $page->appendChild($view); + + $response = new AphrontWebpageResponse(); + return $response->setContent($page->render()); + } + +} diff --git a/src/applications/feed/controller/base/__init__.php b/src/applications/feed/controller/base/__init__.php new file mode 100644 index 0000000000..3ff17d1fb1 --- /dev/null +++ b/src/applications/feed/controller/base/__init__.php @@ -0,0 +1,15 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'aphront/response/webpage'); +phutil_require_module('phabricator', 'applications/base/controller/base'); + +phutil_require_module('phutil', 'utils'); + + +phutil_require_source('PhabricatorFeedController.php'); diff --git a/src/applications/feed/controller/stream/PhabricatorFeedStreamController.php b/src/applications/feed/controller/stream/PhabricatorFeedStreamController.php new file mode 100644 index 0000000000..ce066ec0ca --- /dev/null +++ b/src/applications/feed/controller/stream/PhabricatorFeedStreamController.php @@ -0,0 +1,37 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +final class PhabricatorFeedStreamController extends PhabricatorFeedController { + + public function processRequest() { + + $query = new PhabricatorFeedQuery(); + $stories = $query->execute(); + + $views = array(); + foreach ($stories as $story) { + $views[] = $story->renderView(); + } + + return $this->buildStandardPageResponse( + $views, + array( + 'title' => 'Feed', + )); + } +} diff --git a/src/applications/feed/controller/stream/__init__.php b/src/applications/feed/controller/stream/__init__.php new file mode 100644 index 0000000000..0005773ff0 --- /dev/null +++ b/src/applications/feed/controller/stream/__init__.php @@ -0,0 +1,13 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/feed/controller/base'); +phutil_require_module('phabricator', 'applications/feed/query'); + + +phutil_require_source('PhabricatorFeedStreamController.php'); diff --git a/src/applications/feed/publisher/PhabricatorFeedStoryPublisher.php b/src/applications/feed/publisher/PhabricatorFeedStoryPublisher.php new file mode 100644 index 0000000000..a5c206209c --- /dev/null +++ b/src/applications/feed/publisher/PhabricatorFeedStoryPublisher.php @@ -0,0 +1,113 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +final class PhabricatorFeedStoryPublisher { + + private $relatedPHIDs; + private $storyType; + private $storyData; + private $storyTime; + private $storyAuthorPHID; + + public function setRelatedPHIDs(array $phids) { + $this->relatedPHIDs = $phids; + return $this; + } + + public function setStoryType($story_type) { + $this->storyType = $story_type; + return $this; + } + + public function setStoryData(array $data) { + $this->storyData = $data; + return $this; + } + + public function setStoryTime($time) { + $this->storyTime = $time; + return $this; + } + + public function setStoryAuthorPHID($phid) { + $this->storyAuthorPHID = $phid; + return $this; + } + + public function publish() { + if (!$this->relatedPHIDs) { + throw new Exception("There are no PHIDs related to this story!"); + } + + if (!$this->storyType) { + throw new Exception("Call setStoryType() before publishing!"); + } + + $chrono_key = $this->generateChronologicalKey(); + + $story = new PhabricatorFeedStoryData(); + $story->setStoryType($this->storyType); + $story->setStoryData($this->storyData); + $story->setAuthorPHID($this->storyAuthorPHID); + $story->setChronologicalKey($chrono_key); + $story->save(); + + $ref = new PhabricatorFeedStoryReference(); + + $sql = array(); + $conn = $ref->establishConnection('w'); + foreach ($this->relatedPHIDs as $phid) { + $sql[] = qsprintf( + $conn, + '(%s, %s)', + $phid, + $chrono_key); + } + + queryfx( + $conn, + 'INSERT INTO %T (objectPHID, chronologicalKey) VALUES %Q', + $ref->getTableName(), + implode(', ', $sql)); + + return $story; + } + + /** + * We generate a unique chronological key for each story type because we want + * to be able to page through the stream with a cursor (i.e., select stories + * after ID = X) so we can efficiently perform filtering after selecting data, + * and multiple stories with the same ID make this cumbersome without putting + * a bunch of logic in the client. We could use the primary key, but that + * would prevent publishing stories which happened in the past. Since it's + * potentially useful to do that (e.g., if you're importing another data + * source) build a unique key for each story which has chronological ordering. + * + * @return string A unique, time-ordered key which identifies the story. + */ + private function generateChronologicalKey() { + // Use the epoch timestamp for the upper 32 bits of the key. Default to + // the current time if the story doesn't have an explicit timestamp. + $time = nonempty($this->storyTime, time()); + + // Generate a random number for the lower 32 bits of the key. + $rand = head(unpack('L', Filesystem::readRandomBytes(4))); + + return ($time << 32) + ($rand); + } +} diff --git a/src/applications/feed/publisher/__init__.php b/src/applications/feed/publisher/__init__.php new file mode 100644 index 0000000000..ccdcf4029d --- /dev/null +++ b/src/applications/feed/publisher/__init__.php @@ -0,0 +1,18 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/feed/storage/story'); +phutil_require_module('phabricator', 'applications/feed/storage/storyreference'); +phutil_require_module('phabricator', 'storage/qsprintf'); +phutil_require_module('phabricator', 'storage/queryfx'); + +phutil_require_module('phutil', 'filesystem'); +phutil_require_module('phutil', 'utils'); + + +phutil_require_source('PhabricatorFeedStoryPublisher.php'); diff --git a/src/applications/feed/query/PhabricatorFeedQuery.php b/src/applications/feed/query/PhabricatorFeedQuery.php new file mode 100644 index 0000000000..32e78c45a7 --- /dev/null +++ b/src/applications/feed/query/PhabricatorFeedQuery.php @@ -0,0 +1,95 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +final class PhabricatorFeedQuery { + + private $filterPHIDs; + private $limit = 100; + private $after; + + public function setFilterPHIDs(array $phids) { + $this->filterPHIDs = $phids; + return $this; + } + + public function setLimit($limit) { + $this->limit = $limit; + return $this; + } + + public function setAfter($after) { + $this->after = $after; + return $this; + } + + public function execute() { + + $ref_table = new PhabricatorFeedStoryReference(); + $story_table = new PhabricatorFeedStoryData(); + + $conn = $story_table->establishConnection('r'); + + $where = array(); + if ($this->filterPHIDs) { + $where[] = qsprintf( + $conn, + 'ref.objectPHID IN (%Ls)', + $this->filterPHIDs); + } + + if ($where) { + $where = 'WHERE ('.implode(') AND (', $where).')'; + } else { + $where = ''; + } + + $data = queryfx_all( + $conn, + 'SELECT story.* FROM %T ref + JOIN %T story ON ref.chronologicalKey = story.chronologicalKey + %Q + GROUP BY story.chronologicalKey + ORDER BY story.chronologicalKey DESC + LIMIT %d', + $ref_table->getTableName(), + $story_table->getTableName(), + $where, + $this->limit); + $data = $story_table->loadAllFromArray($data); + + $stories = array(); + foreach ($data as $story_data) { + $class = $story_data->getStoryType(); + + try { + if (!class_exists($class) || + !is_subclass_of($class, 'PhabricatorFeedStory')) { + $class = 'PhabricatorFeedStoryUnknown'; + } + } catch (PhutilMissingSymbolException $ex) { + // If the class can't be loaded, libphutil will throw an exception. + // Render the story using the unknown story view. + $class = 'PhabricatorFeedStoryUnknown'; + } + + $stories[] = newv($class, array($story_data)); + } + + return $stories; + } +} diff --git a/src/applications/feed/query/__init__.php b/src/applications/feed/query/__init__.php new file mode 100644 index 0000000000..4d574245d8 --- /dev/null +++ b/src/applications/feed/query/__init__.php @@ -0,0 +1,17 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/feed/storage/story'); +phutil_require_module('phabricator', 'applications/feed/storage/storyreference'); +phutil_require_module('phabricator', 'storage/qsprintf'); +phutil_require_module('phabricator', 'storage/queryfx'); + +phutil_require_module('phutil', 'utils'); + + +phutil_require_source('PhabricatorFeedQuery.php'); diff --git a/src/applications/feed/storage/base/PhabricatorFeedDAO.php b/src/applications/feed/storage/base/PhabricatorFeedDAO.php new file mode 100644 index 0000000000..b9cea115da --- /dev/null +++ b/src/applications/feed/storage/base/PhabricatorFeedDAO.php @@ -0,0 +1,25 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +class PhabricatorFeedDAO extends PhabricatorLiskDAO { + + public function getApplicationName() { + return 'feed'; + } + +} diff --git a/src/applications/feed/storage/base/__init__.php b/src/applications/feed/storage/base/__init__.php new file mode 100644 index 0000000000..4937e69e3f --- /dev/null +++ b/src/applications/feed/storage/base/__init__.php @@ -0,0 +1,12 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/base/storage/lisk'); + + +phutil_require_source('PhabricatorFeedDAO.php'); diff --git a/src/applications/feed/storage/story/PhabricatorFeedStoryData.php b/src/applications/feed/storage/story/PhabricatorFeedStoryData.php new file mode 100644 index 0000000000..63083b8a3f --- /dev/null +++ b/src/applications/feed/storage/story/PhabricatorFeedStoryData.php @@ -0,0 +1,42 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +class PhabricatorFeedStoryData extends PhabricatorFeedDAO { + + protected $phid; + + protected $storyType; + protected $storyData; + protected $authorPHID; + protected $chronologicalKey; + + public function getConfiguration() { + return array( + self::CONFIG_AUX_PHID => true, + self::CONFIG_SERIALIZATION => array( + 'storyData' => self::SERIALIZATION_JSON, + ), + ) + parent::getConfiguration(); + } + + public function generatePHID() { + return PhabricatorPHID::generateNewPHID( + PhabricatorPHIDConstants::PHID_TYPE_STRY); + } + +} diff --git a/src/applications/feed/storage/story/__init__.php b/src/applications/feed/storage/story/__init__.php new file mode 100644 index 0000000000..9da164a706 --- /dev/null +++ b/src/applications/feed/storage/story/__init__.php @@ -0,0 +1,14 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/feed/storage/base'); +phutil_require_module('phabricator', 'applications/phid/constants'); +phutil_require_module('phabricator', 'applications/phid/storage/phid'); + + +phutil_require_source('PhabricatorFeedStoryData.php'); diff --git a/src/applications/feed/storage/storyreference/PhabricatorFeedStoryReference.php b/src/applications/feed/storage/storyreference/PhabricatorFeedStoryReference.php new file mode 100644 index 0000000000..90dfc0dda0 --- /dev/null +++ b/src/applications/feed/storage/storyreference/PhabricatorFeedStoryReference.php @@ -0,0 +1,31 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +class PhabricatorFeedStoryReference extends PhabricatorFeedDAO { + + protected $objectPHID; + protected $chronologicalKey; + + public function getConfiguration() { + return array( + self::CONFIG_IDS => self::IDS_MANUAL, + self::CONFIG_TIMESTAMPS => false, + ) + parent::getConfiguration(); + } + +} diff --git a/src/applications/feed/storage/storyreference/__init__.php b/src/applications/feed/storage/storyreference/__init__.php new file mode 100644 index 0000000000..a0921444f3 --- /dev/null +++ b/src/applications/feed/storage/storyreference/__init__.php @@ -0,0 +1,12 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/feed/storage/base'); + + +phutil_require_source('PhabricatorFeedStoryReference.php'); diff --git a/src/applications/feed/story/base/PhabricatorFeedStory.php b/src/applications/feed/story/base/PhabricatorFeedStory.php new file mode 100644 index 0000000000..9778874fb6 --- /dev/null +++ b/src/applications/feed/story/base/PhabricatorFeedStory.php @@ -0,0 +1,29 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +abstract class PhabricatorFeedStory { + + private $data; + + final public function __construct(PhabricatorFeedStoryData $data) { + $this->data = $data; + } + + abstract public function renderView(); + +} diff --git a/src/applications/feed/story/base/__init__.php b/src/applications/feed/story/base/__init__.php new file mode 100644 index 0000000000..96e61a9589 --- /dev/null +++ b/src/applications/feed/story/base/__init__.php @@ -0,0 +1,10 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + + +phutil_require_source('PhabricatorFeedStory.php'); diff --git a/src/applications/feed/story/unknown/PhabricatorFeedStoryUnknown.php b/src/applications/feed/story/unknown/PhabricatorFeedStoryUnknown.php new file mode 100644 index 0000000000..ad865f47e4 --- /dev/null +++ b/src/applications/feed/story/unknown/PhabricatorFeedStoryUnknown.php @@ -0,0 +1,25 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +class PhabricatorFeedStoryUnknown extends PhabricatorFeedStory { + + public function renderView() { + return new PhabricatorFeedStoryView(); + } + +} diff --git a/src/applications/feed/story/unknown/__init__.php b/src/applications/feed/story/unknown/__init__.php new file mode 100644 index 0000000000..b799641ee5 --- /dev/null +++ b/src/applications/feed/story/unknown/__init__.php @@ -0,0 +1,13 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/feed/story/base'); +phutil_require_module('phabricator', 'applications/feed/view/story'); + + +phutil_require_source('PhabricatorFeedStoryUnknown.php'); diff --git a/src/applications/feed/view/base/PhabricatorFeedView.php b/src/applications/feed/view/base/PhabricatorFeedView.php new file mode 100644 index 0000000000..f223956abe --- /dev/null +++ b/src/applications/feed/view/base/PhabricatorFeedView.php @@ -0,0 +1,21 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +abstract class PhabricatorFeedView extends AphrontView { + +} diff --git a/src/applications/feed/view/base/__init__.php b/src/applications/feed/view/base/__init__.php new file mode 100644 index 0000000000..71e617494d --- /dev/null +++ b/src/applications/feed/view/base/__init__.php @@ -0,0 +1,12 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'view/base'); + + +phutil_require_source('PhabricatorFeedView.php'); diff --git a/src/applications/feed/view/story/PhabricatorFeedStoryView.php b/src/applications/feed/view/story/PhabricatorFeedStoryView.php new file mode 100644 index 0000000000..450e4d1bab --- /dev/null +++ b/src/applications/feed/view/story/PhabricatorFeedStoryView.php @@ -0,0 +1,32 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +class PhabricatorFeedStoryView extends PhabricatorFeedView { + + public function render() { + return phutil_render_tag( + 'div', + array( + 'style' => 'border: 1px dashed black; '. + 'padding: 1em; margin: 1em; '. + 'background: #ffeedd;', + ), + 'This is a feed story!'); + } + +} diff --git a/src/applications/feed/view/story/__init__.php b/src/applications/feed/view/story/__init__.php new file mode 100644 index 0000000000..df6094318e --- /dev/null +++ b/src/applications/feed/view/story/__init__.php @@ -0,0 +1,14 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/feed/view/base'); + +phutil_require_module('phutil', 'markup'); + + +phutil_require_source('PhabricatorFeedStoryView.php'); diff --git a/src/applications/phid/constants/PhabricatorPHIDConstants.php b/src/applications/phid/constants/PhabricatorPHIDConstants.php index 9a4901e348..87703b4079 100644 --- a/src/applications/phid/constants/PhabricatorPHIDConstants.php +++ b/src/applications/phid/constants/PhabricatorPHIDConstants.php @@ -30,6 +30,7 @@ final class PhabricatorPHIDConstants { const PHID_TYPE_CMIT = 'CMIT'; const PHID_TYPE_OPKG = 'OPKG'; const PHID_TYPE_PSTE = 'PSTE'; + const PHID_TYPE_STRY = 'STRY'; public static function getTypes() { return array( @@ -44,6 +45,8 @@ final class PhabricatorPHIDConstants { self::PHID_TYPE_REPO, self::PHID_TYPE_CMIT, self::PHID_TYPE_PSTE, + self::PHID_TYPE_OPKG, + self::PHID_TYPE_STRY, ); } } From 42fba24523c0440d85ff22e95cacb7475491ac22 Mon Sep 17 00:00:00 2001 From: Ricky Elrod <ricky@elrod.me> Date: Wed, 6 Jul 2011 23:05:35 -0400 Subject: [PATCH 31/34] Fix github and local login redirects. Summary: Send the user where they were intending to go after github and localized logins. Before, because Github didn't send oauthState, we would force / upon them. Test Plan: Tried all three methods of login successfully. Reviewers: epriestley CC: Differential Revision: 602 --- .../login/PhabricatorLoginController.php | 17 ++++++++++------- .../oauth/PhabricatorOAuthLoginController.php | 8 ++------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/applications/auth/controller/login/PhabricatorLoginController.php b/src/applications/auth/controller/login/PhabricatorLoginController.php index 6736eb01f2..6980cac839 100644 --- a/src/applications/auth/controller/login/PhabricatorLoginController.php +++ b/src/applications/auth/controller/login/PhabricatorLoginController.php @@ -31,10 +31,17 @@ class PhabricatorLoginController extends PhabricatorAuthController { } $next_uri = $this->getRequest()->getPath(); - if ($next_uri == '/login/') { - $next_uri = null; + $request->setCookie('next_uri', $next_uri); + if ($next_uri == '/login/' && !$request->isFormPost()) { + // The user went straight to /login/, so presumably they want to go + // to the dashboard upon logging in. Because, you know, that's logical. + // And people are logical. Sometimes... Fine, no they're not. + // We check for POST here because getPath() would get reset to /login/. + $request->setCookie('next_uri', '/'); } + // Always use $request->getCookie('next_uri', '/') after the above. + $password_auth = PhabricatorEnv::getEnvConfig('auth.password-auth-enabled'); $forms = array(); @@ -66,7 +73,7 @@ class PhabricatorLoginController extends PhabricatorAuthController { $request->setCookie('phsid', $session_key); return id(new AphrontRedirectResponse()) - ->setURI('/'); + ->setURI($request->getCookie('next_uri', '/')); } else { $log = PhabricatorUserLog::newLog( null, @@ -93,7 +100,6 @@ class PhabricatorLoginController extends PhabricatorAuthController { $form ->setUser($request->getUser()) ->setAction('/login/') - ->addHiddenInput('next', $next_uri) ->appendChild( id(new AphrontFormTextControl()) ->setLabel('Username/Email') @@ -115,8 +121,6 @@ class PhabricatorLoginController extends PhabricatorAuthController { $forms['Phabricator Login'] = $form; } - $oauth_state = $next_uri; - $providers = array( PhabricatorOAuthProvider::PROVIDER_FACEBOOK, PhabricatorOAuthProvider::PROVIDER_GITHUB, @@ -160,7 +164,6 @@ class PhabricatorLoginController extends PhabricatorAuthController { ->addHiddenInput('client_id', $client_id) ->addHiddenInput('redirect_uri', $redirect_uri) ->addHiddenInput('scope', $minimum_scope) - ->addHiddenInput('state', $oauth_state) ->setUser($request->getUser()) ->setMethod('GET') ->appendChild( diff --git a/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php b/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php index 6baf7fd6f4..bbcdad1538 100644 --- a/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php +++ b/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php @@ -135,12 +135,7 @@ class PhabricatorOAuthLoginController extends PhabricatorAuthController { ->setURI('/settings/page/'.$provider_key.'/'); } - $next_uri = '/'; - if ($this->oauthState) { - // Make sure a blind redirect to evil.com is impossible. - $uri = new PhutilURI($this->oauthState); - $next_uri = $uri->getPath(); - } + $next_uri = $request->getCookie('next_uri', '/'); // Login with known auth. @@ -158,6 +153,7 @@ class PhabricatorOAuthLoginController extends PhabricatorAuthController { $request->setCookie('phusr', $known_user->getUsername()); $request->setCookie('phsid', $session_key); + $request->clearCookie('next_uri'); return id(new AphrontRedirectResponse()) ->setURI($next_uri); } From 49310391e0a39223d8972b33e9d3f8541d7d666f Mon Sep 17 00:00:00 2001 From: tuomaspelkonen <tuomaspelkonen@fb.com> Date: Thu, 7 Jul 2011 10:24:49 -0700 Subject: [PATCH 32/34] Added subscriber view to Maniphest. Summary: People want to see all the tasks they have subscribed to in one view. A new table was added for this to make queries faster. Test Plan: Tested that the view was initially empty. After running the reindex_maniphest.php script, I saw the correct tasks there. Added myself as a subscriber to one task and made sure the view was updated. Removed myself as a subscriber from one task and made sure the view was updated again. Reviewed By: epriestley Reviewers: epriestley, jungejason, codeblock CC: aran, rm, epriestley Differential Revision: 603 --- resources/sql/patches/054.subscribers.sql | 6 ++ scripts/search/reindex_maniphest.php | 1 + src/__phutil_library_map__.php | 2 + .../tasklist/ManiphestTaskListController.php | 11 +++- .../maniphest/query/ManiphestTaskQuery.php | 31 +++++++++ src/applications/maniphest/query/__init__.php | 1 + .../subscriber/ManiphestTaskSubscriber.php | 65 +++++++++++++++++++ .../maniphest/storage/subscriber/__init__.php | 14 ++++ .../maniphest/storage/task/ManiphestTask.php | 20 ++++++ .../maniphest/storage/task/__init__.php | 1 + 10 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 resources/sql/patches/054.subscribers.sql create mode 100644 src/applications/maniphest/storage/subscriber/ManiphestTaskSubscriber.php create mode 100644 src/applications/maniphest/storage/subscriber/__init__.php diff --git a/resources/sql/patches/054.subscribers.sql b/resources/sql/patches/054.subscribers.sql new file mode 100644 index 0000000000..263aa1b692 --- /dev/null +++ b/resources/sql/patches/054.subscribers.sql @@ -0,0 +1,6 @@ +CREATE TABLE phabricator_maniphest.maniphest_tasksubscriber ( + taskPHID varchar(64) BINARY NOT NULL, + subscriberPHID varchar(64) BINARY NOT NULL, + PRIMARY KEY (subscriberPHID, taskPHID), + UNIQUE KEY (taskPHID, subscriberPHID) +); diff --git a/scripts/search/reindex_maniphest.php b/scripts/search/reindex_maniphest.php index 62550d56d2..45a99b4788 100755 --- a/scripts/search/reindex_maniphest.php +++ b/scripts/search/reindex_maniphest.php @@ -26,6 +26,7 @@ $tasks = id(new ManiphestTask())->loadAll(); echo "Updating relationships for ".count($tasks)." tasks"; foreach ($tasks as $task) { ManiphestTaskProject::updateTaskProjects($task); + ManiphestTaskSubscriber::updateTaskSubscribers($task); echo '.'; } echo "\nDone.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4ce8f7c0ef..cddac1af23 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -277,6 +277,7 @@ phutil_register_library_map(array( 'ManiphestTaskProject' => 'applications/maniphest/storage/taskproject', 'ManiphestTaskQuery' => 'applications/maniphest/query', 'ManiphestTaskStatus' => 'applications/maniphest/constants/status', + 'ManiphestTaskSubscriber' => 'applications/maniphest/storage/subscriber', 'ManiphestTaskSummaryView' => 'applications/maniphest/view/tasksummary', 'ManiphestTransaction' => 'applications/maniphest/storage/transaction', 'ManiphestTransactionDetailView' => 'applications/maniphest/view/transactiondetail', @@ -788,6 +789,7 @@ phutil_register_library_map(array( 'ManiphestTaskPriority' => 'ManiphestConstants', 'ManiphestTaskProject' => 'ManiphestDAO', 'ManiphestTaskStatus' => 'ManiphestConstants', + 'ManiphestTaskSubscriber' => 'ManiphestDAO', 'ManiphestTaskSummaryView' => 'ManiphestView', 'ManiphestTransaction' => 'ManiphestDAO', 'ManiphestTransactionDetailView' => 'ManiphestView', diff --git a/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php b/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php index b57e8c55e2..7303c65dd2 100644 --- a/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php +++ b/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php @@ -54,9 +54,10 @@ class ManiphestTaskListController extends ManiphestController { $views = array( 'User Tasks', - 'action' => 'Assigned', - 'created' => 'Created', - 'triage' => 'Need Triage', + 'action' => 'Assigned', + 'created' => 'Created', + 'subscribed' => 'Subscribed', + 'triage' => 'Need Triage', '<hr />', 'All Tasks', 'alltriage' => 'Need Triage', @@ -70,6 +71,7 @@ class ManiphestTaskListController extends ManiphestController { $has_filter = array( 'action' => true, 'created' => true, + 'subscribed' => true, 'triage' => true, ); @@ -270,6 +272,9 @@ class ManiphestTaskListController extends ManiphestController { case 'created': $query->withAuthors($user_phids); break; + case 'subscribed': + $query->withSubscribers($user_phids); + break; case 'triage': $query->withOwners($user_phids); $query->withPriority(ManiphestTaskPriority::PRIORITY_TRIAGE); diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index 1db3f02539..08f543dcc3 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -28,6 +28,7 @@ final class ManiphestTaskQuery { private $ownerPHIDs = array(); private $includeUnowned = null; private $projectPHIDs = array(); + private $subscriberPHIDs = array(); private $status = 'status-any'; const STATUS_ANY = 'status-any'; @@ -89,6 +90,11 @@ final class ManiphestTaskQuery { return $this; } + public function withSubscribers(array $subscribers) { + $this->subscriberPHIDs = $subscribers; + return $this; + } + public function setGroupBy($group) { $this->groupBy = $group; return $this; @@ -139,6 +145,7 @@ final class ManiphestTaskQuery { $where[] = $this->buildPriorityWhereClause($conn); $where[] = $this->buildAuthorWhereClause($conn); $where[] = $this->buildOwnerWhereClause($conn); + $where[] = $this->buildSubscriberWhereClause($conn); $where[] = $this->buildProjectWhereClause($conn); $where = array_filter($where); @@ -150,6 +157,7 @@ final class ManiphestTaskQuery { $join = array(); $join[] = $this->buildProjectJoinClause($conn); + $join[] = $this->buildSubscriberJoinClause($conn); $join = array_filter($join); if ($join) { @@ -272,6 +280,17 @@ final class ManiphestTaskQuery { } } + private function buildSubscriberWhereClause($conn) { + if (!$this->subscriberPHIDs) { + return null; + } + + return qsprintf( + $conn, + 'subscriber.subscriberPHID IN (%Ls)', + $this->subscriberPHIDs); + } + private function buildProjectWhereClause($conn) { if (!$this->projectPHIDs) { return null; @@ -295,6 +314,18 @@ final class ManiphestTaskQuery { $project_dao->getTableName()); } + private function buildSubscriberJoinClause($conn) { + if (!$this->subscriberPHIDs) { + return null; + } + + $subscriber_dao = new ManiphestTaskSubscriber(); + return qsprintf( + $conn, + 'JOIN %T subscriber ON subscriber.taskPHID = task.phid', + $subscriber_dao->getTableName()); + } + private function buildOrderClause($conn) { $order = array(); diff --git a/src/applications/maniphest/query/__init__.php b/src/applications/maniphest/query/__init__.php index 19b4f9fc8c..86a637a392 100644 --- a/src/applications/maniphest/query/__init__.php +++ b/src/applications/maniphest/query/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/maniphest/constants/owner'); +phutil_require_module('phabricator', 'applications/maniphest/storage/subscriber'); phutil_require_module('phabricator', 'applications/maniphest/storage/task'); phutil_require_module('phabricator', 'applications/maniphest/storage/taskproject'); phutil_require_module('phabricator', 'storage/qsprintf'); diff --git a/src/applications/maniphest/storage/subscriber/ManiphestTaskSubscriber.php b/src/applications/maniphest/storage/subscriber/ManiphestTaskSubscriber.php new file mode 100644 index 0000000000..a5b9222910 --- /dev/null +++ b/src/applications/maniphest/storage/subscriber/ManiphestTaskSubscriber.php @@ -0,0 +1,65 @@ +<?php + +/* + * Copyright 2011 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @group maniphest + */ +final class ManiphestTaskSubscriber extends ManiphestDAO { + + protected $taskPHID; + protected $subscriberPHID; + + public function getConfiguration() { + return array( + self::CONFIG_IDS => self::IDS_MANUAL, + self::CONFIG_TIMESTAMPS => false, + ); + } + + public static function updateTaskSubscribers(ManiphestTask $task) { + $dao = new ManiphestTaskSubscriber(); + $conn = $dao->establishConnection('w'); + + $sql = array(); + $subscribers = $task->getCCPHIDs(); + $subscribers[] = $task->getOwnerPHID(); + $subscribers = array_unique($subscribers); + + foreach ($subscribers as $subscriber_phid) { + $sql[] = qsprintf( + $conn, + '(%s, %s)', + $task->getPHID(), + $subscriber_phid); + } + + queryfx( + $conn, + 'DELETE FROM %T WHERE taskPHID = %s', + $dao->getTableName(), + $task->getPHID()); + if ($sql) { + queryfx( + $conn, + 'INSERT INTO %T (taskPHID, subscriberPHID) VALUES %Q', + $dao->getTableName(), + implode(', ', $sql)); + } + } + +} diff --git a/src/applications/maniphest/storage/subscriber/__init__.php b/src/applications/maniphest/storage/subscriber/__init__.php new file mode 100644 index 0000000000..6fe3a8de86 --- /dev/null +++ b/src/applications/maniphest/storage/subscriber/__init__.php @@ -0,0 +1,14 @@ +<?php +/** + * This file is automatically generated. Lint this module to rebuild it. + * @generated + */ + + + +phutil_require_module('phabricator', 'applications/maniphest/storage/base'); +phutil_require_module('phabricator', 'storage/qsprintf'); +phutil_require_module('phabricator', 'storage/queryfx'); + + +phutil_require_source('ManiphestTaskSubscriber.php'); diff --git a/src/applications/maniphest/storage/task/ManiphestTask.php b/src/applications/maniphest/storage/task/ManiphestTask.php index d3ff64b223..ff2c368e23 100644 --- a/src/applications/maniphest/storage/task/ManiphestTask.php +++ b/src/applications/maniphest/storage/task/ManiphestTask.php @@ -37,6 +37,7 @@ class ManiphestTask extends ManiphestDAO { protected $attached = array(); protected $projectPHIDs = array(); private $projectsNeedUpdate; + private $subscribersNeedUpdate; protected $ownerOrdering; @@ -70,6 +71,18 @@ class ManiphestTask extends ManiphestDAO { return $this; } + public function setCCPHIDs(array $phids) { + $this->ccPHIDs = $phids; + $this->subscribersNeedUpdate = true; + return $this; + } + + public function setOwnerPHID($phid) { + $this->ownerPHID = $phid; + $this->subscribersNeedUpdate = true; + return $this; + } + public function save() { if (!$this->mailKey) { $this->mailKey = sha1(Filesystem::readRandomBytes(20)); @@ -84,6 +97,13 @@ class ManiphestTask extends ManiphestDAO { $this->projectsNeedUpdate = false; } + if ($this->subscribersNeedUpdate) { + // If we've changed the subscriber PHIDs for this task, update the link + // table. + ManiphestTaskSubscriber::updateTaskSubscribers($this); + $this->subscribersNeedUpdate = false; + } + return $result; } diff --git a/src/applications/maniphest/storage/task/__init__.php b/src/applications/maniphest/storage/task/__init__.php index 14ba412113..42038f4e2d 100644 --- a/src/applications/maniphest/storage/task/__init__.php +++ b/src/applications/maniphest/storage/task/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/maniphest/storage/base'); +phutil_require_module('phabricator', 'applications/maniphest/storage/subscriber'); phutil_require_module('phabricator', 'applications/maniphest/storage/taskproject'); phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); From 62532ef26d6eacf9b1c226311d7763552f9c2a76 Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Thu, 7 Jul 2011 12:35:31 -0700 Subject: [PATCH 33/34] Fix Herald to accept new JSON encoding of sparse arrays Summary: JX.JSON was recently changed to use JSON.stringify (the native implementation) if it is available. The native implementation has a behavioral difference from the Javelin implementation, in that it does not compact sparse arrays. Ignore nulls resulting from removals when processing the encoded action and condition lists. Test Plan: Removed conditions from Herald rules. Reviewed By: aran Reviewers: aran, cpojer, jungejason CC: aran, epriestley Differential Revision: 606 --- .../herald/controller/rule/HeraldRuleController.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/applications/herald/controller/rule/HeraldRuleController.php b/src/applications/herald/controller/rule/HeraldRuleController.php index 7c9d9d5f5c..b2d0a73440 100644 --- a/src/applications/herald/controller/rule/HeraldRuleController.php +++ b/src/applications/herald/controller/rule/HeraldRuleController.php @@ -93,6 +93,12 @@ class HeraldRuleController extends HeraldController { $conditions = array(); foreach ($data['conditions'] as $condition) { + if ($condition === null) { + // We manage this as a sparse array on the client, so may receive + // NULL if conditions have been removed. + continue; + } + $obj = new HeraldCondition(); $obj->setFieldName($condition[0]); $obj->setFieldCondition($condition[1]); @@ -149,6 +155,11 @@ class HeraldRuleController extends HeraldController { $actions = array(); foreach ($data['actions'] as $action) { + if ($action === null) { + // Sparse on the client; removals can give us NULLs. + continue; + } + $obj = new HeraldAction(); $obj->setAction($action[0]); From cef7664d47e0fac28c97152baad53aa48e04928b Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Thu, 7 Jul 2011 13:50:56 -0700 Subject: [PATCH 34/34] Move Project list to use ManiphestTaskQuery Summary: We decided to move away from driving everything through the search engine since it doesn't scale terribly well, so use ManiphestTaskQuery instead. Also link the open count and tweak some display stuff. Test Plan: Looked at project list, clicked open tasks link Reviewed By: tuomaspelkonen Reviewers: cadamo, aran, jungejason, tuomaspelkonen CC: aran, tuomaspelkonen Differential Revision: 608 --- src/__phutil_library_map__.php | 1 - .../maniphest/query/ManiphestTaskQuery.php | 22 +++++-- .../status/PhabricatorProjectStatus.php | 2 +- .../list/PhabricatorProjectListController.php | 58 ++++++++++--------- .../project/controller/list/__init__.php | 3 +- .../PhabricatorProjectTransactionSearch.php | 55 ------------------ .../project/transactions/search/__init__.php | 13 ----- 7 files changed, 49 insertions(+), 105 deletions(-) delete mode 100644 src/applications/project/transactions/search/PhabricatorProjectTransactionSearch.php delete mode 100644 src/applications/project/transactions/search/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index cddac1af23..dbfcc996fd 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -457,7 +457,6 @@ phutil_register_library_map(array( 'PhabricatorProjectProfileController' => 'applications/project/controller/profile', 'PhabricatorProjectProfileEditController' => 'applications/project/controller/profileedit', 'PhabricatorProjectStatus' => 'applications/project/constants/status', - 'PhabricatorProjectTransactionSearch' => 'applications/project/transactions/search', 'PhabricatorRedirectController' => 'applications/base/controller/redirect', 'PhabricatorRemarkupRuleDifferential' => 'infrastructure/markup/remarkup/markuprule/differential', 'PhabricatorRemarkupRuleDiffusion' => 'infrastructure/markup/remarkup/markuprule/diffusion', diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index 08f543dcc3..4289613735 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -29,6 +29,7 @@ final class ManiphestTaskQuery { private $includeUnowned = null; private $projectPHIDs = array(); private $subscriberPHIDs = array(); + private $anyProject = false; private $status = 'status-any'; const STATUS_ANY = 'status-any'; @@ -129,6 +130,11 @@ final class ManiphestTaskQuery { return $this->rowCount; } + public function withAnyProject($any_project) { + $this->anyProject = $any_project; + return $this; + } + public function execute() { $task_dao = new ManiphestTask(); @@ -176,14 +182,18 @@ final class ManiphestTaskQuery { // multiple times. We use GROUP BY to make them distinct again. // - We want to treat the query as an intersection query, not a union // query. We sum the project count and require it be the same as the - // number of projects we're searching for. + // number of projects we're searching for. (If 'anyProject' is set, + // we do union instead.) $group = 'GROUP BY task.id'; - $count = ', COUNT(1) projectCount'; - $having = qsprintf( - $conn, - 'HAVING projectCount = %d', - count($this->projectPHIDs)); + + if (!$this->anyProject) { + $count = ', COUNT(1) projectCount'; + $having = qsprintf( + $conn, + 'HAVING projectCount = %d', + count($this->projectPHIDs)); + } } $order = $this->buildOrderClause($conn); diff --git a/src/applications/project/constants/status/PhabricatorProjectStatus.php b/src/applications/project/constants/status/PhabricatorProjectStatus.php index d933249636..1bce8fa0e5 100644 --- a/src/applications/project/constants/status/PhabricatorProjectStatus.php +++ b/src/applications/project/constants/status/PhabricatorProjectStatus.php @@ -30,7 +30,7 @@ final class PhabricatorProjectStatus { public static function getNameForStatus($status) { static $map = array( - self::UNKNOWN => 'Who knows?', + self::UNKNOWN => '', self::NOT_STARTED => 'Not started', self::IN_PROGRESS => 'In progress', self::ONGOING => 'Ongoing', diff --git a/src/applications/project/controller/list/PhabricatorProjectListController.php b/src/applications/project/controller/list/PhabricatorProjectListController.php index 759b8c1c61..05faacf307 100644 --- a/src/applications/project/controller/list/PhabricatorProjectListController.php +++ b/src/applications/project/controller/list/PhabricatorProjectListController.php @@ -43,37 +43,39 @@ class PhabricatorProjectListController $handles = id(new PhabricatorObjectHandleData($author_phids)) ->loadHandles(); + $project_phids = mpull($projects, 'getPHID'); + + $query = id(new ManiphestTaskQuery()) + ->withProjects($project_phids) + ->withAnyProject(true) + ->withStatus(ManiphestTaskQuery::STATUS_OPEN) + ->setLimit(PHP_INT_MAX); + + $tasks = $query->execute(); + $groups = array(); + foreach ($tasks as $task) { + foreach ($task->getProjectPHIDs() as $phid) { + $groups[$phid][] = $task; + } + } + + $rows = array(); foreach ($projects as $project) { - $profile = $profiles[$project->getPHID()]; - $affiliations = $affil_groups[$project->getPHID()]; + $phid = $project->getPHID(); - $documents = new PhabricatorProjectTransactionSearch($project->getPHID()); - // search all open documents by default - $documents->setSearchOptions(); - $documents = $documents->executeSearch(); + $profile = $profiles[$phid]; + $affiliations = $affil_groups[$phid]; - $documents_types = igroup($documents, 'documentType'); - $tasks = idx( - $documents_types, - PhabricatorPHIDConstants::PHID_TYPE_TASK); - $tasks_amount = count($tasks); - - // TODO: set up a relationship between the project and the arcanist's - // project, to be able get the revisions. - $revisions = idx( - $documents_types, - PhabricatorPHIDConstants::PHID_TYPE_DREV); - $revisions_amount = count($revisions); + $group = idx($groups, $phid, array()); + $task_count = count($group); $population = count($affiliations); $status = PhabricatorProjectStatus::getNameForStatus( $project->getStatus()); - $blurb = nonempty( - $profile->getBlurb(), - 'Oops!, nothing is known about this elusive project.'); + $blurb = $profile->getBlurb(); $blurb = phutil_utf8_shorten($blurb, $columns = 100); $rows[] = array( @@ -82,8 +84,12 @@ class PhabricatorProjectListController $handles[$project->getAuthorPHID()]->renderLink(), phutil_escape_html($population), phutil_escape_html($status), - phutil_escape_html($tasks_amount), - // phutil_escape_html($revisions_amount), + phutil_render_tag( + 'a', + array( + 'href' => '/maniphest/view/all/?projects='.$phid, + ), + phutil_escape_html($task_count)), phutil_render_tag( 'a', array( @@ -98,12 +104,11 @@ class PhabricatorProjectListController $table->setHeaders( array( 'Project', - 'Blurb', + 'Description', 'Mastermind', 'Population', 'Status', 'Open Tasks', - // 'Open Revisions', '', )); $table->setColumnClasses( @@ -112,9 +117,8 @@ class PhabricatorProjectListController 'wide', '', 'right', - 'pri', + '', 'right', - // 'right', 'action', )); diff --git a/src/applications/project/controller/list/__init__.php b/src/applications/project/controller/list/__init__.php index c7ff3c35e3..c87483a57d 100644 --- a/src/applications/project/controller/list/__init__.php +++ b/src/applications/project/controller/list/__init__.php @@ -6,14 +6,13 @@ -phutil_require_module('phabricator', 'applications/phid/constants'); +phutil_require_module('phabricator', 'applications/maniphest/query'); phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'applications/project/constants/status'); phutil_require_module('phabricator', 'applications/project/controller/base'); phutil_require_module('phabricator', 'applications/project/storage/affiliation'); phutil_require_module('phabricator', 'applications/project/storage/profile'); phutil_require_module('phabricator', 'applications/project/storage/project'); -phutil_require_module('phabricator', 'applications/project/transactions/search'); phutil_require_module('phabricator', 'view/control/table'); phutil_require_module('phabricator', 'view/layout/panel'); diff --git a/src/applications/project/transactions/search/PhabricatorProjectTransactionSearch.php b/src/applications/project/transactions/search/PhabricatorProjectTransactionSearch.php deleted file mode 100644 index 02e569c81b..0000000000 --- a/src/applications/project/transactions/search/PhabricatorProjectTransactionSearch.php +++ /dev/null @@ -1,55 +0,0 @@ -<?php - -/* - * Copyright 2011 Facebook, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -class PhabricatorProjectTransactionSearch { - private $projectPhids; - private $documents; - private $status; - - public function __construct($project_phids) { - if (is_array($project_phids)) { - $this->projectPhids = $project_phids; - } else { - $this->projectPhids = array($project_phids); - } - return $this; - } - - // search all open documents by default - public function setSearchOptions($documents = '', $status = true) { - $this->documents = $documents; - $this->status = $status; - return $this; - } - - public function executeSearch() { - $projects = $this->projectPhids; - $on_documents = $this->documents; - $with_status = $this->status; - - $query = new PhabricatorSearchQuery(); - $query->setQuery(''); - $query->setParameter('project', $projects); - $query->setParameter('type', $on_documents); - $query->setParameter('open', $with_status); - - $executor = new PhabricatorSearchMySQLExecutor(); - $results = $executor->executeSearch($query); - return $results; - } -} diff --git a/src/applications/project/transactions/search/__init__.php b/src/applications/project/transactions/search/__init__.php deleted file mode 100644 index 722f521d91..0000000000 --- a/src/applications/project/transactions/search/__init__.php +++ /dev/null @@ -1,13 +0,0 @@ -<?php -/** - * This file is automatically generated. Lint this module to rebuild it. - * @generated - */ - - - -phutil_require_module('phabricator', 'applications/search/execute/mysql'); -phutil_require_module('phabricator', 'applications/search/storage/query'); - - -phutil_require_source('PhabricatorProjectTransactionSearch.php');