Skip to content

Commit c15e554

Browse files
zikunzzikunz
committed
feat: implement JournalAbbreviationRepositoryManager for efficient repository caching
 - Add singleton repository manager with proper thread safety to cache repo - Fix issue where abbreviation operations would ignore disabled journal lists - Fix issue where toggling journal lists would update preferences when dialog is canceled - Update both abbreviation and unabbreviation to respect disabled sources - Apply double-checked locking pattern to ensure thread safety while preserving performance
1 parent d9437e2 commit c15e554

File tree

6 files changed

+188
-26
lines changed

6 files changed

+188
-26
lines changed

‎src/main/java/org/jabref/gui/journals/AbbreviateAction.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717
import org.jabref.gui.actions.StandardActions;
1818
import org.jabref.gui.undo.NamedCompound;
1919
import org.jabref.logic.journals.Abbreviation;
20-
import org.jabref.logic.journals.JournalAbbreviationLoader;
2120
import org.jabref.logic.journals.JournalAbbreviationPreferences;
2221
import org.jabref.logic.journals.JournalAbbreviationRepository;
22+
import org.jabref.logic.journals.JournalAbbreviationRepositoryManager;
2323
import org.jabref.logic.l10n.Localization;
2424
import org.jabref.logic.util.BackgroundTask;
2525
import org.jabref.logic.util.TaskExecutor;
@@ -108,9 +108,10 @@ public void execute() {
108108
}
109109

110110
private String abbreviate(BibDatabaseContext databaseContext, List<BibEntry> entries) {
111-
// Reload repository to ensure latest preferences are used
112-
abbreviationRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences);
113-
111+
// Use the repository manager to get a cached repository or build a new one if needed
112+
abbreviationRepository = JournalAbbreviationRepositoryManager.getInstance()
113+
.getRepository(journalAbbreviationPreferences);
114+
114115
UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator(
115116
abbreviationRepository,
116117
abbreviationType,
@@ -139,7 +140,8 @@ private String abbreviate(BibDatabaseContext databaseContext, List<BibEntry> ent
139140
private String unabbreviate(BibDatabaseContext databaseContext, List<BibEntry> entries) {
140141
List<BibEntry> filteredEntries = new ArrayList<>();
141142

142-
JournalAbbreviationRepository freshRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences);
143+
JournalAbbreviationRepository freshRepository = JournalAbbreviationRepositoryManager.getInstance()
144+
.getRepository(journalAbbreviationPreferences);
143145

144146
Map<String, Boolean> sourceEnabledStates = new HashMap<>();
145147
String builtInId = JournalAbbreviationRepository.BUILTIN_LIST_ID;

‎src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public boolean abbreviate(BibDatabase database, BibEntry entry, Field fieldName,
4242
String origText = entry.getField(fieldName).get();
4343
String text = database != null ? database.resolveForStrings(origText) : origText;
4444

45-
Optional<Abbreviation> foundAbbreviation = journalAbbreviationRepository.get(text);
45+
Optional<Abbreviation> foundAbbreviation = journalAbbreviationRepository.getForAbbreviation(text);
4646

4747
if (foundAbbreviation.isEmpty() && abbreviationType != AbbreviationType.LTWA) {
4848
return false; // Unknown, cannot abbreviate anything.

‎src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public boolean unabbreviate(BibDatabase database, BibEntry entry, Field field, C
4848
return false; // Cannot unabbreviate unabbreviated name.
4949
}
5050

51-
var abbreviationOpt = journalAbbreviationRepository.get(text);
51+
var abbreviationOpt = journalAbbreviationRepository.getForUnabbreviation(text);
5252
if (abbreviationOpt.isEmpty()) {
5353
return false;
5454
}

‎src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,10 @@
3232
import org.jabref.gui.preferences.PreferencesTab;
3333
import org.jabref.gui.util.ColorUtil;
3434
import org.jabref.gui.util.ValueTableCellFactory;
35-
import org.jabref.logic.journals.JournalAbbreviationLoader;
36-
import org.jabref.logic.journals.JournalAbbreviationPreferences;
3735
import org.jabref.logic.journals.JournalAbbreviationRepository;
3836
import org.jabref.logic.l10n.Localization;
3937
import org.jabref.logic.util.TaskExecutor;
4038

41-
import com.airhacks.afterburner.injection.Injector;
4239
import com.airhacks.afterburner.views.ViewLoader;
4340
import com.tobiasdiez.easybind.EasyBind;
4441
import jakarta.inject.Inject;
@@ -312,22 +309,6 @@ private void toggleEnableList() {
312309

313310
refreshComboBoxDisplay();
314311

315-
JournalAbbreviationPreferences abbreviationPreferences = preferences.getJournalAbbreviationPreferences();
316-
317-
if (selected.isBuiltInListProperty().get()) {
318-
abbreviationPreferences.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, newEnabledState);
319-
} else if (selected.getAbsolutePath().isPresent()) {
320-
String fileName = selected.getAbsolutePath().get().getFileName().toString();
321-
abbreviationPreferences.setSourceEnabled(fileName, newEnabledState);
322-
}
323-
324-
JournalAbbreviationRepository newRepository =
325-
JournalAbbreviationLoader.loadRepository(abbreviationPreferences);
326-
327-
Injector.setModelOrService(JournalAbbreviationRepository.class, newRepository);
328-
329-
abbreviationRepository = newRepository;
330-
331312
viewModel.markAsDirty();
332313
}
333314

‎src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,42 @@ public void setSourceEnabled(String sourceKey, boolean enabled) {
337337
enabledSources.put(sourceKey, enabled);
338338
}
339339

340+
/**
341+
* Specialized method for abbreviation that checks if a journal name is already in its
342+
* full form and requires abbreviation from an enabled source.
343+
*
344+
* @param input The journal name to check and abbreviate.
345+
* @return Optional containing the abbreviation object if the input is a full journal name
346+
* from an enabled source, empty otherwise.
347+
*/
348+
public Optional<Abbreviation> getForAbbreviation(String input) {
349+
String journal = input.trim().replaceAll(Matcher.quoteReplacement("\\&"), "&");
350+
351+
Optional<Abbreviation> customAbbreviation = customAbbreviations.stream()
352+
.filter(abbreviation -> isSourceEnabled(abbreviationSources.getOrDefault(abbreviation, BUILTIN_LIST_ID)))
353+
.filter(abbreviation -> isMatched(journal, abbreviation))
354+
.findFirst();
355+
356+
if (customAbbreviation.isPresent()) {
357+
return customAbbreviation;
358+
}
359+
360+
if (!isSourceEnabled(BUILTIN_LIST_ID)) {
361+
return Optional.empty();
362+
}
363+
364+
Optional<Abbreviation> builtInAbbreviation = Optional.ofNullable(fullToAbbreviationObject.get(journal))
365+
.or(() -> Optional.ofNullable(abbreviationToAbbreviationObject.get(journal)))
366+
.or(() -> Optional.ofNullable(dotlessToAbbreviationObject.get(journal)))
367+
.or(() -> Optional.ofNullable(shortestUniqueToAbbreviationObject.get(journal)));
368+
369+
if (builtInAbbreviation.isPresent()) {
370+
return builtInAbbreviation;
371+
}
372+
373+
return findAbbreviationFuzzyMatched(journal);
374+
}
375+
340376
public Optional<String> getNextAbbreviation(String text) {
341377
return get(text).map(abbreviation -> abbreviation.getNext(text));
342378
}

‎src/main/java/org/jabref/logic/journals/JournalAbbreviationRepositoryManager.java

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
package org.jabref.logic.journals;
2+
3+
import java.util.List;
4+
import java.util.Map;
5+
import java.util.Objects;
6+
import java.util.concurrent.locks.ReadWriteLock;
7+
import java.util.concurrent.locks.ReentrantReadWriteLock;
8+
9+
/**
10+
* Manages caching of JournalAbbreviationRepository instances to improve performance.
11+
* This class uses a thread-safe approach with read-write locks to ensure
12+
* concurrent access while minimizing repository rebuilds.
13+
*/
14+
public class JournalAbbreviationRepositoryManager {
15+
16+
private static final JournalAbbreviationRepositoryManager INSTANCE = new JournalAbbreviationRepositoryManager();
17+
private static final ReadWriteLock LOCK = new ReentrantReadWriteLock();
18+
19+
private JournalAbbreviationRepository repository;
20+
private JournalAbbreviationPreferences lastUsedPreferences;
21+
22+
/**
23+
* Private constructor for singleton pattern
24+
*/
25+
private JournalAbbreviationRepositoryManager() {
26+
// Private constructor to prevent instantiation
27+
}
28+
29+
/**
30+
* Returns the singleton instance of the repository manager
31+
*
32+
* @return The singleton instance
33+
*/
34+
public static JournalAbbreviationRepositoryManager getInstance() {
35+
return INSTANCE;
36+
}
37+
38+
/**
39+
* Gets a repository instance for the given preferences. This method implements
40+
* caching to avoid rebuilding the repository if preferences haven't changed.
41+
* Uses double-checked locking for thread safety and efficiency.
42+
*
43+
* @param preferences The journal abbreviation preferences
44+
* @return A repository instance configured with the given preferences
45+
*/
46+
public JournalAbbreviationRepository getRepository(JournalAbbreviationPreferences preferences) {
47+
Objects.requireNonNull(preferences);
48+
49+
LOCK.readLock().lock();
50+
try {
51+
if (repository != null && !preferencesChanged(preferences)) {
52+
return repository;
53+
}
54+
} finally {
55+
LOCK.readLock().unlock();
56+
}
57+
58+
LOCK.writeLock().lock();
59+
try {
60+
if (repository != null && !preferencesChanged(preferences)) {
61+
return repository;
62+
}
63+
64+
repository = JournalAbbreviationLoader.loadRepository(preferences);
65+
lastUsedPreferences = clonePreferences(preferences);
66+
return repository;
67+
} finally {
68+
LOCK.writeLock().unlock();
69+
}
70+
}
71+
72+
/**
73+
* Checks if the preferences have changed since the last repository build
74+
*
75+
* @param preferences The current preferences to check
76+
* @return true if preferences have changed, false otherwise
77+
*/
78+
private boolean preferencesChanged(JournalAbbreviationPreferences preferences) {
79+
if (lastUsedPreferences == null) {
80+
return true;
81+
}
82+
83+
if (lastUsedPreferences.shouldUseFJournalField() != preferences.shouldUseFJournalField()) {
84+
return true;
85+
}
86+
87+
List<String> oldLists = lastUsedPreferences.getExternalJournalLists();
88+
List<String> newLists = preferences.getExternalJournalLists();
89+
90+
if (oldLists.size() != newLists.size()) {
91+
return true;
92+
}
93+
94+
for (int i = 0; i < oldLists.size(); i++) {
95+
if (!Objects.equals(oldLists.get(i), newLists.get(i))) {
96+
return true;
97+
}
98+
}
99+
100+
Map<String, Boolean> oldEnabled = lastUsedPreferences.getEnabledExternalLists();
101+
Map<String, Boolean> newEnabled = preferences.getEnabledExternalLists();
102+
103+
if (oldEnabled.size() != newEnabled.size()) {
104+
return true;
105+
}
106+
107+
for (Map.Entry<String, Boolean> entry : newEnabled.entrySet()) {
108+
Boolean oldValue = oldEnabled.get(entry.getKey());
109+
if (!Objects.equals(oldValue, entry.getValue())) {
110+
return true;
111+
}
112+
}
113+
114+
return false;
115+
}
116+
117+
/**
118+
* Creates a clone of the preferences for comparison
119+
*
120+
* @param preferences The preferences to clone
121+
* @return A new preferences instance with the same settings
122+
*/
123+
private JournalAbbreviationPreferences clonePreferences(JournalAbbreviationPreferences preferences) {
124+
return new JournalAbbreviationPreferences(
125+
preferences.getExternalJournalLists(),
126+
preferences.shouldUseFJournalField(),
127+
preferences.getEnabledExternalLists()
128+
);
129+
}
130+
131+
/**
132+
* For testing purposes only - clears the cached repository
133+
*/
134+
public void clear() {
135+
LOCK.writeLock().lock();
136+
try {
137+
repository = null;
138+
lastUsedPreferences = null;
139+
} finally {
140+
LOCK.writeLock().unlock();
141+
}
142+
}
143+
}

0 commit comments

Comments
 (0)