refactor(filter): Filter use now always 'simple', not jsonpath #12
Conversation
A JsonSimplePredicate has been introduced which temporarily tries to convert a string to a JSON map / list so that the simple language expression can be applied to it.
| new File(folder, FILE_NAME), | ||
| new File(new File(folder, "config"), FILE_NAME) | ||
| }) { | ||
| if (file.exists() && file.isFile()) { |
There was a problem hiding this comment.
would it make sense to add file.canRead() also here?
| @@ -92,34 +99,25 @@ protected static SyndesisModel validateConfig(SyndesisModel config, URL url) { | |||
| /** | |||
| * Tries to find the configuration from the current directory or a parent folder. | |||
| */ | |||
| public static SyndesisModel findFromFolder(File folder) throws IOException { | |||
| public static SyndesisModel tryFindFromFolder(File folder) throws IOException { | |||
There was a problem hiding this comment.
Perhaps use Optional<SyndesisModel> here?
There was a problem hiding this comment.
Added an with optional
| return model; | ||
| } | ||
| } | ||
| throw new IOException("syndesis.yml could not be found in current directory or parent directories and is not on the classpath"); | ||
| } | ||
|
|
||
|
|
||
| protected static SyndesisModel loadFromFile(File file) throws IOException { |
There was a problem hiding this comment.
Perhaps return Optional<SyndesisModel>
| new File(folder, FILE_NAME), | ||
| new File(new File(folder, "config"), FILE_NAME) | ||
| }) { | ||
| if (file.exists() && file.isFile()) { |
There was a problem hiding this comment.
Do we also need file.canRead() here?
| return answer; | ||
| } | ||
| throw new IOException("SyndesisModel configuration folder does not exist: " + folder.getPath()); | ||
| return null; | ||
| } | ||
|
|
||
| protected static SyndesisModel tryFindConfigOnClassPath() throws IOException { |
There was a problem hiding this comment.
Return Optional<SyndesisModel>?
| } else { | ||
| message.append(".main()"); | ||
| } | ||
| message.append(functionName).append(".").append(method != null ? method : "main").append("()"); |
There was a problem hiding this comment.
I prefer using Optional, but it's a style preference, something like: Optional.ofNullable(method).or("main")
There was a problem hiding this comment.
old guys like tertiary operators ;-) but ok, let's try that hipster thing.
| message.append(name); | ||
| message.append("() "); | ||
|
|
||
| if (trigger.equals("http")) { | ||
| trigger = DEFAULT_HTTP_ENDPOINT_PREFIX; | ||
| } else if (trigger.startsWith("http:") || trigger.startsWith("https:") || | ||
| trigger.startsWith("http://") || trigger.startsWith("https://")) { | ||
| } else if (trigger.startsWith("http:") || trigger.startsWith("https:")) { |
There was a problem hiding this comment.
I wonder if the original intent was to support uri as a component URI or uri as a HTTP URL. In camel there are many HTTP components that have their component URI starting with .*http.*: (like http4:... or netty-http:...). Perhaps a comment here would benefit future readers of the code.
There was a problem hiding this comment.
Actually my reasoning was purely based on the observation that if the uri starts with http:// it also starts with http:, so the last two terms are useless.
No idea what the original intent was (but this was confusing to me). I should have resisted but I had simply to clean up :)
| @@ -281,6 +261,15 @@ protected RouteDefinition fromOrTo(RouteDefinition route, String name, String ur | |||
| return route; | |||
| } | |||
|
|
|||
| private String stripPrefix(String trigger, String ... prefixes) { | |||
There was a problem hiding this comment.
Return Optional; even for private method?
There was a problem hiding this comment.
thats all old funktion code originally, I just inherited the baggage.
The logic is a bit complex here, so
| // Therefor we set a map instead of the string | ||
| Map jsonDocument = jsonStringAsMap((String) msgBody, exchange); | ||
| if (jsonDocument != null) { | ||
| exchangeToCheck = ExchangeHelper.createCopy(exchange, true); |
There was a problem hiding this comment.
So the idea is to have simpleExpression work on the parsed JSON instead of the original exchange, but keep the original exchange (the IN message) intact? Perhaps a comment to attest to that could help here?
There was a problem hiding this comment.
well, that's the actual PR. You are right, the intention is that only the predicate acts on the converted value, but the original in message still continues to carry the same format as the filter is not supposed to change anything within the exchange.
I added a clarifying comment.
|
Fixes #11 |
|
thx :) |
A JsonSimplePredicate has been introduced which temporarily tries to convert a string to a JSON map / list so that the simple language expression can be applied to it.