diff --git a/build.gradle b/build.gradle index 7c812e1c6..4e3673abd 100644 --- a/build.gradle +++ b/build.gradle @@ -204,6 +204,14 @@ dependencyManagement { } } +test { + useJUnitPlatform { + } + testLogging { + events("passed", "skipped", "failed") + } +} + dependencies { compileOnly("org.projectlombok:lombok:${lombokVersion}") annotationProcessor("org.projectlombok:lombok:${lombokVersion}") @@ -251,7 +259,6 @@ dependencies { compile("org.luaj:luaj-jse:${luajVersion}") compile("com.github.micheljung:nocatch:${nocatchVersion}") compile("junit-addons:junit-addons:${junitAddonsVersion}") - 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}") @@ -265,9 +272,17 @@ dependencies { testCompile("org.springframework.boot:spring-boot-starter-test") testCompile("org.springframework.restdocs:spring-restdocs-mockmvc") testCompile("org.springframework.security:spring-security-test") + testImplementation("junit:junit") + testImplementation("org.junit.jupiter:junit-jupiter:${jUnit5Version}") + testImplementation("org.junit.jupiter:junit-jupiter-params:${jUnit5Version}") + testImplementation("org.mockito:mockito-junit-jupiter:${mockitoVersion}") + testRuntimeOnly("org.junit.vintage:junit-vintage-engine:${jUnit5Version}") testCompile("com.h2database:h2:${h2Version}") testCompile("com.jayway.jsonpath:json-path:${jsonPath}") testCompile("com.jayway.jsonpath:json-path-assert:${jsonPathAssert}") codacy("com.github.codacy:codacy-coverage-reporter:${codacyCoverageReporterVersion}") } + +ext["mockito.version"] = "${mockitoVersion}" +ext["junit-jupiter.version"] = "${jUnit5Version}" diff --git a/gradle.properties b/gradle.properties index 19a3f6ae8..61075decd 100644 --- a/gradle.properties +++ b/gradle.properties @@ -16,13 +16,12 @@ springBootAdminClientVersion=2.0.6 luajVersion=3.0.1 nocatchVersion=1.1 junitAddonsVersion=1.4 -zohhakVersion=1.1.1 githubApiVersion=1.84 jgitVersionn=4.5.0.201609210915-r fafCommonsVersion=8860b91752f6b5713926036dbb4740197d27211b h2Version=1.4.193 jacksonDatatypeJsr310Version=2.9.7 -mockitoVersion=2.19.0 +mockitoVersion=2.23.4 lutungVersion=0.0.7 commonsCompressVersion=1.13 jsonPath=2.2.0 @@ -32,3 +31,4 @@ codacyCoverageReporterVersion=4.0.0 jsonVersion=20180813 javaxInterceptorApiVersion=1.2 lombokVersion=1.18.4 +jUnit5Version=5.5.1 diff --git a/src/main/java/com/faforever/api/config/security/WebSecurityConfig.java b/src/main/java/com/faforever/api/config/security/WebSecurityConfig.java index b6cf75d40..802e16b36 100644 --- a/src/main/java/com/faforever/api/config/security/WebSecurityConfig.java +++ b/src/main/java/com/faforever/api/config/security/WebSecurityConfig.java @@ -24,7 +24,6 @@ import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.web.servlet.config.annotation.CorsRegistry; import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; -import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; @@ -91,7 +90,7 @@ public boolean matches(HttpServletRequest request) { @Bean public WebMvcConfigurer corsConfigurer() { - return new WebMvcConfigurerAdapter() { + return new WebMvcConfigurer() { @Override public void addCorsMappings(CorsRegistry registry) { registry.addMapping("/**") diff --git a/src/main/java/com/faforever/api/error/ApiException.java b/src/main/java/com/faforever/api/error/ApiException.java index 4aa304289..5f4fdc113 100644 --- a/src/main/java/com/faforever/api/error/ApiException.java +++ b/src/main/java/com/faforever/api/error/ApiException.java @@ -15,8 +15,12 @@ public ApiException(Error error) { this(new Error[]{error}); } - public ApiException(Error[] errors) { + public ApiException(Error... errors) { super(Arrays.toString(errors)); this.errors = errors; } + + public static ApiException of(ErrorCode errorCode, Object... args) { + return new ApiException(new Error(errorCode, args)); + } } diff --git a/src/main/java/com/faforever/api/error/ErrorCode.java b/src/main/java/com/faforever/api/error/ErrorCode.java index e66c73569..bd1d0963f 100644 --- a/src/main/java/com/faforever/api/error/ErrorCode.java +++ b/src/main/java/com/faforever/api/error/ErrorCode.java @@ -90,9 +90,12 @@ public enum ErrorCode { VOTING_SUBJECT_DOES_NOT_EXIST(180, "Voting subject does not exist", "There is no voting subject with the ID ''{0}''."), VOTING_CHOICE_DOES_NOT_EXIST(181, "Invalid choice", "There is no voting choice with the ID ''{0}''."), STEAM_ID_ALREADY_LINKED(182, " Steam account already linked to a FAF account", "You linked this account already to user with name ''{0}''."), - MAP_NAME_INVALID(183, "Map name invalid", "The name of the map in the scenario file can only contain printable ASCII characters and blanks."), - MOD_NAME_INVALID(184, "Mod name invalid", "The name of the mod in the scenario file can only contain printable ASCII characters and blanks."); - + MAP_NAME_INVALID_CHARACTER(183, "Map name invalid", "Only latin characters, numbers, blanks and the minus are allowed."), + MOD_NAME_INVALID(184, "Mod name invalid", "The name of the mod in the scenario file can only contain printable ASCII characters and blanks."), + MAP_NAME_INVALID_MINUS_OCCURENCE(185, "Map name invalid", "Only a maximum of {0} minus characters are allowed."), + MAP_SCRIPT_LINE_MISSING(186, "Missing scenario.lua line", "The scenario.lua has to contain the following line: {0}"), + MAP_NAME_TOO_SHORT(187, "Map name invalid", "The map name must have at least {0, number} characters, was: {1, number}"), + MAP_NAME_DOES_NOT_START_WITH_LETTER(188, "Map name invalid", "The map name has to begin with a letter"); private final int code; private final String title; diff --git a/src/main/java/com/faforever/api/map/MapService.java b/src/main/java/com/faforever/api/map/MapService.java index 15c3630d8..f14b290a4 100644 --- a/src/main/java/com/faforever/api/map/MapService.java +++ b/src/main/java/com/faforever/api/map/MapService.java @@ -15,21 +15,24 @@ import com.faforever.commons.io.Zipper; import com.faforever.commons.lua.LuaLoader; import com.faforever.commons.map.PreviewGenerator; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableMap; +import lombok.AllArgsConstructor; import lombok.Data; import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.compress.archivers.ArchiveException; import org.luaj.vm2.LuaValue; import org.springframework.cache.annotation.CacheEvict; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.Assert; import org.springframework.util.FileSystemUtils; +import org.springframework.util.StringUtils; import javax.imageio.ImageIO; -import javax.inject.Inject; import java.awt.image.BufferedImage; -import java.io.BufferedInputStream; -import java.io.BufferedOutputStream; +import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -37,35 +40,111 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; +import java.util.Map.Entry; import java.util.Optional; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; -import java.util.zip.ZipInputStream; -import java.util.zip.ZipOutputStream; import static com.github.nocatch.NoCatch.noCatch; +import static java.text.MessageFormat.format; @Service @Slf4j +@AllArgsConstructor public class MapService { + private static final Pattern MAP_NAME_INVALID_CHARACTER_PATTERN = Pattern.compile("[a-zA-Z0-9\\- ]+"); + private static final Pattern MAP_NAME_DOES_NOT_START_WITH_LETTER_PATTERN = Pattern.compile("^[^a-zA-Z]+"); + private static final Pattern ADAPTIVE_MAP_PATTERN = Pattern.compile("AdaptiveMap\\w=\\wtrue"); + private static final int MAP_NAME_MINUS_MAX_OCCURENCE = 3; + private static final int MAP_NAME_MIN_LENGTH = 4; + private static final int MAP_NAME_MAX_LENGTH = 50; + private static final String[] REQUIRED_FILES = new String[]{ ".scmap", "_save.lua", "_scenario.lua", - "_script.lua"}; + "_script.lua", + }; + + private static final String[] ADAPTIVE_REQUIRED_FILES = new String[]{ + "_options.lua", + "_tables.lua", + }; + private static final Charset MAP_CHARSET = StandardCharsets.ISO_8859_1; private static final String STUPID_MAP_FOLDER_PREFIX = "maps/"; - public static final int MAP_DISPLAY_NAME_MAX_LENGTH = 100; + private static final int MAP_DISPLAY_NAME_MAX_LENGTH = 100; private final FafApiProperties fafApiProperties; private final MapRepository mapRepository; private final ContentService contentService; - @Inject - public MapService(FafApiProperties fafApiProperties, MapRepository mapRepository, ContentService contentService) { - this.fafApiProperties = fafApiProperties; - this.mapRepository = mapRepository; - this.contentService = contentService; + @VisibleForTesting + void validate(MapValidationRequest mapValidationRequest) { + String mapName = mapValidationRequest.getName(); + Assert.notNull(mapName, "The map name is mandatory."); + String scenarioLua = mapValidationRequest.getScenarioLua(); + + Collection mapNameErrors = validateMapName(mapName); + if (!mapNameErrors.isEmpty()) { + throw new ApiException(mapNameErrors.toArray(new Error[0])); + } + + if (scenarioLua != null) { + Collection scenarioLuaErrors = validateScenarioLua(mapName, scenarioLua); + if (!scenarioLuaErrors.isEmpty()) { + throw new ApiException(scenarioLuaErrors.toArray(new Error[0])); + } + } + } + + private Collection validateMapName(String mapName) { + List errors = new ArrayList<>(); + + if (!MAP_NAME_INVALID_CHARACTER_PATTERN.matcher(mapName).matches()) { + errors.add(new Error(ErrorCode.MAP_NAME_INVALID_CHARACTER)); + } + + if (mapName.length() < MAP_NAME_MIN_LENGTH) { + errors.add(new Error(ErrorCode.MAP_NAME_TOO_SHORT, MAP_NAME_MIN_LENGTH, mapName.length())); + } + + if (mapName.length() > MAP_NAME_MAX_LENGTH) { + errors.add(new Error(ErrorCode.MAP_NAME_TOO_LONG, MAP_NAME_MAX_LENGTH, mapName.length())); + } + + if (StringUtils.countOccurrencesOf(mapName, "-") > MAP_NAME_MINUS_MAX_OCCURENCE) { + errors.add(new Error(ErrorCode.MAP_NAME_INVALID_MINUS_OCCURENCE, MAP_NAME_MINUS_MAX_OCCURENCE)); + } + + if (MAP_NAME_DOES_NOT_START_WITH_LETTER_PATTERN.matcher(mapName).find()) { + errors.add(new Error(ErrorCode.MAP_NAME_DOES_NOT_START_WITH_LETTER)); + } + + return errors; + } + + private Collection validateScenarioLua(String validMapName, String scenarioLua) { + List errors = new ArrayList<>(); + + java.util.Map declarations = ImmutableMap.builder() + .put("map", ".scmap") + .put("save", "_save.lua") + .put("script", "_script.lua") + .build(); + + for (Entry entry : declarations.entrySet()) { + Pattern pattern = Pattern.compile(format("{0}\\s=\\s''\\/maps\\/{2}(\\.v\\d{4})?\\/{2}{1}'',", entry.getKey(), entry.getValue(), validMapName)); + if (!pattern.matcher(scenarioLua).find()) { + errors.add(new Error(ErrorCode.MAP_SCRIPT_LINE_MISSING, format( + "{0} = ''/maps/{2}/{2}{1}'',", entry.getKey(), entry.getValue(), validMapName + ))); + } + } + + return errors; } @Transactional @@ -103,20 +182,17 @@ public void uploadMap(byte[] mapData, String mapFilename, Player author, boolean } } - @SneakyThrows - private Path copyToTemporaryDirectory(byte[] mapData, MapUploadData progressData) { + private Path copyToTemporaryDirectory(byte[] mapData, MapUploadData progressData) throws IOException { return Files.write(progressData.getUploadedFile(), mapData); } - @SneakyThrows - private void unzipFile(MapUploadData mapData) { - Unzipper.from(mapData.getUploadedFile()) - .to(mapData.getBaseDir()) - .unzip(); + private void unzipFile(MapUploadData mapData) throws IOException, ArchiveException { + Unzipper.from(mapData.getUploadedFile()) + .to(mapData.getBaseDir()) + .unzip(); } - @SneakyThrows - private void postProcessZipFiles(MapUploadData mapUploadData) { + private void postProcessZipFiles(MapUploadData mapUploadData) throws IOException { Optional mapFolder; try (Stream mapFolderStream = Files.list(mapUploadData.getBaseDir())) { mapFolder = mapFolderStream @@ -150,8 +226,7 @@ private void postProcessZipFiles(MapUploadData mapUploadData) { } } - @SneakyThrows - private void parseScenarioLua(MapUploadData progressData) { + private void parseScenarioLua(MapUploadData progressData) throws IOException { try (Stream mapFilesStream = Files.list(progressData.getOriginalMapFolder())) { Path scenarioLuaPath = noCatch(() -> mapFilesStream) .filter(myFile -> myFile.toString().endsWith("_scenario.lua")) @@ -173,7 +248,7 @@ private void checkLua(MapUploadData progressData) { throw new ApiException(new Error(ErrorCode.MAP_NAME_TOO_LONG, MAP_DISPLAY_NAME_MAX_LENGTH, displayName.length())); } if (!NameUtil.isPrintableAsciiString(displayName)) { - throw new ApiException(new Error(ErrorCode.MAP_NAME_INVALID)); + throw new ApiException(new Error(ErrorCode.MAP_NAME_INVALID_CHARACTER)); } if (scenarioInfo.get(ScenarioMapInfo.DESCRIPTION) == LuaValue.NIL) { @@ -288,15 +363,13 @@ private void updateMapEntities(MapUploadData progressData) { mapRepository.save(map); } - @SneakyThrows - private void renameFolderNameAndCorrectPathInLuaFiles(MapUploadData progressData) { + private void renameFolderNameAndCorrectPathInLuaFiles(MapUploadData progressData) throws IOException { progressData.setNewMapFolder(progressData.getBaseDir().resolve(progressData.getNewFolderName())); Files.move(progressData.getOriginalMapFolder(), progressData.getNewMapFolder()); updateLuaFiles(progressData); } - @SneakyThrows - private void updateLuaFiles(MapUploadData mapData) { + private void updateLuaFiles(MapUploadData mapData) throws IOException { String oldNameFolder = "/maps/" + mapData.getUploadFolderName(); String newNameFolder = "/maps/" + mapData.getNewFolderName(); try (Stream mapFileStream = Files.list(mapData.getNewMapFolder())) { @@ -311,9 +384,7 @@ private void updateLuaFiles(MapUploadData mapData) { } } - - @SneakyThrows - private void generatePreview(MapUploadData mapData) { + private void generatePreview(MapUploadData mapData) throws IOException { String previewFilename = mapData.getNewFolderName() + ".png"; generateImage( fafApiProperties.getMap().getDirectoryPreviewPathSmall().resolve(previewFilename), @@ -326,8 +397,7 @@ private void generatePreview(MapUploadData mapData) { fafApiProperties.getMap().getPreviewSizeLarge()); } - @SneakyThrows - private void zipMapData(MapUploadData progressData) { + private void zipMapData(MapUploadData progressData) throws IOException, ArchiveException { cleanupBaseDir(progressData); Path finalZipFile = progressData.getFinalZipFile(); Files.createDirectories(finalZipFile.getParent(), FilePermissionUtil.directoryPermissionFileAttributes()); @@ -339,8 +409,7 @@ private void zipMapData(MapUploadData progressData) { FilePermissionUtil.setDefaultFilePermission(finalZipFile); } - @SneakyThrows - private void cleanupBaseDir(MapUploadData progressData) { + private void cleanupBaseDir(MapUploadData progressData) throws IOException { Files.delete(progressData.getUploadedFile()); try (Stream stream = Files.list(progressData.getBaseDir())) { if (stream.count() != 1) { @@ -349,8 +418,7 @@ private void cleanupBaseDir(MapUploadData progressData) { } } - @SneakyThrows - private void generateImage(Path target, Path baseDir, int size) { + private void generateImage(Path target, Path baseDir, int size) throws IOException { BufferedImage image = PreviewGenerator.generatePreview(baseDir, size, size); if (target.getNameCount() > 0) { Files.createDirectories(target.getParent(), FilePermissionUtil.directoryPermissionFileAttributes()); diff --git a/src/main/java/com/faforever/api/map/MapValidationRequest.java b/src/main/java/com/faforever/api/map/MapValidationRequest.java new file mode 100644 index 000000000..90102dacc --- /dev/null +++ b/src/main/java/com/faforever/api/map/MapValidationRequest.java @@ -0,0 +1,9 @@ +package com.faforever.api.map; + +import lombok.Value; + +@Value +public class MapValidationRequest { + String name; + String scenarioLua; +} diff --git a/src/main/java/com/faforever/api/map/MapsController.java b/src/main/java/com/faforever/api/map/MapsController.java index 9d996172b..21109eea5 100644 --- a/src/main/java/com/faforever/api/map/MapsController.java +++ b/src/main/java/com/faforever/api/map/MapsController.java @@ -12,34 +12,51 @@ import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiResponse; import io.swagger.annotations.ApiResponses; +import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.springframework.http.MediaType; import org.springframework.security.core.Authentication; +import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.multipart.MultipartFile; +import org.springframework.web.servlet.ModelAndView; -import javax.inject.Inject; import java.io.IOException; +import java.util.Map; import static org.springframework.http.MediaType.APPLICATION_JSON_UTF8_VALUE; @RestController @RequestMapping(path = "/maps") @Slf4j +@AllArgsConstructor public class MapsController { private final MapService mapService; private final FafApiProperties fafApiProperties; private final ObjectMapper objectMapper; private final PlayerService playerService; - @Inject - public MapsController(MapService mapService, FafApiProperties fafApiProperties, ObjectMapper objectMapper, PlayerService playerService) { - this.mapService = mapService; - this.fafApiProperties = fafApiProperties; - this.objectMapper = objectMapper; - this.playerService = playerService; + + @RequestMapping(path = "/validate", method = RequestMethod.GET, produces = APPLICATION_JSON_UTF8_VALUE) + public ModelAndView showValidationForm(Map model) { + return new ModelAndView("validate_map_metadata.html"); + } + + @ApiOperation("Validate map metadata") + @ApiResponses(value = { + @ApiResponse(code = 200, message = "Success"), + @ApiResponse(code = 422, message = "Containing information about the errors in the payload")}) + @RequestMapping( + path = "/validate", + method = RequestMethod.POST, + produces = APPLICATION_JSON_UTF8_VALUE, + consumes = MediaType.APPLICATION_JSON_VALUE + ) + public void validateMapMetadata(@RequestBody MapValidationRequest mapValidationRequest) { + mapService.validate(mapValidationRequest); } @ApiOperation("Upload a map") diff --git a/src/main/resources/config/application-dev.yml b/src/main/resources/config/application-dev.yml index 7c405ce76..c6ccaba5e 100644 --- a/src/main/resources/config/application-dev.yml +++ b/src/main/resources/config/application-dev.yml @@ -71,4 +71,5 @@ spring: password: ${ADMIN_CLIENT_PASSWORD:banana} logging: level: - com.faforever.api: trace + com.faforever.api: debug + diff --git a/src/main/resources/templates/login.html b/src/main/resources/templates/login.html index de6681ed1..5d69605bc 100644 --- a/src/main/resources/templates/login.html +++ b/src/main/resources/templates/login.html @@ -3,9 +3,7 @@ Log-in - - - + + + + + + + diff --git a/src/test/java/com/faforever/api/map/MapServiceTest.java b/src/test/java/com/faforever/api/map/MapServiceTest.java index a5eec1518..28904d07e 100644 --- a/src/test/java/com/faforever/api/map/MapServiceTest.java +++ b/src/test/java/com/faforever/api/map/MapServiceTest.java @@ -7,246 +7,325 @@ import com.faforever.api.data.domain.Player; import com.faforever.api.error.ApiException; import com.faforever.api.error.ApiExceptionWithMultipleCodes; +import com.faforever.api.error.Error; import com.faforever.api.error.ErrorCode; 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; import junitx.framework.FileAssert; -import org.junit.After; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.junit.rules.TemporaryFolder; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; +import org.mockito.Mock; import org.mockito.Mockito; -import org.springframework.util.FileSystemUtils; +import org.mockito.junit.jupiter.MockitoExtension; +import org.thymeleaf.util.StringUtils; -import java.io.BufferedInputStream; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import java.util.stream.Stream; -import java.util.zip.ZipInputStream; import static com.faforever.api.error.ApiExceptionWithCode.apiExceptionWithCode; +import static com.faforever.api.error.ErrorCode.MAP_NAME_DOES_NOT_START_WITH_LETTER; +import static com.faforever.api.error.ErrorCode.MAP_NAME_INVALID_CHARACTER; +import static com.faforever.api.error.ErrorCode.MAP_NAME_INVALID_MINUS_OCCURENCE; +import static com.faforever.api.error.ErrorCode.MAP_NAME_TOO_LONG; +import static com.faforever.api.error.ErrorCode.MAP_NAME_TOO_SHORT; +import static com.faforever.api.error.ErrorCode.MAP_SCRIPT_LINE_MISSING; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.mock; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -@RunWith(ZohhakRunner.class) +@ExtendWith(MockitoExtension.class) public class MapServiceTest { - @Rule - public final TemporaryFolder temporaryDirectory = new TemporaryFolder(); - @Rule - public final TemporaryFolder finalDirectory = new TemporaryFolder(); - @Rule - public final ExpectedException expectedException = ExpectedException.none(); - private final MapRepository mapRepository = mock(MapRepository.class); - private final FafApiProperties fafApiProperties = mock(FafApiProperties.class); - private final ContentService contentService = mock(ContentService.class); - private final Player author = mock(Player.class); + private Path temporaryDirectory; + private Path finalDirectory; + + @Mock + private MapRepository mapRepository; + @Mock + private FafApiProperties fafApiProperties; + @Mock + private ContentService contentService; + @Mock + private Player author; + private MapService instance; private Map mapProperties; - @Before - public void setUp() { + @BeforeEach + void beforeEach() { instance = new MapService(fafApiProperties, mapRepository, contentService); - mapProperties = new Map() - .setTargetDirectory(finalDirectory.getRoot().toPath()) - .setDirectoryPreviewPathLarge(finalDirectory.getRoot().toPath().resolve("large")) - .setDirectoryPreviewPathSmall(finalDirectory.getRoot().toPath().resolve("small")); - when(fafApiProperties.getMap()).thenReturn(mapProperties); - when(contentService.createTempDir()).thenReturn(temporaryDirectory.getRoot().toPath()); } - @After - public void shutDown() { - if (Files.exists(temporaryDirectory.getRoot().toPath())) { - FileSystemUtils.deleteRecursively(temporaryDirectory.getRoot()); + @Nested + class Validation { + + @ParameterizedTest + @ValueSource(strings = { + "Map1", + "map-2", + "A very but overall not really too long map name", + "Three - dashes - are - allowed" + }) + void testMapNameValid(String name) { + MapValidationRequest request = new MapValidationRequest(name, null); + instance.validate(request); } - if (Files.exists(temporaryDirectory.getRoot().toPath())) { - FileSystemUtils.deleteRecursively(finalDirectory.getRoot()); + + @Test + void testMapNameMultiErrorsButNoScenarioValidation() { + MapValidationRequest request = new MapValidationRequest("123Invalid-in$-many-ways-atOnce" + StringUtils.repeat("x", 50), ""); + ApiException result = assertThrows(ApiException.class, () -> instance.validate(request)); + List errorCodes = Arrays.stream(result.getErrors()).map(Error::getErrorCode).collect(Collectors.toList()); + assertThat(errorCodes, hasItems(MAP_NAME_INVALID_CHARACTER, MAP_NAME_TOO_LONG, MAP_NAME_INVALID_MINUS_OCCURENCE)); + assertThat(errorCodes, not(contains(MAP_SCRIPT_LINE_MISSING))); } - } - @Test - public void zipFilenamealreadyExists() throws IOException { - Path clashedMap = finalDirectory.getRoot().toPath().resolve("sludge_test____.___..v0001.zip"); - assertTrue(clashedMap.toFile().createNewFile()); - String zipFilename = "scmp_037.zip"; - when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.empty()); - try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) { - try { - byte[] mapData = ByteStreams.toByteArray(inputStream); - instance.uploadMap(mapData, zipFilename, author, true); - fail(); - } catch (ApiException e) { - assertThat(e, apiExceptionWithCode(ErrorCode.MAP_NAME_CONFLICT)); - } - verify(mapRepository, never()).save(any(com.faforever.api.data.domain.Map.class)); + @ParameterizedTest + @ValueSource(strings = { + "Map.With.Dots", + "Map/With/Slashes", + "Map,With,Commas", + "Map|With|Pipes", + "SomeMore:", + "SomeMore(", + "SomeMore)", + "SomeMore[", + "SomeMore]", + "SomeMore?", + "SomeMore$", + }) + void testMapNameInvalidChars(String name) { + MapValidationRequest request = new MapValidationRequest(name, null); + ApiException result = assertThrows(ApiException.class, () -> instance.validate(request)); + assertThat(result, apiExceptionWithCode(MAP_NAME_INVALID_CHARACTER)); } - } - @Test - public void emptyZip() throws IOException { - String zipFilename = "empty.zip"; - try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) { - try { - byte[] mapData = ByteStreams.toByteArray(inputStream); - instance.uploadMap(mapData, zipFilename, author, true); - fail(); - } catch (ApiException e) { - assertThat(e, apiExceptionWithCode(ErrorCode.MAP_MISSING_MAP_FOLDER_INSIDE_ZIP)); - } - verify(mapRepository, never()).save(any(com.faforever.api.data.domain.Map.class)); + @Test + void testMapNameTooManyDashes() { + MapValidationRequest request = new MapValidationRequest("More-than-three-dashes-invalid", null); + ApiException result = assertThrows(ApiException.class, () -> instance.validate(request)); + assertThat(result, apiExceptionWithCode(MAP_NAME_INVALID_MINUS_OCCURENCE)); } - } - @Test - public void notCorrectAuthor() throws IOException { - String zipFilename = "scmp_037.zip"; - - Player me = new Player(); - me.setId(1); - Player bob = new Player(); - bob.setId(2); - - com.faforever.api.data.domain.Map map = new com.faforever.api.data.domain.Map().setAuthor(bob); - when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.of(map)); - try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) { - try { - byte[] mapData = ByteStreams.toByteArray(inputStream); - instance.uploadMap(mapData, zipFilename, me, true); - fail(); - } catch (ApiException e) { - assertThat(e, apiExceptionWithCode(ErrorCode.MAP_NOT_ORIGINAL_AUTHOR)); - } - verify(mapRepository, never()).save(any(com.faforever.api.data.domain.Map.class)); + @Test + void testMapNameToShort() { + MapValidationRequest request = new MapValidationRequest("x", null); + ApiException result = assertThrows(ApiException.class, () -> instance.validate(request)); + assertThat(result, apiExceptionWithCode(MAP_NAME_TOO_SHORT)); } - } - @Test - public void versionExistsAlready() throws IOException { - String zipFilename = "scmp_037.zip"; - - Player me = new Player(); - me.setId(1); - - com.faforever.api.data.domain.Map map = new com.faforever.api.data.domain.Map() - .setAuthor(me) - .setVersions(Collections.singletonList(new MapVersion().setVersion(1))); - - when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.of(map)); - try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) { - try { - byte[] mapData = ByteStreams.toByteArray(inputStream); - instance.uploadMap(mapData, zipFilename, me, true); - fail(); - } catch (ApiException e) { - assertThat(e, apiExceptionWithCode(ErrorCode.MAP_VERSION_EXISTS)); - } - verify(mapRepository, never()).save(any(com.faforever.api.data.domain.Map.class)); + @Test + void testMapNameToLong() { + MapValidationRequest request = new MapValidationRequest(StringUtils.repeat("x", 51), null); + ApiException result = assertThrows(ApiException.class, () -> instance.validate(request)); + assertThat(result, apiExceptionWithCode(MAP_NAME_TOO_LONG)); + } + + @Test + void testMapNameStartsInvalid() { + MapValidationRequest request = new MapValidationRequest("123x", null); + ApiException result = assertThrows(ApiException.class, () -> instance.validate(request)); + assertThat(result, apiExceptionWithCode(MAP_NAME_DOES_NOT_START_WITH_LETTER)); + } + + @Test + void testScenarioLuaSuccessWithoutVersion() { + MapValidationRequest request = new MapValidationRequest("name", + "map = '/maps/name/name.scmap', " + + "save = '/maps/name/name_save.lua', " + + "script = '/maps/name/name_script.lua', " + ); + + instance.validate(request); + } + + @Test + void testScenarioLuaSuccessWithVersion() { + // of course the version number should be the same every time but we don't validate this + MapValidationRequest request = new MapValidationRequest("name", + "map = '/maps/name.v0001/name.scmap', " + + "save = '/maps/name.v0002/name_save.lua', " + + "script = '/maps/name.v0003/name_script.lua', " + ); + + instance.validate(request); + } + + @Test + void testScenarioLuaMissingAllLines() { + MapValidationRequest request = new MapValidationRequest("name", ""); + ApiException result = assertThrows(ApiException.class, () -> instance.validate(request)); + assertThat(result.getErrors().length, is(3)); + } + + @Test + void testScenarioLuaWrongMapLine() { + MapValidationRequest request = new MapValidationRequest("name", + "map = '/maps/name.vINVALID_VERSION/name.scmap', " + + "save = '/maps/name.v0002/name_save.lua', " + + "script = '/maps/name.v0003/name_script.lua', " + ); + + ApiException result = assertThrows(ApiException.class, () -> instance.validate(request)); + + assertThat(result.getErrors().length, is(1)); + assertThat(result.getErrors()[0].getArgs().length, is(1)); + assertThat(result.getErrors()[0].getArgs()[0], is("map = '/maps/name/name.scmap',")); + } + + @Test + void testScenarioLuaWrongSaveLine() { + MapValidationRequest request = new MapValidationRequest("name", + "map = '/maps/name.v0001/name.scmap', " + + "save = '/maps/name.vINVALID_VERSION/name_save.lua', " + + "script = '/maps/name.v0003/name_script.lua', " + ); + + ApiException result = assertThrows(ApiException.class, () -> instance.validate(request)); + + assertThat(result.getErrors().length, is(1)); + assertThat(result.getErrors()[0].getArgs().length, is(1)); + assertThat(result.getErrors()[0].getArgs()[0], is("save = '/maps/name/name_save.lua',")); + } + + @Test + void testScenarioLuaWrongScriptLine() { + MapValidationRequest request = new MapValidationRequest("name", + "map = '/maps/name.v0001/name.scmap', " + + "save = '/maps/name.v0002/name_save.lua', " + + "script = '/maps/name.vINVALID_VERSION/name_script.lua', " + ); + + ApiException result = assertThrows(ApiException.class, () -> instance.validate(request)); + + assertThat(result.getErrors().length, is(1)); + assertThat(result.getErrors()[0].getArgs().length, is(1)); + assertThat(result.getErrors()[0].getArgs()[0], is("script = '/maps/name/name_script.lua',")); } } - @TestWith({"without_savelua.zip", "without_scenariolua.zip", "without_scmap.zip", "without_scriptlua.zip"}) - public void fileIsMissingInsideZip(String zipFilename) throws IOException { - try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) { - try { - byte[] mapData = ByteStreams.toByteArray(inputStream); - instance.uploadMap(mapData, zipFilename, author, true); - fail(); - } catch (ApiException e) { - assertThat(e, apiExceptionWithCode(ErrorCode.MAP_FILE_INSIDE_ZIP_MISSING)); - } + @Nested + class WithTempDir { + @TempDir + Path baseTemporaryDirectory; + + @BeforeEach + void setUp() throws Exception { + temporaryDirectory = Files.createDirectory(baseTemporaryDirectory.resolve("temp")); + finalDirectory = Files.createDirectory(baseTemporaryDirectory.resolve("final")); + + mapProperties = new Map() + .setTargetDirectory(finalDirectory) + .setDirectoryPreviewPathLarge(finalDirectory.resolve("large")) + .setDirectoryPreviewPathSmall(finalDirectory.resolve("small")); + when(contentService.createTempDir()).thenReturn(temporaryDirectory); + } + + @ParameterizedTest(name = "Expecting ErrorCode.{0} with file ''{1}''") + @CsvSource(value = { + "MAP_MISSING_MAP_FOLDER_INSIDE_ZIP,empty.zip", + "MAP_FIRST_TEAM_FFA,wrong_team_name.zip", + "MAP_INVALID_ZIP,scmp_037_invalid.zip", // map with more than 1 root folders in zip + "MAP_NAME_INVALID_CHARACTER,scmp_037_no_ascii.zip", + "MAP_FILE_INSIDE_ZIP_MISSING,without_savelua.zip", + "MAP_FILE_INSIDE_ZIP_MISSING,without_scenariolua.zip", + "MAP_FILE_INSIDE_ZIP_MISSING,without_scmap.zip", + "MAP_FILE_INSIDE_ZIP_MISSING,without_scriptlua.zip", + }) + void uploadFails(String errorCodeEnumValue, String fileName) throws IOException { + uploadFails(ErrorCode.valueOf(errorCodeEnumValue), fileName); + } + + void uploadFails(ErrorCode expectedErrorCode, String fileName) throws IOException { + byte[] mapData = loadMapAsBytes(fileName); + ApiException result = assertThrows(ApiException.class, () -> instance.uploadMap(mapData, fileName, author, true)); + assertThat(result, apiExceptionWithCode(expectedErrorCode)); verify(mapRepository, never()).save(any(com.faforever.api.data.domain.Map.class)); } - } - @Test - public void battleTypeNotFFA() throws IOException { - String zipFilename = "wrong_team_name.zip"; - when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.empty()); - try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) { - byte[] mapData = ByteStreams.toByteArray(inputStream); - expectedException.expect(apiExceptionWithCode(ErrorCode.MAP_FIRST_TEAM_FFA)); - instance.uploadMap(mapData, zipFilename, author, true); + + @Test + void zipFilenameAlreadyExists() throws IOException { + Path clashedMap = finalDirectory.resolve("sludge_test____.___..v0001.zip"); + assertTrue(clashedMap.toFile().createNewFile()); + when(fafApiProperties.getMap()).thenReturn(mapProperties); + when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.empty()); + + uploadFails(ErrorCode.MAP_NAME_CONFLICT, "scmp_037.zip"); } - verify(mapRepository, never()).save(any(com.faforever.api.data.domain.Map.class)); - } - @Test - public void invalidScenario() throws IOException { - String zipFilename = "invalid_scenario.zip"; - when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.empty()); - try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) { - byte[] mapData = ByteStreams.toByteArray(inputStream); - expectedException.expect(ApiExceptionWithMultipleCodes.apiExceptionWithCode( + @Test + void notCorrectAuthor() throws IOException { + Player me = new Player(); + me.setId(1); + Player bob = new Player(); + bob.setId(2); + + com.faforever.api.data.domain.Map map = new com.faforever.api.data.domain.Map().setAuthor(bob); + when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.of(map)); + + uploadFails(ErrorCode.MAP_NOT_ORIGINAL_AUTHOR, "scmp_037.zip"); + } + + @Test + void versionExistsAlready() throws IOException { + com.faforever.api.data.domain.Map map = new com.faforever.api.data.domain.Map() + .setAuthor(author) + .setVersions(Collections.singletonList(new MapVersion().setVersion(1))); + + when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.of(map)); + when(author.getId()).thenReturn(1); + + uploadFails(ErrorCode.MAP_VERSION_EXISTS, "scmp_037.zip"); + } + + @Test + void invalidScenario() throws IOException { + String zipFilename = "invalid_scenario.zip"; + byte[] mapData = loadMapAsBytes(zipFilename); + ApiException result = assertThrows(ApiException.class, () -> instance.uploadMap(mapData, zipFilename, author, true)); + assertThat(result, is(ApiExceptionWithMultipleCodes.apiExceptionWithCode( ErrorCode.MAP_NAME_MISSING, ErrorCode.MAP_DESCRIPTION_MISSING, ErrorCode.MAP_FIRST_TEAM_FFA, ErrorCode.MAP_TYPE_MISSING, ErrorCode.MAP_SIZE_MISSING, - ErrorCode.MAP_VERSION_MISSING)); - instance.uploadMap(mapData, zipFilename, author, true); - } - verify(mapRepository, never()).save(any(com.faforever.api.data.domain.Map.class)); - } - - @Test - public void mapWithMoreRootFoldersInZip() throws IOException { - String zipFilename = "scmp_037_invalid.zip"; - when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.empty()); - try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) { - byte[] mapData = ByteStreams.toByteArray(inputStream); - try { - instance.uploadMap(mapData, zipFilename, author, true); - } catch (ApiException e) { - assertThat(e, apiExceptionWithCode(ErrorCode.MAP_INVALID_ZIP)); - } - } - } - - @Test - public void uploadMapWithInvalidCharactersName() throws IOException { - String zipFilename = "scmp_037_no_ascii.zip"; - when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.empty()); - try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) { - byte[] mapData = ByteStreams.toByteArray(inputStream); - - Path tmpDir = temporaryDirectory.getRoot().toPath(); - try { - instance.uploadMap(mapData, zipFilename, author, true); - } catch (ApiException e) { - assertThat(e, apiExceptionWithCode(ErrorCode.MAP_NAME_INVALID)); - } - + ErrorCode.MAP_VERSION_MISSING))); verify(mapRepository, never()).save(any(com.faforever.api.data.domain.Map.class)); } - } - @Test - public void positiveUploadTest() throws Exception { - String zipFilename = "scmp_037.zip"; - when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.empty()); - try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) { - byte[] mapData = ByteStreams.toByteArray(inputStream); + @Test + void positiveUploadTest() throws Exception { + String zipFilename = "scmp_037.zip"; + when(fafApiProperties.getMap()).thenReturn(mapProperties); + when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.empty()); + byte[] mapData = loadMapAsBytes(zipFilename); - Path tmpDir = temporaryDirectory.getRoot().toPath(); + Path tmpDir = temporaryDirectory; instance.uploadMap(mapData, zipFilename, author, true); ArgumentCaptor mapCaptor = ArgumentCaptor.forClass(com.faforever.api.data.domain.Map.class); @@ -266,16 +345,16 @@ public void positiveUploadTest() throws Exception { assertFalse(Files.exists(tmpDir)); - Path generatedFile = finalDirectory.getRoot().toPath().resolve("sludge_test____.___..v0001.zip"); + Path generatedFile = finalDirectory.resolve("sludge_test____.___..v0001.zip"); assertTrue(Files.exists(generatedFile)); - Path generatedFiles = finalDirectory.getRoot().toPath().resolve("generated_files"); + Path generatedFiles = finalDirectory.resolve("generated_files"); Unzipper.from(generatedFile).to(generatedFiles).unzip(); - Path expectedFiles = finalDirectory.getRoot().toPath().resolve("expected_files"); - Unzipper.from(generatedFile) - .to(expectedFiles) - .unzip(); + Path expectedFiles = finalDirectory.resolve("expected_files"); + Unzipper.from(generatedFile) + .to(expectedFiles) + .unzip(); expectedFiles = expectedFiles.resolve("sludge_test____.___..v0001"); try (Stream fileStream = Files.list(expectedFiles)) { @@ -294,9 +373,11 @@ public void positiveUploadTest() throws Exception { assertTrue(Files.exists(mapProperties.getDirectoryPreviewPathSmall().resolve("sludge_test____.___..v0001.png"))); } } - } - private InputStream loadMapResourceAsStream(String filename) { - return MapServiceTest.class.getResourceAsStream("/maps/" + filename); + private byte[] loadMapAsBytes(String filename) throws IOException { + try (InputStream inputStream = MapServiceTest.class.getResourceAsStream("/maps/" + filename)) { + return ByteStreams.toByteArray(inputStream); + } + } } }