Skip to content

Commit

Permalink
Issue #342: Disable pipelining on MySQL/MariaDB connections if not ex…
Browse files Browse the repository at this point in the history
…plicitly enabled (#346)
  • Loading branch information
erikbra authored Jun 3, 2023
1 parent 99c9f47 commit 1f45209
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 1 deletion.
51 changes: 51 additions & 0 deletions grate.unittests/Basic/Infrastructure/MariaDB/MariaDbDatabase_.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using System.Data.Common;
using System.Threading.Tasks;
using FluentAssertions;
using grate.Configuration;
using grate.Migration;
using grate.unittests.TestInfrastructure;
using Microsoft.Data.SqlClient;
using Microsoft.Extensions.Logging;
using MySqlConnector;
using NUnit.Framework;

namespace grate.unittests.Basic.Infrastructure.MariaDB;

// ReSharper disable once InconsistentNaming
public class MariaDbDatabase_
{
[Test]
public async Task Disables_pipelining_if_not_explicitly_set_in_connection_string()
{
var connStr = "Server=dummy";
var cfg = new GrateConfiguration() { ConnectionString = connStr };
var mariaDb = new InspectableMariaDbDatabase();
await mariaDb.InitializeConnections(cfg);

var conn = mariaDb.GetConnection();
var builder = new MySqlConnectionStringBuilder(conn.ConnectionString);
builder.Pipelining.Should().BeFalse();
}

[Test]
public async Task Leaves_pipelining_as_configured_if_set_explicitly_in_connection_string()
{
var connStr = "Server=dummy;Pipelining=true";
var cfg = new GrateConfiguration() { ConnectionString = connStr };
var mariaDb = new InspectableMariaDbDatabase();
await mariaDb.InitializeConnections(cfg);

var conn = mariaDb.GetConnection();
var builder = new MySqlConnectionStringBuilder(conn.ConnectionString);
builder.Pipelining.Should().BeTrue();
}

private class InspectableMariaDbDatabase : MariaDbDatabase
{
public InspectableMariaDbDatabase() : base(TestConfig.LogFactory.CreateLogger<InspectableMariaDbDatabase>())
{
}

public DbConnection GetConnection() => base.Connection;
}
}
16 changes: 15 additions & 1 deletion grate/Migration/MariaDbDatabase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,21 @@ public MariaDbDatabase(ILogger<MariaDbDatabase> logger)

public override bool SupportsDdlTransactions => false;
public override bool SupportsSchemas => false;
protected override DbConnection GetSqlConnection(string? connectionString) => new MySqlConnection(connectionString);

protected override DbConnection GetSqlConnection(string? connectionString)
{
// If pipelining is not explicitly mentioned in the connection string, turn it off, as enabling it
// might lead to problems in more scenarios than it (potentially) solves, in the most
// common grate scenarios.
if (!(connectionString ?? "").Contains("Pipelining", StringComparison.InvariantCultureIgnoreCase))
{
var builder = new MySqlConnectionStringBuilder(connectionString) { Pipelining = false };
connectionString = builder.ConnectionString;
}

var conn = new MySqlConnection(connectionString);
return conn;
}

public override Task RestoreDatabase(string backupPath)
{
Expand Down

0 comments on commit 1f45209

Please sign in to comment.