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

Use JSON for OSM API instead of XML #3765

Closed
bhousel opened this issue Jan 16, 2017 · 19 comments
Closed

Use JSON for OSM API instead of XML #3765

bhousel opened this issue Jan 16, 2017 · 19 comments
Labels
performance Optimizing for speed and efficiency
Milestone

Comments

@bhousel
Copy link
Member

bhousel commented Jan 16, 2017

We could simplifiy osm.js by using the JSON calls instead of the XML ones, which we need to parse with parser functions.

@pnorman @zerebubuth is there anything in here we need to watch out for, or are all of the API calls available over JSON nowadays?

@bhousel bhousel added the performance Optimizing for speed and efficiency label Jan 16, 2017
@pnorman
Copy link
Contributor

pnorman commented Jan 16, 2017

The OSM API is XML, not JSON. Some calls have a JSON endpoint, generally those which don't return OSM data.

@zerebubuth
Copy link

Unfortunately, it's as @pnorman says. I'd love to ship a full JSON API, and I've been working away at it (very slowly - sorry!) over at https://github.com/zerebubuth/openstreetmap-cgimap. It already supports JSON as an output format, so it's "just" a matter of getting full API coverage written, tested and debugged, then we can enable JSON by default. If anyone's interested in helping out, that would be greatly appreciated.

@bhousel bhousel added the waitfor-upstream Waiting for something in an upstream project label Jan 17, 2017
@bhousel
Copy link
Member Author

bhousel commented Jan 17, 2017

Thanks for the info @pnorman & @zerebubuth and for your work on this 👍 There is no rush, we were just looking into starting work on an OSM Notes layer this week and noticed some JSON support.

@mmd-osm
Copy link
Contributor

mmd-osm commented Oct 27, 2018

I'm continuing our discussion in #5180 here, as JSON output per se doesn't really have anything to do with Web Workers.

Latest status was that cgimap 0.6.2 is deployed on the dev instances, and provides OSM output in Overpass API JSON format:

You can try out some JSON output on the dev server for now: https://master.apis.dev.openstreetmap.org/api/0.6/map.json?bbox=-80.28218507766725,25.773537461179274,-80.24941921234132,25.782676914766654

@bhousel
Copy link
Member Author

bhousel commented Oct 27, 2018

You can try out some JSON output on the dev server for now: https://upload.apis.dev.openstreetmap.org/api/0.6/map.json?bbox=-80.28218507766725,25.773537461179274,-80.24941921234132,25.782676914766654

Nice! I just clicked on that to take a look..
Can the output be minified?
Also I thought it was weird that my browser downloaded a map.osm instead of a map.json. Maybe the mime type is not being set correctly.. I'll dig in more later.

@mmd-osm
Copy link
Contributor

mmd-osm commented Oct 27, 2018

(Json is minified now)

Indeed, map.osm should be better called map.json. It's just a hard-coded string today, which gets set no matter what the mime type is.

@bhousel
Copy link
Member Author

bhousel commented Oct 27, 2018

Technically speaking, it's pretty much a one line change, but there was some concern if that's really worthwhile or if that would break some clients. I'm really undecided here

Just do it - any reduction of data sent over the wire is a good thing. It's also probably even slower for whatever library you're using to pretty format the json.

@mmd-osm
Copy link
Contributor

mmd-osm commented May 2, 2019

Just following up on our previous discussion...

I did a few measurements to get some idea of XML vs. JSON parsing performance.

Data and JSON endpoint: http://api06.dev.openstreetmap.org/

http://localhost:8080/#background=Bing&disable_features=boundaries&map=17.83/49.04575/2.02767

Navigated to http://localhost:8080/
VM1153 iD.js:56307 Call to parseJSON took 18.90000000003056 milliseconds.
VM1153 iD.js:56307 Call to parseJSON took 250.60000000030414 milliseconds.
VM1153 iD.js:56307 Call to parseJSON took 281.800000000203 milliseconds.
VM1153 iD.js:56307 Call to parseJSON took 301.50000000003274 milliseconds.

Navigated to http://localhost:8080/
VM1179 iD.js:56459 Call to parseXML took 532.0999999999003 milliseconds.
VM1179 iD.js:56459 Call to parseXML took 463.59999999958745 milliseconds.
VM1179 iD.js:56459 Call to parseXML took 459.39999999973224 milliseconds.
VM1179 iD.js:56459 Call to parseXML took 491.8999999999869 milliseconds.

Navigated to http://localhost:8080/
osm.js:264 Call to parseJSON took 14.599999999973079 milliseconds.
osm.js:264 Call to parseJSON took 182.99999999999272 milliseconds.
osm.js:264 Call to parseJSON took 201.5000000001237 milliseconds.
osm.js:264 Call to parseJSON took 111.00000000033106 milliseconds.

