Problem/Motivation

Seems like latest version of the module does not work well with imageapi_optimize_webp.

The issue is that they both override code class ImageStyleDownloadController.

Steps to reproduce

Install and configure imageapi_optimize_webp and stage_file_proxy

Upload an image and render it, so it will try to render webp version but it can't generate it

Proposed resolution

Use ImageStyleDownloadController from imageapi_optimize_webp as decorated class.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

nginex created an issue. See original summary.

nginex’s picture

Assigned: nginex » Unassigned
Status: Active » Needs review
Related issues: +#3171559: WebP support - ImageAPI Optimize WebP
smustgrave’s picture

Category: Bug report » Feature request
smustgrave’s picture

Status: Needs review » Needs work

Probably needs a test too, if possible

useernamee’s picture

I've written a Kernel test. (Disclousure: I got the help from AI):

se Drupal\image\Entity\ImageStyle;
use Drupal\KernelTests\KernelTestBase;
use Drupal\stage_file_proxy\Controller\ImageStyleDownloadController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;

/**
 * Tests WebP support integration with stage_file_proxy module.
 *
 * @group ldp_media
 */
class StageFileProxyWebpTest extends KernelTestBase {

  /**
   * {@inheritdoc}
   */
  protected static $modules = [
    'system',
    'file',
    'image',
    'stage_file_proxy',
    'imageapi_optimize_webp',
    'system',
    'user',
    'field',
  ];

  /**
   * {@inheritdoc}
   */
  protected function setUp(): void {
    parent::setUp();

    $this->installEntitySchema('file');
    $this->installSchema('file', ['file_usage']);
    $this->installConfig(['system', 'image']);

    // Create a basic image style for testing.
    ImageStyle::create([
      'name' => 'test_style',
      'label' => 'Test Style',
    ])->save();
  }

  /**
   * Tests that stage_file_proxy uses WebP controller.
   */
  public function testWebpControllerIntegration() {
    // Get the stage_file_proxy controller from container.
    $controller = new ImageStyleDownloadController(
      $this->container->get('lock'),
      $this->container->get('image.factory'),
      $this->container->get('stream_wrapper_manager'),
      $this->container->get('file_system'),
      $this->container->get('module_handler')
    );

    // Use reflection to check the decorated controller type.
    $reflection = new \ReflectionClass($controller);
    $decorated_property = $reflection->getProperty('decorated');
    $decorated_property->setAccessible(TRUE);
    $decorated_controller = $decorated_property->getValue($controller);

    // Assert that WebP controller is being used.
    $this->assertInstanceOf(
      'Drupal\imageapi_optimize_webp\Controller\ImageStyleDownloadController',
      $decorated_controller,
      'Stage file proxy should use WebP controller when imageapi_optimize_webp is enabled.'
    );
  }

  /**
   * Tests WebP request processing through stage_file_proxy controller.
   */
  public function testWebpRequestProcessing() {
    // Create a request for WebP image style.
    $request = Request::create('/styles/test_style/public/test.webp', 'GET', [
      'file' => 'test.webp',
    ]);

    // Get image style entity.
    $image_style = ImageStyle::load('test_style');
    $this->assertNotNull($image_style, 'Test image style should exist.');

    // Get the stage_file_proxy controller.
    $controller = new ImageStyleDownloadController(
      $this->container->get('lock'),
      $this->container->get('image.factory'),
      $this->container->get('stream_wrapper_manager'),
      $this->container->get('file_system'),
      $this->container->get('module_handler')
    );

    // Test that the controller can handle WebP requests without fatal errors.
    try {
      $response = $controller->deliver($request, 'public', $image_style);
      // We expect this to fail gracefully (404 or similar) since we don't
      // have actual files, but it should not throw fatal errors related to
      // WebP processing.
      $this->assertNotEquals(500, $response->getStatusCode(), 'Request should not result in server error.');
      $this->assertTrue(TRUE, 'WebP request processing completed without fatal errors.');
    }
    catch (NotFoundHttpException $e) {
      // Expected when source file doesn't exist - this is fine.
      $this->assertTrue(TRUE, 'WebP request processing handled missing file correctly.');
    }
    catch (\Exception $e) {
      $this->fail('Unexpected exception during WebP processing: ' . $e->getMessage());
    }
  }

}
ricovandevin’s picture

The fix does work. In the current implementation, it does not create a (hard) dependency on the ImageAPI Optimize WebP because no code of that module is called / executed until after it was verified that that module is enabled. So in that sense it is safe.

It might be cleaner to put to support in a submodule. But that would also be quite some overhead for a single line of effective code.

smustgrave’s picture

Version: 3.1.x-dev » 4.0.x-dev

  • smustgrave committed a9029684 on 4.0.x authored by nginex
    feat: #3529745 Add support of ImageAPI Optimize WebP
    
    By: nginex
    By:...
smustgrave’s picture

Status: Needs work » Fixed

Lets finally land this one.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.