Allow Phabricator storage engines to be extended and configured

Summary:
See T344. Currently, there's a hard-coded 12MB filesize limit and some awkward
interactions with MySQL's max_allowed_packet. Make this system generally more
robust:

  - Move the upload limit to configuration.
  - Add setup steps which reconcile max_allowed_packet vs MySQL file storage
limits.
  - Add a layer of indirection between uploading files and storage engines.
  - Allow the definition of new storage engines.
  - Define a local disk storage engine.
  - Add a "storage engine selector" class which manages choosing which storage
engines to put files in.
  - Document storage engines.
  - Document file storage classes.

Test Plan:
Setup mode:

  - Disabled MySQL storage engine, misconfigured it, configured it correctly.
  - Disabled file storage engine, set it to something invalid, set it to
something valid.
  - Verified max_allowed_packet is read correctly.

Application mode:

  - Configured local file storage.
  - Uploaded large and small files.
  - Verified larger files were written to local storage.
  - Verified smaller files were written to MySQL blob storage.

Documentation:

  - Read documentation.

Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 695

.divinerconfig

@@ -17,6 +17,7 @@
     "aphront" : "Aphront (Web Stack)",
     "console" : "DarkConsole (Debugging Console)",
     "storage" : "Storage",
+    "filestorage" : "File Storage",
     "irc" : "IRC",
     "markup" : "Remarkup Extensions"
   },

CHANGELOG

@@ -2,6 +2,11 @@ This is not a complete list of changes, just of API or workflow changes that may
 break existing installs. Newer changes are listed at the top. If you pull new
 changes and things stop working, check here first!
 
+July 21 2011 - New File Storage Engines
+  Some of the default configuration for storage engines has changed.
+  Particularly, the default maximum filesize for MySQL storage is now 1MB, down
+  from 12MB. See the article "Configuring Storage Engines" for details.
+
 July 18 2011 - Phriction Link Syntax Change
   Phriction link sytnax now requires [[double brackets]], not single
   brackets.

conf/default.conf.php

