[Patch] Combine ways refused

[nkamapper]

What steps will reproduce the problem?

1. Download ways with Overpass, for example:

[out:xml][timeout:90];
{{geocodeArea:Arendal}}->.searchArea;
(
  nwr["waterway"](area.searchArea);
);
(._;>;<;);
out meta;

2. Select two ways which are connected with a common node, for example at node 4157936458.

3. Apply Tools -> Combine ways.

What is the expected result?

The two ways should be concatenated into one way.

What happens instead?

Since 19096 I get this error message: "Combine ways refused (A shared node may have additional referrers)."

Please provide any additional information below. Attach a screenshot if possible.

During imports I am doing hundreds of these combinations, so this slows down editing considerably. Parents were already downloaded with the recurse up operator above, so the refusal should not be needed.

Revision:19096
Build-Date:2024-06-04 11:25:36

Identification: JOSM/1.5 (19096 en_GB) Mac OS X 12.7.4
OS Build number: macOS 12.7.4 (21H1123)
Memory Usage: 1684 MB / 4096 MB (349 MB allocated, but free)
Java version: 21.0.3+9-LTS, Azul Systems, Inc., OpenJDK 64-Bit Server VM
Look and Feel: com.apple.laf.AquaLookAndFeel
Screen: Display 69733568 1440×900 (scaling 2.00×2.00)
Maximum Screen Size: 1440×900
Best cursor sizes: 16×16→16×16, 32×32→32×32
System property file.encoding: UTF-8
System property sun.jnu.encoding: UTF-8
Locale info: en_GB
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Djpackage.app-version=19096, --add-modules=java.scripting,java.sql,javafx.controls,javafx.media,javafx.swing,javafx.web, --add-exports=java.base/sun.security.action=ALL-UNNAMED, --add-exports=java.desktop/com.apple.eawt=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED, --add-opens=java.base/java.lang=ALL-UNNAMED, --add-opens=java.base/java.nio=ALL-UNNAMED, --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED, --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED, --add-opens=java.desktop/javax.imageio.spi=ALL-UNNAMED, --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED, -Djpackage.app-path=/Applications/JOSM.app/Contents/MacOS/JOSM]
Dataset consistency test: No problems found

Plugins:
+ PicLayer (1.0.3)
+ RelationDissolve (0.2.0)
+ apache-commons (36273)
+ apache-http (36273)
+ changeset-viewer (0.0.7)
+ conflation (0.6.11)
+ ejml (36176)
+ ext_tools (36126)
+ geotools (36273)
+ imagery-xml-bounds (36196)
+ jackson (36273)
+ jaxb (36118)
+ jna (36273)
+ jts (36004)
+ log4j (36273)
+ opendata (36256)
+ pdfimport (36205)
+ reverter (36256)
+ scripting (v0.3.1)
+ todo (137)
+ utilsplugin2 (36241)

Tagging presets:
+ https://josm.openstreetmap.de/josmfile?page=Presets/LaneAttributes&zip=1
+ https://raw.githubusercontent.com/OpenNauticalChart/josm/master/INT-1-preset.xml

Map paint styles:
+ https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1
- https://raw.githubusercontent.com/OpenSeaMap/josm/master/CEVNI_MapCSS.mapcss
- https://raw.githubusercontent.com/OpenSeaMap/josm/master/INT1_Seamark.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/PublicTransport&zip=1

[skyper]

This check needs to be rewritten. Using the trick that at least one child node needs to be within downloaded area does not work anymore and is wrong plus the message is misleading. Additionally, this should be a non-blocking warning with an option to continue anyway.

Regarding overpass, there might be more work to get the flag that all parents have been loaded correct.

[GerdP]

Yes, since r19078 one has to download the parents of the connection node first. I think this was intended since #18083 but the test didn't work well without any download area.

[GerdP]

Replying to skyper:

This check needs to rewritten. Using the trick that at least one child node needs to be within downloaded area does not work anymore and is wrong plus the message is misleading. Additionally, this should be a non-blocking warning with an option to continue anyway.

I think the correct way would be to check if the selected ways have the "all-referrers-downloaded" status. Instead we only check if the connection node has this status. Before r19078 this was ok because we checked if the node was inside a download area and that implied that also the parents of the ways are known.

Regarding overpass, there might be more work to get the flag that all parents have been loaded correct.

I see no way to set the flag with any query that doesn't download an area. We cannot interpret the query nor can we be sure that overpass will really do what we expect it to do.

[GerdP]

Additionally, this should be a non-blocking warning with an option to continue anyway.

Maybe we could just show a dialog as when you delete a primitive, but with a slighlty different text?

"Combine confirmation"
"You are about to combine ways which can have other refererers not yet downloaded." (rest unchanged)

[GerdP]

@nkamapper:

During imports I am doing hundreds of these combinations, so this slows down editing considerably.

I don't yet understand.

• If you import new data the combine should just work.
• I see no good reason to combine ways which are already on the server.

Parents were already downloaded with the recurse up operator above, so the refusal should not be needed.

(._;>;<;);

You probably suggest to parse the overpass query and detect that this line will tell overpass to send all refererres of the previous result.
The problem here is that JOSM doesn't know which data is send because it was in the initial result and which was added because of the "recurse up" operator. This would only work for nodes because they never refer to other objects.

[nkamapper]

Replying to GerdP:

@nkamapper:

During imports I am doing hundreds of these combinations, so this slows down editing considerably.

I don't yet understand.

