From 62a6e4b122d3a9e0e632dbd19c3e6f5e15a07373 Mon Sep 17 00:00:00 2001
From: mhlavac <info@mhlavac.net>
Date: Thu, 21 Nov 2024 13:30:22 +0100
Subject: [PATCH 1/4] feat: Global cache shared cache key

Change the cache key to repo.org if only global mergeable configuration
can be used.

Before this change, mergeable would already cache the org config for
each repo, but would always use the org/repo cache key. Unfortunately
if only global repository is used, this leads to a lot of cache misses
if PRs are opened across many repositories within the organization.
---
 .../unit/configuration/configuration.test.js  | 40 +++++++++++++++++++
 lib/configuration/configuration.js            |  6 ++-
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/__tests__/unit/configuration/configuration.test.js b/__tests__/unit/configuration/configuration.test.js
index aae2585e..7c77cb8f 100644
--- a/__tests__/unit/configuration/configuration.test.js
+++ b/__tests__/unit/configuration/configuration.test.js
@@ -763,6 +763,46 @@ describe('with version 1', () => {
     expect(config.mergeable.length).toEqual(1)
     expect(config.mergeable[0].name).toBe('repository rules')
   })
+
+  test('check config cache uses only owner as cache key if only global config can be fetched', async () => {
+    const orgConfigStr = `
+          mergeable:
+            issues:
+              stale:
+                days: 20
+                message: Issue test
+            pull_requests:
+              stale:
+                days: 20
+                message: PR test
+        `
+    // Check that on first load, the cache is correctly set
+    const emptyConfig = '{}'
+    const orgConfig = yaml.safeLoad(orgConfigStr)
+    const context = createMockGhConfig(emptyConfig, orgConfigStr)
+    context.globalSettings.use_config_cache = true
+    context.globalSettings.use_config_from_pull_request = false
+    context.globalSettings.use_org_as_default_config = true
+    const configCache = Configuration.getCache()
+    const repo = context.repo()
+    // configCache.set(repo.owner, orgConfig)
+    let keys = await configCache.keys()
+    expect(keys.length).toEqual(0)
+    context.eventName = 'push'
+    context.payload.head_commit = { added: ['.github/mergeable.yml'] }
+    expect(context.probotContext.config.mock.calls.length).toEqual(0)
+    let config = await Configuration.fetchConfigFile(context)
+    expect(config).toEqual(orgConfig)
+    keys = await configCache.keys()
+    expect(keys.length).toEqual(1)
+    const cachedConfig = await configCache.get(repo.owner)
+    expect(cachedConfig).toEqual(orgConfig)
+    // Check that cached value in the repo.owner is really used
+    const injectedConfig = { test: 'test' }
+    await configCache.set(repo.owner, injectedConfig)
+    config = await Configuration.fetchConfigFile(context)
+    expect(config).toEqual(injectedConfig)
+  })
 })
 
 // helper method to return mocked configiration.
