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 parser exception when loading tex-groups with non-existing file #4839

Merged
merged 3 commits into from
Apr 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- We fixed an issue where selecting a group messed up the focus of the main table / entry editor. https://github.com/JabRef/jabref/issues/3367
- We fixed an issue where composite author names were sorted incorrectly. https://github.com/JabRef/jabref/issues/2828
- We fixed an issue where commands followed by `-` didn't work. [#3805](https://github.com/JabRef/jabref/issues/3805)
- We fixed an issue where a non-existing aux file in a group made it impossible to open the library. [#4735](https://github.com/JabRef/jabref/issues/4735)
- We fixed an issue where some journal names were wrongly marked as abbreviated. [#4115](https://github.com/JabRef/jabref/issues/4115)
- We fixed an issue where the custom file column were sorted incorrectly. https://github.com/JabRef/jabref/issues/3119
- We fixed an issues where the entry losses focus when a field is edited and at the same time used for sorting. https://github.com/JabRef/jabref/issues/3373
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/groups/GroupDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ groupName, getContext(),
autoGroupPersonsField.getText().trim());
}
} else if (texRadioButton.isSelected()) {
resultingGroup = new TexGroup(groupName, getContext(),
resultingGroup = TexGroup.create(groupName, getContext(),
Paths.get(texGroupFilePath.getText().trim()), new DefaultAuxParser(new BibDatabase()), Globals.getFileUpdateMonitor(), basePanel.getBibDatabaseContext().getMetaData());
}

Expand Down Expand Up @@ -466,7 +466,7 @@ private VBox createOptionsTexGroup() {
texGroupBrowseButton.setOnAction((ActionEvent e) -> openBrowseDialog());
texGroupHBox.getChildren().add(texGroupFilePath);
texGroupHBox.getChildren().add(texGroupBrowseButton);
texGroupHBox.setHgrow(texGroupFilePath, Priority.ALWAYS);
HBox.setHgrow(texGroupFilePath, Priority.ALWAYS);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this making for a difference?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a static method, so there is no reason to call it from an instance variable (btw: automatic change by Intellj)

texPanel.getChildren().add(texGroupHBox);
return texPanel;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ private AuxParserResult parseAuxFile(Path auxFile) {
matchNestedAux(auxFile, result, fileList, line);
}
} catch (FileNotFoundException e) {
LOGGER.info("Cannot locate input file", e);
LOGGER.warn("Cannot locate input file", e);
} catch (IOException e) {
LOGGER.warn("Problem opening file", e);
}
Expand Down
19 changes: 16 additions & 3 deletions src/main/java/org/jabref/logic/importer/util/GroupsParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,15 @@
import org.jabref.model.strings.StringUtil;
import org.jabref.model.util.FileUpdateMonitor;

import org.slf4j.LoggerFactory;

/**
* Converts string representation of groups to a parsed {@link GroupTreeNode}.
*/
public class GroupsParser {

private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(GroupsParser.class);

private GroupsParser() {
}

Expand Down Expand Up @@ -123,9 +127,18 @@ private static AbstractGroup texGroupFromString(String string, FileUpdateMonitor
GroupHierarchyType context = GroupHierarchyType.getByNumberOrDefault(Integer.parseInt(tok.nextToken()));
try {
Path path = Paths.get(tok.nextToken());
TexGroup newGroup = new TexGroup(name, context, path, new DefaultAuxParser(new BibDatabase()), fileMonitor, metaData);
addGroupDetails(tok, newGroup);
return newGroup;
try {
TexGroup newGroup = TexGroup.create(name, context, path, new DefaultAuxParser(new BibDatabase()), fileMonitor, metaData);
addGroupDetails(tok, newGroup);
return newGroup;
} catch (IOException ex) {
// Problem accessing file -> create without file monitoring
LOGGER.warn("Could not access file " + path + ". The group " + name + " will not reflect changes to the aux file.", ex);

TexGroup newGroup = TexGroup.createWithoutFileMonitoring(name, context, path, new DefaultAuxParser(new BibDatabase()), fileMonitor, metaData);
addGroupDetails(tok, newGroup);
return newGroup;
}
} catch (InvalidPathException | IOException ex) {
throw new ParseException(ex);
}
Expand Down
17 changes: 13 additions & 4 deletions src/main/java/org/jabref/model/groups/TexGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,29 @@ public class TexGroup extends AbstractGroup implements FileUpdateListener {
private final MetaData metaData;
private String user;

public TexGroup(String name, GroupHierarchyType context, Path filePath, AuxParser auxParser, FileUpdateMonitor fileMonitor, MetaData metaData, String user) throws IOException {
TexGroup(String name, GroupHierarchyType context, Path filePath, AuxParser auxParser, FileUpdateMonitor fileMonitor, MetaData metaData, String user) {
super(name, context);
this.metaData = metaData;
this.user = user;
this.filePath = expandPath(Objects.requireNonNull(filePath));
this.auxParser = auxParser;
this.fileMonitor = fileMonitor;
fileMonitor.addListenerForFile(this.filePath, this);
}

public TexGroup(String name, GroupHierarchyType context, Path filePath, AuxParser auxParser, FileUpdateMonitor fileMonitor, MetaData metaData) throws IOException {
TexGroup(String name, GroupHierarchyType context, Path filePath, AuxParser auxParser, FileUpdateMonitor fileMonitor, MetaData metaData) throws IOException {
this(name, context, filePath, auxParser, fileMonitor, metaData, System.getProperty("user.name") + '-' + InetAddress.getLocalHost().getHostName());
}

public static TexGroup create(String name, GroupHierarchyType context, Path filePath, AuxParser auxParser, FileUpdateMonitor fileMonitor, MetaData metaData) throws IOException {
TexGroup group = new TexGroup(name, context, filePath, auxParser, fileMonitor, metaData);
fileMonitor.addListenerForFile(filePath, group);
return group;
}

public static TexGroup createWithoutFileMonitoring(String name, GroupHierarchyType context, Path filePath, AuxParser auxParser, FileUpdateMonitor fileMonitor, MetaData metaData) throws IOException {
return new TexGroup(name, context, filePath, auxParser, fileMonitor, metaData);
}

@Override
public boolean contains(BibEntry entry) {
if (keysUsedInAux == null) {
Expand Down Expand Up @@ -124,7 +133,7 @@ private List<Path> getFileDirectoriesAsPaths() {
List<Path> fileDirs = new ArrayList<>();

metaData.getLaTexFileDirectory(user)
.ifPresent(laTexFileDirectory -> fileDirs.add(laTexFileDirectory));
.ifPresent(fileDirs::add);

return fileDirs;
}
Expand Down
32 changes: 16 additions & 16 deletions src/test/java/org/jabref/logic/exporter/GroupSerializerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,31 +30,31 @@

import static org.junit.jupiter.api.Assertions.assertEquals;

public class GroupSerializerTest {
class GroupSerializerTest {

private GroupSerializer groupSerializer;

@BeforeEach
public void setUp() throws Exception {
void setUp() throws Exception {
groupSerializer = new GroupSerializer();
}

@Test
public void serializeSingleAllEntriesGroup() {
void serializeSingleAllEntriesGroup() {
AllEntriesGroup group = new AllEntriesGroup("");
List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
assertEquals(Collections.singletonList("0 AllEntriesGroup:"), serialization);
}

@Test
public void serializeSingleExplicitGroup() {
void serializeSingleExplicitGroup() {
ExplicitGroup group = new ExplicitGroup("myExplicitGroup", GroupHierarchyType.INDEPENDENT, ',');
List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
assertEquals(Collections.singletonList("0 StaticGroup:myExplicitGroup;0;1;;;;"), serialization);
}

@Test
public void serializeSingleExplicitGroupWithIconAndDescription() {
void serializeSingleExplicitGroupWithIconAndDescription() {
ExplicitGroup group = new ExplicitGroup("myExplicitGroup", GroupHierarchyType.INDEPENDENT, ',');
group.setIconName("test icon");
group.setExpanded(true);
Expand All @@ -66,63 +66,63 @@ public void serializeSingleExplicitGroupWithIconAndDescription() {

@Test
// For https://github.com/JabRef/jabref/issues/1681
public void serializeSingleExplicitGroupWithEscapedSlash() {
void serializeSingleExplicitGroupWithEscapedSlash() {
ExplicitGroup group = new ExplicitGroup("B{\\\"{o}}hmer", GroupHierarchyType.INDEPENDENT, ',');
List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
assertEquals(Collections.singletonList("0 StaticGroup:B{\\\\\"{o}}hmer;0;1;;;;"), serialization);
}

@Test
public void serializeSingleSimpleKeywordGroup() {
void serializeSingleSimpleKeywordGroup() {
WordKeywordGroup group = new WordKeywordGroup("name", GroupHierarchyType.INDEPENDENT, "keywords", "test", false, ',', false);
List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
assertEquals(Collections.singletonList("0 KeywordGroup:name;0;keywords;test;0;0;1;;;;"), serialization);
}

@Test
public void serializeSingleRegexKeywordGroup() {
void serializeSingleRegexKeywordGroup() {
KeywordGroup group = new RegexKeywordGroup("myExplicitGroup", GroupHierarchyType.REFINING, "author", "asdf", false);
List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
assertEquals(Collections.singletonList("0 KeywordGroup:myExplicitGroup;1;author;asdf;0;1;1;;;;"), serialization);
}

@Test
public void serializeSingleSearchGroup() {
void serializeSingleSearchGroup() {
SearchGroup group = new SearchGroup("myExplicitGroup", GroupHierarchyType.INDEPENDENT, "author=harrer", true, true);
List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
assertEquals(Collections.singletonList("0 SearchGroup:myExplicitGroup;0;author=harrer;1;1;1;;;;"), serialization);
}

@Test
public void serializeSingleSearchGroupWithRegex() {
void serializeSingleSearchGroupWithRegex() {
SearchGroup group = new SearchGroup("myExplicitGroup", GroupHierarchyType.INCLUDING, "author=\"harrer\"", true, false);
List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
assertEquals(Collections.singletonList("0 SearchGroup:myExplicitGroup;2;author=\"harrer\";1;0;1;;;;"), serialization);
}

@Test
public void serializeSingleAutomaticKeywordGroup() {
void serializeSingleAutomaticKeywordGroup() {
AutomaticGroup group = new AutomaticKeywordGroup("myAutomaticGroup", GroupHierarchyType.INDEPENDENT, "keywords", ',', '>');
List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
assertEquals(Collections.singletonList("0 AutomaticKeywordGroup:myAutomaticGroup;0;keywords;,;>;1;;;;"), serialization);
}

@Test
public void serializeSingleAutomaticPersonGroup() {
void serializeSingleAutomaticPersonGroup() {
AutomaticPersonsGroup group = new AutomaticPersonsGroup("myAutomaticGroup", GroupHierarchyType.INDEPENDENT, "authors");
List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
assertEquals(Collections.singletonList("0 AutomaticPersonsGroup:myAutomaticGroup;0;authors;1;;;;"), serialization);
}

@Test
public void serializeSingleTexGroup() throws Exception {
TexGroup group = new TexGroup("myTexGroup", GroupHierarchyType.INDEPENDENT, Paths.get("path", "To", "File"), new DefaultAuxParser(new BibDatabase()), new DummyFileUpdateMonitor(), new MetaData());
void serializeSingleTexGroup() throws Exception {
TexGroup group = TexGroup.createWithoutFileMonitoring("myTexGroup", GroupHierarchyType.INDEPENDENT, Paths.get("path", "To", "File"), new DefaultAuxParser(new BibDatabase()), new DummyFileUpdateMonitor(), new MetaData());
List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
assertEquals(Collections.singletonList("0 TexGroup:myTexGroup;0;path/To/File;1;;;;"), serialization);
}

@Test
public void getTreeAsStringInSimpleTree() throws Exception {
void getTreeAsStringInSimpleTree() throws Exception {
GroupTreeNode root = GroupTreeNodeTest.getRoot();
GroupTreeNodeTest.getNodeInSimpleTree(root);

Expand All @@ -136,7 +136,7 @@ public void getTreeAsStringInSimpleTree() throws Exception {
}

@Test
public void getTreeAsStringInComplexTree() throws Exception {
void getTreeAsStringInComplexTree() throws Exception {
GroupTreeNode root = GroupTreeNodeTest.getRoot();
GroupTreeNodeTest.getNodeInComplexTree(root);

Expand Down
26 changes: 13 additions & 13 deletions src/test/java/org/jabref/logic/importer/util/GroupsParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,46 +27,46 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class GroupsParserTest {
class GroupsParserTest {
private FileUpdateMonitor fileMonitor;
private MetaData metaData;

@BeforeEach
public void setUp() throws Exception {
void setUp() throws Exception {
fileMonitor = new DummyFileUpdateMonitor();
metaData = new MetaData();
}

@Test
// For https://github.com/JabRef/jabref/issues/1681
public void fromStringParsesExplicitGroupWithEscapedCharacterInName() throws Exception {
void fromStringParsesExplicitGroupWithEscapedCharacterInName() throws Exception {
ExplicitGroup expected = new ExplicitGroup("B{\\\"{o}}hmer", GroupHierarchyType.INDEPENDENT, ',');
AbstractGroup parsed = GroupsParser.fromString("ExplicitGroup:B{\\\\\"{o}}hmer;0;", ',', fileMonitor, metaData);

assertEquals(expected, parsed);
}

@Test
public void keywordDelimiterThatNeedsToBeEscaped() throws Exception {
void keywordDelimiterThatNeedsToBeEscaped() throws Exception {
AutomaticGroup expected = new AutomaticKeywordGroup("group1", GroupHierarchyType.INDEPENDENT, "keywords", ';', '>');
AbstractGroup parsed = GroupsParser.fromString("AutomaticKeywordGroup:group1;0;keywords;\\;;>;1;;;;;", ';', fileMonitor, metaData);
assertEquals(expected, parsed);
}

@Test
public void hierarchicalDelimiterThatNeedsToBeEscaped() throws Exception {
void hierarchicalDelimiterThatNeedsToBeEscaped() throws Exception {
AutomaticGroup expected = new AutomaticKeywordGroup("group1", GroupHierarchyType.INDEPENDENT, "keywords", ',', ';');
AbstractGroup parsed = GroupsParser.fromString("AutomaticKeywordGroup:group1;0;keywords;,;\\;;1;;;;;", ';', fileMonitor, metaData);
assertEquals(expected, parsed);
}

@Test
public void fromStringThrowsParseExceptionForNotEscapedGroupName() throws Exception {
void fromStringThrowsParseExceptionForNotEscapedGroupName() throws Exception {
assertThrows(ParseException.class, () -> GroupsParser.fromString("ExplicitGroup:slit\\\\;0\\;mertsch_slit2_2007\\;;", ',', fileMonitor, metaData));
}

@Test
public void testImportSubGroups() throws Exception {
void testImportSubGroups() throws Exception {

List<String> orderedData = Arrays.asList("0 AllEntriesGroup:", "1 ExplicitGroup:1;0;",
"2 ExplicitGroup:2;0;", "0 ExplicitGroup:3;0;");
Expand All @@ -93,7 +93,7 @@ public void testImportSubGroups() throws Exception {
}

@Test
public void fromStringParsesExplicitGroupWithIconAndDescription() throws Exception {
void fromStringParsesExplicitGroupWithIconAndDescription() throws Exception {
ExplicitGroup expected = new ExplicitGroup("myExplicitGroup", GroupHierarchyType.INDEPENDENT, ',');
expected.setIconName("test icon");
expected.setExpanded(true);
Expand All @@ -105,28 +105,28 @@ public void fromStringParsesExplicitGroupWithIconAndDescription() throws Excepti
}

@Test
public void fromStringParsesAutomaticKeywordGroup() throws Exception {
void fromStringParsesAutomaticKeywordGroup() throws Exception {
AutomaticGroup expected = new AutomaticKeywordGroup("myAutomaticGroup", GroupHierarchyType.INDEPENDENT, "keywords", ',', '>');
AbstractGroup parsed = GroupsParser.fromString("AutomaticKeywordGroup:myAutomaticGroup;0;keywords;,;>;1;;;;", ',', fileMonitor, metaData);
assertEquals(expected, parsed);
}

@Test
public void fromStringParsesAutomaticPersonGroup() throws Exception {
void fromStringParsesAutomaticPersonGroup() throws Exception {
AutomaticPersonsGroup expected = new AutomaticPersonsGroup("myAutomaticGroup", GroupHierarchyType.INDEPENDENT, "authors");
AbstractGroup parsed = GroupsParser.fromString("AutomaticPersonsGroup:myAutomaticGroup;0;authors;1;;;;", ',', fileMonitor, metaData);
assertEquals(expected, parsed);
}

@Test
public void fromStringParsesTexGroup() throws Exception {
TexGroup expected = new TexGroup("myTexGroup", GroupHierarchyType.INDEPENDENT, Paths.get("path", "To", "File"), new DefaultAuxParser(new BibDatabase()), fileMonitor, metaData);
void fromStringParsesTexGroup() throws Exception {
TexGroup expected = TexGroup.createWithoutFileMonitoring("myTexGroup", GroupHierarchyType.INDEPENDENT, Paths.get("path", "To", "File"), new DefaultAuxParser(new BibDatabase()), fileMonitor, metaData);
AbstractGroup parsed = GroupsParser.fromString("TexGroup:myTexGroup;0;path/To/File;1;;;;", ',', fileMonitor, metaData);
assertEquals(expected, parsed);
}

@Test
public void fromStringUnknownGroupThrowsException() throws Exception {
void fromStringUnknownGroupThrowsException() throws Exception {
assertThrows(ParseException.class, () -> GroupsParser.fromString("0 UnknownGroup:myUnknownGroup;0;;1;;;;", ',', fileMonitor, metaData));
}
}