Skip to content

Commit

Permalink
#1169 - Refactored a bit the database updater service to allow contro…
Browse files Browse the repository at this point in the history 
…lling which update is ran when.
  • Loading branch information
@HarelM
HarelM committed Jun 19, 2020
1 parent 98f0215 commit ebf2399
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 62 deletions.
16 changes: 1 addition & 15 deletions IsraelHiking.API/Controllers/UpdateController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Logging;
using Newtonsoft.Json;
using OsmSharp.Changesets;
using System.Net;
using System.Threading;
using System.Threading.Tasks;
using System.Xml.Serialization;

namespace IsraelHiking.API.Controllers
{
Expand All @@ -22,24 +20,19 @@ public class UpdateController : ControllerBase
private static readonly SemaphoreSlim UpdateSemaphore = new SemaphoreSlim(1, 1);

private readonly ILogger _logger;
private readonly IOsmLatestFileFetcherExecutor _osmLatestFileFetcherExecutor;
private readonly IDatabasesUpdaterService _databasesUpdaterService;

/// <summary>
/// Controller's constructor
/// </summary>
/// <param name="osmLatestFileFetcherExecutor"></param>
/// <param name="databasesUpdaterService"></param>
/// <param name="logger"></param>
public UpdateController(
IOsmLatestFileFetcherExecutor osmLatestFileFetcherExecutor,
IDatabasesUpdaterService databasesUpdaterService,
ILogger logger)
{
_osmLatestFileFetcherExecutor = osmLatestFileFetcherExecutor;
_databasesUpdaterService = databasesUpdaterService;
_logger = logger;

}

/// <summary>
Expand Down Expand Up @@ -87,8 +80,6 @@ public async Task<IActionResult> PostUpdateData(UpdateRequest request)
_logger.LogInformation("No specific filters were applied, updating all databases.");
}
_logger.LogInformation("Starting updating site's databases according to request: " + JsonConvert.SerializeObject(request));
await _osmLatestFileFetcherExecutor.Update(request.DownloadOsmFile, request.UpdateOsmFile);

