Remove teamscale findings in the database package (JabRef#1577)

tschechlovdev · stefan-kolb · commit 357be5c596a3 · 2016-09-03T14:25:44.000+02:00
diff --git a/src/main/java/net/sf/jabref/model/database/BibDatabase.java b/src/main/java/net/sf/jabref/model/database/BibDatabase.java
@@ -13,6 +13,7 @@
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 import net.sf.jabref.event.source.EntryEventSource;
 import net.sf.jabref.model.EntryTypes;
@@ -21,7 +22,6 @@
 import net.sf.jabref.model.entry.EntryType;
 import net.sf.jabref.model.entry.EntryUtil;
 import net.sf.jabref.model.entry.FieldName;
-import net.sf.jabref.model.entry.InternalBibtexFields;
 import net.sf.jabref.model.entry.MonthUtil;
 import net.sf.jabref.model.event.EntryAddedEvent;
 import net.sf.jabref.model.event.EntryChangedEvent;
@@ -59,7 +59,6 @@ public class BibDatabase {
      */
     private final Set<String> internalIDs = new HashSet<>();
 
-
     private final EventBus eventBus = new EventBus();
 
 
@@ -100,22 +99,22 @@ public List<BibEntry> getEntries() {
         return Collections.unmodifiableList(entries);
     }
 
+    /**
+     * Returns a set of Strings, that contains all field names that are visible. This means that the fields
+     * are not internal fields. Internal fields are fields, that are starting with "_".
+     *
+     * @return set of fieldnames, that are visible
+     */
     public Set<String> getAllVisibleFields() {
         Set<String> allFields = new TreeSet<>();
         for (BibEntry e : getEntries()) {
             allFields.addAll(e.getFieldNames());
         }
-        Set<String> toberemoved = new TreeSet<>();
-        for (String field : allFields) {
-            if (InternalBibtexFields.isInternalField(field)) {
-                toberemoved.add(field);
-            }
-        }
+        return allFields.stream().filter(field -> !isInternalField(field)).collect(Collectors.toSet());
+    }
 
-        for (String field : toberemoved) {
-            allFields.remove(field);
-        }
-        return allFields;
+    public static boolean isInternalField(String field) {
+        return field.startsWith("__");
     }
 
     /**
@@ -130,6 +129,13 @@ public synchronized Optional<BibEntry> getEntryByKey(String key) {
         return Optional.empty();
     }
 
+    /**
+     * Collects entries having the specified BibTeX key and returns these entries as list.
+     * The order of the entries is the order they appear in the database.
+     *
+     * @param key
+     * @return list of entries that contains the given key
+     */
     public synchronized List<BibEntry> getEntriesByKey(String key) {
         List<BibEntry> result = new ArrayList<>();
 
@@ -163,8 +169,7 @@ public synchronized boolean insertEntry(BibEntry entry) throws KeyCollisionExcep
      * @param eventSource Source the event is sent from
      * @return false if the insert was done without a duplicate warning
      */
-    public synchronized boolean insertEntry(BibEntry entry, EntryEventSource eventSource)
-            throws KeyCollisionException {
+    public synchronized boolean insertEntry(BibEntry entry, EntryEventSource eventSource) throws KeyCollisionException {
         Objects.requireNonNull(entry);
 
         String id = entry.getId();
@@ -177,7 +182,7 @@ public synchronized boolean insertEntry(BibEntry entry, EntryEventSource eventSo
         entry.registerListener(this);
 
         eventBus.post(new EntryAddedEvent(entry, eventSource));
-        return duplicationChecker.checkForDuplicateKeyAndAdd(null, entry.getCiteKeyOptional().orElse(null));
+        return duplicationChecker.checkForDuplicateKeyAndAdd(null, entry.getCiteKey());
     }
 
     /**
@@ -192,13 +197,14 @@ public synchronized void removeEntry(BibEntry toBeDeleted) {
     /**
      * Removes the given entry.
      * The Entry is removed based on the id {@link BibEntry#id}
+     *
      * @param toBeDeleted Entry to delete
      * @param eventSource Source the event is sent from
      */
     public synchronized void removeEntry(BibEntry toBeDeleted, EntryEventSource eventSource) {
         Objects.requireNonNull(toBeDeleted);
 
-        boolean anyRemoved =  entries.removeIf(entry -> entry.getId().equals(toBeDeleted.getId()));
+        boolean anyRemoved = entries.removeIf(entry -> entry.getId().equals(toBeDeleted.getId()));
         if (anyRemoved) {
             internalIDs.remove(toBeDeleted.getId());
             toBeDeleted.getCiteKeyOptional().ifPresent(duplicationChecker::removeKeyFromSet);
@@ -210,8 +216,14 @@ public int getNumberOfKeyOccurrences(String key) {
         return duplicationChecker.getNumberOfKeyOccurrences(key);
     }
 
+    /**
+     * Sets the given key to the given entry.
+     * If the key is null, the entry field will be cleared.
+     *
+     * @return true, if the entry contains the key, false if not
+     */
     public synchronized boolean setCiteKeyForEntry(BibEntry entry, String key) {
-        String oldKey = entry.getCiteKeyOptional().orElse(null);
+        String oldKey = entry.getCiteKey();
         if (key == null) {
             entry.clearField(BibEntry.KEY_FIELD);
         } else {
@@ -237,8 +249,7 @@ public synchronized Optional<String> getPreamble() {
     /**
      * Inserts a Bibtex String.
      */
-    public synchronized void addString(BibtexString string)
-            throws KeyCollisionException {
+    public synchronized void addString(BibtexString string) throws KeyCollisionException {
         if (hasStringLabel(string.getName())) {
             throw new KeyCollisionException("A string with that label already exists");
         }
@@ -478,8 +489,6 @@ private String resolveContent(String result, Set<String> usedIds) {
         return res;
     }
 
-
-
     /**
      * Returns the text stored in the given field of the given bibtex entry
      * which belongs to the given database.
@@ -511,9 +520,9 @@ public static Optional<String> getResolvedField(String field, BibEntry entry, Bi
             return entry.getCiteKeyOptional();
         }
 
-        // TODO: Changed this to also consider alias fields, which is the expected
+        // Changed this to also consider alias fields, which is the expected
         // behavior for the preview layout and for the check whatever all fields are present.
-        // But there might be unwanted side-effects?!
+        // TODO: But there might be unwanted side-effects?!
         Optional<String> result = entry.getFieldOrAlias(field);
 
         // If this field is not set, and the entry has a crossref, try to look up the
diff --git a/src/main/java/net/sf/jabref/model/database/BibDatabaseMode.java b/src/main/java/net/sf/jabref/model/database/BibDatabaseMode.java
@@ -2,34 +2,66 @@
 
 import java.util.Locale;
 
+/**
+ * An enum which contains the possible {@link BibDatabase} Modes.
+ * Possible are BibTeX and BibLaTeX.
+ */
 public enum BibDatabaseMode {
     BIBTEX,
     BIBLATEX;
 
+    /**
+     * @return the name of the current mode as String
+     */
     public String getFormattedName() {
-        if(this == BIBTEX) {
+        if (this == BIBTEX) {
             return "BibTeX";
         } else {
             return "BibLaTeX";
         }
     }
 
+    /**
+     * Returns the opposite mode of the current mode as {@link BibDatabaseMode}.
+     *
+     * @return BibLaTeX if the current mode is BIBTEX, BibTeX else
+     */
     public BibDatabaseMode getOppositeMode() {
-        if(this == BIBTEX) {
+        if (this == BIBTEX) {
             return BIBLATEX;
         } else {
             return BIBTEX;
         }
     }
 
+    /**
+     * Returns the {@link BibDatabaseMode} from a given boolean.
+     *
+     * @return BIBLATEX if isBibLatex is true, else BIBTEX
+     */
     public static BibDatabaseMode fromPreference(boolean isBibLatex) {
-        return isBibLatex ? BIBLATEX : BIBTEX;
+        if (isBibLatex) {
+            return BIBLATEX;
+        } else {
+            return BIBTEX;
+        }
     }
 
+    /**
+     * Returns the {@link BibDatabaseMode} that equals the given string. The use of capital and small letters
+     * in the string doesn't matter.If neither "bibtex" nor "biblatex" is the given string, then an
+     * {@link IllegalArgumentException} will be thrown.
+     *
+     * @return  BIBTEX, if the string is bibtex<br>
+     *          BIBLATEX, if the string is biblatex<br>
+     */
     public static BibDatabaseMode parse(String data) {
         return BibDatabaseMode.valueOf(data.toUpperCase(Locale.ENGLISH));
     }
 
+    /**
+     * @return The current mode as String in lowercase
+     */
     public String getAsString() {
         return getFormattedName().toLowerCase(Locale.ENGLISH);
     }
diff --git a/src/main/java/net/sf/jabref/model/database/BibDatabaseModeDetection.java b/src/main/java/net/sf/jabref/model/database/BibDatabaseModeDetection.java
@@ -1,6 +1,7 @@
 package net.sf.jabref.model.database;
 
 import java.util.List;
+import java.util.Locale;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -11,9 +12,10 @@
 import net.sf.jabref.model.entry.EntryType;
 
 public class BibDatabaseModeDetection {
-    private static final List<EntryType> bibtex = BibtexEntryTypes.ALL;
-    private static final List<EntryType> biblatex = BibLatexEntryTypes.ALL;
-    private static final List<String> exclusiveBiblatex = filterEntryTypesNames(biblatex, isNotIncludedIn(bibtex));
+
+    private static final List<EntryType> BIBTEX = BibtexEntryTypes.ALL;
+    private static final List<EntryType> BIBLATEX = BibLatexEntryTypes.ALL;
+    private static final List<String> EXCLUSIVE_BIBLATEX = filterEntryTypesNames(BIBLATEX, isNotIncludedIn(BIBTEX));
 
     /**
      * Tries to infer the database type by examining a BibDatabase database.
@@ -32,7 +34,7 @@ public static BibDatabaseMode inferMode(BibDatabase database) {
         final Stream<String> entryTypes = database.getEntries().stream().map(BibEntry::getType);
 
         // type-based check
-        if (entryTypes.anyMatch(type -> exclusiveBiblatex.contains(type.toLowerCase()))) {
+        if (entryTypes.anyMatch(type -> EXCLUSIVE_BIBLATEX.contains(type.toLowerCase(Locale.ENGLISH)))) {
             return BibDatabaseMode.BIBLATEX;
         } else {
             // field-based check
diff --git a/src/main/java/net/sf/jabref/model/database/BibDatabases.java b/src/main/java/net/sf/jabref/model/database/BibDatabases.java
@@ -9,6 +9,13 @@
 
 public class BibDatabases {
 
+    /**
+     * Gets a collection of bibentries and sets an ID for every entry. After that
+     * all entries will be inserted into a new BibDatabase.
+     *
+     * @param bibentries a collection that contains {@link BibEntry}
+     * @return BibDatabase that contains the entries
+     */
     public static BibDatabase createDatabase(Collection<BibEntry> bibentries) {
         BibDatabase database = new BibDatabase();
 
diff --git a/src/main/java/net/sf/jabref/model/database/DuplicationChecker.java b/src/main/java/net/sf/jabref/model/database/DuplicationChecker.java
@@ -7,7 +7,7 @@
 import org.apache.commons.logging.LogFactory;
 
 /**
- * Determines which bibtex cite keys are duplicates in a single {@link BibDatabase}
+ * Determines which bibtex cite keys are duplicates in a single {@link BibDatabase}.
  */
 class DuplicationChecker {
 
@@ -16,33 +16,29 @@ class DuplicationChecker {
     // use a map instead of a set since i need to know how many of each key is in there
     private final Map<String, Integer> allKeys = new HashMap<>();
 
-    //##########################################
-    //  usage:
-    //  isDuplicate=checkForDuplicateKeyAndAdd( null, b.getKey() , issueDuplicateWarning);
-    //############################################
-    // if the newkey already exists and is not the same as oldkey it will give a warning
-    // else it will add the newkey to the to set and remove the oldkey
+    /**
+     * Usage:
+     * <br>
+     * isDuplicate=checkForDuplicateKeyAndAdd( null, b.getKey() , issueDuplicateWarning);
+     *
+     * If the newkey already exists and is not the same as oldkey it will give a warning
+     * else it will add the newkey to the to set and remove the oldkey
+     *
+     * @return true, if there is a duplicate key, else false
+     */
     public boolean checkForDuplicateKeyAndAdd(String oldKey, String newKey) {
-        // LOGGER.debug(" checkForDuplicateKeyAndAdd [oldKey = " + oldKey + "] [newKey = " + newKey + "]");
 
-        boolean duplicate;
-        if (oldKey == null) {// this is a new entry so don't bother removing oldKey
+        boolean duplicate = false;
+        if (oldKey == null) { // No old key
             duplicate = addKeyToSet(newKey);
         } else {
             if (oldKey.equals(newKey)) {// were OK because the user did not change keys
                 duplicate = false;
-            } else {// user changed the key
-
-                // removed the oldkey
-                // But what if more than two have the same key?
-                // this means that user can add another key and would not get a warning!
-                // consider this: i add a key xxx, then i add another key xxx . I get a warning. I delete the key xxx. JBM
-                // removes this key from the allKey. then I add another key xxx. I don't get a warning!
-                // i need a way to count the number of keys of each type
-                // hashmap=>int (increment each time)
-
-                removeKeyFromSet(oldKey);
-                duplicate = addKeyToSet(newKey);
+            } else {
+                removeKeyFromSet(oldKey); // Get rid of old key
+                if (newKey != null) { // Add new key if any
+                    duplicate = addKeyToSet(newKey);
+                }
             }
         }
         if (duplicate) {
@@ -64,38 +60,52 @@ public int getNumberOfKeyOccurrences(String key) {
 
     }
 
-    //========================================================
-    // keep track of all the keys to warn if there are duplicates
-    //========================================================
-    public boolean addKeyToSet(String key) {
+    /**
+     * Helper function for counting the number of the key usages.
+     * Adds the given key to the internal keyset together with the count of it.
+     * The counter is increased if the key already exists, otherwise set to 1.
+     * <br>
+     * Special case: If a null or empty key is passed, it is not counted and thus not added.
+     *
+     * Reasoning:
+     * Consider this: I add a key xxx, then I add another key xxx. I get a warning. I delete the key xxx.
+     * Consider JabRef simply removing this key from a set of allKeys.
+     * Then I add another key xxx. I don't get a warning!
+     * Thus, I need a way to count the number of keys of each type.
+     * Solution: hashmap=>int (increment each time at add and decrement each time at remove)
+     */
+    protected boolean addKeyToSet(String key) {
         if ((key == null) || key.isEmpty()) {
             return false;//don't put empty key
         }
         boolean exists = false;
         if (allKeys.containsKey(key)) {
             // warning
             exists = true;
-            allKeys.put(key, allKeys.get(key) + 1);// incrementInteger( allKeys.get(key)));
+            allKeys.put(key, allKeys.get(key) + 1);
         } else {
             allKeys.put(key, 1);
         }
         return exists;
     }
 
-    //========================================================
-    // reduce the number of keys by 1. if this number goes to zero then remove from the set
-    // note: there is a good reason why we should not use a hashset but use hashmap instead
-    //========================================================
-    public void removeKeyFromSet(String key) {
+    /**
+     * Helper function for counting the number of the key usages.
+     * Removes the given key from the internal keyset together with the count of it, if the key is set to 1.
+     * If it is not set to 1, the counter will be decreased.
+     * <br>
+     * Special case: If a null or empty key is passed, it is not counted and thus not removed.
+     */
+    protected void removeKeyFromSet(String key) {
         if ((key == null) || key.isEmpty()) {
             return;
         }
         if (allKeys.containsKey(key)) {
-            Integer tI = allKeys.get(key); // if(allKeys.get(key) instanceof Integer)
+            Integer tI = allKeys.get(key);
             if (tI == 1) {
                 allKeys.remove(key);
             } else {
-                allKeys.put(key, tI - 1);//decrementInteger( tI ));
+                allKeys.put(key, tI - 1);
             }
         }
     }
diff --git a/src/main/java/net/sf/jabref/model/database/EntrySorter.java b/src/main/java/net/sf/jabref/model/database/EntrySorter.java
