diff --git a/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php b/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php index 068162b..8966ca8 100644 --- a/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php +++ b/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php @@ -7,13 +7,14 @@ namespace Drupal\Core\ParamConverter; -use Symfony\Component\DependencyInjection\ContainerAware; +use Drupal\Component\Utility\String; use Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface; use Symfony\Cmf\Component\Routing\RouteObjectInterface; +use Symfony\Component\DependencyInjection\ContainerAware; use Symfony\Component\HttpFoundation\ParameterBag; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Routing\RouteCollection; -use Symfony\Component\HttpFoundation\Request; /** * Manages converter services for converting request parameters to full objects. @@ -186,7 +187,7 @@ public function enhance(array $defaults, Request $request) { // converted in which case we throw a 404. $defaults[$name] = $this->getConverter($definition['converter'])->convert($defaults[$name], $definition, $name, $defaults, $request); if (!isset($defaults[$name])) { - throw new NotFoundHttpException(format_string('Item "@name" with ID @id not found', array( + throw new NotFoundHttpException(String::format('Item "@name" with ID @id not found', array( '@name' => $name, '@id' => $defaults['_raw_variables']->get($name), ))); diff --git a/core/modules/rest/lib/Drupal/rest/Plugin/Derivative/EntityDerivative.php b/core/modules/rest/lib/Drupal/rest/Plugin/Derivative/EntityDerivative.php index 2b9abd7..88c67b6 100644 --- a/core/modules/rest/lib/Drupal/rest/Plugin/Derivative/EntityDerivative.php +++ b/core/modules/rest/lib/Drupal/rest/Plugin/Derivative/EntityDerivative.php @@ -77,10 +77,10 @@ public function getDerivativeDefinitions(array $base_plugin_definition) { // Use the entity links as REST URL patterns if available. $this->derivatives[$entity_type]['links']['drupal:create'] = isset($entity_info['links']['drupal:create']) ? $entity_info['links']['drupal:create'] : "/entity/$entity_type"; // Replace the default cannonical link pattern with a version that - // directly uses the entity type, because we don't want to hand 2 - // parameters to the entity resource plugin. + // directly uses the entity type, because we want only one parameter and + // automatic upcasting. if ($entity_info['links']['canonical'] == '/entity/{entityType}/{id}') { - $this->derivatives[$entity_type]['links']['canonical'] = "/entity/$entity_type/{id}"; + $this->derivatives[$entity_type]['links']['canonical'] = "/entity/$entity_type/" . '{' . $entity_type . '}'; } else { $this->derivatives[$entity_type]['links']['canonical'] = $entity_info['links']['canonical']; diff --git a/core/modules/rest/lib/Drupal/rest/Plugin/rest/resource/EntityResource.php b/core/modules/rest/lib/Drupal/rest/Plugin/rest/resource/EntityResource.php index e3f9f36..d53687d 100644 --- a/core/modules/rest/lib/Drupal/rest/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/lib/Drupal/rest/Plugin/rest/resource/EntityResource.php @@ -37,32 +37,24 @@ class EntityResource extends ResourceBase { /** * Responds to entity GET requests. * - * @param mixed $entity - * The entity ID or the already upcasted entity object. + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity object. * * @return \Drupal\rest\ResourceResponse - * The response containing the loaded entity. + * The response containing the entity with its accessible fields. * * @throws \Symfony\Component\HttpKernel\Exception\HttpException */ - public function get($entity) { - $id = $entity; - if (is_scalar($entity)) { - $definition = $this->getPluginDefinition(); - $entity = entity_load($definition['entity_type'], $entity); - } - if ($entity) { - if (!$entity->access('view')) { - throw new AccessDeniedHttpException(); - } - foreach ($entity as $field_name => $field) { - if (!$field->access('view')) { - unset($entity->{$field_name}); - } + public function get(EntityInterface $entity) { + if (!$entity->access('view')) { + throw new AccessDeniedHttpException(); + } + foreach ($entity as $field_name => $field) { + if (!$field->access('view')) { + unset($entity->{$field_name}); } - return new ResourceResponse($entity); } - throw new NotFoundHttpException(t('Entity with ID @id not found', array('@id' => $id))); + return new ResourceResponse($entity); } /** @@ -119,8 +111,8 @@ public function post(EntityInterface $entity = NULL) { /** * Responds to entity PATCH requests. * - * @param mixed $original_entity - * The entity ID or the already upcasted original entity object. + * @param \Drupal\Core\Entity\EntityInterface $original_entity + * The original entity object. * @param \Drupal\Core\Entity\EntityInterface $entity * The entity. * @@ -129,26 +121,14 @@ public function post(EntityInterface $entity = NULL) { * * @throws \Symfony\Component\HttpKernel\Exception\HttpException */ - public function patch($original_entity, EntityInterface $entity = NULL) { + public function patch(EntityInterface $original_entity, EntityInterface $entity = NULL) { if ($entity == NULL) { throw new BadRequestHttpException(t('No entity content received.')); } - - if (empty($original_entity)) { - throw new NotFoundHttpException(); - } $definition = $this->getPluginDefinition(); if ($entity->entityType() != $definition['entity_type']) { throw new BadRequestHttpException(t('Invalid entity type')); } - if (is_scalar($original_entity)) { - $original_entity = entity_load($definition['entity_type'], $original_entity); - } - // We don't support creating entities with PATCH, so we throw an error if - // there is no existing entity. - if ($original_entity == FALSE) { - throw new NotFoundHttpException(); - } if (!$original_entity->access('update')) { throw new AccessDeniedHttpException(); } @@ -183,36 +163,28 @@ public function patch($original_entity, EntityInterface $entity = NULL) { /** * Responds to entity DELETE requests. * - * @param mixed $entity - * The entity ID or the already upcasted entity object. + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity object. * * @return \Drupal\rest\ResourceResponse * The HTTP response object. * * @throws \Symfony\Component\HttpKernel\Exception\HttpException */ - public function delete($entity) { - $id = $entity; - if (is_scalar($entity)) { - $definition = $this->getPluginDefinition(); - $entity = entity_load($definition['entity_type'], $id); - } - if ($entity) { - if (!$entity->access('delete')) { - throw new AccessDeniedHttpException(); - } - try { - $entity->delete(); - watchdog('rest', 'Deleted entity %type with ID %id.', array('%type' => $entity->entityType(), '%id' => $entity->id())); + public function delete(EntityInterface $entity) { + if (!$entity->access('delete')) { + throw new AccessDeniedHttpException(); + } + try { + $entity->delete(); + watchdog('rest', 'Deleted entity %type with ID %id.', array('%type' => $entity->entityType(), '%id' => $entity->id())); - // Delete responses have an empty body. - return new ResourceResponse(NULL, 204); - } - catch (EntityStorageException $e) { - throw new HttpException(500, t('Internal Server Error'), $e); - } + // Delete responses have an empty body. + return new ResourceResponse(NULL, 204); + } + catch (EntityStorageException $e) { + throw new HttpException(500, t('Internal Server Error'), $e); } - throw new NotFoundHttpException(t('Entity with ID @id not found', array('@id' => $id))); } /** diff --git a/core/modules/rest/lib/Drupal/rest/RequestHandler.php b/core/modules/rest/lib/Drupal/rest/RequestHandler.php index 23a1022..5012be7 100644 --- a/core/modules/rest/lib/Drupal/rest/RequestHandler.php +++ b/core/modules/rest/lib/Drupal/rest/RequestHandler.php @@ -74,7 +74,7 @@ public function handle(Request $request) { $parameters = array(); // Filter out all internal parameters starting with "_". foreach ($route_parameters as $key => $parameter) { - if (strpos($key, '_') !== 0) { + if ($key{0} !== '_') { $parameters[] = $parameter; } } diff --git a/core/modules/rest/lib/Drupal/rest/Tests/DeleteTest.php b/core/modules/rest/lib/Drupal/rest/Tests/DeleteTest.php index 62341f9..95b1dfd 100644 --- a/core/modules/rest/lib/Drupal/rest/Tests/DeleteTest.php +++ b/core/modules/rest/lib/Drupal/rest/Tests/DeleteTest.php @@ -61,13 +61,7 @@ public function testDelete() { // Try to delete an entity that does not exist. $response = $this->httpRequest($this->entityBasePath($entity_type) . '/9999', 'DELETE'); $this->assertResponse(404); - if ($entity_type == 'entity_test') { - $decoded = drupal_json_decode($response); - $this->assertEqual($decoded['error'], 'Entity with ID 9999 not found', 'Response message is correct.'); - } - else { - $this->assertText('The requested page "/node/9999" could not be found.'); - } + $this->assertText('The requested page "/' . $this->entityBasePath($entity_type) . '/9999" could not be found.'); // Try to delete an entity without proper permissions. $this->drupalLogout(); diff --git a/core/modules/rest/lib/Drupal/rest/Tests/ReadTest.php b/core/modules/rest/lib/Drupal/rest/Tests/ReadTest.php index 071a598..61a0408 100644 --- a/core/modules/rest/lib/Drupal/rest/Tests/ReadTest.php +++ b/core/modules/rest/lib/Drupal/rest/Tests/ReadTest.php @@ -74,15 +74,7 @@ public function testRead() { // Try to read an entity that does not exist. $response = $this->httpRequest($this->entityBasePath($entity_type) . '/9999', 'GET', NULL, $this->defaultMimeType); $this->assertResponse(404); - if ($entity_type == 'entity_test') { - $decoded = drupal_json_decode($response); - $this->assertEqual($decoded['error'], 'Entity with ID 9999 not found'); - } - if ($entity_type == 'node') { - // Nodes are different because they get upcasted by a route enhancer, - // which throws an exception if the node does not exist. - $this->assertEqual($response, 'An error occurred: Item "node" with ID 9999 not found'); - } + $this->assertEqual($response, 'An error occurred: Item "' . $entity_type . '" with ID 9999 not found'); // Make sure that field level access works and that the according field is // not available in the response. Only applies to entity_test. diff --git a/core/tests/Drupal/Tests/Core/Routing/AcceptHeaderMatcherTest.php b/core/tests/Drupal/Tests/Core/Routing/AcceptHeaderMatcherTest.php index 2ad2dde..2fb63cb 100644 --- a/core/tests/Drupal/Tests/Core/Routing/AcceptHeaderMatcherTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/AcceptHeaderMatcherTest.php @@ -12,7 +12,6 @@ use Drupal\system\Tests\Routing\RoutingFixtures; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException; /** * Basic tests for the AcceptHeaderMatcher class. @@ -26,6 +25,13 @@ class AcceptHeaderMatcherTest extends UnitTestCase { */ protected $fixtures; + /** + * The matcher object that is going to be tested. + * + * @var \Drupal\Core\Routing\AcceptHeaderMatcher + */ + protected $matcher; + public static function getInfo() { return array( 'name' => 'Partial matcher MIME types tests', @@ -38,20 +44,19 @@ public function setUp() { parent::setUp(); $this->fixtures = new RoutingFixtures(); + $this->matcher = new AcceptHeaderMatcher(new ContentNegotiation()); } /** - * Confirms that the MimeType matcher matches properly. + * Check that JSON routes get filtered and prioritized correctly. */ - public function testFilterRoutes() { - - $matcher = new AcceptHeaderMatcher(new ContentNegotiation()); + public function testJsonFilterRoutes() { $collection = $this->fixtures->sampleRouteCollection(); // Tests basic JSON request. $request = Request::create('path/two', 'GET'); $request->headers->set('Accept', 'application/json, text/xml;q=0.9'); - $routes = $matcher->filter($collection, $request); + $routes = $this->matcher->filter($collection, $request); $this->assertEquals(count($routes), 4, 'The correct number of routes was found.'); $this->assertNotNull($routes->get('route_c'), 'The json route was found.'); $this->assertNull($routes->get('route_e'), 'The html route was not found.'); @@ -59,11 +64,17 @@ public function testFilterRoutes() { $this->assertEquals($name, 'route_c', 'The json route is the first one in the collection.'); break; } + } + + /** + * Tests a JSON request with alternative JSON MIME type Accept header. + */ + public function testAlternativeJson() { + $collection = $this->fixtures->sampleRouteCollection(); - // Tests JSON request with alternative JSON MIME type Accept header. $request = Request::create('path/two', 'GET'); $request->headers->set('Accept', 'application/x-json, text/xml;q=0.9'); - $routes = $matcher->filter($collection, $request); + $routes = $this->matcher->filter($collection, $request); $this->assertEquals(count($routes), 4, 'The correct number of routes was found.'); $this->assertNotNull($routes->get('route_c'), 'The json route was found.'); $this->assertNull($routes->get('route_e'), 'The html route was not found.'); @@ -71,11 +82,17 @@ public function testFilterRoutes() { $this->assertEquals($name, 'route_c', 'The json route is the first one in the collection.'); break; } + } + + /** + * Tests a standard HTML request. + */ + public function teststandardHtml() { + $collection = $this->fixtures->sampleRouteCollection(); - // Tests basic HTML request. $request = Request::create('path/two', 'GET'); $request->headers->set('Accept', 'text/html, text/xml;q=0.9'); - $routes = $matcher->filter($collection, $request); + $routes = $this->matcher->filter($collection, $request); $this->assertEquals(count($routes), 4, 'The correct number of routes was found.'); $this->assertNull($routes->get('route_c'), 'The json route was not found.'); $this->assertNotNull($routes->get('route_e'), 'The html route was found.'); @@ -86,11 +103,12 @@ public function testFilterRoutes() { } /** - * Confirms that the AcceptHeaderMatcher matcher throws an exception for no-route. + * Confirms that the AcceptHeaderMatcher throws an exception for no-route. + * + * @expectedException \Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException + * @expectedExceptionMessage No route found for the specified formats application/json text/xml. */ public function testNoRouteFound() { - $matcher = new AcceptHeaderMatcher(new ContentNegotiation()); - // Remove the sample routes that would match any method. $routes = $this->fixtures->sampleRouteCollection(); $routes->remove('route_a'); @@ -98,15 +116,10 @@ public function testNoRouteFound() { $routes->remove('route_c'); $routes->remove('route_d'); - try { - $request = Request::create('path/two', 'GET'); - $request->headers->set('Accept', 'application/json, text/xml;q=0.9'); - $matcher->filter($routes, $request); - $this->fail('No exception was thrown.'); - } - catch (NotAcceptableHttpException $e) { - $this->assertEquals($e->getMessage(), 'No route found for the specified formats application/json text/xml.'); - } + $request = Request::create('path/two', 'GET'); + $request->headers->set('Accept', 'application/json, text/xml;q=0.9'); + $this->matcher->filter($routes, $request); + $this->fail('No exception was thrown.'); } } diff --git a/core/tests/Drupal/Tests/Core/Routing/ContentTypeHeaderMatcherTest.php b/core/tests/Drupal/Tests/Core/Routing/ContentTypeHeaderMatcherTest.php index 1ecd142..2b8990f 100644 --- a/core/tests/Drupal/Tests/Core/Routing/ContentTypeHeaderMatcherTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/ContentTypeHeaderMatcherTest.php @@ -11,7 +11,6 @@ use Drupal\system\Tests\Routing\RoutingFixtures; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; /** * Basic tests for the ContentTypeHeaderMatcher class. @@ -25,6 +24,13 @@ class ContentTypeHeaderMatcherTest extends UnitTestCase { */ protected $fixtures; + /** + * The matcher object that is going to be tested. + * + * @var \Drupal\Core\Routing\ContentTypeHeaderMatcher + */ + protected $matcher; + public static function getInfo() { return array( 'name' => 'Content Type header matcher test', @@ -37,26 +43,31 @@ public function setUp() { parent::setUp(); $this->fixtures = new RoutingFixtures(); + $this->matcher = new ContentTypeHeaderMatcher(); } /** - * Confirms that the Content type matcher matches properly. + * Tests that routes are not filtered on GET requests. */ - public function testFilterRoutes() { - - $matcher = new ContentTypeHeaderMatcher(); + public function testGetRequestFilter() { $collection = $this->fixtures->sampleRouteCollection(); $collection->addCollection($this->fixtures->contentRouteCollection()); - // Tests that routes are not filtered on GET requests. $request = Request::create('path/two', 'GET'); - $routes = $matcher->filter($collection, $request); + $routes = $this->matcher->filter($collection, $request); $this->assertEquals(count($routes), 7, 'The correct number of routes was found.'); + } + + /** + * Tests that XML-restricted routes get filtered out on JSON requests. + */ + public function testJsonRequest() { + $collection = $this->fixtures->sampleRouteCollection(); + $collection->addCollection($this->fixtures->contentRouteCollection()); - // Test that XML-restricted routes get filtered out on JSON requests. $request = Request::create('path/two', 'POST'); $request->headers->set('Content-type', 'application/json'); - $routes = $matcher->filter($collection, $request); + $routes = $this->matcher->filter($collection, $request); $this->assertEquals(count($routes), 6, 'The correct number of routes was found.'); $this->assertNotNull($routes->get('route_f'), 'The json route was found.'); $this->assertNull($routes->get('route_g'), 'The xml route was not found.'); @@ -64,12 +75,20 @@ public function testFilterRoutes() { $this->assertEquals($name, 'route_f', 'The json route is the first one in the collection.'); break; } + } - // Test all XML and JSON restricted routes get filtered out on a POST form - // submission. + /** + * Tests route filtering on POST form submission requests. + */ + public function testPostForm() { + $collection = $this->fixtures->sampleRouteCollection(); + $collection->addCollection($this->fixtures->contentRouteCollection()); + + // Test that all XML and JSON restricted routes get filtered out on a POST + // form submission. $request = Request::create('path/two', 'POST'); $request->headers->set('Content-type', 'application/www-form-urlencoded'); - $routes = $matcher->filter($collection, $request); + $routes = $this->matcher->filter($collection, $request); $this->assertEquals(count($routes), 5, 'The correct number of routes was found.'); $this->assertNull($routes->get('route_f'), 'The json route was found.'); $this->assertNull($routes->get('route_g'), 'The xml route was not found.'); @@ -77,20 +96,18 @@ public function testFilterRoutes() { /** * Confirms that the matcher throws an exception for no-route. + * + * @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException + * @expectedExceptionMessage No route found that matches the Content-Type header. */ public function testNoRouteFound() { $matcher = new ContentTypeHeaderMatcher(); $routes = $this->fixtures->contentRouteCollection(); - try { - $request = Request::create('path/two', 'POST'); - $request->headers->set('Content-type', 'application/hal+json'); - $matcher->filter($routes, $request); - $this->fail('No exception was thrown.'); - } - catch (BadRequestHttpException $e) { - $this->assertEquals($e->getMessage(), 'No route found that matches the Content-Type header.'); - } + $request = Request::create('path/two', 'POST'); + $request->headers->set('Content-type', 'application/hal+json'); + $matcher->filter($routes, $request); + $this->fail('No exception was thrown.'); } }