justincanton / geo.net Goto Github PK

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

Hey @JustinCanton

I have posted this just for you to have it on the roadmap. It would be very nice to have support for the Nominatim API.

This API is the free, public, open-source, API of OpenStreetMap. You can host yourself or use one of the few publicly hosted versions available, such as perhaps

• The official public one
• The one from Qgis

So the API server should be configurable to pick the one you use (hosted or private)
What do you think? The documentation is simple and pretty straightforward

Many thanks
Best regards

Hey @JustinCanton,

I was just switching to your latest version and my unit tests already spotted an error.

In Here Maps you are passing the 'in' parameters in this way:
https://geocode.search.hereapi.com/v1/geocode?q=31 Hills Point Road, Charlotte&in=USA&limit=1&lang=es-ES&show=countryInfo,streetInfo&apiKey=xxx

But according to their documentation you should use:

countryCode:USA OR countryCode:CAN,MEX,USA

I used the exception data as you told me the other time and this is what I saw:

{"status":400,"title":"Illegal input for parameter 'in'","cause":"Actual parameter value: 'USA'","action":"Unsupported key: 'USA'. Supported keys: 'countryCode'","correlationId":"fa73fe06-133b-4aaf-b654-e9c75177e7ac","requestId":"REQ-c5ae3f1b-3231-4f50-8cb7-15f0277af84c"}

Do you think it will be fast to fix and redeploy to nuget?
Many thanks!

Hi @JustinCanton,

Followinf my tests I saw that the FindAddressCandidates of ArcGis is missing pretty many parameters, but looking in the source code I think most of the endppoints of ArcGis are missing a lot of parameters.

In this case perhaps are specially interesting countryCode, category, maxLocations, matchOutOfRange, locationType, langCode, sourceCountry among others.

And I was wondering if you are using the outFields=* to retrieve all the response, because in any other case they return the bare minimum.

Many thanks!

Describe the bug
It seems like there is a concurrency issue when creating a resource provider in the services. It doesn't seem to happen often, as it has only happened once in all of the builds.

