Modify

Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#18956 closed defect (fixed)

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

Reported by: Claudius Owned by: Don-vip
Priority: major Milestone: 20.03
Component: Core validator Version: latest
Keywords: template_report opening_hours regression Cc: simon04

Description (last modified by skyper)

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)

Attachments (2)

BUG.osm (302 bytes) - added by skyper 4 months ago.
example file
18956-alternative.patch (12.4 KB) - added by GerdP 4 months ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 4 months ago by skyper

Component: CoreCore validator
Description: modified (diff)
Keywords: opening_hours regression added
Priority: normalmajor
Version: latest

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).

comment:2 Changed 4 months ago by skyper

Summary: NPE from unknown sourceNPE at org.openstreetmap.josm.data.validation.tests.OpeningHourTest.createTestError

comment:3 Changed 4 months ago by skyper

r16155 from two days ago, is already buggy.

comment:4 Changed 4 months ago by Don-vip

Milestone: 20.03
Owner: changed from team to Don-vip
Status: newassigned

comment:5 Changed 4 months ago by skyper

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

comment:6 in reply to:  5 ; Changed 4 months ago by skyper

EDIT by Don-vip: deleted (wrong information)

Last edited 4 months ago by Don-vip (previous) (diff)

comment:7 Changed 4 months ago by Don-vip

Cc: simon04 added

EDIT: deleted after skyper's next comment

Last edited 4 months ago by Don-vip (previous) (diff)

Changed 4 months ago by skyper

Attachment: BUG.osm added

example file

comment:8 in reply to:  6 ; Changed 4 months ago by 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

Version 0, edited 4 months ago by skyper (next)

comment:9 Changed 4 months ago by skyper

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

comment:10 in reply to:  8 Changed 4 months ago by 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 :)

comment:11 Changed 4 months ago by skyper

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

comment:12 Changed 4 months ago by Don-vip

Resolution: fixed
Status: assignedclosed

In 16178/josm:

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

comment:13 Changed 4 months ago by GerdP

Sounds dangerous. How does the validator tree handle this?

comment:14 Changed 4 months ago by Don-vip

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

comment:15 Changed 4 months ago by GerdP

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

comment:16 Changed 4 months ago by Don-vip

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

comment:17 Changed 4 months ago by GerdP

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

Changed 4 months ago by GerdP

Attachment: 18956-alternative.patch added

comment:18 Changed 4 months ago by GerdP

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

comment:19 Changed 4 months ago by 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.

comment:20 Changed 4 months ago by GerdP

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

comment:21 Changed 4 months ago by 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?

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.