From 1f452092dfaf3cee4b3629b0a0f46ab7504c5fae Mon Sep 17 00:00:00 2001
From: "Erik A. Brandstadmoen" <erik@brandstadmoen.net>
Date: Sun, 4 Jun 2023 00:13:15 +0200
Subject: [PATCH] Issue #342: Disable pipelining on MySQL/MariaDB connections
 if not explicitly enabled (#346)

---
 .../MariaDB/MariaDbDatabase_.cs               | 51 +++++++++++++++++++
 grate/Migration/MariaDbDatabase.cs            | 16 +++++-
 2 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 grate.unittests/Basic/Infrastructure/MariaDB/MariaDbDatabase_.cs

diff --git a/grate.unittests/Basic/Infrastructure/MariaDB/MariaDbDatabase_.cs b/grate.unittests/Basic/Infrastructure/MariaDB/MariaDbDatabase_.cs
new file mode 100644
index 00000000..1a7600a9
--- /dev/null
+++ b/grate.unittests/Basic/Infrastructure/MariaDB/MariaDbDatabase_.cs
@@ -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;
+    }
+}
diff --git a/grate/Migration/MariaDbDatabase.cs b/grate/Migration/MariaDbDatabase.cs
index 93cda516..f30ebaa8 100644
--- a/grate/Migration/MariaDbDatabase.cs
+++ b/grate/Migration/MariaDbDatabase.cs
@@ -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)
     {