Support Other Endpoints (Overpass-Turbo, TagInfo) about osmapiclient

[jasonhjohnson]

@Lelelo1 @HarelM Interested to see what you all come up with

[HarelM]

I think this class is a good starting point :-)
I think that creating a request query and executing it can be two separate responsibilities.
Note, as can be seen in my class that there's a way to try and query another server in order to avoid some of the query limitations that currently exist in overpass turbo, this is not related to the query string creation/modification of couse.
Is there a way to get both nodes, relations and ways in the same query?
I see that there's no options to change output format.
In any case, I think this can be a nice addition to this lib in general. :-)

[jasonhjohnson]

@Lelelo1 You are amazing

[HarelM]

No, there's no need for a client to make sure the tests are working as expected.
You might want to make sure the query is working and correct using either the web client or starting by writing the client and using it for verification, but once verified the unit test should not go to overpass or send http request, it should have the expected string as part of the test.

[HarelM]

I'm currently not using overpass turbo, I used it in the past from js code, but since my database on the server is up to date with latest osm changes I use it instead of querying overpass turbo.
It could be a nice addition though :-)

[HarelM]

https://github.com/OffenesJena/OSMImports
Looks abandoned, but the code can probably be migrated to latest .net with little effort...

[Lelelo1]

I am interested in having overpass

[HarelM]

Here's my implementation if anyone is interested.
It is very specific for a query I'm doing related to wikipedia tags but I'm sure it can be altered to be more generic:
https://github.com/IsraelHikingMap/Site/blob/f193e70f1278c5deca1d29c07c087b20ffffd6c1/IsraelHiking.DataAccess/OverpassTurboGateway.cs
Feel free to copy it here if needed by PR or other method.

[Lelelo1]

Great thanks. I will experiment a bit with OSMImports

[Lelelo1]

Seems like it works but as I see it it does some old fashioned json parsing, web request. It also doesn't provide bbox to be passed in. What I really liked is the chained queries though.

// OSMImporting
var query = new OverpassQuery();
var result = await query.WithNodes("amenity").And("leisure", "playground").RunQuery(4000);
var nodes = result.Elements.Select(e => Node.Parse(e)).ToList();

What I need is construction of the url so I will simply look and take out those bits out. My own similar implementation will look like this:

var result = await OsmQuery.ForNodes(boundingBox, 1).Add("name").Add("leisure", "playground").Request();

[HarelM]

Yup, you can make it a lot nicer, especially using chaining etc :-)

[HarelM]

I don't have plans to pursue this soon (if at all), but a PR with a class to handle this would be very much welcome :-)

[blackboxlogic]

The overpass turbo language is complicated and I have given up on trying to model any part of it in c#. Maybe an implementation already exists, but I looked a year ago and there wasn't really anything.

However, it would be trivial to accept a query string (composed by someone else), send the request, and parse the result (assuming the result fits the OSM data model).

I wonder if implementing just that could cover most use-cases?

[HarelM]

The results can be several output types: osm, geojson, csv.
I find it difficult to really wrap this in something genetic enough on one hand and strongly typed on the other.
I trend to say that http client is probably your best friend here...
I might be wrong though, but this is what I did in my implementation...

[Lelelo1]

However, it would be trivial to accept a query string (composed by someone else), send the request, and parse the result (assuming the result fits the OSM data model).

I agree.

I have working overpass querying:

    public class OsmQuery
    {
        
        private static List<KeyValuePair<string, string>> Nodes { get; set; }
        private static List<KeyValuePair<string, string>> Ways { get; set; }
        private static List<KeyValuePair<string, string>> Relations { get; set; }

        // should be:
        // "https://overpass-api.de/api/interpreter?data=[out:json][timeout:2];(node[name](57.69417400839879,11.900681422098906,57.71320555817524,11.927288935231376);<;);out meta;";
        private string OverpassUrl { get; set; } 

        private BoundingBox BoundingBox { get; }

        protected OsmQuery(BoundingBox boundingBox, int timeout)
        {
            var url = "https://overpass-api.de/api/interpreter";

            url += "?data=[out:json]";
            url +="[timeout:"+timeout+"];";

            Logic.Log("url:" + url);

            OverpassUrl = url;

            BoundingBox = boundingBox;

            
        }

        public OsmQuery Add(string key, string value = null)
        {
            Nodes?.Add(new KeyValuePair<string, string>(key, value));
            Ways?.Add(new KeyValuePair<string, string>(key, value));
            Relations?.Add(new KeyValuePair<string, string>(key, value));

            return this;
        }

        public static OsmQuery ForNodes(BoundingBox boundingBox, int timeout)
        {
            var osmQuery = new OsmQuery(boundingBox, timeout);

            Nodes = new List<KeyValuePair<string, string>>();
            osmQuery.OverpassUrl += "(node";
           
            return osmQuery;
        }

        /*
        public static OsmQuery ForWays(BoundingBox boundingBox, int timeout)
        {
            Ways = new List<KeyValuePair<string, string>>();
            return new OsmQuery(boundingBox, timeout);
        }

        public static OsmQuery ForRelations(BoundingBox boundingBox, int timeout)
        {
            Relations = new List<KeyValuePair<string, string>>();
            return new OsmQuery(boundingBox, timeout);
        }
        */

        // make generic 'T'
        public Task<Node> Request()
        {
            OverpassUrl += ToOverpassString(Nodes);
            OverpassUrl += ToOverpassString(BoundingBox);
            OverpassUrl += ";";
            OverpassUrl += "<;);out meta;";

            Console.WriteLine("request url...");
            Console.Write(OverpassUrl);

            HttpClient client = new HttpClient();

            //var s = await client.GetStringAsync(OverpassUrl).

            return null; // (return 'Osm')
        }


        static string ToOverpassString(List<KeyValuePair<string, string>> elements)
        {
            string tags = "";
            elements.ForEach(tag => tags += ToOverpassString(tag));
            return tags;
        }

        static string ToOverpassString(BoundingBox boundingBox)
        {
            var minLat = boundingBox.MinLatitude;
            var minLon = boundingBox.MinLongitude;
            var maxLat = boundingBox.MaxLatitude;
            var maxLon = boundingBox.MaxLongitude;

            return "("+minLat+","+minLon+","+maxLat+","+maxLon+")";
        }

        static string ToOverpassString(KeyValuePair<string, string> keyValuePair)
        {
            var key = keyValuePair.Key;
            var value = keyValuePair.Value;

            if(string.IsNullOrEmpty(value))
            {
                return "["+key+"]";
            }

            return "["+keyValuePair.Key+"="+value+"]";
        }

    }

