From 7e5f65586535a6c6f03a51228ca11014dfbb8e73 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 18 Jun 2024 17:44:03 -0300 Subject: [PATCH 01/11] Misc pre-existing coding standards issues. Addressed with phpcbf: ``` ddev composer exec -- phpcbf --standard=Drupal,DrupalPractice --extensions=php $(ddev drush dd dgi_image_discovery) ``` --- src/DIDImageItemList.php | 2 +- src/EventSubscriber/DiscoverChildThumbnailSubscriber.php | 7 +++---- src/EventSubscriber/DiscoverOwnedThumbnailSubscriber.php | 7 +++---- src/EventSubscriber/ViewsFieldSubscriber.php | 3 +-- src/ImageDiscovery.php | 2 +- src/ImageDiscoveryEvent.php | 4 +--- src/Plugin/Field/FieldType/DIDImageItem.php | 5 ++--- src/Plugin/search_api/processor/DgiImageDiscovery.php | 2 +- 8 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/DIDImageItemList.php b/src/DIDImageItemList.php index 2547e06..df5da8c 100644 --- a/src/DIDImageItemList.php +++ b/src/DIDImageItemList.php @@ -2,8 +2,8 @@ namespace Drupal\dgi_image_discovery; -use Drupal\Core\TypedData\ComputedItemListTrait; use Drupal\Core\Field\EntityReferenceFieldItemList; +use Drupal\Core\TypedData\ComputedItemListTrait; /** * Boiler-plate for our computed field. diff --git a/src/EventSubscriber/DiscoverChildThumbnailSubscriber.php b/src/EventSubscriber/DiscoverChildThumbnailSubscriber.php index e665fdc..0aa622e 100644 --- a/src/EventSubscriber/DiscoverChildThumbnailSubscriber.php +++ b/src/EventSubscriber/DiscoverChildThumbnailSubscriber.php @@ -2,13 +2,12 @@ namespace Drupal\dgi_image_discovery\EventSubscriber; +use Drupal\Core\Entity\EntityStorageInterface; +use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\dgi_image_discovery\ImageDiscoveryEvent; use Drupal\dgi_image_discovery\ImageDiscoveryInterface; use Drupal\node\NodeInterface; -use Drupal\Core\Entity\EntityStorageInterface; -use Drupal\Core\Entity\EntityTypeManagerInterface; - /** * Discovery child thumbnails. */ @@ -45,7 +44,7 @@ class DiscoverChildThumbnailSubscriber extends AbstractImageDiscoverySubscriber */ public function __construct( ImageDiscoveryInterface $image_discovery, - EntityTypeManagerInterface $entity_type_manager + EntityTypeManagerInterface $entity_type_manager, ) { $this->imageDiscovery = $image_discovery; $this->nodeStorage = $entity_type_manager->getStorage('node'); diff --git a/src/EventSubscriber/DiscoverOwnedThumbnailSubscriber.php b/src/EventSubscriber/DiscoverOwnedThumbnailSubscriber.php index 1ea64a4..0eecee8 100644 --- a/src/EventSubscriber/DiscoverOwnedThumbnailSubscriber.php +++ b/src/EventSubscriber/DiscoverOwnedThumbnailSubscriber.php @@ -2,11 +2,10 @@ namespace Drupal\dgi_image_discovery\EventSubscriber; -use Drupal\dgi_image_discovery\ImageDiscoveryEvent; -use Drupal\node\NodeInterface; - use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityTypeManagerInterface; +use Drupal\dgi_image_discovery\ImageDiscoveryEvent; +use Drupal\node\NodeInterface; /** * Discover thumbnails which are owned by a given object. @@ -26,7 +25,7 @@ class DiscoverOwnedThumbnailSubscriber extends AbstractImageDiscoverySubscriber * Constructor. */ public function __construct( - EntityTypeManagerInterface $entity_type_manager + EntityTypeManagerInterface $entity_type_manager, ) { $this->mediaStorage = $entity_type_manager->getStorage('media'); } diff --git a/src/EventSubscriber/ViewsFieldSubscriber.php b/src/EventSubscriber/ViewsFieldSubscriber.php index 603f9ff..b15ef28 100644 --- a/src/EventSubscriber/ViewsFieldSubscriber.php +++ b/src/EventSubscriber/ViewsFieldSubscriber.php @@ -2,9 +2,8 @@ namespace Drupal\dgi_image_discovery\EventSubscriber; -use Drupal\search_api\Event\SearchApiEvents; use Drupal\search_api\Event\MappingViewsFieldHandlersEvent; - +use Drupal\search_api\Event\SearchApiEvents; use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** diff --git a/src/ImageDiscovery.php b/src/ImageDiscovery.php index cb1c4f5..3b455c0 100644 --- a/src/ImageDiscovery.php +++ b/src/ImageDiscovery.php @@ -22,7 +22,7 @@ class ImageDiscovery implements ImageDiscoveryInterface { * Constructor. */ public function __construct( - EventDispatcherInterface $event_dispatcher + EventDispatcherInterface $event_dispatcher, ) { $this->eventDispatcher = $event_dispatcher; } diff --git a/src/ImageDiscoveryEvent.php b/src/ImageDiscoveryEvent.php index b3d2727..1ee66c9 100644 --- a/src/ImageDiscoveryEvent.php +++ b/src/ImageDiscoveryEvent.php @@ -2,12 +2,10 @@ namespace Drupal\dgi_image_discovery; -use Drupal\media\MediaInterface; - use Drupal\Core\Cache\RefinableCacheableDependencyInterface; use Drupal\Core\Cache\RefinableCacheableDependencyTrait; use Drupal\Core\Entity\ContentEntityInterface; - +use Drupal\media\MediaInterface; use Symfony\Contracts\EventDispatcher\Event; /** diff --git a/src/Plugin/Field/FieldType/DIDImageItem.php b/src/Plugin/Field/FieldType/DIDImageItem.php index 7d81434..08c769b 100644 --- a/src/Plugin/Field/FieldType/DIDImageItem.php +++ b/src/Plugin/Field/FieldType/DIDImageItem.php @@ -2,13 +2,12 @@ namespace Drupal\dgi_image_discovery\Plugin\Field\FieldType; -use Drupal\dgi_image_discovery\ImageDiscoveryInterface; - use Drupal\Core\Cache\RefinableCacheableDependencyInterface; use Drupal\Core\Cache\RefinableCacheableDependencyTrait; use Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem; use Drupal\Core\TypedData\DataDefinitionInterface; use Drupal\Core\TypedData\TypedDataInterface; +use Drupal\dgi_image_discovery\ImageDiscoveryInterface; /** * Find image media related to the given node. @@ -46,7 +45,7 @@ class DIDImageItem extends EntityReferenceItem implements RefinableCacheableDepe public function __construct( DataDefinitionInterface $definition, $name = NULL, - TypedDataInterface $parent = NULL + TypedDataInterface $parent = NULL, ) { parent::__construct($definition, $name, $parent); diff --git a/src/Plugin/search_api/processor/DgiImageDiscovery.php b/src/Plugin/search_api/processor/DgiImageDiscovery.php index b0674f8..6084c40 100644 --- a/src/Plugin/search_api/processor/DgiImageDiscovery.php +++ b/src/Plugin/search_api/processor/DgiImageDiscovery.php @@ -50,7 +50,7 @@ public function __construct( $plugin_id, $plugin_definition, ImageDiscoveryInterface $image_discovery, - EntityTypeManagerInterface $entity_type_manager + EntityTypeManagerInterface $entity_type_manager, ) { parent::__construct($configuration, $plugin_id, $plugin_definition); From 4e070d1638956fc4c4a010b21bd135bf3bb73c3a Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 18 Jun 2024 17:48:16 -0300 Subject: [PATCH 02/11] Theoretical plugin definitions for deferring image generation. --- dgi_image_discovery.module | 3 +- dgi_image_discovery.routing.yml | 13 +++ dgi_image_discovery.services.yml | 8 ++ src/Attribute/DeferredResolution.php | 38 +++++++ src/Attribute/UrlGenerator.php | 38 +++++++ src/CacheableBinaryFileResponse.php | 76 +++++++++++++ .../DeferredResolutionController.php | 54 ++++++++++ src/DeferredResolutionInterface.php | 34 ++++++ src/DeferredResolutionPluginBase.php | 68 ++++++++++++ src/DeferredResolutionPluginManager.php | 26 +++++ ...ferredResolutionPluginManagerInterface.php | 10 ++ .../url_generator/Deferred.php | 46 ++++++++ .../url_generator/PreGenerated.php | 100 ++++++++++++++++++ .../url_generator/UrlGenerationTrait.php | 73 +++++++++++++ .../url_generator/deferred/Redirect.php | 35 ++++++ .../url_generator/deferred/Subrequest.php | 66 ++++++++++++ src/UrlGeneratorInterface.php | 34 ++++++ src/UrlGeneratorPluginBase.php | 22 ++++ src/UrlGeneratorPluginManager.php | 26 +++++ 19 files changed, 768 insertions(+), 2 deletions(-) create mode 100644 dgi_image_discovery.routing.yml create mode 100644 src/Attribute/DeferredResolution.php create mode 100644 src/Attribute/UrlGenerator.php create mode 100644 src/CacheableBinaryFileResponse.php create mode 100644 src/Controller/DeferredResolutionController.php create mode 100644 src/DeferredResolutionInterface.php create mode 100644 src/DeferredResolutionPluginBase.php create mode 100644 src/DeferredResolutionPluginManager.php create mode 100644 src/DeferredResolutionPluginManagerInterface.php create mode 100644 src/Plugin/dgi_image_discovery/url_generator/Deferred.php create mode 100644 src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php create mode 100644 src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php create mode 100644 src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php create mode 100644 src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php create mode 100644 src/UrlGeneratorInterface.php create mode 100644 src/UrlGeneratorPluginBase.php create mode 100644 src/UrlGeneratorPluginManager.php diff --git a/dgi_image_discovery.module b/dgi_image_discovery.module index 723250d..a4122af 100644 --- a/dgi_image_discovery.module +++ b/dgi_image_discovery.module @@ -5,10 +5,9 @@ * General hook implementations. */ -use Drupal\dgi_image_discovery\DIDImageItemList; - use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Field\BaseFieldDefinition; +use Drupal\dgi_image_discovery\DIDImageItemList; /** * Implements hook_entity_base_field_info(). diff --git a/dgi_image_discovery.routing.yml b/dgi_image_discovery.routing.yml new file mode 100644 index 0000000..e19c4f4 --- /dev/null +++ b/dgi_image_discovery.routing.yml @@ -0,0 +1,13 @@ +--- +dgi_image_discovery.deferred_resolution: + path: '/dgi_image_discovery/{style}/{node}' + defaults: + _controller: 'dgi_image_discovery.deferred_resolution_controller:resolve' + requirements: + _entity_access: node.view + options: + parameters: + style: + type: entity:image_style + node: + type: entity:node diff --git a/dgi_image_discovery.services.yml b/dgi_image_discovery.services.yml index f027034..d9231bd 100644 --- a/dgi_image_discovery.services.yml +++ b/dgi_image_discovery.services.yml @@ -25,3 +25,11 @@ services: class: '\Drupal\dgi_image_discovery\EventSubscriber\DiscoverRepresentativeImageSubscriber' tags: - name: event_subscriber + dgi_image_discovery.deferred_resolution_controller: + class: '\Drupal\dgi_image_discovery\Controller\DeferredResolutionController' + plugin.manager.dgi_image_discovery.url_generator: + class: Drupal\dgi_image_discovery\UrlGeneratorPluginManager + parent: default_plugin_manager + plugin.manager.dgi_image_discovery.url_generator.deferred: + class: Drupal\dgi_image_discovery\DeferredUrlGeneratorPluginManager + parent: default_plugin_manager diff --git a/src/Attribute/DeferredResolution.php b/src/Attribute/DeferredResolution.php new file mode 100644 index 0000000..6851231 --- /dev/null +++ b/src/Attribute/DeferredResolution.php @@ -0,0 +1,38 @@ +uri = $file instanceof \SplFileInfo ? $file->getPathname() : $file; + return parent::setFile($file, $contentDisposition, $autoEtag, $autoLastModified); + } + + /** + * {@inheritDoc} + */ + public function __sleep() { + $vars = $this->traitSleep(); + + unset($vars['file']); + + return $vars; + } + + /** + * {@inheritDoc} + */ + public function __wakeup() : void { + $this->traitWakeup(); + $this->setFile($this->uri); + } + + /** + * Convert a BinaryFileResponse into a CacheableBinaryFileResponse. + * + * @param \Symfony\Component\HttpFoundation\BinaryFileResponse $response + * The response to convert. + * + * @return static + * The converted response. + */ + public static function convert(BinaryFileResponse $response) : static { + return new static( + $response->getFile(), + $response->getStatusCode(), + $response->headers->all(), + /* $public, $contentDisposition, $autoEtag, $autoLastModified all accounted for in headers */ + ); + } + +} diff --git a/src/Controller/DeferredResolutionController.php b/src/Controller/DeferredResolutionController.php new file mode 100644 index 0000000..92b3eea --- /dev/null +++ b/src/Controller/DeferredResolutionController.php @@ -0,0 +1,54 @@ +get('plugin.manager.dgi_image_discovery.url_generator.deferred'), + ); + } + + /** + * Resolve image for the given node and style. + * + * @param \Drupal\image\ImageStyleInterface $style + * The style of image to get. + * @param \Drupal\node\NodeInterface $node + * The node of which to get an image. + * + * @return \Drupal\Core\Cache\CacheableResponseInterface + * A cacheable response. + * + * @throws \Drupal\Component\Plugin\Exception\PluginException + */ + public function resolve(ImageStyleInterface $style, NodeInterface $node) : CacheableResponseInterface { + // @todo Make plugin configurable. + /** @var \Drupal\dgi_image_discovery\DeferredResolutionInterface $plugin */ + $plugin = $this->deferredResolutionPluginManager->createInstance('redirect'); + + return $plugin->resolve($node, $style); + } + +} diff --git a/src/DeferredResolutionInterface.php b/src/DeferredResolutionInterface.php new file mode 100644 index 0000000..95560e0 --- /dev/null +++ b/src/DeferredResolutionInterface.php @@ -0,0 +1,34 @@ +entityTypeManager; + } + + /** + * {@inheritdoc} + */ + protected function getImageDiscovery(): ImageDiscoveryInterface { + return $this->imageDiscovery; + } + + /** + * {@inheritdoc} + */ + public function label(): string { + // Cast the label to a string since it is a TranslatableMarkup object. + return (string) $this->pluginDefinition['label']; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) : static { + $instance = new static($configuration, $plugin_id, $plugin_definition); + + $instance->entityTypeManager = $container->get('entity_type.manager'); + $instance->imageDiscovery = $container->get('dgi_image_discovery.service'); + + return $instance; + } + +} diff --git a/src/DeferredResolutionPluginManager.php b/src/DeferredResolutionPluginManager.php new file mode 100644 index 0000000..5ce5266 --- /dev/null +++ b/src/DeferredResolutionPluginManager.php @@ -0,0 +1,26 @@ +alterInfo('dgi_image_discovery__url_generator_info__deferred'); + $this->setCacheBackend($cache_backend, 'dgi_image_discovery__url_generator_plugins__deferred'); + } + +} diff --git a/src/DeferredResolutionPluginManagerInterface.php b/src/DeferredResolutionPluginManagerInterface.php new file mode 100644 index 0000000..d370b26 --- /dev/null +++ b/src/DeferredResolutionPluginManagerInterface.php @@ -0,0 +1,10 @@ + $node, + 'style' => $style, + ], + [ + 'absolute' => TRUE, + // XXX: Prevent use of aliased paths. + 'alias' => TRUE, + ], + ) + ->toString(TRUE) + ->addCacheableDependency($node) + ->addCacheableDependency($style); + } + +} diff --git a/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php b/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php new file mode 100644 index 0000000..e758337 --- /dev/null +++ b/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php @@ -0,0 +1,100 @@ +addCacheableDependency($node) + ->addCacheableDependency($style); + + $event = $this->imageDiscovery->getImage($node); + $generated_url->addCacheableDependency($event); + $media = $event->getMedia(); + if (empty($media)) { + return $generated_url; + } + + $generated_url->addCacheableDependency($media); + + $media_source = $media->getSource(); + $file_id = $media_source->getSourceFieldValue($media); + /** @var \Drupal\file\FileInterface|null $image */ + $image = $this->entityTypeManager->getStorage('file')->load($file_id); + if (empty($image)) { + return $generated_url; + } + + $generated_url->addCacheableDependency($image); + + return $generated_url->setGeneratedUrl($style->buildUri($image->getFileUri())); + } + + /** + * {@inheritDoc} + */ + public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { + $instance = new static($configuration, $plugin_id, $plugin_definition); + + $instance->imageDiscovery = $container->get('dgi_image_discovery.service'); + $instance->entityTypeManager = $container->get('entity_type.manager'); + + return $instance; + } + + /** + * {@inheritDoc} + */ + protected function getImageDiscovery(): ImageDiscoveryInterface { + return $this->imageDiscovery; + } + + /** + * {@inheritDoc} + */ + protected function getEntityTypeManager(): EntityTypeManagerInterface { + return $this->entityTypeManager; + } + +} diff --git a/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php b/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php new file mode 100644 index 0000000..16b5145 --- /dev/null +++ b/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php @@ -0,0 +1,73 @@ +addCacheableDependency($node) + ->addCacheableDependency($style); + + $event = $this->getImageDiscovery()->getImage($node); + $generated_url->addCacheableDependency($event); + $media = $event->getMedia(); + if (empty($media)) { + return $generated_url; + } + + $generated_url->addCacheableDependency($media); + + $media_source = $media->getSource(); + $file_id = $media_source->getSourceFieldValue($media); + /** @var \Drupal\file\FileInterface|null $image */ + $image = $this->getEntityTypeManager()->getStorage('file')->load($file_id); + if (empty($image)) { + return $generated_url; + } + + $generated_url->addCacheableDependency($image); + + return $generated_url->setGeneratedUrl($style->buildUri($image->getFileUri())); + } + +} diff --git a/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php b/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php new file mode 100644 index 0000000..a74af14 --- /dev/null +++ b/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php @@ -0,0 +1,35 @@ +getGeneratedUrl($node, $style); + + return (new CacheableRedirectResponse($generated_url->getGeneratedUrl())) + ->addCacheableDependency($generated_url); + } + +} diff --git a/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php b/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php new file mode 100644 index 0000000..b3f940e --- /dev/null +++ b/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php @@ -0,0 +1,66 @@ +getGeneratedUrl($node, $style); + + $current_request = $this->httpKernel->getRequest(); + $request = Request::create($generated_url->getGeneratedUrl()); + $request->setSession($current_request->getSession()); + $response = $this->httpKernel->handle($request, HttpKernelInterface::SUB_REQUEST); + if ($response instanceof BinaryFileResponse) { + return (CacheableBinaryFileResponse::convert($response)) + ->addCacheableDependency($generated_url); + } + + throw new CacheableHttpException($generated_url, $response->getStatusCode(), $response->getContent()); + } + + /** + * {@inheritDoc} + */ + public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition): static { + $instance = parent::create($container, $configuration, $plugin_id, $plugin_definition); + + $instance->httpKernel = $container->get('http_kernel'); + + return $instance; + } + +} diff --git a/src/UrlGeneratorInterface.php b/src/UrlGeneratorInterface.php new file mode 100644 index 0000000..65070b1 --- /dev/null +++ b/src/UrlGeneratorInterface.php @@ -0,0 +1,34 @@ +pluginDefinition['label']; + } + +} diff --git a/src/UrlGeneratorPluginManager.php b/src/UrlGeneratorPluginManager.php new file mode 100644 index 0000000..412dbbc --- /dev/null +++ b/src/UrlGeneratorPluginManager.php @@ -0,0 +1,26 @@ +alterInfo('dgi_image_discovery__url_generator_info'); + $this->setCacheBackend($cache_backend, 'dgi_image_discovery__url_generator_plugins'); + } + +} From ff08d172609a917aa2d4c10c63f624ca123dd56e Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 18 Jun 2024 18:06:20 -0300 Subject: [PATCH 03/11] DRYing up, drop `final` and adjust some comments. `final` was just there from drush's code generation, but not sure we actually want it there. --- .../url_generator/Deferred.php | 2 +- .../url_generator/PreGenerated.php | 27 ++----------------- .../url_generator/deferred/Redirect.php | 4 +-- .../url_generator/deferred/Subrequest.php | 4 +-- 4 files changed, 7 insertions(+), 30 deletions(-) diff --git a/src/Plugin/dgi_image_discovery/url_generator/Deferred.php b/src/Plugin/dgi_image_discovery/url_generator/Deferred.php index 880d676..f26962e 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/Deferred.php +++ b/src/Plugin/dgi_image_discovery/url_generator/Deferred.php @@ -20,7 +20,7 @@ label: new TranslatableMarkup("Deferred URL resolution"), description: new TranslatableMarkup("Generate URLs making use of our deferred resolution endpoint."), )] -final class Deferred extends UrlGeneratorPluginBase { +class Deferred extends UrlGeneratorPluginBase { /** * {@inheritDoc} diff --git a/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php b/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php index e758337..cb814fe 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php +++ b/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php @@ -23,7 +23,7 @@ label: new TranslatableMarkup("Pre-generated URLs"), description: new TranslatableMarkup("Generate full URLs to styled images that should be used. Can be problematic with cache invalidation and/or indexing."), )] -final class PreGenerated extends UrlGeneratorPluginBase implements ContainerFactoryPluginInterface { +class PreGenerated extends UrlGeneratorPluginBase implements ContainerFactoryPluginInterface { use UrlGenerationTrait; @@ -45,30 +45,7 @@ final class PreGenerated extends UrlGeneratorPluginBase implements ContainerFact * {@inheritDoc} */ public function generate(NodeInterface $node, ImageStyleInterface $style): GeneratedUrl { - $generated_url = (new GeneratedUrl()) - ->addCacheableDependency($node) - ->addCacheableDependency($style); - - $event = $this->imageDiscovery->getImage($node); - $generated_url->addCacheableDependency($event); - $media = $event->getMedia(); - if (empty($media)) { - return $generated_url; - } - - $generated_url->addCacheableDependency($media); - - $media_source = $media->getSource(); - $file_id = $media_source->getSourceFieldValue($media); - /** @var \Drupal\file\FileInterface|null $image */ - $image = $this->entityTypeManager->getStorage('file')->load($file_id); - if (empty($image)) { - return $generated_url; - } - - $generated_url->addCacheableDependency($image); - - return $generated_url->setGeneratedUrl($style->buildUri($image->getFileUri())); + return $this->getGeneratedUrl($node, $style); } /** diff --git a/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php b/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php index a74af14..6a0c31d 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php +++ b/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php @@ -13,14 +13,14 @@ use Drupal\node\NodeInterface; /** - * Plugin implementation for deferred URL resolution. + * Plugin implementation for deferred URL resolution via redirect. */ #[DeferredResolution( id: "redirect", label: new TranslatableMarkup("Redirect to styled image URL"), description: new TranslatableMarkup("Explicitly redirect to the styled image URL"), )] -final class Redirect extends DeferredResolutionPluginBase { +class Redirect extends DeferredResolutionPluginBase { /** * {@inheritDoc} diff --git a/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php b/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php index b3f940e..44f95f0 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php +++ b/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php @@ -18,14 +18,14 @@ use Symfony\Component\HttpKernel\HttpKernelInterface; /** - * Plugin implementation for deferred URL resolution. + * Plugin implementation for deferred URL resolution via subrequest. */ #[DeferredResolution( id: "subrequest", label: new TranslatableMarkup("Subrequest to stream the image directly."), description: new TranslatableMarkup("Perform a subrequest to stream the image directly."), )] -final class Subrequest extends DeferredResolutionPluginBase { +class Subrequest extends DeferredResolutionPluginBase { /** * Drupal's HTTP kernel. From 56bd4dbaebedebd6ab2a2ee24a10e6ca5a26b5d4 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 19 Jun 2024 14:03:01 -0300 Subject: [PATCH 04/11] Basic integration. Redirect stuff seems to work. --- dgi_image_discovery.services.yml | 5 +- .../DeferredResolutionController.php | 24 ++++++++-- src/DeferredResolutionPluginManager.php | 12 ++++- ...ferredResolutionPluginManagerInterface.php | 4 +- .../url_generator/Deferred.php | 4 +- .../url_generator/UrlGenerationTrait.php | 2 +- .../url_generator/deferred/Redirect.php | 2 +- .../url_generator/deferred/Subrequest.php | 2 +- .../processor/DgiImageDiscovery.php | 48 +++++++++++-------- .../Property/DgiImageDiscoveryProperty.php | 7 +++ src/UrlGeneratorPluginManager.php | 10 +++- src/UrlGeneratorPluginManagerInterface.php | 12 +++++ 12 files changed, 99 insertions(+), 33 deletions(-) create mode 100644 src/UrlGeneratorPluginManagerInterface.php diff --git a/dgi_image_discovery.services.yml b/dgi_image_discovery.services.yml index d9231bd..d6d6b30 100644 --- a/dgi_image_discovery.services.yml +++ b/dgi_image_discovery.services.yml @@ -27,9 +27,12 @@ services: - name: event_subscriber dgi_image_discovery.deferred_resolution_controller: class: '\Drupal\dgi_image_discovery\Controller\DeferredResolutionController' + factory: [null, 'create'] + arguments: + - '@service_container' plugin.manager.dgi_image_discovery.url_generator: class: Drupal\dgi_image_discovery\UrlGeneratorPluginManager parent: default_plugin_manager plugin.manager.dgi_image_discovery.url_generator.deferred: - class: Drupal\dgi_image_discovery\DeferredUrlGeneratorPluginManager + class: Drupal\dgi_image_discovery\DeferredResolutionPluginManager parent: default_plugin_manager diff --git a/src/Controller/DeferredResolutionController.php b/src/Controller/DeferredResolutionController.php index 92b3eea..a2e911d 100644 --- a/src/Controller/DeferredResolutionController.php +++ b/src/Controller/DeferredResolutionController.php @@ -4,6 +4,8 @@ use Drupal\Core\Cache\CacheableResponseInterface; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; +use Drupal\Core\Render\RenderContext; +use Drupal\Core\Render\RendererInterface; use Drupal\dgi_image_discovery\DeferredResolutionPluginManagerInterface; use Drupal\image\ImageStyleInterface; use Drupal\node\NodeInterface; @@ -19,6 +21,7 @@ class DeferredResolutionController implements ContainerInjectionInterface { */ public function __construct( protected DeferredResolutionPluginManagerInterface $deferredResolutionPluginManager, + protected RendererInterface $renderer, ) {} /** @@ -27,6 +30,7 @@ public function __construct( public static function create(ContainerInterface $container) { return new static( $container->get('plugin.manager.dgi_image_discovery.url_generator.deferred'), + $container->get('renderer'), ); } @@ -44,11 +48,23 @@ public static function create(ContainerInterface $container) { * @throws \Drupal\Component\Plugin\Exception\PluginException */ public function resolve(ImageStyleInterface $style, NodeInterface $node) : CacheableResponseInterface { - // @todo Make plugin configurable. - /** @var \Drupal\dgi_image_discovery\DeferredResolutionInterface $plugin */ - $plugin = $this->deferredResolutionPluginManager->createInstance('redirect'); + $context = new RenderContext(); + /** @var \Drupal\Core\Cache\CacheableResponseInterface $response */ + $response = $this->renderer->executeInRenderContext($context, function () use ($style, $node) { + // @todo Make plugin configurable. + /** @var \Drupal\dgi_image_discovery\DeferredResolutionInterface $plugin */ + $plugin = $this->deferredResolutionPluginManager->createInstance('redirect'); + + return $plugin->resolve($node, $style); + }); + + if (!$context->isEmpty()) { + $metadata = $context->pop(); + $response->addCacheableDependency($metadata); + } + + return $response; - return $plugin->resolve($node, $style); } } diff --git a/src/DeferredResolutionPluginManager.php b/src/DeferredResolutionPluginManager.php index 5ce5266..cf21730 100644 --- a/src/DeferredResolutionPluginManager.php +++ b/src/DeferredResolutionPluginManager.php @@ -4,6 +4,7 @@ namespace Drupal\dgi_image_discovery; +use Drupal\Component\Plugin\FallbackPluginManagerInterface; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Plugin\DefaultPluginManager; @@ -12,15 +13,22 @@ /** * Deferred resolution plugin manager. */ -final class DeferredResolutionPluginManager extends DefaultPluginManager implements DeferredResolutionPluginManagerInterface { +final class DeferredResolutionPluginManager extends DefaultPluginManager implements DeferredResolutionPluginManagerInterface, FallbackPluginManagerInterface { /** * Constructs the object. */ public function __construct(\Traversable $namespaces, CacheBackendInterface $cache_backend, ModuleHandlerInterface $module_handler) { - parent::__construct('Plugin/dgi_image_discovery/url_generator/deferred', $namespaces, $module_handler, UrlGeneratorInterface::class, DeferredResolution::class); + parent::__construct('Plugin/dgi_image_discovery/url_generator/deferred', $namespaces, $module_handler, DeferredResolutionInterface::class, DeferredResolution::class); $this->alterInfo('dgi_image_discovery__url_generator_info__deferred'); $this->setCacheBackend($cache_backend, 'dgi_image_discovery__url_generator_plugins__deferred'); } + /** + * {@inheritDoc} + */ + public function getFallbackPluginId($plugin_id, array $configuration = []) : string { + return 'redirect'; + } + } diff --git a/src/DeferredResolutionPluginManagerInterface.php b/src/DeferredResolutionPluginManagerInterface.php index d370b26..480cbf2 100644 --- a/src/DeferredResolutionPluginManagerInterface.php +++ b/src/DeferredResolutionPluginManagerInterface.php @@ -2,9 +2,11 @@ namespace Drupal\dgi_image_discovery; +use Drupal\Component\Plugin\PluginManagerInterface; + /** * Deferred resolution plugin manager interface definition. */ -interface DeferredResolutionPluginManagerInterface { +interface DeferredResolutionPluginManagerInterface extends PluginManagerInterface { } diff --git a/src/Plugin/dgi_image_discovery/url_generator/Deferred.php b/src/Plugin/dgi_image_discovery/url_generator/Deferred.php index f26962e..0a85649 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/Deferred.php +++ b/src/Plugin/dgi_image_discovery/url_generator/Deferred.php @@ -29,8 +29,8 @@ public function generate(NodeInterface $node, ImageStyleInterface $style): Gener return Url::fromRoute( 'dgi_image_discovery.deferred_resolution', [ - 'node' => $node, - 'style' => $style, + 'node' => $node->id(), + 'style' => $style->id(), ], [ 'absolute' => TRUE, diff --git a/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php b/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php index 16b5145..74c948e 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php +++ b/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php @@ -67,7 +67,7 @@ public function getGeneratedUrl(NodeInterface $node, ImageStyleInterface $style) $generated_url->addCacheableDependency($image); - return $generated_url->setGeneratedUrl($style->buildUri($image->getFileUri())); + return $generated_url->setGeneratedUrl($style->buildUrl($image->getFileUri())); } } diff --git a/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php b/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php index 6a0c31d..e6327b3 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php +++ b/src/Plugin/dgi_image_discovery/url_generator/deferred/Redirect.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Drupal\dgi_image_discovery\Plugin\dgi_image_discovery\url_generator; +namespace Drupal\dgi_image_discovery\Plugin\dgi_image_discovery\url_generator\deferred; use Drupal\Core\Cache\CacheableRedirectResponse; use Drupal\Core\Cache\CacheableResponseInterface; diff --git a/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php b/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php index 44f95f0..320a00e 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php +++ b/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Drupal\dgi_image_discovery\Plugin\dgi_image_discovery\url_generator; +namespace Drupal\dgi_image_discovery\Plugin\dgi_image_discovery\url_generator\deferred; use Drupal\Core\Cache\CacheableResponseInterface; use Drupal\Core\Http\Exception\CacheableHttpException; diff --git a/src/Plugin/search_api/processor/DgiImageDiscovery.php b/src/Plugin/search_api/processor/DgiImageDiscovery.php index 6084c40..ec837c4 100644 --- a/src/Plugin/search_api/processor/DgiImageDiscovery.php +++ b/src/Plugin/search_api/processor/DgiImageDiscovery.php @@ -6,6 +6,7 @@ use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\dgi_image_discovery\ImageDiscoveryInterface; use Drupal\dgi_image_discovery\Plugin\search_api\processor\Property\DgiImageDiscoveryProperty; +use Drupal\dgi_image_discovery\UrlGeneratorPluginManagerInterface; use Drupal\node\NodeInterface; use Drupal\search_api\Datasource\DatasourceInterface; use Drupal\search_api\Item\ItemInterface; @@ -51,6 +52,7 @@ public function __construct( $plugin_definition, ImageDiscoveryInterface $image_discovery, EntityTypeManagerInterface $entity_type_manager, + protected UrlGeneratorPluginManagerInterface $urlGeneratorPluginManager, ) { parent::__construct($configuration, $plugin_id, $plugin_definition); @@ -67,7 +69,8 @@ public static function create(ContainerInterface $container, array $configuratio $plugin_id, $plugin_definition, $container->get('dgi_image_discovery.service'), - $container->get('entity_type.manager') + $container->get('entity_type.manager'), + $container->get('plugin.manager.dgi_image_discovery.url_generator'), ); } @@ -84,6 +87,7 @@ public function getPropertyDefinitions(DatasourceInterface $datasource = NULL) { 'type' => 'string', 'is_list' => FALSE, 'processor_id' => $this->getPluginId(), + 'dgi_image_discovery__url_generator_options' => $this->getGeneratorOptions(), ]; $properties['dgi_image_discovery'] = new DgiImageDiscoveryProperty($definition); } @@ -91,36 +95,42 @@ public function getPropertyDefinitions(DatasourceInterface $datasource = NULL) { return $properties; } + /** + * Helper; get listing of generators for use as form options. + * + * @return string[] + * An array mapping plugin IDs to human-readable strings. + */ + protected function getGeneratorOptions() : array { + $options = []; + + foreach ($this->urlGeneratorPluginManager->getDefinitions() as $plugin_id => $definition) { + $options[$plugin_id] = $definition['label']; + } + + return $options; + } + /** * {@inheritdoc} */ public function addFieldValues(ItemInterface $item) { $entity = $item->getOriginalObject()->getValue(); - $value = NULL; // Get the image discovery URL. if (!$entity->isNew() && $entity instanceof NodeInterface) { - $event = $this->imageDiscovery->getImage($entity); - $media = $event->getMedia(); - if (empty($media)) { - return; - } - - $media_source = $media->getSource(); - $file_id = $media_source->getSourceFieldValue($media); - $image = $this->entityTypeManager->getStorage('file')->load($file_id); - if (empty($image)) { - return; - } - $fields = $item->getFields(FALSE); $fields = $this->getFieldsHelper()->filterForPropertyPath($fields, NULL, 'dgi_image_discovery'); foreach ($fields as $field) { $config = $field->getConfiguration(); - $image_style = $config['image_style']; - $value = $this->entityTypeManager->getStorage('image_style')->load($image_style) - ->buildUrl($image->getFileUri()); - $field->addValue($value); + /** @var \Drupal\image\ImageStyleInterface $image_style */ + $image_style = $this->entityTypeManager->getStorage('image_style')->load($config['image_style']); + /** @var \Drupal\dgi_image_discovery\UrlGeneratorInterface $url_generator */ + $url_generator = $this->urlGeneratorPluginManager->createInstance($config['url_generator'] ?? 'pre_generated'); + $generated_url = $url_generator->generate($entity, $image_style); + if ($generated_url) { + $field->addValue($generated_url->getGeneratedUrl()); + } } } } diff --git a/src/Plugin/search_api/processor/Property/DgiImageDiscoveryProperty.php b/src/Plugin/search_api/processor/Property/DgiImageDiscoveryProperty.php index 3a22b30..20ba26e 100644 --- a/src/Plugin/search_api/processor/Property/DgiImageDiscoveryProperty.php +++ b/src/Plugin/search_api/processor/Property/DgiImageDiscoveryProperty.php @@ -20,6 +20,7 @@ class DgiImageDiscoveryProperty extends ConfigurablePropertyBase { public function defaultConfiguration() { return [ 'image_style' => 'solr_grid_thumbnail', + 'url_generator' => 'pre_generated', ]; } @@ -36,6 +37,12 @@ public function buildConfigurationForm(FieldInterface $field, array $form, FormS '#description' => $this->t('Select the image style that should be applied to derive the DGI Image Discovery image url.'), '#default_value' => $configuration['image_style'], ]; + $form['url_generator'] = [ + '#type' => 'select', + '#options' => $this->definition['dgi_image_discovery__url_generator_options'], + '#title' => $this->t('URL Generator'), + '#default_value' => $configuration['url_generator'], + ]; return $form; } diff --git a/src/UrlGeneratorPluginManager.php b/src/UrlGeneratorPluginManager.php index 412dbbc..3578fe8 100644 --- a/src/UrlGeneratorPluginManager.php +++ b/src/UrlGeneratorPluginManager.php @@ -4,6 +4,7 @@ namespace Drupal\dgi_image_discovery; +use Drupal\Component\Plugin\FallbackPluginManagerInterface; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Plugin\DefaultPluginManager; @@ -12,7 +13,7 @@ /** * UrlGenerator plugin manager. */ -final class UrlGeneratorPluginManager extends DefaultPluginManager { +final class UrlGeneratorPluginManager extends DefaultPluginManager implements UrlGeneratorPluginManagerInterface, FallbackPluginManagerInterface { /** * Constructs the object. @@ -23,4 +24,11 @@ public function __construct(\Traversable $namespaces, CacheBackendInterface $cac $this->setCacheBackend($cache_backend, 'dgi_image_discovery__url_generator_plugins'); } + /** + * {@inheritDoc} + */ + public function getFallbackPluginId($plugin_id, array $configuration = []) : string { + return 'pre_generated'; + } + } diff --git a/src/UrlGeneratorPluginManagerInterface.php b/src/UrlGeneratorPluginManagerInterface.php new file mode 100644 index 0000000..fc236a0 --- /dev/null +++ b/src/UrlGeneratorPluginManagerInterface.php @@ -0,0 +1,12 @@ + Date: Mon, 24 Jun 2024 14:45:00 -0300 Subject: [PATCH 05/11] Get subrequest stuff working. --- README.md | 24 ++++++- dgi_image_discovery.routing.yml | 2 +- src/CacheableBinaryFileResponse.php | 8 +-- .../DeferredResolutionController.php | 4 +- .../url_generator/deferred/Subrequest.php | 72 ++++++++++++++++--- 5 files changed, 92 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 264c02a..3510fb7 100644 --- a/README.md +++ b/README.md @@ -19,9 +19,29 @@ further information. ## Usage This module allow for image discovery on parent aggregate objects such as -collections, compounds and paged objects. +collections, compounds and paged objects in multiple context. -## Configuration +### Search API + +Search API can be made to index URLs to the discovered image in multiple ways: + +- `deferred`: Create URL to dedicated endpoint which can handle the final image lookup. Given responses here can be aware of Drupal's cache tag invalidations, we can accordingly change what is ultimately served. +- `pre_generated`: Creates URL to styled image directly. May cause stale references to stick in the index, due to changing access control constraints. + +#### Deferral mechanism + +There are multiple plugins to dereference deferred URLs: + +- `redirect`: Issue a redirect to the final derived image destination from our endpoint. Easily enough done; however: + - incurs another round trip + - can cause a race condition if two items being displayed in a set of results happen to reference the same image. Drupal maintains a lock/semaphore around the image derivation: If the second request occurs while the first still has the lock for deriving the image, then the second request will receive an HTTP 503 with `Retry-After` of `3` seconds, but many browsers do not make use of the `Retry-After` header. +- `subrequest`: Perform subrequest to stream the image directly from our endpoint. Can deal with the 503 with `Retry-After`. + +The plugin in use is presently controlled with the `DGI_IMAGE_DISCOVERY_DEFERRED_PLUGIN`, which defaults to `subrequest`. + +### Views + +Views referencing node content can directly make use of a virtual field. When configuring a content view, add and configure the virtual field "DGI Image Discovery Discovered Image". diff --git a/dgi_image_discovery.routing.yml b/dgi_image_discovery.routing.yml index e19c4f4..fc32815 100644 --- a/dgi_image_discovery.routing.yml +++ b/dgi_image_discovery.routing.yml @@ -1,6 +1,6 @@ --- dgi_image_discovery.deferred_resolution: - path: '/dgi_image_discovery/{style}/{node}' + path: '/node/{node}/dgi_image_discovery/{style}' defaults: _controller: 'dgi_image_discovery.deferred_resolution_controller:resolve' requirements: diff --git a/src/CacheableBinaryFileResponse.php b/src/CacheableBinaryFileResponse.php index 0f7913b..95ff1dd 100644 --- a/src/CacheableBinaryFileResponse.php +++ b/src/CacheableBinaryFileResponse.php @@ -40,11 +40,9 @@ public function setFile(\SplFileInfo|string $file, ?string $contentDisposition = * {@inheritDoc} */ public function __sleep() { - $vars = $this->traitSleep(); - - unset($vars['file']); - - return $vars; + return array_diff($this->traitSleep(), [ + 'file', + ]); } /** diff --git a/src/Controller/DeferredResolutionController.php b/src/Controller/DeferredResolutionController.php index a2e911d..c3f2e67 100644 --- a/src/Controller/DeferredResolutionController.php +++ b/src/Controller/DeferredResolutionController.php @@ -51,9 +51,9 @@ public function resolve(ImageStyleInterface $style, NodeInterface $node) : Cache $context = new RenderContext(); /** @var \Drupal\Core\Cache\CacheableResponseInterface $response */ $response = $this->renderer->executeInRenderContext($context, function () use ($style, $node) { - // @todo Make plugin configurable. + // @todo Make plugin configurable? /** @var \Drupal\dgi_image_discovery\DeferredResolutionInterface $plugin */ - $plugin = $this->deferredResolutionPluginManager->createInstance('redirect'); + $plugin = $this->deferredResolutionPluginManager->createInstance(getenv('DGI_IMAGE_DISCOVERY_DEFERRED_PLUGIN') ?: 'subrequest'); return $plugin->resolve($node, $style); }); diff --git a/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php b/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php index 320a00e..595b32a 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php +++ b/src/Plugin/dgi_image_discovery/url_generator/deferred/Subrequest.php @@ -5,6 +5,7 @@ namespace Drupal\dgi_image_discovery\Plugin\dgi_image_discovery\url_generator\deferred; use Drupal\Core\Cache\CacheableResponseInterface; +use Drupal\Core\GeneratedUrl; use Drupal\Core\Http\Exception\CacheableHttpException; use Drupal\Core\StringTranslation\TranslatableMarkup; use Drupal\dgi_image_discovery\Attribute\DeferredResolution; @@ -15,6 +16,8 @@ use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\BinaryFileResponse; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\HttpKernelInterface; /** @@ -34,22 +37,74 @@ class Subrequest extends DeferredResolutionPluginBase { */ protected HttpKernelInterface $httpKernel; + /** + * Current stack of requests being served. + * + * @var \Symfony\Component\HttpFoundation\RequestStack + */ + protected RequestStack $requestStack; + /** * {@inheritDoc} */ public function resolve(NodeInterface $node, ImageStyleInterface $style): CacheableResponseInterface { $generated_url = $this->getGeneratedUrl($node, $style); + $response = NULL; - $current_request = $this->httpKernel->getRequest(); - $request = Request::create($generated_url->getGeneratedUrl()); - $request->setSession($current_request->getSession()); - $response = $this->httpKernel->handle($request, HttpKernelInterface::SUB_REQUEST); - if ($response instanceof BinaryFileResponse) { - return (CacheableBinaryFileResponse::convert($response)) - ->addCacheableDependency($generated_url); + $attempts = 3; + while ($attempts-- > 0) { + $current_request = $this->requestStack->getCurrentRequest(); + $request = Request::create($generated_url->getGeneratedUrl()); + $request->setSession($current_request->getSession()); + $response = $this->httpKernel->handle($request, HttpKernelInterface::SUB_REQUEST); + if ($response instanceof BinaryFileResponse) { + return CacheableBinaryFileResponse::convert($response) + ->setAutoEtag() + ->setAutoLastModified() + ->addCacheableDependency($generated_url); + } + elseif ($response->getStatusCode() === 503) { + $after = $response->headers->get('Retry-After'); + if ($after === NULL) { + // No value. + throw $this->getExceptionFromResponse($response, $generated_url); + } + $after = intval($after); + if ($after > 0 && $after <= 5) { + // If we have a requested time for which we are willing to wait, let's + // wait it out. Image derivation indicates 3 seconds, but let's allow + // up to 5 in the case of some other process requesting such, just to + // be nice. + sleep($after); + continue; + } + } + throw $this->getExceptionFromResponse($response, $generated_url); } - throw new CacheableHttpException($generated_url, $response->getStatusCode(), $response->getContent()); + throw $this->getExceptionFromResponse($response, $generated_url); + } + + /** + * Helper; build out cachable exception from the given response for the URL. + * + * @param \Symfony\Component\HttpFoundation\Response $response + * The response for which to build an exception. + * @param \Drupal\Core\GeneratedUrl $generated_url + * The URL related to the response. + * + * @return \Drupal\Core\Http\Exception\CacheableHttpException + * The built exception. + */ + protected function getExceptionFromResponse(Response $response, GeneratedUrl $generated_url) : CacheableHttpException { + if ($after = $response->headers->get('Retry-After')) { + $generated_url->mergeCacheMaxAge($after); + } + return new CacheableHttpException( + $generated_url, + $response->getStatusCode() ?? 500, + $response->getContent() ?? '', + ); } /** @@ -59,6 +114,7 @@ public static function create(ContainerInterface $container, array $configuratio $instance = parent::create($container, $configuration, $plugin_id, $plugin_definition); $instance->httpKernel = $container->get('http_kernel'); + $instance->requestStack = $container->get('request_stack'); return $instance; } From 12934b9ba57d7d23ed47b515c7e8e0a76fb27aca Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 24 Jun 2024 14:57:22 -0300 Subject: [PATCH 06/11] Flesh out README some more. --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 904961a..21854c4 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,8 @@ Search API can be made to index URLs to the discovered image in multiple ways: - `deferred`: Create URL to dedicated endpoint which can handle the final image lookup. Given responses here can be aware of Drupal's cache tag invalidations, we can accordingly change what is ultimately served. - `pre_generated`: Creates URL to styled image directly. May cause stale references to stick in the index, due to changing access control constraints. +This is configurable on the field when it is added to be indexed. Effectively this defaults to `pre_generated` to maintain existing/current behaviour; however, `deferred` should possibly be preferred without other mechanisms to perform bulk reindexing due to changes on other entities. In particular, should there be something such as [Embargo](https://github.com/discoverygarden/embargo) and [Embargo Inheritance](https://github.com/discoverygarden/embargo_inheritance), where an access control statement applied to a parent node is expected to be applied to children. That said, `pre_generated` could be more convenient/efficient when there are no complex access control requirements in play. + #### Deferral mechanism There are multiple plugins to dereference deferred URLs: From 33f6de084637b96cd6b4ec12abd60ded9fb5edfa Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 24 Jun 2024 15:51:17 -0300 Subject: [PATCH 07/11] Slight simplification. --- src/ImageDiscovery.php | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/ImageDiscovery.php b/src/ImageDiscovery.php index 3b455c0..1308b05 100644 --- a/src/ImageDiscovery.php +++ b/src/ImageDiscovery.php @@ -11,20 +11,13 @@ */ class ImageDiscovery implements ImageDiscoveryInterface { - /** - * The event dispatcher service. - * - * @var \Symfony\Contracts\EventDispatcher\EventDispatcherInterface - */ - protected EventDispatcherInterface $eventDispatcher; - /** * Constructor. */ public function __construct( - EventDispatcherInterface $event_dispatcher, + protected EventDispatcherInterface $eventDispatcher, ) { - $this->eventDispatcher = $event_dispatcher; + // No-op, other than setting property. } /** From 08dc5f5c0ad816037d29b25e052dc8df3ab03725 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 27 Jun 2024 13:03:03 -0300 Subject: [PATCH 08/11] The use of attribute-based plugins was only introduced with 10.2. See: https://www.drupal.org/docs/drupal-apis/plugin-api/attribute-based-plugins --- composer.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/composer.json b/composer.json index 0b2bd87..17f2f84 100644 --- a/composer.json +++ b/composer.json @@ -4,5 +4,8 @@ "license": "GPL-3.0-only", "require": { "drupal/search_api": "^1.19" + }, + "conflict": { + "drupal/core": "<10.2" } } From d0db0171e2bc14ce26dfa1f6c79ce8687c757230 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 8 Jul 2024 10:44:34 -0300 Subject: [PATCH 09/11] Throw exceptions when failing lookup an image, to instead 404. Could possibly tie-in to the fast 404 stuff, but such isn't configured for use yet in the project. --- .../dgi_image_discovery/url_generator/UrlGenerationTrait.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php b/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php index 74c948e..a179653 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php +++ b/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php @@ -4,6 +4,7 @@ use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\GeneratedUrl; +use Drupal\Core\Http\Exception\CacheableHttpException; use Drupal\dgi_image_discovery\ImageDiscoveryInterface; use Drupal\image\ImageStyleInterface; use Drupal\node\NodeInterface; @@ -52,7 +53,7 @@ public function getGeneratedUrl(NodeInterface $node, ImageStyleInterface $style) $generated_url->addCacheableDependency($event); $media = $event->getMedia(); if (empty($media)) { - return $generated_url; + throw new CacheableHttpException($generated_url, 404, "No media discovered for node ({$node->id()})."); } $generated_url->addCacheableDependency($media); @@ -62,7 +63,7 @@ public function getGeneratedUrl(NodeInterface $node, ImageStyleInterface $style) /** @var \Drupal\file\FileInterface|null $image */ $image = $this->getEntityTypeManager()->getStorage('file')->load($file_id); if (empty($image)) { - return $generated_url; + throw new CacheableHttpException($generated_url, 404, "File ID ({$file_id}) discovered for node ({$node->id()}) could not be loaded."); } $generated_url->addCacheableDependency($image); From 31961c37bd35b5b932994041bdefaff0fb1de0cd Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 8 Jul 2024 11:27:09 -0300 Subject: [PATCH 10/11] Cache/simplify error response. --- src/Controller/DeferredResolutionController.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Controller/DeferredResolutionController.php b/src/Controller/DeferredResolutionController.php index c3f2e67..fc89bb5 100644 --- a/src/Controller/DeferredResolutionController.php +++ b/src/Controller/DeferredResolutionController.php @@ -2,8 +2,10 @@ namespace Drupal\dgi_image_discovery\Controller; +use Drupal\Core\Cache\CacheableResponse; use Drupal\Core\Cache\CacheableResponseInterface; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; +use Drupal\Core\Http\Exception\CacheableHttpException; use Drupal\Core\Render\RenderContext; use Drupal\Core\Render\RendererInterface; use Drupal\dgi_image_discovery\DeferredResolutionPluginManagerInterface; @@ -55,7 +57,13 @@ public function resolve(ImageStyleInterface $style, NodeInterface $node) : Cache /** @var \Drupal\dgi_image_discovery\DeferredResolutionInterface $plugin */ $plugin = $this->deferredResolutionPluginManager->createInstance(getenv('DGI_IMAGE_DISCOVERY_DEFERRED_PLUGIN') ?: 'subrequest'); - return $plugin->resolve($node, $style); + try { + return $plugin->resolve($node, $style); + } + catch (CacheableHttpException $e) { + return (new CacheableResponse($e->getMessage(), $e->getStatusCode())) + ->addCacheableDependency($e); + } }); if (!$context->isEmpty()) { From 184b392104ffa5b7a76a246a9b01644c3457697f Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 10 Jul 2024 16:04:01 -0300 Subject: [PATCH 11/11] Use more specific exception, and prevent explosions with pre-generation. --- .../dgi_image_discovery/url_generator/PreGenerated.php | 10 ++++++++-- .../url_generator/UrlGenerationTrait.php | 7 ++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php b/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php index cb814fe..bff3226 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php +++ b/src/Plugin/dgi_image_discovery/url_generator/PreGenerated.php @@ -14,6 +14,7 @@ use Drupal\image\ImageStyleInterface; use Drupal\node\NodeInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** * Plugin implementation for pre-generated URLs. @@ -44,8 +45,13 @@ class PreGenerated extends UrlGeneratorPluginBase implements ContainerFactoryPlu /** * {@inheritDoc} */ - public function generate(NodeInterface $node, ImageStyleInterface $style): GeneratedUrl { - return $this->getGeneratedUrl($node, $style); + public function generate(NodeInterface $node, ImageStyleInterface $style): ?GeneratedUrl { + try { + return $this->getGeneratedUrl($node, $style); + } + catch (NotFoundHttpException) { + return NULL; + } } /** diff --git a/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php b/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php index a179653..c03e7f2 100644 --- a/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php +++ b/src/Plugin/dgi_image_discovery/url_generator/UrlGenerationTrait.php @@ -4,7 +4,7 @@ use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\GeneratedUrl; -use Drupal\Core\Http\Exception\CacheableHttpException; +use Drupal\Core\Http\Exception\CacheableNotFoundHttpException; use Drupal\dgi_image_discovery\ImageDiscoveryInterface; use Drupal\image\ImageStyleInterface; use Drupal\node\NodeInterface; @@ -43,6 +43,7 @@ abstract protected function getEntityTypeManager() : EntityTypeManagerInterface; * * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException + * @throws \Drupal\Core\Http\Exception\CacheableNotFoundHttpException */ public function getGeneratedUrl(NodeInterface $node, ImageStyleInterface $style) : GeneratedUrl { $generated_url = (new GeneratedUrl()) @@ -53,7 +54,7 @@ public function getGeneratedUrl(NodeInterface $node, ImageStyleInterface $style) $generated_url->addCacheableDependency($event); $media = $event->getMedia(); if (empty($media)) { - throw new CacheableHttpException($generated_url, 404, "No media discovered for node ({$node->id()})."); + throw new CacheableNotFoundHttpException($generated_url, "No media discovered for node ({$node->id()})."); } $generated_url->addCacheableDependency($media); @@ -63,7 +64,7 @@ public function getGeneratedUrl(NodeInterface $node, ImageStyleInterface $style) /** @var \Drupal\file\FileInterface|null $image */ $image = $this->getEntityTypeManager()->getStorage('file')->load($file_id); if (empty($image)) { - throw new CacheableHttpException($generated_url, 404, "File ID ({$file_id}) discovered for node ({$node->id()}) could not be loaded."); + throw new CacheableNotFoundHttpException($generated_url, "File ID ({$file_id}) discovered for node ({$node->id()}) could not be loaded."); } $generated_url->addCacheableDependency($image);