Index: src/org/openstreetmap/josm/data/validation/tests/ConditionalKeys.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/ConditionalKeys.java (revision 18822) +++ src/org/openstreetmap/josm/data/validation/tests/ConditionalKeys.java (working copy) @@ -6,11 +6,15 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.validation.Severity; @@ -26,10 +30,21 @@ private final OpeningHourTest openingHourTest = new OpeningHourTest(); private static final Set RESTRICTION_TYPES = new HashSet<>(Arrays.asList("oneway", "toll", "noexit", "maxspeed", "minspeed", - "maxstay", "maxweight", "maxaxleload", "maxheight", "maxwidth", "maxlength", "overtaking", "maxgcweight", "maxgcweightrating", - "fee", "restriction", "interval", "duration", "dog", "maxweightrating")); + "maxweight", "maxaxleload", "maxheight", "maxwidth", "maxlength", "overtaking", "maxgcweight", "maxgcweightrating", + "interval", "duration", "dog", "maxweightrating")); + + private static final Set PARKING_RESTRICTION_TYPES = new HashSet<>(Arrays.asList("authentication:disc", + "authentication:ticket", "charge", "fee", "maxstay", "restriction", "restriction:reason")); + + private static final Set ALL_RESTRICTION_TYPES = Stream.concat(RESTRICTION_TYPES.stream(), + PARKING_RESTRICTION_TYPES.stream()).collect(Collectors.toSet()); + private static final Set RESTRICTION_VALUES = new HashSet<>(Arrays.asList("yes", "official", "designated", "destination", "delivery", "customers", "permissive", "private", "agricultural", "forestry", "no")); + + /** + * transport access mode restriction + */ private static final Set TRANSPORT_MODES = new HashSet<>(Arrays.asList("access", "foot", "ski", "inline_skates", "ice_skates", "horse", "vehicle", "bicycle", "carriage", "trailer", "caravan", "motor_vehicle", "motorcycle", "moped", "mofa", "motorcar", "motorhome", "psv", "bus", "taxi", "tourist_bus", "goods", "hgv", "agricultural", "atv", "snowmobile", @@ -36,6 +51,12 @@ "hgv_articulated", "ski:nordic", "ski:alpine", "ski:telemark", "coach", "golf_cart" /*,"minibus","share_taxi","hov","car_sharing","emergency","hazmat","disabled"*/)); + private static final Set DIRECTIONS = new HashSet<>(Arrays.asList("forward", "backward", "both_ways")); + + private static final Set SIDES = new HashSet<>(Arrays.asList("left", "right", "both")); + + private static final Map KNOWN_CONDITIONAL_KEYS = buildKnownKeys(); + private static final Pattern CONDITIONAL_PATTERN; static { final String part = Pattern.compile("([^@\\p{Space}][^@]*?)" @@ -56,15 +77,54 @@ openingHourTest.initialize(); } - /** - * Check if the key is a key for an access restriction - * @param part The key (or the restriction part of it, e.g. for lanes) - * @return true if it is a restriction - */ - public static boolean isRestrictionType(String part) { - return RESTRICTION_TYPES.contains(part); + private enum KeyCharacteristic { + ACCESS, + OTHER } + private static Map buildKnownKeys() { + // [:][:]:conditional + Map knownKeys = new HashMap<>(); + for (String restrictionType : ALL_RESTRICTION_TYPES) { + knownKeys.put(String.format("%s:conditional", restrictionType), KeyCharacteristic.OTHER); + for (String mode : TRANSPORT_MODES) { + knownKeys.put(String.format("%s:%s:conditional", restrictionType, mode), KeyCharacteristic.OTHER); + for (String direction : DIRECTIONS) { + knownKeys.put(String.format("%s:%s:%s:conditional", restrictionType, mode, direction), + KeyCharacteristic.OTHER); + } + } + for (String direction : DIRECTIONS) { + knownKeys.put(String.format("%s:%s:conditional", restrictionType, direction), KeyCharacteristic.OTHER); + } + } + // [:]:conditional + for (String mode : TRANSPORT_MODES) { + knownKeys.put(String.format("%s:conditional", mode), KeyCharacteristic.ACCESS); + for (String direction : DIRECTIONS) { + knownKeys.put(String.format("%s:%s:conditional", mode, direction), KeyCharacteristic.ACCESS); + } + } + // parking::[: : :conditional + for (String side : SIDES) { + for (String mode : TRANSPORT_MODES) { + knownKeys.put(String.format("parking:%s:%s:conditional", side, mode), KeyCharacteristic.ACCESS); + } + } + return knownKeys; + } + /** * Check if the value is a valid restriction value * @param part The value @@ -75,52 +135,14 @@ } /** - * Checks if the key denotes a - * transport access mode restriction - * @param part The key (or the restriction part of it, e.g. for lanes) - * @return true if it is a restriction - */ - public static boolean isTransportationMode(String part) { - return TRANSPORT_MODES.contains(part); - } - - /** - * Check if a key part is a valid direction - * @param part The part of the key - * @return true if it is a direction - */ - public static boolean isDirection(String part) { - return "forward".equals(part) || "backward".equals(part); - } - - /** * Checks if a given key is a valid access key * @param key The conditional key * @return true if the key is valid */ public boolean isKeyValid(String key) { - // [:][:]:conditional - // -- or -- [:]:conditional - if (!key.endsWith(":conditional")) { - return false; - } - final String[] parts = key.replace(":conditional", "").split(":", -1); - return isKeyValid3Parts(parts) || isKeyValid1Part(parts) || isKeyValid2Parts(parts); + return KNOWN_CONDITIONAL_KEYS.containsKey(key); } - private static boolean isKeyValid3Parts(String... parts) { - return parts.length == 3 && isRestrictionType(parts[0]) && isTransportationMode(parts[1]) && isDirection(parts[2]); - } - - private static boolean isKeyValid2Parts(String... parts) { - return parts.length == 2 && ((isRestrictionType(parts[0]) && (isTransportationMode(parts[1]) || isDirection(parts[1]))) - || (isTransportationMode(parts[0]) && isDirection(parts[1]))); - } - - private static boolean isKeyValid1Part(String... parts) { - return parts.length == 1 && (isRestrictionType(parts[0]) || isTransportationMode(parts[0])); - } - /** * Check if a value is valid * @param key The key the value is for @@ -195,7 +217,8 @@ try { for (final ConditionalValue conditional : ConditionalValue.parse(value)) { // validate restriction value - if (isTransportationMode(key.split(":", -1)[0]) && !isRestrictionValue(conditional.restrictionValue)) { + KeyCharacteristic keyCharacteristic = KNOWN_CONDITIONAL_KEYS.get(key); + if (keyCharacteristic == KeyCharacteristic.ACCESS && !isRestrictionValue(conditional.restrictionValue)) { return tr("{0} is not a valid restriction value", conditional.restrictionValue); } // validate opening hour if the value contains an hour (heuristic) Index: test/unit/org/openstreetmap/josm/data/validation/tests/ConditionalKeysTest.java =================================================================== --- test/unit/org/openstreetmap/josm/data/validation/tests/ConditionalKeysTest.java (revision 18822) +++ test/unit/org/openstreetmap/josm/data/validation/tests/ConditionalKeysTest.java (working copy) @@ -7,6 +7,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.openstreetmap.josm.testutils.JOSMTestRules; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -35,22 +37,47 @@ } /** - * Unit test of {@link ConditionalKeys#isKeyValid}. + * Unit tests of {@link ConditionalKeys#isKeyValid}. */ - @Test - void testKeyValid() { - assertTrue(test.isKeyValid("dog:conditional")); - assertTrue(test.isKeyValid("maxspeed:conditional")); - assertTrue(test.isKeyValid("motor_vehicle:conditional")); - assertTrue(test.isKeyValid("bicycle:conditional")); - assertTrue(test.isKeyValid("overtaking:hgv:conditional")); - assertTrue(test.isKeyValid("maxspeed:hgv:backward:conditional")); - assertTrue(test.isKeyValid("oneway:backward:conditional")); - assertTrue(test.isKeyValid("fee:conditional")); - assertTrue(test.isKeyValid("restriction:conditional")); - assertFalse(test.isKeyValid("maxspeed:hgv:conditional:backward")); + @ValueSource(strings = { "dog:conditional", "maxspeed:conditional", "overtaking:hgv:conditional", + "maxspeed:hgv:backward:conditional", "oneway:backward:conditional", "fee:conditional", + "restriction:conditional" }) + @ParameterizedTest + void testKeyIsValid(String key) { + assertTrue(test.isKeyValid(key)); } + @ValueSource(strings = { "bicycle:conditional", "motor_vehicle:conditional", "ski:alpine:conditional" }) + @ParameterizedTest + void testTransportationModeKeyIsValid(String key) { + assertTrue(test.isKeyValid(key)); + } + + @ValueSource(strings = { "parking:both:maxstay:conditional", "parking:left:access:conditional", + "parking:right:restriction:conditional", "parking:right:restriction:reason:conditional" }) + @ParameterizedTest + void testParkingRestrictionBySideKeyIsValid(String key) { + assertTrue(test.isKeyValid(key)); + } + + @ValueSource(strings = { "parking:both:maxstay:hgv:conditional", + "parking:right:authentication:ticket:conditional" }) + @ParameterizedTest + void testParkingRestrictionBySideWithModeKeyIsValid(String key) { + assertTrue(test.isKeyValid(key)); + } + + @ValueSource(strings = { ":conditional", // no base key + "maxspeed:hgv:conditional:backward", // direction must be before :conditional + "parking:forward:maxstay:conditional", // parking side must be both, left or right + "parking:both:fee:forward:conditional", // parking can not have a direction + "parking:left:oneway:conditional" // oneway is not a parking restriction + }) + @ParameterizedTest + void testKeyIsInvalid(String key) { + assertFalse(test.isKeyValid(key)); + } + /** * Unit test of {@link ConditionalKeys#isValueValid}. */ @@ -65,7 +92,9 @@ assertTrue(test.isValueValid("maxspeed:conditional", "120 @ (06:00-20:00); 100 @ (22:00-06:00)")); assertTrue(test.isValueValid("motor_vehicle:conditional", "delivery @ (Mo-Fr 06:00-11:00,17:00-19:00;Sa 03:30-19:00)")); assertTrue(test.isValueValid("motor_vehicle:conditional", "no @ (10:00-18:00 AND length>5)")); + assertTrue(test.isValueValid("parking:right:motor_vehicle:conditional", "no @ (10:00-18:00 AND length>5)")); assertFalse(test.isValueValid("motor_vehicle:conditional", "foo @ (10:00-18:00 AND length>5)")); + assertFalse(test.isValueValid("parking:right:motor_vehicle:conditional", "foo @ (10:00-18:00 AND length>5)")); assertFalse(test.isValueValid("motor_vehicle:conditional", "no @ (10:00until18:00 AND length>5)")); assertTrue(test.isValueValid("maxspeed:hgv:conditional", "60 @ (weight>7.5)")); assertTrue(test.isValueValid("restriction:conditional", "no_left_turn @ (Mo-Fr 16:00-18:00)"));