To Reproduce
It is a random error, that I cannot reproduce. It has only happened in one build.

  Failed Geo.MapQuest.Tests.Services.MapQuestGeocodingShould.ReverseGeocodingAsyncSuccessfully [231 ms]
  Error Message:
   System.ArgumentException : The key already existed in the dictionary.
  Stack Trace:
     at System.Collections.Concurrent.ConcurrentDictionary`2.System.Collections.Generic.IDictionary<TKey,TValue>.Add(TKey key, TValue value)
   at Geo.Core.GeoNETResourceStringProviderFactory.CreateResourceStringProvider(String resourceFileName, Assembly assembly) in /home/runner/work/Geo.NET/Geo.NET/src/Geo.Core/GeoNETResourceStringProviderFactory.cs:line 54
   at Geo.Core.GeoNETResourceStringProviderFactory.CreateResourceStringProvider(Type resourceType) in /home/runner/work/Geo.NET/Geo.NET/src/Geo.Core/GeoNETResourceStringProviderFactory.cs:line 35
   at Geo.Core.GeoNETResourceStringProviderFactory.CreateResourceStringProvider[TResource]() in /home/runner/work/Geo.NET/Geo.NET/src/Geo.Core/GeoNETResourceStringProviderFactory.cs:line 24
   at Geo.Core.ClientExecutor..ctor(HttpClient client, IGeoNETExceptionProvider exceptionProvider, IGeoNETResourceStringProviderFactory resourceStringProviderFactory, ILoggerFactory loggerFactory) in /home/runner/work/Geo.NET/Geo.NET/src/Geo.Core/ClientExecutor.cs:line 43
   at Geo.MapQuest.Services.MapQuestGeocoding..ctor(HttpClient client, IMapQuestKeyContainer keyContainer, IMapQuestEndpoint endpoint, IGeoNETExceptionProvider exceptionProvider, IGeoNETResourceStringProviderFactory resourceStringProviderFactory, ILoggerFactory loggerFactory) in /home/runner/work/Geo.NET/Geo.NET/src/Geo.MapQuest/Services/MapQuestGeocoding.cs:line 54
   at Geo.MapQuest.Tests.Services.MapQuestGeocodingShould.BuildService(MapQuestEndpoint endpoint) in /home/runner/work/Geo.NET/Geo.NET/test/Geo.MapQuest.Tests/Services/MapQuestGeocodingShould.cs:line 526
   at Geo.MapQuest.Tests.Services.MapQuestGeocodingShould.ReverseGeocodingAsyncSuccessfully() in /home/runner/work/Geo.NET/Geo.NET/test/Geo.MapQuest.Tests/Services/MapQuestGeocodingShould.cs:line 473
--- End of stack trace from previous location where exception was thrown ---

Expected behavior
The code should successfully create the resource even in a concurrent environment.

Is your feature request related to a problem? Please describe.
Direct support for net7. needs to be added.

Describe the solution you'd like
Add net7 support

Hi, it seems that an important property, the Language is missing from the MapBox Geocoder.
As from the documentation, a language can be passed to influence the results but, there is no configuration for this, except the readonly Languages property.

Do you plan to implement this in the close future?
Many thanks

I believe we should add support for the OpenRoute API.
https://openrouteservice.org/dev/#/api-docs/geocode

Is your feature request related to a problem? Please describe.
I would like to avoid using Microsoft localization within the library, since using it restricts what other users are able to do in terms of localization.

Describe the solution you'd like
I would like to use the ResourceManager class to fetch resources instead of using the IStringLocalizer wrapper.

Describe alternatives you've considered
There aren't too many alternatives for this unfortunately. I could abandon resource files altogether, but I would prefer to avoid that.

Additional context

Hello @JustinCanton,

I was using the library and I saw something that could improve the usability of it. Basically it seems that there is no possibility to configure API keys of the providers outside the Startup.

This could lead to some problems sometimes, maybe you want to have some custom or complex logic to decide which API key should you use directly in the service using the provider.

Basically, something like this:

public MapboxGeocodingHelpers(IOptions<ApplicationConfiguration> appSettings, IMapBoxGeocoding mapBoxGeocoding)
{
   _mapBoxGeocoding = mapBoxGeocoding;
   _mapBoxGeocoding.SetApiKey(appSettings.Value.MapboxAPIKey)
}

Or like the official Google API is doing

public async Task<CompleteAddressDTO> GeocodeAddress(string address, CultureInfo culture)
{
   AddressGeocodeRequest request = new AddressGeocodeRequest { Key = _appSettings.GoogleMapsApiKeyForBackEnd, Address = address, Language = GetLanguageFromCulture(culture) };
   GeocodeResponse response = await GoogleMaps.Geocode.AddressGeocode.QueryAsync(request);
   ...
}

Do you plan to implement something like that? I think it will be very useful in various scenarios

I believe we should add support for the Trimble Maps API.
https://developer.trimblemaps.com/restful-apis/location/geocoding-api/overview/

Hi @JustinCanton

As told before I will report you missing parameters, in the case of Bing Maps REST Service.

I saw that the most important is the culture which is missing, but if you want a view of what's missing i think here you have the list

Many thanks

In the ArcGIS module, the value of the outFields query param for the Address candidate request and the Place candidate request is hard-coded in the request building methods:

Can support be added to allow overriding of this field, maybe via the parameters class, to include other attributes that may be desired in the ArcGIS response?

Thanks!

Is your feature request related to a problem? Please describe.
Usage of the framework reference is causing issues for usage of the nuget outside of an AspNet Core environment is causing issues. See #59 for more information about the usage.

Describe the solution you'd like
Remove the framework reference.

Describe alternatives you've considered
The alternatives at the moment are:

• Use an older version of Microsoft.AspNetCore.Http.Abstractions for all frameworks
• Pull the QueryString code into this repo and use a local implementation

Is your feature request related to a problem? Please describe.
The HERE endpoints take in a flexible polyline which specify the area to search within. It would be nice to have the ability to take in a coordinate, and convert it to a polyline within the this nuget, instead of requiring a polyline be passed in.

Describe the solution you'd like
Reference the nuget from the https://github.com/heremaps/flexible-polyline for generating the polyline information.

Describe alternatives you've considered
Implement polyline support ourselves to avoid external nuget dependencies.

Additional context
See https://github.com/heremaps/flexible-polyline for more information about how it works.

Hi @JustinCanton , I was making some tests for the HERE connector.

I have got the error: "The call to Here did not return a successful http status code. See the exception data for more information." but no Inner Exception is shown. This leads to that you don't know if your API key is invalid, if you exceeded the limits, if you sent a wrong parameter etc. etc. and it is impossible to debug what's going on.

Here the exampe: https://ibb.co/Y7BwsFf

Many thanks

Hi @JustinCanton

As told before I will report you missing parameters, in the case of Here Maps Forward & Reverse Geocoding.

types, politicalView, show Parameters are missing from the Forward Geocoder

in, types, politicalView, show Parameters are missing from the Forward Geocoder

Many thanks

Hello @JustinCanton , I saw various enhancements about adding new providers, so if I can suggest one, I would suggest Radar. They have well-written APIs, which works pretty well, and a free tier of 100.000 calls a month.

Cheers

I believe we should add support for the PositionStack API.
https://positionstack.com/documentation

Hey @JustinCanton, some responses from ArcGIS are not being deserialized properly due to Rank property of LocationAttribute being unable to handle floating point values.

Per the ArcGIS REST API docs Rank is a floating-point number value, but Rank is an int in

so when a response contains a Rank value that cannot be parsed to an int (e.g. 1.5), a deserialization exception is thrown:

Geo.Core.Models.Exceptions.GeoNETException: Failed to parse the ArcGIS response properly. See the inner exception for more information.
 ---> System.Text.Json.JsonException: The JSON value could not be converted to System.Int32. Path: $.Rank | LineNumber: 0 | BytePositionInLine: 193.
 ---> System.FormatException: Either the JSON value is not in a supported format, or is out of bounds for an Int32.
   at System.Text.Json.ThrowHelper.ThrowFormatException(NumericType numericType)
   at System.Text.Json.Utf8JsonReader.GetInt32()
 ...
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1.DeserializeAsync(Stream utf8Json, CancellationToken cancellationToken)
   at Geo.Core.GeoClient.ParseResponseAsync[TResult](HttpResponseMessage response, CancellationToken cancellationToken)
   at Geo.Core.GeoClient.CallAsync[TResult](Uri uri, HttpMethod method, HttpContent content, CancellationToken cancellationToken)
   at Geo.ArcGIS.Services.ArcGISGeocoding.AddressCandidateAsync(AddressCandidateParameters parameters, CancellationToken cancellationToken)

Is your feature request related to a problem? Please describe.
Net5 has been out of support by Microsoft for a while. It should be dropped from the target frameworks of the Geo packages.

Describe the solution you'd like
We need to remove direct net5 support from the nuget.

When calling .BuildGeocodingRequest or .BuildReverseGeocodingRequest with a query that contains a pound sign (#) the resulting URI is truncated at the location of the # and characters after are not included. This is related to how UriBuilder and UriBuilder .Query interact, specifically the constructed URI is fine (line 125) but when UriBuilder.Query is assigned to at 158 it truncates everything after #. There may be other problematic characters related to the construction of URIs.

According to the MapBox api any such punctuation can be passed but it's treated as a word separator:
https://docs.mapbox.com/help/troubleshooting/address-geocoding-format-guide/

One possible fix is to replace all such punctuation with spaces.

A more ambitious fix would be to implement some sort of GeocodingParameters.Query builder pattern which could handle this and also enforce other MapBox query limitations such as a max "word" length of 20 and a max character length of 256. This pattern could also implement the "batch" feature where multiple queries are separated via semicolons (though batch queries also require mapbox.places-permanent)

Is your feature request related to a problem? Please describe.
The lack of support for netstandard2.0 limits the number of frameworks able to use the library. I would like to expand support to include netstandard2.0 which is far more flexible.

Describe the solution you'd like
Add netstandard2.0 support.

Hi @JustinCanton

As told before I will report you missing parameters, in the case of Mapbox the Forward & Reverse Geocoding worldview parameter is the only one missing

Many thanks

When a query to MapBox returns 0 results the library isn't handling it gracefully.

To Reproduce
using nuget Geo.MapBox 1.1.1

Call Mapbox.GeocodingAsync with some query that returns no results.

Raw request:

• replace NONEXISTENTPLACE with any number of queries that have 0 results
• replace TOKEN with your access token

GET https://api.mapbox.com/geocoding/v5/mapbox.places/NONEXISTENTPLACE?autocomplete=true&fuzzyMatch=true&limit=1&routing=false&access_token=TOKEN HTTP/1.1
Host: api.mapbox.com

Raw response

HTTP/1.1 404 Not Found
Content-Type: application/json; charset=utf-8
Content-Length: 23
Connection: keep-alive
Date: Tue, 09 Aug 2022 00:01:06 GMT
X-Powered-By: Express
Access-Control-Allow-Origin: *
Access-Control-Expose-Headers: x-rate-limit-interval,x-rate-limit-limit,x-rate-limit-remaining,x-rate-limit-reset
Cache-Control: max-age=604800
X-Rate-Limit-Limit: 600
X-Rate-Limit-Interval: 60
X-Rate-Limit-Reset: 1660003326
Last-Modified: Thu, 04 Aug 2022 19:09:01 GMT
Vary: Accept-Encoding
X-Cache: Error from cloudfront
Via: 1.1 5ece3a8d1e959c303daa9320e4fea502.cloudfront.net (CloudFront)
Age: 77

{"message":"Not Found"}

The library throws an exception with no exception data or inner exception.

Geo.MapBox.Models.Exceptions.MapBoxException
  HResult=0x80131500
  Message=The call to MapBox did not return a successful http status code. See the exception data for more information. See the inner exception for more information.
  Source=Geo.Core
  StackTrace:
   at Geo.Core.ClientExecutor.<CallAsync>d__4`2.MoveNext()
   at Geo.MapBox.Services.MapBoxGeocoding.<GeocodingAsync>d__9.MoveNext()
