Add source pointer element to "422 Unprocessable Entity" responses

Thanks for the patch. This will really help consumers to get meaningful server-side form validation.

1. +++ b/src/Error/SerializableHttpException.php
   @@ -3,10 +3,42 @@
   +  public function serialize() {
   

   This is the object to serialize. It should not know how to serialize itself. Instead, the serializer should be the one to do that.

2. +++ b/src/Error/UnprocessableEntityException.php
   @@ -0,0 +1,50 @@
   +class UnprocessableEntityException extends SerializableHttpException {
   

   Maybe UnprocessableHttpEntityException to help avoid confusion?

3. +++ b/src/Error/UnprocessableEntityException.php
   @@ -0,0 +1,50 @@
   +  public function __construct(EntityConstraintViolationListInterface $violations, \Exception $previous = null, array $headers = array(), $code = 0) {
   

   I'd rather have an addViolation($violation) method in the class than passing the array to the constructor.

4. +++ b/src/Normalizer/HttpExceptionNormalizer.php
   +++ b/src/Normalizer/HttpExceptionNormalizer.php
   @@ -41,30 +41,42 @@ class HttpExceptionNormalizer extends SerializationNormalizerBase {
   

   I'd like to have a normalizer that takes UnprocessableHttpEntityException and inherits from HttpExceptionNormalizer to add the pointer.

5. +++ b/src/Normalizer/HttpExceptionNormalizer.php
   @@ -41,30 +41,42 @@ class HttpExceptionNormalizer extends SerializationNormalizerBase {
   +      $error = $object->serialize();
   

   As stated before, let's bring the logic back to this class.

This is definitely in the good direction! I really like this new feature.

A couple more comments on the current implementation:

1. +++ b/src/Error/UnprocessableHttpEntityException.php
   @@ -0,0 +1,47 @@
   +class UnprocessableHttpEntityException extends SerializableHttpException {
   

   Let's add a short class comment explaining when this exception comes into play.

2. +++ b/src/Normalizer/HttpExceptionNormalizer.php
   @@ -40,37 +40,56 @@ class HttpExceptionNormalizer extends SerializationNormalizerBase {
   +      return new FieldItemNormalizerValue($error);
   

   Should this be:

   return new FieldItemNormalizerValue([$error]);
   

   I'm unclear on the specifics at the moment, but that is what the previous code seems to be doing.

3. +++ b/src/Normalizer/HttpExceptionNormalizer.php
   @@ -40,37 +40,56 @@ class HttpExceptionNormalizer extends SerializationNormalizerBase {
   +   * @param object $exception
   +   *  The Exception.
   +   *
   +   * @return array
   +   *  The error objects to include in the response.
   +   */
   +  protected function buildErrorObjects($exception) {
   +    /** @var $exception \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface */
   

   Let's add type hinting to the $exception argument.

4. +++ b/src/Normalizer/UnprocessableHttpEntityExceptionNormalizer.php
   @@ -0,0 +1,74 @@
   +    unset($error['links']);
   

   Why are you removing this?

5. +++ b/src/Normalizer/UnprocessableHttpEntityExceptionNormalizer.php
   @@ -0,0 +1,74 @@
   +        $pointer = '/data/attributes/'
   +          . str_replace('.', '/', $violation->getPropertyPath());
   +        if ($cardinality == 1) {
   +          $pointer = str_replace('/0/', '/', $pointer);
   +        }
   

   Is there any way ew can ensure not to remove additional /0/ that we're not supposed to remove.

We will need testing for this, to make sure that we adhere to the expectations from the front end libraries.

Also, can you please post the response you get to a serialization error with this patch applied with different user roles? I'm trying to make sure we are not leaking any sensitive information.

@hampercm it seems that we only have functional tests on 403 for write operations. We have, however 403 tests for partial success operations (// 12. Collection with one access denied).

I agree that we should add more testing. Do you want to add that here, or in a follow up?

That sounds good. I think that test coverage is not in bad shape, so a Plan sounds like too much. We probably want just a new ticket.

I think this is a great DX improvement! Merging.

Moving to Drupal core's issue queue.

I'm working on https://www.drupal.org/project/drupal/issues/3122113