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 an ability to resolve property placeholders in BasicPersistentEntity #2369

Closed
timsazon opened this issue May 6, 2021 · 6 comments
Closed
Assignees
Labels
type: enhancement A general enhancement

Comments

@timsazon
Copy link

timsazon commented May 6, 2021

It'd be nice to have the ability to resolve placeholders in the entity values.

It can be done by adding PropertyResolver to the entity class.

@mp911de
Copy link
Member

mp911de commented May 10, 2021

Can we take a step back first to understand what you're trying to achieve? Specifically, in Spring Data KeyValue we have support for SpEL expressions that are much more powerful than property resolution.

@mp911de mp911de added the status: waiting-for-feedback We need additional information before we can continue label May 10, 2021
@timsazon
Copy link
Author

timsazon commented May 10, 2021

I want to use application properties placeholders in @KeySpace and @RedisHash annotations, but afaik SpEL expressions can't handle it. For example, @EnableRedisWebSession annotation from Spring Session Data Redis supports such placeholders for namespaces.
You can see a use case in the test: https://github.com/spring-projects/spring-data-keyvalue/pull/375/files#diff-3894631e2abde2a85bfbe3ff79c5eb88c4388932ad2d25907cf0f02d8bf6cbc6R70

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 10, 2021
@mp911de
Copy link
Member

mp911de commented May 11, 2021

Take a look at this example. It's built on top of Spring Data KeyValue and by providing an EvaluationContextExtension you can expose properties that can be resolved/accessed already.

@timsazon
Copy link
Author

In that case, it will require additional configuration. I think it'd be better to support this the same way as @Value or @EnableRedisWebSession do (it doesn't change API anyway).

timsazon added a commit to timsazon/spring-data-commons that referenced this issue May 20, 2021
timsazon added a commit to timsazon/spring-data-keyvalue that referenced this issue May 20, 2021
@mp911de mp911de added this to the 3.x milestone Jul 20, 2021
@mp911de
Copy link
Member

mp911de commented Jul 20, 2021

We decided to postpone the change for now.

@christophstrobl
Copy link
Member

We've been revisiting this issue. The intermitted solution using an EvaluationContextExtension does however not provide an approach we'd like to follow when implementing property placeholder support. The placeholder evaluation should not be tied to any SpEL parsing/evaluation.

Currently we plan to investigate the possibility of extending existing components, like the MappingContext, so they are PropertySource aware an provide facilities for placeholder and potential SpEL resolution.

@mp911de mp911de changed the title Add an ability to resolve property placeholders in BasicPersistentEntity Add an ability to resolve property placeholders in BasicPersistentEntity Feb 2, 2024
mp911de pushed a commit that referenced this issue Feb 12, 2024
We now support Value Expressions such as `#{1+1}-${spring.application.name:fallback-value}` that are composed of SpEL expressions, literals and Property Placeholders.

See #2369
Original pull request: #3036
mp911de added a commit that referenced this issue Feb 12, 2024
Introduce literal, expression and placeholder variants. Add parser for composite expressions.

Closes #2369
Original pull request: #3036
mp911de pushed a commit that referenced this issue Feb 12, 2024
We now support Value Expressions such as `#{1+1}-${spring.application.name:fallback-value}` that are composed of SpEL expressions, literals and Property Placeholders.

See #2369
Original pull request: #3036
@mp911de mp911de added this to the 3.3 M1 (2024.0.0) milestone Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment