Fix #66 Don't manually read JSON values, deserialize into a Java object

src/main/java/com/faforever/api/clan/ClanService.java

@@ -1,5 +1,8 @@
 package com.faforever.api.clan;
 
+import com.faforever.api.clan.result.ClanResult;
+import com.faforever.api.clan.result.InvitationResult;
+import com.faforever.api.clan.result.PlayerResult;
 import com.faforever.api.config.FafApiProperties;
 import com.faforever.api.data.domain.Clan;
 import com.faforever.api.data.domain.ClanMembership;
@@ -11,9 +14,7 @@
 import com.faforever.api.player.PlayerRepository;
 import com.faforever.api.player.PlayerService;
 import com.faforever.api.security.JwtService;
-import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.collect.ImmutableMap;
 import lombok.SneakyThrows;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.jwt.Jwt;
@@ -103,36 +104,31 @@ public String generatePlayerInvitationToken(Player requester, int newMemberId, i
         .plus(fafApiProperties.getClan().getInviteLinkExpireDurationMinutes(), ChronoUnit.MINUTES)
         .toEpochMilli();
 
-    return jwtService.sign(
-        ImmutableMap.of(JwtKeys.NEW_MEMBER_ID, newMemberId,
-            JwtKeys.EXPIRE_IN, expire,
-            JwtKeys.CLAN, ImmutableMap.of(
-                JwtKeys.CLAN_ID, clan.getId(),
-                JwtKeys.CLAN_TAG, clan.getTag(),
-                JwtKeys.CLAN_NAME, clan.getName())
-        ));
+    InvitationResult result = new InvitationResult(expire,
+        ClanResult.of(clan),
+        PlayerResult.of(newMember));
+    return jwtService.sign(result);
   }
 
   @SneakyThrows
