- 🇺🇸United States smustgrave
Not sure if it matters but 🐛 Add a lock around retrieving upstream files Fixed was merged
I tried patch #67 with version 1.1 and private images fail to download from origin server. Since there is no patch for version 2, is there any other approach to make private files work?
- last update
over 1 year ago Patch Failed to Apply - 🇮🇳India kunalkursija Mumbai
Patch #67 on stage_file_proxy 8.x-1.1 worked for me. However, The application that I am working on serves the private files from the overridden path(not
system/files
). Hence I had to save the patch locally and make below changes:- The private files directory in my application is configured via settings.php in
$settings['file_private_path']
, So I updated the FetchManager.php file's code$file_dir = $this->fetchInfoService->getLocalSchemePath($fetchInfo->getScheme());
to$file_dir = \Drupal\Core\Site\Settings::get('file_private_path');
. This ensures that stage_file_proxy saves the private files at the location from which they are read/served. - The functions
getUrlBasePath()
&getFetchInfo()
of SfpPrivateStream.php class has hardcoded the pathssystem/files
&system/files/styles
respectively. However, In my case, the files were being served from different paths and hence I updated it to suit my need.
The above changes made the patch #67 work for me.
- The private files directory in my application is configured via settings.php in
- 🇺🇸United States smustgrave
This is the last ticket needed for the 2.0.3 release fyi.
- 🇬🇧United Kingdom nicrodgers Monmouthshire, UK
@smustgrave it may be worth carrying on with 2.0.3 without this one, and adding this in when it's ready.
- 🇺🇸United States johnle
I did my best to try to reroll this to version 2.1.1, there is quite a bit of code refactoring to 2.0 that made it a challenge. This is working, but probably could use some more tweaking + testing + cleanup. One thing I've notice is that some of the code from 1x. is no longer in 2.x like the webp support code.
- last update
about 1 year ago Patch Failed to Apply - 🇺🇸United States smustgrave
Started a new branch for just this feature
Rerolled what I could 🐛 Error retrieving images from styles that use ConvertImageEffect Fixed will have to be tested again as that was using the variables we removed.
Also removed all deprecated functions and interfaces. - 🇺🇸United States smustgrave
Needs some work for upgrade paths but StageFileProxyServiceProvider needs some work as it's breaking the module from being installed and I can't see why.
- 🇨🇦Canada deviantintegral
We just ran into a case where images failed to load when using the file_mdm module. Our theme needs to load the source image first to pull out some exif information, but the call to
\Drupal\file_mdm\Plugin\FileMetadata\FileMetadataPluginBase::loadMetadataFromFile
fails because it doesn't exist.I have a horrible, horrible hack in place which works by swapping out the exif plugin implementation. But realistically, this would be better if Stage File Proxy swapped out the public stream wrapper so it could intercept file_exists() calls and download as needed.
<?php /** * Implements file_metadata_plugin_info_alter(). * * @param array $info * * @return void */ function MYSITE_stage_file_proxy_file_metadata_plugin_info_alter(array &$info) { if (isset($info['exif'])) { $info['exif']['class'] = "Drupal\MYSITE_stage_file_proxy\Plugin\FileMetadata\Exif"; } }
declare(strict_types=1); namespace Drupal\MYSITE_stage_file_proxy\Plugin\FileMetadata; use Drupal\Core\StreamWrapper\PublicStream; use GuzzleHttp\ClientInterface; use Symfony\Component\DependencyInjection\ContainerInterface; class Exif extends \Drupal\file_mdm_exif\Plugin\FileMetadata\Exif { /** * @var \GuzzleHttp\ClientInterface */ protected readonly ClientInterface $httpClient; public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { $instance = parent::create($container, $configuration, $plugin_id, $plugin_definition); $instance->httpClient = $container->get('http_client'); return $instance; } public function loadMetadataFromFile(): bool { $localTempPath = $this->getLocalTempPath(); if (!file_exists($localTempPath)) { if (strpos($localTempPath, 'public://') === 0) { $stream = new PublicStream(); $stream->setUri($localTempPath); $url = $stream->getExternalUrl(); // We use an http client because we don't have a good simple API for // triggering a download from a public stream. $this->httpClient->request('GET', $url); } } return parent::loadMetadataFromFile(); } }
- First commit to issue fork.
- 🇳🇱Netherlands eelkeblok Netherlands 🇳🇱
I merged in 3.x and one thing led to another... I *think* there was still a problem with trying to add dependency injection to the stream wrappers, unless I misunderstood how things are supposed to work. The stream wrappers end up being registered with PHP as actual stream wrappers, and Drupal/Symfony dependency injection doesn't kick in there, so I was getting errors of the constructor being called without the proper arguments. So, I removed the arguments and replaced them with some methods calling the Drupal global object.
I did my best capturing the spirit of the changes in the 3.x branch, I hope I didn't break anything.
- 🇳🇱Netherlands eelkeblok Netherlands 🇳🇱
The config validation that is enabled on tests is tripping up saving the settings form. I tried turning the origin_path into a sequence, but that wasn't the (entire?) solution. FWIW (not sure if this is documented anywhere), I got the config validation to kick in on my dev environment by putting this in an active services.yml (e.g. development.services.yml, *if* that's enabled), to make debugging easier:
services: testing.config_schema_checker: class: Drupal\Core\Config\Development\ConfigSchemaChecker arguments: ['@config.typed', [], FALSE], tags: [{name: event_subscriber}]
- 🇳🇱Netherlands eelkeblok Netherlands 🇳🇱
Slowly chipping away at test failures. Seem to have arrived at a pretty fundamental one; the test is using the public stream wrapper to test if the original file was created. Not confident this is a great idea, maybe we need to introduce an "unwrapped" version of the original stream wrapper for the test.