The UUID data type should validate that its value is an actual UUID

Note that in config schema, it does:

# A UUID.
uuid:
  type: string
  label: 'UUID'
  constraints:
    Uuid: {}

👆 That was coincidentally also the only config schema type using validation. but nothing was calling it ($config_entity->getTypedData()->validate() , so that mattered … not at all)

Ironic that it's still an issue outside of config!

Change looks good and makes sense.

As discussed on slack with phenaproxima, any current entities that abuse this non-validating behavior (first thing I could think of was somebody mis-using it to store an external ID when importing content from outside Drupal) will now be broken and, depending on submit flow, might be unable to be updated. So maybe not something we want to do in a patch release and should probably have a change record.

I assume the 128 comes from

class UuidItem extends StringItem {

  /**
   * {@inheritdoc}
   */
  public static function defaultStorageSettings() {
    return [
      'max_length' => 128,

While that seems like it would be hard to change now, is it worth adding a more correct Length constraint while we are here?

Moving back to needs review for #10.

Thanks for pointing this out.

The max_length = 128 on UuidItem is a historical storage setting rather than an intended UUID length. With the Uuid constraint in place, format (and effective length) is already validated.

Would adjusting the length or storage behavior be considered a broader, potentially BC-impacting change?

Would adjusting the length or storage behavior be considered a broader, potentially BC-impacting change?

I think so. We'd have to deal with potential issues of data loss. It seems like this would seriously complicate what's currently a fairly simple issue that's impacting a high-profile Drupal CMS module. Maybe we should commit this as-is. Just my $0.02.

General question:
I see some FieldType constraints added in the definition, for example "timestamp": https://git.drupalcode.org/project/drupal/-/blob/main/core/lib/Drupal/Co...
and others in getConstraints(), for example "email": https://git.drupalcode.org/project/drupal/-/blob/main/core/lib/Drupal/Co...

and then there is this implementation for Uuid in propertyDefinitions

Is there any preference of implementing one over the other? I would expect this simple constraint to be added in the #[FieldType] definition itself

Let's add it in the FieldType definition, seems cleanest that way. The Email one is done so we can add the label to the error message.

The Uuid constraint is now applied in the FieldType definition

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Running PHPStan on changed files.

PHPStan: failed

----------------------------------------------------------------------------------------------------

PHPCS: failed

----------------------------------------------------------------------------------------------------

Not sure how these checks are generated, but the MR pipeline does not report any failed checks

The constraint has been added to the field type definition as requested. The change looks good to me.

Committed and pushed 5487950 to main. and 21d984c to 11.x. Thanks!

Correct the branch in the change record.

The tag is only used once so removing per Issue tags field and Issue tags -- special tags and for issue #3565085: Drupal core issue tag cleanup.