[jira] [Updated] (VELOCITY-986) Breaking change- Map size method is called, instead of provided size property

     [ https://issues.apache.org/jira/browse/VELOCITY-986?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Barry updated VELOCITY-986:
-----------------------------------
    Description: 
We recently tried to upgrade to 2.4 and 2.4.1 and both had this issue, so we had to rollback. We have a template looking like this:
[https://my.example.com?size=$|https://my.example.com/?size=$]

{input.size}

 
In our VelocityContext, we have have an InnerContext values that should be used.
IE

{"size"= "1089", "name"="tyler", "age"="28"}

 
In versions 2.3 and below, this would evaluate to 
[https://my.example.com?size=1089|https://my.example.com/?size=1089]
With newest versions, it gives us the number of items in our map. So
[https://my.example.com?size=3|https://my.example.com/?size=1089]
 
Here is a unit test of our usage.  It uses 2 Velocity Contexts
 
{code:keyword}
import org.apache.commons.collections4.map.CaseInsensitiveMap;
import org.apache.velocity.VelocityContext;
import org.apache.velocity.app.Velocity;
import org.apache.velocity.app.VelocityEngine;
import org.junit.Test;

import java.io.StringWriter;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class VelocityRegressionTest {

    @Test
    public void testVelocityTemplate() {
        VelocityEngine velocityEngine = new VelocityEngine();
        velocityEngine.init();
        CaseInsensitiveMap<String, Object> innerMap = new CaseInsensitiveMap<>();
        //Size is both an operation on "Map", as well a key within it
        innerMap.put("size", "1098");
        innerMap.put("name", "Tyler");
        innerMap.put("try", "1");

        CaseInsensitiveMap<String, Object> outerMap = new CaseInsensitiveMap<>();
        outerMap.put("input", innerMap);
        
        VelocityContext context = new VelocityContext(outerMap);
        StringWriter sw = new StringWriter();

        //This works correctly
        String template = "https://my.example.com?size=${input.name}";
        Velocity.evaluate(context, sw, "", template);
        assertEquals("https://my.example.com?size=Tyler", sw.toString());

        sw = new StringWriter();
        // This notation works, however it is not the one we used. Just including it here to show that one notation still works as expected.
        template = "https://my.example.com?size=${input[\"size\"]}";
        Velocity.evaluate(context, sw, "", template);
        assertEquals("https://my.example.com?size=1098", sw.toString());

        //THIS DEMONSTRATES THE BREAKING CHANGE
        sw = new StringWriter();
        // In versions 2.4+, size evaluates to the size function of maps, not the field inside the map with the same name. This is what broke.
        // Seems like order of operations is flipped
        template = "https://my.example.com?size=${input.size}";
        Velocity.evaluate(context, sw, "", template);
        assertEquals("https://my.example.com?size=1098", sw.toString());
    }
}{code}

  was:
We recently tried to upgrade to 2.4 and 2.4.1 and both had this issue, so we had to rollback. We have a template looking like this:
[https://my.example.com?size=$|https://my.example.com/?size=$]{input.size}
 
In our VelocityContext, we have have an InnerContext values that should be used.
IE

{"size"= "1089", "name"="tyler", "age"="28"}

 
In versions 2.3 and below, this would evaluate to 
[https://my.example.com?size=1089|https://my.example.com/?size=1089]
With newest versions, it gives us the number of items in our map. So
[https://my.example.com?size=3|https://my.example.com/?size=1089]
 
Here is a unit test of our usage.  This test includes the ability to register many different "providers", however, I kept it simple and just used one.  Providers have a prefix, so in the example above, we use a prefix provider of "input".  This allows to "scope" contexts.
 
{code:java}
import org.apache.commons.collections4.map.CaseInsensitiveMap;
import org.apache.commons.lang3.NotImplementedException;
import org.apache.commons.lang3.StringUtils;
import org.apache.velocity.VelocityContext;
import org.apache.velocity.app.Velocity;
import org.apache.velocity.app.VelocityEngine;
import org.apache.velocity.context.Context;
import org.junit.Test;

import java.io.StringWriter;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class VelocityRegressionTest {

    @Test
    public void testVelocityTemplate() throws Exception {
        VelocityEngine velocityEngine = new VelocityEngine();
        velocityEngine.init();
        VariableProviderCaseInsensitiveMap<String> innerMap = new VariableProviderCaseInsensitiveMap<>("input");
        innerMap.put("size", "1098");
        innerMap.put("name", "Tyler");
        innerMap.put("try", "1");
        Context innerContext = new VariableProviderVelocityContext(List.of(innerMap));
        VelocityContext context = new VelocityContext(innerContext);

        StringWriter sw = new StringWriter();
        String vt = "https://my.example.com?size=${input.size}";
        Velocity.evaluate(context, sw, "", vt);
        assertEquals("https://my.example.com?size=1098", sw.toString());
    }

    static class VariableProviderCaseInsensitiveMap<T> extends CaseInsensitiveMap<String, T>  {

        private String providerContextPrefix = null;

        public VariableProviderCaseInsensitiveMap(final String providerContextPrefix) {
            if (!StringUtils.isBlank(providerContextPrefix)) {
                this.providerContextPrefix = providerContextPrefix;
            }
        }

        public String getProviderContextPrefix() {
            return providerContextPrefix;
        }

        public Set<String> getKeys() {
            return super.keySet();
        }

        public Map<String, T> getMap() {return this;}
    }

    static class VariableProviderVelocityContext extends VelocityContext {
        protected final List<VariableProviderCaseInsensitiveMap> providers;

        public VariableProviderVelocityContext(
                final List<VariableProviderCaseInsensitiveMap> providers
        ) {
            super();
            this.providers = providers;
        }

        private Object lookup(String key) {
            for (VariableProviderCaseInsensitiveMap provider : providers) {
                final String contextPrefix = provider.getProviderContextPrefix();
                if (contextPrefix.equalsIgnoreCase(key)) {
                    return provider.getMap();
                }
            }

            return null;
        }

        @Override
        public Object internalGet(String var1) {
            return lookup(var1);
        }

        @Override
        public Object internalPut(String key, Object value) {
            throw new NotImplementedException("internalRemove is not valid and is not implemented");
        }

        @Override
        public String[] internalGetKeys() {
            List<String> keys = new LinkedList<>();
            for (VariableProviderCaseInsensitiveMap provider : providers) {
                String prefix = provider.getProviderContextPrefix();
                if (StringUtils.isNotBlank(prefix)) {
                    keys.add(prefix);
                } //the prefix is the key name for a context provider.
                else {
                    keys.addAll(provider.getKeys());
                }
            }
            return keys.toArray(new String[0]);
        }

        @Override
        public boolean internalContainsKey(String var1) {
            return lookup(var1) != null;
        }

        @Override
        public Object internalRemove(String var1) {
            throw new NotImplementedException("internalRemove is not valid and is not implemented");
        }
    }
} {code}


> Breaking change- Map size method is called, instead of provided size property
> -----------------------------------------------------------------------------
>
>                 Key: VELOCITY-986
>                 URL: https://issues.apache.org/jira/browse/VELOCITY-986
>             Project: Velocity
>          Issue Type: Bug
>          Components: Engine
>    Affects Versions: 2.4, 2.4.1
>            Reporter: Patrick Barry
>            Priority: Major
>
> We recently tried to upgrade to 2.4 and 2.4.1 and both had this issue, so we had to rollback. We have a template looking like this:
> [https://my.example.com?size=$|https://my.example.com/?size=$]
> {input.size}
>  
> In our VelocityContext, we have have an InnerContext values that should be used.
> IE
> {"size"= "1089", "name"="tyler", "age"="28"}
>  
> In versions 2.3 and below, this would evaluate to 
> [https://my.example.com?size=1089|https://my.example.com/?size=1089]
> With newest versions, it gives us the number of items in our map. So
> [https://my.example.com?size=3|https://my.example.com/?size=1089]
>  
> Here is a unit test of our usage.  It uses 2 Velocity Contexts
>  
> {code:keyword}
> import org.apache.commons.collections4.map.CaseInsensitiveMap;
> import org.apache.velocity.VelocityContext;
> import org.apache.velocity.app.Velocity;
> import org.apache.velocity.app.VelocityEngine;
> import org.junit.Test;
> import java.io.StringWriter;
> import static org.junit.jupiter.api.Assertions.assertEquals;
> public class VelocityRegressionTest {
>     @Test
>     public void testVelocityTemplate() {
>         VelocityEngine velocityEngine = new VelocityEngine();
>         velocityEngine.init();
>         CaseInsensitiveMap<String, Object> innerMap = new CaseInsensitiveMap<>();
>         //Size is both an operation on "Map", as well a key within it
>         innerMap.put("size", "1098");
>         innerMap.put("name", "Tyler");
>         innerMap.put("try", "1");
>         CaseInsensitiveMap<String, Object> outerMap = new CaseInsensitiveMap<>();
>         outerMap.put("input", innerMap);
>         
>         VelocityContext context = new VelocityContext(outerMap);
>         StringWriter sw = new StringWriter();
>         //This works correctly
>         String template = "https://my.example.com?size=${input.name}";
>         Velocity.evaluate(context, sw, "", template);
>         assertEquals("https://my.example.com?size=Tyler", sw.toString());
>         sw = new StringWriter();
>         // This notation works, however it is not the one we used. Just including it here to show that one notation still works as expected.
>         template = "https://my.example.com?size=${input[\"size\"]}";
>         Velocity.evaluate(context, sw, "", template);
>         assertEquals("https://my.example.com?size=1098", sw.toString());
>         //THIS DEMONSTRATES THE BREAKING CHANGE
>         sw = new StringWriter();
>         // In versions 2.4+, size evaluates to the size function of maps, not the field inside the map with the same name. This is what broke.
>         // Seems like order of operations is flipped
>         template = "https://my.example.com?size=${input.size}";
>         Velocity.evaluate(context, sw, "", template);
>         assertEquals("https://my.example.com?size=1098", sw.toString());
>     }
> }{code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)