diff --git a/src/main/java/org/jabref/logic/exporter/BibWriter.java b/src/main/java/org/jabref/logic/exporter/BibWriter.java index fd03941cf74..dedeb4e64eb 100644 --- a/src/main/java/org/jabref/logic/exporter/BibWriter.java +++ b/src/main/java/org/jabref/logic/exporter/BibWriter.java @@ -26,7 +26,7 @@ public BibWriter(Writer writer, String newLineSeparator) { } /** - * Writes the given string. The newlines of the given string are converted to the newline set for this clas + * Writes the given string. The newlines of the given string are converted to the newline set for this class */ public void write(String string) throws IOException { if (precedingNewLineRequired) { diff --git a/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java b/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java index e51b9d3babd..150dab34e66 100644 --- a/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java +++ b/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java @@ -5,6 +5,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.StringJoiner; import java.util.TreeMap; import java.util.stream.Collectors; @@ -21,6 +22,9 @@ import org.jabref.model.metadata.MetaData; import org.jabref.model.strings.StringUtil; +/** + * Reading is done at {@link org.jabref.logic.importer.util.MetaDataParser} + */ public class MetaDataSerializer { private MetaDataSerializer() { @@ -32,8 +36,12 @@ private MetaDataSerializer() { public static Map getSerializedStringMap(MetaData metaData, GlobalCitationKeyPattern globalCiteKeyPattern) { - // First write all meta data except groups + // metadata-key, list of contents + // - contents to be separated by OS.NEWLINE + // - each meta data item is written as separate @Comment entry - see org.jabref.logic.exporter.BibtexDatabaseWriter.writeMetaDataItem Map> stringyMetaData = new HashMap<>(); + + // First write all meta data except groups metaData.getSaveOrderConfig().ifPresent( saveOrderConfig -> stringyMetaData.put(MetaData.SAVE_ORDER_CONFIG, saveOrderConfig.getAsStringList())); metaData.getSaveActions().ifPresent( @@ -51,7 +59,7 @@ public static Map getSerializedStringMap(MetaData metaData, metaData.getLatexFileDirectories().forEach((user, path) -> stringyMetaData .put(MetaData.FILE_DIRECTORY_LATEX + '-' + user, Collections.singletonList(path.toString().trim()))); metaData.getVersionDBStructure().ifPresent( - VersionDBStructure -> stringyMetaData.put(MetaData.VERSION_DB_STRUCT, Collections.singletonList(VersionDBStructure.trim()))); + versionDBStructure -> stringyMetaData.put(MetaData.VERSION_DB_STRUCT, Collections.singletonList(versionDBStructure.trim()))); for (ContentSelector selector : metaData.getContentSelectorList()) { stringyMetaData.put(MetaData.SELECTOR_META_PREFIX + selector.getField().getName(), selector.getValues()); @@ -67,10 +75,10 @@ public static Map getSerializedStringMap(MetaData metaData, // finally add all unknown meta data items to the serialization map Map> unknownMetaData = metaData.getUnknownMetaData(); for (Map.Entry> entry : unknownMetaData.entrySet()) { - StringBuilder value = new StringBuilder(); - value.append(OS.NEWLINE); + // The last "MetaData.SEPARATOR_STRING" adds compatibility to JabRef v5.9 and earlier + StringJoiner value = new StringJoiner(MetaData.SEPARATOR_STRING + OS.NEWLINE, OS.NEWLINE, MetaData.SEPARATOR_STRING + OS.NEWLINE); for (String line : entry.getValue()) { - value.append(line.replaceAll(";", "\\\\;") + MetaData.SEPARATOR_STRING + OS.NEWLINE); + value.add(line.replace(MetaData.SEPARATOR_STRING, "\\" + MetaData.SEPARATOR_STRING)); } serializedMetaData.put(entry.getKey(), value.toString()); } @@ -81,25 +89,33 @@ public static Map getSerializedStringMap(MetaData metaData, private static Map serializeMetaData(Map> stringyMetaData) { Map serializedMetaData = new TreeMap<>(); for (Map.Entry> metaItem : stringyMetaData.entrySet()) { - StringBuilder stringBuilder = new StringBuilder(); - for (String dataItem : metaItem.getValue()) { - if (!metaItem.getKey().equals(MetaData.VERSION_DB_STRUCT)) { - stringBuilder.append(StringUtil.quote(dataItem, MetaData.SEPARATOR_STRING, MetaData.ESCAPE_CHARACTER)).append(MetaData.SEPARATOR_STRING); + List itemList = metaItem.getValue(); + if (itemList.isEmpty()) { + // Only add non-empty values + continue; + } + + boolean isSaveActions = metaItem.getKey().equals(MetaData.SAVE_ACTIONS); + // The last "MetaData.SEPARATOR_STRING" adds compatibility to JabRef v5.9 and earlier + StringJoiner joiner = new StringJoiner(MetaData.SEPARATOR_STRING, "", MetaData.SEPARATOR_STRING); + boolean lastWasSaveActionsEnablement = false; + for (String dataItem : itemList) { + String string; + if (lastWasSaveActionsEnablement) { + string = OS.NEWLINE; } else { - stringBuilder.append(StringUtil.quote(dataItem, MetaData.SEPARATOR_STRING, MetaData.ESCAPE_CHARACTER)); + string = ""; } - + string += StringUtil.quote(dataItem, MetaData.SEPARATOR_STRING, MetaData.ESCAPE_CHARACTER); // in case of save actions, add an additional newline after the enabled flag - if (metaItem.getKey().equals(MetaData.SAVE_ACTIONS) + lastWasSaveActionsEnablement = isSaveActions && (FieldFormatterCleanups.ENABLED.equals(dataItem) - || FieldFormatterCleanups.DISABLED.equals(dataItem))) { - stringBuilder.append(OS.NEWLINE); - } + || FieldFormatterCleanups.DISABLED.equals(dataItem)); + joiner.add(string); } - - String serializedItem = stringBuilder.toString(); - // Only add non-empty values - if (!serializedItem.isEmpty() && !MetaData.SEPARATOR_STRING.equals(serializedItem)) { + String serializedItem = joiner.toString(); + if (!serializedItem.isEmpty()) { + // Only add non-empty values serializedMetaData.put(metaItem.getKey(), serializedItem); } } 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 1c7963df585..069f7dc9a6c 100644 --- a/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java +++ b/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java @@ -92,7 +92,6 @@ public BibtexParser(ImportFormatPreferences importFormatPreferences, FileUpdateM * It is undetermined which entry is returned, so use this in case you know there is only one entry in the string. * * @return An Optional<BibEntry>. Optional.empty() if non was found or an error occurred. - * @throws ParseException */ public static Optional singleFromString(String bibtexString, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor) throws ParseException { Collection entries = new BibtexParser(importFormatPreferences, fileMonitor).parseEntries(bibtexString); diff --git a/src/main/java/org/jabref/logic/importer/util/MetaDataParser.java b/src/main/java/org/jabref/logic/importer/util/MetaDataParser.java index 8b3efd3989e..bd5ff1179a0 100644 --- a/src/main/java/org/jabref/logic/importer/util/MetaDataParser.java +++ b/src/main/java/org/jabref/logic/importer/util/MetaDataParser.java @@ -11,6 +11,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.regex.Pattern; import org.jabref.logic.cleanup.FieldFormatterCleanups; import org.jabref.logic.importer.ParseException; @@ -29,6 +30,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * Writing is done at {@link org.jabref.logic.exporter.MetaDataSerializer}. + */ public class MetaDataParser { private static final Logger LOGGER = LoggerFactory.getLogger(MetaDataParser.class); @@ -81,46 +85,46 @@ public MetaData parse(MetaData metaData, Map data, Character key entryList.sort(groupsLast()); for (Map.Entry entry : entryList) { - List value = getAsList(entry.getValue()); + List values = getAsList(entry.getValue()); if (entry.getKey().startsWith(MetaData.PREFIX_KEYPATTERN)) { EntryType entryType = EntryTypeFactory.parse(entry.getKey().substring(MetaData.PREFIX_KEYPATTERN.length())); - nonDefaultCiteKeyPatterns.put(entryType, Collections.singletonList(getSingleItem(value))); - } else if (entry.getKey().startsWith(MetaData.FILE_DIRECTORY + '-')) { - // The user name starts directly after FILE_DIRECTORY + '-' - String user = entry.getKey().substring(MetaData.FILE_DIRECTORY.length() + 1); - metaData.setUserFileDirectory(user, getSingleItem(value)); + nonDefaultCiteKeyPatterns.put(entryType, Collections.singletonList(getSingleItem(values))); } else if (entry.getKey().startsWith(MetaData.SELECTOR_META_PREFIX)) { // edge case, it might be one special field e.g. article from biblatex-apa, but we can't distinguish this from any other field and rather prefer to handle it as UnknownField metaData.addContentSelector(ContentSelectors.parse(FieldFactory.parseField(entry.getKey().substring(MetaData.SELECTOR_META_PREFIX.length())), StringUtil.unquote(entry.getValue(), MetaData.ESCAPE_CHARACTER))); + } else if (entry.getKey().equals(MetaData.FILE_DIRECTORY)) { + metaData.setDefaultFileDirectory(parseDirectory(entry.getValue())); + } else if (entry.getKey().startsWith(MetaData.FILE_DIRECTORY + '-')) { + // The user name starts directly after FILE_DIRECTORY + '-' + String user = entry.getKey().substring(MetaData.FILE_DIRECTORY.length() + 1); + metaData.setUserFileDirectory(user, parseDirectory(entry.getValue())); } else if (entry.getKey().startsWith(MetaData.FILE_DIRECTORY_LATEX)) { // The user name starts directly after FILE_DIRECTORY_LATEX" + '-' String user = entry.getKey().substring(MetaData.FILE_DIRECTORY_LATEX.length() + 1); - Path path = Path.of(getSingleItem(value)).normalize(); + Path path = Path.of(parseDirectory(entry.getValue())).normalize(); metaData.setLatexFileDirectory(user, path); } else if (entry.getKey().equals(MetaData.SAVE_ACTIONS)) { - metaData.setSaveActions(FieldFormatterCleanups.parse(value)); + metaData.setSaveActions(FieldFormatterCleanups.parse(values)); } else if (entry.getKey().equals(MetaData.DATABASE_TYPE)) { - metaData.setMode(BibDatabaseMode.parse(getSingleItem(value))); + metaData.setMode(BibDatabaseMode.parse(getSingleItem(values))); } else if (entry.getKey().equals(MetaData.KEYPATTERNDEFAULT)) { - defaultCiteKeyPattern = Collections.singletonList(getSingleItem(value)); + defaultCiteKeyPattern = Collections.singletonList(getSingleItem(values)); } else if (entry.getKey().equals(MetaData.PROTECTED_FLAG_META)) { - if (Boolean.parseBoolean(getSingleItem(value))) { + if (Boolean.parseBoolean(getSingleItem(values))) { metaData.markAsProtected(); } else { metaData.markAsNotProtected(); } - } else if (entry.getKey().equals(MetaData.FILE_DIRECTORY)) { - metaData.setDefaultFileDirectory(getSingleItem(value)); } else if (entry.getKey().equals(MetaData.SAVE_ORDER_CONFIG)) { - metaData.setSaveOrderConfig(SaveOrder.parse(value)); + metaData.setSaveOrderConfig(SaveOrder.parse(values)); } else if (entry.getKey().equals(MetaData.GROUPSTREE) || entry.getKey().equals(MetaData.GROUPSTREE_LEGACY)) { - metaData.setGroups(GroupsParser.importGroups(value, keywordSeparator, fileMonitor, metaData)); + metaData.setGroups(GroupsParser.importGroups(values, keywordSeparator, fileMonitor, metaData)); } else if (entry.getKey().equals(MetaData.VERSION_DB_STRUCT)) { - metaData.setVersionDBStructure(getSingleItem(value)); + metaData.setVersionDBStructure(getSingleItem(values)); } else { // Keep meta data items that we do not know in the file - metaData.putUnknownMetaDataItem(entry.getKey(), value); + metaData.putUnknownMetaDataItem(entry.getKey(), values); } } @@ -131,6 +135,31 @@ public MetaData parse(MetaData metaData, Map data, Character key return metaData; } + /** + * Parse the content of the value as provided by "raw" content. + * + * We do not use unescaped value (created by @link{#getAsList(java.lang.String)}), + * because this leads to difficulties with UNC names. + * + * No normalization is done - the general file directory could be passed as Mac OS X path, but the user could sit on Windows. + * + * @param value the raw value (as stored in the .bib file) + */ + static String parseDirectory(String value) { + value = StringUtil.removeStringAtTheEnd(value, MetaData.SEPARATOR_STRING); + Pattern SINGLE_BACKSLASH = Pattern.compile("[^\\\\]\\\\[^\\\\]"); + if (value.contains("\\\\\\\\")) { + // This is an escaped Windows UNC path + return value.replace("\\\\", "\\"); + } else if (value.contains("\\\\") && !SINGLE_BACKSLASH.matcher(value).find()) { + // All backslashes escaped + return value.replace("\\\\", "\\"); + } else { + // No backslash escaping + return value; + } + } + private static Comparator> groupsLast() { return (s1, s2) -> MetaData.GROUPSTREE.equals(s1.getKey()) || MetaData.GROUPSTREE_LEGACY.equals(s1.getKey()) ? 1 : MetaData.GROUPSTREE.equals(s2.getKey()) || MetaData.GROUPSTREE_LEGACY.equals(s2.getKey()) ? -1 : 0; @@ -174,7 +203,14 @@ private static Optional getNextUnit(Reader reader) throws IOException { StringBuilder res = new StringBuilder(); while ((c = reader.read()) != -1) { if (escape) { - res.append((char) c); + // at org.jabref.logic.exporter.MetaDataSerializer.serializeMetaData, only MetaData.SEPARATOR_CHARACTER, MetaData.ESCAPE_CHARACTER are quoted + // That means ; and \\ + char character = (char) c; + if (character != MetaData.SEPARATOR_CHARACTER && character != MetaData.ESCAPE_CHARACTER) { + // Keep the escape character + res.append("\\"); + } + res.append(character); escape = false; } else if (c == MetaData.ESCAPE_CHARACTER) { escape = true; diff --git a/src/main/java/org/jabref/model/metadata/MetaData.java b/src/main/java/org/jabref/model/metadata/MetaData.java index 0f665e48c39..a8ddc4b3d74 100644 --- a/src/main/java/org/jabref/model/metadata/MetaData.java +++ b/src/main/java/org/jabref/model/metadata/MetaData.java @@ -72,7 +72,7 @@ public class MetaData { private final Map> unknownMetaData = new HashMap<>(); private boolean isEventPropagationEnabled = true; private boolean encodingExplicitlySupplied; - private String VersionDBStructure; + private String versionDBStructure; /** * Constructs an empty metadata. @@ -214,11 +214,11 @@ public void setDefaultFileDirectory(String path) { } public Optional getVersionDBStructure() { - return Optional.ofNullable(VersionDBStructure); + return Optional.ofNullable(versionDBStructure); } public void setVersionDBStructure(String version) { - VersionDBStructure = Objects.requireNonNull(version).trim(); + versionDBStructure = Objects.requireNonNull(version).trim(); postChange(); } @@ -384,17 +384,17 @@ public boolean equals(Object o) { && (mode == metaData.mode) && Objects.equals(defaultFileDirectory, metaData.defaultFileDirectory) && Objects.equals(contentSelectors, metaData.contentSelectors) - && Objects.equals(VersionDBStructure, metaData.VersionDBStructure); + && Objects.equals(versionDBStructure, metaData.versionDBStructure); } @Override public int hashCode() { return Objects.hash(isProtected, groupsRoot.getValue(), encoding, encodingExplicitlySupplied, saveOrder, citeKeyPatterns, userFileDirectory, - laTexFileDirectory, defaultCiteKeyPattern, saveActions, mode, defaultFileDirectory, contentSelectors, VersionDBStructure); + laTexFileDirectory, defaultCiteKeyPattern, saveActions, mode, defaultFileDirectory, contentSelectors, versionDBStructure); } @Override public String toString() { - return "MetaData [citeKeyPatterns=" + citeKeyPatterns + ", userFileDirectory=" + userFileDirectory + ", laTexFileDirectory=" + laTexFileDirectory + ", groupsRoot=" + groupsRoot + ", encoding=" + encoding + ", saveOrderConfig=" + saveOrder + ", defaultCiteKeyPattern=" + defaultCiteKeyPattern + ", saveActions=" + saveActions + ", mode=" + mode + ", isProtected=" + isProtected + ", defaultFileDirectory=" + defaultFileDirectory + ", contentSelectors=" + contentSelectors + ", encodingExplicitlySupplied=" + encodingExplicitlySupplied + ", VersionDBStructure=" + VersionDBStructure + "]"; + return "MetaData [citeKeyPatterns=" + citeKeyPatterns + ", userFileDirectory=" + userFileDirectory + ", laTexFileDirectory=" + laTexFileDirectory + ", groupsRoot=" + groupsRoot + ", encoding=" + encoding + ", saveOrderConfig=" + saveOrder + ", defaultCiteKeyPattern=" + defaultCiteKeyPattern + ", saveActions=" + saveActions + ", mode=" + mode + ", isProtected=" + isProtected + ", defaultFileDirectory=" + defaultFileDirectory + ", contentSelectors=" + contentSelectors + ", encodingExplicitlySupplied=" + encodingExplicitlySupplied + ", VersionDBStructure=" + versionDBStructure + "]"; } } 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 e253b4191a3..f18db297f40 100644 --- a/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java +++ b/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java @@ -60,6 +60,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Answers; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -129,11 +131,10 @@ void singleFromStringRecognizesEntry() throws ParseException { """, importFormatPreferences, fileMonitor); - BibEntry expected = new BibEntry(); - expected.setType(StandardEntryType.Article); - expected.setCitationKey("canh05"); - expected.setField(StandardField.AUTHOR, "Crowston, K. and Annabi, H."); - expected.setField(StandardField.TITLE, "Title A"); + BibEntry expected = new BibEntry(StandardEntryType.Article) + .withCitationKey("canh05") + .withField(StandardField.AUTHOR, "Crowston, K. and Annabi, H.") + .withField(StandardField.TITLE, "Title A"); assertEquals(Optional.of(expected), parsed); } @@ -1655,6 +1656,29 @@ void integrationTestFileDirectories() throws IOException { assertEquals("D:\\Latex", result.getMetaData().getLatexFileDirectory("defaultOwner-user").get().toString()); } + @ParameterizedTest + @CsvSource({ + // single backslash kept + "C:\\temp\\test", + "\\\\servername\\path\\to\\file", + "//servername/path/to/file", + "."}) + void fileDirectoriesUnmodified(String directory) throws IOException { + ParserResult result = parser.parse( + new StringReader("@comment{jabref-meta: fileDirectory:" + directory + "}")); + assertEquals(directory, result.getMetaData().getDefaultFileDirectory().get()); + } + + @ParameterizedTest + @CsvSource({ + "C:\\temp\\test, C:\\\\temp\\\\test", + "\\\\servername\\path\\to\\file, \\\\\\\\servername\\\\path\\\\to\\\\file"}) + void fileDirectoryWithDoubleEscapeIsRead(String expected, String provided) throws IOException { + ParserResult result = parser.parse( + new StringReader("@comment{jabref-meta: fileDirectory: " + provided + "}")); + assertEquals(expected, result.getMetaData().getDefaultFileDirectory().get()); + } + @Test void parseReturnsEntriesInSameOrder() throws IOException { List expected = new ArrayList<>(); diff --git a/src/test/java/org/jabref/logic/importer/util/MetaDataParserTest.java b/src/test/java/org/jabref/logic/importer/util/MetaDataParserTest.java new file mode 100644 index 00000000000..12e584e49c8 --- /dev/null +++ b/src/test/java/org/jabref/logic/importer/util/MetaDataParserTest.java @@ -0,0 +1,23 @@ +package org.jabref.logic.importer.util; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class MetaDataParserTest { + + @ParameterizedTest + @CsvSource({ + "C:\\temp\\test, C:\\temp\\test", + "\\\\servername\\path\\to\\file, \\\\\\\\servername\\\\path\\\\to\\\\file", + "\\\\servername\\path\\to\\file, \\\\servername\\path\\to\\file", + "//servername/path/to/file, //servername/path/to/file", + ".\\pdfs, .\\pdfs,", + ".\\pdfs, .\\\\pdfs,", + "., .", + }) + void parseDirectory(String expected, String input) { + assertEquals(expected, MetaDataParser.parseDirectory(input)); + } +} diff --git a/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java b/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java index 5ce25cb42a7..5a589970aba 100644 --- a/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java +++ b/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java @@ -146,17 +146,16 @@ void testEntryIsUnchangedAfterChecks() { } private BibDatabaseContext createContext(Field field, String value, EntryType type) { - BibEntry entry = new BibEntry(); - entry.setField(field, value); - entry.setType(type); + BibEntry entry = new BibEntry(type) + .withField(field, value); BibDatabase bibDatabase = new BibDatabase(); bibDatabase.insertEntry(entry); return new BibDatabaseContext(bibDatabase); } private BibDatabaseContext createContext(Field field, String value, MetaData metaData) { - BibEntry entry = new BibEntry(); - entry.setField(field, value); + BibEntry entry = new BibEntry() + .withField(field, value); BibDatabase bibDatabase = new BibDatabase(); bibDatabase.insertEntry(entry); return new BibDatabaseContext(bibDatabase, metaData);