Modify

Opened 3 months ago

Closed 7 weeks ago

#18122 closed defect (fixed)

Don't set download bounds for incomplete Overpass queries

Reported by: stoecker Owned by: team
Priority: normal Milestone: 19.10
Component: Core Version:
Keywords: overpass download bounds incomplete Cc:

Description

Incomplete downloads should not set download bounds, as most functions in JOSM assume that data inside download bounds is complete.

See #18116

Attachments (2)

18122-quick-and-dirty.patch (4.0 KB) - added by GerdP 3 months ago.
18122-v1.patch (6.9 KB) - added by GerdP 3 months ago.

Download all attachments as: .zip

Change History (15)

Changed 3 months ago by GerdP

Attachment: 18122-quick-and-dirty.patch added

comment:1 Changed 3 months ago by GerdP

A few remarks:
1) I think one should remember that the normal OSM API will not return ways crossing the bounds unless at least one node of the way is inside the bounds.
2) We have two types of overpass queries:

  • Those which are intended to download partial data because they use filters like highway=*.
  • Those which are intended to download full data for the given bounds.

Unfortunately the download tasks only "sees" the final query which doesn't clearly say which type we have. A quick&dirty solution could parse the overpass query for e.g. "node({{bbox}})" to detect full downloads, but it would be better to store that information somewhere.
3) Downloads (also those from OSM API) might return empty datasets. The current code in DownloadOsmTask.finish() always adds bounds to the DataSet instance in this case. We have to avoid that for partial downloads.

The quick hack allows to experiment, but it will fail if someone executes an overpass query which contains the string "node({{bbox}})" in a comment and the change in DownloadOsmTask.finish() doesn't care if the download was partial or not.

Changed 3 months ago by GerdP

Attachment: 18122-v1.patch added

comment:2 Changed 3 months ago by GerdP

patch v1 might be enough to solve the problem. I see no need to add the "empty" query. Up to now we generated something like

[out:xml];
/*
Place your Overpass query below or generate one using the Overpass Turbo Query Wizard
*/
(
    node({{bbox}});
<;
);
(._;>;);out meta;

if the user query was empty or just a comment. I see no need to include that comment in the "repaired" query, instead a static String is used and I think it is clean to compare the query again this static string to detect a full query?

comment:3 Changed 3 months ago by Don-vip

Keywords: overpass download bounds incomplete added

comment:4 in reply to:  2 Changed 3 months ago by skyper

Duplicate of #17857 ?

Last edited 3 months ago by skyper (previous) (diff)

comment:5 Changed 3 months ago by Don-vip

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

comment:6 Changed 3 months ago by Don-vip

I've closed the older one because Gerd already started the analysis here.

comment:7 Changed 2 months ago by Don-vip

Milestone: 19.0919.10

comment:8 Changed 2 months ago by GerdP

Seems nobody has a better idea to solve this? A clean solution would probably require a lot more changes...

comment:9 Changed 2 months ago by Don-vip

I like the idea of detecting full map queries. But we should also detect manual coordinates. Like if someone copies this example instead of using {{bbox}}:

(
   node(51.249,7.148,51.251,7.152);
   <;
);
out meta;

Also we must be tolerant to additional space characters, comments, etc. So without a full parsing capability, we still need a regex.

comment:10 Changed 2 months ago by GerdP

Well, I started from Dirks "better safe than sorry" approach in ticket:18116#comment:12
My patch catches at least the usual case.
My knowledge about the overpass variants is small and I am also not good with parsers and regex :(, so don't expect something more complex from me.

comment:11 Changed 2 months ago by Don-vip

No problem, keep it simple then.

comment:12 Changed 2 months ago by GerdP

In 15447/josm:

see #18122: Don't set download bounds for incomplete Overpass queries
apply 18122-v1.patch so that Overpass queries don't set download bounds unless they are detected to return a complete result. For now, this is only detected when the user query is empty or just a comment.
TODO: This detection should also detect user written queries which return complete data

comment:13 Changed 7 weeks ago by Don-vip

Resolution: fixed
Status: newclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.