Skip to content

Commit fdc8687

Browse files
kopporcalixtussubhramit
authored
Improve ConsistencyCheck code (and one minor other code place) (#13795)
* Remove strange variable * Remove duplicate statement * Nicer code format. * Support unknown types again * Fix test style * More nicer constructors * Remove obsolete statement * Update jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckDialogViewModel.java Co-authored-by: Subhramit Basu <subhramit.bb@live.in> --------- Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> Co-authored-by: Subhramit Basu <subhramit.bb@live.in>
1 parent 72399dc commit fdc8687

File tree

3 files changed

+45
-96
lines changed

3 files changed

+45
-96
lines changed

‎jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckDialogViewModel.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import java.util.ArrayList;
99
import java.util.Collection;
1010
import java.util.Comparator;
11-
import java.util.LinkedHashSet;
1211
import java.util.List;
1312
import java.util.Map;
1413
import java.util.Optional;
@@ -46,15 +45,13 @@
4645
public class ConsistencyCheckDialogViewModel extends AbstractViewModel {
4746

4847
private static final Logger LOGGER = LoggerFactory.getLogger(ConsistencyCheckDialogViewModel.class);
49-
private static final int EXTRA_COLUMNS_COUNT = 2;
5048

5149
private final BibliographyConsistencyCheck.Result result;
5250
private final DialogService dialogService;
5351
private final GuiPreferences preferences;
5452
private final BibEntryTypesManager entryTypesManager;
5553

5654
private final List<Field> allReportedFields;
57-
private final int columnCount;
5855
private final ObservableList<ConsistencyMessage> tableData = FXCollections.observableArrayList();
5956
private final StringProperty selectedEntryType = new SimpleStringProperty();
6057

@@ -72,7 +69,6 @@ public ConsistencyCheckDialogViewModel(DialogService dialogService,
7269
.sorted(Comparator.comparing(Field::getName))
7370
.distinct()
7471
.toList();
75-
this.columnCount = getColumnNames().size();
7672

7773
result.entryTypeToResultMap().entrySet().stream()
7874
.sorted(Comparator.comparing(entry -> entry.getKey().getName()))
@@ -93,8 +89,8 @@ public ObservableList<ConsistencyMessage> getTableData() {
9389
return tableData;
9490
}
9591

96-
public Set<String> getColumnNames() {
97-
Set<String> result = LinkedHashSet.newLinkedHashSet(columnCount + EXTRA_COLUMNS_COUNT);
92+
public List<String> getColumnNames() {
93+
List<String> result = new ArrayList<>(allReportedFields.size() + 2); // there are two extra columns
9894
result.add("Entry Type");
9995
result.add("CitationKey");
10096
allReportedFields.forEach(field-> result.add(field.getDisplayName().trim()));
@@ -138,7 +134,7 @@ private void writeBibEntry(BibEntry bibEntry, String entryType, Set<Field> requi
138134
}
139135

140136
private List<String> getFindingsAsList(BibEntry bibEntry, String entryType, Set<Field> requiredFields, Set<Field> optionalFields) {
141-
List<String> result = new ArrayList<>(columnCount + EXTRA_COLUMNS_COUNT);
137+
List<String> result = new ArrayList<>(allReportedFields.size() + 2);
142138
result.add(entryType);
143139
result.add(bibEntry.getCitationKey().orElse(""));
144140
allReportedFields.forEach(field ->

‎jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java‎

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,6 @@
3333

3434
public class BibliographyConsistencyCheck {
3535

36-
private static final Set<EntryType> BIBLATEX_TYPES = BiblatexEntryTypeDefinitions.ALL.stream()
37-
.map(BibEntryType::getType)
38-
.collect(Collectors.toSet());
39-
40-
private static final Set<EntryType> BIBTEX_TYPES = BibtexEntryTypeDefinitions.ALL.stream()
41-
.map(BibEntryType::getType)
42-
.collect(Collectors.toSet());
43-
4436
private static final Set<Field> EXPLICITLY_EXCLUDED_FIELDS = Set.of(
4537
InternalField.KEY_FIELD, // Citation key
4638
StandardField.KEY,
@@ -142,8 +134,6 @@ public Result check(BibDatabaseContext bibContext, BiConsumer<Integer, Integer>
142134
differingFields.removeAll(fieldsInAllEntries);
143135
assert fieldsInAllEntries != null;
144136

145-
differingFields.removeAll(fieldsInAllEntries);
146-
147137
Optional<BibEntryType> typeDefOpt = entryTypeDefinitions.stream()
148138
.filter(def -> def.getType().equals(entryType))
149139
.findFirst();
@@ -174,30 +164,22 @@ private static void collectEntriesIntoMaps(BibDatabaseContext bibContext, Map<En
174164
BibDatabaseMode mode = bibContext.getMode();
175165
List<BibEntry> entries = bibContext.getEntries();
176166

177-
Set<EntryType> typeSet = switch (mode) {
178-
case BIBLATEX -> BIBLATEX_TYPES;
179-
case BIBTEX -> BIBTEX_TYPES;
180-
};
181-
182167
for (BibEntry entry : entries) {
183-
if (typeSet.contains(entry.getType())) {
184-
EntryType entryType = entry.getType();
185-
186-
Set<Field> filteredFields = filterExcludedFields(entry.getFields());
187-
188-
entryTypeToFieldsInAnyEntryMap
189-
.computeIfAbsent(entryType, _ -> new HashSet<>())
190-
.addAll(filteredFields);
191-
if (entryTypeToFieldsInAllEntriesMap.containsKey(entryType)) {
192-
entryTypeToFieldsInAllEntriesMap.get(entryType).retainAll(filteredFields);
193-
} else {
194-
entryTypeToFieldsInAllEntriesMap.put(entryType, new HashSet<>(filteredFields));
195-
}
196-
197-
entryTypeToEntriesMap
198-
.computeIfAbsent(entryType, _ -> new HashSet<>())
199-
.add(entry);
200-
}
168+
EntryType entryType = entry.getType();
169+
170+
Set<Field> filteredFields = filterExcludedFields(entry.getFields());
171+
172+
entryTypeToFieldsInAllEntriesMap
173+
.computeIfAbsent(entryType, _ -> new HashSet<>(filteredFields))
174+
.retainAll(filteredFields);
175+
176+
entryTypeToFieldsInAnyEntryMap
177+
.computeIfAbsent(entryType, _ -> new HashSet<>())
178+
.addAll(filteredFields);
179+
180+
entryTypeToEntriesMap
181+
.computeIfAbsent(entryType, _ -> new HashSet<>())
182+
.add(entry);
201183
}
202184
}
203185
}

‎jablib/src/test/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckTest.java‎

Lines changed: 27 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package org.jabref.logic.quality.consistency;
22

33
import java.nio.file.Path;
4-
import java.util.HashSet;
54
import java.util.List;
65
import java.util.Map;
76
import java.util.Set;
@@ -32,10 +31,7 @@ void checkSimpleLibrary(@TempDir Path tempDir) {
3231
BibEntry second = new BibEntry(StandardEntryType.Article, "second")
3332
.withField(StandardField.AUTHOR, "Author One")
3433
.withField(StandardField.PUBLISHER, "publisher");
35-
BibDatabase database = new BibDatabase();
36-
database.insertEntry(first);
37-
database.insertEntry(second);
38-
34+
BibDatabase database = new BibDatabase(List.of(first, second));
3935
BibDatabaseContext bibContext = new BibDatabaseContext(database);
4036
bibContext.setMode(BibDatabaseMode.BIBTEX);
4137
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(bibContext, (count, total) -> { });
@@ -55,9 +51,7 @@ void checkDifferentOutputSymbols(@TempDir Path tempDir) {
5551
.withField(customField, "custom"); // unknown
5652
BibEntry second = new BibEntry(StandardEntryType.Article, "second")
5753
.withField(StandardField.AUTHOR, "Author One");
58-
List<BibEntry> bibEntriesList = List.of(first, second);
59-
BibDatabase bibDatabase = new BibDatabase();
60-
bibDatabase.insertEntries(bibEntriesList);
54+
BibDatabase bibDatabase = new BibDatabase(List.of(first, second));
6155
BibDatabaseContext bibContext = new BibDatabaseContext(bibDatabase);
6256
bibContext.setMode(BibDatabaseMode.BIBTEX);
6357
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(bibContext, (_, _) -> { });
@@ -89,9 +83,7 @@ void checkComplexLibrary(@TempDir Path tempDir) {
8983
.withField(StandardField.AUTHOR, "Author One")
9084
.withField(StandardField.YEAR, "2024");
9185

92-
List<BibEntry> bibEntriesList = List.of(first, second, third, fourth, fifth);
93-
BibDatabase bibDatabase = new BibDatabase();
94-
bibDatabase.insertEntries(bibEntriesList);
86+
BibDatabase bibDatabase = new BibDatabase(List.of(first, second, third, fourth, fifth));
9587
BibDatabaseContext bibContext = new BibDatabaseContext(bibDatabase);
9688

9789
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(bibContext, (_, _) -> { });
@@ -113,10 +105,7 @@ void checkLibraryWithoutIssues(@TempDir Path tempDir) {
113105
BibEntry second = new BibEntry(StandardEntryType.Article, "second")
114106
.withField(StandardField.AUTHOR, "Author One")
115107
.withField(StandardField.PAGES, "some pages");
116-
List<BibEntry> bibEntriesList = List.of(first, second);
117-
BibDatabase bibDatabase = new BibDatabase();
118-
bibDatabase.insertEntries(bibEntriesList);
119-
108+
BibDatabase bibDatabase = new BibDatabase(List.of(first, second));
120109
BibDatabaseContext bibContext = new BibDatabaseContext(bibDatabase);
121110

122111
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(bibContext, (_, _) -> { });
@@ -136,10 +125,7 @@ void filteredFieldsAreIgnored() {
136125
.withField(StandardField.COMMENT, "another note")
137126
.withField(StandardField.PDF, "other.pdf");
138127

139-
List<BibEntry> bibEntriesList = List.of(a, b);
140-
BibDatabase bibDatabase = new BibDatabase();
141-
bibDatabase.insertEntries(bibEntriesList);
142-
128+
BibDatabase bibDatabase = new BibDatabase(List.of(a, b));
143129
BibDatabaseContext bibContext = new BibDatabaseContext(bibDatabase);
144130

145131
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck()
@@ -155,9 +141,7 @@ void nonFilteredFieldDifferenceIsReported() {
155141
.withField(StandardField.AUTHOR, "Knuth");
156142
BibEntry withoutAuthor = new BibEntry(StandardEntryType.Misc, "2");
157143

158-
List<BibEntry> bibEntriesList = List.of(withAuthor, withoutAuthor);
159-
BibDatabase bibDatabase = new BibDatabase(bibEntriesList);
160-
bibDatabase.insertEntries(bibEntriesList);
144+
BibDatabase bibDatabase = new BibDatabase(List.of(withAuthor, withoutAuthor));
161145
BibDatabaseContext bibContext = new BibDatabaseContext(bibDatabase);
162146

163147
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck()
@@ -179,10 +163,7 @@ void unsetRequriedFieldsReported() {
179163
.withCitationKey("withoutDate")
180164
.withField(StandardField.URLDATE, "urldate");
181165

182-
List<BibEntry> bibEntriesList = List.of(withDate, withoutDate);
183-
BibDatabase bibDatabase = new BibDatabase(bibEntriesList);
184-
bibDatabase.insertEntries(bibEntriesList);
185-
166+
BibDatabase bibDatabase = new BibDatabase(List.of(withDate, withoutDate));
186167
BibDatabaseContext bibContext = new BibDatabaseContext(bibDatabase);
187168
bibContext.setMode(BibDatabaseMode.BIBLATEX);
188169

@@ -197,6 +178,7 @@ void unsetRequriedFieldsReported() {
197178

198179
@Test
199180
void unsetFieldsReportedInBibtexMode() {
181+
// "Online" is unknown in BibTeX, thus "date" should be reported as inconsistent (set only in one entry)
200182
BibEntry withDate = new BibEntry(StandardEntryType.Online)
201183
.withCitationKey("withDate")
202184
.withField(StandardField.DATE, "date")
@@ -205,28 +187,19 @@ void unsetFieldsReportedInBibtexMode() {
205187
.withCitationKey("withoutDate")
206188
.withField(StandardField.URLDATE, "urldate");
207189

208-
List<BibEntry> bibEntriesList = List.of(withDate, withoutDate);
209-
BibDatabase bibDatabase = new BibDatabase(bibEntriesList);
210-
bibDatabase.insertEntries(bibEntriesList);
211-
190+
BibDatabase bibDatabase = new BibDatabase(List.of(withDate, withoutDate));
212191
BibDatabaseContext bibContext = new BibDatabaseContext(bibDatabase);
213192
bibContext.setMode(BibDatabaseMode.BIBTEX);
214193
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck()
215194
.check(bibContext, (_, _) -> { });
195+
BibliographyConsistencyCheck.EntryTypeResult typeResult =
196+
result.entryTypeToResultMap().get(StandardEntryType.Online);
216197

217-
assertEquals(Map.of(), result.entryTypeToResultMap());
198+
assertEquals(List.of(withDate), typeResult.sortedEntries().stream().toList());
218199
}
219200

220201
@Test
221202
void checkFieldEntriesWithFieldDifferences() {
222-
Set<BibEntry> entries = new HashSet<>();
223-
Set<Field> differingFields = Set.of(
224-
StandardField.TITLE,
225-
StandardField.PAGES,
226-
new UnknownField("customField"),
227-
StandardField.PUBLISHER
228-
);
229-
230203
BibEntry entry1 = new BibEntry(StandardEntryType.Article, "id1")
231204
.withField(StandardField.AUTHOR, "Author One")
232205
.withField(StandardField.TITLE, "Title ")
@@ -247,18 +220,18 @@ void checkFieldEntriesWithFieldDifferences() {
247220
.withField(StandardField.AUTHOR, "Author Five")
248221
.withField(StandardField.PUBLISHER, "Editor");
249222

250-
entries.add(entry1);
251-
entries.add(entry2);
252-
entries.add(entry3);
253-
entries.add(entry4);
254-
entries.add(entry5);
255-
256-
BibliographyConsistencyCheck check = new BibliographyConsistencyCheck();
257-
258-
List<BibEntry> result = check.filterAndSortEntriesWithFieldDifferences(entries, differingFields, Set.of(StandardField.AUTHOR, StandardField.TITLE, StandardField.PAGES, StandardField.PDF));
259-
List<BibEntry> expected = List.of(entry1, entry2, entry3, entry4, entry5);
223+
Set<Field> differingFields = Set.of(
224+
StandardField.TITLE,
225+
StandardField.PAGES,
226+
new UnknownField("customField"),
227+
StandardField.PUBLISHER
228+
);
229+
List<BibEntry> result = new BibliographyConsistencyCheck().filterAndSortEntriesWithFieldDifferences(
230+
Set.of(entry1, entry2, entry3, entry4, entry5),
231+
differingFields,
232+
Set.of(StandardField.AUTHOR, StandardField.TITLE, StandardField.PAGES, StandardField.PDF));
260233

261-
assertEquals(Set.copyOf(expected), Set.copyOf(result));
234+
assertEquals(List.of(entry1, entry2, entry3, entry4, entry5), result);
262235
}
263236

264237
@Test
@@ -284,19 +257,17 @@ void checkComplexLibraryWithAdditionalEntry(@TempDir Path tempDir) {
284257
BibEntry sixth = new BibEntry(StandardEntryType.InProceedings, "sixth")
285258
.withField(StandardField.AUTHOR, "Author One");
286259

287-
List<BibEntry> bibEntriesList = List.of(first, second, third, fourth, fifth, sixth);
288-
BibDatabase bibDatabase = new BibDatabase();
289-
bibDatabase.insertEntries(bibEntriesList);
260+
BibDatabase bibDatabase = new BibDatabase(List.of(first, second, third, fourth, fifth, sixth));
290261
BibDatabaseContext bibContext = new BibDatabaseContext(bibDatabase);
291262

292-
BibliographyConsistencyCheck.Result expectedArticle = new BibliographyConsistencyCheck().check(bibContext, (_, _) -> { });
263+
BibliographyConsistencyCheck.Result actualResult = new BibliographyConsistencyCheck().check(bibContext, (_, _) -> { });
293264

294265
BibliographyConsistencyCheck.EntryTypeResult articleResult = new BibliographyConsistencyCheck.EntryTypeResult(Set.of(StandardField.PAGES, StandardField.PUBLISHER), List.of(first, second));
295266
BibliographyConsistencyCheck.EntryTypeResult expectedInProceedings = new BibliographyConsistencyCheck.EntryTypeResult(Set.of(StandardField.PAGES, StandardField.PUBLISHER, StandardField.LOCATION, StandardField.YEAR), List.of(fifth, fourth, sixth, third));
296-
BibliographyConsistencyCheck.Result expected = new BibliographyConsistencyCheck.Result(Map.of(
267+
BibliographyConsistencyCheck.Result expectedResult = new BibliographyConsistencyCheck.Result(Map.of(
297268
StandardEntryType.Article, articleResult,
298269
StandardEntryType.InProceedings, expectedInProceedings
299270
));
300-
assertEquals(expected, expectedArticle);
271+
assertEquals(expectedResult, actualResult);
301272
}
302273
}

0 commit comments

Comments
 (0)