Objects should not be downloaded via Overpass API when reverting changesets

[mmd]

I have activated the setting "Use Overpass API to download objects", and try to revert changeset 54295156 (as an example). In this changeset, node 5245392367 is deleted (​https://osmlab.github.io/osm-deep-history/#/node/5245392367).

Unfortunately, the reverted plugin tries to download objects from Overpass API without providing a version number (which isn't possible anyway at this time), nor with a given timestamp for a [date: ....] setting. This results in subsequent error messages, as the node in question cannot be downloaded from Overpass API (it is already deleted).

My recommendation at this time would be to completely ignore the "Download objects via Overpass API" flag when using the reverter, and instead download the respective object version from the Main API. The semantics of downloading objects just don't fit all that well to the Overpass query currently in use.

017-12-03 12:51:01.648 INFORMATION: Defaults for draw.rawgps.layer.audiowpt.pattern differ: ?{ '{name}' | '{desc}' | '{formattedWaypointOffset}' } != 
2017-12-03 12:51:13.138 INFORMATION: GET https://api.openstreetmap.org/api/0.6/changeset/54295156 -> 200
2017-12-03 12:51:13.277 INFORMATION: GET https://api.openstreetmap.org/api/0.6/changeset/54295156/download -> 200
2017-12-03 12:51:13.733 INFORMATION: GET http://lz4.overpass-api.de/api/interpreter?data=%28node%285245392367%29%3B%3E%3B%29%3Bout+meta%3B -> 200 (231 B)
2017-12-03 12:51:13.892 INFORMATION: GET https://api.openstreetmap.org/api/0.6/node/5245392367/1 -> 200 (425 B)
2017-12-03 12:51:14.205 INFORMATION: GET http://lz4.overpass-api.de/api/interpreter?data=%28node%285245392367%29%3B%3E%3B%29%3Bout+meta%3B -> 200 (231 B)
2017-12-03 12:51:14.263 SCHWERWIEGEND: Handled by bug report queue: java.lang.IllegalStateException: Beim Zusammenführen wurde das Ziel-Objekt vom Type Punkt für das Objekt mit der Kennung 5.245.392.367 nicht gefunden
java.lang.IllegalStateException: Beim Zusammenführen wurde das Ziel-Objekt vom Type Punkt für das Objekt mit der Kennung 5.245.392.367 nicht gefunden
	at reverter.DataSetCommandMerger.getMergeTarget(DataSetCommandMerger.java:62)
	at reverter.DataSetCommandMerger.mergeNode(DataSetCommandMerger.java:82)
	at reverter.DataSetCommandMerger.merge(DataSetCommandMerger.java:173)
	at reverter.DataSetCommandMerger.<init>(DataSetCommandMerger.java:46)
	at reverter.ChangesetReverter.getCommands(ChangesetReverter.java:346)
	at reverter.RevertChangesetTask.realRun(RevertChangesetTask.java:113)
	at org.openstreetmap.josm.gui.PleaseWaitRunnable.doRealRun(PleaseWaitRunnable.java:95)
	at org.openstreetmap.josm.gui.PleaseWaitRunnable.run(PleaseWaitRunnable.java:143)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)

[GerdP]

I'd prefer to remove the server option "Use Overpass API to download objects". I think it is likely to confuse other plugins as well, e.g. the undelete plugin. My understanding is that the overpass api is always a bit behind the normal OSM DB, so all commands which require current data might not work properly.
@simon04: If I got that right this was introduced with r9241 for #7670.

[Upliner]

I don't think this option should be removed. Overpass should be used, but of course we need a fallback to regular API when neccesary versions are missing in overpass.

[GerdP]

Ok, If that fallback is implemented in the core methods that should work.

[GerdP]

I've looked at this again. The overpass api simply returns no information for objects with the visible=no status. I assume the same happens for redacted objects. I see no simple way to implement a fall back unless we treat both the same.
So, maybe we should change core method org.openstreetmap.josm.io.MultiFetchServerObjectReader.create(boolean fromMirror)
to public and use it with false in reverter?

[GerdP]

I also think that the automatic "recurse down" of overpass is not what we want in reverter.

[skyper]

The initial report seems to be a duplicate of #13981 and is not a problem only with overpass but either a general connection problem or an error inside the code.

[Don-vip]

Ticket #18268 has been marked as a duplicate of this ticket.

[GerdP]

What do I have to do with the @since when I change the visibility of MultiFetchServerObjectReader create()to public?

[Don-vip]

There is no strict rule, you can replace it, or add a new @since xxx (changed visibility), as you prefer

[GerdP]

In 15520/josm:

see #15626: change visibility of MultiFetchServerObjectReader.create() to public so that plugins can use it

[GerdP]

fixed with [o35226:35227]: reverter now uses MultiFetchServerObjectReader.create(false) so that it doesn't use overpass.