Skip to content

Commit

Permalink
improve performance of trust search by batching sql commands
Browse files Browse the repository at this point in the history
  • Loading branch information
jordan-beard committed Feb 20, 2023
1 parent df27622 commit 034aecb
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 29 deletions.
8 changes: 4 additions & 4 deletions TramsDataApi.Test/Controllers/TrustsControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void SearchTrusts_ReturnsEmptySetOfTrustSummaries_WhenNoTrustsFound()
var companiesHouseNumber = "Mockcompanieshousenumber";

var searchTrusts = new Mock<ISearchTrusts>();
searchTrusts.Setup(s => s.Execute(1, 10, groupName, ukprn, companiesHouseNumber))
searchTrusts.Setup(s => s.Execute(1, 10, groupName, ukprn, companiesHouseNumber, true))
.Returns((new List<TrustSummaryResponse>(), 0));

var controller = new TrustsController(new Mock<IGetTrustByUkprn>().Object, searchTrusts.Object, mockLogger.Object);
Expand All @@ -99,7 +99,7 @@ public void SearchTrusts_ByGroupNameAndCompaniesHouseNumber_ReturnsListOfTrustSu
.Build();

var searchTrusts = new Mock<ISearchTrusts>();
searchTrusts.Setup(s => s.Execute(1, 10, groupName, null, companiesHouseNumber))
searchTrusts.Setup(s => s.Execute(1, 10, groupName, null, companiesHouseNumber, true))
.Returns((expectedTrustSummaries, expectedTrustSummaries.Count));

var controller = new TrustsController(new Mock<IGetTrustByUkprn>().Object, searchTrusts.Object, mockLogger.Object);
Expand All @@ -120,7 +120,7 @@ public void SearchTrusts_ByUrn_ReturnsListOfTrustSummaries_WhenTrustsAreFound()
.Build();

var searchTrusts = new Mock<ISearchTrusts>();
searchTrusts.Setup(s => s.Execute(1, 10, null, ukprn, null))
searchTrusts.Setup(s => s.Execute(1, 10, null, ukprn, null, true))
.Returns((expectedTrustSummaries, expectedTrustSummaries.Count));

var controller = new TrustsController(new Mock<IGetTrustByUkprn>().Object, searchTrusts.Object, mockLogger.Object);
Expand All @@ -135,7 +135,7 @@ public void SearchTrusts_WithNoParams_ReturnsAllTrusts()
var expectedTrustSummaries = Builder<TrustSummaryResponse>.CreateListOfSize(5).Build();

var searchTrusts = new Mock<ISearchTrusts>();
searchTrusts.Setup(s => s.Execute(1, 10, null, null, null))
searchTrusts.Setup(s => s.Execute(1, 10, null, null, null, true))
.Returns((expectedTrustSummaries, expectedTrustSummaries.Count));

var controller = new TrustsController(new Mock<IGetTrustByUkprn>().Object, searchTrusts.Object, mockLogger.Object);
Expand Down
10 changes: 5 additions & 5 deletions TramsDataApi.Test/Controllers/TrustsControllerV2Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void SearchTrusts_ReturnsEmptySetOfTrustSummaries_WhenNoTrustsFound()
NextPageUrl = null
};

_mockSearchTrustsUseCase.Setup(s => s.Execute(1, 10, groupName, ukprn, companiesHouseNumber))
_mockSearchTrustsUseCase.Setup(s => s.Execute(1, 10, groupName, ukprn, companiesHouseNumber, true))
.Returns((new List<TrustSummaryResponse>(), 0));

var result = _controller.SearchTrusts(groupName, ukprn, companiesHouseNumber, 1, 10);
Expand Down Expand Up @@ -126,7 +126,7 @@ public void SearchTrusts_ByGroupNameAndCompaniesHouseNumber_ReturnsListOfTrustSu
NextPageUrl = null
};

_mockSearchTrustsUseCase.Setup(s => s.Execute(1, 10, groupName, null, companiesHouseNumber))
_mockSearchTrustsUseCase.Setup(s => s.Execute(1, 10, groupName, null, companiesHouseNumber, true))
.Returns((expectedTrustSummaries, expectedTrustSummaries.Count));

var result = _controller.SearchTrusts(groupName, null, companiesHouseNumber, 1, 10);
Expand All @@ -153,7 +153,7 @@ public void SearchTrusts_ByUrn_ReturnsListOfTrustSummaries_WhenTrustsAreFound()
NextPageUrl = null
};

_mockSearchTrustsUseCase.Setup(s => s.Execute(1, 10, null, ukprn, null))
_mockSearchTrustsUseCase.Setup(s => s.Execute(1, 10, null, ukprn, null, true))
.Returns((expectedTrustSummaries, expectedTrustSummaries.Count));

var result = _controller.SearchTrusts(null, ukprn, null, 1, 10);
Expand All @@ -172,7 +172,7 @@ public void SearchTrusts_WithNoParams_ReturnsAllTrusts()
RecordCount = 5,
NextPageUrl = null
};
_mockSearchTrustsUseCase.Setup(s => s.Execute(1, 10, null, null, null))
_mockSearchTrustsUseCase.Setup(s => s.Execute(1, 10, null, null, null, true))
.Returns((expectedTrustSummaries, expectedTrustSummaries.Count));