• If you import new data the combine should just work.
• I see no good reason to combine ways which are already on the server.

There are many cases, for example:

• Combining a new way and an old way.
• Combining old ways because they logically belong together (an earlier import may have excessive short segments).
• Combining a way which I earlier accidentally split.
• Adjusting at which node the split between two ways should occur (split + join).
• In general, manually rearranging old and new data.

Parents were already downloaded with the recurse up operator above, so the refusal should not be needed.

(._;>;<;);

You probably suggest to parse the overpass query and detect that this line will tell overpass to send all refererres of the previous result.
The problem here is that JOSM doesn't know which data is send because it was in the initial result and which was added because of the "recurse up" operator. This would only work for nodes because they never refer to other objects.

I am not suggesting anything because I do not know how JOSM stores the data. I just wanted to highlight that parents were present in the given example. I have downloaded this way in thousands of sessions (often with pre-processing in scripts) and never had any conflicts because of it as long as I pay attention to incomplete parents.

[GerdP]

I just wanted to highlight that parents were present in the given example

JOSM cannot know that.

[nkamapper]

Replying to GerdP:

I just wanted to highlight that parents were present in the given example

JOSM cannot know that.

Agree. The source data could also be originating from a file with unknown history. And for this reason I do not think it makes sense to display a warning for every single possible combination in the dataset. At least there have to be a way to turn off the warning.

[GerdP]

Yes, see comment:5

[GerdP]

Improve test to check selected ways and not the node, show dialog to confirm the risky operation

[GerdP]

With the patch the following dialog is displayed if any of the selected ways doesn't have the parents downloaded flag set:

[skyper]

So far the only referrers are relations and I do not think this will change in near future. Therefore, I would call them what they are:

You are about to combine ways which can be members of relations not yet downloaded.
This can lead to damaging these parent relations (that you do not see).
Do you really want to combine.

Maybe, it would be even better to offer the download of parent objects similar to the split way action.

[GerdP]

I wanted to keep as much text as possible so that translators can use paste+copy. But I wouldn't mind to use your text.

[stoecker]

The text is missing what to do instead: I.e. how to download the missing data.

[GerdP]

dialog with hint for action and shortcut

[GerdP]

Replying to stoecker:

The text is missing what to do instead: I.e. how to download the missing data.

Like this?

The hint would be computed with

        DownloadReferrersAction action = MainApplication.getMenu().downloadReferrers;
        final String downloadHint = tr("You should use {0}->{1}({2}) first.",
                MainApplication.getMenu().editMenu.getText(), action.getValue(NAME), action.getShortcut().toString());

Replying to skyper

Maybe, it would be even better to offer the download of parent objects similar to the split way action.

Yes. Unfortunately I am not able to code this because I don't understand how to execute the DownloadReferrersTask and wait for it to finish. I end either up with two parallel threads or one that is waiting forever :(

[GerdP]

add hint reg. Download parent action

[GerdP]

Solution for automatic download of parents? It checks if preference combine.download-parents is true.

[GerdP]

BTW: The split ways action only offers to download further members of known relations. If you split a way with "referrers-not-all-downloaded" status it will only inform about the possibly broken relations.

[MSiipola]

I have the same problem/error using 19096, and I have never seen this before. Despite using JOSM several years.

For example, I have a short service road connected to a short track road, with a common node. Neither of these ways belongs to any relation, and both are well in the download area. Both are new (id=0). And JOSM refuses to combine them. If I copy both ways to new layer, they are possible to combine.

[GerdP]

I cannot reproduce this problem with new ways. Maybe you refer to the popup dialog that asks regarding the tag conflicts?

[MSiipola]

The popup is not shown. I only got the error message.

I have an osm-file with the error. Is it possible to upload it here?

[GerdP]

Yes, there is a button to attach files near below the first post.

[GerdP]

If you get this message after loading the data from file I don't need the data, that's a known problem.

[MSiipola]

File with two ways which can't be combined. File is an extract of a bigger file, where the other objects are deleted.

[GerdP]

Can you confirm that this problem only occurs when you load the data from file?

[MSiipola]

Yes. Loading the file and trying to combine the two visible ways, give me the error.

[GerdP]

OK. The unpatched code checks if the parents of the end nodes of the ways were downloaded. When data is loaded from file this flag is not set, even when nodes are within a download area.

[GerdP]

I am not sure how to continue here.

• With 23735-2.patch we'll just show another popup that tells the user that he has to do something to prevent data corruption unless they know what they do. Other actions show similar dialogs but without a hint what to do.
• With 23735-3.patch (if preference combine.download-parents is set to true) the parents would be downloaded when needed but only for "Combine ways". If we go this way I'd prefer to have a general preference for all actions.
• Another approach would be a new button "Download" in the popup dialog about the missing parent info.

My current feeling is that I should commit 23735-2.patch to close this ticket and then open a new one for a better general solution.

[MSiipola]

Why has this bug(?) jumped up now?
What previous change caused it?

[GerdP]

See comment:3

[GerdP]

In 19119/josm:

fix #23735: Combine ways refused
(patch 23735-2.patch)

• rewrite check so that it checks if the parents of the combined ways are known instead of the parents of the connection node(s)
• show dialog with hint about download parents action that allows to continue

TODO: implement full automatic download of parents in all relavant action or add download button in the common dialog.

[GerdP]

New ticket reg. automatic download of parents see #23744

[anonymous]

Download parent ways/relations is under File not Edit