Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Other Endpoints (Overpass-Turbo, TagInfo) #23

Open
blackboxlogic opened this issue Jun 13, 2020 · 37 comments
Open

Support Other Endpoints (Overpass-Turbo, TagInfo) #23

blackboxlogic opened this issue Jun 13, 2020 · 37 comments

Comments

@blackboxlogic
Copy link
Owner

blackboxlogic commented Jun 13, 2020

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.

@HarelM
Copy link
Collaborator

HarelM commented Jun 14, 2020

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
Copy link
Collaborator

HarelM commented Jun 14, 2020

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

@blackboxlogic blackboxlogic changed the title Support Overpass-Turbo Support Other Endpoints (Overpass-Turbo, TagInfo) Jun 16, 2020
@Lelelo1
Copy link

Lelelo1 commented Jan 30, 2022

I am interested in having overpass

@HarelM
Copy link
Collaborator

HarelM commented Jan 30, 2022

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
Copy link

Lelelo1 commented Jan 30, 2022

Great thanks. I will experiment a bit with OSMImports

@Lelelo1
Copy link

Lelelo1 commented Jan 30, 2022

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
Copy link
Collaborator

HarelM commented Jan 30, 2022

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

@jasonhjohnson
Copy link

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

@HarelM
Copy link
Collaborator

HarelM commented Feb 1, 2022

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
Copy link
Owner Author

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
Copy link
Collaborator

HarelM commented Feb 1, 2022

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
Copy link

Lelelo1 commented Feb 2, 2022

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;

@HarelM
Copy link
Collaborator

HarelM commented Feb 2, 2022

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

@Lelelo1
Copy link

Lelelo1 commented Feb 2, 2022

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";

@jasonhjohnson
Copy link

@Lelelo1 You are amazing

@Lelelo1
Copy link

Lelelo1 commented Feb 3, 2022

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
Copy link

Lelelo1 commented Feb 6, 2022

What urls should be used?

@Lelelo1
Copy link

Lelelo1 commented Feb 7, 2022

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
Copy link
Collaborator

HarelM commented Feb 7, 2022

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
Copy link
Owner Author

blackboxlogic commented Feb 7, 2022

@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
Copy link

Lelelo1 commented Feb 8, 2022

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)

@HarelM
Copy link
Collaborator

HarelM commented Feb 8, 2022

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
Copy link

Lelelo1 commented Feb 8, 2022

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
Copy link
Owner Author

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
Copy link
Collaborator

HarelM commented Feb 8, 2022

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

@blackboxlogic
Copy link
Owner Author

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

@HarelM
Copy link
Collaborator

HarelM commented Feb 8, 2022

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

@Lelelo1
Copy link

Lelelo1 commented Feb 9, 2022

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

@HarelM
Copy link
Collaborator

HarelM commented Feb 9, 2022

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
Copy link

Lelelo1 commented Feb 9, 2022

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
Copy link

Lelelo1 commented Feb 10, 2022

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

@HarelM
Copy link
Collaborator

HarelM commented Feb 10, 2022

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
Copy link

Lelelo1 commented Feb 10, 2022

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

@HarelM
Copy link
Collaborator

HarelM commented Feb 10, 2022

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.

@Lelelo1
Copy link

Lelelo1 commented Feb 10, 2022

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

@HarelM
Copy link
Collaborator

HarelM commented Feb 10, 2022

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

@Lelelo1
Copy link

Lelelo1 commented Feb 10, 2022

It can be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants