Skip to content

Commit 5430832

Browse files
committed
feat: allow multiple libreoffice invocations
LibreOffice only allows one invocation per user profile.[^1] The office provider set the user profile to /tmp/owncloud-instanceid and therefore only one invocation per instance is allowed. This was introduced a while ago, yet it's unclear if this was intentionally or just a side effect.[^2] The limitation on one invocation leads to the situation that the preview generation only works for a couple of files if you upload a whole folder of emf or word files. This commit removes the limitation by using a new user profile for each preview. That's done by using instance id plus file id as postfix for getTemporaryFolder. This has some drawbacks: - Overload protection: If you upload 100 emf files, you may end up with 100 LibreOffice invocations. Though, you can use preview_concurrency_new to limit the number of previews that can be generated concurrently when php-sysvsem is available. - New profile: I assume it takes a few bits to generate a fresh LibreOffice user profile. It appears that there is no way to ask LibreOffice to not create a profile and just work with the defaults. The profile will be cleaned after use by our temp manager. - Remove the configuration option preview_office_cl_parameters: This is not strictly necessary yet, but if you set the configuration option, the generated path for the user profile is also missing. The configuration option is not well documented (e.g., it's unclear that the last option needs to be --outdir) and actually, there should be no reason to change it after all. [^1]: https://wiki.documentfoundation.org/UserProfile [^2]: owncloud/core#9784 Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
1 parent aa48a5f commit 5430832

File tree

2 files changed

+34
-19
lines changed

2 files changed

+34
-19
lines changed

config/config.sample.php

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,14 +1237,6 @@
12371237
* Defaults to ``''`` (empty string)
12381238
*/
12391239
'preview_libreoffice_path' => '/usr/bin/libreoffice',
1240-
/**
1241-
* Use this if LibreOffice/OpenOffice requires additional arguments.
1242-
*
1243-
* Defaults to ``''`` (empty string)
1244-
*/
1245-
'preview_office_cl_parameters' =>
1246-
' --headless --nologo --nofirststartwizard --invisible --norestore '.
1247-
'--convert-to png --outdir ',
12481240

12491241
/**
12501242
* custom path for ffmpeg binary

lib/private/Preview/Office.php

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
use OCP\Files\File;
3232
use OCP\Files\FileInfo;
3333
use OCP\IImage;
34+
use OCP\ITempManager;
35+
use OCP\Server;
3436
use Psr\Log\LoggerInterface;
3537

3638
abstract class Office extends ProviderV2 {
@@ -49,33 +51,55 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
4951
return null;
5052
}
5153

54+
$tempManager = Server::get(ITempManager::class);
55+
56+
// The file to generate the preview for.
5257
$absPath = $this->getLocalFile($file);
5358

54-
$tmpDir = \OC::$server->getTempManager()->getTempBaseDir();
59+
// The destination for the libreoffice user profile.
60+
// Libreoffice can rune once per user profile and therefore instance id and file id are included.
61+
$profile = $tempManager->getTemporaryFolder(
62+
'nextcloud-office-profile-' . \OC_Util::getInstanceId() . '-' . $file->getId()
63+
);
64+
65+
// The destination for the libreoffice convert result.
66+
$outdir = $tempManager->getTemporaryFolder(
67+
'nextcloud-office-preview-' . \OC_Util::getInstanceId() . '-' . $file->getId()
68+
);
5569

56-
$defaultParameters = ' -env:UserInstallation=file://' . escapeshellarg($tmpDir . '/owncloud-' . \OC_Util::getInstanceId() . '/') . ' --headless --nologo --nofirststartwizard --invisible --norestore --convert-to png --outdir ';
57-
$clParameters = \OC::$server->getConfig()->getSystemValue('preview_office_cl_parameters', $defaultParameters);
70+
if ($profile === false || $outdir === false) {
71+
$this->cleanTmpFiles();
72+
return null;
73+
}
5874

59-
$cmd = $this->options['officeBinary'] . $clParameters . escapeshellarg($tmpDir) . ' ' . escapeshellarg($absPath);
75+
$parameters = [
76+
$this->options['officeBinary'],
77+
'-env:UserInstallation=file://' . escapeshellarg($profile),
78+
'--headless',
79+
'--nologo',
80+
'--nofirststartwizard',
81+
'--invisible',
82+
'--norestore',
83+
'--convert-to png',
84+
'--outdir ' . escapeshellarg($outdir),
85+
escapeshellarg($absPath),
86+
];
6087

88+
$cmd = implode(' ', $parameters);
6189
exec($cmd, $output, $returnCode);
6290

6391
if ($returnCode !== 0) {
6492
$this->cleanTmpFiles();
6593
return null;
6694
}
6795

68-
//create imagick object from png
69-
$pngPreview = null;
7096
try {
71-
[$dirname, , , $filename] = array_values(pathinfo($absPath));
72-
$pngPreview = $tmpDir . '/' . $filename . '.png';
97+
$filename = $outdir . pathinfo($absPath, PATHINFO_FILENAME) . '.png';
7398

74-
$png = new \Imagick($pngPreview . '[0]');
99+
$png = new \Imagick($filename . '[0]');
75100
$png->setImageFormat('jpg');
76101
} catch (\Exception $e) {
77102
$this->cleanTmpFiles();
78-
unlink($pngPreview);
79103
\OC::$server->get(LoggerInterface::class)->error($e->getMessage(), [
80104
'exception' => $e,
81105
'app' => 'core',
@@ -87,7 +111,6 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
87111
$image->loadFromData((string) $png);
88112

89113
$this->cleanTmpFiles();
90-
unlink($pngPreview);
91114

92115
if ($image->valid()) {
93116
$image->scaleDownToFit($maxX, $maxY);

0 commit comments

Comments
 (0)