Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve performance of trust search by batching sql commands #305

Merged
merged 2 commits into from
Feb 22, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
46 changes: 40 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,34 @@ 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);
}

[Fact]
public void SearchTrusts_WithoutEstablishments_()
{
var ukprn = "mockurn";
var expectedTrust = Builder<Group>
.CreateNew()
.With(g => g.Ukprn = ukprn)
.Build();

var trusts = Builder<Trust>.CreateListOfSize(3).Build();
var trustGateway = new Mock<ITrustGateway>();

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

var expected = new List<TrustSummaryResponse>
{
TrustSummaryResponseFactory.Create(expectedTrust, Enumerable.Empty<Establishment>(), null)
};

var searchTrusts = new SearchTrusts(trustGateway.Object, null);
var (result, _) = searchTrusts.Execute(1, 10, null, ukprn, null, false);
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 ukPrn, 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