Cl/facet fixes (#46)

* refactor the config for application and infrastructure tiers * Tidied-up and improved comments. * Fixed broken tests. * A bit of a re-jig of the core project structure. * remove project entry for nuget generation * Correction for mapper registration in DI container * Removed package generation --------- Co-authored-by: Spencer O'HEGARTY <[email protected]>
DFE-Digital · Sep 12, 2024 · cd946a1 · cd946a1
1 parent 267915e
commit cd946a1
Show file tree

Hide file tree

Showing 65 changed files with 665 additions and 792 deletions.
diff --git a/Dfe.Data.SearchPrototype/Common/Dfe.Data.SearchPrototype.Common.csproj b/Dfe.Data.SearchPrototype/Common/Dfe.Data.SearchPrototype.Common.csproj
@@ -4,8 +4,6 @@
 		<TargetFramework>net8.0</TargetFramework>
 		<ImplicitUsings>enable</ImplicitUsings>
 		<Nullable>enable</Nullable>
-
-
 		<!-- Package information, for nuget -->
 		<PackageId>Dfe.Data.SearchPrototype.Common</PackageId>
 		<!--<Version>1.0.1</Version>--><!-- This is set within the build pipeline -->

diff --git a/Dfe.Data.SearchPrototype/Dfe.Data.SearchPrototype.csproj b/Dfe.Data.SearchPrototype/Dfe.Data.SearchPrototype.csproj
@@ -4,7 +4,6 @@
 		<TargetFramework>net8.0</TargetFramework>
 		<ImplicitUsings>enable</ImplicitUsings>
 		<Nullable>enable</Nullable>
-
 		<!-- Package information, for nuget -->
 		<PackageId>Dfe.Data.SearchPrototype</PackageId>
 		<!--<Version>1.0.1</Version>--><!-- This is set within the build pipeline -->
@@ -22,7 +21,7 @@
 	</PropertyGroup>
 
 	<ItemGroup>
-		<None Include="../README.md" Pack="true" PackagePath="\"/>
+		<None Include="../README.md" Pack="true" PackagePath="\" />
 	</ItemGroup>
 
 
@@ -50,6 +49,7 @@
 
 	<ItemGroup>
 	  <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />
+	  <PackageReference Include="Microsoft.Extensions.Options" Version="8.0.2" />
 	</ItemGroup>
 
 	<ItemGroup>

diff --git a/Dfe.Data.SearchPrototype/Infrastructure/CognitiveSearchServiceAdapter.cs b/Dfe.Data.SearchPrototype/Infrastructure/CognitiveSearchServiceAdapter.cs
@@ -1,11 +1,12 @@
 using Azure;
 using Azure.Search.Documents;
+using Azure.Search.Documents.Models;
 using Dfe.Data.Common.Infrastructure.CognitiveSearch.SearchByKeyword;
 using Dfe.Data.SearchPrototype.Common.Mappers;
 using Dfe.Data.SearchPrototype.Infrastructure.Options;
-using Dfe.Data.SearchPrototype.SearchForEstablishments;
+using Dfe.Data.SearchPrototype.SearchForEstablishments.ByKeyword.ServiceAdapters;
 using Dfe.Data.SearchPrototype.SearchForEstablishments.Models;
-
+using Microsoft.Extensions.Options;
 using AzureModels = Azure.Search.Documents.Models;
 
 namespace Dfe.Data.SearchPrototype.Infrastructure;
@@ -17,9 +18,9 @@ namespace Dfe.Data.SearchPrototype.Infrastructure;
 public sealed class CognitiveSearchServiceAdapter<TSearchResult> : ISearchServiceAdapter where TSearchResult : class
 {
     private readonly ISearchByKeywordService _searchByKeywordService;
-    private readonly ISearchOptionsFactory _searchOptionsFactory;
     private readonly IMapper<Pageable<AzureModels.SearchResult<TSearchResult>>, EstablishmentResults> _searchResultMapper;
-    private readonly IMapper<Dictionary<string, IList<Azure.Search.Documents.Models.FacetResult>>, EstablishmentFacets> _facetsMapper;
+    private readonly IMapper<Dictionary<string, IList<AzureModels.FacetResult>>, EstablishmentFacets> _facetsMapper;
+    private readonly AzureSearchOptions _azureSearchOptions;
 
     /// <summary>
     /// The following dependencies include the core cognitive search service definition,
@@ -28,37 +29,37 @@ public sealed class CognitiveSearchServiceAdapter<TSearchResult> : ISearchServic
     /// <param name="searchByKeywordService">
     /// Cognitive search (search by keyword) service definition injected via IOC container.
     /// </param>
-    /// <param name="searchOptionsFactory">
-    /// Factory class definition for prescribing the requested search options (by collection context).
+    /// <param name="azureSearchOptions">
+    /// the search options provided through the app-settings
     /// </param>
     /// <param name="searchResultMapper">
     /// Maps the raw Azure search response to the required <see cref="EstablishmentResults"/>
     /// </param>
     /// <param name="facetsMapper">
-    /// Maps the the raw Azure search response to the required <see cref="EstablishmentFacets"/>
+    /// Maps the raw Azure search response to the required <see cref="EstablishmentFacets"/>
     /// </param>
     public CognitiveSearchServiceAdapter(
         ISearchByKeywordService searchByKeywordService,
-        ISearchOptionsFactory searchOptionsFactory,
+        IOptions<AzureSearchOptions> azureSearchOptions,
         IMapper<Pageable<AzureModels.SearchResult<TSearchResult>>, EstablishmentResults> searchResultMapper,
         IMapper<Dictionary<string, IList<AzureModels.FacetResult>>, EstablishmentFacets> facetsMapper)
     {
-        _searchOptionsFactory = searchOptionsFactory;
+        ArgumentNullException.ThrowIfNull(azureSearchOptions.Value);
+        _azureSearchOptions = azureSearchOptions.Value;
         _searchByKeywordService = searchByKeywordService;
         _searchResultMapper = searchResultMapper;
         _facetsMapper = facetsMapper;
     }
 
     /// <summary>
     /// Makes call to underlying azure cognitive search service and uses the prescribed mapper
-    /// to adapt the raw Azure search results to the "T:Dfe.Data.SearchPrototype.Search.Domain.AgregateRoot.Establishments" type.
+    /// to adapt the raw Azure search results to the <see cref="SearchResults"/> type.
     /// </summary>
-    /// <param name="searchContext">
+    /// <param name="searchServiceAdapterRequest">
     /// Prescribes the context of the search including the keyword and collection target.
     /// </param>
     /// <returns>
-    /// A configured "T:Dfe.Data.SearchPrototype.Search.Domain.AgregateRoot.Establishments"
-    /// object hydrated from the results of the azure search.
+    /// A configured <<see cref="SearchResults"/> object hydrated from the results of the azure search.
     /// </returns>
     /// <exception cref="ApplicationException">
     /// An application exception is thrown if we either have no options configured, which
@@ -68,29 +69,36 @@ public CognitiveSearchServiceAdapter(
     /// <exception cref="ArgumentException">
     /// Exception thrown if the data cannot be mapped
     /// </exception>
-
-    public async Task<SearchResults> SearchAsync(SearchContext searchContext)
+    public async Task<SearchResults> SearchAsync(SearchServiceAdapterRequest searchServiceAdapterRequest)
     {
-        SearchOptions searchOptions =
-            _searchOptionsFactory.GetSearchOptions(searchContext.TargetCollection) ??
-            throw new ApplicationException(
-                $"Search options cannot be derived for {searchContext.TargetCollection}.");
+        SearchOptions searchOptions = new()
+        {
+            SearchMode = (SearchMode)_azureSearchOptions.SearchMode,
+            Size = _azureSearchOptions.Size,
+            IncludeTotalCount = _azureSearchOptions.IncludeTotalCount,
+        };
+
+        searchServiceAdapterRequest.SearchFields?.ToList()
+            .ForEach(searchOptions.SearchFields.Add);
+
+        searchServiceAdapterRequest.Facets?.ToList()
+            .ForEach(searchOptions.Facets.Add);
 
-        Response<AzureModels.SearchResults<TSearchResult>> searchResults =
+        Response<SearchResults<TSearchResult>> searchResults =
             await _searchByKeywordService.SearchAsync<TSearchResult>(
-                searchContext.SearchKeyword,
-                searchContext.TargetCollection,
+                searchServiceAdapterRequest.SearchKeyword,
+                _azureSearchOptions.SearchIndex,
                 searchOptions
             )
             .ConfigureAwait(false) ??
                 throw new ApplicationException(
-                    $"Unable to derive search results based on input {searchContext.SearchKeyword}.");
+                    $"Unable to derive search results based on input {searchServiceAdapterRequest.SearchKeyword}.");
 
         var results = new SearchResults()
         {
             Establishments = _searchResultMapper.MapFrom(searchResults.Value.GetResults()),
             Facets = searchResults.Value.Facets != null
-                ? _facetsMapper.MapFrom(searchResults.Value.Facets.ToDictionary<string, IList<AzureModels.FacetResult>>())
+                ? _facetsMapper.MapFrom(searchResults.Value.Facets.ToDictionary())
                 : null
         };
 

diff --git a/Dfe.Data.SearchPrototype/Infrastructure/CompositionRoot.cs b/Dfe.Data.SearchPrototype/Infrastructure/CompositionRoot.cs
@@ -0,0 +1,60 @@
+using Azure;
+using Azure.Search.Documents.Models;
+using Dfe.Data.SearchPrototype.Common.Mappers;
+using Dfe.Data.SearchPrototype.Infrastructure.Mappers;
+using Dfe.Data.SearchPrototype.Infrastructure.Options;
+using Dfe.Data.SearchPrototype.SearchForEstablishments.ByKeyword.ServiceAdapters;
+using Dfe.Data.SearchPrototype.SearchForEstablishments.Models;
+using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace Dfe.Data.SearchPrototype.Infrastructure;
+
+/// <summary>
+/// The composition root provides a unified location in the application where the composition
+/// of the object graphs for the application take place, using the IOC container.
+/// </summary>
+public static class CompositionRoot
+{
+    /// <summary>
+    /// Extension method which provides all the pre-registrations required to
+    /// access the AI azure search service adapter, and perform searches across provisioned indexes.
+    /// </summary>
+    /// <param name="services">
+    /// The originating application services onto which to register the search dependencies.
+    /// </param>
+    /// <param name="configuration">
+    /// The originating configuration block from which to derive search service settings.
+    /// </param>
+    /// <exception cref="ArgumentNullException">
+    /// The exception thrown if no valid <see cref="IServiceCollection"/> is provisioned.
+    /// </exception>
+    public static void AddCognitiveSearchAdaptorServices(this IServiceCollection services, IConfiguration configuration)
+    {
+        if (services is null)
+        {
+            throw new ArgumentNullException(nameof(services),
+                "A service collection is required to configure the azure AI search adapter dependencies.");
+        }
+
+        services.AddOptions<AzureSearchOptions>()
+           .Configure<IConfiguration>(
+               (settings, configuration) =>
+                   configuration
+                       .GetSection(nameof(AzureSearchOptions))
+                       .Bind(settings));
+
+        services.AddOptions<SearchByKeywordCriteria>()
+           .Configure<IConfiguration>(
+               (settings, configuration) =>
+                   configuration
+                       .GetSection(nameof(SearchByKeywordCriteria))
+                       .Bind(settings));
+
+        services.AddScoped(typeof(ISearchServiceAdapter), typeof(CognitiveSearchServiceAdapter<DataTransferObjects.Establishment>));
+        services.AddSingleton(typeof(IMapper<Pageable<SearchResult<DataTransferObjects.Establishment>>, EstablishmentResults>), typeof(PageableSearchResultsToEstablishmentResultsMapper));
+        services.AddSingleton<IMapper<Dictionary<string, IList<Azure.Search.Documents.Models.FacetResult>>, EstablishmentFacets>, AzureFacetResultToEstablishmentFacetsMapper>();
+        services.AddSingleton<IMapper<DataTransferObjects.Establishment, Address>, AzureSearchResultToAddressMapper>();
+        services.AddSingleton<IMapper<DataTransferObjects.Establishment, Establishment>, AzureSearchResultToEstablishmentMapper>();
+    }
+}
diff --git a/...Prototype/Infrastructure/Establishment.cs → ...ture/DataTransferObjects/Establishment.cs b/...Prototype/Infrastructure/Establishment.cs → ...ture/DataTransferObjects/Establishment.cs
@@ -1,46 +1,55 @@
-namespace Dfe.Data.SearchPrototype.Infrastructure;
+namespace Dfe.Data.SearchPrototype.Infrastructure.DataTransferObjects;
 
 /// <summary>
-/// Object used to encapsulate the core response from an Azure cognitive search result.
+/// Data transformation object used to encapsulate the core response from an Azure cognitive search result.
 /// </summary>
 public class Establishment
 {
     /// <summary>
     /// The unique identifier of the retrieved establishment result.
     /// </summary>
     public string? id { get; set; }
+
     /// <summary>
     /// The name associated with the retrieved establishment result.
     /// </summary>
     public string? ESTABLISHMENTNAME { get; set; }
+
     /// <summary>
     /// First line of the address
     /// </summary>
     public string? STREET { get; set; }
+
     /// <summary>
     /// Second line of the address of the retrieved establishment result
     /// </summary>
     public string? LOCALITY { get; set; }
+
     /// <summary>
     /// Third line of the address of the retrieved establishment result
     /// </summary>
     public string? ADDRESS3 { get; set; }
+
     /// <summary>
     /// Fourth line of the address of the retrieved establishment result
     /// </summary>
     public string? TOWN { get; set; }
+
     /// <summary>
     /// Postcode of the retrieved establishment result
     /// </summary>
     public string? POSTCODE { get; set; }
+
     /// <summary>
     /// The type of the establishment of the retrieved establishment result
     /// </summary>
     public string? TYPEOFESTABLISHMENTNAME { get; set; }
+
     /// <summary>
     /// The education phase of the retrieved establishment result
     /// </summary>
-    public string? PHASEOFEDUCATION {  get; set; }
+    public string? PHASEOFEDUCATION { get; set; }
+
     /// <summary>
     /// Status of retrieved establishment result.
     /// </summary>

diff --git a/Dfe.Data.SearchPrototype/Infrastructure/DependencyRegistration.cs b/Dfe.Data.SearchPrototype/Infrastructure/DependencyRegistration.cs
diff --git a/Dfe.Data.SearchPrototype/Infrastructure/Dfe.Data.SearchPrototype.Infrastructure.csproj b/Dfe.Data.SearchPrototype/Infrastructure/Dfe.Data.SearchPrototype.Infrastructure.csproj
@@ -4,7 +4,6 @@
 		<TargetFramework>net8.0</TargetFramework>
 		<ImplicitUsings>enable</ImplicitUsings>
 		<Nullable>enable</Nullable>
-
 		<!-- Package information, for nuget -->
 		<PackageId>Dfe.Data.SearchPrototype.Infrastructure</PackageId>
 		<!--<Version>1.0.1</Version>--><!-- This is set within the build pipeline -->
@@ -22,7 +21,7 @@
 	</PropertyGroup>
 
 	<ItemGroup>
-		<None Include="../../README.md" Pack="true" PackagePath="\"/>
+		<None Include="../../README.md" Pack="true" PackagePath="\" />
 	</ItemGroup>
 
 	<ItemGroup>

diff --git a/...ata.SearchPrototype/Infrastructure/Mappers/AzureFacetResultToEstablishmentFacetsMapper.cs b/...ata.SearchPrototype/Infrastructure/Mappers/AzureFacetResultToEstablishmentFacetsMapper.cs
@@ -6,24 +6,26 @@ namespace Dfe.Data.SearchPrototype.Infrastructure.Mappers;
 using AzureFacetResult = Azure.Search.Documents.Models.FacetResult;
 
 /// <summary>
-/// Maps from an Azure facet result to a collection of
-/// T:Dfe.Data.SearchPrototype.SearchForEstablishments.Models.EstablishmentFacet
+/// Maps from an Azure facet result to a collection of <see cref="EstablishmentFacet"/> types.
 /// </summary>
 public class AzureFacetResultToEstablishmentFacetsMapper : IMapper<Dictionary<string, IList<AzureFacetResult>>, EstablishmentFacets>
 {
     /// <summary>
-    /// Map from an Azure facet result to a collection of
-    /// T:Dfe.Data.SearchPrototype.SearchForEstablishments.Models.EstablishmentFacet
+    /// Map from an Azure facet result to a collection of <see cref="EstablishmentFacet"/> types.
     /// </summary>
     /// <param name="facetResult">The Azure facet result</param>
-    /// <returns></returns>
+    /// <returns>
+    /// A configured <see cref="EstablishmentFacets"/> instance.
+    /// </returns>
     public EstablishmentFacets MapFrom(Dictionary<string, IList<AzureFacetResult>> facetResult)
     {
         var establishmentFacets = new List<EstablishmentFacet>();
 
         foreach (var facetCategory in facetResult.Where(facet => facet.Value != null))
         {
-            var values = facetCategory.Value.Select(f => new FacetResult((string)f.Value, f.Count)).ToList();
+            var values = facetCategory.Value.Select(facetResult =>
+                new FacetResult((string)facetResult.Value, facetResult.Count)).ToList();
+
             var establishmentFacet = new EstablishmentFacet(facetCategory.Key, values);
 
             establishmentFacets.Add(establishmentFacet);

diff --git a/Dfe.Data.SearchPrototype/Infrastructure/Mappers/AzureSearchResultToAddressMapper.cs b/Dfe.Data.SearchPrototype/Infrastructure/Mappers/AzureSearchResultToAddressMapper.cs
@@ -4,23 +4,23 @@
 namespace Dfe.Data.SearchPrototype.Infrastructure.Mappers;
 
 /// <summary>
-/// Facilitates mapping from the received T:Dfe.Data.SearchPrototype.Infrastructure.Establishment
-/// into the required T:Dfe.Data.SearchPrototype.SearchForEstablishments.Address object.
+/// Facilitates mapping from the received <see cref="Establishment"/> instance
+/// into the required <see cref="Address"/> object.
 /// </summary>
-public class AzureSearchResultToAddressMapper : IMapper<Infrastructure.Establishment, Address>
+public class AzureSearchResultToAddressMapper : IMapper<DataTransferObjects.Establishment, Address>
 {
     /// <summary>
     /// The following mapping dependency provides the functionality to map from a raw Azure
-    /// search result, to a configured T:Dfe.Data.SearchPrototype.SearchForEstablishments.Address
-    /// instance, the complete implementation of which is defined in the IOC container.
+    /// search result, to a configured <see cref="Address"/> instance, the complete
+    /// implementation of which is defined in the IOC container.
     /// </summary>
     /// <param name="input">
-    /// The raw T:Dfe.Data.SearchPrototype.Infrastructure.Establishment used to map from.
+    /// The raw <see cref="Establishment"/> instance used to map from.
     /// </param>
     /// <returns>
-    /// The configured T:Dfe.Data.SearchPrototype.SearchForEstablishments.Address instance expected.
+    /// The configured <see cref="Address"/> instance expected.
     /// </returns>
-    public Address MapFrom(Establishment input)
+    public Address MapFrom(DataTransferObjects.Establishment input)
     {
         return new()
         {