From 1aa5f7e63f162eee58ac4121c8e59a0f61c943a6 Mon Sep 17 00:00:00 2001
From: Mikel Alejo <mbarcina@redhat.com>
Date: Wed, 11 Oct 2023 09:23:51 +0200
Subject: [PATCH] feature: cache the group principals results (#2222)

The group principals results need to be cached as well to avoid calling
RBAC too many times.

RHCLOUD-28138
---
 .../connector/email/EmailRouteBuilder.java    | 50 +++++++++++++++----
 .../email/EmailRouteBuilderTest.java          | 11 ++++
 2 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilder.java b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilder.java
index d6ce0f6498..26909fcfa8 100644
--- a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilder.java
+++ b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilder.java
@@ -303,16 +303,34 @@ public void configureRoute() throws Exception {
          */
         from(direct(Routes.FETCH_GROUP_USERS))
             .routeId(Routes.FETCH_GROUP_USERS)
-            // Clear all the headers that may come from the previous route.
-            .removeHeaders("*")
-            // Initialize the offset and the loop condition so that we at least
-            // enter once on it.
-            .setProperty(ExchangeProperty.LIMIT, constant(this.emailConnectorConfig.getRbacElementsPerPage()))
-            .setProperty(ExchangeProperty.OFFSET, constant(0))
-            .loopDoWhile(this.notFinishedFetchingAllPages)
-                .process(this.rbacGroupPrincipalsRequestPreparerProcessor)
-                .to(this.emailConnectorConfig.getRbacURL())
-                .process(this.rbacUsersProcessor)
+            .setHeader(CaffeineConstants.ACTION, constant(CaffeineConstants.ACTION_GET))
+            .setHeader(CaffeineConstants.KEY, this.computeGroupPrincipalsCacheKey())
+            .to(caffeineCache(Routes.FETCH_GROUP_USERS))
+            // Avoid calling RBAC if we do have the usernames cached.
+            .choice()
+                .when(header(CaffeineConstants.ACTION_HAS_RESULT).isEqualTo(Boolean.TRUE))
+                    // The cache engine leaves the usernames in the body of the
+                    // exchange, that is why we need to set them back in the
+                    // property that the subsequent processors expect to find them.
+                    .setProperty(ExchangeProperty.USERNAMES, body())
+                .otherwise()
+                    // Clear all the headers that may come from the previous route.
+                    .removeHeaders("*")
+                    // Initialize the offset and the loop condition so that we at least
+                    // enter once on it.
+                    .setProperty(ExchangeProperty.LIMIT, constant(this.emailConnectorConfig.getRbacElementsPerPage()))
+                    .setProperty(ExchangeProperty.OFFSET, constant(0))
+                    .loopDoWhile(this.notFinishedFetchingAllPages)
+                        .process(this.rbacGroupPrincipalsRequestPreparerProcessor)
+                        .to(this.emailConnectorConfig.getRbacURL())
+                        .process(this.rbacUsersProcessor)
+                    .end()
+                        // Store all the received recipients in the cache.
+                        .setHeader(CaffeineConstants.ACTION, constant(CaffeineConstants.ACTION_PUT))
+                        .setHeader(CaffeineConstants.KEY, this.computeGroupPrincipalsCacheKey())
+                        .setHeader(CaffeineConstants.VALUE, exchangeProperty(ExchangeProperty.USERNAMES))
+                        .to(caffeineCache(Routes.FETCH_GROUP_USERS))
+            .endChoice()
             .end()
             .process(this.recipientsFilter);
 
@@ -367,6 +385,18 @@ protected Expression computeCacheKey() {
         );
     }
 
+    /**
+     * Computes the cache key for the "fetch group principals" route.
+     * @return a simple expression with the "${orgId}-${groupId}" format.
+     */
+    protected Expression computeGroupPrincipalsCacheKey() {
+        return simpleF(
+            "${exchangeProperty.%s}-${exchangeProperty.%s}",
+            ORG_ID,
+            ExchangeProperty.GROUP_UUID
+        );
+    }
+
     /**
      * Sets a common configuration for all the Caffeine caches.
      */
diff --git a/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilderTest.java b/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilderTest.java
index c1300bdfc9..a215e9265b 100644
--- a/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilderTest.java
+++ b/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilderTest.java
@@ -144,6 +144,17 @@ void testComputeCacheKey() {
         Assertions.assertEquals("simple{${exchangeProperty.orgId}-adminsOnly=${exchangeProperty.current_recipient_settings.adminsOnly}}", cacheKey.toString(), "unexpected cache key generated");
     }
 
+    /**
+     * Tests that the correct cache format is computed for the Caffeine cache
+     * keys.
+     */
+    @Test
+    void testComputeGroupsPrincipalCacheKey() {
+        final Expression cacheKey = this.emailRouteBuilder.computeGroupPrincipalsCacheKey();
+
+        Assertions.assertEquals("simple{${exchangeProperty.orgId}-${exchangeProperty.group_uuid}}", cacheKey.toString(), "unexpected cache key generated");
+    }
+
     /**
      * Tests that the Caffeine cache component has the configurations that we
      * expect.