[
https://issues.apache.org/jira/browse/OFBIZ-10194?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jacques Le Roux updated OFBIZ-10194:
------------------------------------
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:
{code:java}
<#assign categoryName = categoryContentWrapper.get("CATEGORY_NAME",
"string")!category.internalName?default(category.productCategoryId) />{code}
Basically this was done to get the non-existing-content cached within the
*.content.rendered cache and let the simple condition
{code:java}
if (cachedValue != null){code}
after a cache.get() respect this empty value. With a simple change to the
condition to
{code:java}
if (cachedValue != null || cache.containsKey(cacheKey)){code}
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.
was:
Since the changes to the ContentWrappers from Ticket
https://issues.apache.org/jira/browse/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:
{code:java}
<#assign categoryName = categoryContentWrapper.get("CATEGORY_NAME",
"string")!category.internalName?default(category.productCategoryId) />{code}
Basically this was done to get the non-existing-content cached within the
*.content.rendered cache and let the simple condition
{code:java}
if (cachedValue != null){code}
after a cache.get() respect this empty value. With a simple change to the
condition to
{code:java}
if (cachedValue != null || cache.containsKey(cacheKey)){code}
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.
> ContentWrapper empty string result breaks simple FTL null check and default
> syntax
> ----------------------------------------------------------------------------------
>
> Key: OFBIZ-10194
> URL: https://issues.apache.org/jira/browse/OFBIZ-10194
> Project: OFBiz
> Issue Type: Bug
> Components: content, order, party, product
> Affects Versions: 16.11.04
> Reporter: Martin Becker
> Assignee: Michael Brohl
> Priority: Minor
> Attachments: OFBiz-10194_ContentWrappers.patch
>
>
> 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:
> {code:java}
> <#assign categoryName = categoryContentWrapper.get("CATEGORY_NAME",
> "string")!category.internalName?default(category.productCategoryId) />{code}
> Basically this was done to get the non-existing-content cached within the
> *.content.rendered cache and let the simple condition
> {code:java}
> if (cachedValue != null){code}
> after a cache.get() respect this empty value. With a simple change to the
> condition to
> {code:java}
> if (cachedValue != null || cache.containsKey(cacheKey)){code}
> 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.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
