[PATCH] NoSuchElementException HashMap$HashIterator.nextNode

[Klumbumbus]

What steps will reproduce the problem?

1. create new layer
2. click on ​http://localhost:8111/load_and_zoom?select=currentselection&bottom=46.840191627005915&top=46.84039162700592&left=9.654214585876465&right=9.654514585876464&addtags=addr%3Astreet%3DFatsch%C3%A8l%7Caddr%3Apostcode%3D7028%7Caddr%3Acity%3DSt.%20Peter%7Caddr%3Acountry%3DCH%7Ccontact%3Aemail%3Dmail%40sporthoteltanne.ch%7Ccontact%3Aphone%3D%2B41%2081%20374%2016%2085%7Ccontact%3Afax%3D%2B41%2081%20374%2016%2086%7Ctourism%3Dhotel%7Cname%3DHotel%20Tanne%7Ccontact%3Awebsite%3Dhttp%3A//www.sporthoteltanne.ch&changeset_source=TOMAS-GR%20%282017-10-22%29
3. click on add all tags

What is the expected result?

What happens instead?

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

link is from #15704

URL:http://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2017-12-28 23:33:18 +0100 (Thu, 28 Dec 2017)
Build-Date:2017-12-29 02:32:16
Revision:13257
Relative:URL: ^/trunk

Identification: JOSM/1.5 (13257 en) Windows 10 64-Bit
OS Build number: Windows 10 Pro 1709 (16299)
Memory Usage: 1904 MB / 3641 MB (1162 MB allocated, but free)
Java version: 1.8.0_151-b12, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1680x1050
Maximum Screen Size: 1680x1050
VM arguments: [-Djava.security.manager, -Djava.security.policy=file:<java.home>\lib\security\javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>\bin, -Djnlpx.origFilenameArg=C:\Program Files (x86)\josm-latest-mehr-RAM.jnlp, -Djnlpx.remove=true, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.heapsize=1024m,4096m, -Djnlpx.splashport=58326, -Djnlpx.jvm=<java.home>\bin\javaw.exe]
Dataset consistency test: No problems found

Plugins:
+ AddrInterpolation (33692)
+ CADTools (1007)
+ DirectDownload (33850)
+ DirectUpload (33882)
+ FastDraw (33731)
+ HouseNumberTaggingTool (33904)
+ OpeningHoursEditor (33876)
+ ShapeTools (1230)
+ Tracer2 (33908)
+ alignways (33784)
+ apache-commons (33668)
+ apache-http (32699)
+ buildings_tools (33735)
+ contourmerge (1032)
+ editgpx (33782)
+ ejml (32680)
+ geotools (33958)
+ imagery-xml-bounds (33830)
+ imagery_offset_db (33774)
+ jogl (1.1.0)
+ jts (32699)
+ log4j (32699)
+ measurement (33760)
+ opendata (33791)
+ photo_geotagging (33878)
+ photoadjust (33746)
+ reltoolbox (33708)
+ reverter (33865)
+ tag2link (33934)
+ tageditor (33806)
+ tagging-preset-tester (33898)
+ terracer (33839)
+ turnlanes-tagging (260)
+ turnrestrictions (33780)
+ undelete (33835)
+ utilsplugin2 (33742)
+ wikipedia (33696)

Tagging presets:
+ https://josm.openstreetmap.de/josmfile?page=Presets/OneClick&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/StolpersteineLight&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/NewTags&zip=1
+ %UserProfile%\Documents\OSM\josm\data\defaultpresets.xml
+ %UserProfile%\Documents\OSM\TestNew\newpresets.xml

Map paint styles:
+ %UserProfile%\Documents\OSM\josm\styles\standard\elemstyles.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/HiDPISupport&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/NewHighwayColors&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Streets&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Maxspeed&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/sac_scale&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/ShowID&zip=1
- %UserProfile%\Documents\OSM\eigene styles\PriorityRoad\PriorityRoad_1.0.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/LayerChecker&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Modified&zip=1
- %UserProfile%\Documents\OSM\TestNew\newicons.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/Surface&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/ParkingLanes&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Sidewalks&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Cycleways&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Osmc&style&zip=1
- https://raw.githubusercontent.com/species/josm-preset-wheelchair/master/sidewalks_kerbs.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/LitObjects&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Lit&style&zip=1
- %UserProfile%\Documents\OSM\eigene styles\Tourenplanung.mapcss
- %UserProfile%\Documents\OSM\eigene styles\SpecificBuildingValues\SpecificBuildingValues.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_buildings&zip=1
- https://github.com/bastik/mapcss-tools/raw/osm/mapnik2mapcss/osm-results/mapnik.zip
- %UserProfile%\Documents\OSM\eigene styles\area-symbol.zip
- http://www.freietonne.de/ft_icons/josm/FreieTonne_rules_presets_zip.php
- http://www.openrailwaymap.org/styles/standard.zip
- https://josm.openstreetmap.de/josmfile?page=Styles/MaxspeedIcons&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/DestinationSignRelation&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/ParkingLanes&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Incline&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/PTStops&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/AdvertisingStyle&zip=1
- https://www.dropbox.com/s/qo3ai47fpv241jf/Styles_Fixme_and_Notes.zip?raw=1
- https://github.com/gmgeo/osmic-josm-style/archive/master.zip
- https://josm.openstreetmap.de/josmfile?page=Styles/hazmat&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Suburb&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Postcode&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/RecyclingMaterials&zip=1
- %UserProfile%\Documents\OSM\eigene styles\maxspeed\maxspeed_2.9_01 basierend auf 2.7_02 Zahlen.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/Bench&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/PublicTransportV2&zip=1
- %UserProfile%\Documents\OSM\eigene styles\colourtag\colourtag_1.0.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/ColourTag&zip=1

