Store downloaded maps unzipped

game-core/src/main/java/games/strategy/engine/framework/map/download/DownloadCoordinator.java

@@ -1,5 +1,7 @@
 package games.strategy.engine.framework.map.download;
 
+import games.strategy.engine.framework.map.file.system.loader.ZippedMapsExtractor;
+import java.io.IOException;
 import java.util.ArrayDeque;
 import java.util.Collection;
 import java.util.HashSet;
@@ -8,11 +10,13 @@
 import java.util.Queue;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
+import lombok.extern.slf4j.Slf4j;
 
 /**
  * Class that accepts and queues download requests. Download requests are started in background
  * thread, this class ensures N are in progress until all are done.
  */
+@Slf4j
 public final class DownloadCoordinator {
   public static final DownloadCoordinator instance = new DownloadCoordinator();
 
@@ -117,6 +121,16 @@ public void downloadUpdated(final DownloadFileDescription download, final long b
 
     @Override
     public void downloadComplete(final DownloadFileDescription download) {
+      try {
+        ZippedMapsExtractor.unzipMap(download.getInstallLocation());
+      } catch (final IOException e) {
+        log.warn(
+            "Error extracting downloaded map zip (map may need to be re-installed: "
+                + download.getMapName()
+                + ", "
+                + e.getMessage(),
+            e);
+      }
       downloadListeners.forEach(it -> it.downloadComplete(download));
 
       synchronized (lock) {

game-core/src/main/java/games/strategy/engine/framework/map/download/MapDownloadList.java

@@ -3,6 +3,8 @@
 import static java.util.function.Predicate.not;
 
 import com.google.common.annotations.VisibleForTesting;
+import games.strategy.engine.framework.map.file.system.loader.DownloadedMaps;
+import java.io.File;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -27,7 +29,13 @@ class MapDownloadList {
       if (download == null) {
         return;
       }
-      final Optional<Integer> mapVersion = strategy.getMapVersion(download.getInstallLocation());
+
+      // note, getParentFile assumes that the folder returned by 'findPathToMapFolder'
+      // is always one folder deep into the map folder (eg: map-name/map)
+      final Optional<Integer> mapVersion =
+          DownloadedMaps.findPathToMapFolder(download.getMapName())
+              .map(File::getParentFile)
+              .flatMap(strategy::getMapVersion);
 
       if (mapVersion.isPresent()) {
         installed.add(download);

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

@@ -8,15 +8,12 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
-import java.util.Set;
 import java.util.stream.Collectors;
 import lombok.experimental.UtilityClass;
 import lombok.extern.slf4j.Slf4j;
 import org.triplea.io.FileUtils;
-import org.triplea.io.ZipFileUtil;
 
 /**
  * Reads from file system to find all available games. We then shallow parse each available game to
@@ -27,15 +24,11 @@
 @Slf4j
 class AvailableGamesFileSystemReader {
 
-  private static final String ZIP_EXTENSION = ".zip";
-
   static synchronized DownloadedMaps parseMapFiles() {
-    final Set<DefaultGameChooserEntry> entries = new HashSet<>();
-    entries.addAll(mapXmlsGameNamesByUri(findAllZippedXmlFiles()));
-    entries.addAll(mapXmlsGameNamesByUri(findAllUnZippedXmlFiles()));
-    entries.forEach(
-        entry -> log.debug("Found game: " + entry.getGameName() + " @ " + entry.getUri()));
-    return new DownloadedMaps(entries);
+    final List<URI> unzippedXmlFiles = findAllUnZippedXmlFiles();
+    final Collection<DefaultGameChooserEntry> gameChooserEntries =
+        mapXmlsGameNamesByUri(unzippedXmlFiles);
+    return new DownloadedMaps(gameChooserEntries);
   }
 
   private Collection<DefaultGameChooserEntry> mapXmlsGameNamesByUri(
@@ -44,15 +37,7 @@ private Collection<DefaultGameChooserEntry> mapXmlsGameNamesByUri(
         .map(DefaultGameChooserEntry::newDefaultGameChooserEntry)
         .filter(Optional::isPresent)
         .map(Optional::get)
-        .collect(Collectors.toList());
-  }
-
-  private static List<URI> findAllZippedXmlFiles() {
-    return FileUtils.listFiles(ClientFileSystemHelper.getUserMapsFolder()).stream()
-        .filter(File::isFile)
-        .filter(file -> file.getName().toLowerCase().endsWith(ZIP_EXTENSION))
-        .map(ZipFileUtil::findXmlFilesInZip)
-        .flatMap(Collection::stream)
+        .peek(entry -> log.debug("Found game: " + entry.getGameName() + " @ " + entry.getUri()))
         .collect(Collectors.toList());
   }
 

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

@@ -3,10 +3,10 @@
 import games.strategy.engine.framework.ui.DefaultGameChooserEntry;
 import java.io.File;
 import java.net.URI;
+import java.util.Collection;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
-import java.util.Set;
 import java.util.stream.Collectors;
 import lombok.AllArgsConstructor;
 
@@ -16,13 +16,14 @@
  */
 @AllArgsConstructor
 public class DownloadedMaps {
-  private final Set<DefaultGameChooserEntry> availableGames;
+  private final Collection<DefaultGameChooserEntry> availableGames;
 
   /**
    * Reads the downloaded maps folder contents, parses those contents to find available games, and
    * returns the list of available games found.
    */
   public static synchronized DownloadedMaps parseMapFiles() {
+    ZippedMapsExtractor.unzipMapFiles();
     return AvailableGamesFileSystemReader.parseMapFiles();
   }
 

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

@@ -4,24 +4,18 @@
 import com.google.common.base.Preconditions;
 import games.strategy.engine.ClientFileSystemHelper;
 import java.io.File;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import lombok.experimental.UtilityClass;
 
+/** Internal utility class to find the path of a given map by name. */
 @UtilityClass
 class FileSystemMapFinder {
 
   static Optional<File> getPath(final String mapName) {
-    return getCandidatePaths(mapName).stream().filter(File::exists).findAny();
-  }
-
-  private static List<File> getCandidatePaths(final String mapName) {
-    final List<File> candidates = new ArrayList<>();
-    candidates.addAll(
-        getMapDirectoryCandidates(mapName, ClientFileSystemHelper.getUserMapsFolder()));
-    candidates.addAll(getMapZipFileCandidates(mapName, ClientFileSystemHelper.getUserMapsFolder()));
-    return candidates;
+    return getCandidatePaths(mapName, ClientFileSystemHelper.getUserMapsFolder()).stream()
+        .filter(File::exists)
+        .findFirst();
   }
 
   /**
@@ -35,7 +29,7 @@ private static List<File> getCandidatePaths(final String mapName) {
    * @return A list of candidate directories; never {@code null}.
    */
   @VisibleForTesting
-  static List<File> getMapDirectoryCandidates(final String mapName, final File userMapsFolder) {
+  static List<File> getCandidatePaths(final String mapName, final File userMapsFolder) {
     Preconditions.checkNotNull(mapName);
 
     final String dirName = File.separator + mapName;
@@ -47,27 +41,6 @@ static List<File> getMapDirectoryCandidates(final String mapName, final File use
         new File(userMapsFolder, normalizedMapName));
   }
 
-  /**
-   * Returns a list of candidate zip files from which the specified map may be loaded.
-   *
-   * <p>The candidate zip files are returned in order of preference. That is, a candidate file
-   * earlier in the list should be preferred to a candidate file later in the list assuming they
-   * both exist.
-   *
-   * @param mapName The map name; must not be {@code null}.
-   * @return A list of candidate zip files; never {@code null}.
-   */
-  public static List<File> getMapZipFileCandidates(
-      final String mapName, final File userMapsFolder) {
-    Preconditions.checkNotNull(mapName);
-
-    final String normalizedMapName = normalizeMapName(mapName);
-    return List.of(
-        new File(userMapsFolder, mapName + ".zip"),
-        new File(userMapsFolder, normalizedMapName + "-master.zip"),
-        new File(userMapsFolder, normalizedMapName + ".zip"));
-  }
-
   @VisibleForTesting
   static String normalizeMapName(final String zipName) {
     final StringBuilder sb = new StringBuilder();

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

@@ -0,0 +1,170 @@
+package games.strategy.engine.framework.map.file.system.loader;
+
+import com.google.common.base.Preconditions;
+import games.strategy.engine.ClientFileSystemHelper;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.FileAlreadyExistsException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import lombok.experimental.UtilityClass;
+import lombok.extern.slf4j.Slf4j;
+import org.triplea.io.FileUtils;
+import org.triplea.io.ZipExtractor;
+import org.triplea.io.ZipExtractor.FileSystemException;
+import org.triplea.io.ZipExtractor.ZipReadException;
+
+/**
+ * Responsible to find downloaded maps and unzip any that are zipped. Any 'bad' map zips that we
+ * fail to unzip will be moved into a bad-zip folder.
+ */
+@UtilityClass
+@Slf4j
+public class ZippedMapsExtractor {
+  private static final String ZIP_EXTENSION = ".zip";
+
+  /** Finds all map zips, extracts them and then removes the original zip. */
+  static void unzipMapFiles() {
+    findAllZippedMapFiles()
+        .forEach(
+            mapZip -> {
+              try {
+                unzipMap(mapZip);
+                renameZipPropertiesFile(mapZip);
+                removeMapZip(mapZip);
+              } catch (final ZipReadException zipReadException) {
+                // Problem reading the zip, move it to a folder so that the user does
+                // not repeatedly see an error trying to read this zip.
+                moveBadZip(mapZip)
+                    .ifPresent(
+                        newLocation ->
+                            log.warn(
+                                "Error extracting map zip: "
+                                    + mapZip.getAbsolutePath()
+                                    + ", zip has been moved to: "
+                                    + newLocation.toFile().getAbsolutePath(),
+                                zipReadException));
+              } catch (final FileSystemException | IOException e) {
+                // Thrown if we are are out of disk space or have file system access issues.
+                // Do not move the zip file to a bad-zip folder as that operation could also
+                // fail.
+                log.warn("Error extracting map zip: " + mapZip + ", " + e.getMessage(), e);
+              }
+            });
+  }
+
+  private static Collection<File> findAllZippedMapFiles() {
+    return FileUtils.listFiles(ClientFileSystemHelper.getUserMapsFolder()).stream()
+        .filter(File::isFile)
+        .filter(file -> file.getName().toLowerCase().endsWith(ZIP_EXTENSION))
+        .collect(Collectors.toList());
+  }
+
+  /**
+   * Unzips are target map file into the downloaded maps folder, deletes the zip file after
+   * extraction. Extracted files are first extracted to a temporary location before being moved into
+   * the downloaded maps folder. This temporary location is to help avoid intermediate results if
+   * for example we run out of disk space while extracting.
+   *
+   * @param mapZip The map zip file to be extracted to the downloaded maps folder.
+   */
+  public static void unzipMap(final File mapZip) throws IOException {
+    Preconditions.checkArgument(mapZip.isFile(), mapZip.getAbsolutePath());
+    Preconditions.checkArgument(mapZip.exists(), mapZip.getAbsolutePath());
+    Preconditions.checkArgument(mapZip.getName().endsWith(".zip"), mapZip.getAbsolutePath());
+
+    final Path extractionTarget = ClientFileSystemHelper.getUserMapsFolder().toPath();
+
+    log.info(
+        "Extracting map zip: {} -> {}",
+        mapZip.getAbsolutePath(),
+        extractionTarget.toAbsolutePath());
+    final Path tempFolder = Files.createTempDirectory("triplea-unzip");
+    ZipExtractor.unzipFile(mapZip, tempFolder.toFile());
+    for (final File file : FileUtils.listFiles(tempFolder.toFile())) {
+      final File extractionTargetFile = extractionTarget.resolve(file.getName()).toFile();
+      if (extractionTargetFile.exists() && extractionTargetFile.isDirectory()) {
+        org.apache.commons.io.FileUtils.deleteDirectory(extractionTargetFile);
+      } else if (extractionTargetFile.exists()) {
+        extractionTargetFile.delete();
+      }
+
+      try {
+        Files.move(file.toPath(), extractionTarget.resolve(file.getName()));
+      } catch (final FileAlreadyExistsException e) {
+        log.error(
+            "Error, destination file already exists, failed to overwrite while unzipping map. Map: "
+                + mapZip.getAbsolutePath()
+                + ",file to write "
+                + extractionTargetFile.getAbsolutePath(),
+            e);
+        return;
+      }
+    }
+  }
+
+  /** Find .properties suffixed map files and renames them to match the output map file. */
+  private static void renameZipPropertiesFile(final File mapZip) {
+    final String newName = mapZip.getName().replace(".zip", "") + ".properties";
+
+    final String oldPropertiesFileName = mapZip.getName() + ".properties";
+    final Path oldPropertiesFilePath = mapZip.toPath().getParent().resolve(oldPropertiesFileName);
+    if (oldPropertiesFilePath.toFile().exists()) {
+      final Path newFilePath = mapZip.toPath().getParent().resolve(newName);
+      try {
+        log.info("Renaming {} -> {}", oldPropertiesFilePath, newFilePath);
+        Files.move(oldPropertiesFilePath, newFilePath);
+      } catch (final IOException e) {
+        throw new FileSystemException(
+            "Failed to rename file: " + oldPropertiesFilePath + " to " + newFilePath, e);
+      }
+    }
+  }
+
+  private static void removeMapZip(final File mapZip) {
+    log.info("Removing map zip: {}", mapZip.getAbsolutePath());
+    final boolean removed = mapZip.delete();
+    if (!removed) {
+      log.info(
+          "Failed to remove (extracted) zip file: {}, marking file to be deleted on exit.",
+          mapZip.getAbsolutePath());
+      mapZip.deleteOnExit();
+    }
+  }
+
+  /**
+   * Moves a target zip file into a 'bad-zip' folder. This is to prevent the file from being picked
+   * up in future unzip operations and cause repeated warning messages to users.
+   *
+   * @return Returns the new location of the file, returns an empty if the file move operation
+   *     failed.
+   */
+  private static Optional<Path> moveBadZip(final File mapZip) {
+    final Path badZipFolder =
+        ClientFileSystemHelper.getUserMapsFolder().toPath().resolve("bad-zips");
+    if (!badZipFolder.toFile().mkdirs()) {
+      log.error(
+          "Unable to create folder: "
+              + badZipFolder.toFile().getAbsolutePath()
+              + ", please report this to TripleA and create the folder manually.");
+      return Optional.empty();
+    }
+    try {
+      final Path newLocation = badZipFolder.resolve(mapZip.getName());
+      Files.move(mapZip.toPath(), newLocation);
+
+      return Optional.of(newLocation);
+    } catch (final IOException e) {
+      log.error(
+          "Failed to move file: "
+              + mapZip.getAbsolutePath()
+              + ", to: "
+              + badZipFolder.toFile().getAbsolutePath(),
+          e);
+      return Optional.empty();
+    }
+  }
+}