AssetRoutes::routes() assumes stream wrapper implements ::getDirectoryPath()

Major as this blocks alternative implementations for assets:// - "Render one feature unusable with no workaround"

Tagging related issue where its been known in the past this method was used when not on an API and should not be used in core.

Can we just use \Drupal\Core\StreamWrapper\StreamWrapperInterface::realpath() ?

The code comments appear to indicate they the alternative stream wrapper is never used to directly serve a file. In which case why change the stream wrapper at all - could override the controller and use a cache backend to store the file contents instead.

Thank you all for chiming in on this. I hope my responses in the MR thread addresses much of the questions around the why and what.

I would like to summarize the reasons for supporting non-local stream wrappers, however. Catch's comment sums up the issue well:

The code comments appear to indicate they the alternative stream wrapper is never used to directly serve a file. In which case why change the stream wrapper at all - could override the controller and use a cache backend to store the file contents instead.

There's two main issues at play here. One is the time to generate the asset, which we can all agree is significant and we want to avoid doing so more than once. Second is the question of how and where the client requests the generated file on subsequent requests.

The first concern is addressed out of the box no matter the local/non-local status of the stream wrapper. The asset controller doesn't find a matching file (loaded with assets:// stream wrapper, works no matter the backing storage), generates it, saves it and sends the result to the client.

We are addressing here the second issue. It is the stream wrapper's job to yield an external URL for the file - see \Drupal\Core\StreamWrapper\StreamWrapperInterface::getExternalUrl(). For local stream wrappers, this very conveniently matches the paths from AssetRoutes::routes(). For others, the stream wrapper must special-case `assets://` and point the external URL at the asset controllers. (I've linked an example from an MR against Flysystem, above.)

Subsequent requests for an already-generated asset would be piped, but it still has the overhead of bootstrapping Drupal. This is where the site owner's responsibility lies to have that external URL behind some sort of caching reverse proxy.

I'm proposing the fix we have here because it 1) doesn't change anything about the default case of using local storage and 2) allows expert site owners to utilize the core asset generation API but in a way that gives you maximum flexibility about the backing storage. For instance, a Drupal application that has many web containers running in a serverless environment wouldn't need to each have its own memory cache copy of the asset.

Linking related 🐛 Aggregated asset generation causes uncacheable assets Fixed .

@bradjones1 the main reason to prevent sending cacheable headers there is to avoid, for example, different CDN endpoints from serving the PHP version of the response vs. the file as it's served from the web server direct from disk. Even if the content can be guaranteed to be exactly the same, the headers would be different.

There are two reasons for this apart from consistency:

I've seen situations in the past (maybe Drupal 7 era) where you end up with different versions of supposedly the same thing in different CDN endpoints and it was not enjoyable. Even something like different max age/expires headers could be annoying.

Also some hosts, the only example I know of is Pantheon but there may be others, bill differently for requests served from PHP vs static assets from disk, even when those requests are cached in the CDN. I don't understand why they make that distinction but it's how I interpret this documentation:

https://docs.pantheon.io/guides/account-mgmt/traffic#pages-served

Re: #12 - that's interesting history, thanks.

I've opened 🐛 Asset cache-control headers are incorrect Active to break out the issue of the cache-control header from the stream wrapper question, so we can perhaps at least unblock this restriction on asset storage from the question of the headers.

Narrowed up, hopefully this is easier to get in without the distraction of how to improve the (kinda currently broken/invalid) cache-control headers.

Re: #6:

Can we just use \Drupal\Core\StreamWrapper\StreamWrapperInterface::realpath() ?

No, because some stream wrappers have no "real path" that's applicable here; the "directory path" is being used (abused?) to generate a route that is _both_ registered as valid in Drupal but also on disk, with the expectation that the file on disk will "win" when the server receives the next request. Clever, but in this case a little too much so.

Adding a couple of other tangentially related open issues with Drupal core that have additional discussions and context.

So without test coverage what's a good way to test this one?