fix(PreviewManager): use the forced mimetype in throwIfPreviewsDisabled #53205
Conversation
| * Check if a preview can be generated for a file | ||
| * | ||
| * @param \OCP\Files\FileInfo $file | ||
| * @param string|null $mimeType To force a given mimetype for the file |
There was a problem hiding this comment.
we might need a @since for this even when it is optional
There was a problem hiding this comment.
@susnux Hey, what would you recommend in terms of annotation for the addition of an optional param?
There was a problem hiding this comment.
The since for 32 looks ok :)
julien-nc
force-pushed
the
fix/noid/get-preview-force-mimetype
branch
2 times, most recently
from
28a6f94 to
dd887f1
Compare
|
/backport to stable31 |
|
/backport to stable30 |
julien-nc
force-pushed
the
fix/noid/get-preview-force-mimetype
branch
from
dd887f1 to
43d695c
Compare
lib/public/IPreview.php
Outdated
| * @since 30.0.12 - isAvailable($mimeType) added the $mimeType argument to the signature | ||
| * @since 31.0.6 - isAvailable($mimeType) added the $mimeType argument to the signature |
There was a problem hiding this comment.
We do not backport features normally to patch releases, so in other cases we only added it to the public interface for the current version (32) and only backported the private implementation.
There was a problem hiding this comment.
Alright, so just to make sure we're on the same page: in this branch we add @since 32.0.0 - isAvailable($mimeType) added blabla in the public interface
and in the backports, we don't even add the new param in the interface but we backport the change in lib/private/PreviewManager.php. Right?
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
julien-nc
force-pushed
the
fix/noid/get-preview-force-mimetype
branch
from
43d695c to
c7e9f2c
Compare
|
@AndyScherzinger @come-nc Cypress failures should not be related. |
|
The backport to # Switch to the target branch and update it
git checkout stable30
git pull origin stable30
# Create the new backport branch
git checkout -b backport/53205/stable30
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick c7e9f2c0
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/53205/stable30Error: Failed to push branch backport/53205/stable30: remote: Invalid username or password. Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports. |
|
@julien-nc restarted it, turned green 👍 |
|
/backport to stable30 |
|
Backport worked for me 😂 |
PreviewManager::getPreviewcan fail because it does not use the forced mimetype when checking if the preview is available.This happens, for example, when
$file->getMimeType()returnsapplication/octet-streambecause the file name has no extension.This broke pretty recently, I saw it because the assistant is requesting previews for files which names have no extensions but we know their mimetype. It used to work fine by forcing the mimetype when calling
PreviewManager::getPreview, now we get aNotFoundExceptionbecauseisAvailablereturns false because it uses the mimetype from$file->getMimeType().I didn't see any other way than to add an optional param to
IPreview::isAvailable. It does not break anything as it's a new optional param. Is that ok? I'm fine with implementing another approach.This can be backported to stable31 and stable30.
stable29 and older are not affected IMO.