Batch oversized bulk operation audit logs

src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Resources/Delete/DeletionServiceAuditLogTests.cs

@@ -0,0 +1,178 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.Health.Fhir.Core.Features.Persistence;
+using Microsoft.Health.Fhir.Tests.Common;
+using Microsoft.Health.Test.Utilities;
+using Xunit;
+
+namespace Microsoft.Health.Fhir.Core.UnitTests.Features.Resources.Delete
+{
+    [Trait(Traits.OwningTeam, OwningTeam.Fhir)]
+    [Trait(Traits.Category, Categories.BulkDelete)]
+    public class DeletionServiceAuditLogTests
+    {
+        private const int MaxAffectedItemsSize = DeletionService.MaxAuditLogSize - DeletionService.AuditLogOverheadSize;
+
+        [Fact]
+        public void GivenEmptyItemsList_WhenCreatingBatches_ThenSingleEmptyBatchIsReturned()
+        {
+            var items = new List<(string resourceType, string resourceId, bool included)>();
+
+            IList<string> batches = DeletionService.CreateAffectedItemBatches(items);
+
+            Assert.Single(batches);
+            Assert.Equal(string.Empty, batches[0]);
+        }
+
+        [Fact]
+        public void GivenSmallItemsList_WhenCreatingBatches_ThenSingleBatchContainsAllItems()
+        {
+            var items = new List<(string resourceType, string resourceId, bool included)>
+            {
+                ("Patient", "123", false),
+                ("Observation", "456", false),
+                ("Encounter", "789", true),
+            };
+
+            IList<string> batches = DeletionService.CreateAffectedItemBatches(items);
+
+            Assert.Single(batches);
+            Assert.Contains("Patient/123", batches[0]);
+            Assert.Contains("Observation/456", batches[0]);
+            Assert.Contains("[Include] Encounter/789", batches[0]);
+        }
+
+        [Fact]
+        public void GivenLargeItemsList_WhenCreatingBatches_ThenMultipleBatchesAreCreated()
+        {
+            // Create enough items to exceed the max size
+            var items = new List<(string resourceType, string resourceId, bool included)>();
+            for (int i = 0; i < 1000; i++)
+            {
+                items.Add(("Patient", $"resource-id-{i:D10}", false));
+            }
+
+            IList<string> batches = DeletionService.CreateAffectedItemBatches(items);
+
+            Assert.True(batches.Count > 1, "Expected multiple batches for a large items list.");
+
+            // Verify each batch is within the size limit
+            foreach (string batch in batches)
+            {
+                Assert.True(batch.Length <= MaxAffectedItemsSize, $"Batch length {batch.Length} exceeds max size {MaxAffectedItemsSize}.");
+            }
+
+            // Verify all items are present across all batches
+            string allBatches = string.Concat(batches);
+            for (int i = 0; i < 1000; i++)
+            {
+                Assert.Contains($"Patient/resource-id-{i:D10}", allBatches);
+            }
+        }
+
+        [Fact]
+        public void GivenItemsExactlyAtLimit_WhenCreatingBatches_ThenSingleBatchIsReturned()
+        {
+            // Build items that fit exactly within the limit
+            var items = new List<(string resourceType, string resourceId, bool included)>();
+            int currentLength = 0;
+
+            for (int i = 0; ; i++)
+            {
+                string nextItem = (i == 0 ? string.Empty : ", ") + "Patient/id" + i;
+                if (currentLength + nextItem.Length > MaxAffectedItemsSize)
+                {
+                    break;
+                }
+
+                items.Add(("Patient", "id" + i, false));
+                currentLength += nextItem.Length;
+            }
+
+            IList<string> batches = DeletionService.CreateAffectedItemBatches(items);
+
+            Assert.Single(batches);
+            Assert.True(batches[0].Length <= MaxAffectedItemsSize);
+        }
+
+        [Fact]
+        public void GivenIncludedItems_WhenCreatingBatches_ThenIncludeTagIsPreserved()
+        {
+            var items = new List<(string resourceType, string resourceId, bool included)>
+            {
+                ("Patient", "1", true),
+                ("Observation", "2", false),
+                ("Patient", "3", true),
+            };
+
+            IList<string> batches = DeletionService.CreateAffectedItemBatches(items);
+
+            Assert.Single(batches);
+            Assert.Contains("[Include] Patient/1", batches[0]);
+            Assert.DoesNotContain("[Include] Observation/2", batches[0]);
+            Assert.Contains("Observation/2", batches[0]);
+            Assert.Contains("[Include] Patient/3", batches[0]);
+        }
+
+        [Fact]
+        public void GivenBatchesSplit_WhenCreatingBatches_ThenNoBatchStartsWithComma()
+        {
+            // Create enough items to force multiple batches
+            var items = new List<(string resourceType, string resourceId, bool included)>();
+            for (int i = 0; i < 1000; i++)
+            {
+                items.Add(("Patient", $"resource-id-{i:D10}", false));
+            }
+
+            IList<string> batches = DeletionService.CreateAffectedItemBatches(items);
+
+            Assert.True(batches.Count > 1);
+
+            // No batch should start with ", "
+            foreach (string batch in batches)
+            {
+                Assert.False(batch.StartsWith(", "), "Batch should not start with a leading comma separator.");
+            }
+        }
+
+        [Fact]
+        public void GivenSingleLargeItem_WhenCreatingBatches_ThenItemIsInOwnBatch()
+        {
+            // A single item with a very long ID - should still be included even if it exceeds max
+            string longId = new string('x', MaxAffectedItemsSize);
+            var items = new List<(string resourceType, string resourceId, bool included)>
+            {
+                ("Patient", longId, false),
+            };
+
+            IList<string> batches = DeletionService.CreateAffectedItemBatches(items);
+
+            Assert.Single(batches);
+            Assert.Contains($"Patient/{longId}", batches[0]);
+        }
+
+        [Fact]
+        public void GivenItemsThatCauseSplit_WhenCreatingBatches_ThenNoItemsAreLost()
+        {
+            var items = new List<(string resourceType, string resourceId, bool included)>();
+            for (int i = 0; i < 2000; i++)
+            {
+                items.Add(("Patient", $"id-{i}", i % 3 == 0));
+            }
+
+            IList<string> batches = DeletionService.CreateAffectedItemBatches(items);
+
+            // Verify no items are lost by counting occurrences
+            string allContent = string.Concat(batches);
+            for (int i = 0; i < 2000; i++)
+            {
+                Assert.Contains($"Patient/id-{i}", allContent);
+            }
+        }
+    }
+}

