Entity::urlInfo() should return \Drupal\Core\Url

This makes the login work for me...

OH and the *** tag.

1. +++ b/core/lib/Drupal/Core/Entity/Entity.php
   @@ -399,27 +399,4 @@ protected function onUpdateBundleEntity() {
    
   -  /**
   -   * Wraps the URL generator.
   -   *
   -   * @return \Drupal\Core\Routing\UrlGeneratorInterface
   -   *   The URL generator.
   -   */
   -  protected function urlGenerator() {
   -    if (!$this->urlGenerator) {
   -      $this->urlGenerator = \Drupal::urlGenerator();
   -    }
   -    return $this->urlGenerator;
   -  }
   -
   

   This is awesome!

2. +++ b/core/lib/Drupal/Core/Entity/EntityFormController.php
   @@ -315,7 +315,9 @@ public function delete(array $form, array &$form_state) {
   +        $redirect_query = $form_state['redirect_route']->getOption('query') ?: array();
   

   Are we sue that 'redirect_route' is available all the time?

3. +++ b/core/lib/Drupal/Core/Entity/EntityInterface.php
   @@ -118,10 +118,7 @@ public function label();
      public function urlInfo($rel = 'canonical');
   

   Are we sure $form_state['redirect_route'] always exists?

+    $uri = $this->urlInfo($rel);
+    $options += $uri->getOptions();
+    $uri->setOptions($options);

I'm not a huge fan of fetching/changing/putting. I prefer methods on the object.
For example:

+    $uri = $this->urlInfo($rel);
+    $uri->mergeOptions($options);

Or does that mess with the options because it gets added in the other direction?

.

I wanted to RTBC this... I know this is a bit nitpicky, but...

+++ b/core/lib/Drupal/Core/Entity/Entity.php
@@ -174,14 +168,18 @@ public function urlInfo($rel = 'canonical') {
+        throw new \UnexpectedValueException(String::format('No link template "@rel" found for the "@entity_type" entity type', array(

Exception thrown if a value does not match with a set of values. Typically this happens when a function calls another function and expects the return value to be of a certain type or value (not including arithmetic or buffer related errors).

In other words, UnexpectedValueException is supposed to be used within a chain of function calls when an argument (or return value) suddenly is not of an expected data type.

For example, DirectoryIterator::__construct():

throws a UnexpectedValueException if the path cannot be opened.

I first wanted to suggest InvalidArgumentException, but that's not completely right either — valid arguments depend on the particular domain object for which URL info is retrieved.

Based on that, DomainException apparently comes closest:

Exception thrown if a value does not adhere to a defined valid data domain.

The example in the user comment essentially is a simplified code logic example of what we're doing here?

RTBC on the assumption that testbot agrees. :)

Overall this looks fine to me, apart from:

+++ b/core/modules/comment/lib/Drupal/comment/CommentBreadcrumbBuilder.php
@@ -53,8 +53,7 @@ public function build(array $attributes) {
-    $breadcrumb[] = \Drupal::l($entity->label(), $uri['route_name'], $uri['route_parameters'], $uri['options']);
+    $breadcrumb[] = \Drupal::linkGenerator()->generateFromUrl($entity->label(), $entity->urlInfo());

:( Going from \Drupal::l() to \Drupal::linkGenerator()->generateFromUrl() is not nice DX. :(

But Tim pointed out that this code is now doing something different from what l() does internally, although we could open an issue to either switch l()'s behaviour wholesale from ->generate() to ->generateFromUrl() or introduce some new helper function (like Drupal::lurl()...lol :P)

I asked Tim about the change notice, since there's no draft here. He said that the draft notice in https://drupal.org/node/2221879 needs a bit of love, and also needs updating based on this change, so he plans to do both post-commit.

Ok, then. Committed and pushed to 8.x. Thanks!

Published the change notice. Removed 'Needs change record' tag.