Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH Improve quality of thumbnails in assetadmin and uploadfield #1447

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions _config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ SilverStripe\Core\Injector\Injector:
class: SilverStripe\AssetAdmin\Model\ThumbnailGenerator
properties:
Generates: true
thumbnailMethod: 'Fill'
SilverStripe\AssetAdmin\Controller\AssetAdmin:
properties:
ThumbnailGenerator: '%$SilverStripe\AssetAdmin\Model\ThumbnailGenerator.assetadmin'
Expand Down Expand Up @@ -53,3 +54,4 @@ SilverStripe\Core\Injector\Injector:
class: SilverStripe\AssetAdmin\Model\ThumbnailGenerator
properties:
Generates: false
thumbnailMethod: 'Fill'
4 changes: 2 additions & 2 deletions code/Forms/UploadField.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ class UploadField extends FormField implements FileHandleField
* @config
* @var int
*/
private static $thumbnail_width = 60;
private static $thumbnail_width = 120;

/**
* @config
* @var int
*/
private static $thumbnail_height = 60;
private static $thumbnail_height = 120;

/**
* Set if uploading new files is enabled.
Expand Down
21 changes: 18 additions & 3 deletions code/GraphQL/Resolvers/FileTypeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,27 @@ public static function resolveFileSmallThumbnail($object)
*/
public static function resolveFileThumbnail($object)
{
// If we're allowed to generate thumbnails for this file, tell the generator it's allowed to do it.
$generator = static::singleton()->getThumbnailGenerator();
$idsAllowed = FolderTypeResolver::getIdsAllowedToGenerateThumbnails();
$shouldGenerateThumbnail = !empty($idsAllowed) && in_array($object->ID, $idsAllowed);
if ($shouldGenerateThumbnail) {
$origGenerates = $generator->getGenerates();
$generator->setGenerates(true);
}

// Make large thumbnail
$width = AssetAdmin::config()->uninherited('thumbnail_width');
$height = AssetAdmin::config()->uninherited('thumbnail_height');
return static::singleton()
->getThumbnailGenerator()
->generateThumbnailLink($object, $width, $height);

try {
return $generator->generateThumbnailLink($object, $width, $height);
} finally {
// Make sure to set the generates value back to what it was, regardless of what happens
if ($shouldGenerateThumbnail) {
$generator->setGenerates($origGenerates);
}
}
}

/**
Expand Down
26 changes: 26 additions & 0 deletions code/GraphQL/Resolvers/FolderTypeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,20 @@
use InvalidArgumentException;
use Exception;
use Closure;
use SilverStripe\AssetAdmin\Controller\AssetAdmin;
use SilverStripe\ORM\DataQuery;

class FolderTypeResolver
{
/**
* Any IDs of files which we're allowed to generate thumbnails for.
* The intention is to (as much as is feasible) only allow generating thumbnails
* during asset admin graphQL requests and not for requests that could be performed
* by an attacker.
* @internal
*/
private static array $idsAllowedToGenerateThumbnails = [];

/**
* @param Folder $object
* @param array $args
Expand Down Expand Up @@ -73,6 +83,17 @@ public static function resolveFolderChildren(
$canViewList = $list->filter('ID', $canViewIDs ?: 0)
->limit(null);

// If we haven't already marked IDs as being okay to generate thumbnails,
// and we have a safe limit for the number of children,
// mark these children as being okay to generate thumbnails for.
if (empty(self::$idsAllowedToGenerateThumbnails)
&& !empty($args['limit'])
&& is_numeric($args['limit'])
&& (int)$args['limit'] <= AssetAdmin::config()->get('page_length')
) {
self::$idsAllowedToGenerateThumbnails = $canViewIDs;
}

return $canViewList;
}

Expand Down Expand Up @@ -181,4 +202,9 @@ public static function sortChildren(array $context): Closure
return $list;
};
}

public static function getIdsAllowedToGenerateThumbnails(): array
{
return self::$idsAllowedToGenerateThumbnails;
}
}
32 changes: 29 additions & 3 deletions code/Model/ThumbnailGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,16 @@ class ThumbnailGenerator
];

/**
* @var string
* @config
* @var string The default method to use for generating thumbnails if a specific method hasn't been
* set for a given generator instance.
*/
private static $method = 'FitMax';

/**
* The method that this generator instance will use to generate thumbnails.
*/
private string $thumbnailMethod = '';

/**
* Generate thumbnail and return the "src" property for this thumbnail
*
Expand Down Expand Up @@ -108,7 +113,7 @@ public function generateThumbnail(AssetContainer $file, $width, $height)
}

// Make large thumbnail
$method = $this->config()->get('method');
$method = $this->getThumbnailMethod();
return $file->$method($width, $height);
}

Expand Down Expand Up @@ -173,4 +178,25 @@ public function setGenerates($generates)
$this->generates = $generates;
return $this;
}

/**
* Get the method which will be used to generate thumbnails.
*/
public function getThumbnailMethod(): string
{
if ($this->thumbnailMethod) {
return $this->thumbnailMethod;
}
return static::config()->get('method');
}

/**
* Set the method which will be used to generate thumbnails.
* Pass an empty string to reset to the default method.
*/
public function setThumbnailMethod(string $method): static
{
$this->thumbnailMethod = $method;
return $this;
}
}
Loading