Fix importing of custom entry types with duplicate fields

CHANGELOG.md

@@ -15,6 +15,8 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 
 ### Fixed
 
+- We fixed an issue where entry type with duplicate fields prevented opening existing libraries with custom entry types [#11127](https://github.com/JabRef/jabref/issues/11127)
+
 ### Removed
 
 

src/main/java/org/jabref/logic/importer/util/MetaDataParser.java

@@ -66,6 +66,10 @@ public static Optional<BibEntryType> parseCustomEntryType(String comment) {
                 // Important fields are optional fields, but displayed first. Thus, they do not need to be separated by "/".
                 // See org.jabref.model.entry.field.FieldPriority for details on important optional fields.
                 .withImportantFields(FieldFactory.parseFieldList(optFields));
+        if (entryTypeBuilder.hasWarnings()) {
+            LOGGER.warn("Following custom entry type definition has duplicate fields: {}", comment);
+            return Optional.empty();
+        }
         return Optional.of(entryTypeBuilder.build());
     }
 

src/main/java/org/jabref/model/entry/BibEntryTypeBuilder.java

@@ -19,15 +19,16 @@
 import org.jabref.model.entry.types.StandardEntryType;
 
 import com.google.common.collect.Streams;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class BibEntryTypeBuilder {
-
-    private EntryType type = StandardEntryType.Misc;
-
-    private SequencedSet<OrFields> requiredFields = new LinkedHashSet<>();
+    private static final Logger LOGGER = LoggerFactory.getLogger(BibEntryTypeBuilder.class);
+    private final SequencedSet<OrFields> requiredFields = new LinkedHashSet<>();
+    private final Set<Field> seenFields = new HashSet<>();
     private SequencedSet<BibField> optionalFields = new LinkedHashSet<>();
-
-    private Set<Field> seenFields = new HashSet<>();
+    private EntryType type = StandardEntryType.Misc;
+    private boolean hasWarnings = false;
 
     public BibEntryTypeBuilder withType(EntryType type) {
         this.type = type;
@@ -37,7 +38,8 @@ public BibEntryTypeBuilder withType(EntryType type) {
     public BibEntryTypeBuilder withImportantFields(SequencedSet<Field> newFields) {
         List<Field> containedFields = containedInSeenFields(newFields);
         if (!containedFields.isEmpty()) {
-            throw new IllegalArgumentException("Fields " + containedFields + " already added");
+            LOGGER.warn("Fields {} already added to type {}.", containedFields, type.getDisplayName());
+            hasWarnings = true;
         }
         this.seenFields.addAll(newFields);
         this.optionalFields = Streams.concat(optionalFields.stream(), newFields.stream().map(field -> new BibField(field, FieldPriority.IMPORTANT)))
@@ -52,7 +54,8 @@ public BibEntryTypeBuilder withImportantFields(Field... newFields) {
     public BibEntryTypeBuilder withDetailFields(SequencedCollection<Field> newFields) {
         List<Field> containedFields = containedInSeenFields(newFields);
         if (!containedFields.isEmpty()) {
-            throw new IllegalArgumentException("Fields " + containedFields + " already added");
+            LOGGER.warn("Fields {} already added to type {}.", containedFields, type.getDisplayName());
+            hasWarnings = true;
         }
         this.seenFields.addAll(newFields);
         this.optionalFields = Streams.concat(optionalFields.stream(), newFields.stream().map(field -> new BibField(field, FieldPriority.DETAIL)))
@@ -72,15 +75,16 @@ public BibEntryTypeBuilder addRequiredFields(SequencedSet<OrFields> requiredFiel
         Set<Field> fieldsToAdd = requiredFields.stream().map(OrFields::getFields).flatMap(Set::stream).collect(Collectors.toSet());
         List<Field> containedFields = containedInSeenFields(fieldsToAdd);
         if (!containedFields.isEmpty()) {
-            throw new IllegalArgumentException("Fields " + containedFields + " already added");
+            LOGGER.warn("Fields {} already added to type {}.", containedFields, type.getDisplayName());
+            hasWarnings = true;
         }
         this.seenFields.addAll(fieldsToAdd);
         this.requiredFields.addAll(requiredFields);
         return this;
     }
 
     public BibEntryTypeBuilder addRequiredFields(OrFields... requiredFields) {
-        return addRequiredFields(Arrays.asList(requiredFields).stream().collect(Collectors.toCollection(LinkedHashSet::new)));
+        return addRequiredFields(new LinkedHashSet<>(List.of(requiredFields)));
     }
 
     public BibEntryTypeBuilder addRequiredFields(Field... requiredFields) {
@@ -109,6 +113,10 @@ public BibEntryType build() {
         return new BibEntryType(type, allFields, requiredFields);
     }
 
+    public boolean hasWarnings() {
+        return hasWarnings;
+    }
+
     private List<Field> containedInSeenFields(Collection<Field> fields) {
         return fields.stream().filter(seenFields::contains).toList();
     }

src/test/java/org/jabref/model/entry/BibEntryTypeBuilderTest.java

@@ -5,6 +5,7 @@
 
 import org.jabref.model.entry.field.StandardField;
 
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -13,6 +14,7 @@
 class BibEntryTypeBuilderTest {
 
     @Test
+    @Disabled("There is just a log message written, but no exception thrown")
     void fieldAlreadySeenSameCategory() {
         assertThrows(IllegalArgumentException.class, () ->
         new BibEntryTypeBuilder()
@@ -31,6 +33,7 @@ void detailOptionalWorks() {
     }
 
     @Test
+    @Disabled("There is just a log message written, but no exception thrown")
     void fieldAlreadySeenDifferentCategories() {
         assertThrows(IllegalArgumentException.class, () ->
         new BibEntryTypeBuilder()