Aw/pagination (#58)

* Add total count and map to model

* Fixed test issues

* WIP - added offset, tests need adding to exercise different offset and limits

* WIP - test setup

* Some stragglers

* Corrected code comment issue

* Removed unused factory method in SearchServiceAdapterRequest

* Added offset test to search options builder

* Search Keyword Request Offset Tests

* Add search service adapter request tests

* Changes made due to PR feedback

---------

Co-authored-by: Spencer O'HEGARTY <Spencer.OHEGARTY@EDUCATION.GOV.UK>
Co-authored-by: CLAWLOR <catherine.lawlor@education.gov.uk>

Dfe.Data.SearchPrototype/Infrastructure/Builders/ISearchOptionsBuilder.cs

@@ -21,6 +21,19 @@ public interface ISearchOptionsBuilder
         /// </returns>
         ISearchOptionsBuilder WithSize(int? size);
 
+        /// <summary>
+        /// Sets the value used to define how many
+        /// records are skipped in the search response (if any),
+        /// by default we have an offset of zero and so choose not to skip any records.
+        /// </summary>
+        /// <param name="offset">
+        /// The number of initial search results to skip, none (zero) by default .
+        /// </param>
+        /// <returns>
+        /// The updated builder instance.
+        /// </returns>
+        ISearchOptionsBuilder WithOffset(int offset = 0);
+
         /// <summary>
         /// Sets the mode of search to invoke, i.e. All or Any.
         /// </summary>

Dfe.Data.SearchPrototype/Infrastructure/Builders/SearchOptionsBuilder.cs

@@ -16,6 +16,7 @@ public sealed class SearchOptionsBuilder : ISearchOptionsBuilder
 
         private SearchMode? _searchMode;
         private int? _size;
+        private int _offset;
         private bool? _includeTotalCount;
         private IList<string>? _searchFields;
         private IList<string>? _facets;
@@ -53,6 +54,23 @@ public ISearchOptionsBuilder WithSize(int? size)
             return this;
         }
 
+        /// <summary>
+        /// Sets the value used to define how many
+        /// records are skipped in the search response (if any),
+        /// by default we have an offset of zero and so choose not to skip any records.
+        /// </summary>
+        /// <param name="offset">
+        /// The number of initial search results to skip, none (zero) by default .
+        /// </param>
+        /// <returns>
+        /// The updated builder instance.
+        /// </returns>
+        public ISearchOptionsBuilder WithOffset(int offset = 0)
+        {
+            _offset = offset;
+            return this;
+        }
+
         /// <summary>
         /// Sets the mode of search to invoke, i.e. All or Any.
         /// </summary>
@@ -139,6 +157,7 @@ public SearchOptions Build()
         {
             _searchOptions.SearchMode = _searchMode;
             _searchOptions.Size = _size;
+            _searchOptions.Skip = _offset;
             _searchOptions.IncludeTotalCount = _includeTotalCount;
             _searchFields?.ToList().ForEach(_searchOptions.SearchFields.Add);
             _facets?.ToList().ForEach(_searchOptions.Facets.Add);

Dfe.Data.SearchPrototype/Infrastructure/CognitiveSearchServiceAdapter.cs

@@ -82,6 +82,7 @@ public async Task<SearchResults> SearchAsync(SearchServiceAdapterRequest searchS
             _searchOptionsBuilder
                 .WithSearchMode((SearchMode)_azureSearchOptions.SearchMode)
                 .WithSize(_azureSearchOptions.Size)
+                .WithOffset(searchServiceAdapterRequest.Offset)
                 .WithIncludeTotalCount(_azureSearchOptions.IncludeTotalCount)
                 .WithSearchFields(searchServiceAdapterRequest.SearchFields)
                 .WithFacets(searchServiceAdapterRequest.Facets)
@@ -100,10 +101,12 @@ await _searchByKeywordService.SearchAsync<TSearchResult>(
 
         var results = new SearchResults()
         {
-            Establishments = _searchResultMapper.MapFrom(searchResults.Value.GetResults()),
+            Establishments = 
+                _searchResultMapper.MapFrom(searchResults.Value.GetResults()),
             Facets = searchResults.Value.Facets != null
                 ? _facetsMapper.MapFrom(searchResults.Value.Facets.ToDictionary())
-                : null
+                : null,
+            TotalNumberOfEstablishments = searchResults.Value.TotalCount
         };
 
         return results;

Dfe.Data.SearchPrototype/Infrastructure/Mappers/PageableSearchResultsToEstablishmentResultsMapper.cs

@@ -7,12 +7,12 @@
 namespace Dfe.Data.SearchPrototype.Infrastructure.Mappers;
 
 /// <summary>
-/// Facilitates mapping from the received <see cref="Models.SearchResults"/> 
-/// into the required <see cref="Models.EstablishmentResults"/>  object.
+/// Facilitates mapping from the received <see cref="Models.SearchResults"/>
+/// into the required <see cref="Models.EstablishmentResults"/> object.
 /// </summary>
-public sealed class PageableSearchResultsToEstablishmentResultsMapper : IMapper<Pageable<SearchResult<DataTransferObjects.Establishment>>, Models.EstablishmentResults>
+public sealed class PageableSearchResultsToEstablishmentResultsMapper : IMapper<Pageable<SearchResult<Establishment>>, Models.EstablishmentResults>
 {
-    private readonly IMapper<DataTransferObjects.Establishment, Models.Establishment> _azureSearchResultToEstablishmentMapper;
+    private readonly IMapper<Establishment, Models.Establishment> _azureSearchResultToEstablishmentMapper;
 
     /// <summary>
     /// The following mapping dependency provides the functionality to map from a raw Azure
@@ -22,7 +22,7 @@ public sealed class PageableSearchResultsToEstablishmentResultsMapper : IMapper<
     /// <param name="azureSearchResultToEstablishmentMapper">
     /// Mapper used to map from the raw Azure search result to a <see cref="Establishment"/> instance.
     /// </param>
-    public PageableSearchResultsToEstablishmentResultsMapper(IMapper<DataTransferObjects.Establishment, Models.Establishment> azureSearchResultToEstablishmentMapper)
+    public PageableSearchResultsToEstablishmentResultsMapper(IMapper<Establishment, Models.Establishment> azureSearchResultToEstablishmentMapper)
     {
         _azureSearchResultToEstablishmentMapper = azureSearchResultToEstablishmentMapper;
     }
@@ -44,23 +44,23 @@ public PageableSearchResultsToEstablishmentResultsMapper(IMapper<DataTransferObj
     /// <exception cref="ArgumentException">
     /// Exception thrown if the data cannot be mapped
     /// </exception>
-    public Models.EstablishmentResults MapFrom(Pageable<SearchResult<DataTransferObjects.Establishment>> input)
+    public Models.EstablishmentResults MapFrom(Pageable<SearchResult<Establishment>> input)
     {
         ArgumentNullException.ThrowIfNull(input);
+        Models.EstablishmentResults establishmentResults = new();
 
         if (input.Any())
         {
             var mappedResults = input.Select(result =>
                  result.Document != null
                     ? _azureSearchResultToEstablishmentMapper.MapFrom(result.Document)
                     : throw new InvalidOperationException(
-                        "Search result document object cannot be null.")
-                );
-            return new Models.EstablishmentResults(mappedResults);
-        }
-        else
-        {
-            return new Models.EstablishmentResults();
+                        "Search result document object cannot be null."));
+
+            establishmentResults =
+                new Models.EstablishmentResults(mappedResults);
         }
+
+        return establishmentResults;
     }
 }

Dfe.Data.SearchPrototype/Infrastructure/Tests/Builders/SearchOptionsBuilderTests.cs

@@ -29,6 +29,22 @@ public void Build_WithSize_SearchOptionsWithCorrectSize()
             searchOptions.Size.Should().Be(100);
         }
 
+        [Fact]
+        public void Build_WithOffset_SearchOptionsWithCorrectOffset()
+        {
+            // arrange
+            ISearchFilterExpressionsBuilder mockSearchFilterExpressionsBuilder = new FilterExpressionBuilderTestDouble().Create();
+
+            ISearchOptionsBuilder searchOptionsBuilder = new SearchOptionsBuilder(mockSearchFilterExpressionsBuilder);
+
+            // act
+            SearchOptions searchOptions = searchOptionsBuilder.WithOffset(offset: 39).Build();
+
+            // assert
+            searchOptions.Should().NotBeNull();
+            searchOptions.Skip.Should().Be(39);
+        }
+
         [Fact]
         public void Build_WithSearchMode_SearchOptionsWithCorrectSearchMode()
         {

Dfe.Data.SearchPrototype/Infrastructure/Tests/CognitiveSearchServiceAdapterAndMapperTests.cs

@@ -57,7 +57,8 @@ await cognitiveSearchServiceAdapter.SearchAsync(
                 new SearchServiceAdapterRequest(
                     searchKeyword: "SearchKeyword",
                     searchFields: ["FIELD1", "FIELD2", "FIELD2"],
-                    facets: ["FACET1", "FACET2", "FACET3"]));
+                    facets: ["FACET1", "FACET2", "FACET3"],
+                    offset: 99));
 
         // assert
         response.Should().NotBeNull();
