Diffusion: encapsulate request in DiffusionRequest

Summary:
Put an indirection layer between controllers and URI management,
adding branches to git repositories.

Test Plan:
Looked at browse, history browse, file browse views, bad branches,
bad commits

Reviewed By: jwilson
Reviewers: aran, jwilson
CC: jwilson, epriestley
Differential Revision: 65

src/__phutil_library_map__.php

@@ -151,8 +151,10 @@ phutil_register_library_map(array(
     'DiffusionFileContentQuery' => 'applications/diffusion/query/filecontent/base',
     'DiffusionGitBrowseQuery' => 'applications/diffusion/query/browse/git',
     'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/git',
+    'DiffusionGitRequest' => 'applications/diffusion/request/git',
     'DiffusionHomeController' => 'applications/diffusion/controller/home',
     'DiffusionRepositoryPath' => 'applications/diffusion/data/repositorypath',
+    'DiffusionRequest' => 'applications/diffusion/request/base',
     'Javelin' => 'infrastructure/javelin/api',
     'LiskDAO' => 'storage/lisk/dao',
     'ManiphestController' => 'applications/maniphest/controller/base',
@@ -437,6 +439,7 @@ phutil_register_library_map(array(
     'DiffusionController' => 'PhabricatorController',
     'DiffusionGitBrowseQuery' => 'DiffusionBrowseQuery',
     'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery',
+    'DiffusionGitRequest' => 'DiffusionRequest',
     'DiffusionHomeController' => 'DiffusionController',
     'ManiphestController' => 'PhabricatorController',
     'ManiphestDAO' => 'PhabricatorLiskDAO',

src/applications/diffusion/controller/base/DiffusionController.php

@@ -32,14 +32,4 @@ abstract class DiffusionController extends PhabricatorController {
     return $response->setContent($page->render());
   }
 
-  protected function loadRepositoryByCallsign($callsign) {
-    $repository = id(new PhabricatorRepository())->loadOneWhere(
-      'callsign = %s',
-      $callsign);
-    if (!$repository) {
-      throw new Exception("No such repository '{$callsign}'.");
-    }
-    return $repository;
-  }
-
 }

src/applications/diffusion/controller/base/__init__.php

@@ -8,7 +8,6 @@
 
 phutil_require_module('phabricator', 'aphront/response/webpage');
 phutil_require_module('phabricator', 'applications/base/controller/base');
-phutil_require_module('phabricator', 'applications/repository/storage/repository');
 
 phutil_require_module('phutil', 'utils');
 

src/applications/diffusion/controller/browse/DiffusionBrowseController.php

@@ -18,35 +18,22 @@
 
 class DiffusionBrowseController extends DiffusionController {
 
-  private $callsign;
-  private $path;
-  private $line;
-  private $commit;
-
-  private $repository;
-
   public function willProcessRequest(array $data) {
-    $this->callsign = $data['callsign'];
+    $this->diffusionRequest = DiffusionRequest::newFromAphrontRequestDictionary(
-    $this->path     = rtrim($data['path'], '/');
+      $data);
-
-    $this->line     = idx($data, 'line');
-    $this->commit   = idx($data, 'commit');
   }
 
   public function processRequest() {
-    $repository = $this->loadRepositoryByCallsign($this->callsign);
+    $drequest = $this->diffusionRequest;
 
-    $browse_data = DiffusionBrowseQuery::newFromRepository(
+    $browse_query = DiffusionBrowseQuery::newFromDiffusionRequest($drequest);
-      $repository,
+    $results = $browse_query->loadPaths();
-      $this->path,
-      $this->commit);
-    $results = $browse_data->loadPaths();
 
     $content = array();
 
     if (!$results) {
 
-      switch ($browse_data->getReasonForEmptyResultSet()) {
+      switch ($browse_query->getReasonForEmptyResultSet()) {
         case DiffusionBrowseQuery::REASON_IS_NONEXISTENT:
           $title = 'Path Does Not Exist';
           // TODO: Under git, this error message should be more specific. It
@@ -56,9 +43,9 @@ class DiffusionBrowseController extends DiffusionController {
           break;
         case DiffusionBrowseQuery::REASON_IS_DELETED:
           // TODO: Format all these commits into nice VCS-agnostic links.
-          $commit = $this->commit;
+          $commit = $drequest->getCommit();
-          $deleted = $browse_data->getDeletedAtCommit();
+          $deleted = $browse_query->getDeletedAtCommit();
-          $existed = $browse_data->getExistedAtCommit();
+          $existed = $browse_query->getExistedAtCommit();
 
           $title = 'Path Was Deleted';
           $body = "This path does not exist at {$commit}. It was deleted in ".
@@ -67,9 +54,7 @@ class DiffusionBrowseController extends DiffusionController {
           break;
         case DiffusionBrowseQuery::REASON_IS_FILE:
           $controller = new DiffusionBrowseFileController($this->getRequest());
-          $controller->setRepository($repository);
+          $controller->setDiffusionRequest($drequest);
-          $controller->setPath($this->path);
-          $controller->setCommit($this->commit);
           return $this->delegateToController($controller);
           break;
         default:
@@ -85,13 +70,11 @@ class DiffusionBrowseController extends DiffusionController {
 
     } else {
       $browse_table = new DiffusionBrowseTableView();
-      $browse_table->setRepository($repository);
+      $browse_table->setDiffusionRequest($drequest);
       $browse_table->setPaths($results);
-      $browse_table->setRoot($this->path);
-      $browse_table->setCommit($this->commit);
 
       $browse_panel = new AphrontPanelView();
-      $browse_panel->setHeader($this->path);
+      $browse_panel->setHeader($drequest->getPath());
       $browse_panel->appendChild($browse_table);
 
       $content[] = $browse_panel;
@@ -105,7 +88,7 @@ class DiffusionBrowseController extends DiffusionController {
     return $this->buildStandardPageResponse(
       $content,
       array(
-        'title' => basename($this->path),
+        'title' => basename($drequest->getPath()),
       ));
   }
 

src/applications/diffusion/controller/browse/__init__.php

@@ -9,11 +9,10 @@
 phutil_require_module('phabricator', 'applications/diffusion/controller/base');
 phutil_require_module('phabricator', 'applications/diffusion/controller/file');
 phutil_require_module('phabricator', 'applications/diffusion/query/browse/base');
+phutil_require_module('phabricator', 'applications/diffusion/request/base');
 phutil_require_module('phabricator', 'applications/diffusion/view/browsetable');
 phutil_require_module('phabricator', 'view/form/error');
 phutil_require_module('phabricator', 'view/layout/panel');
 
-phutil_require_module('phutil', 'utils');
-
 
 phutil_require_source('DiffusionBrowseController.php');

src/applications/diffusion/controller/file/DiffusionBrowseFileController.php

@@ -18,33 +18,16 @@
 
 class DiffusionBrowseFileController extends DiffusionController {
 
-  private $callsign;
+  private $diffusionRequest;
-  private $path;
-  private $line;
-  private $commit;
-  private $view;
 
-  private $repository;
+  public function setDiffusionRequest(DiffusionRequest $request) {
-
+    $this->diffusionRequest = $request;
-  public function setRepository(DiffusionRepository $repository) {
-    $this->repository = $repository;
-  }
-
-  public function setCommit($commit) {
-    $this->commit = $commit;
-    return $this;
-  }
-
-  public function setPath($path) {
-    $this->path = $path;
     return $this;
   }
 
   public function processRequest() {
-    $file_query = DiffusionFileContentQuery::newFromRepository(
+    $file_query = DiffusionFileContentQuery::newFromDiffusionRequest(
-      $this->repository,
+      $this->diffusionRequest);
-      $this->path,
-      $this->commit);
     $file_content = $file_query->loadFileContent();
 
     $corpus = phutil_render_tag(

src/applications/diffusion/query/browse/base/DiffusionBrowseQuery.php

@@ -18,9 +18,7 @@
 
 abstract class DiffusionBrowseQuery {
 
-  private $repository;
+  private $request;
-  private $path;
-  private $commit;
 
   protected $reason;
   protected $existedAtCommit;
@@ -29,15 +27,16 @@ abstract class DiffusionBrowseQuery {
   const REASON_IS_FILE            = 'is-file';
   const REASON_IS_DELETED         = 'is-deleted';
   const REASON_IS_NONEXISTENT     = 'nonexistent';
+  const REASON_BAD_COMMIT         = 'bad-commit';
 
   final private function __construct() {
     // <private>
   }
 
-  final public static function newFromRepository(
+  final public static function newFromDiffusionRequest(
-    PhabricatorRepository $repository,
+    DiffusionRequest $request) {
-    $path = '/',
+
-    $commit = null) {
+    $repository = $request->getRepository();
 
     switch ($repository->getVersionControlSystem()) {
       case 'git':
@@ -51,23 +50,13 @@ abstract class DiffusionBrowseQuery {
     PhutilSymbolLoader::loadClass($class);
     $query = new $class();
 
-    $query->repository = $repository;
+    $query->request = $request;
-    $query->path = $path;
-    $query->commit = $commit;
 
     return $query;
   }
 
-  final protected function getRepository() {
+  final protected function getRequest() {
-    return $this->repository;
+    return $this->request;
-  }
-
-  final protected function getPath() {
-    return $this->path;
-  }
-
-  final protected function getCommit() {
-    return $this->commit;
   }
 
   final public function getReasonForEmptyResultSet() {

src/applications/diffusion/query/browse/git/DiffusionGitBrowseQuery.php

@@ -19,12 +19,13 @@
 final class DiffusionGitBrowseQuery extends DiffusionBrowseQuery {
 
   protected function executeQuery() {
-    $repository = $this->getRepository();
+    $drequest = $this->getRequest();
-    $path = $this->getPath();
+    $repository = $drequest->getRepository();
-    $commit = nonempty($this->getCommit(), 'HEAD');
+
+    $path = $drequest->getPath();
+    $commit = $drequest->getCommit();
 
     $local_path = $repository->getDetail('local-path');
-
     $git = PhabricatorEnv::getEnvConfig('git.path');
 
     try {
@@ -35,7 +36,8 @@ final class DiffusionGitBrowseQuery extends DiffusionBrowseQuery {
         $commit,
         $path);
     } catch (CommandException $e) {
-      if (preg_match('/^fatal: Not a valid object name/', $e->getStderr())) {
+      $stderr = $e->getStdErr();
+      if (preg_match('/^fatal: Not a valid object name/', $stderr)) {
         // Grab two logs, since the first one is when the object was deleted.
         list($stdout) = execx(
           '(cd %s && %s log -n2 --format="%%H" %s -- %s)',

src/applications/diffusion/query/filecontent/base/DiffusionFileContentQuery.php

@@ -18,18 +18,16 @@
 
 abstract class DiffusionFileContentQuery {
 
-  private $repository;
+  private $request;
-  private $path;
-  private $commit;
 
   final private function __construct() {
     // <private>
   }
 
-  final public static function newFromRepository(
+  final public static function newFromDiffusionRequest(
-    PhabricatorRepository $repository,
+    DiffusionRequest $request) {
-    $path = '/',
+
-    $commit = null) {
+    $repository = $request->getRepository();
 
     switch ($repository->getVersionControlSystem()) {
       case 'git':
@@ -42,26 +40,15 @@ abstract class DiffusionFileContentQuery {
     PhutilSymbolLoader::loadClass($class);
     $query = new $class();
 
-    $query->repository = $repository;
+    $query->request = $request;
-    $query->path = $path;
-    $query->commit = $commit;
 
     return $query;
   }
 
-  final protected function getRepository() {
+  final protected function getRequest() {
-    return $this->repository;
+    return $this->request;
   }
 
-  final protected function getPath() {
-    return $this->path;
-  }
-
-  final protected function getCommit() {
-    return $this->commit;
-  }
-
-
   final public function loadFileContent() {
     return $this->executeQuery();
   }

src/applications/diffusion/query/filecontent/git/DiffusionGitFileContentQuery.php

@@ -19,12 +19,14 @@
 final class DiffusionGitFileContentQuery extends DiffusionFileContentQuery {
 
   protected function executeQuery() {
-    $repository = $this->getRepository();
+    $drequest = $this->getRequest();
-    $path = $this->getPath();
+
-    $commit = nonempty($this->getCommit(), 'HEAD');
+    $repository = $drequest->getRepository();
+    $path = $drequest->getPath();
+    $commit = $drequest->getCommit();
 
     $local_path = $repository->getDetail('local-path');
-    $git = PhabricatorEnv::getEnvConfig('git.path');
+    $git = $drequest->getPathToGitBinary();
 
     list($corpus) = execx(
       '(cd %s && %s cat-file blob %s:%s)',

src/applications/diffusion/query/filecontent/git/__init__.php

@@ -8,10 +8,8 @@
 
 phutil_require_module('phabricator', 'applications/diffusion/data/filecontent');
 phutil_require_module('phabricator', 'applications/diffusion/query/filecontent/base');
-phutil_require_module('phabricator', 'infrastructure/env');
 
 phutil_require_module('phutil', 'future/exec');
-phutil_require_module('phutil', 'utils');
 
 
 phutil_require_source('DiffusionGitFileContentQuery.php');

src/applications/diffusion/request/base/DiffusionRequest.php

@@ -0,0 +1,102 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class DiffusionRequest {
+
+  protected $callsign;
+  protected $path;
+  protected $line;
+  protected $commit;
+  protected $branch;
+
+  protected $repository;
+
+  final private function __construct() {
+    // <private>
+  }
+
+  final public static function newFromAphrontRequestDictionary(array $data) {
+
+    $vcs = null;
+    $repository = null;
+    $callsign = idx($data, 'callsign');
+    if ($callsign) {
+      $repository = id(new PhabricatorRepository())->loadOneWhere(
+        'callsign = %s',
+        $callsign);
+      if (!$repository) {
+        throw new Exception("No such repository '{$callsign}'.");
+      }
+      $vcs = $repository->getVersionControlSystem();
+    }
+
+    switch ($vcs) {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+        $class = 'DiffusionGitRequest';
+        break;
+      default:
+        $class = 'DiffusionRequest';
+        break;
+    }
+
+    $object = new $class();
+
+    $object->callsign   = $callsign;
+    $object->repository = $repository;
+    $object->line       = idx($data, 'line');
+    $object->commit     = idx($data, 'commit');
+    $object->path       = idx($data, 'path');
+
+    $object->initializeFromAphrontRequestDictionary();
+
+    return $object;
+  }
+
+  protected function initializeFromAphrontRequestDictionary() {
+
+  }
+
+  protected function parsePath($path) {
+    $this->path = $path;
+  }
+
+  public function getRepository() {
+    return $this->repository;
+  }
+
+  public function getCallsign() {
+    return $this->callsign;
+  }
+
+  public function getPath() {
+    return $this->path;
+  }
+
+  public function getLine() {
+    return $this->line;
+  }
+
+  public function getCommit() {
+    return $this->commit;
+  }
+
+  public function getBranch() {
+    return $this->branch;
+  }
+
+}

src/applications/diffusion/request/base/__init__.php

@@ -0,0 +1,15 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/repository/constants/repositorytype');
+phutil_require_module('phabricator', 'applications/repository/storage/repository');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('DiffusionRequest.php');

src/applications/diffusion/request/git/DiffusionGitRequest.php

@@ -0,0 +1,106 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class DiffusionGitRequest extends DiffusionRequest {
+
+  protected function initializeFromAphrontRequestDictionary() {
+    parent::initializeFromAphrontRequestDictionary();
+
+    $path = $this->path;
+    $parts = explode('/', $path);
+
+    $branch = array_shift($parts);
+    $this->branch = $this->decodeBranchName($branch);
+
+    $this->path = implode('/', $parts);
+
+    if ($this->repository) {
+      $local_path = $this->repository->getDetail('local-path');
+      $git = $this->getPathToGitBinary();
+
+      // TODO: This is not terribly efficient and does not produce terribly
+      // good error messages, but it seems better to put error handling code
+      // here than to try to do it in every query.
+
+      $branch = $this->getBranch();
+      execx(
+        '(cd %s && %s rev-parse --verify %s)',
+        $local_path,
+        $git,
+        $branch);
+
+      if ($this->commit) {
+        execx(
+          '(cd %s && %s rev-parse --verify %s)',
+          $local_path,
+          $git,
+          $this->commit);
+        list($contains) = execx(
+          '(cd %s && %s branch --contains %s)',
+          $local_path,
+          $git,
+          $this->commit);
+        $contains = array_filter(explode("\n", $contains));
+        $found = false;
+        foreach ($contains as $containing_branch) {
+          $containing_branch = trim($containing_branch, "* \n");
+          if ($containing_branch == $branch) {
+            $found = true;
+            break;
+          }
+        }
+        if (!$found) {
+          throw new Exception(
+            "Commit does not exist on this branch!");
+        }
+      }
+    }
+
+
+  }
+
+  public function getPathToGitBinary() {
+    return PhabricatorEnv::getEnvConfig('git.path');
+  }
+
+  public function getBranch() {
+    if ($this->branch) {
+      return $this->branch;
+    }
+    if ($this->repository) {
+      return $this->repository->getDetail('default-branch', 'master');
+    }
+    throw new Exception("Unable to determine branch!");
+  }
+
+  public function getCommit() {
+    if ($this->commit) {
+      return $this->commit;
+    }
+    return $this->getBranch();
+  }
+
+  private function decodeBranchName($branch) {
+    return str_replace(':', '/', $branch);
+  }
+
+  private function encodeBranchName($branch) {
+    return str_replace('/', ':', $branch);
+  }
+
+}

src/applications/diffusion/request/git/__init__.php

@@ -0,0 +1,15 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/diffusion/request/base');
+phutil_require_module('phabricator', 'infrastructure/env');
+
+phutil_require_module('phutil', 'future/exec');
+
+
+phutil_require_source('DiffusionGitRequest.php');

src/applications/diffusion/view/browsetable/DiffusionBrowseTableView.php

@@ -18,13 +18,10 @@
 
 final class DiffusionBrowseTableView extends AphrontView {
 
-  private $repository;
+  private $request;
-  private $paths;
-  private $root;
-  private $commit;
 
-  public function setRepository($repository) {
+  public function setDiffusionRequest(DiffusionRequest $request) {
-    $this->repository = $repository;
+    $this->request = $request;
     return $this;
   }
 
@@ -33,16 +30,6 @@ final class DiffusionBrowseTableView extends AphrontView {
     return $this;
   }
 
-  public function setRoot($root) {
-    $this->root = $root;
-    return $this;
-  }
-
-  public function setCommit($commit) {
-    $this->commit = $commit;
-    return $this;
-  }
-
   public function render() {
     $rows = array();
     foreach ($this->paths as $path) {