Merge pull request #15 from OP-TED/feature/optimise_codelists_loading

Optimise codelist loading

Author: rousso

pom.xml

@@ -58,8 +58,7 @@
     <version.commons-collections>3.2.2</version.commons-collections>
     <version.commons-io>2.11.0</version.commons-io>
     <version.commons-lang3>3.12.0</version.commons-lang3>
-    <version.jackson>2.13.4</version.jackson>
-    <version.jackson-databind>2.13.4.2</version.jackson-databind>
+    <version.jackson>2.15.1</version.jackson>
     <version.jool>0.9.15</version.jool>
     <version.jsr305>3.0.2</version.jsr305>
     <version.junit>5.7.2</version.junit>
@@ -103,7 +102,7 @@
       <dependency>
         <groupId>com.fasterxml.jackson.core</groupId>
         <artifactId>jackson-databind</artifactId>
-        <version>${version.jackson-databind}</version>
+        <version>${version.jackson}</version>
       </dependency>
 
       <!-- Apache Commons -->

src/main/java/eu/europa/ted/eforms/sdk/domain/codelist/CodelistForIndex.java

@@ -0,0 +1,37 @@
+package eu.europa.ted.eforms.sdk.domain.codelist;
+
+import java.io.Serializable;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+public class CodelistForIndex implements Serializable {
+  private static final long serialVersionUID = 4221672498810948002L;
+
+  private String id;
+  private String parentId;
+  private String filename;
+  private String description;
+
+  @JsonProperty("_label")
+  private String labelId;
+
+  public String getId() {
+    return id;
+  }
+
+  public String getParentId() {
+    return parentId;
+  }
+
+  public String getFilename() {
+    return filename;
+  }
+
+  public String getDescription() {
+    return description;
+  }
+
+  @JsonProperty("_label")
+  public String getLabel() {
+    return labelId;
+  }
+}

src/main/java/eu/europa/ted/eforms/sdk/domain/codelist/CodelistsIndex.java

@@ -0,0 +1,14 @@
+package eu.europa.ted.eforms.sdk.domain.codelist;
+
+import java.io.Serializable;
+import java.util.List;
+
+public class CodelistsIndex implements Serializable {
+  private static final long serialVersionUID = -6549217565224309697L;
+
+  private List<CodelistForIndex> codelists;
+
+  public List<CodelistForIndex> getCodelists() {
+    return codelists;
+  }
+}

src/main/java/eu/europa/ted/eforms/sdk/repository/SdkCodelistRepository.java

@@ -12,8 +12,6 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
-import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.Validate;
 import org.apache.commons.lang3.builder.EqualsBuilder;
@@ -22,12 +20,16 @@
 import org.jooq.lambda.Unchecked;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.helger.genericode.v10.CodeListDocument;
 import com.helger.genericode.v10.Identification;
 import com.helger.genericode.v10.LongName;
 import com.helger.genericode.v10.Row;
 import com.helger.genericode.v10.SimpleCodeList;
 import com.helger.genericode.v10.Value;
+import eu.europa.ted.eforms.sdk.domain.codelist.CodelistForIndex;
+import eu.europa.ted.eforms.sdk.domain.codelist.CodelistsIndex;
 import eu.europa.ted.eforms.sdk.entity.SdkCodelist;
 import eu.europa.ted.eforms.sdk.entity.SdkEntityFactory;
 import eu.europa.ted.util.GenericodeTools;
