Check for existing Mod UUID

Fixes #99

src/main/java/com/faforever/api/data/domain/ModVersion.java

@@ -32,7 +32,7 @@
 public class ModVersion {
 
   private Integer id;
-  private String uid;
+  private String uuid;
   private ModType type;
   private String description;
   private short version;
@@ -55,8 +55,8 @@ public Integer getId() {
   }
 
   @Column(name = "uid")
-  public String getUid() {
-    return uid;
+  public String getUuid() {
+    return uuid;
   }
 
   @Column(name = "type")

src/main/java/com/faforever/api/data/domain/Review.java

@@ -11,17 +11,13 @@
 import javax.persistence.Column;
 import javax.persistence.Entity;
 import javax.persistence.FetchType;
-import javax.persistence.GeneratedValue;
-import javax.persistence.GenerationType;
-import javax.persistence.Id;
 import javax.persistence.Inheritance;
 import javax.persistence.InheritanceType;
 import javax.persistence.JoinColumn;
 import javax.persistence.ManyToOne;
 import javax.persistence.Table;
 import javax.validation.constraints.DecimalMax;
 import javax.validation.constraints.DecimalMin;
-import java.sql.Timestamp;
 
 @Setter
 @Include(rootLevel = true, type = "review")
@@ -31,21 +27,11 @@
 @Inheritance(strategy = InheritanceType.JOINED)
 @CreatePermission(expression = "Prefab.Role.All")
 @DeletePermission(expression = IsReviewOwner.EXPRESSION)
-public class Review {
-  private Integer id;
+public class Review extends AbstractEntity {
   private String text;
   private Byte score;
-  private Timestamp createTime;
-  private Timestamp updateTime;
   private Player player;
 
-  @Id
-  @Column(name = "id")
-  @GeneratedValue(strategy = GenerationType.IDENTITY)
-  public Integer getId() {
-    return id;
-  }
-
   @Column(name = "text")
   @UpdatePermission(expression = IsReviewOwner.EXPRESSION)
   public String getText() {
@@ -60,16 +46,6 @@ public Byte getScore() {
     return score;
   }
 
-  @Column(name = "create_time")
-  public Timestamp getCreateTime() {
-    return createTime;
-  }
-
-  @Column(name = "update_time")
-  public Timestamp getUpdateTime() {
-    return updateTime;
-  }
-
   @ManyToOne(fetch = FetchType.LAZY)
   @JoinColumn(name = "user_id")
   @UpdatePermission(expression = "Prefab.Role.All and Prefab.Common.UpdateOnCreate")

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

@@ -30,8 +30,8 @@ public enum ErrorCode {
   @Deprecated
   MOD_UI_ONLY_MISSING(122, "Missing mod type", "The file mod_info.lua must contain a property 'ui_only'."),
   MOD_NAME_TOO_LONG(123, "Invalid mod name", "The mod name must not exceed {0} characters, was: {1}"),
-  MOD_NOT_ORIGINAL_AUTHOR(124, "Permission denied", "Only the original author is allowed to upload new versions of mod: {0}."),
-  MOD_VERSION_EXISTS(125, "Duplicate mod version", "Mod ''{0}'' with version ''{1}'' already exists."),
+  MOD_NOT_ORIGINAL_AUTHOR(124, "Permission denied", "Only the original author ''{0}'' is allowed to upload new versions of mod ''{1}''."),
+  MOD_VERSION_EXISTS(125, "Duplicate mod version", "A mod with name ''{0}'' and version ''{1}'' already exists."),
   MOD_AUTHOR_MISSING(126, "Missing mod author", "The file mod_info.lua must contain a property 'author'."),
   QUERY_INVALID_RATING_TYPE(127, "Invalid rating type", "Rating type is not valid: {0}. Please pick '1v1' or 'global'."),
   LOGIN_DENIED_BANNED(128, "Login denied", "You are currently banned: {0}"),
@@ -64,7 +64,8 @@ public enum ErrorCode {
   CLAN_GENERATE_LINK_PLAYER_NOT_FOUND(155, "Player not found", "Cannot find player with id ''{0}'' who should be invited to the clan"),
   CLAN_NAME_EXISTS(156, "Clan Name already in use", "The clan name ''{0}'' is already in use. Please choose a different clan name."),
   CLAN_TAG_EXISTS(157, "Clan Tag already in use", "The clan tag ''{0}'' is already in use. Please choose a different clan tag."),
-  VALIDATION_FAILED(158, "Validation failed", "{0}");
+  VALIDATION_FAILED(158, "Validation failed", "{0}"),
+  MOD_UUID_EXISTS(159, "Duplicate mod UUID", "A mod with UUID ''{0}'' already exists.");
 
   private final int code;
   private final String title;

src/main/java/com/faforever/api/mod/ModService.java

@@ -62,11 +62,18 @@ public void processUploadedMod(Path uploadedFile, Player uploader) {
     short version = (short) Integer.parseInt(modInfo.getVersion().toString());
 
     if (!canUploadMod(displayName, uploader)) {
-      throw new ApiException(new Error(ErrorCode.MOD_NOT_ORIGINAL_AUTHOR));
+      Mod mod = modRepository.findOneByDisplayName(displayName)
+        .orElseThrow(() -> new IllegalStateException("Mod could not be found"));
+      throw new ApiException(new Error(ErrorCode.MOD_NOT_ORIGINAL_AUTHOR, mod.getAuthor(), displayName));
     }
 
     if (modExists(displayName, version)) {
-      throw new ApiException(new Error(ErrorCode.MOD_VERSION_EXISTS));
+      throw new ApiException(new Error(ErrorCode.MOD_VERSION_EXISTS, displayName, version));
+    }
+
+    String uuid = modInfo.getUid();
+    if (modUuidExists(uuid)) {
+      throw new ApiException(new Error(ErrorCode.MOD_UUID_EXISTS, uuid));
     }
 
     String zipFileName = generateZipFileName(displayName, version);
@@ -99,9 +106,14 @@ private boolean modExists(String displayName, short version) {
       .setMod(new Mod()
         .setDisplayName(displayName)
       );
+
     return modVersionRepository.exists(Example.of(probe, ExampleMatcher.matching().withIgnoreCase()));
   }
 
+  private boolean modUuidExists(String uuid) {
+    return modVersionRepository.existsByUuid(uuid);
+  }
+
   private boolean canUploadMod(String displayName, Player uploader) {
     return !modRepository.existsByDisplayNameIgnoreCaseAndUploaderIsNot(displayName, uploader);
   }
@@ -179,7 +191,7 @@ private String generateFileName(String displayName) {
 
   private void store(com.faforever.commons.mod.Mod modInfo, Optional<Path> thumbnailPath, Player uploader, String zipFileName) {
     ModVersion modVersion = new ModVersion()
-      .setUid(modInfo.getUid())
+      .setUuid(modInfo.getUid())
       .setType(modInfo.isUiOnly() ? ModType.UI : ModType.SIM)
       .setDescription(modInfo.getDescription())
       .setVersion((short) Integer.parseInt(modInfo.getVersion().toString()))

src/main/java/com/faforever/api/mod/ModVersionRepository.java

@@ -6,4 +6,5 @@
 
 @Repository
 public interface ModVersionRepository extends JpaRepository<ModVersion, Integer> {
+  boolean existsByUuid(String uid);
 }

src/test/java/com/faforever/api/mod/ModServiceTest.java

@@ -4,19 +4,26 @@
 import com.faforever.api.data.domain.Mod;
 import com.faforever.api.data.domain.ModVersion;
 import com.faforever.api.data.domain.Player;
+import com.faforever.api.error.ApiExceptionWithCode;
+import com.faforever.api.error.ErrorCode;
+import org.jetbrains.annotations.NotNull;
 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.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.data.domain.Example;
 
 import java.io.BufferedInputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Optional;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.nullValue;
@@ -29,8 +36,11 @@
 public class ModServiceTest {
 
   private static final String TEST_MOD = "/mods/No Friendly Fire.zip";
+
   @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
 
   private ModService instance;
 
@@ -51,11 +61,9 @@ public void setUp() throws Exception {
   }
 
   @Test
+  @SuppressWarnings("unchecked")
   public void processUploadedMod() throws Exception {
-    Path uploadedFile = temporaryFolder.getRoot().toPath().resolve("uploaded-mod.zip");
-    try (InputStream inputStream = new BufferedInputStream(getClass().getResourceAsStream(TEST_MOD))) {
-      Files.copy(inputStream, uploadedFile);
-    }
+    Path uploadedFile = prepareMod();
 
     Player uploader = new Player();
 
@@ -78,11 +86,47 @@ public void processUploadedMod() throws Exception {
     assertThat(savedModVersion.getId(), is(nullValue()));
     assertThat(savedModVersion.getIcon(), is("no_friendly_fire.v0003.png"));
     assertThat(savedModVersion.getFilename(), is("mods/no_friendly_fire.v0003.zip"));
-    assertThat(savedModVersion.getUid(), is("26778D4E-BA75-5CC2-CBA8-63795BDE74AA"));
+    assertThat(savedModVersion.getUuid(), is("26778D4E-BA75-5CC2-CBA8-63795BDE74AA"));
     assertThat(savedModVersion.getDescription(), is("All friendly fire, including between allies, is turned off."));
     assertThat(savedModVersion.getMod(), is(savedMod));
     assertThat(savedModVersion.isRanked(), is(false));
     assertThat(savedModVersion.isHidden(), is(false));
+
+    ArgumentCaptor<Example<ModVersion>> exampleCaptor = ArgumentCaptor.forClass((Class) ModVersion.class);
+    verify(modVersionRepository).exists(exampleCaptor.capture());
+    verify(modVersionRepository).existsByUuid("26778D4E-BA75-5CC2-CBA8-63795BDE74AA");
+  }
+
+  @Test
+  public void testExistingUuid() throws Exception {
+    Path uploadedFile = prepareMod();
+
+    when(modVersionRepository.existsByUuid("26778D4E-BA75-5CC2-CBA8-63795BDE74AA")).thenReturn(true);
+    expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.MOD_UUID_EXISTS));
+
+    instance.processUploadedMod(uploadedFile, new Player());
+  }
+
+  @Test
+  public void testNotOriginalUploader() throws Exception {
+    Path uploadedFile = prepareMod();
+
+    Player uploader = new Player();
+    when(modRepository.existsByDisplayNameIgnoreCaseAndUploaderIsNot("No Friendly Fire", uploader)).thenReturn(true);
+    when(modRepository.findOneByDisplayName("No Friendly Fire")).thenReturn(Optional.of(new Mod()));
+
+    expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.MOD_NOT_ORIGINAL_AUTHOR));
+
+    instance.processUploadedMod(uploadedFile, uploader);
+  }
+
+  @NotNull
+  private Path prepareMod() throws IOException {
+    Path uploadedFile = temporaryFolder.getRoot().toPath().resolve("uploaded-mod.zip");
+    try (InputStream inputStream = new BufferedInputStream(getClass().getResourceAsStream(TEST_MOD))) {
+      Files.copy(inputStream, uploadedFile);
+    }
+    return uploadedFile;
   }
 
   // TODO test error cases