Skip to content

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

Merged
mridulpathak merged 1 commit into
apache:trunkfrom
toaditi:ofbiz-13164-ftl-map-key-iteration
Aug 10, 2026
Merged

Fixed: HashMaps are not properly rendered in FTL with current FTL integration (OFBIZ-13164)#1615
mridulpathak merged 1 commit into
apache:trunkfrom
toaditi:ofbiz-13164-ftl-map-key-iteration

Conversation

@toaditi

@toaditi toaditi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

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.

…egration (OFBIZ-13164)

FreeMarkerWorker built a plain BeansWrapper, so every java.util.Map reached
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, so
key enumeration saw accessors mixed in with the real entries. On a two entry
map, ?size reported 30, ?keys listed getClass, put, entrySet and the rest
alongside alpha and beta, and <#list aMap as key, value> failed outright,
because the values behind those synthetic keys are methods rather than
strings.

Added OfbizBeansWrapper, which substitutes a MapModel whose key set is the
map's own. Only enumeration changes: member lookup still falls back to the
bean model, so templates keep calling methods on maps. That distinction
matters because GenericEntity implements Map, so switching to
DefaultObjectWrapper or enabling simpleMapWrapper would have broken every
${anEntity.getRelatedOne(...)}, ${anEntity.getString(...)} and
${aMap.get(...)} currently in the templates.

HtmlWidget.ExtendedWrapper now extends OfbizBeansWrapper so that screen
rendering picks up the same behaviour.

Two secondary effects are worth noting: ?keys now follows the map's iteration
order rather than being arbitrary, and an empty map now reports ?size 0,
which is consistent with the ?has_content it already reported.

Thanks to Carsten Schinzer for reporting this issue and for identifying the
BeansWrapper versus DefaultObjectWrapper mechanism behind it.
@mridulpathak
mridulpathak merged commit 70b9901 into apache:trunk Aug 10, 2026
7 checks passed
mridulpathak added a commit that referenced this pull request Aug 10, 2026
…egration (OFBIZ-13164) (#1616)

Backported from trunk (#1615). OfbizBeansWrapper substitutes a MapModel whose key set is the map's own, instead of FreeMarker's stock union of the map's keys and its bean property names, so ?keys, ?size, and <#list map as key, value> stop seeing accessors like getClass or entrySet mixed in with the real entries; HtmlWidget.ExtendedWrapper picks up the same fix for screen rendering. Cherry-picked cleanly onto release24.09 with no conflicts and no adaptation needed.

Thanks: Aditi Patel (author of the trunk fix) and Carsten Schinzer (original reporter).

Co-authored-by: toaditi <aditi.patel@hotwax.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants