Feature : provides .epub files preview pictures #21

smarinier · 2024-06-18T08:38:49Z

No description provided.

devnoname120 · 2024-06-22T16:18:25Z

Thanks! Is there no good EPUB library that could do the heavy lifting to get the cover from the EPUB? I would rather not have to maintain custom EPUB extraction code.

For example: https://github.com/mikespub-org/php-epub-meta
I haven't tried it but it looks like a promising library.

What do you think?

gschenck · 2024-06-24T18:27:53Z

I think that a large part of the code is needed anyway, even if one wants an external library to do the real work.
So it would probably not be a problem to merge the commit now and switch to an external library later if necessary.

lib/AppInfo/Application.php

lib/Preview/EPubPreview.php

github-advanced-security

Psalm found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

lib/AppInfo/Application.php

lib/Preview/EPubPreview.php

+	private function getContentPath() {
+		$xml_container = $this->extractXML('META-INF/container.xml');
+		if ($xml_container) {
+			$full_path = $xml_container->rootfiles->rootfile['full-path'][0];


lib/Preview/EPubPreview.php

devnoname120 · 2024-06-24T21:14:36Z

@smarinier Could you fix the stuff flagged by the bot? Thanks

lib/Preview/EPubPreview.php

devnoname120 · 2024-06-25T17:39:09Z

lib/Preview/EPubPreview.php

+	 * extractThumbnail from complicated epub format
+	 */
+	private function extractThumbnail(File $file, string $path): ?IImage {
+		$tmpManager = \OC::$server->get(ITempManager::class);


@smarinier Instance of ITempManager should be obtained through dependency injection from Nextcloud. See here for an example: https://github.com/nextcloud/assistant/pull/71/files

Hmmm. There is also an example in a preview, where i took it i think : lib/private/Preview/Office.php
So this is 50% chance.;) (I think it's the same at least). Not sure the problem doesn't come from Psalm.

Could you confirm here you want this ? As this is less efficient. Indeed, the dependency injection is done by server get class only when the preview is built (then the preview image is stored the cache). If the injection is in constructor, it is called on all files twice (to check the mime type)

Hmmm. There is also an example in a preview, where i took it i think : lib/private/Preview/Office.php

Old code.

As this is less efficient. Indeed, the dependency injection is done by server get class only when the preview is built (then the preview image is stored the cache). If the injection is in constructor, it is called on all files twice (to check the mime type)

Unless there is a bug the performance impact should be negligible. The file access takes many orders of magnitude more time than the near-instant dependency injection. The \OC::$server->get() notation is being phased out so it's not future proof.

lib/Preview/EPubPreview.php

+		$xml_container = $this->extractXML('META-INF/container.xml');
+		if (is_object($xml_container)) {
+			$full_path = $xml_container->rootfiles->rootfile['full-path'][0];
+			if ($full_path) {


lib/Preview/EPubPreview.php

+	 * @return \DOMDocument|null
+	 */
+	protected function extractHTML(string $path): \DOMDocument|null {
+		$html = $this->extractFileData($path);


lib/Preview/EPubPreview.php

+		if (is_string($html)) {
+			$dom = new \DOMDocument('1.0', 'utf-8');
+			$dom->strictErrorChecking = false;
+			if (@$dom->loadHTML($html)) {


devnoname120 · 2024-06-25T18:58:38Z

lib/Preview/EPubPreview.php

+	 * @return false|null|string
+	 */
+	private function extractFileData(string $path): string|false|null {
+		$fp = $this->zip->getStream($path, 'r');


@smarinier Does OC\Archive\ZIP exist at all if the app files_zip isn't installed?

OK so I guess it's a private API: https://github.com/nextcloud/server/blob/952271929d888c8333f5b64aa676f802a8b682af/lib/private/Archive/ZIP.php#L13

How much effort would it require to change the code to use PHP's ZipArchive directly?

I think this would be a pity, as the ZIP class embeds ZipArchive properly, and logs errors in NextCloud. Its probably a definition that is necessary to do with Psalm.

The problem is that it's a private file that should not be used in apps. This means that if we use this implementation it will unexpectedly break in an update without any warnings. I don't want to maintain code that relies on non-standard internal APIs.

devnoname120 · 2024-06-25T18:59:13Z

lib/Preview/EPubPreview.php

+use OCP\ITempManager;
+
+class EPubPreview implements IProviderV2 {
+	private ?ZIP $zip = null;


smarinier · 2024-06-25T14:14:11Z

I did this implementation

@smarinier Could you fix the stuff flagged by the bot? Thanks

Hi Paul, i've done some stuff, but it seems to point errors that are not linked with my commits, and other i'm not sure i can fix them (eg "don't know the class OCA\Files\Event\LoadAdditionalScriptsEvent ??)

devnoname120 · 2024-06-25T18:16:24Z

eg "don't know the class OCA\Files\Event\LoadAdditionalScriptsEvent ??

You can ignore this kind of stuff that is unrelated to your code. I'm not 100% sure why it doesn't get the definitions.

Edit: lol so apparently the right way is to ignore them?
Edit 2: the Memories app solves it by cloning the whole nextcloud backend and including it in the psalm analysis scope
Edit 3: the app integration_mattermost uses a stubs files instead. The app spreed uses even more stubs, the app photos has a giant stubs file as well.

Final edit: yeah so ignore those for now when it's not in a file you touched or when it doesn't make sense. I will implement a trick like the Memories app later after this PR.

devnoname120

I think we are almost there. Can you fix the remaining issues flagged by the bot? Only those related to your changes ofc.

One last question: what happens if the function that generates a thumbnail has an uncaught exception? Will Nextcloud just ignore generating a preview for that particular file? Will it crash the whole backend of the EPUB viewer app? Anything else? I'm really new to this whole preview provider thing so I'm cautious.

The next steps for me will be to do a quick security audit and some defensive exception handling to make sure that in the worst case it doesn't break anything outside of the previews.

smarinier · 2024-06-25T20:40:19Z

I think we are almost there. Can you fix the remaining issues flagged by the bot? Only those related to your changes ofc.

One last question: what happens if the function that generates a thumbnail has an uncaught exception? Will Nextcloud just ignore generating a preview for that particular file? Will it crash the whole backend of the EPUB viewer app? Anything else? I'm really new to this whole preview provider thing so I'm cautious.

The next steps for me will be to do a quick security audit and some defensive exception handling to make sure that in the worst case it doesn't break anything outside of the previews.

You know, i've been using this for almost 8 years (and thousands of epub). I think rewriting with ZipArchive is more risky than finding how to make Psalm know the presence of the ZIP class ;).
Concerning the preview, this is an individual call. NextCloud knows "This file has a preview, so let's generate a rest /preview call". If something is wrong, you won't have any preview for the file. That's all, i guess.

devnoname120 · 2024-06-27T19:33:42Z

I think rewriting with ZipArchive is more risky than finding how to make Psalm know the presence of the ZIP class ;).

The problem is not to figure out how to make Psalm ignore it (it's trivial). The problem is that this ZIP class is private and should not be used by external apps. I don't want to use private APIs unless there is a very strong reason to do that, because they will unexpectedly break in an update.

devnoname120 · 2024-06-27T20:37:46Z

@smarinier OK I fixed the psalm now the remaining issues are specific to your PR.

devnoname120 · 2024-07-18T22:04:30Z

@smarinier Is this PR abandoned? If yes then I'll close it.

smarinier · 2024-07-19T12:24:57Z

Hi, I don't have the time at the moment to satisfy all your requirements. If that's a problem for you, you can close it.

Yours,

devnoname120 · 2024-07-19T21:37:19Z

Thanks for the heads-up. In that case I will close the PR for the time being. Feel free to open a new pull request when you get the chance to look into it again

devnoname120 · 2025-02-24T12:24:13Z

For my future reference, these look interesting:

https://github.com/Yetangitu/owncloud-apps/blob/master/files_opds/lib/epub.php
https://github.com/Yetangitu/owncloud-apps/blob/master/files_opds/lib/epubpreview.php

Looks like they are (at least partly) based on https://github.com/mikespub-org/php-epub-meta so we should use that one directly instead (for the parts that were copy-pasted).

Feature : provides .epub files preview pictures

c35f11a

devnoname120 added 2 commits June 24, 2024 22:08

Merge branch 'master' into feature/epub_files_previews

97ee9d5

Lint — Add PHP cs fixer config

a3fc1bf

github-advanced-security bot found potential problems Jun 24, 2024

View reviewed changes

devnoname120 added 2 commits June 24, 2024 22:44

Lint — Run composer cs:fix

012ad43

Lint — Run composer psalm:fix

c93d33c

github-advanced-security bot found potential problems Jun 24, 2024

View reviewed changes

Merge branch 'master' into feature/epub_files_previews

b0b96e1

github-advanced-security bot found potential problems Jun 24, 2024

View reviewed changes

devnoname120 added 3 commits June 24, 2024 23:02

Merge branch 'master' into feature/epub_files_previews

c3bdea1

Merge branch 'master' into feature/epub_files_previews

37adafd

Lint — Run psalm:fix

b5efb09

github-advanced-security bot found potential problems Jun 24, 2024

View reviewed changes

ifix PR lint warnings

c3eccad

github-advanced-security bot found potential problems Jun 25, 2024

View reviewed changes

zip might be null, without a constructor

762338d

github-advanced-security bot found potential problems Jun 25, 2024

View reviewed changes

devnoname120 reviewed Jun 25, 2024

View reviewed changes

Merge branch 'master' into feature/epub_files_previews

b7f3077

devnoname120 mentioned this pull request Jun 27, 2024

EPub preview with their embeded image nextcloud/server#39085

Open

devnoname120 closed this Jul 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature : provides .epub files preview pictures #21

Feature : provides .epub files preview pictures #21

smarinier commented Jun 18, 2024

devnoname120 commented Jun 22, 2024

gschenck commented Jun 24, 2024

github-advanced-security bot left a comment

devnoname120 commented Jun 24, 2024

devnoname120 Jun 25, 2024 •

edited

Loading

smarinier Jun 25, 2024

devnoname120 Jun 27, 2024

devnoname120 Jun 25, 2024

devnoname120 Jun 25, 2024 •

edited

Loading

smarinier Jun 25, 2024

devnoname120 Jun 27, 2024

devnoname120 Jun 25, 2024

smarinier commented Jun 25, 2024

devnoname120 commented Jun 25, 2024 •

edited

Loading

devnoname120 left a comment

smarinier commented Jun 25, 2024

devnoname120 commented Jun 27, 2024

devnoname120 commented Jun 27, 2024

devnoname120 commented Jul 18, 2024

smarinier commented Jul 19, 2024

devnoname120 commented Jul 19, 2024

devnoname120 commented Feb 24, 2025

Feature : provides .epub files preview pictures #21

Feature : provides .epub files preview pictures #21

Conversation

smarinier commented Jun 18, 2024

devnoname120 commented Jun 22, 2024

gschenck commented Jun 24, 2024

github-advanced-security bot left a comment

Choose a reason for hiding this comment

devnoname120 commented Jun 24, 2024

devnoname120 Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

smarinier Jun 25, 2024

Choose a reason for hiding this comment

devnoname120 Jun 27, 2024

Choose a reason for hiding this comment

devnoname120 Jun 25, 2024

Choose a reason for hiding this comment

devnoname120 Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

smarinier Jun 25, 2024

Choose a reason for hiding this comment

devnoname120 Jun 27, 2024

Choose a reason for hiding this comment

devnoname120 Jun 25, 2024

Choose a reason for hiding this comment

smarinier commented Jun 25, 2024

devnoname120 commented Jun 25, 2024 • edited Loading

devnoname120 left a comment

Choose a reason for hiding this comment

smarinier commented Jun 25, 2024

devnoname120 commented Jun 27, 2024

devnoname120 commented Jun 27, 2024

devnoname120 commented Jul 18, 2024

smarinier commented Jul 19, 2024

devnoname120 commented Jul 19, 2024

devnoname120 commented Feb 24, 2025

devnoname120 Jun 25, 2024 •

edited

Loading

devnoname120 Jun 25, 2024 •

edited

Loading

devnoname120 commented Jun 25, 2024 •

edited

Loading