Allow DELETE requests to return a response body

Hi there,

Normally this could be achieved with:

return (
        new ModifiedResourceResponse(null, $code = 204)
        )->setContent(
                Json::encode($myData)
            );

Best regards.

So I just ran into the same issue, when I wanted to return a validation message after failing to delete an entity.

I'll summarize some debugging in case anyone else stumbles onto this issue:

ResourceResponseSubscriber::getResponseFormat() will determine the format to return the response in.
This will check the route requirements that are dynamically generated by the rest module; e.g. _format & _content_type_format.
For delete routes, format NULL is returned, since dynamically generated DELETE routes do not contain _format or _content_type_format requirements. This is intentional, according to documentation in ResourceResponseSubscriber::renderResponseBody().

The symfony serializer does not seem to support this (anymore?). Also, the $format parameter is string typed now, so passing a NULL value as format now throws Symfony\Component\Serializer\Encoder\ChainEncoder::getEncoder(): Argument #1 ($format) must be of type string, null given in there. Not just a NotEncodableValueException, like in the original issue report.

For now, I have fixed the issue on my project by using a RouteSubscriber that alters the dynamically generated route by the rest module. Example snippet:

class RouteSubscriber extends RouteSubscriberBase {

  /**
   * {@inheritdoc}
   */
  protected function alterRoutes(RouteCollection $collection) {
    // Alter the dynamic delete route.
    if ($route = $collection->get('rest.example.DELETE')) {
      $route->setRequirement('_format', 'json');
    }
  }
}

This will make sure a format ('json' in this case) is passed to the symfony serializer; and avoid the fatal error from being thrown.

The easier solution seems to be expanding the array of request methods in ResourceRoutes::getRoutesForResourceConfig() to include the delete method, not sure yet if this will break some functionality.

In https://www.drupal.org/project/drupal/issues/3277148, documentation was added to indicate _format requirement must always be present when returning a ResourceResponse.

Created a merge request that will add the _format requirement to DELETE routes, to check which tests will break

Steps to reproduce on a clean drupal install:

1. Enable the core rest module
2. Create custom resource plugin with a DELETE method, this function will return a ResourceResponse with content:
   

   /**
    * Provides an example resource.
    *
    * @RestResource(
    *   id = "example_resource",
    *   label = @Translation("Example resource"),
    *   uri_paths = {
    *     "canonical" = "/api/example/{id}",
    *   }
    * )
    */
   class ExampleResource extends ResourceBase {
   
     /**
      * Delete an example entity based on ID.
      *
      * @param string $id
      *   The example ID.
      *
      * @return \Drupal\rest\ResourceResponse
      *   The resource response.
      */
     public function delete(string $id): ResourceResponse {
       return new ResourceResponse(['test123'], 200);
     }
   
   }

3. Configure & enable the plugin:
   

   status: true
   dependencies:
     module:
       - example_module
       - serialization
       - user
   id: example_resource
   plugin_id: example_resource
   granularity: resource
   configuration:
     methods:
       - DELETE
     formats:
       - json
     authentication:
       - cookie

4. Send delete request to /api/example/123
5. Returns statuscode 500; log: TypeError: Symfony\Component\Serializer\Encoder\ChainEncoder::getEncoder(): Argument #1 ($format) must be of type string, null given, called in /var/www/html/vendor/symfony/serializer/Encoder/ChainEncoder.php on line 49 in Symfony\Component\Serializer\Encoder\ChainEncoder->getEncoder() (line 80 of /var/www/html/vendor/symfony/serializer/Encoder/ChainEncoder.php)

I don't really understand the assumption where DELETE requests cannot contain a response body; according to https://www.rfc-editor.org/rfc/rfc9110.html#section-9.3.5-4, it should be allowed to return a 200 status code with extra information in the response. It's also possible to use statuscode 204, and omit the response body.

Any maintainers willing to weigh in on this?

Issue summary should be updated with proposed solution, any api changes, etc.

Also will need a test case showing the issue.

Thanks!

I've reformatted the issue summary and attached a patch containing a test that should fail.
It mocks a DELETE endpoint rest plugin (no _format route requirement defined), where some content is returned.

Removing the needs tests as #14 shows a valid failure.

But can the test be added to the MR to show that the change passes the valid test you uploaded

Thanks!

The test was actually already included in the merge request, since I've removed the $method !== 'DELETE' conditionals from existing tests (testOnResponseWithCacheableResponse & testOnResponseWithUncacheableResponse).

This will make the existing assertions for the other request methods trigger for DELETE method requests too.

Reran the tests on the MR just to confirm it was random failure before (it was).

Committed/pushed to 10.1.x and cherry-picked to 10.0.x and 9.5.x, thanks!