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 10 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
24 changes: 20 additions & 4 deletions src/Config/FileSystemRuntimeConfigLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,32 @@ public string GetConfigFileName()
/// <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">If provided and not empty, this is the data source name that will be used in the loaded config.</param>
/// <returns>True if the config was loaded, otherwise false.</returns>
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);

if (TryParseConfig(
json: json,
config: out config,
connectionString: _connectionString,
replaceEnvVar: replaceEnvVar))
{
if (!string.IsNullOrEmpty(dataSourceName))
{
config.UpdateDefaultDataSourceName(dataSourceName);
}

return true;
}
}
else
{
Expand All @@ -123,10 +138,11 @@ public bool TryLoadConfig(
/// <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 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
26 changes: 25 additions & 1 deletion 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 @@ -273,6 +273,30 @@ 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 is because we hold
/// references to the Data Source itself which depend on this data source name
/// for lookups. To correctly retrieve this information after a hot reload
/// we need the data source name to stay the same after hot reloading. This method takes
/// 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. We then update the DefaultDataSourceName.
/// </summary>
/// <param name="initialDefaultDataSourceName">The name used to update the dictionaries.</param>
public void UpdateDefaultDataSourceName(string initialDefaultDataSourceName)
{
_dataSourceNameToDataSource.Remove(_defaultDataSourceName);
_dataSourceNameToDataSource.Add(initialDefaultDataSourceName, this.DataSource);
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
foreach (KeyValuePair<string, Entity> entity in Entities)
{
_entityNameToDataSourceName[entity.Key] = initialDefaultDataSourceName;
}

_defaultDataSourceName = initialDefaultDataSourceName;
}

/// <summary>
/// Gets datasourceName from EntityNameToDatasourceName dictionary.
/// </summary>
Expand Down
3 changes: 2 additions & 1 deletion src/Config/RuntimeConfigLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ public RuntimeConfigLoader(string? connectionString = null)
/// <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);
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
1 change: 1 addition & 0 deletions src/Core/Configurations/ConfigFileWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ private void OnConfigFileChange(object sender, FileSystemEventArgs e)

if (!_configProvider.IsLateConfigured && _configProvider.GetConfig().IsDevelopmentMode())
{
// pass along the original default data source name for consistency within runtime config's data structures
_configProvider.HotReloadConfig();
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/Core/Configurations/RuntimeConfigProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ public bool TryGetLoadedConfig([NotNullWhen(true)] out RuntimeConfig? runtimeCon
/// </summary>
public void HotReloadConfig()
{
ConfigLoader.TryLoadKnownConfig(out _runtimeConfig, replaceEnvVar: true);
// _runtimeconfig can not be null in a hot reload scenario
ConfigLoader.TryLoadKnownConfig(out _runtimeConfig, replaceEnvVar: true, _runtimeConfig!.GetDefaultDataSourceName());
}

/// <summary>
Expand Down
143 changes: 77 additions & 66 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,48 +35,13 @@ public void HotReloadConfigRestRuntimeOptions()
},
""runtime"": {
""rest"": {
""enabled"": " + initialRestEnabled.ToString().ToLower() + @",
""path"": """ + initialRestPath + @"""
},
""graphql"": {
""enabled"": " + initialGQLEnabled.ToString().ToLower() + @",
""path"": """ + initialGQLPath + @""",
""allow-introspection"": " + initialGQLIntrospection.ToString().ToLower() + @"
},
""host"": {
""cors"": {
""origins"": [
""http://localhost:5000""
],
""allow-credentials"": false
},
""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 + @"""
""enabled"": " + restEnabled.ToString().ToLower() + @",
""path"": """ + restPath + @"""
},
""graphql"": {
""enabled"": " + updatedGQLEnabled.ToString().ToLower() + @",
""path"": """ + updatedGQLPath + @""",
""allow-introspection"": " + updatedGQLIntrospection.ToString().ToLower() + @"
""enabled"": " + gqlEnabled.ToString().ToLower() + @",
""path"": """ + gqlPath + @""",
""allow-introspection"": " + gqlIntrospection.ToString().ToLower() + @"
},
""host"": {
""cors"": {
Expand All @@ -102,11 +53,57 @@ public void HotReloadConfigRestRuntimeOptions()
""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()
{
// Arrange
// 1. Setup the strings that are turned into our initital and runtime config to reload.
// 2. Generate initial runtimeconfig, start file watching, and assert we have valid initial values.
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 @@ -118,8 +115,11 @@ public void HotReloadConfigRestRuntimeOptions()
fileSystem.File.WriteAllText(configName, initialConfig);
FileSystemRuntimeConfigLoader configLoader = new(fileSystem, configName, string.Empty);
RuntimeConfigProvider configProvider = new(configLoader);

// Must GetConfig() to start file watching
RuntimeConfig runtimeConfig = configProvider.GetConfig();
string initialDefaultDataSourceName = runtimeConfig.GetDefaultDataSourceName();

// assert we have a valid config
Assert.IsNotNull(runtimeConfig);
Assert.AreEqual(initialRestEnabled, runtimeConfig.Runtime.Rest.Enabled);
Expand All @@ -128,18 +128,29 @@ public void HotReloadConfigRestRuntimeOptions()
Assert.AreEqual(initialGQLPath, runtimeConfig.Runtime.GraphQL.Path);
Assert.AreEqual(initialGQLIntrospection, runtimeConfig.Runtime.GraphQL.AllowIntrospection);
Assert.AreEqual(initialMode, runtimeConfig.Runtime.Host.Mode);

// Simulate change to the config file
fileSystem.File.WriteAllText(configName, updatedConfig);

// Give ConfigFileWatcher enough time to hot reload the change
System.Threading.Thread.Sleep(1000);
// Hot Reloaded config

// Act
// 1. Hot reload the runtime config
runtimeConfig = configProvider.GetConfig();
string updatedDefaultDataSourceName = runtimeConfig.GetDefaultDataSourceName();

// Assert
// 1. Assert we have the correcr values after a hot reload.
aaronburtle marked this conversation as resolved.
Show resolved Hide resolved
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