core/modules/rest/config/install/rest.settings.yml | 7 ++++++ core/modules/rest/config/schema/rest.schema.yml | 6 +++++ core/modules/rest/rest.install | 10 ++++++++ core/modules/rest/rest.services.yml | 7 +----- .../src/EventSubscriber/RestConfigSubscriber.php | 23 +++++++++++++++--- core/modules/rest/src/Plugin/ResourceBase.php | 20 +++++++++++++--- .../src/Plugin/rest/resource/EntityResource.php | 12 +--------- core/modules/rest/src/RestServiceProvider.php | 20 ++++++++++++---- .../EntityResource/EntityResourceTestBase.php | 27 +++++++++++++++++----- 9 files changed, 99 insertions(+), 33 deletions(-) diff --git a/core/modules/rest/config/install/rest.settings.yml b/core/modules/rest/config/install/rest.settings.yml index 8f70c69..f1f51c9 100644 --- a/core/modules/rest/config/install/rest.settings.yml +++ b/core/modules/rest/config/install/rest.settings.yml @@ -5,3 +5,10 @@ # @see rest_update_8203() # @see https://www.drupal.org/node/2664780 bc_entity_resource_permissions: false +bc: + # Before Drupal 8.5, a GET route would be created for each supported format. + # This was confusing and unnecessary. The old routes are still created for BC + # reasons. New Drupal installations opt out from this by default (hence this + # is set to false), existing installations opt in to it. + # @see rest_update_8501() + format_specific_get_routes: false diff --git a/core/modules/rest/config/schema/rest.schema.yml b/core/modules/rest/config/schema/rest.schema.yml index 98b35ae..60d915e 100644 --- a/core/modules/rest/config/schema/rest.schema.yml +++ b/core/modules/rest/config/schema/rest.schema.yml @@ -11,6 +11,12 @@ rest.settings: bc_entity_resource_permissions: type: boolean label: 'Whether the pre Drupal 8.2.x behavior of having permissions for EntityResource is enabled or not.' + bc: + type: mapping + mapping: + format_specific_get_routes: + type: boolean + label: 'Whether the pre-Drupal 8.5.x behavior of having format-specific GET routes for REST resources is enabled or not.' # Method-level granularity of REST resource configuration. rest_resource.method: diff --git a/core/modules/rest/rest.install b/core/modules/rest/rest.install index 2113b0b..b08d35b 100644 --- a/core/modules/rest/rest.install +++ b/core/modules/rest/rest.install @@ -120,3 +120,13 @@ function rest_update_8401() { } } } + +/** + * Enable BC for GET routes: continue to use format-specific GET routes. + */ +function rest_update_8501() { + $config_factory = \Drupal::configFactory(); + $rest_settings = $config_factory->getEditable('rest.settings'); + $rest_settings->set('bc.format_specific_get_routes', TRUE) + ->save(TRUE); +} diff --git a/core/modules/rest/rest.services.yml b/core/modules/rest/rest.services.yml index 511628e..2233b37 100644 --- a/core/modules/rest/rest.services.yml +++ b/core/modules/rest/rest.services.yml @@ -28,7 +28,7 @@ services: arguments: ['@serializer', '@renderer', '@current_route_match'] rest.config_subscriber: class: Drupal\rest\EventSubscriber\RestConfigSubscriber - arguments: ['@router.builder'] + arguments: ['@router.builder', '@kernel'] tags: - { name: event_subscriber } rest.resource.entity.post_route.subscriber: @@ -43,8 +43,3 @@ services: arguments: ['@entity_type.manager'] tags: - { name: path_processor_inbound } - rest.route_processor_get_bc: - class: \Drupal\rest\RouteProcessor\RestResourceGetRouteProcessorBC - arguments: ['%serializer.formats%', '@router.route_provider'] - tags: - - { name: route_processor_outbound } diff --git a/core/modules/rest/src/EventSubscriber/RestConfigSubscriber.php b/core/modules/rest/src/EventSubscriber/RestConfigSubscriber.php index d199545..8bbb2ff 100644 --- a/core/modules/rest/src/EventSubscriber/RestConfigSubscriber.php +++ b/core/modules/rest/src/EventSubscriber/RestConfigSubscriber.php @@ -4,6 +4,7 @@ use Drupal\Core\Config\ConfigCrudEvent; use Drupal\Core\Config\ConfigEvents; +use Drupal\Core\DrupalKernelInterface; use Drupal\Core\Routing\RouteBuilderInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -20,13 +21,23 @@ class RestConfigSubscriber implements EventSubscriberInterface { protected $routerBuilder; /** + * The Drupal Kernel. + * + * @var \Drupal\Core\DrupalKernelInterface + */ + protected $kernel; + + /** * Constructs the RestConfigSubscriber. * * @param \Drupal\Core\Routing\RouteBuilderInterface $router_builder * The router builder service. + * @param \Drupal\Core\DrupalKernelInterface $kernel + * The Drupal Kernel. */ - public function __construct(RouteBuilderInterface $router_builder) { + public function __construct(RouteBuilderInterface $router_builder, DrupalKernelInterface $kernel) { $this->routerBuilder = $router_builder; + $this->kernel = $kernel; } /** @@ -37,8 +48,14 @@ public function __construct(RouteBuilderInterface $router_builder) { */ public function onSave(ConfigCrudEvent $event) { $saved_config = $event->getConfig(); - if ($saved_config->getName() === 'rest.settings' && $event->isChanged('bc_entity_resource_permissions')) { - $this->routerBuilder->setRebuildNeeded(); + if ($saved_config->getName() === 'rest.settings') { + if ($event->isChanged('bc_entity_resource_permissions')) { + $this->routerBuilder->setRebuildNeeded(); + } + elseif ($event->isChanged('bc.format_specific_get_routes')) { + $this->kernel->invalidateContainer(); + $this->routerBuilder->setRebuildNeeded(); + } } } diff --git a/core/modules/rest/src/Plugin/ResourceBase.php b/core/modules/rest/src/Plugin/ResourceBase.php index a3ea800..c0ae970 100644 --- a/core/modules/rest/src/Plugin/ResourceBase.php +++ b/core/modules/rest/src/Plugin/ResourceBase.php @@ -2,6 +2,7 @@ namespace Drupal\rest\Plugin; +use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Plugin\PluginBase; use Psr\Log\LoggerInterface; @@ -41,6 +42,13 @@ protected $logger; /** + * The config factory. + * + * @var \Drupal\Core\Config\ConfigFactoryInterface + */ + protected $configFactory; + + /** * Constructs a Drupal\rest\Plugin\ResourceBase object. * * @param array $configuration @@ -53,11 +61,14 @@ * The available serialization formats. * @param \Psr\Log\LoggerInterface $logger * A logger instance. + * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory + * The config factory. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, array $serializer_formats, LoggerInterface $logger) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, array $serializer_formats, LoggerInterface $logger, ConfigFactoryInterface $config_factory) { parent::__construct($configuration, $plugin_id, $plugin_definition); $this->serializerFormats = $serializer_formats; $this->logger = $logger; + $this->configFactory = $config_factory; } /** @@ -69,7 +80,8 @@ public static function create(ContainerInterface $container, array $configuratio $plugin_id, $plugin_definition, $container->getParameter('serializer.formats'), - $container->get('logger.factory')->get('rest') + $container->get('logger.factory')->get('rest'), + $container->get('config.factory') ); } @@ -110,6 +122,8 @@ public function routes() { $create_path = $definition['uri_paths']['https://www.drupal.org/link-relations/create']; } + $rest_bc_settings = $this->configFactory->get('rest.settings')->get('bc'); + $route_name = strtr($this->pluginId, ':', '.'); $methods = $this->availableMethods(); @@ -128,7 +142,7 @@ public function routes() { // route definitions that are as empty as possible, plus an outbound route // processor. // @see \Drupal\rest\RouteProcessor\RestResourceGetRouteProcessorBC - if ($method === 'GET' || $method === 'HEAD') { + if ($rest_bc_settings['format_specific_get_routes'] && ($method === 'GET' || $method === 'HEAD')) { foreach ($this->serializerFormats as $format_name) { $collection->add("$route_name.$method.$format_name", (new Route(''))->setOption('bc_route', TRUE)); } diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 5d9849d..1d26daa 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -52,13 +52,6 @@ class EntityResource extends ResourceBase implements DependentPluginInterface { protected $entityType; /** - * The config factory. - * - * @var \Drupal\Core\Config\ConfigFactoryInterface - */ - protected $configFactory; - - /** * The link relation type manager used to create HTTP header links. * * @var \Drupal\Component\Plugin\PluginManagerInterface @@ -80,15 +73,12 @@ class EntityResource extends ResourceBase implements DependentPluginInterface { * The available serialization formats. * @param \Psr\Log\LoggerInterface $logger * A logger instance. - * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory - * The config factory. * @param \Drupal\Component\Plugin\PluginManagerInterface $link_relation_type_manager * The link relation type manager. */ public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, $serializer_formats, LoggerInterface $logger, ConfigFactoryInterface $config_factory, PluginManagerInterface $link_relation_type_manager) { - parent::__construct($configuration, $plugin_id, $plugin_definition, $serializer_formats, $logger); + parent::__construct($configuration, $plugin_id, $plugin_definition, $serializer_formats, $logger, $config_factory); $this->entityType = $entity_type_manager->getDefinition($plugin_definition['entity_type']); - $this->configFactory = $config_factory; $this->linkRelationTypeManager = $link_relation_type_manager; } diff --git a/core/modules/rest/src/RestServiceProvider.php b/core/modules/rest/src/RestServiceProvider.php index e705de4..a3997f6 100644 --- a/core/modules/rest/src/RestServiceProvider.php +++ b/core/modules/rest/src/RestServiceProvider.php @@ -2,20 +2,19 @@ namespace Drupal\rest; +use Drupal\Core\Config\BootstrapConfigStorageFactory; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\DependencyInjection\ServiceProviderInterface; use Drupal\rest\LinkManager\LinkManager; use Drupal\rest\LinkManager\RelationLinkManager; use Drupal\rest\LinkManager\TypeLinkManager; +use Drupal\rest\RouteProcessor\RestResourceGetRouteProcessorBC; use Symfony\Component\DependencyInjection\DefinitionDecorator; +use Symfony\Component\DependencyInjection\Parameter; use Symfony\Component\DependencyInjection\Reference; /** * Provides BC services. - * - * These services are not added via rest.services.yml because the service - * classes extend classes from the HAL module. They also have no use without - * that module. */ class RestServiceProvider implements ServiceProviderInterface { @@ -24,6 +23,9 @@ class RestServiceProvider implements ServiceProviderInterface { */ public function register(ContainerBuilder $container) { $modules = $container->getParameter(('container.modules')); + + // BC: services moved to HAL module. Not added in rest.services.yml because + // the services reference services in the HAL module. if (isset($modules['hal'])) { // @deprecated in Drupal 8.3.x and will be removed before Drupal 9.0.0. // Use hal.link_manager instead. @@ -46,6 +48,16 @@ public function register(ContainerBuilder $container) { $service_definition->setClass(RelationLinkManager::class); $container->setDefinition('rest.link_manager.relation', $service_definition); } + + // BC: outbound route processor for format-specific GET routes. Not added + // in rest.services.yml because its presence depends on configuration. + $settings = BootstrapConfigStorageFactory::get()->read('rest.settings'); + if ($settings['bc']['format_specific_get_routes']) { + $container->register('rest.route_processor_get_bc', RestResourceGetRouteProcessorBC::class) + ->addArgument(new Parameter('serializer.formats')) + ->addArgument(new Reference('router.route_provider')) + ->addTag('route_processor_outbound'); + } } } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 2d6e76b..47e0835 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -15,6 +15,7 @@ use Drupal\Tests\rest\Functional\ResourceTestBase; use GuzzleHttp\RequestOptions; use Psr\Http\Message\ResponseInterface; +use Symfony\Component\Routing\Exception\RouteNotFoundException; /** * Even though there is the generic EntityResource, it's necessary for every @@ -637,18 +638,32 @@ public function testGet() { $url->setRouteParameter(static::$entityTypeId, 987654321); $url->setOption('query', ['_format' => static::$format]); - // BC: ensure old per-format GET routes continue to exist, to ensure any - // code creating links to them continues to work. - $bc_url = Url::fromRoute('rest.entity.' . static::$entityTypeId . '.GET.' . static::$format); - $bc_url->setRouteParameter(static::$entityTypeId, 987654321); - $bc_url->setOption('query', ['_format' => static::$format]); - $this->assertSame($url->toString(TRUE)->getGeneratedUrl(), $bc_url->toString(TRUE)->getGeneratedUrl()); // 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")'; $this->assertResourceErrorResponse(404, $message, $response); + + + // BC: rest_update_8501(). + // Format-specific GET routes are deprecated. On new sites they no longer + // exist. On existing sites, they continue to be available, to ensure code + // creating links to them continues to work. + $bc_route = Url::fromRoute('rest.entity.' . static::$entityTypeId . '.GET.' . static::$format, $url->getRouteParameters(), $url->getOptions()); + try { + $bc_route->toString(TRUE); + $this->assertTrue(FALSE); + } + catch (RouteNotFoundException $e) { + $this->assertTrue(TRUE); + } + // Enable BC layer. + $this->config('rest.settings')->set('bc.format_specific_get_routes', TRUE)->save(TRUE); + $this->rebuildContainer(); + $this->resetAll(); + $bc_route->setUrlGenerator($this->container->get('url_generator')); + $this->assertSame($url->toString(TRUE)->getGeneratedUrl(), $bc_route->toString(TRUE)->getGeneratedUrl()); } /**