[meta] Use FileUrlGenerator::generate() everywhere, then deprecate generateString() and generateAbsoluteString()

This issue has puzzled me for a while in the past week, but it's actually a quite doable issue! We just have to fix the issue title to make it clearer. The original title was written before we decided that Url is what we care about so it's the default, and the strings are the odd-ones out.

This was decided in #2669074-166: Convert file_create_url() & file_url_transform_relative() to service, deprecate it →

I've created a dent of child issues. Adding a list of what else is still needed, unfortunately the end of the day has come.

@Kingdutch please read https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... → and let's discuss before opening any more child issues, file by file is not a preferred way of doing things.

A possible scoping is one issue for generateString and another for generateAbsoluteString, where we add the deprecations at the same time to ensure we caught all uses. From the look of the changes this won't be too hard to review with git diff --color-words.

This issue doesn't exist because there are many calls to it. It exists because some cases would currently require to convert to a string from a Url object and back about 3 times, which is bad for performance.

We need to work bottom up to support Url objects in surrounding API's. It starts with stream wrappers, another example is then \Drupal\image\Entity\ImageStyle::buildUrl(), many of those require complex BC dances as they require new methods. For ImageStyle, ✨ Split ImageStyle into the config entity and a separate event-based image processing service Needs work would be a chance to introduce this as it already introduces a new API.

The original issue on purpose only converted calls that allowed to fix a specific issue where it was worth doing.

@longwave Sorry about that, I tried to split it by subsystem group, though I should've have split the media tests. I was going to do one per method at first, but there were some tests which scaffolded the methods out too, so I figured it wasn't entirely a simple find/replace that could be checked with color-words, and some more granular PRs would be appreciated (as can also be seen by the difference in the patches that just pass testing and those that have some failures).

@Berdir I agree about the fact that the underlying problem is the converting back 'n forth to URL options. However, I disagree that we need to fix everything holistically and getting rid of generateString and generateAbsoluteString is not an improvement. If anything it makes it clearer where a URL object is available in a higher level method, which makes it easier to do the complex BC dances, and it discourages new uses of non-URL implementations.

My plan was to merge the Media patches into 1 and fix the test failure and then continue. But I'll merge those 3 issues and then just pause the issue for now I suppose.

Looks like we need a review from framework managers on the plan.

@Kingdutch asked me to respond here with framework manager hat on.

I'm not sure what's being asked of in terms of FM review - is it whether we want to rework the lower level APIs to unblock this? If so, then I think that's fine, as long as folks realize what we're signing up for - its probably too late for Drupal 11, so we would have to support any existing APIs along with any new APIS throughout the Drupal 11 cycle (at least).

Agreed we won't be able to deprecate all these things in 10.3 to actually drop this in 11.

It's still worth doing this though, because it'll make the DX much simpler.

@larowlan It sounds like you're +1 to the principle though. Did you see @Berdir's comment in #15 that provides a concrete example?

Yes, I did, that and stream wrappers sound like good places to start.

Removing the tag