[DROOLS-4618] Test Scenario: refactor ExpressionEvaluator implementations #2690
Conversation
| @@ -67,6 +69,9 @@ protected boolean isStructuredInput(String className) { | |||
| } | |||
|
|
|||
| protected Object convertResult(String rawString, String className, List<String> genericClasses) { | |||
| if(rawString == null) { | |||
| if (!rawValue.endsWith("]")) { | ||
| throw new IllegalArgumentException(new StringBuilder().append("Malformed expression: ").append(rawValue).toString()); | ||
| } | ||
| return Stream.of(rawValue.substring(1, ((String) raw).length() - 1).split(",")) | ||
| return Stream.of(rawValue.substring(1, (rawValue).length() - 1).split(",")) |
There was a problem hiding this comment.
Please remove parentheses on rawValue variable
There was a problem hiding this comment.
@danielezonca Excellent refactoring, I agree with you this should be considered as first step, the next one is to convert factMappingValue.getRawValue() from Object to String in a future ticket. We will have conflicts with my PR https://github.com/kiegroup/drools/pull/2691, but I think I can resolve them easily. So, I propose to review / test / merge this PR before mine.
| } | ||
| Object expressionResult = compileAndExecute((String) rawExpression, Collections.emptyMap()); | ||
| public Object evaluateLiteralExpression(String rawExpression, String className, List<String> genericClasses) { | ||
| Object expressionResult = compileAndExecute(rawExpression, Collections.emptyMap()); | ||
| Class<Object> requiredClass = loadClass(className, classLoader); | ||
| if (expressionResult != null && !requiredClass.isAssignableFrom(expressionResult.getClass())) { | ||
| throw new IllegalArgumentException("Cannot assign a '" + expressionResult.getClass().getCanonicalName() + |
There was a problem hiding this comment.
Minor thing, we miss ending apostrophe in the exception message.
There was a problem hiding this comment.
Good catch :)
Done
| Object value = expressionEvaluator.evaluateLiteralExpression(factMapping.getClassName(), | ||
| factMapping.getGenericTypes(), | ||
| factMappingValue.getRawValue()); | ||
| Object value = expressionEvaluator.evaluateLiteralExpression((String) factMappingValue.getRawValue(), factMapping.getClassName(), |
There was a problem hiding this comment.
Can we please keep arguments of evaluateLiteralExpression one per line?
There was a problem hiding this comment.
Will we change also signature of evaluateLiteralExpression in future to eliminate this casting?
There was a problem hiding this comment.
We plan to remove the cast in the future changing factMappingValue.rawValue from Object to String
| @@ -301,7 +301,7 @@ protected ResultWrapper getResultWrapper(String className, | |||
| Object resultRaw, | |||
| Class<?> resultClass) { | |||
| try { | |||
| boolean evaluationSucceed = expressionEvaluator.evaluateUnaryExpression(expectedResultRaw, resultRaw, resultClass); | |||
| boolean evaluationSucceed = expressionEvaluator.evaluateUnaryExpression((String) expectedResultRaw, resultRaw, resultClass); | |||
There was a problem hiding this comment.
Will we change also signature of evaluateUnaryExpression in future to eliminate this casting?
There was a problem hiding this comment.
Same as above, after the change of factMappingValue.rawValue from Object to String the cast will be no more needed
Changed
ExpressionEvaluatorinput value toString(it should be possible to refactorfactMappingValue.getRawValue()toStringtoo but impact is big so will be done in the future)@gitgabrio @yesamer @dupliaka
https://issues.redhat.com/browse/DROOLS-4618