Issue #2383165 by sidharrell, alexpott: ResponsiveImageStyle config entities...

Issue #2383165 by sidharrell, alexpott: ResponsiveImageStyle config entities should depend on the image styles they use

core/modules/responsive_image/src/Entity/ResponsiveImageStyle.php

@@ -8,6 +8,7 @@
 namespace Drupal\responsive_image\Entity;
 
 use Drupal\Core\Config\Entity\ConfigEntityBase;
+use Drupal\image\Entity\ImageStyle;
 use Drupal\responsive_image\ResponsiveImageStyleInterface;
 
 /**
@@ -184,6 +185,11 @@ public function calculateDependencies() {
     foreach ($providers as $provider => $type) {
       $this->addDependency($type, $provider);
     }
+    // Extract all the styles from the image style mappings.
+    $styles = ImageStyle::loadMultiple($this->getImageStyleIds());
+    array_walk($styles, function ($style) {
+      $this->addDependency('config', $style->getConfigDependencyName());
+    });
     return $this->dependencies;
   }
 
@@ -221,4 +227,25 @@ public function getImageStyleMapping($breakpoint_id, $multiplier) {
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getImageStyleIds() {
+    $image_styles = [];
+    foreach ($this->getImageStyleMappings() as $image_style_mapping) {
+      // Only image styles of non-empty mappings should be loaded.
+      if (!$this::isEmptyImageStyleMapping($image_style_mapping)) {
+        switch ($image_style_mapping['image_mapping_type']) {
+          case 'image_style':
+            $image_styles[] = $image_style_mapping['image_mapping'];
+            break;
+          case 'sizes':
+            $image_styles = array_merge($image_styles, $image_style_mapping['image_mapping']['sizes_image_styles']);
+            break;
+        }
+      }
+    }
+    return array_values(array_filter(array_unique($image_styles)));
+  }
+
 }

core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php

@@ -200,21 +200,7 @@ public function viewElements(FieldItemListInterface $items) {
     $cache_tags = [];
     if ($responsive_image_style) {
       $cache_tags = Cache::mergeTags($cache_tags, $responsive_image_style->getCacheTags());
-      foreach ($responsive_image_style->getImageStyleMappings() as $image_style_mapping) {
-        // Only image styles of non-empty mappings should be loaded.
-        if (!$responsive_image_style::isEmptyImageStyleMapping($image_style_mapping)) {
-
-          if ($image_style_mapping['image_mapping_type'] == 'image_style') {
-            // This mapping has one image style, add it.
-            $image_styles_to_load[] = $image_style_mapping['image_mapping'];
-          }
-          else {
-            // This mapping has multiple image styles, merge them.
-            $image_style_mapping['image_mapping']['sizes_image_styles'] = array_filter($image_style_mapping['image_mapping']['sizes_image_styles']);
-            $image_styles_to_load = array_merge($image_styles_to_load, $image_style_mapping['image_mapping']['sizes_image_styles']);
-          }
-        }
-      }
+      $image_styles_to_load = $responsive_image_style->getImageStyleIds();
     }
 
     // If there is a fallback image style, add it to the image styles to load.

core/modules/responsive_image/src/ResponsiveImageStyleInterface.php

@@ -126,4 +126,11 @@ public function addImageStyleMapping($breakpoint_id, $multiplier, array $image_s
    */
   public function removeImageStyleMappings();
 
+  /**
+   * Gets all the image styles IDs involved in the responsive image mapping.
+   *
+   * @return string[]
+   */
+  public function getImageStyleIds();
+
 }

core/modules/responsive_image/tests/src/Unit/ResponsiveImageStyleConfigEntityUnitTest.php

@@ -65,8 +65,42 @@ protected function setUp() {
    * @covers ::calculateDependencies
    */
   public function testCalculateDependencies() {
-    $entity = new ResponsiveImageStyle(array('breakpoint_group' => 'test_group'));
+    // Set up image style loading mock.
+    $styles = [];
+    foreach (['small', 'medium', 'large'] as $style) {
+      $mock = $this->getMock('Drupal\Core\Config\Entity\ConfigEntityInterface');
+      $mock->expects($this->any())
+        ->method('getConfigDependencyName')
+        ->willReturn('image.style.' . $style);
+      $styles[$style] = $mock;
+    }
+    $storage = $this->getMock('\Drupal\Core\Config\Entity\ConfigEntityStorageInterface');
+    $storage->expects($this->any())
+      ->method('loadMultiple')
+      ->with(array_keys($styles))
+      ->willReturn($styles);
+    $this->entityManager->expects($this->any())
+      ->method('getStorage')
+      ->with('image_style')
+      ->willReturn($storage);
+    $this->entityManager->expects($this->any())
+      ->method('getEntityTypeFromClass')
+      ->with('Drupal\image\Entity\ImageStyle')
+      ->willReturn('image_style');
+
+    $entity = new ResponsiveImageStyle(['breakpoint_group' => 'test_group']);
     $entity->setBreakpointGroup('test_group');
+    $entity->addImageStyleMapping('test_breakpoint', '1x', ['image_mapping_type' => 'image_style', 'image_mapping' => 'small']);
+    $entity->addImageStyleMapping('test_breakpoint', '2x', [
+      'image_mapping_type' => 'sizes',
+      'image_mapping' => [
+        'sizes' => '(min-width:700px) 700px, 100vw',
+        'sizes_image_styles' => [
+          'medium' => 'medium',
+          'large' => 'large',
+        ],
+      ],
+    ]);
 
     $this->breakpointManager->expects($this->any())
       ->method('getGroupProviders')
@@ -74,8 +108,9 @@ public function testCalculateDependencies() {
       ->willReturn(array('bartik' => 'theme', 'toolbar' => 'module'));
 
     $dependencies = $entity->calculateDependencies();
-    $this->assertContains('toolbar', $dependencies['module']);
-    $this->assertContains('bartik', $dependencies['theme']);
+    $this->assertEquals(['toolbar'], $dependencies['module']);
+    $this->assertEquals(['bartik'], $dependencies['theme']);
+    $this->assertEquals(['image.style.large', 'image.style.medium', 'image.style.small'], $dependencies['config']);
   }
 
   /**