Skip to content
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

Add support for Value Expressions for Repository Query methods #4683

Closed
wants to merge 9 commits into from

Conversation

mp911de
Copy link
Member

@mp911de mp911de commented Apr 5, 2024

No description provided.

@mp911de mp911de added the type: enhancement A general enhancement label Apr 5, 2024
@marcingrzejszczak
Copy link
Contributor

This will also fix #2764

Copy link
Member

@christophstrobl christophstrobl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some since and deprecation tags are off and we should update the documentation.

Comment on lines 485 to 523

return new ValueExpressionEvaluator() {

@Override
public <T> T evaluate(String expressionString) {
ValueExpression expression = valueExpressionDelegate.parse(expressionString);
ValueEvaluationContext evaluationContext = valueEvaluationContextProvider.getEvaluationContext(accessor.getValues(),
expression.getExpressionDependencies());
return (T) expression.evaluate(evaluationContext);
}
};
}

/**
* Obtain a {@link Mono publisher} emitting the {@link ValueExpressionEvaluator} suitable to evaluate expressions
* backed by the given dependencies.
*
* @param dependencies must not be {@literal null}.
* @param accessor must not be {@literal null}.
* @return a {@link Mono} emitting the {@link ValueExpressionEvaluator} when ready.
* @since 4.3
*/
protected Mono<ValueExpressionEvaluator> getValueExpressionEvaluatorLater(ExpressionDependencies dependencies,
MongoParameterAccessor accessor) {

return valueEvaluationContextProvider.getEvaluationContextLater(accessor.getValues(), dependencies)
.map(evaluationContext -> {

return new ValueExpressionEvaluator() {
@Override
public <T> T evaluate(String expressionString) {

ValueExpression expression = valueExpressionDelegate.parse(expressionString);

return (T) expression.evaluate(evaluationContext);
}
};
});
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we move the inner ValueExpressionEvaluator to a dedicated class that holds the evaluation context, so we can use for both?

@@ -406,7 +410,7 @@ public void capturingExpressionDependenciesShouldNotThrowParseErrorForSpelOnlyJs
String json = "?#{ true ? { 'name': #name } : { 'name' : #name + 'trouble' } }";

new ParameterBindingDocumentCodec().captureExpressionDependencies(json, (index) -> args[index],
new SpelExpressionParser());
ValueExpressionParser.create(SpelExpressionParser::new));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might make sense to also add some tests to the binding parser and check if the current test setup is making use of value expressions already.

@Test // DATAMONGO-1244
public void shouldSupportExpressionsInCustomQueriesWithNestedObject() {

ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, true, "param1", "param2");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why's param2 gone?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the method declares 2 parameters and previously we passed in 3 arguments.

org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
org.springframework.data.mongodb.core.query.Query reference = new BasicQuery("{'lastname' : 'bar'}");

assertThat(query.getQueryObject()).isEqualTo(reference.getQueryObject());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do we expect when the property is missing and does not have a default?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it should break (and it does right now, I'll add a test for that)

@mp911de mp911de linked an issue Sep 27, 2024 that may be closed by this pull request
@mp911de mp911de added this to the 4.4 RC1 (2024.1.0) milestone Oct 1, 2024
mp911de pushed a commit that referenced this pull request Oct 9, 2024
@mp911de mp911de closed this Oct 9, 2024
@mp911de mp911de deleted the issue/4677 branch October 9, 2024 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for value expressions in repository query methods
3 participants