Change format for study definition to yamlm and adapt tests

Make long queries safe

Signed-off-by: Dominik Voigt <[email protected]>

Author: DominikVoigt

build.gradle

@@ -141,6 +141,9 @@ dependencies {
 
     implementation group: 'org.eclipse.jgit', name: 'org.eclipse.jgit', version: '5.9.0.202009080501-r'
 
+    implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-yaml', version: '2.12.0-rc2'
+    implementation group: 'com.fasterxml.jackson.datatype', name: 'jackson-datatype-jsr310', version: '2.12.0-rc2'
+
     implementation group: 'org.mariadb.jdbc', name: 'mariadb-java-client', version: '2.7.0'
 
     implementation 'org.postgresql:postgresql:42.2.18'

docs/adr/0018-use-Jackson-to-parse-study-yml.md

@@ -0,0 +1,51 @@
+# Use Jackson to parse study.yml
+
+## Context and Problem Statement
+
+The study definition file is formulated as a YAML document.
+To accessed the definition within JabRef this document has to be parsed.
+What parser should be used to parse YAML files?
+
+## Considered Options
+
+* Jackson
+* SnakeYAML Engine
+* yamlbeans
+* eo-yaml
+* Own parser
+
+## Decision Outcome
+
+Chosen option: Jackson, because as it is a dedicated library for parsing YAML. yamlbeans also seem to be viable. They all offer similar functionality
+
+## Pros and Cons of the Options
+
+### Jackson
+
+* Good, because established YAML parser library
+* Good, because supports YAML 1.2
+* Good, because it can parse LocalDate
+
+### SnakeYAML Engine
+
+* Good, because established YAML parser library
+* Good, because supports YAML 1.2
+* Bad, because cannot parse YAML into Java DTOs
+
+### yamlbeans
+
+* Good, because established YAML parser library
+* Good, because [nice getting started page](https://github.com/EsotericSoftware/yamlbeans)
+* Bad, because objects need to be annotated in the yaml file to be parsed into Java objects
+
+### eo-yaml
+
+* Good, because established YAML parser library
+* Good, because supports YAML 1.2
+* Bad, because cannot parse YAML into Java DTOs?
+
+### Own parser
+
+* Good, because easily customizable
+* Bad, because high effort
+* Bad, because has to be tested extensively

src/main/java/module-info.java

@@ -92,4 +92,7 @@
     requires lucene.queryparser;
     requires lucene.core;
     requires org.eclipse.jgit;
+    requires com.fasterxml.jackson.databind;
+    requires com.fasterxml.jackson.dataformat.yaml;
+    requires com.fasterxml.jackson.datatype.jsr310;
 }

src/main/java/org/jabref/logic/crawler/Crawler.java

@@ -10,7 +10,6 @@
 import org.jabref.logic.importer.ParseException;
 import org.jabref.model.entry.BibEntryTypesManager;
 import org.jabref.model.study.QueryResult;
-import org.jabref.model.study.Study;
 import org.jabref.model.util.FileUpdateMonitor;
 
 import org.eclipse.jgit.api.errors.GitAPIException;
@@ -34,9 +33,8 @@ public class Crawler {
     public Crawler(Path studyDefinitionFile, FileUpdateMonitor fileUpdateMonitor, ImportFormatPreferences importFormatPreferences, SavePreferences savePreferences, BibEntryTypesManager bibEntryTypesManager) throws IllegalArgumentException, IOException, ParseException, GitAPIException {
         Path studyRepositoryRoot = studyDefinitionFile.getParent();
         studyRepository = new StudyRepository(studyRepositoryRoot, new GitHandler(studyRepositoryRoot), importFormatPreferences, fileUpdateMonitor, savePreferences, bibEntryTypesManager);
-        Study study = studyRepository.getStudy();
-        LibraryEntryToFetcherConverter libraryEntryToFetcherConverter = new LibraryEntryToFetcherConverter(study.getActiveLibraryEntries(), importFormatPreferences);
-        this.studyFetcher = new StudyFetcher(libraryEntryToFetcherConverter.getActiveFetchers(), study.getSearchQueryStrings());
+        LibraryEntryToFetcherConverter libraryEntryToFetcherConverter = new LibraryEntryToFetcherConverter(studyRepository.getActiveLibraryEntries(), importFormatPreferences);
+        this.studyFetcher = new StudyFetcher(libraryEntryToFetcherConverter.getActiveFetchers(), studyRepository.getSearchQueryStrings());
     }
 
     /**

src/main/java/org/jabref/logic/crawler/LibraryEntryToFetcherConverter.java

@@ -8,19 +8,16 @@
 import org.jabref.logic.importer.ImportFormatPreferences;
 import org.jabref.logic.importer.SearchBasedFetcher;
 import org.jabref.logic.importer.WebFetchers;
-import org.jabref.model.entry.BibEntry;
-import org.jabref.model.entry.field.UnknownField;
-
-import static org.jabref.model.entry.types.SystematicLiteratureReviewStudyEntryType.LIBRARY_ENTRY;
+import org.jabref.model.study.LibraryEntry;
 
 /**
  * Converts library entries from the given study into their corresponding fetchers.
  */
 class LibraryEntryToFetcherConverter {
-    private final List<BibEntry> libraryEntries;
+    private final List<LibraryEntry> libraryEntries;
     private final ImportFormatPreferences importFormatPreferences;
 
-    public LibraryEntryToFetcherConverter(List<BibEntry> libraryEntries, ImportFormatPreferences importFormatPreferences) {
+    public LibraryEntryToFetcherConverter(List<LibraryEntry> libraryEntries, ImportFormatPreferences importFormatPreferences) {
         this.libraryEntries = libraryEntries;
         this.importFormatPreferences = importFormatPreferences;
     }
@@ -42,9 +39,8 @@ public List<SearchBasedFetcher> getActiveFetchers() {
      * @param libraryEntries List of entries
      * @return List of fetcher instances
      */
-    private List<SearchBasedFetcher> getFetchersFromLibraryEntries(List<BibEntry> libraryEntries) {
+    private List<SearchBasedFetcher> getFetchersFromLibraryEntries(List<LibraryEntry> libraryEntries) {
         return libraryEntries.parallelStream()
-                             .filter(bibEntry -> bibEntry.getType().getName().equals(LIBRARY_ENTRY.getName()))
                              .map(this::createFetcherFromLibraryEntry)
                              .filter(Objects::nonNull)
                              .collect(Collectors.toList());
@@ -56,9 +52,9 @@ private List<SearchBasedFetcher> getFetchersFromLibraryEntries(List<BibEntry> li
      * @param libraryEntry the entry that will be converted
      * @return An instance of the fetcher defined by the library entry.
      */
-    private SearchBasedFetcher createFetcherFromLibraryEntry(BibEntry libraryEntry) {
+    private SearchBasedFetcher createFetcherFromLibraryEntry(LibraryEntry libraryEntry) {
         Set<SearchBasedFetcher> searchBasedFetchers = WebFetchers.getSearchBasedFetchers(importFormatPreferences);
-        String libraryNameFromFetcher = libraryEntry.getField(new UnknownField("name")).orElse("");
+        String libraryNameFromFetcher = libraryEntry.getName();
         return searchBasedFetchers.stream()
                                   .filter(searchBasedFetcher -> searchBasedFetcher.getName().toLowerCase().equals(libraryNameFromFetcher.toLowerCase()))
                                   .findAny()

src/main/java/org/jabref/logic/crawler/StudyRepository.java

@@ -2,14 +2,11 @@
 
 import java.io.FileWriter;
 import java.io.IOException;
-import java.io.InputStream;
 import java.io.Writer;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.time.LocalDate;
-import java.util.ArrayList;
 import java.util.List;
-import java.util.Optional;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
@@ -22,14 +19,12 @@
 import org.jabref.logic.importer.OpenDatabase;
 import org.jabref.logic.importer.ParseException;
 import org.jabref.logic.importer.SearchBasedFetcher;
-import org.jabref.logic.importer.fileformat.BibtexParser;
 import org.jabref.model.database.BibDatabase;
 import org.jabref.model.database.BibDatabaseContext;
-import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.BibEntryTypesManager;
-import org.jabref.model.entry.field.UnknownField;
-import org.jabref.model.entry.types.SystematicLiteratureReviewStudyEntryType;
 import org.jabref.model.study.FetchResult;
+import org.jabref.model.study.LibraryEntry;
+import org.jabref.model.study.QueryEntry;
 import org.jabref.model.study.QueryResult;
 import org.jabref.model.study.Study;
 import org.jabref.model.util.FileUpdateMonitor;
@@ -48,13 +43,13 @@
  */
 class StudyRepository {
     // Tests work with study.bib
-    private static final String STUDY_DEFINITION_FILE_NAME = "study.bib";
+    private static final String STUDY_DEFINITION_FILE_NAME = "study.yml";
     private static final Logger LOGGER = LoggerFactory.getLogger(StudyRepository.class);
     private static final Pattern MATCHCOLON = Pattern.compile(":");
     private static final Pattern MATCHILLEGALCHARACTERS = Pattern.compile("[^A-Za-z0-9_.\\s=-]");
 
     private final Path repositoryPath;
-    private final Path studyDefinitionBib;
+    private final Path studyDefinitionFile;
     private final GitHandler gitHandler;
     private final Study study;
     private final ImportFormatPreferences importFormatPreferences;
@@ -81,13 +76,13 @@ public StudyRepository(Path pathToRepository, GitHandler gitHandler, ImportForma
         }
         this.importFormatPreferences = importFormatPreferences;
         this.fileUpdateMonitor = fileUpdateMonitor;
-        this.studyDefinitionBib = Path.of(repositoryPath.toString(), STUDY_DEFINITION_FILE_NAME);
+        this.studyDefinitionFile = Path.of(repositoryPath.toString(), STUDY_DEFINITION_FILE_NAME);
         this.savePreferences = savePreferences;
         this.bibEntryTypesManager = bibEntryTypesManager;
 
         if (Files.notExists(repositoryPath)) {
             throw new IOException("The given repository does not exists.");
-        } else if (Files.notExists(studyDefinitionBib)) {
+        } else if (Files.notExists(studyDefinitionFile)) {
             throw new IOException("The study definition file does not exist in the given repository.");
         }
         study = parseStudyFile();
@@ -116,30 +111,39 @@ public BibDatabaseContext getStudyResultEntries() throws IOException {
     }
 
     /**
-     * The study definition file contains all the definitions of a study. This method extracts the BibEntries from the study BiB file.
+     * The study definition file contains all the definitions of a study. This method extracts this study from the yaml study definition file
      *
      * @return Returns the BibEntries parsed from the study definition file.
      * @throws IOException    Problem opening the input stream.
      * @throws ParseException Problem parsing the study definition file.
      */
-    private Study parseStudyFile() throws IOException, ParseException {
-        BibtexParser parser = new BibtexParser(importFormatPreferences, fileUpdateMonitor);
-        List<BibEntry> parsedEntries = new ArrayList<>();
-        try (InputStream inputStream = Files.newInputStream(studyDefinitionBib)) {
-            parsedEntries.addAll(parser.parseEntries(inputStream));
-        }
+    private Study parseStudyFile() throws IOException {
+        return new StudyYAMLParser().parseStudyYAMLFile(studyDefinitionFile);
+    }
+
+    /**
+     * Returns all query strings of the study definition
+     *
+     * @return List of all queries as Strings.
+     */
+    public List<String> getSearchQueryStrings() {
+        return study.getQueries()
+                    .parallelStream()
+                    .map(QueryEntry::getQuery)
+                    .collect(Collectors.toList());
+    }
 
-        BibEntry studyEntry = parsedEntries.parallelStream()
-                                           .filter(bibEntry -> bibEntry.getType().equals(SystematicLiteratureReviewStudyEntryType.STUDY_ENTRY)).findAny()
-                                           .orElseThrow(() -> new ParseException("Study definition file does not contain a study entry"));
-        List<BibEntry> queryEntries = parsedEntries.parallelStream()
-                                                   .filter(bibEntry -> bibEntry.getType().equals(SystematicLiteratureReviewStudyEntryType.SEARCH_QUERY_ENTRY))
-                                                   .collect(Collectors.toList());
-        List<BibEntry> libraryEntries = parsedEntries.parallelStream()
-                                                     .filter(bibEntry -> bibEntry.getType().equals(SystematicLiteratureReviewStudyEntryType.LIBRARY_ENTRY))
-                                                     .collect(Collectors.toList());
-
-        return new Study(studyEntry, queryEntries, libraryEntries);
+    /**
+     * Extracts all active fetchers from the library entries.
+     *
+     * @return List of BibEntries of type Library
+     * @throws IllegalArgumentException If a transformation from Library entry to LibraryDefinition fails
+     */
+    public List<LibraryEntry> getActiveLibraryEntries() throws IllegalArgumentException {
+        return study.getLibraries()
+                    .parallelStream()
+                    .filter(LibraryEntry::isEnabled)
+                    .collect(Collectors.toList());
     }
 
     public Study getStudy() {
@@ -163,16 +167,16 @@ public void persist(List<QueryResult> crawlResults) throws IOException, GitAPIEx
     }
 
     private void persistStudy() throws IOException {
-        writeResultToFile(studyDefinitionBib, new BibDatabase(study.getAllEntries()));
+        new StudyYAMLParser().writeStudyYAMLFile(study, studyDefinitionFile);
     }
 
     /**
      * Create for each query a folder, and for each fetcher a bib file in the query folder to store its results.
      */
     private void setUpRepositoryStructure() throws IOException {
         // Cannot use stream here since IOException has to be thrown
-        LibraryEntryToFetcherConverter converter = new LibraryEntryToFetcherConverter(study.getActiveLibraryEntries(), importFormatPreferences);
-        for (String query : study.getSearchQueryStrings()) {
+        LibraryEntryToFetcherConverter converter = new LibraryEntryToFetcherConverter(this.getActiveLibraryEntries(), importFormatPreferences);
+        for (String query : this.getSearchQueryStrings()) {
             createQueryResultFolder(query);
             converter.getActiveFetchers()
                      .forEach(searchBasedFetcher -> createFetcherResultFile(query, searchBasedFetcher));
@@ -229,14 +233,14 @@ private void createBibFile(Path file) {
      * Structure: ID-trimmed query
      *
      * Examples:
-     * Input: '(title: test-title AND abstract: Test)' as a query entry with id 1
-     * Output: '1 - title= test-title AND abstract= Test'
+     * Input: '(title: test-title AND abstract: Test)' as a query entry with id 12345678
+     * Output: '12345678 - title= test-title AND abstract= Test'
      *
-     * Input: 'abstract: Test*' as a query entry with id 1
-     * Output: '1 - abstract= Test'
+     * Input: 'abstract: Test*' as a query entry with id 87654321
+     * Output: '87654321 - abstract= Test'
      *
-     * Input: '"test driven"' as a query entry with id 1
-     * Output: '1 - test driven'
+     * Input: '"test driven"' as a query entry with id 12348765
+     * Output: '12348765 - test driven'
      *
      * @param query that is trimmed and combined with its query id
      * @return a unique folder name for any query.
@@ -245,31 +249,20 @@ private String trimNameAndAddID(String query) {
         // Replace all field: with field= for folder name
         String trimmedNamed = MATCHCOLON.matcher(query).replaceAll("=");
         trimmedNamed = MATCHILLEGALCHARACTERS.matcher(trimmedNamed).replaceAll("");
-        if (query.length() > 240) {
-            trimmedNamed = query.substring(0, 240);
+        String id = computeIDForQuery(query);
+        // Whole path has to be shorter than 260
+        int remainingPathLength = 220 - studyDefinitionFile.toString().length() - id.length();
+        if (query.length() > remainingPathLength) {
+            trimmedNamed = query.substring(0, remainingPathLength);
         }
-        String id = findQueryIDByQueryString(query);
         return id + " - " + trimmedNamed;
     }
 
     /**
-     * Helper to find the query id for folder name creation.
-     * Returns the id of the first SearchQuery BibEntry with a query field that matches the given query.
-     *
-     * @param query The query whose ID is searched
-     * @return ID of the query defined in the study definition.
+     * Helper to compute the query id for folder name creation.
      */
-    private String findQueryIDByQueryString(String query) {
-        String queryField = "query";
-        return study.getSearchQueryEntries()
-                    .parallelStream()
-                    .filter(bibEntry -> bibEntry.getField(new UnknownField(queryField)).orElse("").equals(query))
-                    .map(BibEntry::getCitationKey)
-                    .filter(Optional::isPresent)
-                    .map(Optional::get)
-                    .findFirst()
-                    .orElseThrow()
-                    .replaceFirst(queryField, "");
+    private String computeIDForQuery(String query) {
+        return String.valueOf(query.hashCode());
     }
 
     /**

src/main/java/org/jabref/logic/crawler/StudyYAMLParser.java

@@ -0,0 +1,38 @@
+package org.jabref.logic.crawler;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Path;
+
+import org.jabref.model.study.Study;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.SerializationFeature;
+import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
+import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator;
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+
+public class StudyYAMLParser {
+
+    /**
+     * Parses the given yaml study definition file into a study instance
+     */
+    public Study parseStudyYAMLFile(Path studyYAMLFile) throws IOException {
+        ObjectMapper yamlMapper = new ObjectMapper(new YAMLFactory());
+        yamlMapper.registerModule(new JavaTimeModule());
+        try (InputStream fileInputStream = new FileInputStream(studyYAMLFile.toFile())) {
+            return yamlMapper.readValue(fileInputStream, Study.class);
+        }
+    }
+
+    /**
+     * Writes the given study instance into a yaml file to the given path
+     */
+    public void writeStudyYAMLFile(Study study, Path studyYAMLFile) throws IOException {
+        ObjectMapper yamlMapper = new ObjectMapper(new YAMLFactory().disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER).enable(YAMLGenerator.Feature.MINIMIZE_QUOTES));
+        yamlMapper.registerModule(new JavaTimeModule());
+        yamlMapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
+        yamlMapper.writeValue(studyYAMLFile.toFile(), study);
+    }
+}