[OFBIZ-10194] ContentWrapper empty string result breaks simple FTL null check and default syntax

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 16.11.04
• Fix Version/s: Upcoming Branch
• Component/s: content, order, party, product
• Assignee: Michael Brohl
• Reporter: Martin Becker
• Created: 01/Feb/18 14:21
• Updated: 20/Mar/24 07:48
• Resolved: 19/Mar/24 15:29

Description

Since the changes to the ContentWrappers from Ticket OFBIZ-6701 the result for non existing content is an empty string instead of NULL.
Aside from my opinion, that this is generally a bad design preferred by those who do not like to check for null values within their code, this behavior breaks the simple FTL syntax for using an alternate (default) value for a non existing content, retrieved by a ContentWrapper like this:

<#assign categoryName = categoryContentWrapper.get("CATEGORY_NAME", "string")!category.internalName?default(category.productCategoryId) />

Basically this was done to get the non-existing-content cached within the *.content.rendered cache and let the simple condition 

if (cachedValue != null)

after a cache.get() respect this empty value. With a simple change to the condition to

if (cachedValue != null || cache.containsKey(cacheKey))

it is also possible to cache and successfully retrieve NULL values from the cache.

I observed this now during an upgrade of OFBiz 12 based application code to the current OFBiz release.

Besides this I did following refactorings consistently for all ContentWrapper implementations to reduce code redundancy:

• centralized default mimeTypeId retrieval (static Interface method in ContentWrapper)
• centralized encoding of result string via UtilEncoder (static Interface method in ContentWrapper)
• centralized/generalized candidate field value retrieval (static Interface method in ContentWrapper)
• harmonized content cache name to „xyz.content.rendered“, some wrappers did not use the „.rendered“ suffix in their cache name
• fixed some missing useCache parameter use in EntityQuery…cache()… calls

For Category and Product ContentWrapper I updated the parameter handling of the central getXyzContentAsText method where both, productId and product GenericValue are given but no check is performed, if both are matching if both are given (bad parameter signature, by the way). Now the product GV is looked up, if a productId is given, and the productId is used from a given product GV always, not only if it is missing. The drawback is, that there will always be a lookup for Product/ProductCategory GV, even if a content entry could be found with the productId/productCategoryId only. On the other hand, the GV is always part of the content rendering input context, currently it is missing there, if only a ID is given as parameter, again not really consistent.

I did not wanted to change this for all content wrappers directly before getting a feedback for it, even if it would be more consistent to have a content rendering context with a product GV as input, independent of the original call parameters productId and/or product GV.

Issue Links

• relates to: OFBIZ-6701 Enhancement in the content wrappers
• links to: GitHub Pull Request #654
• links to: GitHub Pull Request #686