Skip to content

Commit

Permalink
Add support for partial delete success (#3781)
Browse files Browse the repository at this point in the history
  • Loading branch information
LTA-Thinking authored and Nathan Lemma (Waferwire LLC) committed Apr 23, 2024
1 parent 996b3fa commit 3285b6d
Show file tree
Hide file tree
Showing 20 changed files with 136 additions and 46 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Microsoft.Health.Abstractions.Exceptions;

namespace Microsoft.Health.Fhir.Core.Exceptions
{
public class IncompleteDeleteException : RequestTooCostlyException
{
public IncompleteDeleteException(int numberOfResourceVersionsDeleted)
: base(message: string.Format(Resources.PartialDeleteSuccess, numberOfResourceVersionsDeleted, StringComparison.Ordinal))
{
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ public async Task<string> ExecuteAsync(JobInfo jobInfo, CancellationToken cancel
definition.DeleteOperation,
maxDeleteCount: null,
deleteAll: true,
versionType: definition.VersionType),
versionType: definition.VersionType,
allowPartialSuccess: false), // Explicitly setting to call out that this can be changed in the future if we want to. Bulk delete offers the possibility of automatically rerunning the operation until it succeeds, fully automating the process.
cancellationToken);
}
catch (IncompleteOperationException<long> ex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,15 @@ public interface IFhirDataStore

Task<ResourceWrapper> GetAsync(ResourceKey key, CancellationToken cancellationToken);

Task HardDeleteAsync(ResourceKey key, bool keepCurrentVersion, CancellationToken cancellationToken);
/// <summary>
/// Hard deletes a resource.
/// </summary>
/// <param name="key">Identifier of the resource</param>
/// <param name="keepCurrentVersion">Keeps the current version of the resource, only deleting history</param>
/// <param name="allowPartialSuccess">Only for Cosmos. Allows for a delete to partially succeed if it fails to delete all versions of a resource in one try.</param>
/// <param name="cancellationToken">Cancellation Token</param>
/// <returns>Async Task</returns>
Task HardDeleteAsync(ResourceKey key, bool keepCurrentVersion, bool allowPartialSuccess, CancellationToken cancellationToken);

Task BulkUpdateSearchParameterIndicesAsync(IReadOnlyCollection<ResourceWrapper> resources, CancellationToken cancellationToken);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ public ConditionalDeleteResourceRequest(
int? maxDeleteCount,
BundleResourceContext bundleResourceContext = null,
bool deleteAll = false,
ResourceVersionType versionType = ResourceVersionType.Latest)
ResourceVersionType versionType = ResourceVersionType.Latest,
bool allowPartialSuccess = false)
: base(resourceType, conditionalParameters, bundleResourceContext)
{
EnsureArg.IsNotNull(conditionalParameters, nameof(conditionalParameters));
Expand All @@ -32,6 +33,7 @@ public ConditionalDeleteResourceRequest(
MaxDeleteCount = maxDeleteCount;
DeleteAll = deleteAll;
VersionType = versionType;
AllowPartialSuccess = allowPartialSuccess;
}

public DeleteOperation DeleteOperation { get; }
Expand All @@ -42,6 +44,8 @@ public ConditionalDeleteResourceRequest(

public ResourceVersionType VersionType { get; }

public bool AllowPartialSuccess { get; }

protected override IEnumerable<string> GetCapabilities() => Capabilities;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,25 @@ namespace Microsoft.Health.Fhir.Core.Messages.Delete
{
public class DeleteResourceRequest : IRequest<DeleteResourceResponse>, IRequireCapability
{
public DeleteResourceRequest(ResourceKey resourceKey, DeleteOperation deleteOperation, BundleResourceContext bundleResourceContext = null)
public DeleteResourceRequest(ResourceKey resourceKey, DeleteOperation deleteOperation, BundleResourceContext bundleResourceContext = null, bool allowPartialSuccess = false)
{
EnsureArg.IsNotNull(resourceKey, nameof(resourceKey));

ResourceKey = resourceKey;
DeleteOperation = deleteOperation;
BundleResourceContext = bundleResourceContext;
AllowPartialSuccess = allowPartialSuccess;
}

public DeleteResourceRequest(string type, string id, DeleteOperation deleteOperation, BundleResourceContext bundleResourceContext = null)
public DeleteResourceRequest(string type, string id, DeleteOperation deleteOperation, BundleResourceContext bundleResourceContext = null, bool allowPartialSuccess = false)
{
EnsureArg.IsNotNull(type, nameof(type));
EnsureArg.IsNotNull(id, nameof(id));

ResourceKey = new ResourceKey(type, id);
DeleteOperation = deleteOperation;
BundleResourceContext = bundleResourceContext;
AllowPartialSuccess = allowPartialSuccess;
}

public ResourceKey ResourceKey { get; }
Expand All @@ -39,6 +41,8 @@ public DeleteResourceRequest(string type, string id, DeleteOperation deleteOpera

public DeleteOperation DeleteOperation { get; }

public bool AllowPartialSuccess { get; }

public IEnumerable<CapabilityQuery> RequiredCapabilities()
{
yield return new CapabilityQuery($"CapabilityStatement.rest.resource.where(type = '{ResourceKey.ResourceType}').interaction.where(code = 'delete').exists()");
Expand Down
9 changes: 9 additions & 0 deletions src/Microsoft.Health.Fhir.Core/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions src/Microsoft.Health.Fhir.Core/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -736,4 +736,8 @@
<value>A resource should only appear once in each Bundle.</value>
<comment>Error message for a duplicate resource key in the same bundle</comment>
</data>
<data name="PartialDeleteSuccess" xml:space="preserve">
<value>Deleted {0} versions of the target resource.</value>
<comment>{0} is replaced with the number of deleted versions of the resource.</comment>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,19 @@
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using Hl7.Fhir.Model;
using Hl7.Fhir.Serialization;
using Microsoft.Azure.Cosmos;
using Microsoft.Azure.Cosmos.Scripts;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using Microsoft.Health.Abstractions.Exceptions;
using Microsoft.Health.Core.Features.Context;
using Microsoft.Health.Extensions.DependencyInjection;
using Microsoft.Health.Fhir.Core.Configs;
using Microsoft.Health.Fhir.Core.Exceptions;
using Microsoft.Health.Fhir.Core.Extensions;
using Microsoft.Health.Fhir.Core.Features.Context;
using Microsoft.Health.Fhir.Core.Features.Definition;
Expand Down Expand Up @@ -283,6 +286,23 @@ public async Task GivenAnUpsertDuringABatch_When408ExceptionOccurs_RetryWillHapp
await _container.Value.ReceivedWithAnyArgs(7).CreateItemAsync(Arg.Any<FhirCosmosResourceWrapper>(), Arg.Any<PartitionKey>(), Arg.Any<ItemRequestOptions>(), Arg.Any<CancellationToken>());
}

[Fact]
public async Task GivenAHardDeleteRequest_WhenPartiallySuccessful_ThenAnExceptionIsThrown()
{
var resourceKey = new ResourceKey(KnownResourceTypes.Patient, "test");

var scripts = Substitute.For<Scripts>();
scripts.ExecuteStoredProcedureAsync<int>(Arg.Any<string>(), Arg.Any<PartitionKey>(), Arg.Any<object[]>(), cancellationToken: Arg.Any<CancellationToken>()).Returns((x) =>
{
var response = Substitute.For<StoredProcedureExecuteResponse<int>>();
response.Resource.Returns(1);
return Task.FromResult(response);
});
_container.Value.Scripts.Returns(scripts);

await Assert.ThrowsAsync<IncompleteDeleteException>(() => _dataStore.HardDeleteAsync(resourceKey, false, true, CancellationToken.None));
}

private void CreateResponses(int pageSize, string continuationToken, params FeedResponse<int>[] responses)
{
ICosmosQuery<int> cosmosQuery = Substitute.For<ICosmosQuery<int>>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -489,23 +489,28 @@ public async Task<ResourceWrapper> GetAsync(ResourceKey key, CancellationToken c
}
}

public async Task HardDeleteAsync(ResourceKey key, bool keepCurrentVersion, CancellationToken cancellationToken)
public async Task HardDeleteAsync(ResourceKey key, bool keepCurrentVersion, bool allowPartialSuccess, CancellationToken cancellationToken)
{
EnsureArg.IsNotNull(key, nameof(key));

try
{
_logger.LogDebug("Obliterating {ResourceType}/{Id}. Keep current version: {KeepCurrentVersion}", key.ResourceType, key.Id, keepCurrentVersion);

StoredProcedureExecuteResponse<IList<string>> response = await _retryExceptionPolicyFactory.RetryPolicy.ExecuteAsync(
StoredProcedureExecuteResponse<int> response = await _retryExceptionPolicyFactory.RetryPolicy.ExecuteAsync(
async ct => await _hardDelete.Execute(
_containerScope.Value.Scripts,
key,
keepCurrentVersion,
allowPartialSuccess,
ct),
cancellationToken);

_logger.LogDebug("Hard-deleted {Count} documents, which consumed {RU} RUs. The list of hard-deleted documents: {Resources}.", response.Resource.Count, response.RequestCharge, string.Join(", ", response.Resource));
if (response.Resource > 0)
{
_logger.LogInformation("Partial success of delete operation. Deleted {NumDeleted} versions of the resource.", response.Resource);
throw new IncompleteDeleteException(response.Resource);
}
}
catch (CosmosException exception)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ namespace Microsoft.Health.Fhir.CosmosDb.Features.Storage.StoredProcedures.HardD
{
internal class HardDelete : StoredProcedureBase
{
public async Task<StoredProcedureExecuteResponse<IList<string>>> Execute(Scripts client, ResourceKey key, bool keepCurrentVersion, CancellationToken cancellationToken)
public async Task<StoredProcedureExecuteResponse<int>> Execute(Scripts client, ResourceKey key, bool keepCurrentVersion, bool allowPartialSuccess, CancellationToken cancellationToken)
{
EnsureArg.IsNotNull(client, nameof(client));
EnsureArg.IsNotNull(key, nameof(key));

return await ExecuteStoredProc<IList<string>>(client, key.ToPartitionKey(), cancellationToken, key.ResourceType, key.Id, keepCurrentVersion);
return await ExecuteStoredProc<int>(client, key.ToPartitionKey(), cancellationToken, key.ResourceType, key.Id, keepCurrentVersion, allowPartialSuccess);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
* @param {string} resourceTypeName - The resource type name.
* @param {string} resourceId - The resource id.
* @param {boolean} keepCurrentVersion - Specifies if the current version of the resource should be kept.
* @param {boolean} partialSuccess - Specifies if partial success of the delete is allowed. This will allow for some versions of the resource to be deleted even if it isn't possible to do all of them in one go.
*/

function hardDelete(resourceTypeName, resourceId, keepCurrentVersion) {
function hardDelete(resourceTypeName, resourceId, keepCurrentVersion, partialSuccess) {
const collection = getContext().getCollection();
const collectionLink = collection.getSelfLink();
const response = getContext().getResponse();
Expand All @@ -21,8 +22,8 @@ function hardDelete(resourceTypeName, resourceId, keepCurrentVersion) {
if (!resourceId) {
throwArgumentValidationError("The resourceId is undefined or null");
}

let deletedResourceIdList = new Array();
let deletedResourceCount = 0;

tryQueryAndHardDelete();

Expand Down Expand Up @@ -53,7 +54,7 @@ function hardDelete(resourceTypeName, resourceId, keepCurrentVersion) {
tryHardDelete(documents);
} else {
// There is no more documents so we are finished.
response.setBody(deletedResourceIdList);
response.setBody(0);
}
});

Expand All @@ -65,7 +66,6 @@ function hardDelete(resourceTypeName, resourceId, keepCurrentVersion) {

function tryHardDelete(documents) {
if (documents.length > 0) {
deletedResourceIdList.push(documents[0].id);

// Delete the first item.
var isAccepted = collection.deleteDocument(
Expand All @@ -77,6 +77,7 @@ function hardDelete(resourceTypeName, resourceId, keepCurrentVersion) {
}

// Successfully deleted the item, continue deleting.
deletedResourceCount++;
documents.shift();
tryHardDelete(documents);
});
Expand All @@ -96,6 +97,11 @@ function hardDelete(resourceTypeName, resourceId, keepCurrentVersion) {
}

function throwTooManyRequestsError() {
throw new Error(429, `The request could not be completed.`);
if (!partialSuccess) {
throw new Error(429, `The request could not be completed.`);
}
else {
response.setBody(deletedResourceCount)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -391,17 +391,19 @@ public async Task<IActionResult> VRead(string typeParameter, string idParameter,
/// <param name="typeParameter">The type.</param>
/// <param name="idParameter">The identifier.</param>
/// <param name="hardDelete">A flag indicating whether to hard-delete the resource or not.</param>
/// <param name="allowPartialSuccess">Allows for partial success of delete operation. Only applicable for hard delete on Cosmos services</param>
[HttpDelete]
[ValidateIdSegmentAttribute]
[Route(KnownRoutes.ResourceTypeById)]
[AuditEventType(AuditEventSubType.Delete)]
public async Task<IActionResult> Delete(string typeParameter, string idParameter, [FromQuery] bool hardDelete)
public async Task<IActionResult> Delete(string typeParameter, string idParameter, [FromQuery] bool hardDelete, [FromQuery] bool allowPartialSuccess)
{
DeleteResourceResponse response = await _mediator.DeleteResourceAsync(
new DeleteResourceRequest(
new ResourceKey(typeParameter, idParameter),
hardDelete ? DeleteOperation.HardDelete : DeleteOperation.SoftDelete,
GetBundleResourceContext()),
GetBundleResourceContext(),
allowPartialSuccess),
HttpContext.RequestAborted);

return FhirResult.NoContent().SetETagHeader(response.WeakETag);
Expand All @@ -412,17 +414,19 @@ public async Task<IActionResult> Delete(string typeParameter, string idParameter
/// </summary>
/// <param name="typeParameter">The type.</param>
/// <param name="idParameter">The identifier.</param>
/// <param name="allowPartialSuccess">Allows for partial success of delete operation. Only applicable on Cosmos services</param>
[HttpDelete]
[ValidateIdSegmentAttribute]
[Route(KnownRoutes.PurgeHistoryResourceTypeById)]
[AuditEventType(AuditEventSubType.PurgeHistory)]
public async Task<IActionResult> PurgeHistory(string typeParameter, string idParameter)
public async Task<IActionResult> PurgeHistory(string typeParameter, string idParameter, [FromQuery] bool allowPartialSuccess)
{
DeleteResourceResponse response = await _mediator.DeleteResourceAsync(
new DeleteResourceRequest(
new ResourceKey(typeParameter, idParameter),
DeleteOperation.PurgeHistory,
GetBundleResourceContext()),
GetBundleResourceContext(),
allowPartialSuccess),
HttpContext.RequestAborted);

return FhirResult.NoContent().SetETagHeader(response.WeakETag);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ public override void OnActionExecuted(ActionExecutedContext context)

operationOutcomeResult.StatusCode = HttpStatusCode.BadRequest;
break;
case IncompleteDeleteException:
operationOutcomeResult.StatusCode = HttpStatusCode.RequestEntityTooLarge;
break;
case BadRequestException _:
case RequestNotValidException _:
case BundleEntryLimitExceededException _:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ public async Task GivenAFhirMediator_WhenHardDeletingWithSufficientPermissions_T

ResourceKey resultKey = (await _mediator.DeleteResourceAsync(resourceKey, DeleteOperation.HardDelete)).ResourceKey;

await _fhirDataStore.Received(1).HardDeleteAsync(resourceKey, Arg.Any<bool>(), Arg.Any<CancellationToken>());
await _fhirDataStore.Received(1).HardDeleteAsync(resourceKey, Arg.Any<bool>(), Arg.Any<bool>(), Arg.Any<CancellationToken>());

Assert.NotNull(resultKey);
Assert.Equal(resourceKey.Id, resultKey.Id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public async Task GivenOneMatchingResource_WhenDeletingConditionallyWithHardDele

await _fhirDataStore.DidNotReceive().UpsertAsync(Arg.Any<ResourceWrapperOperation>(), Arg.Any<CancellationToken>());

await _fhirDataStore.Received().HardDeleteAsync(Arg.Any<ResourceKey>(), Arg.Any<bool>(), Arg.Any<CancellationToken>());
await _fhirDataStore.Received().HardDeleteAsync(Arg.Any<ResourceKey>(), Arg.Any<bool>(), Arg.Any<bool>(), Arg.Any<CancellationToken>());
}

[Fact]
Expand Down Expand Up @@ -128,7 +128,7 @@ private ConditionalDeleteResourceRequest SetupConditionalDelete(

if (hardDelete)
{
_fhirDataStore.HardDeleteAsync(Arg.Any<ResourceKey>(), Arg.Any<bool>(), Arg.Any<CancellationToken>()).Returns(Task.CompletedTask);
_fhirDataStore.HardDeleteAsync(Arg.Any<ResourceKey>(), Arg.Any<bool>(), Arg.Any<bool>(), Arg.Any<CancellationToken>()).Returns(Task.CompletedTask);
}
else
{
Expand Down
Loading

0 comments on commit 3285b6d

Please sign in to comment.