Skip to content
Snippets Groups Projects

Issue #2983456: Expose triggering update of media metadata + thumbnail to end users

Open Issue #2983456: Expose triggering update of media metadata + thumbnail to end users
2 unresolved threads
2 unresolved threads

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
427 429 // regardless of its extension, return its URI.
428 430 $remote_thumbnail_url = $remote_thumbnail_url->toString();
429 431 $hash = Crypt::hashBase64($remote_thumbnail_url);
430 $files = $this->fileSystem->scanDirectory($directory, "/^$hash\..*/");
431 if (count($files) > 0) {
432 return reset($files)->uri;
432
433 if ($forceUpdate === FALSE) {
434 $files = $this->fileSystem->scanDirectory($directory, "/^$hash\..*/");
  • This will get unusably slow if the directory contains a lot of files. It would be better to use file_exists, something file_exists($directory . $hash . '.jpg') (maybe with lazy checking for jpeg, webp, png).

    The problem can be experienced when using the default media table view (which calls GetLocalThumbnail for every media), becomes incredibly slow with 25'000 files.

    Edited by kjauslin
  • this is a valid point, but the goal of this MR is not to improve file system scanning of the OEmbed plugin. This change is simply making the scan conditional with if ($forceUpdate === FALSE) {

  • Please register or sign in to reply
  • Edouard Cunibil added 2020 commits

    added 2020 commits

    • afc5de33...b6aef35b - 760 commits from branch project:10.1.x
    • b6aef35b...d2ffc4d8 - 1250 earlier commits
    • 24122795 - Issue #3419271 by catch: Only run the performance tests once each in performance test runs
    • f665483c - Issue #3101344 by Akhil Babu, Alex Bukach, ravi.shankar, quietone,...
    • 10322daf - Issue #3414349 by deviantintegral, taraskorpach, alexpott, smustgrave, catch:...
    • cfabfbe8 - Issue #3419288 by longwave: Remove withConsecutive() in RouteBuilderTest
    • c2958323 - Issue #3418873 by Keshav Patel, longwave, shweta__sharma: Remove outdated @todo in details.css
    • 5a571f6f - Issue #2820411 by Akhil Babu, smustgrave, larowlan, joachim, quietone: Improve...
    • a9046450 - Issue #3082211 by quietone, Pooja Ganjage, smustgrave, danflanagan8, Spokje,...
    • cc2223a9 - Issue #3419070 by quietone, DanielVeza, smustgrave: Remove 14 foreign words from the dictionary
    • 870e5c1c - Issue #3410476 by mondrake, smustgrave: Add event for statement execution failure
    • 30e96647 - Issue #2983456: expose triggering update of media metadata + thumbnail to end users

    Compare with previous version

  • Edouard Cunibil changed the description

    changed the description

  • Edouard Cunibil changed target branch from 10.1.x to 11.x

    changed target branch from 10.1.x to 11.x

  • Edouard Cunibil added 1 commit

    added 1 commit

    Compare with previous version

  • Luke Leber added 371 commits

    added 371 commits

    Compare with previous version

  • Luke Leber added 1 commit

    added 1 commit

    • 6466e710 - Resolve CS violation; additional config validation work may still be required to land this FR.

    Compare with previous version

  • Luke Leber added 1 commit

    added 1 commit

    • 8300d27b - Resolve CS (for real this time?)

    Compare with previous version

  • Luke Leber added 1 commit

    added 1 commit

    • 9923b5d1 - This particular schema certainly seems fully validatable to me (though I've...

    Compare with previous version

  • Luke Leber added 1 commit

    added 1 commit

    • 048f64dd - Let's revert this; obviously I have no idea what I'm doing with config validation <yet>.

    Compare with previous version

  • Karl Shea added 675 commits

    added 675 commits

    • 048f64dd...77b8e596 - 669 commits from branch project:11.x
    • c45633a6 - Issue #2983456: expose triggering update of media metadata + thumbnail to end users
    • f99c0382 - Fix phpcs linting failures.
    • 0f8e7508 - Resolve CS violation; additional config validation work may still be required to land this FR.
    • d739d7bc - Resolve CS (for real this time?)
    • 5ddf4b2a - This particular schema certainly seems fully validatable to me (though I've...
    • a93954bd - Let's revert this; obviously I have no idea what I'm doing with config validation <yet>.

    Compare with previous version

  • Karl Shea added 1 commit

    added 1 commit

    Compare with previous version

  • Karl Shea added 1 commit
  • Karl Shea added 2 commits

    added 2 commits

    Compare with previous version

  • Edouard Cunibil added 49 commits

    added 49 commits

    • 339799c1...cc9baee9 - 40 commits from branch project:11.x
    • 06d10869 - Issue #2983456: expose triggering update of media metadata + thumbnail to end users
    • d8f494d6 - Fix phpcs linting failures.
    • c84ffb34 - Resolve CS violation; additional config validation work may still be required to land this FR.
    • 58ead100 - Resolve CS (for real this time?)
    • 122ff8c3 - This particular schema certainly seems fully validatable to me (though I've...
    • 82374dfc - Let's revert this; obviously I have no idea what I'm doing with config validation <yet>.
    • fddb874c - Lint
    • d31dbc6f - MediaConfigUpdater is removed
    • c19d4046 - Duplicate import

    Compare with previous version

  • added 1 commit

    • d1ee0a9d - Clean up removed post update.

    Compare with previous version

  • Edouard Cunibil added 12 commits

    added 12 commits

    • d1ee0a9d...89278799 - 2 commits from branch project:11.x
    • e4594cc5 - Issue #2983456: expose triggering update of media metadata + thumbnail to end users
    • 9ba46f33 - Fix phpcs linting failures.
    • ec6e0a34 - Resolve CS violation; additional config validation work may still be required to land this FR.
    • 1638cea3 - Resolve CS (for real this time?)
    • 2937289c - This particular schema certainly seems fully validatable to me (though I've...
    • 88b47527 - Let's revert this; obviously I have no idea what I'm doing with config validation <yet>.
    • c741c449 - Lint
    • fb87b06d - MediaConfigUpdater is removed
    • c6c67c98 - Duplicate import
    • 188d17f4 - Clean up removed post update.

    Compare with previous version

  • 436 438 // regardless of its extension, return its URI.
    437 439 $remote_thumbnail_url = $remote_thumbnail_url->toString();
    438 440 $hash = Crypt::hashBase64($remote_thumbnail_url);
    439 $files = $this->fileSystem->scanDirectory($directory, "/^$hash\..*/");
    440 if (count($files) > 0) {
    441 return reset($files)->uri;
    441
    442 if ($forceUpdate === FALSE) {
    443 $files = $this->fileSystem->scanDirectory($directory, "/^$hash\..*/");
    444 if (count($files) > 0) {
    445 return reset($files)->uri;
    446 }
    442 447 }
    443 448
    444 449 // The local thumbnail doesn't exist yet, so we need to download it.
    • Note:This does not have the browser cache busting that was added in https://www.drupal.org/project/drupal/issues/2983456#comment-15434979.

      Options so far for cache busting:

      1. The thumbnail generation approach as used in patch #145 Pros:
      • Busts through all layers of caching - browser, edge cache reverse proxy.
      • Automatically deletes the old file and any image styles for the old file.
      • ??? Cons:
      • A call to "getMetadata" for the "thumbnail_uri" assumes that the thumbnail is being set on the media entity. If it does not update the media, then this would cause deletion of the old file.
      • ???
      1. Add a modified timestamp URL query parameter as described in #146 and #147 Pros:
      • Does not alter the thumbnail generation.
      • ??? Cons:
      • URL query parameters are not always respected by all layers of caching. For example, edge caching in Akamai could ignore the query parameter for some assets such images depending on the configuration in Akamai.
      • Depending on the approach to get the modified time, you could be incurring a file system operation ("filemtime") on every image_style theme rendering.
      • ???
    • Please register or sign in to reply
    Please register or sign in to reply
    Loading