Validator rules:
+ https://raw.githubusercontent.com/<user.name>n-a-bauer/josm-validators/master/mtb.validator.mapcss
+ %UserProfile%\Documents\OSM\TestNew\new.validator.mapcss
+ https://josm.openstreetmap.de/josmfile?page=Rules/GermanySpecific&zip=1
- %UserProfile%\Documents\OSM\TestNew\germ.validator.mapcss
- https://josm.openstreetmap.de/josmfile?page=Rules/CzechRepublicAddressSystem&zip=1

Last errors/warnings:
- W: No configuration settings found.  Using hardcoded default values for all pools.
- W: Warning: Failed to scan file 'site-svn.openstreetmap.org-_applications_editors_josm_plugins_opendata_modules.txt' for module information. Skipping.
- E: Handled by bug report queue: java.util.NoSuchElementException


=== REPORTED CRASH DATA ===
BugReportExceptionHandler#handleException:
No data collected.

Warning issued by: BugReportExceptionHandler#handleException

=== STACK TRACE ===
Thread: AWT-EventQueue-2 (52) of javawsApplicationThreadGroup
java.util.NoSuchElementException
	at java.util.HashMap$HashIterator.nextNode(Unknown Source)
	at java.util.HashMap$KeyIterator.next(Unknown Source)
	at org.openstreetmap.josm.command.ChangePropertyCommand.<init>(ChangePropertyCommand.java:108)
	at org.openstreetmap.josm.io.remotecontrol.AddTagsDialog.buttonAction(AddTagsDialog.java:260)
	at org.openstreetmap.josm.gui.ExtendedDialog$1.actionPerformed(ExtendedDialog.java:377)
	at javax.swing.AbstractButton.fireActionPerformed(Unknown Source)
	at javax.swing.AbstractButton$Handler.actionPerformed(Unknown Source)
	at javax.swing.DefaultButtonModel.fireActionPerformed(Unknown Source)
	at javax.swing.DefaultButtonModel.setPressed(Unknown Source)
	at javax.swing.plaf.basic.BasicButtonListener.mouseReleased(Unknown Source)
	at java.awt.AWTEventMulticaster.mouseReleased(Unknown Source)
	at java.awt.Component.processMouseEvent(Unknown Source)
	at javax.swing.JComponent.processMouseEvent(Unknown Source)
	at java.awt.Component.processEvent(Unknown Source)
	at java.awt.Container.processEvent(Unknown Source)
	at java.awt.Component.dispatchEventImpl(Unknown Source)
	at java.awt.Container.dispatchEventImpl(Unknown Source)
	at java.awt.Component.dispatchEvent(Unknown Source)
	at java.awt.LightweightDispatcher.retargetMouseEvent(Unknown Source)
	at java.awt.LightweightDispatcher.processMouseEvent(Unknown Source)
	at java.awt.LightweightDispatcher.dispatchEvent(Unknown Source)
	at java.awt.Container.dispatchEventImpl(Unknown Source)
	at java.awt.Window.dispatchEventImpl(Unknown Source)
	at java.awt.Component.dispatchEvent(Unknown Source)
	at java.awt.EventQueue.dispatchEventImpl(Unknown Source)
	at java.awt.EventQueue.access$500(Unknown Source)
	at java.awt.EventQueue$3.run(Unknown Source)
	at java.awt.EventQueue$3.run(Unknown Source)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(Unknown Source)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(Unknown Source)
	at java.awt.EventQueue$4.run(Unknown Source)
	at java.awt.EventQueue$4.run(Unknown Source)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(Unknown Source)
	at java.awt.EventQueue.dispatchEvent(Unknown Source)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(Unknown Source)
	at java.awt.EventDispatchThread.pumpEventsForFilter(Unknown Source)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(Unknown Source)
	at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
	at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
	at java.awt.EventDispatchThread.run(Unknown Source)

[skorbut]

This probably happens since no element was selected onto which tags should be applied.

I see two possible solutions:

1. Silently discard the command.
2. Print a message box basically saying "You clicked on a remotecontrol link that would apply tags onto the selected objects, but no objects were selected."

I guess the second option -- although it's a bit verbose -- would be nicer since it makes it clear why nothing is happening.

I will work on a patch for the second option unless someone disagrees.

