Add bibtex key deviation check (#2328)

* Add bibtex key deviation check
* Remove shared state in BibtexKeyPatternUtil which could cause concurrency issues.
* Make usage of GlobalBibtexKeyPattern easier.
* Simpler makeLabel methods.

CHANGELOG.md

@@ -14,8 +14,15 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 ### Changed
 - URLs can now be passed as arguments to the `-import` and `-importToOpen` command line options. The referenced file is downloaded and then imported as usual.
 - The default emacs executable name on linux changed from `gnuclient` to `emacsclient`. [feature-request 433](https://sourceforge.net/p/jabref/feature-requests/433/)
+- Adds integrity check to detect all bibtex keys which deviate from their generation pattern [#2206](https://github.com/JabRef/jabref/issues/2206)
 - Adds an integrity check that detects invalid DOIs [#1445](https://github.com/JabRef/jabref/issues/1445)
 
+
+
+
+
+
+
 ### Fixed
 - We fixed an issue which caused a metadata loss on reconnection to shared database. [#2219](https://github.com/JabRef/jabref/issues/2219)
 - We fixed an issue which caused an internal error when leaving the file path field empty and connecting to a shared database.

src/databaseTest/java/net/sf/jabref/shared/DBMSSynchronizerTest.java

@@ -10,7 +10,6 @@
 
 import net.sf.jabref.logic.exporter.MetaDataSerializer;
 import net.sf.jabref.logic.formatter.casechanger.LowerCaseFormatter;
-import net.sf.jabref.model.bibtexkeypattern.AbstractBibtexKeyPattern;
 import net.sf.jabref.model.bibtexkeypattern.GlobalBibtexKeyPattern;
 import net.sf.jabref.model.cleanup.FieldFormatterCleanup;
 import net.sf.jabref.model.cleanup.FieldFormatterCleanups;
@@ -53,7 +52,7 @@ public void setUp() throws SQLException, DatabaseNotSupportedException, InvalidD
         bibDatabase = new BibDatabase();
         BibDatabaseContext context = new BibDatabaseContext(bibDatabase);
 
-        pattern = new GlobalBibtexKeyPattern(AbstractBibtexKeyPattern.split("[auth][year]"));
+        pattern = GlobalBibtexKeyPattern.fromPattern("[auth][year]");
 
         dbmsSynchronizer = new DBMSSynchronizer(context, ',', pattern);
         dbmsProcessor = DBMSProcessor.getProcessorInstance(dbmsConnection);

src/databaseTest/java/net/sf/jabref/shared/SynchronizationTestSimulator.java

@@ -4,7 +4,6 @@
 import java.util.Collection;
 
 import net.sf.jabref.model.Defaults;
-import net.sf.jabref.model.bibtexkeypattern.AbstractBibtexKeyPattern;
 import net.sf.jabref.model.bibtexkeypattern.GlobalBibtexKeyPattern;
 import net.sf.jabref.model.database.BibDatabaseContext;
 import net.sf.jabref.model.database.BibDatabaseMode;
@@ -40,7 +39,7 @@ public class SynchronizationTestSimulator {
     public void setUp() throws SQLException, DatabaseNotSupportedException, InvalidDBMSConnectionPropertiesException {
         this.dbmsConnection = TestConnector.getTestDBMSConnection(dbmsType);
 
-        GlobalBibtexKeyPattern pattern = new GlobalBibtexKeyPattern(AbstractBibtexKeyPattern.split("[auth][year]"));
+        GlobalBibtexKeyPattern pattern = GlobalBibtexKeyPattern.fromPattern("[auth][year]");
         clientContextA = new BibDatabaseContext(new Defaults(BibDatabaseMode.BIBTEX), DatabaseLocation.SHARED, ',',
                 pattern);
         clientContextA.getDBMSSynchronizer().openSharedDatabase(dbmsConnection);

src/main/java/net/sf/jabref/cli/ArgumentProcessor.java

@@ -453,7 +453,7 @@ private void regenerateBibtexKeys(List<ParserResult> loaded) {
                 LOGGER.info(Localization.lang("Regenerating BibTeX keys according to metadata"));
                 for (BibEntry entry : database.getEntries()) {
                     // try to make a new label
-                    BibtexKeyPatternUtil.makeLabel(
+                    BibtexKeyPatternUtil.makeAndSetLabel(
                             metaData.getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern()),
                             database, entry, Globals.prefs.getBibtexKeyPatternPreferences());
                 }

src/main/java/net/sf/jabref/gui/BasePanel.java

@@ -433,7 +433,7 @@ public void run() {
                         .getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern());
                 for (BibEntry entry : entries) {
                     String oldCiteKey = entry.getCiteKeyOptional().orElse("");
-                    BibtexKeyPatternUtil.makeLabel(citeKeyPattern, bibDatabaseContext.getDatabase(),
+                    BibtexKeyPatternUtil.makeAndSetLabel(citeKeyPattern, bibDatabaseContext.getDatabase(),
                             entry, Globals.prefs.getBibtexKeyPatternPreferences());
                     String newCiteKey = entry.getCiteKeyOptional().orElse("");
                     if (!oldCiteKey.equals(newCiteKey)) {
@@ -1893,7 +1893,7 @@ public void autoGenerateKeysBeforeSaving() {
             for (BibEntry bes : bibDatabaseContext.getDatabase().getEntries()) {
                 Optional<String> oldKey = bes.getCiteKeyOptional();
                 if (!(oldKey.isPresent()) || oldKey.get().isEmpty()) {
-                    BibtexKeyPatternUtil.makeLabel(bibDatabaseContext.getMetaData()
+                    BibtexKeyPatternUtil.makeAndSetLabel(bibDatabaseContext.getMetaData()
                             .getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern()),
                             bibDatabaseContext.getDatabase(),
                             bes, Globals.prefs.getBibtexKeyPatternPreferences());

src/main/java/net/sf/jabref/gui/actions/IntegrityCheckAction.java

@@ -47,7 +47,8 @@ public IntegrityCheckAction(JabRefFrame frame) {
     @Override
     public void actionPerformed(ActionEvent e) {
         IntegrityCheck check = new IntegrityCheck(frame.getCurrentBasePanel().getBibDatabaseContext(),
-                Globals.prefs.getFileDirectoryPreferences());
+                Globals.prefs.getFileDirectoryPreferences(),
+                Globals.prefs.getBibtexKeyPatternPreferences());
         List<IntegrityMessage> messages = check.checkBibtexDatabase();
 
         if (messages.isEmpty()) {

src/main/java/net/sf/jabref/gui/bibtexkeypattern/BibtexKeyPatternPanel.java

@@ -224,8 +224,9 @@ private void fillPatternUsingPanelData(AbstractBibtexKeyPattern keypatterns) {
     }
 
     protected GlobalBibtexKeyPattern getKeyPatternAsGlobalBibtexKeyPattern() {
-        GlobalBibtexKeyPattern res = new GlobalBibtexKeyPattern(
-                AbstractBibtexKeyPattern.split(JabRefPreferences.getInstance().get(JabRefPreferences.DEFAULT_BIBTEX_KEY_PATTERN)));
+        GlobalBibtexKeyPattern res = GlobalBibtexKeyPattern.fromPattern(
+                JabRefPreferences.getInstance().get(JabRefPreferences.DEFAULT_BIBTEX_KEY_PATTERN)
+        );
         fillPatternUsingPanelData(res);
         return res;
     }

src/main/java/net/sf/jabref/gui/bibtexkeypattern/SearchFixDuplicateLabels.java

@@ -93,7 +93,7 @@ public void update() {
             NamedCompound ce = new NamedCompound(Localization.lang("Resolve duplicate BibTeX keys"));
             for (BibEntry entry : toGenerateFor) {
                 String oldKey = entry.getCiteKeyOptional().orElse(null);
-                BibtexKeyPatternUtil.makeLabel(panel.getBibDatabaseContext().getMetaData()
+                BibtexKeyPatternUtil.makeAndSetLabel(panel.getBibDatabaseContext().getMetaData()
                         .getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern()),
                         panel.getDatabase(), entry,
                         Globals.prefs.getBibtexKeyPatternPreferences());

src/main/java/net/sf/jabref/gui/entryeditor/EntryEditor.java

@@ -1256,7 +1256,7 @@ public void actionPerformed(ActionEvent e) {
                 }
             }
 
-            BibtexKeyPatternUtil.makeLabel(panel.getBibDatabaseContext().getMetaData()
+            BibtexKeyPatternUtil.makeAndSetLabel(panel.getBibDatabaseContext().getMetaData()
                     .getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern()), panel.getDatabase(), entry,
                     Globals.prefs.getBibtexKeyPatternPreferences());
 

src/main/java/net/sf/jabref/gui/importer/ImportInspectionDialog.java

@@ -462,7 +462,7 @@ private void generateKeyForEntry(BibEntry entry) {
             database.insertEntry(entry);
 
             // Generate a unique key:
-            BibtexKeyPatternUtil.makeLabel(
+            BibtexKeyPatternUtil.makeAndSetLabel(
                     localMetaData.getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern()),
                     database, entry,
                     Globals.prefs.getBibtexKeyPatternPreferences());
@@ -506,7 +506,7 @@ private void generateKeys() {
                 entry.setId(IdGenerator.next());
                 database.insertEntry(entry);
 
-                BibtexKeyPatternUtil.makeLabel(
+                BibtexKeyPatternUtil.makeAndSetLabel(
                         localMetaData.getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern()),
                         database, entry,
                         Globals.prefs.getBibtexKeyPatternPreferences());

src/main/java/net/sf/jabref/gui/openoffice/OpenOfficePanel.java

@@ -685,7 +685,7 @@ private boolean checkThatEntriesHaveKeys(List<BibEntry> entries) {
                 if (!entry.getCiteKeyOptional().isPresent()) {
                     // Generate key
                     BibtexKeyPatternUtil
-                            .makeLabel(
+                            .makeAndSetLabel(
                                     panel.getBibDatabaseContext().getMetaData().getCiteKeyPattern(prefs.getKeyPattern()),
                                     panel.getDatabase(), entry,
                             prefs);

src/main/java/net/sf/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java

@@ -16,6 +16,7 @@
 import net.sf.jabref.model.bibtexkeypattern.AbstractBibtexKeyPattern;
 import net.sf.jabref.model.cleanup.Formatter;
 import net.sf.jabref.model.database.BibDatabase;
+import net.sf.jabref.model.database.BibDatabaseContext;
 import net.sf.jabref.model.entry.AuthorList;
 import net.sf.jabref.model.entry.BibEntry;
 import net.sf.jabref.model.entry.FieldName;
@@ -42,17 +43,6 @@ public class BibtexKeyPatternUtil {
 
     private static final int CHARS_OF_FIRST = 5;
 
-    private static BibDatabase database;
-
-    /**
-     * Required for LabelPatternUtilTest
-     *
-     * @param db the DB to use as global database
-     */
-    public static void setDataBase(BibDatabase db) {
-        database = db;
-    }
-
     private static String normalize(String content) {
         List<String> tokens = new ArrayList<>();
         int b = 0;
@@ -374,13 +364,17 @@ private static String generateInstitutionKey(String content) {
      * The given database is used to avoid duplicate keys.
      *
      * @param citeKeyPattern
-     * @param dBase a <code>BibDatabase</code>
+     * @param database a <code>BibDatabase</code>
      * @param entry a <code>BibEntry</code>
      * @return modified BibEntry
      */
-    public static void makeLabel(AbstractBibtexKeyPattern citeKeyPattern, BibDatabase dBase, BibEntry entry,
+    public static void makeAndSetLabel(AbstractBibtexKeyPattern citeKeyPattern, BibDatabase database, BibEntry entry,
             BibtexKeyPatternPreferences bibtexKeyPatternPreferences) {
-        database = dBase;
+        String newKey = makeLabel(citeKeyPattern, database, entry, bibtexKeyPatternPreferences);
+        entry.setCiteKey(newKey);
+    }
+
+    private static String makeLabel(AbstractBibtexKeyPattern citeKeyPattern, BibDatabase database, BibEntry entry, BibtexKeyPatternPreferences bibtexKeyPatternPreferences) {
         String key;
         StringBuilder stringBuilder = new StringBuilder();
         try {
@@ -401,7 +395,7 @@ public static void makeLabel(AbstractBibtexKeyPattern citeKeyPattern, BibDatabas
                     // check whether there is a modifier on the end such as
                     // ":lower"
                     List<String> parts = parseFieldMarker(typeListEntry);
-                    String label = makeLabel(entry, parts.get(0), bibtexKeyPatternPreferences.getKeywordDelimiter());
+                    String label = makeLabel(entry, parts.get(0), bibtexKeyPatternPreferences.getKeywordDelimiter(), database);
 
                     // apply modifier if present
                     if (parts.size() > 1) {
@@ -438,8 +432,9 @@ public static void makeLabel(AbstractBibtexKeyPattern citeKeyPattern, BibDatabas
         boolean alwaysAddLetter = bibtexKeyPatternPreferences.isAlwaysAddLetter();
         boolean firstLetterA = bibtexKeyPatternPreferences.isFirstLetterA();
 
+        String newKey;
         if (!alwaysAddLetter && (occurrences == 0)) {
-            entry.setCiteKey(key);
+            newKey = key;
         } else {
             // The key is already in use, so we must modify it.
             int number = !alwaysAddLetter && !firstLetterA ? 1 : 0;
@@ -456,8 +451,9 @@ public static void makeLabel(AbstractBibtexKeyPattern citeKeyPattern, BibDatabas
                 }
             } while (occurrences > 0);
 
-            entry.setCiteKey(moddedKey);
+            newKey = moddedKey;
         }
+        return newKey;
     }
 
     /**
@@ -508,7 +504,7 @@ public static String applyModifiers(final String label, final List<String> parts
         return resultingLabel;
     }
 
-    public static String makeLabel(BibEntry entry, String value, Character keywordDelimiter) {
+    public static String makeLabel(BibEntry entry, String value, Character keywordDelimiter, BibDatabase database) {
         String val = value;
         try {
             if (val.startsWith("auth") || val.startsWith("pureauth")) {
@@ -1304,4 +1300,10 @@ public static String checkLegalKey(String key, boolean enforceLegalKey) {
         return StringUtil.replaceSpecialCharacters(newKey.toString());
     }
 
+    public static String makeLabel(BibDatabaseContext bibDatabaseContext,
+            BibEntry entry,
+            BibtexKeyPatternPreferences bibtexKeyPatternPreferences) {
+        AbstractBibtexKeyPattern citeKeyPattern = bibDatabaseContext.getMetaData().getCiteKeyPattern(bibtexKeyPatternPreferences.getKeyPattern());
+        return makeLabel(citeKeyPattern, bibDatabaseContext.getDatabase(), entry, bibtexKeyPatternPreferences);
+    }
 }

src/main/java/net/sf/jabref/logic/importer/fileformat/FreeCiteImporter.java

@@ -203,7 +203,7 @@ public ParserResult importEntries(String text) {
                     e.setType(type);
 
                     // autogenerate label (BibTeX key)
-                    BibtexKeyPatternUtil.makeLabel(
+                    BibtexKeyPatternUtil.makeAndSetLabel(
                             JabRefGUI.getMainFrame().getCurrentBasePanel().getBibDatabaseContext().getMetaData()
                                     .getCiteKeyPattern(importFormatPreferences.getBibtexKeyPatternPreferences().getKeyPattern()),
                             JabRefGUI.getMainFrame().getCurrentBasePanel().getDatabase(), e,

src/main/java/net/sf/jabref/logic/integrity/BibtexkeyDeviationChecker.java

@@ -0,0 +1,45 @@
+package net.sf.jabref.logic.integrity;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+import net.sf.jabref.logic.bibtexkeypattern.BibtexKeyPatternPreferences;
+import net.sf.jabref.logic.bibtexkeypattern.BibtexKeyPatternUtil;
+import net.sf.jabref.logic.integrity.IntegrityCheck.Checker;
+import net.sf.jabref.logic.l10n.Localization;
+import net.sf.jabref.model.database.BibDatabaseContext;
+import net.sf.jabref.model.entry.BibEntry;
+
+public class BibtexkeyDeviationChecker implements Checker {
+
+    private final BibDatabaseContext bibDatabaseContext;
+    private final BibtexKeyPatternPreferences bibtexKeyPatternPreferences;
+
+    public BibtexkeyDeviationChecker(BibDatabaseContext bibDatabaseContext, BibtexKeyPatternPreferences bibtexKeyPatternPreferences) {
+        this.bibDatabaseContext = Objects.requireNonNull(bibDatabaseContext);
+        this.bibtexKeyPatternPreferences = Objects.requireNonNull(bibtexKeyPatternPreferences);
+    }
+
+    @Override
+    public List<IntegrityMessage> check(BibEntry entry) {
+        Optional<String> valuekey = entry.getCiteKeyOptional();
+        if (!valuekey.isPresent()) {
+            return Collections.emptyList();
+        }
+
+        String key = valuekey.get();
+
+        // generate new key
+        String generatedKey = BibtexKeyPatternUtil.makeLabel(bibDatabaseContext, entry, bibtexKeyPatternPreferences);
+
+        if (!Objects.equals(key, generatedKey)) {
+            return Collections.singletonList(new IntegrityMessage(
+                    Localization.lang("BibTeX key %0 deviates from generated key %1", key, generatedKey), entry, BibEntry.KEY_FIELD));
+        }
+
+        return Collections.emptyList();
+    }
+
+}

src/main/java/net/sf/jabref/logic/integrity/IntegrityCheck.java

@@ -4,6 +4,7 @@
 import java.util.List;
 import java.util.Objects;
 
+import net.sf.jabref.logic.bibtexkeypattern.BibtexKeyPatternPreferences;
 import net.sf.jabref.model.database.BibDatabaseContext;
 import net.sf.jabref.model.entry.BibEntry;
 import net.sf.jabref.model.entry.FieldName;
@@ -14,10 +15,15 @@ public class IntegrityCheck {
 
     private final BibDatabaseContext bibDatabaseContext;
     private final FileDirectoryPreferences fileDirectoryPreferences;
+    private final BibtexKeyPatternPreferences bibtexKeyPatternPreferences;
 
-    public IntegrityCheck(BibDatabaseContext bibDatabaseContext, FileDirectoryPreferences fileDirectoryPreferences) {
+    public IntegrityCheck(BibDatabaseContext bibDatabaseContext,
+            FileDirectoryPreferences fileDirectoryPreferences,
+            BibtexKeyPatternPreferences bibtexKeyPatternPreferences
+    ) {
         this.bibDatabaseContext = Objects.requireNonNull(bibDatabaseContext);
         this.fileDirectoryPreferences = Objects.requireNonNull(fileDirectoryPreferences);
+        this.bibtexKeyPatternPreferences = Objects.requireNonNull(bibtexKeyPatternPreferences);
     }
 
     public List<IntegrityMessage> checkBibtexDatabase() {
@@ -72,6 +78,7 @@ private List<IntegrityMessage> checkBibtexEntry(BibEntry entry) {
         result.addAll(new ISBNChecker().check(entry));
         result.addAll(new DOIValidityChecker().check(entry));
         result.addAll(new EntryLinkChecker(bibDatabaseContext.getDatabase()).check(entry));
+        result.addAll(new BibtexkeyDeviationChecker(bibDatabaseContext, bibtexKeyPatternPreferences).check(entry));
 
         return result;
     }

src/main/java/net/sf/jabref/logic/util/io/RegExpFileSearch.java

@@ -324,7 +324,7 @@ public static String getFieldAndFormat(String fieldAndFormat, BibEntry entry, Bi
 
         // If no field value was found, try to interpret it as a key generator field marker:
         String fieldValue = entry.getResolvedFieldOrAlias(beforeColon, database)
-                .orElse(BibtexKeyPatternUtil.makeLabel(entry, beforeColon, keywordDelimiter));
+                .orElse(BibtexKeyPatternUtil.makeLabel(entry, beforeColon, keywordDelimiter, database));
 
         if (fieldValue == null) {
             return "";

src/main/java/net/sf/jabref/migrations/PreferencesMigrations.java

@@ -7,7 +7,6 @@
 
 import net.sf.jabref.Globals;
 import net.sf.jabref.JabRefMain;
-import net.sf.jabref.model.bibtexkeypattern.AbstractBibtexKeyPattern;
 import net.sf.jabref.model.bibtexkeypattern.GlobalBibtexKeyPattern;
 import net.sf.jabref.model.entry.FieldName;
 import net.sf.jabref.preferences.JabRefPreferences;
@@ -131,8 +130,9 @@ private static void migrateTypedKeyPrefs(JabRefPreferences prefs, Preferences ol
             throws BackingStoreException {
         LOGGER.info("Found old Bibtex Key patterns which will be migrated to new version.");
 
-        GlobalBibtexKeyPattern keyPattern = new GlobalBibtexKeyPattern(AbstractBibtexKeyPattern
-                .split(prefs.get(JabRefPreferences.DEFAULT_BIBTEX_KEY_PATTERN)));
+        GlobalBibtexKeyPattern keyPattern = GlobalBibtexKeyPattern.fromPattern(
+                prefs.get(JabRefPreferences.DEFAULT_BIBTEX_KEY_PATTERN)
+        );
         for (String key : oldPatternPrefs.keys()) {
             keyPattern.addBibtexKeyPattern(key, oldPatternPrefs.get(key, null));
         }

src/main/java/net/sf/jabref/model/bibtexkeypattern/GlobalBibtexKeyPattern.java

@@ -10,6 +10,10 @@ public GlobalBibtexKeyPattern(List<String> bibtexKeyPattern) {
         defaultBibtexKeyPattern = bibtexKeyPattern;
     }
 
+    public static GlobalBibtexKeyPattern fromPattern(String pattern) {
+        return new GlobalBibtexKeyPattern(split(pattern));
+    }
+
     @Override
     public List<String> getLastLevelBibtexKeyPattern(String key) {
         return defaultBibtexKeyPattern;

src/main/java/net/sf/jabref/pdfimport/PdfImporter.java

@@ -236,7 +236,7 @@ private void doContentImport(String fileName, List<BibEntry> res) {
         // insert entry to database and link file
         panel.getDatabase().insertEntry(entry);
         panel.markBaseChanged();
-        BibtexKeyPatternUtil.makeLabel(panel.getBibDatabaseContext().getMetaData()
+        BibtexKeyPatternUtil.makeAndSetLabel(panel.getBibDatabaseContext().getMetaData()
                 .getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern()), panel.getDatabase(), entry,
                 Globals.prefs.getBibtexKeyPatternPreferences());
         DroppedFileHandler dfh = new DroppedFileHandler(frame, panel);