await _databasesUpdaterService.Rebuild(request);
_logger.LogInformation("Finished updating site's databases according to request");
return Ok();
Expand Down Expand Up @@ -122,12 +113,7 @@ public async Task<IActionResult> PutUpdateData()
await UpdateSemaphore.WaitAsync();
try
{
_logger.LogInformation("Starting incremental site's databases update");
using var updatesStream = await _osmLatestFileFetcherExecutor.GetUpdates();
XmlSerializer serializer = new XmlSerializer(typeof(OsmChange));
var changes = (OsmChange) serializer.Deserialize(updatesStream);
await _databasesUpdaterService.Update(changes);
_logger.LogInformation("Finished incremental site's databases update");
await _databasesUpdaterService.Update();
return Ok();
}
finally
Expand Down
4 changes: 0 additions & 4 deletions IsraelHiking.API/Executors/OsmLatestFileFetcherExecutor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ public OsmLatestFileFetcherExecutor(IFileSystemHelper fileSystemHelper,
/// <inheritdoc />
public async Task Update(bool downloadFile = true, bool updateFile = true)
{
if (downloadFile == false && updateFile == false)
{
return;
}
var workingDirectory = Path.Combine(_options.BinariesFolder, OSM_C_TOOLS_FOLDER);
var directoryContents = _fileProvider.GetDirectoryContents(OSM_C_TOOLS_FOLDER);
if (!directoryContents.Any())
Expand Down
18 changes: 13 additions & 5 deletions IsraelHiking.API/Services/Osm/DatabasesUpdaterService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using System.Xml.Serialization;

namespace IsraelHiking.API.Services.Osm
{
Expand All @@ -28,7 +29,7 @@ public class DatabasesUpdaterService : IDatabasesUpdaterService
private readonly IPointsOfInterestAdapterFactory _pointsOfInterestAdapterFactory;
private readonly IPointsOfInterestProvider _pointsOfInterestProvider;
private readonly IFeaturesMergeExecutor _featuresMergeExecutor;
private readonly IOsmLatestFileFetcherExecutor _latestFileFetcherExecutor;
private readonly IOsmLatestFileFetcherExecutor _osmLatestFileFetcherExecutor;
private readonly IPointsOfInterestFilesCreatorExecutor _pointsOfInterestFilesCreatorExecutor;
private readonly IImagesUrlsStorageExecutor _imagesUrlsStorageExecutor;
private readonly IExternalSourceUpdaterExecutor _externalSourceUpdaterExecutor;
Expand Down Expand Up @@ -69,7 +70,7 @@ public DatabasesUpdaterService(IClientsFactory clinetsFactory,
_pointsOfInterestAdapterFactory = pointsOfInterestAdapterFactory;
_pointsOfInterestFilesCreatorExecutor = pointsOfInterestFilesCreatorExecutor;
_featuresMergeExecutor = featuresMergeExecutor;
_latestFileFetcherExecutor = latestFileFetcherExecutor;
_osmLatestFileFetcherExecutor = latestFileFetcherExecutor;
_pointsOfInterestProvider = pointsOfInterestProvider;
_osmGateway = clinetsFactory.CreateNonAuthClient();
_imagesUrlsStorageExecutor = imagesUrlsStorageExecutor;
Expand All @@ -78,9 +79,12 @@ public DatabasesUpdaterService(IClientsFactory clinetsFactory,
}

/// <inheritdoc />
public async Task Update(OsmChange changes)
public async Task Update()
{
_logger.LogInformation("Staring updating from OSM change file");
using var updatesStream = await _osmLatestFileFetcherExecutor.GetUpdates();
XmlSerializer serializer = new XmlSerializer(typeof(OsmChange));
var changes = (OsmChange)serializer.Deserialize(updatesStream);
await Updatehighways(changes);
await UpdatePointsOfInterest(changes);
_logger.LogInformation("Finished updating from OSM change file");
Expand Down Expand Up @@ -166,6 +170,10 @@ public async Task Rebuild(UpdateRequest request)
{
await UpdateExternalSources();
}
if (request.UpdateOsmFile || request.DownloadOsmFile)
{
await _osmLatestFileFetcherExecutor.Update(request.DownloadOsmFile, request.UpdateOsmFile);
}
if (request.Highways)
{
await RebuildHighways();
Expand Down Expand Up @@ -216,7 +224,7 @@ private async Task RebuildPointsOfInterest()
private async Task RebuildHighways()
{
_logger.LogInformation("Starting rebuilding highways database.");
using var stream = _latestFileFetcherExecutor.Get();
using var stream = _osmLatestFileFetcherExecutor.Get();
var osmHighways = await _osmRepository.GetAllHighways(stream);
var geoJsonHighways = _osmGeoJsonPreprocessorExecutor.Preprocess(osmHighways);
await _elasticSearchGateway.UpdateHighwaysZeroDownTime(geoJsonHighways);
Expand All @@ -227,7 +235,7 @@ private async Task RebuildHighways()
private async Task RebuildImages()
{
_logger.LogInformation("Starting rebuilding images database.");
using var stream = _latestFileFetcherExecutor.Get();
using var stream = _osmLatestFileFetcherExecutor.Get();
var features = await _elasticSearchGateway.GetAllPointsOfInterest(false);
var featuresUrls = features.SelectMany(f =>
f.Attributes.GetNames()
Expand Down
3 changes: 1 addition & 2 deletions IsraelHiking.API/Services/Osm/IDatabasesUpdaterService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ public interface IDatabasesUpdaterService
/// This method is responsible of taking the changes, converting them and updating the database,
/// It also fetches latest data from OSM just before updating the database
/// </summary>
/// <param name="changes">A list of changes occurred since last run</param>
/// <returns></returns>
Task Update(OsmChange changes);
Task Update();

/// <summary>
/// This method is responsible for rebuilding the database from the OSM pbf stream
Expand Down
36 changes: 5 additions & 31 deletions Tests/IsraelHiking.API.Tests/Controllers/UpdateControllerTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,35 +1,28 @@
using IsraelHiking.API.Controllers;
using IsraelHiking.API.Executors;
using IsraelHiking.API.Services.Osm;
using IsraelHiking.Common.Api;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using NSubstitute;
using OsmSharp;
using OsmSharp.Changesets;
using System.IO;
using System.Net;
using System.Threading.Tasks;
using System.Xml.Serialization;

namespace IsraelHiking.API.Tests.Controllers
{
[TestClass]
public class UpdateControllerTests
{
private UpdateController _controller;
private IOsmLatestFileFetcherExecutor _osmLatestFileFetcherExecutor;
private IDatabasesUpdaterService _databasesUpdaterService;

[TestInitialize]
public void TestInitialize()
{
_osmLatestFileFetcherExecutor = Substitute.For<IOsmLatestFileFetcherExecutor>();
var logger = Substitute.For<ILogger>();
_databasesUpdaterService = Substitute.For<IDatabasesUpdaterService>();
_controller = new UpdateController(_osmLatestFileFetcherExecutor, _databasesUpdaterService, logger);
_controller = new UpdateController(_databasesUpdaterService, logger);
}

private void SetupContext(IPAddress localIp, IPAddress remoteIp)
Expand Down Expand Up @@ -61,7 +54,6 @@ public void PostUpdateData_LocalAndRemoteDoNotMatch_ShouldReturnBadRequest()
public void PostUpdateData_RequestIsNull_ShouldUpdateAllGateways()
{
SetupContext(IPAddress.Parse("1.2.3.4"), IPAddress.Loopback);
_osmLatestFileFetcherExecutor.Get().Returns(new MemoryStream(new byte[] { 1 }));

_controller.PostUpdateData(null).Wait();

Expand All @@ -73,7 +65,6 @@ public void PostUpdateData_RequestIsNull_ShouldUpdateAllGateways()
public void PostUpdateData_RemoteIs10101010Defaultrequest_ShouldUpdateAllGateways()
{
SetupContext(IPAddress.Parse("1.2.3.4"), IPAddress.Parse("10.10.10.10"));
_osmLatestFileFetcherExecutor.Get().Returns(new MemoryStream(new byte[] {1}));

_controller.PostUpdateData(new UpdateRequest()).Wait();

Expand All @@ -93,52 +84,35 @@ public void PutUpdateData_NonLocal_ShouldReturnBadReqeust()
[TestMethod]
public void PutUpdateData_Local_ShouldUpdate()
{
var changes = new OsmChange {Create = new OsmGeo[] {new Node()}};
_osmLatestFileFetcherExecutor.GetUpdates().Returns(CreateStream(changes));
SetupContext(IPAddress.Parse("1.2.3.4"), IPAddress.Loopback);

_controller.PutUpdateData().Wait();

_databasesUpdaterService.Received(1).Update(Arg.Is<OsmChange>(x => x.Create.Length == changes.Create.Length));
_databasesUpdaterService.Received(1).Update();
}

[TestMethod]
public void PutUpdateData_FromTwoThreads_ShouldUpdateTwice()
{
var changes = new OsmChange {Create = new OsmGeo[] {new Node()}};

_osmLatestFileFetcherExecutor.GetUpdates().Returns(CreateStream(changes), CreateStream(changes));
SetupContext(IPAddress.Parse("1.2.3.4"), IPAddress.Loopback);

_controller.PutUpdateData().ContinueWith((t) => { });
_controller.PutUpdateData().Wait();

_databasesUpdaterService.Received(2).Update(Arg.Is<OsmChange>(x => x.Create.Length == changes.Create.Length));
_databasesUpdaterService.Received(2).Update();
}

[TestMethod]
public void PutUpdateData_WhileRebuildIsRunning_ShouldNotUpdate()
{
var changes = new OsmChange { Create = new OsmGeo[] { new Node() } };
_osmLatestFileFetcherExecutor.Update().Returns(Task.Delay(100));
_osmLatestFileFetcherExecutor.GetUpdates().Returns(CreateStream(changes));
_databasesUpdaterService.Rebuild(Arg.Any<UpdateRequest>()).Returns(Task.Delay(100));
SetupContext(IPAddress.Parse("1.2.3.4"), IPAddress.Loopback);

_controller.PostUpdateData(new UpdateRequest()).ContinueWith((t) => { });
var results = _controller.PutUpdateData().Result as BadRequestObjectResult;

_databasesUpdaterService.DidNotReceive().Update(Arg.Is<OsmChange>(x => x.Create.Length == changes.Create.Length));
_databasesUpdaterService.DidNotReceive().Update();
Assert.IsNotNull(results);
}

private async Task<Stream> CreateStream(OsmChange changes)
{
await Task.Delay(100);
Stream stream = new MemoryStream();
var serializer = new XmlSerializer(typeof(OsmChange));
serializer.Serialize(stream, changes);
stream.Seek(0, SeekOrigin.Begin);
return stream;
}
}
}
23 changes: 18 additions & 5 deletions Tests/IsraelHiking.API.Tests/Services/Osm/DatabasesUpdaterServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
using OsmSharp.Tags;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Xml.Serialization;

namespace IsraelHiking.API.Tests.Services.Osm
{
Expand Down Expand Up @@ -99,8 +101,9 @@ public void TestUpdate_EmptyOsmChangeFile_ShouldNotUpdateAnything()
_geoJsonPreprocessorExecutor
.Preprocess(Arg.Is<List<CompleteWay>>(x => x.Count == 0))
.Returns(new List<Feature>());
_osmLatestFileFetcherExecutor.GetUpdates().Returns(CreateStream(changes));

_service.Update(changes).Wait();
_service.Update().Wait();

_elasticSearchGateway.Received(1).UpdatePointsOfInterestData(Arg.Is<List<Feature>>(x => x.Count == 0));
_elasticSearchGateway.Received(1).UpdateHighwaysData(Arg.Is<List<Feature>>(x => x.Count == 0));
Expand All @@ -122,8 +125,9 @@ public void TestUpdate_OsmChangeFileWithDeletion_ShouldDeleteFromDatabase()
_geoJsonPreprocessorExecutor
.Preprocess(Arg.Is<List<CompleteWay>>(x => x.Count == 0))
.Returns(new List<Feature>());
_osmLatestFileFetcherExecutor.GetUpdates().Returns(CreateStream(changes));

_service.Update(changes).Wait();
_service.Update().Wait();

_elasticSearchGateway.Received(1).DeleteHighwaysById("way_1");
_elasticSearchGateway.Received(1).DeleteOsmPointOfInterestById("way_1", Arg.Any<DateTime?>());
Expand Down Expand Up @@ -152,8 +156,9 @@ public void TestUpdate_OsmChangeFileWithModification_ShouldUpdateDatabase()
.Preprocess(Arg.Is<List<CompleteWay>>(x => x.Count == 1))
.Returns(list);
_osmGateway.GetCompleteWay(1).Returns(way);
_osmLatestFileFetcherExecutor.GetUpdates().Returns(CreateStream(changes));

_service.Update(changes).Wait();
_service.Update().Wait();

_elasticSearchGateway.Received(1).UpdatePointsOfInterestData(Arg.Is<List<Feature>>(x => x.Count == 1));
_elasticSearchGateway.Received(1).UpdateHighwaysData(Arg.Is<List<Feature>>(x => x.Count == 1));
Expand Down Expand Up @@ -194,12 +199,20 @@ public void TestUpdate_OsmChangeFileWithModification_ShouldUpdateDatabaseUsingPo
.Returns(list);
_osmGateway.GetCompleteWay(1).Returns(way);
_elasticSearchGateway.GetPointOfInterestById("way_1", Sources.OSM).Returns(wayFeatureInDatabase);
_osmLatestFileFetcherExecutor.GetUpdates().Returns(CreateStream(changes));

_service.Update(changes).Wait();
_service.Update().Wait();

_elasticSearchGateway.Received(1).UpdatePointsOfInterestData(Arg.Is<List<Feature>>(x => x.Count == 1 && x.First().Attributes.Exists(FeatureAttributes.POI_CATEGORY)));
}


private Stream CreateStream(OsmChange changes)
{
Stream stream = new MemoryStream();
var serializer = new XmlSerializer(typeof(OsmChange));
serializer.Serialize(stream, changes);
stream.Seek(0, SeekOrigin.Begin);
return stream;
}
}
}

0 comments on commit ebf2399

Please sign in to comment.