Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maintain Default Data Source Name from Original Config after Hot Reloading #2069

Merged
merged 23 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
899bd9d
pass in datasource to save in hot reload
aaronburtle Feb 28, 2024
7b1bfc4
add helper to update dictionaries in runtimeconfig
aaronburtle Feb 29, 2024
efd04ba
add testing, cleanup format
aaronburtle Mar 2, 2024
ecac30f
addressing comments, still need to remove GetDataSourceName()
aaronburtle Mar 11, 2024
2f57773
remove un-needed function for getting dataSourceName, cleanup test st…
aaronburtle Mar 11, 2024
58d418c
whitespace
aaronburtle Mar 11, 2024
732d32e
clarifying comments
aaronburtle Mar 12, 2024
81b2ecd
refactor update
aaronburtle Mar 12, 2024
b3c067b
use private DefaultDataSourceName
aaronburtle Mar 12, 2024
873445a
typos
aaronburtle Mar 12, 2024
1a9229d
push change to rivate
aaronburtle Mar 12, 2024
da0abcd
match mocked loader to new signature
aaronburtle Mar 12, 2024
f16ef6d
change get method to property
aaronburtle Mar 12, 2024
5e1b057
update references to property
aaronburtle Mar 12, 2024
579199f
add ignore for default data source name to unit test verifier
aaronburtle Mar 12, 2024
be78615
style fix
aaronburtle Mar 12, 2024
3d31d89
merging
aaronburtle Mar 12, 2024
7f316c3
merging main
aaronburtle Mar 12, 2024
9d1bd74
add ignore for other tests in verifier for Jsonignored DefaultDatasou…
aaronburtle Mar 12, 2024
f295edb
defensive coding, typo fix
aaronburtle Mar 12, 2024
5148832
format
aaronburtle Mar 13, 2024
be7bd5e
Merge branch 'main' into dev/aaronburtle/SaveDataSourceNameForHotReload
aaronburtle Mar 13, 2024
937b731
Merge branch 'main' into dev/aaronburtle/SaveDataSourceNameForHotReload
aaronburtle Mar 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions src/Config/FileSystemRuntimeConfigLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,26 @@ public string GetConfigFileName()
public bool TryLoadConfig(
string path,
[NotNullWhen(true)] out RuntimeConfig? config,
bool replaceEnvVar = false)
bool replaceEnvVar = false,
string dataSourceName = "")
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
{
if (_fileSystem.File.Exists(path))
{
Console.WriteLine($"Loading config file from {path}.");
string json = _fileSystem.File.ReadAllText(path);
return TryParseConfig(json, out config, connectionString: _connectionString, replaceEnvVar: replaceEnvVar);
bool isParsed = TryParseConfig(
json: json,
config: out config,
connectionString: _connectionString,
replaceEnvVar: replaceEnvVar);

if (isParsed && !string.IsNullOrEmpty(dataSourceName))
{
config!.UpdateDefaultDataSourceNameDependantDictionaries(dataSourceName);
config!.DefaultDataSourceName = dataSourceName;
}
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved

return isParsed;
}
else
{
Expand All @@ -124,9 +137,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)
public override bool TryLoadKnownConfig([NotNullWhen(true)] out RuntimeConfig? config, bool replaceEnvVar = false, string dataSourceName = "")
{
return TryLoadConfig(ConfigFilePath, out config, replaceEnvVar);
return TryLoadConfig(ConfigFilePath, out config, replaceEnvVar, dataSourceName);
}

/// <summary>
Expand Down
32 changes: 26 additions & 6 deletions src/Config/ObjectModel/RuntimeConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ Runtime.GraphQL is null ||
}
}

private string _defaultDataSourceName;
public string DefaultDataSourceName;
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved

private Dictionary<string, DataSource> _dataSourceNameToDataSource;

