EL expressions with chained map indexes in some cases fail to evaluate [SPR-7244]

[spring-projects-issues]

Robert Blumen opened SPR-7244 and commented

In some cases, an EL evaluated against a map of maps fails to evaluate.
The EL has the form "#root['foo']['bar'].
See attached test case for an example.

---

Affects: 3.0.2

Attachments:

• SpringElFails.java (1.19 kB)

Referenced from: commits 5801af9

[spring-projects-issues]

Andy Clement commented

When you have a failure, please include the text of the failure message in the report. Is it this?

org.springframework.expression.spel.SpelEvaluationException: EL1027E:(pos 14): Indexing into type '[TypeDescriptor java.lang.String]' is not supported

[spring-projects-issues]

Robert Blumen commented

Yes that is the actual error message.
org.springframework.expression.spel.SpelEvaluationException: EL1027E:(pos 14): Indexing into type 'java.lang.String' is not supported

[spring-projects-issues]

Robert Blumen commented

Now that I look at what you have and what I have they are very similar but slightly different.

[spring-projects-issues]

Andy Clement commented

A pair of issues here, but only one needs fixing to make the test pass. Question for Juergen below...

The indexer was using the type descriptor for the context object to determine whether it could index into it. This breaks down in this case because the TypeDescriptor logic that builds a type descriptor for the Map is flawed. TypeDescriptor is only looking at the first value in the map values to determine the map value kind (it is attempting to recover the parameterization). The first value in the map is string, the second value is Map. It naively comes up with a parameterization of LinkedHashMap<String,String> - this means once the first indexer runs, it 'apparently' returns a Map object of type String!

In order to make the test pass we just change the indexer to index based on the runtime type rather than the type descriptor value type. This means it realises it is really a Map and indexes into it. With this fix it doesn't matter that the type descriptor is wrong.

I can't decide what to do about the type descriptor being wrong here. Given that I don't need to change the code to make this scenario work, and that going through all the entries in a collection to compute the common superclass and using that as the recovered collection type will be quite expensive, I'm loathed to make that change right now.

Here is the existing code in TypeDescriptor.getMapValueType()

Object val = map.values().iterator().next();
if (val != null) {
  return val.getClass();
}

Here is the version that would compute a common superclass:

Class<?> clazz = null;
for (Object val : map.values()) {
  if (clazz == null) {
    clazz = val.getClass();
  } else {
    if (!val.getClass().isAssignableFrom(clazz)) {
      // common supertype
      Class<?> common = val.getClass();
      do {
        common = common.getSuperclass();
        if (common != null) {
          if (common.isAssignableFrom(clazz)) {
            clazz = common;
            break;
          }
        }
      } while (common != null);
    }
  }
}
return clazz;

What do you think Juergen?

I've committed the fix that addresses this issue, but not the change to value type computation. (I didn't write that original code that computed it based on the first collection value).

[spring-projects-issues]

Juergen Hoeller commented

Good point, Andy - that rather naive TypeDescriptor code is worth reconsidering, although there aren't any usages other than the expression indexer yet (and even that usage is gone now). Analogously, the map key type and collection element type fallbacks need to be revisited as well.

In any case, let's consider this issue resolved, dealing with TypeDescriptor separately.

Juergen