From 2a554518d801c4e0c6d58b5d4a80c7404e2e6fda Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Sat, 17 Jan 2015 20:49:50 +0000 Subject: [PATCH] Issue #2403817 by larowlan: Feed entity validation misses form validation logic --- .../Constraint/UniqueFieldValueValidator.php | 43 +++++++++++ core/modules/aggregator/src/Entity/Feed.php | 6 +- core/modules/aggregator/src/FeedForm.php | 19 ----- .../Constraint/FeedTitleConstraint.php | 31 ++++++++ .../Constraint/FeedUrlConstraint.php | 31 ++++++++ .../aggregator/src/Tests/AddFeedTest.php | 10 +++ .../src/Tests/FeedValidationTest.php | 72 +++++++++++++++++++ .../Validation/Constraint/UserMailUnique.php | 2 +- .../Validation/Constraint/UserNameUnique.php | 2 +- .../Constraint/UserUniqueValidator.php | 39 ---------- 10 files changed, 193 insertions(+), 62 deletions(-) create mode 100644 core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UniqueFieldValueValidator.php create mode 100644 core/modules/aggregator/src/Plugin/Validation/Constraint/FeedTitleConstraint.php create mode 100644 core/modules/aggregator/src/Plugin/Validation/Constraint/FeedUrlConstraint.php create mode 100644 core/modules/aggregator/src/Tests/FeedValidationTest.php delete mode 100644 core/modules/user/src/Plugin/Validation/Constraint/UserUniqueValidator.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 new file mode 100644 index 000000000000..54ace9e7211b --- /dev/null +++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UniqueFieldValueValidator.php @@ -0,0 +1,43 @@ +<?php + +/** + * @file + * Contains \Drupal\Core\Validation\Plugin\Validation\Constraint\UniqueFieldValueValidator. + */ + +namespace Drupal\Core\Validation\Plugin\Validation\Constraint; + +use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\ConstraintValidator; + +/** + * Validates that a field is unique for the given entity type. + */ +class UniqueFieldValueValidator extends ConstraintValidator { + + /** + * {@inheritdoc} + */ + public function validate($items, Constraint $constraint) { + if (!isset($items)) { + return; + } + $field_name = $items->getFieldDefinition()->getName(); + /** @var \Drupal\Core\Entity\EntityInterface $entity */ + $entity = $items->getEntity(); + $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(), '<>') + ->condition($field_name, db_like($items->first()->value), 'LIKE') + ->range(0, 1) + ->count() + ->execute(); + + if ($value_taken) { + $this->context->addViolation($constraint->message, array("%value" => $items->value)); + } + } +} diff --git a/core/modules/aggregator/src/Entity/Feed.php b/core/modules/aggregator/src/Entity/Feed.php index 7f0c2871c27c..6f3d2e2dbe85 100644 --- a/core/modules/aggregator/src/Entity/Feed.php +++ b/core/modules/aggregator/src/Entity/Feed.php @@ -150,7 +150,8 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { 'type' => 'string_textfield', 'weight' => -5, )) - ->setDisplayConfigurable('form', TRUE); + ->setDisplayConfigurable('form', TRUE) + ->addConstraint('FeedTitle', []); $fields['langcode'] = BaseFieldDefinition::create('language') ->setLabel(t('Language code')) @@ -171,7 +172,8 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { 'type' => 'uri', 'weight' => -3, )) - ->setDisplayConfigurable('form', TRUE); + ->setDisplayConfigurable('form', TRUE) + ->addConstraint('FeedUrl', []); $intervals = array(900, 1800, 3600, 7200, 10800, 21600, 32400, 43200, 64800, 86400, 172800, 259200, 604800, 1209600, 2419200); $period = array_map(array(\Drupal::service('date.formatter'), 'formatInterval'), array_combine($intervals, $intervals)); diff --git a/core/modules/aggregator/src/FeedForm.php b/core/modules/aggregator/src/FeedForm.php index d84c11e09d0b..35359ce3c360 100644 --- a/core/modules/aggregator/src/FeedForm.php +++ b/core/modules/aggregator/src/FeedForm.php @@ -29,25 +29,6 @@ public function form(array $form, FormStateInterface $form_state) { return $form; } - /** - * {@inheritdoc} - */ - public function validate(array $form, FormStateInterface $form_state) { - $feed = $this->buildEntity($form, $form_state); - // Check for duplicate titles. - $feed_storage = $this->entityManager->getStorage('aggregator_feed'); - $result = $feed_storage->getFeedDuplicates($feed); - foreach ($result as $item) { - if (strcasecmp($item->label(), $feed->label()) == 0) { - $form_state->setErrorByName('title', $this->t('A feed named %feed already exists. Enter a unique title.', array('%feed' => $feed->label()))); - } - if (strcasecmp($item->getUrl(), $feed->getUrl()) == 0) { - $form_state->setErrorByName('url', $this->t('A feed with this URL %url already exists. Enter a unique URL.', array('%url' => $feed->getUrl()))); - } - } - parent::validate($form, $form_state); - } - /** * {@inheritdoc} */ diff --git a/core/modules/aggregator/src/Plugin/Validation/Constraint/FeedTitleConstraint.php b/core/modules/aggregator/src/Plugin/Validation/Constraint/FeedTitleConstraint.php new file mode 100644 index 000000000000..47d16ac61904 --- /dev/null +++ b/core/modules/aggregator/src/Plugin/Validation/Constraint/FeedTitleConstraint.php @@ -0,0 +1,31 @@ +<?php + +/** + * @file + * Contains \Drupal\aggregator\Plugin\Validation\Constraint\FeedTitleConstraint. + */ + +namespace Drupal\aggregator\Plugin\Validation\Constraint; + +use Symfony\Component\Validator\Constraint; + +/** + * Supports validating feed titles. + * + * @Plugin( + * id = "FeedTitle", + * label = @Translation("Feed title", context = "Validation") + * ) + */ +class FeedTitleConstraint extends Constraint { + + public $message = 'A feed named %value already exists. Enter a unique title.'; + + /** + * {@inheritdoc} + */ + public function validatedBy() { + return '\Drupal\Core\Validation\Plugin\Validation\Constraint\UniqueFieldValueValidator'; + } + +} diff --git a/core/modules/aggregator/src/Plugin/Validation/Constraint/FeedUrlConstraint.php b/core/modules/aggregator/src/Plugin/Validation/Constraint/FeedUrlConstraint.php new file mode 100644 index 000000000000..0e2ce1d2b031 --- /dev/null +++ b/core/modules/aggregator/src/Plugin/Validation/Constraint/FeedUrlConstraint.php @@ -0,0 +1,31 @@ +<?php + +/** + * @file + * Contains \Drupal\aggregator\Plugin\Validation\Constraint\FeedUrlConstraint. + */ + +namespace Drupal\aggregator\Plugin\Validation\Constraint; + +use Symfony\Component\Validator\Constraint; + +/** + * Supports validating feed URLs. + * + * @Plugin( + * id = "FeedUrl", + * label = @Translation("Feed URL", context = "Validation") + * ) + */ +class FeedUrlConstraint extends Constraint { + + public $message = 'A feed with this URL %value already exists. Enter a unique URL.'; + + /** + * {@inheritdoc} + */ + public function validatedBy() { + return '\Drupal\Core\Validation\Plugin\Validation\Constraint\UniqueFieldValueValidator'; + } + +} diff --git a/core/modules/aggregator/src/Tests/AddFeedTest.php b/core/modules/aggregator/src/Tests/AddFeedTest.php index bbeae854fd58..135ed18be90e 100644 --- a/core/modules/aggregator/src/Tests/AddFeedTest.php +++ b/core/modules/aggregator/src/Tests/AddFeedTest.php @@ -30,6 +30,16 @@ function testAddFeed() { $this->assertText($feed->label(), 'Page title'); $this->assertRaw($feed->getWebsiteUrl()); + // Try to add a duplicate. + $edit = [ + 'title[0][value]' => $feed->label(), + 'url[0][value]' => $feed->getUrl(), + 'refresh' => '900', + ]; + $this->drupalPostForm('aggregator/sources/add', $edit, t('Save')); + $this->assertRaw(t('A feed named %feed already exists. Enter a unique title.', array('%feed' => $feed->label()))); + $this->assertRaw(t('A feed with this URL %url already exists. Enter a unique URL.', array('%url' => $feed->getUrl()))); + // Delete feed. $this->deleteFeed($feed); } diff --git a/core/modules/aggregator/src/Tests/FeedValidationTest.php b/core/modules/aggregator/src/Tests/FeedValidationTest.php new file mode 100644 index 000000000000..83c01d0e2f29 --- /dev/null +++ b/core/modules/aggregator/src/Tests/FeedValidationTest.php @@ -0,0 +1,72 @@ +<?php + +/** + * @file + * Contains \Drupal\aggregator\Tests\FeedValidationTest. + */ + +namespace Drupal\aggregator\Tests; + +use Drupal\aggregator\Entity\Feed; +use Drupal\system\Tests\Entity\EntityUnitTestBase; + +/** + * Tests feed validation constraints. + * + * @group aggregator + */ +class FeedValidationTest extends EntityUnitTestBase { + + /** + * Modules to install. + * + * @var array + */ + public static $modules = array('aggregator', 'options'); + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + $this->installEntitySchema('aggregator_feed'); + } + + /** + * Tests the feed validation constraints. + */ + public function testValidation() { + // Add feed. + $feed = Feed::create([ + 'title' => 'Feed 1', + 'url' => 'http://drupal.org/planet/rss', + 'refresh' => 900, + ]); + + $violations = $feed->validate(); + $this->assertEqual(count($violations), 0); + + $feed->save(); + + // Add another feed. + /* @var \Drupal\aggregator\FeedInterface $feed */ + $feed = Feed::create([ + 'title' => 'Feed 1', + 'url' => 'http://drupal.org/planet/rss', + 'refresh' => 900, + ]); + + $violations = $feed->validate(); + + $this->assertEqual(count($violations), 2); + $this->assertEqual($violations[0]->getPropertyPath(), 'title'); + $this->assertEqual($violations[0]->getMessage(), t('A feed named %value already exists. Enter a unique title.', [ + '%value' => $feed->label(), + ])); + $this->assertEqual($violations[1]->getPropertyPath(), 'url'); + $this->assertEqual($violations[1]->getMessage(), t('A feed with this URL %value already exists. Enter a unique URL.', [ + '%value' => $feed->getUrl(), + ])); + } + +} diff --git a/core/modules/user/src/Plugin/Validation/Constraint/UserMailUnique.php b/core/modules/user/src/Plugin/Validation/Constraint/UserMailUnique.php index e9190ae35ca1..e54151872c44 100644 --- a/core/modules/user/src/Plugin/Validation/Constraint/UserMailUnique.php +++ b/core/modules/user/src/Plugin/Validation/Constraint/UserMailUnique.php @@ -25,6 +25,6 @@ class UserMailUnique extends Constraint { * {@inheritdoc} */ public function validatedBy() { - return '\Drupal\user\Plugin\Validation\Constraint\UserUniqueValidator'; + return '\Drupal\Core\Validation\Plugin\Validation\Constraint\UniqueFieldValueValidator'; } } diff --git a/core/modules/user/src/Plugin/Validation/Constraint/UserNameUnique.php b/core/modules/user/src/Plugin/Validation/Constraint/UserNameUnique.php index 597a42e4111b..0a65d3228a1c 100644 --- a/core/modules/user/src/Plugin/Validation/Constraint/UserNameUnique.php +++ b/core/modules/user/src/Plugin/Validation/Constraint/UserNameUnique.php @@ -25,6 +25,6 @@ class UserNameUnique extends Constraint { * {@inheritdoc} */ public function validatedBy() { - return '\Drupal\user\Plugin\Validation\Constraint\UserUniqueValidator'; + return '\Drupal\Core\Validation\Plugin\Validation\Constraint\UniqueFieldValueValidator'; } } diff --git a/core/modules/user/src/Plugin/Validation/Constraint/UserUniqueValidator.php b/core/modules/user/src/Plugin/Validation/Constraint/UserUniqueValidator.php deleted file mode 100644 index 1c65d0616712..000000000000 --- a/core/modules/user/src/Plugin/Validation/Constraint/UserUniqueValidator.php +++ /dev/null @@ -1,39 +0,0 @@ -<?php - -/** - * @file - * Contains \Drupal\user\Plugin\Validation\Constraint\UserUniqueValidator. - */ - -namespace Drupal\user\Plugin\Validation\Constraint; - -use Symfony\Component\Validator\Constraint; -use Symfony\Component\Validator\ConstraintValidator; - -/** - * Validates the unique user property constraint, such as name and email. - */ -class UserUniqueValidator extends ConstraintValidator { - - /** - * {@inheritdoc} - */ - public function validate($items, Constraint $constraint) { - if (!isset($items)) { - return; - } - $field_name = $items->getFieldDefinition()->getName(); - - $value_taken = (bool) \Drupal::entityQuery('user') - // The UID could be NULL, so we cast it to 0 in that case. - ->condition('uid', (int) $items->getEntity()->id(), '<>') - ->condition($field_name, db_like($items->first()->value), 'LIKE') - ->range(0, 1) - ->count() - ->execute(); - - if ($value_taken) { - $this->context->addViolation($constraint->message, array("%value" => $items->value)); - } - } -} -- GitLab