[patch] Improve cancel action on OSM API errors

[gaben]

What steps will reproduce the problem?

1. Set OSM API URL to something unreachable, I've used ​http://httpstat.us/503
2. Try download something as usual
3. The dialog will count retries, try stopping it with the cancel button

What is the expected result?

The cancel button stops the wait task and closes the window.

What happens instead?

The cancel button doesn't do anything.

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

There is an other bug on the UI: when you first try to set it up the unreachable API, the preference window stops working until the wait task finishes.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2023-07-20 22:21:29 +0200 (Thu, 20 Jul 2023)
Build-Date:2023-07-21 01:30:56
Revision:18777
Relative:URL: ^/trunk

Identification: JOSM/1.5 (18777 hu) Windows 10 64-Bit
OS Build number: Windows 10 Pro for Workstations 2009 (19045)
Memory Usage: 1056 MB / 7266 MB (613 MB allocated, but free)
Java version: 1.8.0_382-b05, Azul Systems, Inc., OpenJDK 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920×1200 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1200
Best cursor sizes: 16×16→32×32, 32×32→32×32
System property file.encoding: Cp1250
System property sun.jnu.encoding: Cp1250
Locale info: hu_HU
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Dicedtea-web.bin.name=javaws.exe, -Dicedtea-web.bin.location=C:\Program Files\IcedTeaWeb\WebStart\bin\javaws.exe]

[gaben]

Please review the attached patch. I'm not familiar with UI programming, probably it needs modification.

[taylor.smock]

I think your patch is fine; we just have to modify some plugins as well.

[taylor.smock]

NVM. I thought your patch was fine. There are a few places in JOSM core that also need to be changed.

With that said, I think we might want to keep the cancel field since it is set in the cancel() method, which may not be called by a progress monitor.

[taylor.smock]

In 18778/josm:

(The changeset message doesn't reference this ticket)

[taylor.smock]

I made a typo on a ticket number. I meant #23079, not #23078. Sorry for the noise.

[gaben]

Are you referring to headless mode?

[taylor.smock]

I don't think so. When I said "[there] are a few places in JOSM core that also need to be changed", I meant there are a few other classes that use the cancel field, like:

• OsmServerReader
• OsmServerObjectReader
• OsmServerLocationReader
• OsmServerHistoryReader
• OsmServerBackreferenceReader
• BoundingBoxDownloader

Anyway, I think

src/org/openstreetmap/josm/io/OsmApi.java

@@ -248 +248 @@
         if (initialized)
             return;
         cancel = false;
+        if (monitor != null) {
+            monitor.addCancelListener(this::cancel);
+        }
         try {
             CapabilitiesCache cache = new CapabilitiesCache(monitor, fastFail);
             try {

will work. It at least worked on my system.

[gaben]

Aaaah okay, I totally missed these because used Ant dist, not IntelliJ build, sorry.

Yes, the above code works for me as well.

[taylor.smock]

Replying to gaben:

Aaaah okay, I totally missed these because used Ant dist, not IntelliJ build, sorry.

When removing non-private fields/methods, use ant clean dist -- this forces a full rebuild, which will catch that type of issue. You don't have to use IntelliJ. I did use the find functionality in IntelliJ for the plugins though (since I have most plugins open in the same IntelliJ project).

[taylor.smock]

In 18779/josm:

Fix #23078: Improve cancel action on OSM API errors (patch by gaben, heavily modified)

[gaben]

Heavily modified :D You could have skip me altogether.

Replying to taylor.smock:

When removing non-private fields/methods, use ant clean dist -- this forces a full rebuild, which will catch that type of issue. You don't have to use IntelliJ. I did use the find functionality in IntelliJ for the plugins though (since I have most plugins open in the same IntelliJ project).

The downside of doing it that way is time, which is probably less than the time required to type this sentence, anyway. With IntelliJ built-in build, I don't need to clean up, although it could be a good practice before publishing anything here.

[taylor.smock]

Heavily modified :D You could have skip me altogether.

I could have, but I kind of wanted to acknowledge that you had provided code that could have fixed the problem.

The downside of doing it that way is time

I usually run ant clean dist pmd checkstyle before committing, and every so often as I'm doing development. I usually switch to doing something else at that point though, and come back in an hour or so.