diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SetValueTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SetValueTests.java index 3b02ed5808..acad56c3e3 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SetValueTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SetValueTests.java @@ -17,8 +17,11 @@ package org.springframework.expression.spel; import java.util.Collection; +import java.util.List; import java.util.Set; +import org.assertj.core.api.ThrowableTypeAssert; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.expression.EvaluationException; @@ -35,6 +38,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; * * @author Keith Donald * @author Andy Clement + * @author Sam Brannen */ class SetValueTests extends AbstractExpressionTests { @@ -42,178 +46,200 @@ class SetValueTests extends AbstractExpressionTests { @Test - void testSetProperty() { + void setValueFailsWhenLeftOperandIsNotAssignable() { + setValueAndExpectError("3=4", "enigma"); + } + + @Test + void setProperty() { setValue("wonNobelPrize", true); } @Test - void testSetNestedProperty() { + void setNestedProperty() { setValue("placeOfBirth.city", "Wien"); } @Test - void testSetArrayElementValue() { + void setArrayElementValueToStringFromString() { setValue("inventions[0]", "Just the telephone"); } @Test - void testErrorCase() { - setValueExpectError("3=4", null); + void setArrayElementInNonexistentIndex() { + setValueAndExpectError( + "new org.springframework.expression.spel.testresources.Inventor().inventions[1]", "my invention"); } @Test - void testSetElementOfNull() { - setValueExpectError("new org.springframework.expression.spel.testresources.Inventor().inventions[1]", - SpelMessage.CANNOT_INDEX_INTO_NULL_VALUE); - } - - @Test - void testSetArrayElementValueAllPrimitiveTypes() { - setValue("arrayContainer.ints[1]", 3); - setValue("arrayContainer.floats[1]", 3.0f); + void setArrayElementValueToPrimitiveTypeFromWrapperType() { + // All primitive values below are auto-boxed into their wrapper types. setValue("arrayContainer.booleans[1]", false); - setValue("arrayContainer.doubles[1]", 3.4d); - setValue("arrayContainer.shorts[1]", (short)3); - setValue("arrayContainer.longs[1]", 3L); - setValue("arrayContainer.bytes[1]", (byte) 3); setValue("arrayContainer.chars[1]", (char) 3); + setValue("arrayContainer.shorts[1]", (short) 3); + setValue("arrayContainer.bytes[1]", (byte) 3); + setValue("arrayContainer.ints[1]", 3); + setValue("arrayContainer.longs[1]", 3L); + setValue("arrayContainer.floats[1]", 3.0f); + setValue("arrayContainer.doubles[1]", 3.4d); } @Test - void testIsWritableForInvalidExpressions_SPR10610() { - StandardEvaluationContext lContext = TestScenarioCreator.getTestEvaluationContext(); + void setArrayElementValueToPrimitiveTypeFromStringResultsInError() { + String notPrimitiveOrWrapper = "not primitive or wrapper"; + setValueAndExpectError("arrayContainer.booleans[1]", notPrimitiveOrWrapper); + setValueAndExpectError("arrayContainer.chars[1]", notPrimitiveOrWrapper); + setValueAndExpectError("arrayContainer.shorts[1]", notPrimitiveOrWrapper); + setValueAndExpectError("arrayContainer.bytes[1]", notPrimitiveOrWrapper); + setValueAndExpectError("arrayContainer.ints[1]", notPrimitiveOrWrapper); + setValueAndExpectError("arrayContainer.longs[1]", notPrimitiveOrWrapper); + setValueAndExpectError("arrayContainer.floats[1]", notPrimitiveOrWrapper); + setValueAndExpectError("arrayContainer.doubles[1]", notPrimitiveOrWrapper); + } + + @Test + void setArrayElementValueToPrimitiveTypeFromSingleElementList() { + setValue("arrayContainer.booleans[1]", List.of(false), false); + setValue("arrayContainer.chars[1]", List.of('a'), 'a'); + setValue("arrayContainer.shorts[1]", List.of((short) 3), (short) 3); + setValue("arrayContainer.bytes[1]", List.of((byte) 3), (byte) 3); + setValue("arrayContainer.ints[1]", List.of(42), 42); + setValue("arrayContainer.longs[1]", List.of(42L), 42L); + setValue("arrayContainer.floats[1]", List.of(42F), 42F); + setValue("arrayContainer.doubles[1]", List.of(42D), 42D); + } + + @Disabled("Disabled due to bug in Indexer.setArrayElement() regarding primitive/wrapper types") + @Test + void setArrayElementValueToPrimitiveTypeFromEmptyListResultsInError() { + List emptyList = List.of(); + // TODO These fail because CollectionToObjectConverter returns null. + // It currently throws: java.lang.IllegalStateException: Null conversion result for index [[]]. + // Whereas, it should throw a SpelEvaluationException. + setValueAndExpectError("arrayContainer.booleans[1]", emptyList); + setValueAndExpectError("arrayContainer.chars[1]", emptyList); + setValueAndExpectError("arrayContainer.shorts[1]", emptyList); + setValueAndExpectError("arrayContainer.bytes[1]", emptyList); + setValueAndExpectError("arrayContainer.ints[1]", emptyList); + setValueAndExpectError("arrayContainer.longs[1]", emptyList); + setValueAndExpectError("arrayContainer.floats[1]", emptyList); + setValueAndExpectError("arrayContainer.doubles[1]", emptyList); + } + + @Test // gh-15239 + void isWritableForInvalidExpressions() { + // Do NOT reuse super.context! + StandardEvaluationContext localContext = TestScenarioCreator.getTestEvaluationContext(); // PROPERTYORFIELDREFERENCE // Non-existent field (or property): Expression e1 = parser.parseExpression("arrayContainer.wibble"); - assertThat(e1.isWritable(lContext)).as("Should not be writable!").isFalse(); + assertThat(e1.isWritable(localContext)).as("Should not be writable!").isFalse(); Expression e2 = parser.parseExpression("arrayContainer.wibble.foo"); - assertThatExceptionOfType(SpelEvaluationException.class).isThrownBy(() -> - e2.isWritable(lContext)); -// org.springframework.expression.spel.SpelEvaluationException: EL1008E:(pos 15): Property or field 'wibble' cannot be found on object of type 'org.springframework.expression.spel.testresources.ArrayContainer' - maybe not public? -// at org.springframework.expression.spel.ast.PropertyOrFieldReference.readProperty(PropertyOrFieldReference.java:225) + assertThatSpelEvaluationException().isThrownBy(() -> e2.isWritable(localContext)); // VARIABLE // the variable does not exist (but that is OK, we should be writable) Expression e3 = parser.parseExpression("#madeup1"); - assertThat(e3.isWritable(lContext)).as("Should be writable!").isTrue(); + assertThat(e3.isWritable(localContext)).as("Should be writable!").isTrue(); Expression e4 = parser.parseExpression("#madeup2.bar"); // compound expression - assertThat(e4.isWritable(lContext)).as("Should not be writable!").isFalse(); + assertThat(e4.isWritable(localContext)).as("Should not be writable!").isFalse(); // INDEXER // non-existent indexer (wibble made up) Expression e5 = parser.parseExpression("arrayContainer.wibble[99]"); - assertThatExceptionOfType(SpelEvaluationException.class).isThrownBy(() -> - e5.isWritable(lContext)); + assertThatSpelEvaluationException().isThrownBy(() -> e5.isWritable(localContext)); // non-existent indexer (index via a string) Expression e6 = parser.parseExpression("arrayContainer.ints['abc']"); - assertThatExceptionOfType(SpelEvaluationException.class).isThrownBy(() -> - e6.isWritable(lContext)); + assertThatSpelEvaluationException().isThrownBy(() -> e6.isWritable(localContext)); } @Test - void testSetArrayElementValueAllPrimitiveTypesErrors() { - // none of these sets are possible due to (expected) conversion problems - setValueExpectError("arrayContainer.ints[1]", "wibble"); - setValueExpectError("arrayContainer.floats[1]", "dribble"); - setValueExpectError("arrayContainer.booleans[1]", "nein"); - // TODO -- this fails with NPE due to ArrayToObject converter - discuss with Andy - //setValueExpectError("arrayContainer.doubles[1]", new ArrayList()); - //setValueExpectError("arrayContainer.shorts[1]", new ArrayList()); - //setValueExpectError("arrayContainer.longs[1]", new ArrayList()); - setValueExpectError("arrayContainer.bytes[1]", "NaB"); - setValueExpectError("arrayContainer.chars[1]", "NaC"); - } - - @Test - void testSetArrayElementNestedValue() { + void setArrayElementNestedValue() { setValue("placesLived[0].city", "Wien"); } @Test - void testSetListElementValue() { + void setListElementValue() { setValue("placesLivedList[0]", new PlaceOfBirth("Wien")); } @Test - void testSetGenericListElementValueTypeCoercion() { - // TODO currently failing since setValue does a getValue and "Wien" string != PlaceOfBirth - check with andy + void setGenericListElementValueTypeCoercion() { setValue("placesLivedList[0]", "Wien"); } @Test - void testSetGenericListElementValueTypeCoercionOK() { + void setGenericListElementValueTypeCoercionOK() { setValue("booleanList[0]", "true", Boolean.TRUE); } @Test - void testSetListElementNestedValue() { + void setListElementNestedValue() { setValue("placesLived[0].city", "Wien"); } @Test - void testSetArrayElementInvalidIndex() { - setValueExpectError("placesLived[23]", "Wien"); - setValueExpectError("placesLivedList[23]", "Wien"); + void setArrayElementInvalidIndex() { + setValueAndExpectError("placesLived[23]", "Wien"); + setValueAndExpectError("placesLivedList[23]", "Wien"); } @Test - void testSetMapElements() { + void setMapElements() { setValue("testMap['montag']","lundi"); } @Test - void testIndexingIntoUnsupportedType() { - setValueExpectError("'hello'[3]", 'p'); + void indexingIntoUnsupportedType() { + setValueAndExpectError("'hello'[3]", 'p'); } @Test - void testSetPropertyTypeCoercion() { + void setPropertyTypeCoercion() { setValue("publicBoolean", "true", Boolean.TRUE); } @Test - void testSetPropertyTypeCoercionThroughSetter() { + void setPropertyTypeCoercionThroughSetter() { setValue("SomeProperty", "true", Boolean.TRUE); } @Test - void testAssign() { - StandardEvaluationContext eContext = TestScenarioCreator.getTestEvaluationContext(); + void assign() { + // Do NOT reuse super.context! + StandardEvaluationContext localContext = TestScenarioCreator.getTestEvaluationContext(); Expression e = parse("publicName='Andy'"); - assertThat(e.isWritable(eContext)).isFalse(); - assertThat(e.getValue(eContext)).isEqualTo("Andy"); + assertThat(e.isWritable(localContext)).isFalse(); + assertThat(e.getValue(localContext)).isEqualTo("Andy"); } - /* - * Testing the coercion of both the keys and the values to the correct type + /** + * Testing the coercion of both the keys and the values to the correct type. */ @Test - void testSetGenericMapElementRequiresCoercion() { - StandardEvaluationContext eContext = TestScenarioCreator.getTestEvaluationContext(); + void setGenericMapElementRequiresCoercion() { + // Do NOT reuse super.context! + StandardEvaluationContext localContext = TestScenarioCreator.getTestEvaluationContext(); Expression e = parse("mapOfStringToBoolean[42]"); - assertThat(e.getValue(eContext)).isNull(); + assertThat(e.getValue(localContext)).isNull(); // Key should be coerced to string representation of 42 - e.setValue(eContext, "true"); + e.setValue(localContext, "true"); // All keys should be strings - Set ks = parse("mapOfStringToBoolean.keySet()").getValue(eContext, Set.class); - for (Object o: ks) { - assertThat(o.getClass()).isEqualTo(String.class); - } + Set keys = parse("mapOfStringToBoolean.keySet()").getValue(localContext, Set.class); + assertThat(keys).allSatisfy(key -> assertThat(key).isExactlyInstanceOf(String.class)); // All values should be booleans - Collection vs = parse("mapOfStringToBoolean.values()").getValue(eContext, Collection.class); - for (Object o: vs) { - assertThat(o.getClass()).isEqualTo(Boolean.class); - } + Collection values = parse("mapOfStringToBoolean.values()").getValue(localContext, Collection.class); + assertThat(values).allSatisfy(key -> assertThat(key).isExactlyInstanceOf(Boolean.class)); // One final test check coercion on the key for a map lookup - Object o = e.getValue(eContext); + Object o = e.getValue(localContext); assertThat(o).isEqualTo(Boolean.TRUE); } @@ -225,28 +251,30 @@ class SetValueTests extends AbstractExpressionTests { /** * Call setValue() but expect it to fail. */ - protected void setValueExpectError(String expression, Object value) { + private void setValueAndExpectError(String expression, Object value) { Expression e = parser.parseExpression(expression); assertThat(e).isNotNull(); if (DEBUG) { SpelUtilities.printAbstractSyntaxTree(System.out, e); } - StandardEvaluationContext lContext = TestScenarioCreator.getTestEvaluationContext(); - assertThatExceptionOfType(EvaluationException.class).isThrownBy(() -> - e.setValue(lContext, value)); + // Do NOT reuse super.context! + StandardEvaluationContext localContext = TestScenarioCreator.getTestEvaluationContext(); + assertThatSpelEvaluationException().isThrownBy(() -> e.setValue(localContext, value)); } - protected void setValue(String expression, Object value) { + private void setValue(String expression, Object value) { + Class expectedType = value.getClass(); try { Expression e = parser.parseExpression(expression); assertThat(e).isNotNull(); if (DEBUG) { SpelUtilities.printAbstractSyntaxTree(System.out, e); } - StandardEvaluationContext lContext = TestScenarioCreator.getTestEvaluationContext(); - assertThat(e.isWritable(lContext)).as("Expression is not writeable but should be").isTrue(); - e.setValue(lContext, value); - assertThat(e.getValue(lContext,value.getClass())).as("Retrieved value was not equal to set value").isEqualTo(value); + // Do NOT reuse super.context! + StandardEvaluationContext localContext = TestScenarioCreator.getTestEvaluationContext(); + assertThat(e.isWritable(localContext)).as("Expression is not writeable but should be").isTrue(); + e.setValue(localContext, value); + assertThat(e.getValue(localContext, expectedType)).as("Retrieved value was not equal to set value").isEqualTo(value); } catch (EvaluationException | ParseException ex) { throw new AssertionError("Unexpected Exception: " + ex.getMessage(), ex); @@ -254,26 +282,29 @@ class SetValueTests extends AbstractExpressionTests { } /** - * For use when coercion is happening during a setValue(). The expectedValue should be + * For use when coercion is happening during setValue(). The expectedValue should be * the coerced form of the value. */ - protected void setValue(String expression, Object value, Object expectedValue) { + private void setValue(String expression, Object value, Object expectedValue) { try { Expression e = parser.parseExpression(expression); assertThat(e).isNotNull(); if (DEBUG) { SpelUtilities.printAbstractSyntaxTree(System.out, e); } - StandardEvaluationContext lContext = TestScenarioCreator.getTestEvaluationContext(); - assertThat(e.isWritable(lContext)).as("Expression is not writeable but should be").isTrue(); - e.setValue(lContext, value); - Object a = expectedValue; - Object b = e.getValue(lContext); - assertThat(a).isEqualTo(b); + // Do NOT reuse super.context! + StandardEvaluationContext localContext = TestScenarioCreator.getTestEvaluationContext(); + assertThat(e.isWritable(localContext)).as("Expression is not writeable but should be").isTrue(); + e.setValue(localContext, value); + assertThat(expectedValue).isEqualTo(e.getValue(localContext)); } catch (EvaluationException | ParseException ex) { throw new AssertionError("Unexpected Exception: " + ex.getMessage(), ex); } } + private static ThrowableTypeAssert assertThatSpelEvaluationException() { + return assertThatExceptionOfType(SpelEvaluationException.class); + } + } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelDocumentationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelDocumentationTests.java index f97962c5e7..33097d61fd 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelDocumentationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelDocumentationTests.java @@ -277,6 +277,7 @@ class SpelDocumentationTests extends AbstractExpressionTests { assertThat(falseValue).isFalse(); // -- AND and NOT -- + expression = "isMember('Nikola Tesla') and !isMember('Mihajlo Pupin')"; falseValue = parser.parseExpression(expression).getValue(societyContext, Boolean.class); assertThat(falseValue).isFalse(); @@ -284,15 +285,21 @@ class SpelDocumentationTests extends AbstractExpressionTests { @Test void stringOperators() { - // Concatenation + // -- Concatenation -- + + // evaluates to "hello world" String helloWorld = parser.parseExpression("'hello' + ' ' + 'world'").getValue(String.class); assertThat(helloWorld).isEqualTo("hello world"); - // Character Subtraction + // -- Subtraction -- + + // evaluates to 'a' char ch = parser.parseExpression("'d' - 3").getValue(char.class); assertThat(ch).isEqualTo('a'); - // Repeat + // -- Repeat -- + + // evaluates to "abcabc" String repeated = parser.parseExpression("'abc' * 2").getValue(String.class); assertThat(repeated).isEqualTo("abcabc"); } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java b/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java index c161ae9c45..4f13310b7e 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java @@ -28,7 +28,8 @@ import org.springframework.expression.spel.testresources.PlaceOfBirth; /** * Builds an evaluation context for test expressions. - * Features of the test evaluation context are: + * + *

Features of the test evaluation context are: *

    *
  • The root context object is an Inventor instance {@link Inventor} *
@@ -112,9 +113,9 @@ class TestScenarioCreator { /** * Create the root context object, an Inventor instance. Non-qualified property * and method references will be resolved against this context object. - * @param testContext the evaluation context in which to set the root object + * @param context the evaluation context in which to set the root object */ - private static void setupRootContextObject(StandardEvaluationContext testContext) { + private static void setupRootContextObject(StandardEvaluationContext context) { GregorianCalendar c = new GregorianCalendar(); c.set(1856, 7, 9); Inventor tesla = new Inventor("Nikola Tesla", c.getTime(), "Serbian"); @@ -122,7 +123,7 @@ class TestScenarioCreator { tesla.setInventions("Telephone repeater", "Rotating magnetic field principle", "Polyphase alternating-current system", "Induction motor", "Alternating-current power transmission", "Tesla coil transformer", "Wireless communication", "Radio", "Fluorescent lights"); - testContext.setRootObject(tesla); + context.setRootObject(tesla); } @@ -130,10 +131,7 @@ class TestScenarioCreator { // in test expressions public static String isEven(int i) { - if ((i % 2) == 0) { - return "y"; - } - return "n"; + return ((i % 2) == 0 ? "y" : "n"); } public static int[] reverseInt(int i, int j, int k) { @@ -141,11 +139,7 @@ class TestScenarioCreator { } public static String reverseString(String input) { - StringBuilder backwards = new StringBuilder(); - for (int i = 0; i < input.length(); i++) { - backwards.append(input.charAt(input.length() - 1 - i)); - } - return backwards.toString(); + return new StringBuilder(input).reverse().toString(); } public static String varargsFunction(String... strings) {