Modify

Opened 5 years ago

Closed 4 years 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 5 years ago.
18122-v1.patch (6.9 KB ) - added by GerdP 5 years ago.

Download all attachments as: .zip

Change History (15)

by GerdP, 5 years ago

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

comment:1 by GerdP, 5 years ago

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.

by GerdP, 5 years ago

Attachment: 18122-v1.patch added

comment:2 by GerdP, 5 years ago

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 by Don-vip, 5 years ago

Keywords: overpass download bounds incomplete added

in reply to:  2 comment:4 by skyper, 5 years ago

Duplicate of #17857 ?

Last edited 5 years ago by skyper (previous) (diff)

comment:5 by Don-vip, 5 years ago

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

comment:6 by Don-vip, 5 years ago

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

comment:7 by Don-vip, 5 years ago

Milestone: 19.0919.10

comment:8 by GerdP, 4 years ago

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

comment:9 by Don-vip, 4 years ago

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 by GerdP, 4 years ago

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 by Don-vip, 4 years ago

No problem, keep it simple then.

comment:12 by GerdP, 4 years ago

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 by Don-vip, 4 years ago

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. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.