diff --git a/src/Cli.Tests/EndToEndTests.cs b/src/Cli.Tests/EndToEndTests.cs index 4462f52c4d..eb6e863c46 100644 --- a/src/Cli.Tests/EndToEndTests.cs +++ b/src/Cli.Tests/EndToEndTests.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using Azure.DataApiBuilder.Product; +using Cli.Constants; using Microsoft.Data.SqlClient; namespace Cli.Tests; @@ -21,9 +22,10 @@ public class EndToEndTests public void TestInitialize() { MockFileSystem fileSystem = FileSystemUtils.ProvisionMockFileSystem(); - fileSystem.AddFile( - TEST_SCHEMA_FILE, - new MockFileData("")); + // Mock GraphQL Schema File + fileSystem.AddFile(TEST_SCHEMA_FILE, new MockFileData("")); + // Empty runtime config file + fileSystem.AddFile("dab-config-empty.json", new MockFileData("")); _fileSystem = fileSystem; @@ -691,20 +693,48 @@ public void TestEngineStartUpWithVerboseAndLogLevelOptions(string logLevelOption } /// - /// Test to verify that `--help` and `--version` along with know command/option produce the exit code 0, - /// while unknown commands/options have exit code -1. + /// Validates that valid usage of verbs and associated options produce exit code 0 (CliReturnCode.SUCCESS). + /// Verifies that explicitly implemented verbs (add, update, init, start) and appropriately + /// supplied options produce exit code 0. + /// Verifies that non-explicitly implemented DAB CLI options `--help` and `--version` produce exit code 0. + /// init --config "dab-config.MsSql.json" --database-type mssql --connection-string "InvalidConnectionString" /// [DataTestMethod] - [DataRow(new string[] { "--version" }, 0, DisplayName = "Checking version should have exit code 0.")] - [DataRow(new string[] { "--help" }, 0, DisplayName = "Checking commands with help should have exit code 0.")] - [DataRow(new string[] { "add", "--help" }, 0, DisplayName = "Checking options with help should have exit code 0.")] - [DataRow(new string[] { "initialize" }, -1, DisplayName = "Invalid Command should have exit code -1.")] - [DataRow(new string[] { "init", "--database-name", "mssql" }, -1, DisplayName = "Invalid Options should have exit code -1.")] - [DataRow(new string[] { "init", "--database-type", "mssql", "-c", TEST_RUNTIME_CONFIG_FILE }, 0, - DisplayName = "Correct command with correct options should have exit code 0.")] - public void VerifyExitCodeForCli(string[] cliArguments, int expectedErrorCode) + [DataRow(new string[] { "--version" }, DisplayName = "Checking version.")] + [DataRow(new string[] { "--help" }, DisplayName = "Valid verbs with help.")] + [DataRow(new string[] { "add", "--help" }, DisplayName = "Valid options with help.")] + [DataRow(new string[] { "init", "--database-type", "mssql", "-c", TEST_RUNTIME_CONFIG_FILE }, DisplayName = "Valid verb with supported option.")] + public void ValidVerbsAndOptionsReturnZero(string[] cliArguments) { - Assert.AreEqual(expectedErrorCode, Program.Execute(cliArguments, _cliLogger!, _fileSystem!, _runtimeConfigLoader!)); + Assert.AreEqual(expected: CliReturnCode.SUCCESS, actual: Program.Execute(cliArguments, _cliLogger!, _fileSystem!, _runtimeConfigLoader!)); + } + + /// + /// Validates that invalid verbs and options produce exit code -1 (CliReturnCode.GENERAL_ERROR). + /// + /// cli verbs, options, and option values + [DataTestMethod] + [DataRow(new string[] { "--remove-telemetry" }, DisplayName = "Usage of non-existent verb remove-telemetry")] + [DataRow(new string[] { "--initialize" }, DisplayName = "Usage of invalid verb (longform of init not supported) initialize")] + [DataRow(new string[] { "init", "--database-name", "mssql" }, DisplayName = "Invalid init options database-name")] + public void InvalidVerbsAndOptionsReturnNonZeroExitCode(string[] cliArguments) + { + Assert.AreEqual(expected: CliReturnCode.GENERAL_ERROR, actual: Program.Execute(cliArguments, _cliLogger!, _fileSystem!, _runtimeConfigLoader!)); + } + + /// + /// Usage of valid verbs and options with values triggering exceptions should produce a non-zero exit code. + /// - File read/write issues when reading/writing to the config file. + /// - DAB engine failure. + /// + /// cli verbs, options, and option values + [DataTestMethod] + [DataRow(new string[] { "init", "--config", "dab-config-empty.json", "--database-type", "mssql", "--connection-string", "SampleValue" }, + DisplayName = "Config file value used already exists on the file system and results in init failure.")] + [DataRow(new string[] { "start", "--config", "dab-config-empty.json" }, DisplayName = "Config file value used is empty and engine startup fails")] + public void CliAndEngineFailuresReturnNonZeroExitCode(string[] cliArguments) + { + Assert.AreEqual(expected: CliReturnCode.GENERAL_ERROR, actual: Program.Execute(cliArguments, _cliLogger!, _fileSystem!, _runtimeConfigLoader!)); } /// diff --git a/src/Cli/Commands/AddOptions.cs b/src/Cli/Commands/AddOptions.cs index aa5d4c3667..edba7197ac 100644 --- a/src/Cli/Commands/AddOptions.cs +++ b/src/Cli/Commands/AddOptions.cs @@ -4,6 +4,7 @@ using System.IO.Abstractions; using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Product; +using Cli.Constants; using CommandLine; using Microsoft.Extensions.Logging; using static Cli.Utils; @@ -56,12 +57,12 @@ public AddOptions( [Option("permissions", Required = true, Separator = ':', HelpText = "Permissions required to access the source table or container.")] public IEnumerable Permissions { get; } - public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) + public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) { logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion()); if (!IsEntityProvided(Entity, logger, command: "add")) { - return; + return -1; } bool isSuccess = ConfigGenerator.TryAddEntityToConfigWithOptions(this, loader, fileSystem); @@ -74,6 +75,8 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS { logger.LogError("Could not add entity: {Entity} with source: {Source} and permissions: {permissions}.", Entity, Source, string.Join(SEPARATOR, Permissions)); } + + return isSuccess ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR; } } } diff --git a/src/Cli/Commands/AddTelemetryOptions.cs b/src/Cli/Commands/AddTelemetryOptions.cs index f51bf47a98..7cd3d38be9 100644 --- a/src/Cli/Commands/AddTelemetryOptions.cs +++ b/src/Cli/Commands/AddTelemetryOptions.cs @@ -5,6 +5,7 @@ using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Product; +using Cli.Constants; using CommandLine; using Microsoft.Extensions.Logging; using static Cli.Utils; @@ -32,7 +33,7 @@ public AddTelemetryOptions(string appInsightsConnString, CliBool appInsightsEnab [Option("app-insights-enabled", Default = CliBool.True, Required = false, HelpText = "(Default: true) Enable/Disable Application Insights")] public CliBool AppInsightsEnabled { get; } - public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) + public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) { logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion()); @@ -46,6 +47,8 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS { logger.LogError("Failed to add telemetry to the configuration file."); } + + return isSuccess ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR; } } } diff --git a/src/Cli/Commands/InitOptions.cs b/src/Cli/Commands/InitOptions.cs index 67b8ef5b62..846eee81a6 100644 --- a/src/Cli/Commands/InitOptions.cs +++ b/src/Cli/Commands/InitOptions.cs @@ -5,6 +5,7 @@ using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Product; +using Cli.Constants; using CommandLine; using Microsoft.Extensions.Logging; using static Cli.Utils; @@ -125,7 +126,7 @@ public InitOptions( [Option("graphql.multiple-create.enabled", Required = false, HelpText = "(Default: false) Enables multiple create operation for GraphQL. Supported values: true, false.")] public CliBool MultipleCreateOperationEnabled { get; } - public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) + public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) { logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion()); bool isSuccess = ConfigGenerator.TryGenerateConfig(this, loader, fileSystem); @@ -133,10 +134,12 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS { logger.LogInformation("Config file generated."); logger.LogInformation("SUGGESTION: Use 'dab add [entity-name] [options]' to add new entities in your config."); + return CliReturnCode.SUCCESS; } else { logger.LogError("Could not generate config file."); + return CliReturnCode.GENERAL_ERROR; } } } diff --git a/src/Cli/Commands/StartOptions.cs b/src/Cli/Commands/StartOptions.cs index 7997d7c3c5..c335c6bcc5 100644 --- a/src/Cli/Commands/StartOptions.cs +++ b/src/Cli/Commands/StartOptions.cs @@ -4,6 +4,7 @@ using System.IO.Abstractions; using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Product; +using Cli.Constants; using CommandLine; using Microsoft.Extensions.Logging; using static Cli.Utils; @@ -37,7 +38,7 @@ public StartOptions(bool verbose, LogLevel? logLevel, bool isHttpsRedirectionDis [Option("no-https-redirect", Required = false, HelpText = "Disables automatic https redirects.")] public bool IsHttpsRedirectionDisabled { get; } - public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) + public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) { logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion()); bool isSuccess = ConfigGenerator.TryStartEngineWithOptions(this, loader, fileSystem); @@ -46,6 +47,8 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS { logger.LogError("Failed to start the engine."); } + + return isSuccess ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR; } } } diff --git a/src/Cli/Commands/UpdateOptions.cs b/src/Cli/Commands/UpdateOptions.cs index 700f267a5f..edaf285484 100644 --- a/src/Cli/Commands/UpdateOptions.cs +++ b/src/Cli/Commands/UpdateOptions.cs @@ -4,6 +4,7 @@ using System.IO.Abstractions; using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Product; +using Cli.Constants; using CommandLine; using Microsoft.Extensions.Logging; using static Cli.Utils; @@ -96,12 +97,12 @@ public UpdateOptions( [Option('m', "map", Separator = ',', Required = false, HelpText = "Specify mappings between database fields and GraphQL and REST fields. format: --map \"backendName1:exposedName1,backendName2:exposedName2,...\".")] public IEnumerable? Map { get; } - public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) + public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) { logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion()); if (!IsEntityProvided(Entity, logger, command: "update")) { - return; + return CliReturnCode.GENERAL_ERROR; } bool isSuccess = ConfigGenerator.TryUpdateEntityWithOptions(this, loader, fileSystem); @@ -114,6 +115,8 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS { logger.LogError("Could not update the entity: {Entity}.", Entity); } + + return isSuccess ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR; } } } diff --git a/src/Cli/Commands/ValidateOptions.cs b/src/Cli/Commands/ValidateOptions.cs index 1bd13ac658..8f2ca124d2 100644 --- a/src/Cli/Commands/ValidateOptions.cs +++ b/src/Cli/Commands/ValidateOptions.cs @@ -4,6 +4,7 @@ using System.IO.Abstractions; using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Product; +using Cli.Constants; using CommandLine; using Microsoft.Extensions.Logging; using static Cli.Utils; @@ -24,7 +25,7 @@ public ValidateOptions(string config) /// This Handler method is responsible for validating the config file and is called when `validate` /// command is invoked. /// - public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) + public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) { logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion()); bool isValidConfig = ConfigGenerator.IsConfigValid(this, loader, fileSystem); @@ -37,6 +38,8 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS { logger.LogError("Config is invalid. Check above logs for details."); } + + return isValidConfig ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR; } } } diff --git a/src/Cli/Constants/CliReturnCode.cs b/src/Cli/Constants/CliReturnCode.cs new file mode 100644 index 0000000000..0a83ef50a6 --- /dev/null +++ b/src/Cli/Constants/CliReturnCode.cs @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("Cli.Tests")] +namespace Cli.Constants +{ + internal class CliReturnCode + { + public const int SUCCESS = 0; + public const int GENERAL_ERROR = -1; + } +} diff --git a/src/Cli/DabCliParserErrorHandler.cs b/src/Cli/DabCliParserErrorHandler.cs new file mode 100644 index 0000000000..d6b71338b2 --- /dev/null +++ b/src/Cli/DabCliParserErrorHandler.cs @@ -0,0 +1,47 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using CommandLine; + +namespace Cli +{ + /// + /// Processes errors that occur during parsing of CLI verbs (start, init, export, add, update, etc) and their arguments. + /// + public class DabCliParserErrorHandler + { + /// + /// Processes errors accumulated by each parser in parser.ParseArguments(). + /// For DAB CLI, this only includes scenarios where the user provides invalid DAB CLI input. + /// e.g. incorrectly formed or missing options and parameters. + /// Additionally, an error is tracked if the user uses: + /// -> an unsupported CLI verb + /// -> --help. + /// -> --version + /// + /// Collection of Error objects collected by the CLI parser. + /// Return code: 0 when --help is used, otherwise -1. + public static int ProcessErrorsAndReturnExitCode(IEnumerable err) + { + // To know if `--help` or `--version` was requested. + bool isHelpOrVersionRequested = false; + + /// System.CommandLine considers --help and --version as NonParsed Errors + /// ref: https://github.com/commandlineparser/commandline/issues/630 + /// This is a workaround to make sure our app exits with exit code 0, + /// when user does --help or --versions. + /// dab --help -> ErrorType.HelpVerbRequestedError + /// dab [command-name] --help -> ErrorType.HelpRequestedError + /// dab --version -> ErrorType.VersionRequestedError + List errors = err.ToList(); + if (errors.Any(e => e.Tag == ErrorType.VersionRequestedError + || e.Tag == ErrorType.HelpRequestedError + || e.Tag == ErrorType.HelpVerbRequestedError)) + { + isHelpOrVersionRequested = true; + } + + return isHelpOrVersionRequested ? 0 : -1; + } + } +} diff --git a/src/Cli/Exporter.cs b/src/Cli/Exporter.cs index f5bb90fc09..4b24c4b23f 100644 --- a/src/Cli/Exporter.cs +++ b/src/Cli/Exporter.cs @@ -13,7 +13,7 @@ namespace Cli { internal static class Exporter { - public static void Export(ExportOptions options, ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) + public static int Export(ExportOptions options, ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) { StartOptions startOptions = new(false, LogLevel.None, false, options.Config!); @@ -23,7 +23,7 @@ public static void Export(ExportOptions options, ILogger logger, FileSystemRunti if (!TryGetConfigFileBasedOnCliPrecedence(loader, options.Config, out string runtimeConfigFile)) { logger.LogError("Failed to find the config file provided, check your options and try again."); - return; + return -1; } if (!loader.TryLoadConfig( @@ -32,7 +32,7 @@ public static void Export(ExportOptions options, ILogger logger, FileSystemRunti replaceEnvVar: true) || runtimeConfig is null) { logger.LogError("Failed to read the config file: {runtimeConfigFile}.", runtimeConfigFile); - return; + return -1; } Task server = Task.Run(() => @@ -40,6 +40,7 @@ public static void Export(ExportOptions options, ILogger logger, FileSystemRunti _ = ConfigGenerator.TryStartEngineWithOptions(startOptions, loader, fileSystem); }, cancellationToken); + bool isSuccess = false; if (options.GraphQL) { int retryCount = 5; @@ -50,6 +51,7 @@ public static void Export(ExportOptions options, ILogger logger, FileSystemRunti try { ExportGraphQL(options, runtimeConfig, fileSystem); + isSuccess = true; break; } catch @@ -65,6 +67,7 @@ public static void Export(ExportOptions options, ILogger logger, FileSystemRunti } cancellationTokenSource.Cancel(); + return isSuccess ? 0 : -1; } private static void ExportGraphQL(ExportOptions options, RuntimeConfig runtimeConfig, System.IO.Abstractions.IFileSystem fileSystem) diff --git a/src/Cli/Program.cs b/src/Cli/Program.cs index 4731d13ef2..1b9ae47a88 100644 --- a/src/Cli/Program.cs +++ b/src/Cli/Program.cs @@ -26,25 +26,31 @@ public static int Main(string[] args) // Load environment variables from .env file if present. DotNetEnv.Env.Load(); - // Setting up Logger for CLI. + // Logger setup and configuration ILoggerFactory loggerFactory = Utils.LoggerFactoryForCli; - ILogger cliLogger = loggerFactory.CreateLogger(); ILogger configGeneratorLogger = loggerFactory.CreateLogger(); ILogger cliUtilsLogger = loggerFactory.CreateLogger(); ConfigGenerator.SetLoggerForCliConfigGenerator(configGeneratorLogger); Utils.SetCliUtilsLogger(cliUtilsLogger); + + // Sets up the filesystem used for reading and writing runtime configuration files. IFileSystem fileSystem = new FileSystem(); FileSystemRuntimeConfigLoader loader = new(fileSystem); return Execute(args, cliLogger, fileSystem, loader); } + /// + /// Execute the CLI command + /// + /// Command line arguments + /// Logger used as sink for informational and error messages. + /// Filesystem used for reading and writing configuration files, and exporting GraphQL schemas. + /// Loads the runtime config. + /// Exit Code: 0 success, -1 failure public static int Execute(string[] args, ILogger cliLogger, IFileSystem fileSystem, FileSystemRuntimeConfigLoader loader) { - // To know if `--help` or `--version` was requested. - bool isHelpOrVersionRequested = false; - Parser parser = new(settings => { settings.CaseInsensitiveEnumValues = true; @@ -52,33 +58,18 @@ public static int Execute(string[] args, ILogger cliLogger, IFileSystem fileSyst }); // Parsing user arguments and executing required methods. - ParserResult? result = parser.ParseArguments(args) - .WithParsed((Action)(options => options.Handler(cliLogger, loader, fileSystem))) - .WithParsed((Action)(options => options.Handler(cliLogger, loader, fileSystem))) - .WithParsed((Action)(options => options.Handler(cliLogger, loader, fileSystem))) - .WithParsed((Action)(options => options.Handler(cliLogger, loader, fileSystem))) - .WithParsed((Action)(options => options.Handler(cliLogger, loader, fileSystem))) - .WithParsed((Action)(options => options.Handler(cliLogger, loader, fileSystem))) - .WithParsed((Action)(options => Exporter.Export(options, cliLogger, loader, fileSystem))) - .WithNotParsed(err => - { - /// System.CommandLine considers --help and --version as NonParsed Errors - /// ref: https://github.com/commandlineparser/commandline/issues/630 - /// This is a workaround to make sure our app exits with exit code 0, - /// when user does --help or --versions. - /// dab --help -> ErrorType.HelpVerbRequestedError - /// dab [command-name] --help -> ErrorType.HelpRequestedError - /// dab --version -> ErrorType.VersionRequestedError - List errors = err.ToList(); - if (errors.Any(e => e.Tag == ErrorType.VersionRequestedError - || e.Tag == ErrorType.HelpRequestedError - || e.Tag == ErrorType.HelpVerbRequestedError)) - { - isHelpOrVersionRequested = true; - } - }); + int result = parser.ParseArguments(args) + .MapResult( + (InitOptions options) => options.Handler(cliLogger, loader, fileSystem), + (AddOptions options) => options.Handler(cliLogger, loader, fileSystem), + (UpdateOptions options) => options.Handler(cliLogger, loader, fileSystem), + (StartOptions options) => options.Handler(cliLogger, loader, fileSystem), + (ValidateOptions options) => options.Handler(cliLogger, loader, fileSystem), + (AddTelemetryOptions options) => options.Handler(cliLogger, loader, fileSystem), + (ExportOptions options) => Exporter.Export(options, cliLogger, loader, fileSystem), + errors => DabCliParserErrorHandler.ProcessErrorsAndReturnExitCode(errors)); - return ((result is Parsed) || (isHelpOrVersionRequested)) ? 0 : -1; + return result; } } }