The cache check creates a race condition issue when multiple calls are made at almost exactly the same time (different microtime but same time enough for the latter request to override the cache key before it was read by the previous one). Attached is a patch that fixes this by using a non-static cache key with a random number. Additionally fixed some whitespace CS issues.
benjifisher → credited zeip → .
I removed your access to the project. Thanks for your help with the project!
Attached is a patch to implement this.
Just to note in case someone wants to move this forward: The user permission considered here for bypassing the group access ”bypass group access” has been removed from Group 2 in #3259076: Get rid of the bypass permission → .
We had another kind of problem related to that, namely that the deployments took quite long as we're using an S3 API for images and deleting the image styles takes a long time during config import. We decided to solve this by adding the image styles to a queue that is handled after the config import has been done. Attaching a patch of the concept here (it's not supposed to be completely working, but just to share our idea of the solution.)
The patch in #179 fixed this for us. The merge requests !4604 and !6351 both include the patch file, which shouldn't be included in the MR's but at least !6351 looks otherwise ok.
Core issue ✨ Add possibility to skip flushing all image style derivatives when an image style configuration changes Needs work is also about this.
Our custom stream wrapper's getType returns StreamWrapperInterface::READ_VISIBLE. getExternalUrl returns FALSE with the intent of indicating that no direct URL is available (the method documentation indicates that this would be a web-accessible URL, while we want to serve the file through Drupal). Should this be something else? I'll admit that I didn't find much documentation regarding implementing a custom stream wrapper, but the current version is working except for the needed change in s3fs.
I should also mention that we do have the imageapi_optimize_webp module enabled which does something to the image style controller route.
Thanks for being so helpful so far, really appreciate it!
I left the itok arguments from my examples but yeah, they're there. We're indeed using ImageStyleInterface::buildUrl() to generate the image style URL's with something like streamwrappername://202/15481/upload_000922.jpg as the URI, and we're getting the URLs I mentioned ie. ones that have streamwrappername as the scheme.
We have a custom read-only streamwrapper that is used to reference files on another server to allow us to use Drupal image styles on them.
We're currently using the URL pattern /s3/files/styles/image_style_name/streamwrappername/202/15481/upload_000922.jpg , which works now that I added streamwrappername to the PathProcessor. So basically the idea is that the client requests the file from Drupal, which in turn uses the stream wrapper to fetch the image (if necessary) and create the image style version of it, saving that to S3 and serving it from there.
It's certainly possible that I've misunderstood something and there's a better way to do this. Did I understand correctly that in this case an URL pattern like /s3/files/styles/image_style_name/public/styles/streamwrappername/202/81/upload_02.jpg should work instead? I tried this and at least on a first try it doesn't work.
The patch in #2 worked for us, thanks!
We were also experiencing this after D10.1 upgrade. For us it seems to be caused and fixed by 🐛 Operation is null in custom menu link form Needs review . The patch also did fix it though.
ZeiP → made their first commit to this issue’s fork.
Rerolled the patch on 1.6, this fixed the PHP Fatal error: Uncaught TypeError: Drupal\Core\Entity\Sql\DefaultTableMapping::Drupal\Core\Entity\Sql\{closure}(): Argument #1 ($definition) must be of type Drupal\Core\Field\FieldStorageDefinitionInterface, __PHP_Incomplete_Class given in /app/public/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php:174 for us.
It seems that the 1.2 and 1.3 versions have been created from a branch that has the fix, but the 1.4 branch including the D9 fixes are in a different branch that has derived from the branch used before.
The easiest way is probably to do
$ git checkout 8.x-1.x
$ git merge 8.x-1.4
$ git push origin 8.x-1.x
Which would cause the D9 fixed to be correctly merged in the main branch.
Works for us also. It'd be nice to get a new 1.x release for this and other issues or if the 1.x version is indeed EOL, it should be marked as unsupported to urge users to upgrade. At least for us this bug breaks the permissions view so I'm setting the priority up to major per the issue priority guidelines.
Re-rolled for 3.x. Seems to work for us; I think it's a good approach that the file system checks are optional, as at least we're mostly interested in cache and database availability and those can be sufficiently tested with this. The expandability is a big plus. Marking RTBC.
We're also having problems with pages jumping to wrong places because the correct menu parent isn't visible to the editors due to workbench access permissions. I created a patch partly reverting the changes done in #2988119 and building on top of it to always show the current menu parent in the options regardless of the permission definitions. This likely isn't ideal, though – maybe the menu element should be disabled altogether if the parent doesn't match? Anyway, it seems that the attached patch solves the problem for us for now.