Migrate local disk storage setup check

Summary:
Migrate to the new hotness. Also:

  - Remove a string test, which is now impossible since the config will repair itself and raise a type error.
  - Restore the header even in /config/ -- this check is kind of hacky and it feels a bit more natural now that it's above the menu.

Test Plan: Set my local disk path to something invalid, verified I got a setup error.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2228

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

src/__phutil_library_map__.php

@@ -1207,6 +1207,7 @@ phutil_register_library_map(array(
     'PhabricatorSetupCheckExtraConfig' => 'applications/config/check/PhabricatorSetupCheckExtraConfig.php',
     'PhabricatorSetupCheckInvalidConfig' => 'applications/config/check/PhabricatorSetupCheckInvalidConfig.php',
     'PhabricatorSetupCheckMail' => 'applications/config/check/PhabricatorSetupCheckMail.php',
+    'PhabricatorSetupCheckStorage' => 'applications/config/check/PhabricatorSetupCheckStorage.php',
     'PhabricatorSetupCheckTimezone' => 'applications/config/check/PhabricatorSetupCheckTimezone.php',
     'PhabricatorSetupIssue' => 'applications/config/issue/PhabricatorSetupIssue.php',
     'PhabricatorSetupIssueView' => 'applications/config/view/PhabricatorSetupIssueView.php',
@@ -2556,6 +2557,7 @@ phutil_register_library_map(array(
     'PhabricatorSetupCheckExtraConfig' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckInvalidConfig' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckMail' => 'PhabricatorSetupCheck',
+    'PhabricatorSetupCheckStorage' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckTimezone' => 'PhabricatorSetupCheck',
     'PhabricatorSetupIssueView' => 'AphrontView',
     'PhabricatorSlowvoteChoice' => 'PhabricatorSlowvoteDAO',

src/applications/config/check/PhabricatorSetupCheckStorage.php

@@ -0,0 +1,27 @@
+<?php
+
+final class PhabricatorSetupCheckStorage extends PhabricatorSetupCheck {
+
+  protected function executeChecks() {
+    $local_key = 'storage.local-disk.path';
+    $local_path = PhabricatorEnv::getEnvConfig($local_key);
+
+    if (!Filesystem::pathExists($local_path) ||
+        !is_readable($local_path) ||
+        !is_writable($local_path)) {
+
+      $message = pht(
+        'Configured location for storing uploaded files on disk ("%s") does '.
+        'not exist, or is not readable or writable. Verify the directory '.
+        'exists and is readable and writable by the webserver.',
+        $local_path);
+
+      $this
+        ->newIssue('config.storage.local-disk.path')
+        ->setShortName(pht('Local Disk Storage'))
+        ->setName(pht('Local Disk Storage Not Readable/Writable'))
+        ->setMessage($message)
+        ->addPhabricatorConfig('storage.local-disk.path');
+    }
+  }
+}

src/infrastructure/PhabricatorSetup.php

@@ -412,15 +412,6 @@ final class PhabricatorSetup {
         self::write(" okay  'facebook.application-id' is set.\n");
       }
 
-      if (!is_string($app_id)) {
-        self::writeFailure();
-        self::write(
-          "Setup failure! 'facebook.application-id' should be a string.");
-        return;
-      } else {
-        self::write(" okay  'facebook.application-id' is string.\n");
-      }
-
       if (!$app_secret) {
         self::writeFailure();
         self::write(
@@ -547,30 +538,6 @@ final class PhabricatorSetup {
       self::write(" okay  max_allowed_packet = {$max_allowed_packet}.\n");
     }
 
-    $local_key = 'storage.local-disk.path';
-    $local_path = PhabricatorEnv::getEnvConfig($local_key);
-    if ($local_path) {
-      if (!Filesystem::pathExists($local_path) ||
-          !is_readable($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 ".
-          "readable or writable.\n");
-        if ($open_basedir) {
-          self::write(
-            "You have an 'open_basedir' setting -- make sure Phabricator is ".
-            "allowed to open files in the local storage directory.\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");
-    }
-
     self::write("[OKAY] Database and storage configuration OKAY\n");
 
     self::writeHeader('SUCCESS!');

src/view/page/PhabricatorStandardPageView.php

@@ -252,28 +252,19 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
     // Render the "you have unresolved setup issues..." warning.
     $setup_warning = null;
     if ($user && $user->getIsAdmin()) {
-      $application = null;
+      $open = PhabricatorSetupCheck::getOpenSetupIssueCount();
-      $controller = $this->getController();
+      if ($open) {
-      if ($controller) {
+        $setup_warning = phutil_render_tag(
-        $application = $controller->getCurrentApplication();
+          'div',
-      }
+          array(
-
+            'class' => 'setup-warning-callout',
-      // Don't show the banner inside the config application itself.
+          ),
-      if (!($application instanceof PhabricatorApplicationConfig)) {
+          phutil_render_tag(
-        $open = PhabricatorSetupCheck::getOpenSetupIssueCount();
+            'a',
-        if ($open) {
-          $setup_warning = phutil_render_tag(
-            'div',
             array(
-              'class' => 'setup-warning-callout',
+              'href' => '/config/issue/',
             ),
-            phutil_render_tag(
+            pht('You have %d unresolved setup issue(s)...', $open)));
-              'a',
-              array(
-                'href' => '/config/issue/',
-              ),
-              pht('You have %d unresolved setup issue(s)...', $open)));
-        }
       }
     }