Begin modernizing the Arcanist interaction with Conduit

Summary:
Ref T10895.

NOTE: I'm going to land this and other changes to a new `experimental` branch until `arc` is more substantially rebuilt, since everything I touch feels like it requires me to rebuild 30 other things first.

Currently, many `arc` workflows are unnecessarily slow because they call `conduit.connect` on startup. There's no need to do this with the modern way the API works, and we've generally moved away from explicit version testing to more granular capability testing on specific workflows.

Additionally, some workflows like `arc patch` are huge messes (see T11434) because they're trying to run in anonymous mode but it doesn't really work with all the upfront stuff Conduit does now. It's not possible, in the general case, for a workflow to know upfront if it needs Conduit or not.

And:

  - `ArcanistWorkflow` has piles of Conduit logic, but should not.
  - Pooling Conduit requests isn't very easy.
  - There's a lot of general cruft around the workflow.
  - We should drop certificate support.

This pulls out Conduit into a separate on-demand class with modern support, future pooling, less cruft, inline handling of login issues, and generally less garbage.

Also adds an `--anonymous` flag, mostly to make testing easier.

Test Plan: Ran `arc browse`, used `--anonymous` and `--trace`, fiddled with credentials, got approximatley the same behavior that mainline `arc` has.

Reviewers: chad, avivey

Reviewed By: avivey

Subscribers: avivey

Maniphest Tasks: T10895

Differential Revision: https://secure.phabricator.com/D16921

.gitignore

@@ -7,6 +7,7 @@
 
 # libphutil
 /src/.phutil_module_cache
