GH-10083: Fix Nullability in some missed packages #10312
Conversation
Related to: spring-projects#10083 * Turn `EntryEventMessagePayload` into `record`. Fix access from tests, respectively * Extract `ParameterExpressionEvaluator` into a top-level class as it is expected by the `ExpressionEvaluatingParameterSource`
| if (jpaParameter.getExpression() != null) { | ||
| private @Nullable Object obtainParameterValue(JpaParameter jpaParameter) { | ||
| Object value = jpaParameter.getValue(); | ||
| if (value == null && jpaParameter.getExpression() != null) { |
There was a problem hiding this comment.
I agree with change. Should we add this as a note, since it is a change in behavior?
There was a problem hiding this comment.
I don’t think so.
I tried to keep outcome of the method the same. Semantically logic is not changed. Just perspective how the value is handled .
What do you think I have broken since it gives an impression that behavior has changed?
There was a problem hiding this comment.
The original behavior would allow the expression to be evaluated regardless if value was null or not (and expression was not null). Now it is only evaluated if value is null and expression is not null.
I'm wondering if people built their code around the fact that they would prefer to get the result from a expression evaluation even if the value is not null.
There was a problem hiding this comment.
Oh! I see. I thought that is else if.
Let me look into this again .
Thank you for spotting!
There was a problem hiding this comment.
So, the contract is this:
/**
* Instantiates a new Jpa Parameter without a name. This is useful for specifying
* positional Jpa parameters.
* @param value If null, the expression property must be set
* @param expression If null, the value property must be set
*/
public JpaParameter(@Nullable Object value, @Nullable String expression) {
Therefore an existing logic is wrong: we really should not go and check for getExression() if getValue() is not null.
It might rely on assumption that expression is not set, but I don't think we can rely on that.
With that I think it is better to honor that contract even more.
…erSourceFactory.obtainParameterValue()`
Related to: #10083
EntryEventMessagePayloadintorecord. Fix access from tests, respectivelyParameterExpressionEvaluatorinto a top-level class as it is expected by theExpressionEvaluatingParameterSource