[FEATURE] Add a location header after creating an entity

Patch for adding Location in response headers.

Fix the broken test and change array to the small version []
Test for this path is still pending

I believe that JSON API specification compliance issue should be resolved. Attached the patch with test.

My pleasure. FIY: tests failed because of the issue not related to the patch.

The further progress is blocked by #2877589: Fix failing test in the dev branch, because tests fail.

The blocker is cleared now, submitting the patch for re-testing.

Now all tests pass, awaiting for manual review.

This patch is incorrect, because it's not taking into account the fact that not every entity type has a canonical URL.

See #2853211: EntityResource::post() incorrectly assumes that every entity type has a canonical URL for Drupal core.

Very good point, thanks @Wim. I'll provide a new patch.

(my 2 cents)

Weird as this committed patch give a location to http://drupal.d8/node/16 but the redirection resolves to the expected application/vnd.api+json response.

curl
  --include
  --user api:api
  --header 'Accept: application/vnd.api+json'
  --header 'Content-type: application/vnd.api+json'
  --request POST http://drupal.d8/jsonapi/node/article --data-binary @jsonapi-node-article-user.json
HTTP/1.1 201 Created
Date: Sat, 13 May 2017 10:37:09 GMT
Server: Apache/2.4.18 (Unix) PHP/5.6.29
X-Content-Type-Options: nosniff
X-Powered-By: PHP/5.6.29
Cache-Control: must-revalidate, no-cache, private
Location: http://drupal.d8/node/16 <=================================

Fetch that location manually breaks of course
curl --header 'Accept: application/vnd.api+json' --request GET http://drupal.d8/node/16

Not sure what this means.

Yet another good point, thanks @clemens.tolboom. Trying to bring all points together:

1. I assume that when an entity doesn't have a canonical URL we should not send Location header. However, specification explicitly says:

The response SHOULD include a Location header identifying the location of the newly created resource.

Probably empty Location header doesn't make sense here, as it might bring some confusion into DX. Any other suggestions?

2. Very good question in #22. Specification says that Location should identify the location of the newly created resource. The correct Drupal location is entity's canonical URL, which doesn't give any value for frontend apps using JSON API. So I suppose we should provide JSONAPI-friendly url to fetch an entity. It sort of kills issue #1 above (because there is no need to check for canonical url), but potentially brings in additional permission check to view the entity (or no need?).

3. From the resource POSTing specification:

If the resource object returned by the response contains a self key in its links member and a Location header is provided, the value of the self member MUST match the value of the Location header.

This confirms my assumption in #3 - Location URL should match ['links']['self'], which currently provides json api-friendly endpoint to fetch the resource.

Any comments / notes before I move on with the implementation?

Yeah, thanks everyone for the feedback! Attaching the patch for the most thorough review ever :P

Updated dependent tests as well.

Actually nothing bad will happen :) I was going to create a follow up to get rid of it after this issue is done, to avoid patch conflicts. But we can get rid of it right now as well if you want.

Oh, it's already done :) thanks a lot Mateu and other guys for getting this issue resolved.

Note that this is exactly why I argued a long time ago that the JSON API module should offer new link relation types. Then this could would have been far more elegant, and this mistake would likely have been prevented.
That can still happen at a later time of course.

Finally, test coverage is far too weak here. This should have a hardcoded relative URL, then the problem would have been obvious. Furthermore, there is no test coverage for entity types without a canonical URL: in core REST there would not be a Location response header, but for JSON API there would be.

All things that could (and should!) be addressed later.

1. I brought this up in #2829746: Clean up \Drupal\jsonapi\LinkManager\LinkManager(Interface) before, but that's been closed since.

2. Yep, we do want it to match the JSON API self link. Maybe that's tested against a hardcoded URL somewhere. But in the patches committed here, that was definitely not the case. Otherwise this bug would never have been introduced, because the problem would've been obvious. That's the value of seeing hardcoded strings in your test expectations.

3. Imagine that entity type foo does not have a canonical URL. That means https://d8/foo/5 does not exist. But https://d8/jsonapi/foo/5 will exist, because JSON API generates a "individual JSON API resource" URL for every entity, even if it's an entity of an entity type with no canonical link relation type.