REST should respond with a 409 for a POST request to an existing entity

The goal is to make this test pass.

I think that this cannot be implemented in EntityResource::post(). Because the 409 must be thrown when POSTing to /node/1, not when POSTing to /node. But for the /node/1 URL, only GET, PATCH and DELETE routes exist.

Therefore I think the only way to implement this is via a \Drupal\Core\Routing\RoutingEvents::ALTER event subscriber, which modifies all REST GET routes to also accept POST and then adding a route filter that explicitly detects this case ("POSTing to a GET-only route") and then throws a 409 error response exception.

If I'm right, then this is ridiculously convoluted, and probably not worth it…

That's very clever! :)

So let's add back the test coverage I wrote in #2, to verify that this is working as intended.

#10 is failing because of a tiny bug in #9.

But then there is another problem: because the "GET" route now also supports the "POST" method, an important assertion that was added in #2775479: Try to remove the "map HEAD to GET" logic in \Drupal\rest\RequestHandler::handle() no longer holds true.

This should be green!

And clearly this patch is blocked on #2720233: Use arguments resolver in RequestHandler to have consistent parameter ordering: it needs the improvements there, otherwise this can't work:

+++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
@@ -114,12 +116,19 @@ public static function create(ContainerInterface $container, array $configuratio
-  public function get(EntityInterface $entity) {
+  public function get(EntityInterface $entity, Request $request) {

This will also need a change record to inform developers that this is now supported.

This is one of those cases where doing things right, actually helps :)

:)

#11 wasn't green though! There are failures for all ItemResourceTestBase and EntityTestResourceTestBase tests (6 each), plus 2 failures in RequestHandlerTest.

Also… while thinking some more about this yesterday evening, I realized this is not yet sufficient:

+++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
@@ -114,12 +116,19 @@ public static function create(ContainerInterface $container, array $configuratio
-  public function get(EntityInterface $entity) {
+  public function get(EntityInterface $entity, Request $request) {
+    if ($request->getMethod() === 'POST') {
+      throw new ConflictHttpException('POSTING to an already existing entity resource');
+    }

+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
@@ -869,6 +869,11 @@ public function testPost() {
+    // DX: 409 when POSTing to an existing entity.
+    $response = $this->request('POST', $this->getEntityResourceUrl()->setOption('query', ['_format' => static::$format]), $request_options);
+    $this->assertResourceErrorResponse(409, 'POSTING to an already existing entity resource', $response);

i.e. this is testing POSTing to /node/5 results in a failure. But what we want, is that POSTing to /node results in a failure, by checking whether the UniqueField constraint has been violated!

I guess that could be a follow-up, but it might be better to handle it here, because it's conceptually about the same thing. Perhaps our implementation becomes better because of it, because we'll be throwing a 409 HTTP exception from two places.

#12 said this was blocked on #2720233: Use arguments resolver in RequestHandler to have consistent parameter ordering, but it hasn't been blocked on that for a long while, because that landed in September 2017!

#19: haha :P Thanks for bubbling this issue to the top again, it's how I noticed #20!

Most of this patch was committed in #2720233: Use arguments resolver in RequestHandler to have consistent parameter ordering. Which means this patch can become a lot smaller! 🎉

In the mean time, #2858482: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent got committed, which massively conflicted with this patch's changes in ResourceBase. The attached interdiff is not really a diff. It shows the change we're making to ResourceBase as of this new patch.