Refactor clan creation to be done via Elide POST call

+ other cleanups
FAForever · Nov 13, 2019 · fba6a91 · fba6a91
1 parent b02e3f3
commit fba6a91
Show file tree

Hide file tree

Showing 13 changed files with 331 additions and 216 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -25,7 +25,7 @@ before_install:
 install:
   - git clone https://github.com/FAForever/faf-stack.git faf-stack
     && pushd faf-stack
-    && git checkout 79c5d9d
+    && git checkout 13687d9
     && cp -r config.template config
     && cp .env.template .env
     && ./scripts/init-db.sh

diff --git a/src/inttest/java/com/faforever/api/clan/ClanControllerTest.java b/src/inttest/java/com/faforever/api/clan/ClanControllerTest.java
@@ -180,6 +180,6 @@ public void createSecondClan() throws Exception {
       .andExpect(status().isUnprocessableEntity())
       .andReturn();
 
-    assertApiError(result, ErrorCode.CLAN_CREATE_CREATOR_IS_IN_A_CLAN);
+    assertApiError(result, ErrorCode.CLAN_CREATE_FOUNDER_IS_IN_A_CLAN);
   }
 }
diff --git a/src/main/java/com/faforever/api/clan/ClanService.java b/src/main/java/com/faforever/api/clan/ClanService.java
@@ -10,43 +10,75 @@
 import com.faforever.api.error.ApiException;
 import com.faforever.api.error.Error;
 import com.faforever.api.error.ErrorCode;
-import com.faforever.api.error.ProgrammingError;
-import com.faforever.api.player.PlayerRepository;
 import com.faforever.api.player.PlayerService;
 import com.faforever.api.security.JwtService;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import lombok.RequiredArgsConstructor;
 import lombok.SneakyThrows;