+/src/.cache
 
 # User extensions
 /externals/includes/*

scripts/arcanist.php

@@ -35,6 +35,10 @@ $base_args->parsePartial(
       'param' => 'token',
       'help' => pht('Use a specific authentication token.'),
     ),
+    array(
+      'name' => 'anonymous',
+      'help' => pht('Run workflow as a public user, without authenticating.'),
+    ),
     array(
       'name'    => 'conduit-version',
       'param'   => 'version',
@@ -64,6 +68,7 @@ $force_conduit_version = $base_args->getArg('conduit-version');
 $conduit_timeout = $base_args->getArg('conduit-timeout');
 $skip_arcconfig = $base_args->getArg('skip-arcconfig');
 $custom_arcrc = $base_args->getArg('arcrc-file');
+$is_anonymous = $base_args->getArg('anonymous');
 $load = $base_args->getArg('load-phutil-library');
 $help = $base_args->getArg('help');
 $args = array_values($base_args->getUnconsumedArgumentVector());
@@ -324,6 +329,10 @@ try {
     $conduit_token = $force_token;
   }
 
+  if ($is_anonymous) {
+    $conduit_token = null;
+  }
+
   $description = implode(' ', $original_argv);
   $credentials = array(
     'user' => $user_name,
@@ -333,6 +342,23 @@ try {
   );
   $workflow->setConduitCredentials($credentials);
 
+  $basic_user = $configuration_manager->getConfigFromAnySource(
+    'http.basicauth.user');
+  $basic_pass = $configuration_manager->getConfigFromAnySource(
+    'http.basicauth.pass');
+
+  $engine = id(new ArcanistConduitEngine())
+    ->setConduitURI($conduit_uri)
+    ->setConduitToken($conduit_token)
+    ->setBasicAuthUser($basic_user)
+    ->setBasicAuthPass($basic_pass);
+
+  if ($conduit_timeout) {
+    $engine->setConduitTimeout($conduit_timeout);
+  }
+
+  $workflow->setConduitEngine($engine);
+
   if ($need_auth) {
     if ((!$user_name || !$certificate) && (!$conduit_token)) {
       $arc = 'arc';
@@ -409,7 +435,7 @@ try {
     fwrite(STDERR, phutil_console_format(
       "**%s** %s\n",
       pht('Usage Exception:'),
-      $ex->getMessage()));
+      rtrim($ex->getMessage())));
   }
 
   if ($config) {

src/__phutil_library_map__.php

@@ -82,6 +82,8 @@ phutil_register_library_map(array(
     'ArcanistComprehensiveLintEngine' => 'lint/engine/ArcanistComprehensiveLintEngine.php',
     'ArcanistConcatenationOperatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistConcatenationOperatorXHPASTLinterRule.php',
     'ArcanistConcatenationOperatorXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistConcatenationOperatorXHPASTLinterRuleTestCase.php',
+    'ArcanistConduitCall' => 'conduit/ArcanistConduitCall.php',
+    'ArcanistConduitEngine' => 'conduit/ArcanistConduitEngine.php',
     'ArcanistConfiguration' => 'configuration/ArcanistConfiguration.php',
     'ArcanistConfigurationDrivenLintEngine' => 'lint/engine/ArcanistConfigurationDrivenLintEngine.php',
     'ArcanistConfigurationDrivenUnitTestEngine' => 'unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php',
@@ -496,6 +498,8 @@ phutil_register_library_map(array(
     'ArcanistComprehensiveLintEngine' => 'ArcanistLintEngine',
     'ArcanistConcatenationOperatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistConcatenationOperatorXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
+    'ArcanistConduitCall' => 'Phobject',
+    'ArcanistConduitEngine' => 'Phobject',
     'ArcanistConfiguration' => 'Phobject',
     'ArcanistConfigurationDrivenLintEngine' => 'ArcanistLintEngine',
     'ArcanistConfigurationDrivenUnitTestEngine' => 'ArcanistUnitTestEngine',

src/conduit/ArcanistConduitCall.php

@@ -0,0 +1,155 @@
+<?php
+
+final class ArcanistConduitCall
+  extends Phobject {
+
+  private $key;
+  private $engine;
+  private $method;
+  private $parameters;
+  private $future;
+
+  public function setKey($key) {
+    $this->key = $key;
+    return $this;
+  }
+
+  public function getKey() {
+    return $this->key;
+  }
+
+  public function setEngine(ArcanistConduitEngine $engine) {
+    $this->engine = $engine;
+    return $this;
+  }
+
+  public function getEngine() {
+    return $this->engine;
+  }
+
+  public function setMethod($method) {
+    $this->method = $method;
+    return $this;
+  }
+
+  public function getMethod() {
+    return $this->method;
+  }
+
+  public function setParameters(array $parameters) {
+    $this->parameters = $parameters;
+    return $this;
+  }
+
+  public function getParameters() {
+    return $this->parameters;
+  }
+
+  private function newFuture() {
+    if ($this->future) {
+      throw new Exception(
+        pht(
+          'Call has previously generated a future. Create a '.
+          'new call object for each API method invocation.'));
+    }
+
+    $method = $this->getMethod();
+    $parameters = $this->getParameters();
+    $future = $this->getEngine()->newFuture($this);
+    $this->future = $future;
+
+    return $this->future;
+  }
+
+  public function resolve() {
+    if (!$this->future) {
+      $this->newFuture();
+    }
+
+    $this->getEngine()->resolveFuture($this->getKey());
+
+    return $this->resolveFuture();
+  }
+
+  private function resolveFuture() {
+    $future = $this->future;
+
+    try {
+      $result = $future->resolve();
+    } catch (ConduitClientException $ex) {
+      switch ($ex->getErrorCode()) {
+        case 'ERR-INVALID-SESSION':
+          if (!$this->getEngine()->getConduitToken()) {
+            $this->raiseLoginRequired();
+          }
+          break;
+        case 'ERR-INVALID-AUTH':
+          $this->raiseInvalidAuth();
+          break;
+      }
+
+      throw $ex;
+    }
+
+    return $result;
+  }
+
+  private function raiseLoginRequired() {
+    $conduit_uri = $this->getEngine()->getConduitURI();
+    $conduit_uri = new PhutilURI($conduit_uri);
+    $conduit_uri->setPath('/');
+
+    $conduit_domain = $conduit_uri->getDomain();
+
+    $block = id(new PhutilConsoleBlock())
+      ->addParagraph(
+        tsprintf(
+          '**<bg:red> %s </bg>**',
+          pht('LOGIN REQUIRED')))
+      ->addParagraph(
+        pht(
+          'You are trying to connect to a server ("%s") that you do not '.
+          'have any stored credentials for, but the command you are '.
+          'running requires authentication.',
+          $conduit_domain))
+      ->addParagraph(
+        pht(
+          'To login and save credentials for this server, run this '.
+          'command:'))
+      ->addParagraph(
+        tsprintf(
+          "    $ arc install-certificate %s\n",
+          $conduit_uri));
+
+    throw new ArcanistUsageException($block->drawConsoleString());
+  }
+
+  private function raiseInvalidAuth() {
+    $conduit_uri = $this->getEngine()->getConduitURI();
+    $conduit_uri = new PhutilURI($conduit_uri);
+    $conduit_uri->setPath('/');
+
+    $conduit_domain = $conduit_uri->getDomain();
+
+    $block = id(new PhutilConsoleBlock())
+      ->addParagraph(
+        tsprintf(
+          '**<bg:red> %s </bg>**',
+          pht('INVALID CREDENTIALS')))
+      ->addParagraph(
+        pht(
+          'Your stored credentials for this server ("%s") are not valid.',
+          $conduit_domain))
+      ->addParagraph(
+        pht(
+          'To login and save valid credentials for this server, run this '.
+          'command:'))
+      ->addParagraph(
+        tsprintf(
+          "    $ arc install-certificate %s\n",
+          $conduit_uri));
+
+    throw new ArcanistUsageException($block->drawConsoleString());
+  }
+
+}

src/conduit/ArcanistConduitEngine.php

@@ -0,0 +1,162 @@
+<?php
+
+final class ArcanistConduitEngine
+  extends Phobject {
+
+  private $conduitURI;
+  private $conduitToken;
+
+  private $conduitTimeout;
+  private $basicAuthUser;
+  private $basicAuthPass;
+
+  private $client;
+  private $callKey = 0;
+  private $activeFutures = array();
+  private $resolvedFutures = array();
+
+  public function isCallable() {
+    return ($this->conduitURI !== null);
+  }
+
+  public function setConduitURI($conduit_uri) {
+    $this->conduitURI = $conduit_uri;
+    return $this;
+  }
+
+  public function getConduitURI() {
+    return $this->conduitURI;
+  }
+
+  public function setConduitToken($conduit_token) {
+    $this->conduitToken = $conduit_token;
+    return $this;
+  }
+
+  public function getConduitToken() {
+    return $this->conduitToken;
+  }
+
+  public function setConduitTimeout($conduit_timeout) {
+    $this->conduitTimeout = $conduit_timeout;
+    return $this;
+  }
+
+  public function getConduitTimeout() {
+    return $this->conduitTimeout;
+  }
+
+  public function setBasicAuthUser($basic_auth_user) {
+    $this->basicAuthUser = $basic_auth_user;
+    return $this;
+  }
+
+  public function getBasicAuthUser() {
+    return $this->basicAuthUser;
+  }
+
+  public function setBasicAuthPass($basic_auth_pass) {
+    $this->basicAuthPass = $basic_auth_pass;
+    return $this;
+  }
+
+  public function getBasicAuthPass() {
+    return $this->basicAuthPass;
+  }
+
+  public function newCall($method, array $parameters) {
+    if ($this->conduitURI == null) {
+      $this->raiseURIException();
+    }
+
+    $next_key = ++$this->callKey;
+
+    return id(new ArcanistConduitCall())
+      ->setKey($next_key)
+      ->setEngine($this)
+      ->setMethod($method)
+      ->setParameters($parameters);
+  }
+
+  public function newFuture(ArcanistConduitCall $call) {
+    $method = $call->getMethod();
+    $parameters = $call->getParameters();
+
+    $future = $this->getClient()->callMethod($method, $parameters);
+    $this->activeFutures[$call->getKey()] = $future;
+    return $future;
+  }
+
+  private function getClient() {
+    if (!$this->client) {
+      $conduit_uri = $this->getConduitURI();
+
+      $client = new ConduitClient($conduit_uri);
+
+      $timeout = $this->getConduitTimeout();
+      if ($timeout) {
+        $client->setTimeout($timeout);
+      }
+
+      $basic_user = $this->getBasicAuthUser();
+      $basic_pass = $this->getBasicAuthPass();
+      if ($basic_user !== null || $basic_pass !== null) {
+        $client->setBasicAuthCredentials($basic_user, $basic_pass);
+      }
+
+      $token = $this->getConduitToken();
+      if ($token) {
+        $client->setConduitToken($this->getConduitToken());
+      }
+    }
+
+    return $client;
+  }
+
+  public function resolveFuture($key) {
+    if (isset($this->resolvedFutures[$key])) {
+      return;
+    }
+
+    if (!isset($this->activeFutures[$key])) {
+      throw new Exception(
+        pht(
+          'No future with key "%s" is present in pool.',
+          $key));
+    }
+
+    $iterator = new FutureIterator($this->activeFutures);
+    foreach ($iterator as $future_key => $future) {
+      $this->resolvedFutures[$future_key] = $future;
+      unset($this->activeFutures[$future_key]);
+      if ($future_key == $key) {
+        break;
+      }
+    }
+
+    return;
+  }
+
+  private function raiseURIException() {
+    $list = id(new PhutilConsoleList())
+      ->addItem(
+        pht(
+          'Run in a working copy with "phabricator.uri" set in ".arcconfig".'))
+      ->addItem(
+        pht(
+          'Set a default URI with `arc set-config default <uri>`.'))
+      ->addItem(
+        pht(
+          'Specify a URI explicitly with `--conduit-uri=<uri>`.'));
+
+    $block = id(new PhutilConsoleBlock())
+      ->addParagraph(
+        pht(
+          'This command needs to communicate with Phabricator, but no '.
+          'Phabricator URI is configured.'))
+      ->addList($list);
+
+    throw new ArcanistUsageException($block->drawConsoleString());
+  }
+
+}

src/workflow/ArcanistBrowseWorkflow.php

@@ -53,19 +53,13 @@ EOTEXT
     return true;
   }
 
-  public function requiresConduit() {
-    return true;
-  }
-
-  public function requiresAuthentication() {
-    return true;
-  }
-
   public function desiresRepositoryAPI() {
     return true;
   }
 
   public function run() {
+    $conduit = $this->getConduitEngine();
+
     $console = PhutilConsole::getConsole();
 
     $is_force = $this->getArgument('force');
@@ -80,11 +74,13 @@ EOTEXT
     }
     $things = array_fuse($things);
 
-    $objects = $this->getConduit()->callMethodSynchronous(
+    $method = 'phid.lookup';
-      'phid.lookup',
+    $params = array(
-      array(
       'names' => array_keys($things),
-      ));
+    );
+
+    $objects = $conduit->newCall($method, $params)
+      ->resolve();
 
     $uris = array();
     foreach ($objects as $name => $object) {
@@ -123,12 +119,15 @@ EOTEXT
       }
 
       if ($commits) {
-        $commit_info = $this->getConduit()->callMethodSynchronous(
+        $method = 'diffusion.querycommits';
-          'diffusion.querycommits',
+
-          array(
+        $params = array(
           'repositoryPHID' => $this->getRepositoryPHID(),
           'names' => array_keys($commits),
-          ));
+        );
+
+        $commit_info = $conduit->newCall($method, $params)
+          ->resolve();
 
         foreach ($commit_info['identifierMap'] as $ckey => $cphid) {
           $thing = $commits[$ckey];

src/workflow/ArcanistWorkflow.php

@@ -69,6 +69,7 @@ abstract class ArcanistWorkflow extends Phobject {
   private $repositoryVersion;
 
   private $changeCache = array();
+  private $conduitEngine;
 
 
   public function __construct() {}
@@ -1767,9 +1768,9 @@ abstract class ArcanistWorkflow extends Phobject {
     }
 
     try {
-      $results = $this->getConduit()->callMethodSynchronous(
+      $method = 'repository.query';
-        'repository.query',
+      $results = $this->getConduitEngine()->newCall($method, $query)
-        $query);
+        ->resolve();
     } catch (ConduitClientException $ex) {
       if ($ex->getErrorCode() == 'ERR-CONDUIT-CALL') {
         $reasons[] = pht(
@@ -2062,5 +2063,14 @@ abstract class ArcanistWorkflow extends Phobject {
     return $map;
   }
 
+  final public function setConduitEngine(
+    ArcanistConduitEngine $conduit_engine) {
+    $this->conduitEngine = $conduit_engine;
+    return $this;
+  }
+
+  final public function getConduitEngine() {
+    return $this->conduitEngine;
+  }
 
 }