Switch back to URI templates in entity annotations

No. As discussed, we *cannot* use routes in the annotation. That forces people to write a manual static route for text/html, which we don't want. But we can build the text/html route off of the path pattern the same way we do for rest.module.

There's no reason I can think of that we cannot put such links into the HAL representation. A well-behaved HAL client will just ignore the ones it doesn't care about. (And a non-well-behaved HAL client is one we don't care about.)

#2019123: Use the same canonical URI paths as for HTML routes is blocked by this issue.

First attempt at a rollback. This will fail on the content_translation module stuff, because I'm not sure yet what to revert and what to keep there.

We shouldn't completely revert the content_translation module stuff, that was a good clean-up enabled by the patch switching this to routes.

Aw, only 2 fails short of being an awesome patch. :-)

Discussed with @catch. This issue needs to be resolved one way or the other before beta. (That's not an approval for the API change though; catch expressed that he's not actually sure about this change.) So it's a beta blocker -- whether it's fix or wontfix. :)

Another reroll, just fixed the merge conflicts. This will still have a lot of test fails.

Fixed the PHPUnit tests as a first step, I lost a hunk of a previous patch and ContentTranslationLocalTasks.php now actually makes use of the route provider.

Fixed 3 things, let's see how many other fails this clears up.

Failed bot, so needs work.

Resolving more test fails by using "/" as link prefix.

I'm also introducing route lookups by path patterns here for local tasks of field UI and content translation UI.

