From ee822bb01d38859b3900301c9f7a5ff101489857 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Fri, 22 Feb 2019 08:45:40 +0000
Subject: [PATCH] Issue #3027745 by claudiu.cristea, idimopoulos, wengerk,
alexpott: UniqueFieldConstraint doesn't work for entities with string IDs
---
.../Constraint/UniqueFieldValueValidator.php | 13 +-
.../unique_field_constraint_test.info.yml | 6 +
.../unique_field_constraint_test.module | 18 +++
.../Validation/UniqueFieldConstraintTest.php | 115 ++++++++++++++++++
4 files changed, 149 insertions(+), 3 deletions(-)
create mode 100644 core/modules/system/tests/modules/unique_field_constraint_test/unique_field_constraint_test.info.yml
create mode 100644 core/modules/system/tests/modules/unique_field_constraint_test/unique_field_constraint_test.module
create mode 100644 core/tests/Drupal/KernelTests/Core/Validation/UniqueFieldConstraintTest.php
diff --git a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UniqueFieldValueValidator.php b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UniqueFieldValueValidator.php
index 3b8fd036f95c..3c34ec9665e2 100644
--- a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UniqueFieldValueValidator.php
+++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UniqueFieldValueValidator.php
@@ -23,9 +23,16 @@ public function validate($items, Constraint $constraint) {
$entity_type_id = $entity->getEntityTypeId();
$id_key = $entity->getEntityType()->getKey('id');
- $value_taken = (bool) \Drupal::entityQuery($entity_type_id)
- // The id could be NULL, so we cast it to 0 in that case.
- ->condition($id_key, (int) $items->getEntity()->id(), '<>')
+ $query = \Drupal::entityQuery($entity_type_id);
+
+ $entity_id = $entity->id();
+ // Using isset() instead of !empty() as 0 and '0' are valid ID values for
+ // entity types using string IDs.
+ if (isset($entity_id)) {
+ $query->condition($id_key, $entity_id, '<>');
+ }
+
+ $value_taken = (bool) $query
->condition($field_name, $item->value)
->range(0, 1)
->count()
diff --git a/core/modules/system/tests/modules/unique_field_constraint_test/unique_field_constraint_test.info.yml b/core/modules/system/tests/modules/unique_field_constraint_test/unique_field_constraint_test.info.yml
new file mode 100644
index 000000000000..a667c6217d0d
--- /dev/null
+++ b/core/modules/system/tests/modules/unique_field_constraint_test/unique_field_constraint_test.info.yml
@@ -0,0 +1,6 @@
+name: 'UniqueField Constraint Test'
+type: module
+description: 'Support module for UniqueField Constraint testing.'
+package: Testing
+version: VERSION
+core: 8.x
diff --git a/core/modules/system/tests/modules/unique_field_constraint_test/unique_field_constraint_test.module b/core/modules/system/tests/modules/unique_field_constraint_test/unique_field_constraint_test.module
new file mode 100644
index 000000000000..c9bb6ca484a8
--- /dev/null
+++ b/core/modules/system/tests/modules/unique_field_constraint_test/unique_field_constraint_test.module
@@ -0,0 +1,18 @@
+<?php
+
+/**
+ * @file
+ * Contains unique_field_constraint_test.module
+ */
+
+use Drupal\Core\Entity\EntityTypeInterface;
+
+/**
+ * Implements hook_entity_base_field_info_alter().
+ */
+function unique_field_constraint_test_entity_base_field_info_alter(&$fields, EntityTypeInterface $entity_type) {
+ if ($entity_type->id() === 'entity_test_string_id') {
+ /** @var \Drupal\Core\Field\BaseFieldDefinition[] $fields */
+ $fields['name']->addConstraint('UniqueField');
+ }
+}
diff --git a/core/tests/Drupal/KernelTests/Core/Validation/UniqueFieldConstraintTest.php b/core/tests/Drupal/KernelTests/Core/Validation/UniqueFieldConstraintTest.php
new file mode 100644
index 000000000000..3fbdf4e14143
--- /dev/null
+++ b/core/tests/Drupal/KernelTests/Core/Validation/UniqueFieldConstraintTest.php
@@ -0,0 +1,115 @@
+<?php
+
+namespace Drupal\KernelTests\Core\Validation;
+
+use Drupal\Component\Render\FormattableMarkup;
+use Drupal\entity_test\Entity\EntityTestStringId;
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * Tests the unique field value validation constraint.
+ *
+ * @coversDefaultClass \Drupal\Core\Validation\Plugin\Validation\Constraint\UniqueFieldValueValidator
+ *
+ * @group Validation
+ */
+class UniqueFieldConstraintTest extends KernelTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected static $modules = [
+ 'entity_test',
+ 'unique_field_constraint_test',
+ 'user',
+ ];
+
+ /**
+ * Tests cases where the validation passes for entities with string IDs.
+ *
+ * @covers ::validate
+ */
+ public function testEntityWithStringId() {
+ $this->installEntitySchema('entity_test_string_id');
+
+ EntityTestStringId::create([
+ 'id' => 'foo',
+ 'name' => $this->randomString(),
+ ])->save();
+
+ // Reload the entity.
+ $entity = EntityTestStringId::load('foo');
+
+ // Check that an existing entity validates when the value is preserved.
+ $violations = $entity->name->validate();
+ $this->assertCount(0, $violations);
+
+ // Create a new entity with a different ID and a different field value.
+ EntityTestStringId::create([
+ 'id' => 'bar',
+ 'name' => $this->randomString(),
+ ]);
+
+ // Check that a new entity with a different field value validates.
+ $violations = $entity->name->validate();
+ $this->assertCount(0, $violations);
+ }
+
+ /**
+ * Tests cases when validation raises violations for entities with string IDs.
+ *
+ * @param string|int|null $id
+ * The entity ID.
+ *
+ * @covers ::validate
+ *
+ * @dataProvider providerTestEntityWithStringIdWithViolation
+ */
+ public function testEntityWithStringIdWithViolation($id) {
+ $this->installEntitySchema('entity_test_string_id');
+
+ $value = $this->randomString();
+
+ EntityTestStringId::create([
+ 'id' => 'first_entity',
+ 'name' => $value,
+ ])->save();
+
+ $entity = EntityTestStringId::create([
+ 'id' => $id,
+ 'name' => $value,
+ ]);
+ /** @var \Symfony\Component\Validator\ConstraintViolationList $violations */
+ $violations = $entity->get('name')->validate();
+
+ $message = new FormattableMarkup('A @entity_type with @field_name %value already exists.', [
+ '%value' => $value,
+ '@entity_type' => $entity->getEntityType()->getLowercaseLabel(),
+ '@field_name' => 'name',
+ ]);
+
+ // Check that the validation has created the appropriate violation.
+ $this->assertCount(1, $violations);
+ $this->assertEquals($message, $violations[0]->getMessage());
+ }
+
+ /**
+ * Data provider for ::testEntityWithStringIdWithViolation().
+ *
+ * @return array
+ * An array of test cases.
+ *
+ * @see self::testEntityWithStringIdWithViolation()
+ */
+ public function providerTestEntityWithStringIdWithViolation() {
+ return [
+ 'without an id' => [NULL],
+ 'zero as integer' => [0],
+ 'zero as string' => ["0"],
+ 'non-zero as integer' => [mt_rand(1, 127)],
+ 'non-zero as string' => [(string) mt_rand(1, 127)],
+ 'alphanumeric' => [$this->randomMachineName()],
+ ];
+ }
+
+}
--
GitLab