Add race condition safe InvoiceRepository.UpdateMetadata (#7475)

diff --git a/BTCPayServer.Client/Models/InvoiceData.cs b/BTCPayServer.Client/Models/InvoiceData.cs
index 174bc06..4f22845 100644
--- a/BTCPayServer.Client/Models/InvoiceData.cs
+++ b/BTCPayServer.Client/Models/InvoiceData.cs
@@ -19,7 +19,6 @@ namespace BTCPayServer.Client.Models
         public InvoiceType Type { get; set; }
         public string Currency { get; set; }
         public JObject Metadata { get; set; }
-        public string Comment { get; set; }
         public CheckoutOptions Checkout { get; set; } = new CheckoutOptions();
         public ReceiptOptions Receipt { get; set; } = new ReceiptOptions();
         public class ReceiptOptions
diff --git a/BTCPayServer.Client/Models/UpdateInvoiceRequest.cs b/BTCPayServer.Client/Models/UpdateInvoiceRequest.cs
index 0f4dcd5..af45868 100644
--- a/BTCPayServer.Client/Models/UpdateInvoiceRequest.cs
+++ b/BTCPayServer.Client/Models/UpdateInvoiceRequest.cs
@@ -5,6 +5,5 @@ namespace BTCPayServer.Client.Models
     public class UpdateInvoiceRequest
     {
         public JObject Metadata { get; set; }
-        public string Comment { get; set; }
     }
 }
diff --git a/BTCPayServer.Tests/UnitTest1.cs b/BTCPayServer.Tests/UnitTest1.cs
index 5ffb29f..c3530f3 100644
--- a/BTCPayServer.Tests/UnitTest1.cs
+++ b/BTCPayServer.Tests/UnitTest1.cs
@@ -1641,36 +1641,19 @@ namespace BTCPayServer.Tests
             {
                 Amount = 50.513m,
                 Currency = "USD",
-                Comment = "  API comment  ",
                 Metadata = new JObject() { new JProperty("taxIncluded", 50.516m), new JProperty("orderId", "000000161") }
             });
             Assert.Equal(50.51m, invoice5g.Amount);
             Assert.Equal(50.51m, (decimal)invoice5g.Metadata["taxIncluded"]);
             Assert.Equal("000000161", (string)invoice5g.Metadata["orderId"]);
-            Assert.Equal("API comment", invoice5g.Comment);
 
-            invoice5g = await greenfield.GetInvoice(invoice5g.Id);
-            Assert.Equal("API comment", invoice5g.Comment);
-
-            invoice5g = await greenfield.UpdateInvoice(invoice5g.Id, new UpdateInvoiceRequest
-            {
-                Comment = "  Updated API comment  "
-            });
-            Assert.Equal("Updated API comment", invoice5g.Comment);
 
             invoice5g = await greenfield.UpdateInvoice(invoice5g.Id, new UpdateInvoiceRequest
             {
                 Metadata = new JObject { new JProperty("orderId", "000000162") }
             });
-            Assert.Equal("Updated API comment", invoice5g.Comment);
             Assert.Equal("000000162", (string)invoice5g.Metadata["orderId"]);
 
