-
Notifications
You must be signed in to change notification settings - Fork 396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Store downloaded maps unzipped #8622
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
c8f412d
Store downloaded maps unzipped
DanVanAtta 2dc2eb2
Fix up tests
DanVanAtta ce91d27
Rename method 'findAllUnzippedXmlFiles' -> 'findAllGameXmlFiles'
DanVanAtta 0c627ce
Remove now duplicative resource paths
DanVanAtta cb76162
Rename variable
DanVanAtta File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
170 changes: 170 additions & 0 deletions
170
...main/java/games/strategy/engine/framework/map/file/system/loader/ZippedMapsExtractor.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(); | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah one thing I forgot to mention: |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly related to this PR, but I start wondering how much code we could end up saving by migrating away from
java.io.File
tojava.nio.Path
.This simple method could get simplified to something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not having to deal with file separator is a win. We likely should be preferring path where we can. Working in existing code is a bit odd as it's mixed now, but I'm certainly on board to convert and use Path in new code.