Problem/Motivation
ProductVariationAttributeMapper::prepareAttributes() builds a filter callback for each attribute after the first. For every variation, that callback calls getAttributeValueId() once per previous attribute, and each of those calls runs getAttributeFieldNames() (service lookup, field map, array_column()) plus an in_array() check. The cost is roughly variations × attributes² / 2.
This is invisible with 2–3 attributes, but it adds up quickly with more. On a site with a variation type that has 65 attribute fields, one product page made 20,800 getAttributeValueId() calls (5 add-to-cart forms). Locally, prepareAttributes() takes 70.9ms on a 42-variation product, and that cost is paid again on every AJAX attribute change.
Separately, when a product has variations of more than one type, a variation that has the current attribute but lacks one of the previous ones makes getAttributeValueId() throw InvalidArgumentException: Unknown attribute field name, which breaks the add-to-cart form. ProductVariationInterface::getAttributeValueId() doesn't document this exception (it returns "The attribute value ID, or NULL").
Steps to reproduce
- Create two variation types for one product type: one with "Color" and "Size" attributes, one with only "Size".
- Add variations of both types to a product.
- View the product: the add-to-cart form throws
InvalidArgumentExceptionwhile preparing the "Size" attribute.
For the performance part, create a variation type with many attributes (dozens) and profile prepareAttributes().
Proposed resolution
Read each variation's attribute values once with getAttributeValueIds() before the loop, and have the callback compare against those arrays ($values[$field_name] ?? NULL), returning early on the first mismatch. This makes the cost linear in the number of attributes and no longer calls getAttributeValueId(), so a variation without an attribute is treated as having no value and is filtered out instead of throwing.
No API change. getAttributeValueId() itself is unchanged.
Remaining tasks
- Review the MR.
- Kernel test for mixed variation types (included).
- Possible follow-up:
selectVariation()still callsgetAttributeValueId()per attribute and can throw for mixed variation types too.
User interface changes
None.
API changes
None.
Issue fork commerce-3635025
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
jsacksick commented