Expand Down Expand Up @@ -169,18 +169,18 @@ public RuntimeConfig(string? Schema, DataSource DataSource, RuntimeEntities Enti
this.DataSource = DataSource;
this.Runtime = Runtime;
this.Entities = Entities;
_defaultDataSourceName = Guid.NewGuid().ToString();
DefaultDataSourceName = Guid.NewGuid().ToString();
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved

// we will set them up with default values
_dataSourceNameToDataSource = new Dictionary<string, DataSource>
{
{ _defaultDataSourceName, this.DataSource }
{ DefaultDataSourceName, this.DataSource }
};

_entityNameToDataSourceName = new Dictionary<string, string>();
foreach (KeyValuePair<string, Entity> entity in Entities)
{
_entityNameToDataSourceName.TryAdd(entity.Key, _defaultDataSourceName);
_entityNameToDataSourceName.TryAdd(entity.Key, DefaultDataSourceName);
}

// Process data source and entities information for each database in multiple database scenario.
Expand Down Expand Up @@ -241,7 +241,7 @@ public RuntimeConfig(string Schema, DataSource DataSource, RuntimeOptions Runtim
this.DataSource = DataSource;
this.Runtime = Runtime;
this.Entities = Entities;
_defaultDataSourceName = DefaultDataSourceName;
this.DefaultDataSourceName = DefaultDataSourceName;
_dataSourceNameToDataSource = DataSourceNameToDataSource;
_entityNameToDataSourceName = EntityNameToDataSourceName;
this.DataSourceFiles = DataSourceFiles;
Expand Down Expand Up @@ -273,6 +273,26 @@ public void UpdateDataSourceNameToDataSource(string dataSourceName, DataSource d
_dataSourceNameToDataSource[dataSourceName] = dataSource;
}

/// <summary>
/// In a Hot Reload scenario we should maintain the same default data source
/// name before the hot reload as after the hot reload. This method takes
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
/// a default data source name, such as the one from before hot reload, and
/// replaces the current dictionary entries of this RuntimeConfig that were
/// built using a new, unique guid during the construction of this RuntimeConfig
/// with entries using the provided default data source name.
/// </summary>
/// <param name="defaultDataSourceName">The name used to update the dictionaries.</param>
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
public void UpdateDefaultDataSourceNameDependantDictionaries(string defaultDataSourceName)
{
_dataSourceNameToDataSource.Remove(DefaultDataSourceName);
_dataSourceNameToDataSource.Add(defaultDataSourceName, this.DataSource);
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
foreach (KeyValuePair<string, Entity> entity in Entities)
{
_entityNameToDataSourceName.Remove(entity.Key);
_entityNameToDataSourceName.TryAdd(entity.Key, DefaultDataSourceName);
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// <summary>
/// Gets datasourceName from EntityNameToDatasourceName dictionary.
/// </summary>
Expand Down Expand Up @@ -311,7 +331,7 @@ public bool CheckDataSourceExists(string dataSourceName)
public string GetDefaultDataSourceName()
#pragma warning restore CA1024 // Use properties where appropriate
{
return _defaultDataSourceName;
return DefaultDataSourceName;
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Config/RuntimeConfigLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public RuntimeConfigLoader(string? connectionString = null)
/// <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 abstract bool TryLoadKnownConfig([NotNullWhen(true)] out RuntimeConfig? config, bool replaceEnvVar = false);
public abstract bool TryLoadKnownConfig([NotNullWhen(true)] out RuntimeConfig? config, bool replaceEnvVar = false, string dataSourceName = "");
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Returns the link to the published draft schema.
Expand Down
3 changes: 2 additions & 1 deletion src/Core/Configurations/ConfigFileWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ private void OnConfigFileChange(object sender, FileSystemEventArgs e)

if (!_configProvider.IsLateConfigured && _configProvider.GetConfig().IsDevelopmentMode())
{
_configProvider.HotReloadConfig();
// pass along the original default data source name for consistency within runtime config's data structures
_configProvider.HotReloadConfig(_configProvider.GetConfig().DefaultDataSourceName);
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
}
}
catch (Exception ex)
Expand Down
4 changes: 2 additions & 2 deletions src/Core/Configurations/RuntimeConfigProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,9 @@ public bool TryGetLoadedConfig([NotNullWhen(true)] out RuntimeConfig? runtimeCon
/// Hot Reloads the runtime config when the file watcher
/// is active and detects a change to the underlying config file.
/// </summary>
public void HotReloadConfig()
public void HotReloadConfig(string dataSourceName)
{
ConfigLoader.TryLoadKnownConfig(out _runtimeConfig, replaceEnvVar: true);
ConfigLoader.TryLoadKnownConfig(out _runtimeConfig, replaceEnvVar: true, dataSourceName: dataSourceName);
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
Expand Down
128 changes: 63 additions & 65 deletions src/Service.Tests/Unittests/ConfigFileWatcherUnitTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,17 @@ namespace Azure.DataApiBuilder.Service.Tests.Unittests
[TestClass]
public class ConfigFileWatcherUnitTests
{
/// <summary>
/// Use the file system (not mocked) to create a hot reload
/// scenario of the REST runtime options and verify that we
/// correctly hot reload those options.
/// </summary>
[TestMethod]
public void HotReloadConfigRestRuntimeOptions()
{
string initialRestPath = "/api";
string updatedRestPath = "/rest";
string initialGQLPath = "/api";
string updatedGQLPath = "/gql";

bool initialRestEnabled = true;
bool updatedRestEnabled = false;
bool initialGQLEnabled = true;
bool updatedGQLEnabled = false;

bool initialGQLIntrospection = true;
bool updatedGQLIntrospection = false;

HostMode initialMode = HostMode.Development;
HostMode updatedMode = HostMode.Production;

string initialConfig = @"
private static string GenerateRuntimeSectionStringFromParams(
string restPath,
string gqlPath,
bool restEnabled,
bool gqlEnabled,
bool gqlIntrospection,
HostMode mode
)
{
string runtimeString = @"
{
""$schema"": ""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch/dab.draft.schema.json"",
""data-source"": {
Expand All @@ -49,13 +35,13 @@ public void HotReloadConfigRestRuntimeOptions()
},
""runtime"": {
""rest"": {
""enabled"": " + initialRestEnabled.ToString().ToLower() + @",
""path"": """ + initialRestPath + @"""
""enabled"": " + restEnabled.ToString().ToLower() + @",
""path"": """ + restPath + @"""
},
""graphql"": {
""enabled"": " + initialGQLEnabled.ToString().ToLower() + @",
""path"": """ + initialGQLPath + @""",
""allow-introspection"": " + initialGQLIntrospection.ToString().ToLower() + @"
""enabled"": " + gqlEnabled.ToString().ToLower() + @",
""path"": """ + gqlPath + @""",
""allow-introspection"": " + gqlIntrospection.ToString().ToLower() + @"
},
""host"": {
""cors"": {
Expand All @@ -67,46 +53,54 @@ public void HotReloadConfigRestRuntimeOptions()
""authentication"": {
""provider"": ""StaticWebApps""
},
""mode"": """ + initialMode + @"""
}
},
""entities"": {}
}";
string updatedConfig = @"
{
""$schema"": ""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch/dab.draft.schema.json"",
""data-source"": {
""database-type"": ""mssql"",
""connection-string"": ""Server=test;Database=test;User ID=test"",
""options"": {
""set-session-context"": true
}
},
""runtime"": {
""rest"": {
""enabled"": " + updatedRestEnabled.ToString().ToLower() + @",
""path"": """ + updatedRestPath + @"""
},
""graphql"": {
""enabled"": " + updatedGQLEnabled.ToString().ToLower() + @",
""path"": """ + updatedGQLPath + @""",
""allow-introspection"": " + updatedGQLIntrospection.ToString().ToLower() + @"
},
""host"": {
""cors"": {
""origins"": [
""http://localhost:5000""
],
""allow-credentials"": false
},
""authentication"": {
""provider"": ""StaticWebApps""
},
""mode"": """ + updatedMode + @"""
""mode"": """ + mode + @"""
}
},
""entities"": {}
}";
return runtimeString;
}

/// <summary>
/// Use the file system (not mocked) to create a hot reload
/// scenario of the REST runtime options and verify that we
/// correctly hot reload those options.
/// </summary>
[TestMethod]
public void HotReloadConfigRestRuntimeOptions()
{
string initialRestPath = "/api";
string updatedRestPath = "/rest";
string initialGQLPath = "/api";
string updatedGQLPath = "/gql";

bool initialRestEnabled = true;
bool updatedRestEnabled = false;
bool initialGQLEnabled = true;
bool updatedGQLEnabled = false;

bool initialGQLIntrospection = true;
bool updatedGQLIntrospection = false;

HostMode initialMode = HostMode.Development;
HostMode updatedMode = HostMode.Production;

string initialConfig = GenerateRuntimeSectionStringFromParams(
initialRestPath,
initialGQLPath,
initialRestEnabled,
initialGQLEnabled,
initialGQLIntrospection,
initialMode);

string updatedConfig = GenerateRuntimeSectionStringFromParams(
updatedRestPath,
updatedGQLPath,
updatedRestEnabled,
updatedGQLEnabled,
updatedGQLIntrospection,
updatedMode);

string configName = "hotreload-config.json";
if (File.Exists(configName))
{
Expand All @@ -120,6 +114,7 @@ public void HotReloadConfigRestRuntimeOptions()
RuntimeConfigProvider configProvider = new(configLoader);
// Must GetConfig() to start file watching
RuntimeConfig runtimeConfig = configProvider.GetConfig();
string initialDefaultDataSourceName = runtimeConfig.DefaultDataSourceName;
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
// assert we have a valid config
Assert.IsNotNull(runtimeConfig);
Assert.AreEqual(initialRestEnabled, runtimeConfig.Runtime.Rest.Enabled);
Expand All @@ -134,12 +129,15 @@ public void HotReloadConfigRestRuntimeOptions()
System.Threading.Thread.Sleep(1000);
// Hot Reloaded config
runtimeConfig = configProvider.GetConfig();
string updatedDefaultDataSourceName = runtimeConfig.DefaultDataSourceName;
Assert.AreEqual(updatedRestEnabled, runtimeConfig.Runtime.Rest.Enabled);
Assert.AreEqual(updatedRestPath, runtimeConfig.Runtime.Rest.Path);
Assert.AreEqual(updatedGQLEnabled, runtimeConfig.Runtime.GraphQL.Enabled);
Assert.AreEqual(updatedGQLPath, runtimeConfig.Runtime.GraphQL.Path);
Assert.AreEqual(updatedGQLIntrospection, runtimeConfig.Runtime.GraphQL.AllowIntrospection);
Assert.AreEqual(updatedMode, runtimeConfig.Runtime.Host.Mode);
// DefaultDataSourceName should not change after a hot reload.
Assert.AreEqual(initialDefaultDataSourceName, updatedDefaultDataSourceName);
if (File.Exists(configName))
{
File.Delete(configName);
Expand Down