src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Microsoft.Health.Fhir.Shared.Core.UnitTests.projitems

@@ -39,6 +39,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Features\Resources\Create\ConditionalCreateResourceHandlerTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Features\Resources\Create\CreateResourceValidatorTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Features\Resources\Delete\ReferenceRemoverTests.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Features\Resources\Delete\DeletionServiceAuditLogTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Features\Resources\Get\GetResourceHandlerTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Features\Resources\MemberMatch\MemberMatchResourceValidatorTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Features\Resources\Patch\ConditionalPatchResourceHandlerTests.cs" />

src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/Delete/DeletionService.cs

@@ -55,6 +55,8 @@ public class DeletionService : IDeletionService
         private readonly IResourceDeserializer _resourceDeserializer;
         private readonly ILogger<DeletionService> _logger;
         internal const string DefaultCallerAgent = "Microsoft.Health.Fhir.Server";
+        internal const int MaxAuditLogSize = 16000;
+        internal const int AuditLogOverheadSize = 1000;
         private const int MaxParallelThreads = 64;
 
         public DeletionService(
@@ -507,14 +509,6 @@ private System.Threading.Tasks.Task CreateAuditLog(string primaryResourceType, D
             {
                 AuditAction action = complete ? AuditAction.Executed : AuditAction.Executing;
                 var context = _contextAccessor.RequestContext;
-                var deleteAdditionalProperties = new Dictionary<string, string>();
-                deleteAdditionalProperties["Affected Items"] = items.Aggregate(
-                    string.Empty,
-                    (aggregate, item) =>
-                    {
-                        aggregate += ", " + (item.included ? "[Include] " : string.Empty) + item.resourceType + "/" + item.resourceId;
-                        return aggregate;
-                    });
 
                 Uri uri = null;
 
@@ -527,24 +521,59 @@ private System.Threading.Tasks.Task CreateAuditLog(string primaryResourceType, D
                     _logger.LogWarning(ex, "Failed to read request URI from the request context during delete audit logging.");
                 }
 
-                _auditLogger.LogAudit(
-                    auditAction: action,
-                    operation: operation.ToString(),
-                    resourceType: primaryResourceType,
-                    requestUri: uri,
-                    statusCode: statusCode,
-                    correlationId: context.CorrelationId,
-                    callerIpAddress: string.Empty,
-                    callerClaims: null,
-                    customHeaders: null,
-                    operationType: string.Empty,
-                    callerAgent: DefaultCallerAgent,
-                    additionalProperties: deleteAdditionalProperties);
+                var batches = CreateAffectedItemBatches(items.ToList());
+
+                foreach (var batch in batches)
+                {
+                    var deleteAdditionalProperties = new Dictionary<string, string>();
+                    deleteAdditionalProperties["Affected Items"] = batch;
+
+                    _auditLogger.LogAudit(
+                        auditAction: action,
+                        operation: operation.ToString(),
+                        resourceType: primaryResourceType,
+                        requestUri: uri,
+                        statusCode: statusCode,
+                        correlationId: context.CorrelationId,
+                        callerIpAddress: string.Empty,
+                        callerClaims: null,
+                        customHeaders: null,
+                        operationType: string.Empty,
+                        callerAgent: DefaultCallerAgent,
+                        additionalProperties: deleteAdditionalProperties);
+                }
             });
 
             return auditTask;
         }
 
+        internal static IList<string> CreateAffectedItemBatches(IList<(string resourceType, string resourceId, bool included)> items)
+        {
+            int maxAffectedItemsSize = MaxAuditLogSize - AuditLogOverheadSize;
+            var batches = new List<string>();
+            var currentBatch = new System.Text.StringBuilder();
+
+            foreach (var item in items)
+            {
+                string itemString = $"{(item.included ? "[Include] " : string.Empty)}{item.resourceType}/{item.resourceId}";
+                string separator = currentBatch.Length > 0 ? ", " : string.Empty;
+
+                if (currentBatch.Length > 0 && currentBatch.Length + separator.Length + itemString.Length > maxAffectedItemsSize)
+                {
+                    batches.Add(currentBatch.ToString());
+                    currentBatch.Clear();
+                    separator = string.Empty;
+                }
+
+                currentBatch.Append(separator);
+                currentBatch.Append(itemString);
+            }
+
+            batches.Add(currentBatch.ToString());
+
+            return batches;
+        }
+
         private async Task RemoveReferences(SearchResultEntry resource, ConditionalDeleteResourceRequest request, CancellationToken cancellationToken)
         {
             using (var searchService = _searchServiceFactory.Invoke())