InnerException | null | System.Exception

Expected behavior
One of the following in order of preference (you should probably standardize the "no result" response across your other providers:

1. A Empty set/object
2. null
3. An exception properly indicating no results found.

Describe the bug
When generating the query string, some objects are using their ToString method to build up information. This is causing a problem when it comes to different cutltures though. Some cultures print out floats differently. We need to make sure the ToString generation is culture invariant.

To Reproduce
Steps to reproduce the behavior:

1. Call MapBox Reverse Geocoding passing in a coordinate in a Turkish culture
2. Check the generated url, the url has a point with a comma (29,2323234545)

Expected behavior
The generate url should have a point with a decimal (29.2323234545)

I believe we should add support for the Geoapify API.
https://apidocs.geoapify.com/docs/geocoding/forward-geocoding/#about

We need to move from Newtonsoft to System.Text.Json, since Newtonsoft has reached its end of life and is slower in comparison to other json serializers.

Utf8Json is an alternative that can be considered, but I would prefer to not have external dependencies within the code aside from Microsoft dependencies.

Is your feature request related to a problem? Please describe.
There are currently different exceptions for each API being called. This is a lot to deal with when catching the exceptions or using the code.

Describe the solution you'd like
I would like to consolidate these and have them under one exception to allow for easier exception handling.