Modify ↓

Opened 11 years ago

Closed 11 years ago

Last modified 7 years 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 11 years ago.
9157b.patch​ (68.2 KB ) - added by simon04 11 years ago.
9157c.patch​ (104.1 KB ) - added by simon04 11 years ago.
9157error.png​ (28.1 KB ) - added by rickmastfan67 11 years ago.
Error when starting r6370 in Windows

Download all attachments as: .zip

Change History (29)

comment:1 by Don-vip, 11 years ago

This would be a nice enhancement.

comment:2 by skyper, 11 years ago

Cc: skyper added
Keywords: opening hour time syntax added

comment:3 by simon04, 11 years ago

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?

by simon04, 11 years ago

Attachment: 9157a.patch​ added

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

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 by simon04, 11 years ago

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.

by simon04, 11 years ago

Attachment: 9157b.patch​ added

comment:6 by simon04, 11 years ago

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 by Don-vip, 11 years ago

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

comment:8 by ypid23, 11 years ago

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 by geri-oc, 11 years ago

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.

in reply to:  9 comment:10 by skyper, 11 years ago

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

by simon04, 11 years ago

Attachment: 9157c.patch​ added

comment:11 by simon04, 11 years ago

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 by Don-vip, 11 years ago

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

comment:13 by simon04, 11 years ago

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

comment:14 by Don-vip, 11 years ago

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 by simon04, 11 years ago

Resolution: → fixed
Status: new → closed

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 by simon04, 11 years ago

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

comment:17 by rickmastfan67, 11 years ago

Resolution: fixed
Status: closed → reopened

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]

by rickmastfan67, 11 years ago

Attachment: 9157error.png​ added

Error when starting r6370 in Windows

comment:18 by simon04, 11 years ago

Resolution: → fixed
Status: reopened → closed

In 6371/josm:

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

comment:19 by ypid23, 11 years ago

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 by simon04, 11 years ago

In 6376/josm:

see #9157 - check collection_times and service_times as well

And fix/add unit tests

comment:21 by simon04, 11 years ago

In 6377/josm:

see #9157 - make some errors in opening_hours fixable

comment:22 by simon04, 11 years ago

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

comment:23 by skyper, 11 years ago

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

comment:24 by Don-vip, 11 years ago

Milestone: → 13.11 (6383)

comment:25 by stoecker, 7 years ago

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. Next status will be 'reopened'.

Add Comment
E-mail address and name can be saved in the Preferences .
Attachments ↑ Description ↑
 
Note: See TracTickets for help on using tickets.