Rename bin/files metadata to bin/files rebuild and let it rebuild MIME information

Summary:
See <https://github.com/facebook/phabricator/issues/320>. Files can end up with a bad MIME type, and we don't update it when uploading another copy of the file since obviously the new copy has the same data and thus the same MIME type.

  - Rename `bin/files metadata` to `bin/files rebuild` to make it a more consistent verb.
  - Let it rebuild MIME types so users who hit issues like this can run `bin/files rebuild --all --rebuild-mime` to straighten things out.

Test Plan: Ran `bin/files` in various modes, examined output.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

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

scripts/files/manage_files.php

@@ -18,7 +18,7 @@ $workflows = array(
   new PhabricatorFilesManagementEnginesWorkflow(),
   new PhabricatorFilesManagementMigrateWorkflow(),
   new PhutilHelpArgumentWorkflow(),
-  new PhabricatorFilesManagementMetadataWorkflow(),
+  new PhabricatorFilesManagementRebuildWorkflow(),
 );
 
 $args->parseWorkflows($workflows);

src/__phutil_library_map__.php

@@ -998,8 +998,8 @@ phutil_register_library_map(array(
     'PhabricatorFileUploadException' => 'applications/files/exception/PhabricatorFileUploadException.php',
     'PhabricatorFilesConfigOptions' => 'applications/files/config/PhabricatorFilesConfigOptions.php',
     'PhabricatorFilesManagementEnginesWorkflow' => 'applications/files/management/PhabricatorFilesManagementEnginesWorkflow.php',
-    'PhabricatorFilesManagementMetadataWorkflow' => 'applications/files/management/PhabricatorFilesManagementMetadataWorkflow.php',
     'PhabricatorFilesManagementMigrateWorkflow' => 'applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php',
+    'PhabricatorFilesManagementRebuildWorkflow' => 'applications/files/management/PhabricatorFilesManagementRebuildWorkflow.php',
     'PhabricatorFilesManagementWorkflow' => 'applications/files/management/PhabricatorFilesManagementWorkflow.php',
     'PhabricatorFlag' => 'applications/flag/storage/PhabricatorFlag.php',
     'PhabricatorFlagColor' => 'applications/flag/constants/PhabricatorFlagColor.php',
@@ -2761,8 +2761,8 @@ phutil_register_library_map(array(
     'PhabricatorFileUploadException' => 'Exception',
     'PhabricatorFilesConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorFilesManagementEnginesWorkflow' => 'PhabricatorFilesManagementWorkflow',
-    'PhabricatorFilesManagementMetadataWorkflow' => 'PhabricatorFilesManagementWorkflow',
     'PhabricatorFilesManagementMigrateWorkflow' => 'PhabricatorFilesManagementWorkflow',
+    'PhabricatorFilesManagementRebuildWorkflow' => 'PhabricatorFilesManagementWorkflow',
     'PhabricatorFilesManagementWorkflow' => 'PhutilArgumentWorkflow',
     'PhabricatorFlag' => 'PhabricatorFlagDAO',
     'PhabricatorFlagColor' => 'PhabricatorFlagConstants',

src/applications/files/management/PhabricatorFilesManagementMetadataWorkflow.php

@@ -1,130 +0,0 @@
-<?php
-
-final class PhabricatorFilesManagementMetadataWorkflow
-  extends PhabricatorFilesManagementWorkflow {
-
-  public function didConstruct() {
-    $this
-      ->setName('metadata')
-      ->setSynopsis('Update metadata of old files.')
-      ->setArguments(
-        array(
-          array(
-            'name'      => 'all',
-            'help'      => 'Update all files.',
-          ),
-          array(
-            'name'      => 'names',
-            'wildcard'  => true,
-            'help'      => 'Update the given files.',
-          ),
-          array(
-            'name'      => 'dry-run',
-            'help'      => 'Show what would be updated.',
-          ),
-        ));
-  }
-
-  public function execute(PhutilArgumentParser $args) {
-    $console = PhutilConsole::getConsole();
-
-    if ($args->getArg('all')) {
-      if ($args->getArg('names')) {
-        throw new PhutilArgumentUsageException(
-          "Specify either a list of files or `--all`, but not both.");
-      }
-      $iterator = new LiskMigrationIterator(new PhabricatorFile());
-    } else if ($args->getArg('names')) {
-      $iterator = array();
-
-      foreach ($args->getArg('names') as $name) {
-        $name = trim($name);
-
-        $id = preg_replace('/^F/i', '', $name);
-        if (ctype_digit($id)) {
-          $file = id(new PhabricatorFile())->loadOneWhere(
-            'id = %d',
-            $id);
-          if (!$file) {
-            throw new PhutilArgumentUsageException(
-              "No file exists with id '{$name}'.");
-          }
-        } else {
-          $file = id(new PhabricatorFile())->loadOneWhere(
-            'phid = %d',
-            $name);
-          if (!$file) {
-            throw new PhutilArgumentUsageException(
-              "No file exists with PHID '{$name}'.");
-          }
-        }
-        $iterator[] = $file;
-      }
-    } else {
-      throw new PhutilArgumentUsageException(
-        "Either specify a list of files to update, or use `--all` ".
-        "to update all files.");
-    }
-
-    $is_dry_run = $args->getArg('dry-run');
-
-    $failed = array();
-
-    foreach ($iterator as $file) {
-      $fid = 'F'.$file->getID();
-
-      if (!$file->isViewableImage()) {
-        $console->writeOut(
-          "%s: Not an image file.\n",
-          $fid);
-        continue;
-      }
-
-      $metadata = $file->getMetadata();
-      $image_width = idx($metadata, PhabricatorFile::METADATA_IMAGE_WIDTH);
-      $image_height = idx($metadata, PhabricatorFile::METADATA_IMAGE_HEIGHT);
-      if ($image_width && $image_height) {
-        $console->writeOut(
-          "%s: Already updated\n",
-          $fid);
-        continue;
-      }
-
-      if ($is_dry_run) {
-        $console->writeOut(
-          "%s: Would update file (dry run)\n",
-          $fid);
-        continue;
-      }
-
-      $console->writeOut(
-        "%s: Updating metadata... ",
-        $fid);
-
-      try {
-        $file->updateDimensions();
-        $console->writeOut("done.\n");
-      } catch (Exception $ex) {
-        $console->writeOut("failed!\n");
-        $console->writeErr("%s\n", (string)$ex);
-        $failed[] = $file;
-      }
-    }
-
-    if ($failed) {
-      $console->writeOut("**Failures!**\n");
-      $ids = array();
-      foreach ($failed as $file) {
-        $ids[] = 'F'.$file->getID();
-      }
-      $console->writeOut("%s\n", implode(', ', $ids));
-
-      return 1;
-    } else {
-      $console->writeOut("**Success!**\n");
-      return 0;
-    }
-
-    return 0;
-  }
-}

src/applications/files/management/PhabricatorFilesManagementRebuildWorkflow.php

@@ -0,0 +1,174 @@
+<?php
+
+final class PhabricatorFilesManagementRebuildWorkflow
+  extends PhabricatorFilesManagementWorkflow {
+
+  public function didConstruct() {
+    $this
+      ->setName('rebuild')
+      ->setSynopsis('Rebuild metadata of old files.')
+      ->setArguments(
+        array(
+          array(
+            'name'      => 'all',
+            'help'      => 'Update all files.',
+          ),
+          array(
+            'name'      => 'id',
+            'wildcard'  => true,
+            'help'      => 'Update the given file. You can specify this flag '.
+                           'multiple times.',
+          ),
+          array(
+            'name'      => 'dry-run',
+            'help'      => 'Show what would be updated.',
+          ),
+          array(
+            'name'      => 'rebuild-mime',
+            'help'      => 'Rebuild MIME information.',
+          ),
+          array(
+            'name'      => 'rebuild-dimensions',
+            'help'      => 'Rebuild image dimension information.',
+          ),
+        ));
+  }
+
+  public function execute(PhutilArgumentParser $args) {
+    $console = PhutilConsole::getConsole();
+
+    if ($args->getArg('all')) {
+      if ($args->getArg('id')) {
+        throw new PhutilArgumentUsageException(
+          "Specify either a list of files or `--all`, but not both.");
+      }
+      $iterator = new LiskMigrationIterator(new PhabricatorFile());
+    } else if ($args->getArg('id')) {
+      $iterator = array();
+
+      foreach ($args->getArg('id') as $id) {
+        $id = trim($id);
+
+        $id = preg_replace('/^F/i', '', $id);
+        if (ctype_digit($id)) {
+          $file = id(new PhabricatorFile())->loadOneWhere(
+            'id = %d',
+            $id);
+          if (!$file) {
+            throw new PhutilArgumentUsageException(
+              "No file exists with ID '{$id}'.");
+          }
+        }
+        $iterator[] = $file;
+      }
+    } else {
+      throw new PhutilArgumentUsageException(
+        "Either specify a list of files to update, or use `--all` ".
+        "to update all files.");
+    }
+
+    $update = array(
+      'mime'          => $args->getArg('rebuild-mime'),
+      'dimensions'    => $args->getArg('rebuild-dimensions'),
+    );
+
+    // If the user didn't select anything, rebuild everything.
+    if (!array_filter($update)) {
+      foreach ($update as $key => $ignored) {
+        $update[$key] = true;
+      }
+    }
+
+    $is_dry_run = $args->getArg('dry-run');
+
+    $failed = array();
+
+    foreach ($iterator as $file) {
+      $fid = 'F'.$file->getID();
+
+      if ($update['mime']) {
+        $tmp = new TempFile();
+        Filesystem::writeFile($tmp, $file->loadFileData());
+        $new_type = Filesystem::getMimeType($tmp);
+
+        if ($new_type == $file->getMimeType()) {
+          $console->writeOut(
+            "%s: Mime type not changed (%s).\n",
+            $fid,
+            $new_type);
+        } else {
+          if ($is_dry_run) {
+            $console->writeOut(
+              "%s: Would update Mime type: '%s' -> '%s'.\n",
+              $fid,
+              $file->getMimeType(),
+              $new_type);
+          } else {
+            $console->writeOut(
+              "%s: Updating Mime type: '%s' -> '%s'.\n",
+              $fid,
+              $file->getMimeType(),
+              $new_type);
+            $file->setMimeType($new_type);
+            $file->save();
+          }
+        }
+      }
+
+      if ($update['dimensions']) {
+        if (!$file->isViewableImage()) {
+          $console->writeOut(
+            "%s: Not an image file.\n",
+            $fid);
+          continue;
+        }
+
+        $metadata = $file->getMetadata();
+        $image_width = idx($metadata, PhabricatorFile::METADATA_IMAGE_WIDTH);
+        $image_height = idx($metadata, PhabricatorFile::METADATA_IMAGE_HEIGHT);
+        if ($image_width && $image_height) {
+          $console->writeOut(
+            "%s: Image dimensions already exist.\n",
+            $fid);
+          continue;
+        }
+
+        if ($is_dry_run) {
+          $console->writeOut(
+            "%s: Would update file dimensions (dry run)\n",
+            $fid);
+          continue;
+        }
+
+        $console->writeOut(
+          "%s: Updating metadata... ",
+          $fid);
+
+        try {
+          $file->updateDimensions();
+          $console->writeOut("done.\n");
+        } catch (Exception $ex) {
+          $console->writeOut("failed!\n");
+          $console->writeErr("%s\n", (string)$ex);
+          $failed[] = $file;
+        }
+      }
+    }
+
+    if ($failed) {
+      $console->writeOut("**Failures!**\n");
+      $ids = array();
+      foreach ($failed as $file) {
+        $ids[] = 'F'.$file->getID();
+      }
+      $console->writeOut("%s\n", implode(', ', $ids));
+
+      return 1;
+    } else {
+      $console->writeOut("**Success!**\n");
+      return 0;
+    }
+
+    return 0;
+  }
+}