core/core.services.yml | 13 +++++-- core/lib/Drupal/Core/CoreServiceProvider.php | 2 -- .../Compiler/RegisterLazyRouteFilters.php | 32 ----------------- core/lib/Drupal/Core/Routing/LazyRouteEnhancer.php | 2 +- core/lib/Drupal/Core/Routing/LazyRouteFilter.php | 38 +++++++++++++-------- .../dblog/src/Tests/Rest/DbLogResourceTest.php | 6 ++-- core/modules/rest/src/Plugin/ResourceBase.php | 34 ++++++------------ core/modules/rest/src/Routing/ResourceRoutes.php | 18 +++++----- core/modules/rest/src/Tests/ResourceTest.php | 3 +- ....rest-rest_post_update_resource_granularity.php | Bin 5199 -> 5233 bytes .../EntityResource/EntityResourceTestBase.php | 5 +-- .../user/src/Tests/RestRegisterUserTest.php | 3 +- 12 files changed, 64 insertions(+), 92 deletions(-) diff --git a/core/core.services.yml b/core/core.services.yml index 796ece3..a59190c 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -824,9 +824,10 @@ services: deprecated: The "%service_id%" service is deprecated. You should use the 'router.no_access_checks' service instead. route_filter.lazy_collector: class: Drupal\Core\Routing\LazyRouteFilter + arguments: ['@class_resolver'] tags: + - { name: service_id_collector, tag: route_filter } - { name: non_lazy_route_filter } - parent: container.trait route_filter_subscriber: class: Drupal\Core\EventSubscriber\RouteFilterSubscriber arguments: ['@route_filter.lazy_collector'] @@ -938,15 +939,21 @@ services: request_format_route_filter: class: Drupal\Core\Routing\RequestFormatRouteFilter tags: + # The request format route filter must run last. - { name: route_filter } method_filter: class: Drupal\Core\Routing\MethodFilter tags: - - { name: route_filter, priority: 1 } + # The HTTP method route filter must run first: based on the request method, content type request header-based + # route filtering (content_type_header_matcher) may or may not be necessary. + - { name: route_filter, priority: 10 } content_type_header_matcher: class: Drupal\Core\Routing\ContentTypeHeaderMatcher tags: - - { name: route_filter } + # The content type request header router filter must run before the request format route filter + # (request_format_route_filter), because without a valid request body (for HTTP methods that need it, such as POST + # and PATCH), no successful response is possible. + - { name: route_filter, priority: 5 } paramconverter_manager: class: Drupal\Core\ParamConverter\ParamConverterManager tags: diff --git a/core/lib/Drupal/Core/CoreServiceProvider.php b/core/lib/Drupal/Core/CoreServiceProvider.php index 8a22bc5..719df12 100644 --- a/core/lib/Drupal/Core/CoreServiceProvider.php +++ b/core/lib/Drupal/Core/CoreServiceProvider.php @@ -11,7 +11,6 @@ use Drupal\Core\DependencyInjection\Compiler\ContextProvidersPass; use Drupal\Core\DependencyInjection\Compiler\ProxyServicesPass; use Drupal\Core\DependencyInjection\Compiler\RegisterLazyRouteEnhancers; -use Drupal\Core\DependencyInjection\Compiler\RegisterLazyRouteFilters; use Drupal\Core\DependencyInjection\Compiler\DependencySerializationTraitPass; use Drupal\Core\DependencyInjection\Compiler\StackedKernelPass; use Drupal\Core\DependencyInjection\Compiler\StackedSessionHandlerPass; @@ -85,7 +84,6 @@ public function register(ContainerBuilder $container) { $container->addCompilerPass(new RegisterAccessChecksPass()); $container->addCompilerPass(new RegisterLazyRouteEnhancers()); - $container->addCompilerPass(new RegisterLazyRouteFilters()); // Add a compiler pass for registering services needing destruction. $container->addCompilerPass(new RegisterServicesForDestructionPass()); diff --git a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterLazyRouteFilters.php b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterLazyRouteFilters.php deleted file mode 100644 index d774779..0000000 --- a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterLazyRouteFilters.php +++ /dev/null @@ -1,32 +0,0 @@ -hasDefinition('route_filter.lazy_collector')) { - return; - } - - $service_ids = []; - - foreach ($container->findTaggedServiceIds('route_filter') as $id => $attributes) { - $service_ids[$id] = $id; - } - - $container - ->getDefinition('route_filter.lazy_collector') - ->addArgument($service_ids); - } - -} diff --git a/core/lib/Drupal/Core/Routing/LazyRouteEnhancer.php b/core/lib/Drupal/Core/Routing/LazyRouteEnhancer.php index 356fe3e..5299604 100644 --- a/core/lib/Drupal/Core/Routing/LazyRouteEnhancer.php +++ b/core/lib/Drupal/Core/Routing/LazyRouteEnhancer.php @@ -67,7 +67,7 @@ public function setEnhancers(RouteCollection $route_collection) { } /** - * For each route, gets a list of applicable enhancer to the route. + * Gets all lazy route enhancers. * * @return \Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface[]|\Drupal\Core\Routing\Enhancer\RouteEnhancerInterface[] */ diff --git a/core/lib/Drupal/Core/Routing/LazyRouteFilter.php b/core/lib/Drupal/Core/Routing/LazyRouteFilter.php index 10f3d51..f6672ff 100644 --- a/core/lib/Drupal/Core/Routing/LazyRouteFilter.php +++ b/core/lib/Drupal/Core/Routing/LazyRouteFilter.php @@ -2,10 +2,8 @@ namespace Drupal\Core\Routing; +use Drupal\Core\DependencyInjection\ClassResolverInterface; use Symfony\Cmf\Component\Routing\NestedMatcher\RouteFilterInterface as BaseRouteFilterInterface; -use Symfony\Component\DependencyInjection\ContainerAwareInterface; -use Symfony\Component\DependencyInjection\ContainerAwareTrait; -use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\RouteCollection; @@ -16,13 +14,20 @@ * route filters are initialized on every request, which is slow. However, with * the use of lazy loading, dependencies are instantiated only when used. */ -class LazyRouteFilter implements BaseRouteFilterInterface, ContainerAwareInterface { +class LazyRouteFilter implements BaseRouteFilterInterface { - use ContainerAwareTrait; + /** + * The class resolver. + * + * @var \Drupal\Core\DependencyInjection\ClassResolverInterface + */ + protected $classResolver; /** * Array of route filter service IDs. * + * @see \Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass + * * @var array */ protected $serviceIds = []; @@ -30,22 +35,27 @@ class LazyRouteFilter implements BaseRouteFilterInterface, ContainerAwareInterfa /** * The initialized route filters. * + * Used only for ::setFilters(), to accelerate route rebuilding. + * * @var \Drupal\Core\Routing\RouteFilterInterface[] */ protected $filters = NULL; /** - * Constructs the LazyRouteEnhancer object. + * Constructs the LazyRouteFilter object. * + * @param \Drupal\Core\DependencyInjection\ClassResolverInterface $class_resolver + * The class resolver. * @param $service_ids * Array of route filter service IDs. */ - public function __construct($service_ids) { + public function __construct(ClassResolverInterface $class_resolver, $service_ids) { + $this->classResolver = $class_resolver; $this->serviceIds = $service_ids; } /** - * For each route, filter down the route collection. + * For each route, saves a list of applicable filters to the route. * * @param \Symfony\Component\Routing\RouteCollection $route_collection * A collection of routes to apply filter checks to. @@ -66,14 +76,14 @@ public function setFilters(RouteCollection $route_collection) { } /** - * For each route, gets a list of applicable enhancers to the route. + * Gets all lazy route filters. * * @return \Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface[]|\Drupal\Core\Routing\Enhancer\RouteEnhancerInterface[] */ protected function getFilters() { if (!isset($this->filters)) { foreach ($this->serviceIds as $service_id) { - $this->filters[$service_id] = $this->container->get($service_id); + $this->filters[$service_id] = $this->classResolver->getInstanceFromDefinition($service_id); } } return $this->filters; @@ -87,13 +97,13 @@ public function filter(RouteCollection $collection, Request $request) { foreach ($collection->all() as $route) { $filter_ids = array_merge($filter_ids, $route->getOption('_route_filters') ?: []); } - $filter_ids = array_unique($filter_ids); + // Keep only unique filter IDs, sorted by filter service priority. + $filter_ids = array_intersect($this->serviceIds, $filter_ids); if (isset($filter_ids)) { foreach ($filter_ids as $filter_id) { - if ($filter = $this->container->get($filter_id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) { - $collection = $filter->filter($collection, $request); - } + $filter = $this->classResolver->getInstanceFromDefinition($filter_id); + $collection = $filter->filter($collection, $request); } } return $collection; diff --git a/core/modules/dblog/src/Tests/Rest/DbLogResourceTest.php b/core/modules/dblog/src/Tests/Rest/DbLogResourceTest.php index 44dbe99..abf1030 100644 --- a/core/modules/dblog/src/Tests/Rest/DbLogResourceTest.php +++ b/core/modules/dblog/src/Tests/Rest/DbLogResourceTest.php @@ -41,7 +41,7 @@ public function testWatchdog() { $account = $this->drupalCreateUser(['restful get dblog']); $this->drupalLogin($account); - $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET.' . $this->defaultFormat, ['id' => $id, '_format' => $this->defaultFormat]), 'GET'); + $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET', ['id' => $id, '_format' => $this->defaultFormat]), 'GET'); $this->assertResponse(200); $this->assertHeader('content-type', $this->defaultMimeType); $log = Json::decode($response); @@ -50,13 +50,13 @@ public function testWatchdog() { $this->assertEqual($log['message'], 'Test message', 'Log message text is correct.'); // Request an unknown log entry. - $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET.' . $this->defaultFormat, ['id' => 9999, '_format' => $this->defaultFormat]), 'GET'); + $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET', ['id' => 9999, '_format' => $this->defaultFormat]), 'GET'); $this->assertResponse(404); $decoded = Json::decode($response); $this->assertEqual($decoded['message'], 'Log entry with ID 9999 was not found', 'Response message is correct.'); // Make a bad request (a true malformed request would never be a route match). - $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET.' . $this->defaultFormat, ['id' => 0, '_format' => $this->defaultFormat]), 'GET'); + $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET', ['id' => 0, '_format' => $this->defaultFormat]), 'GET'); $this->assertResponse(400); $decoded = Json::decode($response); $this->assertEqual($decoded['message'], 'No log entry ID was provided', 'Response message is correct.'); diff --git a/core/modules/rest/src/Plugin/ResourceBase.php b/core/modules/rest/src/Plugin/ResourceBase.php index b40a031..40c62bb 100644 --- a/core/modules/rest/src/Plugin/ResourceBase.php +++ b/core/modules/rest/src/Plugin/ResourceBase.php @@ -114,30 +114,16 @@ public function routes() { $methods = $this->availableMethods(); foreach ($methods as $method) { - $route = $this->getBaseRoute($canonical_path, $method); - - switch ($method) { - case 'POST': - $route->setPath($create_path); - $collection->add("$route_name.$method", $route); - break; - - case 'GET': - case 'HEAD': - // Restrict GET and HEAD requests to the media type specified in the - // HTTP Accept headers. - foreach ($this->serializerFormats as $format_name) { - // Expose one route per available format. - $format_route = clone $route; - $format_route->addRequirements(['_format' => $format_name]); - $collection->add("$route_name.$method.$format_name", $format_route); - } - break; - - default: - $collection->add("$route_name.$method", $route); - break; - } + $path = $method !== 'POST' + ? $canonical_path + : $create_path; + $route = $this->getBaseRoute($path, $method); + + // @todo _format and _content_type_format route requirements are added in + // ResourceRoutes::getRoutesForResourceConfig(), because their + // values depend on the RestResourceConfig entity, which + // @RestResource plugins cannot access. Fix this in Drupal 9. + $collection->add("$route_name.$method", $route); } return $collection; diff --git a/core/modules/rest/src/Routing/ResourceRoutes.php b/core/modules/rest/src/Routing/ResourceRoutes.php index 5ba4c5d..751ca76 100644 --- a/core/modules/rest/src/Routing/ResourceRoutes.php +++ b/core/modules/rest/src/Routing/ResourceRoutes.php @@ -108,20 +108,20 @@ protected function getRoutesForResourceConfig(RestResourceConfigInterface $rest_ continue; } - // If the route has a format requirement, then verify that the - // resource has it. - $format_requirement = $route->getRequirement('_format'); - if ($format_requirement && !in_array($format_requirement, $rest_resource_config->getFormats($method))) { - continue; - } - // The configuration has been validated, so we update the route to: + // - set the allowed response body content types/formats for methods + // that may send response bodies // - set the allowed request body content types/formats for methods that // allow request bodies to be sent // - set the allowed authentication providers + // @todo Remove this in Drupal 9. @RestResource plugins should be + // given the RESTResourceConfig entity, and should generate the + // appropriate routes in ::routes() based on that. But doing so + // would be a BC break. This is the next best thing we can do. + if (in_array($method, ['GET', 'HEAD', 'POST', 'PUT', 'PATCH'], TRUE)) { + $route->addRequirements(['_format' => implode('|', $rest_resource_config->getFormats($method))]); + } if (in_array($method, ['POST', 'PATCH', 'PUT'], TRUE)) { - // Restrict the incoming HTTP Content-type header to the allowed - // formats. $route->addRequirements(['_content_type_format' => implode('|', $rest_resource_config->getFormats($method))]); } $route->setOption('_auth', $rest_resource_config->getAuthenticationProviders($method)); diff --git a/core/modules/rest/src/Tests/ResourceTest.php b/core/modules/rest/src/Tests/ResourceTest.php index baf91e9..38e6330 100644 --- a/core/modules/rest/src/Tests/ResourceTest.php +++ b/core/modules/rest/src/Tests/ResourceTest.php @@ -3,6 +3,7 @@ namespace Drupal\rest\Tests; use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Url; use Drupal\rest\RestResourceConfigInterface; use Drupal\user\Entity\Role; use Drupal\user\RoleInterface; @@ -107,7 +108,7 @@ public function testSerializationClassIsOptional() { ->save(); $serialized = $this->container->get('serializer')->serialize(['foo', 'bar'], 'json'); - $this->httpRequest('serialization_test', 'POST', $serialized, 'application/json'); + $this->httpRequest(Url::fromRoute('rest.serialization_test.POST', ['_format' => 'json']), 'POST', $serialized, 'application/json'); $this->assertResponse(200); $this->assertResponseBody('["foo","bar"]'); } diff --git a/core/modules/rest/tests/fixtures/update/drupal-8.rest-rest_post_update_resource_granularity.php b/core/modules/rest/tests/fixtures/update/drupal-8.rest-rest_post_update_resource_granularity.php index aa94c63e555977ccc5ada2d04fd8632562e09a14..989f57927209b7c3bdc84818df5ee0f8ab177aef 100644 GIT binary patch delta 20 ccmX@F@lj*LH74ea#GJ{unB+HKW2zJc0APg(bN~PV delta 12 TcmeyUab9D?HKxr!m?{MUDqIE7 diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 277c470..c02b5d1 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -638,7 +638,7 @@ public function testGet() { $this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type')); - $url = Url::fromRoute('rest.entity.' . static::$entityTypeId . '.GET.' . static::$format); + $url = Url::fromRoute('rest.entity.' . static::$entityTypeId . '.GET'); $url->setRouteParameter(static::$entityTypeId, 987654321); $url->setOption('query', ['_format' => static::$format]); @@ -646,7 +646,7 @@ public function testGet() { // DX: 404 when GETting non-existing entity. $response = $this->request('GET', $url, $request_options); $path = str_replace('987654321', '{' . static::$entityTypeId . '}', $url->setAbsolute()->setOptions(['base_url' => '', 'query' => []])->toString()); - $message = 'The "' . static::$entityTypeId . '" parameter was not converted for the path "' . $path . '" (route name: "rest.entity.' . static::$entityTypeId . '.GET.' . static::$format . '")'; + $message = 'The "' . static::$entityTypeId . '" parameter was not converted for the path "' . $path . '" (route name: "rest.entity.' . static::$entityTypeId . '.GET")'; $this->assertResourceErrorResponse(404, $message, $response); } @@ -892,6 +892,7 @@ public function testPost() { if ($this->entity->getEntityType()->hasLinkTemplate('create')) { $this->entityStorage->load(static::$secondCreatedEntityId)->delete(); $old_url = Url::fromUri('base:entity/' . static::$entityTypeId); + $old_url->setOption('query', ['_format' => static::$format]); $response = $this->request('POST', $old_url, $request_options); $this->assertResourceResponse(201, FALSE, $response); } diff --git a/core/modules/user/src/Tests/RestRegisterUserTest.php b/core/modules/user/src/Tests/RestRegisterUserTest.php index 62e6bbc..3461a82 100644 --- a/core/modules/user/src/Tests/RestRegisterUserTest.php +++ b/core/modules/user/src/Tests/RestRegisterUserTest.php @@ -2,6 +2,7 @@ namespace Drupal\user\Tests; +use Drupal\Core\Url; use Drupal\rest\Tests\RESTTestBase; use Drupal\user\Entity\Role; use Drupal\user\RoleInterface; @@ -166,7 +167,7 @@ protected function registerUser($name, $include_password = TRUE) { */ protected function registerRequest($name, $include_password = TRUE) { $serialized = $this->createSerializedUser($name, $include_password); - $this->httpRequest('/user/register', 'POST', $serialized, 'application/hal+json'); + $this->httpRequest(Url::fromRoute('rest.user_registration.POST', ['_format' => 'hal_json']), 'POST', $serialized, 'application/hal+json'); } }