diff --git a/lib/configuration/configuration.js b/lib/configuration/configuration.js
index 9b34bb1d..945b9034 100644
--- a/lib/configuration/configuration.js
+++ b/lib/configuration/configuration.js
@@ -120,8 +120,10 @@ class Configuration {
     // using pull request configuration by default, this behaviour
     // can be controlled using USE_CONFIG_FROM_PULL_REQUEST environment variable
     let usePullRequestConfig = true
+    let cacheKey = `${repo.owner}/${repo.repo}`
     if (globalSettings.use_config_from_pull_request !== undefined && globalSettings.use_config_from_pull_request === false) {
       usePullRequestConfig = false
+      cacheKey = `${repo.owner}`
     }
     if (usePullRequestConfig && ['pull_request', 'pull_request_review'].includes(context.eventName)) {
       const payload = context.payload.pull_request
@@ -157,7 +159,7 @@ class Configuration {
 
     if (globalSettings.use_config_cache !== undefined && globalSettings.use_config_cache === true) {
       Configuration.resetCache(context)
-      config = await cacheManager.get(`${repo.owner}/${repo.repo}`)
+      config = await cacheManager.get(cacheKey)
       if (config) {
         return config
       }
@@ -181,7 +183,7 @@ class Configuration {
     }
 
     if (globalSettings.use_config_cache !== undefined && globalSettings.use_config_cache === true) {
-      cacheManager.set(`${repo.owner}/${repo.repo}`, config)
+      cacheManager.set(cacheKey, config)
     }
 
     return config

From b270d3c2fa2990059cabc08c58df7e67af6cb4eb Mon Sep 17 00:00:00 2001
From: mhlavac <info@mhlavac.net>
Date: Thu, 21 Nov 2024 13:50:46 +0100
Subject: [PATCH 2/4] Fix the unit test to clean cache after itself

---
 __tests__/unit/configuration/configuration.test.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/__tests__/unit/configuration/configuration.test.js b/__tests__/unit/configuration/configuration.test.js
index 7c77cb8f..fa67bff0 100644
--- a/__tests__/unit/configuration/configuration.test.js
+++ b/__tests__/unit/configuration/configuration.test.js
@@ -762,6 +762,10 @@ describe('with version 1', () => {
     config = await Configuration.fetchConfigFile(context)
     expect(config.mergeable.length).toEqual(1)
     expect(config.mergeable[0].name).toBe('repository rules')
+
+    // Clean-up the cache state after test
+    const configCache = Configuration.getCache()
+    await configCache.reset()
   })
 
   test('check config cache uses only owner as cache key if only global config can be fetched', async () => {
@@ -785,7 +789,6 @@ describe('with version 1', () => {
     context.globalSettings.use_org_as_default_config = true
     const configCache = Configuration.getCache()
     const repo = context.repo()
-    // configCache.set(repo.owner, orgConfig)
     let keys = await configCache.keys()
     expect(keys.length).toEqual(0)
     context.eventName = 'push'
@@ -802,6 +805,9 @@ describe('with version 1', () => {
     await configCache.set(repo.owner, injectedConfig)
     config = await Configuration.fetchConfigFile(context)
     expect(config).toEqual(injectedConfig)
+
+    // Clean-up the cache state after test
+    await configCache.reset()
   })
 })
 

From a6f6be674db224876cc027fe330a8adb5b6e52dd Mon Sep 17 00:00:00 2001
From: mhlavac <info@mhlavac.net>
Date: Thu, 21 Nov 2024 14:05:48 +0100
Subject: [PATCH 3/4] fixed cache reset

---
 lib/configuration/configuration.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/configuration/configuration.js b/lib/configuration/configuration.js
index 945b9034..d26a15d2 100644
--- a/lib/configuration/configuration.js
+++ b/lib/configuration/configuration.js
@@ -107,6 +107,12 @@ class Configuration {
     const repo = context.repo()
 
     if (repo.repo === '.github') {
+      // Only org cache needs to be deleted if use_config_from_pull_request is disabled
+      if (globalSettings.use_config_from_pull_request !== undefined && globalSettings.use_config_from_pull_request === false) {
+        cacheManager.del(repo.owner)
+        return
+      }
+
       const keys = await cacheManager.keys()
       keys.filter(key => key.startsWith(`${repo.owner}/`)).map(key => cacheManager.del(key))
     } else {

From 19d1a59ff1b2371b0b0751614ec3974ae85c0af8 Mon Sep 17 00:00:00 2001
From: mhlavac <info@mhlavac.net>
Date: Thu, 21 Nov 2024 14:16:12 +0100
Subject: [PATCH 4/4] One more test case for cache reset

---
 .../unit/configuration/configuration.test.js  | 33 ++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/__tests__/unit/configuration/configuration.test.js b/__tests__/unit/configuration/configuration.test.js
index fa67bff0..5f1b6237 100644
--- a/__tests__/unit/configuration/configuration.test.js
+++ b/__tests__/unit/configuration/configuration.test.js
@@ -768,7 +768,7 @@ describe('with version 1', () => {
     await configCache.reset()
   })
 
-  test('check config cache uses only owner as cache key if only global config can be fetched', async () => {
+  test('config cache uses only owner as cache key if only global config can be fetched', async () => {
     const orgConfigStr = `
           mergeable:
             issues:
@@ -809,6 +809,37 @@ describe('with version 1', () => {
     // Clean-up the cache state after test
     await configCache.reset()
   })
+
+  test('cache is reset for org correctly when .github repo mergeable file is modified', async () => {
+    const orgConfigStr = `
+          mergeable:
+            pull_requests:
+              stale:
+                days: 20
+        `
+    const orgConfig = yaml.safeLoad(orgConfigStr)
+    const context = createMockGhConfig('{}', orgConfigStr)
+    context.globalSettings.use_config_cache = true
+    context.globalSettings.use_config_from_pull_request = false
+    context.globalSettings.use_org_as_default_config = true
+    const repo = context.repo()
+    const configCache = Configuration.getCache()
+    // Inject config that will be reset and overriden with the actual orgConfig
+    const injectedConfig = { test: 'test' }
+    configCache.set(repo.owner, injectedConfig)
+    let keys = await configCache.keys()
+    expect(keys.length).toEqual(1)
+    context.eventName = 'push'
+    context.repo = jest.fn().mockReturnValue({ owner: repo.owner, repo: '.github' })
+    context.payload.head_commit = { added: ['.github/mergeable.yml'] }
+    const config = await Configuration.fetchConfigFile(context)
+    expect(config).toEqual(orgConfig)
+    keys = await configCache.keys()
+    expect(keys.length).toEqual(1)
+
+    // Clean-up the cache state after test
+    await configCache.reset()
+  })
 })
 
 // helper method to return mocked configiration.