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

Fix XMP write #8658

Merged
merged 23 commits into from
Apr 11, 2022
Merged

Fix XMP write #8658

merged 23 commits into from
Apr 11, 2022

Conversation

koppor
Copy link
Member

@koppor koppor commented Apr 10, 2022

Follow-up to #8656 after feedback.

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@ThiloteE
Copy link
Member

fixes #8657

@koppor
Copy link
Member Author

koppor commented Apr 10, 2022

We need the hacky workarounds because of Apache PDF Box 3.0 limitations

Save the document to a file using default compression.
Don't use the input file as target as this will produce a corrupted file.

See https://issues.apache.org/jira/browse/PDFBOX-4028 for a discussion.

There is a warning output

grafik

Source code:

https://github.com/apache/pdfbox/blob/641efac28791f18f6afd6dee378911c4fa77710d/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/PDDocument.java#L957

@koppor
Copy link
Member Author

koppor commented Apr 10, 2022

Some other programmers might also have issues with corrupt PDFs: https://stackoverflow.com/q/68904812/873282

@koppor
Copy link
Member Author

koppor commented Apr 10, 2022

Think, we need a log bridge for import org.apache.commons.logging.LogFactory;

@koppor
Copy link
Member Author

koppor commented Apr 10, 2022

I set this ready-for-review. I can undo the logging change if you want. See https://www.slf4j.org/legacy.html

I "hacked" with synchronized, but it works :)

@koppor koppor marked this pull request as ready for review April 10, 2022 22:54
@Siedlerchr
Copy link
Member

The logging change results in method too large on windows

* upstream/main:
  Bump h2-mvstore from 2.1.210 to 2.1.212 in /buildSrc (#8665)
  Bump actions/setup-java from 2 to 3 (#8660)
  Bump hmarr/auto-approve-action from 2.2.0 to 2.2.1 (#8661)
  Bump unoloader from 7.3.0 to 7.3.2 (#8662)
  Bump antlr from 3.5.2 to 3.5.3 (#8663)
@ThiloteE
Copy link
Member

ThiloteE commented Apr 11, 2022

Edit: Never mind. Had not downloaded newest changes on main branch yet. After I did that, no error. except the usual warning:

INFO: Index path for E:\server-ALLE\Computer Know-How\JabRef\Issues\#8278 Writing XMP metadata to PDFs skips my linked pdf file. Improve error messages\test writing metadata to pdfs.bib is C:\Users\Thilo\AppData\Local\org.jabref\JabRef\0.5a
ANTLR Tool version 4.7.2 used for code generation does not match the current runtime version 4.9.3
ANTLR Tool version 4.7.2 used for code generation does not match the current runtime version 4.9.3
[WARN] PDDocument - You are overwriting an existing file, this will produce a corrupted file if you're also read[WARN] PDDocument - You are overwriting an existing file, this will produce a corrupted file if you're also read[WARN] PDDocument - You are overwriting an existing file, this will produce a corrupted file if you're also read2022-04-11 18:33:37 [pool-2-thread-3] org.jabref.gui.JabRefDialogService.notify()
INFO: Success! Finished writing metadata.

Edit 2: Can still trigger this error with newest version of pull request:

com.google.common.util.concurrent.UncheckedExecutionException: java.lang.IllegalStateException: 1-based index not found: 11
	at com.google.common@31.1-jre/com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2055)
	at com.google.common@31.1-jre/com.google.common.cache.LocalCache.get(LocalCache.java:3966)
	at com.google.common@31.1-jre/com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:3989)
	at com.google.common@31.1-jre/com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:4950)
	at com.google.common@31.1-jre/com.google.common.cache.LocalCache$LocalLoadingCache.getUnchecked(LocalCache.java:4956)
	at org.jabref@100.0.0/org.jabref.logic.pdf.FileAnnotationCache.getFromCache(FileAnnotationCache.java:52)
	at org.jabref@100.0.0/org.jabref.gui.entryeditor.fileannotationtab.FileAnnotationTabViewModel.lambda$reloadAnnotations$1(FileAnnotationTabViewModel.java:104)
	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:457)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:456)
	at javafx.graphics/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:96)
	at javafx.graphics/com.sun.glass.ui.win.WinApplication._enterNestedEventLoopImpl(Native Method)
	at javafx.graphics/com.sun.glass.ui.win.WinApplication._enterNestedEventLoop(WinApplication.java:211)
	at javafx.graphics/com.sun.glass.ui.Application.enterNestedEventLoop(Application.java:515)
	at javafx.graphics/com.sun.glass.ui.EventLoop.enter(EventLoop.java:107)
	at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit.enterNestedEventLoop(QuantumToolkit.java:631)
	at javafx.graphics/javafx.stage.Stage.showAndWait(Stage.java:469)
	at javafx.controls/javafx.scene.control.HeavyweightDialog.showAndWait(HeavyweightDialog.java:162)
	at javafx.controls/javafx.scene.control.Dialog.showAndWait(Dialog.java:346)
	at org.jabref@100.0.0/org.jabref.gui.JabRefDialogService.showErrorDialogAndWait(JabRefDialogService.java:192)
	at org.jabref@100.0.0/org.jabref.gui.FallbackExceptionHandler.lambda$uncaughtException$0(FallbackExceptionHandler.java:26)
	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:457)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:456)
	at javafx.graphics/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:96)
	at javafx.graphics/com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
	at javafx.graphics/com.sun.glass.ui.win.WinApplication.lambda$runLoop$3(WinApplication.java:184)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.IllegalStateException: 1-based index not found: 11
	at pdfbox@3.0.0-RC1/org.apache.pdfbox.pdmodel.PDPageTree.get(PDPageTree.java:316)
	at pdfbox@3.0.0-RC1/org.apache.pdfbox.pdmodel.PDPageTree.get(PDPageTree.java:297)
	at pdfbox@3.0.0-RC1/org.apache.pdfbox.pdmodel.PDPageTree.get(PDPageTree.java:243)
	at org.jabref@100.0.0/org.jabref.logic.pdf.PdfAnnotationImporter.importAnnotations(PdfAnnotationImporter.java:49)
	at org.jabref@100.0.0/org.jabref.logic.pdf.EntryAnnotationImporter.lambda$importAnnotationsFromFiles$2(EntryAnnotationImporter.java:53)
	at java.base/java.util.Optional.ifPresent(Optional.java:178)
	at org.jabref@100.0.0/org.jabref.logic.pdf.EntryAnnotationImporter.importAnnotationsFromFiles(EntryAnnotationImporter.java:53)
	at org.jabref@100.0.0/org.jabref.logic.pdf.FileAnnotationCache$1.load(FileAnnotationCache.java:39)
	at org.jabref@100.0.0/org.jabref.logic.pdf.FileAnnotationCache$1.load(FileAnnotationCache.java:36)
	at com.google.common@31.1-jre/com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3533)
	at com.google.common@31.1-jre/com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2282)
	at com.google.common@31.1-jre/com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2159)
	at com.google.common@31.1-jre/com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2049)
	... 27 more

@Siedlerchr
Copy link
Member

Siedlerchr commented Apr 11, 2022

Works without errors for me.

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 11, 2022
@koppor
Copy link
Member Author

koppor commented Apr 11, 2022

Note to self: File Annotation Service worked better if temp file is written to

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants