Merge pull request #52221 from nextcloud/feat/no-issue/add-logging-preview-generation

feat: add logging to preview generation
fix/allow-255-filenames
F. E Noel Nfebe 2025-04-22 12:37:49 +07:00 committed by GitHub
commit 25bc18c6dc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 40 additions and 26 deletions

@ -1,4 +1,5 @@
<?php <?php
/** /**
* SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later * SPDX-License-Identifier: AGPL-3.0-or-later
@ -20,34 +21,20 @@ use OCP\IStreamImage;
use OCP\Preview\BeforePreviewFetchedEvent; use OCP\Preview\BeforePreviewFetchedEvent;
use OCP\Preview\IProviderV2; use OCP\Preview\IProviderV2;
use OCP\Preview\IVersionedPreviewFile; use OCP\Preview\IVersionedPreviewFile;
use Psr\Log\LoggerInterface;
class Generator { class Generator {
public const SEMAPHORE_ID_ALL = 0x0a11; public const SEMAPHORE_ID_ALL = 0x0a11;
public const SEMAPHORE_ID_NEW = 0x07ea; public const SEMAPHORE_ID_NEW = 0x07ea;
/** @var IPreview */
private $previewManager;
/** @var IConfig */
private $config;
/** @var IAppData */
private $appData;
/** @var GeneratorHelper */
private $helper;
/** @var IEventDispatcher */
private $eventDispatcher;
public function __construct( public function __construct(
IConfig $config, private IConfig $config,
IPreview $previewManager, private IPreview $previewManager,
IAppData $appData, private IAppData $appData,
GeneratorHelper $helper, private GeneratorHelper $helper,
IEventDispatcher $eventDispatcher, private IEventDispatcher $eventDispatcher,
private LoggerInterface $logger,
) { ) {
$this->config = $config;
$this->previewManager = $previewManager;
$this->appData = $appData;
$this->helper = $helper;
$this->eventDispatcher = $eventDispatcher;
} }
/** /**
@ -83,6 +70,16 @@ class Generator {
$mimeType, $mimeType,
)); ));
$this->logger->debug('Requesting preview for {path} with width={width}, height={height}, crop={crop}, mode={mode}, mimeType={mimeType}', [
'path' => $file->getPath(),
'width' => $width,
'height' => $height,
'crop' => $crop,
'mode' => $mode,
'mimeType' => $mimeType,
]);
// since we only ask for one preview, and the generate method return the last one it created, it returns the one we want // since we only ask for one preview, and the generate method return the last one it created, it returns the one we want
return $this->generatePreviews($file, [$specification], $mimeType); return $this->generatePreviews($file, [$specification], $mimeType);
} }
@ -100,6 +97,7 @@ class Generator {
public function generatePreviews(File $file, array $specifications, $mimeType = null) { public function generatePreviews(File $file, array $specifications, $mimeType = null) {
//Make sure that we can read the file //Make sure that we can read the file
if (!$file->isReadable()) { if (!$file->isReadable()) {
$this->logger->warning('Cannot read file: {path}, skipping preview generation.', ['path' => $file->getPath()]);
throw new NotFoundException('Cannot read file'); throw new NotFoundException('Cannot read file');
} }
@ -121,6 +119,7 @@ class Generator {
$maxPreviewImage = null; // only load the image when we need it $maxPreviewImage = null; // only load the image when we need it
if ($maxPreview->getSize() === 0) { if ($maxPreview->getSize() === 0) {
$maxPreview->delete(); $maxPreview->delete();
$this->logger->error("Max preview generated for file {$file->getPath()} has size 0, deleting and throwing exception.");
throw new NotFoundException('Max preview size 0, invalid!'); throw new NotFoundException('Max preview size 0, invalid!');
} }
@ -167,6 +166,7 @@ class Generator {
$maxPreviewImage = $this->helper->getImage($maxPreview); $maxPreviewImage = $this->helper->getImage($maxPreview);
} }
$this->logger->warning('Cached preview not found for file {path}, generating a new preview.', ['path' => $file->getPath()]);
$preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); $preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion);
// New file, augment our array // New file, augment our array
$previewFiles[] = $preview; $previewFiles[] = $preview;
@ -335,6 +335,11 @@ class Generator {
$previewConcurrency = $this->getNumConcurrentPreviews('preview_concurrency_new'); $previewConcurrency = $this->getNumConcurrentPreviews('preview_concurrency_new');
$sem = self::guardWithSemaphore(self::SEMAPHORE_ID_NEW, $previewConcurrency); $sem = self::guardWithSemaphore(self::SEMAPHORE_ID_NEW, $previewConcurrency);
try { try {
$this->logger->debug('Calling preview provider for {mimeType} with width={width}, height={height}', [
'mimeType' => $mimeType,
'width' => $width,
'height' => $height,
]);
$preview = $this->helper->getThumbnail($provider, $file, $width, $height); $preview = $this->helper->getThumbnail($provider, $file, $width, $height);
} finally { } finally {
self::unguardWithSemaphore($sem); self::unguardWithSemaphore($sem);
@ -558,6 +563,7 @@ class Generator {
$path = $this->generatePath($width, $height, $crop, false, $mimeType, $prefix); $path = $this->generatePath($width, $height, $crop, false, $mimeType, $prefix);
foreach ($files as $file) { foreach ($files as $file) {
if ($file->getName() === $path) { if ($file->getName() === $path) {
$this->logger->debug('Found cached preview: {path}', ['path' => $path]);
return $file; return $file;
} }
} }

@ -21,8 +21,10 @@ use OCP\Files\SimpleFS\ISimpleFile;
use OCP\IBinaryFinder; use OCP\IBinaryFinder;
use OCP\IConfig; use OCP\IConfig;
use OCP\IPreview; use OCP\IPreview;
use OCP\IServerContainer;
use OCP\Preview\IProviderV2; use OCP\Preview\IProviderV2;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
use function array_key_exists; use function array_key_exists;
class PreviewManager implements IPreview { class PreviewManager implements IPreview {
@ -47,7 +49,7 @@ class PreviewManager implements IPreview {
* @psalm-var array<string, null> * @psalm-var array<string, null>
*/ */
private array $loadedBootstrapProviders = []; private array $loadedBootstrapProviders = [];
private IServerContainer $container; private ContainerInterface $container;
private IBinaryFinder $binaryFinder; private IBinaryFinder $binaryFinder;
private IMagickSupport $imagickSupport; private IMagickSupport $imagickSupport;
private bool $enablePreviews; private bool $enablePreviews;
@ -60,7 +62,7 @@ class PreviewManager implements IPreview {
GeneratorHelper $helper, GeneratorHelper $helper,
?string $userId, ?string $userId,
Coordinator $bootstrapCoordinator, Coordinator $bootstrapCoordinator,
IServerContainer $container, ContainerInterface $container,
IBinaryFinder $binaryFinder, IBinaryFinder $binaryFinder,
IMagickSupport $imagickSupport, IMagickSupport $imagickSupport,
) { ) {
@ -136,7 +138,8 @@ class PreviewManager implements IPreview {
$this->rootFolder, $this->rootFolder,
$this->config $this->config
), ),
$this->eventDispatcher $this->eventDispatcher,
$this->container->get(LoggerInterface::class),
); );
} }
return $this->generator; return $this->generator;

