From a3a805e57c5fe951c2a37c1c8635df01a97e866e Mon Sep 17 00:00:00 2001 From: aaron burtle Date: Tue, 24 Mar 2026 14:42:22 -0700 Subject: [PATCH 01/10] fix schema validation message for no entities --- src/Cli.Tests/EndToEndTests.cs | 5 -- src/Cli.Tests/EnvironmentTests.cs | 4 +- src/Cli.Tests/ValidateConfigTests.cs | 50 +++++++++++++++++++ src/Cli/Commands/ValidateOptions.cs | 2 +- src/Cli/ConfigGenerator.cs | 26 +++++++++- src/Config/RuntimeConfigLoader.cs | 20 +++++--- .../Configuration/RuntimeConfigLoaderTests.cs | 6 ++- 7 files changed, 96 insertions(+), 17 deletions(-) diff --git a/src/Cli.Tests/EndToEndTests.cs b/src/Cli.Tests/EndToEndTests.cs index 4ae27f790f..9ab7085bbc 100644 --- a/src/Cli.Tests/EndToEndTests.cs +++ b/src/Cli.Tests/EndToEndTests.cs @@ -1093,11 +1093,6 @@ public async Task TestExitOfRuntimeEngineWithInvalidConfig( { output = await process.StandardError.ReadLineAsync(); Assert.IsNotNull(output); - StringAssert.Contains(output, $"Deserialization of the configuration file failed.", StringComparison.Ordinal); - - output = await process.StandardOutput.ReadLineAsync(); - Assert.IsNotNull(output); - StringAssert.Contains(output, $"Error: Failed to parse the config file: {TEST_RUNTIME_CONFIG_FILE}.", StringComparison.Ordinal); output = await process.StandardOutput.ReadLineAsync(); Assert.IsNotNull(output); diff --git a/src/Cli.Tests/EnvironmentTests.cs b/src/Cli.Tests/EnvironmentTests.cs index d7b359783a..421a8ef8fc 100644 --- a/src/Cli.Tests/EnvironmentTests.cs +++ b/src/Cli.Tests/EnvironmentTests.cs @@ -163,8 +163,8 @@ public async Task FailureToStartEngineWhenEnvVarNamedWrong() ); string? output = await process.StandardError.ReadLineAsync(); - Assert.AreEqual("Deserialization of the configuration file failed during a post-processing step.", output); - output = await process.StandardError.ReadToEndAsync(); + Assert.IsNotNull(output); + // Clean error message with no wrapper prefix or stack trace. StringAssert.Contains(output, "A valid Connection String should be provided.", StringComparison.Ordinal); process.Kill(); } diff --git a/src/Cli.Tests/ValidateConfigTests.cs b/src/Cli.Tests/ValidateConfigTests.cs index e40a32e291..af4e6128bd 100644 --- a/src/Cli.Tests/ValidateConfigTests.cs +++ b/src/Cli.Tests/ValidateConfigTests.cs @@ -199,6 +199,56 @@ public void TestValidateConfigFailsWithNoEntities() } } + /// + /// Validates that when the config has no entities or autoentities, TryParseConfig + /// sets a clean error message (not a raw exception with stack trace) and + /// IsConfigValid returns false without throwing. + /// Regression test for https://github.com/Azure/data-api-builder/issues/3268 + /// + [TestMethod] + public void TestValidateConfigWithNoEntitiesProducesCleanError() + { + string configWithoutEntities = $"{{{SAMPLE_SCHEMA_DATA_SOURCE},{RUNTIME_SECTION}}}"; + + // Verify TryParseConfig produces a clean error without stack traces. + RuntimeConfigLoader.LastParseError = null; + + StringWriter errorWriter = new(); + TextWriter originalError = Console.Error; + Console.SetError(errorWriter); + try + { + bool parsed = RuntimeConfigLoader.TryParseConfig(configWithoutEntities, out _); + Assert.IsFalse(parsed, "Config with no entities should fail to parse."); + } + finally + { + Console.SetError(originalError); + } + + // LastParseError should contain the clean validation message. + Assert.IsNotNull(RuntimeConfigLoader.LastParseError, + "LastParseError should be set when config parsing fails."); + StringAssert.Contains(RuntimeConfigLoader.LastParseError, + "Configuration file should contain either at least the entities or autoentities property", + "Parse error should contain the clean validation message."); + + // Console.Error output should be clean (no stack trace, no "Deserialization" wrapper). + string stderrOutput = errorWriter.ToString(); + Assert.IsFalse(stderrOutput.Contains("Stack Trace"), + "Stack trace should not be present in stderr output."); + Assert.IsFalse(stderrOutput.Contains("Deserialization of the configuration file failed"), + "Deserialization wrapper should not appear for DataApiBuilderException errors."); + StringAssert.Contains(stderrOutput, + "Configuration file should contain either at least the entities or autoentities property", + "stderr should show just the clean error message."); + + // Verify IsConfigValid also returns false cleanly (no exception thrown). + ((MockFileSystem)_fileSystem!).AddFile(TEST_RUNTIME_CONFIG_FILE, configWithoutEntities); + ValidateOptions validateOptions = new(TEST_RUNTIME_CONFIG_FILE); + Assert.IsFalse(ConfigGenerator.IsConfigValid(validateOptions, _runtimeConfigLoader!, _fileSystem!)); + } + /// /// This Test is used to verify that the validate command is able to catch when data source field is missing. /// diff --git a/src/Cli/Commands/ValidateOptions.cs b/src/Cli/Commands/ValidateOptions.cs index b522383945..6cef26ba5b 100644 --- a/src/Cli/Commands/ValidateOptions.cs +++ b/src/Cli/Commands/ValidateOptions.cs @@ -38,7 +38,7 @@ public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSy } else { - logger.LogError("Config is invalid. Check above logs for details."); + logger.LogInformation("Config is invalid."); } return isValidConfig ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR; diff --git a/src/Cli/ConfigGenerator.cs b/src/Cli/ConfigGenerator.cs index 8cec9dd239..948ca54c9e 100644 --- a/src/Cli/ConfigGenerator.cs +++ b/src/Cli/ConfigGenerator.cs @@ -2564,7 +2564,9 @@ public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRun // Replaces all the environment variables while deserializing when starting DAB. if (!loader.TryLoadKnownConfig(out RuntimeConfig? deserializedRuntimeConfig, replaceEnvVar: true)) { - _logger.LogError("Failed to parse the config file: {runtimeConfigFile}.", runtimeConfigFile); + string? parseError = RuntimeConfigLoader.LastParseError; + RuntimeConfigLoader.LastParseError = null; + _logger.LogError("{parseError}", parseError ?? $"Failed to parse the config file: {runtimeConfigFile}."); return false; } else @@ -2643,6 +2645,28 @@ public static bool IsConfigValid(ValidateOptions options, FileSystemRuntimeConfi RuntimeConfigProvider runtimeConfigProvider = new(loader); + // Suppress Console.Error during config loading so that parse errors + // are routed through the CLI's structured logger instead of raw stderr. + TextWriter originalError = Console.Error; + Console.SetError(TextWriter.Null); + bool configLoaded; + try + { + configLoaded = runtimeConfigProvider.TryGetConfig(out RuntimeConfig? _); + } + finally + { + Console.SetError(originalError); + } + + if (!configLoaded) + { + string? parseError = RuntimeConfigLoader.LastParseError; + RuntimeConfigLoader.LastParseError = null; + _logger.LogError("{parseError}", parseError ?? "Failed to parse the config file."); + return false; + } + ILogger runtimeConfigValidatorLogger = LoggerFactoryForCli.CreateLogger(); RuntimeConfigValidator runtimeConfigValidator = new(runtimeConfigProvider, fileSystem, runtimeConfigValidatorLogger, true); diff --git a/src/Config/RuntimeConfigLoader.cs b/src/Config/RuntimeConfigLoader.cs index ae5c2dde95..68d852b50a 100644 --- a/src/Config/RuntimeConfigLoader.cs +++ b/src/Config/RuntimeConfigLoader.cs @@ -38,6 +38,12 @@ public abstract class RuntimeConfigLoader public bool IsNewConfigValidated; + /// + /// Stores the last config parse error message for callers that need + /// to retrieve and log it through their own logging infrastructure. + /// + public static string? LastParseError { get; set; } + public RuntimeConfigLoader(HotReloadEventHandler? handler = null, string? connectionString = null) { _changeToken = new DabChangeToken(); @@ -263,17 +269,19 @@ public static bool TryParseConfig(string json, ex is JsonException || ex is DataApiBuilderException) { - string errorMessage = ex is JsonException ? "Deserialization of the configuration file failed." : - "Deserialization of the configuration file failed during a post-processing step."; + // Store the parse error so callers (e.g. CLI) can retrieve and + // log it through their own structured logging infrastructure. + LastParseError = ex is DataApiBuilderException + ? ex.Message + : $"Deserialization of the configuration file failed. {ex.Message}"; - // logger can be null when called from CLI - if (logger is null) + if (logger is not null) { - Console.Error.WriteLine(errorMessage + $"\n" + $"Message:\n {ex.Message}\n" + $"Stack Trace:\n {ex.StackTrace}"); + logger.LogError(exception: ex, message: "{ParseError}", LastParseError); } else { - logger.LogError(exception: ex, message: errorMessage); + Console.Error.WriteLine(LastParseError); } config = null; diff --git a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs index d6f19ec65f..eeaf43c282 100644 --- a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs +++ b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs @@ -98,8 +98,10 @@ public async Task FailLoadMultiDataSourceConfigDuplicateEntities(string configPa loader.TryLoadConfig("dab-config.json", out RuntimeConfig _); string error = sw.ToString(); - Assert.IsTrue(error.StartsWith("Deserialization of the configuration file failed during a post-processing step.")); - Assert.IsTrue(error.Contains("An item with the same key has already been added.")); + Assert.IsTrue(error.Contains("An item with the same key has already been added."), + "Error should contain the specific validation message."); + Assert.IsFalse(error.Contains("Stack Trace"), + "Stack trace should not be present in error output."); } /// From f99a646b62f86936047bb6b3bf70cbe24fed0169 Mon Sep 17 00:00:00 2001 From: aaron burtle Date: Tue, 24 Mar 2026 21:29:43 -0700 Subject: [PATCH 02/10] slight refactor to avoid duplicate logs --- src/Cli.Tests/EnvironmentTests.cs | 2 +- src/Cli.Tests/ValidateConfigTests.cs | 40 +++++++------------ src/Cli/ConfigGenerator.cs | 38 +++++++++--------- src/Config/RuntimeConfigLoader.cs | 18 +++++---- .../Configuration/RuntimeConfigLoaderTests.cs | 16 +++++--- 5 files changed, 54 insertions(+), 60 deletions(-) diff --git a/src/Cli.Tests/EnvironmentTests.cs b/src/Cli.Tests/EnvironmentTests.cs index 421a8ef8fc..c03025c584 100644 --- a/src/Cli.Tests/EnvironmentTests.cs +++ b/src/Cli.Tests/EnvironmentTests.cs @@ -164,7 +164,7 @@ public async Task FailureToStartEngineWhenEnvVarNamedWrong() string? output = await process.StandardError.ReadLineAsync(); Assert.IsNotNull(output); - // Clean error message with no wrapper prefix or stack trace. + // Clean error message on stderr with no stack trace. StringAssert.Contains(output, "A valid Connection String should be provided.", StringComparison.Ordinal); process.Kill(); } diff --git a/src/Cli.Tests/ValidateConfigTests.cs b/src/Cli.Tests/ValidateConfigTests.cs index af4e6128bd..08e59a4e5a 100644 --- a/src/Cli.Tests/ValidateConfigTests.cs +++ b/src/Cli.Tests/ValidateConfigTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.IO; using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; using Serilog; @@ -210,38 +211,25 @@ public void TestValidateConfigWithNoEntitiesProducesCleanError() { string configWithoutEntities = $"{{{SAMPLE_SCHEMA_DATA_SOURCE},{RUNTIME_SECTION}}}"; - // Verify TryParseConfig produces a clean error without stack traces. - RuntimeConfigLoader.LastParseError = null; - + // Capture stderr to verify TryParseConfig produces a clean error without stack traces. + RuntimeConfigLoader.HasParseError = false; StringWriter errorWriter = new(); TextWriter originalError = Console.Error; Console.SetError(errorWriter); - try - { - bool parsed = RuntimeConfigLoader.TryParseConfig(configWithoutEntities, out _); - Assert.IsFalse(parsed, "Config with no entities should fail to parse."); - } - finally - { - Console.SetError(originalError); - } - // LastParseError should contain the clean validation message. - Assert.IsNotNull(RuntimeConfigLoader.LastParseError, - "LastParseError should be set when config parsing fails."); - StringAssert.Contains(RuntimeConfigLoader.LastParseError, - "Configuration file should contain either at least the entities or autoentities property", - "Parse error should contain the clean validation message."); + bool parsed = RuntimeConfigLoader.TryParseConfig(configWithoutEntities, out _); - // Console.Error output should be clean (no stack trace, no "Deserialization" wrapper). - string stderrOutput = errorWriter.ToString(); - Assert.IsFalse(stderrOutput.Contains("Stack Trace"), - "Stack trace should not be present in stderr output."); - Assert.IsFalse(stderrOutput.Contains("Deserialization of the configuration file failed"), - "Deserialization wrapper should not appear for DataApiBuilderException errors."); - StringAssert.Contains(stderrOutput, + Console.SetError(originalError); + string errorOutput = errorWriter.ToString(); + + Assert.IsFalse(parsed, "Config with no entities should fail to parse."); + Assert.IsTrue(RuntimeConfigLoader.HasParseError, + "HasParseError should be true when config parsing fails."); + StringAssert.Contains(errorOutput, "Configuration file should contain either at least the entities or autoentities property", - "stderr should show just the clean error message."); + "Parse error should contain the clean validation message."); + Assert.IsFalse(errorOutput.Contains("StackTrace"), + "Stack trace should not be present in parse error."); // Verify IsConfigValid also returns false cleanly (no exception thrown). ((MockFileSystem)_fileSystem!).AddFile(TEST_RUNTIME_CONFIG_FILE, configWithoutEntities); diff --git a/src/Cli/ConfigGenerator.cs b/src/Cli/ConfigGenerator.cs index 948ca54c9e..4f1e2d047b 100644 --- a/src/Cli/ConfigGenerator.cs +++ b/src/Cli/ConfigGenerator.cs @@ -2564,9 +2564,15 @@ public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRun // Replaces all the environment variables while deserializing when starting DAB. if (!loader.TryLoadKnownConfig(out RuntimeConfig? deserializedRuntimeConfig, replaceEnvVar: true)) { - string? parseError = RuntimeConfigLoader.LastParseError; - RuntimeConfigLoader.LastParseError = null; - _logger.LogError("{parseError}", parseError ?? $"Failed to parse the config file: {runtimeConfigFile}."); + // When HasParseError is true, TryParseConfig already emitted the + // detailed error to Console.Error. Only log a generic message to avoid + // duplicate output (stderr + stdout). + if (!RuntimeConfigLoader.HasParseError) + { + _logger.LogError("Failed to parse the config file: {runtimeConfigFile}.", runtimeConfigFile); + } + + RuntimeConfigLoader.HasParseError = false; return false; } else @@ -2645,25 +2651,17 @@ public static bool IsConfigValid(ValidateOptions options, FileSystemRuntimeConfi RuntimeConfigProvider runtimeConfigProvider = new(loader); - // Suppress Console.Error during config loading so that parse errors - // are routed through the CLI's structured logger instead of raw stderr. - TextWriter originalError = Console.Error; - Console.SetError(TextWriter.Null); - bool configLoaded; - try - { - configLoaded = runtimeConfigProvider.TryGetConfig(out RuntimeConfig? _); - } - finally + if (!runtimeConfigProvider.TryGetConfig(out RuntimeConfig? _)) { - Console.SetError(originalError); - } + // When HasParseError is true, TryParseConfig already emitted the + // detailed error to Console.Error. Only log a generic message to avoid + // duplicate output (stderr + stdout). + if (!RuntimeConfigLoader.HasParseError) + { + _logger.LogError("Failed to parse the config file."); + } - if (!configLoaded) - { - string? parseError = RuntimeConfigLoader.LastParseError; - RuntimeConfigLoader.LastParseError = null; - _logger.LogError("{parseError}", parseError ?? "Failed to parse the config file."); + RuntimeConfigLoader.HasParseError = false; return false; } diff --git a/src/Config/RuntimeConfigLoader.cs b/src/Config/RuntimeConfigLoader.cs index 68d852b50a..6b9e7e1814 100644 --- a/src/Config/RuntimeConfigLoader.cs +++ b/src/Config/RuntimeConfigLoader.cs @@ -39,10 +39,10 @@ public abstract class RuntimeConfigLoader public bool IsNewConfigValidated; /// - /// Stores the last config parse error message for callers that need - /// to retrieve and log it through their own logging infrastructure. + /// Flag indicating that TryParseConfig already emitted a parse error + /// to Console.Error, so callers can skip their own generic message. /// - public static string? LastParseError { get; set; } + public static bool HasParseError { get; set; } public RuntimeConfigLoader(HotReloadEventHandler? handler = null, string? connectionString = null) { @@ -269,19 +269,21 @@ public static bool TryParseConfig(string json, ex is JsonException || ex is DataApiBuilderException) { - // Store the parse error so callers (e.g. CLI) can retrieve and - // log it through their own structured logging infrastructure. - LastParseError = ex is DataApiBuilderException + string parseError = ex is DataApiBuilderException ? ex.Message : $"Deserialization of the configuration file failed. {ex.Message}"; + HasParseError = true; + if (logger is not null) { - logger.LogError(exception: ex, message: "{ParseError}", LastParseError); + logger.LogError(exception: ex, message: "{ParseError}", parseError); } else { - Console.Error.WriteLine(LastParseError); + // Fallback for callers that don't provide a logger (e.g. engine startup). + // CLI callers check HasParseError and skip their own generic message. + Console.Error.WriteLine(parseError); } config = null; diff --git a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs index eeaf43c282..f864a7aec5 100644 --- a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs +++ b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs @@ -92,15 +92,21 @@ public async Task FailLoadMultiDataSourceConfigDuplicateEntities(string configPa FileSystemRuntimeConfigLoader loader = new(fs); - StringWriter sw = new(); - Console.SetError(sw); + RuntimeConfigLoader.HasParseError = false; + StringWriter errorWriter = new(); + TextWriter originalError = Console.Error; + Console.SetError(errorWriter); loader.TryLoadConfig("dab-config.json", out RuntimeConfig _); - string error = sw.ToString(); - Assert.IsTrue(error.Contains("An item with the same key has already been added."), + Console.SetError(originalError); + string errorOutput = errorWriter.ToString(); + + Assert.IsTrue(RuntimeConfigLoader.HasParseError, + "HasParseError should be true when config parsing fails."); + Assert.IsTrue(errorOutput.Contains("An item with the same key has already been added."), "Error should contain the specific validation message."); - Assert.IsFalse(error.Contains("Stack Trace"), + Assert.IsFalse(errorOutput.Contains("StackTrace"), "Stack trace should not be present in error output."); } From 72ec3c176451ba53aac98314c6dadc997add7e15 Mon Sep 17 00:00:00 2001 From: aaron burtle Date: Tue, 24 Mar 2026 23:22:01 -0700 Subject: [PATCH 03/10] use out param for parallel callers operations --- src/Cli.Tests/ValidateConfigTests.cs | 19 +++----- src/Cli/ConfigGenerator.cs | 10 ++--- src/Config/FileSystemRuntimeConfigLoader.cs | 17 +++++++- src/Config/RuntimeConfigLoader.cs | 43 +++++++++---------- .../Configurations/RuntimeConfigProvider.cs | 3 +- .../Configuration/ConfigurationTests.cs | 9 ++-- .../Configuration/RuntimeConfigLoaderTests.cs | 16 +------ ...untimeConfigLoaderJsonDeserializerTests.cs | 15 ++----- 8 files changed, 57 insertions(+), 75 deletions(-) diff --git a/src/Cli.Tests/ValidateConfigTests.cs b/src/Cli.Tests/ValidateConfigTests.cs index 08e59a4e5a..1a726de3ed 100644 --- a/src/Cli.Tests/ValidateConfigTests.cs +++ b/src/Cli.Tests/ValidateConfigTests.cs @@ -211,24 +211,15 @@ public void TestValidateConfigWithNoEntitiesProducesCleanError() { string configWithoutEntities = $"{{{SAMPLE_SCHEMA_DATA_SOURCE},{RUNTIME_SECTION}}}"; - // Capture stderr to verify TryParseConfig produces a clean error without stack traces. - RuntimeConfigLoader.HasParseError = false; - StringWriter errorWriter = new(); - TextWriter originalError = Console.Error; - Console.SetError(errorWriter); - - bool parsed = RuntimeConfigLoader.TryParseConfig(configWithoutEntities, out _); - - Console.SetError(originalError); - string errorOutput = errorWriter.ToString(); + // Verify TryParseConfig produces a clean error without stack traces. + bool parsed = RuntimeConfigLoader.TryParseConfig(configWithoutEntities, out _, out string? parseError); Assert.IsFalse(parsed, "Config with no entities should fail to parse."); - Assert.IsTrue(RuntimeConfigLoader.HasParseError, - "HasParseError should be true when config parsing fails."); - StringAssert.Contains(errorOutput, + Assert.IsNotNull(parseError, "parseError should be set when config parsing fails."); + StringAssert.Contains(parseError, "Configuration file should contain either at least the entities or autoentities property", "Parse error should contain the clean validation message."); - Assert.IsFalse(errorOutput.Contains("StackTrace"), + Assert.IsFalse(parseError.Contains("StackTrace"), "Stack trace should not be present in parse error."); // Verify IsConfigValid also returns false cleanly (no exception thrown). diff --git a/src/Cli/ConfigGenerator.cs b/src/Cli/ConfigGenerator.cs index 4f1e2d047b..7b24dba3bc 100644 --- a/src/Cli/ConfigGenerator.cs +++ b/src/Cli/ConfigGenerator.cs @@ -2564,15 +2564,14 @@ public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRun // Replaces all the environment variables while deserializing when starting DAB. if (!loader.TryLoadKnownConfig(out RuntimeConfig? deserializedRuntimeConfig, replaceEnvVar: true)) { - // When HasParseError is true, TryParseConfig already emitted the + // When ParseErrorEmitted is true, TryParseConfig already emitted the // detailed error to Console.Error. Only log a generic message to avoid // duplicate output (stderr + stdout). - if (!RuntimeConfigLoader.HasParseError) + if (!loader.ParseErrorEmitted) { _logger.LogError("Failed to parse the config file: {runtimeConfigFile}.", runtimeConfigFile); } - RuntimeConfigLoader.HasParseError = false; return false; } else @@ -2653,15 +2652,14 @@ public static bool IsConfigValid(ValidateOptions options, FileSystemRuntimeConfi if (!runtimeConfigProvider.TryGetConfig(out RuntimeConfig? _)) { - // When HasParseError is true, TryParseConfig already emitted the + // When ParseErrorEmitted is true, TryParseConfig already emitted the // detailed error to Console.Error. Only log a generic message to avoid // duplicate output (stderr + stdout). - if (!RuntimeConfigLoader.HasParseError) + if (!loader.ParseErrorEmitted) { _logger.LogError("Failed to parse the config file."); } - RuntimeConfigLoader.HasParseError = false; return false; } diff --git a/src/Config/FileSystemRuntimeConfigLoader.cs b/src/Config/FileSystemRuntimeConfigLoader.cs index 7b888a82bf..33ec469118 100644 --- a/src/Config/FileSystemRuntimeConfigLoader.cs +++ b/src/Config/FileSystemRuntimeConfigLoader.cs @@ -85,6 +85,12 @@ public class FileSystemRuntimeConfigLoader : RuntimeConfigLoader, IDisposable /// public string ConfigFilePath { get; internal set; } + /// + /// Indicates whether the most recent TryLoadConfig call encountered a parse error + /// that was already emitted to Console.Error. + /// + public bool ParseErrorEmitted { get; private set; } + public FileSystemRuntimeConfigLoader( IFileSystem fileSystem, HotReloadEventHandler? handler = null, @@ -227,6 +233,7 @@ public bool TryLoadConfig( bool? isDevMode = null, DeserializationVariableReplacementSettings? replacementSettings = null) { + ParseErrorEmitted = false; if (_fileSystem.File.Exists(path)) { SendLogToBufferOrLogger(LogLevel.Information, $"Loading config file from {_fileSystem.Path.GetFullPath(path)}."); @@ -263,11 +270,12 @@ public bool TryLoadConfig( // Use default replacement settings if none provided replacementSettings ??= new DeserializationVariableReplacementSettings(); + string? parseError = null; if (!string.IsNullOrEmpty(json) && TryParseConfig( json, out RuntimeConfig, + out parseError, replacementSettings, - logger: null, connectionString: _connectionString)) { if (TrySetupConfigFileWatcher()) @@ -303,6 +311,13 @@ public bool TryLoadConfig( RuntimeConfig = LastValidRuntimeConfig; } + ParseErrorEmitted = parseError is not null; + + if (parseError is not null) + { + Console.Error.WriteLine(parseError); + } + config = null; return false; } diff --git a/src/Config/RuntimeConfigLoader.cs b/src/Config/RuntimeConfigLoader.cs index 6b9e7e1814..dc148d2ca0 100644 --- a/src/Config/RuntimeConfigLoader.cs +++ b/src/Config/RuntimeConfigLoader.cs @@ -38,12 +38,6 @@ public abstract class RuntimeConfigLoader public bool IsNewConfigValidated; - /// - /// Flag indicating that TryParseConfig already emitted a parse error - /// to Console.Error, so callers can skip their own generic message. - /// - public static bool HasParseError { get; set; } - public RuntimeConfigLoader(HotReloadEventHandler? handler = null, string? connectionString = null) { _changeToken = new DabChangeToken(); @@ -185,16 +179,34 @@ protected void SignalConfigChanged(string message = "") /// /// JSON that represents the config file. /// The parsed config, or null if it parsed unsuccessfully. + /// A clean error message when parsing fails, or null on success. + /// Settings for variable replacement during deserialization. If null, no variable replacement will be performed. + /// connectionString to add to config if specified + /// True if the config was parsed, otherwise false. + public static bool TryParseConfig(string json, + [NotNullWhen(true)] out RuntimeConfig? config, + DeserializationVariableReplacementSettings? replacementSettings = null, + string? connectionString = null) + { + return TryParseConfig(json, out config, out _, replacementSettings, connectionString); + } + + /// + /// Parses a JSON string into a RuntimeConfig object for single database scenario. + /// + /// JSON that represents the config file. + /// The parsed config, or null if it parsed unsuccessfully. + /// A clean error message when parsing fails, or null on success. /// Settings for variable replacement during deserialization. If null, no variable replacement will be performed. - /// logger to log messages /// connectionString to add to config if specified /// True if the config was parsed, otherwise false. public static bool TryParseConfig(string json, [NotNullWhen(true)] out RuntimeConfig? config, + out string? parseError, DeserializationVariableReplacementSettings? replacementSettings = null, - ILogger? logger = null, string? connectionString = null) { + parseError = null; // First pass: extract AzureKeyVault options if AKV replacement is requested if (replacementSettings?.DoReplaceAkvVar is true) { @@ -269,23 +281,10 @@ public static bool TryParseConfig(string json, ex is JsonException || ex is DataApiBuilderException) { - string parseError = ex is DataApiBuilderException + parseError = ex is DataApiBuilderException ? ex.Message : $"Deserialization of the configuration file failed. {ex.Message}"; - HasParseError = true; - - if (logger is not null) - { - logger.LogError(exception: ex, message: "{ParseError}", parseError); - } - else - { - // Fallback for callers that don't provide a logger (e.g. engine startup). - // CLI callers check HasParseError and skip their own generic message. - Console.Error.WriteLine(parseError); - } - config = null; return false; } diff --git a/src/Core/Configurations/RuntimeConfigProvider.cs b/src/Core/Configurations/RuntimeConfigProvider.cs index 70327326ba..c3b33ac20d 100644 --- a/src/Core/Configurations/RuntimeConfigProvider.cs +++ b/src/Core/Configurations/RuntimeConfigProvider.cs @@ -188,6 +188,7 @@ public async Task Initialize( if (RuntimeConfigLoader.TryParseConfig( configuration, out RuntimeConfig? runtimeConfig, + out string? parseError, replacementSettings: null)) { _configLoader.RuntimeConfig = runtimeConfig; @@ -269,7 +270,7 @@ public async Task Initialize( IsLateConfigured = true; - if (RuntimeConfigLoader.TryParseConfig(jsonConfig, out RuntimeConfig? runtimeConfig, replacementSettings)) + if (RuntimeConfigLoader.TryParseConfig(jsonConfig, out RuntimeConfig? runtimeConfig, out string? _, replacementSettings)) { _configLoader.RuntimeConfig = runtimeConfig.DataSource.DatabaseType switch { diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 6dac0266bd..82c79e0a0b 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -2521,8 +2521,7 @@ public async Task TestSPRestDefaultsForManuallyConstructedConfigs( configJson, out RuntimeConfig deserializedConfig, replacementSettings: new(), - logger: null, - GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL)); + connectionString: GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL)); string configFileName = "custom-config.json"; File.WriteAllText(configFileName, deserializedConfig.ToJson()); string[] args = new[] @@ -2609,8 +2608,7 @@ public async Task SanityTestForRestAndGQLRequestsWithoutMultipleMutationFeatureF configJson, out RuntimeConfig deserializedConfig, replacementSettings: new(), - logger: null, - GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL))); + connectionString: GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL))); string configFileName = "custom-config.json"; File.WriteAllText(configFileName, deserializedConfig.ToJson()); string[] args = new[] @@ -3640,8 +3638,7 @@ public async Task ValidateStrictModeAsDefaultForRestRequestBody(bool includeExtr configJson, out RuntimeConfig deserializedConfig, replacementSettings: new(), - logger: null, - GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL)); + connectionString: GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL)); const string CUSTOM_CONFIG = "custom-config.json"; File.WriteAllText(CUSTOM_CONFIG, deserializedConfig.ToJson()); string[] args = new[] diff --git a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs index f864a7aec5..401ebe4a09 100644 --- a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs +++ b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs @@ -92,22 +92,10 @@ public async Task FailLoadMultiDataSourceConfigDuplicateEntities(string configPa FileSystemRuntimeConfigLoader loader = new(fs); - RuntimeConfigLoader.HasParseError = false; - StringWriter errorWriter = new(); - TextWriter originalError = Console.Error; - Console.SetError(errorWriter); - loader.TryLoadConfig("dab-config.json", out RuntimeConfig _); - Console.SetError(originalError); - string errorOutput = errorWriter.ToString(); - - Assert.IsTrue(RuntimeConfigLoader.HasParseError, - "HasParseError should be true when config parsing fails."); - Assert.IsTrue(errorOutput.Contains("An item with the same key has already been added."), - "Error should contain the specific validation message."); - Assert.IsFalse(errorOutput.Contains("StackTrace"), - "Stack trace should not be present in error output."); + Assert.IsTrue(loader.ParseErrorEmitted, + "ParseErrorEmitted should be true when config parsing fails."); } /// diff --git a/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs b/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs index 347bf5e328..dcd78180ff 100644 --- a/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs +++ b/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs @@ -325,28 +325,21 @@ public void TestTelemetryApplicationInsightsEnabledShouldError(string configValu ""entities"": { } }"; - // Arrange - Mock mockLogger = new(); - // Act bool isParsed = RuntimeConfigLoader.TryParseConfig( configJson, out RuntimeConfig runtimeConfig, + out string? parseError, replacementSettings: new DeserializationVariableReplacementSettings( azureKeyVaultOptions: null, doReplaceEnvVar: true, - doReplaceAkvVar: false), - logger: mockLogger.Object); + doReplaceAkvVar: false)); // Assert Assert.IsFalse(isParsed); Assert.IsNull(runtimeConfig); - - Assert.AreEqual(1, mockLogger.Invocations.Count, "Should raise 1 exception"); - Assert.AreEqual(5, mockLogger.Invocations[0].Arguments.Count, "Log should have 4 arguments"); - var ConfigException = mockLogger.Invocations[0].Arguments[3] as JsonException; - Assert.IsInstanceOfType(ConfigException, typeof(JsonException), "Should have raised a Json Exception"); - Assert.AreEqual(message, ConfigException.Message); + Assert.IsNotNull(parseError, "parseError should be set when parsing fails."); + StringAssert.Contains(parseError, message, "parseError should contain the expected error message."); } /// From 791c935099998e35708b90e271d9f97927fa3bdd Mon Sep 17 00:00:00 2001 From: aaron burtle Date: Tue, 24 Mar 2026 23:47:39 -0700 Subject: [PATCH 04/10] format issues --- src/Cli.Tests/ValidateConfigTests.cs | 1 - src/Config/RuntimeConfigLoader.cs | 1 - src/Core/Configurations/RuntimeConfigProvider.cs | 2 +- .../UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs | 4 +--- 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Cli.Tests/ValidateConfigTests.cs b/src/Cli.Tests/ValidateConfigTests.cs index 1a726de3ed..0383d9072d 100644 --- a/src/Cli.Tests/ValidateConfigTests.cs +++ b/src/Cli.Tests/ValidateConfigTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System.IO; using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; using Serilog; diff --git a/src/Config/RuntimeConfigLoader.cs b/src/Config/RuntimeConfigLoader.cs index dc148d2ca0..83b7b3969e 100644 --- a/src/Config/RuntimeConfigLoader.cs +++ b/src/Config/RuntimeConfigLoader.cs @@ -13,7 +13,6 @@ using Azure.DataApiBuilder.Product; using Azure.DataApiBuilder.Service.Exceptions; using Microsoft.Data.SqlClient; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; using Npgsql; using static Azure.DataApiBuilder.Config.DabConfigEvents; diff --git a/src/Core/Configurations/RuntimeConfigProvider.cs b/src/Core/Configurations/RuntimeConfigProvider.cs index c3b33ac20d..3375f30074 100644 --- a/src/Core/Configurations/RuntimeConfigProvider.cs +++ b/src/Core/Configurations/RuntimeConfigProvider.cs @@ -188,7 +188,7 @@ public async Task Initialize( if (RuntimeConfigLoader.TryParseConfig( configuration, out RuntimeConfig? runtimeConfig, - out string? parseError, + out _, replacementSettings: null)) { _configLoader.RuntimeConfig = runtimeConfig; diff --git a/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs b/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs index dcd78180ff..a80de4fd64 100644 --- a/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs +++ b/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs @@ -14,9 +14,7 @@ using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Service.Exceptions; using Microsoft.Data.SqlClient; -using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; -using Moq; namespace Azure.DataApiBuilder.Service.Tests.UnitTests { @@ -329,7 +327,7 @@ public void TestTelemetryApplicationInsightsEnabledShouldError(string configValu bool isParsed = RuntimeConfigLoader.TryParseConfig( configJson, out RuntimeConfig runtimeConfig, - out string? parseError, + out string parseError, replacementSettings: new DeserializationVariableReplacementSettings( azureKeyVaultOptions: null, doReplaceEnvVar: true, From b72e7f4d85d6a8cb014fa43c5d8de255c7d6844b Mon Sep 17 00:00:00 2001 From: aaron burtle Date: Wed, 25 Mar 2026 00:57:58 -0700 Subject: [PATCH 05/10] change bool name --- src/Cli/ConfigGenerator.cs | 8 ++++---- src/Config/FileSystemRuntimeConfigLoader.cs | 6 +++--- .../Configuration/RuntimeConfigLoaderTests.cs | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Cli/ConfigGenerator.cs b/src/Cli/ConfigGenerator.cs index 938acc9e45..faada6c1eb 100644 --- a/src/Cli/ConfigGenerator.cs +++ b/src/Cli/ConfigGenerator.cs @@ -2564,10 +2564,10 @@ public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRun // Replaces all the environment variables while deserializing when starting DAB. if (!loader.TryLoadKnownConfig(out RuntimeConfig? deserializedRuntimeConfig, replaceEnvVar: true)) { - // When ParseErrorEmitted is true, TryParseConfig already emitted the + // When IsParseErrorEmitted is true, TryParseConfig already emitted the // detailed error to Console.Error. Only log a generic message to avoid // duplicate output (stderr + stdout). - if (!loader.ParseErrorEmitted) + if (!loader.IsParseErrorEmitted) { _logger.LogError("Failed to parse the config file: {runtimeConfigFile}.", runtimeConfigFile); } @@ -2652,10 +2652,10 @@ public static bool IsConfigValid(ValidateOptions options, FileSystemRuntimeConfi if (!runtimeConfigProvider.TryGetConfig(out RuntimeConfig? _)) { - // When ParseErrorEmitted is true, TryParseConfig already emitted the + // When IsParseErrorEmitted is true, TryParseConfig already emitted the // detailed error to Console.Error. Only log a generic message to avoid // duplicate output (stderr + stdout). - if (!loader.ParseErrorEmitted) + if (!loader.IsParseErrorEmitted) { _logger.LogError("Failed to parse the config file."); } diff --git a/src/Config/FileSystemRuntimeConfigLoader.cs b/src/Config/FileSystemRuntimeConfigLoader.cs index 33ec469118..7ad92e0cd2 100644 --- a/src/Config/FileSystemRuntimeConfigLoader.cs +++ b/src/Config/FileSystemRuntimeConfigLoader.cs @@ -89,7 +89,7 @@ public class FileSystemRuntimeConfigLoader : RuntimeConfigLoader, IDisposable /// Indicates whether the most recent TryLoadConfig call encountered a parse error /// that was already emitted to Console.Error. /// - public bool ParseErrorEmitted { get; private set; } + public bool IsParseErrorEmitted { get; private set; } public FileSystemRuntimeConfigLoader( IFileSystem fileSystem, @@ -233,7 +233,7 @@ public bool TryLoadConfig( bool? isDevMode = null, DeserializationVariableReplacementSettings? replacementSettings = null) { - ParseErrorEmitted = false; + IsParseErrorEmitted = false; if (_fileSystem.File.Exists(path)) { SendLogToBufferOrLogger(LogLevel.Information, $"Loading config file from {_fileSystem.Path.GetFullPath(path)}."); @@ -311,7 +311,7 @@ public bool TryLoadConfig( RuntimeConfig = LastValidRuntimeConfig; } - ParseErrorEmitted = parseError is not null; + IsParseErrorEmitted = parseError is not null; if (parseError is not null) { diff --git a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs index 401ebe4a09..c4a010a791 100644 --- a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs +++ b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs @@ -94,8 +94,8 @@ public async Task FailLoadMultiDataSourceConfigDuplicateEntities(string configPa loader.TryLoadConfig("dab-config.json", out RuntimeConfig _); - Assert.IsTrue(loader.ParseErrorEmitted, - "ParseErrorEmitted should be true when config parsing fails."); + Assert.IsTrue(loader.IsParseErrorEmitted, + "IsParseErrorEmitted should be true when config parsing fails."); } /// From 2b953b40c7e3a77b24417b61a5dd3d6995145777 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 08:09:05 +0000 Subject: [PATCH 06/10] Fix LogError/comment issues found in PR review Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com> Agent-Logs-Url: https://github.com/Azure/data-api-builder/sessions/5aec902b-92d0-4b71-ba4b-32ca2c049908 --- src/Cli/Commands/ValidateOptions.cs | 2 +- src/Cli/ConfigGenerator.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Cli/Commands/ValidateOptions.cs b/src/Cli/Commands/ValidateOptions.cs index 6cef26ba5b..e3d54666c4 100644 --- a/src/Cli/Commands/ValidateOptions.cs +++ b/src/Cli/Commands/ValidateOptions.cs @@ -38,7 +38,7 @@ public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSy } else { - logger.LogInformation("Config is invalid."); + logger.LogError("Config is invalid."); } return isValidConfig ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR; diff --git a/src/Cli/ConfigGenerator.cs b/src/Cli/ConfigGenerator.cs index faada6c1eb..38d20383bd 100644 --- a/src/Cli/ConfigGenerator.cs +++ b/src/Cli/ConfigGenerator.cs @@ -2564,7 +2564,7 @@ public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRun // Replaces all the environment variables while deserializing when starting DAB. if (!loader.TryLoadKnownConfig(out RuntimeConfig? deserializedRuntimeConfig, replaceEnvVar: true)) { - // When IsParseErrorEmitted is true, TryParseConfig already emitted the + // When IsParseErrorEmitted is true, TryLoadConfig already emitted the // detailed error to Console.Error. Only log a generic message to avoid // duplicate output (stderr + stdout). if (!loader.IsParseErrorEmitted) @@ -2652,7 +2652,7 @@ public static bool IsConfigValid(ValidateOptions options, FileSystemRuntimeConfi if (!runtimeConfigProvider.TryGetConfig(out RuntimeConfig? _)) { - // When IsParseErrorEmitted is true, TryParseConfig already emitted the + // When IsParseErrorEmitted is true, TryLoadConfig already emitted the // detailed error to Console.Error. Only log a generic message to avoid // duplicate output (stderr + stdout). if (!loader.IsParseErrorEmitted) From ff35e28f3dd8b1a8d34bf76a2f70d41c49c8e920 Mon Sep 17 00:00:00 2001 From: aaron burtle Date: Thu, 2 Apr 2026 23:37:48 -0700 Subject: [PATCH 07/10] address comments --- src/Cli.Tests/EndToEndTests.cs | 1 + .../Configurations/RuntimeConfigProvider.cs | 2 +- .../MissingRoleNotFoundTests.cs | 98 +++++++++++++++++++ 3 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs diff --git a/src/Cli.Tests/EndToEndTests.cs b/src/Cli.Tests/EndToEndTests.cs index 9ab7085bbc..9017e69cc5 100644 --- a/src/Cli.Tests/EndToEndTests.cs +++ b/src/Cli.Tests/EndToEndTests.cs @@ -1093,6 +1093,7 @@ public async Task TestExitOfRuntimeEngineWithInvalidConfig( { output = await process.StandardError.ReadLineAsync(); Assert.IsNotNull(output); + StringAssert.Contains(output, $"Deserialization of the configuration file failed.", StringComparison.Ordinal); output = await process.StandardOutput.ReadLineAsync(); Assert.IsNotNull(output); diff --git a/src/Core/Configurations/RuntimeConfigProvider.cs b/src/Core/Configurations/RuntimeConfigProvider.cs index 3375f30074..0fce915edf 100644 --- a/src/Core/Configurations/RuntimeConfigProvider.cs +++ b/src/Core/Configurations/RuntimeConfigProvider.cs @@ -270,7 +270,7 @@ public async Task Initialize( IsLateConfigured = true; - if (RuntimeConfigLoader.TryParseConfig(jsonConfig, out RuntimeConfig? runtimeConfig, out string? _, replacementSettings)) + if (RuntimeConfigLoader.TryParseConfig(jsonConfig, out RuntimeConfig? runtimeConfig, out _, replacementSettings)) { _configLoader.RuntimeConfig = runtimeConfig.DataSource.DatabaseType switch { diff --git a/src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs b/src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs new file mode 100644 index 0000000000..dd9be13c35 --- /dev/null +++ b/src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.IO; +using System.IO.Abstractions; +using System.Net; +using System.Net.Http; +using System.Text.Json; +using System.Threading.Tasks; +using Azure.DataApiBuilder.Config; +using Azure.DataApiBuilder.Config.ObjectModel; +using Microsoft.AspNetCore.TestHost; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Azure.DataApiBuilder.Service.Tests.OpenApiIntegration +{ + /// + /// Tests validating that requesting an OpenAPI document for a role not present + /// in the configuration returns a 404 ProblemDetails response with a descriptive message. + /// + [TestCategory(TestCategory.MSSQL)] + [TestClass] + public class MissingRoleNotFoundTests + { + private const string CONFIG_FILE = "missing-role-notfound-config.MsSql.json"; + private const string DB_ENV = TestCategory.MSSQL; + + /// + /// Validates that a request for /api/openapi/{role} returns a 404 + /// ProblemDetails response containing the role name in the detail message + /// when the role is not present in the configuration or contains invalid characters. + /// + [DataTestMethod] + [DataRow("nonexistentrole", DisplayName = "Missing role returns 404 ProblemDetails")] + [DataRow("foo/bar", DisplayName = "Role with path separator returns 404 ProblemDetails")] + public async Task MissingRole_Returns404ProblemDetailsWithMessage(string roleName) + { + TestHelper.SetupDatabaseEnvironment(DB_ENV); + FileSystem fileSystem = new(); + FileSystemRuntimeConfigLoader loader = new(fileSystem); + loader.TryLoadKnownConfig(out RuntimeConfig config); + + Entity entity = new( + Source: new("books", EntitySourceType.Table, null, null), + Fields: null, + GraphQL: new(null, null, false), + Rest: new(EntityRestOptions.DEFAULT_SUPPORTED_VERBS), + Permissions: OpenApiTestBootstrap.CreateBasicPermissions(), + Mappings: null, + Relationships: null); + + RuntimeConfig testConfig = config with + { + Runtime = config.Runtime with + { + Host = config.Runtime?.Host with { Mode = HostMode.Development } + }, + Entities = new RuntimeEntities(new Dictionary { { "book", entity } }) + }; + + File.WriteAllText(CONFIG_FILE, testConfig.ToJson()); + string[] args = new[] { $"--ConfigFileName={CONFIG_FILE}" }; + + try + { + using TestServer server = new(Program.CreateWebHostBuilder(args)); + using HttpClient client = server.CreateClient(); + + HttpResponseMessage response = await client.GetAsync($"/api/openapi/{Uri.EscapeDataString(roleName)}"); + + Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode, "Expected 404 for a role not in the configuration."); + + string responseBody = await response.Content.ReadAsStringAsync(); + using JsonDocument doc = JsonDocument.Parse(responseBody); + JsonElement root = doc.RootElement; + + Assert.AreEqual("Not Found", root.GetProperty("title").GetString(), "ProblemDetails title should be 'Not Found'."); + Assert.AreEqual(404, root.GetProperty("status").GetInt32(), "ProblemDetails status should be 404."); + Assert.IsTrue(root.TryGetProperty("type", out _), "ProblemDetails should contain a 'type' field."); + Assert.IsTrue(root.TryGetProperty("traceId", out _), "ProblemDetails should contain a 'traceId' field."); + + string detail = root.GetProperty("detail").GetString(); + Assert.IsTrue(detail.Contains(roleName), $"Detail should contain the role name '{roleName}'. Actual: {detail}"); + } + finally + { + if (File.Exists(CONFIG_FILE)) + { + File.Delete(CONFIG_FILE); + } + } + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + } +} From ff3cdff0494131266fb9a3dbcebd98ce5b5e45ca Mon Sep 17 00:00:00 2001 From: aaron burtle Date: Fri, 3 Apr 2026 01:42:11 -0700 Subject: [PATCH 08/10] fix accidental merging --- .../MissingRoleNotFoundTests.cs | 98 ------------------- 1 file changed, 98 deletions(-) delete mode 100644 src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs diff --git a/src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs b/src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs deleted file mode 100644 index dd9be13c35..0000000000 --- a/src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs +++ /dev/null @@ -1,98 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System; -using System.Collections.Generic; -using System.IO; -using System.IO.Abstractions; -using System.Net; -using System.Net.Http; -using System.Text.Json; -using System.Threading.Tasks; -using Azure.DataApiBuilder.Config; -using Azure.DataApiBuilder.Config.ObjectModel; -using Microsoft.AspNetCore.TestHost; -using Microsoft.VisualStudio.TestTools.UnitTesting; - -namespace Azure.DataApiBuilder.Service.Tests.OpenApiIntegration -{ - /// - /// Tests validating that requesting an OpenAPI document for a role not present - /// in the configuration returns a 404 ProblemDetails response with a descriptive message. - /// - [TestCategory(TestCategory.MSSQL)] - [TestClass] - public class MissingRoleNotFoundTests - { - private const string CONFIG_FILE = "missing-role-notfound-config.MsSql.json"; - private const string DB_ENV = TestCategory.MSSQL; - - /// - /// Validates that a request for /api/openapi/{role} returns a 404 - /// ProblemDetails response containing the role name in the detail message - /// when the role is not present in the configuration or contains invalid characters. - /// - [DataTestMethod] - [DataRow("nonexistentrole", DisplayName = "Missing role returns 404 ProblemDetails")] - [DataRow("foo/bar", DisplayName = "Role with path separator returns 404 ProblemDetails")] - public async Task MissingRole_Returns404ProblemDetailsWithMessage(string roleName) - { - TestHelper.SetupDatabaseEnvironment(DB_ENV); - FileSystem fileSystem = new(); - FileSystemRuntimeConfigLoader loader = new(fileSystem); - loader.TryLoadKnownConfig(out RuntimeConfig config); - - Entity entity = new( - Source: new("books", EntitySourceType.Table, null, null), - Fields: null, - GraphQL: new(null, null, false), - Rest: new(EntityRestOptions.DEFAULT_SUPPORTED_VERBS), - Permissions: OpenApiTestBootstrap.CreateBasicPermissions(), - Mappings: null, - Relationships: null); - - RuntimeConfig testConfig = config with - { - Runtime = config.Runtime with - { - Host = config.Runtime?.Host with { Mode = HostMode.Development } - }, - Entities = new RuntimeEntities(new Dictionary { { "book", entity } }) - }; - - File.WriteAllText(CONFIG_FILE, testConfig.ToJson()); - string[] args = new[] { $"--ConfigFileName={CONFIG_FILE}" }; - - try - { - using TestServer server = new(Program.CreateWebHostBuilder(args)); - using HttpClient client = server.CreateClient(); - - HttpResponseMessage response = await client.GetAsync($"/api/openapi/{Uri.EscapeDataString(roleName)}"); - - Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode, "Expected 404 for a role not in the configuration."); - - string responseBody = await response.Content.ReadAsStringAsync(); - using JsonDocument doc = JsonDocument.Parse(responseBody); - JsonElement root = doc.RootElement; - - Assert.AreEqual("Not Found", root.GetProperty("title").GetString(), "ProblemDetails title should be 'Not Found'."); - Assert.AreEqual(404, root.GetProperty("status").GetInt32(), "ProblemDetails status should be 404."); - Assert.IsTrue(root.TryGetProperty("type", out _), "ProblemDetails should contain a 'type' field."); - Assert.IsTrue(root.TryGetProperty("traceId", out _), "ProblemDetails should contain a 'traceId' field."); - - string detail = root.GetProperty("detail").GetString(); - Assert.IsTrue(detail.Contains(roleName), $"Detail should contain the role name '{roleName}'. Actual: {detail}"); - } - finally - { - if (File.Exists(CONFIG_FILE)) - { - File.Delete(CONFIG_FILE); - } - } - - TestHelper.UnsetAllDABEnvironmentVariables(); - } - } -} From ad4d38aba9ba74202bf17ef68fc88c034ea82d7f Mon Sep 17 00:00:00 2001 From: aaron burtle Date: Fri, 3 Apr 2026 14:10:43 -0700 Subject: [PATCH 09/10] cleanup assertions, logic for isParsed --- src/Config/FileSystemRuntimeConfigLoader.cs | 3 +-- src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Config/FileSystemRuntimeConfigLoader.cs b/src/Config/FileSystemRuntimeConfigLoader.cs index 7ad92e0cd2..933fa6369e 100644 --- a/src/Config/FileSystemRuntimeConfigLoader.cs +++ b/src/Config/FileSystemRuntimeConfigLoader.cs @@ -311,11 +311,10 @@ public bool TryLoadConfig( RuntimeConfig = LastValidRuntimeConfig; } - IsParseErrorEmitted = parseError is not null; - if (parseError is not null) { Console.Error.WriteLine(parseError); + IsParseErrorEmitted = true; } config = null; diff --git a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs index 7d3b238c01..d724753f97 100644 --- a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs +++ b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs @@ -93,10 +93,15 @@ public async Task FailLoadMultiDataSourceConfigDuplicateEntities(string configPa FileSystemRuntimeConfigLoader loader = new(fs); + StringWriter sw = new(); + Console.SetError(sw); + loader.TryLoadConfig("dab-config.json", out RuntimeConfig _); Assert.IsTrue(loader.IsParseErrorEmitted, "IsParseErrorEmitted should be true when config parsing fails."); + Assert.IsFalse(string.IsNullOrWhiteSpace(sw.ToString()), + "An error message should have been emitted to Console.Error."); } /// From d24d80ab20a2f9f9d2351d8a04ee0041a24bfda6 Mon Sep 17 00:00:00 2001 From: aaron burtle Date: Mon, 6 Apr 2026 08:14:51 -0700 Subject: [PATCH 10/10] dont delete file --- .../MissingRoleNotFoundTests.cs | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs diff --git a/src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs b/src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs new file mode 100644 index 0000000000..dd9be13c35 --- /dev/null +++ b/src/Service.Tests/OpenApiDocumentor/MissingRoleNotFoundTests.cs @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.IO; +using System.IO.Abstractions; +using System.Net; +using System.Net.Http; +using System.Text.Json; +using System.Threading.Tasks; +using Azure.DataApiBuilder.Config; +using Azure.DataApiBuilder.Config.ObjectModel; +using Microsoft.AspNetCore.TestHost; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Azure.DataApiBuilder.Service.Tests.OpenApiIntegration +{ + /// + /// Tests validating that requesting an OpenAPI document for a role not present + /// in the configuration returns a 404 ProblemDetails response with a descriptive message. + /// + [TestCategory(TestCategory.MSSQL)] + [TestClass] + public class MissingRoleNotFoundTests + { + private const string CONFIG_FILE = "missing-role-notfound-config.MsSql.json"; + private const string DB_ENV = TestCategory.MSSQL; + + /// + /// Validates that a request for /api/openapi/{role} returns a 404 + /// ProblemDetails response containing the role name in the detail message + /// when the role is not present in the configuration or contains invalid characters. + /// + [DataTestMethod] + [DataRow("nonexistentrole", DisplayName = "Missing role returns 404 ProblemDetails")] + [DataRow("foo/bar", DisplayName = "Role with path separator returns 404 ProblemDetails")] + public async Task MissingRole_Returns404ProblemDetailsWithMessage(string roleName) + { + TestHelper.SetupDatabaseEnvironment(DB_ENV); + FileSystem fileSystem = new(); + FileSystemRuntimeConfigLoader loader = new(fileSystem); + loader.TryLoadKnownConfig(out RuntimeConfig config); + + Entity entity = new( + Source: new("books", EntitySourceType.Table, null, null), + Fields: null, + GraphQL: new(null, null, false), + Rest: new(EntityRestOptions.DEFAULT_SUPPORTED_VERBS), + Permissions: OpenApiTestBootstrap.CreateBasicPermissions(), + Mappings: null, + Relationships: null); + + RuntimeConfig testConfig = config with + { + Runtime = config.Runtime with + { + Host = config.Runtime?.Host with { Mode = HostMode.Development } + }, + Entities = new RuntimeEntities(new Dictionary { { "book", entity } }) + }; + + File.WriteAllText(CONFIG_FILE, testConfig.ToJson()); + string[] args = new[] { $"--ConfigFileName={CONFIG_FILE}" }; + + try + { + using TestServer server = new(Program.CreateWebHostBuilder(args)); + using HttpClient client = server.CreateClient(); + + HttpResponseMessage response = await client.GetAsync($"/api/openapi/{Uri.EscapeDataString(roleName)}"); + + Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode, "Expected 404 for a role not in the configuration."); + + string responseBody = await response.Content.ReadAsStringAsync(); + using JsonDocument doc = JsonDocument.Parse(responseBody); + JsonElement root = doc.RootElement; + + Assert.AreEqual("Not Found", root.GetProperty("title").GetString(), "ProblemDetails title should be 'Not Found'."); + Assert.AreEqual(404, root.GetProperty("status").GetInt32(), "ProblemDetails status should be 404."); + Assert.IsTrue(root.TryGetProperty("type", out _), "ProblemDetails should contain a 'type' field."); + Assert.IsTrue(root.TryGetProperty("traceId", out _), "ProblemDetails should contain a 'traceId' field."); + + string detail = root.GetProperty("detail").GetString(); + Assert.IsTrue(detail.Contains(roleName), $"Detail should contain the role name '{roleName}'. Actual: {detail}"); + } + finally + { + if (File.Exists(CONFIG_FILE)) + { + File.Delete(CONFIG_FILE); + } + } + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + } +}