From f254f545e41848d1ce8fc4103e22a2f426be19bb Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Wed, 17 Jun 2026 13:46:16 +0700 Subject: [PATCH 1/4] feat: Remove the deprecated NamingConventionBinder package and migrate to System.CommandLine --- Directory.Packages.props | 2 +- src/grate/Commands/MigrateCommand.cs | 519 +++++++++++------- src/grate/Program.cs | 81 ++- src/grate/grate.csproj | 2 +- .../Basic_CommandLineParsing.cs | 32 +- .../FolderConfiguration_.cs | 17 +- .../TestInfrastructure/DockerGrateMigrator.cs | 11 +- 7 files changed, 368 insertions(+), 296 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index d0129509..ba294fa7 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -14,7 +14,7 @@ - + diff --git a/src/grate/Commands/MigrateCommand.cs b/src/grate/Commands/MigrateCommand.cs index cf3a87f4..9bfc0711 100644 --- a/src/grate/Commands/MigrateCommand.cs +++ b/src/grate/Commands/MigrateCommand.cs @@ -1,96 +1,200 @@ using System.CommandLine; -using System.CommandLine.NamingConventionBinder; +using System.CommandLine.Parsing; using grate.Configuration; using grate.Infrastructure; using grate.Migration; +using Microsoft.Extensions.Logging; using static grate.Configuration.DefaultConfiguration; namespace grate.Commands; internal sealed class MigrateCommand : RootCommand { + // The options are stored as fields so that their parsed values can be read back + // explicitly in GetConfiguration. This replaces the (now removed and deprecated) + // System.CommandLine.NamingConventionBinder, which used to bind option values to the + // configuration record by reflection/convention. + private readonly Option _connectionString = ConnectionString(); + private readonly Option _adminConnectionString = AdminConnectionString(); + private readonly Option _sqlFilesDirectory = SqlFilesDirectory(); + private readonly Option _outputPath = OutputPath(); + private readonly Option _folders = Folders(); + private readonly Option _accessToken = AccessToken(); + private readonly Option _commandTimeout = CommandTimeout(); + private readonly Option _commandTimeoutAdmin = CommandTimeoutAdmin(); + private readonly Option _databaseType = DatabaseType(); + private readonly Option _runInTransaction = RunInTransaction(); + private readonly Option _environment = Environments(); + private readonly Option _schemaName = SchemaName(); + private readonly Option _silent = Silent(); + private readonly Option _repositoryPath = RepositoryPath(); + private readonly Option _version = Version(); + private readonly Option _drop = Drop(); + private readonly Option _createDatabase = CreateDatabase(); + private readonly Option _disableTokenReplacement = Tokens(); + private readonly Option _warnOnOneTimeScriptChanges = WarnAndRunOnScriptChange(); + private readonly Option _warnAndIgnoreOnOneTimeScriptChanges = WarnAndIgnoreOnScriptChange(); + private readonly Option> _userTokens = UserTokens(); + private readonly Option _doNotStoreScriptsRunText = DoNotStoreScriptText(); + private readonly Option _baseline = Baseline(); + private readonly Option _runAllAnyTimeScripts = RunAllAnyTimeScripts(); + private readonly Option _dryRun = DryRun(); + private readonly Option _restore = Restore(); + private readonly Option _ignoreDirectoryNames = IgnoreDirectoryNames(); + private readonly Option _upToDateCheck = UpToDateCheck(); + private readonly Option _verbosity = Verbosity(); + public MigrateCommand(IGrateMigrator mi) : base("Migrates the database") { - Add(Database()); - Add(ConnectionString()); - Add(SqlFilesDirectory()); - Add(OutputPath()); - Add(Folders()); - Add(ServerName()); - Add(AdminConnectionString()); - Add(AccessToken()); - Add(CommandTimeout()); - Add(CommandTimeoutAdmin()); - Add(DatabaseType()); - Add(RunInTransaction()); - Add(Environments()); - Add(SchemaName()); - Add(Silent()); - Add(RepositoryPath()); - Add(Version()); - Add(Drop()); - Add(CreateDatabase()); - Add(Tokens()); - Add(WarnAndRunOnScriptChange()); - Add(WarnAndIgnoreOnScriptChange()); - Add(UserTokens()); - Add(DoNotStoreScriptText()); - Add(Baseline()); - Add(RunAllAnyTimeScripts()); - Add(DryRun()); - Add(Restore()); - Add(IgnoreDirectoryNames()); - Add(UpToDateCheck()); - - Handler = CommandHandler.Create( - async () => - { - await mi.Migrate(); - }); + // System.CommandLine's RootCommand adds a built-in --version option by default. + // grate uses --version for its own purpose (the database version being migrated to), + // so remove the built-in one to avoid a conflicting option name. + foreach (var versionOption in Options.OfType().ToList()) + { + Options.Remove(versionOption); + } + + Options.Add(_connectionString); + Options.Add(_adminConnectionString); + Options.Add(_sqlFilesDirectory); + Options.Add(_outputPath); + Options.Add(_folders); + Options.Add(_accessToken); + Options.Add(_commandTimeout); + Options.Add(_commandTimeoutAdmin); + Options.Add(_databaseType); + Options.Add(_runInTransaction); + Options.Add(_environment); + Options.Add(_schemaName); + Options.Add(_silent); + Options.Add(_repositoryPath); + Options.Add(_version); + Options.Add(_drop); + Options.Add(_createDatabase); + Options.Add(_disableTokenReplacement); + Options.Add(_warnOnOneTimeScriptChanges); + Options.Add(_warnAndIgnoreOnOneTimeScriptChanges); + Options.Add(_userTokens); + Options.Add(_doNotStoreScriptsRunText); + Options.Add(_baseline); + Options.Add(_runAllAnyTimeScripts); + Options.Add(_dryRun); + Options.Add(_restore); + Options.Add(_ignoreDirectoryNames); + Options.Add(_upToDateCheck); + Options.Add(_verbosity); + + // Obsolete options - kept so that grate keeps reporting helpful messages, but not bound. + Options.Add(Database()); + Options.Add(ServerName()); + + SetAction((ParseResult _, CancellationToken _) => mi.Migrate()); + } + + /// + /// Reads the parsed option values and maps them onto a . + /// This is the explicit replacement for the convention-based model binder that used to live in + /// System.CommandLine.NamingConventionBinder. + /// + public CommandLineGrateConfiguration GetConfiguration(ParseResult parseResult) + { + // Defaults are used for options that have no default value and weren't supplied on the + // command line, to preserve the record's initialised defaults (mirroring the old binder, + // which left such properties untouched). + var defaults = new CommandLineGrateConfiguration(); + + return new CommandLineGrateConfiguration + { + ConnectionString = GetValueOrDefault(parseResult, _connectionString), + AdminConnectionString = GetValueOrDefault(parseResult, _adminConnectionString), + SqlFilesDirectory = GetValueOrDefault(parseResult, _sqlFilesDirectory)!, + OutputPath = GetValueOrDefault(parseResult, _outputPath)!, + Folders = GetValueOrDefault(parseResult, _folders) ?? defaults.Folders, + AccessToken = GetValueOrDefault(parseResult, _accessToken), + CommandTimeout = GetValueOrDefault(parseResult, _commandTimeout), + AdminCommandTimeout = GetValueOrDefault(parseResult, _commandTimeoutAdmin), + DatabaseType = GetValueOrDefault(parseResult, _databaseType), + Transaction = GetValueOrDefault(parseResult, _runInTransaction), + Environment = GetValueOrDefault(parseResult, _environment), + SchemaName = GetValueOrDefault(parseResult, _schemaName)!, + NonInteractive = GetValueOrDefault(parseResult, _silent), + RepositoryPath = GetValueOrDefault(parseResult, _repositoryPath), + Version = GetValueOrDefault(parseResult, _version) ?? defaults.Version, + Drop = GetValueOrDefault(parseResult, _drop), + CreateDatabase = GetValueOrDefault(parseResult, _createDatabase), + DisableTokenReplacement = GetValueOrDefault(parseResult, _disableTokenReplacement), + WarnOnOneTimeScriptChanges = GetValueOrDefault(parseResult, _warnOnOneTimeScriptChanges), + WarnAndIgnoreOnOneTimeScriptChanges = GetValueOrDefault(parseResult, _warnAndIgnoreOnOneTimeScriptChanges), + UserTokens = GetValueOrDefault(parseResult, _userTokens), + DoNotStoreScriptsRunText = GetValueOrDefault(parseResult, _doNotStoreScriptsRunText), + Baseline = GetValueOrDefault(parseResult, _baseline), + RunAllAnyTimeScripts = GetValueOrDefault(parseResult, _runAllAnyTimeScripts), + DryRun = GetValueOrDefault(parseResult, _dryRun), + Restore = GetValueOrDefault(parseResult, _restore), + IgnoreDirectoryNames = GetValueOrDefault(parseResult, _ignoreDirectoryNames), + UpToDateCheck = GetValueOrDefault(parseResult, _upToDateCheck), + // Verbosity has no default value factory; when it isn't supplied keep the record default. + Verbosity = parseResult.GetResult(_verbosity) is { Implicit: false } + ? GetValueOrDefault(parseResult, _verbosity) + : defaults.Verbosity, + }; + } + + // Reads an option value, returning the type default when the option wasn't supplied (or failed to + // convert). This keeps configuration binding tolerant - e.g. the required --connectionstring throws + // from GetValue when missing, but this preliminary binding must not crash. The authoritative parse + // error reporting happens when the command is invoked. + private static T? GetValueOrDefault(ParseResult parseResult, Option option) + { + try + { + return parseResult.GetValue(option); + } + catch (InvalidOperationException) + { + return default; + } } //REQUIRED OPTIONS - private static Option ConnectionString() => - new Option( - new[] { "--connectionstring", "-c", "-cs", "--connstring" }, - "You now provide an entire connection string. ServerName and Database are obsolete." - ) - { IsRequired = true }; + private static Option ConnectionString() => + new("--connectionstring", "-c", "-cs", "--connstring") + { + Description = "You now provide an entire connection string. ServerName and Database are obsolete.", + Required = true + }; //CONNECTIONSTRING OPTIONS - private static Option AdminConnectionString() => - new Option( - new[] { "-csa", "-a", "--adminconnectionstring", "-acs", "--adminconnstring" }, - "The connection string for connecting to master, if you want to create the database. Defaults to the same as --connstring." - ) - { IsRequired = false }; + private static Option AdminConnectionString() => + new("--adminconnectionstring", "-csa", "-a", "-acs", "--adminconnstring") + { + Description = "The connection string for connecting to master, if you want to create the database. Defaults to the same as --connstring.", + Required = false + }; //DIRECTORY OPTIONS - private static Option SqlFilesDirectory() => - new Option( - new[] { "--sqlfilesdirectory", "-f", "--files" }, - () => new DirectoryInfo(DefaultFilesDirectory), - "The directory where your SQL scripts are" - ).ExistingOnly(); - - private static Option OutputPath() => - new Option( - new[] { "--output", "-o", "--outputPath" }, - () => new DirectoryInfo(DefaultOutputPath), - "This is where everything related to the migration is stored. This includes any backups, all items that ran, permission dumps, logs, etc." - ); - - private static Option Folders() => - new Option( - new[] { "--folders" }, - -#pragma warning disable CA1826 - // The code that's violating the rule is on this line. - result => FoldersCommand.Parse(result?.Tokens?.FirstOrDefault()?.ToString()), -#pragma warning restore CA1826 - description: + private static Option SqlFilesDirectory() => + new Option("--sqlfilesdirectory", "-f", "--files") + { + Description = "The directory where your SQL scripts are", + DefaultValueFactory = _ => new DirectoryInfo(DefaultFilesDirectory) + }.AcceptExistingOnly(); + + private static Option OutputPath() => + new("--outputPath", "-o", "--output") + { + Description = "This is where everything related to the migration is stored. This includes any backups, all items that ran, permission dumps, logs, etc.", + DefaultValueFactory = _ => new DirectoryInfo(DefaultOutputPath) + }; + + private static Option Folders() => + new("--folders") + { + CustomParser = result => + FoldersCommand.Parse(result.Tokens.Count > 0 ? result.Tokens[0].Value : null), + Description = @"Folder configuration. If you wish to override any of the default folder names, supply a semicolon separated list of mappings. @@ -101,7 +205,7 @@ private static Option Folders() => --folders 'up=ddl;views=projections;beforemigration=preparefordeploy' -or +or --folders myfolderconfig.txt @@ -130,7 +234,7 @@ Defaults to the name given above. * Type - the type of the migration (Once, EveryTime, Anytime), defaults to Once, * ConnectionType - whether to run on the default connection, or on the admin (defaults to Default), Allowed values: default, admin - * TransactionHandling - whether to be part of the transaction, or run the script even on a rollback, + * TransactionHandling - whether to be part of the transaction, or run the script even on a rollback, defaults to Default Allowed values: default, autonomous @@ -146,197 +250,190 @@ defaults to Default the last one will expect the folders to be named 'folder1', 'folder2', and 'folder3', in the sqlfilesdirectory. -", - isDefault: false - ); +" + }; //SECURITY OPTIONS - private static Option AccessToken() => - new Option( - new[] { "--accesstoken" }, - "Access token to be used for logging in to SQL Server / Azure SQL Database." - ); + private static Option AccessToken() => + new("--accesstoken") + { + Description = "Access token to be used for logging in to SQL Server / Azure SQL Database." + }; //TIMEOUT OPTIONS - private static Option CommandTimeout() => - new Option( - new[] { "--commandtimeout", "-ct" }, - () => DefaultCommandTimeout, - "This is the timeout when commands are run. This is not for admin commands or restore." - ); - - private static Option CommandTimeoutAdmin() => - new Option( - new[] { "--admincommandtimeout", "-cta" }, - () => DefaultAdminCommandTimeout, - "This is the timeout when administration commands are run (except for restore, which has its own)." - ); + private static Option CommandTimeout() => + new("--commandtimeout", "-ct") + { + Description = "This is the timeout when commands are run. This is not for admin commands or restore.", + DefaultValueFactory = _ => DefaultCommandTimeout + }; + + private static Option CommandTimeoutAdmin() => + new("--admincommandtimeout", "-cta") + { + Description = "This is the timeout when administration commands are run (except for restore, which has its own).", + DefaultValueFactory = _ => DefaultAdminCommandTimeout + }; //DATABASE OPTIONS - private static Option DatabaseType() => - new Option( - new[] { "--databasetype", "--dt", "--dbt" }, - () => Configuration.DatabaseType.SQLServer, - "TELLS GRATE WHAT TYPE OF DATABASE IT IS RUNNING ON." - ); - - private static Option RunInTransaction() => //new Argument("-t"); - new Option( - new[] { "--transaction", "--trx", "-t" }, - "Run the migration in a transaction" - ); + private static Option DatabaseType() => + new("--databasetype", "--dt", "--dbt") + { + Description = "TELLS GRATE WHAT TYPE OF DATABASE IT IS RUNNING ON.", + DefaultValueFactory = _ => Configuration.DatabaseType.SQLServer + }; + + private static Option RunInTransaction() => + new("--transaction", "--trx", "-t") + { + Description = "Run the migration in a transaction" + }; private static Option SchemaName() => - new( - new[] { "--sc", "--schema", "--schemaname" }, - () => "grate", - "The schema to use for the migration tables" - ); + new("--schemaname", "--sc", "--schema") + { + Description = "The schema to use for the migration tables", + DefaultValueFactory = _ => "grate" + }; private static Option Drop() => - new(new[] { "--drop" }, - "Drop - This instructs grate to remove the target database. Unlike RoundhousE grate will continue to run the migration scripts after the drop." - ); + new("--drop") + { + Description = "Drop - This instructs grate to remove the target database. Unlike RoundhousE grate will continue to run the migration scripts after the drop." + }; private static Option CreateDatabase() => - new(new[] { "--createdatabase", "--create" }, - () => true, - "Create - This instructs grate to create the target database if it does not exist. Defaults to true. Set to false to emulate the --donotcreatedatabase flag in roundhouse." - ); + new("--createdatabase", "--create") + { + Description = "Create - This instructs grate to create the target database if it does not exist. Defaults to true. Set to false to emulate the --donotcreatedatabase flag in roundhouse.", + DefaultValueFactory = _ => true + }; //ENVIRONMENT OPTIONS - //private static Option Environments() => private static Option Environments() => - new( - aliases: new[] { "--env", "--environment" }, - parseArgument: ArgumentParsers - .ParseEnvironment, // Needed in System.CommandLine beta3: https://github.com/dotnet/command-line-api/issues/1664 - description: - "Environment Name - This allows grate to be environment aware and only run scripts that are in a particular environment based on the name of the script. 'something.ENV.LOCAL.sql' would only be run if --env=LOCAL was set." - ) - { - AllowMultipleArgumentsPerToken = true - //Arity = ArgumentArity.ZeroOrMore + new("--environment", "--env") + { + // A custom parser is needed to support combining environments separated by space, ',' or ';'. + CustomParser = ArgumentParsers.ParseEnvironment, + Description = + "Environment Name - This allows grate to be environment aware and only run scripts that are in a particular environment based on the name of the script. 'something.ENV.LOCAL.sql' would only be run if --env=LOCAL was set.", + AllowMultipleArgumentsPerToken = true, + Arity = ArgumentArity.ZeroOrMore }; - // - // private static Option Environments() => - // new( - // aliases: new[] { "--envs", "--environments" }, - // parseArgument: ArgumentParsers.ParseEnvironmentString, // Needed in System.CommandLine beta3: https://github.com/dotnet/command-line-api/issues/1664 - // description: "Environment Name - This allows grate to be environment aware and only run scripts that are in a particular environment based on the name of the script. 'something.ENV.LOCAL.sql' would only be run if --env=LOCAL was set." - // ); - - // private static Option Environments() => - // new( - // aliases: new[] { "--env", "--environment" }, - // description: "Environment Name - This allows grate to be environment aware and only run scripts that are in a particular environment based on the name of the script. 'something.ENV.LOCAL.sql' would only be run if --env=LOCAL was set." - // ); //WARNING OPTIONS private static Option WarnAndRunOnScriptChange() => - new( - new[] { "-w", "--warnononetimescriptchanges" }, - "WarnOnOneTimeScriptChanges - Instructs grate to execute changed one time scripts(DDL / DML in Up folder) that have previously been run against the database instead of failing. A warning is logged for each one time script that is rerun. Defaults to false." - ); + new("--warnononetimescriptchanges", "-w") + { + Description = "WarnOnOneTimeScriptChanges - Instructs grate to execute changed one time scripts(DDL / DML in Up folder) that have previously been run against the database instead of failing. A warning is logged for each one time script that is rerun. Defaults to false." + }; private static Option WarnAndIgnoreOnScriptChange() => - new( - new[] { "--warnandignoreononetimescriptchanges" }, - "WarnAndIgnoreOnOneTimeScriptChanges - Instructs grate to ignore and update the hash of changed one time scripts (DDL/DML in Up folder) that have previously been run against the database instead of failing. A warning is logged for each one time scripts that is rerun. Defaults to false." - ); + new("--warnandignoreononetimescriptchanges") + { + Description = "WarnAndIgnoreOnOneTimeScriptChanges - Instructs grate to ignore and update the hash of changed one time scripts (DDL/DML in Up folder) that have previously been run against the database instead of failing. A warning is logged for each one time scripts that is rerun. Defaults to false." + }; //TOKEN OPTIONS private static Option Tokens() => - new( - new[] { "--disabletokenreplacement", "--disabletokens" }, - "Tokens - This instructs grate to not perform token replacement ({{somename}}). Defaults to false." - ); + new("--disabletokenreplacement", "--disabletokens") + { + Description = "Tokens - This instructs grate to not perform token replacement ({{somename}}). Defaults to false." + }; private static Option> UserTokens() => - new( - new[] { "--ut", "--usertokens" }, - "User Tokens - Allows grate to perform token replacement on custom tokens ({{my_token}}). Set as a key=value pair, eg '--ut=my_token=myvalue'. Can be specified multiple times." - ); + new("--usertokens", "--ut") + { + Description = "User Tokens - Allows grate to perform token replacement on custom tokens ({{my_token}}). Set as a key=value pair, eg '--ut=my_token=myvalue'. Can be specified multiple times." + }; //SCRIPT OPTIONS private static Option DoNotStoreScriptText() => - new( - new[] { "--donotstorescriptsruntext" }, - "DoNotStoreScriptsRunText - This instructs grate to not store the full script text in the database. Defaults to false." - ); + new("--donotstorescriptsruntext") + { + Description = "DoNotStoreScriptsRunText - This instructs grate to not store the full script text in the database. Defaults to false." + }; private static Option RunAllAnyTimeScripts() => - new( - new[] { "--runallanytimescripts", "--forceanytimescripts" }, - "RunAllAnyTimeScripts - This instructs grate to run any time scripts every time it is run even if they haven't changed. Defaults to false." - ); + new("--runallanytimescripts", "--forceanytimescripts") + { + Description = "RunAllAnyTimeScripts - This instructs grate to run any time scripts every time it is run even if they haven't changed. Defaults to false." + }; //MISC OPTIONS private static Option Baseline() => - new( - new[] { "--baseline" }, - "Baseline - This instructs grate to mark the scripts as run, but not to actually run anything against the database. Use this option if you already have scripts that have been run through other means (and BEFORE you start the new ones)." - ); + new("--baseline") + { + Description = "Baseline - This instructs grate to mark the scripts as run, but not to actually run anything against the database. Use this option if you already have scripts that have been run through other means (and BEFORE you start the new ones)." + }; private static Option DryRun() => - new( - new[] { "--dryrun" }, - " DryRun - This instructs grate to log what would have run, but not to actually run anything against the database. Use this option if you are trying to figure out what grate is going to do." - ); + new("--dryrun") + { + Description = " DryRun - This instructs grate to log what would have run, but not to actually run anything against the database. Use this option if you are trying to figure out what grate is going to do." + }; private static Option Restore() => - new( - new[] { "--restore" }, - " Restore - This instructs grate where to get the backed up database file. Defaults to NULL." - ); + new("--restore") + { + Description = " Restore - This instructs grate where to get the backed up database file. Defaults to NULL." + }; private static Option Silent() => - new( - new[] { "--noninteractive", "-ni", "--ni", "--silent" }, - "Silent - tells grate not to ask for any input when it runs." - ); + new("--noninteractive", "-ni", "--ni", "--silent") + { + Description = "Silent - tells grate not to ask for any input when it runs." + }; private static Option RepositoryPath() => - new( - new[] { "-r", "--repo", "--repositorypath" }, - "Repository Path - The repository. A string that can be anything. Used to track versioning along with the version. Defaults to NULL." - ); + new("--repositorypath", "-r", "--repo") + { + Description = "Repository Path - The repository. A string that can be anything. Used to track versioning along with the version. Defaults to NULL." + }; private static Option Version() => - new( - new[] { "--version" }, - "Database Version - specify the version of the current migration directly on the command line." - ); + new("--version") + { + Description = "Database Version - specify the version of the current migration directly on the command line." + }; //OBSOLETE OPTIONS - private static Option Database() => - new Option( - new[] { "--database" }, - "OBSOLETE: Please specify the connection string instead") - { IsRequired = false }; - - private static Option ServerName() => - new Option( - new[] { "--instance", "--server", "--servername", "-s" }, - //() => DefaultServerName, - "OBSOLETE: Please specify the connection string instead." - ); + private static Option Database() => + new("--database") + { + Description = "OBSOLETE: Please specify the connection string instead", + Required = false + }; + + private static Option ServerName() => + new("--servername", "--instance", "--server", "-s") + { + Description = "OBSOLETE: Please specify the connection string instead." + }; private static Option IgnoreDirectoryNames() => - new( - new[] { "--ignoredirectorynames", "--searchallinsteadoftraverse", "--searchallsubdirectoriesinsteadoftraverse" }, - "IgnoreDirectoryNames - By default, scripts are ordered by relative path including subdirectories. This option searches subdirectories, but order is based on filename alone." - ); - - internal static Option UpToDateCheck() => new( - ["--uptodatecheck", "--isuptodate"], - "Outputs whether the database is up to date or not (whether any non-everytime scripts would be run)"); + new("--ignoredirectorynames", "--searchallinsteadoftraverse", "--searchallsubdirectoriesinsteadoftraverse") + { + Description = "IgnoreDirectoryNames - By default, scripts are ordered by relative path including subdirectories. This option searches subdirectories, but order is based on filename alone." + }; + + private static Option UpToDateCheck() => + new("--uptodatecheck", "--isuptodate") + { + Description = "Outputs whether the database is up to date or not (whether any non-everytime scripts would be run)" + }; + + private static Option Verbosity() => + new("--verbosity", "-v") + { + Description = "Verbosity level (as defined here: https://docs.microsoft.com/dotnet/api/Microsoft.Extensions.Logging.LogLevel)" + }; } diff --git a/src/grate/Program.cs b/src/grate/Program.cs index accfdd04..cfeb711f 100644 --- a/src/grate/Program.cs +++ b/src/grate/Program.cs @@ -1,9 +1,4 @@ using System.CommandLine; -using System.CommandLine.Builder; -using System.CommandLine.Invocation; -using System.CommandLine.IO; -using System.CommandLine.NamingConventionBinder; -using System.CommandLine.Parsing; using System.Reflection; using grate.Commands; using grate.Configuration; @@ -28,7 +23,7 @@ public static async Task Main(string[] args) { // Temporarily parse the configuration, to get the verbosity level, and potentially set parameters // to support the "IsUpToDate" check. - var cfg = await ParseGrateConfiguration(args); + var cfg = ParseGrateConfiguration(args); if (cfg.UpToDateCheck) { cfg = cfg with { Verbosity = LogLevel.Critical, DryRun = true }; @@ -37,64 +32,56 @@ public static async Task Main(string[] args) _serviceProvider = BuildServiceProvider(cfg).CreateAsyncScope().ServiceProvider; var rootCommand = Create(); - rootCommand.Add(Verbosity()); rootCommand.Description = $"grate v{GetVersion()} - sql for the 20s"; - var parser = new CommandLineBuilder(rootCommand) - // These are all the CommandLine features enabled by default - // .UseVersionOption() //but we don't want version (as we use the --version option ourselves) - .UseHelp() - .UseEnvironmentVariableDirective() - .UseParseDirective() - .UseSuggestDirective() - .RegisterWithDotnetSuggest() - .UseTypoCorrections() - .UseParseErrorReporting() - .UseExceptionHandler(ExceptionHandler) - .CancelOnProcessTermination() - .Build(); - - var result = await parser.InvokeAsync(args); + // System.CommandLine enables help, suggestions, typo corrections and parse-error reporting + // by default. We handle exceptions ourselves (see below), so disable the built-in handler. + var configuration = new InvocationConfiguration + { + EnableDefaultExceptionHandler = false + }; + + var parseResult = rootCommand.Parse(args); + + int result; + try + { + result = await parseResult.InvokeAsync(configuration); + } + catch (Exception ex) + { + result = ExceptionHandler(ex, configuration); + } await WaitForLoggerToFinish(); return result; } - - private static void ExceptionHandler(Exception ex, InvocationContext context) + + private static int ExceptionHandler(Exception ex, InvocationConfiguration configuration) { // Log the error message at the highest level, and the exception at debug level. // Avoids logging the exception stack trace to the end user, if logging level is not set to debug. - + var logger = _serviceProvider.GetRequiredService>(); - - context.Console.Error.CreateTextWriter().WriteColoredMessage("An error occurred: ", GrateConsoleColor.Foreground.Red); - + + configuration.Error.WriteColoredMessage("An error occurred: ", GrateConsoleColor.Foreground.Red); + logger.LogDebug(ex, "{ErrorMessage}", ex.Message); logger.LogError("{ErrorMessage}", ex.Message); - - context.ExitCode = 1; + + return 1; } - + private static string GetVersion() => Assembly.GetEntryAssembly()?.GetName().Version?.ToString() ?? "1.0.0.1"; - private static async Task ParseGrateConfiguration(IReadOnlyList commandline) + private static CommandLineGrateConfiguration ParseGrateConfiguration(IReadOnlyList commandline) { - CommandLineGrateConfiguration cfg = new CommandLineGrateConfiguration(); - var handler = CommandHandler.Create((CommandLineGrateConfiguration config) => cfg = config); - - var cmd = new MigrateCommand(null!) - { - Verbosity(), - }; - - ParseResult p = - new Parser(cmd).Parse(commandline); - await handler.InvokeAsync(new InvocationContext(p)); - - return cfg; + var cmd = new MigrateCommand(null!); + var parseResult = cmd.Parse(commandline); + return cmd.GetConfiguration(parseResult); } @@ -154,9 +141,5 @@ private static ServiceProvider BuildServiceProvider(CommandLineGrateConfiguratio return services.BuildServiceProvider(); } - internal static Option Verbosity() => new( - ["-v", "--verbosity"], - "Verbosity level (as defined here: https://docs.microsoft.com/dotnet/api/Microsoft.Extensions.Logging.LogLevel)"); - private static T Create() where T : notnull => _serviceProvider.GetRequiredService(); } diff --git a/src/grate/grate.csproj b/src/grate/grate.csproj index 53f96755..751f2092 100644 --- a/src/grate/grate.csproj +++ b/src/grate/grate.csproj @@ -75,7 +75,7 @@ - + diff --git a/unittests/Basic_tests/CommandLineParsing/Basic_CommandLineParsing.cs b/unittests/Basic_tests/CommandLineParsing/Basic_CommandLineParsing.cs index b7596ae6..1ac416be 100644 --- a/unittests/Basic_tests/CommandLineParsing/Basic_CommandLineParsing.cs +++ b/unittests/Basic_tests/CommandLineParsing/Basic_CommandLineParsing.cs @@ -1,7 +1,4 @@ using System.CommandLine; -using System.CommandLine.Invocation; -using System.CommandLine.NamingConventionBinder; -using System.CommandLine.Parsing; using System.Configuration; using FluentAssertions; using grate.Commands; @@ -23,10 +20,15 @@ public class Basic_CommandLineParsing [Fact] public void ParserIsConfiguredCorrectly() { - // Test that the parser configuration is valid, see https://github.com/dotnet/command-line-api/issues/1613 - var command = new MigrateCommand(null!); - var configuration = new CommandLineConfiguration(command); - configuration.ThrowIfInvalid(); + // Test that the command configuration is valid, see https://github.com/dotnet/command-line-api/issues/1613 + // In System.CommandLine, invalid configuration (e.g. duplicate option aliases) throws while the + // command is being constructed and when it parses, so constructing and parsing must not throw. + var act = () => + { + var command = new MigrateCommand(null!); + _ = command.Parse(""); + }; + act.Should().NotThrow(); } [Theory] @@ -397,7 +399,7 @@ public async Task UpToDateCheck(string args, bool expected) cfg?.UpToDateCheck.Should().Be(expected); } - private static async Task ParseGrateConfiguration(string commandline) + private static Task ParseGrateConfiguration(string commandline) { // All parsing fails if the connectionstring is not supplied, so we need to add it here, if it's not in the commandline. if ( @@ -409,19 +411,15 @@ public async Task UpToDateCheck(string args, bool expected) commandline += " -c \"Server=.;Database=master;Trusted_Connection=True;\""; } - CommandLineGrateConfiguration? cfg = null; - var cmd = CommandHandler.Create((CommandLineGrateConfiguration config) => cfg = config); - - ParseResult p = - new Parser(new MigrateCommand(null!)).Parse(commandline); + var command = new MigrateCommand(null!); + var parseResult = command.Parse(commandline); - if (p.Errors.Any()) + if (parseResult.Errors.Any()) { - var exceptions = p.Errors.Select(error => new ConfigurationErrorsException(error.Message)).ToList(); + var exceptions = parseResult.Errors.Select(error => new ConfigurationErrorsException(error.Message)).ToList(); throw new MigrationFailed(exceptions); } - await cmd.InvokeAsync(new InvocationContext(p)); - return cfg; + return Task.FromResult(command.GetConfiguration(parseResult)); } } diff --git a/unittests/Basic_tests/CommandLineParsing/FolderConfiguration_.cs b/unittests/Basic_tests/CommandLineParsing/FolderConfiguration_.cs index ad54736b..812094a1 100644 --- a/unittests/Basic_tests/CommandLineParsing/FolderConfiguration_.cs +++ b/unittests/Basic_tests/CommandLineParsing/FolderConfiguration_.cs @@ -1,7 +1,4 @@ using System.Collections.Immutable; -using System.CommandLine.Invocation; -using System.CommandLine.NamingConventionBinder; -using System.CommandLine.Parsing; using System.Runtime.CompilerServices; using FluentAssertions; using grate.Commands; @@ -25,7 +22,7 @@ public async Task Default() AssertEquivalent(expected.Values, actual?.Values); } - + [Fact] public async Task Default_with_overridden_transaction_handling_for_one_folder() { @@ -33,7 +30,7 @@ public async Task Default_with_overridden_transaction_handling_for_one_folder() var expected = FoldersConfiguration.Default(); expected[RunAfterCreateDatabase] = expected[RunAfterCreateDatabase]! with { TransactionHandling = TransactionHandling.Autonomous }; - + var actual = cfg?.Folders; actual![RunAfterCreateDatabase]!.TransactionHandling.Should().Be(TransactionHandling.Autonomous); AssertEquivalent(expected.Values, actual.Values); @@ -117,16 +114,12 @@ private static void AssertEquivalent(MigrationsFolder? expected, MigrationsFolde } - private static async Task ParseGrateConfiguration(params string[] commandline) + private static Task ParseGrateConfiguration(params string[] commandline) { - GrateConfiguration? cfg = null; - var cmd = CommandHandler.Create((GrateConfiguration config) => cfg = config); - var migrateCommand = new MigrateCommand(null!); - ParseResult p = new Parser(migrateCommand).Parse(commandline); - await cmd.InvokeAsync(new InvocationContext(p)); + var parseResult = migrateCommand.Parse(commandline); - return cfg; + return Task.FromResult(migrateCommand.GetConfiguration(parseResult)); } private static KnownFolderNamesWithDescription? Wrap(KnownFolderNames? names, [CallerArgumentExpression(nameof(names))] string description = "") => diff --git a/unittests/Docker/Docker.Common/TestInfrastructure/DockerGrateMigrator.cs b/unittests/Docker/Docker.Common/TestInfrastructure/DockerGrateMigrator.cs index 860cf9f7..8f4a508e 100644 --- a/unittests/Docker/Docker.Common/TestInfrastructure/DockerGrateMigrator.cs +++ b/unittests/Docker/Docker.Common/TestInfrastructure/DockerGrateMigrator.cs @@ -171,7 +171,6 @@ private List ConvertToDockerArguments(GrateConfiguration configuration) var properties = configuration.GetType().GetProperties(); var cmd = new MigrateCommand(this); - cmd.Add(Program.Verbosity()); foreach (var prop in properties) { @@ -194,12 +193,14 @@ private List ConvertToDockerArguments(GrateConfiguration configuration) } var name = prop.Name; - var option = cmd.Options.FirstOrDefault(o => string.Equals(o.Name, name, StringComparison.OrdinalIgnoreCase)); - + // In System.CommandLine the option Name now includes the prefix (e.g. "--connectionstring") + // and Aliases no longer contains the name, so match on the prefix-stripped name and use + // the option's (always "--"-prefixed) Name as the flag to pass on the command line. + var option = cmd.Options.FirstOrDefault(o => string.Equals(o.Name.TrimStart('-'), name, StringComparison.OrdinalIgnoreCase)); + if (option is not null && value is not null) { - var optionName = option.Aliases.FirstOrDefault(alias => alias.StartsWith("--")) - ?? option.Aliases.First(); + var optionName = option.Name; if (value is string[] arr) { From 931b321f5796d356d110cde21fee2d77c322d6ce Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Wed, 17 Jun 2026 14:04:21 +0700 Subject: [PATCH 2/4] Fix the unit test error when parsing the command line in TestInfrastructure project --- .../TestInfrastructure/CommandLineGrateMigrator.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/unittests/CommandLine/CommandLine.Common/TestInfrastructure/CommandLineGrateMigrator.cs b/unittests/CommandLine/CommandLine.Common/TestInfrastructure/CommandLineGrateMigrator.cs index ecf9ce8e..de7e8b10 100644 --- a/unittests/CommandLine/CommandLine.Common/TestInfrastructure/CommandLineGrateMigrator.cs +++ b/unittests/CommandLine/CommandLine.Common/TestInfrastructure/CommandLineGrateMigrator.cs @@ -100,12 +100,14 @@ private List ConvertToCommandLineArguments(GrateConfiguration configurat } var name = prop.Name; - var option = cmd.Options.FirstOrDefault(o => string.Equals(o.Name, name, StringComparison.OrdinalIgnoreCase)); - + // In System.CommandLine the option Name now includes the prefix (e.g. "--connectionstring") + // and Aliases no longer contains the name, so match on the prefix-stripped name and use + // the option's (always "--"-prefixed) Name as the flag to pass on the command line. + var option = cmd.Options.FirstOrDefault(o => string.Equals(o.Name.TrimStart('-'), name, StringComparison.OrdinalIgnoreCase)); + if (option is not null && value is not null) { - var optionName = option.Aliases.FirstOrDefault(alias => alias.StartsWith("--")) - ?? option.Aliases.First(); + var optionName = option.Name; if (value is string[] arr) { From 91c48b355a47cc7623e76d69a52fd13c5cee6986 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Sat, 20 Jun 2026 14:33:05 +0700 Subject: [PATCH 3/4] Fix the code review and remove the NET6_0 flag. --- src/grate/Commands/MigrateCommand.cs | 221 ++++++++---------- .../Basic_CommandLineParsing.cs | 55 ++--- .../IsUpToDate_.cs | 32 ++- .../MigrationStatus_.cs | 10 +- .../TestInfrastructure/DockerGrateMigrator.cs | 47 ++-- .../TestInfrastructure/SqliteTestDatabase.cs | 10 +- .../TestInfrastructure/DatabaseHelpers.cs | 12 +- .../TestInfrastructure/IGrateTestContext.cs | 12 +- .../TestInfrastructure/Net6PolyFills.cs | 18 -- .../TestInfrastructure/TestConfig.cs | 19 +- 10 files changed, 169 insertions(+), 267 deletions(-) delete mode 100644 unittests/TestCommon/TestInfrastructure/Net6PolyFills.cs diff --git a/src/grate/Commands/MigrateCommand.cs b/src/grate/Commands/MigrateCommand.cs index 9bfc0711..b24cc209 100644 --- a/src/grate/Commands/MigrateCommand.cs +++ b/src/grate/Commands/MigrateCommand.cs @@ -1,7 +1,5 @@ using System.CommandLine; -using System.CommandLine.Parsing; using grate.Configuration; -using grate.Infrastructure; using grate.Migration; using Microsoft.Extensions.Logging; using static grate.Configuration.DefaultConfiguration; @@ -10,39 +8,6 @@ namespace grate.Commands; internal sealed class MigrateCommand : RootCommand { - // The options are stored as fields so that their parsed values can be read back - // explicitly in GetConfiguration. This replaces the (now removed and deprecated) - // System.CommandLine.NamingConventionBinder, which used to bind option values to the - // configuration record by reflection/convention. - private readonly Option _connectionString = ConnectionString(); - private readonly Option _adminConnectionString = AdminConnectionString(); - private readonly Option _sqlFilesDirectory = SqlFilesDirectory(); - private readonly Option _outputPath = OutputPath(); - private readonly Option _folders = Folders(); - private readonly Option _accessToken = AccessToken(); - private readonly Option _commandTimeout = CommandTimeout(); - private readonly Option _commandTimeoutAdmin = CommandTimeoutAdmin(); - private readonly Option _databaseType = DatabaseType(); - private readonly Option _runInTransaction = RunInTransaction(); - private readonly Option _environment = Environments(); - private readonly Option _schemaName = SchemaName(); - private readonly Option _silent = Silent(); - private readonly Option _repositoryPath = RepositoryPath(); - private readonly Option _version = Version(); - private readonly Option _drop = Drop(); - private readonly Option _createDatabase = CreateDatabase(); - private readonly Option _disableTokenReplacement = Tokens(); - private readonly Option _warnOnOneTimeScriptChanges = WarnAndRunOnScriptChange(); - private readonly Option _warnAndIgnoreOnOneTimeScriptChanges = WarnAndIgnoreOnScriptChange(); - private readonly Option> _userTokens = UserTokens(); - private readonly Option _doNotStoreScriptsRunText = DoNotStoreScriptText(); - private readonly Option _baseline = Baseline(); - private readonly Option _runAllAnyTimeScripts = RunAllAnyTimeScripts(); - private readonly Option _dryRun = DryRun(); - private readonly Option _restore = Restore(); - private readonly Option _ignoreDirectoryNames = IgnoreDirectoryNames(); - private readonly Option _upToDateCheck = UpToDateCheck(); - private readonly Option _verbosity = Verbosity(); public MigrateCommand(IGrateMigrator mi) : base("Migrates the database") { @@ -54,41 +19,41 @@ public MigrateCommand(IGrateMigrator mi) : base("Migrates the database") Options.Remove(versionOption); } - Options.Add(_connectionString); - Options.Add(_adminConnectionString); - Options.Add(_sqlFilesDirectory); - Options.Add(_outputPath); - Options.Add(_folders); - Options.Add(_accessToken); - Options.Add(_commandTimeout); - Options.Add(_commandTimeoutAdmin); - Options.Add(_databaseType); - Options.Add(_runInTransaction); - Options.Add(_environment); - Options.Add(_schemaName); - Options.Add(_silent); - Options.Add(_repositoryPath); - Options.Add(_version); - Options.Add(_drop); - Options.Add(_createDatabase); - Options.Add(_disableTokenReplacement); - Options.Add(_warnOnOneTimeScriptChanges); - Options.Add(_warnAndIgnoreOnOneTimeScriptChanges); - Options.Add(_userTokens); - Options.Add(_doNotStoreScriptsRunText); - Options.Add(_baseline); - Options.Add(_runAllAnyTimeScripts); - Options.Add(_dryRun); - Options.Add(_restore); - Options.Add(_ignoreDirectoryNames); - Options.Add(_upToDateCheck); - Options.Add(_verbosity); + Options.Add(ConnectionString); + Options.Add(AdminConnectionString); + Options.Add(SqlFilesDirectory); + Options.Add(OutputPath); + Options.Add(Folders); + Options.Add(AccessToken); + Options.Add(CommandTimeout); + Options.Add(CommandTimeoutAdmin); + Options.Add(DatabaseType); + Options.Add(RunInTransaction); + Options.Add(Environments); + Options.Add(SchemaName); + Options.Add(Silent); + Options.Add(RepositoryPath); + Options.Add(Version); + Options.Add(Drop); + Options.Add(CreateDatabase); + Options.Add(Tokens); //DisableTokenReplacement + Options.Add(WarnAndRunOnScriptChange); + Options.Add(WarnAndIgnoreOnScriptChange); + Options.Add(UserTokens); + Options.Add(DoNotStoreScriptText); + Options.Add(Baseline); + Options.Add(RunAllAnyTimeScripts); + Options.Add(DryRun); + Options.Add(Restore); + Options.Add(IgnoreDirectoryNames); + Options.Add(UpToDateCheck); + Options.Add(Verbosity); // Obsolete options - kept so that grate keeps reporting helpful messages, but not bound. - Options.Add(Database()); - Options.Add(ServerName()); + Options.Add(Database); + Options.Add(ServerName); - SetAction((ParseResult _, CancellationToken _) => mi.Migrate()); + SetAction((_, _) => mi.Migrate()); } /// @@ -105,37 +70,37 @@ public CommandLineGrateConfiguration GetConfiguration(ParseResult parseResult) return new CommandLineGrateConfiguration { - ConnectionString = GetValueOrDefault(parseResult, _connectionString), - AdminConnectionString = GetValueOrDefault(parseResult, _adminConnectionString), - SqlFilesDirectory = GetValueOrDefault(parseResult, _sqlFilesDirectory)!, - OutputPath = GetValueOrDefault(parseResult, _outputPath)!, - Folders = GetValueOrDefault(parseResult, _folders) ?? defaults.Folders, - AccessToken = GetValueOrDefault(parseResult, _accessToken), - CommandTimeout = GetValueOrDefault(parseResult, _commandTimeout), - AdminCommandTimeout = GetValueOrDefault(parseResult, _commandTimeoutAdmin), - DatabaseType = GetValueOrDefault(parseResult, _databaseType), - Transaction = GetValueOrDefault(parseResult, _runInTransaction), - Environment = GetValueOrDefault(parseResult, _environment), - SchemaName = GetValueOrDefault(parseResult, _schemaName)!, - NonInteractive = GetValueOrDefault(parseResult, _silent), - RepositoryPath = GetValueOrDefault(parseResult, _repositoryPath), - Version = GetValueOrDefault(parseResult, _version) ?? defaults.Version, - Drop = GetValueOrDefault(parseResult, _drop), - CreateDatabase = GetValueOrDefault(parseResult, _createDatabase), - DisableTokenReplacement = GetValueOrDefault(parseResult, _disableTokenReplacement), - WarnOnOneTimeScriptChanges = GetValueOrDefault(parseResult, _warnOnOneTimeScriptChanges), - WarnAndIgnoreOnOneTimeScriptChanges = GetValueOrDefault(parseResult, _warnAndIgnoreOnOneTimeScriptChanges), - UserTokens = GetValueOrDefault(parseResult, _userTokens), - DoNotStoreScriptsRunText = GetValueOrDefault(parseResult, _doNotStoreScriptsRunText), - Baseline = GetValueOrDefault(parseResult, _baseline), - RunAllAnyTimeScripts = GetValueOrDefault(parseResult, _runAllAnyTimeScripts), - DryRun = GetValueOrDefault(parseResult, _dryRun), - Restore = GetValueOrDefault(parseResult, _restore), - IgnoreDirectoryNames = GetValueOrDefault(parseResult, _ignoreDirectoryNames), - UpToDateCheck = GetValueOrDefault(parseResult, _upToDateCheck), + ConnectionString = GetValueOrDefault(parseResult, ConnectionString), + AdminConnectionString = GetValueOrDefault(parseResult, AdminConnectionString), + SqlFilesDirectory = GetValueOrDefault(parseResult, SqlFilesDirectory)!, + OutputPath = GetValueOrDefault(parseResult, OutputPath)!, + Folders = GetValueOrDefault(parseResult, Folders) ?? defaults.Folders, + AccessToken = GetValueOrDefault(parseResult, AccessToken), + CommandTimeout = GetValueOrDefault(parseResult, CommandTimeout), + AdminCommandTimeout = GetValueOrDefault(parseResult, CommandTimeoutAdmin), + DatabaseType = GetValueOrDefault(parseResult, DatabaseType), + Transaction = GetValueOrDefault(parseResult, RunInTransaction), + Environment = GetValueOrDefault(parseResult, Environments), + SchemaName = GetValueOrDefault(parseResult, SchemaName)!, + NonInteractive = GetValueOrDefault(parseResult, Silent), + RepositoryPath = GetValueOrDefault(parseResult, RepositoryPath), + Version = GetValueOrDefault(parseResult, Version) ?? defaults.Version, + Drop = GetValueOrDefault(parseResult, Drop), + CreateDatabase = GetValueOrDefault(parseResult, CreateDatabase), + DisableTokenReplacement = GetValueOrDefault(parseResult, Tokens), + WarnOnOneTimeScriptChanges = GetValueOrDefault(parseResult, WarnAndRunOnScriptChange), + WarnAndIgnoreOnOneTimeScriptChanges = GetValueOrDefault(parseResult, WarnAndIgnoreOnScriptChange), + UserTokens = GetValueOrDefault(parseResult, UserTokens), + DoNotStoreScriptsRunText = GetValueOrDefault(parseResult, DoNotStoreScriptText), + Baseline = GetValueOrDefault(parseResult, Baseline), + RunAllAnyTimeScripts = GetValueOrDefault(parseResult, RunAllAnyTimeScripts), + DryRun = GetValueOrDefault(parseResult, DryRun), + Restore = GetValueOrDefault(parseResult, Restore), + IgnoreDirectoryNames = GetValueOrDefault(parseResult, IgnoreDirectoryNames), + UpToDateCheck = GetValueOrDefault(parseResult, UpToDateCheck), // Verbosity has no default value factory; when it isn't supplied keep the record default. - Verbosity = parseResult.GetResult(_verbosity) is { Implicit: false } - ? GetValueOrDefault(parseResult, _verbosity) + Verbosity = parseResult.GetResult(Verbosity) is { Implicit: false } + ? GetValueOrDefault(parseResult, Verbosity) : defaults.Verbosity, }; } @@ -157,7 +122,7 @@ public CommandLineGrateConfiguration GetConfiguration(ParseResult parseResult) } //REQUIRED OPTIONS - private static Option ConnectionString() => + private static readonly Option ConnectionString = new("--connectionstring", "-c", "-cs", "--connstring") { Description = "You now provide an entire connection string. ServerName and Database are obsolete.", @@ -166,7 +131,7 @@ private static Option ConnectionString() => //CONNECTIONSTRING OPTIONS - private static Option AdminConnectionString() => + private static readonly Option AdminConnectionString = new("--adminconnectionstring", "-csa", "-a", "-acs", "--adminconnstring") { Description = "The connection string for connecting to master, if you want to create the database. Defaults to the same as --connstring.", @@ -175,21 +140,21 @@ private static Option AdminConnectionString() => //DIRECTORY OPTIONS - private static Option SqlFilesDirectory() => + private static readonly Option SqlFilesDirectory = new Option("--sqlfilesdirectory", "-f", "--files") { Description = "The directory where your SQL scripts are", DefaultValueFactory = _ => new DirectoryInfo(DefaultFilesDirectory) }.AcceptExistingOnly(); - private static Option OutputPath() => + private static readonly Option OutputPath = new("--outputPath", "-o", "--output") { Description = "This is where everything related to the migration is stored. This includes any backups, all items that ran, permission dumps, logs, etc.", DefaultValueFactory = _ => new DirectoryInfo(DefaultOutputPath) }; - private static Option Folders() => + private static readonly Option Folders = new("--folders") { CustomParser = result => @@ -255,7 +220,7 @@ defaults to Default //SECURITY OPTIONS - private static Option AccessToken() => + private static readonly Option AccessToken = new("--accesstoken") { Description = "Access token to be used for logging in to SQL Server / Azure SQL Database." @@ -263,14 +228,14 @@ private static Option AccessToken() => //TIMEOUT OPTIONS - private static Option CommandTimeout() => + private static readonly Option CommandTimeout = new("--commandtimeout", "-ct") { Description = "This is the timeout when commands are run. This is not for admin commands or restore.", DefaultValueFactory = _ => DefaultCommandTimeout }; - private static Option CommandTimeoutAdmin() => + private static readonly Option CommandTimeoutAdmin = new("--admincommandtimeout", "-cta") { Description = "This is the timeout when administration commands are run (except for restore, which has its own).", @@ -278,33 +243,33 @@ private static Option CommandTimeoutAdmin() => }; //DATABASE OPTIONS - private static Option DatabaseType() => + private static readonly Option DatabaseType = new("--databasetype", "--dt", "--dbt") { Description = "TELLS GRATE WHAT TYPE OF DATABASE IT IS RUNNING ON.", DefaultValueFactory = _ => Configuration.DatabaseType.SQLServer }; - private static Option RunInTransaction() => + private static readonly Option RunInTransaction = new("--transaction", "--trx", "-t") { Description = "Run the migration in a transaction" }; - private static Option SchemaName() => + private static readonly Option SchemaName = new("--schemaname", "--sc", "--schema") { Description = "The schema to use for the migration tables", DefaultValueFactory = _ => "grate" }; - private static Option Drop() => + private static readonly Option Drop = new("--drop") { Description = "Drop - This instructs grate to remove the target database. Unlike RoundhousE grate will continue to run the migration scripts after the drop." }; - private static Option CreateDatabase() => + private static readonly Option CreateDatabase = new("--createdatabase", "--create") { Description = "Create - This instructs grate to create the target database if it does not exist. Defaults to true. Set to false to emulate the --donotcreatedatabase flag in roundhouse.", @@ -313,7 +278,7 @@ private static Option CreateDatabase() => //ENVIRONMENT OPTIONS - private static Option Environments() => + private static readonly Option Environments = new("--environment", "--env") { // A custom parser is needed to support combining environments separated by space, ',' or ';'. @@ -326,13 +291,13 @@ private static Option CreateDatabase() => //WARNING OPTIONS - private static Option WarnAndRunOnScriptChange() => + private static readonly Option WarnAndRunOnScriptChange = new("--warnononetimescriptchanges", "-w") { Description = "WarnOnOneTimeScriptChanges - Instructs grate to execute changed one time scripts(DDL / DML in Up folder) that have previously been run against the database instead of failing. A warning is logged for each one time script that is rerun. Defaults to false." }; - private static Option WarnAndIgnoreOnScriptChange() => + private static readonly Option WarnAndIgnoreOnScriptChange = new("--warnandignoreononetimescriptchanges") { Description = "WarnAndIgnoreOnOneTimeScriptChanges - Instructs grate to ignore and update the hash of changed one time scripts (DDL/DML in Up folder) that have previously been run against the database instead of failing. A warning is logged for each one time scripts that is rerun. Defaults to false." @@ -340,13 +305,13 @@ private static Option WarnAndIgnoreOnScriptChange() => //TOKEN OPTIONS - private static Option Tokens() => + private static readonly Option Tokens = new("--disabletokenreplacement", "--disabletokens") { Description = "Tokens - This instructs grate to not perform token replacement ({{somename}}). Defaults to false." }; - private static Option> UserTokens() => + private static Option> UserTokens = new("--usertokens", "--ut") { Description = "User Tokens - Allows grate to perform token replacement on custom tokens ({{my_token}}). Set as a key=value pair, eg '--ut=my_token=myvalue'. Can be specified multiple times." @@ -354,13 +319,13 @@ private static Option> UserTokens() => //SCRIPT OPTIONS - private static Option DoNotStoreScriptText() => + private static readonly Option DoNotStoreScriptText = new("--donotstorescriptsruntext") { Description = "DoNotStoreScriptsRunText - This instructs grate to not store the full script text in the database. Defaults to false." }; - private static Option RunAllAnyTimeScripts() => + private static readonly Option RunAllAnyTimeScripts = new("--runallanytimescripts", "--forceanytimescripts") { Description = "RunAllAnyTimeScripts - This instructs grate to run any time scripts every time it is run even if they haven't changed. Defaults to false." @@ -368,37 +333,37 @@ private static Option RunAllAnyTimeScripts() => //MISC OPTIONS - private static Option Baseline() => + private static readonly Option Baseline = new("--baseline") { Description = "Baseline - This instructs grate to mark the scripts as run, but not to actually run anything against the database. Use this option if you already have scripts that have been run through other means (and BEFORE you start the new ones)." }; - private static Option DryRun() => + private static readonly Option DryRun = new("--dryrun") { Description = " DryRun - This instructs grate to log what would have run, but not to actually run anything against the database. Use this option if you are trying to figure out what grate is going to do." }; - private static Option Restore() => + private static readonly Option Restore = new("--restore") { Description = " Restore - This instructs grate where to get the backed up database file. Defaults to NULL." }; - private static Option Silent() => + private static readonly Option Silent = new("--noninteractive", "-ni", "--ni", "--silent") { Description = "Silent - tells grate not to ask for any input when it runs." }; - private static Option RepositoryPath() => + private static readonly Option RepositoryPath = new("--repositorypath", "-r", "--repo") { Description = "Repository Path - The repository. A string that can be anything. Used to track versioning along with the version. Defaults to NULL." }; - private static Option Version() => + private static readonly Option Version = new("--version") { Description = "Database Version - specify the version of the current migration directly on the command line." @@ -406,32 +371,32 @@ private static Option Version() => //OBSOLETE OPTIONS - private static Option Database() => + private static readonly Option Database = new("--database") { Description = "OBSOLETE: Please specify the connection string instead", Required = false }; - private static Option ServerName() => + private static readonly Option ServerName = new("--servername", "--instance", "--server", "-s") { Description = "OBSOLETE: Please specify the connection string instead." }; - private static Option IgnoreDirectoryNames() => + private static readonly Option IgnoreDirectoryNames = new("--ignoredirectorynames", "--searchallinsteadoftraverse", "--searchallsubdirectoriesinsteadoftraverse") { Description = "IgnoreDirectoryNames - By default, scripts are ordered by relative path including subdirectories. This option searches subdirectories, but order is based on filename alone." }; - private static Option UpToDateCheck() => + private static readonly Option UpToDateCheck = new("--uptodatecheck", "--isuptodate") { Description = "Outputs whether the database is up to date or not (whether any non-everytime scripts would be run)" }; - private static Option Verbosity() => + private static readonly Option Verbosity = new("--verbosity", "-v") { Description = "Verbosity level (as defined here: https://docs.microsoft.com/dotnet/api/Microsoft.Extensions.Logging.LogLevel)" diff --git a/unittests/Basic_tests/CommandLineParsing/Basic_CommandLineParsing.cs b/unittests/Basic_tests/CommandLineParsing/Basic_CommandLineParsing.cs index 1ac416be..0c4fa8ba 100644 --- a/unittests/Basic_tests/CommandLineParsing/Basic_CommandLineParsing.cs +++ b/unittests/Basic_tests/CommandLineParsing/Basic_CommandLineParsing.cs @@ -1,34 +1,27 @@ -using System.CommandLine; -using System.Configuration; +using System.Configuration; using FluentAssertions; using grate.Commands; using grate.Configuration; using grate.Exceptions; using grate.Infrastructure; - -#if NET6_0 -using Dir = TestCommon.TestInfrastructure.Net6PolyFills.Directory; -#else using Dir = System.IO.Directory; -#endif namespace Basic_tests.CommandLineParsing; // ReSharper disable once InconsistentNaming public class Basic_CommandLineParsing { - [Fact] - public void ParserIsConfiguredCorrectly() + + [Theory] + [InlineData("")] + [InlineData("-ct=100")] + public void ParserIsConfiguredCorrectly(string commandline) { - // Test that the command configuration is valid, see https://github.com/dotnet/command-line-api/issues/1613 - // In System.CommandLine, invalid configuration (e.g. duplicate option aliases) throws while the - // command is being constructed and when it parses, so constructing and parsing must not throw. - var act = () => - { - var command = new MigrateCommand(null!); - _ = command.Parse(""); - }; - act.Should().NotThrow(); + var command = new MigrateCommand(null!); + var parseResult = command.Parse(commandline); + Assert.NotNull(parseResult.Errors); + Assert.Single(parseResult.Errors); + Assert.Equal("Option '--connectionstring' is required.", parseResult.Errors[0].Message); } [Theory] @@ -190,8 +183,8 @@ public async Task WithoutTransaction(string argName) cfg?.Transaction.Should().Be(false); } - - + + /// /// We can use multiple environments, separated by space, ; or , /// This makes it possible to create orhotogonal environments, and run scripts @@ -223,16 +216,16 @@ public async Task WithoutTransaction(string argName) /// [Theory] - [InlineData("--env KASHMIR", new[] {"KASHMIR"})] - [InlineData("--env JALLA", new[] {"JALLA"})] - [InlineData("--env JALLA KASHMIR", new[] {"JALLA", "KASHMIR"})] - [InlineData("--env JALLA,BERGEN", new[] {"JALLA", "BERGEN"})] - [InlineData("--env Dev;Azure;OnlyOnMondays", new[] {"Dev", "Azure", "OnlyOnMondays"})] - [InlineData("--env Customer1;Azure;Dev", new[] {"Customer1", "Azure", "Dev"})] - [InlineData("--env Customer1;Azure;Test", new[] {"Customer1", "Azure", "Test"})] - [InlineData("--env Customer2;Azure;Dev", new[] {"Customer2", "Azure", "Dev"})] - [InlineData("--env Customer2;Aws;QA", new[] {"Customer2", "Aws", "QA"})] - [InlineData("--env Customer2;Azure;Prod", new[] {"Customer2", "Azure", "Prod"})] + [InlineData("--env KASHMIR", new[] { "KASHMIR" })] + [InlineData("--env JALLA", new[] { "JALLA" })] + [InlineData("--env JALLA KASHMIR", new[] { "JALLA", "KASHMIR" })] + [InlineData("--env JALLA,BERGEN", new[] { "JALLA", "BERGEN" })] + [InlineData("--env Dev;Azure;OnlyOnMondays", new[] { "Dev", "Azure", "OnlyOnMondays" })] + [InlineData("--env Customer1;Azure;Dev", new[] { "Customer1", "Azure", "Dev" })] + [InlineData("--env Customer1;Azure;Test", new[] { "Customer1", "Azure", "Test" })] + [InlineData("--env Customer2;Azure;Dev", new[] { "Customer2", "Azure", "Dev" })] + [InlineData("--env Customer2;Aws;QA", new[] { "Customer2", "Aws", "QA" })] + [InlineData("--env Customer2;Azure;Prod", new[] { "Customer2", "Azure", "Prod" })] public async Task Environments(string argName, IEnumerable expected) { var commandline = argName; @@ -386,7 +379,7 @@ public async Task IgnoreDirectoryNames(string args, bool expected) var cfg = await ParseGrateConfiguration(args); cfg?.IgnoreDirectoryNames.Should().Be(expected); } - + [Theory] [InlineData("", false)] [InlineData("--isuptodate", true)] diff --git a/unittests/Basic_tests/GrateMigrator_MigrationStatus/IsUpToDate_.cs b/unittests/Basic_tests/GrateMigrator_MigrationStatus/IsUpToDate_.cs index 20b31ef4..7b800877 100644 --- a/unittests/Basic_tests/GrateMigrator_MigrationStatus/IsUpToDate_.cs +++ b/unittests/Basic_tests/GrateMigrator_MigrationStatus/IsUpToDate_.cs @@ -4,17 +4,11 @@ using grate.Infrastructure; using grate.Migration; using NSubstitute; - -#if NET6_0 -using Dir = TestCommon.TestInfrastructure.Net6PolyFills.Directory; -#else using Dir = System.IO.Directory; -#endif - namespace Basic_tests.GrateMigrator_MigrationStatus; // ReSharper disable once InconsistentNaming -public class IsUpToDate_: IDisposable +public class IsUpToDate_ : IDisposable { private static readonly DirectoryInfo SqlFilesDirectory = Dir.CreateTempSubdirectory(); @@ -25,24 +19,24 @@ public async Task False_if_scripts_run(bool dryRun) { var folders = new Dictionary> { - { "up", + { "up", [ ("script_that_is_run.sql", "-- ThisIsRun"), ("script_that_is_not_run.sql", "-- ThisIsNotRun") ] } }; - + var grateMigrator = CreateMigrator(folders, dryRun); await grateMigrator.Migrate(); grateMigrator.MigrationResult.Should().NotBeNull(); grateMigrator.MigrationResult.IsUpToDate.Should().BeFalse(); - + _logger.LoggedMessages.Should().Contain("Up to date: False"); _logger.LoggedMessages.Should().Contain("Changed script: script_that_is_run.sql"); } - + [Theory] [InlineData(true)] [InlineData(false)] @@ -50,21 +44,21 @@ public async Task True_if_no_scripts_run(bool dryRun) { var folders = new Dictionary> { - { "up", + { "up", [ ("script_that_is_not_run_either.sql", "-- ThisIsDefinitelyNotRun"), ("script_that_is_not_run.sql", "-- ThisIsNotRun") ] } }; - + var grateMigrator = CreateMigrator(folders, dryRun); await grateMigrator.Migrate(); grateMigrator.MigrationResult.Should().NotBeNull(); grateMigrator.MigrationResult.IsUpToDate.Should().BeTrue(); } - + [Theory] [InlineData(true)] [InlineData(false)] @@ -72,27 +66,27 @@ public async Task True_if_only_everytime_scripts_run(bool dryRun) { var folders = new Dictionary> { - { "up", + { "up", [ ("script_that_is_not_run_either.sql", "-- ThisIsDefinitelyNotRun"), ("script_that_is_not_run.sql", "-- ThisIsNotRun") ] }, - { "permissions", + { "permissions", [ ("script_that_is_run.sql", "-- ThisIsRun"), ("script_that_is_not_run.sql", "-- ThisIsNotRun") ] } }; - + var grateMigrator = CreateMigrator(folders, dryRun); await grateMigrator.Migrate(); grateMigrator.MigrationResult.Should().NotBeNull(); grateMigrator.MigrationResult.IsUpToDate.Should().BeTrue(); } - + private GrateMigrator CreateMigrator(Dictionary> scripts, bool dryRun) { foreach (var folder in scripts.Keys) @@ -101,7 +95,7 @@ private GrateMigrator CreateMigrator(Dictionary> { var parent = Path.Combine(SqlFilesDirectory.ToString(), folder); Directory.CreateDirectory(parent); - + var fullPath = Path.Combine(parent, filename); File.WriteAllText(fullPath, content); } diff --git a/unittests/Basic_tests/GrateMigrator_MigrationStatus/MigrationStatus_.cs b/unittests/Basic_tests/GrateMigrator_MigrationStatus/MigrationStatus_.cs index 1320779f..2e9575ca 100644 --- a/unittests/Basic_tests/GrateMigrator_MigrationStatus/MigrationStatus_.cs +++ b/unittests/Basic_tests/GrateMigrator_MigrationStatus/MigrationStatus_.cs @@ -4,17 +4,13 @@ using grate.Infrastructure; using grate.Migration; using NSubstitute; - -#if NET6_0 -using Dir = TestCommon.TestInfrastructure.Net6PolyFills.Directory; -#else using Dir = System.IO.Directory; -#endif + namespace Basic_tests.GrateMigrator_MigrationStatus; // ReSharper disable once InconsistentNaming -public class MigrationStatus_: IDisposable +public class MigrationStatus_ : IDisposable { private static readonly DirectoryInfo SqlFilesDirectory = Dir.CreateTempSubdirectory(); @@ -38,7 +34,7 @@ private static GrateMigrator CreateMigrator(List<(string, string)> scripts) { var parent = Path.Combine(SqlFilesDirectory.ToString(), "up"); Directory.CreateDirectory(parent); - + var fullPath = Path.Combine(parent, filename); File.WriteAllText(fullPath, content); } diff --git a/unittests/Docker/Docker.Common/TestInfrastructure/DockerGrateMigrator.cs b/unittests/Docker/Docker.Common/TestInfrastructure/DockerGrateMigrator.cs index 8f4a508e..58de63f6 100644 --- a/unittests/Docker/Docker.Common/TestInfrastructure/DockerGrateMigrator.cs +++ b/unittests/Docker/Docker.Common/TestInfrastructure/DockerGrateMigrator.cs @@ -2,23 +2,17 @@ using System.Text.Json.Serialization; using DotNet.Testcontainers.Builders; using DotNet.Testcontainers.Networks; -using grate; using grate.Commands; using grate.Configuration; using grate.Exceptions; using grate.Migration; using Microsoft.Extensions.Logging; - -#if NET6_0 -using Dir = TestCommon.TestInfrastructure.Net6PolyFills.Directory; -#else using Dir = System.IO.Directory; -#endif namespace Docker.Common.TestInfrastructure; public record DockerGrateMigrator( - DatabaseType DatabaseType, + DatabaseType DatabaseType, ILogger Logger, INetwork Network ) @@ -36,10 +30,10 @@ public async Task Migrate() // /home/app/.net/grate/fdcA3gxdjBiIcVt0mGoBJ5IgxSbD0kE=/libe_sqlite3.so: failed to map segment from shared object // (similarly) /tmp/dotnet-bundle-extract/grate/fdcA3gxdjBiIcVt0mGoBJ5IgxSbD0kE=/libe_sqlite3.so: failed to map segment from shared object var tmpFolder = Dir.CreateTempSubdirectory().ToString(); - + // Need to map the SQL files directory to the container var sqlFilesDirectory = Configuration.SqlFilesDirectory.ToString(); - + Dictionary bindMounts = new Dictionary { { tmpFolder, "/home/app" }, @@ -50,29 +44,29 @@ public async Task Migrate() // both from the host and the container. if (DatabaseType == DatabaseType.SQLite) { - foreach (var connectionString in new [] {Configuration.ConnectionString, Configuration.AdminConnectionString}) + foreach (var connectionString in new[] { Configuration.ConnectionString, Configuration.AdminConnectionString }) { var dbFile = connectionString?.Split("=", 2)[1]; var folder = Path.GetDirectoryName(dbFile)!; bindMounts[folder] = folder; } } - + // Convert configuration to command-line arguments var convertToDockerArguments = ConvertToDockerArguments(Configuration); var dockerArguments = convertToDockerArguments.ToList(); - + // Add the database type dockerArguments.Add("--databasetype=" + DatabaseType.ToString().ToLowerInvariant()); - + //dockerArguments.Add("--verbosity=debug"); - + // Needed when overriding the entrypoint, not the command dockerArguments.Insert(0, "./grate"); var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(20)); var token = cancellationTokenSource.Token; - + var containerBuilder = new ContainerBuilder(DockerImage) .WithEntrypoint(dockerArguments.ToArray()) .WithEnvironment("DOTNET_BUNDLE_EXTRACT_BASE_DIR", "/home/app") @@ -87,7 +81,7 @@ public async Task Migrate() { containerBuilder = containerBuilder.WithBindMount(bindMount.Key, bindMount.Value); } - + await using var container = containerBuilder.Build(); try @@ -134,7 +128,7 @@ public async Task Migrate() } public GrateConfiguration Configuration { get; private init; } = null!; - + public IGrateMigrator WithConfiguration(GrateConfiguration configuration) { return this with @@ -152,18 +146,21 @@ public IGrateMigrator WithConfiguration(Action builde { var b = GrateConfigurationBuilder.Create(Configuration); builder.Invoke(b); - return this with { Configuration = b.Build() with + return this with { + Configuration = b.Build() with + { // Need to overwrite the output path, as we don't have the same tmp folders on the host as in the container, // and the root file system is read-only in the test container OutputPath = new DirectoryInfo(Path.Combine("/tmp", "grate-tests-output", Dir.CreateTempSubdirectory().Name)) - }}; + } + }; } - + public IGrateMigrator WithDatabase(IDatabase database) => this with { Database = database }; public IDatabase? Database { get; set; } - - + + private List ConvertToDockerArguments(GrateConfiguration configuration) { List result = new(); @@ -177,7 +174,7 @@ private List ConvertToDockerArguments(GrateConfiguration configuration) // Skip properties with default values var value = prop.GetValue(configuration); var defaultValue = prop.GetValue(GrateConfiguration.Default); - + var serializedValue = JsonSerializer.Serialize(value?.ToString(), SerializerOptions); var serializedDefault = JsonSerializer.Serialize(defaultValue?.ToString(), SerializerOptions); @@ -191,7 +188,7 @@ private List ConvertToDockerArguments(GrateConfiguration configuration) { value = string.Join(';', foldersConfiguration.Values); } - + var name = prop.Name; // In System.CommandLine the option Name now includes the prefix (e.g. "--connectionstring") // and Aliases no longer contains the name, so match on the prefix-stripped name and use @@ -216,7 +213,7 @@ private List ConvertToDockerArguments(GrateConfiguration configuration) return result; #pragma warning restore CS0162 // Unreachable code detected } - + private static readonly JsonSerializerOptions SerializerOptions = new(JsonSerializerOptions.Default) { ReferenceHandler = ReferenceHandler.IgnoreCycles diff --git a/unittests/Sqlite/TestInfrastructure/SqliteTestDatabase.cs b/unittests/Sqlite/TestInfrastructure/SqliteTestDatabase.cs index 156c7af6..42f0ff32 100644 --- a/unittests/Sqlite/TestInfrastructure/SqliteTestDatabase.cs +++ b/unittests/Sqlite/TestInfrastructure/SqliteTestDatabase.cs @@ -1,10 +1,4 @@ -using Xunit.Sdk; - -#if NET6_0 -using Dir = TestCommon.TestInfrastructure.Net6PolyFills.Directory; -#else -using Dir = System.IO.Directory; -#endif +using Dir = System.IO.Directory; namespace TestCommon.TestInfrastructure; @@ -27,7 +21,7 @@ public ValueTask InitializeAsync() { return ValueTask.CompletedTask; } - + public string AdminConnectionString => $"Data Source={Wrap("grate-sqlite.db")}"; public string ConnectionString(string database) => $"Data Source={Wrap(database + ".db")}"; public string UserConnectionString(string database) => $"Data Source={Wrap(database + ".db")}"; diff --git a/unittests/TestCommon/TestInfrastructure/DatabaseHelpers.cs b/unittests/TestCommon/TestInfrastructure/DatabaseHelpers.cs index 092a5666..3383640b 100644 --- a/unittests/TestCommon/TestInfrastructure/DatabaseHelpers.cs +++ b/unittests/TestCommon/TestInfrastructure/DatabaseHelpers.cs @@ -2,13 +2,7 @@ using System.Transactions; using Dapper; using Microsoft.Data.Sqlite; -using Xunit.Sdk; - -#if NET6_0 -using Dir = TestCommon.TestInfrastructure.Net6PolyFills.Directory; -#else using Dir = System.IO.Directory; -#endif namespace TestCommon.TestInfrastructure; @@ -102,7 +96,7 @@ internal static async Task> GetDatabases(this IGrateTestCont } return databases.ToArray(); } - + public static async Task CreateSqliteDatabaseFromConnectionString(string connectionString) { await using var conn = new SqliteConnection(connectionString); @@ -123,7 +117,7 @@ public static async Task CreateSqliteDatabaseFromConnectionString(string connect public static async Task> GetSqliteDatabases(this IGrateTestContext context) { var builder = new SqliteConnectionStringBuilder(context.AdminConnectionString); - var root = Path.GetDirectoryName(builder.DataSource) ?? Dir.CreateTempSubdirectory().ToString() ; + var root = Path.GetDirectoryName(builder.DataSource) ?? Dir.CreateTempSubdirectory().ToString(); var dbFiles = Directory.EnumerateFiles(root, "*.db"); IEnumerable dbNames = dbFiles .Select(Path.GetFileNameWithoutExtension) @@ -133,5 +127,5 @@ public static async Task> GetSqliteDatabases(this IGrateTest return await ValueTask.FromResult(dbNames); } - + } diff --git a/unittests/TestCommon/TestInfrastructure/IGrateTestContext.cs b/unittests/TestCommon/TestInfrastructure/IGrateTestContext.cs index b258ea54..a6457717 100644 --- a/unittests/TestCommon/TestInfrastructure/IGrateTestContext.cs +++ b/unittests/TestCommon/TestInfrastructure/IGrateTestContext.cs @@ -1,16 +1,8 @@ using System.Data; -using System.Diagnostics.CodeAnalysis; -using System.Runtime.CompilerServices; using grate.Configuration; using grate.Infrastructure; using grate.Migration; -using Microsoft.Extensions.Logging; - -#if NET6_0 -using Dir = TestCommon.TestInfrastructure.Net6PolyFills.Directory; -#else using Dir = System.IO.Directory; -#endif namespace TestCommon.TestInfrastructure; @@ -19,7 +11,7 @@ public interface IGrateTestContext string AdminConnectionString { get; } string ConnectionString(string database); string UserConnectionString(string database); - + IGrateTestContext External => this; IDbConnection CreateAdminDbConnection() => GetDbConnection(AdminConnectionString); @@ -50,7 +42,7 @@ public interface IGrateTestContext public IGrateMigrator Migrator { get; } - + //public bool SupportsSchemas => Migrator.SupportsSchemas(); bool SupportsCreateDatabase { get; } diff --git a/unittests/TestCommon/TestInfrastructure/Net6PolyFills.cs b/unittests/TestCommon/TestInfrastructure/Net6PolyFills.cs deleted file mode 100644 index 5fd53abe..00000000 --- a/unittests/TestCommon/TestInfrastructure/Net6PolyFills.cs +++ /dev/null @@ -1,18 +0,0 @@ -namespace TestCommon.TestInfrastructure; -using System.IO; - -#if NET6_0 -public static class Net6PolyFills -{ - public static class Directory - { - public static DirectoryInfo CreateTempSubdirectory() - => CreateTempSubdirectory(Path.GetRandomFileName()); - - public static DirectoryInfo CreateTempSubdirectory(string name) - { - return new DirectoryInfo(Path.GetTempPath()).CreateSubdirectory(name); - } - } -} -#endif diff --git a/unittests/TestCommon/TestInfrastructure/TestConfig.cs b/unittests/TestCommon/TestInfrastructure/TestConfig.cs index bdbb4e09..e61868d1 100644 --- a/unittests/TestCommon/TestInfrastructure/TestConfig.cs +++ b/unittests/TestCommon/TestInfrastructure/TestConfig.cs @@ -1,12 +1,7 @@ using Microsoft.Extensions.Logging; using static System.StringComparison; using static System.StringSplitOptions; - -#if NET6_0 -using Dir = TestCommon.TestInfrastructure.Net6PolyFills.Directory; -#else using Dir = System.IO.Directory; -#endif namespace TestCommon.TestInfrastructure; @@ -23,22 +18,22 @@ public static DirectoryInfo CreateRandomTempDirectory() public static DirectoryInfo Wrap(DirectoryInfo root, string? subFolder) => new(Path.Combine(root.ToString(), subFolder ?? "")); public static string? Username(string connectionString) => connectionString.Split(";", TrimEntries | RemoveEmptyEntries) - .SingleOrDefault(entry => - entry.StartsWith("Uid", OrdinalIgnoreCase) || - entry.StartsWith("User Id", OrdinalIgnoreCase) || + .SingleOrDefault(entry => + entry.StartsWith("Uid", OrdinalIgnoreCase) || + entry.StartsWith("User Id", OrdinalIgnoreCase) || entry.StartsWith("Username", OrdinalIgnoreCase)) ? .Split("=", TrimEntries | RemoveEmptyEntries).Last(); public static string? Password(string connectionString) => connectionString.Split(";", TrimEntries | RemoveEmptyEntries) - .SingleOrDefault(entry => - entry.StartsWith("Password", OrdinalIgnoreCase) || + .SingleOrDefault(entry => + entry.StartsWith("Password", OrdinalIgnoreCase) || entry.StartsWith("Pwd", OrdinalIgnoreCase))? .Split("=", TrimEntries | RemoveEmptyEntries).Last(); public static LogLevel GetLogLevel() => LogLevelFromEnvironmentVariable(); - - + + private static LogLevel LogLevelFromEnvironmentVariable() { if (!Enum.TryParse(Environment.GetEnvironmentVariable("LogLevel"), out LogLevel logLevel)) From b8b9cfd65a1764431e16e3ae5bc65ec6a8c885bf Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 22 Jun 2026 12:08:48 +0700 Subject: [PATCH 4/4] Fix the code review. --- src/grate/Commands/MigrateCommand.cs | 60 ++++++++++++++-------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/grate/Commands/MigrateCommand.cs b/src/grate/Commands/MigrateCommand.cs index b24cc209..d5549627 100644 --- a/src/grate/Commands/MigrateCommand.cs +++ b/src/grate/Commands/MigrateCommand.cs @@ -122,7 +122,7 @@ public CommandLineGrateConfiguration GetConfiguration(ParseResult parseResult) } //REQUIRED OPTIONS - private static readonly Option ConnectionString = + private readonly Option ConnectionString = new("--connectionstring", "-c", "-cs", "--connstring") { Description = "You now provide an entire connection string. ServerName and Database are obsolete.", @@ -131,7 +131,7 @@ public CommandLineGrateConfiguration GetConfiguration(ParseResult parseResult) //CONNECTIONSTRING OPTIONS - private static readonly Option AdminConnectionString = + private readonly Option AdminConnectionString = new("--adminconnectionstring", "-csa", "-a", "-acs", "--adminconnstring") { Description = "The connection string for connecting to master, if you want to create the database. Defaults to the same as --connstring.", @@ -140,21 +140,21 @@ public CommandLineGrateConfiguration GetConfiguration(ParseResult parseResult) //DIRECTORY OPTIONS - private static readonly Option SqlFilesDirectory = + private readonly Option SqlFilesDirectory = new Option("--sqlfilesdirectory", "-f", "--files") { Description = "The directory where your SQL scripts are", DefaultValueFactory = _ => new DirectoryInfo(DefaultFilesDirectory) }.AcceptExistingOnly(); - private static readonly Option OutputPath = + private readonly Option OutputPath = new("--outputPath", "-o", "--output") { Description = "This is where everything related to the migration is stored. This includes any backups, all items that ran, permission dumps, logs, etc.", DefaultValueFactory = _ => new DirectoryInfo(DefaultOutputPath) }; - private static readonly Option Folders = + private readonly Option Folders = new("--folders") { CustomParser = result => @@ -220,7 +220,7 @@ defaults to Default //SECURITY OPTIONS - private static readonly Option AccessToken = + private readonly Option AccessToken = new("--accesstoken") { Description = "Access token to be used for logging in to SQL Server / Azure SQL Database." @@ -228,14 +228,14 @@ defaults to Default //TIMEOUT OPTIONS - private static readonly Option CommandTimeout = + private readonly Option CommandTimeout = new("--commandtimeout", "-ct") { Description = "This is the timeout when commands are run. This is not for admin commands or restore.", DefaultValueFactory = _ => DefaultCommandTimeout }; - private static readonly Option CommandTimeoutAdmin = + private readonly Option CommandTimeoutAdmin = new("--admincommandtimeout", "-cta") { Description = "This is the timeout when administration commands are run (except for restore, which has its own).", @@ -243,33 +243,33 @@ defaults to Default }; //DATABASE OPTIONS - private static readonly Option DatabaseType = + private readonly Option DatabaseType = new("--databasetype", "--dt", "--dbt") { Description = "TELLS GRATE WHAT TYPE OF DATABASE IT IS RUNNING ON.", DefaultValueFactory = _ => Configuration.DatabaseType.SQLServer }; - private static readonly Option RunInTransaction = + private readonly Option RunInTransaction = new("--transaction", "--trx", "-t") { Description = "Run the migration in a transaction" }; - private static readonly Option SchemaName = + private readonly Option SchemaName = new("--schemaname", "--sc", "--schema") { Description = "The schema to use for the migration tables", DefaultValueFactory = _ => "grate" }; - private static readonly Option Drop = + private readonly Option Drop = new("--drop") { Description = "Drop - This instructs grate to remove the target database. Unlike RoundhousE grate will continue to run the migration scripts after the drop." }; - private static readonly Option CreateDatabase = + private readonly Option CreateDatabase = new("--createdatabase", "--create") { Description = "Create - This instructs grate to create the target database if it does not exist. Defaults to true. Set to false to emulate the --donotcreatedatabase flag in roundhouse.", @@ -278,7 +278,7 @@ defaults to Default //ENVIRONMENT OPTIONS - private static readonly Option Environments = + private readonly Option Environments = new("--environment", "--env") { // A custom parser is needed to support combining environments separated by space, ',' or ';'. @@ -291,13 +291,13 @@ defaults to Default //WARNING OPTIONS - private static readonly Option WarnAndRunOnScriptChange = + private readonly Option WarnAndRunOnScriptChange = new("--warnononetimescriptchanges", "-w") { Description = "WarnOnOneTimeScriptChanges - Instructs grate to execute changed one time scripts(DDL / DML in Up folder) that have previously been run against the database instead of failing. A warning is logged for each one time script that is rerun. Defaults to false." }; - private static readonly Option WarnAndIgnoreOnScriptChange = + private readonly Option WarnAndIgnoreOnScriptChange = new("--warnandignoreononetimescriptchanges") { Description = "WarnAndIgnoreOnOneTimeScriptChanges - Instructs grate to ignore and update the hash of changed one time scripts (DDL/DML in Up folder) that have previously been run against the database instead of failing. A warning is logged for each one time scripts that is rerun. Defaults to false." @@ -305,7 +305,7 @@ defaults to Default //TOKEN OPTIONS - private static readonly Option Tokens = + private readonly Option Tokens = new("--disabletokenreplacement", "--disabletokens") { Description = "Tokens - This instructs grate to not perform token replacement ({{somename}}). Defaults to false." @@ -319,13 +319,13 @@ defaults to Default //SCRIPT OPTIONS - private static readonly Option DoNotStoreScriptText = + private readonly Option DoNotStoreScriptText = new("--donotstorescriptsruntext") { Description = "DoNotStoreScriptsRunText - This instructs grate to not store the full script text in the database. Defaults to false." }; - private static readonly Option RunAllAnyTimeScripts = + private readonly Option RunAllAnyTimeScripts = new("--runallanytimescripts", "--forceanytimescripts") { Description = "RunAllAnyTimeScripts - This instructs grate to run any time scripts every time it is run even if they haven't changed. Defaults to false." @@ -333,37 +333,37 @@ defaults to Default //MISC OPTIONS - private static readonly Option Baseline = + private readonly Option Baseline = new("--baseline") { Description = "Baseline - This instructs grate to mark the scripts as run, but not to actually run anything against the database. Use this option if you already have scripts that have been run through other means (and BEFORE you start the new ones)." }; - private static readonly Option DryRun = + private readonly Option DryRun = new("--dryrun") { Description = " DryRun - This instructs grate to log what would have run, but not to actually run anything against the database. Use this option if you are trying to figure out what grate is going to do." }; - private static readonly Option Restore = + private readonly Option Restore = new("--restore") { Description = " Restore - This instructs grate where to get the backed up database file. Defaults to NULL." }; - private static readonly Option Silent = + private readonly Option Silent = new("--noninteractive", "-ni", "--ni", "--silent") { Description = "Silent - tells grate not to ask for any input when it runs." }; - private static readonly Option RepositoryPath = + private readonly Option RepositoryPath = new("--repositorypath", "-r", "--repo") { Description = "Repository Path - The repository. A string that can be anything. Used to track versioning along with the version. Defaults to NULL." }; - private static readonly Option Version = + private readonly Option Version = new("--version") { Description = "Database Version - specify the version of the current migration directly on the command line." @@ -371,32 +371,32 @@ defaults to Default //OBSOLETE OPTIONS - private static readonly Option Database = + private readonly Option Database = new("--database") { Description = "OBSOLETE: Please specify the connection string instead", Required = false }; - private static readonly Option ServerName = + private readonly Option ServerName = new("--servername", "--instance", "--server", "-s") { Description = "OBSOLETE: Please specify the connection string instead." }; - private static readonly Option IgnoreDirectoryNames = + private readonly Option IgnoreDirectoryNames = new("--ignoredirectorynames", "--searchallinsteadoftraverse", "--searchallsubdirectoriesinsteadoftraverse") { Description = "IgnoreDirectoryNames - By default, scripts are ordered by relative path including subdirectories. This option searches subdirectories, but order is based on filename alone." }; - private static readonly Option UpToDateCheck = + private readonly Option UpToDateCheck = new("--uptodatecheck", "--isuptodate") { Description = "Outputs whether the database is up to date or not (whether any non-everytime scripts would be run)" }; - private static readonly Option Verbosity = + private readonly Option Verbosity = new("--verbosity", "-v") { Description = "Verbosity level (as defined here: https://docs.microsoft.com/dotnet/api/Microsoft.Extensions.Logging.LogLevel)"