Add ValueExpression infrastructure #3036

mp911de · 2024-02-02T11:08:17Z

We now support parsing of value expressions in the context of persistent entities and persistent properties. Value expressions are literals, simple expressions or composite expressions that can refer to SpEL expressions or property placeholders resolved against the Environment:

@Document("my-collection")
@Document("#{tenantService.getCollectionName()}")
@Document("${tenant-config.collection}")
@Document("orders-#{tenantService.getCollectionName()}-${tenant-config.collection}")

Closes #2369

christophstrobl · 2024-02-05T13:12:47Z

src/main/antora/modules/ROOT/pages/value-expressions.adoc

+Value Expressions a composition of {spring-framework-docs}/core/expressions.html[Spring Expression Language (SpEL)] and {spring-framework-docs}/core/beans/environment.html#beans-placeholder-resolution-in-statements[Property Placeholder Resolution].
+Value Expressions a combine powerful evaluation of programmatic expressions with the simplicity to resort to property-placeholder resolution to obtain values from the `Environment` such as configuration properties.


Those do not read well, starting sentences right after one another with same Value Expressions. 1st one also misses a verb and in the 2nd the a should be removed.

christophstrobl · 2024-02-05T13:14:34Z

src/main/antora/modules/ROOT/pages/value-expressions.adoc

+<1> Value Expression using a single SpEL Expression.
+<2> Value Expression using a static SpEL Expression evaluating to `2-hello-world`.
+<3> Value Expression using a single Property Placeholder.
+<4> Composite expression comprised from the literal `orders-` and the Property Placeholder `${tenant-config.suffix}`.


should this be comprised of?

christophstrobl · 2024-02-05T13:16:05Z

src/main/antora/modules/ROOT/pages/value-expressions.adoc

+====
+
+Using value expressions introduces a lot of flexibility to your code.
+Keep in mind that using value expressions requires evaluation of the expression on each usage.


should we stress this a bit more by moving it to a dedicated callout?

christophstrobl · 2024-02-06T07:12:45Z

src/test/java/org/springframework/data/util/ValueEvaluationUnitTests.java

+		assertThat(eval("#{\"foo\"}-")).isEqualTo("foo-");
+		assertThat(eval("#{\"fo'o\"}-")).isEqualTo("fo'o-");
+		assertThat(eval("${key.one}-")).isEqualTo("value-");
+		assertThat(eval("#{#foo}")).isEqualTo("bar");


we should also add tests that reference properties that do not exist.

value defaulting would also be good to check ${non-existing.key:defaultValue}

christophstrobl · 2024-02-06T07:15:20Z

src/test/java/org/springframework/data/util/ValueEvaluationUnitTests.java

