From 9d9ad85f1f9131e3c00a24f2f26ca961f7eb2fe5 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 17 Mar 2023 00:49:21 +0100 Subject: [PATCH 1/4] Initial ImportHandlerTest --- .../java/org/jabref/gui/ClipBoardManager.java | 2 +- .../gui/externalfiles/ImportHandler.java | 2 +- .../org/jabref/gui/maintable/MainTable.java | 6 +- .../gui/externalfiles/ImportHandlerTest.java | 69 +++++++++++++++++++ .../logic/bibtex/BibEntryWriterTest.java | 40 ++++++++++- 5 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java diff --git a/src/main/java/org/jabref/gui/ClipBoardManager.java b/src/main/java/org/jabref/gui/ClipBoardManager.java index a75dd1798d9..b91d1be858b 100644 --- a/src/main/java/org/jabref/gui/ClipBoardManager.java +++ b/src/main/java/org/jabref/gui/ClipBoardManager.java @@ -87,7 +87,7 @@ public static String getContents() { return result; } - public Optional getBibTeXEntriesFromClipbaord() { + public Optional getBibTeXEntriesFromClipboard() { return Optional.ofNullable(clipboard.getContent(DragAndDropDataFormats.ENTRIES)).map(String.class::cast); } diff --git a/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java b/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java index 959d45ba5e5..d2da01c37b4 100644 --- a/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java +++ b/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java @@ -258,7 +258,7 @@ private void generateKeys(List entries) { } public List handleBibTeXData(String entries) { - BibtexParser parser = new BibtexParser(preferencesService.getImportFormatPreferences(), Globals.getFileUpdateMonitor()); + BibtexParser parser = new BibtexParser(preferencesService.getImportFormatPreferences(), fileUpdateMonitor); try { return parser.parseEntries(new ByteArrayInputStream(entries.getBytes(StandardCharsets.UTF_8))); } catch (ParseException ex) { diff --git a/src/main/java/org/jabref/gui/maintable/MainTable.java b/src/main/java/org/jabref/gui/maintable/MainTable.java index d5951089c1d..7f1bd38c38b 100644 --- a/src/main/java/org/jabref/gui/maintable/MainTable.java +++ b/src/main/java/org/jabref/gui/maintable/MainTable.java @@ -322,9 +322,9 @@ private void clearAndSelectLast() { public void paste() { List entriesToAdd; - entriesToAdd = this.clipBoardManager.getBibTeXEntriesFromClipbaord() + entriesToAdd = this.clipBoardManager.getBibTeXEntriesFromClipboard() .map(importHandler::handleBibTeXData) - .orElseGet(this::handleNonBibteXStringData); + .orElseGet(this::handleNonBibTeXStringData); for (BibEntry entry : entriesToAdd) { importHandler.importEntryWithDuplicateCheck(database, entry); @@ -334,7 +334,7 @@ public void paste() { } } - private List handleNonBibteXStringData() { + private List handleNonBibTeXStringData() { String data = ClipBoardManager.getContents(); List entries = new ArrayList<>(); try { diff --git a/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java b/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java new file mode 100644 index 00000000000..b50b2835432 --- /dev/null +++ b/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java @@ -0,0 +1,69 @@ +package org.jabref.gui.externalfiles; + +import java.util.List; + +import javax.swing.undo.UndoManager; + +import org.jabref.gui.DialogService; +import org.jabref.gui.StateManager; +import org.jabref.logic.importer.ImportFormatPreferences; +import org.jabref.logic.importer.ImportFormatReader; +import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.StandardField; +import org.jabref.model.entry.types.StandardEntryType; +import org.jabref.model.util.DummyFileUpdateMonitor; +import org.jabref.preferences.FilePreferences; +import org.jabref.preferences.PreferencesService; + +import org.junit.jupiter.api.Test; +import org.mockito.Answers; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class ImportHandlerTest { + + @Test + void handleBibTeXData() { + ImportFormatPreferences importFormatPreferences = mock(ImportFormatPreferences.class, Answers.RETURNS_DEEP_STUBS); + + PreferencesService preferencesService = mock(PreferencesService.class); + when(preferencesService.getImportFormatPreferences()).thenReturn(importFormatPreferences); + when(preferencesService.getFilePreferences()).thenReturn(mock(FilePreferences.class)); + + ImportHandler importHandler = new ImportHandler( + mock(BibDatabaseContext.class), + preferencesService, + new DummyFileUpdateMonitor(), + mock(UndoManager.class), + mock(StateManager.class), + mock(DialogService.class), + mock(ImportFormatReader.class)); + + List bibEntries = importHandler.handleBibTeXData(""" + @InProceedings{Wen2013, + author = {Tao Wen}, + booktitle = {45. Kraftwerkstechnisches Kolloquium}, + library = {Tagungen\\2013\\KWTK45\\}, + title = {Zünd- Und Abbrandverhalten Von Staubförmigen Brennstoffen}, + year = {2013}, + comment = {978-3-944310-04-06}, + creationdate = {2023-03-13T13:09:59}, + file = {:Tagungen/2013/KWTK45/Wen2013 - Zünd Und Abbrandverhalten Von Staubförmigen Brennstoffen.pdf:PDF}, + keywords = {zündverzug, kohle, braunkohle, lausitz, zündofen, experiment, verzögerung, partikeldurchmesser, zündhyperbel, holzhackschnitzel, torf, staub, steinkohle, getreidestaub, brennstofffeuchte, biomasse, fossile, flüchtige, volatile, FIELD-Rohr, drop-tube, model, vergleich, berechnung,}, + modificationdate = {2023-03-13T13:17:17}, + owner = {JOJ}, + url = {https://fis.tu-dresden.de/portal/de/publications/zund-und-abbrandverhalten-von-staubfoermigen-brennstoffen(2f9dd517-794a-4cf5-9897-bc9eb6f125c6).html}, + } + """); + + BibEntry expected = new BibEntry(StandardEntryType.InProceedings) + .withField(StandardField.AUTHOR, "Tao Wen"); + + assertEquals(List.of(expected), bibEntries); + // when(stateManager.activeDatabaseProperty()).thenReturn(OptionalObjectProperty.empty()); + // when(stateManager.getActiveDatabase()).thenReturn(Optional.of(current)); + } +} diff --git a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java index 9c1dc3f3aa9..4079aaeb3f6 100644 --- a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java +++ b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java @@ -184,7 +184,6 @@ void writeReallyUnknownTypeTest() throws Exception { @Test void roundTripTest() throws IOException { - // @formatter:off String bibtexEntry = """ @Article{test, Author = {Foo Bar}, @@ -193,7 +192,44 @@ void roundTripTest() throws IOException { Number = {1} } """.replaceAll("\n", OS.NEWLINE); - // @formatter:on + + // read in bibtex string + ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(new StringReader(bibtexEntry)); + Collection entries = result.getDatabase().getEntries(); + BibEntry entry = entries.iterator().next(); + + // write out bibtex string + bibEntryWriter.write(entry, bibWriter, BibDatabaseMode.BIBTEX); + + assertEquals(bibtexEntry, stringWriter.toString()); + } + + @Test + void roundTripKeepsFilePathWithBackslashes() throws IOException { + String bibtexEntry = """ + @Article{, + file = {Tagungen\\2013\\KWTK45}, + } + """.replaceAll("\n", OS.NEWLINE); + + // read in bibtex string + ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(new StringReader(bibtexEntry)); + Collection entries = result.getDatabase().getEntries(); + BibEntry entry = entries.iterator().next(); + + // write out bibtex string + bibEntryWriter.write(entry, bibWriter, BibDatabaseMode.BIBTEX); + + assertEquals(bibtexEntry, stringWriter.toString()); + } + + @Test + void roundTripKeepsEscapedCharacters() throws IOException { + String bibtexEntry = """ + @Article{, + demofield = {Tagungen\\2013\\KWTK45}, + } + """.replaceAll("\n", OS.NEWLINE); // read in bibtex string ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(new StringReader(bibtexEntry)); From 7354874e5acac6c5330c6216a982f55b8e5512ac Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 17 Mar 2023 13:23:17 +0100 Subject: [PATCH 2/4] Add hanling of "field = {content\}" --- CHANGELOG.md | 2 + .../importer/fileformat/BibtexParser.java | 52 ++++++++++++++++--- .../gui/externalfiles/ImportHandlerTest.java | 20 ++----- .../logic/bibtex/BibEntryWriterTest.java | 19 +++++++ .../importer/fileformat/BibtexParserTest.java | 26 ++++++++++ 5 files changed, 97 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f1277ee13f..ab6e55b2b86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve ### Changed - 'Get full text' now also checks the file url. [#568](https://github.com/koppor/jabref/issues/568) +- `log.txt` now contains an entry if a BibTeX entry could not be parsed. - We refined the 'main directory not found' error message. [#9625](https://github.com/JabRef/jabref/pull/9625) - We modified the `Add Group` dialog to use the most recently selected group hierarchical context [#9141](https://github.com/JabRef/jabref/issues/9141) @@ -37,6 +38,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed an issue where the command line export using `--exportMatches` flag does not create an output bib file [#9581](https://github.com/JabRef/jabref/issues/9581) - We fixed an issue where custom field in the custom entry types could not be set to mulitline [#9609](https://github.com/JabRef/jabref/issues/9609) - We fixed an issue where the Office XML exporter did not resolve BibTeX-Strings when exporting entries [forum#3741](https://discourse.jabref.org/t/exporting-bibtex-constant-strings-to-ms-office-2007-xml/3741) +- JabRef is now more relaxed when parsing field content: In case a field content ended with `\`, the combination `\}` was treated as plain `}`. [#9668](https://github.com/JabRef/jabref/issues/9668) ### Removed diff --git a/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java b/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java index 20da30e16ed..6884a68b30a 100644 --- a/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java +++ b/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java @@ -278,10 +278,10 @@ private void parseAndAddEntry(String type) { database.insertEntry(entry); } catch (IOException ex) { - // Trying to make the parser more robust. + // This makes the parser more robust: // If an exception is thrown when parsing an entry, drop the entry and try to resume parsing. - LOGGER.debug("Could not parse entry", ex); + LOGGER.warn("Could not parse entry", ex); parserResult.addWarning(Localization.lang("Error occurred when parsing entry") + ": '" + ex.getMessage() + "'. " + "\n\n" + Localization.lang("JabRef skipped the entry.")); } @@ -290,7 +290,7 @@ private void parseAndAddEntry(String type) { private void parseJabRefComment(Map meta) { StringBuilder buffer; try { - buffer = parseBracketedTextExactly(); + buffer = parseBracketedFieldContent(); } catch (IOException e) { // if we get an IO Exception here, then we have an unbracketed comment, // which means that we should just return and the comment will be picked up as arbitrary text @@ -500,6 +500,16 @@ private int peek() throws IOException { return character; } + private char[] peek2() throws IOException { + char character1 = (char) read(); + char character2 = (char) read(); + unread(character2); + unread(character1); + return new char[] { + character1, character2 + }; + } + private int read() throws IOException { int character = pushbackReader.read(); @@ -635,7 +645,7 @@ private String parseFieldContent(Field field) throws IOException { // Value is a string enclosed in brackets. There can be pairs // of brackets inside a field, so we need to count the // brackets to know when the string is finished. - StringBuilder text = parseBracketedTextExactly(); + StringBuilder text = parseBracketedFieldContent(); value.append(fieldContentFormatter.format(text, field)); } else if (Character.isDigit((char) character)) { // value is a number String number = parseTextToken(); @@ -668,7 +678,6 @@ private String parseTextToken() throws IOException { int character = read(); if (character == -1) { eof = true; - return token.toString(); } @@ -886,7 +895,11 @@ private boolean isClosingBracketNext() { } } - private StringBuilder parseBracketedTextExactly() throws IOException { + /** + * This is called if a field in the form of field = {content} is parsed. + * The global variable character contains {. + */ + private StringBuilder parseBracketedFieldContent() throws IOException { StringBuilder value = new StringBuilder(); consume('{'); @@ -898,7 +911,32 @@ private StringBuilder parseBracketedTextExactly() throws IOException { while (true) { character = (char) read(); - boolean isClosingBracket = (character == '}') && (lastCharacter != '\\'); + boolean isClosingBracket = false; + if (character == '}') { + if (lastCharacter == '\\') { + // We hit `\}` + // It could be that a user has a backslash at the end of the entry, but intended to put a file path + // We want to be relaxed at that case + // First described at https://github.com/JabRef/jabref/issues/9668 + char[] peek2 = peek2(); + if ((peek2[0] == ',') && ((peek2[1] == OS.NEWLINE.charAt(0)) || (peek2[1] == '\n'))) { + // We hit '\}\r` or `\}\n` + // Heuristics: Unwanted escaping of } + // + // Two consequences: + // + // 1. Keep `\` as read + // This is already done + // + // 2. Treat `}` as closing bracket + isClosingBracket = true; + } else { + isClosingBracket = false; + } + } else { + isClosingBracket = true; + } + } if (isClosingBracket && (brackets == 0)) { return value; diff --git a/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java b/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java index b50b2835432..05c32fb4792 100644 --- a/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java +++ b/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java @@ -6,11 +6,13 @@ import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; +import org.jabref.logic.bibtex.BibEntryAssert; import org.jabref.logic.importer.ImportFormatPreferences; import org.jabref.logic.importer.ImportFormatReader; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.StandardField; +import org.jabref.model.entry.field.UnknownField; import org.jabref.model.entry.types.StandardEntryType; import org.jabref.model.util.DummyFileUpdateMonitor; import org.jabref.preferences.FilePreferences; @@ -44,26 +46,14 @@ void handleBibTeXData() { List bibEntries = importHandler.handleBibTeXData(""" @InProceedings{Wen2013, - author = {Tao Wen}, - booktitle = {45. Kraftwerkstechnisches Kolloquium}, library = {Tagungen\\2013\\KWTK45\\}, - title = {Zünd- Und Abbrandverhalten Von Staubförmigen Brennstoffen}, - year = {2013}, - comment = {978-3-944310-04-06}, - creationdate = {2023-03-13T13:09:59}, - file = {:Tagungen/2013/KWTK45/Wen2013 - Zünd Und Abbrandverhalten Von Staubförmigen Brennstoffen.pdf:PDF}, - keywords = {zündverzug, kohle, braunkohle, lausitz, zündofen, experiment, verzögerung, partikeldurchmesser, zündhyperbel, holzhackschnitzel, torf, staub, steinkohle, getreidestaub, brennstofffeuchte, biomasse, fossile, flüchtige, volatile, FIELD-Rohr, drop-tube, model, vergleich, berechnung,}, - modificationdate = {2023-03-13T13:17:17}, - owner = {JOJ}, - url = {https://fis.tu-dresden.de/portal/de/publications/zund-und-abbrandverhalten-von-staubfoermigen-brennstoffen(2f9dd517-794a-4cf5-9897-bc9eb6f125c6).html}, } """); BibEntry expected = new BibEntry(StandardEntryType.InProceedings) - .withField(StandardField.AUTHOR, "Tao Wen"); + .withCitationKey("Wen2013") + .withField(StandardField.LIBRARY, "Tagungen\\2013\\KWTK45\\"); - assertEquals(List.of(expected), bibEntries); - // when(stateManager.activeDatabaseProperty()).thenReturn(OptionalObjectProperty.empty()); - // when(stateManager.getActiveDatabase()).thenReturn(Optional.of(current)); + assertEquals(List.of(expected), bibEntries.stream().toList()); } } diff --git a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java index 4079aaeb3f6..ab3346c87e4 100644 --- a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java +++ b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java @@ -242,6 +242,25 @@ void roundTripKeepsEscapedCharacters() throws IOException { assertEquals(bibtexEntry, stringWriter.toString()); } + @Test + void roundTripKeepsFilePathEndingWithBackslash() throws IOException { + String bibtexEntry = """ + @Article{, + file = {dir\\}, + } + """.replaceAll("\n", OS.NEWLINE); + + // read in bibtex string + ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(new StringReader(bibtexEntry)); + Collection entries = result.getDatabase().getEntries(); + BibEntry entry = entries.iterator().next(); + + // write out bibtex string + bibEntryWriter.write(entry, bibWriter, BibDatabaseMode.BIBTEX); + + assertEquals(bibtexEntry, stringWriter.toString()); + } + @Test void roundTripWithPrependingNewlines() throws IOException { // @formatter:off diff --git a/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java b/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java index 97bc6f3b8a8..4888a9c6780 100644 --- a/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java +++ b/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java @@ -607,6 +607,32 @@ void parseRecognizesAbsoluteFile() throws IOException { assertEquals(Optional.of("D:\\Documents\\literature\\Tansel-PRL2006.pdf"), entry.getField(StandardField.FILE)); } + @Test + void parseRecognizesFinalSlashAsSlash() throws Exception { + ParserResult result = parser + .parse(new StringReader(""" + @misc{, + test = {wired\\}, + } + """)); + + assertEquals( + List.of(new BibEntry() + .withField(new UnknownField("test"), "wired\\")), + result.getDatabase().getEntries() + ); + } + + /** + * JabRef's heuristics is not able to parse this special case. + */ + @Test + void parseFailsWithFinalSlashAsSlashWhenSingleLine() throws Exception { + ParserResult parserResult = parser.parse(new StringReader("@misc{, test = {wired\\}}")); + // In case JabRef was more relaxed, `assertFalse` would be provided here. + assertTrue(parserResult.hasWarnings()); + } + @Test void parseRecognizesDateFieldWithConcatenation() throws IOException { ParserResult result = parser From fd5735e941ec11f119250a8fb9acb426ad1af3b5 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 17 Mar 2023 13:28:46 +0100 Subject: [PATCH 3/4] Fix checkstyle --- .../java/org/jabref/gui/externalfiles/ImportHandlerTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java b/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java index 05c32fb4792..4a076ac80aa 100644 --- a/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java +++ b/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java @@ -6,13 +6,11 @@ import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; -import org.jabref.logic.bibtex.BibEntryAssert; import org.jabref.logic.importer.ImportFormatPreferences; import org.jabref.logic.importer.ImportFormatReader; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.StandardField; -import org.jabref.model.entry.field.UnknownField; import org.jabref.model.entry.types.StandardEntryType; import org.jabref.model.util.DummyFileUpdateMonitor; import org.jabref.preferences.FilePreferences; From 2dcab10b9cb9796fff94a0ce9a52afc6cc2c7da4 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 18 Mar 2023 10:06:01 +0100 Subject: [PATCH 4/4] Rename variable and add more documentation --- .../jabref/logic/importer/fileformat/BibtexParser.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java b/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java index 6884a68b30a..5d24472dba0 100644 --- a/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java +++ b/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java @@ -500,7 +500,7 @@ private int peek() throws IOException { return character; } - private char[] peek2() throws IOException { + private char[] peekTwoCharacters() throws IOException { char character1 = (char) read(); char character2 = (char) read(); unread(character2); @@ -918,8 +918,11 @@ private StringBuilder parseBracketedFieldContent() throws IOException { // It could be that a user has a backslash at the end of the entry, but intended to put a file path // We want to be relaxed at that case // First described at https://github.com/JabRef/jabref/issues/9668 - char[] peek2 = peek2(); - if ((peek2[0] == ',') && ((peek2[1] == OS.NEWLINE.charAt(0)) || (peek2[1] == '\n'))) { + char[] nextTwoCharacters = peekTwoCharacters(); + // Check for "\},\n" - Example context: ` path = {c:\temp\},\n` + // On Windows, it could be "\},\r\n", thus we rely in OS.NEWLINE.charAt(0) (which returns '\r' or '\n'). + // In all cases, we should check for '\n' as the file could be encoded with Linux line endings on Windows. + if ((nextTwoCharacters[0] == ',') && ((nextTwoCharacters[1] == OS.NEWLINE.charAt(0)) || (nextTwoCharacters[1] == '\n'))) { // We hit '\}\r` or `\}\n` // Heuristics: Unwanted escaping of } //