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/field inheritance #5282

Merged
merged 7 commits into from
Sep 9, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#

### Fixed

- Inherit fields from cross-referenced entries as specified by biblatex [#5045](https://github.com/JabRef/jabref/issues/5045)

### Removed


Expand Down
126 changes: 113 additions & 13 deletions src/main/java/org/jabref/model/entry/BibEntry.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.entry.identifier.DOI;
import org.jabref.model.entry.types.EntryType;
import org.jabref.model.entry.types.IEEETranEntryType;
import org.jabref.model.entry.types.StandardEntryType;
import org.jabref.model.strings.LatexToUnicodeAdapter;
import org.jabref.model.strings.StringUtil;
Expand Down Expand Up @@ -112,6 +113,110 @@ public Optional<String> getResolvedFieldOrAlias(OrFields fields, BibDatabase dat
return Optional.empty();
}

/**
* Map an (empty) field of a BibEntry to a field of a cross-referenced entry.
*
* @param targetField field name of the BibEntry
* @param targetEntry type of the BibEntry
* @param sourceEntry type of the cross-referenced BibEntry
*
* @return the mapped field or null if there is no valid mapping available
*/
private Field getSourceField(Field targetField, EntryType targetEntry, EntryType sourceEntry) {
sfo marked this conversation as resolved.
Show resolved Hide resolved
//// 1. Sort out forbidden fields
if ((targetField == StandardField.IDS) ||
Copy link
Member

Choose a reason for hiding this comment

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

you can simply the code a bit:
Create an EnumSet of the fields and check with contains if the field is in the set.
https://docs.oracle.com/javase/8/docs/api/java/util/EnumSet.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestion, but I am not sure, if this fits here. First, I have to list the fields anyway, so I don't think, it would simplify the code. Second, the set is accessed only in one single place, so making it a static class member would extend the scope unnecessarily and clutter the class member list. Creating it locally would improve neither code nor performance, I guess. What do you think?

(targetField == StandardField.CROSSREF) ||
(targetField == StandardField.XREF) ||
(targetField == StandardField.ENTRYSET) ||
(targetField == StandardField.RELATED) ||
(targetField == StandardField.SORTKEY)) {
return null;
}

//// 2. Handle special field mappings
if ((sourceEntry == StandardEntryType.MvBook && targetEntry == StandardEntryType.InBook) ||
(sourceEntry == StandardEntryType.MvBook && targetEntry == StandardEntryType.BookInBook) ||
(sourceEntry == StandardEntryType.MvBook && targetEntry == StandardEntryType.SuppBook) ||
(sourceEntry == StandardEntryType.Book && targetEntry == StandardEntryType.InBook) ||
(sourceEntry == StandardEntryType.Book && targetEntry == StandardEntryType.BookInBook) ||
(sourceEntry == StandardEntryType.Book && targetEntry == StandardEntryType.SuppBook)) {
if (targetField == StandardField.AUTHOR) { return StandardField.AUTHOR; }
if (targetField == StandardField.BOOKAUTHOR) { return StandardField.AUTHOR; }
}

if ((sourceEntry == StandardEntryType.MvBook && targetEntry == StandardEntryType.Book) ||
(sourceEntry == StandardEntryType.MvBook && targetEntry == StandardEntryType.InBook) ||
(sourceEntry == StandardEntryType.MvBook && targetEntry == StandardEntryType.BookInBook) ||
(sourceEntry == StandardEntryType.MvBook && targetEntry == StandardEntryType.SuppBook) ||
(sourceEntry == StandardEntryType.MvCollection && targetEntry == StandardEntryType.Collection) ||
(sourceEntry == StandardEntryType.MvCollection && targetEntry == StandardEntryType.InCollection) ||
(sourceEntry == StandardEntryType.MvCollection && targetEntry == StandardEntryType.SuppCollection) ||
(sourceEntry == StandardEntryType.MvProceedings && targetEntry == StandardEntryType.Proceedings) ||
(sourceEntry == StandardEntryType.MvProceedings && targetEntry == StandardEntryType.InProceedings) ||
(sourceEntry == StandardEntryType.MvReference && targetEntry == StandardEntryType.Reference) ||
(sourceEntry == StandardEntryType.MvReference && targetEntry == StandardEntryType.InReference)) {
if (targetField == StandardField.MAINTITLE) { return StandardField.TITLE; }
if (targetField == StandardField.MAINSUBTITLE) { return StandardField.SUBTITLE; }
if (targetField == StandardField.MAINTITLEADDON) { return StandardField.TITLEADDON; }

// those fields are no more available for the same-name inheritance strategy
if ((targetField == StandardField.TITLE) ||
(targetField == StandardField.SUBTITLE) ||
(targetField == StandardField.TITLEADDON)) {
return null;
}

// for these fields, inheritance is not allowed for the specified entry types
if ((targetField == StandardField.SHORTTITLE)) {
return null;
}
}

if ((sourceEntry == StandardEntryType.Book && targetEntry == StandardEntryType.InBook) ||
(sourceEntry == StandardEntryType.Book && targetEntry == StandardEntryType.BookInBook) ||
(sourceEntry == StandardEntryType.Book && targetEntry == StandardEntryType.SuppBook) ||
(sourceEntry == StandardEntryType.Collection && targetEntry == StandardEntryType.InCollection) ||
(sourceEntry == StandardEntryType.Collection && targetEntry == StandardEntryType.SuppCollection) ||
(sourceEntry == StandardEntryType.Reference && targetEntry == StandardEntryType.InReference) ||
(sourceEntry == StandardEntryType.Proceedings && targetEntry == StandardEntryType.InProceedings)) {
if (targetField == StandardField.BOOKTITLE) { return StandardField.TITLE; }
if (targetField == StandardField.BOOKSUBTITLE) { return StandardField.SUBTITLE; }
if (targetField == StandardField.BOOKTITLEADDON) { return StandardField.TITLEADDON; }

// those fields are no more available for the same-name inheritance strategy
if ((targetField == StandardField.TITLE) ||
(targetField == StandardField.SUBTITLE) ||
(targetField == StandardField.TITLEADDON)) {
return null;
}

// for these fields, inheritance is not allowed for the specified entry types
if ((targetField == StandardField.SHORTTITLE)) {
return null;
}
}

if ((sourceEntry == IEEETranEntryType.Periodical && targetEntry == StandardEntryType.Article) ||
(sourceEntry == IEEETranEntryType.Periodical && targetEntry == StandardEntryType.SuppPeriodical)) {
if (targetField == StandardField.JOURNALTITLE) { return StandardField.TITLE; }
if (targetField == StandardField.JOURNALSUBTITLE) { return StandardField.SUBTITLE; }

// those fields are no more available for the same-name inheritance strategy
if ((targetField == StandardField.TITLE) ||
(targetField == StandardField.SUBTITLE)) {
return null;
}

// for these fields, inheritance is not allowed for the specified entry types
if ((targetField == StandardField.SHORTTITLE)) {
return null;
}
}

//// 3. Fallback to inherit the field with the same name.
return targetField;
}

/**
* Returns the text stored in the given field of the given bibtex entry
* which belongs to the given database.
Expand All @@ -137,21 +242,16 @@ public Optional<String> getResolvedFieldOrAlias(Field field, BibDatabase databas

Optional<String> result = getFieldOrAlias(field);
// If this field is not set, and the entry has a crossref, try to look up the
// field in the referred entry: Do not do this for the bibtex key.
if (!result.isPresent() && (database != null)) {
// field in the referred entry, following the biblatex rules
if (result.isEmpty() && (database != null)) {
Optional<BibEntry> referred = database.getReferencedEntry(this);
if (referred.isPresent()) {
result = referred.get().getFieldOrAlias(field);
if (!result.isPresent() && type.equals(StandardEntryType.InProceedings)) {
if (field == StandardField.BOOKTITLE) {
result = referred.get().getFieldOrAlias(StandardField.TITLE);
}
else if (field == StandardField.BOOKSUBTITLE) {
result = referred.get().getFieldOrAlias(StandardField.SUBTITLE);
}
else if (field == StandardField.BOOKAUTHOR) {
result = referred.get().getFieldOrAlias(StandardField.AUTHOR);
}
EntryType sourceEntry = referred.get().type.get();
EntryType targetEntry = type.get();
Field sourceField = getSourceField(field, targetEntry, sourceEntry);

if (sourceField != null) {
result = referred.get().getFieldOrAlias(sourceField);
}
}
}
Expand Down
85 changes: 85 additions & 0 deletions src/test/java/org/jabref/model/entry/CrossrefTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package org.jabref.model.entry;

import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;

import org.jabref.logic.importer.ImportException;
import org.jabref.logic.importer.ImportFormatPreferences;
import org.jabref.logic.importer.ImportFormatReader;
import org.jabref.logic.xmp.XmpPreferences;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.util.DummyFileUpdateMonitor;

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.mockito.Answers;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class CrossrefTest {

private static BibDatabase database;

@BeforeAll
static void SetUp() throws ImportException, URISyntaxException {
ImportFormatReader reader = new ImportFormatReader();
ImportFormatPreferences importFormatPreferences = mock(ImportFormatPreferences.class, Answers.RETURNS_DEEP_STUBS);
when(importFormatPreferences.getEncoding()).thenReturn(StandardCharsets.UTF_8);
reader.resetImportFormats(importFormatPreferences, mock(XmpPreferences.class), new DummyFileUpdateMonitor());

Path file = Paths.get(CrossrefTest.class.getResource("crossref.bib").toURI());
database = reader.importFromFile("bibtex", file).getDatabase();
sfo marked this conversation as resolved.
Show resolved Hide resolved
}

private BibEntry getEntry(String key) {
var entries = database.getEntriesByKey(key);
assertEquals(1, entries.size());
return entries.get(0);
}

@Test
void inproceedings_proceedings_inheritance() {
var source = getEntry("pr_001");
var target = getEntry("inpr_001");

assertEquals(
source.getResolvedFieldOrAlias(StandardField.YEAR, database),
target.getResolvedFieldOrAlias(StandardField.YEAR, database)
);

assertEquals(
source.getResolvedFieldOrAlias(StandardField.TITLE, database),
target.getResolvedFieldOrAlias(StandardField.BOOKTITLE, database)
);
}

@Test
void inproceedings_proceedings_no_inheritance() {
var target = getEntry("inpr_001");

assertFalse(target.getResolvedFieldOrAlias(StandardField.TITLE, database).isPresent());
}

@Test
void inproceedings_proceedings_no_overwrite() {
var source = getEntry("pr_001");
var target = getEntry("inpr_002");

assertNotEquals(
source.getResolvedFieldOrAlias(StandardField.YEAR, database),
target.getResolvedFieldOrAlias(StandardField.YEAR, database)
);

assertNotEquals(
source.getResolvedFieldOrAlias(StandardField.TITLE, database),
target.getResolvedFieldOrAlias(StandardField.BOOKTITLE, database)
);
}
}
14 changes: 14 additions & 0 deletions src/test/resources/org/jabref/model/entry/crossref.bib
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
@Proceedings{pr_001,
title = {Proceedings_001},
year = {2019},
}

@InProceedings{inpr_001,
crossref = {pr_001},
}

@InProceedings{inpr_002,
crossref = {pr_001},
booktitle = {foobar},
year = {42}
}