I uploaded larger parts of Halifax to the dev server, so this might also be a good location for testing: http://localhost:8080/#background=Bing&disable_features=boundaries&map=18.36/44.65918/-63.59597

My previous assumption that JSON feels faster seems to be backed by those numbers. I added some code below in case someone wants to try this out.


Prototype coding: jsonparser_prototype.txt

NB: Most likely that patch won't apply cleanly, as I manually removed some (unrelated) parts.

@mmd-osm
Copy link
Contributor

mmd-osm commented May 2, 2019

@gravitystorm: my measurements seem to indicate, having native JSON support in the API would bring some noticeable parsing speed ups in iD, immediately impacting UX.

As we already have JSON support available in cgimap (based on the commonly used Overpass API JSON format as per openstreetmap/operations#244), the question has come up, what steps would be required to add similar support for the Rails port as well.

Could you open an issue on the OSM website repo where we can track all required activities, and possibly dispatch this to multiple devs in case this makes sense?

As Rails noob I think I'm not the right person to drive this forward in a meaningful way.

@gravitystorm
Copy link

Could you open an issue on the OSM website repo where we can track all required activities

Sure, I can do that.

and possibly dispatch this to multiple devs in case this makes sense?

I'm not sure that we have all that many devs to split the work yet!

@bhousel
Copy link
Member Author

bhousel commented May 3, 2019

I'm not sure that we have all that many devs to split the work yet!

Tag me, maybe I'll JFDI.

@mmd-osm
Copy link
Contributor

mmd-osm commented May 4, 2019

I'd say, most of the work is to move all that XML generation code from the controller to the view where it belongs. Once that bit is done, we could eventually add some JSON related stuff to the view as well and teach teh controller to render either XML or JSON.

If I'm not completely mistaken, @woodpeck already started some work to move XML to the views, maybe as part of GDPR (see https://wiki.osmfoundation.org/wiki/Board/Minutes/2019-04-24). I have no idea what his overall plans and timelines are. After all, there may be a few people we could ping to get JSON support in Rails.

@woodpeck
Copy link

woodpeck commented May 4, 2019

Yes, I started with openstreetmap/openstreetmap-website#2164 and now there's the pending openstreetmap/openstreetmap-website#2207, I was planning to work my way through the stuff from easiest to hardest ;) I cannot devote too much time on it and I'm not a Rails ace to begin with, hence the snail's pace. Also most of the tests currently use the to_xml methods in the models and when XML generation is moved to views, the tests need to receive their own "build an XML thingie for this" methods. Sadly if you want to add JSON you'll probably have to do the same again, i.e. not only generate JSON in the views, but also have your own JSON-generating code in the tests (because it's difficult to co-opt the views from inside the tests, and also bad design to use parts of the code we're testing in building the test case).

It is true that my motivation for separating output generation from the models was that eventually I want to be able to restrict parts of the output from showing to anonymous users.

If someone wants to help with this or, better still, take it off my plate, I certainly won't object. OSMF would also be willing to pay for (at the least the GDPR related part of) this if a suitable contractor can be found. Nobody has come forward in response to https://blog.openstreetmap.org/2018/11/03/rfq-changes_to_rails_api_and_cgimap/ yet.

@mmd-osm
Copy link
Contributor

mmd-osm commented May 5, 2019

Ok, I gave it a try and added a prototype for the map, nodes, ways and the relations endpoint, both producing XML and JSON (Overpass API style) output: mmd-osm/openstreetmap-website@913dd07

Those should be the basic building blocks of what we need for any of the other related calls. Test cases are also deliberately left out. Same applies to user_display_name_cache and changeset_cache

@woodpeck
Copy link

woodpeck commented May 5, 2019

Hm yes, if you only want JSON as an output format and not for input then it might not be all that bad with the tests.

@mmd-osm
Copy link
Contributor

mmd-osm commented May 5, 2019

About the only reason why I'm looking into this is JSON deployment as part of CGImap 0.6.2, which faced so much opposition, that I had to cancel it altogether (see openstreetmap/operations#244 (comment)). I can fully understand those concerns, but as we're so extremely limited in available devs on the Rails side, this increasingly starts impacting other parts. Anyway, let's don't look back, get this thing done and move on!

@mmd-osm
Copy link
Contributor

mmd-osm commented Dec 30, 2019

Follow ups: openstreetmap/openstreetmap-website#2485, #7188

@mmd-osm
Copy link
Contributor

mmd-osm commented Sep 9, 2020

Json is in production, let’s close this issue.

@quincylvania
Copy link
Collaborator

@mmd-osm Thanks, this should have been closed months ago 😆

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

No branches or pull requests

7 participants