.../Validation/RecursiveContextualValidator.php | 16 +++---- core/modules/comment/src/Entity/Comment.php | 1 + .../Constraint/CommentNameConstraintValidator.php | 6 +++ .../Comment/CommentResourceTestBase.php | 21 ++------- .../EntityResource/User/UserResourceTestBase.php | 2 +- .../TypedData/RecursiveContextualValidatorTest.php | 50 +++++++++++----------- 6 files changed, 44 insertions(+), 52 deletions(-) diff --git a/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php b/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php index b765b05..f98d813 100644 --- a/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php +++ b/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php @@ -132,14 +132,6 @@ protected function validateNode(TypedDataInterface $data, $constraints = NULL, $ $value = $this->typedDataManager->getCanonicalRepresentation($data); $this->context->setNode($value, $data, $metadata, $property_path); - if (isset($constraints) || !$this->context->isGroupValidated($cache_key, Constraint::DEFAULT_GROUP)) { - if (!isset($constraints)) { - $this->context->markGroupAsValidated($cache_key, Constraint::DEFAULT_GROUP); - $constraints = $metadata->findConstraints(Constraint::DEFAULT_GROUP); - } - $this->validateConstraints($value, $cache_key, $constraints); - } - // If the data is a list or complex data, validate the contained list items // or properties. However, do not recurse if the data is empty. if (($data instanceof ListInterface || $data instanceof ComplexDataInterface) && !$data->isEmpty()) { @@ -148,6 +140,14 @@ protected function validateNode(TypedDataInterface $data, $constraints = NULL, $ } } + if (isset($constraints) || !$this->context->isGroupValidated($cache_key, Constraint::DEFAULT_GROUP)) { + if (!isset($constraints)) { + $this->context->markGroupAsValidated($cache_key, Constraint::DEFAULT_GROUP); + $constraints = $metadata->findConstraints(Constraint::DEFAULT_GROUP); + } + $this->validateConstraints($value, $cache_key, $constraints); + } + $this->context->setNode($previous_value, $previous_object, $previous_metadata, $previous_path); return $this; diff --git a/core/modules/comment/src/Entity/Comment.php b/core/modules/comment/src/Entity/Comment.php index d52f04d..a6b3a68 100644 --- a/core/modules/comment/src/Entity/Comment.php +++ b/core/modules/comment/src/Entity/Comment.php @@ -307,6 +307,7 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { $fields['entity_type'] = BaseFieldDefinition::create('string') ->setLabel(t('Entity type')) ->setDescription(t('The entity type to which this comment is attached.')) + ->setRequired(TRUE) ->setSetting('is_ascii', TRUE) ->setSetting('max_length', EntityTypeInterface::ID_MAX_LENGTH); diff --git a/core/modules/comment/src/Plugin/Validation/Constraint/CommentNameConstraintValidator.php b/core/modules/comment/src/Plugin/Validation/Constraint/CommentNameConstraintValidator.php index f06dfd8..7063f36 100644 --- a/core/modules/comment/src/Plugin/Validation/Constraint/CommentNameConstraintValidator.php +++ b/core/modules/comment/src/Plugin/Validation/Constraint/CommentNameConstraintValidator.php @@ -52,6 +52,12 @@ public function validate($entity, Constraint $constraint) { $author_name = $entity->name->value; $owner_id = (int) $entity->uid->target_id; + // Return early if no entity_id is set; without it, we can't perform an + // @todo this actually even requires a VALID entity ID & comment field name. So this is not sufficient yet! + if ($entity->entity_id->value === NULL && $entity->field_name->value) { + return; + } + // Do not allow unauthenticated comment authors to use a name that is // taken by a registered user. if (isset($author_name) && $author_name !== '' && $owner_id === 0) { diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php index ecae9f8..230e7e0 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php @@ -278,27 +278,12 @@ public function testPostDxWithoutCriticalBaseFields() { // DX: 422 when missing 'entity_type' field. $request_options[RequestOptions::BODY] = $this->serializer->encode(array_diff_key($this->getNormalizedPostEntity(), ['entity_type' => TRUE]), static::$format); $response = $this->request('POST', $url, $request_options); - // @todo Uncomment, remove next 3 lines in https://www.drupal.org/node/2820364. - $this->assertSame(500, $response->getStatusCode()); - $this->assertSame(['application/json'], $response->getHeader('Content-Type')); - $this->assertSame('{"message":"A fatal error occurred: Internal Server Error"}', (string) $response->getBody()); - //$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nentity_type: This value should not be null.\n", $response); + $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nentity_type: This value should not be null.\n", $response); // DX: 422 when missing 'entity_id' field. $request_options[RequestOptions::BODY] = $this->serializer->encode(array_diff_key($this->getNormalizedPostEntity(), ['entity_id' => TRUE]), static::$format); - // @todo Remove the try/catch in favor of the two commented lines in - // https://www.drupal.org/node/2820364. - try { - $response = $this->request('POST', $url, $request_options); - // This happens on DrupalCI. - //$this->assertSame(500, $response->getStatusCode()); - } - catch (\Exception $e) { - // This happens on Wim's local machine. - //$this->assertSame("Error: Call to a member function get() on null\nDrupal\\comment\\Plugin\\Validation\\Constraint\\CommentNameConstraintValidator->getAnonymousContactDetailsSetting()() (Line: 96)\n", $e->getMessage()); - } - //$response = $this->request('POST', $url, $request_options); - //$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nentity_type: This value should not be null.\n", $response); + $response = $this->request('POST', $url, $request_options); + $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nentity_id: This value should not be null.\n", $response); // DX: 422 when missing 'entity_type' field. $request_options[RequestOptions::BODY] = $this->serializer->encode(array_diff_key($this->getNormalizedPostEntity(), ['field_name' => TRUE]), static::$format); diff --git a/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php index 38452c2..5be693d 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php @@ -163,7 +163,7 @@ public function testPatchDxForSecuritySensitiveBaseFields() { // DX: 422 when changing email without providing the password. $response = $this->request('PATCH', $url, $request_options); - $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nmail: Your current password is missing or incorrect; it's required to change the Email.\n", $response); + $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\npass: Your current password is missing or incorrect; it's required to change the Password.\nmail: Your current password is missing or incorrect; it's required to change the Email.\n", $response); $normalization['pass'] = [['existing' => 'wrong']]; diff --git a/core/tests/Drupal/Tests/Core/TypedData/RecursiveContextualValidatorTest.php b/core/tests/Drupal/Tests/Core/TypedData/RecursiveContextualValidatorTest.php index 5e3558e..878c3cd 100644 --- a/core/tests/Drupal/Tests/Core/TypedData/RecursiveContextualValidatorTest.php +++ b/core/tests/Drupal/Tests/Core/TypedData/RecursiveContextualValidatorTest.php @@ -170,19 +170,19 @@ public function testPropertiesValidateWithMultipleLevels() { $violations = $this->recursiveValidator->validate($typed_data); $this->assertCount(6, $violations); - $this->assertEquals('violation: 3', $violations->get(0)->getMessage()); - $this->assertEquals('violation: value1', $violations->get(1)->getMessage()); - $this->assertEquals('violation: value2', $violations->get(2)->getMessage()); - $this->assertEquals('violation: 2', $violations->get(3)->getMessage()); - $this->assertEquals('violation: subvalue1', $violations->get(4)->getMessage()); - $this->assertEquals('violation: subvalue2', $violations->get(5)->getMessage()); - - $this->assertEquals('', $violations->get(0)->getPropertyPath()); - $this->assertEquals('key1', $violations->get(1)->getPropertyPath()); - $this->assertEquals('key2', $violations->get(2)->getPropertyPath()); - $this->assertEquals('key_with_properties', $violations->get(3)->getPropertyPath()); - $this->assertEquals('key_with_properties.subkey1', $violations->get(4)->getPropertyPath()); - $this->assertEquals('key_with_properties.subkey2', $violations->get(5)->getPropertyPath()); + $this->assertEquals('violation: value1', $violations->get(0)->getMessage()); + $this->assertEquals('violation: value2', $violations->get(1)->getMessage()); + $this->assertEquals('violation: subvalue1', $violations->get(2)->getMessage()); + $this->assertEquals('violation: subvalue2', $violations->get(3)->getMessage()); + $this->assertEquals('violation: 2', $violations->get(4)->getMessage()); + $this->assertEquals('violation: 3', $violations->get(5)->getMessage()); + + $this->assertEquals('key1', $violations->get(0)->getPropertyPath()); + $this->assertEquals('key2', $violations->get(1)->getPropertyPath()); + $this->assertEquals('key_with_properties.subkey1', $violations->get(2)->getPropertyPath()); + $this->assertEquals('key_with_properties.subkey2', $violations->get(3)->getPropertyPath()); + $this->assertEquals('key_with_properties', $violations->get(4)->getPropertyPath()); + $this->assertEquals('', $violations->get(5)->getPropertyPath()); } /** @@ -276,13 +276,13 @@ public function testValidateProperty() { $violations = $this->recursiveValidator->validateProperty($typed_data, 'key_with_properties'); $this->assertCount(3, $violations); - $this->assertEquals('violation: 2', $violations->get(0)->getMessage()); - $this->assertEquals('violation: subvalue1', $violations->get(1)->getMessage()); - $this->assertEquals('violation: subvalue2', $violations->get(2)->getMessage()); + $this->assertEquals('violation: subvalue1', $violations->get(0)->getMessage()); + $this->assertEquals('violation: subvalue2', $violations->get(1)->getMessage()); + $this->assertEquals('violation: 2', $violations->get(2)->getMessage()); - $this->assertEquals('', $violations->get(0)->getPropertyPath()); - $this->assertEquals('subkey1', $violations->get(1)->getPropertyPath()); - $this->assertEquals('subkey2', $violations->get(2)->getPropertyPath()); + $this->assertEquals('subkey1', $violations->get(0)->getPropertyPath()); + $this->assertEquals('subkey2', $violations->get(1)->getPropertyPath()); + $this->assertEquals('', $violations->get(2)->getPropertyPath()); } /** @@ -305,13 +305,13 @@ public function testValidatePropertyValue() { $violations = $this->recursiveValidator->validatePropertyValue($typed_data, 'key_with_properties', $typed_data->get('key_with_properties')); $this->assertCount(3, $violations); - $this->assertEquals('violation: 2', $violations->get(0)->getMessage()); - $this->assertEquals('violation: subvalue11', $violations->get(1)->getMessage()); - $this->assertEquals('violation: subvalue22', $violations->get(2)->getMessage()); + $this->assertEquals('violation: subvalue11', $violations->get(0)->getMessage()); + $this->assertEquals('violation: subvalue22', $violations->get(1)->getMessage()); + $this->assertEquals('violation: 2', $violations->get(2)->getMessage()); - $this->assertEquals('', $violations->get(0)->getPropertyPath()); - $this->assertEquals('subkey1', $violations->get(1)->getPropertyPath()); - $this->assertEquals('subkey2', $violations->get(2)->getPropertyPath()); + $this->assertEquals('subkey1', $violations->get(0)->getPropertyPath()); + $this->assertEquals('subkey2', $violations->get(1)->getPropertyPath()); + $this->assertEquals('', $violations->get(2)->getPropertyPath()); } /**