diff --git a/docs/rest/customSearchExample.http b/docs/rest/customSearchExample.http index 73f6c3a6ec..a64fb256b8 100644 --- a/docs/rest/customSearchExample.http +++ b/docs/rest/customSearchExample.http @@ -147,3 +147,57 @@ Authorization: Bearer {{bearer.response.body.access_token}} ### Same as above but reversed. GET https://{{hostname}}/Patient?CreatedDate:missing=false&_sort=-CreatedDate Authorization: Bearer {{bearer.response.body.access_token}} + + +### Try to update an out of the box search param +PUT https://{{hostname}}/SearchParameter/AllergyIntolerance-clinical-status +content-type: application/json +Authorization: Bearer {{bearer.response.body.access_token}} + +{ + "resourceType": "SearchParameter", + "id": "AllergyIntolerance-clinical-status", + "url": "http://hl7.org/fhir/SearchParameter/AllergyIntolerance-clinical-status", + "version": "4.0.1", + "name": "clinical-status", + "status": "active", + "experimental": false, + "date": "2019-11-01T09:29:23+11:00", + "publisher": "Health Level Seven International (Patient Care)", + "contact": [ + { + "telecom": [ + { + "system": "url", + "value": "http://hl7.org/fhir" + } + ] + } + ], + "description": "active | inactive | resolved", + "code": "clinical-status", + "base": [ + "AllergyIntolerance" + ], + "type": "token", + "expression": "AllergyIntolerance.clinicalStatus", + "xpath": "f:AllergyIntolerance/f:clinicalStatus", + "xpathUsage": "normal", + "multipleOr": true, + "multipleAnd": true +} + +### Try to delete an out of the box search param +DELETE https://{{hostname}}/SearchParameter/AllergyIntolerance-clinical-status +content-type: application/json +Authorization: Bearer {{bearer.response.body.access_token}} + +### Delete custom search param example. +DELETE https://{{hostname}}/SearchParameter/resource-created-date2 +content-type: application/json +Authorization: Bearer {{bearer.response.body.access_token}} + +### Delete non-existent search param example. +DELETE https://{{hostname}}/SearchParameter/unknown +content-type: application/json +Authorization: Bearer {{bearer.response.body.access_token}} \ No newline at end of file diff --git a/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/DeleteSearchParameterBehavior.cs b/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/DeleteSearchParameterBehavior.cs index a89af97692..24fb5faa24 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/DeleteSearchParameterBehavior.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/DeleteSearchParameterBehavior.cs @@ -4,10 +4,13 @@ // ------------------------------------------------------------------------------------------------- using System; +using System.Linq; using System.Threading; using System.Threading.Tasks; using EnsureThat; using MediatR; +using Microsoft.Health.Fhir.Core.Exceptions; +using Microsoft.Health.Fhir.Core.Features.Definition; using Microsoft.Health.Fhir.Core.Features.Persistence; using Microsoft.Health.Fhir.Core.Messages.Delete; using Microsoft.Health.Fhir.Core.Models; @@ -17,33 +20,63 @@ namespace Microsoft.Health.Fhir.Core.Features.Search.Parameters public class DeleteSearchParameterBehavior : IPipelineBehavior where TDeleteResourceRequest : DeleteResourceRequest, IRequest { - private ISearchParameterOperations _searchParameterOperations; - private IFhirDataStore _fhirDataStore; + private readonly ISearchParameterOperations _searchParameterOperations; + private readonly IFhirDataStore _fhirDataStore; + private readonly ISearchParameterDefinitionManager _searchParameterDefinitionManager; - public DeleteSearchParameterBehavior(ISearchParameterOperations searchParameterOperations, IFhirDataStore fhirDataStore) + public DeleteSearchParameterBehavior( + ISearchParameterOperations searchParameterOperations, + IFhirDataStore fhirDataStore, + ISearchParameterDefinitionManager searchParameterDefinitionManager) { EnsureArg.IsNotNull(searchParameterOperations, nameof(searchParameterOperations)); EnsureArg.IsNotNull(fhirDataStore, nameof(fhirDataStore)); + EnsureArg.IsNotNull(searchParameterDefinitionManager, nameof(searchParameterDefinitionManager)); _searchParameterOperations = searchParameterOperations; _fhirDataStore = fhirDataStore; + _searchParameterDefinitionManager = searchParameterDefinitionManager; } public async Task Handle(TDeleteResourceRequest request, RequestHandlerDelegate next, CancellationToken cancellationToken) { var deleteRequest = request as DeleteResourceRequest; - ResourceWrapper searchParamResource = null; + ResourceWrapper customSearchParamResource = null; if (deleteRequest.ResourceKey.ResourceType.Equals(KnownResourceTypes.SearchParameter, StringComparison.Ordinal)) { - searchParamResource = await _fhirDataStore.GetAsync(deleteRequest.ResourceKey, cancellationToken); + customSearchParamResource = await _fhirDataStore.GetAsync(deleteRequest.ResourceKey, cancellationToken); + + // Check if this is a spec-defined SearchParameter (exists in definition manager but not in Resource table) + if (customSearchParamResource == null) + { + // Try to find by URL constructed from the resource ID + // Spec-defined parameters typically have URLs like: http://hl7.org/fhir/SearchParameter/{id} + var possibleUrl = $"http://hl7.org/fhir/SearchParameter/{deleteRequest.ResourceKey.Id}"; + if (_searchParameterDefinitionManager.TryGetSearchParameter(possibleUrl, out var searchParameterInfo)) + { + // Found in definition manager but not in Resource table - this is a spec-defined parameter + throw new MethodNotAllowedException(string.Format(Core.Resources.SearchParameterDefinitionCannotDeleteSpecDefined, possibleUrl)); + } + + // Also check all search parameters to see if any match this ID + var matchingParam = _searchParameterDefinitionManager.AllSearchParameters + .FirstOrDefault(sp => sp.Url.ToString().EndsWith($"/{deleteRequest.ResourceKey.Id}", StringComparison.OrdinalIgnoreCase)); + + if (matchingParam != null) + { + throw new MethodNotAllowedException(string.Format(Core.Resources.SearchParameterDefinitionCannotDeleteSpecDefined, matchingParam.Url)); + } + + // Not found in Resource table or definition manager - doesn't exist + throw new ResourceNotFoundException(string.Format(Core.Resources.ResourceNotFoundById, deleteRequest.ResourceKey.ResourceType, deleteRequest.ResourceKey.Id)); + } } - if (searchParamResource != null && searchParamResource.IsDeleted == false) + if (customSearchParamResource != null && customSearchParamResource.IsDeleted == false) { - // First update the in-memory SearchParameterDefinitionManager, and remove the status metadata from the data store - // then remove the SearchParameter resource from the data store - await _searchParameterOperations.DeleteSearchParameterAsync(searchParamResource.RawResource, cancellationToken); + // This is a custom search parameter + await _searchParameterOperations.DeleteSearchParameterAsync(customSearchParamResource.RawResource, cancellationToken); } return await next(cancellationToken); diff --git a/src/Microsoft.Health.Fhir.Core/Resources.Designer.cs b/src/Microsoft.Health.Fhir.Core/Resources.Designer.cs index 2b5dc0c803..1cf80fd515 100644 --- a/src/Microsoft.Health.Fhir.Core/Resources.Designer.cs +++ b/src/Microsoft.Health.Fhir.Core/Resources.Designer.cs @@ -1638,6 +1638,24 @@ internal static string SearchParameterDefinitionDuplicatedEntry { } } + /// + /// Looks up a localized string similar to Cannot update a spec-defined search parameter with URL '{0}'. Only custom search parameters can be updated.. + /// + internal static string SearchParameterDefinitionCannotUpdateSpecDefined { + get { + return ResourceManager.GetString("SearchParameterDefinitionCannotUpdateSpecDefined", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Cannot delete a spec-defined search parameter with URL '{0}'. Only custom search parameters can be deleted.. + /// + internal static string SearchParameterDefinitionCannotDeleteSpecDefined { + get { + return ResourceManager.GetString("SearchParameterDefinitionCannotDeleteSpecDefined", resourceCulture); + } + } + /// /// Looks up a localized string similar to SearchParameter[{0}].component is null or empty.. /// diff --git a/src/Microsoft.Health.Fhir.Core/Resources.resx b/src/Microsoft.Health.Fhir.Core/Resources.resx index 5a0f2b076f..05c13d7ce6 100644 --- a/src/Microsoft.Health.Fhir.Core/Resources.resx +++ b/src/Microsoft.Health.Fhir.Core/Resources.resx @@ -398,6 +398,12 @@ A search parameter with the same definition URL '{0}' already exists. + + Cannot update a spec-defined search parameter with URL '{0}'. Only custom search parameters can be updated. + + + Cannot delete a spec-defined search parameter with URL '{0}'. Only custom search parameters can be deleted. + SearchParameter[{0}].component is null or empty. {0} Is the Uri Identifying the SearchParameter diff --git a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterBehaviorTests.cs b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterBehaviorTests.cs index d4f590343e..ebf1e74eef 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterBehaviorTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterBehaviorTests.cs @@ -10,8 +10,10 @@ using Hl7.Fhir.Serialization; using Microsoft.Health.Fhir.Core.Exceptions; using Microsoft.Health.Fhir.Core.Extensions; +using Microsoft.Health.Fhir.Core.Features.Definition; using Microsoft.Health.Fhir.Core.Features.Persistence; using Microsoft.Health.Fhir.Core.Features.Search.Parameters; +using Microsoft.Health.Fhir.Core.Features.Search.Registry; using Microsoft.Health.Fhir.Core.Messages.Create; using Microsoft.Health.Fhir.Core.Messages.Delete; using Microsoft.Health.Fhir.Core.Messages.Upsert; @@ -109,7 +111,8 @@ public async Task GivenADeleteResourceRequest_WhenDeletingAResourceOtherThanSear var response = new DeleteResourceResponse(key); - var behavior = new DeleteSearchParameterBehavior(_searchParameterOperations, _fhirDataStore); + var searchParameterDefinitionManager = Substitute.For(); + var behavior = new DeleteSearchParameterBehavior(_searchParameterOperations, _fhirDataStore, searchParameterDefinitionManager); await behavior.Handle(request, async (ct) => await Task.Run(() => response), CancellationToken.None); // Ensure for non-SearchParameter, that we do not call Add SearchParameter @@ -130,7 +133,8 @@ public async Task GivenADeleteResourceRequest_WhenDeletingASearchParameterResour var response = new DeleteResourceResponse(key); - var behavior = new DeleteSearchParameterBehavior(_searchParameterOperations, _fhirDataStore); + var searchParameterDefinitionManager = Substitute.For(); + var behavior = new DeleteSearchParameterBehavior(_searchParameterOperations, _fhirDataStore, searchParameterDefinitionManager); await behavior.Handle(request, async (ct) => await Task.Run(() => response), CancellationToken.None); await _searchParameterOperations.Received().DeleteSearchParameterAsync(Arg.Any(), Arg.Any()); @@ -150,12 +154,56 @@ public async Task GivenADeleteResourceRequest_WhenDeletingAnAlreadyDeletedSearch var response = new DeleteResourceResponse(key); - var behavior = new DeleteSearchParameterBehavior(_searchParameterOperations, _fhirDataStore); + var searchParameterDefinitionManager = Substitute.For(); + var behavior = new DeleteSearchParameterBehavior(_searchParameterOperations, _fhirDataStore, searchParameterDefinitionManager); await behavior.Handle(request, async (ct) => await Task.Run(() => response), CancellationToken.None); await _searchParameterOperations.DidNotReceive().DeleteSearchParameterAsync(Arg.Any(), Arg.Any()); } + [Fact] + public async Task GivenADeleteResourceRequest_WhenDeletingASpecDefinedSearchParameterResource_ThenDeleteSearchParameterShouldNotBeCalled() + { + var searchParameter = new SearchParameter() { Id = "code", Url = "http://hl7.org/fhir/SearchParameter/code" }; + var key = new ResourceKey("SearchParameter", "code"); + var request = new DeleteResourceRequest(key, DeleteOperation.SoftDelete); + + // Spec-defined parameters do not exist in the Resource table, so GetAsync returns null + _fhirDataStore.GetAsync(key, Arg.Any()).Returns((ResourceWrapper)null); + + var response = new DeleteResourceResponse(key); + + // Setup the definition manager to recognize this as a spec-defined parameter + var searchParameterDefinitionManager = Substitute.For(); + var specDefinedInfo = new SearchParameterInfo( + "code", + "code", + ValueSets.SearchParamType.Token, + new System.Uri(searchParameter.Url)) + { + SearchParameterStatus = SearchParameterStatus.Supported, + }; + + // Mock TryGetSearchParameter to return false (it won't match the simple URL pattern) + // But AllSearchParameters will contain it and be found by the EndsWith check + searchParameterDefinitionManager.TryGetSearchParameter(Arg.Any(), out Arg.Any()) + .Returns(x => + { + x[1] = null; + return false; + }); + + searchParameterDefinitionManager.AllSearchParameters.Returns(new[] { specDefinedInfo }); + + var behavior = new DeleteSearchParameterBehavior(_searchParameterOperations, _fhirDataStore, searchParameterDefinitionManager); + + // Expect MethodNotAllowedException to be thrown for spec-defined parameters + await Assert.ThrowsAsync(() => behavior.Handle(request, async (ct) => await Task.Run(() => response), CancellationToken.None)); + + // Ensure DeleteSearchParameterAsync is not called for spec-defined parameters + await _searchParameterOperations.DidNotReceive().DeleteSearchParameterAsync(Arg.Any(), Arg.Any()); + } + [Fact] public async Task GivenAnUpsertResourceRequest_WhenSearchParameterDoesNotExist_ThenAddSearchParameterShouldBeCalled() { diff --git a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterValidatorTests.cs b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterValidatorTests.cs index e2c58911d2..3e55fddedf 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterValidatorTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterValidatorTests.cs @@ -8,9 +8,11 @@ using Hl7.Fhir.Model; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Health.Core.Features.Security.Authorization; +using Microsoft.Health.Extensions.DependencyInjection; using Microsoft.Health.Fhir.Core.Exceptions; using Microsoft.Health.Fhir.Core.Features.Definition; using Microsoft.Health.Fhir.Core.Features.Operations; +using Microsoft.Health.Fhir.Core.Features.Persistence; using Microsoft.Health.Fhir.Core.Features.Search.Parameters; using Microsoft.Health.Fhir.Core.Features.Search.Registry; using Microsoft.Health.Fhir.Core.Features.Security; @@ -37,10 +39,18 @@ public class SearchParameterValidatorTests private readonly IModelInfoProvider _modelInfoProvider = MockModelInfoProviderBuilder.Create(FhirSpecification.R4).AddKnownTypes("Patient").Build(); private readonly ISearchParameterOperations _searchParameterOperations = Substitute.For(); private readonly ISearchParameterComparer _searchParameterComparer = Substitute.For>(); + private readonly IFhirDataStore _fhirDataStore = Substitute.For(); + private readonly IScoped _fhirDataStoreScoped; public SearchParameterValidatorTests() { - SearchParameterInfo searchParameterInfo = new SearchParameterInfo("USCoreRace", "race") + _fhirDataStoreScoped = _fhirDataStore.CreateMockScope(); + + SearchParameterInfo searchParameterInfo = new SearchParameterInfo( + "USCoreRace", + "race", + ValueSets.SearchParamType.String, + new System.Uri("http://duplicate")) { SearchParameterStatus = SearchParameterStatus.Supported, }; @@ -71,13 +81,20 @@ public SearchParameterValidatorTests() return true; }); _fhirOperationDataStore.CheckActiveReindexJobsAsync(CancellationToken.None).Returns((false, string.Empty)); + + // Setup GetAsync to return a resource for "duplicate" (custom parameter) + // Extract ID from URL: "http://duplicate" -> "duplicate" + var mockResourceWrapper = Substitute.For(); + _fhirDataStore.GetAsync( + Arg.Is(key => key.Id == "duplicate"), + Arg.Any()).Returns(mockResourceWrapper); } [Theory] [MemberData(nameof(InvalidSearchParamData))] public async Task GivenInvalidSearchParam_WhenValidatingSearchParam_ThenResourceNotValidExceptionThrown(SearchParameter searchParam, string method) { - var validator = new SearchParameterValidator(() => _fhirOperationDataStore.CreateMockScope(), _authorizationService, _searchParameterDefinitionManager, _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, NullLogger.Instance); + var validator = new SearchParameterValidator(() => _fhirOperationDataStore.CreateMockScope(), _authorizationService, _searchParameterDefinitionManager, _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, _fhirDataStoreScoped, NullLogger.Instance); await Assert.ThrowsAsync(() => validator.ValidateSearchParameterInput(searchParam, method, CancellationToken.None)); } @@ -85,7 +102,7 @@ public async Task GivenInvalidSearchParam_WhenValidatingSearchParam_ThenResource [MemberData(nameof(ValidSearchParamData))] public async Task GivenValidSearchParam_WhenValidatingSearchParam_ThenNoExceptionThrown(SearchParameter searchParam, string method) { - var validator = new SearchParameterValidator(() => _fhirOperationDataStore.CreateMockScope(), _authorizationService, _searchParameterDefinitionManager, _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, NullLogger.Instance); + var validator = new SearchParameterValidator(() => _fhirOperationDataStore.CreateMockScope(), _authorizationService, _searchParameterDefinitionManager, _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, _fhirDataStoreScoped, NullLogger.Instance); await validator.ValidateSearchParameterInput(searchParam, method, CancellationToken.None); } @@ -94,7 +111,7 @@ public async Task GivenUnauthorizedUser_WhenValidatingSearchParam_ThenExceptionT { var authorizationService = Substitute.For>(); authorizationService.CheckAccess(DataActions.Reindex, Arg.Any()).Returns(DataActions.Write); - var validator = new SearchParameterValidator(() => _fhirOperationDataStore.CreateMockScope(), authorizationService, _searchParameterDefinitionManager, _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, NullLogger.Instance); + var validator = new SearchParameterValidator(() => _fhirOperationDataStore.CreateMockScope(), authorizationService, _searchParameterDefinitionManager, _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, _fhirDataStoreScoped, NullLogger.Instance); await Assert.ThrowsAsync(() => validator.ValidateSearchParameterInput(new SearchParameter(), "POST", CancellationToken.None)); } @@ -103,7 +120,7 @@ public async Task GivenUnauthorizedUser_WhenValidatingSearchParam_ThenExceptionT [MemberData(nameof(DuplicateCodeAtBaseResourceData))] public async Task GivenInvalidSearchParamWithDuplicateCode_WhenValidatingSearchParam_ThenResourceNotValidExceptionThrown(SearchParameter searchParam, string method) { - var validator = new SearchParameterValidator(() => _fhirOperationDataStore.CreateMockScope(), _authorizationService, _searchParameterDefinitionManager, _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, NullLogger.Instance); + var validator = new SearchParameterValidator(() => _fhirOperationDataStore.CreateMockScope(), _authorizationService, _searchParameterDefinitionManager, _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, _fhirDataStoreScoped, NullLogger.Instance); await Assert.ThrowsAsync(() => validator.ValidateSearchParameterInput(searchParam, method, CancellationToken.None)); } @@ -122,7 +139,7 @@ public async Task GivenValidSearchParamWithDuplicateUrl_WhenValidatingSearchPara return true; }); - var validator = new SearchParameterValidator(() => _fhirOperationDataStore.CreateMockScope(), _authorizationService, _searchParameterDefinitionManager, _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, NullLogger.Instance); + var validator = new SearchParameterValidator(() => _fhirOperationDataStore.CreateMockScope(), _authorizationService, _searchParameterDefinitionManager, _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, _fhirDataStoreScoped, NullLogger.Instance); if (searchParameterStatus == SearchParameterStatus.PendingDelete) { // Expecting no exception being thrown. @@ -168,6 +185,7 @@ public async Task GivenSearchParameter_WhenValidatingProperties_ThenConflictingP _modelInfoProvider, _searchParameterOperations, _searchParameterComparer, + _fhirDataStoreScoped, NullLogger.Instance); _searchParameterDefinitionManager.TryGetSearchParameter("DocumentReference", "relationship", Arg.Any(), out Arg.Any()).Returns( @@ -263,5 +281,138 @@ public static IEnumerable DuplicateUrlData() data.Add(new object[] { searchParam, "POST", SearchParameterStatus.PendingDelete }); return data; } + + [Fact] + public async Task GivenPutRequestOnSpecDefinedSearchParameter_WhenValidatingSearchParam_ThenMethodNotAllowedExceptionThrown() + { + // Spec-defined search parameters exist in the definition manager but have NO stored resource + var specDefinedUrl = "http://spec-defined"; + var searchParam = new SearchParameter { Url = specDefinedUrl, Id = "spec-123" }; + + // Setup: Parameter exists in definition manager + SearchParameterInfo specDefinedInfo = new SearchParameterInfo( + "SpecDefined", + "spec-code", + ValueSets.SearchParamType.String, + new System.Uri(specDefinedUrl)) + { + SearchParameterStatus = SearchParameterStatus.Supported, + }; + _searchParameterDefinitionManager.TryGetSearchParameter(specDefinedUrl, out Arg.Any()).Returns( + x => + { + x[1] = specDefinedInfo; + return true; + }); + + // Setup: No stored resource (returns null) + var resourceKey = new ResourceKey(KnownResourceTypes.SearchParameter, searchParam.Id); + _fhirDataStore.GetAsync(resourceKey, Arg.Any()).Returns((ResourceWrapper)null); + + var validator = new SearchParameterValidator( + () => _fhirOperationDataStore.CreateMockScope(), + _authorizationService, + _searchParameterDefinitionManager, + _modelInfoProvider, + _searchParameterOperations, + _searchParameterComparer, + _fhirDataStoreScoped, + NullLogger.Instance); + + // PUT on spec-defined SearchParameter should fail + await Assert.ThrowsAsync(() => validator.ValidateSearchParameterInput(searchParam, "PUT", CancellationToken.None)); + } + + [Fact] + public async Task GivenPutRequestOnCustomSearchParameter_WhenValidatingSearchParam_ThenNoExceptionThrown() + { + // Custom search parameters exist in BOTH the definition manager AND have a stored resource + var customUrl = "http://custom"; + var searchParam = new SearchParameter { Url = customUrl, Id = "custom-123", Code = "custom-code" }; +#if Stu3 || R4 || R4B + searchParam.Base = new[] { ResourceType.Patient as ResourceType? }; +#else + searchParam.Base = new[] { VersionIndependentResourceTypesAll.Patient as VersionIndependentResourceTypesAll? }; +#endif + + // Setup: Parameter exists in definition manager + SearchParameterInfo customInfo = new SearchParameterInfo( + "CustomParam", + "custom-code", + ValueSets.SearchParamType.String, + new System.Uri(customUrl)) + { + SearchParameterStatus = SearchParameterStatus.Supported, + }; + _searchParameterDefinitionManager.TryGetSearchParameter(customUrl, out Arg.Any()).Returns( + x => + { + x[1] = customInfo; + return true; + }); + + // Setup: Code does not conflict + _searchParameterDefinitionManager.TryGetSearchParameter("Patient", "custom-code", Arg.Any(), out _).Returns(false); + + // Setup: Stored resource exists (does NOT throw ResourceNotFoundException) + var resourceKey = new ResourceKey(KnownResourceTypes.SearchParameter, searchParam.Id); + var mockResourceWrapper = Substitute.For(); + _fhirDataStore.GetAsync(resourceKey, Arg.Any()).Returns(mockResourceWrapper); + + var validator = new SearchParameterValidator( + () => _fhirOperationDataStore.CreateMockScope(), + _authorizationService, + _searchParameterDefinitionManager, + _modelInfoProvider, + _searchParameterOperations, + _searchParameterComparer, + _fhirDataStoreScoped, + NullLogger.Instance); + + // PUT on custom SearchParameter should succeed (no exception) + await validator.ValidateSearchParameterInput(searchParam, "PUT", CancellationToken.None); + } + + [Fact] + public async Task GivenDeleteRequestOnCustomSearchParameter_WhenValidatingSearchParam_ThenNoExceptionThrown() + { + // Custom search parameters exist in BOTH the definition manager AND have a stored resource + var customUrl = "http://custom-delete"; + var searchParam = new SearchParameter { Url = customUrl, Id = "custom-delete-123" }; + + // Setup: Parameter exists in definition manager + SearchParameterInfo customInfo = new SearchParameterInfo( + "CustomParamDelete", + "custom-delete-code", + ValueSets.SearchParamType.String, + new System.Uri(customUrl)) + { + SearchParameterStatus = SearchParameterStatus.Supported, + }; + _searchParameterDefinitionManager.TryGetSearchParameter(customUrl, out Arg.Any()).Returns( + x => + { + x[1] = customInfo; + return true; + }); + + // Setup: Stored resource exists (does NOT throw ResourceNotFoundException) + var resourceKey = new ResourceKey(KnownResourceTypes.SearchParameter, searchParam.Id); + var mockResourceWrapper = Substitute.For(); + _fhirDataStore.GetAsync(resourceKey, Arg.Any()).Returns(mockResourceWrapper); + + var validator = new SearchParameterValidator( + () => _fhirOperationDataStore.CreateMockScope(), + _authorizationService, + _searchParameterDefinitionManager, + _modelInfoProvider, + _searchParameterOperations, + _searchParameterComparer, + _fhirDataStoreScoped, + NullLogger.Instance); + + // DELETE on custom SearchParameter should succeed (no exception) + await validator.ValidateSearchParameterInput(searchParam, "DELETE", CancellationToken.None); + } } } diff --git a/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/Parameters/SearchParameterValidator.cs b/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/Parameters/SearchParameterValidator.cs index 822bc1d07b..fae6762900 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/Parameters/SearchParameterValidator.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/Parameters/SearchParameterValidator.cs @@ -19,6 +19,7 @@ using Microsoft.Health.Fhir.Core.Exceptions; using Microsoft.Health.Fhir.Core.Features.Definition; using Microsoft.Health.Fhir.Core.Features.Operations; +using Microsoft.Health.Fhir.Core.Features.Persistence; using Microsoft.Health.Fhir.Core.Features.Search; using Microsoft.Health.Fhir.Core.Features.Search.Parameters; using Microsoft.Health.Fhir.Core.Features.Search.Registry; @@ -40,6 +41,7 @@ public class SearchParameterValidator : ISearchParameterValidator private readonly IModelInfoProvider _modelInfoProvider; private readonly ISearchParameterOperations _searchParameterOperations; private readonly ISearchParameterComparer _searchParameterComparer; + private readonly IScoped _fhirDataStore; private readonly ILogger _logger; private const string HttpPostName = "POST"; @@ -54,6 +56,7 @@ public SearchParameterValidator( IModelInfoProvider modelInfoProvider, ISearchParameterOperations searchParameterOperations, ISearchParameterComparer searchParameterComparer, + IScoped fhirDataStore, ILogger logger) { EnsureArg.IsNotNull(fhirOperationDataStoreFactory, nameof(fhirOperationDataStoreFactory)); @@ -62,6 +65,7 @@ public SearchParameterValidator( EnsureArg.IsNotNull(modelInfoProvider, nameof(modelInfoProvider)); EnsureArg.IsNotNull(searchParameterOperations, nameof(searchParameterOperations)); EnsureArg.IsNotNull(searchParameterComparer, nameof(searchParameterComparer)); + EnsureArg.IsNotNull(fhirDataStore, nameof(fhirDataStore)); _fhirOperationDataStoreFactory = fhirOperationDataStoreFactory; _authorizationService = authorizationService; @@ -69,6 +73,7 @@ public SearchParameterValidator( _modelInfoProvider = modelInfoProvider; _searchParameterOperations = searchParameterOperations; _searchParameterComparer = searchParameterComparer; + _fhirDataStore = fhirDataStore; _logger = EnsureArg.IsNotNull(logger, nameof(logger)); } @@ -89,7 +94,7 @@ public async Task ValidateSearchParameterInput(SearchParameter searchParam, stri } } - if (string.IsNullOrEmpty(searchParam.Url) && (method.Equals(HttpDeleteName, StringComparison.Ordinal) || method.Equals(HttpPatchName, StringComparison.Ordinal))) + if ((string.IsNullOrEmpty(searchParam.Url) && method.Equals(HttpDeleteName, StringComparison.Ordinal)) || method.Equals(HttpPatchName, StringComparison.Ordinal)) { // Return out if this is delete OR patch call and no Url so FHIRController can move to next action return; @@ -143,7 +148,7 @@ public async Task ValidateSearchParameterInput(SearchParameter searchParam, stri } else if (method.Equals(HttpPutName, StringComparison.OrdinalIgnoreCase)) { - CheckForConflictingCodeValue(searchParam, validationFailures); + await ValidateOperationOnExistingSearchParameter(searchParam, searchParameterInfo, validationFailures, cancellationToken); } } else @@ -171,6 +176,42 @@ public async Task ValidateSearchParameterInput(SearchParameter searchParam, stri } } + private async Task ValidateOperationOnExistingSearchParameter( + SearchParameter searchParam, + SearchParameterInfo searchParameterInfo, + List validationFailures, + CancellationToken cancellationToken) + { + // Check if this is a spec-defined SearchParameter by checking if it exists in the Resource table + // Spec-defined parameters exist only in SearchParam table (not in Resource table) + // Custom parameters exist in both SearchParam and Resource tables + + // Extract the ID from the searchParameterInfo.Url (last segment after the last '/') + // For spec-defined parameters, the URL is like: http://hl7.org/fhir/SearchParameter/AllergyIntolerance-clinical-status + // For custom parameters, the ID would be in searchParam.Id or derived from the URL + string searchParamId = searchParam.Id ?? searchParameterInfo.Url.ToString().TrimEnd('/').Split('/').Last(); + + // Try to get the resource from the Resource table using the ID + var resourceKey = new ResourceKey(KnownResourceTypes.SearchParameter, searchParamId); + var existingResource = await _fhirDataStore.Value.GetAsync(resourceKey, cancellationToken); + + if (existingResource != null) + { + // Resource exists in Resource table, so it's a custom SearchParameter + // Allow the PUT operation and validate for code conflicts + CheckForConflictingCodeValue(searchParam, validationFailures); + } + else + { + // No resource in Resource table - this is a spec-defined SearchParameter + // Block PUT operations on spec-defined parameters with MethodNotAllowedException (405) + var errorMessage = string.Format(Resources.SearchParameterDefinitionCannotUpdateSpecDefined, searchParam.Url); + + _logger.LogInformation("Attempted to update a spec-defined search parameter. url: {Url}", searchParam.Url); + throw new MethodNotAllowedException(errorMessage); + } + } + private void CheckForConflictingCodeValue(SearchParameter searchParam, List validationFailures) { // Ensure the search parameter's code value does not already exist for its base type(s)