-
Notifications
You must be signed in to change notification settings - Fork 0
Warnings feature - configuration part #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: add-warnings
Are you sure you want to change the base?
Changes from all commits
edbc0f9
d9cb4b5
b899f86
c8f9e10
794da12
6e5244d
c966c3f
7fc3c45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
package btools.router.warning; | ||
|
||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
class MessageDataWarnings { | ||
|
||
// maybe empty | ||
Map<String, String> nodeTags; | ||
// maybe empty | ||
Map<String, String> wayTags; | ||
|
||
MessageDataWarnings(String nodeKeyValues, String wayKeyValues) { | ||
nodeTags = buildTags(nodeKeyValues); | ||
wayTags = buildTags(wayKeyValues); | ||
} | ||
|
||
// convert wayKeyValues, nodeKeyValues into fast lookup data | ||
// sample of keyValues | ||
// highway=path sac_scale=mountain_hiking route_hiking_rwn=yes | ||
// we assume no multiple values for single key | ||
// TODO Is it possible to use "all values for a key" syntax in the | ||
// TODO lookup files? e.g. highway=* | ||
// TODO this will be necessary for e.g. a seasonal access parsing. | ||
// TODO Values like key=v1;v2;v3 | ||
// TODO https://wiki.openstreetmap.org/wiki/Multiple_values | ||
// TODO If we specify 'processUnusedTags = true', will such a value | ||
// TODO be visible here? | ||
|
||
private static final String WHITESPACE_SEPARATOR = "[ ]"; | ||
private static final String EQUALS_SEPARATOR = "[=]"; | ||
|
||
private Map<String, String> buildTags(String keyValues) { | ||
Map<String, String> keyValuesMap = new HashMap<>(); | ||
if (keyValues != null && !keyValues.isBlank()) { | ||
String[] tokens = keyValues.trim().split(WHITESPACE_SEPARATOR); | ||
for (String token : tokens) { | ||
String[] kv = token.split(EQUALS_SEPARATOR); | ||
if (kv.length == 2) { | ||
keyValuesMap.put(kv[0].trim(), kv[1].trim()); | ||
} else { | ||
System.out.println("Unexpected token in: " + keyValues); | ||
} | ||
} | ||
} | ||
return keyValuesMap; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
package btools.router.warning.impl; | ||
|
||
interface Detector { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
package btools.router.warning.impl; | ||
|
||
class FordDetector implements Detector { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
package btools.router.warning; | ||
|
||
import org.junit.Assert; | ||
import org.junit.Test; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
public class MessageDataWarningsTest { | ||
|
||
private static class TestResource { | ||
String n; | ||
String w; | ||
int sizeN; | ||
int sizeW; | ||
String message; | ||
|
||
public TestResource(String n, String w, int sizeN, int sizeW, String message) { | ||
this.n = n; | ||
this.w = w; | ||
this.sizeN = sizeN; | ||
this.sizeW = sizeW; | ||
this.message = message; | ||
} | ||
} | ||
|
||
@Test | ||
public void testParseIntoStructureAssertSizes() { | ||
List<TestResource> resources = new ArrayList<>(); | ||
resources.add(new TestResource(null, null, 0, 0, "Empty for null values")); | ||
resources.add(new TestResource("", " ", 0, 0, "Empty for empty or blank values")); | ||
resources.add(new TestResource("key", "key-value", 0, 0, "Empty for not key=value")); | ||
resources.add(new TestResource("key", "key-value", 0, 0, "Empty for not key=value")); | ||
resources.add(new TestResource("key=value \n\t", " key=value key=value ", 1, 1, "Expected 1")); | ||
for (TestResource tr : resources) { | ||
MessageDataWarnings w = new MessageDataWarnings(tr.n, tr.w); | ||
Assert.assertTrue(tr.message, w.nodeTags.size() == tr.sizeN && w.wayTags.size() == tr.sizeW); | ||
} | ||
} | ||
|
||
@Test | ||
public void testSpecificCase(){ | ||
MessageDataWarnings w = new MessageDataWarnings("key=value \n\t", " key0=value0 key0=value1 key1=value2 "); | ||
Assert.assertEquals("value", w.nodeTags.get("key")); | ||
Assert.assertEquals("value1", w.wayTags.get("key0")); | ||
Assert.assertEquals("value2", w.wayTags.get("key1")); | ||
} | ||
|
||
@Test | ||
public void testNoUnexpectedToken(){ | ||
// eyeball test there is no warning printed for proper syntax | ||
MessageDataWarnings w = new MessageDataWarnings("key10=value10", "key20=value20 key20=value10 key100=value200"); | ||
Assert.assertEquals(2, w.wayTags.size()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
package btools.expressions; | ||
|
||
import java.util.StringTokenizer; | ||
import java.util.*; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; | ||
|
||
final class BExpression { | ||
private static final int OR_EXP = 10; | ||
|
@@ -25,7 +27,8 @@ final class BExpression { | |
private static final int VARIABLE_EXP = 34; | ||
private static final int FOREIGN_VARIABLE_EXP = 35; | ||
private static final int VARIABLE_GET_EXP = 36; | ||
|
||
private static final int WARNINGS_EXP = 37; | ||
private static final String WARNINGS_DELIMITERS_REGEX_PATTERN = "[&]"; | ||
private int typ; | ||
private BExpression op1; | ||
private BExpression op2; | ||
|
@@ -34,6 +37,44 @@ final class BExpression { | |
private int variableIdx; | ||
private int lookupNameIdx; | ||
private int[] lookupValueIdxArray; | ||
private String warnings; | ||
|
||
public BExpression() { | ||
} | ||
|
||
// for tests | ||
protected BExpression(String warnings) { | ||
this.warnings = warnings; | ||
} | ||
|
||
private boolean isValidWarningToken(String token) { | ||
return token != null && !token.isBlank(); | ||
// maybe more validating conditions here? | ||
} | ||
|
||
// Converts the raw value assigned to specially treated variable 'warnings' | ||
// into a set of identifiers. | ||
// See WARNINGS_DELIMITERS_REGEX. | ||
// Always returns a set, maybe an empty set | ||
public Set<String> parseWarnings() { | ||
if (warnings == null) { | ||
return new HashSet<>(); | ||
} else { | ||
// do parse | ||
Map<Boolean, List<String>> validityGroups = Stream | ||
.of(warnings.split(WARNINGS_DELIMITERS_REGEX_PATTERN)) | ||
.collect(Collectors.groupingBy(this::isValidWarningToken)); | ||
List<String> invalid = validityGroups.get(false); | ||
if (invalid != null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it desired to continue processing if warning rule is invalid? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we want to continue. Another question is: Do we need validation of warning identifiers at all? Explanation: We say for example assign warnings = walk.danger.exposure&walk.danger.slippery&walk.access.seasonal. This or similar expression is hard-coded in many profile files. Now we find there is an error in (possibly complex) implementation of seasonal access warning functionality. Here we can disable warnings for seasonal access all at once. (Ignore it, but log it). That was my reasoning. |
||
System.out.println("Invalid warning identifier(s): " + invalid); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this standard approach to log? ... no logging system used at all? :) |
||
} | ||
List<String> valid = validityGroups.get(true); | ||
if (valid == null) return new HashSet<>(); | ||
return valid.stream() | ||
.flatMap(v -> Stream.of(v.trim())) // " w1&w2 " | ||
.collect(Collectors.toSet()); | ||
} | ||
} | ||
|
||
// Parse the expression and all subexpression | ||
public static BExpression parse(BExpressionContext ctx, int level) throws Exception { | ||
|
@@ -107,13 +148,24 @@ private static BExpression parse(BExpressionContext ctx, int level, String optio | |
throw new IllegalArgumentException("variable name cannot contain '=': " + variable); | ||
if (variable.indexOf(':') >= 0) | ||
throw new IllegalArgumentException("cannot assign context-prefixed variable: " + variable); | ||
exp.variableIdx = ctx.getVariableIdx(variable, true); | ||
if (exp.variableIdx < ctx.getMinWriteIdx()) | ||
throw new IllegalArgumentException("cannot assign to readonly variable " + variable); | ||
// specially treated variable: warnings | ||
if ("warnings".equals(variable)) { | ||
String token = ctx.parseToken(); | ||
// we need to support both assign warnings=value and assign warnings value syntax | ||
if (token.equals("=")) { | ||
token = ctx.parseToken(); | ||
} | ||
exp.warnings = token; | ||
exp.typ = WARNINGS_EXP; | ||
} else { | ||
exp.variableIdx = ctx.getVariableIdx(variable, true); | ||
if (exp.variableIdx < ctx.getMinWriteIdx()) | ||
throw new IllegalArgumentException("cannot assign to readonly variable " + variable); | ||
} | ||
} else if ("not".equals(operator)) { | ||
exp.typ = NOT_EXP; | ||
} else { | ||
nops = 0; // check elemantary expressions | ||
nops = 0; // check elementary expressions | ||
int idx = operator.indexOf('='); | ||
if (idx >= 0) { | ||
exp.typ = LOOKUP_EXP; | ||
|
@@ -176,7 +228,9 @@ private static BExpression parse(BExpressionContext ctx, int level, String optio | |
} | ||
// parse operands | ||
if (nops > 0) { | ||
exp.op1 = BExpression.parse(ctx, level + 1, exp.typ == ASSIGN_EXP ? "=" : null); | ||
if (exp.typ != WARNINGS_EXP) { | ||
exp.op1 = BExpression.parse(ctx, level + 1, exp.typ == ASSIGN_EXP ? "=" : null); | ||
} | ||
} | ||
if (nops > 1) { | ||
if (ifThenElse) checkExpectedToken(ctx, "then"); | ||
|
@@ -228,6 +282,8 @@ public float evaluate(BExpressionContext ctx) { | |
return op1.evaluate(ctx) != 0.f ? op2.evaluate(ctx) : op3.evaluate(ctx); | ||
case ASSIGN_EXP: | ||
return ctx.assign(variableIdx, op1.evaluate(ctx)); | ||
case WARNINGS_EXP: | ||
return (float) -1.0; | ||
case LOOKUP_EXP: | ||
return ctx.getLookupMatch(lookupNameIdx, lookupValueIdxArray); | ||
case NUMBER_EXP: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[minor] Probably no need to create new instance?
Collections.emptySet()