zerebubuth/openstreetmap-cgimap

wrong value comments_count reported by API

Closed this issue · 8 comments

The response from API https://api.openstreetmap.org/api/0.6/changeset/79214306
says comments_count="0" while https://api.openstreetmap.org/api/0.6/changesets?changesets=79214306
correctly says comments_count="1"

See also https://josm.openstreetmap.de/ticket/23642#comment:2 which shows the responses.

Probably to avoid caching unwanted data. The call is only executed to fill the list of changesets. Do you suggest to change JOSM code instead of fixing the wrong API result?

Thank you for reporting this issue. Indeed, this looks like a regression in 0.9.1. comments_count should be 1 in both cases.

By the way, changesets?changesets=79214306 is implemented on Rails only, that's why the changes in #388 have no impact on this endpoint.

Link to JOSM discussion: https://josm.openstreetmap.de/ticket/23642

@tomhughes : one option I see here is to route the /changeset endpoint to Rails until we have fixed the bug on CGImap side: https://github.com/openstreetmap/chef/blob/16da621fbfec07c850ffc210e300556db3f8d4b2/cookbooks/dev/templates/default/apache.rails.erb#L46

This depends a bit how severe the issue is for JOSM. It has been created with "normal" priority, so we might as well wait until we have the fix available.

FIx should be available on the dev instance in the next 30 mins. Example: https://master.apis.dev.openstreetmap.org/api/0.6/changeset/299208 vs. https://master.apis.dev.openstreetmap.org/api/0.6/changeset/299208?include_discussion

New release 0.9.2 based on 0.9.1 + cherry picking c9c222c

New build is in progress: https://launchpad.net/~mmd-osm/+archive/ubuntu/cgimap-902-jammy
Operations issue is linked below. Closing here.

Probably to avoid caching unwanted data. The call is only executed to fill the list of changesets. Do you suggest to change JOSM code instead of fixing the wrong API result?

No? Obviously the API should be fixed. I assumed the third request for the single changeset details as listed in the josm issue is triggered by clicking on the button to show the changeset comments. That's why it seemed strange that it didn't set the include_discussion parameter, considering fetching the comments was the main purpose.

https://josm.openstreetmap.de/ticket/23642#comment:1

2024-04-25 09:00:38.390 INFO: GET https://api.openstreetmap.org/api/0.6/way/37407704/history -> HTTP/1.1 200 (175 ms)
2024-04-25 09:00:38.541 INFO: GET https://api.openstreetmap.org/api/0.6/changesets?changesets=1770190,16412412,79214306 -> HTTP/1.1 200 (136 ms; 636 B)
2024-04-25 09:00:40.660 INFO: GET https://api.openstreetmap.org/api/0.6/changeset/79214306 -> HTTP/1.1 200 (38 ms)

I haven't actually been able to reproduce the josm issue. I always get two GET requests for the changeset, one without the parameter and then one with the parameter right after.

2024-04-25 23:56:01.446 INFO: GET https://api.openstreetmap.org/api/0.6/way/37407704/history -> HTTP_2 200 (36 ms)
2024-04-25 23:56:01.515 INFO: GET https://api.openstreetmap.org/api/0.6/changesets?changesets=1770190,16412412,79214306 -> HTTP_2 200 (62 ms; 636 B)
2024-04-25 23:56:05.335 INFO: GET https://api.openstreetmap.org/api/0.6/changeset/79214306 -> HTTP_2 200 (43 ms)
2024-04-25 23:56:05.482 INFO: GET https://api.openstreetmap.org/api/0.6/changeset/79214306?include_discussion=true -> HTTP_2 200 (32 ms)

Edit: Fix is already deployed, so that's why.

Yes, I also wondered why JOSM sends two requests, The patch 23642-2.patch which I attached to the JOSM ticket solves this:

2024-04-26 06:55:50.890 INFO: GET https://api.openstreetmap.org/api/0.6/way/37407704/history -> HTTP/1.1 200 (106 ms)
2024-04-26 06:55:51.034 INFO: GET https://api.openstreetmap.org/api/0.6/changesets?changesets=1770190,16412412,79214306 -> HTTP/1.1 200 (132 ms; 636 B)
2024-04-26 06:55:53.141 INFO: GET https://api.openstreetmap.org/api/0.6/changeset/79214306?include_discussion=true -> HTTP/1.1 200 (37 ms)

Never used it before but there is a side window "Changesets" which shows the changesets for the downloaded data. For this scenario 23642-2.patch would possibly download more data than before.