Preconditions and environment
- Magento version: reproduced identically on 2.4.4, 2.4.5, 2.4.6, 2.4.7, 2.4.8 and 2.4.9
(Community Edition; vendor/magento/module-wishlist/Helper/Data.php is byte-for-byte
identical across all of these for the relevant method)
- Module:
magento/module-wishlist
- Customer is logged in and has requested the
wishlist customer-data section at least once
(e.g. via the default header mini-wishlist link)
Steps to reproduce
- Log in as a customer whose browser has already requested the
wishlist section once (so
Magento\Customer\Model\Session::hasDisplayOutOfStockProducts() has been set at least once).
- Call
customer/section/load?sections=wishlist again, with unchanged store configuration
(wishlist/wishlist_link/use_qty and cataloginventory/options/show_out_of_stock both
unchanged since step 1).
- Measure the server-side time spent in
Wishlist\Helper\Data::getItemCount() on this second
call, e.g. by timing calculate() directly.
Expected result
Since none of the four short-circuit conditions in getItemCount() actually changed since the
first call, the cached value from Magento\Customer\Model\Session::getWishlistItemCount()
should be reused, and calculate() should not run again.
Actual result
calculate() runs on every single call, regardless of whether anything actually changed. Root
cause: the third condition only checks whether a session value was ever set, not whether it
changed, unlike the sibling condition right after it.
// vendor/magento/module-wishlist/Helper/Data.php
public function getItemCount()
{
...
if (!$this->_customerSession->hasWishlistItemCount() ||
$currentDisplayType != $storedDisplayType ||
$this->_customerSession->hasDisplayOutOfStockProducts() || // <- always true after the first calculate()
$currentDisplayOutOfStockProducts != $storedDisplayOutOfStockProducts
) {
$this->calculate();
}
...
}
public function calculate()
{
...
$this->_customerSession->setDisplayOutOfStockProducts(...); // <- sets it unconditionally every time
...
}
calculate() unconditionally calls setDisplayOutOfStockProducts(), which means
hasDisplayOutOfStockProducts() becomes permanently true for the rest of the customer session
after the very first call. From that point on, the third condition alone forces a full
recalculation on every subsequent getItemCount() call, defeating the caching this method is
supposed to provide entirely, for the remainder of the session.
Additional information
Impact
For customers with a non-trivial wishlist (real product collection query, price rendering,
etc.), this turns every customer/section/load call that includes the wishlist section into a
full, uncached recalculation. Measured on a production-like shop: 1.7-2.4 seconds per call,
consistently, instead of a cheap cache read. This also extends how long the PHP session lock is
held on every such AJAX call, compounding under concurrent requests.
Suggested fix
Make the third condition a value comparison, consistent with its sibling right below it:
if (!$this->_customerSession->hasWishlistItemCount() ||
$currentDisplayType != $storedDisplayType ||
$currentDisplayOutOfStockProducts != $storedDisplayOutOfStockProducts
) {
$this->calculate();
}
(the now-redundant hasDisplayOutOfStockProducts() check can simply be dropped, since the value
comparison right after it already covers "changed since last time", including "never set
before" as long as getDisplayOutOfStockProducts() returns null when unset.)
Additional context
No existing public issue or community patch for this was found after a search. The bug appears
to have been present unchanged since at least 2.4.4.
Release note
No response
Triage and priority
Preconditions and environment
(Community Edition;
vendor/magento/module-wishlist/Helper/Data.phpis byte-for-byteidentical across all of these for the relevant method)
magento/module-wishlistwishlistcustomer-data section at least once(e.g. via the default header mini-wishlist link)
Steps to reproduce
wishlistsection once (soMagento\Customer\Model\Session::hasDisplayOutOfStockProducts()has been set at least once).customer/section/load?sections=wishlistagain, with unchanged store configuration(
wishlist/wishlist_link/use_qtyandcataloginventory/options/show_out_of_stockbothunchanged since step 1).
Wishlist\Helper\Data::getItemCount()on this secondcall, e.g. by timing
calculate()directly.Expected result
Since none of the four short-circuit conditions in
getItemCount()actually changed since thefirst call, the cached value from
Magento\Customer\Model\Session::getWishlistItemCount()should be reused, and
calculate()should not run again.Actual result
calculate()runs on every single call, regardless of whether anything actually changed. Rootcause: the third condition only checks whether a session value was ever set, not whether it
changed, unlike the sibling condition right after it.
calculate()unconditionally callssetDisplayOutOfStockProducts(), which meanshasDisplayOutOfStockProducts()becomes permanentlytruefor the rest of the customer sessionafter the very first call. From that point on, the third condition alone forces a full
recalculation on every subsequent
getItemCount()call, defeating the caching this method issupposed to provide entirely, for the remainder of the session.
Additional information
Impact
For customers with a non-trivial wishlist (real product collection query, price rendering,
etc.), this turns every
customer/section/loadcall that includes thewishlistsection into afull, uncached recalculation. Measured on a production-like shop: 1.7-2.4 seconds per call,
consistently, instead of a cheap cache read. This also extends how long the PHP session lock is
held on every such AJAX call, compounding under concurrent requests.
Suggested fix
Make the third condition a value comparison, consistent with its sibling right below it:
(the now-redundant
hasDisplayOutOfStockProducts()check can simply be dropped, since the valuecomparison right after it already covers "changed since last time", including "never set
before" as long as
getDisplayOutOfStockProducts()returnsnullwhen unset.)Additional context
No existing public issue or community patch for this was found after a search. The bug appears
to have been present unchanged since at least 2.4.4.
Release note
No response
Triage and priority