Skip to content

JSON support for /changeset/:id/download #5970

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.

Sign up for GitHub

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

Jump to bottom
Open
k-yle opened this issue Apr 30, 2025 · 5 comments · May be fixed by #5973
Open

JSON support for /changeset/:id/download #5970

k-yle opened this issue Apr 30, 2025 · 5 comments · May be fixed by #5973

Comments

@k-yle
Copy link
Contributor

k-yle commented Apr 30, 2025

Problem

  • As a developer of web-based tools,
  • I want to avoid bundling a massive xml-parsing library just for a single API endpoint (since almost all others support JSON now)
  • so that the page loads faster, we avoid unnecessary library, avoid boilerplate code, etc. etc.

Description

Over the years, JSON support has gradually been added to most APIs.

Uploading & downloading changesets are one of the few APIs that are still XML-only.

I imagine we could just create /changeset/:id/download.json to match /changeset/:id/download. The JSON format would be the same as what zerebubuth/openstreetmap-cgimap#407 is proposing1

Screenshots

No response

Footnotes

  1. specially, a single array with "action": "create" | "modify" | "delete" instead of the 3 arrays for create, modify, delete which is more similar to the current XML format. ↩
@k-yle k-yle mentioned this issue Apr 30, 2025
Closed
@AntonKhorev
Copy link
Collaborator

Browsers can parse both xml and json natively.

@k-yle
Copy link
Contributor Author

k-yle commented Apr 30, 2025

true, but it doesn't work in nodejs, so you can't reuse the same code in the browser and node.

It's also extremely cumbersome to parse and generate xml, and you lose all the typesafety benefits from the JS/TS ecosystem

@mmd-osm
Copy link
Contributor

mmd-osm commented Apr 30, 2025 •
edited
Loading

Changeset download in XML format is currently being served by https://github.com/zerebubuth/openstreetmap-cgimap. That's where the productive implementation for JSON format would have to go as well.

It doesn't mean that this issue isn't necessary. It's still required for the reference implementation.

@k-yle
Copy link
Contributor Author

k-yle commented Apr 30, 2025

would it make sense to implement it here first, and then port it to cgimap? I assume that the idealistic outcome is an identical implementation in ruby and c++

@mmd-osm
Copy link
Contributor

mmd-osm commented May 1, 2025 •
edited
Loading

I would start in this repo with a json version of app/views/api/changesets/downloads/show.xml.builder and additional render format in app/controllers/api/changesets/downloads_controller.rb, then add some test cases.

@k-yle k-yle linked a pull request May 1, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement JSON support for the changeset download API k-yle/openstreetmap-website
4 participants
@AntonKhorev @mmd-osm @k-yle and others