OsmApiClient icon indicating copy to clipboard operation
OsmApiClient copied to clipboard

Support Other Endpoints (Overpass-Turbo, TagInfo)

Open blackboxlogic opened this issue 4 years ago • 37 comments

https://wiki.openstreetmap.org/wiki/Overpass_API https://taginfo.openstreetmap.org/taginfo/apidoc https://nominatim.org/release-docs/develop/api/Overview/

I put this here as something which could be added. Please comment if this would be helpful or you.

First, see if a .net client already exists.

Add the code, add tests, update documentation, update this, release a new version.

blackboxlogic avatar Jun 13 '20 22:06 blackboxlogic

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 avatar Jun 14 '20 07:06 HarelM

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

HarelM avatar Jun 14 '20 07:06 HarelM

I am interested in having overpass

Lelelo1 avatar Jan 30 '22 10:01 Lelelo1

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.

HarelM avatar Jan 30 '22 10:01 HarelM

Great thanks. I will experiment a bit with OSMImports

Lelelo1 avatar Jan 30 '22 11:01 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();

Lelelo1 avatar Jan 30 '22 15:01 Lelelo1

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

HarelM avatar Jan 30 '22 17:01 HarelM

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

jasonhjohnson avatar Feb 01 '22 19:02 jasonhjohnson

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 :-)

HarelM avatar Feb 01 '22 20:02 HarelM

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?

blackboxlogic avatar Feb 01 '22 21:02 blackboxlogic

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...

HarelM avatar Feb 01 '22 22:02 HarelM

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;<;);out meta;

Lelelo1 avatar Feb 02 '22 08:02 Lelelo1

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. :-)

HarelM avatar Feb 02 '22 09:02 HarelM

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 avatar Feb 02 '22 15:02 Lelelo1

@Lelelo1 You are amazing

jasonhjohnson avatar Feb 02 '22 18:02 jasonhjohnson

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 avatar Feb 03 '22 17:02 Lelelo1

What urls should be used?

Lelelo1 avatar Feb 06 '22 08:02 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

Lelelo1 avatar Feb 07 '22 17:02 Lelelo1

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

HarelM avatar Feb 07 '22 17:02 HarelM

@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.)

blackboxlogic avatar Feb 07 '22 18:02 blackboxlogic

Ok I will make a pr then.

I am having problems creating test:

runTests

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)

Lelelo1 avatar Feb 08 '22 08:02 Lelelo1

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

HarelM avatar Feb 08 '22 10:02 HarelM

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?

Lelelo1 avatar Feb 08 '22 16:02 Lelelo1

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).

blackboxlogic avatar Feb 08 '22 18:02 blackboxlogic

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

HarelM avatar Feb 08 '22 19:02 HarelM

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

blackboxlogic avatar Feb 08 '22 20:02 blackboxlogic

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

HarelM avatar Feb 08 '22 20:02 HarelM

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

Lelelo1 avatar Feb 09 '22 08:02 Lelelo1

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.

HarelM avatar Feb 09 '22 09:02 HarelM

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 avatar Feb 09 '22 10:02 Lelelo1

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

Lelelo1 avatar Feb 10 '22 09:02 Lelelo1

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.

HarelM avatar Feb 10 '22 09:02 HarelM

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

Lelelo1 avatar Feb 10 '22 09:02 Lelelo1

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 avatar Feb 10 '22 09:02 HarelM

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

Lelelo1 avatar Feb 10 '22 10:02 Lelelo1

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

HarelM avatar Feb 10 '22 11:02 HarelM

It can be closed

Lelelo1 avatar Feb 10 '22 12:02 Lelelo1