@@ -345,6 +345,42 @@ return array(
   // unsure, it is safer to leave this disabled.
   'files.enable-proxy' => false,
 
+
+// -- Storage --------------------------------------------------------------- //
+
+  // Phabricator allows users to upload files, and can keep them in various
+  // storage engines. This section allows you to configure which engines
+  // Phabricator will use, and how it will use them.
+
+  // The largest filesize Phabricator will store in the MySQL BLOB storage
+  // engine, which just uses a database table to store files. While this isn't a
+  // best practice, it's really easy to set up. This is hard-limited by the
+  // value of 'max_allowed_packet' in MySQL (since this often defaults to 1MB,
+  // the default here is slightly smaller than 1MB). Set this to 0 to disable
+  // use of the MySQL blob engine.
+  'storage.mysql-engine.max-size' => 1000000,
+
+  // Phabricator provides a local disk storage engine, which just writes files
+  // to some directory on local disk. The webserver must have read/write
+  // permissions on this directory. This is straightforward and suitable for
+  // most installs, but will not scale past one web frontend unless the path
+  // is actually an NFS mount, since you'll end up with some of the files
+  // written to each web frontend and no way for them to share. To use the
+  // local disk storage engine, specify the path to a directory here. To
+  // disable it, specify null.
+  'storage.local-disk.path'       => null,
+
+  // TODO: Implement S3.
+
+  // Phabricator uses a storage engine selector to choose which storage engine
+  // to use when writing file data. If you add new storage engines or want to
+  // provide very custom rules (e.g., write images to one storage engine and
+  // other files to a different one), you can provide an alternate
+  // implementation here. The default engine will use choose MySQL, Local Disk,
+  // and S3, in that order, if they have valid configurations above and a file
+  // fits within configured limits.
+  'storage.engine-selector' => 'PhabricatorDefaultFileStorageEngineSelector',
+
 // -- Differential ---------------------------------------------------------- //
 
   'differential.revision-custom-detail-renderer'  => null,

src/__phutil_library_map__.php

@@ -321,6 +321,7 @@ phutil_register_library_map(array(
     'PhabricatorDaemonReference' => 'infrastructure/daemon/control/reference',
     'PhabricatorDaemonTimelineConsoleController' => 'applications/daemon/controller/timeline',
     'PhabricatorDaemonTimelineEventController' => 'applications/daemon/controller/timelineevent',
+    'PhabricatorDefaultFileStorageEngineSelector' => 'applications/files/engineselector/default',
     'PhabricatorDifferenceEngine' => 'infrastructure/diff/engine',
     'PhabricatorDirectoryCategory' => 'applications/directory/storage/category',
     'PhabricatorDirectoryCategoryDeleteController' => 'applications/directory/controller/categorydelete',
@@ -369,6 +370,8 @@ phutil_register_library_map(array(
     'PhabricatorFileProxyController' => 'applications/files/controller/proxy',
     'PhabricatorFileProxyImage' => 'applications/files/storage/proxyimage',
     'PhabricatorFileStorageBlob' => 'applications/files/storage/storageblob',
+    'PhabricatorFileStorageEngine' => 'applications/files/engine/base',
+    'PhabricatorFileStorageEngineSelector' => 'applications/files/engineselector/base',
     'PhabricatorFileTransformController' => 'applications/files/controller/transform',
     'PhabricatorFileURI' => 'applications/files/uri',
     'PhabricatorFileUploadController' => 'applications/files/controller/upload',
@@ -387,6 +390,7 @@ phutil_register_library_map(array(
     'PhabricatorJavelinLinter' => 'infrastructure/lint/linter/javelin',
     'PhabricatorLintEngine' => 'infrastructure/lint/engine',
     'PhabricatorLiskDAO' => 'applications/base/storage/lisk',
+    'PhabricatorLocalDiskFileStorageEngine' => 'applications/files/engine/localdisk',
     'PhabricatorLoginController' => 'applications/auth/controller/login',
     'PhabricatorLogoutController' => 'applications/auth/controller/logout',
     'PhabricatorMailImplementationAdapter' => 'applications/metamta/adapter/base',
@@ -413,6 +417,7 @@ phutil_register_library_map(array(
     'PhabricatorMetaMTASendController' => 'applications/metamta/controller/send',
     'PhabricatorMetaMTASendGridReceiveController' => 'applications/metamta/controller/sendgridreceive',
     'PhabricatorMetaMTAViewController' => 'applications/metamta/controller/view',
+    'PhabricatorMySQLFileStorageEngine' => 'applications/files/engine/mysql',
     'PhabricatorOAuthDefaultRegistrationController' => 'applications/auth/controller/oauthregistration/default',
     'PhabricatorOAuthDiagnosticsController' => 'applications/auth/controller/oauthdiagnostics',
     'PhabricatorOAuthFailureView' => 'applications/auth/view/oauthfailure',
@@ -465,6 +470,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectProfileController' => 'applications/project/controller/profile',
     'PhabricatorProjectProfileEditController' => 'applications/project/controller/profileedit',
     'PhabricatorProjectStatus' => 'applications/project/constants/status',
+    'PhabricatorProjectSubproject' => 'applications/project/storage/subproject',
     'PhabricatorRedirectController' => 'applications/base/controller/redirect',
     'PhabricatorRefreshCSRFController' => 'applications/auth/controller/refresh',
     'PhabricatorRemarkupRuleDifferential' => 'infrastructure/markup/remarkup/markuprule/differential',
@@ -868,6 +874,7 @@ phutil_register_library_map(array(
     'PhabricatorDaemonLogViewController' => 'PhabricatorDaemonController',
     'PhabricatorDaemonTimelineConsoleController' => 'PhabricatorDaemonController',
     'PhabricatorDaemonTimelineEventController' => 'PhabricatorDaemonController',
+    'PhabricatorDefaultFileStorageEngineSelector' => 'PhabricatorFileStorageEngineSelector',
     'PhabricatorDirectoryCategory' => 'PhabricatorDirectoryDAO',
     'PhabricatorDirectoryCategoryDeleteController' => 'PhabricatorDirectoryController',
     'PhabricatorDirectoryCategoryEditController' => 'PhabricatorDirectoryController',
@@ -923,6 +930,7 @@ phutil_register_library_map(array(
     'PhabricatorJavelinLinter' => 'ArcanistLinter',
     'PhabricatorLintEngine' => 'PhutilLintEngine',
     'PhabricatorLiskDAO' => 'LiskDAO',
+    'PhabricatorLocalDiskFileStorageEngine' => 'PhabricatorFileStorageEngine',
     'PhabricatorLoginController' => 'PhabricatorAuthController',
     'PhabricatorLogoutController' => 'PhabricatorAuthController',
     'PhabricatorMailImplementationAmazonSESAdapter' => 'PhabricatorMailImplementationPHPMailerLiteAdapter',
@@ -945,6 +953,7 @@ phutil_register_library_map(array(
     'PhabricatorMetaMTASendController' => 'PhabricatorMetaMTAController',
     'PhabricatorMetaMTASendGridReceiveController' => 'PhabricatorMetaMTAController',
     'PhabricatorMetaMTAViewController' => 'PhabricatorMetaMTAController',
+    'PhabricatorMySQLFileStorageEngine' => 'PhabricatorFileStorageEngine',
     'PhabricatorOAuthDefaultRegistrationController' => 'PhabricatorOAuthRegistrationController',
     'PhabricatorOAuthDiagnosticsController' => 'PhabricatorAuthController',
     'PhabricatorOAuthFailureView' => 'AphrontView',
@@ -991,6 +1000,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectProfile' => 'PhabricatorProjectDAO',
     'PhabricatorProjectProfileController' => 'PhabricatorProjectController',
     'PhabricatorProjectProfileEditController' => 'PhabricatorProjectController',
+    'PhabricatorProjectSubproject' => 'PhabricatorProjectDAO',
     'PhabricatorRedirectController' => 'PhabricatorController',
     'PhabricatorRefreshCSRFController' => 'PhabricatorAuthController',
     'PhabricatorRemarkupRuleDifferential' => 'PhabricatorRemarkupRuleObjectName',

src/applications/files/engine/base/PhabricatorFileStorageEngine.php

@@ -0,0 +1,104 @@
+<?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.
+ */
+
+/**
+ * Defines a storage engine which can write file data somewhere (like a
+ * database, local disk, Amazon S3, the A:\ drive, or a custom filer) and
+ * retrieve it later.
+ *
+ * You can extend this class to provide new file storage backends.
+ *
+ * For more information, see @{article:File Storage Technical Documentation}.
+ *
+ * @task meta Engine Metadata
+ * @task file Managing File Data
+ * @group filestorage
+ */
+abstract class PhabricatorFileStorageEngine {
+
+  final public function __construct() {
+    // <empty>
+  }
+
+
+/* -(  Engine Metadata  )---------------------------------------------------- */
+
+
+  /**
+   * Return a unique, nonempty string which identifies this storage engine.
+   * This is used to look up the storage engine when files needs to be read or
+   * deleted. For instance, if you store files by giving them to a duck for
+   * safe keeping in his nest down by the pond, you might return 'duck' from
+   * this method.
+   *
+   * @return string Unique string for this engine, max length 32.
+   * @task meta
+   */
+  abstract public function getEngineIdentifier();
+
+
+/* -(  Managing File Data  )------------------------------------------------- */
+
+
+  /**
+   * Write file data to the backing storage and return a handle which can later
+   * be used to read or delete it. For example, if the backing storage is local
+   * disk, the handle could be the path to the file.
+   *
+   * The caller will provide a $params array, which may be empty or may have
+   * some metadata keys (like "name" and "author") in it. You should be prepared
+   * to handle writes which specify no metadata, but might want to optionally
+   * use some keys in this array for debugging or logging purposes. This is
+   * the same dictionary passed to @{method:PhabricatorFile::NewFromFileData},
+   * so you could conceivably do custom things with it.
+   *
+   * If you are unable to write for whatever reason (e.g., the disk is full),
+   * throw an exception. If there are other satisfactory but less-preferred
+   * storage engines available, they will be tried.
+   *
+   * @param  string The file data to write.
+   * @param  array  File metadata (name, author), if available.
+   * @return string Unique string which identifies the stored file, max length
+   *                255.
+   * @task file
+   */
+  abstract public function writeFile($data, array $params);
+
+
+  /**
+   * Read the contents of a file previously written by @{method:writeFile}.
+   *
+   * @param   string  The handle returned from @{method:writeFile} when the
+   *                  file was written.
+   * @return  string  File contents.
+   * @task file
+   */
+  abstract public function readFile($handle);
+
+
+  /**
+   * Delete the data for a file previously written by @{method:writeFile}.
+   *
+   * @param   string  The handle returned from @{method:writeFile} when the
+   *                  file was written.
+   * @return  void
+   * @task file
+   */
+  abstract public function deleteFile($handle);
+
+}

src/applications/files/engine/base/__init__.php

@@ -0,0 +1,10 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+
+phutil_require_source('PhabricatorFileStorageEngine.php');

src/applications/files/engine/localdisk/PhabricatorLocalDiskFileStorageEngine.php

@@ -0,0 +1,138 @@
+<?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.
+ */
+
+/**
+ * Local disk storage engine. Keeps files on local disk. This engine is easy
+ * to set up, but it doesn't work if you have multiple web frontends!
+ *
+ * @task impl     Implementation
+ * @task internal Internals
+ * @group filestorage
+ */
+final class PhabricatorLocalDiskFileStorageEngine
+  extends PhabricatorFileStorageEngine {
+
+
+/* -(  Implementation  )----------------------------------------------------- */
+
+
+  /**
+   * This engine identifies as "local-disk".
+   * @task impl
+   */
+  public function getEngineIdentifier() {
+    return 'local-disk';
+  }
+
+
+  /**
+   * Write the file data to local disk. Returns the relative path as the
+   * file data handle.
+   * @task impl
+   */
+  public function writeFile($data, array $params) {
+
+    $root = $this->getLocalDiskFileStorageRoot();
+
+    // Generate a random, unique file path like "ab/29/1f918a9ac39201ff". We
+    // put a couple of subdirectories up front to avoid a situation where we
+    // have one directory with a zillion files in it, since this is generally
+    // bad news.
+    do {
+      $name = md5(mt_rand());
+      $name = preg_replace('/^(..)(..)(.*)$/', '\\1/\\2/\\3', $name);
+      if (!Filesystem::pathExists($root.'/'.$name)) {
+        break;
+      }
+    } while (true);
+
+    $parent = $root.'/'.dirname($name);
+    if (!Filesystem::pathExists($parent)) {
+      execx('mkdir -p %s', $parent);
+    }
+
+    Filesystem::writeFile($root.'/'.$name, $data);
+
+    return $name;
+  }
+
+
+  /**
+   * Read the file data off local disk.
+   * @task impl
+   */
+  public function readFile($handle) {
+    $path = $this->getLocalDiskFileStorageFullPath($handle);
+    return Filesystem::readFile($path);
+  }
+
+
+  /**
+   * Deletes the file from local disk, if it exists.
+   * @task impl
+   */
+  public function deleteFile($handle) {
+    $path = $this->getLocalDiskFileStorageFullPath($handle);
+    if (Filesystem::pathExists($path)) {
+      Filesystem::remove($path);
+    }
+  }
+
+
+/* -(  Internals  )---------------------------------------------------------- */
+
+
+  /**
+   * Get the configured local disk path for file storage.
+   *
+   * @return string Absolute path to somewhere that files can be stored.
+   * @task internal
+   */
+  private function getLocalDiskFileStorageRoot() {
+    $root = PhabricatorEnv::getEnvConfig('storage.local-disk.path');
+
+    if (!$root || $root == '/' || $root[0] != '/') {
+      throw new Exception(
+        "Malformed local disk storage root. You must provide an absolute ".
+        "path, and can not use '/' as the root.");
+    }
+
+    return rtrim($root, '/');
+  }
+
+
+  /**
+   * Convert a handle into an absolute local disk path.
+   *
+   * @param string File data handle.
+   * @return string Absolute path to the corresponding file.
+   * @task internal
+   */
+  private function getLocalDiskFileStorageFullPath($handle) {
+    // Make sure there's no funny business going on here. Users normally have
+    // no ability to affect the content of handles, but double-check that
+    // we're only accessing local storage just in case.
+    if (!preg_match('@^[a-f0-9]{2}/[a-f0-9]{2}/[a-f0-9]{28}$@', $handle)) {
+      throw new Exception(
+        "Local disk filesystem handle '{$handle}' is malformed!");
+    }
+    $root = $this->getLocalDiskFileStorageRoot();
+    return $root.'/'.$handle;
+  }
+
+}

src/applications/files/engine/localdisk/__init__.php

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

src/applications/files/engine/mysql/PhabricatorMySQLFileStorageEngine.php

@@ -0,0 +1,97 @@
+<?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.
+ */
+
+/**
+ * MySQL blob storage engine. This engine is the easiest to set up but doesn't
+ * scale very well.
+ *
+ * It uses the @{class:PhabricatorFileStorageBlob} to actually access the
+ * underlying database table.
+ *
+ * @task impl     Implementation
+ * @task internal Internals
+ * @group filestorage
+ */
+final class PhabricatorMySQLFileStorageEngine
+  extends PhabricatorFileStorageEngine {
+
+/* -(  Implementation  )----------------------------------------------------- */
+
+
+  /**
+   * For historical reasons, this engine identifies as "blob".
+   *
+   * @task impl
+   */
+  public function getEngineIdentifier() {
+    return 'blob';
+  }
+
+
+  /**
+   * Write file data into the big blob store table in MySQL. Returns the row
+   * ID as the file data handle.
+   *
+   * @task impl
+   */
+  public function writeFile($data, array $params) {
+    $blob = new PhabricatorFileStorageBlob();
+    $blob->setData($data);
+    $blob->save();
+
+    return $blob->getID();
+  }
+
+
+  /**
+   * Load a stored blob from MySQL.
+   * @task impl
+   */
+  public function readFile($handle) {
+    return $this->loadFromMySQLFileStorage($handle)->getData();
+  }
+
+
+  /**
+   * Delete a blob from MySQL.
+   * @task impl
+   */
+  public function deleteFile($handle) {
+    $this->loadFromMySQLFileStorage($handle)->delete();
+  }
+
+
+/* -(  Internals  )---------------------------------------------------------- */
+
+
+  /**
+   * Load the Lisk object that stores the file data for a handle.
+   *
+   * @param string  File data handle.
+   * @return PhabricatorFileStorageBlob Data DAO.
+   * @task internal
+   */
+  private function loadFromMySQLFileStorage($handle) {
+    $blob = id(new PhabricatorFileStorageBlob())->load($handle);
+    if (!$blob) {
+      throw new Exception("Unable to load MySQL blob file '{$handle}'!");
+    }
+    return $blob;
+  }
+
+}

src/applications/files/engine/mysql/__init__.php

@@ -0,0 +1,15 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/files/engine/base');
+phutil_require_module('phabricator', 'applications/files/storage/storageblob');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('PhabricatorMySQLFileStorageEngine.php');

src/applications/files/engineselector/base/PhabricatorFileStorageEngineSelector.php

@@ -0,0 +1,63 @@
+<?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.
+ */
+
+/**
+ * Chooses appropriate storage engine(s) for files. When Phabricator needs
+ * to write a blob of file data, it uses the configured selector to get a list
+ * of suitable @{class:PhabricatorFileStorageEngine}s. For more information,
+ * see @{article:File Storage Technical Documentation}.
+ *
+ * @group filestorage
+ * @task  select  Selecting Storage Engines
+ */
+abstract class PhabricatorFileStorageEngineSelector {
+
+  final public function __construct() {
+    // <empty>
+  }
+
+
+/* -(  Selecting Storage Engines  )------------------------------------------ */
+
+
+  /**
+   * Select valid storage engines for a file. This method will be called by
+   * Phabricator when it needs to store a file permanently. It must return a
+   * list of valid @{class:PhabricatorFileStorageEngine}s.
+   *
+   * If you are are extending this class to provide a custom selector, you
+   * probably just want it to look like this:
+   *
+   *   return array(new MyCustomFileStorageEngine());
+   *
+   * ...that is, store every file in whatever storage engine you're using.
+   * However, you can also provide multiple storage engines, or store some files
+   * in one engine and some files in a different engine by implementing a more
+   * complex selector.
+   *
+   * @param string  File data.
+   * @param dict    Dictionary of optional file metadata. This may be empty, or
+   *                have some additional keys like 'file' and 'author' which
+   *                provide metadata.
+   * @return list   List of @{class:PhabricatorFileStorageEngine}s, ordered by
+   *                preference.
+   * @task select
+   */
+  abstract public function selectStorageEngines($data, array $params);
+
+}

src/applications/files/engineselector/base/__init__.php

@@ -0,0 +1,10 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+
+phutil_require_source('PhabricatorFileStorageEngineSelector.php');

src/applications/files/engineselector/default/PhabricatorDefaultFileStorageEngineSelector.php

@@ -0,0 +1,65 @@
+<?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.
+ */
+
+/**
+ * Default storage engine selector. See
+ * @{class:PhabricatorFileStorageEngineSelector} and @{article:File Storage
+ * Technical Documentation} for more information.
+ *
+ * @group filestorage
+ */
+final class PhabricatorDefaultFileStorageEngineSelector
+  extends PhabricatorFileStorageEngineSelector {
+
+  /**
+   * Select viable default storage engines according to configuration. We'll
+   * select the MySQL and Local Disk storage engines if they are configured
+   * to allow a given file.
+   */
+  public function selectStorageEngines($data, array $params) {
+    $length = strlen($data);
+
+    $mysql_key = 'storage.mysql-engine.max-size';
+    $mysql_limit = PhabricatorEnv::getEnvConfig($mysql_key);
+
+    $engines = array();
+    if ($mysql_limit && $length <= $mysql_limit) {
+      $engines[] = new PhabricatorMySQLFileStorageEngine();
+    }
+
+    $local_key = 'storage.local-disk.path';
+    $local_path = PhabricatorEnv::getEnvConfig($local_key);
+    if ($local_path) {
+      $engines[] = new PhabricatorLocalDiskFileStorageEngine();
+    }
+
+    if ($mysql_limit && empty($engines)) {
+      // If we return no engines, an exception will be thrown but it will be
+      // a little vague ("No valid storage engines"). Since this is a default
+      // case, throw a more specific exception.
+      throw new Exception(
+        "This file exceeds the configured MySQL storage engine filesize ".
+        "limit, but no other storage engines are configured. Increase the ".
+        "MySQL storage engine limit or configure a storage engine suitable ".
+        "for larger files.");
+    }
+
+    return $engines;
+  }
+
+}

src/applications/files/engineselector/default/__init__.php

@@ -0,0 +1,15 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/files/engine/localdisk');
+phutil_require_module('phabricator', 'applications/files/engine/mysql');
+phutil_require_module('phabricator', 'applications/files/engineselector/base');
+phutil_require_module('phabricator', 'infrastructure/env');
+
+
+phutil_require_source('PhabricatorDefaultFileStorageEngineSelector.php');

src/applications/files/storage/file/PhabricatorFile.php

@@ -18,13 +18,8 @@
 
 class PhabricatorFile extends PhabricatorFileDAO {
 
-  const STORAGE_ENGINE_BLOB = 'blob';
-
   const STORAGE_FORMAT_RAW  = 'raw';
 
-  // TODO: We need to reconcile this with MySQL packet size.
-  const FILE_SIZE_BYTE_LIMIT = 12582912;
-
   protected $phid;
   protected $name;
   protected $mimeType;
@@ -80,10 +75,49 @@ class PhabricatorFile extends PhabricatorFileDAO {
   }
 
   public static function newFromFileData($data, array $params = array()) {
-    $file_size = strlen($data);
 
-    if ($file_size > self::FILE_SIZE_BYTE_LIMIT) {
+    $selector_class = PhabricatorEnv::getEnvConfig('storage.engine-selector');
-      throw new Exception("File is too large to store.");
+    $selector = newv($selector_class, array());
+
+    $engines = $selector->selectStorageEngines($data, $params);
+    if (!$engines) {
+      throw new Exception("No valid storage engines are available!");
+    }
+
+    $data_handle = null;
+    $engine_identifier = null;
+    foreach ($engines as $engine) {
+      try {
+        // Perform the actual write.
+        $data_handle = $engine->writeFile($data, $params);
+        if (!$data_handle || strlen($data_handle) > 255) {
+          // This indicates an improperly implemented storage engine.
+          throw new Exception(
+            "Storage engine '{$engine}' executed writeFile() but did not ".
+            "return a valid handle ('{$data_handle}') to the data: it must ".
+            "be nonempty and no longer than 255 characters.");
+        }
+
+        $engine_identifier = $engine->getEngineIdentifier();
+        if (!$engine_identifier || strlen($engine_identifier) > 32) {
+          throw new Exception(
+            "Storage engine '{$engine}' returned an improper engine ".
+            "identifier '{$engine_identifier}': it must be nonempty ".
+            "and no longer than 32 characters.");
+        }
+
+        // We stored the file somewhere so stop trying to write it to other
+        // places.
+        break;
+      } catch (Exception $ex) {
+        // If an engine doesn't work, keep trying all the other valid engines
+        // in case something else works.
+        phlog($ex);
+      }
+    }
+
+    if (!$data_handle) {
+      throw new Exception("All storage engines failed to write file!");
     }
 
     $file_name = idx($params, 'name');
@@ -98,16 +132,12 @@ class PhabricatorFile extends PhabricatorFileDAO {
     $file->setByteSize(strlen($data));
     $file->setAuthorPHID($authorPHID);
 
-    $blob = new PhabricatorFileStorageBlob();
+    $file->setStorageEngine($engine_identifier);
-    $blob->setData($data);
+    $file->setStorageHandle($data_handle);
-    $blob->save();
 
-    // TODO: This stuff is almost certainly YAGNI, but we could imagine having
+    // TODO: This is probably YAGNI, but allows for us to do encryption or
-    // an alternate disk store and gzipping or encrypting things or something
+    // compression later if we want.
-    // crazy like that and this isn't toooo much extra code.
-    $file->setStorageEngine(self::STORAGE_ENGINE_BLOB);
     $file->setStorageFormat(self::STORAGE_FORMAT_RAW);
-    $file->setStorageHandle($blob->getID());
 
     if (isset($params['mime-type'])) {
       $file->setMimeType($params['mime-type']);
@@ -160,38 +190,19 @@ class PhabricatorFile extends PhabricatorFileDAO {
   }
 
   public function delete() {
-    $this->openTransaction();
+    $engine = $this->instantiateStorageEngine();
-      switch ($this->getStorageEngine()) {
-        case self::STORAGE_ENGINE_BLOB:
-          $handle = $this->getStorageHandle();
-          $blob = id(new PhabricatorFileStorageBlob())->load($handle);
-          $blob->delete();
-          break;
-        default:
-          throw new Exception("Unknown storage engine!");
-      }
 
     $ret = parent::delete();
-    $this->saveTransaction();
+
+    $engine->deleteFile($this->getStorageHandle());
+
     return $ret;
   }
 
   public function loadFileData() {
 
-    $handle = $this->getStorageHandle();
+    $engine = $this->instantiateStorageEngine();
-    $data   = null;
+    $data = $engine->readFile($this->getStorageHandle());
-
-    switch ($this->getStorageEngine()) {
-      case self::STORAGE_ENGINE_BLOB:
-        $blob = id(new PhabricatorFileStorageBlob())->load($handle);
-        if (!$blob) {
-          throw new Exception("Failed to load file blob data.");
-        }
-        $data = $blob->getData();
-        break;
-      default:
-        throw new Exception("Unknown storage engine.");
-    }
 
     switch ($this->getStorageFormat()) {
       case self::STORAGE_FORMAT_RAW:
@@ -253,6 +264,22 @@ class PhabricatorFile extends PhabricatorFileDAO {
     }
   }
 
+  protected function instantiateStorageEngine() {
+    $engines = id(new PhutilSymbolLoader())
+      ->setType('class')
+      ->setAncestorClass('PhabricatorFileStorageEngine')
+      ->selectAndLoadSymbols();
+
+    foreach ($engines as $engine_class) {
+      $engine = newv($engine_class['name'], array());
+      if ($engine->getEngineIdentifier() == $this->getStorageEngine()) {
+        return $engine;
+      }
+    }
+
+    throw new Exception("File's storage engine could be located!");
+  }
+
   public function getViewableMimeType() {
     $mime_map = PhabricatorEnv::getEnvConfig('files.viewable-mime-types');
 

src/applications/files/storage/file/__init__.php

@@ -7,16 +7,17 @@
 
 
 phutil_require_module('phabricator', 'applications/files/storage/base');
-phutil_require_module('phabricator', 'applications/files/storage/storageblob');
 phutil_require_module('phabricator', 'applications/files/uri');
 phutil_require_module('phabricator', 'applications/phid/constants');
 phutil_require_module('phabricator', 'applications/phid/storage/phid');
 phutil_require_module('phabricator', 'infrastructure/env');
 
+phutil_require_module('phutil', 'error');
 phutil_require_module('phutil', 'filesystem');
 phutil_require_module('phutil', 'filesystem/tempfile');
 phutil_require_module('phutil', 'future/exec');
 phutil_require_module('phutil', 'parser/uri');
+phutil_require_module('phutil', 'symbols');
 phutil_require_module('phutil', 'utils');
 
 

src/applications/files/storage/storageblob/PhabricatorFileStorageBlob.php

@@ -16,6 +16,11 @@
  * limitations under the License.
  */
 
+/**
+ * Simple blob store DAO for @{class:PhabricatorMySQLFileStorageEngine}.
+ *
+ * @group filestorage
+ */
 class PhabricatorFileStorageBlob extends PhabricatorFileDAO {
 
   protected $data;

src/docs/configuration/configuration_guide.diviner

@@ -138,6 +138,8 @@ Continue by:
   - upgrading the database schema with @{article:Upgrading Schema}; or
   - setting up your admin account and login/registration with
     @{article:Configuring Accounts and Registration}; or
+  - configuring where uploaded fils and attachments will be stored with
+    @{article:Configuring File Storage}; or
   - configuring Phabricator so it can send mail with
     @{article:Configuring Outbound Email}; or
   - configuring inbound mail with @{article:Configuring Inbound Email}; or

src/docs/configuration/configuring_file_storage.diviner

@@ -0,0 +1,77 @@
+@title Configuring File Storage
+@group config
+
+Setup how Phabricator will store files.
+
+= Overview =
+
+Phabricator allows users to upload files, and several applications use file
+storage (for instance, Maniphest allows you to attach files to tasks). You can
+configure several different storage systems:
+
+  - you can store data in MySQL: this is the easiest to set up, but doesn't
+    scale well;
+  - you can store data on local disk: this is also easy to set up but won't
+    scale to multiple web frontends without NFS;
+  - or you can build a custom storage engine.
+
+By default, Phabricator is configured to store files up to 1MB in MySQL, and
+reject files larger than 1MB. It is recommended you set up local disk storage
+for files larger than 1MB. This should be sufficient for most installs. If you
+have a larger install or more unique requirements, you may want to customize
+this further.
+
+For technical documentation (including instructions on building custom storage
+engines) see @{article:File Storage Technical Documentation}.
+
+You don't have to fully configure this immediately, the defaults are okay until
+you need to upload larger files and it's relatively easy to port files between
+storage engines later.
+
+= Storage Engines =
+
+Builtin storage engines and information on how to configure them.
+
+== MySQL ==
+
+  - **Pros**: Fast, no setup required.
+  - **Cons**: Storing files in a database is a classic bad idea. Does not scale
+    well. Maximum file size is limited.
+
+MySQL storage is configured by default, for files up to (just under) 1MB. You
+can configure it with these keys:
+
+  - ##storage.mysql-engine.max-size##: Change the filesize limit. Note that
+    this must be smaller than 'max_allowed_packet' on the server. Set to 0
+    to disable.
+
+For most installs, it is recommended you configure local disk storage below,
+and then either leave this as is or disable it, depending on how upset you feel
+about putting files in a database.
+
+== Local Disk ==
+
+  - **Pros**: Very simple. Almost no setup required.
+  - **Cons**: Doesn't scale to multiple web frontends without NFS.
+
+For most installs, it is **strongly recommended** that you configure local disk
+storage. To do this, set the configuration key:
+
+  - ##storage.local-disk.path##: Set to some writable directory on local disk.
+    Make that directory. You're done.
+
+== Custom Engine ==
+
+For details about writing a custom storage engine, see @{article:File Storage
+Technical Documentation}.
+
+= Testing Storage Engines =
+
+You can test that things are correctly configured by going to the Files
+application (##/files/##) and uploading files.
+
+= Next Steps =
+
+Continue by:
+
+  - returning to the @{article:Configuration Guide}.

src/docs/technical/files.diviner

@@ -0,0 +1,39 @@
+@title File Storage Technical Documentation
+@group filestorage
+
+Phabricator file storage details.
+
+= Overview =
+
+Phabricator has a simple, general-purpose file storage system with configurable
+storage backends that allows you to choose where files are stored. For a user
+guide, see @{article:Configuring File Storage}.
+
+= Class Relationships =
+
+@{class:PhabricatorFile} holds file metadata (name, author, phid), including an
+identifier for a @{class:PhabricatorFileStorageEngine} where the actual file
+data is stored, and a data handle which identifies the data within that storage
+engine.
+
+When writing data, a @{class:PhabricatorFileStorageEngineSelector} is
+instantiated (by default, @{class:PhabricatorDefaultFileStorageEngineSelector},
+but you can change this by setting the ##storage.engine-selector## key in your
+configuration). The selector returns a list of satisfactory
+@{class:PhabricatorFileStorageEngine}s, in order of preference.
+
+For instance, suppose the user is uploading a picture. The upload pipeline would
+instantiate the configured selector, which might return a
+@{class:PhabricatorMySQLFileStorageEngine} and a
+@{class:PhabricatorLocalDiskFileStorageEngine}, indicating that the picture may
+be stored in either storage engine but MySQL is preferred. If a given storage
+engine fails to perform the write, it will fall back to the next engine.
+
+= Adding New Storage Engines =
+
+To add a new storage engine, extend @{class:PhabricatorFileStorageEngine}. In
+order to make files actually get written to it, you also need to extend
+@{class:PhabricatorFileStorageEngineSelector}, provide an implementation which
+selects your storage engine for whatever files you want to store there, and then
+configure Phabricator to use your selector by setting
+##storage.engine-selector##.

src/infrastructure/setup/PhabricatorSetup.php

@@ -239,7 +239,7 @@ class PhabricatorSetup {
       self::write("[OKAY] Facebook integration OKAY\n");
     }
 
-    self::writeHeader("MySQL DATABASE CONFIGURATION");
+    self::writeHeader("MySQL DATABASE & STORAGE CONFIGURATION");
 
     $conf = DatabaseConfigurationProvider::getConfiguration();
     $conn_user = $conf->getUser();
@@ -334,7 +334,77 @@ class PhabricatorSetup {
         "...to reindex existing documents.");
     }
 
-    self::write("[OKAY] Database configuration OKAY\n");
+    $max_allowed_packet = queryfx_one(
+      $conn_raw,
+      'SHOW VARIABLES LIKE %s',
+      'max_allowed_packet');
+    $max_allowed_packet = idx($max_allowed_packet, 'Value', PHP_INT_MAX);
+
+    $recommended_minimum = 1024 * 1024;
+    if ($max_allowed_packet < $recommended_minimum) {
+      self::writeNote(
+        "MySQL is configured with a small 'max_allowed_packet' ".
+        "('{$max_allowed_packet}'), which may cause some large writes to ".
+        "fail. Consider raising this to at least {$recommended_minimum}.");
+    } else {
+      self::write(" okay  max_allowed_packet = {$max_allowed_packet}.\n");
+    }
+
+    $mysql_key = 'storage.mysql-engine.max-size';
+    $mysql_limit = PhabricatorEnv::getEnvConfig($mysql_key);
+
+    if ($mysql_limit && ($mysql_limit + 8192) > $max_allowed_packet) {
+      self::writeFailure();
+      self::write(
+        "Setup failure! Your Phabricator 'storage.mysql-engine.max-size' ".
+        "configuration ('{$mysql_limit}') must be at least 8KB smaller ".
+        "than your MySQL 'max_allowed_packet' configuration ".
+        "('{$max_allowed_packet}'). Raise the 'max_allowed_packet' in your ".
+        "MySQL configuration, or reduce the maximum file size allowed by ".
+        "the Phabricator configuration.\n");
+      return;
+    } else if (!$mysql_limit) {
+      self::write(" skip  MySQL file storage engine not configured.\n");
+    } else {
+      self::write(" okay  MySQL file storage engine configuration okay.\n");
+    }
+
+    $local_key = 'storage.local-disk.path';
+    $local_path = PhabricatorEnv::getEnvConfig($local_key);
+    if ($local_path) {
+      if (!Filesystem::pathExists($local_path) || !is_writable($local_path)) {
+        self::writeFailure();
+        self::write(
+          "Setup failure! You have configured local disk storage but the ".
+          "path you specified ('{$local_path}') does not exist or is not ".
+          "writable.\n");
+        return;
+      } else {
+        self::write(" okay  Local disk storage exists and is writable.\n");
+      }
+    } else {
+      self::write(" skip  Not configured for local disk storage.\n");
+    }
+
+    $selector = PhabricatorEnv::getEnvConfig('storage.engine-selector');
+
+    try {
+      $storage_selector_exists = class_exists($selector);
+    } catch (Exception $ex) {
+      $storage_selector_exists = false;
+    }
+
+    if ($storage_selector_exists) {
+      self::write(" okay  Using '{$selector}' as a storage engine selector.\n");
+    } else {
+      self::writeFailure();
+      self::write(
+        "Setup failure! You have configured '{$selector}' as a storage engine ".
+        "selector but it does not exist or could not be loaded.\n");
+      return;
+    }
+
+    self::write("[OKAY] Database and storage configuration OKAY\n");
 
 
     self::writeHeader("OUTBOUND EMAIL CONFIGURATION");
@@ -475,7 +545,7 @@ class PhabricatorSetup {
     flush();
 
     // This, uh, makes it look cool. -_-
-    usleep(40000);
+    usleep(20000);
   }
 
   private static function writeNote($note) {