Contrib.social

Breaking changes for public:// takeover in D10.1 from #1014086

Open on Drupal.org →
Created on 27 November 2022, about 2 years ago
Updated 18 April 2023, over 1 year ago

Problem/Motivation

It appears 🐛 Stampedes and cold cache performance issues with css/js aggregation Fixed breaks compatibility with s3fs. While this new method appears to be a boost for Drupal performance relies on css no longer being generated with the page request and instead uses an asynchronous call to generate css.

Drupal Core assumes that the link for the async request will be the same as the storage location (getExternalUrl()) which is not possible with s3fs and other streamWrapeprs that store files outside of local storage.

Steps to reproduce

Drupal 10.1 dev build with s3fs installed and public:// takeover enabled.

Proposed resolution

  • Add detection of CSS/JS object existence to S3fsStream::getExternalUrl() and modify url generation accordingly.
  • Decorate or Replace Drupal\Core\Asset\CssCollectionRenderer and Drupal\Core\Asset\JsCollectionRenderer to include cache metadata in the render array so that we may purge pages that have links to css that has not been generated yet.
  • Replace Drupal\system\Controller\CssAssetController and Drupal\system\Controller\JsAssetController as they would also be subject to the same type of vulnerability as addressed in #3298703: Core ImageStyleDownloadControler allow DoS for s3fs. → . Additionally we also need to call a cache purge on pages that have the link to css generation so the pages regenerate with the link to the bucket.

Some sites may want to consider adopting ✨ Provide a module for serving agregated CSS and JSS from local public filesystem instead of S3 when option "use_s3_for_public" is used Closed: won't fix and rely solely on local css/js objects,

Remaining tasks

  • Suggest Drupal Core make this change opt-in until D11 as it appears to break backwards compatibility with external streamWrappers in a minor release.
  • Encourage Core to use bubbleMetadata in CssCollectionRender and JsCollectionRender to avoid code duplication in s3fs.
  • Investigate if Drupal\Core\Asset\CssCollectionOptimizerLazy and Drupal\Core\Asset\JsCollectionOptimizerLazy require replacement, especially around the calls to generating file url's.
  • Investigate if an OutboundRouteProcessor can and should be added provide cache tags for other locations that generate Url objects.

User interface changes

None

API changes

TBD

Data model changes

TBD

🐛 Bug report
Status

Closed: outdated

Version

3.0

Component

Code

Created by

🇺🇸United States cmlara

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇺🇸United States cmlara

    With 📌 Make css/js optimized assets path configurable Fixed committed the above requirements are no longer necessary as Drupal no longer will store assets on the public:// streamWrapper making the s3fs module no longer responsible for CSS/JS storage.

    Testing against Drupal 10.1.x dev branch and s3fs 8.x-3.1 pages load, assets are stored on the Drupal instance.

    As noted above, the new core change also handles a previous s3fs feature request of not storing css/js assets inside s3 storage.

    Thank you again @catch for pointing out an issue that proved to be a reasonable compromise to avoid a re-work inside of s3fs.

    Closing this out as outdated, we can track the rest of D10.1 support in ✨ Use with Drupal 10.1 Fixed as we get closer to a release date.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024