.../src/Plugin/rest/resource/EntityResource.php | 8 ++++++- .../resource/EntityResourceValidationTrait.php | 16 ++++++++++--- .../rest/tests/modules/rest_test/rest_test.module | 26 ++++++++++++++++++++++ .../Validation/Constraint/RestTestConstraint.php | 21 +++++++++++++++++ .../Constraint/RestTestConstraintValidator.php | 26 ++++++++++++++++++++++ .../EntityResource/EntityResourceTestBase.php | 19 ++++++++++++++++ .../EntityTest/EntityTestResourceTestBase.php | 5 +++++ 7 files changed, 117 insertions(+), 4 deletions(-) diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 4b6febc..3cc1872 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -228,6 +228,8 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity } // Overwrite the received fields. + // @todo Remove $changed_fields in https://www.drupal.org/node/2906129. + $changed_fields = []; foreach ($entity->_restSubmittedFields as $field_name) { $field = $entity->get($field_name); // It is not possible to set the language to NULL as it is automatically @@ -237,12 +239,16 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity continue; } if ($this->checkPatchFieldAccess($original_entity->get($field_name), $field)) { + // Track which fields actually changed: which ones should be validated. + if (!$original_entity->get($field_name)->equals($field)) { + $changed_fields[] = $field_name; + } $original_entity->set($field_name, $field->getValue()); } } // Validate the received data before saving. - $this->validate($original_entity); + $this->validate($original_entity, $changed_fields); try { $original_entity->save(); $this->logger->notice('Updated entity %type with ID %id.', ['%type' => $original_entity->getEntityTypeId(), '%id' => $original_entity->id()]); diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResourceValidationTrait.php b/core/modules/rest/src/Plugin/rest/resource/EntityResourceValidationTrait.php index 09b4b64..096f3d3 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResourceValidationTrait.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResourceValidationTrait.php @@ -3,6 +3,7 @@ namespace Drupal\rest\Plugin\rest\resource; use Drupal\Component\Render\PlainTextOutput; +use Drupal\Core\Entity\EntityConstraintViolationListInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\FieldableEntityInterface; use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException; @@ -14,16 +15,20 @@ trait EntityResourceValidationTrait { /** - * Verifies that the whole entity does not violate any validation constraints. + * Verifies that an entity does not violate any validation constraints. * * @param \Drupal\Core\Entity\EntityInterface $entity * The entity to validate. + * @param string[] $changed_fields + * (optional) An array of field names. If specified, filters the violations + * list to include only this set of fields. Defaults to an empty array, + * which means that all violations will be reported. * * @throws \Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException * If validation errors are found. */ - protected function validate(EntityInterface $entity) { - // @todo Remove when https://www.drupal.org/node/2164373 is committed. + protected function validate(EntityInterface $entity, array $changed_fields = []) { + // @todo Update this check in https://www.drupal.org/node/2300677. if (!$entity instanceof FieldableEntityInterface) { return; } @@ -33,6 +38,11 @@ protected function validate(EntityInterface $entity) { // changes. $violations->filterByFieldAccess(); + // Filter violations by the specified fields. + if ($changed_fields) { + $violations->filterByFields(array_diff(array_keys($entity->getFieldDefinitions()), $changed_fields)); + } + if ($violations->count() > 0) { $message = "Unprocessable Entity: validation failed.\n"; foreach ($violations as $violation) { diff --git a/core/modules/rest/tests/modules/rest_test/rest_test.module b/core/modules/rest/tests/modules/rest_test/rest_test.module index 8897fb9..9839edb 100644 --- a/core/modules/rest/tests/modules/rest_test/rest_test.module +++ b/core/modules/rest/tests/modules/rest_test/rest_test.module @@ -5,6 +5,8 @@ * Contains hook implementations for testing REST module. */ +use Drupal\Core\Entity\EntityTypeInterface; +use Drupal\Core\Field\BaseFieldDefinition; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Field\FieldItemListInterface; @@ -40,6 +42,30 @@ function rest_test_entity_field_access($operation, FieldDefinitionInterface $fie } } + // @see \Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase::testGet() + // @see \Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase::testPatch() + if ($field_definition->getName() === 'rest_test_validation') { + switch ($operation) { + case 'view': + // Never ever allow this field to be viewed: this lets + // EntityResourceTestBase::testGet() test in a "vanilla" way. + return AccessResult::forbidden(); + } + } + // No opinion. return AccessResult::neutral(); } + +/** + * Implements hook_entity_base_field_info(). + */ +function rest_test_entity_base_field_info(EntityTypeInterface $entity_type) { + $fields = []; + $fields['rest_test_validation'] = BaseFieldDefinition::create('string') + ->setLabel(t('REST test validation field')) + ->setDescription(t('A text field with some special validations attached used for testing purposes')) + ->addConstraint('rest_test_validation'); + + return $fields; +} diff --git a/core/modules/rest/tests/modules/rest_test/src/Plugin/Validation/Constraint/RestTestConstraint.php b/core/modules/rest/tests/modules/rest_test/src/Plugin/Validation/Constraint/RestTestConstraint.php new file mode 100644 index 0000000..ab4f1fe --- /dev/null +++ b/core/modules/rest/tests/modules/rest_test/src/Plugin/Validation/Constraint/RestTestConstraint.php @@ -0,0 +1,21 @@ +getValue(); + if (!empty($value[0]['value']) && $value[0]['value'] === 'ALWAYS_FAIL') { + $this->context->addViolation($constraint->message); + } + } + } + +} diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 4c7947d..55825c8 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -208,6 +208,7 @@ public function setUp() { // Set a default value on the fields. $this->entity->set('field_rest_test', ['value' => 'All the faith he had had had had no effect on the outcome of his life.']); $this->entity->set('field_rest_test_multivalue', [['value' => 'One'], ['value' => 'Two']]); + $this->entity->set('rest_test_validation', ['value' => 'allowed value']); $this->entity->save(); } } @@ -564,6 +565,7 @@ public function testGet() { // ::formatExpectedTimestampValue() to generate the timestamp value. This // will take into account the above config setting. $expected = $this->getExpectedNormalizedEntity(); + // Config entities are not affected. // @see \Drupal\serialization\Normalizer\ConfigEntityNormalizer::normalize() static::recursiveKSort($expected); @@ -1027,6 +1029,23 @@ public function testPatch() { $modified_entity->get($patch_protected_field_name)->setValue($original_values[$patch_protected_field_name]); } + if ($this->entity instanceof FieldableEntityInterface) { + // Change the rest_test_validation field to prove that then its validation + // does run. In subsequent test assertions, it will not be modified, and + // then should not trigger validation errors. + $modified_entity->get('rest_test_validation')->setValue('ALWAYS_FAIL'); + $valid_request_body = $this->getNormalizedPatchEntity() + $this->serializer->normalize($modified_entity, static::$format); + $request_options[RequestOptions::BODY] = $this->serializer->serialize($valid_request_body, static::$format); + $response = $this->request('PATCH', $url, $request_options); + $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nrest_test_validation: REST test validation failed\n", $response); + + // Set the rest_test_validation field to always fail validation, which + // allows asserting that not modifying that field does not trigger + // validation errors. + $this->entity->set('rest_test_validation', 'ALWAYS_FAIL'); + $this->entity->save(); + } + // 200 for well-formed PATCH request that sends all fields (even including // read-only ones, but with unchanged values). $valid_request_body = $this->getNormalizedPatchEntity() + $this->serializer->normalize($this->entity, static::$format); diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityTest/EntityTestResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityTest/EntityTestResourceTestBase.php index d14ec38..a56df06 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityTest/EntityTestResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityTest/EntityTestResourceTestBase.php @@ -117,6 +117,11 @@ protected function getExpectedNormalizedEntity() { ] ], 'field_test_text' => [], + 'rest_test_validation' => [ + [ + 'value' => 'allowed value', + ] + ] ]; return $normalization;