-            invoice5g = await greenfield.UpdateInvoice(invoice5g.Id, new UpdateInvoiceRequest
-            {
-                Comment = ""
-            });
-            Assert.Equal("", invoice5g.Comment);
-
             var zeroInvoice = await greenfield.CreateInvoice(user.StoreId, new CreateInvoiceRequest()
             {
                 Amount = 0m,
diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldInvoiceController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldInvoiceController.cs
index acade91..38e008f 100644
--- a/BTCPayServer/Controllers/GreenField/GreenfieldInvoiceController.cs
+++ b/BTCPayServer/Controllers/GreenField/GreenfieldInvoiceController.cs
@@ -159,9 +159,7 @@ namespace BTCPayServer.Controllers.Greenfield
             if (HttpContext.GetInvoiceDataOrNull() is null)
                 return InvoiceNotFound();
             if (request.Metadata is not null)
-                await _invoiceRepository.UpdateInvoiceMetadata(invoiceId, request.Metadata);
-            if (request.Comment is not null)
-                await _invoiceRepository.UpdateInvoiceComment(invoiceId, request.Comment);
+                await _invoiceRepository.UpdateInvoiceMetadataCore(invoiceId, request.Metadata);
             var invoice = await _invoiceRepository.GetInvoice(invoiceId);
             if (invoice is null)
                 return InvoiceNotFound();
@@ -698,7 +696,6 @@ namespace BTCPayServer.Controllers.Greenfield
                 AdditionalStatus = entity.ExceptionStatus,
                 Currency = entity.Currency,
                 Archived = entity.Archived,
-                Comment = string.IsNullOrWhiteSpace(entity.Comment) ? "" : entity.Comment,
                 Metadata = entity.Metadata.ToJObject(),
                 AvailableStatusesForManualMarking = statuses.ToArray(),
                 Checkout = new InvoiceDataBase.CheckoutOptions
diff --git a/BTCPayServer/Controllers/UIInvoiceController.UI.cs b/BTCPayServer/Controllers/UIInvoiceController.UI.cs
index 68d724b..b4f56aa 100644
--- a/BTCPayServer/Controllers/UIInvoiceController.UI.cs
+++ b/BTCPayServer/Controllers/UIInvoiceController.UI.cs
@@ -151,7 +151,7 @@ namespace BTCPayServer.Controllers
                 Events = await _InvoiceRepository.GetInvoiceLogs(invoice.Id),
                 Metadata = metaData,
                 Archived = invoice.Archived,
-                Comment = invoice.Comment,
+                Comment = invoice.Metadata.Comment,
                 HasRefund = invoice.Refunds.Any(),
                 CanRefund = invoiceState.CanRefund(),
                 Refunds = invoice.Refunds,
@@ -618,7 +618,7 @@ namespace BTCPayServer.Controllers
             if (invoice is null)
                 return NotFound();
 
-            await _InvoiceRepository.UpdateInvoiceComment(invoiceId, comment);
+            await _InvoiceRepository.UpdateInvoiceMetadata(invoiceId, "comment", string.IsNullOrWhiteSpace(comment?.Trim()) ? null : comment.Trim());
             TempData.SetStatusMessageModel(new StatusMessageModel
             {
                 Severity = StatusMessageModel.StatusSeverity.Success,
@@ -1124,7 +1124,7 @@ namespace BTCPayServer.Controllers
                     RedirectUrl = invoice.RedirectURL?.AbsoluteUri ?? string.Empty,
                     Amount = invoice.Price,
                     Currency = invoice.Currency,
-                    Comment = invoice.Comment,
+                    Comment = invoice.Metadata.Comment ?? string.Empty,
                     CanMarkInvalid = state.CanMarkInvalid(),
                     CanMarkSettled = state.CanMarkComplete(),
                     Details = InvoicePopulatePayments(invoice),
diff --git a/BTCPayServer/Controllers/UIInvoiceController.cs b/BTCPayServer/Controllers/UIInvoiceController.cs
index 15a1d91..9ddcf11 100644
--- a/BTCPayServer/Controllers/UIInvoiceController.cs
+++ b/BTCPayServer/Controllers/UIInvoiceController.cs
@@ -167,7 +167,6 @@ namespace BTCPayServer.Controllers
             entity.ReceiptOptions = invoice.Receipt ?? new InvoiceDataBase.ReceiptOptions();
             if (invoice.Metadata != null)
                 entity.Metadata = InvoiceMetadata.FromJObject(invoice.Metadata);
-            entity.Comment = string.IsNullOrWhiteSpace(invoice.Comment) ? null : invoice.Comment.Trim();
             invoice.Checkout ??= new CreateInvoiceRequest.CheckoutOptions();
             entity.Currency = invoice.Currency;
             if (invoice.Amount is decimal v)
diff --git a/BTCPayServer/Services/Invoices/InvoiceEntity.cs b/BTCPayServer/Services/Invoices/InvoiceEntity.cs
index fdfeb70..1515a54 100644
--- a/BTCPayServer/Services/Invoices/InvoiceEntity.cs
+++ b/BTCPayServer/Services/Invoices/InvoiceEntity.cs
@@ -51,6 +51,12 @@ namespace BTCPayServer.Services.Invoices
             set => this.SetAdditionalData("orderId", value);
         }
         [JsonIgnore]
+        public string Comment
+        {
+            get => this.GetAdditionalData<string>("comment");
+            set => this.SetAdditionalData("comment", value);
+        }
+        [JsonIgnore]
         public string OrderUrl
         {
             get => this.GetAdditionalData<string>("orderUrl");
@@ -762,8 +768,6 @@ namespace BTCPayServer.Services.Invoices
         [JsonIgnore]
         public bool DisableAccounting { get; set; }
 
-        public string Comment { get; set; }
-
         public RequestBaseUrl GetRequestBaseUrl() => RequestBaseUrl.FromUrl(ServerUrl);
     }
 
diff --git a/BTCPayServer/Services/Invoices/InvoiceRepository.cs b/BTCPayServer/Services/Invoices/InvoiceRepository.cs
index a3aa41b..ae1ccb2 100644
--- a/BTCPayServer/Services/Invoices/InvoiceRepository.cs
+++ b/BTCPayServer/Services/Invoices/InvoiceRepository.cs
@@ -449,14 +449,6 @@ retry:
             context.AddRange(filteredTerms);
         }
 
-        public static void RemoveFromTextSearch(ApplicationDbContext context, InvoiceData invoice,
-            string term)
-        {
-            var query = context.InvoiceSearches.AsQueryable();
-            var filteredQuery = query.Where(st => st.InvoiceDataId.Equals(invoice.Id) && st.Value.Equals(term));
-            context.InvoiceSearches.RemoveRange(filteredQuery);
-        }
-
         public async Task UpdateInvoiceStatus(string invoiceId, InvoiceState invoiceState)
         {
             using var context = _applicationDbContextFactory.CreateContext();
@@ -544,67 +536,65 @@ retry:
         [Obsolete("The storeId parameter is now ignored. This method is deprecated and will be removed in a future release.")]
         public Task<InvoiceEntity> UpdateInvoiceMetadata(string invoiceId, string storeId, JObject metadata)
             => UpdateInvoiceMetadata(invoiceId, metadata);
+
+        [Obsolete("This method replace the whole metadata, and thus is subject to racing condition issues. Use 'UpdateInvoiceMetadata(string invoiceId, string metadataKey, JToken metadataValue)' instead")]
         public async Task<InvoiceEntity> UpdateInvoiceMetadata(string invoiceId, JObject metadata)
         {
-retry:
-            using (var context = _applicationDbContextFactory.CreateContext())
-            {
-                var invoiceData = await GetInvoiceRaw(invoiceId, context);
-                if (invoiceData == null)
-                    return null;
-                var blob = invoiceData.GetBlob();
-
-                var newMetadata = InvoiceMetadata.FromJObject(metadata);
-                var oldOrderId = blob.Metadata.OrderId;
-                var newOrderId = newMetadata.OrderId;
+            ArgumentNullException.ThrowIfNull(invoiceId);
+            ArgumentNullException.ThrowIfNull(metadata);
+            await UpdateInvoiceMetadataCore(invoiceId, metadata);
+            return await GetInvoice(invoiceId);
+        }
 
-                if (newOrderId != oldOrderId)
-                {
-                    if (oldOrderId != null && (newOrderId is null || !newOrderId.Equals(oldOrderId, StringComparison.InvariantCulture)))
-                    {
-                        RemoveFromTextSearch(context, invoiceData, oldOrderId);
-                    }
-                    if (newOrderId != null)
-                    {
-                        AddToTextSearch(context, invoiceData, new[] { newOrderId });
-                    }
-                }
+        internal async Task UpdateInvoiceMetadataCore(string invoiceId, JObject metadata)
+        {
+            ArgumentNullException.ThrowIfNull(invoiceId);
+            ArgumentNullException.ThrowIfNull(metadata);
 
-                blob.Metadata = newMetadata;
-                invoiceData.SetBlob(blob);
-                try
-                {
-                    await context.SaveChangesAsync();
-                }
-                catch (DbUpdateConcurrencyException)
+            await using var context = _applicationDbContextFactory.CreateContext();
+            await context.Database.GetDbConnection().QuerySingleOrDefaultAsync<(int Updated, string OldOrderId, string NewOrderId)>(
+                """
+                UPDATE "Invoices" i
+                SET "Blob2" = jsonb_set(COALESCE(i."Blob2", '{}'::jsonb), '{metadata}', @metadata::jsonb, true)
+                WHERE i."Id" = @invoiceId;
+                """,
+                new
                 {
-                    goto retry;
-                }
-                return ToEntity(invoiceData);
-            }
+                    invoiceId,
+                    metadata = metadata.ToString(Formatting.None)
+                });
         }
-        public async Task UpdateInvoiceComment(string invoiceId, string comment)
+
+        public async Task UpdateInvoiceMetadata(string invoiceId, string metadataKey, object metadataValue)
         {
+            ArgumentNullException.ThrowIfNull(invoiceId);
+            ArgumentNullException.ThrowIfNull(metadataKey);
+            if (string.IsNullOrWhiteSpace(metadataKey))
+                throw new ArgumentException("Metadata key must not be empty.", nameof(metadataKey));
+
             await using var context = _applicationDbContextFactory.CreateContext();
-            var newComment = string.IsNullOrWhiteSpace(comment?.Trim()) ? null : comment.Trim();
-            var sql = newComment is null
+
+            var value = metadataValue is null ? null : JsonConvert.SerializeObject(metadataValue);
+            var sql = value is null
                 ? """
                   UPDATE "Invoices"
-                  SET "Blob2" = COALESCE("Blob2", '{}'::jsonb) - 'comment'
-                  WHERE "Id" = @Id
+                  SET "Blob2" = jsonb_set(COALESCE("Blob2", '{}'::jsonb), '{metadata}', COALESCE("Blob2"->'metadata', '{}'::jsonb) - @metadataKey, true)
+                  WHERE "Id" = @invoiceId
                   """
                 : """
                   UPDATE "Invoices"
-                  SET "Blob2" = jsonb_set(COALESCE("Blob2", '{}'::jsonb), '{comment}', to_jsonb(@Comment::text), true)
-                  WHERE "Id" = @Id
+                  SET "Blob2" = jsonb_set(COALESCE("Blob2", '{}'::jsonb), '{metadata}', COALESCE("Blob2"->'metadata', '{}'::jsonb) || jsonb_build_object(@metadataKey, @value::jsonb), true)
+                  WHERE "Id" = @invoiceId
                   """;
 
             await context.Database.GetDbConnection().ExecuteAsync(sql, new
             {
-                Id = invoiceId,
-                Comment = newComment
+                invoiceId,
+                metadataKey,
+                value
             });
         }
+
         public async Task<bool> MarkInvoiceStatus(string invoiceId, InvoiceStatus status)
         {
             using (var context = _applicationDbContextFactory.CreateContext())
diff --git a/BTCPayServer/Services/Reporting/InvoicesReportProvider.cs b/BTCPayServer/Services/Reporting/InvoicesReportProvider.cs
index 1f4f7d2..0bcbb51 100644
--- a/BTCPayServer/Services/Reporting/InvoicesReportProvider.cs
+++ b/BTCPayServer/Services/Reporting/InvoicesReportProvider.cs
@@ -182,7 +182,7 @@ public class InvoicesReportProvider : ReportProvider
         data.Add(invoiceEntity?.GetInvoiceState().ToString());
         data.Add(invoiceEntity?.Status.ToString());
         data.Add(invoiceEntity?.ExceptionStatus is null or InvoiceExceptionStatus.None ? "" : invoiceEntity.ExceptionStatus.ToString());
-        data.Add(invoiceEntity?.Comment);
+        data.Add(invoiceEntity?.Metadata.Comment ?? "");
 
 
         data.Add(payment?.ReceivedTime);
diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.invoices.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.invoices.json
index 5779960..c4af224 100644
--- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.invoices.json
+++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.invoices.json
@@ -754,11 +754,6 @@
                     "metadata": {
                         "$ref": "#/components/schemas/InvoiceMetadata"
                     },
-                    "comment": {
-                        "type": "string",
-                        "nullable": true,
-                        "description": "A private comment on the invoice, visible to store users."
-                    },
                     "checkout": {
                         "type": "object",
                         "nullable": true,
@@ -1082,11 +1077,6 @@
                 "properties": {
                     "metadata": {
                         "$ref": "#/components/schemas/InvoiceMetadata"
-                    },
-                    "comment": {
-                        "type": "string",
-                        "nullable": true,
-                        "description": "A private comment on the invoice, visible to store users."
                     }
                 }
             },