var result = _controller.SearchTrusts(null, null, null, 1, 10);
Expand All @@ -194,7 +194,7 @@ public void SearchTrusts_WithMultiplePagesOfResults_ReturnsAllTrustsWithPagingRe
};

_mockSearchTrustsUseCase
.Setup(s => s.Execute(1, 10, null, null, null))
.Setup(s => s.Execute(1, 10, null, null, null, true))
.Returns((expectedTrustSummaries.Take(10).ToList(), expectedTrustSummaries.Count));

_controller.ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() };
Expand Down
19 changes: 13 additions & 6 deletions TramsDataApi.Test/UseCases/SearchTrustsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public void SearchTrusts_ReturnsEmptyList_WhenNoTrustsFound()
.Returns((new List<Group>(), 0));

var useCase = new SearchTrusts(gateway.Object, new Mock<IEstablishmentGateway>().Object);
var (result, _) = useCase.Execute(1, 10, groupName, ukprn, companiesHouseNumber);
var (result, _) = useCase.Execute(1, 10, groupName, ukprn, companiesHouseNumber, true);

result.Should().BeEquivalentTo(new List<TrustSummaryResponse>());
}
Expand All @@ -50,19 +50,22 @@ public void SearchTrusts_ReturnsListOfTrustSummaryResponses_WhenTrustsFound()

var trustsGateway = new Mock<ITrustGateway>();
var establishmentsGateway = new Mock<IEstablishmentGateway>();

trustsGateway.Setup(g => g.SearchGroups(1, 10, groupName, null, null))
.Returns((expectedTrusts, expectedTrusts.Count));

establishmentsGateway.Setup(g => g.GetByTrustUid(It.IsAny<string>()))
trustsGateway.Setup(m => m.GetIfdTrustsByTrustRef(It.IsAny<string[]>()))
.Returns(new List<Trust>());

establishmentsGateway.Setup(g => g.GetByTrustUids(It.IsAny<string[]>()))
.Returns(new List<Establishment>());

var expected = expectedTrusts.
Select(e => TrustSummaryResponseFactory.Create(e, new List<Establishment>(), null))
.ToList();

var searchTrusts = new SearchTrusts(trustsGateway.Object, establishmentsGateway.Object);
var (result, _) = searchTrusts.Execute(1, 10, groupName, null, null);
var (result, _) = searchTrusts.Execute(1, 10, groupName, null, null, true);

result.Should().BeEquivalentTo(expected);
}
Expand All @@ -81,12 +84,16 @@ public void SearchTrusts_ShouldGetTrustsWithEstablishments_WhenTrustsAndEstablis
.With(e => e.TrustsCode = expectedTrust.GroupUid)
.Build();

var trusts = Builder<Trust>.CreateListOfSize(3).Build();

var trustGateway = new Mock<ITrustGateway>();
var establishmentGateway = new Mock<IEstablishmentGateway>();

trustGateway.Setup(g => g.SearchGroups(1, 10, null, ukprn, null))
.Returns((new List<Group> {expectedTrust}, 1));
establishmentGateway.Setup(g => g.GetByTrustUid(expectedTrust.GroupUid))
trustGateway.Setup(m => m.GetIfdTrustsByTrustRef(It.IsAny<string[]>()))
.Returns(trusts);
establishmentGateway.Setup(g => g.GetByTrustUids(It.IsAny<string[]>()))
.Returns(expectedEstablishments);

var expected = new List<TrustSummaryResponse>
Expand All @@ -95,7 +102,7 @@ public void SearchTrusts_ShouldGetTrustsWithEstablishments_WhenTrustsAndEstablis
};

var searchTrusts = new SearchTrusts(trustGateway.Object, establishmentGateway.Object);
var (result, _) = searchTrusts.Execute(1, 10, null, ukprn, null);
var (result, _) = searchTrusts.Execute(1, 10, null, ukprn, null, true);
result.Should().BeEquivalentTo(expected);
}
}
Expand Down
2 changes: 1 addition & 1 deletion TramsDataApi/Controllers/TrustsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public ActionResult<List<TrustSummaryResponse>> SearchTrusts(string groupName, s
groupName, ukPrn, companiesHouseNumber, page, count);

var trusts = _searchTrusts
.Execute(page, count, groupName, ukPrn, companiesHouseNumber)
.Execute(page, count, groupName, ukPrn, companiesHouseNumber, true)
.Item1.ToList();

