From 2a63483b4c5f234fa441cb5a5477aa59c9e196ee Mon Sep 17 00:00:00 2001
From: Rohit Yadav <rohit.yadav@shapeblue.com>
Date: Wed, 22 May 2024 20:20:15 +0530
Subject: [PATCH] framework/config: make logic in ::value() defensive (#9108)

This adds a NPE check on the s_depot.global() which can cause NPE in
case of unit tests, where s_depot is not null but the underlying config
dao is null (not mocked or initialised) via `s_depot.global()` becomes
null.

This reverts commit 5f73172bcbe975e4ef416e525dc95bad63fa6d3a.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
---
 .../org/apache/cloudstack/framework/config/ConfigKey.java    | 2 +-
 .../config/ConfigKeyScheduledExecutionWrapperTest.java       | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java
index df93f78fa83..46923de3c7c 100644
--- a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java
+++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java
@@ -211,7 +211,7 @@ public class ConfigKey<T> {
 
     public T value() {
         if (_value == null || isDynamic()) {
-            ConfigurationVO vo = s_depot != null ? s_depot.global().findById(key()) : null;
+            ConfigurationVO vo = (s_depot != null && s_depot.global() != null) ? s_depot.global().findById(key()) : null;
             final String value = (vo != null && vo.getValue() != null) ? vo.getValue() : defaultValue();
             _value = ((value == null) ? (T)defaultValue() : valueOf(value));
         }
diff --git a/framework/config/src/test/java/org/apache/cloudstack/framework/config/ConfigKeyScheduledExecutionWrapperTest.java b/framework/config/src/test/java/org/apache/cloudstack/framework/config/ConfigKeyScheduledExecutionWrapperTest.java
index 0eb2f6286ca..fbb4dc24fca 100644
--- a/framework/config/src/test/java/org/apache/cloudstack/framework/config/ConfigKeyScheduledExecutionWrapperTest.java
+++ b/framework/config/src/test/java/org/apache/cloudstack/framework/config/ConfigKeyScheduledExecutionWrapperTest.java
@@ -20,7 +20,6 @@ import com.cloud.utils.concurrency.NamedThreadFactory;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
-import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import java.util.concurrent.Executors;
@@ -59,8 +58,8 @@ public class ConfigKeyScheduledExecutionWrapperTest {
     @Test(expected = IllegalArgumentException.class)
     public void invalidConfigKeyTest() {
         TestRunnable runnable = new TestRunnable();
-        ConfigKey<String> configKey = Mockito.mock(ConfigKey.class);
-        when(configKey.value()).thenReturn("test");
+        ConfigKey<String> configKey = new ConfigKey<>(String.class, "test", "test", "test", "test", true,
+                ConfigKey.Scope.Global, null, null, null, null, null, ConfigKey.Kind.CSV, null);
         ConfigKeyScheduledExecutionWrapper runner = new ConfigKeyScheduledExecutionWrapper(executorService, runnable, configKey, TimeUnit.SECONDS);
     }