From 4c124201623acfe27487faf715a4045cd72c5d6b Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 12 Feb 2019 10:45:33 -0800 Subject: [PATCH] Replace "URI->setQueryParams()" after initialization with a constructor argument Summary: Ref T13250. See D20149. In a number of cases, we use `setQueryParams()` immediately after URI construction. To simplify this slightly, let the constructor take parameters, similar to `HTTPSFuture`. Test Plan: See inlines. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13250 Differential Revision: https://secure.phabricator.com/D20151 --- .../almanac/controller/AlmanacController.php | 18 +++------ .../PhabricatorAuthOneTimeLoginController.php | 10 ++--- .../auth/future/PhabricatorDuoFuture.php | 19 +++++---- .../diffusion/view/DiffusionView.php | 12 +++--- .../markup/DivinerSymbolRemarkupRule.php | 18 ++++----- .../herald/controller/HeraldNewController.php | 40 +++++++++---------- .../PhabricatorOwnersDetailController.php | 10 ++--- .../cart/PhortuneCartCheckoutController.php | 12 +++--- .../PhortunePayPalPaymentProvider.php | 14 ++++--- .../provider/PhortunePaymentProvider.php | 3 +- .../storage/PhabricatorRepository.php | 8 +--- .../editengine/PhabricatorEditEngine.php | 3 +- .../PhabricatorTypeaheadDatasource.php | 8 ++-- 13 files changed, 83 insertions(+), 92 deletions(-) diff --git a/src/applications/almanac/controller/AlmanacController.php b/src/applications/almanac/controller/AlmanacController.php index 24a8986766..7fad62a509 100644 --- a/src/applications/almanac/controller/AlmanacController.php +++ b/src/applications/almanac/controller/AlmanacController.php @@ -67,19 +67,13 @@ abstract class AlmanacController $is_builtin = isset($builtins[$key]); $is_persistent = (bool)$property->getID(); - $delete_uri = id(new PhutilURI($delete_base)) - ->setQueryParams( - array( - 'key' => $key, - 'objectPHID' => $object->getPHID(), - )); + $params = array( + 'key' => $key, + 'objectPHID' => $object->getPHID(), + ); - $edit_uri = id(new PhutilURI($edit_base)) - ->setQueryParams( - array( - 'key' => $key, - 'objectPHID' => $object->getPHID(), - )); + $delete_uri = new PhutilURI($delete_base, $params); + $edit_uri = new PhutilURI($edit_base, $params); $delete = javelin_tag( 'a', diff --git a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php index 26f8785c0a..353f31562c 100644 --- a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php +++ b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php @@ -218,11 +218,11 @@ final class PhabricatorAuthOneTimeLoginController $request->setTemporaryCookie(PhabricatorCookies::COOKIE_HISEC, 'yes'); - return (string)id(new PhutilURI($panel_uri)) - ->setQueryParams( - array( - 'key' => $key, - )); + $params = array( + 'key' => $key, + ); + + return (string)new PhutilURI($panel_uri, $params); } $providers = id(new PhabricatorAuthProviderConfigQuery()) diff --git a/src/applications/auth/future/PhabricatorDuoFuture.php b/src/applications/auth/future/PhabricatorDuoFuture.php index 81a5a2a2b8..1e70ec2a57 100644 --- a/src/applications/auth/future/PhabricatorDuoFuture.php +++ b/src/applications/auth/future/PhabricatorDuoFuture.php @@ -80,11 +80,6 @@ final class PhabricatorDuoFuture $host = $this->apiHostname; $host = phutil_utf8_strtolower($host); - $uri = id(new PhutilURI('')) - ->setProtocol('https') - ->setDomain($host) - ->setPath($path); - $data = $this->parameters; $date = date('r'); @@ -109,11 +104,19 @@ final class PhabricatorDuoFuture $signature = new PhutilOpaqueEnvelope($signature); if ($http_method === 'GET') { - $uri->setQueryParams($data); - $data = array(); + $uri_data = $data; + $body_data = array(); + } else { + $uri_data = array(); + $body_data = $data; } - $future = id(new HTTPSFuture($uri, $data)) + $uri = id(new PhutilURI('', $uri_data)) + ->setProtocol('https') + ->setDomain($host) + ->setPath($path); + + $future = id(new HTTPSFuture($uri, $body_data)) ->setHTTPBasicAuthCredentials($this->integrationKey, $signature) ->setMethod($http_method) ->addHeader('Accept', 'application/json') diff --git a/src/applications/diffusion/view/DiffusionView.php b/src/applications/diffusion/view/DiffusionView.php index eb7f3eb72f..123d22af5e 100644 --- a/src/applications/diffusion/view/DiffusionView.php +++ b/src/applications/diffusion/view/DiffusionView.php @@ -81,12 +81,12 @@ abstract class DiffusionView extends AphrontView { } if (isset($details['external'])) { - $href = id(new PhutilURI('/diffusion/external/')) - ->setQueryParams( - array( - 'uri' => idx($details, 'external'), - 'id' => idx($details, 'hash'), - )); + $params = array( + 'uri' => idx($details, 'external'), + 'id' => idx($details, 'hash'), + ); + + $href = new PhutilURI('/diffusion/external/', $params); $tip = pht('Browse External'); } else { $href = $this->getDiffusionRequest()->generateURI( diff --git a/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php b/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php index 7fdb4cb37e..db68927625 100644 --- a/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php +++ b/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php @@ -111,15 +111,15 @@ final class DivinerSymbolRemarkupRule extends PhutilRemarkupRule { // Here, we're generating comment text or something like that. Just // link to Diviner and let it sort things out. - $href = id(new PhutilURI('/diviner/find/')) - ->setQueryParams( - array( - 'book' => $ref->getBook(), - 'name' => $ref->getName(), - 'type' => $ref->getType(), - 'context' => $ref->getContext(), - 'jump' => true, - )); + $params = array( + 'book' => $ref->getBook(), + 'name' => $ref->getName(), + 'type' => $ref->getType(), + 'context' => $ref->getContext(), + 'jump' => true, + ); + + $href = new PhutilURI('/diviner/find/', $params); } // TODO: This probably is not the best place to do this. Move it somewhere diff --git a/src/applications/herald/controller/HeraldNewController.php b/src/applications/herald/controller/HeraldNewController.php index fbaf1aeb9e..f571aeb395 100644 --- a/src/applications/herald/controller/HeraldNewController.php +++ b/src/applications/herald/controller/HeraldNewController.php @@ -81,13 +81,13 @@ final class HeraldNewController extends HeraldController { } if (!$errors && $done) { - $uri = id(new PhutilURI('edit/')) - ->setQueryParams( - array( - 'content_type' => $content_type, - 'rule_type' => $rule_type, - 'targetPHID' => $target_phid, - )); + $params = array( + 'content_type' => $content_type, + 'rule_type' => $rule_type, + 'targetPHID' => $target_phid, + ); + + $uri = new PhutilURI('edit/', $params); $uri = $this->getApplicationURI($uri); return id(new AphrontRedirectResponse())->setURI($uri); } @@ -126,13 +126,13 @@ final class HeraldNewController extends HeraldController { ->addHiddenInput('step', 2) ->appendChild($rule_types); + $params = array( + 'content_type' => $content_type, + 'step' => '0', + ); + $cancel_text = pht('Back'); - $cancel_uri = id(new PhutilURI('new/')) - ->setQueryParams( - array( - 'content_type' => $content_type, - 'step' => 0, - )); + $cancel_uri = new PhutilURI('new/', $params); $cancel_uri = $this->getApplicationURI($cancel_uri); $title = pht('Create Herald Rule: %s', idx($content_type_map, $content_type)); @@ -173,14 +173,14 @@ final class HeraldNewController extends HeraldController { ->setValue($request->getStr('objectName')) ->setLabel(pht('Object'))); + $params = array( + 'content_type' => $content_type, + 'rule_type' => $rule_type, + 'step' => 1, + ); + $cancel_text = pht('Back'); - $cancel_uri = id(new PhutilURI('new/')) - ->setQueryParams( - array( - 'content_type' => $content_type, - 'rule_type' => $rule_type, - 'step' => 1, - )); + $cancel_uri = new PhutilURI('new/', $params); $cancel_uri = $this->getApplicationURI($cancel_uri); $title = pht('Create Herald Rule: %s', idx($content_type_map, $content_type)); diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php index e28ae2b3bb..c458e4dbd1 100644 --- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php +++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php @@ -65,11 +65,11 @@ final class PhabricatorOwnersDetailController $commit_views = array(); - $commit_uri = id(new PhutilURI('/diffusion/commit/')) - ->setQueryParams( - array( - 'package' => $package->getPHID(), - )); + $params = array( + 'package' => $package->getPHID(), + ); + + $commit_uri = new PhutilURI('/diffusion/commit/', $params); $status_concern = DiffusionCommitAuditStatus::CONCERN_RAISED; diff --git a/src/applications/phortune/controller/cart/PhortuneCartCheckoutController.php b/src/applications/phortune/controller/cart/PhortuneCartCheckoutController.php index dd611ecb7b..874ecf63aa 100644 --- a/src/applications/phortune/controller/cart/PhortuneCartCheckoutController.php +++ b/src/applications/phortune/controller/cart/PhortuneCartCheckoutController.php @@ -134,13 +134,13 @@ final class PhortuneCartCheckoutController $account_id = $account->getID(); + $params = array( + 'merchantID' => $merchant->getID(), + 'cartID' => $cart->getID(), + ); + $payment_method_uri = $this->getApplicationURI("{$account_id}/card/new/"); - $payment_method_uri = new PhutilURI($payment_method_uri); - $payment_method_uri->setQueryParams( - array( - 'merchantID' => $merchant->getID(), - 'cartID' => $cart->getID(), - )); + $payment_method_uri = new PhutilURI($payment_method_uri, $params); $form = id(new AphrontFormView()) ->setUser($viewer) diff --git a/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php b/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php index 078141f8a1..262606ca62 100644 --- a/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php +++ b/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php @@ -348,12 +348,14 @@ final class PhortunePayPalPaymentProvider extends PhortunePaymentProvider { ->setRawPayPalQuery('SetExpressCheckout', $params) ->resolve(); - $uri = new PhutilURI('https://www.sandbox.paypal.com/cgi-bin/webscr'); - $uri->setQueryParams( - array( - 'cmd' => '_express-checkout', - 'token' => $result['TOKEN'], - )); + $params = array( + 'cmd' => '_express-checkout', + 'token' => $result['TOKEN'], + ); + + $uri = new PhutilURI( + 'https://www.sandbox.paypal.com/cgi-bin/webscr', + $params); $cart->setMetadataValue('provider.checkoutURI', (string)$uri); $cart->save(); diff --git a/src/applications/phortune/provider/PhortunePaymentProvider.php b/src/applications/phortune/provider/PhortunePaymentProvider.php index 90e354c5dc..57b2956ecb 100644 --- a/src/applications/phortune/provider/PhortunePaymentProvider.php +++ b/src/applications/phortune/provider/PhortunePaymentProvider.php @@ -273,8 +273,7 @@ abstract class PhortunePaymentProvider extends Phobject { $app = PhabricatorApplication::getByClass('PhabricatorPhortuneApplication'); $path = $app->getBaseURI().'provider/'.$id.'/'.$action.'/'; - $uri = new PhutilURI($path); - $uri->setQueryParams($params); + $uri = new PhutilURI($path, $params); if ($local) { return $uri; diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index e66fb78afa..d5ad83b6d6 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -820,8 +820,6 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return $uri; } - $uri = new PhutilURI($uri); - if (isset($params['lint'])) { $params['params'] = idx($params, 'params', array()) + array( 'lint' => $params['lint'], @@ -830,11 +828,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO $query = idx($params, 'params', array()) + $query; - if ($query) { - $uri->setQueryParams($query); - } - - return $uri; + return new PhutilURI($uri, $query); } public function updateURIIndex() { diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index d1a2fb72b9..353d7ee385 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -1541,8 +1541,7 @@ abstract class PhabricatorEditEngine $config_uri = $config->getCreateURI(); if ($parameters) { - $config_uri = (string)id(new PhutilURI($config_uri)) - ->setQueryParams($parameters); + $config_uri = (string)new PhutilURI($config_uri, $parameters); } $specs[] = array( diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php index 43cc72eaaa..e077d7a7ec 100644 --- a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php @@ -99,8 +99,8 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { } public function getDatasourceURI() { - $uri = new PhutilURI('/typeahead/class/'.get_class($this).'/'); - $uri->setQueryParams($this->newURIParameters()); + $params = $this->newURIParameters(); + $uri = new PhutilURI('/typeahead/class/'.get_class($this).'/', $params); return phutil_string_cast($uri); } @@ -109,8 +109,8 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { return null; } - $uri = new PhutilURI('/typeahead/browse/'.get_class($this).'/'); - $uri->setQueryParams($this->newURIParameters()); + $params = $this->newURIParameters(); + $uri = new PhutilURI('/typeahead/browse/'.get_class($this).'/', $params); return phutil_string_cast($uri); }