This is needed for configuration translation module. We create a bunch of config_translation.item.* routes and then need to fetch those to create local tasks from them.

Comments

tstoeckler’s picture

Status: Active » Needs review
StatusFileSize
new3.37 KB

Here we go.

Status: Needs review » Needs work

The last submitted patch, 2113687-1.patch, failed testing.

tstoeckler’s picture

Status: Needs work » Needs review
StatusFileSize
new856 bytes
new4.21 KB

Forgot MockRouteProvider

Status: Needs review » Needs work

The last submitted patch, 2113687-3.patch, failed testing.

tstoeckler’s picture

Status: Needs work » Needs review
StatusFileSize
new744 bytes
new4.2 KB

That's quite stupid that I copy-pasted this code from config_translation without actually looking at it. Tests++

Status: Needs review » Needs work

The last submitted patch, 2113687-5.patch, failed testing.

tstoeckler’s picture

Status: Needs work » Needs review
StatusFileSize
new2.6 KB
new4.5 KB

So MySQL LIKE works differently than I thought. :-)
Also instead of asserting that the entire objects are equal, let's just assert that the correct keys are there.
For anyone interested: The second fail above was due to a different compiler class being used, due to Drupal's MatcherDumper.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

It is odd that we need such kind of system but well, if there is no way around that, there is no way around that.

tstoeckler’s picture

Well, actually I think the use-case of providing local tasks / actions / contextual links for dynamically defined routes is actually not that rare, so even without config_translation I think this would make a lot of sense. The alternative would be e.g. for config_translation to store the names of the translated routes in state() or something, but I would really like to avoid stuff like that.

dawehner’s picture

SO this is basically a different approach to the collection to list all routes?

tstoeckler’s picture

??

Do you mean #2098197: Add getAllRoutes() method to RouteProvider ? That was a completely different use-case in config_translation that we're now trying to avoid completely.

Sorry, I don't think I understand what you mean.

catch’s picture

We create a bunch of config_translation.item.* routes and then need to fetch those to create local tasks from them.

Is that during the same request or a different request?

Crell’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +WSCCI

Wait, wait, what? Route names are supposed to be opaque strings. They're not queryable by design. What's the use case here? This, like many such issues recently, strikes me as a completely wrong approach.

tstoeckler’s picture

Re #12: Quite honestly, I don't know. I *think* it's the same request, as AFAIK route building and local task building both only happens on cache clear/ module install /... requests.

Re #13: I've explained the use-case above. Can you be more specific about what is not clear?

dawehner’s picture

Re #12: Quite honestly, I don't know. I *think* it's the same request, as AFAIK route building and local task building both only happens on cache clear/ module install /... requests.

It depends ... if you clear the cache using drush and the next request is a REST request, for example it won't be on the same "request".
If the cache clear is on drush and the next request is a ordinary drupal page, the rebuild will also NOT happen on the same request,
given that we don't invalidate the router, though rebuild it directly (maybe we should change that?)

The only case is when you do some action in the UI, like clearing the cache or adding a view, which triggers rebuild of the routes. In this cases the local tasks will be requested and then fetched.

This is needed for configuration translation module. We create a bunch of config_translation.item.* routes and then need to fetch those to create local tasks from them.

As far as I understand the usecase you kind of want to reuse the route system as a place to list all of your config translation plugins.
Semantically it feels better to not rely on stored route objects, but rather use the actual existing plugins.

Crell’s picture

tstoeckler: Please come to the WSCCI meeting this Tuesday. I think we need to work through the various RouteProvider patches you've been putting forward all together in real-time. This piecemeal issue approach is not working.

dawehner’s picture

@tstoeckler
So to summary, you want to be able to fetch all the routes defined by a module in a sane way?

There is a general problem given that you can override existing routes within your custom module, so the route is maybe not longer provided by the module you expect.

here is a simple example probably implemented by core at some day: taxonomy.term_page will be overridden by views, so what is the actual module providing the routes.

For the case of config_translation it seems to be more sane to have some helper method on the config translation route subscriber to fetch the routes you need manually from your own code.

tstoeckler’s picture

Re #16: Sorry, I've been a pretty bad watcher of my tracker recently so I missed that :-/

Re #17: That's an interesting example.

In general I think it makes more sense for the config_translation.module use-case to do what Views does (if I understand that correctly) and store a list of generated route names in \Drupal::state() from the RouteSubscriber. We can then conveniently fetch that when generating local tasks etc. I was reluctant to that at first, because the list of route names was just 1 very simple sql query away (which is why I opened various issues to encapsulate that query), but @dawehner's concerns are valid, and if I can make @Crell happy with that as well, that's not too bad.

So, although I still think this would make some amount of sense, I don't have any personal interest in it anymore. So if someone wants to close (won't fix), go for it!

dawehner’s picture

Just a general idea: We could introduce some sort of IntrospectableRouteProviderInterface which you can use for some purposes. One example would be such advanced usecases but maybe also some debugging tools could leverage it.

tstoeckler’s picture

@dawehner that sounds like a good idea. Still, if it's optional, that would mean that in can *only* be used by debugging tools, etc. and not by module business logic. Still makes a lot of sense, IMO.

Crell’s picture

Let's put this on the agenda for this Tuesday. tstoeckler, can you attend? (Noon US Eastern Time.)

dawehner’s picture

Issue summary: View changes
Status: Needs review » Fixed

We really don't need this anymore with the rewritten rebuilding. In case you want you can also use the lazyroutecollection.

gábor hojtsy’s picture

Status: Fixed » Closed (duplicate)

Duplicate then?