Files now share storage. New files are written only if no other files have the same contentHash. Storage is only deleted if no other file is sharing it.

Summary: Storage is shared between files in a smart way. When uploading files, if other file have the same contentHash, then share storage. On delete, storage is permanently deleted only if no other files are sharing it

Test Plan: Upload multiple copies of the same file, while tracking database. Delete copies of files and check to see that the storage is only deleted if no other files are using it

Reviewers: epriestley

CC: aran, Korvin

Maniphest Tasks: T2454

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

src/applications/files/engine/PhabricatorTestStorageEngine.php

@@ -18,7 +18,7 @@ final class PhabricatorTestStorageEngine
   public function writeFile($data, array $params) {
     AphrontWriteGuard::willWrite();
     self::$storage[self::$nextHandle] = $data;
-    return self::$nextHandle++;
+    return (string)self::$nextHandle++;
   }
 
   public function readFile($handle) {

src/applications/files/storage/PhabricatorFile.php

@@ -132,8 +132,46 @@ final class PhabricatorFile extends PhabricatorFileDAO
     return $file;
   }
 
+  public static function newFileFromContentHash($hash, $params) {
 
-  public static function newFromFileData($data, array $params = array()) {
+    // Check to see if a file with same contentHash exist
+    $file = id(new PhabricatorFile())->loadOneWhere(
+      'contentHash = %s LIMIT 1', $hash);
+
+    if ($file) {
+      // copy storageEngine, storageHandle, storageFormat
+      $copy_of_storage_engine = $file->getStorageEngine();
+      $copy_of_storage_handle = $file->getStorageHandle();
+      $copy_of_storage_format = $file->getStorageFormat();
+      $copy_of_byteSize = $file->getByteSize();
+      $copy_of_mimeType = $file->getMimeType();
+
+      $file_name = idx($params, 'name');
+      $file_name = self::normalizeFileName($file_name);
+      $authorPHID = idx($params, 'authorPHID');
+
+      $new_file = new  PhabricatorFile();
+
+      $new_file->setName($file_name);
+      $new_file->setByteSize($copy_of_byteSize);
+      $new_file->setAuthorPHID($authorPHID);
+
+      $new_file->setContentHash($hash);
+      $new_file->setStorageEngine($copy_of_storage_engine);
+      $new_file->setStorageHandle($copy_of_storage_handle);
+      $new_file->setStorageFormat($copy_of_storage_format);
+      $new_file->setMimeType($copy_of_mimeType);
+
+      $new_file->save();
+
+      return $new_file;
+    }
+
+    return $file;
+  }
+
+  private static function buildFromFileData($data, array $params = array()) {
+    $selector = PhabricatorEnv::newObjectFromConfig('storage.engine-selector');
 
     if (isset($params['storageEngines'])) {
       $engines = $params['storageEngines'];
@@ -221,6 +259,17 @@ final class PhabricatorFile extends PhabricatorFileDAO
     return $file;
   }
 
+  public static function newFromFileData($data, array $params = array()) {
+    $hash = self::hashFileContent($data);
+    $file = self::newFileFromContentHash($hash, $params);
+
+    if ($file) {
+      return $file;
+    }
+
+    return self::buildFromFileData($data, $params);
+  }
+
   public function migrateToEngine(PhabricatorFileStorageEngine $engine) {
     if (!$this->getID() || !$this->getStorageHandle()) {
       throw new Exception(
@@ -305,15 +354,28 @@ final class PhabricatorFile extends PhabricatorFileDAO
   }
 
   public function delete() {
-    $engine = $this->instantiateStorageEngine();
+    // Check to see if other files are using storage
+    $other_file = id(new PhabricatorFile())->loadAllWhere(
+      'storageEngine = %s AND storageHandle = %s AND
+      storageFormat = %s AND id != %d LIMIT 1', $this->getStorageEngine(),
+      $this->getStorageHandle(), $this->getStorageFormat(),
+      $this->getID());
+
+    // If this is the only file using the storage, delete storage
+    if (count($other_file) == 0) {
+      $engine = $this->instantiateStorageEngine();
+      $engine->deleteFile($this->getStorageHandle());
+    }
 
     $ret = parent::delete();
 
-    $engine->deleteFile($this->getStorageHandle());
-
     return $ret;
   }
 
+  public static function hashFileContent($data) {
+    return PhabricatorHash::digest($data);
+  }
+
   public function loadFileData() {
 
     $engine = $this->instantiateStorageEngine();

src/applications/files/storage/__tests__/PhabricatorFileTestCase.php

@@ -31,6 +31,55 @@ final class PhabricatorFileTestCase extends PhabricatorTestCase {
     $this->assertEqual($data, $file->loadFileData());
   }
 
+  public function testFileStorageUploadDifferentFiles() {
+    $engine = new PhabricatorTestStorageEngine();
+
+    $data = Filesystem::readRandomCharacters(64);
+    $other_data = Filesystem::readRandomCharacters(64);
+
+    $params = array(
+      'name' => 'test.dat',
+      'storageEngines' => array(
+        $engine,
+      ),
+    );
+
+    $first_file = PhabricatorFile::newFromFileData($data, $params);
+
+    $second_file = PhabricatorFile::newFromFileData($other_data, $params);
+
+    // Test that the the second file uses  different storage handle from
+    // the first file.
+    $first_handle = $first_file->getStorageHandle();
+    $second_handle = $second_file->getStorageHandle();
+
+    $this->assertEqual(true, ($first_handle != $second_handle));
+  }
+
+
+  public function testFileStorageUploadSameFile() {
+    $engine = new PhabricatorTestStorageEngine();
+
+    $data = Filesystem::readRandomCharacters(64);
+
+    $params = array(
+      'name' => 'test.dat',
+      'storageEngines' => array(
+        $engine,
+      ),
+    );
+
+    $first_file = PhabricatorFile::newFromFileData($data, $params);
+
+    $second_file = PhabricatorFile::newFromFileData($data, $params);
+
+    // Test that the the second file uses the same storage handle as
+    // the first file.
+    $handle = $first_file->getStorageHandle();
+    $second_handle = $second_file->getStorageHandle();
+
+    $this->assertEqual($handle, $second_handle);
+  }
 
   public function testFileStorageDelete() {
     $engine = new PhabricatorTestStorageEngine();
@@ -58,4 +107,23 @@ final class PhabricatorFileTestCase extends PhabricatorTestCase {
     $this->assertEqual(true, $caught instanceof Exception);
   }
 
+  public function testFileStorageDeleteSharedHandle() {
+    $engine = new PhabricatorTestStorageEngine();
+
+    $data = Filesystem::readRandomCharacters(64);
+
+    $params = array(
+      'name' => 'test.dat',
+      'storageEngines' => array(
+        $engine,
+      ),
+    );
+
+    $first_file = PhabricatorFile::newFromFileData($data, $params);
+    $second_file = PhabricatorFile::newFromFileData($data, $params);
+    $first_file->delete();
+
+    $this->assertEqual($data, $second_file->loadFileData());
+  }
+
 }