Simplify build file from data-or-hash code

Summary:
We have a bit more copy-paste than we need, consolidate a bit.

(Also switch Mercurial to download git diffs, which it handles well; we use them in "arc patch".)

Test Plan:
  - Downloaded a raw diff from Differential.
  - Downloaded a raw change from Diffusion.
  - Downloaded a raw file from Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

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

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -940,49 +940,36 @@ final class DifferentialRevisionViewController extends DifferentialController {
     $vcs = $repository ? $repository->getVersionControlSystem() : null;
     switch ($vcs) {
       case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
         $raw_diff = $bundle->toGitPatch();
         break;
       case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
       default:
         $raw_diff = $bundle->toUnifiedDiff();
         break;
     }
 
-    $hash = PhabricatorHash::digest($raw_diff);
+    $request_uri = $this->getRequest()->getRequestURI();
 
-    $file = id(new PhabricatorFile())->loadOneWhere(
+    // this ends up being something like
-      'contentHash = %s LIMIT 1',
+    //   D123.diff
-      $hash);
+    // or the verbose
-
+    //   D123.vs123.id123.whitespaceignore-all.diff
-    if (!$file) {
+    // lame but nice to include these options
-      $request_uri = $this->getRequest()->getRequestURI();
+    $file_name = ltrim($request_uri->getPath(), '/').'.';
-
+    foreach ($request_uri->getQueryParams() as $key => $value) {
-      // this ends up being something like
+      if ($key == 'download') {
-      //   D123.diff
+        continue;
-      // or the verbose
-      //   D123.vs123.id123.whitespaceignore-all.diff
-      // lame but nice to include these options
-      $file_name = ltrim($request_uri->getPath(), '/') . '.';
-      foreach ($request_uri->getQueryParams() as $key => $value) {
-        if ($key == 'download') {
-          continue;
-        }
-        $file_name .= $key . $value . '.';
       }
-      $file_name .= 'diff';
+      $file_name .= $key.$value.'.';
-
-      // We're just caching the data; this is always safe.
-      $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
-
-      $file = PhabricatorFile::newFromFileData(
-        $raw_diff,
-        array(
-          'name' => $file_name,
-        ));
-
-      unset($unguarded);
     }
+    $file_name .= 'diff';
+
+    $file = PhabricatorFile::buildFromFileDataOrHash(
+      $raw_diff,
+      array(
+        'name' => $file_name,
+      ));
 
     return id(new AphrontRedirectResponse())->setURI($file->getBestURI());
 

src/applications/diffusion/controller/DiffusionBrowseFileController.php

@@ -639,25 +639,11 @@ final class DiffusionBrowseFileController extends DiffusionController {
   }
 
   private function loadFileForData($path, $data) {
-    $hash = PhabricatorHash::digest($data);
+    return PhabricatorFile::buildFromFileDataOrHash(
-
+      $data,
-    $file = id(new PhabricatorFile())->loadOneWhere(
+      array(
-      'contentHash = %s LIMIT 1',
+        'name' => basename($path),
-      $hash);
+      ));
-    if (!$file) {
-      // We're just caching the data; this is always safe.
-      $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
-
-      $file = PhabricatorFile::newFromFileData(
-        $data,
-        array(
-          'name' => basename($path),
-        ));
-
-      unset($unguarded);
-    }
-
-    return $file;
   }
 
   private function buildRawResponse($path, $data) {

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -872,23 +872,11 @@ final class DiffusionCommitController extends DiffusionController {
     $raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest);
     $raw_diff  = $raw_query->loadRawDiff();
 
-    $hash = PhabricatorHash::digest($raw_diff);
+    $file = PhabricatorFile::buildFromFileDataOrHash(
-
+      $raw_diff,
-    $file = id(new PhabricatorFile())->loadOneWhere(
+      array(
-      'contentHash = %s LIMIT 1',
+        'name' => $drequest->getCommit().'.diff',
-      $hash);
+      ));
-    if (!$file) {
-      // We're just caching the data; this is always safe.
-      $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
-
-      $file = PhabricatorFile::newFromFileData(
-        $raw_diff,
-        array(
-          'name' => $drequest->getCommit().'.diff',
-        ));
-
-      unset($unguarded);
-    }
 
     return id(new AphrontRedirectResponse())->setURI($file->getBestURI());
   }

src/applications/files/storage/PhabricatorFile.php

@@ -101,6 +101,45 @@ final class PhabricatorFile extends PhabricatorFileDAO {
     }
   }
 
+
+  /**
+   * Given a block of data, try to load an existing file with the same content
+   * if one exists. If it does not, build a new file.
+   *
+   * This method is generally used when we have some piece of semi-trusted data
+   * like a diff or a file from a repository that we want to show to the user.
+   * We can't just dump it out because it may be dangerous for any number of
+   * reasons; instead, we need to serve it through the File abstraction so it
+   * ends up on the CDN domain if one is configured and so on. However, if we
+   * simply wrote a new file every time we'd potentially end up with a lot
+   * of redundant data in file storage.
+   *
+   * To solve these problems, we use file storage as a cache and reuse the
+   * same file again if we've previously written it.
+   *
+   * NOTE: This method unguards writes.
+   *
+   * @param string  Raw file data.
+   * @param dict    Dictionary of file information.
+   */
+  public static function buildFromFileDataOrHash(
+    $data,
+    array $params = array()) {
+
+    $file = id(new PhabricatorFile())->loadOneWhere(
+      'contentHash = %s LIMIT 1',
+      PhabricatorHash::digest($data));
+
+    if (!$file) {
+      $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+      $file = PhabricatorFile::newFromFileData($data, $params);
+      unset($unguarded);
+    }
+
+    return $file;
+  }
+
+
   public static function newFromFileData($data, array $params = array()) {
     $selector = PhabricatorEnv::newObjectFromConfig('storage.engine-selector');