It is separate from the OsmApiClient api currently.

The constructor takes permanent values for the query, the keys and values are added with chaining while request call puts the strings together.

I could add it into OsmApiClient but then the http client would need to take to accept custom urls are mentioned

// (OverpassQuery)
OsmQuery.ForNodes(boundingBox, 1).Add("name").Add("leisure", "playground").Request()

https://overpass-api.de/api/interpreter?data=[out:json][timeout:1];(node[name][leisure=playground](57.6937217712402,11.9008731842041,57.7117309570313,11.9345788955688);<;);out meta;

[Lelelo1]

I agree request and query building should probably be separated.

I don't think requesting from different servers should be a responsibility of the query building either. Maybe query builder would create this part:

data=[out:json][timeout:1];(node[name]leisure=playground;<;);out meta;

And in NonAuthClient eg it could prepend (baseUrl1 += query) and try like in your wikipedia implementation.

When it comes to node, way and relation I think this line can be changed out (depending on ForWays, For ForNode static constructors)

osmQuery.OverpassUrl += "(node";

[Lelelo1]

Here is proposal of the overpass client.

    public class OverpassClient
    {

        static IEnumerable<string> BaseUrls = new List<string>()
        {
            "https://overpass-api.de/api/interpreter"
            // ...
        };

        HttpClient Client { get; } = new HttpClient();

        public Task<string> RequestJsonAsync(string overpassQuery)
        {

            var baseUrl = BaseUrls.GetEnumerator().Current;

            var url = baseUrl + overpassQuery;

            try
            {
                var json = Client.GetStringAsync(url);
                return json;
            }
            catch(Exception exc)
            {
                var hasNext = BaseUrls.GetEnumerator().MoveNext();

                if(!hasNext)
                {
                    return null;
                }

                return RequestJsonAsync(overpassQuery);
            }

        }
    }

[Lelelo1]

What urls should be used?

[Lelelo1]

The BaseUrl should probably be as l list and lopped instead?

Are someone up for adding this feature?

Something I don't understand is how the models should look like.

How should I proceed if I want this overpass feature? Create a fork?

If someone could use the query builder I made and assemble the rest parts like adding tests that would be appreciated. I am not comfortable following rest of the code standards and make the web requests

[HarelM]

I'll review the PR in the next few days, hopefully, and if everything is approved we'll need to release a new version.

[blackboxlogic]

@Lelelo1 If you want the feature right now, create a fork and reference that in your project.
Thanks for making a PR, I meant to look at it yesterday but it will probably be this weekend. I'll likely have some questions/feedback/changes. Once merged, it only takes a few minutes to publish a new version to Nuget.org.
If a week goes by and you don't see any movement, please do pester us.
-Alex
(p.s. I've put it on my calendar for Saturday. That usually gets me to do things.)

[Lelelo1]

Ok I will make a pr then.

I am having problems creating test:

Run Tests is greyed out

The file looks like:
https://github.com/Lelelo1/OsmApiClient/blob/overpass/OsmSharp.IO.API.Tests/OverpassTests.cs

I can run existing tests but how do you I create new tests (I am using Visual Studio for Mac)

[HarelM]

Visual studio for mac is just the worst.
If you can, try using rider from jet brains.
It's free for open source developers.

[Lelelo1]

I was able to get it to work in visual studio code.

Should timeout and data (json eg) be configurable? If so should it be in the client or in the query?

[blackboxlogic]

I think the OverpassClient should only accept a query string, make the request, and parse the result. That way the OverpassQuery can change over time, or calling code could get the query from anywhere else (like copy/paste out of the OverpassTuro website).

[HarelM]

I'm not even sure it should parse the results as there can be few response formats (csv, json, osm, others?).

[blackboxlogic]

Then get rid of OverpassClient entirely? Seems like the value is in OverpassQuery.

[HarelM]

That's actually a valid option as the client won't do much beside slightly wrap httpclient.

[Lelelo1]

There is some value in (re)using the Osm model I think. It works when setting xml to be returned

[HarelM]

No argue here. But the client shouldn't only support one format, it should at least support osm, json and csv (I'm not familiar with others).
It might be 3 different client and a factory to create them based on the required return format but narrowing it to just one format is not the right solution for this library, I believe, as this library should support as many cases as possible.

[Lelelo1]

To make it simple we can indeed have the OverpassQuery on it's own.

However I think you would want to use C# models in the end regardless of the serialization type and use built in (shared ideally) client.

[Lelelo1]

Also, there is need of testing the OverpassQuery. What would that look like?

[HarelM]

Unit test to try and create as many scenarios as possible to make sure that adding nodes, ways, attributes, etc. generates the right query string I believe.

[Lelelo1]

It's just that I need a client to test the query string?

[Lelelo1]

I will probably have a pr up at the start of next week

[HarelM]

So we can close the current PR that you opened? Is it still relevant?

[Lelelo1]

It can be closed