Fix MetadataProvider initialization in Hot Reload and remove duplicat…

…e Query Executor reloading (#2428)

## Why make this change?

Closes #2426
Closes #2427

## What is this change?

Properly create and initialize a new set of `SqlMetadataProviders` as
part of the hot-reload process of the `SqlMetadataProviderFactory`, by
utilizing the `initiaizeAsync()` function. This guarantees a correctly
configured set of meta data providers for the factory.

Remove the event handling to hot-reload the `QueryExecutors` classes
specifically, since this is already handled during the hot-reload
process of the `QueryManagerFactory`. That process creates new and up to
date `QueryExecutors` as part of a hot-reload.

With the correct initialization of the meta data providers, we can
remove the code that saves the `DefaultDataSourceName` to be passed
along the hot-reloaded configs. With all of the data structures we
initialized on new meta data providers, we do not need to save the data
source name and hot-reloading will be more resilient. This is because we
will build all of the data structures which depend on this
DefaultDataSourceName fresh from newly created or cleared objects, and
we do not have to worry about maintaining previous configuration's
DefaultDataSourceName, everything will simply use the new name generated
during a hot-reload.

## How was this tested?

### Testing Scenario
Scenario A: Handling a change in entities in the config 
Objective: Verify that hot-reload correctly refreshes the
MetadataProviders and QueryExecutors when we hot-reload the entities by
making a REST request.

Steps:

Start with MSSQL with a valid config file that contains some valid
entities.
Make a REST request that targets one of the entities and observe the
results to compare against later.
Change the entities section by removing an entity and any entities that
have a relationship with that entity.
Make a REST request for that entity and observe that the entity is not
found.
Add the removed entities back to the config file.
Make a REST request for that same entity and observe the results match
the original REST request.

## Sample Request(s)

For the "Book" entity with a rest path of "rest"

https://localhost:5001/rest/Book

src/Config/FileSystemRuntimeConfigLoader.cs

@@ -142,8 +142,7 @@ public bool TryLoadConfig(
         string path,
         [NotNullWhen(true)] out RuntimeConfig? config,
         bool replaceEnvVar = false,
-        ILogger? logger = null,
-        string defaultDataSourceName = "")
+        ILogger? logger = null)
     {
         if (_fileSystem.File.Exists(path))
         {
@@ -156,11 +155,6 @@ public bool TryLoadConfig(
                     logger?.LogInformation("Monitoring config: {ConfigFilePath} for hot-reloading.", ConfigFilePath);
                 }
 
-                if (!string.IsNullOrEmpty(defaultDataSourceName))
-                {
-                    RuntimeConfig.UpdateDefaultDataSourceName(defaultDataSourceName);
-                }
-
                 config = RuntimeConfig;
                 return true;
             }
@@ -190,9 +184,9 @@ public bool TryLoadConfig(
     /// <param name="replaceEnvVar">Whether to replace environment variable with its
     /// value or not while deserializing.</param>
     /// <returns>True if the config was loaded, otherwise false.</returns>
-    public override bool TryLoadKnownConfig([NotNullWhen(true)] out RuntimeConfig? config, bool replaceEnvVar = false, string defaultDataSourceName = "")
+    public override bool TryLoadKnownConfig([NotNullWhen(true)] out RuntimeConfig? config, bool replaceEnvVar = false)
     {
-        return TryLoadConfig(ConfigFilePath, out config, replaceEnvVar, defaultDataSourceName: defaultDataSourceName);
+        return TryLoadConfig(ConfigFilePath, out config, replaceEnvVar);
     }
 
     /// <summary>
@@ -202,7 +196,7 @@ public override bool TryLoadKnownConfig([NotNullWhen(true)] out RuntimeConfig? c
     public void HotReloadConfig(string defaultDataSourceName, ILogger? logger = null)
     {
         logger?.LogInformation(message: "Starting hot-reload process for config: {ConfigFilePath}", ConfigFilePath);
-        TryLoadConfig(ConfigFilePath, out _, replaceEnvVar: true, defaultDataSourceName: defaultDataSourceName);
+        TryLoadConfig(ConfigFilePath, out _, replaceEnvVar: true);
         SignalConfigChanged();
     }
 

src/Config/HotReloadEventHandler.cs

@@ -22,14 +22,13 @@ public HotReloadEventHandler()
     {
         _eventHandlers = new Dictionary<string, EventHandler<TEventArgs>?>
         {
+            // QueryManagerFactory will create a new set of QueryExecutors
+            // during Hot-Reload so it is not necessary to specifically reconfigure
+            // QueryExecutors as part of the Hot-Reload process.
             { QUERY_MANAGER_FACTORY_ON_CONFIG_CHANGED, null },
             { METADATA_PROVIDER_FACTORY_ON_CONFIG_CHANGED, null },
-            { QUERY_ENGINE_FACTORY_ON_CONFIG_CHANGED,null },
-            { MUTATION_ENGINE_FACTORY_ON_CONFIG_CHANGED,null },
-            { QUERY_EXECUTOR_ON_CONFIG_CHANGED, null },
-            { MSSQL_QUERY_EXECUTOR_ON_CONFIG_CHANGED, null },
-            { MYSQL_QUERY_EXECUTOR_ON_CONFIG_CHANGED, null },
-            { POSTGRESQL_QUERY_EXECUTOR_ON_CONFIG_CHANGED, null },
+            { QUERY_ENGINE_FACTORY_ON_CONFIG_CHANGED, null },
+            { MUTATION_ENGINE_FACTORY_ON_CONFIG_CHANGED, null },
             { DOCUMENTOR_ON_CONFIG_CHANGED, null }
         };
     }

src/Config/ObjectModel/RuntimeConfig.cs

@@ -178,7 +178,12 @@ public bool TryGetEntityNameFromPath(string entityPathName, [NotNullWhen(true)]
     /// <param name="Runtime">Runtime settings.</param>
     /// <param name="DataSourceFiles">List of datasource files for multiple db scenario. Null for single db scenario.</param>
     [JsonConstructor]
-    public RuntimeConfig(string? Schema, DataSource DataSource, RuntimeEntities Entities, RuntimeOptions? Runtime = null, DataSourceFiles? DataSourceFiles = null)
+    public RuntimeConfig(
+        string? Schema,
+        DataSource DataSource,
+        RuntimeEntities Entities,
+        RuntimeOptions? Runtime = null,
+        DataSourceFiles? DataSourceFiles = null)
     {
         this.Schema = Schema ?? DEFAULT_CONFIG_SCHEMA_LINK;
         this.DataSource = DataSource;
@@ -189,13 +194,13 @@ public RuntimeConfig(string? Schema, DataSource DataSource, RuntimeEntities Enti
         // we will set them up with default values
         _dataSourceNameToDataSource = new Dictionary<string, DataSource>
         {
-            { DefaultDataSourceName, this.DataSource }
+            { this.DefaultDataSourceName, this.DataSource }
         };
 
         _entityNameToDataSourceName = new Dictionary<string, string>();
         foreach (KeyValuePair<string, Entity> entity in Entities)
         {
-            _entityNameToDataSourceName.TryAdd(entity.Key, DefaultDataSourceName);
+            _entityNameToDataSourceName.TryAdd(entity.Key, this.DefaultDataSourceName);
         }
 
         // Process data source and entities information for each database in multiple database scenario.

src/Config/RuntimeConfigLoader.cs

@@ -84,10 +84,6 @@ protected void SignalConfigChanged(string message = "")
         OnConfigChangedEvent(new HotReloadEventArgs(METADATA_PROVIDER_FACTORY_ON_CONFIG_CHANGED, message));
         OnConfigChangedEvent(new HotReloadEventArgs(QUERY_ENGINE_FACTORY_ON_CONFIG_CHANGED, message));
         OnConfigChangedEvent(new HotReloadEventArgs(MUTATION_ENGINE_FACTORY_ON_CONFIG_CHANGED, message));
-        OnConfigChangedEvent(new HotReloadEventArgs(QUERY_EXECUTOR_ON_CONFIG_CHANGED, message));
-        OnConfigChangedEvent(new HotReloadEventArgs(MSSQL_QUERY_EXECUTOR_ON_CONFIG_CHANGED, message));
-        OnConfigChangedEvent(new HotReloadEventArgs(MYSQL_QUERY_EXECUTOR_ON_CONFIG_CHANGED, message));
-        OnConfigChangedEvent(new HotReloadEventArgs(POSTGRESQL_QUERY_EXECUTOR_ON_CONFIG_CHANGED, message));
         OnConfigChangedEvent(new HotReloadEventArgs(DOCUMENTOR_ON_CONFIG_CHANGED, message));
 
         // Signal that a change has occurred to all change token listeners.
@@ -100,9 +96,8 @@ protected void SignalConfigChanged(string message = "")
     /// <param name="config">The loaded <c>RuntimeConfig</c>, or null if none was loaded.</param>
     /// <param name="replaceEnvVar">Whether to replace environment variable with its
     /// value or not while deserializing.</param>
-    /// <param name="dataSourceName">The data source name to be used in the loaded config.</param>
     /// <returns>True if the config was loaded, otherwise false.</returns>
-    public abstract bool TryLoadKnownConfig([NotNullWhen(true)] out RuntimeConfig? config, bool replaceEnvVar = false, string dataSourceName = "");
+    public abstract bool TryLoadKnownConfig([NotNullWhen(true)] out RuntimeConfig? config, bool replaceEnvVar = false);
 
     /// <summary>
     /// Returns the link to the published draft schema.
@@ -120,16 +115,12 @@ protected void SignalConfigChanged(string message = "")
     /// <param name="connectionString">connectionString to add to config if specified</param>
     /// <param name="replaceEnvVar">Whether to replace environment variable with its
     /// value or not while deserializing. By default, no replacement happens.</param>
-    /// <param name="dataSourceName"> datasource name for which to add connection string</param>
-    /// <param name="datasourceNameToConnectionString"> dictionary of datasource name to connection string</param>
     /// <param name="replacementFailureMode">Determines failure mode for env variable replacement.</param>
     public static bool TryParseConfig(string json,
         [NotNullWhen(true)] out RuntimeConfig? config,
         ILogger? logger = null,
         string? connectionString = null,
         bool replaceEnvVar = false,
-        string dataSourceName = "",
-        Dictionary<string, string>? datasourceNameToConnectionString = null,
         EnvironmentVariableReplacementFailureMode replacementFailureMode = EnvironmentVariableReplacementFailureMode.Throw)
     {
         JsonSerializerOptions options = GetSerializationOptions(replaceEnvVar, replacementFailureMode);
@@ -146,25 +137,16 @@ public static bool TryParseConfig(string json,
             // retreive current connection string from config
             string updatedConnectionString = config.DataSource.ConnectionString;
 
-            // set dataSourceName to default if not provided
-            if (string.IsNullOrEmpty(dataSourceName))
-            {
-                dataSourceName = config.DefaultDataSourceName;
-            }
-
             if (!string.IsNullOrEmpty(connectionString))
             {
                 // update connection string if provided.
                 updatedConnectionString = connectionString;
             }
 
-            if (datasourceNameToConnectionString is null)
-            {
-                datasourceNameToConnectionString = new Dictionary<string, string>();
-            }
+            Dictionary<string, string> datasourceNameToConnectionString = new();
 
-            // add to dictionary if datasourceName is present (will either be the default or the one provided)
-            datasourceNameToConnectionString.TryAdd(dataSourceName, updatedConnectionString);
+            // add to dictionary if datasourceName is present
+            datasourceNameToConnectionString.TryAdd(config.DefaultDataSourceName, updatedConnectionString);
 
             // iterate over dictionary and update runtime config with connection strings.
             foreach ((string dataSourceKey, string connectionValue) in datasourceNameToConnectionString)
@@ -184,7 +166,7 @@ public static bool TryParseConfig(string json,
                 }
 
                 ds = ds with { ConnectionString = updatedConnection };
-                config.UpdateDataSourceNameToDataSource(dataSourceName, ds);
+                config.UpdateDataSourceNameToDataSource(config.DefaultDataSourceName, ds);
 
                 if (string.Equals(dataSourceKey, config.DefaultDataSourceName, StringComparison.OrdinalIgnoreCase))
                 {

src/Core/Resolvers/MsSqlQueryExecutor.cs

@@ -16,7 +16,6 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.Data.SqlClient;
 using Microsoft.Extensions.Logging;
-using static Azure.DataApiBuilder.Config.DabConfigEvents;
 
 namespace Azure.DataApiBuilder.Core.Resolvers
 {
@@ -77,7 +76,6 @@ public MsSqlQueryExecutor(
                   httpContextAccessor,
                   handler)
         {
-            handler?.Subscribe(MSSQL_QUERY_EXECUTOR_ON_CONFIG_CHANGED, MsSqlQueryExecutorOnConfigChanged);
             _dataSourceAccessTokenUsage = new Dictionary<string, bool>();
             _dataSourceToSessionContextUsage = new Dictionary<string, bool>();
             _accessTokensFromConfiguration = runtimeConfigProvider.ManagedIdentityAccessToken;
@@ -109,19 +107,6 @@ private void ConfigureMsSqlQueryEecutor()
             }
         }
 
-        /// <summary>
-        /// Function registered for callback during a hot-reload scenario.
-        /// </summary>
-        /// <param name="sender">The calling object.</param>
-        /// <param name="args">Event arguments.</param>
-        public void MsSqlQueryExecutorOnConfigChanged(object? sender, HotReloadEventArgs args)
-        {
-            _dataSourceAccessTokenUsage = new Dictionary<string, bool>();
-            _dataSourceToSessionContextUsage = new Dictionary<string, bool>();
-            _accessTokensFromConfiguration = _runtimeConfigProvider.ManagedIdentityAccessToken;
-            ConfigureMsSqlQueryEecutor();
-        }
-
         /// <summary>
         /// Modifies the properties of the supplied connection to support managed identity access.
         /// In the case of MsSql, gets access token if deemed necessary and sets it on the connection.

src/Core/Resolvers/MySqlQueryExecutor.cs

@@ -11,7 +11,6 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Logging;
 using MySqlConnector;
-using static Azure.DataApiBuilder.Config.DabConfigEvents;
 
 namespace Azure.DataApiBuilder.Core.Resolvers
 {
@@ -67,7 +66,6 @@ public MySqlQueryExecutor(
                   httpContextAccessor,
                   handler)
         {
-            handler?.Subscribe(MYSQL_QUERY_EXECUTOR_ON_CONFIG_CHANGED, MySqlQueryExecutorOnConfigChanged);
             _dataSourceAccessTokenUsage = new Dictionary<string, bool>();
             _accessTokensFromConfiguration = runtimeConfigProvider.ManagedIdentityAccessToken;
             _runtimeConfigProvider = runtimeConfigProvider;
@@ -100,18 +98,6 @@ private void ConfigureMySqlQueryExecutor()
             }
         }
 
-        /// <summary>
-        /// Function registered for callback during a hot-reload scenario.
-        /// </summary>
-        /// <param name="sender">The calling object.</param>
-        /// <param name="args">Event arguments.</param>
-        public void MySqlQueryExecutorOnConfigChanged(object? sender, HotReloadEventArgs args)
-        {
-            _dataSourceAccessTokenUsage = new Dictionary<string, bool>();
-            _accessTokensFromConfiguration = _runtimeConfigProvider.ManagedIdentityAccessToken;
-            ConfigureMySqlQueryExecutor();
-        }
-
         /// <summary>
         /// Modifies the properties of the supplied connection to support managed identity access.
         /// In the case of MySql, gets access token if deemed necessary and sets it on the connection.

src/Core/Resolvers/PostgreSqlExecutor.cs

@@ -11,7 +11,6 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Logging;
 using Npgsql;
-using static Azure.DataApiBuilder.Config.DabConfigEvents;
 
 namespace Azure.DataApiBuilder.Core.Resolvers
 {
@@ -68,7 +67,6 @@ public PostgreSqlQueryExecutor(
                   httpContextAccessor,
                   handler)
         {
-            handler?.Subscribe(POSTGRESQL_QUERY_EXECUTOR_ON_CONFIG_CHANGED, PostgreSqlQueryExecutorOnConfigChanged);
             _dataSourceAccessTokenUsage = new Dictionary<string, bool>();
             _accessTokensFromConfiguration = runtimeConfigProvider.ManagedIdentityAccessToken;
             _runtimeConfigProvider = runtimeConfigProvider;
@@ -97,18 +95,6 @@ private void ConfigurePostgreSqlQueryExecutor()
             }
         }
 
-        /// <summary>
-        /// Function registered for callback during a hot-reload scenario.
-        /// </summary>
-        /// <param name="sender">The calling object.</param>
-        /// <param name="args">Event arguments.</param>
-        public void PostgreSqlQueryExecutorOnConfigChanged(object? sender, HotReloadEventArgs args)
-        {
-            _dataSourceAccessTokenUsage = new Dictionary<string, bool>();
-            _accessTokensFromConfiguration = _runtimeConfigProvider.ManagedIdentityAccessToken;
-            ConfigurePostgreSqlQueryExecutor();
-        }
-
         /// <summary>
         /// Modifies the properties of the supplied connection string to support managed identity access.
         /// In the case of Postgres, if a default managed identity needs to be used, the password in the

src/Core/Resolvers/QueryExecutor.cs

@@ -16,7 +16,6 @@
 using Microsoft.Extensions.Logging;
 using Polly;
 using Polly.Retry;
-using static Azure.DataApiBuilder.Config.DabConfigEvents;
 
 namespace Azure.DataApiBuilder.Core.Resolvers
 {
@@ -56,7 +55,6 @@ public QueryExecutor(DbExceptionParser dbExceptionParser,
                              IHttpContextAccessor httpContextAccessor,
                              HotReloadEventHandler<HotReloadEventArgs>? handler)
         {
-            handler?.Subscribe(QUERY_EXECUTOR_ON_CONFIG_CHANGED, OnConfigChanged);
             DbExceptionParser = dbExceptionParser;
             QueryExecutorLogger = logger;
             ConnectionStringBuilders = new Dictionary<string, DbConnectionStringBuilder>();
@@ -86,18 +84,6 @@ public QueryExecutor(DbExceptionParser dbExceptionParser,
                     });
         }
 
-        /// <summary>
-        /// Function registered for callback during a hot-reload scenario.
-        /// </summary>
-        /// <param name="sender">The calling object.</param>
-        /// <param name="args">Event arguments.</param>
-        public void OnConfigChanged(object? sender, HotReloadEventArgs args)
-        {
-            ConnectionStringBuilders = new Dictionary<string, DbConnectionStringBuilder>();
-            _maxResponseSizeMB = ConfigProvider.GetConfig().MaxResponseSizeMB();
-            _maxResponseSizeBytes = _maxResponseSizeMB * 1024 * 1024;
-        }
-
         /// <inheritdoc/>
         public virtual TResult? ExecuteQuery<TResult>(
             string sqltext,

src/Core/Services/MetadataProviders/MetadataProviderFactory.cs

@@ -64,7 +64,8 @@ public void OnConfigChanged(object? sender, HotReloadEventArgs args)
         {
             _metadataProviders.Clear();
             ConfigureMetadataProviders();
-            InitializeAsync().Wait();
+            // Blocks the current thread until initialization is finished.
+            this.InitializeAsync().GetAwaiter().GetResult();
         }
 
         /// <inheritdoc />

src/Service.Tests/Caching/CachingConfigProcessingTests.cs

@@ -59,8 +59,6 @@ public void EntityCacheOptionsDeserialization_ValidJson(
             logger: null,
             connectionString: null,
             replaceEnvVar: false,
-            dataSourceName: string.Empty,
-            datasourceNameToConnectionString: null,
             replacementFailureMode: EnvironmentVariableReplacementFailureMode.Throw);
 
         // Assert
@@ -108,8 +106,6 @@ public void EntityCacheOptionsDeserialization_InvalidValues(string entityCacheCo
             logger: null,
             connectionString: null,
             replaceEnvVar: false,
-            dataSourceName: string.Empty,
-            datasourceNameToConnectionString: null,
             replacementFailureMode: EnvironmentVariableReplacementFailureMode.Throw);
 
         // Assert
@@ -148,8 +144,6 @@ public void GlobalCacheOptionsDeserialization_ValidValues(
             logger: null,
             connectionString: null,
             replaceEnvVar: false,
-            dataSourceName: string.Empty,
-            datasourceNameToConnectionString: null,
             replacementFailureMode: EnvironmentVariableReplacementFailureMode.Throw);
 
         // Assert
@@ -196,8 +190,6 @@ public void GlobalCacheOptionsDeserialization_InvalidValues(string globalCacheCo
             logger: null,
             connectionString: null,
             replaceEnvVar: false,
-            dataSourceName: string.Empty,
-            datasourceNameToConnectionString: null,
             replacementFailureMode: EnvironmentVariableReplacementFailureMode.Throw);
 
         // Assert
@@ -227,8 +219,6 @@ public void GlobalCacheOptionsOverridesEntityCacheOptions(string globalCacheConf
                        logger: null,
                        connectionString: null,
                        replaceEnvVar: false,
-                       dataSourceName: string.Empty,
-                       datasourceNameToConnectionString: null,
                        replacementFailureMode: EnvironmentVariableReplacementFailureMode.Throw);
 
         // Assert
@@ -265,8 +255,6 @@ public void UserDefinedTtlWrittenToSerializedJsonConfigFile(bool expectIsUserDef
                        logger: null,
                        connectionString: null,
                        replaceEnvVar: false,
-                       dataSourceName: string.Empty,
-                       datasourceNameToConnectionString: null,
                        replacementFailureMode: EnvironmentVariableReplacementFailureMode.Throw);
         Assert.IsNotNull(config, message: "Test setup failure. Config must not be null, runtime config JSON deserialization failed.");
 
@@ -315,8 +303,6 @@ public void CachePropertyNotWrittenToSerializedJsonConfigFile(string cacheConfig
                        logger: null,
                        connectionString: null,
                        replaceEnvVar: false,
-                       dataSourceName: string.Empty,
-                       datasourceNameToConnectionString: null,
                        replacementFailureMode: EnvironmentVariableReplacementFailureMode.Throw);
         Assert.IsNotNull(config, message: "Test setup failure. Config must not be null, runtime config JSON deserialization failed.");
 
@@ -359,8 +345,6 @@ public void DefaultTtlNotWrittenToSerializedJsonConfigFile(string cacheConfig)
                        logger: null,
                        connectionString: null,
                        replaceEnvVar: false,
-                       dataSourceName: string.Empty,
-                       datasourceNameToConnectionString: null,
                        replacementFailureMode: EnvironmentVariableReplacementFailureMode.Throw);
         Assert.IsNotNull(config, message: "Test setup failure. Config must not be null, runtime config JSON deserialization failed.");