Fix errors with map resources being not found in 2.6. (#10504)

* Fix errors with map resources being not found in 2.6.

This was caused by some maps having duplicate files (e.g. a polygon.txt file in baseTiles folder). Before this change, it's undefined which of these files will be found first and all other files will be expected to be alongside it.

With this change, the search returns files in a deterministic order, sorted by path length. So files closer to the root will be first in the list. This makes the behavior deterministic across platforms and the map to use the file closest to the map root.

Renames findAny() to findClosestToRoot() and adds tests.

Author: asvitkine

game-app/game-core/src/main/java/games/strategy/engine/framework/map/file/system/loader/InstalledMap.java

@@ -45,7 +45,7 @@ Optional<Path> findContentRoot() {
       // a polygons file, the location of the polygons file is the map content root.
       final Path mapYamlParentFolder = mapDescriptionYaml.getYamlFileLocation().getParent();
       contentRoot =
-          FileUtils.findAny(mapYamlParentFolder, 3, MapData.POLYGON_FILE)
+          FileUtils.findClosestToRoot(mapYamlParentFolder, 3, MapData.POLYGON_FILE)
               .map(Path::getParent)
               .orElse(null);
     }

game-app/map-data/src/main/java/org/triplea/map/description/file/MapDescriptionYaml.java

@@ -210,7 +210,7 @@ private Optional<String> findFileNameForGame(final String gameName) {
   /** Find 'games' folder starting from map.yml parent folder. */
   private Optional<Path> findGamesFolder() {
     final Path mapFolder = yamlFileLocation.getParent();
-    final Optional<Path> gamesFolder = FileUtils.findAny(mapFolder, 5, "games");
+    final Optional<Path> gamesFolder = FileUtils.findClosestToRoot(mapFolder, 5, "games");
 
     if (gamesFolder.isEmpty()) {
       log.warn("No 'games' folder found under location: {}", mapFolder.toAbsolutePath());
@@ -220,7 +220,7 @@ private Optional<Path> findGamesFolder() {
 
   /** Search 'games' folder for a game-xml-file. */
   private Optional<Path> searchForGameFile(final Path gamesFolder, final String xmlFileName) {
-    final Optional<Path> gameFile = FileUtils.findAny(gamesFolder, 3, xmlFileName);
+    final Optional<Path> gameFile = FileUtils.findClosestToRoot(gamesFolder, 3, xmlFileName);
     if (gameFile.isEmpty()) {
       log.warn(
           "Failed to find game file: {}, within directory tree rooted at: {}",

game-app/map-data/src/main/java/org/triplea/map/description/file/MapDescriptionYamlReader.java

@@ -37,7 +37,7 @@ private static Optional<Path> findMapYmlFile(final Path mapFolder) {
      * 'map.yml' file at depth 2, eg: 'map_folder-master/map_folder/map/map.yml'.
      */
     final int maxMapYmlSearchDepth = 2;
-    return FileUtils.findAny(
+    return FileUtils.findClosestToRoot(
         mapFolder, maxMapYmlSearchDepth, MapDescriptionYaml.MAP_YAML_FILE_NAME);
   }
 

lib/java-extras/src/main/java/org/triplea/io/FileUtils.java

@@ -14,6 +14,7 @@
 import java.nio.file.Path;
 import java.time.Instant;
 import java.util.Collection;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Function;
@@ -74,26 +75,41 @@ public static URL toUrl(final Path file) {
 
   /**
    * Searches a file system starting from a given directory looking for a file or directory with a
-   * matching name.
+   * matching name. If multiple matching paths are found, returns the one closest to the root (via
+   * minimum length of the absolute path).
    *
-   * @param searchRoot The directory whose contents we will search (and sub-directories)
+   * @param searchRoot The directory whose contents we will search (and subdirectories).
    * @param maxDepth The maximum number of subdirectories to search. Zero means only search the
    *     'searchRoot' directory.
    * @param fileName The name of the file to be search for.
    * @return A file matching the given name or empty if not found.
    */
-  public Optional<Path> findAny(final Path searchRoot, final int maxDepth, final String fileName) {
+  public Optional<Path> findClosestToRoot(
+      final Path searchRoot, final int maxDepth, final String fileName) {
     return find(searchRoot, maxDepth, fileName).stream().findAny();
   }
 
-  public Collection<Path> find(final Path searchRoot, final int maxDepth, final String fileName) {
+  /**
+   * Searches a file system starting from a given directory looking for files or directories with
+   * matching names. The resulting list will be in ascending order by absolute path length.
+   *
+   * @param searchRoot The directory whose contents we will search (and subdirectories).
+   * @param maxDepth The maximum number of subdirectories to search. Zero means only search the
+   *     'searchRoot' directory.
+   * @param fileName The name of the file to be search for.
+   * @return A list of files matching the given name or an empty list if not.
+   */
+  public List<Path> find(final Path searchRoot, final int maxDepth, final String fileName) {
     Preconditions.checkArgument(Files.isDirectory(searchRoot), searchRoot.toAbsolutePath());
     Preconditions.checkArgument(Files.exists(searchRoot), searchRoot.toAbsolutePath());
     Preconditions.checkArgument(maxDepth > -1);
     Preconditions.checkArgument(!fileName.isBlank());
     try (Stream<Path> files = Files.walk(searchRoot, maxDepth)) {
       return files
           .filter(f -> f.getFileName().toString().equals(fileName))
+          // Sort by path length (shortest to longest), so that the ordering is deterministic and
+          // paths closer to the root are earlier in the list.
+          .sorted(Comparator.comparingInt(f -> f.toAbsolutePath().toString().length()))
           .collect(Collectors.toList());
     } catch (final IOException e) {
       log.error(
@@ -243,7 +259,7 @@ public static void deleteDirectory(final Path path) throws IOException {
   }
 
   /**
-   * Does an overwrite of one folder onto another and rolls back if there any errors. The rollback
+   * Does an overwrite of one folder onto another and rolls back if there were errors. The rollback
    * is done by first moving the destination folder to a backup location. If there are any errors
    * then we delete whatever we copied and move the backup location back to the destination
    * location.
@@ -287,8 +303,7 @@ static boolean replaceFolder(
       return true;
     }
 
-    // otherwise create a backup of the destination folder before we replace it
-
+    // otherwise, create a backup of the destination folder before we replace it
     final Path backupFolder;
     try {
       backupFolder = Files.createTempDirectory("temp-dir").resolve(dest.getFileName());
@@ -298,7 +313,7 @@ static boolean replaceFolder(
     }
 
     try {
-      // make a complete backup by moving the dest folder to backup
+      // make a complete backup by moving the dest folder to back up
       fileMoveOperation.move(dest, backupFolder);
 
       // do the folder move

lib/java-extras/src/test/java/org/triplea/io/FileUtilsTest.java

@@ -4,14 +4,15 @@
 import static com.github.npathai.hamcrestopt.OptionalMatchers.isPresent;
 import static com.github.npathai.hamcrestopt.OptionalMatchers.isPresentAndIs;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.is;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doCallRealMethod;
 
-import java.io.File;
 import java.io.IOException;
+import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.time.Instant;
@@ -59,10 +60,7 @@ void shouldReturnEmptyCollectionWhenDirectoryIsEmpty() throws Exception {
   @Test
   @DisplayName("Verify we can read file contents with ISO-8859-1 encoded characters")
   void readContents() {
-    final File testFile =
-        new File(
-            FileUtilsTest.class.getClassLoader().getResource("ISO-8859-1-test-file.txt").getFile());
-    final Path testFilePath = Path.of(testFile.toURI());
+    final Path testFilePath = getPathOfResource("ISO-8859-1-test-file.txt");
 
     final Optional<String> contentRead = FileUtils.readContents(testFilePath);
 
@@ -87,9 +85,7 @@ void fileExists() {
       //   |- child1/
       //      |- child2/
       //         |- touch-file
-      final File testFolderFile =
-          new File(FileUtilsTest.class.getClassLoader().getResource("test-folder-path").getFile());
-      final Path testFolderPath = Path.of(testFolderFile.toURI());
+      final Path testFolderPath = getPathOfResource("test-folder-path");
       final Path child1 = testFolderPath.resolve("child1");
       final Path child2 = child1.resolve("child2");
 
@@ -118,6 +114,45 @@ void fileExists() {
     }
   }
 
+  @Nested
+  class Find {
+    // Folder structure:
+    // |- test-folder-path/
+    //   |- test-file
+    //   |- child1/
+    //      |- child2/
+    //         |- test-file
+    final Path testFolderPath = getPathOfResource("test-folder-path");
+    final Path child1 = testFolderPath.resolve("child1");
+    final Path child2 = child1.resolve("child2");
+
+    @Test
+    void findClosestToRootDepth1() {
+      assertThat(
+          FileUtils.findClosestToRoot(child2, 1, "test-file"),
+          isPresentAndIs(child2.resolve("test-file")));
+    }
+
+    @Test
+    void findClosestToRootDepth3() {
+      assertThat(
+          FileUtils.findClosestToRoot(testFolderPath, 3, "test-file"),
+          isPresentAndIs(testFolderPath.resolve("test-file")));
+    }
+
+    @Test
+    void findDepth1() {
+      assertThat(FileUtils.find(child2, 1, "test-file"), contains(child2.resolve("test-file")));
+    }
+
+    @Test
+    void findDepth3() {
+      assertThat(
+          FileUtils.find(testFolderPath, 3, "test-file"),
+          contains(testFolderPath.resolve("test-file"), child2.resolve("test-file")));
+    }
+  }
+
   /**
    * Creates a non-empty directory, run delete, verify directory is deleted. Linux systems will fail
    * the delete operation if the directory is not empty, this test verifies that we do a recursive
@@ -251,4 +286,12 @@ void getLastModified() throws IOException {
     assertThat(FileUtils.getLastModified(tempFile), isPresent());
     assertThat(FileUtils.getLastModified(tempFile).get().isBefore(Instant.now()), is(true));
   }
+
+  private Path getPathOfResource(String name) {
+    try {
+      return Path.of(FileUtilsTest.class.getClassLoader().getResource(name).toURI());
+    } catch (URISyntaxException e) {
+      throw new IllegalStateException(e);
+    }
+  }
 }