-import org.springframework.security.core.Authentication;
 import org.springframework.security.jwt.Jwt;
 import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+import org.springframework.util.Assert;
 
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.Collections;
+import java.util.List;
 
 @Service
 @RequiredArgsConstructor
 public class ClanService {
 
   private final ClanRepository clanRepository;
-  private final PlayerRepository playerRepository;
   private final FafApiProperties fafApiProperties;
   private final JwtService jwtService;
   private final ObjectMapper objectMapper;
   private final PlayerService playerService;
   private final ClanMembershipRepository clanMembershipRepository;
 
+  @Transactional
+  public void preCreate(Clan clan) {
+    Assert.isNull(clan.getId(), "Clan payload with id can not be used for creation.");
+
+    Player player = playerService.getCurrentPlayer();
+
+    if (player.getClanMembership() != null) {
+      throw ApiException.of(ErrorCode.CLAN_CREATE_FOUNDER_IS_IN_A_CLAN);
+    }
+
+    if (!player.equals(clan.getFounder())) {
+      throw ApiException.of(ErrorCode.CLAN_INVALID_FOUNDER);
+    }
+
+    clanRepository.findOneByName(clan.getName()).ifPresent(c -> {
+      throw ApiException.of(ErrorCode.CLAN_NAME_EXISTS, clan.getName());
+    });
+
+    clanRepository.findOneByTag(clan.getTag()).ifPresent(c -> {
+      throw ApiException.of(ErrorCode.CLAN_TAG_EXISTS, clan.getTag());
+    });
+
+    clan.setLeader(player);
+    clan.setMemberships(List.of(new ClanMembership()
+      .setClan(clan)
+      .setPlayer(player)));
+  }
+
   @SneakyThrows
-  Clan create(String name, String tag, String description, Player creator) {
+  @Transactional
+  @Deprecated
+    // use preCreate instead
+  Clan create(String name, String tag, String description) {
+    Player creator = playerService.getCurrentPlayer();
+
     if (creator.getClanMembership() != null) {
-      throw new ApiException(new Error(ErrorCode.CLAN_CREATE_CREATOR_IS_IN_A_CLAN));
+      throw ApiException.of(ErrorCode.CLAN_CREATE_FOUNDER_IS_IN_A_CLAN);
     }
     if (clanRepository.findOneByName(name).isPresent()) {
-      throw new ApiException(new Error(ErrorCode.CLAN_NAME_EXISTS, name));
+      throw ApiException.of(ErrorCode.CLAN_NAME_EXISTS, name);
     }
     if (clanRepository.findOneByTag(tag).isPresent()) {
-      throw new ApiException(new Error(ErrorCode.CLAN_TAG_EXISTS, tag));
+      throw ApiException.of(ErrorCode.CLAN_TAG_EXISTS, tag);
     }
 
     Clan clan = new Clan();
@@ -69,16 +101,18 @@ Clan create(String name, String tag, String description, Player creator) {
   }
 
   @SneakyThrows
-  String generatePlayerInvitationToken(Player requester, int newMemberId, int clanId) {
+  @Transactional
+  String generatePlayerInvitationToken(int newMemberId, int clanId) {
+    Player requester = playerService.getCurrentPlayer();
+
     Clan clan = clanRepository.findById(clanId)
       .orElseThrow(() -> new ApiException(new Error(ErrorCode.CLAN_NOT_EXISTS, clanId)));
 
     if (requester.getId() != clan.getLeader().getId()) {
-      throw new ApiException(new Error(ErrorCode.CLAN_NOT_LEADER, clanId));
+      throw ApiException.of(ErrorCode.CLAN_NOT_LEADER, clanId);
     }
 
-    Player newMember = playerRepository.findById(newMemberId)
-      .orElseThrow(() -> new ApiException(new Error(ErrorCode.CLAN_GENERATE_LINK_PLAYER_NOT_FOUND, newMemberId)));
+    Player newMember = playerService.getById(newMemberId);
 
     long expire = Instant.now()
       .plus(fafApiProperties.getClan().getInviteLinkExpireDurationMinutes(), ChronoUnit.MINUTES)
@@ -91,28 +125,27 @@ String generatePlayerInvitationToken(Player requester, int newMemberId, int clan
   }
 
   @SneakyThrows
-  void acceptPlayerInvitationToken(String stringToken, Authentication authentication) {
+  @Transactional
+  void acceptPlayerInvitationToken(String stringToken) {
     Jwt token = jwtService.decodeAndVerify(stringToken);
     InvitationResult invitation = objectMapper.readValue(token.getClaims(), InvitationResult.class);
 
     if (invitation.isExpired()) {
-      throw new ApiException(new Error(ErrorCode.CLAN_ACCEPT_TOKEN_EXPIRE));
+      throw ApiException.of(ErrorCode.CLAN_ACCEPT_TOKEN_EXPIRE);
     }
 
     final Integer clanId = invitation.getClan().getId();
-    Player player = playerService.getPlayer(authentication);
+    Player player = playerService.getCurrentPlayer();
     Clan clan = clanRepository.findById(clanId)
       .orElseThrow(() -> new ApiException(new Error(ErrorCode.CLAN_NOT_EXISTS, clanId)));
 
-    Player newMember = playerRepository.findById(invitation.getNewMember().getId())
-      .orElseThrow(() -> new ProgrammingError("ClanMember does not exist: " + invitation.getNewMember().getId()));
-
+    Player newMember = playerService.getById(invitation.getNewMember().getId());
 
     if (player.getId() != newMember.getId()) {
-      throw new ApiException(new Error(ErrorCode.CLAN_ACCEPT_WRONG_PLAYER));
+      throw ApiException.of(ErrorCode.CLAN_ACCEPT_WRONG_PLAYER);
     }
     if (newMember.getClan() != null) {
-      throw new ApiException(new Error(ErrorCode.CLAN_ACCEPT_PLAYER_IN_A_CLAN));
+      throw ApiException.of(ErrorCode.CLAN_ACCEPT_PLAYER_IN_A_CLAN);
     }
 
     ClanMembership membership = new ClanMembership();

diff --git a/src/main/java/com/faforever/api/clan/ClansController.java b/src/main/java/com/faforever/api/clan/ClansController.java
@@ -6,24 +6,21 @@
 import com.faforever.api.data.domain.Clan;
 import com.faforever.api.data.domain.Player;
 import com.faforever.api.player.PlayerService;
-import com.google.common.collect.ImmutableMap;
 import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
 import lombok.RequiredArgsConstructor;
 import org.springframework.security.access.prepost.PreAuthorize;
-import org.springframework.security.core.Authentication;
-import org.springframework.transaction.annotation.Transactional;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PostMapping;
 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 java.io.IOException;
 import java.io.Serializable;
 import java.util.Map;
 
-import static org.springframework.http.MediaType.APPLICATION_JSON_UTF8_VALUE;
+import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
 
 
 @RestController
@@ -37,11 +34,12 @@ public class ClansController {
 
   @ApiOperation("Grab data about yourself and the clan")
   @ApiResponses(value = {
-      @ApiResponse(code = 200, message = "Success with JSON { player: {id: ?, login: ?}, clan: { id: ?, name: ?, tag: ?}}"),
-      @ApiResponse(code = 400, message = "Bad Request")})
-  @RequestMapping(path = "/me", method = RequestMethod.GET, produces = APPLICATION_JSON_UTF8_VALUE)
-  public MeResult me(Authentication authentication) {
-    Player player = playerService.getPlayer(authentication);
+    @ApiResponse(code = 200, message = "Success with JSON { player: {id: ?, login: ?}, clan: { id: ?, name: ?, tag: ?}}"),
+    @ApiResponse(code = 400, message = "Bad Request")})
+  @GetMapping(path = "/me", produces = APPLICATION_JSON_VALUE)
+  @Deprecated // use regular /me route instead
+  public MeResult me() {
+    Player player = playerService.getCurrentPlayer();
 
     Clan clan = player.getClan();
     ClanResult clanResult = null;
@@ -55,44 +53,33 @@ public MeResult me(Authentication authentication) {
   // a: the new clan with the leader membership, b: the leader membership with the new clan
   @ApiOperation("Create a clan with correct leader, founder and clan membership")
   @ApiResponses(value = {
-      @ApiResponse(code = 200, message = "Success with JSON { id: ?, type: 'clan'}"),
-      @ApiResponse(code = 400, message = "Bad Request")})
-  @RequestMapping(path = "/create", method = RequestMethod.POST, produces = APPLICATION_JSON_UTF8_VALUE)
+    @ApiResponse(code = 200, message = "Success with JSON { id: ?, type: 'clan'}"),
+    @ApiResponse(code = 400, message = "Bad Request")})
+  @PostMapping(path = "/create", produces = APPLICATION_JSON_VALUE)
   @PreAuthorize("hasRole('ROLE_USER')")
-  @Transactional
+  @Deprecated // use POST /data/clans instead (with a founder in relationships)
   public Map<String, Serializable> createClan(@RequestParam(value = "name") String name,
                                               @RequestParam(value = "tag") String tag,
-                                              @RequestParam(value = "description", required = false) String description,
-                                              Authentication authentication) throws IOException {
-    Player player = playerService.getPlayer(authentication);
-    Clan clan = clanService.create(name, tag, description, player);
-    return ImmutableMap.of("id", clan.getId(), "type", "clan");
+                                              @RequestParam(value = "description", required = false) String description) {
+    Clan clan = clanService.create(name, tag, description);
+    return Map.of("id", clan.getId(), "type", "clan");
   }
 
   @ApiOperation("Generate invitation link")
   @ApiResponses(value = {
-      @ApiResponse(code = 200, message = "Success with JSON { jwtToken: ? }"),
-      @ApiResponse(code = 400, message = "Bad Request")})
-  @RequestMapping(path = "/generateInvitationLink",
-      method = RequestMethod.GET,
-    produces = APPLICATION_JSON_UTF8_VALUE)
+    @ApiResponse(code = 200, message = "Success with JSON { jwtToken: ? }"),
+    @ApiResponse(code = 400, message = "Bad Request")})
+  @GetMapping(path = "/generateInvitationLink", produces = APPLICATION_JSON_VALUE)
   public Map<String, Serializable> generateInvitationLink(
-      @RequestParam(value = "clanId") int clanId,
-      @RequestParam(value = "playerId") int newMemberId,
-      Authentication authentication) throws IOException {
-    Player player = playerService.getPlayer(authentication);
-    String jwtToken = clanService.generatePlayerInvitationToken(player, newMemberId, clanId);
-    return ImmutableMap.of("jwtToken", jwtToken);
+    @RequestParam(value = "clanId") int clanId,
+    @RequestParam(value = "playerId") int newMemberId) {
+    String jwtToken = clanService.generatePlayerInvitationToken(newMemberId, clanId);
+    return Map.of("jwtToken", jwtToken);
   }
 
-  @ApiOperation("Check invitation link and add Member to Clan")
-  @RequestMapping(path = "/joinClan",
-      method = RequestMethod.POST,
-    produces = APPLICATION_JSON_UTF8_VALUE)
-  @Transactional
-  public void joinClan(
-      @RequestParam(value = "token") String stringToken,
-      Authentication authentication) throws IOException {
-    clanService.acceptPlayerInvitationToken(stringToken, authentication);
+  @ApiOperation("Check invitation link and add member to Clan")
+  @PostMapping(path = "/joinClan", produces = APPLICATION_JSON_VALUE)
+  public void joinClan(@RequestParam(value = "token") String stringToken) {
+    clanService.acceptPlayerInvitationToken(stringToken);
   }
 }
diff --git a/src/main/java/com/faforever/api/data/domain/Clan.java b/src/main/java/com/faforever/api/data/domain/Clan.java
@@ -47,6 +47,7 @@ public class Clan extends AbstractEntity implements OwnableEntity {
   private String description;
   private String tagColor;
   private String websiteUrl;
+  private Boolean requiresInvitation = Boolean.TRUE;
   private List<ClanMembership> memberships;
 
   @Column(name = "name")
@@ -88,6 +89,11 @@ public String getTagColor() {
     return tagColor;
   }
 
+  @Column(name = "requires_invitation", nullable = false)
+  public Boolean getRequiresInvitation() {
+    return requiresInvitation;
+  }
+
   // Cascading is needed for Create & Delete
   @OneToMany(mappedBy = "clan", cascade = CascadeType.ALL, orphanRemoval = true)
   // Permission is managed by ClanMembership class

diff --git a/src/main/java/com/faforever/api/data/listeners/ClanEnricherListener.java b/src/main/java/com/faforever/api/data/listeners/ClanEnricherListener.java
@@ -1,20 +1,31 @@
 package com.faforever.api.data.listeners;
 
+import com.faforever.api.clan.ClanService;
 import com.faforever.api.config.FafApiProperties;
 import com.faforever.api.data.domain.Clan;
 import org.springframework.stereotype.Component;
 
 import javax.inject.Inject;
 import javax.persistence.PostLoad;
+import javax.persistence.PrePersist;
 
 @Component
 public class ClanEnricherListener {
 
   private static FafApiProperties fafApiProperties;
+  private static ClanService clanService;
 
   @Inject
-  public void init(FafApiProperties fafApiProperties) {
+  public void init(FafApiProperties fafApiProperties, ClanService clanService) {
     ClanEnricherListener.fafApiProperties = fafApiProperties;
+    ClanEnricherListener.clanService = clanService;
+  }
+
+  @PrePersist
+  public void prePersist(Clan clan) {
+    if (clan.getId() == null) {
+      clanService.preCreate(clan);
+    }
   }
 
   @PostLoad

diff --git a/src/main/java/com/faforever/api/error/ErrorCode.java b/src/main/java/com/faforever/api/error/ErrorCode.java
@@ -55,13 +55,13 @@ public enum ErrorCode {
   INVALID_METADATA(146, "Invalid metadata", "Metadata is not valid: {0}"),
   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"),
+  CLAN_CREATE_FOUNDER_IS_IN_A_CLAN(149, "You are already in a clan", "Clan founder is already member of a clan"),
   CLAN_ACCEPT_TOKEN_EXPIRE(150, "Token Expire", "The Invitation Link expire"),
   CLAN_ACCEPT_WRONG_PLAYER(151, "Wrong Player", "Your are not the invited player"),
   CLAN_ACCEPT_PLAYER_IN_A_CLAN(152, "Player is in a clan", "You are already in a clan"),
   CLAN_NOT_LEADER(153, "You Permission", "You are not the leader of the clan"),
   CLAN_NOT_EXISTS(154, "Cannot find Clan", "Clan with id {0, number} is not available"),
-  CLAN_GENERATE_LINK_PLAYER_NOT_FOUND(155, "Player not found", "Cannot find player with id {0, number} who should be invited to the clan"),
+  PLAYER_NOT_FOUND(155, "Player not found", "Cannot find player with id {0, number}."),
   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}"),
@@ -99,7 +99,8 @@ public enum ErrorCode {
   PARSING_LUA_FILE_FAILED(189, "Parsing lua files failed", "During the parsing of the lua file an error occured: {0}"),
   NO_RUSH_RADIUS_MISSING(190, "No rush radius missing", "The scenario file must specify a no rush radius"),
   INVALID_FEATURED_MOD(191, "Invalid featured mod name", "The featured mod name ''{0}'' is not allowed in this context."),
-  API_KEY_INVALID(192, "Api key is invalid", "The api key is invalid.");
+  API_KEY_INVALID(192, "Api key is invalid", "The api key is invalid."),
+  CLAN_INVALID_FOUNDER(193, "Invalid clan founder", "If you create a clan you must be the founder of it.");
 
   private final int code;
   private final String title;

diff --git a/src/main/java/com/faforever/api/map/MapsController.java b/src/main/java/com/faforever/api/map/MapsController.java
@@ -14,7 +14,6 @@
 import io.swagger.annotations.ApiResponses;
 import lombok.AllArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
-import org.springframework.security.core.Authentication;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RequestParam;
@@ -77,8 +76,7 @@ public void validateScenarioLua(@RequestParam(name = "scenarioLua") String scena
     @ApiResponse(code = 500, message = "Failure")})
   @RequestMapping(path = "/upload", method = RequestMethod.POST, produces = APPLICATION_JSON_UTF8_VALUE)
   public void uploadMap(@RequestParam("file") MultipartFile file,
-                        @RequestParam("metadata") String jsonString,
-                        Authentication authentication) throws IOException {
+                        @RequestParam("metadata") String jsonString) throws IOException {
     if (file == null) {
       throw new ApiException(new Error(ErrorCode.UPLOAD_FILE_MISSING));
     }
@@ -97,7 +95,7 @@ public void uploadMap(@RequestParam("file") MultipartFile file,
       throw new ApiException(new Error(ErrorCode.INVALID_METADATA, e.getMessage()));
     }
 
-    Player player = playerService.getPlayer(authentication);
+    Player player = playerService.getCurrentPlayer();
     mapService.uploadMap(file.getInputStream(), file.getOriginalFilename(), player, ranked);
   }
 }
diff --git a/src/main/java/com/faforever/api/mod/ModsController.java b/src/main/java/com/faforever/api/mod/ModsController.java
@@ -7,7 +7,6 @@
 import com.faforever.api.player.PlayerService;
 import com.google.common.io.Files;
 import io.swagger.annotations.ApiOperation;
-import org.springframework.security.core.Authentication;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RequestParam;
@@ -35,7 +34,7 @@ public ModsController(PlayerService playerService, ModService modService, FafApi
 
   @ApiOperation("Upload a mod")
   @RequestMapping(path = "/upload", method = RequestMethod.POST, produces = APPLICATION_JSON_UTF8_VALUE)
-  public void uploadMod(@RequestParam("file") MultipartFile file, Authentication authentication) throws IOException {
+  public void uploadMod(@RequestParam("file") MultipartFile file) throws IOException {
     if (file == null) {
       throw new ApiException(new Error(ErrorCode.UPLOAD_FILE_MISSING));
     }
@@ -48,6 +47,6 @@ public void uploadMod(@RequestParam("file") MultipartFile file, Authentication a
     Path tempFile = java.nio.file.Files.createTempFile("mod", ".tmp");
     file.transferTo(tempFile.getFileName().toFile());
 
-    modService.processUploadedMod(tempFile, playerService.getPlayer(authentication));
+    modService.processUploadedMod(tempFile, playerService.getCurrentPlayer());
   }
 }