From 4761cb8d739036956fea43779f3f426448d01a27 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Thu, 18 Jun 2015 07:06:37 +1000 Subject: [PATCH 01/17] Use PhutilInvalidStateException Summary: Use `PhutilInvalidStateException` where appropriate. Test Plan: Eyeball it. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Differential Revision: https://secure.phabricator.com/D13327 --- src/aphront/AphrontController.php | 16 ++++++++++------ .../auth/engine/PhabricatorAuthInviteEngine.php | 2 +- .../view/ConpherenceTransactionView.php | 2 +- .../PhabricatorDashboardPanelRenderingEngine.php | 4 +--- .../markup/PhabricatorMarkupEngine.php | 5 +---- .../form/control/AphrontFormPolicyControl.php | 4 ++-- .../form/control/AphrontFormTokenizerControl.php | 7 +++++-- src/view/layout/AphrontSideNavFilterView.php | 4 ++-- src/view/layout/PhabricatorActionListView.php | 2 +- src/view/phui/PHUITagView.php | 2 +- 10 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/aphront/AphrontController.php b/src/aphront/AphrontController.php index 55c2f9f30e..df6cff6abc 100644 --- a/src/aphront/AphrontController.php +++ b/src/aphront/AphrontController.php @@ -35,8 +35,10 @@ abstract class AphrontController extends Phobject { throw new PhutilMethodNotImplementedException( pht( - 'Controllers must implement either handleRequest() (recommended) '. - 'or processRequest() (deprecated).')); + 'Controllers must implement either %s (recommended) '. + 'or %s (deprecated).', + 'handleRequest()', + 'processRequest()')); } final public function setRequest(AphrontRequest $request) { @@ -46,7 +48,7 @@ abstract class AphrontController extends Phobject { final public function getRequest() { if (!$this->request) { - throw new Exception(pht('Call setRequest() before getRequest()!')); + throw new PhutilInvalidStateException('setRequest'); } return $this->request; } @@ -81,10 +83,12 @@ abstract class AphrontController extends Phobject { } public function getDefaultResourceSource() { - throw new Exception( + throw new MethodNotImplementedException( pht( - 'A Controller must implement getDefaultResourceSource() before you '. - 'can invoke requireResource() or initBehavior().')); + 'A Controller must implement %s before you can invoke %s or %s.', + 'getDefaultResourceSource()', + 'requireResource()', + 'initBehavior()')); } public function requireResource($symbol) { diff --git a/src/applications/auth/engine/PhabricatorAuthInviteEngine.php b/src/applications/auth/engine/PhabricatorAuthInviteEngine.php index bbdb6000f8..f1cb45483e 100644 --- a/src/applications/auth/engine/PhabricatorAuthInviteEngine.php +++ b/src/applications/auth/engine/PhabricatorAuthInviteEngine.php @@ -18,7 +18,7 @@ final class PhabricatorAuthInviteEngine extends Phobject { public function getViewer() { if (!$this->viewer) { - throw new Exception(pht('Call setViewer() before getViewer()!')); + throw new PhutilInvalidStateException('setViewer'); } return $this->viewer; } diff --git a/src/applications/conpherence/view/ConpherenceTransactionView.php b/src/applications/conpherence/view/ConpherenceTransactionView.php index 57bec773fc..fa94b26ba6 100644 --- a/src/applications/conpherence/view/ConpherenceTransactionView.php +++ b/src/applications/conpherence/view/ConpherenceTransactionView.php @@ -64,7 +64,7 @@ final class ConpherenceTransactionView extends AphrontView { public function render() { $viewer = $this->getUser(); if (!$viewer) { - throw new Exception(pht('Call setUser() before render()!')); + throw new PhutilInvalidStateException('setUser'); } require_celerity_resource('conpherence-transaction-css'); diff --git a/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php b/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php index dfba37eaa5..fcf1e3d7c9 100644 --- a/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php +++ b/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php @@ -277,9 +277,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { */ private function detectRenderingCycle(PhabricatorDashboardPanel $panel) { if ($this->parentPanelPHIDs === null) { - throw new Exception( - pht( - 'You must call setParentPanelPHIDs() before rendering panels.')); + throw new PhutilInvalidStateException('setParentPanelPHIDs'); } $max_depth = 4; diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php index 1694d42db0..f00c8a1ef5 100644 --- a/src/infrastructure/markup/PhabricatorMarkupEngine.php +++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php @@ -198,10 +198,7 @@ final class PhabricatorMarkupEngine extends Phobject { } if (!isset($this->objects[$key]['output'])) { - throw new Exception( - pht( - 'Call %s before using results.', - 'process()')); + throw new PhutilInvalidStateException('process'); } } diff --git a/src/view/form/control/AphrontFormPolicyControl.php b/src/view/form/control/AphrontFormPolicyControl.php index 7c4fefcac6..f062bb1b5a 100644 --- a/src/view/form/control/AphrontFormPolicyControl.php +++ b/src/view/form/control/AphrontFormPolicyControl.php @@ -162,10 +162,10 @@ final class AphrontFormPolicyControl extends AphrontFormControl { protected function renderInput() { if (!$this->object) { - throw new Exception(pht('Call setPolicyObject() before rendering!')); + throw new PhutilInvalidStateException('setPolicyObject'); } if (!$this->capability) { - throw new Exception(pht('Call setCapability() before rendering!')); + throw new PhutilInvalidStateException('setCapability'); } $policy = $this->object->getPolicy($this->capability); diff --git a/src/view/form/control/AphrontFormTokenizerControl.php b/src/view/form/control/AphrontFormTokenizerControl.php index 841c1006e5..fd25f66a1f 100644 --- a/src/view/form/control/AphrontFormTokenizerControl.php +++ b/src/view/form/control/AphrontFormTokenizerControl.php @@ -109,8 +109,11 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { if (!$viewer) { throw new Exception( pht( - 'Call setUser() before rendering tokenizers. Use appendControl() '. - 'on AphrontFormView to do this easily.')); + 'Call %s before rendering tokenizers. '. + 'Use %s on %s to do this easily.', + 'setUser()', + 'appendControl()', + 'AphrontFormView')); } $values = nonempty($this->getValue(), array()); diff --git a/src/view/layout/AphrontSideNavFilterView.php b/src/view/layout/AphrontSideNavFilterView.php index fe8025026b..5e85f89254 100644 --- a/src/view/layout/AphrontSideNavFilterView.php +++ b/src/view/layout/AphrontSideNavFilterView.php @@ -187,10 +187,10 @@ final class AphrontSideNavFilterView extends AphrontView { public function render() { if ($this->menu->getItems()) { if (!$this->baseURI) { - throw new Exception(pht('Call setBaseURI() before render()!')); + throw new PhutilInvalidStateException('setBaseURI'); } if ($this->selectedFilter === false) { - throw new Exception(pht('Call selectFilter() before render()!')); + throw new PhutilInvalidStateException('selectFilter'); } } diff --git a/src/view/layout/PhabricatorActionListView.php b/src/view/layout/PhabricatorActionListView.php index 6db62c129e..82449951d7 100644 --- a/src/view/layout/PhabricatorActionListView.php +++ b/src/view/layout/PhabricatorActionListView.php @@ -29,7 +29,7 @@ final class PhabricatorActionListView extends AphrontView { public function render() { if (!$this->user) { - throw new Exception(pht('Call setUser() before render()!')); + throw new PhutilInvalidStateException('setUser'); } $event = new PhabricatorEvent( diff --git a/src/view/phui/PHUITagView.php b/src/view/phui/PHUITagView.php index 8f49caa82a..439feb77c5 100644 --- a/src/view/phui/PHUITagView.php +++ b/src/view/phui/PHUITagView.php @@ -144,7 +144,7 @@ final class PHUITagView extends AphrontTagView { protected function getTagContent() { if (!$this->type) { - throw new Exception(pht('You must call setType() before render()!')); + throw new PhutilInvalidStateException('setType', 'render'); } $color = null; From e270157fcbd5736b9d6318694662df0478880b69 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Thu, 18 Jun 2015 07:25:41 +1000 Subject: [PATCH 02/17] Fix a broken class name Auditors: epriestley --- src/aphront/AphrontController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aphront/AphrontController.php b/src/aphront/AphrontController.php index df6cff6abc..97a5fc1fd1 100644 --- a/src/aphront/AphrontController.php +++ b/src/aphront/AphrontController.php @@ -83,7 +83,7 @@ abstract class AphrontController extends Phobject { } public function getDefaultResourceSource() { - throw new MethodNotImplementedException( + throw new PhutilMethodNotImplementedException( pht( 'A Controller must implement %s before you can invoke %s or %s.', 'getDefaultResourceSource()', From 46a225c7b1cba077979b59166211011cf939cdce Mon Sep 17 00:00:00 2001 From: lkassianik Date: Wed, 17 Jun 2015 18:34:01 -0700 Subject: [PATCH 03/17] Quoted text remarkup should be smart enough to know when to add a '>' and when to add '> ' Summary: Fixes T8565, Quoted text remarkup should be smart enough to know when to add a '>' and when to add '> ' Test Plan: Open an object with remarkup comments, add 'quote', select that text click the quote button in the remarkup menu, text should become '> quote'. Select and click again, text should become '>> quote'. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T8565 Differential Revision: https://secure.phabricator.com/D13334 --- resources/celerity/map.php | 24 +++++++++---------- .../behavior-phabricator-remarkup-assist.js | 20 ++++++++++++---- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 80107b8d27..6cb0832e1d 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -8,7 +8,7 @@ return array( 'names' => array( 'core.pkg.css' => 'd7ecac6d', - 'core.pkg.js' => '288f6571', + 'core.pkg.js' => 'e0117d99', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '02273347', 'differential.pkg.js' => 'ebef29b1', @@ -460,7 +460,7 @@ return array( 'rsrc/js/core/behavior-object-selector.js' => '49b73b36', 'rsrc/js/core/behavior-oncopy.js' => '2926fff2', 'rsrc/js/core/behavior-phabricator-nav.js' => '14d7a8b8', - 'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => '095ed313', + 'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => 'eeaa9e5a', 'rsrc/js/core/behavior-refresh-csrf.js' => '7814b593', 'rsrc/js/core/behavior-remarkup-preview.js' => 'f7379f45', 'rsrc/js/core/behavior-reorder-applications.js' => '76b9fc3e', @@ -611,7 +611,7 @@ return array( 'javelin-behavior-phabricator-notification-example' => '8ce821c5', 'javelin-behavior-phabricator-object-selector' => '49b73b36', 'javelin-behavior-phabricator-oncopy' => '2926fff2', - 'javelin-behavior-phabricator-remarkup-assist' => '095ed313', + 'javelin-behavior-phabricator-remarkup-assist' => 'eeaa9e5a', 'javelin-behavior-phabricator-reveal-content' => '60821bc7', 'javelin-behavior-phabricator-search-typeahead' => '048330fa', 'javelin-behavior-phabricator-show-older-transactions' => 'dbbf48b6', @@ -885,15 +885,6 @@ return array( 'javelin-stratcom', 'javelin-vector', ), - '095ed313' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - 'phabricator-phtize', - 'phabricator-textareautils', - 'javelin-workflow', - 'javelin-vector', - ), '0a3f3021' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1948,6 +1939,15 @@ return array( 'phabricator-phtize', 'javelin-dom', ), + 'eeaa9e5a' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'phabricator-phtize', + 'phabricator-textareautils', + 'javelin-workflow', + 'javelin-vector', + ), 'efe49472' => array( 'javelin-install', 'javelin-util', diff --git a/webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js b/webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js index 787b30898b..266d2ecd37 100644 --- a/webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js +++ b/webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js @@ -99,8 +99,20 @@ JX.behavior('phabricator-remarkup-assist', function(config) { } else { sel = [def]; } - sel = sel.join('\n' + ch); - return sel; + + if (ch === '>') { + for(var i=0; i < sel.length; i++) { + if (sel[i][0] === '>') { + ch = '>'; + } else { + ch = '> '; + } + sel[i] = ch + sel[i]; + } + return sel.join('\n'); + } + + return sel.join('\n' + ch); } function assist(area, action, root) { @@ -141,9 +153,9 @@ JX.behavior('phabricator-remarkup-assist', function(config) { update(area, code_prefix + '```\n', sel, '\n```'); break; case 'fa-quote-right': - ch = '> '; + ch = '>'; sel = prepend_char_to_lines(ch, sel, pht('Quoted Text')); - update(area, ((r.start === 0) ? '' : '\n\n') + ch, sel, '\n\n'); + update(area, ((r.start === 0) ? '' : '\n\n'), sel, '\n\n'); break; case 'fa-table': var table_prefix = (r.start === 0 ? '' : '\n\n'); From dad29171fffb764e3c66253f57c259204dc2375a Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Thu, 18 Jun 2015 22:18:08 +1000 Subject: [PATCH 04/17] Add more languages to syntax highlighting options Summary: Fixes T8589. Adds a bunch of new languages to the syntax highlighting config options so that they are supported by #paste. Test Plan: Saw new filetypes in Paste. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T8589 Differential Revision: https://secure.phabricator.com/D13337 --- ...abricatorSyntaxHighlightingConfigOptions.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/applications/config/option/PhabricatorSyntaxHighlightingConfigOptions.php b/src/applications/config/option/PhabricatorSyntaxHighlightingConfigOptions.php index c2511b0f8e..3a5305dde6 100644 --- a/src/applications/config/option/PhabricatorSyntaxHighlightingConfigOptions.php +++ b/src/applications/config/option/PhabricatorSyntaxHighlightingConfigOptions.php @@ -20,7 +20,6 @@ final class PhabricatorSyntaxHighlightingConfigOptions } public function getOptions() { - $caches_href = PhabricatorEnv::getDocLink('Managing Caches'); return array( @@ -74,31 +73,43 @@ final class PhabricatorSyntaxHighlightingConfigOptions 'c' => 'C', 'coffee-script' => 'CoffeeScript', 'cpp' => 'C++', + 'csharp' => 'C#', 'css' => 'CSS', 'd' => 'D', 'diff' => 'Diff', 'django' => 'Django Templating', + 'docker' => 'Docker', 'erb' => 'Embedded Ruby/ERB', 'erlang' => 'Erlang', 'go' => 'Golang', 'groovy' => 'Groovy', 'haskell' => 'Haskell', 'html' => 'HTML', + 'http' => 'HTTP', 'invisible' => 'Invisible', 'java' => 'Java', 'js' => 'Javascript', 'json' => 'JSON', + 'make' => 'Makefile', 'mysql' => 'MySQL', + 'nginx' => 'Nginx Configuration', 'objc' => 'Objective-C', 'perl' => 'Perl', 'php' => 'PHP', + 'postgresql' => 'PostgreSQL', + 'pot' => 'Gettext Catalog', 'puppet' => 'Puppet', - 'rest' => 'reStructuredText', - 'text' => 'Plain Text', 'python' => 'Python', 'rainbow' => 'Rainbow', 'remarkup' => 'Remarkup', + 'rest' => 'reStructuredText', + 'robotframework' => 'RobotFramework', + 'rst' => 'reStructuredText', 'ruby' => 'Ruby', + 'sql' => 'SQL', + 'tex' => 'LaTeX', + 'text' => 'Plain Text', + 'twig' => 'Twig', 'xml' => 'XML', 'yaml' => 'YAML', )) From 7f6508af5a3d79ddcc7c6e97864794f657315233 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Thu, 18 Jun 2015 22:40:04 +1000 Subject: [PATCH 05/17] Add missing execution on archived task query Summary: Fixes T8599. I'm not sure how to reproduce the original issue, but I'm fairly confident that the issue is that the issue is that `execute()` is not called on the query object. Test Plan: Created a Harbormaster build plan with a single "Lease Host" step. Ran `./bin/harbormaster build --plan 1 D1` from the command line and hit the exception as described in T8599. Applied patch and hit a different exception (which I think is just because I don't know how to use #drydock and #harbormaster). Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: hach-que, epriestley, Korvin Maniphest Tasks: T8599 Differential Revision: https://secure.phabricator.com/D13335 --- src/infrastructure/daemon/workers/PhabricatorWorker.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/infrastructure/daemon/workers/PhabricatorWorker.php b/src/infrastructure/daemon/workers/PhabricatorWorker.php index c64135531b..29f3e0ae39 100644 --- a/src/infrastructure/daemon/workers/PhabricatorWorker.php +++ b/src/infrastructure/daemon/workers/PhabricatorWorker.php @@ -199,7 +199,8 @@ abstract class PhabricatorWorker extends Phobject { } $tasks = id(new PhabricatorWorkerArchiveTaskQuery()) - ->withIDs($task_ids); + ->withIDs($task_ids) + ->execute(); foreach ($tasks as $task) { if ($task->getResult() != PhabricatorWorkerArchiveTask::RESULT_SUCCESS) { From 9109cf62e464b94ad5ce481641b6c16d4da12a92 Mon Sep 17 00:00:00 2001 From: lkassianik Date: Thu, 18 Jun 2015 09:06:12 -0700 Subject: [PATCH 06/17] More correct fix - set default value array() for `getParameter` call. Summary: Ref T8577, More correct fix - set default value array() for `getParameter` call. Test Plan: Make sure saved queries like Month View, Day View, and Upcoming Events, still work. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Maniphest Tasks: T8577 Differential Revision: https://secure.phabricator.com/D13330 --- .../query/PhabricatorCalendarEventSearchEngine.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php index 0dc4d569f1..d2898177ad 100644 --- a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php @@ -126,15 +126,15 @@ final class PhabricatorCalendarEventSearchEngine $query->withDateRange($min_range, $max_range); } - $invited_phids = $saved->getParameter('invitedPHIDs'); + $invited_phids = $saved->getParameter('invitedPHIDs', array()); + $invited_phids = $user_datasource->evaluateTokens($invited_phids); if ($invited_phids) { - $invited_phids = $user_datasource->evaluateTokens($invited_phids); $query->withInvitedPHIDs($invited_phids); } - $creator_phids = $saved->getParameter('creatorPHIDs'); + $creator_phids = $saved->getParameter('creatorPHIDs', array()); + $creator_phids = $user_datasource->evaluateTokens($creator_phids); if ($creator_phids) { - $creator_phids = $user_datasource->evaluateTokens($creator_phids); $query->withCreatorPHIDs($creator_phids); } From e291b906419a7b06fe5cc06546f59c7f075055f4 Mon Sep 17 00:00:00 2001 From: lkassianik Date: Thu, 18 Jun 2015 11:02:12 -0700 Subject: [PATCH 07/17] Format Calendar list objects Summary: Closes T8050, Format Calendar list objects Test Plan: Open Calendar list, check that new formatting is true to mocks. Reviewers: chad, #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T8050 Differential Revision: https://secure.phabricator.com/D13318 --- .../PhabricatorCalendarEventSearchEngine.php | 30 ++++++++++++++----- .../storage/PhabricatorCalendarEvent.php | 23 ++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php index d2898177ad..cac6c39252 100644 --- a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php @@ -313,17 +313,31 @@ final class PhabricatorCalendarEventSearchEngine $list = new PHUIObjectItemListView(); foreach ($events as $event) { $from = phabricator_datetime($event->getDateFrom(), $viewer); - $to = phabricator_datetime($event->getDateTo(), $viewer); + $duration = ''; $creator_handle = $handles[$event->getUserPHID()]; + $attendees = array(); + foreach ($event->getInvitees() as $invitee) { + $attendees[] = $invitee->getInviteePHID(); + } + + $attendees = pht( + 'Attending: %s', + $viewer->renderHandleList($attendees) + ->setAsInline(1) + ->render()); + + if (strlen($event->getDuration()) > 0) { + $duration = pht( + 'Duration: %s', + $event->getDuration()); + } + $item = id(new PHUIObjectItemView()) - ->setHeader($event->getName()) - ->setHref($event->getURI()) - ->addByline(pht('Creator: %s', $creator_handle->renderLink())) - ->addAttribute(pht('From %s to %s', $from, $to)) - ->addAttribute(id(new PhutilUTF8StringTruncator()) - ->setMaximumGlyphs(64) - ->truncateString($event->getDescription())); + ->setHeader($viewer->renderHandle($event->getPHID())->render()) + ->addAttribute($attendees) + ->addIcon('none', $from) + ->addIcon('none', $duration); $list->addItem($item); } diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 2b3fc4bdb8..874c62fc4b 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -373,6 +373,29 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return false; } + public function getDuration() { + $seconds = $this->dateTo - $this->dateFrom; + $minutes = round($seconds / 60, 1); + $hours = round($minutes / 60, 3); + $days = round($hours / 24, 2); + + $duration = ''; + + if ($days >= 1) { + return pht( + '%s day(s)', + round($days, 1)); + } else if ($hours >= 1) { + return pht( + '%s hour(s)', + round($hours, 1)); + } else if ($minutes >= 1) { + return pht( + '%s minute(s)', + round($minutes, 0)); + } + } + /* -( Markup Interface )--------------------------------------------------- */ From 2eb73619d1c45eb0ec1efc3dc310c1ab98422b75 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 18 Jun 2015 13:05:32 -0700 Subject: [PATCH 08/17] Truncate large strings in DarkConsole Summary: Ref T8597. If a page issues a large query (like inserting a blob into file storage), we may try to utf8ize the entire thing. This is slow and pointless. Instead, truncate tab data after 4096 bytes before sanitizing. Test Plan: Adjusted limit to 256 bytes, saw long queries get truncated reasonably. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8597 Differential Revision: https://secure.phabricator.com/D13347 --- src/applications/console/core/DarkConsoleCore.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/applications/console/core/DarkConsoleCore.php b/src/applications/console/core/DarkConsoleCore.php index 6696eb453f..09398df73a 100644 --- a/src/applications/console/core/DarkConsoleCore.php +++ b/src/applications/console/core/DarkConsoleCore.php @@ -127,6 +127,13 @@ final class DarkConsoleCore extends Phobject { } return $data; } else { + // Truncate huge strings. Since the data doesn't really matter much, + // just truncate bytes to avoid PhutilUTF8StringTruncator overhead. + $length = strlen($data); + $max = 4096; + if ($length > $max) { + $data = substr($data, 0, $max).'...<'.$length.' bytes>...'; + } return phutil_utf8ize($data); } } From 90078fe06ea53c1c9eecd0df3f25faa648cedeba Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 18 Jun 2015 13:05:44 -0700 Subject: [PATCH 09/17] Clarify that 'order' is an optional parameter in Conduit API methods Summary: Fixes T8603. For automatic 'order' parameters provided by infrastructure en route to T7715, clarify that they are optional (we will use the default builtin order for the underlying Query if an order is not provided). Test Plan: Used web UI to see "optional" hint. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8603 Differential Revision: https://secure.phabricator.com/D13342 --- src/applications/conduit/method/ConduitAPIMethod.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/conduit/method/ConduitAPIMethod.php b/src/applications/conduit/method/ConduitAPIMethod.php index a56f680432..4472f80864 100644 --- a/src/applications/conduit/method/ConduitAPIMethod.php +++ b/src/applications/conduit/method/ConduitAPIMethod.php @@ -30,7 +30,7 @@ abstract class ConduitAPIMethod $query = $this->newQueryObject(); if ($query) { - $types['order'] = 'order'; + $types['order'] = 'optional order'; $types += $this->getPagerParamTypes(); } From e96cd29efff7ebfcbf765eb2c70a1a434c96bb91 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 18 Jun 2015 13:06:01 -0700 Subject: [PATCH 10/17] Reduce badness in viewing large files in the Diffusion web UI Summary: Fixes T8597. Second issue there is that if you look at a huge file in Diffusion (like `/path/to/300MB.pdf`) we pull the whole thing over Conduit upfront, then try to shove it into file storage. Instead, pull no more than the chunk limit (normally 4MB) and don't spend more than 10s pulling data. If we get 4MB of data and/or time out, just fail with a message in the vein of "this is a really big file". Eventually, we could improve this: - We can determine the //size// of very large files correctly in at least some VCSes, this just takes a little more work. This would let us show the true filesize, at least. - We could eventually stream the data out of the VCS, but we can't stream data over Conduit right now and this is a lot of work. This is just "stop crashing". Test Plan: Changed limits to 0.01 seconds and 8 bytes and saw reasonable errors. Changed them back and got normal beahvior. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8597 Differential Revision: https://secure.phabricator.com/D13348 --- ...fusionFileContentQueryConduitAPIMethod.php | 16 +++++++ .../DiffusionBrowseFileController.php | 48 +++++++++++++++---- .../filecontent/DiffusionFileContentQuery.php | 46 +++++++++++++++++- 3 files changed, 101 insertions(+), 9 deletions(-) diff --git a/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php index 671ec3ef6f..61987b4d77 100644 --- a/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php @@ -20,6 +20,8 @@ final class DiffusionFileContentQueryConduitAPIMethod 'path' => 'required string', 'commit' => 'required string', 'needsBlame' => 'optional bool', + 'timeout' => 'optional int', + 'byteLimit' => 'optional int', ); } @@ -31,16 +33,30 @@ final class DiffusionFileContentQueryConduitAPIMethod $file_query ->setViewer($request->getUser()) ->setNeedsBlame($needs_blame); + + $timeout = $request->getValue('timeout'); + if ($timeout) { + $file_query->setTimeout($timeout); + } + + $byte_limit = $request->getValue('byteLimit'); + if ($byte_limit) { + $file_query->setByteLimit($byte_limit); + } + $file_content = $file_query->loadFileContent(); + if ($needs_blame) { list($text_list, $rev_list, $blame_dict) = $file_query->getBlameData(); } else { $text_list = $rev_list = $blame_dict = array(); } + $file_content ->setBlameDict($blame_dict) ->setRevList($rev_list) ->setTextList($text_list); + return $file_content->toDictionary(); } diff --git a/src/applications/diffusion/controller/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/DiffusionBrowseFileController.php index c55d217bfe..f975984789 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseFileController.php @@ -54,14 +54,27 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController { $needs_blame = ($show_blame && !$show_color) || ($show_blame && $request->isAjax()); + $params = array( + 'commit' => $drequest->getCommit(), + 'path' => $drequest->getPath(), + 'needsBlame' => $needs_blame, + ); + + $byte_limit = null; + if ($view !== 'raw') { + $byte_limit = PhabricatorFileStorageEngine::getChunkThreshold(); + $time_limit = 10; + + $params += array( + 'timeout' => $time_limit, + 'byteLimit' => $byte_limit, + ); + } + $file_content = DiffusionFileContent::newFromConduit( $this->callConduitWithDiffusionRequest( 'diffusion.filecontentquery', - array( - 'commit' => $drequest->getCommit(), - 'path' => $drequest->getPath(), - 'needsBlame' => $needs_blame, - ))); + $params)); $data = $file_content->getCorpus(); if ($view === 'raw') { @@ -71,8 +84,13 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController { $this->loadLintMessages(); $this->coverage = $drequest->loadCoverage(); - $binary_uri = null; - if (ArcanistDiffUtils::isHeuristicBinaryFile($data)) { + if ($byte_limit && (strlen($data) == $byte_limit)) { + $corpus = $this->buildErrorCorpus( + pht( + 'This file is larger than %s byte(s), and too large to display '. + 'in the web UI.', + $byte_limit)); + } else if (ArcanistDiffUtils::isHeuristicBinaryFile($data)) { $file = $this->loadFileForData($path, $data); $file_uri = $file->getBestURI(); @@ -80,7 +98,6 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController { $corpus = $this->buildImageCorpus($file_uri); } else { $corpus = $this->buildBinaryCorpus($file_uri, $data); - $binary_uri = $file_uri; } } else { // Build the content of the file. @@ -940,6 +957,21 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController { return $box; } + private function buildErrorCorpus($message) { + $text = id(new PHUIBoxView()) + ->addPadding(PHUI::PADDING_LARGE) + ->appendChild($message); + + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Details')); + + $box = id(new PHUIObjectBoxView()) + ->setHeader($header) + ->appendChild($text); + + return $box; + } + private function buildBeforeResponse($before) { $request = $this->getRequest(); $drequest = $this->getDiffusionRequest(); diff --git a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php index 2f3493737a..9664d98f07 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php @@ -11,6 +11,26 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery { private $needsBlame; private $fileContent; private $viewer; + private $timeout; + private $byteLimit; + + public function setTimeout($timeout) { + $this->timeout = $timeout; + return $this; + } + + public function getTimeout() { + return $this->timeout; + } + + public function setByteLimit($byte_limit) { + $this->byteLimit = $byte_limit; + return $this; + } + + public function getByteLimit() { + return $this->byteLimit; + } final public static function newFromDiffusionRequest( DiffusionRequest $request) { @@ -21,7 +41,31 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery { abstract protected function executeQueryFromFuture(Future $future); final public function loadFileContentFromFuture(Future $future) { - $this->fileContent = $this->executeQueryFromFuture($future); + + if ($this->timeout) { + $future->setTimeout($this->timeout); + } + + if ($this->getByteLimit()) { + $future->setStdoutSizeLimit($this->getByteLimit()); + } + + try { + $file_content = $this->executeQueryFromFuture($future); + } catch (CommandException $ex) { + if (!$future->getWasKilledByTimeout()) { + throw $ex; + } + + $message = pht( + '', + $this->timeout); + + $file_content = new DiffusionFileContent(); + $file_content->setCorpus($message); + } + + $this->fileContent = $file_content; $repository = $this->getRequest()->getRepository(); $try_encoding = $repository->getDetail('encoding'); From b12f13efd8d0803b906a83ea5f689097a24b69c6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 18 Jun 2015 15:11:01 -0700 Subject: [PATCH 11/17] Force date/time preferences to valid values Summary: Fixes T8601. To reproduce the problem: - Set your time preference to `""` (the empty string). This isn't possible from the modern UI, but can be done with "Right Click > Inspect Element", or users may have carried it forward from an older setting (this is the case with me and @hach-que on this install). - Load Calendar with some events. - This parses an epoch, which sets `valueTime` to `""` (since there are no format characters in the preference) and then `getEpoch()` fails because `strlen($time)` is 0. - Since `getEpoch()` failed, `getDateTime()` also fails. To fix this: - Only permit the date and time preferences to have valid values. Test Plan: - Loaded page before patch, saw fatal. - Applied patch. - No more fatal. - Viewed tooltips, dates/times, dates/times in other apps. - Changed my preferences, saw them respected. Reviewers: lpriestley Reviewed By: lpriestley Subscribers: epriestley, hach-que Maniphest Tasks: T8601 Differential Revision: https://secure.phabricator.com/D13346 --- src/__phutil_library_map__.php | 1 - .../people/storage/PhabricatorUser.php | 35 +++++++++++++ .../form/control/AphrontFormDateControl.php | 14 ++--- .../control/AphrontFormDateControlValue.php | 51 +++++++++---------- .../phui/calendar/PHUICalendarListView.php | 7 +-- src/view/viewutils.php | 19 ++----- 6 files changed, 68 insertions(+), 59 deletions(-) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index fc465aeea6..5d9b7ad113 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3290,7 +3290,6 @@ phutil_register_library_map(array( 'phabricator_format_local_time' => 'view/viewutils.php', 'phabricator_relative_date' => 'view/viewutils.php', 'phabricator_time' => 'view/viewutils.php', - 'phabricator_time_format' => 'view/viewutils.php', 'phid_get_subtype' => 'applications/phid/utils.php', 'phid_get_type' => 'applications/phid/utils.php', 'phid_group_by_type' => 'applications/phid/utils.php', diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 55ade255a7..b4985bfaa5 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -738,6 +738,41 @@ final class PhabricatorUser return new DateTimeZone($this->getTimezoneIdentifier()); } + public function getPreference($key) { + $preferences = $this->loadPreferences(); + + // TODO: After T4103 and T7707 this should eventually be pushed down the + // stack into modular preference definitions and role profiles. This is + // just fixing T8601 and mildly anticipating those changes. + $value = $preferences->getPreference($key); + + $allowed_values = null; + switch ($key) { + case PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT: + $allowed_values = array( + 'g:i A', + 'H:i', + ); + break; + case PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT: + $allowed_values = array( + 'Y-m-d', + 'n/j/Y', + 'd-m-Y', + ); + break; + } + + if ($allowed_values !== null) { + $allowed_values = array_fuse($allowed_values); + if (empty($allowed_values[$value])) { + $value = head($allowed_values); + } + } + + return $value; + } + public function __toString() { return $this->getUsername(); } diff --git a/src/view/form/control/AphrontFormDateControl.php b/src/view/form/control/AphrontFormDateControl.php index 20172593e9..ca803c9bd1 100644 --- a/src/view/form/control/AphrontFormDateControl.php +++ b/src/view/form/control/AphrontFormDateControl.php @@ -137,19 +137,13 @@ final class AphrontFormDateControl extends AphrontFormControl { } private function getTimeFormat() { - $viewer = $this->getUser(); - $preferences = $viewer->loadPreferences(); - $pref_time_format = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT; - - return $preferences->getPreference($pref_time_format, 'g:i A'); + return $this->getUser() + ->getPreference(PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT); } private function getDateFormat() { - $viewer = $this->getUser(); - $preferences = $viewer->loadPreferences(); - $pref_date_format = PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT; - - return $preferences->getPreference($pref_date_format, 'Y-m-d'); + return $this->getUser() + ->getPreference(PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT); } private function getTimeInputValue() { diff --git a/src/view/form/control/AphrontFormDateControlValue.php b/src/view/form/control/AphrontFormDateControlValue.php index c5d7e81852..f533bcaddb 100644 --- a/src/view/form/control/AphrontFormDateControlValue.php +++ b/src/view/form/control/AphrontFormDateControlValue.php @@ -10,7 +10,6 @@ final class AphrontFormDateControlValue extends Phobject { private $zone; private $optional; - public function getValueDate() { return $this->valueDate; } @@ -56,6 +55,10 @@ final class AphrontFormDateControlValue extends Phobject { return $this->optional; } + public function getViewer() { + return $this->viewer; + } + public static function newFromParts( PhabricatorUser $viewer, $year, @@ -71,8 +74,7 @@ final class AphrontFormDateControlValue extends Phobject { $year, $month, $day, - coalesce($time, '12:00 AM'), - $value); + coalesce($time, '12:00 AM')); $value->valueEnabled = $enabled; return $value; @@ -85,8 +87,7 @@ final class AphrontFormDateControlValue extends Phobject { list($value->valueDate, $value->valueTime) = $value->getFormattedDateFromDate( $request->getStr($key.'_d'), - $request->getStr($key.'_t'), - $value); + $request->getStr($key.'_t')); $value->valueEnabled = $request->getStr($key.'_e'); return $value; @@ -108,8 +109,7 @@ final class AphrontFormDateControlValue extends Phobject { $year, $month, $day, - $time, - $value); + $time); return $value; } @@ -123,8 +123,7 @@ final class AphrontFormDateControlValue extends Phobject { list($value->valueDate, $value->valueTime) = $value->getFormattedDateFromDate( idx($dictionary, 'd'), - idx($dictionary, 't'), - $value); + idx($dictionary, 't')); $value->valueEnabled = idx($dictionary, 'e'); @@ -205,29 +204,25 @@ final class AphrontFormDateControlValue extends Phobject { } private function getTimeFormat() { - $preferences = $this->viewer->loadPreferences(); - $pref_time_format = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT; - - return $preferences->getPreference($pref_time_format, 'g:i A'); + return $this->getViewer() + ->getPreference(PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT); } private function getDateFormat() { - $preferences = $this->viewer->loadPreferences(); - $pref_date_format = PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT; - - return $preferences->getPreference($pref_date_format, 'Y-m-d'); + return $this->getViewer() + ->getPreference(PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT); } - private function getFormattedDateFromDate($date, $time, $value) { + private function getFormattedDateFromDate($date, $time) { $original_input = $date; - $zone = $value->getTimezone(); - $separator = $value->getFormatSeparator(); + $zone = $this->getTimezone(); + $separator = $this->getFormatSeparator(); $parts = preg_split('@[,./:-]@', $date); $date = implode($separator, $parts); $date = id(new DateTime($date, $zone)); if ($date) { - $date = $date->format($value->getDateFormat()); + $date = $date->format($this->getDateFormat()); } else { $date = $original_input; } @@ -235,8 +230,8 @@ final class AphrontFormDateControlValue extends Phobject { $date = id(new DateTime("{$date} {$time}", $zone)); return array( - $date->format($value->getDateFormat()), - $date->format($value->getTimeFormat()), + $date->format($this->getDateFormat()), + $date->format($this->getTimeFormat()), ); } @@ -244,14 +239,14 @@ final class AphrontFormDateControlValue extends Phobject { $year, $month, $day, - $time, - $value) { - $zone = $value->getTimezone(); + $time) { + + $zone = $this->getTimezone(); $date_time = id(new DateTime("{$year}-{$month}-{$day} {$time}", $zone)); return array( - $date_time->format($value->getDateFormat()), - $date_time->format($value->getTimeFormat()), + $date_time->format($this->getDateFormat()), + $date_time->format($this->getTimeFormat()), ); } diff --git a/src/view/phui/calendar/PHUICalendarListView.php b/src/view/phui/calendar/PHUICalendarListView.php index 80468dcb62..4de84ce3fd 100644 --- a/src/view/phui/calendar/PHUICalendarListView.php +++ b/src/view/phui/calendar/PHUICalendarListView.php @@ -141,11 +141,8 @@ final class PHUICalendarListView extends AphrontTagView { } private function getEventTooltip(AphrontCalendarEventView $event) { - $viewer = $this->getUser(); - $preferences = $viewer->loadPreferences(); - $time_pref = $preferences->getPreference( - PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT, - 'g:i A'); + $time_pref = $this->getUser() + ->getPreference(PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT); Javelin::initBehavior('phabricator-tooltips'); diff --git a/src/view/viewutils.php b/src/view/viewutils.php index 990c811c84..8d57cd02d0 100644 --- a/src/view/viewutils.php +++ b/src/view/viewutils.php @@ -31,32 +31,21 @@ function phabricator_relative_date($epoch, $user, $on = false) { } function phabricator_time($epoch, $user) { + $time_key = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT; return phabricator_format_local_time( $epoch, $user, - phabricator_time_format($user)); + $user->getPreference($time_key)); } function phabricator_datetime($epoch, $user) { + $time_key = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT; return phabricator_format_local_time( $epoch, $user, pht('%s, %s', phutil_date_format($epoch), - phabricator_time_format($user))); -} - -function phabricator_time_format($user) { - $prefs = $user->loadPreferences(); - - $pref = $prefs->getPreference( - PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT); - - if (strlen($pref)) { - return $pref; - } - - return pht('g:i A'); + $user->getPreference($time_key))); } /** From 1851ca2d71aa65e99d536ac6f9369eb81e4b213f Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 18 Jun 2015 17:16:37 -0700 Subject: [PATCH 12/17] Make owners typeahead mostly reasonable Summary: Ref T8320. Fixes T8427. This is still a little funky because Owners has weird name rules, but should fix the bugs (unselectable packages) in T8427. Test Plan: Browsed Owners typaheads, used various search functions. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8320, T8427 Differential Revision: https://secure.phabricator.com/D13349 --- .../query/PhabricatorOwnersPackageQuery.php | 113 +++++++++++++----- .../PhabricatorOwnersPackageSearchEngine.php | 75 ++++-------- .../PhabricatorOwnersPackageDatasource.php | 14 +-- 3 files changed, 110 insertions(+), 92 deletions(-) diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php index 64ed2f4098..1d34d7be58 100644 --- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php @@ -7,6 +7,7 @@ final class PhabricatorOwnersPackageQuery private $phids; private $ownerPHIDs; private $repositoryPHIDs; + private $namePrefix; /** * Owners are direct owners, and members of owning projects. @@ -31,62 +32,59 @@ final class PhabricatorOwnersPackageQuery return $this; } - protected function loadPage() { - $table = new PhabricatorOwnersPackage(); - $conn_r = $table->establishConnection('r'); - - $data = queryfx_all( - $conn_r, - 'SELECT p.* FROM %T p %Q %Q %Q %Q', - $table->getTableName(), - $this->buildJoinClause($conn_r), - $this->buildWhereClause($conn_r), - $this->buildOrderClause($conn_r), - $this->buildLimitClause($conn_r)); - - return $table->loadAllFromArray($data); + public function withNamePrefix($prefix) { + $this->namePrefix = $prefix; + return $this; } - protected function buildJoinClause(AphrontDatabaseConnection $conn_r) { - $joins = array(); + public function newResultObject() { + return new PhabricatorOwnersPackage(); + } + + protected function loadPage() { + return $this->loadStandardPage(new PhabricatorOwnersPackage()); + } + + protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { + $joins = parent::buildJoinClauseParts($conn); if ($this->ownerPHIDs !== null) { $joins[] = qsprintf( - $conn_r, + $conn, 'JOIN %T o ON o.packageID = p.id', id(new PhabricatorOwnersOwner())->getTableName()); } if ($this->repositoryPHIDs !== null) { $joins[] = qsprintf( - $conn_r, + $conn, 'JOIN %T rpath ON rpath.packageID = p.id', id(new PhabricatorOwnersPath())->getTableName()); } - return implode(' ', $joins); + return $joins; } - protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { - $where = array(); + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); if ($this->phids !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'p.phid IN (%Ls)', $this->phids); } if ($this->ids !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'p.id IN (%Ld)', $this->ids); } if ($this->repositoryPHIDs !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'rpath.repositoryPHID IN (%Ls)', $this->repositoryPHIDs); } @@ -94,26 +92,79 @@ final class PhabricatorOwnersPackageQuery if ($this->ownerPHIDs !== null) { $base_phids = $this->ownerPHIDs; - $query = new PhabricatorProjectQuery(); - $query->setViewer($this->getViewer()); - $query->withMemberPHIDs($base_phids); - $projects = $query->execute(); + $projects = id(new PhabricatorProjectQuery()) + ->setViewer($this->getViewer()) + ->withMemberPHIDs($base_phids) + ->execute(); $project_phids = mpull($projects, 'getPHID'); $all_phids = array_merge($base_phids, $project_phids); $where[] = qsprintf( - $conn_r, + $conn, 'o.userPHID IN (%Ls)', $all_phids); } - $where[] = $this->buildPagingClause($conn_r); - return $this->formatWhereClause($where); + if (strlen($this->namePrefix)) { + // NOTE: This is a hacky mess, but this column is currently case + // sensitive and unique. + $where[] = qsprintf( + $conn, + 'LOWER(p.name) LIKE %>', + phutil_utf8_strtolower($this->namePrefix)); + } + + return $where; + } + + protected function shouldGroupQueryResultRows() { + if ($this->repositoryPHIDs) { + return true; + } + + if ($this->ownerPHIDs) { + return true; + } + + return parent::shouldGroupQueryResultRows(); + } + + public function getBuiltinOrders() { + return array( + 'name' => array( + 'vector' => array('name'), + 'name' => pht('Name'), + ), + ) + parent::getBuiltinOrders(); + } + + public function getOrderableColumns() { + return parent::getOrderableColumns() + array( + 'name' => array( + 'table' => $this->getPrimaryTableAlias(), + 'column' => 'name', + 'type' => 'string', + 'unique' => true, + 'reverse' => true, + ), + ); + } + + protected function getPagingValueMap($cursor, array $keys) { + $package = $this->loadCursorObject($cursor); + return array( + 'id' => $package->getID(), + 'name' => $package->getName(), + ); } public function getQueryApplicationClass() { return 'PhabricatorOwnersApplication'; } + protected function getPrimaryTableAlias() { + return 'p'; + } + } diff --git a/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php b/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php index f421e3d3ce..7998076e17 100644 --- a/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php @@ -11,68 +11,39 @@ final class PhabricatorOwnersPackageSearchEngine return 'PhabricatorOwnersApplication'; } - public function buildSavedQueryFromRequest(AphrontRequest $request) { - $saved = new PhabricatorSavedQuery(); - - $saved->setParameter( - 'ownerPHIDs', - $this->readUsersFromRequest( - $request, - 'owners', - array( - PhabricatorProjectProjectPHIDType::TYPECONST, - ))); - - $saved->setParameter( - 'repositoryPHIDs', - $this->readPHIDsFromRequest( - $request, - 'repositories', - array( - PhabricatorRepositoryRepositoryPHIDType::TYPECONST, - ))); - - return $saved; + public function newQuery() { + return new PhabricatorOwnersPackageQuery(); } - public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { - $query = id(new PhabricatorOwnersPackageQuery()); + protected function buildCustomSearchFields() { + return array( + id(new PhabricatorSearchDatasourceField()) + ->setLabel(pht('Owners')) + ->setKey('ownerPHIDs') + ->setAliases(array('owner', 'owners')) + ->setDatasource(new PhabricatorProjectOrUserDatasource()), + id(new PhabricatorSearchDatasourceField()) + ->setLabel(pht('Repositories')) + ->setKey('repositoryPHIDs') + ->setAliases(array('repository', 'repositories')) + ->setDatasource(new DiffusionRepositoryDatasource()), + ); + } - $owner_phids = $saved->getParameter('ownerPHIDs', array()); - if ($owner_phids) { - $query->withOwnerPHIDs($owner_phids); + protected function buildQueryFromParameters(array $map) { + $query = $this->newQuery(); + + if ($map['ownerPHIDs']) { + $query->withOwnerPHIDs($map['ownerPHIDs']); } - $repository_phids = $saved->getParameter('repositoryPHIDs', array()); - if ($repository_phids) { - $query->withRepositoryPHIDs($repository_phids); + if ($map['repositoryPHIDs']) { + $query->withRepositoryPHIDs($map['repositoryPHIDs']); } return $query; } - public function buildSearchForm( - AphrontFormView $form, - PhabricatorSavedQuery $saved) { - - $owner_phids = $saved->getParameter('ownerPHIDs', array()); - $repository_phids = $saved->getParameter('repositoryPHIDs', array()); - - $form - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorProjectOrUserDatasource()) - ->setName('owners') - ->setLabel(pht('Owners')) - ->setValue($owner_phids)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setDatasource(new DiffusionRepositoryDatasource()) - ->setName('repositories') - ->setLabel(pht('Repositories')) - ->setValue($repository_phids)); - } - protected function getURI($path) { return '/owners/'.$path; } diff --git a/src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php b/src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php index 73105f0908..061cf91b64 100644 --- a/src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php +++ b/src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php @@ -3,11 +3,6 @@ final class PhabricatorOwnersPackageDatasource extends PhabricatorTypeaheadDatasource { - public function isBrowsable() { - // TODO: Make this browsable. - return false; - } - public function getBrowseTitle() { return pht('Browse Packages'); } @@ -26,10 +21,11 @@ final class PhabricatorOwnersPackageDatasource $results = array(); - $packages = id(new PhabricatorOwnersPackageQuery()) - ->setViewer($viewer) - ->execute(); + $query = id(new PhabricatorOwnersPackageQuery()) + ->withNamePrefix($raw_query) + ->setOrder('name'); + $packages = $this->executeQuery($query); foreach ($packages as $package) { $results[] = id(new PhabricatorTypeaheadResult()) ->setName($package->getName()) @@ -37,7 +33,7 @@ final class PhabricatorOwnersPackageDatasource ->setPHID($package->getPHID()); } - return $results; + return $this->filterResultsAgainstTokens($results); } } From 68bb60e52f92e5186ab9c0e39284c0843cf74eb4 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Fri, 19 Jun 2015 13:09:21 +1000 Subject: [PATCH 13/17] Use PhutilConsoleTable in `./bin/storage probe` Summary: Fixes T8477. Use `PhutilConsoleTable` to render the output from `./bin/storage probe`. Test Plan: Ran `./bin/storage probe` and saw tabulated output. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Maniphest Tasks: T8477 Differential Revision: https://secure.phabricator.com/D13339 --- ...bricatorStorageManagementProbeWorkflow.php | 59 +++++++++++++------ 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php index a4ef562c37..d2fbc95326 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php @@ -45,34 +45,57 @@ final class PhabricatorStorageManagementProbeWorkflow } } - $console->writeOut("%s\n", pht('APPROXIMATE TABLE SIZES')); asort($totals); + + $table = id(new PhutilConsoleTable()) + ->setShowHeader(false) + ->setPadding(2) + ->addColumn('name', array('title' => pht('Database / Table'))) + ->addColumn('size', array('title' => pht('Size'))) + ->addColumn('percentage', array('title' => pht('Percentage'))); + foreach ($totals as $db => $size) { - $database_size = $this->formatSize($totals[$db], $overall); - $console->writeOut( - "**%s**\n", - sprintf('%-32.32s %18s', $db, $database_size)); + list($database_size, $database_percentage) = $this->formatSize( + $totals[$db], + $overall); + + $table->addRow(array( + 'name' => phutil_console_format('**%s**', $db), + 'size' => phutil_console_format('**%s**', $database_size), + 'percentage' => phutil_console_format('**%s**', $database_percentage), + )); $data[$db] = isort($data[$db], '_totalSize'); - foreach ($data[$db] as $table => $info) { - $table_size = $this->formatSize($info['_totalSize'], $overall); - $console->writeOut( - "%s\n", - sprintf(' %-28.28s %18s', $table, $table_size)); + foreach ($data[$db] as $table_name => $info) { + list($table_size, $table_percentage) = $this->formatSize( + $info['_totalSize'], + $overall); + + $table->addRow(array( + 'name' => ' '.$table_name, + 'size' => $table_size, + 'percentage' => $table_percentage, + )); } } - $overall_size = $this->formatSize($overall, $overall); - $console->writeOut( - "**%s**\n", - sprintf('%-32.32s %18s', pht('TOTAL'), $overall_size)); + list($overall_size, $overall_percentage) = $this->formatSize( + $overall, + $overall); + $table->addRow(array( + 'name' => phutil_console_format('**%s**', pht('TOTAL')), + 'size' => phutil_console_format('**%s**', $overall_size), + 'percentage' => phutil_console_format('**%s**', $overall_percentage), + )); + + $table->draw(); return 0; } private function formatSize($n, $o) { - return sprintf( - '%8.8s MB %5.5s%%', - number_format($n / (1024 * 1024), 1), - sprintf('%3.1f', 100 * ($n / $o))); + return array( + sprintf('%8.8s MB', number_format($n / (1024 * 1024), 1)), + sprintf('%3.1f%%', 100 * ($n / $o)), + ); } } From 827aa05a676e5973e93b294cccc604aa8dbfefa7 Mon Sep 17 00:00:00 2001 From: J Rhodes Date: Fri, 19 Jun 2015 15:05:39 +1000 Subject: [PATCH 14/17] Drop Windows-specific SSH code Summary: Ref T2015. This code is only relevant when attempting to run commands on a Windows host over SSH. Since SSH on Windows is extremely fragile and hard to maintain, and WinRM is a better long-term solution, drop this code (which will end up being unused when later diffs introduce the WinRM command interface). Test Plan: This code won't be used when D10495 lands. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Projects: #drydock Maniphest Tasks: T2015 Differential Revision: https://secure.phabricator.com/D13340 --- .../command/DrydockSSHCommandInterface.php | 42 +------------------ 1 file changed, 2 insertions(+), 40 deletions(-) diff --git a/src/applications/drydock/interface/command/DrydockSSHCommandInterface.php b/src/applications/drydock/interface/command/DrydockSSHCommandInterface.php index 362e60e3c0..d75dcfb780 100644 --- a/src/applications/drydock/interface/command/DrydockSSHCommandInterface.php +++ b/src/applications/drydock/interface/command/DrydockSSHCommandInterface.php @@ -42,46 +42,8 @@ final class DrydockSSHCommandInterface extends DrydockCommandInterface { $this->openCredentialsIfNotOpen(); $argv = func_get_args(); - - if ($this->getConfig('platform') === 'windows') { - // Handle Windows by executing the command under PowerShell. - $command = id(new PhutilCommandString($argv)) - ->setEscapingMode(PhutilCommandString::MODE_POWERSHELL); - - $change_directory = ''; - if ($this->getWorkingDirectory() !== null) { - $change_directory .= 'cd '.$this->getWorkingDirectory(); - } - - $script = <<applyWorkingDirectoryToArgv($argv); - - $full_command = call_user_func_array('csprintf', $argv); - } + $argv = $this->applyWorkingDirectoryToArgv($argv); + $full_command = call_user_func_array('csprintf', $argv); $command_timeout = ''; if ($this->connectTimeout !== null) { From 70a82017b3b9ca43f38178a09f0269bb3d039676 Mon Sep 17 00:00:00 2001 From: J Rhodes Date: Fri, 19 Jun 2015 15:06:32 +1000 Subject: [PATCH 15/17] Drop Windows-specific escaping in preallocated host Summary: This drops the Windows-specific escaping code for the creation of directories when acquiring a lease. This is basically the change from D10378 without the other, no longer necessary changes. Test Plan: This code hasn't been run in a production environment for a while (any instances of Phabricator using Drydock / Harbormaster with Windows have had this code removed by the D10378 patch for a while). Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Projects: #drydock Differential Revision: https://secure.phabricator.com/D13341 --- ...ockPreallocatedHostBlueprintImplementation.php | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php index af6a9e5f2f..9e5d3a2320 100644 --- a/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php @@ -77,20 +77,7 @@ final class DrydockPreallocatedHostBlueprintImplementation $cmd = $lease->getInterface('command'); - if ($v_platform !== 'windows') { - $cmd->execx('mkdir %s', $full_path); - } else { - // Windows is terrible. The mkdir command doesn't even support putting - // the path in quotes. IN QUOTES. ARGUHRGHUGHHGG!! Do some terribly - // inaccurate sanity checking since we can't safely escape the path. - if (preg_match('/^[A-Z]\\:\\\\[a-zA-Z0-9\\\\\\ ]/', $full_path) === 0) { - throw new Exception( - pht( - 'Unsafe path detected for Windows platform: "%s".', - $full_path)); - } - $cmd->execx('mkdir %C', $full_path); - } + $cmd->execx('mkdir %s', $full_path); $lease->setAttribute('path', $full_path); } From 69d12f64baf90133df499c055deb7db9882c5025 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Fri, 19 Jun 2015 17:24:23 +1000 Subject: [PATCH 16/17] Add repositories to Diviner Summary: Fixes T8352. Associate Diviner books and atoms with a repository. This relationship is not really surfaced anywhere in the UI but provides metadata that contextualises search results. Depends on D13091. Test Plan: Ran `diviner generate --repository ARC` and then went to `/diviner/book/arcanist/`. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7703, T8352 Differential Revision: https://secure.phabricator.com/D13070 --- .../20150616.divinerrepository.sql | 5 ++ .../controller/DivinerAtomController.php | 10 ++++ .../controller/DivinerBookController.php | 10 ++++ .../controller/DivinerBookEditController.php | 10 ++++ .../publisher/DivinerLivePublisher.php | 28 +++++++++-- .../diviner/publisher/DivinerPublisher.php | 10 ++++ .../diviner/query/DivinerAtomQuery.php | 46 +++++++++++++++++++ .../diviner/query/DivinerAtomSearchEngine.php | 37 ++++++++++----- .../diviner/query/DivinerBookQuery.php | 44 ++++++++++++++++++ .../search/DivinerAtomSearchIndexer.php | 6 +++ .../search/DivinerBookSearchIndexer.php | 6 +++ .../diviner/storage/DivinerLiveBook.php | 14 +++++- .../diviner/storage/DivinerLiveSymbol.php | 12 +++++ .../workflow/DivinerGenerateWorkflow.php | 23 ++++++++++ .../storage/PhabricatorRepository.php | 20 +++++++- 15 files changed, 262 insertions(+), 19 deletions(-) create mode 100644 resources/sql/autopatches/20150616.divinerrepository.sql diff --git a/resources/sql/autopatches/20150616.divinerrepository.sql b/resources/sql/autopatches/20150616.divinerrepository.sql new file mode 100644 index 0000000000..8e6875c7d9 --- /dev/null +++ b/resources/sql/autopatches/20150616.divinerrepository.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_diviner.diviner_livebook + ADD COLUMN repositoryPHID VARBINARY(64) AFTER name; + +ALTER TABLE {$NAMESPACE}_diviner.diviner_livesymbol + ADD COLUMN repositoryPHID VARBINARY(64) AFTER bookPHID; diff --git a/src/applications/diviner/controller/DivinerAtomController.php b/src/applications/diviner/controller/DivinerAtomController.php index 8cfeb8e42a..f5b5b6d9e1 100644 --- a/src/applications/diviner/controller/DivinerAtomController.php +++ b/src/applications/diviner/controller/DivinerAtomController.php @@ -85,6 +85,7 @@ final class DivinerAtomController extends DivinerController { if ($atom) { $this->buildDefined($properties, $symbol); $this->buildExtendsAndImplements($properties, $symbol); + $this->buildRepository($properties, $symbol); $warnings = $atom->getWarnings(); if ($warnings) { @@ -295,6 +296,15 @@ final class DivinerAtomController extends DivinerController { } } + private function buildRepository( + PHUIPropertyListView $view, + DivinerLiveSymbol $symbol) { + + $view->addProperty( + pht('Repository'), + $this->getViewer()->renderHandle($symbol->getRepositoryPHID())); + } + private function renderAtomTag(DivinerLiveSymbol $symbol) { return id(new PHUITagView()) ->setType(PHUITagView::TYPE_OBJECT) diff --git a/src/applications/diviner/controller/DivinerBookController.php b/src/applications/diviner/controller/DivinerBookController.php index f8d222b116..a0e2f0cf51 100644 --- a/src/applications/diviner/controller/DivinerBookController.php +++ b/src/applications/diviner/controller/DivinerBookController.php @@ -14,6 +14,7 @@ final class DivinerBookController extends DivinerController { $book = id(new DivinerBookQuery()) ->setViewer($viewer) ->withNames(array($book_name)) + ->needRepositories(true) ->executeOne(); if (!$book) { @@ -43,6 +44,15 @@ final class DivinerBookController extends DivinerController { ->setEpoch($book->getDateModified()) ->addActionLink($action_button); + // TODO: This could probably look better. + if ($book->getRepositoryPHID()) { + $header->addTag( + id(new PHUITagView()) + ->setType(PHUITagView::TYPE_STATE) + ->setBackgroundColor(PHUITagView::COLOR_BLUE) + ->setName($book->getRepository()->getMonogram())); + } + $document = new PHUIDocumentView(); $document->setHeader($header); $document->addClass('diviner-view'); diff --git a/src/applications/diviner/controller/DivinerBookEditController.php b/src/applications/diviner/controller/DivinerBookEditController.php index 3ac8c4fe2e..b5c3e2dea5 100644 --- a/src/applications/diviner/controller/DivinerBookEditController.php +++ b/src/applications/diviner/controller/DivinerBookEditController.php @@ -75,6 +75,16 @@ final class DivinerBookEditController extends DivinerController { ->setName('projectPHIDs') ->setLabel(pht('Projects')) ->setValue($book->getProjectPHIDs())) + ->appendControl( + id(new AphrontFormTokenizerControl()) + ->setDatasource(new DiffusionRepositoryDatasource()) + ->setName('repositoryPHIDs') + ->setLabel(pht('Repository')) + ->setDisableBehavior(true) + ->setLimit(1) + ->setValue($book->getRepositoryPHID() + ? array($book->getRepositoryPHID()) + : null)) ->appendChild( id(new AphrontFormPolicyControl()) ->setName('viewPolicy') diff --git a/src/applications/diviner/publisher/DivinerLivePublisher.php b/src/applications/diviner/publisher/DivinerLivePublisher.php index cdb3fe1936..4af0a68c8e 100644 --- a/src/applications/diviner/publisher/DivinerLivePublisher.php +++ b/src/applications/diviner/publisher/DivinerLivePublisher.php @@ -4,7 +4,7 @@ final class DivinerLivePublisher extends DivinerPublisher { private $book; - private function loadBook() { + protected function getBook() { if (!$this->book) { $book_name = $this->getConfig('name'); @@ -20,7 +20,24 @@ final class DivinerLivePublisher extends DivinerPublisher { ->save(); } - $book->setConfigurationData($this->getConfigurationData())->save(); + $conn_w = $book->establishConnection('w'); + $conn_w->openTransaction(); + + $book + ->setRepositoryPHID($this->getRepositoryPHID()) + ->setConfigurationData($this->getConfigurationData()) + ->save(); + + // TODO: This is gross. Without this, the repository won't be updated for + // atoms which have already been published. + queryfx( + $conn_w, + 'UPDATE %T SET repositoryPHID = %s WHERE bookPHID = %s', + id(new DivinerLiveSymbol())->getTableName(), + $this->getRepositoryPHID(), + $book->getPHID()); + + $conn_w->saveTransaction(); $this->book = $book; id(new PhabricatorSearchIndexer()) @@ -33,7 +50,7 @@ final class DivinerLivePublisher extends DivinerPublisher { private function loadSymbolForAtom(DivinerAtom $atom) { $symbol = id(new DivinerAtomQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withBookPHIDs(array($this->loadBook()->getPHID())) + ->withBookPHIDs(array($atom->getBook())) ->withTypes(array($atom->getType())) ->withNames(array($atom->getName())) ->withContexts(array($atom->getContext())) @@ -45,7 +62,7 @@ final class DivinerLivePublisher extends DivinerPublisher { } return id(new DivinerLiveSymbol()) - ->setBookPHID($this->loadBook()->getPHID()) + ->setBookPHID($this->getBook()->getPHID()) ->setType($atom->getType()) ->setName($atom->getName()) ->setContext($atom->getContext()) @@ -68,7 +85,7 @@ final class DivinerLivePublisher extends DivinerPublisher { protected function loadAllPublishedHashes() { $symbols = id(new DivinerAtomQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withBookPHIDs(array($this->loadBook()->getPHID())) + ->withBookPHIDs(array($this->getBook()->getPHID())) ->withGhosts(false) ->execute(); @@ -113,6 +130,7 @@ final class DivinerLivePublisher extends DivinerPublisher { $is_documentable = $this->shouldGenerateDocumentForAtom($atom); $symbol + ->setRepositoryPHID($this->getRepositoryPHID()) ->setGraphHash($hash) ->setIsDocumentable((int)$is_documentable) ->setTitle($ref->getTitle()) diff --git a/src/applications/diviner/publisher/DivinerPublisher.php b/src/applications/diviner/publisher/DivinerPublisher.php index 4b6490639d..401a1a331f 100644 --- a/src/applications/diviner/publisher/DivinerPublisher.php +++ b/src/applications/diviner/publisher/DivinerPublisher.php @@ -9,6 +9,7 @@ abstract class DivinerPublisher extends Phobject { private $config; private $symbolReverseMap; private $dropCaches; + private $repositoryPHID; final public function setDropCaches($drop_caches) { $this->dropCaches = $drop_caches; @@ -163,4 +164,13 @@ abstract class DivinerPublisher extends Phobject { return true; } + final public function getRepositoryPHID() { + return $this->repositoryPHID; + } + + final public function setRepositoryPHID($repository_phid) { + $this->repositoryPHID = $repository_phid; + return $this; + } + } diff --git a/src/applications/diviner/query/DivinerAtomQuery.php b/src/applications/diviner/query/DivinerAtomQuery.php index b8856141a5..c2a5247d78 100644 --- a/src/applications/diviner/query/DivinerAtomQuery.php +++ b/src/applications/diviner/query/DivinerAtomQuery.php @@ -14,10 +14,12 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { private $nodeHashes; private $titles; private $nameContains; + private $repositoryPHIDs; private $needAtoms; private $needExtends; private $needChildren; + private $needRepositories; public function withIDs(array $ids) { $this->ids = $ids; @@ -109,6 +111,16 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $this; } + public function withRepositoryPHIDs(array $repository_phids) { + $this->repositoryPHIDs = $repository_phids; + return $this; + } + + public function needRepositories($need_repositories) { + $this->needRepositories = $need_repositories; + return $this; + } + protected function loadPage() { $table = new DivinerLiveSymbol(); $conn_r = $table->establishConnection('r'); @@ -125,6 +137,8 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { } protected function willFilterPage(array $atoms) { + assert_instances_of($atoms, 'DivinerLiveSymbol'); + $books = array_unique(mpull($atoms, 'getBookPHID')); $books = id(new DivinerBookQuery()) @@ -257,6 +271,31 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { $this->attachAllChildren($atoms, $children, $this->needExtends); } + if ($this->needRepositories) { + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs(mpull($atoms, 'getRepositoryPHID')) + ->execute(); + $repositories = mpull($repositories, null, 'getPHID'); + + foreach ($atoms as $key => $atom) { + if ($atom->getRepositoryPHID() === null) { + $atom->attachRepository(null); + continue; + } + + $repository = idx($repositories, $atom->getRepositoryPHID()); + + if (!$repository) { + $this->didRejectResult($atom); + unset($atom[$key]); + continue; + } + + $atom->attachRepository($repository); + } + } + return $atoms; } @@ -381,6 +420,13 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { $this->nameContains); } + if ($this->repositoryPHIDs) { + $where[] = qsprintf( + $conn_r, + 'repositoryPHID IN (%Ls)', + $this->repositoryPHIDs); + } + $where[] = $this->buildPagingClause($conn_r); return $this->formatWhereClause($where); diff --git a/src/applications/diviner/query/DivinerAtomSearchEngine.php b/src/applications/diviner/query/DivinerAtomSearchEngine.php index d1c0f734ed..70f77f3195 100644 --- a/src/applications/diviner/query/DivinerAtomSearchEngine.php +++ b/src/applications/diviner/query/DivinerAtomSearchEngine.php @@ -13,21 +13,23 @@ final class DivinerAtomSearchEngine extends PhabricatorApplicationSearchEngine { public function buildSavedQueryFromRequest(AphrontRequest $request) { $saved = new PhabricatorSavedQuery(); + $saved->setParameter( + 'repositoryPHIDs', + $this->readPHIDsFromRequest($request, 'repositoryPHIDs')); + $saved->setParameter('name', $request->getStr('name')); $saved->setParameter( 'types', $this->readListFromRequest($request, 'types')); - $saved->setParameter('name', $request->getStr('name')); - return $saved; } public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { $query = id(new DivinerAtomQuery()); - $types = $saved->getParameter('types'); - if ($types) { - $query->withTypes($types); + $repository_phids = $saved->getParameter('repositoryPHIDs'); + if ($repository_phids) { + $query->withRepositoryPHIDs($repository_phids); } $name = $saved->getParameter('name'); @@ -35,6 +37,11 @@ final class DivinerAtomSearchEngine extends PhabricatorApplicationSearchEngine { $query->withNameContains($name); } + $types = $saved->getParameter('types'); + if ($types) { + $query->withTypes($types); + } + return $query; } @@ -42,6 +49,12 @@ final class DivinerAtomSearchEngine extends PhabricatorApplicationSearchEngine { AphrontFormView $form, PhabricatorSavedQuery $saved) { + $form->appendChild( + id(new AphrontFormTextControl()) + ->setLabel(pht('Name Contains')) + ->setName('name') + ->setValue($saved->getParameter('name'))); + $all_types = array(); foreach (DivinerAtom::getAllTypes() as $type) { $all_types[$type] = DivinerAtom::getAtomTypeNameString($type); @@ -59,14 +72,14 @@ final class DivinerAtomSearchEngine extends PhabricatorApplicationSearchEngine { $name, isset($types[$type])); } + $form->appendChild($type_control); - $form - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel(pht('Name Contains')) - ->setName('name') - ->setValue($saved->getParameter('name'))) - ->appendChild($type_control); + $form->appendControl( + id(new AphrontFormTokenizerControl()) + ->setLabel(pht('Repositories')) + ->setName('repositoryPHIDs') + ->setDatasource(new DiffusionRepositoryDatasource()) + ->setValue($saved->getParameter('repositoryPHIDs'))); } protected function getURI($path) { diff --git a/src/applications/diviner/query/DivinerBookQuery.php b/src/applications/diviner/query/DivinerBookQuery.php index b1f5f0a995..dfd236f3d1 100644 --- a/src/applications/diviner/query/DivinerBookQuery.php +++ b/src/applications/diviner/query/DivinerBookQuery.php @@ -5,8 +5,10 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { private $ids; private $phids; private $names; + private $repositoryPHIDs; private $needProjectPHIDs; + private $needRepositories; public function withIDs(array $ids) { $this->ids = $ids; @@ -23,11 +25,21 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $this; } + public function withRepositoryPHIDs(array $repository_phids) { + $this->repositoryPHIDs = $repository_phids; + return $this; + } + public function needProjectPHIDs($need_phids) { $this->needProjectPHIDs = $need_phids; return $this; } + public function needRepositories($need_repositories) { + $this->needRepositories = $need_repositories; + return $this; + } + protected function loadPage() { $table = new DivinerLiveBook(); $conn_r = $table->establishConnection('r'); @@ -46,6 +58,31 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { protected function didFilterPage(array $books) { assert_instances_of($books, 'DivinerLiveBook'); + if ($this->needRepositories) { + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs(mpull($books, 'getRepositoryPHID')) + ->execute(); + $repositories = mpull($repositories, null, 'getPHID'); + + foreach ($books as $key => $book) { + if ($book->getRepositoryPHID() === null) { + $book->attachRepository(null); + continue; + } + + $repository = idx($repositories, $book->getRepositoryPHID()); + + if (!$repository) { + $this->didRejectResult($book); + unset($books[$key]); + continue; + } + + $book->attachRepository($repository); + } + } + if ($this->needProjectPHIDs) { $edge_query = id(new PhabricatorEdgeQuery()) ->withSourcePHIDs(mpull($books, 'getPHID')) @@ -91,6 +128,13 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { $this->names); } + if ($this->repositoryPHIDs !== null) { + $where[] = qsprintf( + $conn_r, + 'repositoryPHID IN (%Ls)', + $this->repositoryPHIDs); + } + $where[] = $this->buildPagingClause($conn_r); return $this->formatWhereClause($where); diff --git a/src/applications/diviner/search/DivinerAtomSearchIndexer.php b/src/applications/diviner/search/DivinerAtomSearchIndexer.php index 51a3866641..0719f807f6 100644 --- a/src/applications/diviner/search/DivinerAtomSearchIndexer.php +++ b/src/applications/diviner/search/DivinerAtomSearchIndexer.php @@ -29,6 +29,12 @@ final class DivinerAtomSearchIndexer extends PhabricatorSearchDocumentIndexer { DivinerBookPHIDType::TYPECONST, PhabricatorTime::getNow()); + $doc->addRelationship( + PhabricatorSearchRelationship::RELATIONSHIP_REPOSITORY, + $atom->getRepositoryPHID(), + PhabricatorRepositoryRepositoryPHIDType::TYPECONST, + PhabricatorTime::getNow()); + $doc->addRelationship( $atom->getGraphHash() ? PhabricatorSearchRelationship::RELATIONSHIP_CLOSED diff --git a/src/applications/diviner/search/DivinerBookSearchIndexer.php b/src/applications/diviner/search/DivinerBookSearchIndexer.php index 106ae9e389..3cc819e275 100644 --- a/src/applications/diviner/search/DivinerBookSearchIndexer.php +++ b/src/applications/diviner/search/DivinerBookSearchIndexer.php @@ -18,6 +18,12 @@ final class DivinerBookSearchIndexer extends PhabricatorSearchDocumentIndexer { PhabricatorSearchDocumentFieldType::FIELD_BODY, $book->getPreface()); + $doc->addRelationship( + PhabricatorSearchRelationship::RELATIONSHIP_REPOSITORY, + $book->getRepositoryPHID(), + PhabricatorRepositoryRepositoryPHIDType::TYPECONST, + $book->getDateCreated()); + $this->indexTransactions( $doc, new DivinerLiveBookTransactionQuery(), diff --git a/src/applications/diviner/storage/DivinerLiveBook.php b/src/applications/diviner/storage/DivinerLiveBook.php index e689955171..872dcb6eeb 100644 --- a/src/applications/diviner/storage/DivinerLiveBook.php +++ b/src/applications/diviner/storage/DivinerLiveBook.php @@ -8,11 +8,13 @@ final class DivinerLiveBook extends DivinerDAO PhabricatorApplicationTransactionInterface { protected $name; + protected $repositoryPHID; protected $viewPolicy; protected $editPolicy; protected $configurationData = array(); private $projectPHIDs = self::ATTACHABLE; + private $repository = self::ATTACHABLE; protected function getConfiguration() { return array( @@ -22,6 +24,7 @@ final class DivinerLiveBook extends DivinerDAO ), self::CONFIG_COLUMN_SCHEMA => array( 'name' => 'text64', + 'repositoryPHID' => 'phid?', ), self::CONFIG_KEY_SCHEMA => array( 'key_phid' => null, @@ -68,6 +71,15 @@ final class DivinerLiveBook extends DivinerDAO return idx($spec, 'name', $group); } + public function attachRepository(PhabricatorRepository $repository = null) { + $this->repository = $repository; + return $this; + } + + public function getRepository() { + return $this->assertAttached($this->repository); + } + public function attachProjectPHIDs(array $project_phids) { $this->projectPHIDs = $project_phids; return $this; @@ -98,7 +110,7 @@ final class DivinerLiveBook extends DivinerDAO } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { - return false; + return false; } public function describeAutomaticCapability($capability) { diff --git a/src/applications/diviner/storage/DivinerLiveSymbol.php b/src/applications/diviner/storage/DivinerLiveSymbol.php index d6fbc710df..bdda976081 100644 --- a/src/applications/diviner/storage/DivinerLiveSymbol.php +++ b/src/applications/diviner/storage/DivinerLiveSymbol.php @@ -7,6 +7,7 @@ final class DivinerLiveSymbol extends DivinerDAO PhabricatorDestructibleInterface { protected $bookPHID; + protected $repositoryPHID; protected $context; protected $type; protected $name; @@ -22,6 +23,7 @@ final class DivinerLiveSymbol extends DivinerDAO protected $isDocumentable = 0; private $book = self::ATTACHABLE; + private $repository = self::ATTACHABLE; private $atom = self::ATTACHABLE; private $extends = self::ATTACHABLE; private $children = self::ATTACHABLE; @@ -43,6 +45,7 @@ final class DivinerLiveSymbol extends DivinerDAO 'summary' => 'text?', 'isDocumentable' => 'bool', 'nodeHash' => 'text64?', + 'repositoryPHID' => 'phid?', ), self::CONFIG_KEY_SCHEMA => array( 'key_phid' => null, @@ -94,6 +97,15 @@ final class DivinerLiveSymbol extends DivinerDAO return $this; } + public function getRepository() { + return $this->assertAttached($this->repository); + } + + public function attachRepository(PhabricatorRepository $repository = null) { + $this->repository = $repository; + return $this; + } + public function getAtom() { return $this->assertAttached($this->atom); } diff --git a/src/applications/diviner/workflow/DivinerGenerateWorkflow.php b/src/applications/diviner/workflow/DivinerGenerateWorkflow.php index a08180e4a1..a3847391c5 100644 --- a/src/applications/diviner/workflow/DivinerGenerateWorkflow.php +++ b/src/applications/diviner/workflow/DivinerGenerateWorkflow.php @@ -25,6 +25,11 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { 'help' => pht('Specify a subclass of %s.', 'DivinerPublisher'), 'default' => 'DivinerLivePublisher', ), + array( + 'name' => 'repository', + 'param' => 'callsign', + 'help' => pht('Repository that the documentation belongs to.'), + ), )); } @@ -187,6 +192,24 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { } $publisher = newv($publisher_class, array()); + $callsign = $args->getArg('repository'); + $repository = null; + if ($callsign) { + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withCallsigns(array($callsign)) + ->executeOne(); + + if (!$repository) { + throw new PhutilArgumentUsageException( + pht( + "Repository '%s' does not exist.", + $callsign)); + } + + $publisher->setRepositoryPHID($repository->getPHID()); + } + $this->publishDocumentation($args->getArg('clean'), $publisher); } diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 13dd10a878..12865a0e59 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -1913,7 +1913,25 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO PhabricatorDestructionEngine $engine) { $this->openTransaction(); - $this->delete(); + + $this->delete(); + + $books = id(new DivinerBookQuery()) + ->setViewer($engine->getViewer()) + ->withRepositoryPHIDs(array($this->getPHID())) + ->execute(); + foreach ($books as $book) { + $engine->destroyObject($book); + } + + $atoms = id(new DivinerAtomQuery()) + ->setViewer($engine->getViewer()) + ->withRepositoryPHIDs(array($this->getPHID())) + ->execute(); + foreach ($atoms as $atom) { + $engine->destroyObject($atom); + } + $this->saveTransaction(); } From 40851e5b2529438f6871f399735b82dcf9a5da55 Mon Sep 17 00:00:00 2001 From: Paul Kassianik Date: Fri, 19 Jun 2015 08:32:50 -0700 Subject: [PATCH 17/17] Adds Remarkup Rendering to Calendar Events' Descriptions. Summary: Closes T8032 Test Plan: Verify that when editing a calendar event's description, there is a UI bar helping with adding markup to the description. Also verify that markup is displayed correctly on the event page once the event has been updated. Reviewers: lpriestley, #blessed_reviewers, epriestley Reviewed By: lpriestley, #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T8032 Differential Revision: https://secure.phabricator.com/D13355 --- .../PhabricatorCalendarEventEditController.php | 5 +++-- .../PhabricatorCalendarEventViewController.php | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php b/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php index 47736f38fe..d374dc4a05 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php @@ -437,10 +437,11 @@ final class PhabricatorCalendarEventEditController ->setValue($end_disabled); } - $description = id(new AphrontFormTextAreaControl()) + $description = id(new PhabricatorRemarkupControl()) ->setLabel(pht('Description')) ->setName('description') - ->setValue($description); + ->setValue($description) + ->setUser($viewer); $view_policies = id(new AphrontFormPolicyControl()) ->setUser($viewer) diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php index b6eb901bb8..0aea0c5607 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php @@ -362,10 +362,19 @@ final class PhabricatorCalendarEventViewController pht('Icon'), $icon_display); - $properties->addSectionHeader( - pht('Description'), - PHUIPropertyListView::ICON_SUMMARY); - $properties->addTextContent($event->getDescription()); + if (strlen($event->getDescription())) { + + $description = PhabricatorMarkupEngine::renderOneObject( + id(new PhabricatorMarkupOneOff())->setContent($event->getDescription()), + 'default', + $viewer); + + $properties->addSectionHeader( + pht('Description'), + PHUIPropertyListView::ICON_SUMMARY); + + $properties->addTextContent($description); + } return $properties; }