Merge pull request #152

Improve resource disposal practices

ChatTwo.Tests/MessageStoreTest.cs

@@ -47,7 +47,8 @@ public class MessageStoreTest {
         store.UpsertMessage(input);
 
         // Read the message back.
-        var messages = store.GetMostRecentMessages().ToList();
+        using var messageEnumerator = store.GetMostRecentMessages();
+        var messages = messageEnumerator.ToList();
         Assert.AreEqual(1, messages.Count);
         AssertMessagesEqual(input, messages.First());
     }
@@ -80,7 +81,8 @@ public class MessageStoreTest {
         // Query the most recent 5 messages. Should return the 4 newest messages
         // from the list, as well as the different receiver message because we
         // aren't filtering.
-        var outputMessages = store.GetMostRecentMessages(count: 5).ToList();
+        using var unfilteredMessageEnumerator = store.GetMostRecentMessages(count: 5);
+        var outputMessages = unfilteredMessageEnumerator.ToList();
         var gotIds = outputMessages.Select(m => m.Id).ToList();
         TestContext.WriteLine($"Query 1 got IDs: {string.Join(", ", gotIds)}");
         AssertGuidsEqual(new List<Guid> {
@@ -92,7 +94,8 @@ public class MessageStoreTest {
         }, gotIds);
 
         // Query the most recent 5 messages but filter by receiver ID.
-        outputMessages = store.GetMostRecentMessages(receiver: receiver, count: 5).ToList();
+        using var filteredByReceiverMessageEnumerator = store.GetMostRecentMessages(receiver: receiver, count: 5);
+        outputMessages = filteredByReceiverMessageEnumerator.ToList();
         gotIds = outputMessages.Select(m => m.Id).ToList();
         TestContext.WriteLine($"Query 2 got IDs: {string.Join(", ", gotIds)}");
         AssertGuidsEqual(new List<Guid> {
@@ -104,7 +107,8 @@ public class MessageStoreTest {
         }, gotIds);
 
         // Query the most recent 5 messages but only since a specific date.
-        outputMessages = store.GetMostRecentMessages(receiver, since: messages[1].Date, count: 5).ToList();
+        using var filteredByReceiverAndDateMessageEnumerator = store.GetMostRecentMessages(receiver, since: messages[1].Date, count: 5);
+        outputMessages = filteredByReceiverAndDateMessageEnumerator.ToList();
         gotIds = outputMessages.Select(m => m.Id).ToList();
         TestContext.WriteLine($"Query 3 got IDs: {string.Join(", ", gotIds)}");
         AssertGuidsEqual(new List<Guid> {
@@ -132,7 +136,8 @@ public class MessageStoreTest {
         */
 
         using var store = new MessageStore(dbPath);
-        var output = store.GetMostRecentMessages().ToList();
+        using var existingMessageEnumerator = store.GetMostRecentMessages();
+        var output = existingMessageEnumerator.ToList();
         Assert.AreEqual(1, output.Count);
         AssertMessagesEqual(input, output[0]);
     }
@@ -152,7 +157,8 @@ public class MessageStoreTest {
             store.UpsertMessage(message);
         }
 
-        var messages = store.GetMostRecentMessages(count: count).ToList();
+        using var messageEnumerator = store.GetMostRecentMessages(count: count);
+        var messages = messageEnumerator.ToList();
         Assert.AreEqual(count, messages.Count);
         foreach (var message in messages) {
             // Load the message because they are lazily parsed.

ChatTwo/MessageManager.cs

@@ -141,7 +141,7 @@ internal class MessageManager : IAsyncDisposable
         if (!Plugin.Config.FilterIncludePreviousSessions)
             since = Plugin.GameStarted;
 
-        var messages = Store.GetMostRecentMessages(CurrentContentId, since);
+        using var messages = Store.GetMostRecentMessages(CurrentContentId, since);
 
         // We store the pending messages to be added to the chat log in a
         // temporary list, and apply them all at once after filtering.

ChatTwo/MessageStore.cs

@@ -160,7 +160,7 @@ internal class MessageStore : IDisposable
     private void Migrate()
     {
         // Get current user_version.
-        var cmd = Connection.CreateCommand();
+        using var cmd = Connection.CreateCommand();
         cmd.CommandText = "PRAGMA user_version;";
         var userVersion = Convert.ToInt32(cmd.ExecuteScalar());
 
@@ -229,7 +229,7 @@ internal class MessageStore : IDisposable
 
     private void SetMigrationVersion(int version)
     {
-        var cmd = Connection.CreateCommand();
+        using var cmd = Connection.CreateCommand();
         // Parameters aren't supported for PRAGMA queries, and you can't set the
         // version with a pragma_ function.
         cmd.CommandText = $"PRAGMA user_version = {version};";
@@ -257,14 +257,14 @@ internal class MessageStore : IDisposable
 
     internal int MessageCount()
     {
-        var cmd = Connection.CreateCommand();
+        using var cmd = Connection.CreateCommand();
         cmd.CommandText = "SELECT COUNT(*) FROM messages;";
         return Convert.ToInt32(cmd.ExecuteScalar());
     }
 
     internal void UpsertMessage(Message message)
     {
-        var cmd = Connection.CreateCommand();
+        using var cmd = Connection.CreateCommand();
         cmd.CommandText = @"
             INSERT INTO messages (
                 Id,
@@ -381,7 +381,7 @@ internal class MessageStore : IDisposable
     /// </summary>
     internal void DeleteMessage(Guid id)
     {
-        var cmd = Connection.CreateCommand();
+        using var cmd = Connection.CreateCommand();
         cmd.CommandText = "UPDATE messages SET Deleted = true WHERE Id = $Id;";
         cmd.Parameters.AddWithValue("$Id", id);
         cmd.ExecuteNonQuery();
@@ -398,7 +398,7 @@ internal class MessageStore : IDisposable
 
         var whereClause = "WHERE " + string.Join(" AND ", whereClauses);
 
-        var cmd = Connection.CreateCommand();
+        using var cmd = Connection.CreateCommand();
         // Select last N messages by date DESC, but reverse the order to get
         // them in ascending order.
         cmd.CommandText = @"
@@ -461,7 +461,7 @@ internal class MessageStore : IDisposable
     }
 }
 
-internal class MessageEnumerator(DbDataReader reader) : IEnumerable<Message>
+internal class MessageEnumerator(DbDataReader reader) : IEnumerable<Message>, IDisposable, IAsyncDisposable
 {
     private const int MaxErrorLogs = 10;
 
@@ -520,4 +520,13 @@ internal class MessageEnumerator(DbDataReader reader) : IEnumerable<Message>
     {
         return FailedIds;
     }
+
+    public void Dispose()
+    {
+        reader.Dispose();
+    }
+    public async ValueTask DisposeAsync()
+    {
+        await reader.DisposeAsync();
+    }
 }

ChatTwo/Ui/DbViewer.cs

@@ -134,7 +134,9 @@ public class DbViewer : Window
                     // We only want to fetch count if this is the first page
                     if (CurrentPage == 1)
                         Count = Plugin.MessageManager.Store.CountDateRange(AfterDate, BeforeDate, channels, character);
-                    Messages = Plugin.MessageManager.Store.GetDateRange(AfterDate, BeforeDate, channels, character, CurrentPage - 1).ToArray();
+
+                    using var dateRangeMessageEnumerator = Plugin.MessageManager.Store.GetDateRange(AfterDate, BeforeDate, channels, character, CurrentPage - 1);
+                    Messages = dateRangeMessageEnumerator.ToArray();
 
                     Filter();
                 }