[jira] [Commented] (OFBIZ-13164) HashMaps are not properly rendered in FTL with current FTL integration

["ASF subversion and git services (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OFBIZ-13164?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18103381#comment-18103381 ] 

ASF subversion and git services commented on OFBIZ-13164:
---------------------------------------------------------

Commit 70b9901e4559d45eef186963b974b3f01fb3a142 in ofbiz-framework's branch refs/heads/trunk from toaditi
[ https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=70b9901e45 ]

Fixed: HashMaps are not properly rendered in FTL with current FTL integration (OFBIZ-13164) (#1615)

Jira: https://issues.apache.org/jira/browse/OFBIZ-13164

Thanks to Carsten Schinzer for reporting this one, and for naming the `BeansWrapper` versus `DefaultObjectWrapper` distinction in the report. That turned out to be exactly the right thread to pull, and the note that a simple fix did not work saved me from taking the wrong turn.

### What happens today

`FreeMarkerWorker` builds a plain `BeansWrapper`, so every `java.util.Map` reaches templates as `freemarker.ext.beans.MapModel`. `MapModel.keySet()` returns the union of the map's own keys and the bean property names of the map object. On a two entry `LinkedHashMap` holding only `alpha` and `beta`:

| expression | result on trunk |
| --- | --- |
| `${aMap?size}` | `30` |
| `${aMap?keys?join(",")}` |
`getClass,getOrDefault,values,computeIfAbsent,replace,...,alpha,class,keySet,beta,entrySet,...`
|
| `<#list aMap as key, value>` | fails, because the values behind those
synthetic keys are methods rather than strings |

### Why I did not take either of the usual fixes

`GenericEntity` implements `Map`, so every `GenericValue` in a template is a map as far as the wrapper is concerned. That makes both of the obvious options quite wide:
- `DefaultObjectWrapper` also swaps `List` for `DefaultListAdapter` and stops method calls generally, giving `NonHashException` on `${aList.size()}` and `${aBean.getSomething()}`.
- `BeansWrapperBuilder.setSimpleMapWrapper(true)` is narrower, but still ends `${aMap.get(...)}` and `${aMap.entrySet()}`, along with every `${anEntity.getRelatedOne(...)}` and `${anEntity.getString(...)}`.

Framework templates alone use `.get(` in 161 files, `keySet()` in 14 and `.size()` in 40, before counting plugins, so I was wary of changing the wrapper wholesale.

### The change

`OfbizBeansWrapper` substitutes a `MapModel` whose key set is the map's own, and overrides nothing else. `get()` is untouched, so member lookup still falls back to the bean model and templates keep calling methods on maps, including on `GenericValue`. `HtmlWidget.ExtendedWrapper` now extends it so that screen rendering picks up the same behaviour.

Three files, 82 insertions, 3 deletions.

### Two secondary effects

- `?keys` now follows the map's iteration order rather than being arbitrary.
- An empty map now reports `?size` of `0`. On trunk it reports `28` while `?has_content` already reports `false`, so this brings the two into agreement.

### Verification

- Full unit suite on this branch: 78 classes, 589 tests, 0 failures, 0 errors, 0 skipped.
- `checkstyleMain`, `codenarcMain`, `codenarcTest` and `javadoc` all pass, and the new class compiles clean under `javac -Xlint:all`.
- Each expression in the tables above was checked against freemarker 2.3.34, the version the build pins, before and after the change, including the `GenericValue` shaped case of a map that also exposes business methods.

These checks are all unit level; I have not yet exercised the change in a running instance, so a second pair of eyes on the screen rendering path would be very welcome.

I kept this patch to the production change alone. If reviewers would prefer regression tests alongside it, I have them ready and would be glad to push them to this branch. They cover the three built ins above, plus guards that method access still works both on plain maps and on map backed entity values.

> HashMaps are not properly rendered in FTL with current FTL integration
> ----------------------------------------------------------------------
>
>                 Key: OFBIZ-13164
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-13164
>             Project: OFBiz
>          Issue Type: Bug
>          Components: base
>            Reporter: Carsten Schinzer
>            Assignee: Mridul Pathak
>            Priority: Major
>
> When stating hashmap iterations in FTL as
>  
> <#list someHashMap as key, value>
> keys are listed, but contain methods.
>  
> According to [https://stackoverflow.com/questions/1497777/freemarker-iterating-over-hashmap-keys] this is caused by the FTP integration using BeansWrapper and not DefaultObjectWrapper in the FTP integration e.g. in class org.apache.ofbiz.base.util.template.FreeMarkerWorker
> First attempts to apply a simple fix are not successful hence logging the issue in order to:
>  * get help from others
>  * take a deeper look later
> Right now (as urgently required) I shall apply a fix on the FTL side and use the "very old way" mentioned on the above entry at 
> [https://stackoverflow.com/a/38272341]
>  



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