Restructure cache checks to improve modularity

Summary:
Ref T5501. This code was headed down a bad road; dump an indirection layer between rendering and data gatehring.

In particular, this will make it much easier to lift these issues into setup warnings eventually.

Test Plan: Viewed cache status page.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5501

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

src/__phutil_library_map__.php

@@ -1474,6 +1474,7 @@ phutil_register_library_map(array(
     'PhabricatorCacheManagementWorkflow' => 'applications/cache/management/PhabricatorCacheManagementWorkflow.php',
     'PhabricatorCacheMarkupGarbageCollector' => 'applications/cache/garbagecollector/PhabricatorCacheMarkupGarbageCollector.php',
     'PhabricatorCacheSchemaSpec' => 'applications/cache/storage/PhabricatorCacheSchemaSpec.php',
+    'PhabricatorCacheSpec' => 'applications/cache/spec/PhabricatorCacheSpec.php',
     'PhabricatorCacheTTLGarbageCollector' => 'applications/cache/garbagecollector/PhabricatorCacheTTLGarbageCollector.php',
     'PhabricatorCaches' => 'applications/cache/PhabricatorCaches.php',
     'PhabricatorCalendarApplication' => 'applications/calendar/application/PhabricatorCalendarApplication.php',
@@ -1709,6 +1710,7 @@ phutil_register_library_map(array(
     'PhabricatorDashboardTransactionQuery' => 'applications/dashboard/query/PhabricatorDashboardTransactionQuery.php',
     'PhabricatorDashboardUninstallController' => 'applications/dashboard/controller/PhabricatorDashboardUninstallController.php',
     'PhabricatorDashboardViewController' => 'applications/dashboard/controller/PhabricatorDashboardViewController.php',
+    'PhabricatorDataCacheSpec' => 'applications/cache/spec/PhabricatorDataCacheSpec.php',
     'PhabricatorDataNotAttachedException' => 'infrastructure/storage/lisk/PhabricatorDataNotAttachedException.php',
     'PhabricatorDatabaseSetupCheck' => 'applications/config/check/PhabricatorDatabaseSetupCheck.php',
     'PhabricatorDebugController' => 'applications/system/controller/PhabricatorDebugController.php',
@@ -2126,6 +2128,7 @@ phutil_register_library_map(array(
     'PhabricatorObjectUsesCredentialsEdgeType' => 'applications/transactions/edges/PhabricatorObjectUsesCredentialsEdgeType.php',
     'PhabricatorOffsetPagedQuery' => 'infrastructure/query/PhabricatorOffsetPagedQuery.php',
     'PhabricatorOneTimeTriggerClock' => 'infrastructure/daemon/workers/clock/PhabricatorOneTimeTriggerClock.php',
+    'PhabricatorOpcodeCacheSpec' => 'applications/cache/spec/PhabricatorOpcodeCacheSpec.php',
     'PhabricatorOwnerPathQuery' => 'applications/owners/query/PhabricatorOwnerPathQuery.php',
     'PhabricatorOwnersApplication' => 'applications/owners/application/PhabricatorOwnersApplication.php',
     'PhabricatorOwnersConfigOptions' => 'applications/owners/config/PhabricatorOwnersConfigOptions.php',
@@ -4765,6 +4768,7 @@ phutil_register_library_map(array(
     'PhabricatorCacheManagementWorkflow' => 'PhabricatorManagementWorkflow',
     'PhabricatorCacheMarkupGarbageCollector' => 'PhabricatorGarbageCollector',
     'PhabricatorCacheSchemaSpec' => 'PhabricatorConfigSchemaSpec',
+    'PhabricatorCacheSpec' => 'Phobject',
     'PhabricatorCacheTTLGarbageCollector' => 'PhabricatorGarbageCollector',
     'PhabricatorCalendarApplication' => 'PhabricatorApplication',
     'PhabricatorCalendarBrowseController' => 'PhabricatorCalendarController',
@@ -5032,6 +5036,7 @@ phutil_register_library_map(array(
     'PhabricatorDashboardTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'PhabricatorDashboardUninstallController' => 'PhabricatorDashboardController',
     'PhabricatorDashboardViewController' => 'PhabricatorDashboardController',
+    'PhabricatorDataCacheSpec' => 'PhabricatorCacheSpec',
     'PhabricatorDataNotAttachedException' => 'Exception',
     'PhabricatorDatabaseSetupCheck' => 'PhabricatorSetupCheck',
     'PhabricatorDebugController' => 'PhabricatorController',
@@ -5464,6 +5469,7 @@ phutil_register_library_map(array(
     'PhabricatorObjectUsesCredentialsEdgeType' => 'PhabricatorEdgeType',
     'PhabricatorOffsetPagedQuery' => 'PhabricatorQuery',
     'PhabricatorOneTimeTriggerClock' => 'PhabricatorTriggerClock',
+    'PhabricatorOpcodeCacheSpec' => 'PhabricatorCacheSpec',
     'PhabricatorOwnersApplication' => 'PhabricatorApplication',
     'PhabricatorOwnersConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorOwnersController' => 'PhabricatorController',

src/applications/cache/spec/PhabricatorCacheSpec.php

@@ -0,0 +1,53 @@
+<?php
+
+abstract class PhabricatorCacheSpec extends Phobject {
+
+  private $name;
+  private $isEnabled = false;
+  private $version;
+  private $issues = array();
+
+  public function setName($name) {
+    $this->name = $name;
+    return $this;
+  }
+
+  public function getName() {
+    return $this->name;
+  }
+
+  public function setIsEnabled($is_enabled) {
+    $this->isEnabled = $is_enabled;
+    return $this;
+  }
+
+  public function getIsEnabled() {
+    return $this->isEnabled;
+  }
+
+  public function setVersion($version) {
+    $this->version = $version;
+    return $this;
+  }
+
+  public function getVersion() {
+    return $this->version;
+  }
+
+  protected function newIssue($title, $body, $option = null) {
+    $issue = array(
+      'title' => $title,
+      'body' => $body,
+      'option' => $option,
+    );
+
+    $this->issues[] = $issue;
+
+    return $issue;
+  }
+
+  public function getIssues() {
+    return $this->issues;
+  }
+
+}

src/applications/cache/spec/PhabricatorDataCacheSpec.php

@@ -0,0 +1,73 @@
+<?php
+
+final class PhabricatorDataCacheSpec extends PhabricatorCacheSpec {
+
+  public static function getActiveCacheSpec() {
+    $spec = new PhabricatorDataCacheSpec();
+    // NOTE: If APCu is installed, it reports that APC is installed.
+    if (extension_loaded('apc') && !extension_loaded('apcu')) {
+      return self::getAPCSpec($spec);
+    } else if (extension_loaded('apcu')) {
+      return self::getAPCuSpec($spec);
+    } else {
+      return self::getNoneSpec($spec);
+    }
+  }
+
+  private static function getAPCSpec(PhabricatorDataCacheSpec $spec) {
+    $spec
+      ->setName(pht('APC User Cache'))
+      ->setVersion(phpversion('apc'));
+
+    if (ini_get('apc.enabled')) {
+      $spec->setIsEnabled(true);
+    } else {
+      $spec->setIsEnabled(false);
+      $spec->newIssue(
+        pht('Enable APC'),
+        pht(
+          'The "APC" extension is currently disabled. Set "apc.enabled" to '.
+          'true to provide caching.'),
+        'apc.enabled');
+    }
+
+    return $spec;
+  }
+
+  private static function getAPCuSpec(PhabricatorDataCacheSpec $spec) {
+    $spec
+      ->setName(pht('APCu'))
+      ->setVersion(phpversion('apcu'));
+
+    if (ini_get('apc.enabled')) {
+      $spec->setIsEnabled(true);
+    } else {
+      $spec->setIsEnabled(false);
+      $spec->newissue(
+        pht('Enable APCu'),
+        pht(
+          'The "APCu" extension is currently disabled. Set '.
+          '"apc.enabled" to true to provide caching.'),
+        'apc.enabled');
+    }
+
+    return $spec;
+  }
+
+  private static function getNoneSpec(PhabricatorDataCacheSpec $spec) {
+    if (version_compare(phpversion(), '5.5', '>=')) {
+      $spec->newIssue(
+        pht('Install APCu'),
+        pht(
+          'Install the "APCu" PHP extension to provide data caching.'));
+    } else {
+      $spec->newIssue(
+        pht('Install APC'),
+        pht(
+          'Install the "APC" PHP extension to provide data caching.'));
+    }
+
+    return $spec;
+  }
+
+}

src/applications/cache/spec/PhabricatorOpcodeCacheSpec.php

@@ -0,0 +1,73 @@
+<?php
+
+final class PhabricatorOpcodeCacheSpec extends PhabricatorCacheSpec {
+
+  public static function getActiveCacheSpec() {
+    $spec = new PhabricatorOpcodeCacheSpec();
+    // NOTE: If APCu is installed, it reports that APC is installed.
+    if (extension_loaded('apc') && !extension_loaded('apcu')) {
+      return self::getAPCSpec($spec);
+    } else if (extension_loaded('Zend OPcache')) {
+      return self::getOpcacheSpec($spec);
+    } else {
+      return self::getNoneSpec($spec);
+    }
+  }
+
+  private static function getAPCSpec(PhabricatorOpcodeCacheSpec $spec) {
+    $spec
+      ->setName(pht('APC'))
+      ->setVersion(phpversion('apc'));
+
+    if (ini_get('apc.enabled')) {
+      $spec->setIsEnabled(true);
+    } else {
+      $spec->setIsEnabled(false);
+      $spec->newIssue(
+        pht('Enable APC'),
+        pht(
+          'The "APC" extension is currently disabled. Set "apc.enabled" to '.
+          'true to improve performance.'),
+        'apc.enabled');
+    }
+
+    return $spec;
+  }
+
+  private static function getOpcacheSpec(PhabricatorOpcodeCacheSpec $spec) {
+    $spec
+      ->setName(pht('Zend OPcache'))
+      ->setVersion(phpversion('Zend OPcache'));
+
+    if (ini_get('opcache.enable')) {
+      $spec->setIsEnabled(true);
+    } else {
+      $spec->setIsEnabled(false);
+      $spec->newissue(
+        pht('Enable Zend OPcache'),
+        pht(
+          'The "Zend OPcache" extension is currently disabled. Set '.
+          '"opcache.enable" to true to improve performance.'),
+        'opcache.enable');
+    }
+
+    return $spec;
+  }
+
+  private static function getNoneSpec(PhabricatorOpcodeCacheSpec $spec) {
+    if (version_compare(phpversion(), '5.5', '>=')) {
+      $spec->newIssue(
+        pht('Install OPcache'),
+        pht(
+          'Install the "Zend OPcache" PHP extension to improve performance.'));
+    } else {
+      $spec->newIssue(
+        pht('Install APC'),
+        pht(
+          'Install the "APC" PHP extension to improve performance.'));
+    }
+
+    return $spec;
+  }
+
+}

src/applications/config/controller/PhabricatorConfigCacheController.php

@@ -15,23 +15,15 @@ final class PhabricatorConfigCacheController
       ->buildApplicationCrumbs()
       ->addTextCrumb(pht('Cache Status'));
 
-    $nav->setCrumbs($crumbs);
+    $code_box = $this->renderCodeBox();
+    $data_box = $this->renderDataBox();
 
-    list($remedy, $properties) = $this->getProperties();
+    $nav->appendChild(
-
+      array(
-    $property_list = id(new PHUIPropertyListView());
+        $crumbs,
-    foreach ($properties as $property) {
+        $code_box,
-      list($name, $value) = $property;
+        $data_box,
-      $property_list->addProperty($name, $value);
+      ));
-    }
-
-
-    $box = id(new PHUIObjectBoxView())
-      ->setFormErrors($remedy)
-      ->setHeaderText(pht('Cache'))
-      ->addPropertyList($property_list);
-
-    $nav->appendChild($box);
 
     return $this->buildApplicationPage(
       $nav,
@@ -40,109 +32,67 @@ final class PhabricatorConfigCacheController
       ));
   }
 
-  private function getProperties() {
+  private function renderCodeBox() {
-    $remedy = array();
+    $cache = PhabricatorOpcodeCacheSpec::getActiveCacheSpec();
 
-    $properties = array();
+    $properties = id(new PHUIPropertyListView());
 
-    // NOTE: If APCu is installed, it reports that APC is installed.
+    $this->renderCommonProperties($properties, $cache);
-    if (extension_loaded('apc') && !extension_loaded('apcu')) {
+
-      $cache_installed = true;
+    return id(new PHUIObjectBoxView())
-      $cache_name = pht('APC');
+      ->setFormErrors($this->renderIssues($cache->getIssues()))
-      $cache_version = phpversion('apc');
+      ->setHeaderText(pht('Opcode Cache'))
-      $cache_enabled = (bool)ini_get('apc.enabled');
+      ->addPropertyList($properties);
-      if (!$cache_enabled) {
+  }
-        $remedy[] = pht('Enable APC');
+
-      }
+  private function renderDataBox() {
-      $datacache_installed = true;
+    $cache = PhabricatorDataCacheSpec::getActiveCacheSpec();
-      $datacache_name = pht('APC User Cache');
+
-      $datacache_version = phpversion('apc');
+    $properties = id(new PHUIPropertyListView());
-      $datacache_enabled = true;
+
+    $this->renderCommonProperties($properties, $cache);
+
+    return id(new PHUIObjectBoxView())
+      ->setHeaderText(pht('Data Cache'))
+      ->addPropertyList($properties);
+  }
+
+  private function renderCommonProperties(
+    PHUIPropertyListView $properties,
+    PhabricatorCacheSpec $cache) {
+
+    if ($cache->getName() !== null) {
+      $name = $this->renderYes($cache->getName());
     } else {
-      if (extension_loaded('Zend OPcache')) {
+      $name = $this->renderNo(pht('None'));
-        $cache_installed = true;
-        $cache_name = pht('Zend Opcache');
-        $cache_enabled = (bool)ini_get('opcache.enable');
-        $cache_version = phpversion('Zend OPcache');
-        if (!$cache_enabled) {
-          $remedy[] = pht('Enable Opcache.');
-        }
-      } else {
-        if (version_compare(phpversion(), '5.5', '>=')) {
-          $remedy[] = pht('Install OPcache.');
-        } else {
-          $remedy[] = pht('Install APC.');
-        }
-
-        $cache_installed = false;
-        $cache_name = pht('None');
-        $cache_enabled = false;
-        $cache_version = null;
-      }
-
-      if (extension_loaded('apcu')) {
-        $datacache_installed = true;
-        $datacache_name = pht('APCu');
-        $datacache_version = phpversion('apcu');
-        $datacache_enabled = (bool)ini_get('apc.enabled');
-      } else {
-        if (version_compare(phpversion(), '5.5', '>=')) {
-          $remedy[] = pht('Install APCu.');
-        } else {
-          // We already suggested installing APC above.
-        }
-
-        $datacache_installed = false;
-        $datacache_name = pht('None');
-        $datacache_version = null;
-        $datacache_enabled = false;
-      }
     }
+    $properties->addProperty(pht('Cache'), $name);
 
-    if ($cache_installed) {
+    if ($cache->getIsEnabled()) {
-      $cache_property = $this->renderYes($cache_name);
+      $enabled = $this->renderYes(pht('Enabled'));
     } else {
-      $cache_property = $this->renderNo($cache_name);
+      $enabled = $this->renderNo(pht('Not Enabled'));
     }
+    $properties->addProperty(pht('Enabled'), $enabled);
 
-    if ($cache_enabled) {
+    $version = $cache->getVersion();
-      $cache_enabled_property = $this->renderYes(pht('Enabled'));
+    if ($version) {
-    } else {
+      $properties->addProperty(pht('Version'), $this->renderInfo($version));
-      $cache_enabled_property = $this->renderNo(pht('Not Enabled'));
     }
+  }
 
-    $properties[] = array(pht('Opcode Cache'), $cache_property);
+  private function renderIssues(array $issues) {
-    $properties[] = array(pht('Enabled'), $cache_enabled_property);
+    $result = array();
-    if ($cache_version) {
+    foreach ($issues as $issue) {
-      $properties[] = array(
+      $title = $issue['title'];
-        pht('Version'),
+      $body = $issue['body'];
-        $this->renderInfo($cache_version),
+      $result[] = array(
+        phutil_tag('strong', array(), $title.':'),
+        ' ',
+        $body,
       );
     }
-
+    return $result;
-    if ($datacache_installed) {
-      $datacache_property = $this->renderYes($datacache_name);
-    } else {
-      $datacache_property = $this->renderNo($datacache_name);
-    }
-
-    if ($datacache_enabled) {
-      $datacache_enabled_property = $this->renderYes(pht('Enabled'));
-    } else {
-      $datacache_enabled_property = $this->renderNo(pht('Not Enabled'));
-    }
-
-    $properties[] = array(pht('Data Cache'), $datacache_property);
-    $properties[] = array(pht('Enabled'), $datacache_enabled_property);
-    if ($datacache_version) {
-      $properties[] = array(
-        pht('Version'),
-        $this->renderInfo($datacache_version),
-      );
-    }
-
-
-    return array($remedy, $properties);
   }
 
   private function renderYes($info) {