Merge branch 'release/0.4.6'

build.gradle

@@ -17,7 +17,7 @@ apply plugin: 'org.springframework.boot'
 apply plugin: 'propdeps'
 
 group = 'micheljung'
-version = '0.4.5'
+version = '0.4.6'
 
 sourceCompatibility = 1.8
 targetCompatibility = 1.8
@@ -185,6 +185,7 @@ dependencies {
   compile("com.googlecode.zohhak:zohhak:${zohhakVersion}")
   compile("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${jacksonDatatypeJsr310Version}")
   compile("com.mandrillapp.wrapper.lutung:lutung:${lutungVersion}")
+  compile("org.apache.commons:commons-compress:${commonsCompressVersion}")
 
   runtime("mysql:mysql-connector-java:${mysqlConnectorVersion}")
 

gradle.properties

@@ -23,8 +23,9 @@ junitAddonsVersion=1.4
 zohhakVersion=1.1.1
 githubApiVersion=1.84
 jgitVersionn=4.5.0.201609210915-r
-fafCommonsVersion=81da093d61b937a2433a5c14d39cf66c4b5f20f2
+fafCommonsVersion=1abae565c0cdd3b34bdffc848f62ef6ba24c6bac
 h2Version=1.4.193
 jacksonDatatypeJsr310Version=2.8.6
 mockitoVersion=2.7.0
 lutungVersion=0.0.7
+commonsCompressVersion=1.13

src/main/java/com/faforever/api/deployment/LegacyFeaturedModDeploymentTask.java

@@ -8,29 +8,35 @@
 import com.faforever.api.featuredmods.FeaturedModService;
 import com.faforever.commons.fa.ForgedAllianceExePatcher;
 import com.faforever.commons.mod.ModReader;
-import com.faforever.commons.zip.Zipper;
 import com.google.common.collect.Sets;
+import com.google.common.io.ByteStreams;
 import lombok.Data;
 import lombok.Setter;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
+import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.stereotype.Component;
 import org.springframework.util.Assert;
 
 import javax.validation.ValidationException;
+import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.SimpleFileVisitor;
 import java.nio.file.StandardCopyOption;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import java.util.zip.ZipOutputStream;
 
 import static com.github.nocatch.NoCatch.noCatch;
 import static com.google.common.hash.Hashing.md5;
@@ -79,11 +85,6 @@ public void run() {
     log.info("Starting deployment of '{}' from '{}', branch '{}', replaceExisting '{}', modFilesExtension '{}'",
         modName, repositoryUrl, branch, replaceExisting, modFilesExtension);
 
-    if (fileIds.isEmpty()) {
-      log.warn("Could not find any files to deploy. Is the configuration correct?");
-      return;
-    }
-
     Path repositoryDirectory = buildRepositoryDirectoryPath(repositoryUrl);
     checkoutCode(repositoryDirectory, repositoryUrl, branch);
 
@@ -95,6 +96,10 @@ public void run() {
     List<StagedFile> files = packageDirectories(repositoryDirectory, version, fileIds, targetFolder);
     createPatchedExe(version, fileIds, targetFolder).ifPresent(files::add);
 
+    if (files.isEmpty()) {
+      log.warn("Could not find any files to deploy. Is the configuration correct?");
+      return;
+    }
     files.forEach(this::renameToFinalFile);
 
     updateDatabase(files, version, modName);
@@ -111,6 +116,7 @@ private Optional<StagedFile> createPatchedExe(short version, Map<String, Short>
     String clientFileName = "ForgedAlliance.exe";
     Short fileId = fileIds.get(clientFileName);
     if (fileId == null) {
+      log.debug("Skipping '{}' because there's no file ID available", clientFileName);
       return Optional.empty();
     }
 
@@ -157,7 +163,7 @@ private List<StagedFile> packageDirectories(Path repositoryDirectory, short vers
     try (Stream<Path> stream = Files.list(repositoryDirectory)) {
       return stream
           .filter((path) -> Files.isDirectory(path) && !path.getFileName().toString().startsWith("."))
-          .map(path -> packFile(path, version, targetFolder, fileIds))
+          .map(path -> packDirectory(path, version, targetFolder, fileIds))
           .filter(Optional::isPresent)
           .map(Optional::get)
           .collect(Collectors.toList());
@@ -181,24 +187,24 @@ private StagedFile renameToFinalFile(StagedFile file) {
    * content of the directory. If no file ID is available, an empty optional is returned.
    */
   @SneakyThrows
-  private Optional<StagedFile> packFile(Path folderToBeZipped, Short version, Path targetFolder, Map<String, Short> fileIds) {
-    String folderName = folderToBeZipped.getFileName().toString();
-    Path targetNxtFile = targetFolder.resolve(String.format("%s.%d.nxt", folderName, version));
+  private Optional<StagedFile> packDirectory(Path directory, Short version, Path targetFolder, Map<String, Short> fileIds) {
+    String directoryName = directory.getFileName().toString();
+    Path targetNxtFile = targetFolder.resolve(String.format("%s.%d.nxt", directoryName, version));
     Path tmpNxtFile = toTmpFile(targetNxtFile);
 
     // E.g. "effects.nx2"
-    String clientFileName = String.format("%s.%s", folderName, configuration.getModFilesExtension());
+    String clientFileName = String.format("%s.%s", directoryName, configuration.getModFilesExtension());
     Short fileId = fileIds.get(clientFileName);
     if (fileId == null) {
-      log.debug("Skipping folder '{}' because there's no file ID available", folderName);
+      log.debug("Skipping folder '{}' because there's no file ID available", directoryName);
       return Optional.empty();
     }
 
-    log.trace("Packaging '{}' to '{}'", folderToBeZipped, targetFolder);
+    log.trace("Packaging '{}' to '{}'", directory, targetFolder);
 
     createDirectories(targetFolder);
-    try (ZipOutputStream outputStream = new ZipOutputStream(Files.newOutputStream(tmpNxtFile))) {
-      Zipper.contentOf(folderToBeZipped).to(outputStream).zip();
+    try (ZipArchiveOutputStream outputStream = new ZipArchiveOutputStream(tmpNxtFile.toFile())) {
+      zipContents(directory, outputStream);
     }
     return Optional.of(new StagedFile(fileId, tmpNxtFile, targetNxtFile, clientFileName));
   }
@@ -222,6 +228,38 @@ private Path toTmpFile(Path targetFile) {
     return targetFile.getParent().resolve(targetFile.getFileName().toString() + ".tmp");
   }
 
+  /**
+   * Since Java's ZIP implementation uses data descriptors, which FA doesn't implement and therefore cant' read,
+   * this implementation uses Apache's commons compress which doesn't use data descriptors as long as the target is a
+   * file or a seekable byte channel.
+   */
+  private void zipContents(Path directoryToZip, ZipArchiveOutputStream outputStream) throws IOException {
+    Files.walkFileTree(directoryToZip, new SimpleFileVisitor<Path>() {
+      public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
+        Path relativized = directoryToZip.relativize(dir);
+        if (relativized.getNameCount() != 0) {
+          outputStream.putArchiveEntry(new ZipArchiveEntry(relativized.toString() + "/"));
+          outputStream.closeArchiveEntry();
+        }
+        return FileVisitResult.CONTINUE;
+      }
+
+      public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+        log.trace("Zipping file {}", file.toAbsolutePath());
+        outputStream.putArchiveEntry(new ZipArchiveEntry(
+            file.toFile(),
+            directoryToZip.relativize(file).toString().replace(File.separatorChar, '/'))
+        );
+
+        try (InputStream inputStream = Files.newInputStream(file)) {
+          ByteStreams.copy(inputStream, outputStream);
+        }
+        outputStream.closeArchiveEntry();
+        return FileVisitResult.CONTINUE;
+      }
+    });
+  }
+
   /**
    * Describes a file that is ready to be deployed. All files should be staged as temporary files first so they can be
    * renamed to their target file name in one go, thus minimizing the time of inconsistent file system state.

src/main/java/com/faforever/api/error/ErrorCode.java

@@ -52,7 +52,7 @@ public enum ErrorCode {
   MAP_SCENARIO_LUA_MISSING(143, "Invalid Map File", "Zip file does not contain a *_scenario.lua"),
   MAP_MISSING_MAP_FOLDER_INSIDE_ZIP(144, "No folder inside Zip", "Zip file must contain a folder with all map data"),
   MAP_FILE_INSIDE_ZIP_MISSING(145, "File is missing", "Cannot find needed file with pattern ''{0}'' inside zip file"),
-  MAP_NO_VALID_JSON_METADATA(146, "No valid json", "Metadata json is not valid"),
+  MAP_UPLOAD_INVALID_METADATA(146, "Invalid metadata", "Metadata is not valid: {}"),
   MAP_RENAME_FAILED(147, "Cannot rename to correct name failed ", "Cannot rename file ''{0}''"),
   MAP_INVALID_ZIP(148, "Invalid zip file", "The zip file should only contain one folder at the root level"),
   CLAN_CREATE_CREATOR_IS_IN_A_CLAN(149, "You are already in a clan", "Clan creator is already member of a clan"),

src/main/java/com/faforever/api/map/MapService.java

@@ -10,13 +10,14 @@
 import com.faforever.api.error.ErrorCode;
 import com.faforever.api.error.ProgrammingError;
 import com.faforever.api.utils.JavaFxUtil;
+import com.faforever.commons.io.Unzipper;
+import com.faforever.commons.io.Zipper;
 import com.faforever.commons.lua.LuaLoader;
 import com.faforever.commons.map.PreviewGenerator;
-import com.faforever.commons.zip.Unzipper;
-import com.faforever.commons.zip.Zipper;
 import javafx.scene.image.Image;
 import lombok.Data;
 import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
 import org.luaj.vm2.LuaValue;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
@@ -43,6 +44,7 @@
 import static com.github.nocatch.NoCatch.noCatch;
 
 @Service
+@Slf4j
 public class MapService {
   private static final String[] REQUIRED_FILES = new String[]{
       ".scmap",

src/main/java/com/faforever/api/map/MapsController.java

@@ -12,6 +12,7 @@
 import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
+import lombok.extern.slf4j.Slf4j;
 import org.springframework.http.MediaType;
 import org.springframework.security.core.Authentication;
 import org.springframework.web.bind.annotation.RequestMapping;
@@ -26,6 +27,7 @@
 
 @RestController
 @RequestMapping(path = "/maps")
+@Slf4j
 public class MapsController {
   private final MapService mapService;
   private final FafApiProperties fafApiProperties;
@@ -63,7 +65,8 @@ public void uploadMap(@RequestParam("file") MultipartFile file,
       JsonNode node = objectMapper.readTree(jsonString);
       ranked = node.path("is_ranked").asBoolean(false);
     } catch (IOException e) {
-      throw new ApiException(new Error(ErrorCode.MAP_NO_VALID_JSON_METADATA));
+      log.debug("Could not parse metadata", e);
+      throw new ApiException(new Error(ErrorCode.MAP_UPLOAD_INVALID_METADATA, e.getMessage()));
     }
 
     Player player = playerService.getPlayer(authentication);

src/main/resources/config/application-dev.yml

@@ -24,6 +24,23 @@ faf-api:
     forged-alliance-exe-path: ${FORGED_ALLIANCE_EXE_PATH}
     repositories-directory: ${REPOSITORIES_DIRECTORY:build/cache/repos}
     featured-mods-target-directory: ${FEATURED_MODS_TARGET_DIRECTORY:build/cache/deployment}
+    # TODO make this runtime configuration
+    configurations:
+      - repositoryUrl: https://github.com/FAForever/fa.git
+        branch: deploy/faf
+        modName: faf
+        modFilesExtension: nx2
+        replaceExisting: false
+      - repositoryUrl: https://github.com/FAForever/fa.git
+        branch: deploy/fafbeta
+        modName: fafbeta
+        modFilesExtension: nx4
+        replaceExisting: true
+      - repositoryUrl: https://github.com/FAForever/fa.git
+        branch: deploy/fafdevelop
+        modName: fafdevelop
+        modFilesExtension: nx5
+        replaceExisting: true
 
 spring:
   datasource:

src/test/java/com/faforever/api/deployment/LegacyFeaturedModDeploymentTaskTest.java

@@ -21,14 +21,18 @@
 import java.io.RandomAccessFile;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.junit.Assert.assertThat;
+import static org.mockito.ArgumentMatchers.anyShort;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -61,6 +65,36 @@ public void testRunWithoutConfigurationThrowsException() throws Exception {
     instance.run();
   }
 
+  @Test
+  public void testRunNoFileIds() throws Exception {
+    instance.setConfiguration(new DeploymentConfiguration()
+        .setBranch("branch")
+        .setModFilesExtension("nx3")
+        .setModName("faf")
+        .setReplaceExisting(true)
+        .setRepositoryUrl("git@example.com/FAForever/faf"));
+
+    Mockito.doAnswer(invocation -> {
+      Path repoFolder = invocation.getArgument(0);
+      Files.createDirectories(repoFolder.resolve("someDir"));
+      Files.copy(
+          LegacyFeaturedModDeploymentTaskTest.class.getResourceAsStream("/featured_mod/mod_info.lua"),
+          repoFolder.resolve("mod_info.lua")
+      );
+      return null;
+    }).when(gitWrapper).checkoutRef(any(), any());
+
+    when(featuredModService.getFileIds("faf")).thenReturn(Collections.emptyMap());
+
+    Path dummyExe = repositoriesFolder.getRoot().toPath().resolve("TemplateForgedAlliance.exe");
+    createDummyExe(dummyExe);
+    properties.getDeployment().setForgedAllianceExePath(dummyExe.toAbsolutePath().toString());
+
+    instance.run();
+
+    verify(featuredModService, never()).save(anyString(), anyShort(), any());
+  }
+
   @Test
   @SuppressWarnings("unchecked")
   public void testRun() throws Exception {
@@ -95,7 +129,7 @@ public void testRun() throws Exception {
 
     instance.run();
 
-    ArgumentCaptor<List<FeaturedModFile>> filesCaptor = ArgumentCaptor.forClass((Class) List.class);
+    ArgumentCaptor<List<FeaturedModFile>> filesCaptor = ArgumentCaptor.forClass(List.class);
     verify(featuredModService).save(eq("faf"), eq((short) 1337), filesCaptor.capture());
 
     List<FeaturedModFile> files = filesCaptor.getValue();

src/test/java/com/faforever/api/map/MapServiceTest.java

@@ -8,7 +8,7 @@
 import com.faforever.api.error.ApiException;
 import com.faforever.api.error.ApiExceptionWithMultipleCodes;
 import com.faforever.api.error.ErrorCode;
-import com.faforever.commons.zip.Unzipper;
+import com.faforever.commons.io.Unzipper;
 import com.google.common.io.ByteStreams;
 import com.googlecode.zohhak.api.TestWith;
 import com.googlecode.zohhak.api.runners.ZohhakRunner;