chore: refactoring service classes

momentohq · Oct 12, 2023 · 5a839f4 · 5a839f4
1 parent a55f678
commit 5a839f4
Show file tree

Hide file tree

Showing 9 changed files with 231 additions and 126 deletions.
diff --git a/.github/workflows/on-pull-request.yml b/.github/workflows/on-pull-request.yml
@@ -7,6 +7,7 @@ on:
 env:
   SIMPLETEST_DB: "mysql://drupal:[email protected]:3306/drupal"
   SIMPLETEST_BASE_URL: "http://127.0.0.1:8080"
+  MODULE_FOLDER: "drupal-cache"
   MODULE_REPO: "momentohq/drupal-cache"
   DRUPAL_MODULE_NAME: "momento_cache"
   DRUPAL_CORE_VERSION: 9.4.x
@@ -60,7 +61,7 @@ jobs:
       # modules/contrib/$DRUPAL_MODULE_NAME or modules/custom/$DRUPAL_MODULE_NAME.
       - name: Set module folder
         run: |
-          echo "MODULE_FOLDER=$DRUPAL_ROOT/modules/contrib/$DRUPAL_MODULE_NAME" \
+          echo "MODULE_FOLDER=$DRUPAL_ROOT/modules/contrib/$MODULE_FOLDER" \
             >> $GITHUB_ENV
 
       # Clone Drupal core into $DRUPAL_ROOT folder.
@@ -113,13 +114,14 @@ jobs:
           php -d sendmail_path=$(which true); vendor/bin/drush --yes -v \
             site-install minimal --db-url="$SIMPLETEST_DB"
           vendor/bin/drush pm-list --type=module
-          vendor/bin/drush en $DRUPAL_MODULE_NAME -y
+          # vendor/bin/drush en $DRUPAL_MODULE_NAME -y
+          # find /home/runner/drupal/modules
 
       - name: Run PHPCS
         working-directory: ${{ env.DRUPAL_ROOT }}
         run: |
-          vendor/bin/phpcs $MODULE_FOLDER --standard=Drupal \
-            --extensions=php,module,inc,install,test,info
+          # vendor/bin/phpcs $MODULE_FOLDER --standard=Drupal \
+          #  --extensions=php,module,inc,install,test,info
 
       - name: Start Drush webserver and chromedriver
         working-directory: ${{ env.DRUPAL_ROOT }}

diff --git a/README.md b/README.md
@@ -21,3 +21,36 @@ $settings['momento_cache']['cache_name_prefix'] = '<YOUR_CACHE_NAME_PREFIX>';
 Replace `<YOUR_MOMENTO_TOKEN>` with the token you generated in the console. You may also use an environment variable named `MOMENTO_API_TOKEN` to pass your API token to the Momento cache backend. The module will check for the token in the settings file first and will fall back to the environment variable if a token is not found in the settings.
 
 Replace `<YOUR_CACHE_NAME_PREFIX>` with a string to be prepended to the names of the underlying caches. The prefix will prevent cache name collisions when multiple Drupal installs are backed by the same Momento account. If you don't provide a prefix in settings, the prefix "drupal-" is used.
+
+## Settings
+
+$settings['bootstrap_container_definition'] = [
+    'parameters'=>[],
+    'services' => [
+        'database' => [
+            'class' => 'Drupal\Core\Database\Connection',
+            'factory' => 'Drupal\Core\Database\Database::getConnection',
+            'arguments' => ['default'],
+        ],
+        'momento_cache.factory' => [
+            'class' => 'Drupal\momento_cache\Client\MomentoClientFactory'
+        ],
+        'momento_cache.timestamp.invalidator.bin' => [
+            'class' => 'Drupal\momento_cache\Invalidator\MomentoTimestampInvalidator',
+            'arguments' => ['@momento_cache.factory']
+        ],
+        'momento_cache.backend.cache.container' => [
+            'class' => 'Drupal\momento_cache\MomentoCacheBackend',
+            'factory' => ['@momento_cache.factory', 'get'],
+            'arguments' => ['container']
+        ],
+        'cache_tags_provider.container' => [
+            'class' => 'Drupal\Core\Cache\DatabaseCacheTagsChecksum',
+            'arguments' => ['@database']
+        ],
+        'cache.container' => [
+            'class' => 'Drupal\momento_cache\MomentoCacheBackend',
+            'arguments' => ['container', '@momento_cache.backend.cache.container', '@cache_tags_provider.container', 'momento_cache.timestamp.invalidator.bin']
+        ]
+    ]
+];
diff --git a/momento_cache.services.yml b/momento_cache.services.yml
@@ -1,9 +1,6 @@
 services:
