- ๐บ๐ธUnited States recrit
@KarlShea
Regarding your comment "I added most of #145 except for removing image_path_flush() in Media, that will affect every type not just OEmbed."
- That seems like an edge case. You would need to have another media type that is using the thumbnail file of an oEmbed resource. The normal usage would be a 1:1 of media to thumbnail file.Regarding your comment "Edit: actually I removed #145, it looks like there was some disagreement."
There still needs to be a solution for busting cache - browser, edge cache, reverse proxy.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.
- ???2. 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.
- ??? - @karlshea opened merge request.
- First commit to issue fork.
- ๐ฌ๐งUnited Kingdom james.williams
Ah sorry, this should have been left at needs work, as per comment 82:
Leaving at NW due to some open comments by @wimleers, @phenaproxima and @smustgrave in the MR + no CR yet
- ๐ฌ๐งUnited Kingdom james.williams
!MR553 (which is the one for 11.x) now shows tests passing as they should, and the test-only feature failing as expected. Since the file upload in the test was switched to use the public stream, it needed a slight adjustment to take into account user access.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ changed the visibility of the branch 3008292-imageitemgetuploadvalidators-should-be to hidden.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ changed the visibility of the branch 11.x to hidden.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ changed the visibility of the branch 10.3.x to hidden.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ changed the visibility of the branch drupal-3008292/10.2.x to hidden.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ changed the visibility of the branch 3008292-imageitem-getuploadvalidators-10-3 to hidden.
- ๐ฌ๐งUnited Kingdom james.williams
I've opened MR !8484 in an attempt to get something usable with Drupal 10.3. Work from ๐ Provide a trait to create file upload validators from settings Needs review is in 10.3, but conflicts with this issue's work; I'm not entirely sure whether my attempt has resolved that satisfactorily. It looks to me a bit like these two issues have gone in divergent directions; at the least because
FileValidatorSettingsTrait::getFileUploadValidators()
only takes$field_definition->getSettings()
as input, whereas this issue requires the whole field definition to be passed around.I had a look at !7021, but that was missing changes to ImageWidget.php which seemed wrong to me. Sorry if opening yet another MR is unhelpful! It's only really to help those of us that need to be able to work with 10.3. The ideal is to get a correct MR against 11.x still.
- @jameswilliams-0 opened merge request.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ made their first commit to this issueโs fork.