Add stage_file_proxy stream wrapper

Created on 5 December 2017, about 7 years ago
Updated 3 September 2024, 4 months ago

Calls to Image::isValid() return false when the file doesn't exist on the local environment, regardless of whether or not it exists in the origin.

Expected behaviour:
stage_file_proxy should intercept calls to Image::isValid() and try to obtain the image from the origin if it doesn't exist locally.

Feature request
Status

Needs work

Version

3.0

Component

Code

Created by

🇬🇧United Kingdom nicrodgers Monmouthshire, UK

Live updates comments and jobs are added and updated live.
  • Needs reroll

    The patch will have to be re-rolled with new suggestions/changes described in the comments in the issue.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

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

  • 🇺🇸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?

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 5.7
    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 paths system/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.

  • 🇺🇸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.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.2 & MySQL 8
    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.

  • Merge request !59Draft: Reroll → (Open) created by smustgrave
  • 🇺🇸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.
  • Pipeline finished with Canceled
    3 months ago
    Total: 85s
    #316271
  • Pipeline finished with Failed
    3 months ago
    Total: 203s
    #316272
  • Pipeline finished with Failed
    3 months ago
    Total: 142s
    #316316
  • 🇳🇱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.

  • Pipeline finished with Failed
    3 months ago
    Total: 151s
    #316319
  • Pipeline finished with Failed
    3 months ago
    Total: 160s
    #317172
  • 🇳🇱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}]
  • Pipeline finished with Failed
    3 months ago
    Total: 153s
    #319178
  • Pipeline finished with Failed
    3 months ago
    Total: 164s
    #319221
  • 🇳🇱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.

  • 🇫🇷France andypost

    Re #88 sounds good 👍 idea

  • Pipeline finished with Failed
    3 months ago
    Total: 1251s
    #325043
  • 🇩🇪Germany geek-merlin Freiburg, Germany

    Great how this is moving forward!!

Production build 0.71.5 2024