[PR] Fixed: HashMaps are not properly rendered in FTL with current FTL integration (OFBIZ-13164) [ofbiz-framework]

["toaditi (via GitHub)" <gi...@apache.org>]

toaditi opened a new pull request, #1615:
URL: https://github.com/apache/ofbiz-framework/pull/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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ofbiz.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fixed: HashMaps are not properly rendered in FTL with current FTL integration (OFBIZ-13164) [ofbiz-framework]

["mridulpathak (via GitHub)" <gi...@apache.org>]

mridulpathak merged PR #1615:
URL: https://github.com/apache/ofbiz-framework/pull/1615


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ofbiz.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org