Commit 2bb04688 authored by alexpott's avatar alexpott
Browse files

Issue #2662284 by Wim Leers, swim, tedbow, dawehner: Return complete entity after successful PATCH

parent 8db32d1a
......@@ -229,8 +229,8 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity
$original_entity->save();
$this->logger->notice('Updated entity %type with ID %id.', array('%type' => $original_entity->getEntityTypeId(), '%id' => $original_entity->id()));
// Update responses have an empty body.
return new ModifiedResourceResponse(NULL, 204);
// Return the updated entity in the response body.
return new ModifiedResourceResponse($original_entity, 200);
}
catch (EntityStorageException $e) {
throw new HttpException(500, 'Internal Server Error', $e);
......
......@@ -115,10 +115,7 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
}
// Invoke the operation on the resource plugin.
// All REST routes are restricted to exactly one format, so instead of
// parsing it out of the Accept headers again, we can simply retrieve the
// format requirement. If there is no format associated, just pick JSON.
$format = $route_match->getRouteObject()->getRequirement('_format') ?: 'json';
$format = $this->getResponseFormat($route_match, $request);
try {
$response = call_user_func_array(array($resource, $method), array_merge($parameters, array($unserialized, $request)));
}
......@@ -136,6 +133,54 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
$response;
}
/**
* Determines the format to respond in.
*
* Respects the requested format if one is specified. However, it is common to
* forget to specify a request format in case of a POST or PATCH. Rather than
* simply throwing an error, we apply the robustness principle: when POSTing
* or PATCHing using a certain format, you probably expect a response in that
* same format.
*
* @param \Drupal\Core\Routing\RouteMatchInterface $route_match
* The current route match.
* @param \Symfony\Component\HttpFoundation\Request $request
* The current request.
*
* @return string
* The response format.
*/
protected function getResponseFormat(RouteMatchInterface $route_match, Request $request) {
$route = $route_match->getRouteObject();
$acceptable_request_formats = $route->hasRequirement('_format') ? explode('|', $route->getRequirement('_format')) : [];
$acceptable_content_type_formats = $route->hasRequirement('_content_type_format') ? explode('|', $route->getRequirement('_content_type_format')) : [];
$acceptable_formats = $request->isMethodSafe() ? $acceptable_request_formats : $acceptable_content_type_formats;
$requested_format = $request->getRequestFormat();
$content_type_format = $request->getContentType();
// If an acceptable format is requested, then use that. Otherwise, including
// and particularly when the client forgot to specify a format, then use
// heuristics to select the format that is most likely expected.
if (in_array($requested_format, $acceptable_formats)) {
return $requested_format;
}
// If a request body is present, then use the format corresponding to the
// request body's Content-Type for the response, if it's an acceptable
// format for the request.
elseif (!empty($request->getContent()) && in_array($content_type_format, $acceptable_content_type_formats)) {
return $content_type_format;
}
// Otherwise, use the first acceptable format.
elseif (!empty($acceptable_formats)) {
return $acceptable_formats[0];
}
// Sometimes, there are no acceptable formats, e.g. DELETE routes.
else {
return NULL;
}
}
/**
* Generates a CSRF protecting session token.
*
......@@ -161,8 +206,9 @@ public function csrfToken() {
* The response from the REST resource.
* @param \Symfony\Component\Serializer\SerializerInterface $serializer
* The serializer to use.
* @param string $format
* The response format.
* @param string|null $format
* The response format, or NULL in case the response does not need a format,
* for example for the response to a DELETE request.
* @param \Drupal\rest\RestResourceConfigInterface $resource_config
* The resource config.
*
......@@ -176,24 +222,30 @@ protected function renderResponse(Request $request, ResourceResponseInterface $r
$data = $response->getResponseData();
if ($response instanceof CacheableResponseInterface) {
$context = new RenderContext();
$output = $this->container->get('renderer')
->executeInRenderContext($context, function () use ($serializer, $data, $format) {
return $serializer->serialize($data, $format);
});
if (!$context->isEmpty()) {
$response->addCacheableDependency($context->pop());
}
// Add rest config's cache tags.
$response->addCacheableDependency($resource_config);
}
else {
$output = $serializer->serialize($data, $format);
// If there is data to send, serialize and set it as the response body.
if ($data !== NULL) {
if ($response instanceof CacheableResponseInterface) {
$context = new RenderContext();
$output = $this->container->get('renderer')
->executeInRenderContext($context, function () use ($serializer, $data, $format) {
return $serializer->serialize($data, $format);
});
if (!$context->isEmpty()) {
$response->addCacheableDependency($context->pop());
}
}
else {
$output = $serializer->serialize($data, $format);
}
$response->setContent($output);
$response->headers->set('Content-Type', $request->getMimeType($format));
}
$response->setContent($output);
$response->headers->set('Content-Type', $request->getMimeType($format));
return $response;
}
......
......@@ -110,7 +110,7 @@ public function testNodes() {
);
$serialized = $this->container->get('serializer')->serialize($data, $this->defaultFormat);
$this->httpRequest($node->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType);
$this->assertResponse(204);
$this->assertResponse(200);
// Reload the node from the DB and check if the title was correctly updated.
$node_storage->resetCache(array($node->id()));
......
......@@ -119,7 +119,7 @@ public function testConfigChangePageCache() {
// Update the entity over the REST API.
$this->httpRequest($url, 'PATCH', $serialized, $this->defaultMimeType);
$this->assertResponse(204);
$this->assertResponse(200);
if ($this->getCacheHeaderValues('x-drupal-cache')) {
$this->fail('Patch request is cached.');
......
......@@ -68,8 +68,20 @@ public function testPatchUpdate() {
$serialized = $serializer->serialize($patch_entity, $this->defaultFormat, $context);
// Update the entity over the REST API.
$this->httpRequest($entity->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType);
$this->assertResponse(204);
$response = $this->httpRequest($entity->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType);
$this->assertResponse(200);
// Make sure that the response includes an entity in the body, check the
// updated field as an example.
$request = Json::decode($serialized);
$response = Json::decode($response);
$this->assertEqual($request['field_test_text'][0]['value'], $response['field_test_text'][0]['value']);
unset($request['_links']);
unset($response['_links']);
unset($response['id']);
unset($response['uuid']);
unset($response['name']);
$this->assertEqual($request, $response);
// Re-load updated entity from the database.
$entity = entity_load($entity_type, $entity->id(), TRUE);
......@@ -81,7 +93,7 @@ public function testPatchUpdate() {
unset($normalized['field_test_text']);
$serialized = $serializer->encode($normalized, $this->defaultFormat);
$this->httpRequest($entity->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType);
$this->assertResponse(204);
$this->assertResponse(200);
$entity = entity_load($entity_type, $entity->id(), TRUE);
$this->assertNotNull($entity->field_test_text->value . 'Test field has not been deleted.');
......@@ -92,7 +104,7 @@ public function testPatchUpdate() {
// Update the entity over the REST API.
$this->httpRequest($entity->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType);
$this->assertResponse(204);
$this->assertResponse(200);
// Re-load updated entity from the database.
$entity = entity_load($entity_type, $entity->id(), TRUE);
......@@ -214,7 +226,7 @@ public function testUpdateUser() {
$normalized['pass'][0]['existing'] = $account->pass_raw;
$serialized = $serializer->serialize($normalized, $this->defaultFormat, $context);
$this->httpRequest($account->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType);
$this->assertResponse(204);
$this->assertResponse(200);
// Try to change the password without providing the current password.
$new_password = $this->randomString();
......@@ -230,7 +242,7 @@ public function testUpdateUser() {
$normalized['pass'][0]['existing'] = $account->pass_raw;
$serialized = $serializer->serialize($normalized, $this->defaultFormat, $context);
$this->httpRequest($account->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType);
$this->assertResponse(204);
$this->assertResponse(200);
// Verify that we can log in with the new password.
$account->pass_raw = $new_password;
......@@ -356,8 +368,7 @@ protected function patchEntity(EntityInterface $entity, array $read_only_fields,
$serialized = $serializer->serialize($normalized, $format, $context);
$this->httpRequest($url, 'PATCH', $serialized, $mime_type);
$this->assertResponse(204);
$this->assertResponseBody('');
$this->assertResponse(200);
}
}
......@@ -2,6 +2,7 @@
namespace Drupal\Tests\rest\Kernel;
use Drupal\Component\Serialization\Json;
use Drupal\Core\Entity\EntityStorageInterface;
use Drupal\Core\Routing\RouteMatch;
use Drupal\KernelTests\KernelTestBase;
......@@ -9,6 +10,8 @@
use Drupal\rest\RequestHandler;
use Drupal\rest\ResourceResponse;
use Drupal\rest\RestResourceConfigInterface;
use Prophecy\Argument;
use Prophecy\Prophecy\MethodProphecy;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Route;
......@@ -51,7 +54,7 @@ public function setUp() {
*/
public function testBaseHandler() {
$request = new Request();
$route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => 'restplugin', '_format' => 'json']));
$route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => 'restplugin'], ['_format' => 'json']));
$resource = $this->prophesize(StubRequestHandlerResourcePlugin::class);
$resource->get(NULL, $request)
......@@ -78,6 +81,7 @@ public function testBaseHandler() {
$this->assertEquals($response, $handler_response);
// We will call the patch method this time.
$route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => 'restplugin'], ['_content_type_format' => 'json']));
$request->setMethod('PATCH');
$response = new ResourceResponse([]);
$resource->patch(NULL, $request)
......@@ -93,13 +97,13 @@ public function testBaseHandler() {
* @dataProvider providerTestSerialization
* @covers ::handle
*/
public function testSerialization($data) {
public function testSerialization($data, $expected_response = FALSE) {
$request = new Request();
$route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => 'restplugin', '_format' => 'json']));
$route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => 'restplugin'], ['_format' => 'json']));
$resource = $this->prophesize(StubRequestHandlerResourcePlugin::class);
// Setup the configuration.
// Mock the configuration.
$config = $this->prophesize(RestResourceConfigInterface::class);
$config->getResourcePlugin()->willReturn($resource->reveal());
$config->getCacheContexts()->willReturn([]);
......@@ -112,12 +116,13 @@ public function testSerialization($data) {
->willReturn($response);
$handler_response = $this->requestHandler->handle($route_match, $request);
// Content is a serialized version of the data we provided.
$this->assertEquals(json_encode($data), $handler_response->getContent());
$this->assertEquals($expected_response !== FALSE ? $expected_response : json_encode($data), $handler_response->getContent());
}
public function providerTestSerialization() {
return [
[NULL],
// The default data for \Drupal\rest\ResourceResponse.
[NULL, ''],
[''],
['string'],
['Complex \ string $%^&@ with unicode ΑΒΓΔΕΖΗΘΙΚΛΜΝΞΟΣὨ'],
......@@ -132,6 +137,203 @@ public function providerTestSerialization() {
];
}
/**
* @covers ::getResponseFormat
*
* Note this does *not* need to test formats being requested that are not
* accepted by the server, because the routing system would have already
* prevented those from reaching RequestHandler.
*
* @param string[] $methods
* The HTTP methods to test.
* @param string[] $supported_formats
* The supported formats for the REST route to be tested.
* @param string|FALSE $request_format
* The value for the ?_format URL query argument, if any.
* @param string[] $request_headers
* The request headers to send, if any.
* @param string|NULL $request_body
* The request body to send, if any.
* @param string|NULL $expected_response_content_type
* The expected MIME type of the response, if any.
* @param string $expected_response_content
* The expected content of the response.
*
* @dataProvider providerTestResponseFormat
*/
public function testResponseFormat($methods, array $supported_formats, $request_format, array $request_headers, $request_body, $expected_response_content_type, $expected_response_content) {
$rest_config_name = $this->randomMachineName();
$parameters = [];
if ($request_format !== FALSE) {
$parameters['_format'] = $request_format;
}
foreach ($request_headers as $key => $value) {
unset($request_headers[$key]);
$key = strtoupper(str_replace('-', '_', $key));
$request_headers[$key] = $value;
}
foreach ($methods as $method) {
$request = Request::create('/rest/test', $method, $parameters, [], [], $request_headers, $request_body);
$route_requirement_key_format = $request->isMethodSafe() ? '_format' : '_content_type_format';
$route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => $rest_config_name], [$route_requirement_key_format => implode('|', $supported_formats)]));
$resource = $this->prophesize(StubRequestHandlerResourcePlugin::class);
// Mock the configuration.
$config = $this->prophesize(RestResourceConfigInterface::class);
$config->getFormats($method)->willReturn($supported_formats);
$config->getResourcePlugin()->willReturn($resource->reveal());
$config->getCacheContexts()->willReturn([]);
$config->getCacheTags()->willReturn([]);
$config->getCacheMaxAge()->willReturn(12);
$this->entityStorage->load($rest_config_name)->willReturn($config->reveal());
// Mock the resource plugin.
$response = new ResourceResponse($method !== 'DELETE' ? ['REST' => 'Drupal'] : NULL);
$resource->getPluginDefinition()->willReturn([]);
$method_prophecy = new MethodProphecy($resource, strtolower($method), [Argument::any(), $request]);
$method_prophecy->willReturn($response);
$resource->addMethodProphecy($method_prophecy);
// Test the request handler.
$handler_response = $this->requestHandler->handle($route_match, $request);
$this->assertSame($expected_response_content_type, $handler_response->headers->get('Content-Type'));
$this->assertEquals($expected_response_content, $handler_response->getContent());
}
}
/**
* @return array
* 0. methods to test
* 1. supported formats for route requirements
* 2. request format
* 3. request headers
* 4. request body
* 5. expected response content type
* 6. expected response body
*/
public function providerTestResponseFormat() {
$json_encoded = Json::encode(['REST' => 'Drupal']);
$xml_encoded = "<?xml version=\"1.0\"?>\n<response><REST>Drupal</REST></response>\n";
$safe_method_test_cases = [
'safe methods: client requested format (JSON)' => [
// @todo add 'HEAD' in https://www.drupal.org/node/2752325
['GET'],
['xml', 'json'],
'json',
[],
NULL,
'application/json',
$json_encoded,
],
'safe methods: client requested format (XML)' => [
// @todo add 'HEAD' in https://www.drupal.org/node/2752325
['GET'],
['xml', 'json'],
'xml',
[],
NULL,
'text/xml',
$xml_encoded,
],
'safe methods: client requested no format: response should use the first configured format (JSON)' => [
// @todo add 'HEAD' in https://www.drupal.org/node/2752325
['GET'],
['json', 'xml'],
FALSE,
[],
NULL,
'application/json',
$json_encoded,
],
'safe methods: client requested no format: response should use the first configured format (XML)' => [
// @todo add 'HEAD' in https://www.drupal.org/node/2752325
['GET'],
['xml', 'json'],
FALSE,
[],
NULL,
'text/xml',
$xml_encoded,
],
];
$unsafe_method_bodied_test_cases = [
'unsafe methods with response (POST, PATCH): client requested no format, response should use request body format (JSON)' => [
['POST', 'PATCH'],
['xml', 'json'],
FALSE,
['Content-Type' => 'application/json'],
$json_encoded,
'application/json',
$json_encoded,
],
'unsafe methods with response (POST, PATCH): client requested no format, response should use request body format (XML)' => [
['POST', 'PATCH'],
['xml', 'json'],
FALSE,
['Content-Type' => 'text/xml'],
$xml_encoded,
'text/xml',
$xml_encoded,
],
'unsafe methods with response (POST, PATCH): client requested format other than request body format (JSON): response format should use requested format (XML)' => [
['POST', 'PATCH'],
['xml', 'json'],
'xml',
['Content-Type' => 'application/json'],
$json_encoded,
'text/xml',
$xml_encoded,
],
'unsafe methods with response (POST, PATCH): client requested format other than request body format (XML), but is allowed for the request body (JSON)' => [
['POST', 'PATCH'],
['xml', 'json'],
'json',
['Content-Type' => 'text/xml'],
$xml_encoded,
'application/json',
$json_encoded,
],
];
$unsafe_method_bodyless_test_cases = [
'unsafe methods with response bodies (DELETE): client requested no format, response should have no format' => [
['DELETE'],
['xml', 'json'],
FALSE,
['Content-Type' => 'application/json'],
$json_encoded,
NULL,
'',
],
'unsafe methods with response bodies (DELETE): client requested format (XML), response should have no format' => [
['DELETE'],
['xml', 'json'],
'xml',
['Content-Type' => 'application/json'],
$json_encoded,
NULL,
'',
],
'unsafe methods with response bodies (DELETE): client requested format (JSON), response should have no format' => [
['DELETE'],
['xml', 'json'],
'json',
['Content-Type' => 'application/json'],
$json_encoded,
NULL,
'',
],
];
return $safe_method_test_cases + $unsafe_method_bodied_test_cases + $unsafe_method_bodyless_test_cases;
}
}
/**
......@@ -140,6 +342,8 @@ public function providerTestSerialization() {
class StubRequestHandlerResourcePlugin extends ResourceBase {
function get() {}
function post() {}
function patch() {}
function delete() {}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment