dev/cloud-native#17 Store extdir cache in sqlcache rather than the filesystem

diff --git a/CRM/Extension/Browser.php b/CRM/Extension/Browser.php
index 4fe82f18e518efa33afd0b3ddb68c0277f9e6a93..af1a0e84de649d6fa15cf1145f58d9fedafc5f34 100644 (file)
--- a/CRM/Extension/Browser.php
+++ b/CRM/Extension/Browser.php
@@ -35,11 +35,8 @@ class CRM_Extension_Browser {
   const SINGLE_FILE_PATH = '/single';
 
   /**
-   * The name of the single JSON extension cache file.
+   * Timeout for when the connection or the server is slow
    */
-  const CACHE_JSON_FILE = 'extensions.json';
-
-  // timeout for when the connection or the server is slow
   const CHECK_TIMEOUT = 5;
 
   /**
@@ -66,16 +63,10 @@ class CRM_Extension_Browser {
    *   URL of the remote repository.
    * @param string $indexPath
    *   Relative path of the 'index' file within the repository.
-   * @param string $cacheDir
-   *   Local path in which to cache files.
    */
-  public function __construct($repoUrl, $indexPath, $cacheDir) {
+  public function __construct($repoUrl, $indexPath) {
     $this->repoUrl = $repoUrl;
-    $this->cacheDir = $cacheDir;
     $this->indexPath = empty($indexPath) ? self::SINGLE_FILE_PATH : $indexPath;
-    if ($cacheDir && !file_exists($cacheDir) && is_dir(dirname($cacheDir)) && is_writable(dirname($cacheDir))) {
-      CRM_Utils_File::createDir($cacheDir, FALSE);
-    }
   }
 
   /**
@@ -101,10 +92,7 @@ class CRM_Extension_Browser {
    * Refresh the cache of remotely-available extensions.
    */
   public function refresh() {
-    $file = $this->getTsPath();
-    if (file_exists($file)) {
-      unlink($file);
-    }
+    \Civi::cache('extension_browser')->flush();
   }
 
   /**
@@ -118,22 +106,10 @@ class CRM_Extension_Browser {
       return [];
     }
 
+    // We used to check for the cache filesystem permissions, but it is now stored in DB
+    // If no new requirements have come up, consider removing this function after CiviCRM 5.60.
+    // The tests may need to be updated as well (tests/phpunit/CRM/Extension/BrowserTest.php).
     $errors = [];
-
-    if (!$this->cacheDir || !is_dir($this->cacheDir) || !is_writable($this->cacheDir)) {
-      $civicrmDestination = urlencode(CRM_Utils_System::url('civicrm/admin/extensions', 'reset=1'));
-      $url = CRM_Utils_System::url('civicrm/admin/setting/path', "reset=1&civicrmDestination=${civicrmDestination}");
-      $errors[] = array(
-        'title' => ts('Directory Unwritable'),
-        'message' => ts('Your extensions cache directory (%1) is not web server writable. Please go to the <a href="%2">path setting page</a> and correct it.<br/>',
-          array(
-            1 => $this->cacheDir,
-            2 => $url,
-          )
-        ),
-      );
-    }
-
     return $errors;
   }
 
@@ -149,8 +125,8 @@ class CRM_Extension_Browser {
     }
 
     $exts = [];
-
     $remote = $this->_discoverRemote();
+
     if (is_array($remote)) {
       foreach ($remote as $dc => $e) {
         $exts[$e->key] = $e;
@@ -184,33 +160,14 @@ class CRM_Extension_Browser {
    * @throws CRM_Extension_Exception_ParseException
    */
   private function _discoverRemote() {
-    $tsPath = $this->getTsPath();
-    $timestamp = FALSE;
-
-    if (file_exists($tsPath)) {
-      $timestamp = file_get_contents($tsPath);
-    }
-
-    // 3 minutes ago for now
-    $outdated = (int) $timestamp < (time() - 180) ? TRUE : FALSE;
-
-    if (!$timestamp || $outdated) {
-      $remotes = json_decode($this->grabRemoteJson(), TRUE);
-    }
-    else {
-      $remotes = json_decode($this->grabCachedJson(), TRUE);
-    }
-
+    $remotes = json_decode($this->grabCachedJson(), TRUE);
     $this->_remotesDiscovered = [];
+
     foreach ((array) $remotes as $id => $xml) {
       $ext = CRM_Extension_Info::loadFromString($xml);
       $this->_remotesDiscovered[] = $ext;
     }
 
-    if (file_exists(dirname($tsPath))) {
-      file_put_contents($tsPath, (string) time());
-    }
-
     return $this->_remotesDiscovered;
   }
 
@@ -221,12 +178,9 @@ class CRM_Extension_Browser {
    * @return string
    */
   private function grabCachedJson() {
-    $filename = $this->cacheDir . DIRECTORY_SEPARATOR . self::CACHE_JSON_FILE . '.' . md5($this->getRepositoryUrl());
-    $json = NULL;
-    if (file_exists($filename)) {
-      $json = file_get_contents($filename);
-    }
-    if (empty($json)) {
+    $cacheKey = $this->getCacheKey();
+    $json = \Civi::cache('extension_browser')->get($cacheKey);
+    if ($json === NULL) {
       $json = $this->grabRemoteJson();
     }
     return $json;
@@ -248,13 +202,10 @@ class CRM_Extension_Browser {
       return '';
     }
 
-    $filename = $this->cacheDir . DIRECTORY_SEPARATOR . self::CACHE_JSON_FILE . '.' . md5($this->getRepositoryUrl());
     $url = $this->getRepositoryUrl() . $this->indexPath;
-
     $client = $this->getGuzzleClient();
     try {
       $response = $client->request('GET', $url, [
-        'sink' => $filename,
         'timeout' => \Civi::settings()->get('http_timeout'),
       ]);
     }
@@ -267,15 +218,18 @@ class CRM_Extension_Browser {
       throw new CRM_Extension_Exception(ts('The CiviCRM public extensions directory at %1 could not be contacted - please check your webserver can make external HTTP requests', [1 => $this->getRepositoryUrl()]), 'connection_error');
     }
 
-    // Don't call grabCachedJson here, that would risk infinite recursion
-    return file_get_contents($filename);
+    $json = $response->getBody()->getContents();
+    $cacheKey = $this->getCacheKey();
+    \Civi::cache('extension_browser')->set($cacheKey, $json);
+    return $json;
   }
 
   /**
-   * @return string
+   * Returns a cache key based on the repository URL, which can be updated
+   * by admins in civicrm.settings.php or passed as a command-line option to cv.
    */
-  private function getTsPath() {
-    return $this->cacheDir . DIRECTORY_SEPARATOR . 'timestamp.txt';
+  private function getCacheKey() {
+    return 'extdir_' . md5($this->getRepositoryUrl());
   }
 
   /**

diff --git a/CRM/Extension/System.php b/CRM/Extension/System.php
index e27d93fe1dc5b80762e3b0af5c1450d406b53243..99a920de37e25a3165be28b0781c9c0214af433e 100644 (file)
--- a/CRM/Extension/System.php
+++ b/CRM/Extension/System.php
@@ -217,11 +217,7 @@ class CRM_Extension_System {
    */
   public function getBrowser() {
     if ($this->browser === NULL) {
-      $cacheDir = NULL;
-      if (!empty($this->parameters['uploadDir'])) {
-        $cacheDir = CRM_Utils_File::addTrailingSlash($this->parameters['uploadDir']) . 'cache';
-      }
-      $this->browser = new CRM_Extension_Browser($this->getRepositoryUrl(), '', $cacheDir);
+      $this->browser = new CRM_Extension_Browser($this->getRepositoryUrl(), '');
     }
     return $this->browser;
   }

diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php
index 6da903c0d55170a3e88b299392eee44ff5ad1bbb..dc48b4926357e5160c0f8c42e8e17ac1f9830492 100644 (file)
--- a/Civi/Core/Container.php
+++ b/Civi/Core/Container.php
@@ -200,6 +200,18 @@ class Container {
       ]
     ))->setFactory('CRM_Utils_Cache::create')->setPublic(TRUE);
 
+    // Memcache is limited to 1 MB by default, and since this is not read often
+    // it does not make much sense in Redis either.
+    $container->setDefinition('cache.extension_browser', new Definition(
+      'CRM_Utils_Cache_Interface',
+      [
+        [
+          'name' => 'extension_browser',
+          'type' => ['SqlGroup', 'ArrayCache'],
+        ],
+      ]
+    ))->setFactory('CRM_Utils_Cache::create')->setPublic(TRUE);
+
     $container->setDefinition('sql_triggers', new Definition(
       'Civi\Core\SqlTriggers',
       []

diff --git a/tests/phpunit/CRM/Extension/BrowserTest.php b/tests/phpunit/CRM/Extension/BrowserTest.php
index 6869b8c289fc460ae2452f22855c53b63331e0bd..05199961960a3159e3aa8afc232072cdabb4c409 100644 (file)
--- a/tests/phpunit/CRM/Extension/BrowserTest.php
+++ b/tests/phpunit/CRM/Extension/BrowserTest.php
@@ -36,28 +36,14 @@ class CRM_Extension_BrowserTest extends CiviUnitTestCase {
   }
 
   public function testDisabled() {
-    $this->browser = new CRM_Extension_Browser(FALSE, '/index.html', 'file:///itd/oesn/tmat/ter');
+    $this->browser = new CRM_Extension_Browser(FALSE, '/index.html');
     $this->assertEquals(FALSE, $this->browser->isEnabled());
     $this->assertEquals([], $this->browser->checkRequirements());
     $this->assertEquals([], $this->browser->getExtensions());
   }
 
-  public function testCheckRequirements_BadCachedir_false() {
-    $this->browser = new CRM_Extension_Browser('file://' . dirname(__FILE__) . '/dataset/good-repository', NULL, FALSE);
-    $this->assertEquals(TRUE, $this->browser->isEnabled());
-    $reqs = $this->browser->checkRequirements();
-    $this->assertEquals(1, count($reqs));
-  }
-
-  public function testCheckRequirements_BadCachedir_nonexistent() {
-    $this->browser = new CRM_Extension_Browser('file://' . dirname(__FILE__) . '/dataset/good-repository', NULL, '/tot/all/yin/v/alid');
-    $this->assertEquals(TRUE, $this->browser->isEnabled());
-    $reqs = $this->browser->checkRequirements();
-    $this->assertEquals(1, count($reqs));
-  }
-
   public function testGetExtensions_good() {
-    $this->browser = new CRM_Extension_Browser('file://' . dirname(__FILE__) . '/dataset/good-repository', NULL, $this->createTempDir('ext-cache-'));
+    $this->browser = new CRM_Extension_Browser('file://' . dirname(__FILE__) . '/dataset/good-repository', NULL);
     $this->assertEquals(TRUE, $this->browser->isEnabled());
     $this->assertEquals([], $this->browser->checkRequirements());
     $this->setupMockHandler();
@@ -72,7 +58,7 @@ class CRM_Extension_BrowserTest extends CiviUnitTestCase {
   }
 
   public function testGetExtension_good() {
-    $this->browser = new CRM_Extension_Browser('file://' . dirname(__FILE__) . '/dataset/good-repository', NULL, $this->createTempDir('ext-cache-'));
+    $this->browser = new CRM_Extension_Browser('file://' . dirname(__FILE__) . '/dataset/good-repository', NULL);
     $this->assertEquals(TRUE, $this->browser->isEnabled());
     $this->assertEquals([], $this->browser->checkRequirements());
     $this->setupMockHandler();
@@ -82,7 +68,7 @@ class CRM_Extension_BrowserTest extends CiviUnitTestCase {
   }
 
   public function testGetExtension_nonexistent() {
-    $this->browser = new CRM_Extension_Browser('file://' . dirname(__FILE__) . '/dataset/good-repository', NULL, $this->createTempDir('ext-cache-'));
+    $this->browser = new CRM_Extension_Browser('file://' . dirname(__FILE__) . '/dataset/good-repository', NULL);
     $this->assertEquals(TRUE, $this->browser->isEnabled());
     $this->assertEquals([], $this->browser->checkRequirements());
     $this->setupMockHandler();