Describe your bug or feature request.

Currently class ProductVariation has method getCacheTagsToInvalidate()

  /**
   * {@inheritdoc}
   */
  public function getCacheTagsToInvalidate() {
    $tags = parent::getCacheTagsToInvalidate();
    // Invalidate the variations view builder and product caches.
    return Cache::mergeTags($tags, [
      'commerce_product:' . $this->getProductId(),
      'commerce_product_variation_view',
    ]);
  }

Tags that end with _view used by EntityViewBuilder class and invalidates when entity view display is changed, which is logical. (check EntityViewBuilder::getCacheTags())

Method ProductVariation::getCacheTagsToInvalidate() returns commerce_product_variation_view tag.
This means, that after updating of product variation, the render cache for all product variations will be invalidated. When you update 1 product variation, the render cache for all product variations is invalidated.

Proposed resolution:
Remove commerce_product_variation_view tag from the ProductVariation::getCacheTagsToInvalidate()

CommentFileSizeAuthor
#4 remove-cache-tag-3285023-4.patch2.14 KB_shY
#3 remove-cache-tag-tests-3285023-3.patch1.59 KB_shY
#2 commerce-remove-cache-tag-3285023-2.patch567 bytes_shY
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

_shY created an issue. See original summary.

_shY’s picture

_shY
Primary language Ukrainian
Location Ukraine, Lutsk 🇺🇦
CreditAttribution: _shY at DevBranch, Drupal Ukraine Community commented
FileSize
commerce-remove-cache-tag-3285023-2.patch567 bytes
_shY’s picture

_shY
Primary language Ukrainian
Location Ukraine, Lutsk 🇺🇦
CreditAttribution: _shY at DevBranch, Drupal Ukraine Community commented
FileSize
remove-cache-tag-tests-3285023-3.patch1.59 KB

Found why tests fail.

Tests failed with the patch because after injecting the field to the view display, the cache wasn't clear and the field doesn't display correctly.

 /** @var \Drupal\Core\Entity\Entity\EntityViewDisplay $variation_view_display */
    $variation_view_display = commerce_get_entity_display('commerce_product_variation', 'default', 'view');
    $variation_view_display->removeComponent('title');
    $variation_view_display->setComponent('attribute_color', [
      'label' => 'above',
      'type' => 'entity_reference_label',
    ]);
    $variation_view_display->setComponent('sku', [
      'label' => 'hidden',
      'type' => 'string',
    ]);
    $variation_view_display->save();

    $this->drupalGet($this->product->toUrl());
    $this->assertSession()->pageTextContains('INJECTION-CYAN');

Meanwhile, the previous test testInjectedVariationDefault(), made almost the same validation, but added a small thing. Submitted product variation view display form after field injection.

   /** @var \Drupal\Core\Entity\Entity\EntityViewDisplay $variation_view_display */
    $variation_view_display = commerce_get_entity_display('commerce_product_variation', 'default', 'view');
    $variation_view_display->removeComponent('title');
    $variation_view_display->setComponent('attribute_color', [
      'label' => 'above',
      'type' => 'entity_reference_label',
    ]);
    // Set the display for the SKU.
    $variation_view_display->setComponent('sku', [
      'label' => 'hidden',
      'type' => 'string',
    ]);

    $variation_view_display->save();

    // Have to call this save to get the cache to clear, we set the tags
    // correctly in a hook, but unless you trigger the submit it doesn't seem
    // to clear. Something additional happens on save that we're missing.
    $this->drupalGet('admin/commerce/config/product-variation-types/default/edit/display');
    $this->submitForm([], 'Save');

Comment said:

Something additional happens on save that we're missing

After saving the view display form, invalidated commerce_product_variation_view tag, and everything works fine.

So, I added view display form submitting to the testInjectedVariationFromUrl() test.
Here is a patch only with changes for the Tests, to be sure that it doesn't break anything.

_shY’s picture

_shY
Primary language Ukrainian
Location Ukraine, Lutsk 🇺🇦
CreditAttribution: _shY at DevBranch, Drupal Ukraine Community commented
Status: Active » Needs review
FileSize
remove-cache-tag-3285023-4.patch2.14 KB

And here is a full patch: with tests and cache tag removes.

abramm’s picture

abramm
Primary language Ukrainian
Location Lutsk
CreditAttribution: abramm at DevBranch, Drupal Ukraine Community commented
Status: Needs review » Reviewed & tested by the community

I can confirm we're using this patch on our project and it passed our QA.

This patch solves a huge performance degradation for sites actively using products/variations displays; e.g. in our case, we have a Search API view displaying rendered products displays. The view was slowing down dramatically due to entity displays cache misses every time any change to products data was made (even if the product being changed is not displayed).

I'll change the status to RTBC but since me and @_shY are working on the same project it'd be good if someone else could take a look as well.

  • jsacksick committed 7d9eb31 on 8.x-2.x authored by _shY 
    Issue #3285023 by _shY, abramm: Remove the "...
jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
Status: Reviewed & tested by the community » Fixed

Committed!

  • jsacksick committed f6e4380 on 3.0.x authored by _shY 
    Issue #3285023 by _shY, abramm: Remove the "...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.