diff --git a/docs/rest/Bug Repros/Pagination Bug - 8-2024/PaginationBug.http b/docs/rest/Bug Repros/Pagination Bug - 8-2024/PaginationBug.http new file mode 100644 index 000000000..a6d297792 --- /dev/null +++ b/docs/rest/Bug Repros/Pagination Bug - 8-2024/PaginationBug.http @@ -0,0 +1,110 @@ +@hostname = localhost:44348 + +### Get the bearer token, if authentication is enabled +# @name bearer +POST https://{{hostname}}/connect/token +content-type: application/x-www-form-urlencoded + +grant_type=client_credentials +&client_id=globalAdminServicePrincipal +&client_secret=globalAdminServicePrincipal +&scope=fhir-api + +### +@token = {{bearer.response.body.access_token}} + +### Pagination bug 1 - Counting included resources and missing matched resources when using include, decending sort, and the right _count value +# Setup test data +# @name batch +POST https://{{hostname}} +Content-Type: application/json +Authorization: Bearer {{token}} + +< ./PaginationBugData.json + +### Reindex +# Trigger a reindexing operation. +# @name reindex +POST https://{{hostname}}/$reindex HTTP/1.1 +Authorization: Bearer {{token}} +content-type: application/json + +{ "resourceType": "Parameters", "parameter": [] } + +### Records the reindex job location +@reindexLocation = {{reindex.response.headers.Content-Location}} + +### +# Check the status of the reindexing operation +GET {{reindexLocation}} HTTP/1.1 +Authorization: Bearer {{token}} + +### +# Count <=2 gives one match and no next link, total = 1 +# Count =3 gives one match and a next link, total = 2 +# Count >=4 gives two matches and no next link, total = 2 +# +# With just Condition:extension-care-goals a count of 2 produces the issue +POST https://{{hostname}}/Condition/_search +Content-Type: application/x-www-form-urlencoded +Accept: application/json, text/plain, */* +Authorization: Bearer {{token}} + +patient=859f091b-75a0-4690-8f45-ce192c5e045a +&category=main +&_count=2 +&_total=accurate +&_sort=-onset-date +&_include=Condition:extension-care-goals + +### +# when the sort order is decreasing +GET https://{{hostname}}/MedicationDispense?_include=MedicationDispense:prescription&_sort=-whenprepared&_count=3&_total=accurate&_tag=a696ad4e-9e07-4266-8727-d1e3b9f193cd +Authorization: Bearer {{token}} + + +### Pagination bug 2 - Reverse chain counting deleted resources when it shouldn't +# Setup data +POST https://{{hostname}} +Content-Type: application/json +Authorization: Bearer {{token}} + +< ./PaginationBugData2.json + +### Delete Patient +DELETE https://{{hostname}}/Patient/pagination-patient2 +Authorization: Bearer {{token}} + +### Gets the first patient but has the wrong total value +# @name getservicerequest +POST https://{{hostname}}/Patient/_search +Content-Type: application/x-www-form-urlencoded +Authorization: Bearer {{token}} + +_total=accurate +&_count=1 +&_has:CareTeam:patient:participant=pagination-practitioner1 + +### Gets the summary count (also wrong) +GET https://{{hostname}}/Patient?_summary=count&_has:CareTeam:patient:participant=pagination-practitioner1 +Authorization: Bearer {{token}} + +### Tests if a non-existant referenced resource is counted (it isn't) +PUT https://{{hostname}}/CareTeam/pagination-careteam4 +Content-Type: application/json +Authorization: Bearer {{token}} + +{ + "resourceType": "CareTeam", + "id": "pagination-careteam4", + "participant": [ + { + "member": { + "reference": "Practitioner/pagination-practitioner1" + } + } + ], + "subject": { + "reference": "Patient/pagination-patient-invalid" + } +} diff --git a/docs/rest/Bug Repros/Pagination Bug - 8-2024/PaginationBugData.json b/docs/rest/Bug Repros/Pagination Bug - 8-2024/PaginationBugData.json new file mode 100644 index 000000000..2718afd75 --- /dev/null +++ b/docs/rest/Bug Repros/Pagination Bug - 8-2024/PaginationBugData.json @@ -0,0 +1,153 @@ +{ + "resourceType": "Bundle", + "type": "batch", + "entry": [ + { + "resource": { + "resourceType": "Condition", + "id": "c96b32a1-ac09-4c1a-8007-0baa001f16ac", + "extension": [ + { + "extension": [ + { + "url": "care_goal", + "valueReference": { + "reference": "Goal/fc7e482e-c69c-4027-9d06-a3ca1131c03f", + "type": "Goal", + "display": "John Smith" + } + }, + { + "url": "care_goal", + "valueReference": { + "reference": "Goal/18f1d605-351e-4db9-ae2a-c2749b42ccbf", + "type": "Goal", + "display": "John Smith" + } + }], + "url": "https://domo.health/fhir/extension/care_goals" + }], + "category": [ + { + "coding": [ + { + "system": "http://www.domo.health/coding", + "code": "main", + "display": "John Smith" + }], + "text": "Diagnosis that can have Subsidiary diagnosis" + }], + "subject": { + "reference": "Patient/859f091b-75a0-4690-8f45-ce192c5e045a", + "type": "Patient" + }, + "onsetDateTime": "2024", + "recordedDate": "2024-08-07T10:50:35.441Z" + }, + "request": { + "method": "PUT", + "url": "Condition/c96b32a1-ac09-4c1a-8007-0baa001f16ac" + } + }, + { + "resource": { + "resourceType": "Goal", + "id": "fc7e482e-c69c-4027-9d06-a3ca1131c03f", + "lifecycleStatus": "active", + "subject": { + "reference": "Patient/859f091b-75a0-4690-8f45-ce192c5e045a", + "type": "Patient" + }, + "description": [ + { + "text": "something" + }], + "startDate": "2024-06-24", + "statusDate": "2024-06-24" + }, + "request": { + "method": "PUT", + "url": "Goal/fc7e482e-c69c-4027-9d06-a3ca1131c03f" + } + }, + { + "resource": { + "resourceType": "Goal", + "id": "18f1d605-351e-4db9-ae2a-c2749b42ccbf", + "lifecycleStatus": "active", + "subject": { + "reference": "Patient/859f091b-75a0-4690-8f45-ce192c5e045a", + "type": "Patient" + }, + "description": [ + { + "text": "something" + }], + "startDate": "2024-07-10", + "statusDate": "2024-07-10" + }, + "request": { + "method": "PUT", + "url": "Goal/18f1d605-351e-4db9-ae2a-c2749b42ccbf" + } + }, + { + "resource": { + "resourceType": "Condition", + "id": "7b06a53c-1eb4-4352-a810-d9b4651d1b1f", + "category": [ + { + "coding": [ + { + "system": "http://www.domo.health/coding", + "code": "main", + "display": "John Smith" + }], + "text": "Diagnosis that can have Subsidiary diagnosis" + }], + "subject": { + "reference": "Patient/859f091b-75a0-4690-8f45-ce192c5e045a", + "type": "Patient" + }, + "recordedDate": "2024-06-12T11:40:41.846Z" + }, + "request": { + "method": "PUT", + "url": "Condition/7b06a53c-1eb4-4352-a810-d9b4651d1b1f" + } + }, + { + "resource": { + "resourceType": "SearchParameter", + "url": "https://domo.health/fhir/search-params/extension-care-goals", + "name": "extensionCareGoals", + "publisher": "DomoSafety SA", + "status": "active", + "contact": [ + { + "telecom": [ + { + "system": "url", + "value": "https://www.domo-safety.com/" + }] + }], + "description": "Returns care goals with a goal id matching the specified string.", + "code": "extension-care-goals", + "base": [ + "Composition", + "Condition", + "MedicationRequest", + "ServiceRequest" + ], + "type": "reference", + "expression": "extension.where(url = 'https://domo.health/fhir/extension/care_goals').extension.where(url = 'care_goal').value", + "multipleOr": true, + "multipleAnd": true + }, + "request": { + "method": "POST", + "url": "SearchParameter" + } + } + ] +} diff --git a/docs/rest/Bug Repros/Pagination Bug - 8-2024/PaginationBugData2.json b/docs/rest/Bug Repros/Pagination Bug - 8-2024/PaginationBugData2.json new file mode 100644 index 000000000..d7db5839f --- /dev/null +++ b/docs/rest/Bug Repros/Pagination Bug - 8-2024/PaginationBugData2.json @@ -0,0 +1,106 @@ +{ + "resourceType": "Bundle", + "type": "batch", + "entry": [ + { + "resource": { + "resourceType": "CareTeam", + "id": "pagination-careteam1", + "participant": [ + { + "member": { + "reference": "Practitioner/pagination-practitioner1" + } + } + ], + "subject": { + "reference": "Patient/pagination-patient1" + } + }, + "request": { + "method": "PUT", + "url": "CareTeam/pagination-careteam1" + } + }, + { + "resource": { + "resourceType": "CareTeam", + "id": "pagination-careteam2", + "participant": [ + { + "member": { + "reference": "Practitioner/pagination-practitioner1" + } + } + ], + "subject": { + "reference": "Patient/pagination-patient2" + } + }, + "request": { + "method": "PUT", + "url": "CareTeam/pagination-careteam2" + } + }, + { + "resource": { + "resourceType": "CareTeam", + "id": "pagination-careteam3", + "participant": [ + { + "member": { + "reference": "Practitioner/pagination-practitioner1" + } + } + ], + "subject": { + "reference": "Patient/pagination-patient3" + } + }, + "request": { + "method": "PUT", + "url": "CareTeam/pagination-careteam3" + } + }, + { + "resource": { + "resourceType": "Patient", + "id": "pagination-patient1" + }, + "request": { + "method": "PUT", + "url": "Patient/pagination-patient1" + } + }, + { + "resource": { + "resourceType": "Patient", + "id": "pagination-patient2" + }, + "request": { + "method": "PUT", + "url": "Patient/pagination-patient2" + } + }, + { + "resource": { + "resourceType": "Patient", + "id": "pagination-patient3" + }, + "request": { + "method": "PUT", + "url": "Patient/pagination-patient3" + } + }, + { + "resource": { + "resourceType": "Practitioner", + "id": "pagination-practitioner1" + }, + "request": { + "method": "PUT", + "url": "Practitioner/pagination-practitioner1" + } + } + ] +} \ No newline at end of file diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/SqlQueryGenerator.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/SqlQueryGenerator.cs index 70c2baad9..e229ae723 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/SqlQueryGenerator.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/SqlQueryGenerator.cs @@ -735,6 +735,7 @@ namespace Microsoft.Health.Fhir.SqlServer.Features.Search.Expressions.Visitors.Q // We should remove IsHistory from ReferenceSearchParam (Source) only but keep on Resource (Target) AppendHistoryClause(delimited, context.ResourceVersionTypes, null, referenceTargetResourceTableAlias); + AppendDeletedClause(delimited, context.ResourceVersionTypes, referenceTargetResourceTableAlias); delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ResourceTypeId, referenceSourceTableAlias) .Append(" IN (") diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs index b075c08f1..aa078b616 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs @@ -146,7 +146,7 @@ namespace Microsoft.Health.Fhir.SqlServer.Features.Search SqlSearchOptions sqlSearchOptions = new SqlSearchOptions(searchOptions); SearchResult searchResult = await SearchImpl(sqlSearchOptions, cancellationToken); - int resultCount = searchResult.Results.Count(); + int resultCount = searchResult.Results.Count(r => r.SearchEntryMode == SearchEntryMode.Match); if (!sqlSearchOptions.IsSortWithFilter && searchResult.ContinuationToken == null && @@ -157,6 +157,10 @@ namespace Microsoft.Health.Fhir.SqlServer.Features.Search { // We seem to have run a sort which has returned less results than what max we can return. // Let's determine whether we need to execute another query or not. + // + // When order is descending, what would make DidWeSearchForSortValue false? + // Right now if no results are found that have the sort value and the order is descending this won't run. + // What is the point of DidweSearchForSortValue at all??? if ((sqlSearchOptions.Sort[0].sortOrder == SortOrder.Ascending && sqlSearchOptions.DidWeSearchForSortValue.HasValue && !sqlSearchOptions.DidWeSearchForSortValue.Value) || (sqlSearchOptions.Sort[0].sortOrder == SortOrder.Descending && sqlSearchOptions.DidWeSearchForSortValue.HasValue && sqlSearchOptions.DidWeSearchForSortValue.Value && !sqlSearchOptions.SortHasMissingModifier)) { diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/ChainingSearchTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/ChainingSearchTests.cs index 5783eff8d..160adb0eb 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/ChainingSearchTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/ChainingSearchTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.Metrics; using System.Linq; using System.Threading.Tasks; using Hl7.Fhir.Model; @@ -348,6 +349,15 @@ namespace Microsoft.Health.Fhir.Tests.E2E.Rest.Search } #endif + [Fact] + public async Task GivenCountOnlyReverseChainSearchWithDeletedResource_WhenSearched_ThenCorrectCountIsReturned() + { + string query = $"_has:CareTeam:patient:_tag={Fixture.Tag}&_summary=count"; + + Bundle bundle = await Client.SearchAsync(ResourceType.Patient, query); + Assert.Equal(1, bundle.Total); + } + public class ClassFixture : HttpIntegrationTestFixture { public ClassFixture(DataStore dataStore, Format format, TestFhirServerFactory testFhirServerFactory) @@ -431,6 +441,12 @@ namespace Microsoft.Health.Fhir.Tests.E2E.Rest.Search SmithLoincDiagnosticReport = await CreateDiagnosticReport(SmithPatient, smithLoincObservation, loincCode); TrumanLoincDiagnosticReport = await CreateDiagnosticReport(TrumanPatient, trumanLoincObservation, loincCode); + var deletedPatient = (await TestFhirClient.CreateAsync(new Patient { Meta = meta, Gender = AdministrativeGender.Male, Name = new List { new HumanName { Given = new[] { "Delete" }, Family = "Delete" } } })).Resource; + await TestFhirClient.CreateAsync(new CareTeam() { Meta = meta, Subject = new ResourceReference($"Patient/{AdamsPatient.Id}") }); + await TestFhirClient.CreateAsync(new CareTeam() { Meta = meta, Subject = new ResourceReference($"Patient/{deletedPatient.Id}") }); + + await TestFhirClient.DeleteAsync(deletedPatient); + var group = new Group { Meta = new Meta { Tag = new List { new Coding("testTag", Tag) } }, diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/IncludeSearchTestFixture.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/IncludeSearchTestFixture.cs index 82f9d3288..866bbef03 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/IncludeSearchTestFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/IncludeSearchTestFixture.cs @@ -172,9 +172,9 @@ namespace Microsoft.Health.Fhir.Shared.Tests.E2E.Rest.Search AdamsMedicationRequest = await CreateMedicationRequest(AdamsPatient, AndersonPractitioner, PercocetMedication); SmithMedicationRequest = await CreateMedicationRequest(SmithPatient, SanchezPractitioner, PercocetMedication); - AdamsMedicationDispense = await CreateMedicationDispense(AdamsMedicationRequest, AdamsPatient, TramadolMedication); - SmithMedicationDispense = await CreateMedicationDispense(SmithMedicationRequest, SmithPatient, TramadolMedication); - TrumanMedicationDispenseWithoutRequest = await CreateMedicationDispense(null, TrumanPatient, TramadolMedication); + AdamsMedicationDispense = await CreateMedicationDispense(AdamsMedicationRequest, AdamsPatient, TramadolMedication, "2000-01-01"); + SmithMedicationDispense = await CreateMedicationDispense(SmithMedicationRequest, SmithPatient, TramadolMedication, "1990-01-01"); + TrumanMedicationDispenseWithoutRequest = await CreateMedicationDispense(null, TrumanPatient, TramadolMedication, null); CareTeam = await CreateCareTeam(); @@ -250,7 +250,7 @@ namespace Microsoft.Health.Fhir.Shared.Tests.E2E.Rest.Search })).Resource; } - async Task CreateMedicationDispense(MedicationRequest medicationRequest, Patient patient, Medication medication) + async Task CreateMedicationDispense(MedicationRequest medicationRequest, Patient patient, Medication medication, string prepared) { return (await TestFhirClient.CreateAsync( new MedicationDispense @@ -270,6 +270,7 @@ namespace Microsoft.Health.Fhir.Shared.Tests.E2E.Rest.Search Actor = new ResourceReference($"Practitioner/{Practitioner.Id}"), }, }, + WhenPrepared = prepared, #if R5 Medication = new CodeableReference { diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/IncludeSearchTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/IncludeSearchTests.cs index f151a182a..af7e1bf92 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/IncludeSearchTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/IncludeSearchTests.cs @@ -1261,6 +1261,25 @@ namespace Microsoft.Health.Fhir.Tests.E2E.Rest.Search ValidateOperationOutcome(expectedDiagnostics, expectedIssueSeverities, expectedCodeTypes, fhirException.OperationOutcome); } + [Fact] + [HttpIntegrationFixtureArgumentSets(DataStore.SqlServer)] // Cosmos doesn't support the sort parameter + public async Task GivenAnIncludeSearchWithSortAndResourcesWithAndWithoutTheIncludeParameter_WhenSearched_ThenCorrectResultsAreReturned() + { + string query = $"_include=MedicationDispense:prescription&_sort=-whenprepared&_count=3&_tag={Fixture.Tag}"; + + Bundle bundle = await Client.SearchAsync(ResourceType.MedicationDispense, query); + + Assert.Equal("self", bundle.Link[0].Relation); + + ValidateBundle( + bundle, + Fixture.AdamsMedicationDispense, + Fixture.SmithMedicationDispense, + Fixture.TrumanMedicationDispenseWithoutRequest, + Fixture.AdamsMedicationRequest, + Fixture.SmithMedicationRequest); + } + // This will not work for circular reference private static void ValidateSearchEntryMode(Bundle bundle, ResourceType matchResourceType) { diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs index 0fbb99ebf..0ebb335c9 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs @@ -10,6 +10,7 @@ using System.Linq; using System.Net; using System.Threading.Tasks; using System.Web; +using DotLiquid; using Hl7.Fhir.Model; using Microsoft.Health.Fhir.Client; using Microsoft.Health.Fhir.Core.Extensions; @@ -1116,6 +1117,23 @@ namespace Microsoft.Health.Fhir.Tests.E2E.Rest.Search SortTestsAssert.AssertOrganizationNamesAreEqualInRange(expectedOrganizations.Length, expectedOrganizations, returnedResults); } + /* + * Needs investigation, this breaks GivenPatients_WhenSearchedWithSortParamAndMissingIdentifier_SearchResultsReturnedShouldHonorMissingIdentifier + [Theory] + [InlineData("address-postalcode")] + [InlineData("-address-postalcode")] + [HttpIntegrationFixtureArgumentSets(DataStore.SqlServer)] + public async Task GivenNoResourcesWithSortValue_WhenSearchedWithSortParameter_ThenResourcesAreReturned(string sort) + { + var tag = Guid.NewGuid().ToString(); + var patients = await CreatePatients(tag); + + var returnedResults = await GetResultsFromAllPagesAsync($"Patient?_tag={tag}&_sort={sort}"); + + SortTestsAssert.AssertNumberOfResources(patients, returnedResults); + } + */ + private async Task CreatePatients(string tag) { // Create various resources.