From 8df1161f14e41e9b0a0b71422de510b3510500e1 Mon Sep 17 00:00:00 2001
From: Abhishek Kumar <abhishek.mrt22@gmail.com>
Date: Fri, 21 Mar 2025 21:27:24 +0530
Subject: [PATCH] framework-config: improve configkey caching (#10513)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Using a simple hyphen as a delimiter for config cache key can lead to ambiguity if the “name” field itself contains hyphens. To address this, a Ternary object of configkey name, scope and scope ID is used as the config cache keys.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
---
 .../config/impl/ConfigDepotImpl.java          | 19 +++++++------------
 .../config/impl/ConfigDepotImplTest.java      |  6 ++++++
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java
index 911a4ad3707..8b8b6368527 100644
--- a/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java
+++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java
@@ -85,7 +85,7 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin {
     List<ScopedConfigStorage> _scopedStorages;
     Set<Configurable> _configured = Collections.synchronizedSet(new HashSet<Configurable>());
     Set<String> newConfigs = Collections.synchronizedSet(new HashSet<>());
-    LazyCache<String, String> configCache;
+    LazyCache<Ternary<String, ConfigKey.Scope, Long>, String> configCache;
 
     private HashMap<String, Pair<String, ConfigKey<?>>> _allKeys = new HashMap<String, Pair<String, ConfigKey<?>>>(1007);
 
@@ -273,15 +273,10 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin {
         return _configDao;
     }
 
-    protected String getConfigStringValueInternal(String cacheKey) {
-        String[] parts = cacheKey.split("-");
-        String key = parts[0];
-        ConfigKey.Scope scope = ConfigKey.Scope.Global;
-        Long scopeId = null;
-        try {
-            scope = ConfigKey.Scope.valueOf(parts[1]);
-            scopeId = Long.valueOf(parts[2]);
-        } catch (IllegalArgumentException ignored) {}
+    protected String getConfigStringValueInternal(Ternary<String, ConfigKey.Scope, Long> cacheKey) {
+        String key = cacheKey.first();
+        ConfigKey.Scope scope = cacheKey.second();
+        Long scopeId = cacheKey.third();
         if (!ConfigKey.Scope.Global.equals(scope) && scopeId != null) {
             ScopedConfigStorage scopedConfigStorage = null;
             for (ScopedConfigStorage storage : _scopedStorages) {
@@ -301,8 +296,8 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin {
         return null;
     }
 
-    private String getConfigCacheKey(String key, ConfigKey.Scope scope, Long scopeId) {
-        return String.format("%s-%s-%d", key, scope, (scopeId == null ? 0 : scopeId));
+    protected Ternary<String, ConfigKey.Scope, Long> getConfigCacheKey(String key, ConfigKey.Scope scope, Long scopeId) {
+        return new Ternary<>(key, scope, scopeId);
     }
 
     @Override
diff --git a/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java b/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java
index 8a7da795345..ca2f54f1442 100644
--- a/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java
+++ b/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java
@@ -81,6 +81,12 @@ public class ConfigDepotImplTest {
         runTestGetConfigStringValue("test", "value");
     }
 
+    @Test
+    public void testGetConfigStringValue_nameWithCharacters() {
+        runTestGetConfigStringValue("test.1-1", "value");
+        runTestGetConfigStringValue("test_1#2", "value");
+    }
+
     private void runTestGetConfigStringValueExpiry(long wait, int configDBRetrieval) {
         String key = "test1";
         String value = "expiry";