JustinCanton / Geo.NET

A lightweight method for communicating with the multiple online geocoding APIs.
MIT License
13 stars 8 forks source link

Problem in adapting new version and tests #104

Open ollie10 opened 2 weeks ago

ollie10 commented 2 weeks ago

Hello @JustinCanton,

I was updating the version in order to add the Radar Geocoding you added support for.

I saw that the dependency injection has changed, and I can't find the way to make it work. In a second post, I will also show you some issues with the tests I was trying to do.

But let's see the first issue:

The problem is that the DI moved from services.AddHereServices to var builder = services.AddHereGeocoding(); and this won't work with my actual code.

Do you know any solution?

This Is how I was doing it before:

Program.cs

public class Program
{
    public static void Main(string[] args)
    {
        Logger logger = LogManager.Setup().GetCurrentClassLogger();

        try
        {
            logger.Debug("Starting the website");
            CreateHostBuilder(args).Build().Run();
        }
        catch (Exception exception)
        {
            //NLog: catch setup errors
            logger.Error(exception, "Stopped the website of an exception");
            throw;
        }
        finally
        {
            // Ensure to flush and stop internal timers/threads before application-exit (Avoid segmentation fault on Linux)
            LogManager.Shutdown();
        }
    }

    public static IHostBuilder CreateHostBuilder(string[] args) => Host.CreateDefaultBuilder(args)
    .ConfigureAppConfiguration((hostContext, configApp) =>
    {
        configApp.AddJsonFile("appSettings.json", optional: false);
        // OTHER CONFIGURATIONS
        configApp.AddJsonFile($"appSettings.{hostContext.HostingEnvironment.EnvironmentName}.json", optional: false);
    })
    .ConfigureWebHostDefaults(webBuilder =>
    {
        webBuilder.UseStartup<Startup>();
    }).ConfigureLogging(logging =>
    {
        logging.ClearProviders();
    })
  .UseNLog();
}

Startup.cs

public class Startup
{
    private IConfiguration Configuration { get; }

    public Startup(IConfiguration configuration)
    {
        Configuration = configuration;
    }

    public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
    {
        if (env.IsDevelopment())
        {
            app.UseDeveloperExceptionPage();
        }

        app.UseRouting();
        // OTHER CONFIGURATIONS
    }

    public void ConfigureServices(IServiceCollection services)
    {
        // DEPENDENCY INJECTION
        IOC.InjectDependencies(Configuration, services);

        // OTHER CONFIGURATIONS
    }
}

IOC.cs

public class IOC
{
    public static void InjectDependencies(IConfiguration configuration, IServiceCollection services)
    {
        // APPLICATION CONFIGURATION
        //services.Configure<...

        // SERVICES & REPOSITORIES
        //services.AddTransient<...

        // REPOSITORIES
        //services.AddTransient<...

        //HELPERS
        //services.AddSingleton<...

        // GEOCODING HELPERS
        services.AddHereServices(options => options.UseKey(configuration.GetValue<string>("ApplicationConfiguration:GeoCoding:HereGeocodingApiKey")));
        services.AddSingleton<IGeocodingHelpers, HereGeocodingHelpers>();
    }
}

Now I can't find a way to adapt

services.AddHereServices(options => options.UseKey(configuration.GetValue<string>("ApplicationConfiguration:GeoCoding:HereGeocodingApiKey")));

With

services.AddHereGeocoding();

This also returns to a conversation we had some time ago, about configuring the API keys / secrets at runtime.

Is there a way of doing this?? Many thanks

ollie10 commented 2 weeks ago

About the tests I was speaking about before, I had a test project that I used to compare the accuracy of various providers, and in order to bypass the problem of the runtime API key injection, I was doing it like this:

Startup.cs

public class BingStartup
{
    public IConfiguration _configuration { get; }

    // BING STARTUP CONFIGURATION
    public BingStartup(IConfiguration configuration)
    {
        _configuration = configuration;
    }

