From 5736060af94f073a75956717245269a018d7d094 Mon Sep 17 00:00:00 2001 From: Mathieu Lutfy Date: Sat, 11 Jun 2022 10:19:11 -0400 Subject: [PATCH] dev/cloud-native#17 Store extdir cache in sqlcache rather than the filesystem --- CRM/Extension/Browser.php | 86 +++++---------------- CRM/Extension/System.php | 6 +- Civi/Core/Container.php | 12 +++ tests/phpunit/CRM/Extension/BrowserTest.php | 22 +----- 4 files changed, 37 insertions(+), 89 deletions(-) diff --git a/CRM/Extension/Browser.php b/CRM/Extension/Browser.php index 4fe82f18e5..af1a0e84de 100644 --- 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 path setting page and correct it.
', - 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 e27d93fe1d..99a920de37 100644 --- 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 6da903c0d5..dc48b49263 100644 --- 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 6869b8c289..0519996196 100644 --- 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(); -- 2.25.1