Skip to content

Commit

Permalink
Support _hardDelete and hardDelete for all delete operations. (#4662)
Browse files Browse the repository at this point in the history
  • Loading branch information
@v-iyamauchi
v-iyamauchi authored Oct 8, 2024
1 parent d59ab94 commit 57f5421
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 23 deletions.
21 changes: 21 additions & 0 deletions src/Microsoft.Health.Fhir.Api/Models/HardDeleteModel.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using Microsoft.AspNetCore.Mvc;
using Microsoft.Health.Fhir.Core.Features;

namespace Microsoft.Health.Fhir.Api.Models
{
public class HardDeleteModel
{
[FromQuery(Name = KnownQueryParameterNames.BulkHardDelete)]
public bool? BulkHardDelete { get; set; }

[FromQuery(Name = KnownQueryParameterNames.HardDelete)]
public bool? HardDelete { get; set; }

public bool IsHardDelete => (BulkHardDelete ?? false) || (HardDelete ?? false);
}
}
4 changes: 3 additions & 1 deletion src/Microsoft.Health.Fhir.Core/Features/KnownQueryParameterNames.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ public static class KnownQueryParameterNames
/// </summary>
public const string Identifier = "identifier";

public const string HardDelete = "_hardDelete";
public const string BulkHardDelete = "_hardDelete";

public const string HardDelete = "hardDelete";

public const string PurgeHistory = "_purgeHistory";

Expand Down
94 changes: 94 additions & 0 deletions src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Controllers/FhirControllerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,31 @@
using System;
using System.Linq;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using MediatR;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Controllers;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Options;
using Microsoft.Health.Api.Features.Audit;
using Microsoft.Health.Core.Features.Context;
using Microsoft.Health.Fhir.Api.Configs;
using Microsoft.Health.Fhir.Api.Controllers;
using Microsoft.Health.Fhir.Api.Features.Filters;
using Microsoft.Health.Fhir.Api.Features.Filters.Metrics;
using Microsoft.Health.Fhir.Api.Models;
using Microsoft.Health.Fhir.Core.Features;
using Microsoft.Health.Fhir.Core.Features.Context;
using Microsoft.Health.Fhir.Core.Features.Persistence;
using Microsoft.Health.Fhir.Core.Features.Routing;
using Microsoft.Health.Fhir.Core.Messages.Delete;
using Microsoft.Health.Fhir.Core.Models;
using Microsoft.Health.Fhir.Tests.Common;
using Microsoft.Health.Test.Utilities;
using NSubstitute;
using Xunit;

namespace Microsoft.Health.Fhir.Api.UnitTests.Controllers
Expand All @@ -22,6 +40,42 @@ namespace Microsoft.Health.Fhir.Api.UnitTests.Controllers
public sealed class FhirControllerTests
{
private readonly Type _targetFhirControllerClass = typeof(FhirController);
private readonly FhirController _fhirController;
private readonly IMediator _mediator;
private readonly RequestContextAccessor<IFhirRequestContext> _requestContextAccessor;
private readonly IUrlResolver _urlResolver;
private readonly IOptions<FeatureConfiguration> _configuration;
private readonly IAuthorizationService _authorizationService;

public FhirControllerTests()
{
_mediator = Substitute.For<IMediator>();
_requestContextAccessor = Substitute.For<RequestContextAccessor<IFhirRequestContext>>();
_urlResolver = Substitute.For<IUrlResolver>();
_configuration = Substitute.For<IOptions<FeatureConfiguration>>();
_configuration.Value.Returns(new FeatureConfiguration());
_authorizationService = Substitute.For<IAuthorizationService>();

_mediator.Send(
Arg.Any<DeleteResourceRequest>(),
Arg.Any<CancellationToken>())
.Returns(Task.FromResult(new DeleteResourceResponse(new ResourceKey(KnownResourceTypes.Patient, Guid.NewGuid().ToString()))));
_mediator.Send(
Arg.Any<ConditionalDeleteResourceRequest>(),
Arg.Any<CancellationToken>())
.Returns(Task.FromResult(new DeleteResourceResponse(new ResourceKey(KnownResourceTypes.Patient, Guid.NewGuid().ToString()))));
_fhirController = new FhirController(
_mediator,
_requestContextAccessor,
_urlResolver,
_configuration,
_authorizationService);
_fhirController.ControllerContext = new ControllerContext(
new ActionContext(
Substitute.For<HttpContext>(),
new RouteData(),
new ControllerActionDescriptor()));
}

[Fact]
public void WhenProvidedAFhirController_CheckIfAllExpectedServiceFilterAttributesArePresent()
Expand Down Expand Up @@ -83,6 +137,46 @@ public void WhenProvidedAFhirController_CheckIfTheCrudEndpointsHaveTheLatencyMet
TestIfTargetMethodContainsCustomAttribute(expectedCustomAttribute, "VRead", _targetFhirControllerClass);
}

[Theory]
[InlineData(KnownQueryParameterNames.BulkHardDelete, DeleteOperation.HardDelete)]
[InlineData(KnownQueryParameterNames.HardDelete, DeleteOperation.HardDelete)]
[InlineData(null, DeleteOperation.SoftDelete)]
[InlineData("", DeleteOperation.SoftDelete)]
[InlineData("xyz", DeleteOperation.SoftDelete)]
public async Task GivenConditionalDeleteResourceRequest_WhenHardDeleteFlagProvided_HardDeleteShouldBePerformed(string hardDeleteFlag, DeleteOperation operation)
{
HardDeleteModel hardDeleteModel = new HardDeleteModel
{
BulkHardDelete = (hardDeleteFlag?.Equals(KnownQueryParameterNames.BulkHardDelete) ?? false) ? true : null,
HardDelete = (hardDeleteFlag?.Equals(KnownQueryParameterNames.HardDelete) ?? false) ? true : null,
};

await _fhirController.ConditionalDelete(KnownResourceTypes.Patient, hardDeleteModel, null);
await _mediator.Received(1).Send(
Arg.Is<ConditionalDeleteResourceRequest>(x => x.DeleteOperation == operation),
Arg.Any<CancellationToken>());
}

[Theory]
[InlineData(KnownQueryParameterNames.BulkHardDelete, DeleteOperation.HardDelete)]
[InlineData(KnownQueryParameterNames.HardDelete, DeleteOperation.HardDelete)]
[InlineData(null, DeleteOperation.SoftDelete)]
[InlineData("", DeleteOperation.SoftDelete)]
[InlineData("xyz", DeleteOperation.SoftDelete)]
public async Task GivenDeleteResourceRequest_WhenHardDeleteFlagProvided_HardDeleteShouldBePerformed(string hardDeleteKey, DeleteOperation operation)
{
HardDeleteModel hardDeleteModel = new HardDeleteModel
{
BulkHardDelete = (hardDeleteKey?.Equals(KnownQueryParameterNames.BulkHardDelete) ?? false) ? true : null,
HardDelete = (hardDeleteKey?.Equals(KnownQueryParameterNames.HardDelete) ?? false) ? true : null,
};

await _fhirController.Delete(KnownResourceTypes.Patient, Guid.NewGuid().ToString(), hardDeleteModel, false);
await _mediator.Received(1).Send(
Arg.Is<DeleteResourceRequest>(x => x.DeleteOperation == operation),
Arg.Any<CancellationToken>());
}

private static void TestIfTargetMethodContainsCustomAttribute(Type expectedCustomAttributeType, string methodName, Type targetClassType)
{
MethodInfo bundleMethodInfo = targetClassType.GetMethod(methodName);
Expand Down
10 changes: 6 additions & 4 deletions src/Microsoft.Health.Fhir.Shared.Api/Controllers/BulkDeleteController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Microsoft.Health.Fhir.Api.Features.Filters;
using Microsoft.Health.Fhir.Api.Features.Headers;
using Microsoft.Health.Fhir.Api.Features.Routing;
using Microsoft.Health.Fhir.Api.Models;
using Microsoft.Health.Fhir.Core.Exceptions;
using Microsoft.Health.Fhir.Core.Features;
using Microsoft.Health.Fhir.Core.Features.Conformance.Models;
Expand All @@ -38,6 +39,7 @@ public class BulkDeleteController : Controller

private readonly HashSet<string> _exlcudedParameters = new(new PropertyEqualityComparer<string>(StringComparison.OrdinalIgnoreCase, s => s))
{
KnownQueryParameterNames.BulkHardDelete,
KnownQueryParameterNames.HardDelete,
KnownQueryParameterNames.PurgeHistory,
};
Expand All @@ -54,18 +56,18 @@ public BulkDeleteController(
[Route(KnownRoutes.BulkDelete)]
[ServiceFilter(typeof(ValidateAsyncRequestFilterAttribute))]
[AuditEventType(AuditEventSubType.BulkDelete)]
public async Task<IActionResult> BulkDelete([FromQuery(Name = KnownQueryParameterNames.HardDelete)] bool hardDelete, [FromQuery(Name = KnownQueryParameterNames.PurgeHistory)] bool purgeHistory)
public async Task<IActionResult> BulkDelete(HardDeleteModel hardDeleteModel, [FromQuery(Name = KnownQueryParameterNames.PurgeHistory)] bool purgeHistory)
{
return await SendDeleteRequest(null, hardDelete, purgeHistory, false);
return await SendDeleteRequest(null, hardDeleteModel.IsHardDelete, purgeHistory, false);
}

[HttpDelete]
[Route(KnownRoutes.BulkDeleteResourceType)]
[ServiceFilter(typeof(ValidateAsyncRequestFilterAttribute))]
[AuditEventType(AuditEventSubType.BulkDelete)]
public async Task<IActionResult> BulkDeleteByResourceType(string typeParameter, [FromQuery(Name = KnownQueryParameterNames.HardDelete)] bool hardDelete, [FromQuery(Name = KnownQueryParameterNames.PurgeHistory)] bool purgeHistory)
public async Task<IActionResult> BulkDeleteByResourceType(string typeParameter, HardDeleteModel hardDeleteModel, [FromQuery(Name = KnownQueryParameterNames.PurgeHistory)] bool purgeHistory)
{
return await SendDeleteRequest(typeParameter, hardDelete, purgeHistory, false);
return await SendDeleteRequest(typeParameter, hardDeleteModel.IsHardDelete, purgeHistory, false);
}

[HttpDelete]
Expand Down
12 changes: 6 additions & 6 deletions src/Microsoft.Health.Fhir.Shared.Api/Controllers/FhirController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,19 +395,19 @@ public async Task<IActionResult> VRead(string typeParameter, string idParameter,
/// </summary>
/// <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="hardDeleteModel">The model for hard-delete 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)]
[TypeFilter(typeof(CrudEndpointMetricEmitterAttribute))]
public async Task<IActionResult> Delete(string typeParameter, string idParameter, [FromQuery] bool hardDelete, [FromQuery] bool allowPartialSuccess)
public async Task<IActionResult> Delete(string typeParameter, string idParameter, HardDeleteModel hardDeleteModel, [FromQuery] bool allowPartialSuccess)
{
DeleteResourceResponse response = await _mediator.DeleteResourceAsync(
new DeleteResourceRequest(
new ResourceKey(typeParameter, idParameter),
hardDelete ? DeleteOperation.HardDelete : DeleteOperation.SoftDelete,
hardDeleteModel.IsHardDelete ? DeleteOperation.HardDelete : DeleteOperation.SoftDelete,
GetBundleResourceContext(),
allowPartialSuccess),
HttpContext.RequestAborted);
Expand Down Expand Up @@ -442,12 +442,12 @@ public async Task<IActionResult> PurgeHistory(string typeParameter, string idPar
/// Deletes the specified resource
/// </summary>
/// <param name="typeParameter">The type.</param>
/// <param name="hardDelete">A flag indicating whether to hard-delete the resource or not.</param>
/// <param name="hardDeleteModel">The model for hard-delete indicating whether to hard-delete the resource or not.</param>
/// <param name="maxDeleteCount">Specifies the maximum number of resources that can be deleted.</param>
[HttpDelete]
[Route(KnownRoutes.ResourceType)]
[AuditEventType(AuditEventSubType.ConditionalDelete)]
public async Task<IActionResult> ConditionalDelete(string typeParameter, [FromQuery] bool hardDelete, [FromQuery(Name = KnownQueryParameterNames.Count)] int? maxDeleteCount)
public async Task<IActionResult> ConditionalDelete(string typeParameter, HardDeleteModel hardDeleteModel, [FromQuery(Name = KnownQueryParameterNames.Count)] int? maxDeleteCount)
{
IReadOnlyList<Tuple<string, string>> conditionalParameters = GetQueriesForSearch();

Expand All @@ -457,7 +457,7 @@ public async Task<IActionResult> ConditionalDelete(string typeParameter, [FromQu
new ConditionalDeleteResourceRequest(
typeParameter,
conditionalParameters,
hardDelete ? DeleteOperation.HardDelete : DeleteOperation.SoftDelete,
hardDeleteModel.IsHardDelete ? DeleteOperation.HardDelete : DeleteOperation.SoftDelete,
maxDeleteCount.GetValueOrDefault(1),
GetBundleResourceContext()),
HttpContext.RequestAborted);
Expand Down
8 changes: 5 additions & 3 deletions test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,10 @@ public async Task GivenSoftBulkDeleteRequest_WhenCompleted_ThenHistoricalRecords
Assert.Equal(2, history.Resource.Entry.Count);
}

[SkippableFact]
public async Task GivenHardBulkDeleteRequest_WhenCompleted_ThenHistoricalRecordsDontExist()
[SkippableTheory]
[InlineData(KnownQueryParameterNames.BulkHardDelete)]
[InlineData(KnownQueryParameterNames.HardDelete)]
public async Task GivenHardBulkDeleteRequest_WhenCompleted_ThenHistoricalRecordsDontExist(string hardDeleteKey)
{
CheckBulkDeleteEnabled();

Expand All @@ -126,7 +128,7 @@ public async Task GivenHardBulkDeleteRequest_WhenCompleted_ThenHistoricalRecords
tag,
queryParams: new Dictionary<string, string>
{
{ "_hardDelete", "true" },
{ hardDeleteKey, "true" },
});

using HttpResponseMessage response = await _httpClient.SendAsync(request);
Expand Down
12 changes: 6 additions & 6 deletions test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/ConditionalDeleteTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,19 @@ public async Task GivenNoExistingResources_WhenDeletingConditionally_TheServerSh
Assert.Equal(HttpStatusCode.NoContent, response.StatusCode);
}

[InlineData(true, 1)]
[InlineData(true, 100)]
[InlineData(false, 1)]
[InlineData(false, 100)]
[InlineData(KnownQueryParameterNames.BulkHardDelete, true, 1)]
[InlineData(KnownQueryParameterNames.HardDelete, true, 100)]
[InlineData(KnownQueryParameterNames.HardDelete, false, 1)]
[InlineData(KnownQueryParameterNames.BulkHardDelete, false, 100)]
[Theory]
[Trait(Traits.Priority, Priority.One)]
public async Task GivenOneMatchingResource_WhenDeletingConditionally_TheServerShouldDeleteSuccessfully(bool hardDelete, int deleteCount)
public async Task GivenOneMatchingResource_WhenDeletingConditionally_TheServerShouldDeleteSuccessfully(string hardDeleteKey, bool hardDeleteValue, int deleteCount)
{
var identifier = Guid.NewGuid().ToString();
await CreateWithIdentifier(identifier);
await ValidateResults(identifier, 1);

FhirResponse response = await _client.DeleteAsync($"{_resourceType}?identifier={identifier}&hardDelete={hardDelete}&_count={deleteCount}", CancellationToken.None);
FhirResponse response = await _client.DeleteAsync($"{_resourceType}?identifier={identifier}&{hardDeleteKey}={hardDeleteValue}&_count={deleteCount}", CancellationToken.None);
Assert.Equal(HttpStatusCode.NoContent, response.StatusCode);

await ValidateResults(identifier, 0);
Expand Down
10 changes: 7 additions & 3 deletions test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/DeleteTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Hl7.Fhir.Model;
using Microsoft.Health.Fhir.Client;
using Microsoft.Health.Fhir.Core.Extensions;
using Microsoft.Health.Fhir.Core.Features;
using Microsoft.Health.Fhir.Core.Features.Persistence;
using Microsoft.Health.Fhir.Tests.Common;
using Microsoft.Health.Fhir.Tests.Common.FixtureParameters;
Expand Down Expand Up @@ -86,9 +87,11 @@ public async Task GivenADeletedResource_WhenSearchWithNotModifier_ThenDeletedRes
Assert.True(searchResults.Resource.Entry == null || searchResults.Resource.Entry.Count == 0);
}

[Fact]
[Theory]
[InlineData(KnownQueryParameterNames.BulkHardDelete)]
[InlineData(KnownQueryParameterNames.HardDelete)]
[Trait(Traits.Priority, Priority.One)]
public async Task GivenAResource_WhenHardDeleting_ThenServerShouldDeleteAllRelatedResourcesSuccessfully()
public async Task GivenAResource_WhenHardDeleting_ThenServerShouldDeleteAllRelatedResourcesSuccessfully(string hardDeleteKey)
{
List<string> versionIds = new List<string>();

Expand Down Expand Up @@ -118,7 +121,8 @@ public async Task GivenAResource_WhenHardDeleting_ThenServerShouldDeleteAllRelat
versionIds.Add(response3.Resource.Meta.VersionId);

// Hard-delete the resource.
await _client.HardDeleteAsync(observation);
string url = $"{observation.TypeName}/{observation.Id}?{hardDeleteKey}=true";
await _client.DeleteAsync(url);

// Getting the resource should result in NotFound.
await ExecuteAndValidateNotFoundStatus(() => _client.ReadAsync<Observation>(ResourceType.Observation, resourceId));
Expand Down

0 comments on commit 57f5421

Please sign in to comment.