[PP-1] Expose entity validation errors in a machine readable REST API:

Adding related issues.

What a great start! Thanks, @dawehner!

--- /dev/null
+++ b/core/modules/rest/src/Exception/EntityValidationException.php

@@ -0,0 +1,34 @@
+class EntityValidationExceptionNormalizer extends NormalizerBase {

I'm pretty sure this belongs in the serialization module.

One question is: How do we deal with the potential BC break?

I doubt any of the existing clients are parsing the current error response bodies. Which is exactly why you're doing this. Not even https://github.com/acquia/waterwheel.js is doing this.

So… yes this is a BC break technically. But it's also just to make something even remotely usable. The current responses are not usable.

Therefore I think the same BC strategy as we used in #2751325: All serialized values are strings, should be integers/booleans when appropriate seems appropriate.

Let's see whether this helps to get rid of couple errors.

Gosh...

@dawehner, I was wrongly thought that supportsNormalization method called somewhere. But now I'm seeing it's not.

By the way, how are you running this locally? In case when I don't want to run whole rest group. Single test (e.g. core/modules/rest/tests/src/Functional/EntityResource/Node/NodeJsonAnonTest.php) fails every time with something like:

No route found that matches &quot;Content-Type: &quot; in <em class="placeholder">Drupal\Core\Routing\ContentTypeHeaderMatcher-&gt;filter()</em> (line <em class="placeholder">49</em> of <em class="placeholder">core/lib/Drupal/Core/Routing/ContentTypeHeaderMatcher.php</em>). <pre class="backtrace">Drupal\Core\Routing\LazyRouteFilter-&gt;filter(Object, Object) (Line: 337)

Hm, but nevertheless we shouldn't call supportsNormalization() since this happens in \Symfony\Component\Serializer\Serializer::getNormalizer().

Okay, this will solve couple errors.

A few comments:

1. +++ b/core/modules/serialization/src/Normalizer/EntityValidationExceptionNormalizer.php
   @@ -0,0 +1,38 @@
   +      $result['errors'] = array_reduce(iterator_to_array($object->getViolations()), function (array $carry, ConstraintViolation $violation) {
   

   I don't think we can do this at all, when we support \Exception, so this normalizer would catch anything. I am also not 100% convinced about having a normalizer for this exception handling. This should only support EntityValidationExceptions I think, in any case. Which means possibly an exception normalizer for regular exceptions too. That wouild also require things to be handled a bit differently in \Drupal\serialization\EventSubscriber\DefaultExceptionSubscriber::on4xx.

2. +++ b/core/modules/serialization/src/Normalizer/NormalizerBase.php
   @@ -21,39 +21,57 @@
   -    return (bool) array_filter($supported, function($name) use ($data) {
   -      return $data instanceof $name;
   -    });
   

   Does having the single code in checkSupport() behave the same as the instanceof check we have now? I'm not sure..

3. +++ b/core/modules/serialization/src/Normalizer/NormalizerBase.php
   @@ -21,39 +21,57 @@
   +   * @param string|object $object
   +   *   Fully-qualified object name or object itself.
   ...
   +  protected function checkSupport($object, $format = NULL) {
   

   Maybe this parameter should be named $type and have the casting from object still?

4. +++ b/core/modules/serialization/src/Normalizer/NormalizerBase.php
   @@ -21,39 +21,57 @@
   +    if (is_object($object)) {
   +      $object = get_class($object);
   +    }
   +
   +    // Handle immediate match.
   +    if (in_array($object, $supported)) {
   +      return TRUE;
   +    }
   +
   +    // Handle inheritance.
   +    foreach ($supported as $type) {
   +      if ((class_exists($type) || interface_exists($type)) && is_subclass_of($object, $type, TRUE)) {
   +        return TRUE;
   +      }
   +    }
   ...
   -    return in_array($type, $supported) || array_filter($supported, $subclass_check);
   

   Refactoring this piece of the logic seems possibly out of scope here?

5. +++ b/core/modules/serialization/tests/src/Unit/EventSubscriber/DefaultExceptionSubscriberTest.php
   @@ -28,7 +29,7 @@ public function testOn4xx() {
   +    $subscriber = new DefaultExceptionSubscriber(new Serializer([new EntityValidationExceptionNormalizer()], [new JsonEncoder()]), []);
   

   Why is this change needed for this test?

@damiankloip

1. +++ b/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php
   @@ -62,13 +62,12 @@ protected static function getPriority() {
   +    $encoded_content = $this->serializer->serialize($exception, $format);
   

   Now, instead of an array, serializes exception object.

2. instanceof is not good approach, because for instance \RuntimeException is not an instance of \Exception. That's why these changes were made.
3. I didn't understand what you meant.
4. Otherwise this logic will be duplicated to supportsNormalization method
5. It follows from your first question. Now whole exception object going to be serialized and it fails if no normalizer will be found.

@damiankloip, visual answer on your 5 question available here: https://www.drupal.org/pift-ci-job/630951. The Serialization.Serialization section.

Quoting #1916302-16: RFC 7807: "Problem Details for HTTP APIs" — serve REST error responses as application/problem+json:

Also, I wonder if #2856110: [PP-1] Expose entity validation errors in a machine readable REST API: is basically a duplicate of this? It focuses on entity validation errors. But if we're going to do that, we should just adopt this RFC right away, rather than first creating our own format, and then causing disruption for REST clients by moving from HEAD's validation error responses (which are basically unparseable plain text) to the custom JSON format there and then finally to the format defined by RFC 7807.

Please share your thoughts here or on that issue. I think this issue still makes a ton of sense, but I think it'd be good to adopt RFC 7807 for this, instead of inventing our own approach.

Seems postponed issue has since been closed.