-  // TODO @dragonfire don't manually read JSON values, deserialize into a Java object?
   public void acceptPlayerInvitationToken(String stringToken, Authentication authentication) {
     Jwt token = jwtService.decodeAndVerify(stringToken);
-    JsonNode data = objectMapper.readTree(token.getClaims());
+    InvitationResult invitation = objectMapper.readValue(token.getClaims(), InvitationResult.class);
 
-    if (data.get(JwtKeys.EXPIRE_IN).asLong() < System.currentTimeMillis()) {
+    if (invitation.isExpired()) {
       throw new ApiException(new Error(ErrorCode.CLAN_ACCEPT_TOKEN_EXPIRE));
     }
 
     Player player = playerService.getPlayer(authentication);
-    Clan clan = clanRepository.findOne(data.get(JwtKeys.CLAN).get(JwtKeys.CLAN_ID).asInt());
+    Clan clan = clanRepository.findOne(invitation.getClan().getId());
 
     if (clan == null) {
       throw new ApiException(new Error(ErrorCode.CLAN_NOT_EXISTS));
     }
 
-    Player newMember = playerRepository.findOne(data.get(JwtKeys.NEW_MEMBER_ID).asInt());
+    Player newMember = playerRepository.findOne(invitation.getNewMember().getId());
     if (newMember == null) {
-      throw new ProgrammingError("ClanMember does not exist: " + data.get(JwtKeys.NEW_MEMBER_ID).asInt());
+      throw new ProgrammingError("ClanMember does not exist: " + invitation.getNewMember().getId());
     }
 
     if (player.getId() != newMember.getId()) {

src/main/java/com/faforever/api/clan/ClansController.java

@@ -51,9 +51,9 @@ public MeResult me(Authentication authentication) {
         : null;
     ClanResult clanResult = null;
     if (clan != null) {
-      clanResult = new ClanResult(clan.getId(), clan.getTag(), clan.getName());
+      clanResult = ClanResult.of(clan);
     }
-    return new MeResult(new PlayerResult(player.getId(), player.getLogin()), clanResult);
+    return new MeResult(PlayerResult.of(player), clanResult);
   }
 
   // This request cannot be handled by JSON API because we must simultaneously create two resources (a,b)

src/main/java/com/faforever/api/clan/result/ClanResult.java

@@ -1,10 +1,15 @@
 package com.faforever.api.clan.result;
 
+import com.faforever.api.data.domain.Clan;
 import lombok.Data;
 
 @Data
 public class ClanResult {
   private final int id;
   private final String tag;
   private final String name;
+
+  public static ClanResult of(Clan clan) {
+    return new ClanResult(clan.getId(), clan.getTag(), clan.getName());
+  }
 }

src/main/java/com/faforever/api/clan/result/InvitationResult.java

@@ -0,0 +1,14 @@
+package com.faforever.api.clan.result;
+
+import lombok.Data;
+
+@Data
+public class InvitationResult {
+  private final long expire;
+  private final ClanResult clan;
+  private final PlayerResult newMember;
+
+  public boolean isExpired() {
+    return expire < System.currentTimeMillis();
+  }
+}

src/main/java/com/faforever/api/clan/result/PlayerResult.java

@@ -1,9 +1,14 @@
 package com.faforever.api.clan.result;
 
+import com.faforever.api.data.domain.Player;
 import lombok.Data;
 
 @Data
 public class PlayerResult {
   private final int id;
   private final String login;
+
+  public static PlayerResult of(Player newMember) {
+    return new PlayerResult(newMember.getId(), newMember.getLogin());
+  }
 }

src/main/java/com/faforever/api/security/JwtService.java

@@ -9,8 +9,6 @@
 
 import javax.inject.Inject;
 import java.io.IOException;
-import java.io.Serializable;
-import java.util.Map;
 
 @Service
 public class JwtService {
@@ -23,7 +21,7 @@ public JwtService(FafApiProperties fafApiProperties, ObjectMapper objectMapper)
     this.objectMapper = objectMapper;
   }
 
-  public String sign(Map<String, Serializable> data) throws IOException {
+  public String sign(Object data) throws IOException {
     return JwtHelper.encode(objectMapper.writeValueAsString(data), this.macSigner).getEncoded();
   }
 

src/test/java/com/faforever/api/clan/ClanServiceTest.java

@@ -1,6 +1,7 @@
 package com.faforever.api.clan;
 
 
+import com.faforever.api.clan.result.InvitationResult;
 import com.faforever.api.config.FafApiProperties;
 import com.faforever.api.data.domain.Clan;
 import com.faforever.api.data.domain.ClanMembership;
@@ -26,7 +27,6 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Map;
 import java.util.Optional;
 
 import static com.faforever.api.error.ApiExceptionWithCode.apiExceptionWithCode;
@@ -236,15 +236,16 @@ public void generatePlayerInvitationToken() throws IOException {
     when(fafApiProperties.getClan()).thenReturn(props.getClan());
 
     instance.generatePlayerInvitationToken(requester, newMember.getId(), clan.getId());
-    ArgumentCaptor<Map> captor = ArgumentCaptor.forClass(Map.class);
+    ArgumentCaptor<InvitationResult> captor = ArgumentCaptor.forClass(InvitationResult.class);
     verify(jwtService, Mockito.times(1)).sign(captor.capture());
     assertThat("expire",
-        Long.parseLong(captor.getValue().get("expire").toString()),
+        captor.getValue().getExpire(),
         greaterThan(System.currentTimeMillis()));
-    assertEquals(newMember.getId(), captor.getValue().get("newMemberId"));
-    assertEquals(clan.getId(), ((Map) captor.getValue().get("clan")).get("id"));
-    assertEquals(clan.getTag(), ((Map) captor.getValue().get("clan")).get("tag"));
-    assertEquals(clan.getName(), ((Map) captor.getValue().get("clan")).get("name"));
+    assertEquals(newMember.getId(), captor.getValue().getNewMember().getId());
+    assertEquals(newMember.getLogin(), captor.getValue().getNewMember().getLogin());
+    assertEquals(clan.getId(), captor.getValue().getClan().getId());
+    assertEquals(clan.getTag(), captor.getValue().getClan().getTag());
+    assertEquals(clan.getName(), captor.getValue().getClan().getName());
   }
 
   @Test
@@ -296,7 +297,7 @@ public void acceptPlayerInvitationTokenInvalidPlayer() throws IOException {
     Jwt jwtToken = Mockito.mock(Jwt.class);
 
     when(jwtToken.getClaims()).thenReturn(
-        String.format("{\"expire\":%s,\"newMemberId\":2,\"clan\":{\"id\":%s}}",
+        String.format("{\"expire\":%s,\"newMember\":{\"id\":2},\"clan\":{\"id\":%s}}",
             expire, clan.getId()));
     when(jwtService.decodeAndVerify(any())).thenReturn(jwtToken);
     when(clanRepository.findOne(clan.getId())).thenReturn(clan);
@@ -326,7 +327,7 @@ public void acceptPlayerInvitationTokenWrongPlayer() throws IOException {
     Jwt jwtToken = Mockito.mock(Jwt.class);
 
     when(jwtToken.getClaims()).thenReturn(
-        String.format("{\"expire\":%s,\"newMemberId\":%s,\"clan\":{\"id\":%s}}",
+        String.format("{\"expire\":%s,\"newMember\":{\"id\":%s},\"clan\":{\"id\":%s}}",
             expire, newMember.getId(), clan.getId()));
     when(jwtService.decodeAndVerify(any())).thenReturn(jwtToken);
     when(clanRepository.findOne(clan.getId())).thenReturn(clan);
@@ -357,7 +358,7 @@ public void acceptPlayerInvitationTokenPlayerIAlreadyInAClan() throws IOExceptio
     Jwt jwtToken = Mockito.mock(Jwt.class);
 
     when(jwtToken.getClaims()).thenReturn(
-        String.format("{\"expire\":%s,\"newMemberId\":%s,\"clan\":{\"id\":%s}}",
+        String.format("{\"expire\":%s,\"newMember\":{\"id\":%s},\"clan\":{\"id\":%s}}",
             expire, newMember.getId(), clan.getId()));
     when(jwtService.decodeAndVerify(any())).thenReturn(jwtToken);
     when(clanRepository.findOne(clan.getId())).thenReturn(clan);
@@ -383,7 +384,7 @@ public void acceptPlayerInvitationToken() throws IOException {
     Jwt jwtToken = Mockito.mock(Jwt.class);
 
     when(jwtToken.getClaims()).thenReturn(
-        String.format("{\"expire\":%s,\"newMemberId\":%s,\"clan\":{\"id\":%s}}",
+        String.format("{\"expire\":%s,\"newMember\":{\"id\":%s},\"clan\":{\"id\":%s}}",
             expire, newMember.getId(), clan.getId()));
     when(jwtService.decodeAndVerify(any())).thenReturn(jwtToken);
     when(clanRepository.findOne(clan.getId())).thenReturn(clan);