@@ -94,7 +95,8 @@ await cognitiveSearchServiceAdapter.SearchAsync(
                 new SearchServiceAdapterRequest(
                     searchKeyword: "SearchKeyword",
                     searchFields: ["FIELD1", "FIELD2", "FIELD2"],
-                    facets: ["FACET1", "FACET2", "FACET3"]));
+                    facets: ["FACET1", "FACET2", "FACET3"],
+                    offset: 0));
 
         // assert
         response.Should().NotBeNull();
@@ -128,7 +130,8 @@ await cognitiveSearchServiceAdapter.SearchAsync(
                 new SearchServiceAdapterRequest(
                     searchKeyword: "SearchKeyword",
                     searchFields: ["FIELD1", "FIELD2", "FIELD2"],
-                    facets: ["FACET1", "FACET2", "FACET3"]));
+                    facets: ["FACET1", "FACET2", "FACET3"],
+                    offset: 0));
 
         // assert
         response.Should().NotBeNull();

Dfe.Data.SearchPrototype/Infrastructure/Tests/Mappers/PageableSearchResultsToEstablishmentResultsMapperTests.cs

@@ -1,7 +1,6 @@
 using Azure;
 using Azure.Search.Documents.Models;
 using Dfe.Data.SearchPrototype.Common.Mappers;
-using Dfe.Data.SearchPrototype.Infrastructure.DataTransferObjects;
 using Dfe.Data.SearchPrototype.Infrastructure.Mappers;
 using Dfe.Data.SearchPrototype.Infrastructure.Tests.TestDoubles;
 using Dfe.Data.SearchPrototype.Infrastructure.Tests.TestHelpers;
@@ -29,14 +28,16 @@ public void MapFrom_WithValidSearchResults_ReturnsConfiguredEstablishments()
         // arrange
         List<SearchResult<DataTransferObjects.Establishment>> searchResultDocuments =
             SearchResultFake.SearchResults();
+
         var pageableSearchResults = PageableTestDouble.FromResults(searchResultDocuments);
 
         // act
         EstablishmentResults? mappedResult = _searchResultsMapper.MapFrom(pageableSearchResults);
 
         // assert
         mappedResult.Should().NotBeNull();
