Modify

Opened 4 years ago

Closed 4 years ago

Last modified 2 months ago

#9157 closed enhancement (fixed)

[Patch] validate values of opening_hours, collection_times and service_times

Reported by: mkoniecz Owned by: team
Priority: normal Milestone: 13.11
Component: Core validator Version:
Keywords: opening hour time syntax Cc: skyper

Description

It would be an integration of validators from http://www.netzwolf.info/en/cartography/osm/time_domain/

Attachments (4)

9157a.patch (67.5 KB) - added by simon04 4 years ago.
9157b.patch (68.2 KB) - added by simon04 4 years ago.
9157c.patch (104.1 KB) - added by simon04 4 years ago.
9157error.png (28.1 KB) - added by rickmastfan67 4 years ago.
Error when starting r6370 in Windows

Download all attachments as: .zip

Change History (29)

comment:1 Changed 4 years ago by Don-vip

This would be a nice enhancement.

comment:2 Changed 4 years ago by skyper

Cc: skyper added
Keywords: opening hour time syntax added

comment:3 Changed 4 years ago by simon04

Summary: validate values of opening_hours, collection_times and service_times[Patch] validate values of opening_hours, collection_times and service_times

Hi all!

(After a long time, I found some time and interest in contributing to JOSM again.)

Patch adds a validator test which directly uses http://www.netzwolf.info/j/osm/time_domain.js to obtain errors (by using Java's ScriptEngine). Some unit tests successfully pass. I expect little performance issues, since relatively few objects have opening_hours set …

I contacted netzwolf, the author of time_domain.js, with respect to the license (whether it is okay to include time_domain.js).

What do you think?

Changed 4 years ago by simon04

Attachment: 9157a.patch added

comment:4 in reply to:  3 Changed 4 years ago by Don-vip

Replying to simon04:

Hi all!

(After a long time, I found some time and interest in contributing to JOSM again.)

Hi Simon ! Happy to hear news from you :)

Patch adds a validator test which directly uses http://www.netzwolf.info/j/osm/time_domain.js to obtain errors (by using Java's ScriptEngine). Some unit tests successfully pass. I expect little performance issues, since relatively few objects have opening_hours set …

Nice ! I didn't even think about that :)

I contacted netzwolf, the author of time_domain.js, with respect to the license (whether it is okay to include time_domain.js).

I think he will be OK but let's wait for his answer.

What do you think?

It's a good idea and nicely implemented. Only one thing, could you add Javadoc for all public methods ? (except ones with @override). I have setup a Jenkins/Sonar server to improve the quality of our code, Javadoc coverage increases a little bit at each commit :)

comment:5 Changed 4 years ago by simon04

Nice to see some continuous integration at place. :-)
I'm happy to increase the Javadoc stats

I'm waiting for an answer of netzwolf before committing.

Changed 4 years ago by simon04

Attachment: 9157b.patch added

comment:6 Changed 4 years ago by simon04

After my mail, the library time_domain.js has been withdrawn in favour of https://github.com/ypid/opening_hours.js … Looking into that now …

comment:7 Changed 4 years ago by Don-vip

Nice, at least it is easier to watch now :)

comment:8 Changed 4 years ago by ypid23

Thanks. Nice idea to add a validation for opening_hours values. The functions oh.getWarnings() and oh.prettifyValue() are probably also worth a look ;)

Points in time are not supported yet (collection_times). I probably add this feature since there is a use case for this now. https://github.com/AMDmi3/opening_hours.js/issues/12

comment:9 Changed 4 years ago by geri-oc

Wo finde ich eine entsprechend WIKI-Seite über die Verwendung? Ich war sehr erstaunt, einen etablierten Schlüssel nicht mehr benutzen zu können - vor allem wurde konnte er ausgewertet verwendet werden.

comment:10 in reply to:  9 Changed 4 years ago by skyper

Replying to geri-oc:

Wo finde ich eine entsprechend WIKI-Seite über die Verwendung? Ich war sehr erstaunt, einen etablierten Schlüssel nicht mehr benutzen zu können - vor allem wurde konnte er ausgewertet verwendet werden.

Sorry, verstehe nicht was Du uns mitteilen willst. Bisher wurde nichts geändert sondern nur diskutiert bzw mögliche Änderungen als Patch bereitgestellt. Falls Du in der aktuellen Version von JOSM etwas vermissen solltest, schreibe doch bitte ein neues Ticket. - Danke

Changed 4 years ago by simon04

Attachment: 9157c.patch added

comment:11 Changed 4 years ago by simon04

Now using the https://github.com/ypid/opening_hours.js library to validate opening_hours. Also added the library to CONTRIBUTION file. Removed validation for collection_times, service_times, since those are not supported at the moment.

comment:12 Changed 4 years ago by Don-vip

I think we could commit this patch, no ? (Maybe after updating the .js I see the library has been updated recently)

comment:13 Changed 4 years ago by simon04

Depending on DevelopersGuide/Schedule, but I guess we can give it a try. :-)

comment:14 Changed 4 years ago by Don-vip

It will hardly break anything and it's a nice feature to bring in next tested :) We have a few days left for stabilization anyway.

comment:15 Changed 4 years ago by simon04

Resolution: fixed
Status: newclosed

In 6370/josm:

fix #9157 - add opening_hours validation test

This test utilizes https://github.com/ypid/opening_hours.js which is licensed
under the New (2-clause) BSD license.

comment:16 Changed 4 years ago by simon04

Btw: The new version of opening_hours.js broke 3 out of 5 unit tests. After some quick fixes they succeeded again … :-)

comment:17 Changed 4 years ago by rickmastfan67

Resolution: fixed
Status: closedreopened