@ -19,6 +19,7 @@ use OCP\IImage;
use OCP\IPreview; use OCP\IPreview;
use OCP\Preview\BeforePreviewFetchedEvent; use OCP\Preview\BeforePreviewFetchedEvent;
use OCP\Preview\IProviderV2; use OCP\Preview\IProviderV2;
use Psr\Log\LoggerInterface;
class GeneratorTest extends \Test\TestCase { class GeneratorTest extends \Test\TestCase {
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
@ -39,6 +40,8 @@ class GeneratorTest extends \Test\TestCase {
/** @var Generator */ /** @var Generator */
private $generator; private $generator;
private LoggerInterface|\PHPUnit\Framework\MockObject\MockObject $logger;
protected function setUp(): void { protected function setUp(): void {
parent::setUp(); parent::setUp();
@ -47,13 +50,15 @@ class GeneratorTest extends \Test\TestCase {
$this->appData = $this->createMock(IAppData::class); $this->appData = $this->createMock(IAppData::class);
$this->helper = $this->createMock(GeneratorHelper::class); $this->helper = $this->createMock(GeneratorHelper::class);
$this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->generator = new Generator( $this->generator = new Generator(
$this->config, $this->config,
$this->previewManager, $this->previewManager,
$this->appData, $this->appData,
$this->helper, $this->helper,
$this->eventDispatcher $this->eventDispatcher,
$this->logger,
); );
} }