#4142 closed defect (fixed)
[PATCH] JOSM does not query API for referring relations when downloading primitives
Reported by: | Nakor | Owned by: | team |
---|---|---|---|
Priority: | critical | Milestone: | 24.05 |
Component: | Core | Version: | latest |
Keywords: | Cc: |
Description (last modified by )
Download data using the following bounding box: http://www.openstreetmap.org/?lat=42.65924&lon=-83.3199&zoom=17
Download all members from the relation
Search for way 43418202. It is shown only as a member of relation 308,352 but if it is also member of another relation (see http://www.openstreetmap.org/browse/way/43418202).
This would cause inconsistencies if the way is split.
Attachments (5)
Change History (77)
comment:1 by , 15 years ago
Summary: | Incomplete data when downloading all memebers of a relation → JOSM does not query API for referring relations when downloading primitives |
---|
comment:6 by , 14 years ago
Priority: | major → critical |
---|
comment:7 by , 13 years ago
Description: | modified (diff) |
---|
Any update on this critically prioritized ticket?
comment:8 by , 12 years ago
Maybe a warning with an option to download parents before splitting could help as these ways are outside of the download area.
comment:9 by , 10 years ago
Learn CTRL+ALT+D before splitting or merging nodes !
Or place the icon for the "Download referers" tool just beside the two icons on the toolbar for merging or splitting ways (use the preferences panel to add it to your toolbar).
This "Download referers" tool should be on the status bar by default !
comment:10 by , 10 years ago
Note: not just referers of ways, but also referers of nodes should be downloaded to avoid conflicts.
But more critically, referers of relations can also include other relations, and this could recurse at deep levels;
- However siblings found as members of referer relations do not need to be downloaded: only the fact that they are members of a known relation is enough. These members will also be known with a role in the referer relation, though the role is not strictly needed, we just need the member ID and type (node/way/relation).
- Recursion will only occur if the referer relation is not alread loaded (at least partially). There should be an API that can recurse over parent relations but loading them with only their IDs (we know theu can only be relations). These relations would be loaded in "incomplete" state, without any of its tags and with partial lists of members with unknown roles (and in the partial list of members, not all of them would be loaded).
This would speed up the download, and would also save memory... provided that JOSM can accept such object state for relations. That "incomplete" relation is NOT locally modifiable (not even just to set new tags) as long as it has not been correctly loaded with its tags and the fll ist of members (but not necessarily their roles). We would continue to use "donwload incomplete" to completely load tags of the relation, compeltely load roles of existing members (with current "<unknown>" role, and get the full list of members as references (but not necessarily their own data, i.e. their own tags or geometry or submembers).
comment:11 by , 8 years ago
Note that frequently, the server cannot return all dependencies for some objects, notably when selecting ways that are part of several/many large relations:
The query fails (visible on the console with an HTTP error status), even when retrying it. Apparently the server has limited ressources for doing it.
It is probably not a bug of JOSM, but of the server trying to perform too costly requests (trying to build some costly temporary index for joining results: incorrect execution plan, or existing index estimated to be insufficiently selective). But...
Workaround: select a node one that way, zoom on it to view a very small area around it (to avoid loading too many unnecessary surrounding objects), and then load the tiny area around this node : it works instantly !
For doing the same thing on relations, you need to recurse down to get at least one node (this may require loading members if the relation is incomplete), and then use that node to perform the download by area.
Suggestion: possibly, JOSM could detect that failure and retry by downloading automatically with a query by area, using such tiny rectangle (it can be as small as a few centimeters around that node).
comment:12 by , 7 years ago
This or a variant of it (such as for when an Overpass API query only downloads certain objects and not all objects in an area, but it should to avoid editing conflicts when that object is later uploaded) may now be possible as of Overpass API 0.7.55:
https://wiki.openstreetmap.org/wiki/Overpass_API/Overpass_QL#The_block_statement_complete
It should probably be used both when downloading objects in a bounding box (by default), and at least be allowed as a user-selectable option when downloading objects through Overpass API (e.g. '[x] Also download object dependencies', enabled by default (See https://wiki.openstreetmap.org/wiki/Overpass_API/Sparse_Editing for the use case of that).
comment:14 by , 4 years ago
We have the same problem when downloading relations: A relation can have parent relations and can therefore be 'incomplete' by itself.
This is especially a problem for route relations (=> route master) or multipolygons (=> their corresponding route, stop_area, … relations)
Instead of trying to download all those relations and recurse into them, I would suggest to have a 'incompleteReferrers' flag. This flag can then be unset when JOSM is sure to have downloaded all referrers of that node/way/relation. That way, we can show a warning when modifying this incomplete data.
comment:16 by , 4 years ago
Replying to GerdP:
See also #18959
Thanks for the hint. This is why I could not find a open ticket for it.
Well, since JOSM does not provide a way to indicate that referrers were downloaded, it seems like I need to do the tracking in the PT plugin on my own then. Because for PT routes, the information if parent relations are downloaded is important (e.g. I don't want to present the user with a 'add to route master' or 'create route area group' dialog if that primitive is already inside one. Otherwise, we will have tons of duplicate relations - I already found those on several stops.
comment:17 by , 4 years ago
The PT-Plugin validator rules need an update, too. At least for nodes and ways it should only warn about new objects or within downloaded area, see #15038 or PT: Stop_position is not part of a way
for example.
comment:18 by , 12 months ago
My opinion is that this is not a bug. If you want to download the rest of the relation's members, do so by right-clicking on the relation or when the dialog box tells you to.
comment:19 by , 12 months ago
Yes, and I think a 14 year old ticket about an API call can be closed anyway. It's not even clear to me what problem this ticket is about.
comment:20 by , 12 months ago
It is not about an API call, but about a user interaction problem:
If you download a street / node / relation, JOSM won't neccessarely download parent relations. And won't know if any parent relations exist.
So the actual problem which seems to regularely destroy relations in actual OSM data (observed by me in public transport):
Download a way that is part of a PT route by other means than "Download Area", split the way, then the relation will only be on the old way and not on the newly created one.
follow-up: 22 comment:21 by , 12 months ago
Yes, I've made this mistake as well a few times when I downloaded only highways via overpass api. I think we have many newer tickets about this issue?
I think it would be good to warn at least when the nodes of ways with a positive id are changed and there is no download area.
comment:22 by , 12 months ago
Replying to GerdP:
Yes, I've made this mistake as well a few times when I downloaded only highways via overpass api. I think we have many newer tickets about this issue?
I think it would be good to warn at least when the nodes of ways with a positive id are changed and there is no download area.
This would cause many false-positives and not provide a "solution" to users.
My proposal in #18959 was:
- Store if all parent relations were downloaded as flag on the node/way/relation.
- We set this flag if a node/way was downloaded by "Download area" or if parent relations were downloaded automatically
That way, we can warn that the way does not have all parent relations downloaded and directly provide a solution to the user: Download those (=> then flag is set) and retry the operation. That way, we do not only provide a warnig but an automated solution and it will probably be not much harder than checking if a way is in a downloaded area (which is not that trivial with multiple areas, the way ways that extend over the download area are handled, ...)
follow-up: 24 comment:23 by , 12 months ago
This would cause many false-positives
Why? In what situation?
comment:24 by , 12 months ago
Replying to GerdP:
This would cause many false-positives
Why? In what situation?
I don't think the typical case where we need to warn is for people editing just an area. They will download the area and then edit it.
One use case for splitting ways with relations on them without just downloading an area is public transport. In this case, often areas are not downloaded, but instead one downloads only "Relation members". This is also the case described in this ticket. Imageine bus line 1 and 2 share a lot of the way. Typical workflow:
- Download members of relation "Bus Line 1"
- Then download referenced relations of that street
- Then split the street, e.g. for inserting an end stop for one of the bus lines.
In this case, you would get a warning, wether you forgot to do step2 or not. Therefore, the warning won't help there. And those are the critical cases, where data editing is so "complicated" that such errors happen easily.
follow-up: 26 comment:25 by , 12 months ago
I would still prefer that JOSM recommends to download an area around the split node.
My work case was this:
- Download all major highways in an area to set zone:traffic.
- Note that some ways have a node with traffic_sign=city_limit which is not an end node but very close to that.
- Merge the two nodes (without downloading relations first)
I ended up with many small gaps in route relations because I simply forgot that JOSM won't warn in that situation.
comment:26 by , 12 months ago
Replying to GerdP:
I would still prefer that JOSM recommends to download an area around the split node.
This sould be: Around the original position of the split node (in case the node was moved). And then this won't work if the way was already modified (e.g. merging the node as you mentioned: If the node is not part of the way on the server, downloading the area won't download the way)
And that's why I meant that adding a flag on the way (like "we are sure all relations are loaded") is much easier than handling all the corner cases for downloading the area.
follow-up: 28 comment:27 by , 12 months ago
Why not add a flag to the DataSource? If a way is not inside fully downloaded bounds, ask the user if we can get referring objects. We could add a flag to the way (and that is arguably the right place to put the flag), but that would increase the memory size of the IPrimitive hierarchy. For reference, AbstractPrimitive
has a short flags
field instead of boolean
s.
With that said, now I'm wondering if the short flags
still saves memory. I assume it did once, but the JVM has advanced significantly since at least 2011 (when the short flags
got moved from OsmPrimitive
to AbstractPrimitive
). Time for some heap dumps...
EDIT: Using short bits is still more space efficient. Tested with sixteen boolean
fields in a class (both record
and traditional
), a class with a boolean[]
field, and a class with a short
field. 20k instances of the boolean
field class was 640kb. 20k instances of the short
field instance was 320kb. The boolean[]
field array class was the worst at 960kb. All values were randomly generated to avoid any efficiency shortcuts from the JVM.
Just for the heck of it, I also took a look at JEP 401 value objects. I don't know what is going on, but (as of the Valhalla JDK 20 EA build), it was not as efficient as the short
field (it was as efficient as the more standard classes). This should probably be double-checked using real data in production once we move to a JVM with value classes.
comment:28 by , 12 months ago
Replying to taylor.smock:
Why not add a flag to the DataSource? If a way is not inside fully downloaded bounds, ask the user if we can get referring objects. We could add a flag to the way (and that is arguably the right place to put the flag), but that would increase the memory size of the IPrimitive hierarchy. For reference,
AbstractPrimitive
has ashort flags
field instead ofboolean
s.
As I stated: The current position of the node and the current ways that belong to the node may have changed after download. And therefore the "inside fully download bounds" check does not reliabely indicate if the node was fully downloaded.
Yes, I mean such a flag on the AbstractPrimitive. I would make it universal: put it on every primitive. Set it when downloaded using "download area" and it was in the orignal bounds. Set it when related objects were downloaded. Read it during merge ways.
Name: "FLAG_PARENTS_COMPLETE". Which means: All ways (for nodes) + relations this primitive is a member of are fetched.
With that said, now I'm wondering if the
short flags
still saves memory. I assume it did once, but the JVM has advanced significantly since at least 2011 (when theshort flags
got moved fromOsmPrimitive
toAbstractPrimitive
). Time for some heap dumps...
EDIT: Using short bits is still more space efficient. Tested with sixteen
boolean
fields in a class (bothrecord
andtraditional
), a class with aboolean[]
field, and a class with ashort
field. 20k instances of theboolean
field class was 640kb. 20k instances of theshort
field instance was 320kb. Theboolean[]
field array class was the worst at 960kb. All values were randomly generated to avoid any efficiency shortcuts from the JVM.
Just for the heck of it, I also took a look at JEP 401 value objects. I don't know what is going on, but (as of the Valhalla JDK 20 EA build), it was not as efficient as the
short
field (it was as efficient as the more standard classes). This should probably be double-checked using real data in production once we move to a JVM with value classes.
Object layout has not changed significantly. A boolean is still one byte and byte-aligned, e.g. for performance reasons and for some atomic write operations. Your example did not even get the full benefit, since short
s are still padded and that way your short took more than 16 bit ;-)
Memory layout of the AbstractPrimitive should be to my knowledge (have not checked by docs...):
12 byte object header 2 bytes flags 2 bytes mappaintCacheIdx 8 byte id 4 byte user-reference (8 bytes for large heap) 4 byte changesetId 4 byte timestamp (potentially 4 byte padding, if no other fields are present in subclass)
comment:29 by , 12 months ago
comment:30 by , 7 months ago
I did run into this issue recently updating the missing maxspeed. By spare editing, querying only the ways without maxspeed and then splitting a way halfway where there is a traffic sign you break the route using that way. Yes, I know how to prevent it but this can be handled better.
Reading this topic I wonder why not just do a
GET /api/0.6/way/#id/relations
on the way before doing the split as suggested by comment 1. For a merge do two of these calls before the actual merge.
If what comment 11 indicates is (still) true use that suggested method.
comment:31 by , 7 months ago
That's easy to answer. JOSM isn't an online editor. By design it cannot do a download before an operation.
comment:32 by , 7 months ago
A design can be changed, if there is a good reason for it ;-)
There is also "continuous download"...
comment:33 by , 7 months ago
continuousDownload
is a plugin; it is not part of core (and I don't think it ever will be). We do support it (insofar as bug fixes only is considered "support").
Realistically, our only real option is what I investigated in comment:27; adding another bit to our bit field (which is used to decrease the memory usage with lots of primitives).
The problem is that we would have to increase the base memory size of OSM primitives (which is where the vast bulk of our static memory usage is going to be when large areas are loaded).
For reference, we currently use a short
for the bit field. A short
(per java specification) has 16 bits (2 bytes). The next smallest primitive is an int
with 32 bits (4 bytes).
For the "largest" download allowed by the OSM server (10k nodes, figure 5k ways and 5k relations for a total of 20k objects):
short
: 320,000 bits (or 0.305 Mib)int
: 640,000 bits (or 0.610 Mib).
This doesn't sound like much, until you realize that some workflows involve loading massive amounts of data. Think millions of objects. If at all possible, we don't want to significantly increase the memory usage of the OSM primitive classes.
follow-up: 36 comment:34 by , 7 months ago
Not a Java programmer or familiar with the josm internals but from comment 28 I understand what is needed is a new flag added to the AbstractPrimitive class.
I opened src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java and it looks to me one could add:
protected static final short FLAG_PARENTS_COMPLETE = 1 << 14;
And things can be implemented without a data size increase.
comment:35 by , 7 months ago
Right, I also see no memory problem with a new flag, we just have to check / modify lots of code to make sure that the flag is really used and updated. We probably need a routine to fake an empty download result for some existing unit tests.
follow-ups: 37 66 comment:36 by , 7 months ago
Replying to Emvee:
I opened src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java and it looks to me one could add:
[...]
And things can be implemented without a data size increase.
I could have sworn something was using bit 14 and 15 somewhere. I'm not finding it right now though.
The problem we have here is that AbstractPrimitive
and OsmPrimitive
are not sealed classes (sealed classes require Java 15+). Downstream consumers can extend either one of them. Even if they aren't supposed to (for OsmPrimitive
).
So adding another flag to AbstractPrimitive
won't happen, since extenders of that class may add their own arbitrary flags.
Realistically, we will want to put the additional flag in OsmPrimitive
and (maybe) PrimitiveData
, since those are supposed to be extended by JOSM core classes only, so there should be minimal breakage in plugins. I'd be happier if we could mark OsmPrimitive
and PrimitiveData
as sealed classes, but we can't until we drop support for Java 11, which I think is currently kept for Debian Bullseye (EOL in ~3 months) and Ubuntu 22.04 (EOL in ~3 years).
comment:37 by , 7 months ago
Replying to taylor.smock:
So adding another flag to
AbstractPrimitive
won't happen, since extenders of that class may add their own arbitrary flags.
Hmm. While we care a bit about plugins, there is no need to restrict the core for this. We have no fixed external API. Core needs are more important.
comment:38 by , 7 months ago
The problem is I think I saw one of our (core) subclasses using additional bits. But it might have been something else.
We can try adding it as a bit and see what happens (AKA: what does it break).
If so, now is a good time, since that will hopefully give josm-latest users a chance to tell us that something broke.
comment:39 by , 7 months ago
I really don't fear that some plugin adds its own meaning to bits in such a basic field. I think that would be very error prone.
comment:40 by , 7 months ago
When I set "flags" to private I get no error indicating any additional use.
Also as some flags are mutual exclusive we could create virtual flags, e.g. FLAG_INCOMPLETE|FLAG_MODIFIED could be a new flag or FLAG_VISIBLE|FLAG_DELETED. But I'd use such a mechanism only as last resort.
follow-up: 43 comment:41 by , 7 months ago
I think that would be very error prone.
The last time we added to the flags was back in r13309 (~5 years ago). Yes, you would think it would be very error prone, but the bits we have used have remained relatively stable, so it would "just work".
Of note, the javadocs (from 2011) for flags
indicates that "[other] bits of this field are used in subclasses".
I haven't been able to find those other bits though. And I've checked all usages of updateFlags
and writes of flags
.
What I think I'll do is:
- Set the flag when we know that there has been a full download (do we want to do
FULL_DOWNLOAD
orALL_REFERRERS_DOWNLOADED
? I think the latter.)- Standard OSM download
- Specific primitive download with
Download referrers
- Maybe some known-good overpass query patterns?
- Add the necessary supporting methods (
setReferrersDownloaded
,isReferrersDownloaded
) - Commit the changes before Friday so that hopefully someone with a plugin that adds their own flag(s) notices and reports it before the next tested release.
When I set "flags" to private I get no error indicating any additional use.
I'm not certain how you got no error -- it is specifically referenced by several different subclasses.
comment:42 by , 7 months ago
It would probably be good to change flags to private.
The protected methdod updateFlags()
could be changed to ignore flags which are undocumented for core or throw an exception.
comment:43 by , 7 months ago
Replying to taylor.smock:
When I set "flags" to private I get no error indicating any additional use.
I'm not certain how you got no error -- it is specifically referenced by several different subclasses.
I thought I was specific enough: no error indicating any additional use
comment:44 by , 7 months ago
I thought I was specific enough: no error indicating any additional use
I thought you were talking about any additional use outside of AbstractPrimitive
. My bad.
It would probably be good to change flags to private.
We'll need a new method (e.g. flags()
) since it is used by OsmPrimitive
and VectorPrimitive
to do bit math. Although I think some of those overriden methods can be moved to AbstractPrimitive
instead. I'll want to check why they aren't there already.
comment:45 by , 7 months ago
I think they all can move to AbstractPrimitive. And probably a copyFlagsFrom(oldPrimitive) function is needed.
follow-up: 49 comment:46 by , 7 months ago
We don't need copyFlagsFrom
-- we can reuse cloneFrom
(and get rid of some of the other sets, but not all). I noticed the setIncomplete
as well. It shouldn't be necessary. Maybe it is from a time when it was a separate field?
comment:47 by , 7 months ago
We can try with something like this in latest and later drop the store and restore function and thus the io stuff and make the field protected again in tested? That should be enough to find bad actors ;-)
comment:48 by , 7 months ago
That might work, but I'd rather not depend upon someone remembering to change stuff back.
comment:49 by , 7 months ago
Replying to taylor.smock:
We don't need
copyFlagsFrom
-- we can reusecloneFrom
(and get rid of some of the other sets, but not all).
Feel free to improve. Seemed easy so I tried instead of talking ;-)
I noticed the
setIncomplete
as well. It shouldn't be necessary. Maybe it is from a time when it was a separate field?
Or there is somewhere an Overridden function which does a bit more. Needs to be checked.
That might work, but I'd rather not depend upon someone remembering to change stuff back.
New Ticket for 24.05 should help ;-)
comment:50 by , 7 months ago
Feel free to improve. Seemed easy so I tried instead of talking ;-)
Fair enough. I was doing the same thing at the same time. What I have looks a little different from what you uploaded (mostly order).
Or there is somewhere an Overridden function which does a bit more
Fair enough. I'm not seeing anything anyway (at least for OsmPrimitive
).
New Ticket for 24.05 should help ;-)
You'd think so...
adds check for temporary commits in milestone tickets prior to release for wiki:DevelopersGuide/Releasing
comment:51 by , 7 months ago
BTW: In C++ there is a friend feature which would allow to access protected or private fields from another class. Didn't find this for Java. Not there or did I overlook?
adds check for temporary commits in milestone tickets prior to release for wiki:DevelopersGuide/Releasing
These are guidelines, not laws ;-)
comment:52 by , 7 months ago
BTW: a question is: Will this cause binary incompatibility (i.e. need plugins be recompiled)?
comment:53 by , 7 months ago
allow to access protected fields from another class
I think the only way to do so would be if any of the following are true:
- The class in question is a subclass of the originating class
- The class in question makes another instance of the originating class that happens to be a subclass, and that subclass makes the protected field available.
These are guidelines, not laws ;-)
Yep, but if it isn't there, I'm probably not going to remember to do it.
BTW: a question is: Will this cause binary incompatibility (i.e. need plugins be recompiled)?
As long as they didn't use the flags
field, I don't think so. We aren't changing any method definitions anyway.
by , 7 months ago
Attachment: | 4142.patch added |
---|
Extract common serialization/deserialization steps, initial functions for setting whether or not all referrers are downloaded
comment:54 by , 7 months ago
Annotations going back to to r2578 show that the additional setIncomplete() is really from ancient times and can be dropped.
Only thing I don't like about your patch is the IO stuff moving. As said we should revert that later. But for now it's ok.
comment:55 by , 7 months ago
additional setIncomplete() is really from ancient times and can be dropped
As suspected, it was from a time when it was a separate field. But it is always good to know.
Only thing I don't like about your patch is the IO stuff moving
Fair. I moved the most of the IO stuff to AbstractPrimitive
since it dealt with fields in AbstractPrimitive
. If we have to have another method in AbstractPrimitive
in order to serialize/deserialize the flags
field, I felt it would be better to move all the serialization/deserialization items that dealt with AbstractPrimitive
fields to AbstractPrimitive
(it would also make it "easier" to implement serialization/deserialization in additional child classes).
by , 7 months ago
Attachment: | 4142.2.patch added |
---|
Use new check method in circle alignment, node deletion, and a few other misc places where it should be a drop-in replacement
comment:56 by , 7 months ago
Milestone: | → 24.05 |
---|---|
Summary: | JOSM does not query API for referring relations when downloading primitives → [PATCH] JOSM does not query API for referring relations when downloading primitives |
comment:57 by , 7 months ago
Nice to see a patch!
With the same disclaimer as earlier (no experience with Java or josm code) I had a look at it and it looks to me join/merge has been handled but split not.
comment:58 by , 7 months ago
@Emvee: IIRC, that was covered by the changes to other classes (specifically in JosmAction and Command). I think I tested it anyway. I'd have to pull the patch back down and double-check.
comment:59 by , 7 months ago
I think we also need a method to use the flag in mapcss rules similar to in-downloaded-area
comment:60 by , 7 months ago
Your DeleteAction i18n doesn't work. You forgot that there are languages where surrounding text changes with the words. Extract the first sentence and make a new sentence for each variant. Then join again (Duplicating the whole block seems a bit too much).
follow-up: 63 comment:61 by , 7 months ago
It doesn't work yet.
- Download way 263221396 with referrers (Download object dialog)
- Select a way node (e.g. 1318619874) and "Download parents ways/relations"
- Select way 263221396 and the node and try to split
Expected: Split is done since parents of way and node should be known
Result: Popup "You are about to split..." asking to download the parents
I try to find out what's wrong...
comment:62 by , 7 months ago
BTW: Maybe only for debugging: It would be nice to be able to check the status of the new flag using the "Advanced object info" dialog.
comment:63 by , 7 months ago
I try to find out what's wrong...
DatasetMerger
is not yet aware of the new flag. Each merge()
is potentially resetting the flag when a freshly downloaded object is preferred and that object doesn't have the flag set. With 4142.3.patch DownloadReferrersTask
changes the flag in the "wrong" data set
I'll add a patch which seems to fix this and also changes the "Advanced object info" dialog.
by , 7 months ago
Attachment: | 4142.4.patch added |
---|
display status of new flag , improve DatasetMerger (please review, looks rather ugly) and DownloadReferrersTask
comment:64 by , 7 months ago
Reg. bbox download: I don't yet understand why relations are not marked as having all referrers downloaded when they are completely inside the bbox. My understanding is that there parent relations should in the downloaded data. On the other hand it probably makes no difference for most actions?
comment:65 by , 7 months ago
I don't yet understand why relations are not marked as having all referrers downloaded [...]
- From /api/0.6/map documentation,
All relations that reference one of the nodes, ways or relations included due to the above rules. (Does not apply recursively, see explanation below.)
Effectively, if a relation has no child relation, we can safely consider that relation to have all parent referrers. If, on the other hand, the relation has a child relation, it may not have all parent referrers. I could probably do some additional processing (does the relation have any node/way inside the download box? then mark as fully downloaded).
- Our "full" overpass download is not the same as a standard OSM download. I don't know if we want to try and fix that, but we probably should. It reuses the bounding box downloader, so any "full" download will also have the parent referrers flag set.
comment:66 by , 7 months ago
Replying to taylor.smock:
I'd be happier if we could mark
OsmPrimitive
andPrimitiveData
as sealed classes, but we can't until we drop support for Java 11, which I think is currently kept for Debian Bullseye (EOL in ~3 months) and Ubuntu 22.04 (EOL in ~3 years).
Both, Debian Bullseye and Ubuntu 22.04 (jammy), include openjdk-17-jre. It is not the default-jre, though.
comment:67 by , 7 months ago
As expected some unit tests are failing, e.g. JoinAreasActionTest
, DeleteCommandTest
What's the best way to fix them?
comment:68 by , 6 months ago
For those unit tests, something like ds.allPrimitives().forEach(p -> p.setReferrersDownloaded(true));
should do it.
I think that would keep the "meaning" of the tests anyway, since those tests aren't really concerned with the download status of the primitives.
comment:70 by , 6 months ago
I get strange results out of this. I get warnings about possible references on objects where a large surrounding area was downloaded too. I see no obvious reason why there should be some unknown references yet. The patch does allow splitting ways, but warns very often. But it does not allow joining ways (C) which is very weird.
comment:71 by , 6 months ago
Please open a new ticket with full status report and detailed steps how to reproduce
When downloading primitives from the API (with multi get) the API does not return relations the primitive is member of. There is a special API call for that:
JOSM needs to iterate over all downloaded primitives and check whether they are members of any relation.
Better would be if the API provided a call like
that returns the same kind of data like the map call.