From 2417e5fbd289b4f10aa77e672155371b9754dba3 Mon Sep 17 00:00:00 2001
From: Dan Van Atta <dhvatta@gmail.com>
Date: Wed, 24 Jul 2024 16:25:33 -0700
Subject: [PATCH] Startup behavior: Map extraction enhancements (#12766)

Changes behavior of the system when zipped map files are encountered on startup.

For context, current system will:
- see zip folder in downloaded maps folders
- create a temp folder in the downloaded maps folder
- extracts the zip file to the new temp folder
- creates the target map folder and moves all extracted files to that folder
- deletes the zip file

Changes:
- zip file is now extracted to a temporary folder in the systems temporary folder.
This way if the process fails later on, the temp files will not be seen as potential maps
(which can create duplicates). Additionally, any such files will now be automatically
cleaned up, the system temp folder is usually emptied on system reboot.

- smarter attempt to move zip files to a 'bad-zip' folder. Only attempt to do so if
the 'bad-zip' still exists

- cleanup temporary folder where map files are extracted. The folder was not being deleted
in a successful extraction case.
---
 .../system/loader/ZippedMapsExtractor.java    | 40 ++++++++++++-------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/game-app/game-core/src/main/java/games/strategy/engine/framework/map/file/system/loader/ZippedMapsExtractor.java b/game-app/game-core/src/main/java/games/strategy/engine/framework/map/file/system/loader/ZippedMapsExtractor.java
index 3872d348cd0..1c84de430d4 100644
--- a/game-app/game-core/src/main/java/games/strategy/engine/framework/map/file/system/loader/ZippedMapsExtractor.java
+++ b/game-app/game-core/src/main/java/games/strategy/engine/framework/map/file/system/loader/ZippedMapsExtractor.java
@@ -60,27 +60,33 @@ public void unzipMapFiles() {
                         // Before 2.6 maps did not include a 'map.yaml' file and were zipped.
                         .ifPresent(MapDescriptionYaml::generateForMap);
                   } catch (final ZipReadException zipReadException) {
+                    log.warn(
+                        "Error reading zip: {}{}",
+                        mapZip.toAbsolutePath(),
+                        Files.exists(mapZip) ? ", moving to 'bad-zips' folder" : "",
+                        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.toAbsolutePath()
-                                        + ", zip has been moved to: "
-                                        + newLocation.toAbsolutePath(),
-                                    zipReadException));
+                    if (Files.exists(mapZip)) {
+                      moveBadZip(mapZip)
+                          .ifPresent(
+                              newLocation ->
+                                  log.warn(
+                                      "Error extracting map zip: {}, zip has been moved to: {}",
+                                      mapZip.toAbsolutePath(),
+                                      newLocation.toAbsolutePath(),
+                                      zipReadException));
+                    }
                   } catch (final FileSystemException 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);
+                    log.warn("Error extracting map zip: {}, {}", mapZip, e.getMessage(), e);
                   } catch (final ZipExtractor.ZipSecurityException e) {
                     log.error(
-                        "Malicious zip file detected: "
-                            + mapZip.toAbsolutePath()
-                            + ", please report this to TripleA and delete the zip file",
+                        "Malicious zip file detected: {}, please report this to TripleA and delete the zip file",
+                        mapZip.toAbsolutePath(),
                         e);
                   }
                 }));
@@ -130,7 +136,7 @@ private static Optional<Path> unzipMapThrowing(final Path mapZip) throws IOExcep
 
     // extract into a temp folder first
     // put the temp folder in the maps folder, so they're on the same partition (to move later)
-    final Path tempFolder = Files.createTempDirectory(mapsFolder, "triplea-unzip");
+    final Path tempFolder = Files.createTempDirectory("triplea-unzip");
     ZipExtractor.unzipFile(mapZip, tempFolder);
     tempFolder.toFile().deleteOnExit();
 
@@ -145,8 +151,12 @@ private static Optional<Path> unzipMapThrowing(final Path mapZip) throws IOExcep
     // replace extraction target folder contents with the temp folder containing the extracted zip
     final boolean folderReplaced =
         FileUtils.replaceFolder(tempFolderWithExtractedMap, extractionTarget);
+
+    // Either we have moved and renamed the temp folder, or we have moved and renamed the contents
+    // of the temp folder. Either way, we can go ahead and remove the now possibly empty tempFolder
+    FileUtils.deleteDirectory(tempFolder);
+
     if (!folderReplaced) {
-      FileUtils.deleteDirectory(tempFolder);
       return Optional.empty();
     }