This new edit is popping up an error on startup of JOSM. Tested with a brand new profile and my normal profile. Got this error both times. Doesn't seem to break anything that I can tell, but should be fixed anyways.

INFO: Missing preference file '\AppData\Roaming\JOSM\preferences.xml'. Creating a default preference file.
GET http://api.openstreetmap.org/api/capabilities... INFO: OK
javax.script.ScriptException: sun.org.mozilla.javascript.internal.EvaluatorException: illegal character (<Unknown source>#1185) in <Unknown source> at line number 1185
        at com.sun.script.javascript.RhinoScriptEngine.eval(Unknown Source)
        at javax.script.AbstractScriptEngine.eval(Unknown Source)
        at org.openstreetmap.josm.data.validation.tests.OpeningHourTest.initialize(OpeningHourTest.java:50)
        at org.openstreetmap.josm.data.validation.OsmValidator.initializeTests(OsmValidator.java:291)
        at org.openstreetmap.josm.data.validation.OsmValidator.<init>(OsmValidator.java:126)
        at org.openstreetmap.josm.Main$2.call(Main.java:441)
        at org.openstreetmap.josm.Main$2.call(Main.java:428)
        at java.util.concurrent.FutureTask.run(Unknown Source)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
        at java.lang.Thread.run(Unknown Source)
Caused by: sun.org.mozilla.javascript.internal.EvaluatorException: illegal character (<Unknown source>#1185)
        at sun.org.mozilla.javascript.internal.DefaultErrorReporter.runtimeError(Unknown Source)
        at sun.org.mozilla.javascript.internal.DefaultErrorReporter.error(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.addError(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.addError(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.addError(Unknown Source)
        at sun.org.mozilla.javascript.internal.TokenStream.getToken(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.peekToken(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.objectLiteral(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.primaryExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.memberExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.unaryExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.mulExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.addExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.shiftExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.relExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.eqExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.bitAndExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.bitXorExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.bitOrExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.andExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.orExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.condExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.assignExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.plainProperty(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.objectLiteral(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.primaryExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.memberExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.unaryExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.mulExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.addExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.shiftExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.relExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.eqExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.bitAndExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.bitXorExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.bitOrExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.andExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.orExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.condExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.assignExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.plainProperty(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.objectLiteral(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.primaryExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.memberExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.unaryExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.mulExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.addExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.shiftExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.relExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.eqExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.bitAndExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.bitXorExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.bitOrExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.andExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.orExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.condExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.assignExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.variables(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.statementHelper(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.statement(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.parseFunctionBody(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.function(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.primaryExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.memberExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.unaryExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.mulExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.addExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.shiftExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.relExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.eqExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.bitAndExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.bitXorExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.bitOrExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.andExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.orExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.condExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.assignExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.expr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.parenExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.primaryExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.memberExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.unaryExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.mulExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.addExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.shiftExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.relExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.eqExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.bitAndExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.bitXorExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.bitOrExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.andExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.orExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.condExpr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.assignExpr(Unknown Source)

        at sun.org.mozilla.javascript.internal.Parser.expr(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.statementHelper(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.statement(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.parse(Unknown Source)
        at sun.org.mozilla.javascript.internal.Parser.parse(Unknown Source)
        at sun.org.mozilla.javascript.internal.Context.compileImpl(Unknown Source)
        at sun.org.mozilla.javascript.internal.Context.compileReader(Unknown Source)
        at sun.org.mozilla.javascript.internal.Context.compileReader(Unknown Source)
        at sun.org.mozilla.javascript.internal.Context.evaluateReader(Unknown Source)
        ... 11 more
Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2013-11-07 02:35:06
Last Changed Author: simon04
Revision: 6370
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2013-11-06 21:20:01 +0100 (Wed, 06 Nov 2013)
Last Changed Rev: 6370

Identification: JOSM/1.5 (6370 en) Windows 7 64-Bit
Memory Usage: 156 MB / 1820 MB (76 MB allocated, but free)
Java version: 1.7.0_45, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
VM arguments: [-Xmx2048M]

Changed 4 years ago by rickmastfan67

Attachment: 9157error.png added

Error when starting r6370 in Windows

comment:18 Changed 4 years ago by simon04

Resolution: fixed
Status: reopenedclosed

In 6371/josm:

fix #9157 - read opening_hours.js in UTF-8

comment:19 Changed 4 years ago by ypid23

Good evening

I have tested it and it works great :)

You might want to add a default location and country as I did in unit tests for opening_hours.js and other test programs: https://github.com/ypid/opening_hours.js/blob/master/interactive_testing.js

This way, opening_hours like PH open "always open on public holidays" would pass the validation.

Next thing is that I added support for collection_times and service_times some days ago. See the mode parameter and the demo page for this.

And one feature request :) The library is able to fix most warnings itself with the function oh.prettifyValue. See the demo page: http://robin.de.marissa.hostorama.ch/osm/opening_hours.js/demo.html

Keep up the good work.

comment:20 Changed 4 years ago by simon04

In 6376/josm:

see #9157 - check collection_times and service_times as well

And fix/add unit tests

comment:21 Changed 4 years ago by simon04

In 6377/josm:

see #9157 - make some errors in opening_hours fixable

comment:22 Changed 4 years ago by simon04

ypid23, thank you for your suggestions; I guess, we now got a reasonable implementation.

comment:23 Changed 4 years ago by skyper

Ticket #8291 has been marked as a duplicate of this ticket.

comment:24 Changed 4 years ago by Don-vip

Milestone: 13.11 (6383)

comment:25 Changed 2 months ago by stoecker

Milestone: 13.11 (6383)13.11

Milestone renamed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
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.