    public void Configure(IApplicationBuilder app, IWebHostEnvironment env) { }

    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {
        // GEOCODING CONFIGURATIONS
        KeyBuilder<IHereGeocoding> builder = services.AddHereGeocoding();
        builder.AddKey(_configuration.GetValue<string>("ApplicationConfiguration:GeoCoding:BingGeocodingApiKey"));
        builder.HttpClientBuilder.ConfigureHttpClient(configure_client);

        services.AddSingleton<IGeocodingHelpers, BingGeocodingHelpers>();
    }
}

public class HereStartup
{
    public IConfiguration _configuration { get; }

    // BING STARTUP CONFIGURATION
    public HereStartup(IConfiguration configuration)
    {
        _configuration = configuration;
    }

    public void Configure(IApplicationBuilder app, IWebHostEnvironment env) { }

    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {
        // GEOCODING CONFIGURATIONS
        services.AddHereServices(options => options.UseKey(_configuration.GetValue<string>("ApplicationConfiguration:GeoCoding:HereGeocodingApiKey")));
        services.AddSingleton<IGeocodingHelpers, HereGeocodingHelpers>();
    }
}

public class MapboxStartup
{
    public IConfiguration _configuration { get; }

    // BING STARTUP CONFIGURATION
    public MapboxStartup(IConfiguration configuration)
    {
        _configuration = configuration;
    }

    public void Configure(IApplicationBuilder app, IWebHostEnvironment env) { }

    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {
        // GEOCODING CONFIGURATIONS
        services.AddMapBoxServices(options => options.UseKey(_configuration.GetValue<string>("ApplicationConfiguration:GeoCoding:MapBoxGeocodingApiKey")));
        services.AddSingleton<IGeocodingHelpers, MapboxGeocodingHelpers>();
    }
}

public class ArcGisStartup
{
    public IConfiguration _configuration { get; }

    // BING STARTUP CONFIGURATION
    public ArcGisStartup(IConfiguration configuration)
    {
        _configuration = configuration;
    }

    public void Configure(IApplicationBuilder app, IWebHostEnvironment env) { }

    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {
        // GEOCODING CONFIGURATIONS
        services.AddArcGISServices(options => options.UseClientCredentials(_configuration.GetValue<string>("ApplicationConfiguration:GeoCoding:ArcGISGeocodingClientID"), _configuration.GetValue<string>("ApplicationConfiguration:GeoCoding:ArcGISGeocodingClientSecret")));
        services.AddSingleton<IGeocodingHelpers, ArcgisGeocodingHelpers>();
    }
}

// For Google, I don't use the library; I use the official NuGet, that's why it has a slightly different Startup
public class GoogleStartup
{
    public IConfiguration _configuration { get; }

    // BING STARTUP CONFIGURATION
    public GoogleStartup(IConfiguration configuration)
    {
        _configuration = configuration;
    }

    public void Configure(IApplicationBuilder app, IWebHostEnvironment env) { }

    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {
        // GEOCODING CONFIGURATIONS
        services.Configure<ApplicationConfiguration>(_configuration.GetSection("ApplicationConfiguration"));
        services.AddSingleton<IGeocodingHelpers, GoogleGeocodingHelpers>();
    }
}

And testing like this perhaps for each provider I would like to test let's see Arcgis, but the only thing changing is the call to UseStartup which refers to the Startup method of each provider created in the Startup.cs file

ArcGisTests.cs

[TestClass]
public class ArcGisTests
{
    private IGeocodingHelpers _arcgisGeocoding;
    private TestServer _testServer;
    private TestCityDTO[] _cities;
    private TestAddressesDTO _addresses;
    private Random _random;

    private readonly CultureInfo CULTURE = new CultureInfo("es-ES");

    [TestInitialize]
    public void Init()
    {
        _testServer = new TestServer(new WebHostBuilder().ConfigureAppConfiguration((hostContext, configApp) =>
        {
            configApp.AddJsonFile("appSettings.json", optional: false);
        }).UseStartup<ArcGisStartup>());

        _random = new Random();
        _arcgisGeocoding = _testServer.Services.GetRequiredService<IGeocodingHelpers>();

        // READING TEST FILE
        string citiesArray = File.ReadAllText("C:\\...\\cities.json");
        _cities = JsonSerializer.Deserialize<TestCityDTO[]>(citiesArray);

        string addressesArray = File.ReadAllText("C:\\...\\addresses-us-all.json");
        _addresses = JsonSerializer.Deserialize<TestAddressesDTO>(addressesArray);
    }

    #region GeocodeWithoutRegion
    [TestMethod]
    public async Task Test_GeocodeCity_Without_Region_Without_Culture_1()
    {
        TestCityDTO testCity = _cities[_random.Next(0, _cities.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.GeocodeAddress(string.Concat(testCity.Name, ", ", testCity.CountryName), CultureInfo.InvariantCulture);

        AssertsHelpers.AssertCity(testCity, completeAddress);
    }

    [TestMethod]
    public async Task Test_GeocodeAddress_Without_Region_Without_Culture_2()
    {
        TestAddressDTO testAddress = _addresses.Addresses[_random.Next(0, _addresses.Addresses.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.GeocodeAddress(string.Concat(testAddress.Address1, ", ", testAddress.City), CultureInfo.InvariantCulture);

        AssertsHelpers.AssertAddress(testAddress, completeAddress);
    }

    [TestMethod]
    public async Task Test_GeocodeCity_Without_Region_With_Culture_3()
    {
        TestCityDTO testCity = _cities[_random.Next(0, _cities.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.GeocodeAddress(string.Concat(testCity.Name, ", ", testCity.CountryName), CULTURE);

        AssertsHelpers.AssertCity(testCity, completeAddress);
    }

    [TestMethod]
    public async Task Test_GeocodeAddress_Without_Region_With_Culture_4()
    {
        TestAddressDTO testAddress = _addresses.Addresses[_random.Next(0, _addresses.Addresses.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.GeocodeAddress(string.Concat(testAddress.Address1, ", ", testAddress.City), CULTURE);

        AssertsHelpers.AssertAddress(testAddress, completeAddress);
    }
    #endregion

    #region GeocodeWithRegion
    [TestMethod]
    public async Task Test_GeocodeCity_With_Region_Without_Culture_5()
    {
        TestCityDTO testCity = _cities[_random.Next(0, _cities.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.GeocodeAddress(string.Concat(testCity.Name, ", ", testCity.CountryName), CultureInfo.InvariantCulture, new RegionInfo(testCity.CountryCode));

        AssertsHelpers.AssertCity(testCity, completeAddress);
    }

    [TestMethod]
    public async Task Test_GeocodeAddress_With_Region_Without_Culture_6()
    {
        TestAddressDTO testAddress = _addresses.Addresses[_random.Next(0, _addresses.Addresses.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.GeocodeAddress(string.Concat(testAddress.Address1, ", ", testAddress.City), CultureInfo.InvariantCulture, new RegionInfo("US"));

        AssertsHelpers.AssertAddress(testAddress, completeAddress);
    }

    [TestMethod]
    public async Task Test_GeocodeCity_With_Region_With_Culture_7()
    {
        TestCityDTO testCity = _cities[_random.Next(0, _cities.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.GeocodeAddress(string.Concat(testCity.Name, ", ", testCity.CountryName), CULTURE, new RegionInfo(testCity.CountryCode));

        AssertsHelpers.AssertCity(testCity, completeAddress);
    }

    [TestMethod]
    public async Task Test_GeocodeAddress_With_Region_With_Culture_8()
    {
        TestAddressDTO testAddress = _addresses.Addresses[_random.Next(0, _addresses.Addresses.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.GeocodeAddress(string.Concat(testAddress.Address1, ", ", testAddress.City), CULTURE, new RegionInfo("US"));

        AssertsHelpers.AssertAddress(testAddress, completeAddress);
    }
    #endregion

    #region ReverseGeocodeCity
    [TestMethod]
    public async Task Test_ReverseGeocodeCity_Without_Parameters_9()
    {
        TestCityDTO testCity = _cities[_random.Next(0, _cities.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.ReverseGeocodeCoordinates(double.Parse(testCity.Latitude, CultureInfo.InvariantCulture), double.Parse(testCity.Longitude, CultureInfo.InvariantCulture));

        AssertsHelpers.AssertCity(testCity, completeAddress);
    }

    [TestMethod]
    public async Task Test_ReverseGeocodeCity_Without_Culture_10()
    {
        TestCityDTO testCity = _cities[_random.Next(0, _cities.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.ReverseGeocodeCoordinates(double.Parse(testCity.Latitude, CultureInfo.InvariantCulture), double.Parse(testCity.Longitude, CultureInfo.InvariantCulture), CultureInfo.InvariantCulture);

        AssertsHelpers.AssertCity(testCity, completeAddress);
    }

    [TestMethod]
    public async Task Test_ReverseGeocodeCity_With_Culture_11()
    {
        TestCityDTO testCity = _cities[_random.Next(0, _cities.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.ReverseGeocodeCoordinates(double.Parse(testCity.Latitude, CultureInfo.InvariantCulture), double.Parse(testCity.Longitude, CultureInfo.InvariantCulture), CULTURE);

        AssertsHelpers.AssertCity(testCity, completeAddress);
    }

    [TestMethod]
    public async Task Test_ReverseGeocodeCity_Without_Culture_With_Region_12()
    {
        TestCityDTO testCity = _cities[_random.Next(0, _cities.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.ReverseGeocodeCoordinates(double.Parse(testCity.Latitude, CultureInfo.InvariantCulture), double.Parse(testCity.Longitude, CultureInfo.InvariantCulture), CultureInfo.InvariantCulture, new RegionInfo(testCity.CountryCode));

        AssertsHelpers.AssertCity(testCity, completeAddress);
    }

    [TestMethod]
    public async Task Test_ReverseGeocodeCity_With_Culture_With_Region_13()
    {
        TestCityDTO testCity = _cities[_random.Next(0, _cities.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.ReverseGeocodeCoordinates(double.Parse(testCity.Latitude, CultureInfo.InvariantCulture), double.Parse(testCity.Longitude, CultureInfo.InvariantCulture), CULTURE, new RegionInfo(testCity.CountryCode));

        AssertsHelpers.AssertCity(testCity, completeAddress);
    }
    #endregion

    #region ReverseGeocodeAddress
    [TestMethod]
    public async Task Test_ReverseGeocodeAddress_Without_Parameters_14()
    {
        TestAddressDTO testAddress = _addresses.Addresses[_random.Next(0, _addresses.Addresses.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.ReverseGeocodeCoordinates(testAddress.Coordinates.Latitude, testAddress.Coordinates.Longitude);

        AssertsHelpers.AssertAddress(testAddress, completeAddress);
    }

    [TestMethod]
    public async Task Test_ReverseGeocodeAddress_Without_Culture_15()
    {
        TestAddressDTO testAddress = _addresses.Addresses[_random.Next(0, _addresses.Addresses.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.ReverseGeocodeCoordinates(testAddress.Coordinates.Latitude, testAddress.Coordinates.Longitude, CultureInfo.InvariantCulture);

        AssertsHelpers.AssertAddress(testAddress, completeAddress);
    }

    [TestMethod]
    public async Task Test_ReverseGeocodeAddress_With_Culture_16()
    {
        TestAddressDTO testAddress = _addresses.Addresses[_random.Next(0, _addresses.Addresses.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.ReverseGeocodeCoordinates(testAddress.Coordinates.Latitude, testAddress.Coordinates.Longitude, CULTURE);

        AssertsHelpers.AssertAddress(testAddress, completeAddress);
    }

    [TestMethod]
    public async Task Test_ReverseGeocodeAddress_Without_Culture_With_Region_17()
    {
        TestAddressDTO testAddress = _addresses.Addresses[_random.Next(0, _addresses.Addresses.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.ReverseGeocodeCoordinates(testAddress.Coordinates.Latitude, testAddress.Coordinates.Longitude, CultureInfo.InvariantCulture, new RegionInfo("US"));

        AssertsHelpers.AssertAddress(testAddress, completeAddress);
    }

    [TestMethod]
    public async Task Test_ReverseGeocodeAddress_With_Culture_With_Region_18()
    {
        TestAddressDTO testAddress = _addresses.Addresses[_random.Next(0, _addresses.Addresses.Length)];
        CompleteAddressDTO completeAddress = await _arcgisGeocoding.ReverseGeocodeCoordinates(testAddress.Coordinates.Latitude, testAddress.Coordinates.Longitude, CULTURE, new RegionInfo("US"));

        AssertsHelpers.AssertAddress(testAddress, completeAddress);
    }
    #endregion
}

This won't work either due to the changes.

Is there a way to achieve the same result? Build and compare the real results of each provider calling the real APIs of each one of them in a TEST project?

JustinCanton commented 1 week ago

Hello @ollie10

With the new approach, you get returned a builder to configure the services. Now your key initialization would change to there:

var builder = services.AddHereGeocoding();
builder.AddKey(your_here_api_key_here);
builder.HttpClientBuilder.ConfigureHttpClient(configure_client);

So in your case, it would just be

var builder = services.AddHereGeocoding();
builder.AddKey(configuration.GetValue<string>("ApplicationConfiguration:GeoCoding:HereGeocodingApiKey"));

As for configuring the key at runtime, yes. The geocoding wrappers now support that. You can pass the key in with the Parameters object and it will be used if passed.

Please let me know if this solves your issues for you.

Justin

ollie10 commented 1 week ago

Hey @JustinCanton, in my case this won't work, because in the ConfigureServices of an application developed with .NET 5.0 or earlier, you won't have the IApplicationBuilder in it. This is different from .NET 6.0 and later, but if you started with 5.0 or earlier, you won't. You can see the structure here; where would you put it in this case? https://learn.microsoft.com/en-us/aspnet/core/fundamentals/startup?view=aspnetcore-5.0

JustinCanton commented 6 days ago

@ollie10, sorry, I was on vacation. I am confused, and maybe I am not understanding. You don't need the IApplicationBuilder to register this. It can be registered using that, but it can also be registered using the IServiceCollection.

In my example above, the builder is being created when you call AddHereGeocoding() on the IServiceCollection.

For .net5, it would be

public void ConfigureServices(IServiceCollection services)
{
    var hereBuilder = services.AddHereGeocoding();
    hereBuilder.AddKey(your_here_api_key_here);
    hereBuilder.HttpClientBuilder.ConfigureHttpClient(configure_client);

    services.AddRazorPages();
}

For .net6, it would be

var builder = WebApplication.CreateBuilder(args);

var hereBuilder = builder.Service.AddHereGeocoding();
hereBuilder.AddKey(your_here_api_key_here);
hereBuilder.HttpClientBuilder.ConfigureHttpClient(configure_client);

builder.Services.AddRazorPages();

Let me know if this clears things up for you.

Justin

ollie10 commented 6 days ago

Hi @JustinCanton many thanks. I didn't know it was possible to use the IServiceCollection because it wasn't documented in the examples; now it's clear, thanks.

There is only one problem: I can't understand what exactly the configure_client property is, and where do you pick this variable? It's not clear from the examples

Besides that, do you have some examples / tests where it shows how to configure the API key or secrets at runtime? Many thanks

JustinCanton commented 5 days ago

I think the documentation is clear enough. It actually still uses ConfigureServices, which is the .net5 method of doing things, clearly using the IServiceCollection for the configuration.

As for configure_client, that is just showing that you have the ability to configure the HttpClientBuilder is you so choose. It isn't required, just another item exposed and usable with the new builder approach.

I don't have any examples in code, but there are tests. The key is just a new property on the Parameter set: https://github.com/JustinCanton/Geo.NET/blob/develop/src/Geo.Here/Models/Parameters/AutosuggestParameters.cs

Notice the Key property. If it has a key in it, it will be used. Otherwise the startup configured key will be used.

A test case creating the object can be found here: https://github.com/JustinCanton/Geo.NET/blob/develop/test/Geo.Here.Tests/Services/HereGeocodingShould.cs Look for AddHereKey_WithParameterOverride_SuccessfullyAddsKey

Hopefully this clears up any more confusion.

Justin