NPE in AutoCompletionManager.getAllForKeys()

[GerdP]

What steps will reproduce the problem?

1. Start clean JOSM installation
2. Create a new node in an empty data layer
3. Open preset preferences and remove all presets, press OK
4. Note that the toolbar still shows the icons for some presets
5. Select the node and click on any of the shown icons and select one preset

What is the expected result?

Not sure. Certanly no crash.
After step 3 I would have expected that all icons for presets disappear.

What happens instead?

NPE

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

The NPE doesn't happen with tested version. Presets seem to work as expected.
Stumbled over this while looking at #21360.

Build-Date:2021-10-16 10:27:49
Revision:18275
Is-Local-Build:true

Identification: JOSM/1.5 (18275 SVN de) Windows 10 64-Bit
OS Build number: Windows 10 Home 2009 (19043)
Memory Usage: 369 MB / 1753 MB (201 MB allocated, but free)
Java version: 1.8.0_221-b11, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920×1080 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→32×32, 32×32→32×32
System property file.encoding: Cp1252
System property sun.jnu.encoding: Cp1252
Locale info: de_DE
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Djosm.home=<josm.pref>]
Dataset consistency test: No problems found

Last errors/warnings:
- 00032.087 E: Fehler beim Laden des Bildes '[[Image(pond.png)]]'
- 00032.098 E: Fehler beim Laden des Bildes '[[Image(oil-palm.png)]]'
- 00041.622 E: Handled by bug report queue: java.lang.NullPointerException



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

Warning issued by: BugReportExceptionHandler#handleException

=== STACK TRACE ===
Thread: AWT-EventQueue-0 (18) of main
java.lang.NullPointerException
	at org.openstreetmap.josm.gui.tagging.ac.AutoCompletionManager.getAllForKeys(AutoCompletionManager.java:358)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItem.getAllForKeys(TaggingPresetItem.java:73)
	at org.openstreetmap.josm.gui.tagging.presets.items.Combo.addToPanel(Combo.java:127)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPreset.createPanel(TaggingPreset.java:435)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPreset.showDialog(TaggingPreset.java:596)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPreset.showAndApply(TaggingPreset.java:508)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPreset.actionPerformed(TaggingPreset.java:498)
	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.AbstractButton.doClick(Unknown Source)
	at javax.swing.plaf.basic.BasicMenuItemUI.doClick(Unknown Source)
	at javax.swing.plaf.basic.BasicMenuItemUI$Handler.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)

[GerdP]

regression of r18221?

[marcello@…]

This should fix the NPE (but not the icons, which are completely orthogonal to r18221).

[GerdP]

I think this patch will not work with the code in TagChecker which expects the null:

    public static boolean isKeyInPresets(String key) {
        return TaggingPresets.getPresetValues(key) != null;
    }

[marcello@…]

Patch

[skyper]

I have noticed similar reloading a preset with an object selected, the preset link in the Tags/Memberships Panel are only updated after deselecting and selecting the object again but not right away. This has nothing to do with r18221 but is present for a long time, now.

[GerdP]

I assume that much older JOSM releases required a restart after changing the presets. A lot of the code doesn't seem to expect that a preset is removed or replaced.

[GerdP]

Yes, see #18381

[marcello@…]

From a cursory glance I see that PropertiesDialog has no TaggingPresetListener. It should be easy to add one.

[marcello@…]

Patch for Tags/Memberships in #21441.

[Don-vip]

In 18277/josm:

fix #21438 - NPE (patch by marcello)

[GerdP]

I don't think this will work. Method PRESET_TAG_CACHE.get(key) may return an empty set for a known key. We have to distinguish between "key not in preset" and "no value for known key"

[GerdP]

Example: For way 270921055 (a highway with minspeed=40 and ref=B 213) I see two false informational messages now:

Other (2)
Presets do not contain property key (2)
Key 'minspeed' not in presets. (1)
Key 'ref' not in presets. (1)

[GerdP]

fix TagChecker

[GerdP]

The patch should restore the old behaviour in TagChecker. I found no plugin which uses
TaggingPresets.getPresetValues(String key) but I still think it is risky to change the documented behaviour as in r18277. Maybe better add code in AutoCompletionManager to handle the null value properly?

[marcello@…]

I must disagree. Just don't use getPresetValues(key) != null when you mean getPresetKeys().contains(key). That was a totally unnecessary overload of the getPresetValues function.

Best would be to move isKeyInPresets(String key) to TaggingPresets and make it query PRESET_TAG_CACHE directly. Can you add that?

[GerdP]

I agree that isKeyInPresets(String key) should better be in TaggingPresets. Maybe I'm overestimating the importance of backward compatability here? For me, documented behaviour in public methods is somehow "fixed" and I always assume there might be a plugin that relies on it.

[GerdP]

Like this?

[marcello@…]

Looks OK.

[GerdP]

In 18281/josm:

fix #21438: NPE in AutoCompletionManager.getAllForKeys()

• fix regression introduced with r18277

[GerdP]

In 18282/josm:

see #21438
Add unit test for tags keys which appear in presets but never with a value, e.g. ref
This should not produce Presets do not contain property key message.

[GerdP]

In 18363/josm:

see #6725, #21438
correct @since xxx