Issue #2049465 by niko-, andypost, mradcliffe, claudiu.cristea, LinL: Fixed...

Issue #2049465 by niko-, andypost, mradcliffe, claudiu.cristea, LinL: Fixed Upgrade of image styles and effects broken.

core/modules/image/image.install

@@ -58,7 +58,7 @@ function image_requirements($phase) {
 }
 
 /**
- * Loads all effects for an image style.
+ * Loads all effects for an image style from backend.
  *
  * @param array $style
  *   The image style (array) to retrieve effects for.
@@ -68,28 +68,118 @@ function image_requirements($phase) {
  *
  * @see image_update_8000()
  */
-function _image_update_get_style_with_effects(array $style) {
-  // Retrieve image effects.
-  $effects = array();
+function _image_update_get_backend_effects(array $style) {
   $result = db_select('image_effects', NULL, array('fetch' => PDO::FETCH_ASSOC))
     ->fields('image_effects')
     ->condition('isid', $style['isid'])
     ->execute();
+  $effects = array();
   foreach ($result as $effect) {
     $effect['data'] = unserialize($effect['data']);
+    $effects[] = $effect;
+  }
+
+  return _image_update_convert_effects($effects);
+}
+
+/**
+ * Retuns the default image styles shipped with Drupal 7.
+ *
+ * @return array
+ *   An associative array keyed by style id, having the style and associated
+ *   effects as values. The array values are defined to be usable in
+ *   image_update_8000().
+ *
+ * @see image_update_8000()
+ * @see https://api.drupal.org/api/drupal/modules%21image%21image.module/function/image_image_default_styles/7
+ */
+function _image_update_get_default_styles() {
+  // Clone from Drupal 7 image_image_default_styles().
+  $styles = array(
+    'thumbnail' => array(
+      'label' => 'Thumbnail (100x100)',
+      'effects' => array(
+        array(
+          'name' => 'image_scale',
+          'data' => array(
+            'width' => '100',
+            'height' => '100',
+            'upscale' => '1',
+          ),
+          'weight' => '0',
+        ),
+      ),
+    ),
+    'medium' => array(
+      'label' => 'Medium (220x220)',
+      'effects' => array(
+        array(
+          'name' => 'image_scale',
+          'data' => array(
+            'width' => '220',
+            'height' => '220',
+            'upscale' => '1',
+          ),
+          'weight' => '0',
+        ),
+      ),
+    ),
+    'large' => array(
+      'label' => 'Large (480x480)',
+      'effects' => array(
+        array(
+          'name' => 'image_scale',
+          'data' => array(
+            'width' => '480',
+            'height' => '480',
+            'upscale' => '0',
+          ),
+          'weight' => '0',
+        ),
+      ),
+    ),
+  );
+
+  // Add uuid, status, langcode and convert effects.
+  $langcode = language_default()->id;
+  $uuid = \Drupal::service('uuid');
+  foreach ($styles as $id => $style) {
+    $styles[$id]['name'] = $id;
+    $styles[$id]['uuid'] = $uuid->generate();
+    $styles[$id]['status'] = 1;
+    $styles[$id]['langcode'] = $langcode;
+    $styles[$id]['effects'] = _image_update_convert_effects($style['effects']);
+  }
+
+  return $styles;
+}
 
-    // Generate a unique image effect ID for the effect.
-    $uuid = \Drupal::service('uuid');
+/**
+ * Normalizes effects from Drupal 7 to Drupal 8 format.
+ *
+ * @param array $legacy_effects
+ *   An array containing a list of effects obtained from database or from code.
+ *
+ * @return array
+ *   A list of effects keyed by effect uuid and having the effect definition in
+ *   a Drupal 8 configuration format.
+ */
+function _image_update_convert_effects(array $legacy_effects) {
+  $uuid = \Drupal::service('uuid');
+  $effects = array();
+
+  foreach ($legacy_effects as $effect) {
     $effect['uuid'] = $uuid->generate();
 
     // Use 'id' instead of 'name'.
     $effect['id'] = $effect['name'];
 
-    // Clear out legacy keys.
+    // Clear out legacy keys, if case.
     unset($effect['isid'], $effect['ieid'], $effect['name']);
 
     $effects[$effect['uuid']] = $effect;
   }
+
   return $effects;
 }
 
@@ -97,22 +187,36 @@ function _image_update_get_style_with_effects(array $style) {
  * Convert existing image styles to the new config system.
  */
 function image_update_8000() {
-  $styles = array();
+  $langcode = language_default()->id;
+  $uuid = \Drupal::service('uuid');
+
   $result = db_select('image_styles', NULL, array('fetch' => PDO::FETCH_ASSOC))
     ->fields('image_styles')
-    ->execute()
-    ->fetchAllAssoc('name', PDO::FETCH_ASSOC);
-  foreach ($result as $style_name => $style) {
-    $style['effects'] = _image_update_get_style_with_effects($style);
-    $styles[$style_name] = $style;
+    ->execute();
+
+  // Prepare image styles from backend.
+  $styles = array();
+  foreach ($result as $style) {
+    $styles[$style['name']] = array(
+      'name' => $style['name'],
+      'label' => $style['label'],
+      'uuid' => $uuid->generate(),
+      'status' => 1,
+      'langcode' => $langcode,
+      'effects' => _image_update_get_backend_effects($style),
+    );
   }
 
-  // Convert each style into a configuration object.
-  foreach ($styles as $name => $style) {
-    $config = \Drupal::config('image.style.' . $name);
-    $config->set('name', $name);
-    $config->set('effects', $style['effects']);
-    $config->save();
+  // Append Drupal 7 default image styles from code. Note that some of them may
+  // be overwritten in the backend. Using this array operator assures that we
+  // are migrating the overwritten version of the image style.
+  $styles += _image_update_get_default_styles();
+
+  // Save as Drupal 8 configuration.
+  foreach ($styles as $id => $style) {
+    \Drupal::config('image.style.' . $id)
+      ->setData($style)
+      ->save();
   }
 }
 

core/modules/system/lib/Drupal/system/Tests/Upgrade/ImageUpgradePathTest.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\system\Tests\Upgrade;
 
+use Drupal\Component\Utility\String;
+
 /**
  * Test upgrade of overridden and custom image styles.
  */
@@ -33,9 +35,12 @@ public function setUp() {
   public function testImageStyleUpgrade() {
     $this->assertTrue($this->performUpgrade(), 'The upgrade was completed successfully.');
 
-    // Verify that image styles were properly upgraded.
+    // A custom user image style.
     $expected_styles['test-custom'] = array(
       'name' => 'test-custom',
+      'label' => 'Test custom',
+      'status' => '1',
+      'langcode' => 'en',
       'effects' => array(
         'image_rotate' => array(
           'id' => 'image_rotate',
@@ -53,8 +58,12 @@ public function testImageStyleUpgrade() {
         ),
       ),
     );
+    // An image style shipped with Drupal 7 but overwritten by the user.
     $expected_styles['thumbnail'] = array(
       'name' => 'thumbnail',
+      'label' => 'Thumbnail (100x100)',
+      'status' => '1',
+      'langcode' => 'en',
       'effects' => array (
         'image_scale' => array(
           'id' => 'image_scale',
@@ -67,11 +76,38 @@ public function testImageStyleUpgrade() {
         ),
       ),
     );
+    // A non-overwritten image style. Has to be imported in Drupal 8 as well.
+    $expected_styles['large'] = array(
+      'name' => 'large',
+      'label' => 'Large (480x480)',
+      'status' => '1',
+      'langcode' => 'en',
+      'effects' => array (
+        'image_scale' => array(
+          'id' => 'image_scale',
+          'data' => array (
+            'width' => '480',
+            'height' => '480',
+            'upscale' => '0',
+          ),
+          'weight' => '0',
+        ),
+      ),
+    );
+    $config_factory = $this->container->get('config.factory');
     foreach ($expected_styles as $name => $style) {
-      $config = \Drupal::config('image.style.' . $name);
+      $configured_style = $config_factory->get('image.style.' . $name)->get();
+
+      // Maybe the image style hasn't been imported?
+      $imported = !empty($configured_style);
+      $this->assertTrue($imported, String::format('%name has been imported', array('%name' => $style['label'])));
+      if (!$imported) {
+        continue;
+      }
+
       // Replace placeholder with image effect name keys with UUID's generated
       // during by the image style upgrade functions.
-      foreach ($config->get('effects') as $uuid => $effect) {
+      foreach ($configured_style['effects'] as $uuid => $effect) {
         // Copy placeholder data.
         $style['effects'][$uuid] = $style['effects'][$effect['id']];
         // Set the missing uuid key as this is unknown because it is a UUID.
@@ -79,12 +115,14 @@ public function testImageStyleUpgrade() {
         // Remove the placeholder data.
         unset($style['effects'][$effect['id']]);
       }
-      $this->assertEqual($this->sortByKey($style), $config->get(), format_string('@first is equal to @second.', array(
-        '@first' => var_export($this->sortByKey($style), TRUE),
-        '@second' => var_export($config->get(), TRUE),
-      )));
+      // Make sure UUID assigned to new style.
+      $this->assertTrue($configured_style['uuid'], 'UUID assigned to converted style.');
+      // Copy generated UUID to compared style.
+      $style['uuid'] = $configured_style['uuid'];
+      $this->assertEqual($this->sortByKey($style), $this->sortByKey($configured_style));
     }
   }
+
   /**
    * Sorts all keys in configuration data.
    *
@@ -102,7 +140,7 @@ public function sortByKey(array $data) {
     ksort($data);
     foreach ($data as &$value) {
       if (is_array($value)) {
-        $this->sortByKey($value);
+        $value = $this->sortByKey($value);
       }
     }
     return $data;

core/modules/system/tests/upgrade/drupal-7.image.database.php

@@ -14,16 +14,19 @@
 db_insert('image_styles')->fields(array(
   'isid',
   'name',
+  'label'
 ))
 // Override thumbnail style.
 ->values(array(
   'isid' => '1',
   'name' => 'thumbnail',
+  'label' => 'Thumbnail (100x100)',
 ))
 // Custom style.
 ->values(array(
   'isid' => '2',
   'name' => 'test-custom',
+  'label' => 'Test custom',
 ))
 ->execute();