+	@BeforeEach
+	void setUp() {
+
+		MapPropertySource propertySource = new MapPropertySource("map", Map.of("key.one", "value"));


MapPropertySource may contain non String values. What is the expected output when evaluating expressions like "${numeric.key}" having "numeric.key", 1L?

christophstrobl · 2024-02-06T07:31:52Z

src/test/java/org/springframework/data/util/ValueEvaluationUnitTests.java

+		assertThat(eval("#{\"foo\"}-")).isEqualTo("foo-");
+		assertThat(eval("#{\"fo'o\"}-")).isEqualTo("fo'o-");
+		assertThat(eval("${key.one}-")).isEqualTo("value-");
+		assertThat(eval("${key.one}-and-some-#{'foo'}-#{#foo}")).isEqualTo("value-and-some-foo-bar");


is it allowed/possible to reference properties from within an expression? like #{#foo - ${numeric.key}}?

christophstrobl · 2024-02-06T07:33:38Z

src/main/java/org/springframework/data/spel/CompositeValueExpression.java

+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.spel;


shall we use expression instead of spel for the package name?

That's what I was wondering, too. What about the spel package then? I think we should migrate it into ….expression as well in a compatible way.

christophstrobl · 2024-02-06T07:35:01Z

src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java

+	}
+
+	@Override
+	public void setEnvironment(Environment environment) {


though it's an override, we could add some doc around it having a since tag.

christophstrobl · 2024-02-06T07:36:20Z

src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java

@@ -137,14 +141,20 @@ public void setApplicationContext(ApplicationContext applicationContext) throws
 		if (applicationEventPublisher == null) {
 			this.applicationEventPublisher = applicationContext;
 		}
+
+		this.environment = applicationContext.getEnvironment();


which one wins? this one or setEnvironment?

This is an ambiguity overall. We could implement BeanFactoryAware and set the environment from setApplicationContext only if it wasn't already set.

christophstrobl · 2024-02-06T07:40:27Z

src/main/java/org/springframework/data/spel/DefaultValueExpressionParser.java

+		while (startIndex != -1) {
+
+			int endIndex = findPlaceholderEndIndex(expression, startIndex);
+			if (endIndex != -1) {
+
+				String part = expression.substring(startIndex, endIndex + 1);
+
+				if (part.startsWith(PLACEHOLDER_PREFIX)) {
+					expressions.add(createPlaceholder(part));
+				} else {
+					expressions.add(createExpression(part));
+				}
+
+				placerholderIndex = expression.indexOf(PLACEHOLDER_PREFIX, endIndex);
+				expressionIndex = expression.indexOf(EXPRESSION_PREFIX, endIndex);
+
+				startIndex = getStartIndex(placerholderIndex, expressionIndex);
+
+				if (startIndex == -1) {
+					expressions.add(new LiteralValueExpression(expression.substring(endIndex + 1)));
+				} else {
+					expressions.add(new LiteralValueExpression(expression.substring(endIndex + 1, startIndex)));
+				}
+			} else {
+				expressions.add(new LiteralValueExpression(expression.substring(startIndex)));
+				startIndex = -1;
+			}


this part is really hard to grasp - care to leave some comments in there please.
like why do we substring from end to start here expression.substring(endIndex + 1, startIndex))?

Sure. +1 is to skip the closing parenthesis and to not include it in the literal.

christophstrobl · 2024-02-09T13:54:13Z

LGTM 👍

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

Introduce literal, expression and placeholder variants. Add parser for composite expressions. Closes #2369 Original pull request: #3036

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

Introduce literal, expression and placeholder variants. Add parser for composite expressions. Closes #2369 Original pull request: #3036

mp911de added the type: enhancement A general enhancement label Feb 2, 2024

mp911de added this to the 3.3 M1 (2024.0.0) milestone Feb 2, 2024

mp911de mentioned this pull request Feb 2, 2024

Add an ability to resolve property placeholders in BasicPersistentEntity #2370

Closed

3 tasks

mp911de requested review from christophstrobl and odrotbohm February 5, 2024 10:52

christophstrobl reviewed Feb 7, 2024

View reviewed changes

Prepare issue branch.

a7dcff8

mp911de force-pushed the issue/2369 branch 3 times, most recently from b4d4d4b to 622087a Compare February 9, 2024 09:14

christophstrobl and others added 2 commits February 12, 2024 09:07

Add support for Value Expressions.

0d1ffe3

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

Evolve ValueExpressionParser.

1711f49

Introduce literal, expression and placeholder variants. Add parser for composite expressions. Closes #2369 Original pull request: #3036

mp911de force-pushed the issue/2369 branch from a63b519 to 1711f49 Compare February 12, 2024 08:13

mp911de mentioned this pull request Feb 12, 2024

Add support for ValueExpression spring-projects/spring-data-mongodb#4634

Closed

mp911de added a commit that referenced this pull request Feb 12, 2024

Evolve ValueExpressionParser.

67163d0

Introduce literal, expression and placeholder variants. Add parser for composite expressions. Closes #2369 Original pull request: #3036

mp911de closed this Feb 12, 2024

mp911de deleted the issue/2369 branch February 12, 2024 14:43

This was referenced Feb 12, 2024

Add support for ValueExpression spring-projects/spring-data-cassandra#1477

Closed

Add support for ValueExpression spring-projects/spring-data-relational#1738

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ValueExpression infrastructure #3036

Add ValueExpression infrastructure #3036

mp911de commented Feb 2, 2024

christophstrobl Feb 5, 2024

christophstrobl Feb 5, 2024

christophstrobl Feb 5, 2024

christophstrobl Feb 6, 2024

christophstrobl Feb 6, 2024

christophstrobl Feb 6, 2024

christophstrobl Feb 6, 2024

christophstrobl Feb 6, 2024

mp911de Feb 7, 2024

christophstrobl Feb 6, 2024

christophstrobl Feb 6, 2024

mp911de Feb 7, 2024

christophstrobl Feb 6, 2024

mp911de Feb 7, 2024

christophstrobl commented Feb 9, 2024

		Value Expressions a composition of {spring-framework-docs}/core/expressions.html[Spring Expression Language (SpEL)] and {spring-framework-docs}/core/beans/environment.html#beans-placeholder-resolution-in-statements[Property Placeholder Resolution].
		Value Expressions a combine powerful evaluation of programmatic expressions with the simplicity to resort to property-placeholder resolution to obtain values from the `Environment` such as configuration properties.

Add ValueExpression infrastructure #3036

Add ValueExpression infrastructure #3036

Conversation

mp911de commented Feb 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christophstrobl commented Feb 9, 2024