Fix for model not listing after import

Author: gotsysdba

src/server/api/utils/settings.py

@@ -164,17 +164,26 @@ def _load_prompt_configs(config_data: dict) -> None:
 
 
 def update_server(config_data: dict) -> None:
-    """Update server configuration"""
+    """Update server configuration.
+
+    Note: We mutate the existing lists (clear + extend) rather than replacing them.
+    This is critical because other modules import these lists directly
+    (e.g., `from server.bootstrap.bootstrap import DATABASE_OBJECTS`).
+    Replacing the list would leave those modules with stale references.
+    """
     config = Configuration(**config_data)
 
     if "database_configs" in config_data:
-        bootstrap.DATABASE_OBJECTS = config.database_configs or []
+        bootstrap.DATABASE_OBJECTS.clear()
+        bootstrap.DATABASE_OBJECTS.extend(config.database_configs or [])
 
     if "model_configs" in config_data:
-        bootstrap.MODEL_OBJECTS = config.model_configs or []
+        bootstrap.MODEL_OBJECTS.clear()
+        bootstrap.MODEL_OBJECTS.extend(config.model_configs or [])
 
     if "oci_configs" in config_data:
-        bootstrap.OCI_OBJECTS = config.oci_configs or []
+        bootstrap.OCI_OBJECTS.clear()
+        bootstrap.OCI_OBJECTS.extend(config.oci_configs or [])
 
     # Load MCP prompt text into cache from prompt_configs
     _load_prompt_configs(config_data)

tests/server/unit/api/utils/test_utils_settings.py

@@ -14,61 +14,105 @@
 from common.schema import Settings, Configuration, Database, Model, OracleCloudSettings
 
 
-class TestSettings:
-    """Test settings module functionality"""
-
-    def __init__(self):
-        """Setup test data for all tests"""
-        self.default_settings = Settings(client="default")
-        self.test_client_settings = Settings(client="test_client")
-        self.sample_config_data = {
-            "database_configs": [{"name": "test_db", "user": "user", "password": "pass", "dsn": "dsn"}],
-            "model_configs": [{"id": "test-model", "provider": "openai", "type": "ll"}],
-            "oci_configs": [{"auth_profile": "DEFAULT", "compartment_id": "ocid1.compartment.oc1..test"}],
-            "prompt_overrides": {"optimizer_basic-default": "You are helpful"},
-            "client_settings": {"client": "default", "max_tokens": 1000, "temperature": 0.7},
-        }
+#####################################################
+# Helper functions for test data
+#####################################################
+def make_default_settings():
+    """Create default settings for tests"""
+    return Settings(client="default")
+
+
+def make_test_client_settings():
+    """Create test client settings for tests"""
+    return Settings(client="test_client")
+
+
+def make_sample_config_data():
+    """Create sample configuration data for tests"""
+    return {
+        "database_configs": [{"name": "test_db", "user": "user", "password": "pass", "dsn": "dsn"}],
+        "model_configs": [{"id": "test-model", "provider": "openai", "type": "ll"}],
+        "oci_configs": [{"auth_profile": "DEFAULT", "compartment_id": "ocid1.compartment.oc1..test"}],
+        "prompt_overrides": {"optimizer_basic-default": "You are helpful"},
+        "client_settings": {"client": "default", "max_tokens": 1000, "temperature": 0.7},
+    }
+
+
+#####################################################
+# Client Settings Tests
+#####################################################
+class TestClientSettings:
+    """Test client settings CRUD operations"""
 
     @patch("server.api.utils.settings.bootstrap")
     def test_create_client_success(self, mock_bootstrap):
         """Test successful client settings creation"""
-        # Create a list that includes the default settings and will be appended to
-        settings_list = [self.default_settings]
+        default_cfg = make_default_settings()
+        settings_list = [default_cfg]
         mock_bootstrap.SETTINGS_OBJECTS = settings_list
 
         result = settings.create_client("new_client")
 
         assert result.client == "new_client"
-        assert result.ll_model.max_tokens == self.default_settings.ll_model.max_tokens  # pylint: disable=no-member
-        # Check that a new client was added to the list
+        # Verify ll_model settings are copied from default
+        result_ll_model = result.model_dump()["ll_model"]
+        default_ll_model = default_cfg.model_dump()["ll_model"]
+        assert result_ll_model["max_tokens"] == default_ll_model["max_tokens"]
         assert len(settings_list) == 2
         assert settings_list[-1].client == "new_client"
 
     @patch("server.api.utils.settings.bootstrap.SETTINGS_OBJECTS")
     def test_create_client_already_exists(self, mock_settings_objects):
         """Test creating client settings when client already exists"""
-        mock_settings_objects.__iter__ = MagicMock(return_value=iter([self.test_client_settings]))
+        test_cfg = make_test_client_settings()
+        mock_settings_objects.__iter__ = MagicMock(return_value=iter([test_cfg]))
 
         with pytest.raises(ValueError, match="client test_client already exists"):
             settings.create_client("test_client")
 
     @patch("server.api.utils.settings.bootstrap.SETTINGS_OBJECTS")
     def test_get_client_found(self, mock_settings_objects):
         """Test getting client settings when client exists"""
-        mock_settings_objects.__iter__ = MagicMock(return_value=iter([self.test_client_settings]))
+        test_cfg = make_test_client_settings()
+        mock_settings_objects.__iter__ = MagicMock(return_value=iter([test_cfg]))
 
         result = settings.get_client("test_client")
 
-        assert result == self.test_client_settings
+        assert result == test_cfg
 
     @patch("server.api.utils.settings.bootstrap.SETTINGS_OBJECTS")
     def test_get_client_not_found(self, mock_settings_objects):
         """Test getting client settings when client doesn't exist"""
-        mock_settings_objects.__iter__ = MagicMock(return_value=iter([self.default_settings]))
+        default_cfg = make_default_settings()
+        mock_settings_objects.__iter__ = MagicMock(return_value=iter([default_cfg]))
 
         with pytest.raises(ValueError, match="client nonexistent not found"):
             settings.get_client("nonexistent")
 
+    @patch("server.api.utils.settings.bootstrap.SETTINGS_OBJECTS")
+    @patch("server.api.utils.settings.get_client")
+    def test_update_client(self, mock_get_settings, mock_settings_objects):
+        """Test updating client settings"""
+        test_cfg = make_test_client_settings()
+        mock_get_settings.return_value = test_cfg
+        mock_settings_objects.remove = MagicMock()
+        mock_settings_objects.append = MagicMock()
+        mock_settings_objects.__iter__ = MagicMock(return_value=iter([test_cfg]))
+
+        new_settings = Settings(client="test_client", max_tokens=800, temperature=0.9)
+        result = settings.update_client(new_settings, "test_client")
+
+        assert result.client == "test_client"
+        mock_settings_objects.remove.assert_called_once_with(test_cfg)
+        mock_settings_objects.append.assert_called_once()
+
+
+#####################################################
+# Server Configuration Tests
+#####################################################
+class TestServerConfiguration:
+    """Test server configuration operations"""
+
     @pytest.mark.asyncio
     @patch("server.api.utils.settings.get_mcp_prompts_with_overrides")
     @patch("server.api.utils.settings.bootstrap.DATABASE_OBJECTS")
@@ -81,7 +125,7 @@ async def test_get_server(self, mock_oci, mock_models, mock_databases, mock_get_
         )
         mock_models.__iter__ = MagicMock(return_value=iter([Model(id="test", provider="openai", type="ll")]))
         mock_oci.__iter__ = MagicMock(return_value=iter([OracleCloudSettings(auth_profile="DEFAULT")]))
-        mock_get_prompts.return_value = []  # Return empty list of prompts
+        mock_get_prompts.return_value = []
 
         mock_mcp_engine = MagicMock()
         result = await settings.get_server(mock_mcp_engine)
@@ -91,54 +135,79 @@ async def test_get_server(self, mock_oci, mock_models, mock_databases, mock_get_
         assert "oci_configs" in result
         assert "prompt_configs" in result
 
-    @patch("server.api.utils.settings.bootstrap.SETTINGS_OBJECTS")
-    @patch("server.api.utils.settings.get_client")
-    def test_update_client(self, mock_get_settings, mock_settings_objects):
-        """Test updating client settings"""
-        mock_get_settings.return_value = self.test_client_settings
-        mock_settings_objects.remove = MagicMock()
-        mock_settings_objects.append = MagicMock()
-        mock_settings_objects.__iter__ = MagicMock(return_value=iter([self.test_client_settings]))
-
-        new_settings = Settings(client="test_client", max_tokens=800, temperature=0.9)
-        result = settings.update_client(new_settings, "test_client")
-
-        assert result.client == "test_client"
-        mock_settings_objects.remove.assert_called_once_with(self.test_client_settings)
-        mock_settings_objects.append.assert_called_once()
-
     @patch("server.api.utils.settings.bootstrap")
     def test_update_server(self, mock_bootstrap):
         """Test updating server configuration"""
-        # Use the valid sample config data that includes client_settings
-        settings.update_server(self.sample_config_data)
+        mock_bootstrap.DATABASE_OBJECTS = []
+        mock_bootstrap.MODEL_OBJECTS = []
+        mock_bootstrap.OCI_OBJECTS = []
+
+        settings.update_server(make_sample_config_data())
 
         assert hasattr(mock_bootstrap, "DATABASE_OBJECTS")
         assert hasattr(mock_bootstrap, "MODEL_OBJECTS")
 
+    @patch("server.api.utils.settings.bootstrap")
+    def test_update_server_mutates_lists_not_replaces(self, mock_bootstrap):
+        """Test that update_server mutates existing lists rather than replacing them.
+
+        This is critical because other modules import these lists directly
+        (e.g., `from server.bootstrap.bootstrap import DATABASE_OBJECTS`).
+        If we replace the list, those modules would hold stale references.
+        """
+        original_db_list = []
+        original_model_list = []
+        original_oci_list = []
+
+        mock_bootstrap.DATABASE_OBJECTS = original_db_list
+        mock_bootstrap.MODEL_OBJECTS = original_model_list
+        mock_bootstrap.OCI_OBJECTS = original_oci_list
+
+        settings.update_server(make_sample_config_data())
+
+        # Verify the lists are the SAME objects (mutated, not replaced)
+        assert mock_bootstrap.DATABASE_OBJECTS is original_db_list, "DATABASE_OBJECTS was replaced instead of mutated"
+        assert mock_bootstrap.MODEL_OBJECTS is original_model_list, "MODEL_OBJECTS was replaced instead of mutated"
+        assert mock_bootstrap.OCI_OBJECTS is original_oci_list, "OCI_OBJECTS was replaced instead of mutated"
+
+        # Verify the lists now contain the new data
+        assert len(original_db_list) == 1
+        assert original_db_list[0].name == "test_db"
+        assert len(original_model_list) == 1
+        assert original_model_list[0].id == "test-model"
+        assert len(original_oci_list) == 1
+        assert original_oci_list[0].auth_profile == "DEFAULT"
+
+
+#####################################################
+# Config Loading Tests
+#####################################################
+class TestConfigLoading:
+    """Test configuration loading operations"""
+
     @patch("server.api.utils.settings.update_server")
     @patch("server.api.utils.settings.update_client")
     def test_load_config_from_json_data_with_client(self, mock_update_client, mock_update_server):
         """Test loading config from JSON data with specific client"""
-        settings.load_config_from_json_data(self.sample_config_data, client="test_client")
+        config_data = make_sample_config_data()
+        settings.load_config_from_json_data(config_data, client="test_client")
 
-        mock_update_server.assert_called_once_with(self.sample_config_data)
+        mock_update_server.assert_called_once_with(config_data)
         mock_update_client.assert_called_once()
 
     @patch("server.api.utils.settings.update_server")
     @patch("server.api.utils.settings.update_client")
     def test_load_config_from_json_data_without_client(self, mock_update_client, mock_update_server):
         """Test loading config from JSON data without specific client"""
-        settings.load_config_from_json_data(self.sample_config_data)
+        config_data = make_sample_config_data()
+        settings.load_config_from_json_data(config_data)
 
-        mock_update_server.assert_called_once_with(self.sample_config_data)
-        # Should be called twice: once for "server" and once for "default"
+        mock_update_server.assert_called_once_with(config_data)
         assert mock_update_client.call_count == 2
 
     @patch("server.api.utils.settings.update_server")
     def test_load_config_from_json_data_missing_client_settings(self, _mock_update_server):
         """Test loading config from JSON data without client_settings"""
-        # Create config without client_settings
         invalid_config = {"database_configs": [], "model_configs": [], "oci_configs": [], "prompt_configs": []}
 
         with pytest.raises(KeyError, match="Missing client_settings in config file"):
@@ -153,7 +222,7 @@ def test_read_config_from_json_file_success(self, mock_json_load, mock_access, m
         """Test successful reading of config file"""
         mock_isfile.return_value = True
         mock_access.return_value = True
-        mock_json_load.return_value = self.sample_config_data
+        mock_json_load.return_value = make_sample_config_data()
 
         result = settings.read_config_from_json_file()
 
@@ -166,19 +235,22 @@ def test_read_config_from_json_file_not_exists(self, mock_isfile):
         """Test reading config file that doesn't exist"""
         mock_isfile.return_value = False
 
-        # This should still attempt to process, but will log a warning
-        # The actual behavior depends on the implementation
-
     @patch.dict(os.environ, {"CONFIG_FILE": "/path/to/config.txt"})
     def test_read_config_from_json_file_wrong_extension(self):
         """Test reading config file with wrong extension"""
-        # This should log a warning about the file extension
 
     def test_logger_exists(self):
         """Test that logger is properly configured"""
         assert hasattr(settings, "logger")
         assert settings.logger.name == "api.core.settings"
 
+
+#####################################################
+# Prompt Override Tests
+#####################################################
+class TestPromptOverrides:
+    """Test prompt override operations"""
+
     @patch("server.api.utils.settings.cache")
     def test_load_prompt_override_with_text(self, mock_cache):
         """Test loading prompt override when text is provided"""