Skip to content

SpEL, InlineMap with negative values are not cached [SPR-17614] #22146

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

Closed
spring-projects-issues opened this issue Dec 20, 2018 · 3 comments
Closed
Labels
status: superseded An issue that has been superseded by another

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Dec 20, 2018

Davide Pallaoro opened SPR-17614 and commented

Hi there,

I’ve seen that InlineMap class have a nice mechanism that, cache the value of the map if it is constant.

But if the map contains a negative number this caching mechanism is not working.

 

Take for example the following test

package test;

import org.springframework.expression.Expression;
import org.springframework.expression.ExpressionParser;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.testng.Assert;
import org.testng.annotations.Test;

public class MapCacheTest {

    @Test
    public void testMapCached() {
        parseMap("{1 : 2, 3 : 4}");
    }

    @Test
    public void testMapNOTCached() {
        parseMap("{-1 : 2, 3 : 4}");
    }

    private void parseMap(String s) {
        ExpressionParser parser = new SpelExpressionParser();
        Expression expression = parser.parseExpression(s);
        Object value = expression.getValue();
        Assert.assertNotNull(value);
    }
}

 If you run the previous code, (using org.springframework.spring-expression:5.1.3.RELEASE) and set a breakpoint in

org.springframework.expression.spel.ast.InlineMap#getValueInternal

which code is the following..
 

@Override
public TypedValue getValueInternal(ExpressionState expressionState) throws EvaluationException {
     if (this.constant != null) {
          return this.constant;
     }
     else {
          Map<Object, Object> returnValue = new LinkedHashMap<>();
          int childcount = getChildCount();
          for (int c = 0; c < childcount; c++) {

You will notice that when executing MapCacheTest.testMapCached() this.constant is returned.

But when running MapCacheTest.testMapNOTCached() the code jump into the else statement and create a new map each time this method is called.

 

In a scenario with an high concurrency and with pretty big maps, this could decrease the performance of the app.

 

Let me know if everything is clear or you need some further details.

Many thanks and kind regards.

 


Affects: 5.1.3

Issue Links:

@spring-projects-issues spring-projects-issues added type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 5.2 RC1 milestone Jan 11, 2019
@jhoeller jhoeller modified the milestones: 5.2 M1, 5.2 M2 Jan 30, 2019
@masosky
Copy link

masosky commented Feb 22, 2019

Hi there,

I would like to add that this is also occurring in InlineMap.java class.
Please contemplate fixing this error also for lists with negative values.

@SammyVimes
Copy link
Contributor

SammyVimes commented May 11, 2020

Hello! I just created a merge request for this issue, fixing InlineMap and InlineList

@bclozel bclozel removed in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement labels Nov 25, 2021
@bclozel bclozel removed this from the 6.x Backlog milestone Nov 25, 2021
@bclozel bclozel added the status: superseded An issue that has been superseded by another label Nov 25, 2021
@bclozel
Copy link
Member

bclozel commented Nov 25, 2021

Closing in favor of #25921

@bclozel bclozel closed this as completed Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

5 participants