[Klumbumbus]

Replying to skorbut:

This probably happens since no element was selected onto which tags should be applied.

Yes.

[Don-vip]

second option is better but requires i18n and we're releasing new version tomorrow. Can you please make a patch just for first option, then work on the second option? So we will include first option in next version.

[skorbut]

Ok. I'll work on preventing the exception...

[skorbut]

I wrote a patch for preventing this exception. Some remarks:

• I don't have much experience in writing GUI code/working with EDT, so please check thoroughly.
• I skipped the part where error messages should be created since I didn't know how to do it, but it should be easy to fill them in. I already prepared drafts for their texts. Maybe someone can add them.
• I propose to also generate a warning message for invalid select= query parameters. Some error checking is already being done at the end of LoadAndZoomHandler.validateRequest().

For testing you can use ​http://localhost:8111/load_and_zoom?select=currentselection&bottom=46.840191627005915&top=46.84039162700592&left=9.654214585876465&right=9.654514585876464&addtags=addr%3Astreet%3DFatsch%C3%A8l%7Caddr%3Apostcode%3D7028%7Caddr%3Acity%3DSt.%20Peter%7Caddr%3Acountry%3DCH%7Ccontact%3Aemail%3Dmail%40sporthoteltanne.ch%7Ccontact%3Aphone%3D%2B41%2081%20374%2016%2085%7Ccontact%3Afax%3D%2B41%2081%20374%2016%2086%7Ctourism%3Dhotel%7Cname%3DHotel%20Tanne%7Ccontact%3Awebsite%3Dhttp%3A//www.sporthoteltanne.ch&changeset_source=TOMAS-GR%20%282017-10-22%29
The link should add tags if an object is selected, otherwise it should generate a warning log message (and later on create a message window).

For testing an invalid select= query parameter you can use: ​http://localhost:8111/load_and_zoom?select=XXXXXXXXXX&bottom=46.840191627005915&top=46.84039162700592&left=9.654214585876465&right=9.654514585876464&addtags=addr%3Astreet%3DFatsch%C3%A8l%7Caddr%3Apostcode%3D7028%7Caddr%3Acity%3DSt.%20Peter%7Caddr%3Acountry%3DCH%7Ccontact%3Aemail%3Dmail%40sporthoteltanne.ch%7Ccontact%3Aphone%3D%2B41%2081%20374%2016%2085%7Ccontact%3Afax%3D%2B41%2081%20374%2016%2086%7Ctourism%3Dhotel%7Cname%3DHotel%20Tanne%7Ccontact%3Awebsite%3Dhttp%3A//www.sporthoteltanne.ch&changeset_source=TOMAS-GR%20%282017-10-22%29

[skorbut]

There is a bug in the patch. I'll submit another patch shortly.

[skorbut]

I submitted another file called no_exception_for_empty_selection2.patch, which hopefully should be correct now.

This new patch removes the line isKeepingCurrentSelection = false;. This line was unneeded before and with the other changes of the patch is now actually wrong. isKeepingCurrentSelection may get set in LoadAndZoomHandler.validateRequest(). It is not necessary to reset it, since for every remote control command, LoadAndZoomHandler will be newly instantiated.

[Don-vip]

Thanks. Sorry I forget to apply the patch in tested release, so you can display the messages :)
Please also replace forTagAdd.size() > 0 by !forTagAdd.isEmpty()

[skorbut]

I added a new patch no_exception_for_empty_selection3.patch, which replaces the previous two patches. This one prints error messages in the case of errors.

[Don-vip]

It would be better and simpler to add the verification logic in LoadAndZoomHandler.validateRequest() and throw an exception. We're trying step by step to modularize JOSM (see #15229), so the new import javax.swing.* gets in the way.

[skorbut]

@Don-vip: I'm not quite sure about what you propose. What exactly is the job of validateRequest()? (The Javadoc is missing for this.) To my understanding it is to make sure that the request is well-formed or otherwise throw a RequestHandlerBadRequestException that will result in an error 400. Since the exact same request may work in one case and fail in another, depending on the current selection of objects, I considered validateRequest() not the right place for this check, since for my interpretation it's more like a runtime error (such as trying to open a file with insufficient permissions, etc.) Maybe we should throw a RequestHandlerErrorException? (i.e. generating an internal server error 500) To me this seems a bit user-unfriendly, since he/she might not always see the message (depending on how exactly the remotecontrol URL was called in the user's browser). On the other hand it's fitting since the request per se is fine, but the state on the server (i.e. JOSM) is wrong. That justifies an error 500.

How would I go about preventing importing javax.swing.*? Is there an alternative to the aforementioned throwing of a RequestHandlerErrorException in order to still get a nice Swing error message?

[Don-vip]

Sorry for not answering earlier.
I didn't see that the problem was depending of JOSM state. So yes, the best way is to go for an error 500 + an error log.

[Don-vip]

In 13973/josm:

fix #15707 - NoSuchElementException at remotecontrol.AddTagsDialog (patch by skorbut, modified)