_logger.LogInformation(
Expand Down
8 changes: 5 additions & 3 deletions TramsDataApi/Controllers/V2/TrustsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,17 @@ public TrustsController(IGetTrustByUkprn getTrustByUkPrn, ISearchTrusts searchTr

[HttpGet("trusts")]
[MapToApiVersion("2.0")]
public ActionResult<ApiResponseV2<TrustSummaryResponse>> SearchTrusts(string groupName, string ukPrn, string companiesHouseNumber, int page = 1, int count = 50)
public ActionResult<ApiResponseV2<TrustSummaryResponse>> SearchTrusts(string groupName, string ukPrn, string companiesHouseNumber,
int page = 1, int count = 50, bool includeEstablishments = true)
{
_logger.LogInformation(
"Searching for trusts by groupName \"{name}\", UKPRN \"{prn}\", companiesHouseNumber \"{number}\", page {page}, count {count}",
groupName, ukPrn, companiesHouseNumber, page, count);

var (trusts, recordCount) = _searchTrusts
.Execute(page, count, groupName, ukPrn, companiesHouseNumber);

.Execute(page, count, groupName, ukPrn, companiesHouseNumber, includeEstablishments);
trusts = trusts.ToList();

_logger.LogInformation(
"Found {count} trusts for groupName \"{name}\", UKPRN \"{prn}\", companiesHouseNumber \"{number}\", page {page}, count {count}",
trusts.Count(), groupName, ukPrn, companiesHouseNumber, page, count);
Expand Down
2 changes: 1 addition & 1 deletion TramsDataApi/Gateways/ITrustGateway.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public interface ITrustGateway
Group GetGroupByUkPrn(string ukPrn);
Trust GetIfdTrustByGroupId(string groupId);
Trust GetIfdTrustByRID(string RID);
IQueryable<Trust> GetIfdTrustsByTrustRef(string[] trustRefs);
IList<Trust> GetIfdTrustsByTrustRef(string[] trustRefs);
(IList<Group>, int) SearchGroups(int page, int count, string groupName, string ukPrn, string companiesHouseNumber);
IEnumerable<Group> GetMultipleGroupsByUkprn(IEnumerable<string> ukprns);
IEnumerable<Trust> GetMultipleTrustsByGroupId(IEnumerable<string> groupIds);
Expand Down
7 changes: 2 additions & 5 deletions TramsDataApi/Gateways/TrustGateway.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,9 @@ public Trust GetIfdTrustByRID(string RID)
return _dbContext.Trust.FirstOrDefault(x => x.Rid.Equals(RID));
}

public IQueryable<Trust> GetIfdTrustsByTrustRef(string[] trustRefs)
public IList<Trust> GetIfdTrustsByTrustRef(string[] trustRefs)
{
Expression<Func<Trust, bool>> predicate = PredicateBuilder.False<Trust>();
foreach (var trustRef in trustRefs) predicate = predicate.Or(t => t.TrustRef == trustRef);

return _dbContext.Trust.Where(predicate);
return _dbContext.Trust.Where(t => trustRefs.Contains(t.TrustRef)).ToList();
}

public (IList<Group>, int) SearchGroups(int page, int count, string groupName, string ukPrn,
Expand Down
3 changes: 2 additions & 1 deletion TramsDataApi/UseCases/ISearchTrusts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ namespace TramsDataApi.UseCases
{
public interface ISearchTrusts
{
public (IEnumerable<TrustSummaryResponse>, int) Execute(int page, int count, string groupName, string urn, string companiesHouseNumber);
public (IEnumerable<TrustSummaryResponse>, int) Execute(int page, int count, string groupName, string urn, string companiesHouseNumber,
bool includeEstablishments);
}
}
19 changes: 16 additions & 3 deletions TramsDataApi/UseCases/SearchTrusts.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Collections.Generic;
using System.Linq;
using TramsDataApi.DatabaseModels;
using TramsDataApi.Factories;
using TramsDataApi.Gateways;
using TramsDataApi.ResponseModels;
Expand All @@ -17,15 +18,27 @@ public SearchTrusts(ITrustGateway trustGateway, IEstablishmentGateway establishm
_establishmentGateway = establishmentGateway;
}

public (IEnumerable<TrustSummaryResponse>, int) Execute(int page, int count, string groupName, string ukPrn, string companiesHouseNumber)
public (IEnumerable<TrustSummaryResponse>, int) Execute(int page, int count, string groupName, string ukPrn, string companiesHouseNumber,
bool includeEstablishments)
{
var (groups, recordCount) = _trustGateway.SearchGroups(page, count, groupName, ukPrn, companiesHouseNumber);

var groupIds = groups.Select(g => g.GroupId).ToArray();
var trustsForGroup = _trustGateway.GetIfdTrustsByTrustRef(groupIds);

IEnumerable<Establishment> establishmentsForGroup = Enumerable.Empty<Establishment>();

if (includeEstablishments)
{
var groupUids = groups.Select(g => g.GroupUid).ToArray();
establishmentsForGroup = _establishmentGateway.GetByTrustUids(groupUids);
}

return (
groups.Select(group =>
{
var trust = _trustGateway.GetIfdTrustByGroupId(group.GroupId);
var establishments = _establishmentGateway.GetByTrustUid(group.GroupUid);
var establishments = establishmentsForGroup.Where(e => e.TrustsCode == group.GroupUid);
var trust = trustsForGroup.FirstOrDefault(e => e.TrustRef == group.GroupUid);
return TrustSummaryResponseFactory.Create(group, establishments, trust);
}).ToArray(),
recordCount
Expand Down

0 comments on commit 034aecb

Please sign in to comment.