From 813cc3b339fb073e578bc99b2d447bf9be9e45ce Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Wed, 30 Jul 2014 12:40:57 -0700 Subject: [PATCH] Fix compilation of SpEL Indexer nodes involving map references There is special handling for SpEL expressions involving a map and an unquoted string literal key (e.g. mymap[key1]). SpEL does not require key1 to be quoted. This special handling which is done in Indexer getValueRef() was not being also done in the Indexer generateCode() method that compiles the expression. Also fixed a problem where the key was not being compiled in a new sub scope. Without the new scope the key expression was failing to reload the relevant context object when it needed it. Issue: SPR-12045 --- .../expression/spel/ast/Indexer.java | 43 ++++++++++++- .../spel/SpelCompilationCoverageTests.java | 62 +++++++++++++++++++ 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index 1ecc46ac5c3..e1d6b3a4bb3 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java @@ -180,8 +180,17 @@ public class Indexer extends SpelNodeImpl { if (this.indexedType == IndexedType.array) { return exitTypeDescriptor != null; } - else if (this.indexedType == IndexedType.list || this.indexedType == IndexedType.map) { - return true; + else if (this.indexedType == IndexedType.list) { + return this.children[0].isCompilable(); + } + else if (this.indexedType == IndexedType.map) { + if (this.children[0] instanceof PropertyOrFieldReference) { + // Only the name will be used, so that is OK + return true; + } + else { + return this.children[0].isCompilable(); + } } else if (this.indexedType == IndexedType.object) { // If the string name is changing the accessor is clearly going to change (so compilation is not possible) @@ -207,61 +216,89 @@ public class Indexer extends SpelNodeImpl { if (exitTypeDescriptor == "I") { mv.visitTypeInsn(CHECKCAST,"[I"); SpelNodeImpl index = this.children[0]; + codeflow.enterCompilationScope(); index.generateCode(mv, codeflow); + codeflow.exitCompilationScope(); mv.visitInsn(IALOAD); } else if (exitTypeDescriptor == "D") { mv.visitTypeInsn(CHECKCAST,"[D"); SpelNodeImpl index = this.children[0]; + codeflow.enterCompilationScope(); index.generateCode(mv, codeflow); mv.visitInsn(DALOAD); } else if (exitTypeDescriptor == "J") { mv.visitTypeInsn(CHECKCAST,"[J"); SpelNodeImpl index = this.children[0]; + codeflow.enterCompilationScope(); index.generateCode(mv, codeflow); + codeflow.exitCompilationScope(); mv.visitInsn(LALOAD); } else if (exitTypeDescriptor == "F") { mv.visitTypeInsn(CHECKCAST,"[F"); SpelNodeImpl index = this.children[0]; + codeflow.enterCompilationScope(); index.generateCode(mv, codeflow); + codeflow.exitCompilationScope(); mv.visitInsn(FALOAD); } else if (exitTypeDescriptor == "S") { mv.visitTypeInsn(CHECKCAST,"[S"); SpelNodeImpl index = this.children[0]; + codeflow.enterCompilationScope(); index.generateCode(mv, codeflow); + codeflow.exitCompilationScope(); mv.visitInsn(SALOAD); } else if (exitTypeDescriptor == "B") { mv.visitTypeInsn(CHECKCAST,"[B"); SpelNodeImpl index = this.children[0]; + codeflow.enterCompilationScope(); index.generateCode(mv, codeflow); + codeflow.exitCompilationScope(); mv.visitInsn(BALOAD); } else if (exitTypeDescriptor == "C") { mv.visitTypeInsn(CHECKCAST,"[C"); SpelNodeImpl index = this.children[0]; + codeflow.enterCompilationScope(); index.generateCode(mv, codeflow); + codeflow.exitCompilationScope(); mv.visitInsn(CALOAD); } else { mv.visitTypeInsn(CHECKCAST,"["+exitTypeDescriptor+(CodeFlow.isPrimitiveArray(exitTypeDescriptor)?"":";"));//depthPlusOne(exitTypeDescriptor)+"Ljava/lang/Object;"); SpelNodeImpl index = this.children[0]; + codeflow.enterCompilationScope(); index.generateCode(mv, codeflow); + codeflow.exitCompilationScope(); mv.visitInsn(AALOAD); } } else if (this.indexedType == IndexedType.list) { mv.visitTypeInsn(CHECKCAST,"java/util/List"); + codeflow.enterCompilationScope(); this.children[0].generateCode(mv, codeflow); + codeflow.exitCompilationScope(); mv.visitMethodInsn(INVOKEINTERFACE,"java/util/List","get","(I)Ljava/lang/Object;", true); CodeFlow.insertCheckCast(mv,exitTypeDescriptor); } else if (this.indexedType == IndexedType.map) { mv.visitTypeInsn(CHECKCAST,"java/util/Map"); - this.children[0].generateCode(mv, codeflow); + // Special case when the key is an unquoted string literal that will be parsed as + // a property/field reference + if ((this.children[0] instanceof PropertyOrFieldReference)) { + PropertyOrFieldReference reference = (PropertyOrFieldReference) this.children[0]; + String mapKeyName = reference.getName(); + mv.visitLdcInsn(mapKeyName); + } + else { + codeflow.enterCompilationScope(); + this.children[0].generateCode(mv, codeflow); + codeflow.exitCompilationScope(); + } mv.visitMethodInsn(INVOKEINTERFACE,"java/util/Map","get","(Ljava/lang/Object;)Ljava/lang/Object;", true); CodeFlow.insertCheckCast(mv,exitTypeDescriptor); } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index 28b348e770b..9756d8b8f50 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -37,6 +37,7 @@ import org.springframework.expression.spel.ast.SpelNodeImpl; import org.springframework.expression.spel.ast.Ternary; import org.springframework.expression.spel.standard.SpelCompiler; import org.springframework.expression.spel.standard.SpelExpression; +import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; import static org.junit.Assert.*; @@ -2468,8 +2469,69 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertTrue(expression.getValue(new GenericMessageTestHelper2(6),Boolean.TYPE)); } + @Test + public void indexerMapAccessor_12045() throws Exception { + SpelParserConfiguration spc = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE,this.getClass().getClassLoader()); + SpelExpressionParser sep = new SpelExpressionParser(spc); + expression=sep.parseExpression("headers[command]"); + MyMessage root = new MyMessage(); + assertEquals("wibble",expression.getValue(root)); + // This next call was failing because the isCompilable check in Indexer did not check on the key being compilable + // (and also generateCode in the Indexer was missing the optimization that it didn't need necessarily need to call + // generateCode for that accessor) + assertEquals("wibble",expression.getValue(root)); + assertCanCompile(expression); + + // What about a map key that is an expression - ensure the getKey() is evaluated in the right scope + expression=sep.parseExpression("headers[getKey()]"); + assertEquals("wobble",expression.getValue(root)); + assertEquals("wobble",expression.getValue(root)); + + expression=sep.parseExpression("list[getKey2()]"); + assertEquals("wobble",expression.getValue(root)); + assertEquals("wobble",expression.getValue(root)); + + expression = sep.parseExpression("ia[getKey2()]"); + assertEquals(3,expression.getValue(root)); + assertEquals(3,expression.getValue(root)); + } + // --- + public static interface Message { + MessageHeaders getHeaders(); + @SuppressWarnings("rawtypes") + List getList(); + int[] getIa(); + } + + public static class MyMessage implements Message { + public MessageHeaders getHeaders() { + MessageHeaders mh = new MessageHeaders(); + mh.put("command", "wibble"); + mh.put("command2", "wobble"); + return mh; + } + public int[] getIa() { return new int[]{5,3}; } + @SuppressWarnings({ "rawtypes", "unchecked" }) + public List getList() { + List l = new ArrayList(); + l.add("wibble"); + l.add("wobble"); + return l; + } + + public String getKey() { + return "command2"; + } + + public int getKey2() { + return 1; + } + } + + public static class MessageHeaders extends HashMap { } + public static class GenericMessageTestHelper { private T payload;