search

JustinCanton / Geo.NET

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

Don't pull in entire ASP.NET framework #59

Closed Farthom closed 1 year ago

Farthom commented 1 year ago

I've attempted to use this library in a background application responsible for processing GPS coordinates. Our application being a backend processor is NOT an AspNet application and that framework is not available in the production environment on which our application runs. The problem is, Geo.NET contains a hard reference to the AspNet framework. This means, simply adding a reference to Geo.NET causes the production services to fail to start. (Errors about missing AspNet framework)

I've poked through a bit, and it seems the only usage is "QueryString" class for building request Uris. This class exists in the Http.Abstractions package which is perfectly fine to reference even in non aspnet applications. I've not dug too deeply but I can't seem to see any good reason to be pulling in this entire framework. I've made a temporary one off build of this library encompassing the below changes and it is working great for us. Hopefully they can be merged into the official package.

JustinCanton commented 1 year ago

@Farthom, the usage of the Microsoft.AspNetCore.Http.Abstractions nuget has been abandon by Microsoft. It is an old netcoreapp2.2 nuget (as evidenced by the versioning of it).

The proper upgrade path for that nuget it to use a framework reference in newer versions of .net: https://github.com/dotnet/aspnetcore/issues/18408

Now, I can dig into other solutions for this problem, to try and see if there are any workarounds. But I would prefer not to maintain the usage of the Microsoft.AspNetCore.Http.Abstractions nuget in newer .net versions.

JustinCanton commented 1 year ago

Unfortunately even this ticket talks about how the code has been moved into .NET Core: https://github.com/dotnet/aspnetcore/issues/20946

They do mention that it is possible to use the older package, as long as it doesn't cause conflict. I am worried that it may cause conflicts for people consuming Geo.NET though.

I will need to think it over for a bit more. Another option is to just take the source code for the QueryString class and add the class locally.

JustinCanton commented 1 year ago

I have created #60 for this issue. At the moment, I am leaning towards pulling QueryString into the Geo.Core project and building from there.

JustinCanton commented 1 year ago

I have thought it over for a while, and I think the best thing to do here is to create a local copy of the QueryString class. I will be closing this PR and making a new PR with the changes.

Farthom commented 1 year ago

Fantastic! I look forward to the next version I can use.

Thanks so much!

JustinCanton commented 1 year ago

No problem! I will merge the code tonight, and you should have an alpha build you can play with, if you want.