Problem/Motivation

In #2110615: Do not ship with default UUIDs we removed default configuration entity UUIDs remove all default configuration provided by core (modules, themes and profiles). However, UUIDs are still present in image style configuration as effect IDs. For example, image.style.large.yml contents are:

name: large
label: 'Large (480x480)'
effects:
  ddd73aa7-4bd6-4c85-b600-bdf2b1628d1d:
    id: image_scale
    data:
      width: 480
      height: 480
      upscale: true
    weight: 0
    uuid: ddd73aa7-4bd6-4c85-b600-bdf2b1628d1d
langcode: en

In some ways this is not a problem because the use of the UUID is totally internal to the image style. It just exists so that there can be multiple effects of the same type - ie. you can added another image_scale transformation to this style. However this shows us that we really have a naming problem here where id: image_scale is not really an identifier but it is an effect type. And we're are just using UUID as a convenient way of generating a unique ID.

Proposed resolution

Depends on what we decide.

Remaining tasks

Decide if we need to change this.

User interface changes

Urls like admin/config/media/image-styles/manage/large/effects/ddd73aa7-4bd6-4c85-b600-bdf2b1628d1d will change

API changes

Probably

Comments

alexpott’s picture

xjm’s picture

Issue tags: +beta target
jessebeach’s picture

Change the id key to effect.

yched’s picture

Does this unique identifier have any purpose besides being able to generate unambiguous admin URIs for "the admin config form for the Nth 'scale' effect of the 'large' style" ?

Coz it seems we could simply use effect ordering and numeric indexes here ?
admin/config/media/image-styles/manage/large/effects/[N] ?

mtift’s picture

Changing id to effect makes sense to me. It also seems like uuid should have another name.

tim.plunkett’s picture

It's called 'id' because it's the plugin ID:


/**
 * Scales an image resource.
 *
 * @ImageEffect(
 *   id = "image_scale",
 *   label = @Translation("Scale"),
 *   description = @Translation("Scaling will maintain the aspect-ratio of the original image. If only a single dimension is specified, the other dimension will be calculated.")
 * )
 */
class ScaleImageEffect extends ResizeImageEffect {

We can change it to effect, the DefaultPluginBag::$pluginKey controls that.

However, unless we add a machine name field to the image effect form (not ideal), I think we're stuck with a UUID.

xjm’s picture

Issue tags: -beta target +beta deadline
catch’s picture

I think it's more this?

xjm’s picture

Well, technically it would be a data model change for image styles. (Earlier in the spring we were making config key changes like these beta blockers, even. This isn't critical, though.) It is a small upgrade path, though.

jhedstrom’s picture

Has a path here been decided on? I'd be happy to help implement.

xjm’s picture

Issue tags: -beta target

This issue was marked as a beta target for the 8.0.x beta, but is not applicable as an 8.1.x beta target, so untagging.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Steven Jones’s picture

I'm copying core's architecture of image styles and effects for a module I'm porting to D8, so have an interest in this.

I'm not sure how much we can/could change things in the Drupal 8 dev cycle but a way forward would be to just mirror what views does:

  1. Change the uuid property name to be effect_id or somesuch
  2. Change the implementation to generate something other than a uuid to go in the new 'effect_id' column, say the effect plugin with _1, _2, etc appended until it was unique. Views must have some code that does this, so there's good precedent in core.
  3. Change the id property to plugin_id if wanted
  4. Update the config to reflect those changes

From a quick glance at the code, it looks like the UUID is used in the URL, but not outside the context of an ImageStyle, so it only needs to be unique within each style.

I'm guessing that the only people who really care about the internal structure of the config changing would be people who are handling and managing uuids, config_devel, config_update types, and they might welcome the change of removing things that aren't really UUIDs from config.
I appreciate that in reality that this change is almost entirely cosmetic however.

dpagini’s picture

Sorry if I'm missing this point in the discussion, but what's the downside of using a YML array and then using a numeric index in the URL?

name: large
label: 'Large (480x480)'
effects:
  -
    id: image_scale
    data:
      width: 480
      height: 480
      upscale: true
    weight: 0
  -
    id: image_scale
    data:
      width: 240
      height: 240
      upscale: true
    weight: 0
langcode: en

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.