Skip to content

Commit

Permalink
fix erikbra#230: Fix schema (in)sensitive casing issue
Browse files Browse the repository at this point in the history
  • Loading branch information
wokket committed Sep 6, 2022
1 parent 5dccb38 commit 8f92646
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public async Task Returns_The_New_Version_Id()
var db = TestConfig.RandomDatabase();

GrateMigrator? migrator;

var parent = CreateRandomTempDirectory();
var knownFolders = FoldersConfiguration.Default(null);
CreateDummySql(parent, knownFolders[Sprocs]);
Expand All @@ -49,7 +49,7 @@ public async Task Does_Not_Create_Versions_When_Dryrun()
{
//for bug #204 - when running --baseline and --dryrun on a new db it shouldn't create the grate schema's etc
var db = TestConfig.RandomDatabase();

var parent = CreateRandomTempDirectory();
var knownFolders = FoldersConfiguration.Default(null);

Expand Down Expand Up @@ -80,7 +80,7 @@ public async Task Creates_A_New_Version_In_Progress()
CreateDummySql(parent, knownFolders[Up]);

long newVersionId = 0;

await using (migrator = Context.GetMigrator(db, parent, knownFolders))
{
//Calling migrate here to setup the database and such.
Expand All @@ -100,4 +100,33 @@ public async Task Creates_A_New_Version_In_Progress()
var version = entries.Single(x => x.version == dbVersion);
version.status.Should().Be(MigrationStatus.InProgress);
}

[Test]
public async Task Bug230_Uses_Server_Casing_Rules_For_Schema()
{
//for bug #230 - when targeting an existing schema use the servers casing rules, not .Net's
var db = TestConfig.RandomDatabase();
var parent = CreateRandomTempDirectory();
var knownFolders = FoldersConfiguration.Default(null);

CreateDummySql(parent, knownFolders[Sprocs]); // make sure there's something that could be logged...

await using (var migrator = Context.GetMigrator(db, parent, knownFolders))
{
await migrator.Migrate();
Assert.True(await migrator.DbMigrator.Database.VersionTableExists()); // we migrated into the `grate` schema.
}

// Now we'll run again with the same name but different cased schema
var grateConfig = Context.GetConfiguration(db, parent, knownFolders) with
{
SchemaName = "GRATE"
};

await using (var migrator = Context.GetMigrator(grateConfig))
{
await migrator.Migrate(); // should either reuse the existing schema if a case-insensitive server, or create a new second schema for use if case-sensitive.
Assert.True(await migrator.DbMigrator.Database.VersionTableExists()); // we migrated into the `GRATE` schema, which may be the same as 'grate' depending on server settings.
}
}
}
32 changes: 16 additions & 16 deletions grate/Migration/AnsiSqlDatabase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public virtual Task InitializeConnections(GrateConfiguration configuration)
protected abstract DbConnection GetSqlConnection(string? connectionString);

protected DbConnection AdminConnection => _adminConnection ??= GetSqlConnection(AdminConnectionString);

protected DbConnection Connection => _connection ??= GetSqlConnection(ConnectionString);

public DbConnection ActiveConnection { protected get; set; } = default!;
Expand Down Expand Up @@ -110,8 +110,8 @@ protected async Task<TResult> RunInAutonomousTransaction<TResult>(string? connec
{
TResult res;
using (var s = new TransactionScope(
TransactionScopeOption.Suppress,
new TransactionOptions() { IsolationLevel = IsolationLevel.ReadUncommitted} ,
TransactionScopeOption.Suppress,
new TransactionOptions() { IsolationLevel = IsolationLevel.ReadUncommitted },
TransactionScopeAsyncFlowOption.Enabled))
{
await using (var connection = GetSqlConnection(connectionString))
Expand All @@ -124,7 +124,7 @@ protected async Task<TResult> RunInAutonomousTransaction<TResult>(string? connec
}
return res;
}

protected async Task RunInAutonomousTransaction(string? connectionString, Func<DbConnection, Task> func)
{
using (var s = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled))
Expand All @@ -138,7 +138,7 @@ protected async Task RunInAutonomousTransaction(string? connectionString, Func<D
s.Complete();
}
}

public async Task OpenActiveConnection()
{
await Open(ActiveConnection);
Expand Down Expand Up @@ -170,9 +170,9 @@ public async Task CreateDatabase()
if (!await DatabaseExists())
{
Logger.LogTrace("Creating database {DatabaseName}", DatabaseName);

await OpenAdminConnection();

var sql = _syntax.CreateDatabase(DatabaseName, Password);
await ExecuteNonQuery(AdminConnection, sql, Config?.AdminCommandTimeout);
}
Expand Down Expand Up @@ -256,7 +256,7 @@ private async Task<bool> RunSchemaExists()
{
string sql = $"SELECT s.schema_name FROM information_schema.schemata s WHERE s.schema_name = '{SchemaName}'";
var res = await ExecuteScalarAsync<string>(ActiveConnection, sql);
return res == SchemaName;
return res != null; // #230: If the server found a record that's good enough for us
}

// TODO: Change MySQL/MariaDB from using schemas to using grate_ prefix
Expand Down Expand Up @@ -348,7 +348,7 @@ public async Task<bool> TableExists(string schemaName, string tableName)
string existsSql = ExistsSql(tableSchema, fullTableName);

var res = await ExecuteScalarAsync<object>(ActiveConnection, existsSql);

return !DBNull.Value.Equals(res) && res is not null;
}

Expand Down Expand Up @@ -453,27 +453,27 @@ public void Rollback()

public async Task RunSql(string sql, ConnectionType connectionType, TransactionHandling transactionHandling)
{
Logger.LogTrace("[SQL] Running (on connection '{ConnType}', transaction handling '{TransactionHandling}'): \n{Sql}",
connectionType.ToString(),
Logger.LogTrace("[SQL] Running (on connection '{ConnType}', transaction handling '{TransactionHandling}'): \n{Sql}",
connectionType.ToString(),
transactionHandling,
sql);

int? timeout = GetTimeout(connectionType);
var connection = GetDbConnection(connectionType);

await ExecuteNonQuery(connection, sql, timeout);
}

private DbConnection GetDbConnection(ConnectionType connectionType) =>

private DbConnection GetDbConnection(ConnectionType connectionType) =>
connectionType switch
{
ConnectionType.Default => ActiveConnection,
ConnectionType.Admin => AdminConnection,
_ => throw new UnknownConnectionType(connectionType)
};
private int? GetTimeout(ConnectionType connectionType) =>

private int? GetTimeout(ConnectionType connectionType) =>
connectionType switch
{
ConnectionType.Default => Config?.CommandTimeout,
Expand Down
18 changes: 16 additions & 2 deletions grate/Migration/PostgreSqlDatabase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace grate.Migration;

public class PostgreSqlDatabase : AnsiSqlDatabase
{
public PostgreSqlDatabase(ILogger<PostgreSqlDatabase> logger)
public PostgreSqlDatabase(ILogger<PostgreSqlDatabase> logger)
: base(logger, new PostgreSqlSyntax())
{ }

Expand All @@ -20,4 +20,18 @@ public override Task RestoreDatabase(string backupPath)
{
throw new System.NotImplementedException("Restoring a database from file is not currently supported for Postgresql.");
}
}

protected override string ExistsSql(string tableSchema, string fullTableName)
{
// For #230. Postgres tables are lowercase by default unless you quote them when created, which we do. We _don't_ quote the schema though, so it will always be lowercase
// Ensure the table check uses the lowercase version of anything we're passed, as that's what we would have created.
return base.ExistsSql(tableSchema.ToLower(), fullTableName);
}

protected override string ExistsSql(string tableSchema, string fullTableName, string columnName)
{
// For #230. Postgres tables are lowercase by default unless you quote them when created, which we do. We _don't_ quote the schema though, so it will always be lowercase
// Ensure the table check uses the lowercase version of anything we're passed, as that's what we would have created.
return base.ExistsSql(tableSchema.ToLower(), fullTableName, columnName);
}
}
13 changes: 7 additions & 6 deletions grate/Migration/SqLiteDatabase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ namespace grate.Migration;
public class SqliteDatabase : AnsiSqlDatabase
{
private static readonly SqliteSyntax Syntax = new();
public SqliteDatabase(ILogger<SqliteDatabase> logger)


public SqliteDatabase(ILogger<SqliteDatabase> logger)
: base(logger, Syntax)
{ }

Expand All @@ -24,8 +24,9 @@ protected override string ExistsSql(string tableSchema, string fullTableName) =>
$@"
SELECT name FROM sqlite_master
WHERE type ='table' AND
name = '{fullTableName}';
";
name = '{fullTableName}' COLLATE NOCASE;
"; // #230: Correct mismatched schema casing, sqllite is case-insensitive but the string comparisons in queries _are_ case sensitive by default


protected override string ExistsSql(string tableSchema, string fullTableName, string columnName) =>
$@"SELECT * FROM pragma_table_info('{fullTableName}')
Expand All @@ -47,7 +48,7 @@ public override Task DropDatabase()
{
File.Delete(db);
}

return Task.CompletedTask;
}

Expand Down

0 comments on commit 8f92646

Please sign in to comment.