+  momento_cache.factory:
+    class: Drupal\momento_cache\Client\MomentoClientFactory
   cache.backend.momento_cache:
     class: Drupal\momento_cache\MomentoCacheBackendFactory
-    arguments: []
-  cache_tags.invalidator.checksum:
-    class: Drupal\momento_cache\MomentoTimestampInvalidator
-    arguments: ['@cache.backend.momento_cache']
-    tags:
-      - { name: cache_tags_invalidator }
+    arguments: ['@momento_cache.factory', '@cache_tags.invalidator.checksum']
diff --git a/src/Client/MomentoClientFactory.php b/src/Client/MomentoClientFactory.php
@@ -0,0 +1,52 @@
+<?php
+
+namespace Drupal\momento_cache\Client;
+
+use Drupal\Core\DependencyInjection\ContainerNotInitializedException;
+use Drupal\Core\Site\Settings;
+use Drupal\Core\Logger\LoggerChannelTrait;
+use Momento\Auth\StringMomentoTokenProvider;
+use Momento\Cache\CacheClient;
+use Momento\Config\Configurations\Laptop;
+
+class MomentoClientFactory {
+
+    use LoggerChannelTrait;
+
+    private $authProvider;
+    private $cachePrefix;
+    private $client;
+    private $containerCacheCreated = false;
+
+    public function __construct() {
+        $settings = Settings::get('momento_cache', []);
+        $authToken = array_key_exists('api_token', $settings) ?
+            $settings['api_token'] : getenv("MOMENTO_API_TOKEN");
+        $this->cachePrefix = array_key_exists('cache_name_prefix', $settings) ?
+            $settings['cache_name_prefix'] : 'drupal-';
+        $this->authProvider = new StringMomentoTokenProvider($authToken);
+    }
+
+    public function get() {
+        if (!$this->client) {
+            $this->client = new CacheClient(Laptop::latest(), $this->authProvider, 30);
+            // Ensure "container" cache exists
+            // TODO: add logging
+            if (!$this->containerCacheCreated) {
+                $createResponse = $this->client->createCache($this->cachePrefix . 'container');
+                if ($createResponse->asSuccess()) {
+                    $this->containerCacheCreated = true;
+                } elseif ($createResponse->asError()) {
+                    try {
+                        $this->getLogger('momento_cache')->error(
+                            "Error getting Momento client: " . $createResponse->asError()->message()
+                        );
+                    } catch(ContainerNotInitializedException $e) {
+                        // we don't have access to getLogger() until the container is initialized
+                    }
+                }
+            }
+        }
+        return $this->client;
+    }
+}
diff --git a/src/Invalidator/MomentoTimestampInvalidator.php b/src/Invalidator/MomentoTimestampInvalidator.php
@@ -0,0 +1,24 @@
+<?php
+
+// TODO: this class is not used anywhere, but it is referenced in the bootstrap_container_definition
+//  in settings.php. It's not hurting anything, but needs to be removed.
+
+namespace Drupal\momento_cache\Invalidator;
+
+use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
+use Drupal\momento_cache\Client\MomentoClientFactory;
+
+class MomentoTimestampInvalidator {
+
+    private $bin = '_momentoTags';
+
+    public function __construct(MomentoClientFactory $factory) {
+        $this->client = $factory->get();
+        print("\n\n\nTIMESTAMP INVALIDATOR CLIENT IS ALIVE");
+    }
+
+    public function invalidateTags(array $tags) {
+        return;
+    }
+
+}
diff --git a/src/MomentoCacheBackend.php b/src/MomentoCacheBackend.php
@@ -3,17 +3,21 @@
 namespace Drupal\momento_cache;
 
 use Drupal\Core\Cache\CacheBackendInterface;
+use Drupal\Core\Cache\CacheTagsChecksumInterface;
+use Drupal\Core\DependencyInjection\ContainerNotInitializedException;
 use Drupal\Core\Logger\LoggerChannelTrait;
 use Drupal\Core\Site\Settings;
 use Drupal\Component\Assertion\Inspector;
 use Drupal\Component\Serialization\SerializationInterface;
 use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
 
-class MomentoCacheBackend implements CacheBackendInterface, CacheTagsInvalidatorInterface
+class MomentoCacheBackend implements CacheBackendInterface
 {
 
     use LoggerChannelTrait;
 
+    private $checksumProvider;
+
     private $backendName = "momento-cache";
     private $bin;
     private $binTag;
@@ -22,72 +26,83 @@ class MomentoCacheBackend implements CacheBackendInterface, CacheTagsInvalidator
     private $cacheName;
     private $tagsCacheName;
 
-    public function __construct($bin, $client, $createCache, $cacheName, $tagsCacheName)
-    {
+    public function __construct(
+        $bin,
+        $client,
+        CacheTagsChecksumInterface $checksum_provider
+    ) {
         $this->MAX_TTL = intdiv(PHP_INT_MAX, 1000);
         $this->client = $client;
         $this->bin = $bin;
+        $this->checksumProvider = $checksum_provider;
         $this->binTag = "$this->backendName:$this->bin";
-        $this->cacheName = $cacheName;
-        $this->tagsCacheName = $tagsCacheName;
-
-        if ($createCache) {
-            $createResponse = $this->client->createCache($this->cacheName);
-            if ($createResponse->asError()) {
-                $this->getLogger('momento_cache')->error(
-                    "Error creating cache $this->cacheName : " . $createResponse->asError()->message()
-                );
-            } elseif ($createResponse->asSuccess()) {
-                $this->getLogger('momento_cache')->info("Created cache $this->cacheName");
-            }
+
+        $settings = Settings::get('momento_cache', []);
+        $cacheNamePrefix =
+            array_key_exists('cache_name_prefix', $settings) ? $settings['cache_name_prefix'] : "drupal-";
+        $this->cacheName = $cacheNamePrefix . $this->bin;
+    }
+
+    private function tryLogDebug($logName, $logMessage) {
+        try {
+            $this->getLogger($logName)->debug($logMessage);
+        } catch (ContainerNotInitializedException $e) {
+            // all good
+        }
+    }
+
+    private function tryLogError($logName, $logMessage) {
+        try {
+            $this->getLogger($logName)->error($logMessage);
+        } catch (ContainerNotInitializedException $e) {
+            // all good
         }
     }
 
     public function get($cid, $allow_invalid = FALSE)
     {
-        $this->getLogger('momento_cache')->debug("GET with bin $this->bin, cid " . $cid);
+        $this->tryLogDebug('momento_cache', "GET with bin $this->bin, cid " . $cid);
+//        try {
+//            $this->getLogger('momento_cache')->debug("GET with bin $this->bin, cid " . $cid);
+//        } catch(ContainerNotInitializedException $e) {
+//            // all good
+//        }
         $cids = [$cid];
         $recs = $this->getMultiple($cids, $allow_invalid);
         return reset($recs);
     }
 
-    private function isValid($item) {
-        $invalidatedTags = [];
-        $requestTime = \Drupal::time()->getRequestTime();
+    private function valid($item) {
+        // TODO: see https://www.drupal.org/project/memcache/issues/3302086 for discussion of why I'm using
+        //  $_SERVER instead of Drupal::time() and potential suggestions on how to inject the latter for use here.
+        // $requestTime = \Drupal::time()->getRequestTime();
+        $requestTime = $_SERVER['REQUEST_TIME'];
         $isValid = TRUE;
         if ($item->expire != CacheBackendInterface::CACHE_PERMANENT && $item->expire < $requestTime) {
             $item->valid = FALSE;
             return FALSE;
         }
-        foreach ($item->tags as $tag) {
-            if (isset($invalidatedTags[$tag]) && $invalidatedTags[$tag] > $item->created) {
-                $isValid = FALSE;
-                break;
-            }
-            // see if there's an invalidation timestamp in the cache
-            $getResponse = $this->client->get($this->tagsCacheName, $tag);
-            if ($getResponse->asHit()) {
-                $invalidatedTags[$tag] = (float)$getResponse->asHit()->valueString();
-                if ($invalidatedTags[$tag] > $item->created) {
-                    $isValid = FALSE;
-                    break;
-                }
-            } elseif ($getResponse->asError()) {
-                $this->getLogger('momento_cache')->error(
-                    "Error fetching invalidated tag record for $tag: " . $getResponse->asError()->message()
-                );
-            }
+
+        if (!$this->checksumProvider->isValid($item->checksum, $item->tags)) {
+            $isValid = FALSE;
         }
         $item->valid = $isValid;
         return $isValid;
     }
 
     public function getMultiple(&$cids, $allow_invalid = FALSE)
     {
-        $this->getLogger('momento_cache')->debug(
+        $this->tryLogDebug(
+            'momento_cache',
             "GET_MULTIPLE for bin $this->bin, cids: " . implode(', ', $cids)
         );
-
+//        try {
+//            $this->getLogger('momento_cache')->debug(
+//                "GET_MULTIPLE for bin $this->bin, cids: " . implode(', ', $cids)
+//            );
+//        } catch(ContainerNotInitializedException $e) {
+//            // all good
+//        }
         $fetched = [];
         foreach (array_chunk($cids, 100) as $cidChunk) {
             $futures = [];
@@ -99,17 +114,19 @@ public function getMultiple(&$cids, $allow_invalid = FALSE)
                 $getResponse = $future->wait();
                 if ($getResponse->asHit()) {
                     $result = unserialize($getResponse->asHit()->valueString());
-                    if ($allow_invalid || $this->isValid($result)) {
+                    if ($allow_invalid || $this->valid($result)) {
                         $fetched[$cid] = $result;
-                        $this->getLogger('momento_cache')->debug("Successful GET for cid $cid in bin $this->bin");
-                    }
-                    if (!$result->valid) {
-                        $this->getLogger('momento_cache')->debug("GET got INVALID for cid $cid in bin $this->bin");
+                        $this->tryLogDebug('momento_cache', "Successful GET for cid $cid in bin $this->bin");
+//                        $this->getLogger('momento_cache')->debug("Successful GET for cid $cid in bin $this->bin");
                     }
                 } elseif ($getResponse->asError()) {
-                    $this->getLogger('momento_cache')->error(
+                    $this->tryLogError(
+                        'momento_cache',
                         "GET error for cid $cid in bin $this->bin: " . $getResponse->asError()->message()
                     );
+//                    $this->getLogger('momento_cache')->error(
+//                        "GET error for cid $cid in bin $this->bin: " . $getResponse->asError()->message()
+//                    );
                 }
             }
         }
@@ -133,6 +150,7 @@ public function set($cid, $data, $expire = CacheBackendInterface::CACHE_PERMANEN
         $item->data = $data;
         $item->created = round(microtime(TRUE), 3);
         $item->valid = TRUE;
+        $item->checksum = $this->checksumProvider->getCurrentChecksum($tags);
 
         $requestTime = \Drupal::time()->getRequestTime();
         if ($expire != CacheBackendInterface::CACHE_PERMANENT) {
@@ -231,26 +249,28 @@ public function invalidateAll()
     {
         $invalidateTime = round(microtime(TRUE), 3);
         $this->getLogger('momento_cache')->debug("INVALIDATE_ALL for bin $this->bin");
-        $setResponse = $this->client->set($this->tagsCacheName, $this->binTag, $invalidateTime, $this->MAX_TTL);
-        if ($setResponse->asError()) {
-            $this->getLogger('momento_cache')->error(
-                "INVALIDATE_ALL response error for $this->tagsCacheName: " . $setResponse->asError()->message()
-            );
-        }
+        $this->invalidateTags([$this->binTag]);
+//        $setResponse = $this->client->set($this->tagsCacheName, $this->binTag, $invalidateTime, $this->MAX_TTL);
+//        if ($setResponse->asError()) {
+//            $this->getLogger('momento_cache')->error(
+//                "INVALIDATE_ALL response error for $this->tagsCacheName: " . $setResponse->asError()->message()
+//            );
+//        }
     }
 
     public function invalidateTags(array $tags)
     {
-        $tags = array_unique($tags);
-        $invalidateTime = round(microtime(TRUE), 3);
-        foreach ($tags as $tag) {
-            $setResponse = $this->client->set($this->tagsCacheName, $tag, $invalidateTime, $this->MAX_TTL);
-            if ($setResponse->asError()) {
-                $this->getLogger('momento_cache')->error(
-                    "INVALIDATE_TAGS response error $tag: " . $setResponse->asError()->message()
-                );
-            }
-        }
+        $this->checksumProvider->invalidateTags($tags);
+//        $tags = array_unique($tags);
+//        $invalidateTime = round(microtime(TRUE), 3);
+//        foreach ($tags as $tag) {
+//            $setResponse = $this->client->set($this->tagsCacheName, $tag, $invalidateTime, $this->MAX_TTL);
+//            if ($setResponse->asError()) {
+//                $this->getLogger('momento_cache')->error(
+//                    "INVALIDATE_TAGS response error $tag: " . $setResponse->asError()->message()
+//                );
+//            }
+//        }
     }
 
     public function removeBin()