NPE at org.openstreetmap.josm.data.validation.tests.OpeningHourTest.createTestError

[Claudius]

What steps will reproduce the problem?

1. Loaded relation via object loader: ​osmwww:relation/62146
2. Change "colour" tag to #67C0DD
3. Attempted upload

What is the expected result?

Upload to work

What happens instead?

NullPointerException

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

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-03-19 22:48:47 +0100 (Thu, 19 Mar 2020)
Build-Date:2020-03-20 02:30:58
Revision:16177
Relative:URL: ^/trunk

Identification: JOSM/1.5 (16177 en) Windows 10 64-Bit
OS Build number: Windows 10 Home 1909 (18363)
Memory Usage: 160 MB / 989 MB (39 MB allocated, but free)
Java version: 1.8.0_241-b07, Oracle Corporation, Java HotSpot(TM) Client VM
Screen: \Display0 1920x1080
Maximum Screen Size: 1920x1080
VM arguments: [-Dsun.java2d.opengl=true]
Dataset consistency test: No problems found

Plugins:
+ OpeningHoursEditor (35250)
+ apache-commons (35362)
+ apache-http (35092)
+ buildings_tools (35364)
+ jna (35092)
+ pt_assistant (2.1.10-79-gb14a159)
+ tageditor (35258)
+ turnlanes-tagging (283)
+ utilsplugin2 (35366)
+ wikipedia (1.1.3)

Tagging presets:
+ https://github.com/kendzi/Simple3dBuildingsPreset/releases/download/0.9_2018-05-08/s3db-preset.zip
+ https://josm.openstreetmap.de/josmfile?page=Presets/OneClick&zip=1

Map paint styles:
+ https://josm.openstreetmap.de/josmfile?page=Styles/PublicTransport&zip=1
+ https://raw.githubusercontent.com/yopaseopor/indoormap/master/indoormap-style.mapcss
+ https://josm.openstreetmap.de/josmfile?page=Styles/AdvertisingStyle&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Bench&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Building_Levels_Labels&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Modified&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Maxspeed&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/SimpleRoofTags&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/RU-SubwayEntranceLabeling&zip=1
+ https://github.com/osmlab/appledata/archive/josm_paint_inline_validation.zip
+ https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1

Validator rules:
+ https://josm.openstreetmap.de/josmfile?page=Rules/QAToolInspiredValidations&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Rules/OSMLint&zip=1
+ https://www.openrailwaymap.org/validator/openrailwaymap.validator.mapcss
+ https://josm.openstreetmap.de/josmfile?page=Rules/KeepRight&zip=1

Last errors/warnings:
- 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 (16) of main
java.lang.NullPointerException
	at org.openstreetmap.josm.data.validation.tests.OpeningHourTest.createTestError(OpeningHourTest.java:56)
	at org.openstreetmap.josm.data.validation.tests.OpeningHourTest.checkOpeningHourSyntax(OpeningHourTest.java:98)
	at org.openstreetmap.josm.data.validation.tests.OpeningHourTest.checkOpeningHourSyntax(OpeningHourTest.java:71)
	at org.openstreetmap.josm.data.validation.tests.ConditionalKeys.validateValue(ConditionalKeys.java:205)
	at org.openstreetmap.josm.data.validation.tests.ConditionalKeys.validatePrimitive(ConditionalKeys.java:236)
	at org.openstreetmap.josm.data.validation.tests.ConditionalKeys.check(ConditionalKeys.java:250)
	at org.openstreetmap.josm.data.validation.Test$TagTest.visit(Test.java:138)
	at org.openstreetmap.josm.data.osm.Relation.accept(Relation.java:177)
	at org.openstreetmap.josm.data.validation.Test.visit(Test.java:215)
	at org.openstreetmap.josm.actions.upload.ValidateUploadHook.checkUpload(ValidateUploadHook.java:66)
	at org.openstreetmap.josm.actions.UploadAction.checkPreUploadConditions(UploadAction.java:219)
	at org.openstreetmap.josm.actions.UploadAction.uploadData(UploadAction.java:238)
	at org.openstreetmap.josm.actions.UploadAction.actionPerformed(UploadAction.java:294)
	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)

[skyper]

Can reproduce: The offending tag is interval:conditional=7 @ (Sa-Su 06:00-00:00); 10 @ (Fr-Sa 00:00-01:30); 15 @ (Su-Th 00:00-01:30)

In order to be able to upload, you need to disable the validator run on upload under wikitr:/Help/Preferences/Validator.

Does not happen with tested (r15937).

[skyper]

r16155 from two days ago, is already buggy.

[skyper]

I narrowed it down to between r16052 and r16087. r16051 is still working.

[skyper]

EDIT by Don-vip: deleted (wrong information)

[Don-vip]

I guess it's a regression from r16060 or r16061 but I don't see why.

[skyper]

example file

[skyper]

Replying to skyper:

Replying to skyper:

I narrowed it down to between r16052 and r16087. r16051 is still working.

Even smaller gap (r16065 not working): changelog

Sorry, I was wrong r16065 is completely buggy but working. The gap is between r16066 and r16087: changelog

[skyper]

The value part which triggers the exception is 00:00 in 06:00-00:00.

[Don-vip]

Replying to skyper:

Sorry, I was wrong r16065 is completely buggy but working. The gap is between r16066 and r16087: changelog

OK so it's a regression from r16086 and it makes much more sense. Please be careful when you give revision ranges. No information is better than a wrong one :)

[skyper]

Yeah, I am really sorry, will recheck three times on the next issues, I promise.

[Don-vip]

In 16178/josm:

fix #18956 - allow TestError to be built with an empty list of primitives + make OpeningHourTestTest work on Windows

[GerdP]

Sounds dangerous. How does the validator tree handle this?

[Don-vip]

It does not. These errors are not used directly by the validator dialog.

[GerdP]

Why are they created then? I think it is a good idea that TestError always "knows" the primitive(s).

[Don-vip]

It simplifies the code by avoiding the need of a class similar to TestError, see what Simon did in r16086.

[GerdP]

My understanding is that class TestError.Builder should be used to collect incomplete data.

[GerdP]

I've attached an alternative approach which reverts the changes to TestError.

[Don-vip]

I wrote exactly this initially, then was concerned by the API breakage + I found to create an empty fake primitive in JOSM core was ugly.

[GerdP]

The fake primitive is only created in the unit test. The same is done in OpeningHourTestTest.checkOpeningHourSyntax()

[GerdP]

If we allow an empty list of primitives in TestError we can also remove the code for TestError.Builder. The buider pattern was introduced to avoid incomplete TestError instances, right?