Fix FHIR Search query with _count=0 and SqlCustomQueryTest issue (#3491)

* Fix FHIR Serach query with params _count=0 (#3230) * fixes code review comments * Fix for CustomQueryTests #105699 * letter summary fixes * Adds Testcase for Invalid _count and updates existing testcase to check for totalcount * worked on review comments
microsoft · Nov 9, 2023 · 5b8e2e4 · 5b8e2e4
1 parent e5dff84
commit 5b8e2e4
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 10 deletions.
diff --git a/...rosoft.Health.Fhir.Shared.Api.UnitTests/Features/Formatters/HttpContextExtensionsTests.cs b/...rosoft.Health.Fhir.Shared.Api.UnitTests/Features/Formatters/HttpContextExtensionsTests.cs
@@ -21,7 +21,7 @@ public HttpContextExtensionsTests()
         }
 
         [Fact]
-        public void GivenARequestWithSummaryType_WhenSerializingTheResponse_ThenTheCorrectSummeryTypeIsApplied()
+        public void GivenARequestWithSummaryType_WhenSerializingTheResponse_ThenTheCorrectSummaryTypeIsApplied()
         {
             var context = new DefaultHttpContext();
             context.Request.QueryString = QueryString.Create("_summary", "text");
@@ -32,7 +32,7 @@ public void GivenARequestWithSummaryType_WhenSerializingTheResponse_ThenTheCorre
         }
 
         [Fact]
-        public void GivenARequestWithCapsSummaryType_WhenSerializingTheResponse_ThenTheCorrectSummeryTypeIsApplied()
+        public void GivenARequestWithCapsSummaryType_WhenSerializingTheResponse_ThenTheCorrectSummaryTypeIsApplied()
         {
             var context = new DefaultHttpContext();
             context.Request.QueryString = QueryString.Create("_SUMMARY", "DATA");
@@ -42,6 +42,17 @@ public void GivenARequestWithCapsSummaryType_WhenSerializingTheResponse_ThenTheC
             Assert.Equal(SummaryType.Data, summary);
         }
 
+        [Fact]
+        public void GivenARequestWithCountType_WhenSerializingTheResponse_ThenTheCorrectSummaryTypeIsApplied()
+        {
+            var context = new DefaultHttpContext();
+            context.Request.QueryString = QueryString.Create("_count", "0");
+
+            var summary = context.GetSummaryTypeOrDefault();
+
+            Assert.Equal(SummaryType.Count, summary);
+        }
+
         [Fact]
         public void GivenARequestWithUnknownSummaryType_WhenSerializingTheResponse_DefaultSummaryReturned()
         {

diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Features/Formatters/HttpContextExtensions.cs b/src/Microsoft.Health.Fhir.Shared.Api/Features/Formatters/HttpContextExtensions.cs
@@ -25,6 +25,16 @@ public static SummaryType GetSummaryTypeOrDefault(this HttpContext context)
             {
                 return summary;
             }
+            else if (string.IsNullOrWhiteSpace(query))
+            {
+                var result = context.Request.Query[KnownQueryParameterNames.Count].FirstOrDefault();
+
+                if (!string.IsNullOrWhiteSpace(result) && int.TryParse(result, out var count) && count == 0 &&
+                    (context.Response.StatusCode == (int)HttpStatusCode.OK || context.Response.StatusCode == (int)HttpStatusCode.Created))
+                {
+                    return Hl7.Fhir.Rest.SummaryType.Count;
+                }
+            }
 
             return SummaryType.False;
         }

diff --git a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchOptionsFactoryTests.cs b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchOptionsFactoryTests.cs
@@ -51,7 +51,7 @@ public partial class SearchOptionsFactoryTests
         public SearchOptionsFactoryTests()
         {
             var searchParameterDefinitionManager = Substitute.For<ISearchParameterDefinitionManager>();
-            _resourceTypeSearchParameterInfo = new SearchParameter { Name = SearchParameterNames.ResourceType, Code = SearchParameterNames.ResourceType, Type = SearchParamType.String }.ToInfo();
+            _resourceTypeSearchParameterInfo = new SearchParameter { Name = SearchParameterNames.ResourceType, Code = SearchParameterNames.ResourceType, Type = SearchParamType.String, Url = SearchParameterNames.ResourceTypeUri.AbsoluteUri }.ToInfo();
             _lastUpdatedSearchParameterInfo = new SearchParameter { Name = SearchParameterNames.LastUpdated, Code = SearchParameterNames.LastUpdated, Type = SearchParamType.String }.ToInfo();
             searchParameterDefinitionManager.GetSearchParameter(Arg.Any<string>(), Arg.Any<string>()).Throws(ci => new SearchParameterNotSupportedException(ci.ArgAt<string>(0), ci.ArgAt<string>(1)));
             searchParameterDefinitionManager.GetSearchParameter(Arg.Any<string>(), SearchParameterNames.ResourceType).Returns(_resourceTypeSearchParameterInfo);
@@ -110,6 +110,39 @@ public void GivenACount_WhenCreated_ThenCorrectMaxItemCountShouldBeSet()
             Assert.Equal(5, options.MaxItemCount);
         }
 
+        [Fact]
+        public void GivenACountWithValueZero_WhenCreated_ThenCorrectMaxItemCountShouldBeSet()
+        {
+            const ResourceType resourceType = ResourceType.Encounter;
+            var queryParameters = new[]
+            {
+               Tuple.Create("_count", "0"),
+            };
+
+            SearchOptions options = CreateSearchOptions(
+            resourceType: resourceType.ToString(),
+            queryParameters: queryParameters);
+
+            Assert.NotNull(options);
+            Assert.True(options.CountOnly);
+        }
+
+        [Theory]
+        [InlineData("a")]
+        [InlineData("1.1")]
+        public void GivenACountWithInvalidValue_WhenCreated_ThenExceptionShouldBeThrown(string value)
+        {
+            const ResourceType resourceType = ResourceType.Encounter;
+            var queryParameters = new[]
+            {
+               Tuple.Create("_count", value),
+            };
+
+            Assert.Throws<System.FormatException>(() => CreateSearchOptions(
+            resourceType: resourceType.ToString(),
+            queryParameters: queryParameters));
+        }
+
         [Fact]
         public void GivenNoneOfTheSearchParamIsSupported_WhenCreated_ThenCorrectExpressionShouldBeGenerated()
         {

diff --git a/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs b/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs
@@ -184,6 +184,17 @@ public SearchOptions Create(
                         throw new BadRequestException(string.Format(Core.Resources.InvalidTotalParameter, query.Item2, SupportedTotalTypes));
                     }
                 }
+                else if (query.Item1 == KnownQueryParameterNames.Count && Convert.ToInt32(query.Item2) == 0)
+                {
+                    try
+                    {
+                        searchParams.Add(KnownQueryParameterNames.Summary, SummaryType.Count.ToString());
+                    }
+                    catch (Exception ex)
+                    {
+                        throw new BadRequestException(ex.Message);
+                    }
+                }
                 else
                 {
                     // Parse the search parameters.

diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlCustomQueryTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlCustomQueryTests.cs
@@ -49,16 +49,13 @@ public async Task GivenASqlQuery_IfAStoredProcExistsWithMatchingHash_ThenStoredP
             // set the wait time to 1 second
             CustomQueries.WaitTime = 1;
 
-            // Create a query with a known hash
             var queryParameters = new[]
             {
-                Tuple.Create("participating-organization.identifier", $"TAB%7C910000290"),
-                Tuple.Create(SearchParameterNames.Include, $"OrganizationAffiliation:location"),
-                Tuple.Create("participating-organization.type", $"practice"),
+               Tuple.Create("_count", "5"),
             };
 
             // Query before adding an sproc to the database
-            await _fixture.SearchService.SearchAsync(KnownResourceTypes.OrganizationAffiliation, queryParameters, CancellationToken.None);
+            await _fixture.SearchService.SearchAsync(KnownResourceTypes.Patient, queryParameters, CancellationToken.None);
 
             var hash = _fixture.SqlQueryHashCalculator.MostRecentSqlHash;
 
@@ -75,7 +72,7 @@ public async Task GivenASqlQuery_IfAStoredProcExistsWithMatchingHash_ThenStoredP
             while (sw.Elapsed.TotalSeconds < 10) // previous single try after 1.1 sec delay was not reliable.
             {
                 await Task.Delay(300);
-                await _fixture.SearchService.SearchAsync(KnownResourceTypes.OrganizationAffiliation, queryParameters, CancellationToken.None);
+                await _fixture.SearchService.SearchAsync(KnownResourceTypes.Patient, queryParameters, CancellationToken.None);
                 Assert.Equal(hash, _fixture.SqlQueryHashCalculator.MostRecentSqlHash);
                 if (await CheckIfSprocUsed(hash))
                 {
@@ -120,7 +117,7 @@ private void AddSproc(string hash)
         {
             _fixture.SqlHelper.ExecuteSqlCmd(
                 "CREATE OR ALTER PROCEDURE [dbo].[CustomQuery_" + hash + "]\r\n" +
-                "(@p0 SmallInt, @p1 SmallInt, @p2 SmallInt, @p3 SmallInt, @p4 NVarChar(256), @p5 SmallInt, @p6 VarChar(256), @p7 Int, @p8 SmallInt, @p9 Int)\r\n" +
+                "(@p0 SmallInt=1, @p1 SmallInt=1, @p2 SmallInt=1, @p3 SmallInt=1, @p4 NVarChar(256)='code', @p5 SmallInt=1, @p6 VarChar(256)='code', @p7 Int=1, @p8 SmallInt=1, @p9 Int=1)\r\n" +
                 "AS\r\n" +
                 "BEGIN\r\n" +
                 "WITH cte0 AS\r\n(\r\n" +