1. +++ b/core/modules/content_translation/tests/Drupal/content_translation/Tests/Menu/ContentTranslationLocalTasksTest.php
   @@ -36,8 +36,10 @@ public function setUp() {
   -        array('canonical', 'node.view'),
   -        array('drupal:content-translation-overview', 'content_translation.translation_overview_node'),
   +        array('canonical', '/node/{node}'),
   +        // @todo: this will obviously fail, what is the translation overview
   +        // path for nodes?
   +        array('drupal:content-translation-overview', '/i/have/no/idea'),
   

   This is why we don't want to spread these paths around. There has been a momentous effort to use route names so things are safely alterable. This highlights why this patch is a step backwards.

2. +++ b/core/modules/field_ui/lib/Drupal/field_ui/Plugin/Derivative/FieldUiLocalTask.php
   @@ -176,19 +176,24 @@ public function getDerivativeDefinitions(array $base_plugin_definition) {
   +        $routes = $this->routeProvider->getRoutesByPattern($entity_info->getLinkTemplate('admin-form'))->all();
   ...
        foreach ($this->entityManager->getDefinitions() as $entity_type => $entity_info) {
          if ($entity_info->isFieldable() && $entity_info->hasLinkTemplate('admin-form')) {
   

   So now instead of storing the canonical route name that we use everywhere else, we get an extra DB query!

3. +++ b/core/modules/views/lib/Drupal/views/Entity/View.php
   @@ -32,6 +32,9 @@
   + *   links = {
   + *     "edit-form" = "/admin/structure/views/view/{view}"
     *   }
   

   Nope, you cannot do this. This is the view module, it has no idea the Views UI exists.

4. +++ b/core/modules/user/lib/Drupal/user/Entity/User.php
   @@ -42,9 +42,9 @@
   - *     "canonical" = "user.view",
   - *     "edit-form" = "user.edit",
   - *     "admin-form" = "user.account_settings"
   + *     "canonical" = "/user/{user}",
   + *     "edit-form" = "/user/{user}/edit",
   + *     "admin-form" = "/admin/config/people/accounts"
   

   So the gain here is that we get to hardcode this in 2 places instead of 1?

Tried to lay out pros and cons of sticking with route names vs. URI templates in the links annotation. Please edit to make it more clear!

I'm not convinced either way, @Crell and @timplunkett could you improve/correct the issue summary with things I forgot?

Interestingly the patch now passes, did some minor fixes.

Fixed the new SearchPage.php.

Just a reroll.

At the moment, I'm still in favor of option 1 from the issue summary (to continue with this issue, rather than "won't fix"ing it), because I think the concerns raised against this are a function of the patch being incomplete. For example,

The canonical place of URI paths would then be in the entity annotation (and only there!). That would also mean that routes would not be statically defined in some entity-type.routing.yml file, but dynamically with a route subscriber that reads the path from the entity link annotation.

The patch here so far doesn't do that.

Local tasks generators such as ContentTranslationLocalTasks.php deal with route names, so they have to issue $this->routeProvider->getRoutesByPattern($entity_info->getLinkTemplate('canonical')) calls to "guess" the route that is associated with the URI pattern. What if there is more than one route? Which is the right one?

They shouldn't need to guess. Once the routes are generated by a RouteSubscriber, ContentTranslationLocalTasks can "know" what that naming pattern is, and target the route by its name.

Of course, it's possible I'm wrong and the folks arguing against this are right, and that even once this patch is complete, it will turn out to cause more problems than it solves; I'm just expressing my current opinion.

Rerolled. #27 Patch did no longer apply.

EntityManager::getAdminPath() got replaced by EntityManager::getAdminRouteInfo() in #2141329: Replace getAdminPath() with getAdminRouteInfo() in Field UI.
The changes in EntityManager::getAdminPath() in #27 are not included in #30. This may have caused the failing test.

Originally #2019123: Use the same canonical URI paths as for HTML routes was blocked on this, but moved forward and is now RTBC.

The second proposal in the OP says Entity::uri() would need to be rewritten to query the routing system to build the path.

That was done in #2167641: EntityInterface::uri() should use route name and not path.

With both of those in, I think we can "won't fix" this.

OK, thank you for bearing with me. I've finally gotten some time to put together my thoughts here. I still believe that this issue should move forward, for many reasons. I am going to break them into two main arguments.

First, DX:

I've been working on a module recently for myself that, among other things, defines a custom entity type. So far so good, and I'm able to mostly learn by doing. However, getting to the annotation, I found that I had to then break out and go make routes, and learn what magic values to put on routes for entities. My form and storage controllers right now are empty, so I could just use the default/base classes if I wanted to. That is, I could almost do the entire entity in just one file! (OK, 2; a class and an interface.) That's a huge DX improvement over Drupal 7, and props to the Entity team for coming this far. (There's still some issues with it, but they're not germaine at the moment.)

Of course, that is just for defining the entity. For actually using it I need pages. Or rather, I need routes. (It's mostly for non-rest.module RESTful purposes.) But I am going to need some basic pages to put new, edit, delete, etc. pages on. Specifying the form controller implies, to me, that I now have a form. So... where does it go? Wait, why do I have to specify a form controller AND then go wire up a form elsewhere? That feels odd. I don't yet see the value, especially when in the typical case I won't want to do anything other than the standard entity form. If Drupal can build all of that for me (which I'm sure it can), why shouldn't it? I want it to. I want to just say "put the admin section here, put the entities themselves here, and build up the normal stuff around 'em", unless I specifically say I want to override "the normal stuff". (I should be able to, but I shouldn't have to.)

Second, RESTful philosophy:

A major goal for Drupal 8, and WSCCI in particular, has been to make Drupal primarily a REST server; just sometimes those REST responses are HTML. We've come a very very long way in that regard, but this issue means we're not quite there yet.

To be clear: REST resources do not conceptually map 1:1 to domain objects/entities. They are different things, but can inform each other. In Drupal's case, we've made the decision that all entities should be resource-able (that's what rest.module does), although plenty of other resources can exist. (Views can generate them, you can do them manually, etc.)

That is, consistent with that approach, the HTML version of an entity is simply its abstracted form rendered to HTML, as opposed to JSON, XML, SVG, PDF, or whatever. From a REST perspective, a given resource (entity represented as a resource) lives at a canonical URI. It can be rendered into various formats based on the Accept header, but it's still the "same" resource. And while we will usually want an HTML representation, that's not always the case.

In HEAD right now, we are privileging HTML representations of Entities. That's directly contradictory to the design goal of "it's all just REST, HTML is not special." We're saying that the way you get a non-HTML route is to define an HTML route first, then mutate it. That's not what WSCCI has been trying to accomplish for the last 3 years.

Rather, we should define:

• This is an entity
• Its canonical URI relationship is at /foo/bar/{id} (which is an IETF standard way to define link patterns)
• Its other relevant link relationships are to /blah, /baz/{id}, etc.
• Drupal, please apply magic options A, B, and D.
• Now override magic B this way.

Those magic options largely boil down to creating routes (and possibly database tables). It should be really easy, but not required, to tell an entity that it should have an HTML route. That's important; Dave Reid, I believe, has noted that he will want/need entities that have no meaningful HTML page. Commerce will, likely, have multiple entities that have no meaningful HTML representation, but we may still want non-HTML representations. Eg, if I want to create Commerce products from a 3rd party API (completely legit thing to do) but not give them their own HTML pages.

The exact mechanism for "now please apply magic options A, B, and D" I'm flexible on. Probably something else in the annotation. Some we can likely just assume; if an entity defines an edit-form link relationship, it's a pretty safe bet that it wants an HTML form put there (since forms are an HTML thing). But we SHOULD NOT assume that the existence of an entity means the existence of an HTML page.

The original argument for switching to routes in the annotation was "WAT? Why do I have to define the path on the entity and in the route? That's stupid!" I agree entirely. However, the answer isn't to tightly couple Entities to hand-crafted HTML routes. It's to, in the typical case, create Entity HTML routes dynamically just as we do for non-HTML routes, while allowing for some override.

My knee-jerk thought for that, actually, is to generate standard routes in the form entity_name.relationship (if the magic is triggered as above), or possibly entity_name.relationship.html (debatable). If you want to override the setup of a particular entity's route (as, for instance, node module does currently) just define that particular route yourself and it will be skipped. Note that would imply route names change to node.canonical, node.edit_form, etc. (Unless we alias a few?) Which, when I think about it, is really not such a bad thing, particularly as it standardizes the names and encourages people to think in terms of resources and relationships: Which is appropriate for a REST framework that happens to serve a lot of HTML, which is what Drupal 8 is intended to be.

That also opens the door for a ton of useful and poweful automation around #2113345: Define a mechanism for custom link relationships. And from a DX perspective, it means that as a module developer I can spin up a new entity, complete with standard paths and standard CRUD behavior, with one interface and one class (and a few annotation toggles) and that's the end of it. That's a pretty epic level of power and we should make sure we fully achieve that.

We have the Node entity, with an edit link template of '/node/{node}/edit'
We have the View entity, with an edit link template of '/admin/structure/views/view/{view}'

Apparently @Crell has a magical way to take that and generate routes for it.

I'd like to see code that takes that and turns it into this:

node.page_edit:
  path: '/node/{node}/edit'
  defaults:
    _entity_form: 'node.edit'
  requirements:
    _entity_access: 'node.update'

views_ui.edit:
  path: '/admin/structure/views/view/{view}'
  options:
    parameters:
      view:
        tempstore: TRUE
  defaults:
    _content: '\Drupal\views_ui\Controller\ViewsUIController::edit'
  requirements:
    _permission: 'administer views'

Until that code materializes, I don't see how we can proceed here.

 requirements:
    _permission: 'administer views'

If we're going to even attempt automation, this would need to be:

requirements:
    _entity_access: 'view.update'

That's not necessarily a bad thing, but tying the access to the form like that isn't something we've remotely done like that before - we add access to routes rather than forms.

Tim: Your first request is quite easy:

/**
 * @Entity = {
 *   links = {
 *     'edit-form' = '/node/{node}/edit'
 *   }
 * }
 */
class Node {

}

/**
 * @Entity = {
 *   links = {
 *     'edit-form' = '/admin/structure/views/view/{view}'
 *   }
 * }
 */
class View {

}

For the second, I specifically noted that we can and should allow manual definition of routes if you want to go off the common path. Node would right now, and Views may. I'm impartial on the access syntax we end up using. My point is that we can auto-handle the 80% case, and thereby most people won't have to double-enter paths. For that other 20% (it may be less, actually), we can have a clean way to specify manually instead. For most entities (or at least content entities), though, it provides a very clean, consistent, extensible way for people to get a lot of power (including HATEOAS link relationships) at very little cost, then tweak as needed, if needed.

Here's a very rough start at prototyping what the magic for #35/#36 might look like.

Since #2019123: Use the same canonical URI paths as for HTML routes got fixed, this issue can wait until after beta. It's less important compared to other beta-blocking API changes in the Entity API, and while architecturally nice, will not significantly impact DX or contributed modules. I'm demoting this to a major task that is a beta target.

Dries: Per #34, this has a direct impact on DX and would be a non-trivial API change for any module that defines an entity.

Per #35, the claims of #34 are not shared by everyone.

Basically, we should see how #2089757: Auto generate routing entries for entities based on an 'admin_path' and 'admin_permission' annotation goes before even considering anything further here.

This was discussed in Amsterdam. Per catch we're closing this issue as a dupe of #2259445: Entity Resource unification and will continue with the implementation plan there.