[PATCH] mysterious validator message - Member for role '<empty>' does not match 'highway=path || highway=footway || highway=track' (1)

[mkoniecz]

​​osmwww:browse/way/163267076

It is unclear which relation is affected, this way belongs to two different ones.

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2014-03-18 02:36:30
Last Changed Author: Don-vip
Revision: 6910
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2014-03-18 00:57:31 +0100 (Tue, 18 Mar 2014)
Last Changed Rev: 6910

Identification: JOSM/1.5 (6910 en) Windows 7 32-Bit
Memory Usage: 63 MB / 247 MB (17 MB allocated, but free)
Java version: 1.7.0_51, Oracle Corporation, Java HotSpot(TM) Client VM
Dataset consistency test: No problems found

Plugin: OpeningHoursEditor (30235)
Plugin: RoadSigns (30320)
Plugin: buildings_tools (30277)
Plugin: continuosDownload (28565)
Plugin: lakewalker (30277)
Plugin: notes (v0.9.2)
Plugin: wikipedia (30277)

[Don-vip]

the route=hiking can be deduced from message.

[mkoniecz]

Maybe it would better to not force user to deduce what is happening? Especially in cases with multiple route=hiking at the same way (here it is not so bad).

Maybe at least show relation id.

[anatoly77g]

I am another user affected by this bug/feature.
What can I do to make the warning disappear?

This note is supposed to help me, but I don't know how:
"the route=hiking can be deduced from message"

[Don-vip]

Please attach a sample data set.

[anatoly77g]

I can only guess what a "data set" is: I understand it as a bounding box.
Here is a sample bounding box that contains this problem:
min lat 32.7161537
max lat 32.7188798
min lon 35.0836158
max lon 35.0861478
This area contains just two intersecting paths.
Validation reports a bunch of warnings.

Just in case it's needed, here is my Status Report:

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2014-02-27 09:27:48
Last Changed Author: simon04
Revision: 6891
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2014-02-27 08:25:03 +0100 (Thu, 27 Feb 2014)
Last Changed Rev: 6891

Identification: JOSM/1.5 (6891 en) Windows 7 64-Bit
Memory Usage: 123 MB / 494 MB (50 MB allocated, but free)
Java version: 1.7.0_55, Oracle Corporation, Java HotSpot(TM) Client VM
VM arguments: [-Xms16m, -Xmx512m]
Dataset consistency test: No problems found

[anatoly77g]

I did some research; it seems that the validator warnings are caused by the following lines:

<role key="" text="route segment" requisite="required" type="way" member_expression="highway=path OR highway=footway OR highway=track"/>
<role key="" text="infrastructure" requisite="optional" type="node,closedway" member_expression="tourism OR amenity"/>
<role key="" text="natural" requisite="optional" type="node,closedway" member_expression="natural=peak OR natural=volcano OR mountain_pass=yes OR natural=water OR tourism=viewpoint OR amenity=drinking_water OR natural=spring OR place=locality"/>

They are in the file core/data/defaultpresets.xml; were added in changeset 6810.

If I remove the last two lines (leaving only the first of the 3 lines), the warnings go away.

[Don-vip]

In 7061/josm:

fix #9844 - incorrect validator warnings if relation preset contain several empty roles + refactoring

[aceman]

I was also wondering about this message.
Thanks for fixing it.

[wiktorn]

Patch for defect

[wiktorn]

It is still complaining about empty relation type, as for example for:

​http://www.openstreetmap.org/relation/3791666

I've attached the fix for this error, which also provides better information about validation errors.

I'm not sure why, it also fixes roles verification warnings with forward/backward role type for biking routes.

[wiktorn]

Attached patch may fix also: #9992

[simon04]

Thank you for your submission! This patch fixes the issue #10133.

Some remarks:

• checkMemberType can be simplified to r.types.contains(member.getDisplayType())
• For consistency, please use "did not" instead of "didn't"
• The types in the error "... match accepted list of ..." cannot be translated. To fix this, consider this fragment which transforms a TaggingPresetTypes to a nicer string

                      Utils.join("/", Utils.transform(types, new Utils.Function<TaggingPresetType, Object>() {
                          @Override
                          public Object apply(TaggingPresetType x) {
                              return tr(x.getName());
                          }
                      }))
  

• Here, one can also use EnumSet<TaggingPresetType> types = EnumSet.noneOf(TaggingPresetType.class);
• When documenting a method (e.g. checkMemberExpressionAndType), please use the Javadoc style /** Foobar */.
• The deep nesting of loops/conditionals in checkMemberExpressionAndType is hard to follow …
• In the same method, do you really want to return true if a single r.memberExpression == null? (for instance)
• In the same method, the else branch is empty

[wiktorn]

corrected version

[wiktorn]

Thank you for your review. Corrected patch attached.

I've changed order of conditional expressions in checkMemberExpressionAndType, so it should be easier to follow. I don't have yet the idea, how to refactor that part to make it more comprehensive, as I need the possibleError from the innermost condition. I hope that additional comments help understanding how it works.

[simon04]

In 7254/josm:

see #9844 - RelationChecker: add unit tests, fix detected problem concerning role counts

[simon04]

I added some unit test cases for the current implementation of the relation checker. It seems that your patch breaks the following ones:

• org.openstreetmap.josm.data.validation.tests.RelationCheckerTest#testOuter2
• org.openstreetmap.josm.data.validation.tests.RelationCheckerTest#testPowerMemberExpression
• org.openstreetmap.josm.data.validation.tests.RelationCheckerTest#testRestrictionEmpty
• org.openstreetmap.josm.data.validation.tests.RelationCheckerTest#testRestrictionViaRelation

[wiktorn]

patch version 1.2

[wiktorn]

Corrected patch attached.

• testRestrictionViaRelation - I've corrected the presets, so it says, that it is mandatory parameter (as in ​http://wiki.openstreetmap.org/wiki/Relation:restriction) - TaggingPresetItems.getValidCount() returns 0, if count is not set and requisite="required" is set in preset
• I've corrected the tests, to they reflect error messages now returned
• I've corrected bug that relations not present in preset were not detected

[wiktorn]

And one more note:
checkMemberType can't be simplified to r.types.contains(member.getDisplayType()) - because Role.types uses TaggingPresetType and RelationMember.getDisplayType() returns OsmPrimitiveType.

Maybe TaggingPresetType is surplus and should be refactored to OsmPrimitiveType?

[wiktorn]

Replying to simon04:

I added some unit test cases for the current implementation of the relation checker. It seems that your patch breaks the following ones:

• org.openstreetmap.josm.data.validation.tests.RelationCheckerTest#testOuter2
• org.openstreetmap.josm.data.validation.tests.RelationCheckerTest#testPowerMemberExpression
• org.openstreetmap.josm.data.validation.tests.RelationCheckerTest#testRestrictionEmpty
• org.openstreetmap.josm.data.validation.tests.RelationCheckerTest#testRestrictionViaRelation

Any chance that you'll have time to look again into revised patch?

[Don-vip]

In 7884/josm:

fix #9844, fix #9992, fix #10133 - fix relation checker tests against member expressions (modified patch by wiktorn)

[Don-vip]

Thanks for the patch! Sorry for taking this long to incorporate it, this is great job! :)

[Don-vip]

In 7885/josm:

see #9844 - code cleanup