@@ -40,7 +42,7 @@ public class SdkCodelistRepository extends HashMap<String, SdkCodelist> {
   private transient Path codelistsDir;
   private String sdkVersion;
 
-  private final Map<String, CodeListDocument> codelistContentsByCodelistId;
+  private final Map<String, Pair<Path, CodeListDocument>> codelistInfoByCodelistIds;
 
   @SuppressWarnings("unused")
   private SdkCodelistRepository() {
@@ -55,7 +57,7 @@ public SdkCodelistRepository(final String sdkVersion, final Path codelistsDir) {
         "Codelists directory [%s] is not found or not a directory", codelistsDir);
 
     try {
-      this.codelistContentsByCodelistId = getCodelistContentsByCodelistIds(codelistsDir);
+      this.codelistInfoByCodelistIds = getCodelistInfoByCodelistIds(codelistsDir);
     } catch (IOException e) {
       throw new RuntimeException(
           MessageFormat.format("Failed to load codelists from [{0}]", codelistsDir), e);
@@ -86,13 +88,15 @@ public SdkCodelist getOrDefault(final Object codelistId, final SdkCodelist defau
 
   private Optional<SdkCodelist> loadSdkCodelist(final String codeListId)
       throws InstantiationException {
-    logger.debug("Loading SDK codelist with ID [{}] for SDK version [{}] from path [{}]",
-        codeListId, sdkVersion, codelistsDir);
+    logger.debug("Loading SDK codelist with ID [{}] for SDK version [{}]", codeListId, sdkVersion);
 
     // Find the SDK codelist .gc file that corresponds to the passed reference.
     // Stream the data from that file.
     final Optional<CodeListDocument> codelist =
-        Optional.ofNullable(codelistContentsByCodelistId.get(codeListId));
+        Optional.ofNullable(codelistInfoByCodelistIds.get(codeListId))
+            .map(Unchecked.function((Pair<Path, CodeListDocument> codelistInfo) -> Optional
+                .ofNullable(codelistInfo.getValue())
+                .orElse(getCodelistContents(codelistInfo.getKey()))));
 
     if (codelist.isEmpty()) {
       return Optional.empty();
@@ -127,8 +131,8 @@ private Optional<SdkCodelist> loadSdkCodelist(final String codeListId)
     final Optional<SdkCodelist> result = Optional.of(SdkEntityFactory.getSdkCodelist(sdkVersion,
         codeListId, codelistVersion.orElse(null), codes, parentId));
 
-    logger.debug("Finished loading SDK codelist with ID [{}] for SDK version [{}] from path [{}]",
-        codeListId, sdkVersion, codelistsDir);
+    logger.debug("Finished loading SDK codelist with ID [{}] for SDK version [{}]", codeListId,
+        sdkVersion);
 
     return result;
   }
@@ -158,69 +162,67 @@ public static Optional<String> extractLongNameWithIdentifier(
   }
 
   /**
-   * Loads all of the codelists by reading codelist files under a given a folder.
+   * Loads the paths of all of the codelists by looking for and reading the codelists index.
+   * <p>
+   * The result is a map which associates information (file path and a placeholder for contents) for
+   * each codelist with its ID.
    *
-   * @param codelistsDir The folder containing all codelist files
-   * @return A map of codelist IDs to codelist contents
+   * @param codelistsDir The folder containing the codelists index and files
+   * @return A map of codelist IDs to pairs of codelist file paths and contents
    * @throws IOException If there are failures when discovering and parsing the files
    */
-  private Map<String, CodeListDocument> getCodelistContentsByCodelistIds(final Path codelistsDir)
-      throws IOException {
+  private Map<String, Pair<Path, CodeListDocument>> getCodelistInfoByCodelistIds(
+      final Path codelistsDir) throws IOException {
     Validate.notNull(codelistsDir, "Undefined codelists directory");
     Validate.isTrue(Files.isDirectory(codelistsDir),
         MessageFormat.format("Not a directory: {0}", codelistsDir));
 
-    logger.debug("Getting codelist file paths from directory [{}]", codelistsDir);
+    final Path indexFile = Path.of(codelistsDir.toString(), "codelists.json");
 
-    final int depth = 1; // Flat folder, not recursive for now.
+    logger.debug("Loading codelists index from [{}]", indexFile);
+    CodelistsIndex codelistsIndex =
+        createObjectMapper().readValue(indexFile.toFile(), CodelistsIndex.class);
+
+    final Map<String, Pair<Path, CodeListDocument>> result = new HashMap<>();
+    codelistsIndex.getCodelists().stream().forEach((CodelistForIndex codelist) -> {
+      final String codelistId = codelist.getId();
+      final Path codelistPath = Path.of(codelistsDir.toString(), codelist.getFilename());
+
+      logger.trace("Adding path [{}] for codelist [{}]", codelistPath, codelistId);
+      if (!Files.isRegularFile(codelistPath)) {
+        logger.warn("Codelist file [{}] not found. Codelist [{}] will be skipped", codelistPath,
+            codelistId);
+      } else {
+        // We're only interested in populating the codelist filepaths for now.
+        // The contents of each document will be populated for each codelist later, on demand.
+        result.put(codelistId, Pair.of(codelistPath, null));
+      }
+    });
+
+    return result;
 
-    try (Stream<Path> walk = Files.walk(codelistsDir, depth)) {
-      return walk
-          .filter(this::isGenericodeFile)
-          .map(Unchecked.function(this::getCodelistIdAndContents))
-          .filter(Objects::nonNull)
-          .collect(Collectors.toMap(Pair::getKey, Pair::getValue));
-    }
   }
 
   /**
-   * Gets the codelist ID from a codelist file.
+   * Gets the codelist contents from a codelist file.
    *
    * @param codelistPath The codelist file's path
-   * @return The codelist ID and the codelist file's contents as a key/value pair
+   * @return The codelist file's contents
    * @throws FileNotFoundException If the codelist file's path is undefined or not an existing file
    */
-  private Pair<String, CodeListDocument> getCodelistIdAndContents(Path codelistPath)
-      throws FileNotFoundException {
+  private CodeListDocument getCodelistContents(Path codelistPath) throws FileNotFoundException {
     Validate.notNull(codelistPath, "Undefined codelist path");
 
     if (!Files.isRegularFile(codelistPath)) {
       throw new FileNotFoundException(codelistPath.toString());
     }
 
-    final Optional<CodeListDocument> codelist =
-        Optional.ofNullable(GenericodeTools.getMarshaller().read(codelistPath));
-
-    if (codelist.isPresent()) {
-      // We use the longName as a ID, PK in the the DB.
-      // But for the filenames we do not always follow this convention.
-      // So we need to map.
-      return codelist
-          .map(CodeListDocument::getIdentification)
-          .map((Identification identification) -> identification.getLongNameAtIndex(0))
-          .map(LongName::getValue)
-          .map((String longName) -> Pair.of(longName, codelist.get()))
-          .orElse(null);
-    }
-
-    return null;
+    logger.debug("Reading from file [{}]", codelistPath);
+    return Optional.ofNullable(GenericodeTools.getMarshaller().read(codelistPath)).orElse(null);
   }
 
-  private boolean isGenericodeFile(final Path path) {
-    return path != null
-        && Files.isRegularFile(path)
-        && GenericodeTools.EXTENSION_DOT_GC
-            .equals(MessageFormat.format(".{0}", FilenameUtils.getExtension(path.toString())));
+  private ObjectMapper createObjectMapper() {
+    return new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
   }
 
   @Override

src/test/java/eu/europa/ted/eforms/sdk/repository/SdkCodelistRepositoryTest.java

@@ -11,14 +11,10 @@
 import eu.europa.ted.eforms.sdk.entity.SdkCodelist;
 
 class SdkCodelistRepositoryTest {
-  private SdkCodelistRepository repository;
   private Identification identification;
 
   @BeforeEach
   public void setUp() {
-    repository =
-        new SdkCodelistRepository("999.0", Path.of("src", "test", "resources", "codelists", "/"));
-
     identification = new Identification();
 
     final LongName longName1 = new LongName("test-codelist-parentId");
@@ -31,20 +27,32 @@ public void setUp() {
 
   @Test
   void testGetObject() {
-    Optional<SdkCodelist> codelist = Optional.ofNullable(repository.get("test-codelist"));
+    final SdkCodelistRepository repository =
+        new SdkCodelistRepository("999.0", Path.of("src", "test", "resources", "codelists", "/"));
+
+    Optional<SdkCodelist> codelist = Optional.ofNullable(repository.get("accessibility"));
 
-    assertEquals("test-codelist", codelist.map(SdkCodelist::getCodelistId).orElse(null));
-    assertEquals(Arrays.asList("inc"), codelist.map(SdkCodelist::getCodes).orElse(null));
+    assertEquals("accessibility", codelist.map(SdkCodelist::getCodelistId).orElse(null));
+    assertEquals(Arrays.asList("inc", "n-inc", "n-inc-just"),
+        codelist.map(SdkCodelist::getCodes).orElse(null));
+
+    codelist = Optional.ofNullable(repository.get("criterion"));
+    assertEquals("criterion", codelist.map(SdkCodelist::getCodelistId).orElse(null));
+    assertEquals(Arrays.asList("autorisation", "aver-year-to", "bankr-nat", "bankruptcy"),
+        codelist.map(SdkCodelist::getCodes).orElse(null));
   }
 
   @Test
   void testGetOrDefaultObjectSdkCodelist() {
-    SdkCodelist defaultCodelist =
+    final SdkCodelistRepository repository =
+        new SdkCodelistRepository("999.0", Path.of("src", "test", "resources", "codelists", "/"));
+
+    final SdkCodelist defaultCodelist =
         new DummySdkCodelist("default-codelist", "1", Arrays.asList("code1", "code2"), null);
 
     Optional<SdkCodelist> codelist =
-        Optional.ofNullable(repository.getOrDefault("test-codelist", defaultCodelist));
-    assertEquals("test-codelist", codelist.map(SdkCodelist::getCodelistId).orElse(null));
+        Optional.ofNullable(repository.getOrDefault("accessibility", defaultCodelist));
+    assertEquals("accessibility", codelist.map(SdkCodelist::getCodelistId).orElse(null));
 
     codelist =
         Optional.ofNullable(repository.getOrDefault("nonexisting-codelist", defaultCodelist));