diff --git a/core/lib/Drupal/Core/ParamConverter/EntityConverter.php b/core/lib/Drupal/Core/ParamConverter/EntityConverter.php index f28906f..64a8b06 100644 --- a/core/lib/Drupal/Core/ParamConverter/EntityConverter.php +++ b/core/lib/Drupal/Core/ParamConverter/EntityConverter.php @@ -35,72 +35,37 @@ public function __construct(EntityManager $entityManager) { } /** - * Tries to upcast every variable to an entity type. + * Implements \Drupal\Core\ParamConverter\ParamConverterInterface::convert(). * - * If there is a type denoted in the route options it will try to upcast to - * it, if there is no definition in the options it will try to upcast to an - * entity type of that name. If the chosen enity type does not exists it will - * leave the variable untouched. - * If the entity type exist, but there is no entity with the given id it will - * convert the variable to NULL. - * - * Example: - * - * pattern: '/a/{user}/some/{foo}/and/{bar}/' - * options: - * converters: - * foo: 'node' - * - * The value for {user} will be converted to a user entity and the value - * for {foo} to a node entity, but it will not touch the value for {bar}. - * - * It will not process variables which are marked as converted. It will mark - * any variable it processes as converted. - * - * @param array &$variables - * Array of values to convert to their corresponding objects, if applicable. - * @param \Symfony\Component\Routing\Route $route - * The route object. - * @param array &$converted - * Array collecting the names of all variables which have been - * altered by a converter. + * For every parameter whose desired type is an entity type, convert it to + * the loaded entity. Leave other parameters untouched. */ - public function process(array &$variables, Route $route, array &$converted) { - $variable_names = $route->compile()->getVariables(); - - $options = $route->getOptions(); - $configuredTypes = isset($options['converters']) ? $options['converters'] : array(); - + public function convert($unconverted_parameters, $parameter_options, Route $route, array $defaults) { $entityTypes = array_keys($this->entityManager->getDefinitions()); - foreach ($variable_names as $name) { - // Do not process this variable if it's already marked as converted. - if (in_array($name, $converted)) { - continue; - } - - // Obtain entity type to convert to from the route configuration or just - // use the variable name as default. - if (array_key_exists($name, $configuredTypes)) { - $type = $configuredTypes[$name]; - } - else { - $type = $name; + $converted = array(); + foreach ($unconverted_parameters as $name => $value) { + if (isset($parameter_options[$name]['type'])) { + $type = $parameter_options[$name]['type']; + if (in_array($type, $entityTypes)) { + $converted[$name] = $this->loadEntity($type, $value); + } } + } - if (in_array($type, $entityTypes)) { - $value = $variables[$name]; - - $storageController = $this->entityManager->getStorageController($type); - $entities = $storageController->load(array($value)); - - // Make sure $entities is null, if upcasting fails. - $entity = $entities ? reset($entities) : NULL; - $variables[$name] = $entity; + return $converted; + } - // Mark this variable as converted. - $converted[] = $name; - } - } + /** + * Loads a single entity by id. + * + * @todo Remove when EntityManager provides a direct API for loading a single + * entity: http://drupal.org/node/1867228. + */ + protected function loadEntity($type, $id) { + $storageController = $this->entityManager->getStorageController($type); + $entities = $storageController->load(array($id)); + $entity = $entities ? reset($entities) : NULL; + return $entity; } } diff --git a/core/lib/Drupal/Core/ParamConverter/ParamConverterInterface.php b/core/lib/Drupal/Core/ParamConverter/ParamConverterInterface.php index 74307c2..747089f 100644 --- a/core/lib/Drupal/Core/ParamConverter/ParamConverterInterface.php +++ b/core/lib/Drupal/Core/ParamConverter/ParamConverterInterface.php @@ -17,13 +17,24 @@ /** * Allows to convert variables to their corresponding objects. * - * @param array &$variables + * @param array $unconverted_parameters * Array of values to convert to their corresponding objects, if applicable. + * This array excludes parameters that have already been converted by prior + * converters. + * @param array $parameter_options + * An array of options for each parameter, as specified by the route's + * 'parameters' option, and altered by the param converter manager. * @param \Symfony\Component\Routing\Route $route * The route object. - * @param array &$converted - * Array collecting the names of all variables which have been - * altered by a converter. + * @param array $defaults + * The complete array of parameters returned by + * \Symfony\Component\Routing\Matcher\RequestMatcherInterface::matchRequest() + * and processed by prior route enhancers and parameter converters. + * + * @return array + * An array of converted parameters. Omitted parameters will be treated as + * still unconverted and forwarded to the following converter. Parameters + * set to NULL will result in a 404 response. */ - public function process(array &$variables, Route $route, array &$converted); + public function convert($unconverted_parameters, $parameter_options, Route $route, array $defaults); } diff --git a/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php b/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php index be5676e..71c03ef 100644 --- a/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php +++ b/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php @@ -61,27 +61,29 @@ public function addConverter(ParamConverterInterface $converter) { * The modified defaults. */ public function enhance(array $defaults, Request $request) { - // This array will collect the names of all variables which have been - // altered by a converter. - // This serves two purposes: - // 1. It might prevent converters later in the pipeline to process - // a variable again. - // 2. To check if upcasting was successfull after each converter had - // a go. See below. - $converters = array(); - $route = $defaults[RouteObjectInterface::ROUTE_OBJECT]; + $options = $route->getOptions(); + $parameter_options = isset($options['parameters']) ? $options['parameters'] : array(); + + // For each parameter without an explicit type, default it to the + // parameter name. + foreach ($route->compile()->getVariables() as $parameter_name) { + $parameter_options[$parameter_name] += array('type' => $parameter_name); + } + // Invoke each registered converter. + $unconverted_parameters = $defaults; foreach ($this->converters as $converter) { - $converter->process($defaults, $route, $converters); + $converted_parameters = $converter->convert($unconverted_parameters, $parameter_options, $route, $defaults); + $defaults = $converted_parameters + $defaults; + $unconverted_parameters = array_diff_key($unconverted_parameters, $converted_parameters); } - // Check if all upcasting yielded a result. - // If an upcast value is NULL do a 404. - foreach ($converters as $variable) { - if ($defaults[$variable] === NULL) { - throw new NotFoundHttpException(); - } + // A parameter explicitly converted to NULL means the requested resource + // wasn't found. + $converted_parameters = array_diff_key($defaults, $unconverted_parameters); + if (in_array(NULL, $converted_parameters)) { + throw new NotFoundHttpException(); } return $defaults; diff --git a/core/modules/system/lib/Drupal/system/Tests/ParamConverter/UpcastingTest.php b/core/modules/system/lib/Drupal/system/Tests/ParamConverter/UpcastingTest.php index 14a0422..ca01ebc 100644 --- a/core/modules/system/lib/Drupal/system/Tests/ParamConverter/UpcastingTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/ParamConverter/UpcastingTest.php @@ -50,14 +50,12 @@ public function testUpcasting() { $this->assertRaw("user: {$user->label()}, node: {$node->label()}, foo: $foo", 'user and node upcast by entity name'); // paramconverter_test/test_node_user_user/{node}/{foo}/{user} - // converters: - // foo: 'user' + // options.parameters.foo.type = user $this->drupalGet("paramconverter_test/test_node_user_user/{$node->nid}/{$user->uid}/{$user->uid}"); $this->assertRaw("user: {$user->label()}, node: {$node->label()}, foo: {$user->label()}", 'foo converted to user as well'); // paramconverter_test/test_node_node_foo/{user}/{node}/{foo} - // converters: - // user: 'node' + // options.parameters.user.type = node $this->drupalGet("paramconverter_test/test_node_node_foo/{$node->nid}/{$node->nid}/$foo"); $this->assertRaw("user: {$node->label()}, node: {$node->label()}, foo: $foo", 'user is upcast to node (rather than to user)'); } @@ -69,8 +67,7 @@ public function testSameTypes() { $node = $this->drupalCreateNode(array('title' => $this->randomName(8))); $parent = $this->drupalCreateNode(array('title' => $this->randomName(8))); // paramconverter_test/node/{node}/set/parent/{parent} - // converters: - // parent: 'node' + // options.parameters.parent.type = node $this->drupalGet("paramconverter_test/node/" . $node->nid . "/set/parent/" . $parent->nid); $this->assertRaw("Setting '" . $parent->title . "' as parent of '" . $node->title . "'."); } diff --git a/core/modules/system/tests/modules/paramconverter_test/paramconverter_test.routing.yml b/core/modules/system/tests/modules/paramconverter_test/paramconverter_test.routing.yml index 9d226e4..7957ff3 100644 --- a/core/modules/system/tests/modules/paramconverter_test/paramconverter_test.routing.yml +++ b/core/modules/system/tests/modules/paramconverter_test/paramconverter_test.routing.yml @@ -12,8 +12,9 @@ paramconverter_test_node_user_user: requirements: _access: 'TRUE' options: - converters: - foo: 'user' + parameters: + foo: + type: 'user' paramconverter_test_node_node_foo: pattern: '/paramconverter_test/test_node_node_foo/{user}/{node}/{foo}' @@ -22,8 +23,9 @@ paramconverter_test_node_node_foo: requirements: _access: 'TRUE' options: - converters: - user: 'node' + parameters: + user: + type: 'node' paramconverter_test_node_set_parent: pattern: '/paramconverter_test/node/{node}/set/parent/{parent}' @@ -32,5 +34,6 @@ paramconverter_test_node_set_parent: defaults: _content: '\Drupal\paramconverter_test\TestControllers::testNodeSetParent' options: - converters: - parent: 'node' + parameters: + parent: + type: 'node' diff --git a/core/modules/views/views_ui/lib/Drupal/views_ui/ParamConverter/ViewUIConverter.php b/core/modules/views/views_ui/lib/Drupal/views_ui/ParamConverter/ViewUIConverter.php index 2a15977..a9222ec 100644 --- a/core/modules/views/views_ui/lib/Drupal/views_ui/ParamConverter/ViewUIConverter.php +++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ParamConverter/ViewUIConverter.php @@ -36,72 +36,43 @@ public function __construct(TempStoreFactory $temp_store_factory) { } /** - * Tries to upcast every view entity to a decorated ViewUI object. + * Implements \Drupal\Core\ParamConverter\ParamConverterInterface::convert(). * - * The key refers to the portion of the route that is a view entity that - * should be prepared for the Views UI. If there is a non-null value, it will - * be used as the collection of a temp store object used for loading. - * - * Example: - * - * pattern: '/some/{view}/and/{foo}/and/{bar}' - * options: - * converters: - * foo: 'view' - * tempstore: - * view: 'views' - * foo: NULL - * - * The values for {view} and {foo} will be converted to view entities prepared - * for the Views UI, with {view} being loaded from the views temp store, but - * it will not touch the value for {bar}. - * - * Note: This requires that the placeholder either be named {view}, or that a - * converter is specified as done above for {foo}. - * - * It will still process variables which are marked as converted. It will mark - * any variable it processes as converted. - * - * @param array &$variables - * Array of values to convert to their corresponding objects, if applicable. - * @param \Symfony\Component\Routing\Route $route - * The route object. - * @param array &$converted - * Array collecting the names of all variables which have been - * altered by a converter. + * For every route parameter that uses a 'tempstore' option and has already + * been upcast to a View, this converter upcasts it further to a decorated + * ViewUI object. */ - public function process(array &$variables, Route $route, array &$converted) { - // If nothing was specified to convert, return. - $options = $route->getOptions(); - if (!isset($options['tempstore'])) { - return; - } - - foreach ($options['tempstore'] as $name => $collection) { - // Only convert if the variable is a view. - if ($variables[$name] instanceof ViewStorageInterface) { + public function convert($unconverted_parameters, $parameter_options, Route $route, array $defaults) { + $converted = array(); + foreach ($parameter_options as $parameter_name => $options) { + // Many things, not just Views, can use the tempstore. Here, we are only + // interested in upcasting Views. We are relying on an earlier converter + // (e.g., Drupal\Core\ParamConverter\EntityConverter) to have converted + // the desired parameters to View entities, so we must grab it out of + // $defaults, rather than $unconverted_parameters. + if (isset($options['tempstore']) && ($defaults[$parameter_name] instanceof ViewStorageInterface)) { + $collection = $options['tempstore']; + $saved_view = $defaults[$parameter_name]; // Get the temp store for this variable if it needs one. // Attempt to load the view from the temp store, synchronize its // status with the existing view, and store the lock metadata. - if ($collection && ($temp_store = $this->tempStoreFactory->get($collection)) && ($view = $temp_store->get($variables[$name]->id()))) { - if ($variables[$name]->status()) { + if (($temp_store = $this->tempStoreFactory->get($collection)) && ($view = $temp_store->get($saved_view->id()))) { + if ($saved_view->status()) { $view->enable(); } else { $view->disable(); } - $view->locked = $temp_store->getMetadata($variables[$name]->id()); + $view->locked = $temp_store->getMetadata($saved_view->id()); } // Otherwise, decorate the existing view for use in the UI. else { - $view = new ViewUI($variables[$name]); + $view = new ViewUI($saved_view); } - - // Store the new view and mark this variable as converted. - $variables[$name] = $view; - $converted[] = $name; + $converted[$parameter_name] = $view; } } + return $converted; } } diff --git a/core/modules/views/views_ui/views_ui.routing.yml b/core/modules/views/views_ui/views_ui.routing.yml index 9c64891..d676d1e 100644 --- a/core/modules/views/views_ui/views_ui.routing.yml +++ b/core/modules/views/views_ui/views_ui.routing.yml @@ -72,8 +72,9 @@ views_ui.autocomplete: views_ui.edit: pattern: '/admin/structure/views/view/{view}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Routing\ViewsUIController::edit' requirements: @@ -82,8 +83,9 @@ views_ui.edit: views_ui.edit.display: pattern: '/admin/structure/views/view/{view}/edit/{display_id}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Routing\ViewsUIController::edit' display_id: NULL @@ -93,8 +95,9 @@ views_ui.edit.display: views_ui.preview: pattern: '/admin/structure/views/view/{view}/preview/{display_id}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Routing\ViewsUIController::preview' display_id: NULL @@ -112,8 +115,9 @@ views_ui.breakLock: views_ui.form.addItem: pattern: '/admin/structure/views/{js}/add-item/{view}/{display_id}/{type}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Form\Ajax\AddItem::getForm' requirements: @@ -123,8 +127,9 @@ views_ui.form.addItem: views_ui.form.editDetails: pattern: '/admin/structure/views/{js}/edit-details/{view}/{display_id}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Form\Ajax\EditDetails::getForm' requirements: @@ -134,8 +139,9 @@ views_ui.form.editDetails: views_ui.form.reorderDisplays: pattern: '/admin/structure/views/{js}/reorder-displays/{view}/{display_id}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Form\Ajax\ReorderDisplays::getForm' requirements: @@ -145,8 +151,9 @@ views_ui.form.reorderDisplays: views_ui.form.analyze: pattern: '/admin/structure/views/{js}/analyze/{view}/{display_id}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Form\Ajax\Analyze::getForm' requirements: @@ -156,8 +163,9 @@ views_ui.form.analyze: views_ui.form.rearrange: pattern: '/admin/structure/views/{js}/rearrange/{view}/{display_id}/{type}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Form\Ajax\Rearrange::getForm' requirements: @@ -167,8 +175,9 @@ views_ui.form.rearrange: views_ui.form.rearrangeFilter: pattern: '/admin/structure/views/{js}/rearrange-filter/{view}/{display_id}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Form\Ajax\RearrangeFilter::getForm' requirements: @@ -178,8 +187,9 @@ views_ui.form.rearrangeFilter: views_ui.form.display: pattern: '/admin/structure/views/{js}/display/{view}/{display_id}/{type}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Form\Ajax\Display::getForm' requirements: @@ -189,8 +199,9 @@ views_ui.form.display: views_ui.form.configItem: pattern: '/admin/structure/views/{js}/config-item/{view}/{display_id}/{type}/{id}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Form\Ajax\ConfigItem::getForm' requirements: @@ -200,8 +211,9 @@ views_ui.form.configItem: views_ui.form.configItemExtra: pattern: '/admin/structure/views/{js}/config-item-extra/{view}/{display_id}/{type}/{id}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Form\Ajax\ConfigItemExtra::getForm' requirements: @@ -211,8 +223,9 @@ views_ui.form.configItemExtra: views_ui.form.configItemGroup: pattern: '/admin/structure/views/{js}/config-item-group/{view}/{display_id}/{type}/{id}' options: - tempstore: - view: 'views' + parameters: + view: + tempstore: 'views' defaults: _controller: '\Drupal\views_ui\Form\Ajax\ConfigItemGroup::getForm' form_state: NULL