Apple best practices (OpenRewrite Upgrade Spring Boot 3.3) + removed …

…some warnings
FAForever · Oct 25, 2024 · 87c87cd · 87c87cd
1 parent a44ae24
commit 87c87cd
Show file tree

Hide file tree

Showing 57 changed files with 314 additions and 330 deletions.
diff --git a/build.gradle b/build.gradle
@@ -10,10 +10,11 @@ buildscript {
 }
 
 plugins {
-  id "java"
-  id "org.springframework.boot" version "3.3.2"
-  id "idea"
-  id "com.adarshr.test-logger" version "4.0.0"
+  id("java")
+  id("org.springframework.boot") version ("3.3.5")
+  id("idea")
+  id("com.adarshr.test-logger") version ("4.0.0")
+  id("org.openrewrite.rewrite") version("6.26.0")
 }
 
 group = 'faforever'

diff --git a/configuration.md b/configuration.md
@@ -1,7 +1,6 @@
 | Env variable  | Default value | Optional if env is defined | Default value (local) |
 | ---- | ---- | ---- | ---- |
 | ACTIVATION_URL_FORMAT | `https://www.${FAF_DOMAIN}/account/activate?username=%s&token=%s` | `FAF_DOMAIN` | `http://localhost:8020/account/activate?username=%s&token=%s` |
-| ANOPE_DATABASE_NAME | `faf-anope` |  |  |
 | API_PORT | `8010` |  |  |
 | API_PROFILE | `local` |  |  |
 | AVATAR_ALLOWED_FILE_EXTENSIONS | `png` |  | `png` |

diff --git a/src/inttest/java/com/faforever/api/AbstractIntegrationTest.java b/src/inttest/java/com/faforever/api/AbstractIntegrationTest.java
@@ -18,7 +18,6 @@
 import com.github.jasminb.jsonapi.exceptions.DocumentSerializationException;
 import org.json.JSONObject;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.extension.ExtendWith;
 import org.skyscreamer.jsonassert.JSONAssert;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
@@ -27,7 +26,6 @@
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.context.jdbc.Sql;
 import org.springframework.test.context.jdbc.Sql.ExecutionPhase;
-import org.springframework.test.context.junit.jupiter.SpringExtension;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.MvcResult;
 import org.springframework.test.web.servlet.request.RequestPostProcessor;
@@ -44,7 +42,6 @@
 import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity;
 
 @Testcontainers
-@ExtendWith(SpringExtension.class)
 @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
 @ActiveProfiles(ApplicationProfile.INTEGRATION_TEST)
 @Import(OAuthHelper.class)
@@ -119,7 +116,7 @@ protected RequestPostProcessor getOAuthTokenForUserId(int userId, String... scop
 
   protected void assertApiError(MvcResult mvcResult, ErrorCode errorCode) throws Exception {
     JSONObject resonseJson = new JSONObject(mvcResult.getResponse().getContentAsString());
-    JSONAssert.assertEquals(String.format("{\"errors\":[{\"code\":\"%s\"}]}", errorCode.getCode()), resonseJson, false);
+    JSONAssert.assertEquals("{\"errors\":[{\"code\":\"%s\"}]}".formatted(errorCode.getCode()), resonseJson, false);
   }
 
   protected <T extends AbstractEntity> byte[] createJsonApiContent(T entity) throws DocumentSerializationException {

diff --git a/src/inttest/java/com/faforever/api/avatar/AvatarControllerTest.java b/src/inttest/java/com/faforever/api/avatar/AvatarControllerTest.java
@@ -19,7 +19,7 @@
 
 import java.io.IOException;
 import java.nio.file.Files;
-import java.nio.file.Paths;
+import java.nio.file.Path;
 import java.util.Optional;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -57,8 +57,8 @@ public void canUploadWithScopeAndRole() throws Exception {
 
   @Test
   public void canReuploadWithScopeAndRole() throws Exception {
-    Files.createDirectories(Paths.get("build/cache/avatars"));
-    Files.copy(FileHandlingHelper.loadResourceAsStream("/avatars/donator.png"), Paths.get("build/cache/avatars/avatar1.png"));
+    Files.createDirectories(Path.of("build/cache/avatars"));
+    Files.copy(FileHandlingHelper.loadResourceAsStream("/avatars/donator.png"), Path.of("build/cache/avatars/avatar1.png"));
     mockMvc.perform(
       createAvatarReuploadRequest(1)
         .with(getOAuthTokenWithActiveUser(OAuthScope._ADMINISTRATIVE_ACTION, GroupPermission.ROLE_WRITE_AVATAR))
@@ -72,8 +72,8 @@ public void canReuploadWithScopeAndRole() throws Exception {
 
   @Test
   public void canDeleteAvatarWithScopeAndRole() throws Exception {
-    Files.createDirectories(Paths.get("build/cache/avatars"));
-    Files.copy(FileHandlingHelper.loadResourceAsStream("/avatars/donator.png"), Paths.get("build/cache/avatars/avatar1.png"));
+    Files.createDirectories(Path.of("build/cache/avatars"));
+    Files.copy(FileHandlingHelper.loadResourceAsStream("/avatars/donator.png"), Path.of("build/cache/avatars/avatar1.png"));
     mockMvc.perform(
       delete("/avatars/3")
         .with(getOAuthTokenWithActiveUser(OAuthScope._ADMINISTRATIVE_ACTION, GroupPermission.ROLE_WRITE_AVATAR))
@@ -138,8 +138,8 @@ public void cannotDeleteWithoutScope() throws Exception {
 
   @AfterEach
   public void tearDown() throws Exception {
-    Files.deleteIfExists(Paths.get("build/cache/avatars/avatar1.png"));
-    Files.deleteIfExists(Paths.get("build/cache/avatars/avatar3.png"));
+    Files.deleteIfExists(Path.of("build/cache/avatars/avatar1.png"));
+    Files.deleteIfExists(Path.of("build/cache/avatars/avatar3.png"));
   }
 
   private MockMultipartHttpServletRequestBuilder createAvatarUploadRequest() throws IOException {

diff --git a/src/inttest/java/com/faforever/api/clan/ClanControllerTest.java b/src/inttest/java/com/faforever/api/clan/ClanControllerTest.java
@@ -16,10 +16,7 @@
 
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.core.IsNull.nullValue;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
@@ -40,12 +37,10 @@ public class ClanControllerTest extends AbstractIntegrationTest {
   PlayerRepository playerRepository;
   @Autowired
   ClanRepository clanRepository;
-  @Autowired
-  ClanMembershipRepository clanMembershipRepository;
 
   @Test
   public void meDataWithoutClan() throws Exception {
-    Player player = playerRepository.getById(USERID_USER);
+    Player player = playerRepository.getReferenceById(USERID_USER);
 
     mockMvc.perform(get("/clans/me")
         .with(getOAuthTokenForUserId(USERID_USER)))
@@ -57,8 +52,8 @@ public void meDataWithoutClan() throws Exception {
 
   @Test
   public void meDataWithClan() throws Exception {
-    Player player = playerRepository.getById(USERID_CLAN_MEMBER);
-    Clan clan = clanRepository.getById(1);
+    Player player = playerRepository.getReferenceById(USERID_CLAN_MEMBER);
+    Clan clan = clanRepository.getReferenceById(1);
 
     mockMvc.perform(
         get("/clans/me")
@@ -73,7 +68,7 @@ public void meDataWithClan() throws Exception {
 
   @Test
   public void createClanWithSuccess() throws Exception {
-    Player player = playerRepository.getById(USERID_USER);
+    Player player = playerRepository.getReferenceById(USERID_USER);
 
     assertNull(player.getClan());
     assertFalse(clanRepository.findOneByName(NEW_CLAN_NAME).isPresent());
@@ -111,7 +106,7 @@ public void createClanWithoutAuth() throws Exception {
 
   @Test
   public void createClanWithExistingName() throws Exception {
-    Player player = playerRepository.getById(USERID_USER);
+    Player player = playerRepository.getReferenceById(USERID_USER);
 
     assertNull(player.getClan());
     assertTrue(clanRepository.findOneByName(EXISTING_CLAN).isPresent());
@@ -133,7 +128,7 @@ public void createClanWithExistingName() throws Exception {
 
   @Test
   public void createClanWithExistingTag() throws Exception {
-    Player player = playerRepository.getById(USERID_USER);
+    Player player = playerRepository.getReferenceById(USERID_USER);
 
     assertNull(player.getClan());
     assertFalse(clanRepository.findOneByName(NEW_CLAN_NAME).isPresent());
@@ -155,7 +150,7 @@ public void createClanWithExistingTag() throws Exception {
 
   @Test
   public void createSecondClan() throws Exception {
-    Player player = playerRepository.getById(USERID_CLAN_MEMBER);
+    Player player = playerRepository.getReferenceById(USERID_CLAN_MEMBER);
 
     assertNotNull(player.getClan());
     assertFalse(clanRepository.findOneByName(NEW_CLAN_NAME).isPresent());

diff --git a/src/inttest/java/com/faforever/api/data/ClanElideTest.java b/src/inttest/java/com/faforever/api/data/ClanElideTest.java
@@ -21,10 +21,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch;
@@ -48,13 +45,13 @@ public class ClanElideTest extends AbstractIntegrationTest {
 
   @Test
   public void canDeleteMemberOfOwnClan() throws Exception {
-    assertNotNull(playerRepository.getById(USERID_CLAN_MEMBER).getClan());
+    assertNotNull(playerRepository.getReferenceById(USERID_CLAN_MEMBER).getClan());
 
     mockMvc.perform(
         delete("/data/clanMembership/2") // magic value from prepClanData.sql
           .with(getOAuthTokenForUserId(USERID_CLAN_LEADER)))
       .andExpect(status().isNoContent());
-    assertNull(playerRepository.getById(USERID_CLAN_MEMBER).getClan());
+    assertNull(playerRepository.getReferenceById(USERID_CLAN_MEMBER).getClan());
   }
 
   @Test
@@ -77,14 +74,14 @@ public void cannotDeleteLeaderFromClan() throws Exception {
 
   @Test
   public void canLeaveClan() throws Exception {
-    assertNotNull(playerRepository.getById(USERID_CLAN_MEMBER).getClan());
+    assertNotNull(playerRepository.getReferenceById(USERID_CLAN_MEMBER).getClan());
 
     mockMvc.perform(
         delete("/data/clanMembership/2") // magic value from prepClanData.sql
           .with(getOAuthTokenForUserId(USERID_CLAN_MEMBER)))
       .andExpect(status().isNoContent());
 
-    assertNull(playerRepository.getById(USERID_CLAN_MEMBER).getClan());
+    assertNull(playerRepository.getReferenceById(USERID_CLAN_MEMBER).getClan());
   }
 
   @Test
@@ -108,7 +105,7 @@ public void getFilteredPlayerForClanInvite() throws Exception {
 
   @Test
   public void canTransferLeadershipAsLeader() throws Exception {
-    assertThat(clanRepository.getById(1).getLeader().getLogin(), is(AUTH_CLAN_LEADER));
+    assertThat(clanRepository.getReferenceById(1).getLeader().getLogin(), is(AUTH_CLAN_LEADER));
 
     mockMvc.perform(
         patch("/data/clan/1")
@@ -117,12 +114,12 @@ public void canTransferLeadershipAsLeader() throws Exception {
           .content(generateTransferLeadershipContent(1, USERID_CLAN_MEMBER))) // magic value from prepClanData.sql
       .andExpect(status().isNoContent());
 
-    assertThat(clanRepository.getById(1).getLeader().getLogin(), is(AUTH_CLAN_MEMBER));
+    assertThat(clanRepository.getReferenceById(1).getLeader().getLogin(), is(AUTH_CLAN_MEMBER));
   }
 
   @Test
   public void cannotTransferLeadershipAsMember() throws Exception {
-    assertThat(clanRepository.getById(1).getLeader().getLogin(), is(AUTH_CLAN_LEADER));
+    assertThat(clanRepository.getReferenceById(1).getLeader().getLogin(), is(AUTH_CLAN_LEADER));
 
     mockMvc.perform(
         patch("/data/clan/1")
@@ -132,7 +129,7 @@ public void cannotTransferLeadershipAsMember() throws Exception {
       .andExpect(status().isForbidden())
       .andExpect(jsonPath("$.errors[0].detail", is("UpdatePermission Denied")));
 
-    assertThat(clanRepository.getById(1).getLeader().getLogin(), is(AUTH_CLAN_LEADER));
+    assertThat(clanRepository.getReferenceById(1).getLeader().getLogin(), is(AUTH_CLAN_LEADER));
   }
 
   @Test
@@ -170,11 +167,6 @@ public void canDeleteClanAsLeader() throws Exception {
     Clan clan = clanRepository.findOneByName("Alpha Clan")
       .orElseThrow(() -> new IllegalStateException("Alpha Clan could not be found"));
 
-    List<Player> clanMember = new ArrayList<>();
-    clan.getMemberships().stream()
-      .map(ClanMembership::getPlayer)
-      .forEach(clanMember::add);
-
     mockMvc.perform(
         delete("/data/clan/1")
           .with(getOAuthTokenForUserId(USERID_CLAN_LEADER)))
@@ -188,11 +180,6 @@ public void cannotDeleteClanAsMember() throws Exception {
     Optional<Clan> clanOptional = clanRepository.findOneByName("Alpha Clan");
     assertTrue(clanOptional.isPresent());
 
-    List<Player> clanMember = new ArrayList<>();
-    clanOptional.get().getMemberships().stream()
-      .map(ClanMembership::getPlayer)
-      .forEach(clanMember::add);
-
     mockMvc.perform(
         delete("/data/clan/1")
           .with(getOAuthTokenForUserId(USERID_CLAN_MEMBER)))

diff --git a/src/inttest/java/com/faforever/api/data/MapElideTest.java b/src/inttest/java/com/faforever/api/data/MapElideTest.java
@@ -68,7 +68,7 @@ public void canUpdateMapRecommendationToTrueWithScopeAndRole() throws Exception
         .content(MAP_RECOMMENDED_TRUE_ID_1))
       .andExpect(status().isNoContent());
 
-    assertThat(mapRepository.getById(1).getRecommended(), is(true));
+    assertThat(mapRepository.getReferenceById(1).getRecommended(), is(true));
   }
 
   @Test
@@ -81,7 +81,7 @@ public void cannotUpdateMapRecommendationToTrueWithoutScope() throws Exception {
         .content(MAP_RECOMMENDED_TRUE_ID_1))
       .andExpect(status().isForbidden());
 
-    assertThat(mapRepository.getById(1).getRecommended(), is(false));
+    assertThat(mapRepository.getReferenceById(1).getRecommended(), is(false));
   }
 
   @Test
@@ -94,7 +94,7 @@ public void cannotUpdateMapRecommendationToTrueWithoutRole() throws Exception {
         .content(MAP_RECOMMENDED_TRUE_ID_1))
       .andExpect(status().isForbidden());
 
-    assertThat(mapRepository.getById(1).getRecommended(), is(false));
+    assertThat(mapRepository.getReferenceById(1).getRecommended(), is(false));
   }
 
   @Test
@@ -107,7 +107,7 @@ public void canUpdateMapRecommendationToFalseWithScopeAndRole() throws Exception
         .content(MAP_RECOMMENDED_FALSE_ID_1))
       .andExpect(status().isNoContent());
 
-    assertThat(mapRepository.getById(1).getRecommended(), is(false));
+    assertThat(mapRepository.getReferenceById(1).getRecommended(), is(false));
   }
 
   @Test

diff --git a/src/inttest/java/com/faforever/api/data/MapVersionElideTest.java b/src/inttest/java/com/faforever/api/data/MapVersionElideTest.java
@@ -20,42 +20,50 @@
 @Sql(executionPhase = ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:sql/prepMapVersion.sql")
 public class MapVersionElideTest extends AbstractIntegrationTest {
 
-  private static final String MAP_VERSION_HIDE_FALSE_ID_1 = "{\n" +
-    "  \"data\": {\n" +
-    "    \"type\": \"mapVersion\",\n" +
-    "    \"id\": \"1\",\n" +
-    "    \"attributes\": {\n" +
-    "    \t\"hidden\": false\n" +
-    "    }\n" +
-    "  } \n" +
-    "}";
-  private static final String MAP_VERSION_HIDE_TRUE_ID_1 = "{\n" +
-    "  \"data\": {\n" +
-    "    \"type\": \"mapVersion\",\n" +
-    "    \"id\": \"1\",\n" +
-    "    \"attributes\": {\n" +
-    "    \t\"hidden\": true\n" +
-    "    }\n" +
-    "  } \n" +
-    "}";
-  private static final String MAP_VERSION_RANKED_FALSE_ID_1 = "{\n" +
-    "  \"data\": {\n" +
-    "    \"type\": \"mapVersion\",\n" +
-    "    \"id\": \"1\",\n" +
-    "    \"attributes\": {\n" +
-    "    \t\"ranked\": false\n" +
-    "    }\n" +
-    "  } \n" +
-    "}";
-  private static final String MAP_VERSION_RANKED_TRUE_ID_1 = "{\n" +
-    "  \"data\": {\n" +
-    "    \"type\": \"mapVersion\",\n" +
-    "    \"id\": \"1\",\n" +
-    "    \"attributes\": {\n" +
-    "    \t\"ranked\": true\n" +
-    "    }\n" +
-    "  } \n" +
-    "}";
+  private static final String MAP_VERSION_HIDE_FALSE_ID_1 = """
+    {
+      "data": {
+        "type": "mapVersion",
+        "id": "1",
+        "attributes": {
+        	"hidden": false
+        }
+      }\s
+    }\
+    """;
+  private static final String MAP_VERSION_HIDE_TRUE_ID_1 = """
+    {
+      "data": {
+        "type": "mapVersion",
+        "id": "1",
+        "attributes": {
+        	"hidden": true
+        }
+      }\s
+    }\
+    """;
+  private static final String MAP_VERSION_RANKED_FALSE_ID_1 = """
+    {
+      "data": {
+        "type": "mapVersion",
+        "id": "1",
+        "attributes": {
+        	"ranked": false
+        }
+      }\s
+    }\
+    """;
+  private static final String MAP_VERSION_RANKED_TRUE_ID_1 = """
+    {
+      "data": {
+        "type": "mapVersion",
+        "id": "1",
+        "attributes": {
+        	"ranked": true
+        }
+      }\s
+    }\
+    """;
 
 
   @Test