Split up ParamConverterManager and stop throwing NotFoundHttpException

Looks like a very nice improvement.

I am a bit confused ... the main point of the param converter system is to upcast stuff, which is done in the enhance bit.
I would have argued that setRouteParameterConverters does not really belong on there, because this is the part also couples to the routing system.

In general can we agree that pure internal rewrites should not be "major"?

I really like this patch now!

1. +++ b/core/lib/Drupal/Core/EventSubscriber/RouterListener.php
   @@ -0,0 +1,34 @@
   +    }
   ...
   +  public function onKernelRequest(GetResponseEvent $event) {
   +    // Catch failed parameter conversions and throw a 404 instead. This cannot
   +    // be done on the router level as it does not always deal with HTTP.
   +    try {
   +      parent::onKernelRequest($event);
   +    }
   +    catch (ParamNotConvertedException $e) {
   +      throw new NotFoundHttpException('', $e);
   

   So does that mean that our router does not support something else than HTTP?

2. +++ b/core/modules/edit/lib/Drupal/edit/Access/EditEntityAccessCheck.php
   @@ -65,14 +67,16 @@ protected function validateAndUpcastRequestAttributes(Request $request) {
      }
   
   +++ b/core/modules/config_translation/lib/Drupal/config_translation/Controller/ConfigTranslationController.php
   --- a/core/modules/edit/lib/Drupal/edit/Access/EditEntityAccessCheck.php
   +++ b/core/modules/edit/lib/Drupal/edit/Access/EditEntityAccessCheck.php
   
   +++ b/core/modules/edit/lib/Drupal/edit/Access/EditEntityAccessCheck.php
   +++ b/core/modules/edit/lib/Drupal/edit/Access/EditEntityAccessCheck.php
   @@ -44,7 +44,9 @@ public function access(Route $route, Request $request, AccountInterface $account
   
   @@ -44,7 +44,9 @@ public function access(Route $route, Request $request, AccountInterface $account
        // @todo Request argument validation and object loading should happen
        //   elsewhere in the request processing pipeline:
        //   http://drupal.org/node/1798214.
   -    $this->validateAndUpcastRequestAttributes($request);
   +    if (!$this->validateAndUpcastRequestAttributes($request)) {
   +      return static::KILL;
   +    }
    
        return $this->accessEditEntity($request->attributes->get('entity'), $account)  ? static::ALLOW : static::DENY;
      }
   @@ -65,14 +67,16 @@ protected function validateAndUpcastRequestAttributes(Request $request) {
   
   @@ -65,14 +67,16 @@ protected function validateAndUpcastRequestAttributes(Request $request) {
          $entity_id = $entity;
          $entity_type = $request->attributes->get('entity_type');
          if (!$entity_type || !$this->entityManager->getDefinition($entity_type)) {
   -        throw new NotFoundHttpException();
   +        return FALSE;
          }
          $entity = $this->entityManager->getStorageController($entity_type)->load($entity_id);
          if (!$entity) {
   -        throw new NotFoundHttpException();
   +        return FALSE;
          }
          $request->attributes->set('entity', $entity);
        }
   

   This is certainly a progress!

1. +++ b/core/lib/Drupal/Core/Access/AccessManager.php
   @@ -91,10 +91,10 @@ class AccessManager extends ContainerAware {
   @@ -202,14 +202,14 @@ public function checkNamedRoute($route_name, array $parameters = array(), Accoun
   
   +++ b/core/lib/Drupal/Core/EventSubscriber/RouterListener.php
   @@ -0,0 +1,34 @@
   +class RouterListener extends SymfonyRouterListener {
   ...
   +
   +/**
   + * Wraps the upstream request subscriber.
   

   It feels wrong that out RouterListener is aware of the ParamConverter, which is a subsystem that extending the router horizontally. Usually the param converter manager throws its exception because it is/was a route enhancer. Can't we achieve that by not hardcoding that into our RouterListener?

2. +++ b/core/lib/Drupal/Core/Routing/Enhancer/ParamConversionEnhancer.php
   @@ -0,0 +1,62 @@
   +  }
   ...
   +
   +  /**
   +   * @var \Drupal\Core\ParamConverter\ParamConverterManagerInterface
   +   */
   +  protected $paramConverterManager;
   +
   +  /**
   +   * @param \Drupal\Core\ParamConverter\ParamConverterManagerInterface $param_converter_manager
   +   */
   +  public function __construct(ParamConverterManagerInterface $param_converter_manager) {
   +    $this->paramConverterManager = $param_converter_manager;
   ...
   +   * @param array $defaults
   +   *
   +   * @return \Symfony\Component\HttpFoundation\ParameterBag
   +   */
   

   A little bit more docs would be nice.

1. +++ b/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php
   @@ -138,7 +138,7 @@ public function convert(array $defaults, Request $request) {
   -        throw new ParamNotConvertedException(sprintf('The "%s" parameter was not converted for the path "%s"', $name, $route->getPath()));
   +        throw new ParamNotConvertedException(sprintf('The "%s" parameter was not converted for the path "%s" (route name: "%s")', $name, $route->getPath(), $defaults[RouteObjectInterface::ROUTE_NAME]));
   

   <3

2. +++ b/core/lib/Drupal/Core/Routing/Enhancer/ParamConversionEnhancer.php
   @@ -59,4 +70,24 @@ protected function copyRawVariables(array $defaults) {
   +    $events[KernelEvents::EXCEPTION][] = array('onException', 0);
   

   You don't really need to specify priority 0, but this is not commit-blocking at all.

The listener is way better than extending the class. Great work!

I read the draft change record, edited it a tiny bit. Made sense to me.

Committed and pushed to 8.x. Thanks!

shouldnt the change record be published now? or there are other issues required for that?