-        mappedResult.Establishments.Should().HaveCount(searchResultDocuments.Count());
+        mappedResult.Establishments.Should().HaveCount(searchResultDocuments.Count);
+
         foreach (var searchResult in searchResultDocuments)
         {
             searchResult.ShouldHaveMatchingMappedEstablishment(mappedResult);
@@ -47,7 +48,9 @@ public void MapFrom_WithValidSearchResults_ReturnsConfiguredEstablishments()
     public void MapFrom_WithEmptySearchResults_ReturnsEmptyList()
     {
         // act
-        EstablishmentResults? result = _searchResultsMapper.MapFrom(PageableTestDouble.FromResults(SearchResultFake.EmptySearchResult()));
+        EstablishmentResults? result =
+            _searchResultsMapper.MapFrom(
+                PageableTestDouble.FromResults(SearchResultFake.EmptySearchResult()));
 
         // assert
         result.Should().NotBeNull();

Dfe.Data.SearchPrototype/Infrastructure/Tests/TestDoubles/SearchOptionsBuilderTestDouble.cs

@@ -13,7 +13,9 @@ public static ISearchOptionsBuilder MockFor(SearchOptions searchOptions)
             var mockSearchOptionsBuilder = new Mock<ISearchOptionsBuilder>();
 
             mockSearchOptionsBuilder.Setup(searchOptionsBuilder =>
-                searchOptionsBuilder.WithSize(It.IsAny<int?>())).Returns(mockSearchOptionsBuilder.Object);
+                searchOptionsBuilder.WithSize(It.IsAny<int>())).Returns(mockSearchOptionsBuilder.Object);
+            mockSearchOptionsBuilder.Setup(searchOptionsBuilder =>
+                searchOptionsBuilder.WithOffset(It.IsAny<int>())).Returns(mockSearchOptionsBuilder.Object);
             mockSearchOptionsBuilder.Setup(searchOptionsBuilder =>
                 searchOptionsBuilder.WithSearchMode(It.IsAny<SearchMode>())).Returns(mockSearchOptionsBuilder.Object);
             mockSearchOptionsBuilder.Setup(searchOptionsBuilder =>

Dfe.Data.SearchPrototype/SearchForEstablishments/ByKeyword/ServiceAdapters/SearchServiceAdapterRequest.cs

@@ -13,6 +13,12 @@ public sealed class SearchServiceAdapterRequest
     /// </summary>
     public string SearchKeyword { get; }
 
+    /// <summary>
+    /// The value used to define how many records are skipped in the search response (if any),
+    /// by default we have an offset of zero and so choose not to skip any records.
+    /// </summary>
+    public int Offset { get; } = 0;
+
     /// <summary>
     /// The collection of fields in the underlying collection to search over.
     /// </summary>
@@ -44,44 +50,36 @@ public sealed class SearchServiceAdapterRequest
     /// <param name="searchFilterRequests">
     /// Dictionary of search filter requests where key is the name of the filter and the value is the list of filter values.
     /// </param>
+    /// <param name="offset">
+    /// The value used to define how many records are skipped in the search response
+    /// (if any), by default we choose not to skip any records.
+    /// </param>
     /// <exception cref="ArgumentNullException">
     /// The exception thrown if an invalid search keyword (null or whitespace) is prescribed.
     /// </exception>
     /// <exception cref="ArgumentException">
     /// The exception type thrown if either a null or empty collection of search fields,
     /// or search facets are prescribed.
     /// </exception>
-    public SearchServiceAdapterRequest(string searchKeyword, IList<string> searchFields, IList<string> facets, IList<FilterRequest>? searchFilterRequests = null)
+    public SearchServiceAdapterRequest(
+        string searchKeyword,
+        IList<string> searchFields,
+        IList<string> facets,
+        IList<FilterRequest>? searchFilterRequests = null,
+        int offset = 0)
     {
         SearchKeyword =
             string.IsNullOrWhiteSpace(searchKeyword) ?
                 throw new ArgumentNullException(nameof(searchKeyword)) : searchKeyword;
 
         SearchFields = searchFields == null || searchFields.Count <= 0 ?
-            throw new ArgumentException("", nameof(searchFields)) : searchFields;
+            throw new ArgumentException($"A valid {nameof(searchFields)} argument must be provided.") : searchFields;
 
         Facets = facets == null || facets.Count <= 0 ?
-            throw new ArgumentException("", nameof(facets)) : facets;
+            throw new ArgumentException($"A valid {nameof(facets)} argument must be provided.") : facets;
 
         SearchFilterRequests = searchFilterRequests;
-    }
 
-    /// <summary>
-    /// Factory method to allow implicit creation of a T:Dfe.Data.SearchPrototype.Search.SearchContext instance.
-    /// </summary>
-    /// <param name="searchKeyword">
-    /// The keyword string which defines the search.
-    /// </param>
-    /// <param name="searchFields">
-    /// The collection of fields in the underlying collection to search over.
-    /// </param>
-    /// <param name="facets">
-    /// The collection of facets to apply in the search request.
-    /// </param>
-    /// <returns>
-    /// A configured <see cref="SearchServiceAdapterRequest"/> instance.
-    /// </returns>
-    public static SearchServiceAdapterRequest Create(
-        string searchKeyword, IList<string> searchFields, IList<string> facets) =>
-            new(searchKeyword, searchFields, facets);
+        Offset = offset;
+    }
 }

Dfe.Data.SearchPrototype/SearchForEstablishments/ByKeyword/Usecase/SearchByKeywordRequest.cs

@@ -14,11 +14,16 @@ public sealed class SearchByKeywordRequest
     /// <param name="searchKeyword">
     /// The string keyword used to search the collection specified.
     /// </param>
-    public SearchByKeywordRequest(string searchKeyword)
+    /// <param name="offset">
+    /// The value used to define how many records are skipped in the search
+    /// response (if any), by default we choose not to skip any records.
+    /// </param> 
+    public SearchByKeywordRequest(string searchKeyword, int offset = 0)
     {
         ArgumentException.ThrowIfNullOrEmpty(nameof(searchKeyword));
 
         SearchKeyword = searchKeyword;
+        Offset = offset;
     }
 
     /// <summary>
@@ -32,7 +37,11 @@ public SearchByKeywordRequest(string searchKeyword)
     /// <param name="filterRequests">
     /// The <see cref="FilterRequest"/> used to refine the search criteria.
     /// </param>
-    public SearchByKeywordRequest(string searchKeyword, IList<FilterRequest> filterRequests) : this(searchKeyword)
+    /// <param name="offset">
+    /// The value used to define how many records are skipped in the search response (if any).
+    /// </param>
+    public SearchByKeywordRequest(
+        string searchKeyword,IList<FilterRequest> filterRequests, int offset = 0) : this(searchKeyword, offset)
     {
         FilterRequests = filterRequests;
     }
@@ -42,6 +51,11 @@ public SearchByKeywordRequest(string searchKeyword, IList<FilterRequest> filterR
     /// </summary>
     public string SearchKeyword { get; }
 
+    /// <summary>
+    /// The value used to define how many records are skipped in the search response (if any).
+    /// </summary>
+    public int Offset { get; }
+
     /// <summary>
     /// The filter (key/values) used to refine the search criteria.
     /// </summary>

Dfe.Data.SearchPrototype/SearchForEstablishments/ByKeyword/Usecase/SearchByKeywordResponse.cs

@@ -47,7 +47,11 @@ public SearchByKeywordResponse(SearchResponseStatus status)
     /// <param name="status">
     /// The <see cref="SearchResponseStatus"/> of the result of the search.
     /// </param>
-    public SearchByKeywordResponse(EstablishmentResults establishments, EstablishmentFacets facetResults, SearchResponseStatus status)
+    public SearchByKeywordResponse(
+        EstablishmentResults establishments,
+        EstablishmentFacets facetResults,
+        SearchResponseStatus status
+        )
     {
         EstablishmentResults = establishments;
         EstablishmentFacetResults = facetResults;

Dfe.Data.SearchPrototype/SearchForEstablishments/ByKeyword/Usecase/SearchByKeywordUseCase.cs

@@ -78,7 +78,8 @@ await _searchServiceAdapter.SearchAsync(
                         request.SearchKeyword,
                         _searchByKeywordCriteria.SearchFields,
                         _searchByKeywordCriteria.Facets,
-                        request.FilterRequests));
+                        request.FilterRequests,
+                        request.Offset));
 
             return results switch
             {

Dfe.Data.SearchPrototype/SearchForEstablishments/Models/EstablishmentResults.cs

@@ -6,14 +6,14 @@
 /// </summary>
 public sealed class EstablishmentResults
 {
+    private readonly List<Establishment> _establishments;
+
     /// <summary>
     /// The readonly collection of <see cref="Establishment"/>
     /// types derived from the underlying search mechanism.
     /// </summary>
     public IReadOnlyCollection<Establishment> Establishments => _establishments.AsReadOnly();
 
-    private readonly List<Establishment> _establishments;
-
     /// <summary>
     ///  Default constructor initialises a new readonly
     ///  collection of <see cref="Establishment"/> instances.