From 53270a8a3977ada436d523f83949499e26d7cfca Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Wed, 9 May 2018 09:56:20 +0200 Subject: [PATCH 01/10] Upgrade to Elide 4.2.3 and Hibernate 5.2.17 --- build.gradle | 4 --- gradle.properties | 4 +-- .../faforever/api/config/ElideTestConfig.java | 7 +++--- .../faforever/api/clan/ClansController.java | 4 +-- .../api/config/elide/ElideConfig.java | 9 +++---- .../com/faforever/api/data/domain/Clan.java | 2 +- .../com/faforever/api/data/domain/Login.java | 20 +++++++-------- .../com/faforever/api/data/domain/Player.java | 25 ++++++++++--------- .../com/faforever/api/data/package-info.java | 2 +- .../faforever/api/clan/ClanServiceTest.java | 7 +----- 10 files changed, 36 insertions(+), 48 deletions(-) diff --git a/build.gradle b/build.gradle index 2412892e6..ced9bd217 100644 --- a/build.gradle +++ b/build.gradle @@ -270,9 +270,6 @@ build.dependsOn inttest dependencyManagement { dependencies { - dependency("org.hibernate:hibernate-core:${hibernateVersion}") - dependency("org.hibernate:hibernate-entitymanager:${hibernateVersion}") - dependency("org.hibernate:hibernate-validator:${hibernateVersion}") dependency("org.mockito:mockito-core:${mockitoVersion}") } } @@ -304,7 +301,6 @@ dependencies { compile("com.yahoo.elide:elide-core:${elideVersion}") compile("com.yahoo.elide:elide-swagger:${elideVersion}") compile("com.yahoo.elide:elide-datastore-hibernate5:${elideVersion}") - compile("org.hibernate:hibernate-java8:${hibernateVersion}") // compile("org.glassfish.jersey.core:jersey-common:2.26") compile("com.zaxxer:HikariCP:${hikariCpVersion}") { exclude(module: 'tools') diff --git a/gradle.properties b/gradle.properties index af9ab99c1..d4bab6548 100644 --- a/gradle.properties +++ b/gradle.properties @@ -2,8 +2,8 @@ profile=dev springBootVersion=1.5.13.RELEASE jjwtVersion=0.7.0 javaxInjectVersion=1 -elideVersion=3.1.4 -hibernateVersion=5.1.0.Final +elideVersion=4.2.3 +hibernate.version=5.2.17.Final mysqlConnectorVersion=6.0.5 gradleDockerVersion=3.2.1 propdepsVersion=0.0.7 diff --git a/src/inttest/java/com/faforever/api/config/ElideTestConfig.java b/src/inttest/java/com/faforever/api/config/ElideTestConfig.java index 146a04f6f..ee79e4ea5 100644 --- a/src/inttest/java/com/faforever/api/config/ElideTestConfig.java +++ b/src/inttest/java/com/faforever/api/config/ElideTestConfig.java @@ -1,7 +1,6 @@ package com.faforever.api.config; -import com.yahoo.elide.datastores.hibernate5.HibernateStore; -import com.yahoo.elide.datastores.hibernate5.HibernateStore.Builder; +import com.yahoo.elide.datastores.hibernate5.AbstractHibernateStore; import org.hibernate.jpa.HibernateEntityManager; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -13,7 +12,7 @@ @Profile(ApplicationProfile.INTEGRATION_TEST) public class ElideTestConfig { @Bean - HibernateStore hibernateStore(EntityManager entityManager) { - return new Builder((HibernateEntityManager) entityManager).build(); + AbstractHibernateStore hibernateStore(EntityManager entityManager) { + return new AbstractHibernateStore.Builder((HibernateEntityManager) entityManager).build(); } } diff --git a/src/main/java/com/faforever/api/clan/ClansController.java b/src/main/java/com/faforever/api/clan/ClansController.java index 59782f06d..6586a59d9 100644 --- a/src/main/java/com/faforever/api/clan/ClansController.java +++ b/src/main/java/com/faforever/api/clan/ClansController.java @@ -47,9 +47,7 @@ public ClansController(ClanService clanService, PlayerService playerService) { public MeResult me(Authentication authentication) { Player player = playerService.getPlayer(authentication); - Clan clan = (!player.getClanMemberships().isEmpty()) - ? player.getClanMemberships().get(0).getClan() - : null; + Clan clan = player.getClan(); ClanResult clanResult = null; if (clan != null) { clanResult = ClanResult.of(clan); diff --git a/src/main/java/com/faforever/api/config/elide/ElideConfig.java b/src/main/java/com/faforever/api/config/elide/ElideConfig.java index 10b1de7bd..3f17ada6e 100644 --- a/src/main/java/com/faforever/api/config/elide/ElideConfig.java +++ b/src/main/java/com/faforever/api/config/elide/ElideConfig.java @@ -15,8 +15,7 @@ import com.yahoo.elide.ElideSettingsBuilder; import com.yahoo.elide.core.EntityDictionary; import com.yahoo.elide.core.filter.dialect.RSQLFilterDialect; -import com.yahoo.elide.datastores.hibernate5.HibernateStore; -import com.yahoo.elide.datastores.hibernate5.HibernateStore.Builder; +import com.yahoo.elide.datastores.hibernate5.AbstractHibernateStore; import com.yahoo.elide.jsonapi.JsonApiMapper; import com.yahoo.elide.security.checks.Check; import com.yahoo.elide.utils.coerce.CoerceUtil; @@ -39,7 +38,7 @@ public class ElideConfig { public static final String DEFAULT_CACHE_NAME = "Elide.defaultCache"; @Bean - public Elide elide(HibernateStore hibernateStore, ObjectMapper objectMapper, EntityDictionary entityDictionary, ExtendedAuditLogger extendedAuditLogger) { + public Elide elide(AbstractHibernateStore hibernateStore, ObjectMapper objectMapper, EntityDictionary entityDictionary, ExtendedAuditLogger extendedAuditLogger) { RSQLFilterDialect rsqlFilterDialect = new RSQLFilterDialect(entityDictionary); registerAdditionalConverters(); @@ -55,8 +54,8 @@ public Elide elide(HibernateStore hibernateStore, ObjectMapper objectMapper, Ent @Bean @Profile("!" + ApplicationProfile.INTEGRATION_TEST) - HibernateStore hibernateStore(EntityManagerFactory entityManagerFactory) { - return new Builder(entityManagerFactory.unwrap(SessionFactory.class)).build(); + AbstractHibernateStore hibernateStore(EntityManagerFactory entityManagerFactory) { + return new AbstractHibernateStore.Builder(entityManagerFactory.unwrap(SessionFactory.class)).build(); } /** diff --git a/src/main/java/com/faforever/api/data/domain/Clan.java b/src/main/java/com/faforever/api/data/domain/Clan.java index b57af06b7..eb58b749d 100644 --- a/src/main/java/com/faforever/api/data/domain/Clan.java +++ b/src/main/java/com/faforever/api/data/domain/Clan.java @@ -29,7 +29,7 @@ @Entity @Table(name = "clan") @Include(rootLevel = true, type = Clan.TYPE_NAME) -@SharePermission(expression = IsEntityOwner.EXPRESSION) +@SharePermission @DeletePermission(expression = IsEntityOwner.EXPRESSION) @CreatePermission(expression = "Prefab.Role.All") @Setter diff --git a/src/main/java/com/faforever/api/data/domain/Login.java b/src/main/java/com/faforever/api/data/domain/Login.java index d999911e7..48a83d89c 100644 --- a/src/main/java/com/faforever/api/data/domain/Login.java +++ b/src/main/java/com/faforever/api/data/domain/Login.java @@ -12,8 +12,8 @@ import javax.persistence.OneToMany; import javax.persistence.OneToOne; import javax.persistence.Transient; -import java.util.ArrayList; -import java.util.List; +import java.util.HashSet; +import java.util.Set; import java.util.stream.Collectors; @MappedSuperclass @@ -24,14 +24,14 @@ public abstract class Login extends AbstractEntity implements OwnableEntity { private String email; private String steamId; private String userAgent; - private List bans; - private List userNotes; + private Set bans; + private Set userNotes; private LobbyGroup lobbyGroup; private String recentIpAddress; public Login() { - this.bans = new ArrayList<>(0); - this.userNotes = new ArrayList<>(0); + this.bans = new HashSet<>(0); + this.userNotes = new HashSet<>(0); } @Column(name = "login") @@ -65,19 +65,19 @@ public String getUserAgent() { @OneToMany(mappedBy = "player", fetch = FetchType.EAGER) // Permission is managed by BanInfo class @UpdatePermission(expression = IsModerator.EXPRESSION) - public List getBans() { + public Set getBans() { return this.bans; } @OneToMany(mappedBy = "player", fetch = FetchType.EAGER) @UpdatePermission(expression = IsModerator.EXPRESSION) - public List getUserNotes() { + public Set getUserNotes() { return this.userNotes; } @Transient - public List getActiveBans() { - return getBans().stream().filter(ban -> ban.getBanStatus() == BanStatus.BANNED).collect(Collectors.toList()); + public Set getActiveBans() { + return getBans().stream().filter(ban -> ban.getBanStatus() == BanStatus.BANNED).collect(Collectors.toSet()); } @Transient diff --git a/src/main/java/com/faforever/api/data/domain/Player.java b/src/main/java/com/faforever/api/data/domain/Player.java index bab0ebb12..44c0a143d 100644 --- a/src/main/java/com/faforever/api/data/domain/Player.java +++ b/src/main/java/com/faforever/api/data/domain/Player.java @@ -13,13 +13,14 @@ import javax.persistence.OneToOne; import javax.persistence.Table; import javax.persistence.Transient; -import java.util.List; +import java.util.HashSet; +import java.util.Set; @Entity @Table(name = "login") @Include(rootLevel = true, type = Player.TYPE_NAME) // Needed to change leader of a clan -@SharePermission(expression = "Prefab.Role.All") +@SharePermission @Setter @Type(Player.TYPE_NAME) public class Player extends Login { @@ -27,9 +28,9 @@ public class Player extends Login { public static final String TYPE_NAME = "player"; private Ladder1v1Rating ladder1v1Rating; private GlobalRating globalRating; - private List clanMemberships; - private List names; - private List avatarAssignments; + private Set clanMemberships = new HashSet<>(); + private Set names; + private Set avatarAssignments; @OneToOne(mappedBy = "player", fetch = FetchType.LAZY) @BatchSize(size = 1000) @@ -47,22 +48,22 @@ public GlobalRating getGlobalRating() { @UpdatePermission(expression = "Prefab.Role.All") @OneToMany(mappedBy = "player") @BatchSize(size = 1000) - public List getClanMemberships() { + public Set getClanMemberships() { return this.clanMemberships; } @Transient public Clan getClan() { - if (getClanMemberships() != null && getClanMemberships().size() == 1) { - return getClanMemberships().get(0).getClan(); - } - return null; + return getClanMemberships().stream() + .findFirst() + .map(ClanMembership::getClan) + .orElse(null); } // Permission is managed by NameRecord class @UpdatePermission(expression = "Prefab.Role.All") @OneToMany(mappedBy = "player") - public List getNames() { + public Set getNames() { return this.names; } @@ -70,7 +71,7 @@ public List getNames() { @UpdatePermission(expression = "Prefab.Role.All") @OneToMany(mappedBy = "player") @BatchSize(size = 1000) - public List getAvatarAssignments() { + public Set getAvatarAssignments() { return avatarAssignments; } diff --git a/src/main/java/com/faforever/api/data/package-info.java b/src/main/java/com/faforever/api/data/package-info.java index 89fd2cd15..530493b6a 100644 --- a/src/main/java/com/faforever/api/data/package-info.java +++ b/src/main/java/com/faforever/api/data/package-info.java @@ -1,7 +1,7 @@ /** * Contains classes to access data according to the JSON-API specification. */ -@SharePermission(expression = "Prefab.Role.All") +@SharePermission // Everybody can read from the api @ReadPermission(expression = "Prefab.Role.All") // By default restrict all data manipulation operation diff --git a/src/test/java/com/faforever/api/clan/ClanServiceTest.java b/src/test/java/com/faforever/api/clan/ClanServiceTest.java index dae5aae0c..e1f5df9b1 100644 --- a/src/test/java/com/faforever/api/clan/ClanServiceTest.java +++ b/src/test/java/com/faforever/api/clan/ClanServiceTest.java @@ -25,7 +25,6 @@ import org.springframework.security.jwt.Jwt; import java.io.IOException; -import java.util.ArrayList; import java.util.Collections; import java.util.Optional; @@ -68,7 +67,6 @@ public void createClanWhereLeaderIsAlreadyInAClan() { String description = "A cool clan for testing"; Player creator = new Player(); creator.setId(1); - creator.setClanMemberships(new ArrayList<>()); creator.getClanMemberships().add(new ClanMembership()); try { instance.create(clanName, tag, description, creator); @@ -87,7 +85,6 @@ public void createClanWithSameName() { Player creator = new Player(); creator.setId(1); - creator.setClanMemberships(new ArrayList<>()); when(clanRepository.findOneByName(clanName)).thenReturn(Optional.of(new Clan())); try { @@ -109,7 +106,6 @@ public void createClanWithSameTag() { Player creator = new Player(); creator.setId(1); - creator.setClanMemberships(new ArrayList<>()); when(clanRepository.findOneByName(clanName)).thenReturn(Optional.empty()); when(clanRepository.findOneByTag(tag)).thenReturn(Optional.of(new Clan())); @@ -133,7 +129,6 @@ public void createClanSuccessful() { Player creator = new Player(); creator.setId(1); - creator.setClanMemberships(new ArrayList<>()); when(clanRepository.findOneByName(clanName)).thenReturn(Optional.empty()); when(clanRepository.findOneByTag(tag)).thenReturn(Optional.empty()); @@ -340,7 +335,7 @@ public void acceptPlayerInvitationTokenPlayerIAlreadyInAClan() throws IOExceptio Player newMember = new Player(); newMember.setId(2); newMember.setClanMemberships( - Collections.singletonList(new ClanMembership().setClan(clan).setPlayer(newMember))); + Collections.singleton(new ClanMembership().setClan(clan).setPlayer(newMember))); long expire = System.currentTimeMillis() + 1000 * 3; Jwt jwtToken = Mockito.mock(Jwt.class); From b39892f5885580f13585397a990b32ace33a3468 Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Thu, 24 May 2018 22:58:27 +0200 Subject: [PATCH 02/10] Build elide correctly in integration test --- .../faforever/api/AbstractIntegrationTest.java | 3 +-- .../faforever/api/config/ElideTestConfig.java | 18 ------------------ .../api/config/elide/ElideConfig.java | 3 --- 3 files changed, 1 insertion(+), 23 deletions(-) delete mode 100644 src/inttest/java/com/faforever/api/config/ElideTestConfig.java diff --git a/src/inttest/java/com/faforever/api/AbstractIntegrationTest.java b/src/inttest/java/com/faforever/api/AbstractIntegrationTest.java index 0dde7fa1e..ff6853b20 100644 --- a/src/inttest/java/com/faforever/api/AbstractIntegrationTest.java +++ b/src/inttest/java/com/faforever/api/AbstractIntegrationTest.java @@ -1,7 +1,6 @@ package com.faforever.api; import com.faforever.api.config.ApplicationProfile; -import com.faforever.api.config.ElideTestConfig; import com.faforever.api.error.ErrorCode; import com.faforever.api.utils.OAuthHelper; import com.fasterxml.jackson.annotation.JsonInclude.Include; @@ -34,7 +33,7 @@ @RunWith(SpringRunner.class) @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) @ActiveProfiles(ApplicationProfile.INTEGRATION_TEST) -@Import({ElideTestConfig.class, OAuthHelper.class}) +@Import({OAuthHelper.class}) @Transactional @Sql(executionPhase = ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:sql/prepDefaultUser.sql") public abstract class AbstractIntegrationTest { diff --git a/src/inttest/java/com/faforever/api/config/ElideTestConfig.java b/src/inttest/java/com/faforever/api/config/ElideTestConfig.java deleted file mode 100644 index ee79e4ea5..000000000 --- a/src/inttest/java/com/faforever/api/config/ElideTestConfig.java +++ /dev/null @@ -1,18 +0,0 @@ -package com.faforever.api.config; - -import com.yahoo.elide.datastores.hibernate5.AbstractHibernateStore; -import org.hibernate.jpa.HibernateEntityManager; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Profile; - -import javax.persistence.EntityManager; - -@Configuration -@Profile(ApplicationProfile.INTEGRATION_TEST) -public class ElideTestConfig { - @Bean - AbstractHibernateStore hibernateStore(EntityManager entityManager) { - return new AbstractHibernateStore.Builder((HibernateEntityManager) entityManager).build(); - } -} diff --git a/src/main/java/com/faforever/api/config/elide/ElideConfig.java b/src/main/java/com/faforever/api/config/elide/ElideConfig.java index 3f17ada6e..bc491fbee 100644 --- a/src/main/java/com/faforever/api/config/elide/ElideConfig.java +++ b/src/main/java/com/faforever/api/config/elide/ElideConfig.java @@ -1,6 +1,5 @@ package com.faforever.api.config.elide; -import com.faforever.api.config.ApplicationProfile; import com.faforever.api.data.checks.BooleanChange; import com.faforever.api.data.checks.IsAuthenticated; import com.faforever.api.data.checks.IsClanMembershipDeletable; @@ -24,7 +23,6 @@ import org.hibernate.SessionFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Profile; import javax.persistence.EntityManagerFactory; import java.time.Duration; @@ -53,7 +51,6 @@ public Elide elide(AbstractHibernateStore hibernateStore, ObjectMapper objectMap } @Bean - @Profile("!" + ApplicationProfile.INTEGRATION_TEST) AbstractHibernateStore hibernateStore(EntityManagerFactory entityManagerFactory) { return new AbstractHibernateStore.Builder(entityManagerFactory.unwrap(SessionFactory.class)).build(); } From 37f93446b038bcc07a44fe16750b77e70b596980 Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Fri, 1 Jun 2018 12:54:58 +0200 Subject: [PATCH 03/10] Migration to Spring Boot 2 WIP --- build.gradle | 13 ++++--- gradle.properties | 10 ++--- .../api/clan/ClanControllerTest.java | 2 +- .../faforever/api/data/AvatarElideTest.java | 15 ++++++-- .../com/faforever/api/data/ClanElideTest.java | 18 ++++----- .../api/user/UsersControllerTest.java | 12 +++--- src/inttest/resources/config/application.yml | 5 +++ .../com/faforever/api/FafApiApplication.java | 4 -- .../avatar/AvatarAssignmentRepository.java | 2 + .../com/faforever/api/clan/ClanService.java | 24 ++++-------- .../com/faforever/api/config/CacheConfig.java | 38 +++++++++---------- .../config/security/WebSecurityConfig.java | 6 ++- .../api/data/domain/VotingChoice.java | 2 - .../api/data/domain/VotingQuestion.java | 2 - .../api/data/domain/VotingSubject.java | 2 - .../faforever/api/player/PlayerService.java | 3 +- .../api/security/OAuthApprovalController.java | 3 +- .../security/OAuthClientDetailsService.java | 4 +- .../com/faforever/api/user/UserService.java | 19 ++++------ .../faforever/api/voting/VotingService.java | 15 +++----- src/main/resources/config/application.yml | 6 +-- .../faforever/api/clan/ClanServiceTest.java | 22 +++++------ .../faforever/api/map/MapsControllerTest.java | 4 -- .../OAuthClientDetailsServiceTest.java | 4 +- .../faforever/api/user/UserServiceTest.java | 11 ++---- .../api/voting/VotingServiceTest.java | 28 +++++++------- 26 files changed, 130 insertions(+), 144 deletions(-) diff --git a/build.gradle b/build.gradle index ced9bd217..ce8d221ea 100644 --- a/build.gradle +++ b/build.gradle @@ -106,6 +106,7 @@ tasks.withType(Test) { apply plugin: 'java' apply plugin: 'org.springframework.boot' +apply plugin: 'io.spring.dependency-management' apply plugin: 'propdeps' apply plugin: 'idea' @@ -275,7 +276,9 @@ dependencyManagement { } dependencies { - compile("org.springframework.boot:spring-boot-starter-actuator") + runtime("org.springframework.boot:spring-boot-properties-migrator") + runtime("io.micrometer:micrometer-registry-prometheus") + compile("org.springframework.boot:spring-boot-starter-jdbc") compile("org.springframework.boot:spring-boot-starter-data-jpa") compile("org.springframework.boot:spring-boot-starter-web") @@ -283,14 +286,13 @@ dependencies { compile("org.springframework.boot:spring-boot-starter-security") compile("org.springframework.boot:spring-boot-starter-thymeleaf") compile("org.springframework.boot:spring-boot-starter-mail") - compile("de.codecentric:spring-boot-admin-starter-client:${springBootAdminClientVersion}") + compile("com.github.ben-manes.caffeine:caffeine") compile("com.github.FAForever:faf-java-commons:${fafCommonsVersion}") compile("org.kohsuke:github-api:${githubApiVersion}") compile("org.jolokia:jolokia-core") compile("org.springframework.security:spring-security-jwt:${springSecurityJwtVersion}") compile("org.springframework.security.oauth:spring-security-oauth2:${springSecurityOauth2Version}") - compile("org.springframework:spring-context-support:${springContextSupportVersion}") compile("org.eclipse.jgit:org.eclipse.jgit:${jgitVersionn}") compile("org.jetbrains:annotations:${jetbrainsAnnotationsVersion}") compile("com.google.guava:guava:${guavaVersion}") @@ -298,6 +300,8 @@ dependencies { compile("io.springfox:springfox-swagger2:${swaggerVersion}") compile("io.swagger:swagger-core:${swaggerCoreVersion}") compile("javax.inject:javax.inject:${javaxInjectVersion}") + // When switching from Java 8 to 9, I got "class file for javax.interceptor.interceptorbinding not found". Adding this fixed it, but IDK what caused it. + compile("javax.interceptor:javax.interceptor-api:${javaxInterceptorApiVersion}") compile("com.yahoo.elide:elide-core:${elideVersion}") compile("com.yahoo.elide:elide-swagger:${elideVersion}") compile("com.yahoo.elide:elide-datastore-hibernate5:${elideVersion}") @@ -313,9 +317,6 @@ dependencies { compile("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${jacksonDatatypeJsr310Version}") compile("com.mandrillapp.wrapper.lutung:lutung:${lutungVersion}") compile("org.apache.commons:commons-compress:${commonsCompressVersion}") - compile("io.prometheus:simpleclient_hotspot:${prometheusVersion}") - compile("io.prometheus:simpleclient_servlet:${prometheusVersion}") - compile("io.prometheus:simpleclient_spring_boot:${prometheusVersion}") compile("org.json:json:${jsonVersion}") compile("com.github.jasminb:jsonapi-converter:${jsonapiConverterVersion}") diff --git a/gradle.properties b/gradle.properties index d4bab6548..4c327d428 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,9 +1,8 @@ profile=dev -springBootVersion=1.5.13.RELEASE +springBootVersion=2.0.3.RELEASE jjwtVersion=0.7.0 javaxInjectVersion=1 elideVersion=4.2.3 -hibernate.version=5.2.17.Final mysqlConnectorVersion=6.0.5 gradleDockerVersion=3.2.1 propdepsVersion=0.0.7 @@ -13,10 +12,9 @@ coverallsGradlePluginVersion=2.4.0 hikariCpVersion=2.4.6 springSecurityJwtVersion=1.0.7.RELEASE springSecurityOauth2Version=2.0.12.RELEASE -springContextSupportVersion=4.2.2.RELEASE guavaVersion=21.0-rc1 jetbrainsAnnotationsVersion=13.0 -springBootAdminClientVersion=1.4.5 +springBootAdminClientVersion=2.0.0 luajVersion=3.0.1 nocatchVersion=1.1 junitAddonsVersion=1.4 @@ -26,13 +24,13 @@ jgitVersionn=4.5.0.201609210915-r fafCommonsVersion=09ffb08f9538f66e6e9bccabceae1e6123a3fa27 h2Version=1.4.193 jacksonDatatypeJsr310Version=2.8.11 -mockitoVersion=2.7.0 +mockitoVersion=2.19.0 lutungVersion=0.0.7 commonsCompressVersion=1.13 jsonPath=2.2.0 jsonPathAssert=2.2.0 thymeleafVersion=3.0.5.RELEASE -prometheusVersion=0.0.26 jsonapiConverterVersion=0.8 codacyCoverageReporterVersion=4.0.0 jsonVersion=20180130 +javaxInterceptorApiVersion=1.2 diff --git a/src/inttest/java/com/faforever/api/clan/ClanControllerTest.java b/src/inttest/java/com/faforever/api/clan/ClanControllerTest.java index 8002abdb2..57d4462b5 100644 --- a/src/inttest/java/com/faforever/api/clan/ClanControllerTest.java +++ b/src/inttest/java/com/faforever/api/clan/ClanControllerTest.java @@ -67,7 +67,7 @@ public void meDataWithoutClan() throws Exception { @WithUserDetails(AUTH_CLAN_MEMBER) public void meDataWithClan() throws Exception { Player player = getPlayer(); - Clan clan = clanRepository.findOne(1); + Clan clan = clanRepository.findById(1).get(); mockMvc.perform( get("/clans/me/")) diff --git a/src/inttest/java/com/faforever/api/data/AvatarElideTest.java b/src/inttest/java/com/faforever/api/data/AvatarElideTest.java index 45dade190..696a964b6 100644 --- a/src/inttest/java/com/faforever/api/data/AvatarElideTest.java +++ b/src/inttest/java/com/faforever/api/data/AvatarElideTest.java @@ -39,7 +39,7 @@ public class AvatarElideTest extends AbstractIntegrationTest { @Test public void getUnusedAvatar() throws Exception { - Avatar avatar = avatarRepository.findOne(1); + Avatar avatar = avatarRepository.findById(1).get(); mockMvc.perform(get("/data/avatar/1")) .andExpect(status().isOk()) @@ -52,7 +52,7 @@ public void getUnusedAvatar() throws Exception { @Test public void getAvatarWithPlayer() throws Exception { - Avatar avatar = avatarRepository.findOne(2); + Avatar avatar = avatarRepository.findById(2).get(); mockMvc.perform(get("/data/avatar/2")) .andExpect(status().isOk()) @@ -80,7 +80,16 @@ public void moderatorCanAssignAvatar() throws Exception { ) )) ).andExpect(status().isCreated()); - final Optional createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); + Optional createdAssignment = avatarAssignmentRepository.findOneByAvatarIdAndPlayerId(1, 1); + createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); + createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); + createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); + createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); + createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); + createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); + createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); + createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); + createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); assertThat(createdAssignment.isPresent(), is(true)); assertThat(createdAssignment.get().getPlayer().getId(), is(player.getId())); assertThat(createdAssignment.get().getAvatar().getId(), is(avatar.getId())); diff --git a/src/inttest/java/com/faforever/api/data/ClanElideTest.java b/src/inttest/java/com/faforever/api/data/ClanElideTest.java index 369470468..7964c18a1 100644 --- a/src/inttest/java/com/faforever/api/data/ClanElideTest.java +++ b/src/inttest/java/com/faforever/api/data/ClanElideTest.java @@ -46,12 +46,12 @@ public class ClanElideTest extends AbstractIntegrationTest { @Test @WithUserDetails(AUTH_CLAN_LEADER) public void canDeleteMemberOfOwnClan() throws Exception { - assertNotNull(playerRepository.findOne(12).getClan()); + assertNotNull(playerRepository.findById(12).get().getClan()); mockMvc.perform( delete("/data/clanMembership/2")) // magic value from prepClanData.sql .andExpect(status().isNoContent()); - assertNull(playerRepository.findOne(12).getClan()); + assertNull(playerRepository.findById(12).get().getClan()); } @Test @@ -75,13 +75,13 @@ public void cannotDeleteLeaderFromClan() throws Exception { @Test @WithUserDetails(AUTH_CLAN_MEMBER) public void canLeaveClan() throws Exception { - assertNotNull(playerRepository.findOne(12).getClan()); + assertNotNull(playerRepository.findById(12).get().getClan()); mockMvc.perform( delete("/data/clanMembership/2")) // magic value from prepClanData.sql .andExpect(status().isNoContent()); - assertNull(playerRepository.findOne(12).getClan()); + assertNull(playerRepository.findById(12).get().getClan()); } @Test @@ -105,20 +105,20 @@ public void getFilteredPlayerForClanInvite() throws Exception { @Test @WithUserDetails(AUTH_CLAN_LEADER) public void canTransferLeadershipAsLeader() throws Exception { - assertThat(clanRepository.findOne(1).getLeader().getLogin(), is(AUTH_CLAN_LEADER)); + assertThat(clanRepository.findById(1).get().getLeader().getLogin(), is(AUTH_CLAN_LEADER)); mockMvc.perform( patch("/data/clan/1") .content(generateTransferLeadershipContent(1, 12))) // magic value from prepClanData.sql .andExpect(status().isNoContent()); - assertThat(clanRepository.findOne(1).getLeader().getLogin(), is(AUTH_CLAN_MEMBER)); + assertThat(clanRepository.findById(1).get().getLeader().getLogin(), is(AUTH_CLAN_MEMBER)); } @Test @WithUserDetails(AUTH_CLAN_MEMBER) public void cannotTransferLeadershipAsMember() throws Exception { - assertThat(clanRepository.findOne(1).getLeader().getLogin(), is(AUTH_CLAN_LEADER)); + assertThat(clanRepository.findById(1).get().getLeader().getLogin(), is(AUTH_CLAN_LEADER)); mockMvc.perform( patch("/data/clan/1") @@ -126,7 +126,7 @@ public void cannotTransferLeadershipAsMember() throws Exception { .andExpect(status().isForbidden()) .andExpect(jsonPath("$.errors[0]", is("ForbiddenAccessException"))); - assertThat(clanRepository.findOne(1).getLeader().getLogin(), is(AUTH_CLAN_LEADER)); + assertThat(clanRepository.findById(1).get().getLeader().getLogin(), is(AUTH_CLAN_LEADER)); } @Test @@ -174,7 +174,7 @@ public void canDeleteClanAsLeader() throws Exception { .andExpect(status().isNoContent()); // TODO: Catch javax.validation.ConstraintViolationException and wrap it into a regular ApiException assertFalse(clanRepository.findOneByName("Alpha Clan").isPresent()); - clanMember.forEach(player -> assertNull(playerRepository.findOne(player.getId()).getClan())); + clanMember.forEach(player -> assertNull(playerRepository.findById(player.getId()).get().getClan())); } @Test diff --git a/src/inttest/java/com/faforever/api/user/UsersControllerTest.java b/src/inttest/java/com/faforever/api/user/UsersControllerTest.java index d505814c3..9135f7661 100644 --- a/src/inttest/java/com/faforever/api/user/UsersControllerTest.java +++ b/src/inttest/java/com/faforever/api/user/UsersControllerTest.java @@ -317,7 +317,7 @@ public void buildSteamLinkUrl() throws Exception { @Test @WithAnonymousUser public void linkToSteam() throws Exception { - assertThat(userRepository.findOne(1).getSteamId(), nullValue()); + assertThat(userRepository.findById(1).get().getSteamId(), nullValue()); String steamId = "1234"; String callbackUrl = "callbackUrl"; @@ -337,7 +337,7 @@ public void linkToSteam() throws Exception { .andExpect(status().isFound()) .andExpect(redirectedUrl(callbackUrl)); - assertThat(userRepository.findOne(2).getSteamId(), is(steamId)); + assertThat(userRepository.findById(2).get().getSteamId(), is(steamId)); } @Test @@ -367,7 +367,7 @@ public void changeUsernameWithWrongScope() throws Exception { @Test @WithUserDetails(AUTH_USER) public void changeUsernameSuccess() throws Exception { - assertThat(userRepository.findOne(1).getLogin(), is(AUTH_USER)); + assertThat(userRepository.findById(1).get().getLogin(), is(AUTH_USER)); MultiValueMap params = new HttpHeaders(); params.add("newUsername", NEW_USER); @@ -378,13 +378,13 @@ public void changeUsernameSuccess() throws Exception { .params(params)) .andExpect(status().isOk()); - assertThat(userRepository.findOne(1).getLogin(), is(NEW_USER)); + assertThat(userRepository.findById(1).get().getLogin(), is(NEW_USER)); } @Test @WithUserDetails(AUTH_MODERATOR) public void changeUsernameTooEarly() throws Exception { - assertThat(userRepository.findOne(2).getLogin(), is(AUTH_MODERATOR)); + assertThat(userRepository.findById(2).get().getLogin(), is(AUTH_MODERATOR)); MultiValueMap params = new HttpHeaders(); params.add("newUsername", NEW_USER); @@ -398,6 +398,6 @@ public void changeUsernameTooEarly() throws Exception { assertApiError(result, ErrorCode.USERNAME_CHANGE_TOO_EARLY); - assertThat(userRepository.findOne(2).getLogin(), is(AUTH_MODERATOR)); + assertThat(userRepository.findById(2).get().getLogin(), is(AUTH_MODERATOR)); } } diff --git a/src/inttest/resources/config/application.yml b/src/inttest/resources/config/application.yml index a43c661d8..6e828e8ad 100644 --- a/src/inttest/resources/config/application.yml +++ b/src/inttest/resources/config/application.yml @@ -70,3 +70,8 @@ faf-api: mautic: client-id: banana client-secret: banana + + +logging: + level: + org.springframework.security: DEBUG diff --git a/src/main/java/com/faforever/api/FafApiApplication.java b/src/main/java/com/faforever/api/FafApiApplication.java index 4fa300633..1f6ee9d70 100644 --- a/src/main/java/com/faforever/api/FafApiApplication.java +++ b/src/main/java/com/faforever/api/FafApiApplication.java @@ -1,16 +1,12 @@ package com.faforever.api; import com.faforever.api.config.FafApiProperties; -import io.prometheus.client.spring.boot.EnablePrometheusEndpoint; -import io.prometheus.client.spring.boot.EnableSpringBootMetricsCollector; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.transaction.annotation.EnableTransactionManagement; @SpringBootApplication -@EnablePrometheusEndpoint -@EnableSpringBootMetricsCollector @EnableTransactionManagement @EnableConfigurationProperties({FafApiProperties.class}) public class FafApiApplication { diff --git a/src/main/java/com/faforever/api/avatar/AvatarAssignmentRepository.java b/src/main/java/com/faforever/api/avatar/AvatarAssignmentRepository.java index d85fb03e9..1e13219d0 100644 --- a/src/main/java/com/faforever/api/avatar/AvatarAssignmentRepository.java +++ b/src/main/java/com/faforever/api/avatar/AvatarAssignmentRepository.java @@ -12,5 +12,7 @@ public interface AvatarAssignmentRepository extends JpaRepository { Optional findOneByAvatarAndPlayer(Avatar avatar, Player player); + Optional findOneByAvatarIdAndPlayerId(int avatarId, int playerId); + Optional findOneById(Integer i); } diff --git a/src/main/java/com/faforever/api/clan/ClanService.java b/src/main/java/com/faforever/api/clan/ClanService.java index 66bdf4fcc..8789387f1 100644 --- a/src/main/java/com/faforever/api/clan/ClanService.java +++ b/src/main/java/com/faforever/api/clan/ClanService.java @@ -86,19 +86,15 @@ Clan create(String name, String tag, String description, Player creator) { @SneakyThrows String generatePlayerInvitationToken(Player requester, int newMemberId, int clanId) { - Clan clan = clanRepository.findOne(clanId); + Clan clan = clanRepository.findById(clanId) + .orElseThrow(() -> new ApiException(new Error(ErrorCode.CLAN_NOT_EXISTS, clanId))); - if (clan == null) { - throw 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)); } - Player newMember = playerRepository.findOne(newMemberId); - if (newMember == null) { - throw new ApiException(new Error(ErrorCode.CLAN_GENERATE_LINK_PLAYER_NOT_FOUND, newMemberId)); - } + Player newMember = playerRepository.findById(newMemberId) + .orElseThrow(() -> new ApiException(new Error(ErrorCode.CLAN_GENERATE_LINK_PLAYER_NOT_FOUND, newMemberId))); long expire = Instant.now() .plus(fafApiProperties.getClan().getInviteLinkExpireDurationMinutes(), ChronoUnit.MINUTES) @@ -121,16 +117,12 @@ void acceptPlayerInvitationToken(String stringToken, Authentication authenticati final Integer clanId = invitation.getClan().getId(); Player player = playerService.getPlayer(authentication); - Clan clan = clanRepository.findOne(clanId); + Clan clan = clanRepository.findById(clanId) + .orElseThrow(() -> new ApiException(new Error(ErrorCode.CLAN_NOT_EXISTS, clanId))); - if (clan == null) { - throw 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 = playerRepository.findOne(invitation.getNewMember().getId()); - if (newMember == null) { - throw new ProgrammingError("ClanMember does not exist: " + invitation.getNewMember().getId()); - } if (player.getId() != newMember.getId()) { throw new ApiException(new Error(ErrorCode.CLAN_ACCEPT_WRONG_PLAYER)); diff --git a/src/main/java/com/faforever/api/config/CacheConfig.java b/src/main/java/com/faforever/api/config/CacheConfig.java index d203be0b1..b83960303 100644 --- a/src/main/java/com/faforever/api/config/CacheConfig.java +++ b/src/main/java/com/faforever/api/config/CacheConfig.java @@ -14,7 +14,7 @@ import com.faforever.api.data.domain.ModVersion; import org.springframework.cache.CacheManager; import org.springframework.cache.annotation.EnableCaching; -import org.springframework.cache.guava.GuavaCache; +import org.springframework.cache.caffeine.CaffeineCache; import org.springframework.cache.interceptor.AbstractCacheResolver; import org.springframework.cache.interceptor.CacheOperationInvocationContext; import org.springframework.cache.interceptor.CacheResolver; @@ -34,7 +34,7 @@ import static com.faforever.api.leaderboard.LeaderboardService.LEADERBOARD_GLOBAL_CACHE_NAME; import static com.faforever.api.leaderboard.LeaderboardService.LEADERBOARD_RANKED_1V1_CACHE_NAME; import static com.faforever.api.security.OAuthClientDetailsService.CLIENTS_CACHE_NAME; -import static com.google.common.cache.CacheBuilder.newBuilder; +import static com.github.benmanes.caffeine.cache.Caffeine.newBuilder; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; @@ -48,24 +48,24 @@ public CacheManager cacheManager() { SimpleCacheManager cacheManager = new SimpleCacheManager(); cacheManager.setCaches(Arrays.asList( // Elide entity caches - new GuavaCache(ElideConfig.DEFAULT_CACHE_NAME, newBuilder().maximumSize(0).build()), - new GuavaCache(Avatar.TYPE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), - new GuavaCache(AvatarAssignment.TYPE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), - new GuavaCache(Achievement.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), - new GuavaCache(Clan.TYPE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), - new GuavaCache(Event.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), - new GuavaCache(FeaturedMod.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), - new GuavaCache(Map.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), - new GuavaCache(MapVersion.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), - new GuavaCache(MapStatistics.TYPE_NAME, newBuilder().expireAfterWrite(1, MINUTES).build()), - new GuavaCache(Mod.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), - new GuavaCache(ModVersion.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), + new CaffeineCache(ElideConfig.DEFAULT_CACHE_NAME, newBuilder().maximumSize(0).build()), + new CaffeineCache(Avatar.TYPE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), + new CaffeineCache(AvatarAssignment.TYPE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), + new CaffeineCache(Achievement.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), + new CaffeineCache(Clan.TYPE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), + new CaffeineCache(Event.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), + new CaffeineCache(FeaturedMod.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), + new CaffeineCache(Map.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), + new CaffeineCache(MapVersion.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), + new CaffeineCache(MapStatistics.TYPE_NAME, newBuilder().expireAfterWrite(1, MINUTES).build()), + new CaffeineCache(Mod.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), + new CaffeineCache(ModVersion.TYPE_NAME, newBuilder().expireAfterWrite(60, MINUTES).build()), // Other caches - new GuavaCache(CHALLONGE_READ_CACHE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), - new GuavaCache(LEADERBOARD_RANKED_1V1_CACHE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), - new GuavaCache(LEADERBOARD_GLOBAL_CACHE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), - new GuavaCache(FEATURED_MOD_FILES_CACHE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), - new GuavaCache(CLIENTS_CACHE_NAME, newBuilder().expireAfterWrite(5, SECONDS).build()) + new CaffeineCache(CHALLONGE_READ_CACHE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), + new CaffeineCache(LEADERBOARD_RANKED_1V1_CACHE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), + new CaffeineCache(LEADERBOARD_GLOBAL_CACHE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), + new CaffeineCache(FEATURED_MOD_FILES_CACHE_NAME, newBuilder().expireAfterWrite(5, MINUTES).build()), + new CaffeineCache(CLIENTS_CACHE_NAME, newBuilder().expireAfterWrite(5, SECONDS).build()) )); return cacheManager; } 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 cc529f7b8..b6cf75d40 100644 --- a/src/main/java/com/faforever/api/config/security/WebSecurityConfig.java +++ b/src/main/java/com/faforever/api/config/security/WebSecurityConfig.java @@ -10,13 +10,13 @@ import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.InternalAuthenticationServiceException; import org.springframework.security.authentication.LockedException; -import org.springframework.security.authentication.encoding.ShaPasswordEncoder; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; import org.springframework.security.core.userdetails.UserDetailsService; +import org.springframework.security.crypto.password.MessageDigestPasswordEncoder; import org.springframework.security.web.authentication.AuthenticationFailureHandler; import org.springframework.security.web.authentication.ExceptionMappingAuthenticationFailureHandler; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; @@ -45,9 +45,11 @@ public void developUserDetails(AuthenticationManagerBuilder auth) throws Excepti @Inject public void prodUserDetails(AuthenticationManagerBuilder auth, UserDetailsService userDetailsService) throws Exception { + // TODO: Use a proper, backward-compatible password encryption + auth .userDetailsService(userDetailsService) - .passwordEncoder(new ShaPasswordEncoder(256)); + .passwordEncoder(new MessageDigestPasswordEncoder("SHA-256")); } @Bean diff --git a/src/main/java/com/faforever/api/data/domain/VotingChoice.java b/src/main/java/com/faforever/api/data/domain/VotingChoice.java index c8c24d2a3..aa96a8a96 100644 --- a/src/main/java/com/faforever/api/data/domain/VotingChoice.java +++ b/src/main/java/com/faforever/api/data/domain/VotingChoice.java @@ -12,7 +12,6 @@ import com.yahoo.elide.annotation.Exclude; import com.yahoo.elide.annotation.Include; import com.yahoo.elide.annotation.ReadPermission; -import com.yahoo.elide.annotation.SharePermission; import com.yahoo.elide.annotation.UpdatePermission; import lombok.Setter; @@ -31,7 +30,6 @@ @Entity @Table(name = "voting_choice") @ReadPermission(expression = "Prefab.Role.All") -@SharePermission(expression = IsModerator.EXPRESSION) @DeletePermission(expression = IsModerator.EXPRESSION) @UpdatePermission(expression = IsModerator.EXPRESSION) @CreatePermission(expression = IsModerator.EXPRESSION) diff --git a/src/main/java/com/faforever/api/data/domain/VotingQuestion.java b/src/main/java/com/faforever/api/data/domain/VotingQuestion.java index d6b44623c..7dc51a4c9 100644 --- a/src/main/java/com/faforever/api/data/domain/VotingQuestion.java +++ b/src/main/java/com/faforever/api/data/domain/VotingQuestion.java @@ -11,7 +11,6 @@ import com.yahoo.elide.annotation.DeletePermission; import com.yahoo.elide.annotation.Include; import com.yahoo.elide.annotation.ReadPermission; -import com.yahoo.elide.annotation.SharePermission; import com.yahoo.elide.annotation.UpdatePermission; import lombok.EqualsAndHashCode; import lombok.Setter; @@ -36,7 +35,6 @@ @Entity @Table(name = "voting_question") @ReadPermission(expression = "Prefab.Role.All") -@SharePermission(expression = IsModerator.EXPRESSION) @DeletePermission(expression = IsModerator.EXPRESSION) @UpdatePermission(expression = IsModerator.EXPRESSION) @CreatePermission(expression = IsModerator.EXPRESSION) diff --git a/src/main/java/com/faforever/api/data/domain/VotingSubject.java b/src/main/java/com/faforever/api/data/domain/VotingSubject.java index 2d23121f1..cfc20c550 100644 --- a/src/main/java/com/faforever/api/data/domain/VotingSubject.java +++ b/src/main/java/com/faforever/api/data/domain/VotingSubject.java @@ -12,7 +12,6 @@ import com.yahoo.elide.annotation.Exclude; import com.yahoo.elide.annotation.Include; import com.yahoo.elide.annotation.ReadPermission; -import com.yahoo.elide.annotation.SharePermission; import com.yahoo.elide.annotation.UpdatePermission; import lombok.Setter; @@ -32,7 +31,6 @@ @Table(name = "voting_subject") @Include(rootLevel = true, type = VotingSubject.TYPE_NAME) @ReadPermission(expression = "Prefab.Role.All") -@SharePermission(expression = IsModerator.EXPRESSION) @DeletePermission(expression = IsModerator.EXPRESSION) @UpdatePermission(expression = IsModerator.EXPRESSION) @CreatePermission(expression = IsModerator.EXPRESSION) diff --git a/src/main/java/com/faforever/api/player/PlayerService.java b/src/main/java/com/faforever/api/player/PlayerService.java index 288b34f70..d0a64cc9c 100644 --- a/src/main/java/com/faforever/api/player/PlayerService.java +++ b/src/main/java/com/faforever/api/player/PlayerService.java @@ -25,7 +25,8 @@ public Player getPlayer(Authentication authentication) { if (authentication != null && authentication.getPrincipal() != null && authentication.getPrincipal() instanceof FafUserDetails) { - return playerRepository.findOne(((FafUserDetails) authentication.getPrincipal()).getId()); + return playerRepository.findById(((FafUserDetails) authentication.getPrincipal()).getId()) + .orElseThrow(() -> new ApiException(new Error(TOKEN_INVALID))); } throw new ApiException(new Error(TOKEN_INVALID)); } diff --git a/src/main/java/com/faforever/api/security/OAuthApprovalController.java b/src/main/java/com/faforever/api/security/OAuthApprovalController.java index 0d5ae50d9..4ac41d6ee 100644 --- a/src/main/java/com/faforever/api/security/OAuthApprovalController.java +++ b/src/main/java/com/faforever/api/security/OAuthApprovalController.java @@ -27,7 +27,8 @@ public OAuthApprovalController(OAuthClientRepository oAuthClientRepository) { @GetMapping("/confirm_access") public ModelAndView confirmAccess(Map model) { final AuthorizationRequest authorizationRequest = (AuthorizationRequest) model.get("authorizationRequest"); - final OAuthClient client = oAuthClientRepository.findOne(authorizationRequest.getClientId()); + final OAuthClient client = oAuthClientRepository.findById(authorizationRequest.getClientId()) + .orElseThrow(() -> new IllegalArgumentException("No client with ID: " + authorizationRequest.getClientId())); Set scopes = getScopesFromScopeString(authorizationRequest.getScope()); return new ModelAndView("oauth_confirm_access") diff --git a/src/main/java/com/faforever/api/security/OAuthClientDetailsService.java b/src/main/java/com/faforever/api/security/OAuthClientDetailsService.java index 3b91ad962..e4deed103 100644 --- a/src/main/java/com/faforever/api/security/OAuthClientDetailsService.java +++ b/src/main/java/com/faforever/api/security/OAuthClientDetailsService.java @@ -10,8 +10,6 @@ import org.springframework.security.oauth2.provider.ClientRegistrationException; import org.springframework.stereotype.Service; -import java.util.Optional; - @Service public class OAuthClientDetailsService implements ClientDetailsService { @@ -27,7 +25,7 @@ public OAuthClientDetailsService(OAuthClientRepository oAuthClientRepository, Fa @Override @Cacheable(CLIENTS_CACHE_NAME) public ClientDetails loadClientByClientId(String clientId) throws ClientRegistrationException { - OAuthClient oAuthClient = Optional.ofNullable(oAuthClientRepository.findOne(clientId)) + OAuthClient oAuthClient = oAuthClientRepository.findById(clientId) .orElseThrow(() -> new ClientRegistrationException("Unknown client: " + clientId)); OAuthClientDetails clientDetails = new OAuthClientDetails(oAuthClient); diff --git a/src/main/java/com/faforever/api/user/UserService.java b/src/main/java/com/faforever/api/user/UserService.java index 40932fcf6..24974cb3b 100644 --- a/src/main/java/com/faforever/api/user/UserService.java +++ b/src/main/java/com/faforever/api/user/UserService.java @@ -194,7 +194,7 @@ public void changeLogin(String newLogin, User user, String ipAddress) { log.debug("Changing username for user ''{}'' to ''{}''", user, newLogin); NameRecord nameRecord = new NameRecord() .setName(user.getLogin()) - .setPlayer(playerRepository.findOne(user.getId())); + .setPlayer(playerRepository.getOne(user.getId())); nameRecordRepository.save(nameRecord); user.setLogin(newLogin); @@ -257,11 +257,8 @@ void claimPasswordResetToken(String token) { int userId = Integer.parseInt(claims.get(KEY_USER_ID)); String newPassword = claims.get(KEY_PASSWORD); - User user = userRepository.findOne(userId); - - if (user == null) { - throw new ApiException(new Error(ErrorCode.TOKEN_INVALID)); - } + User user = userRepository.findById(userId) + .orElseThrow(() -> new ApiException(new Error(ErrorCode.TOKEN_INVALID))); setPassword(user, newPassword); } @@ -278,7 +275,8 @@ public User getUser(Authentication authentication) { if (authentication != null && authentication.getPrincipal() != null && authentication.getPrincipal() instanceof FafUserDetails) { - return userRepository.findOne(((FafUserDetails) authentication.getPrincipal()).getId()); + return userRepository.findById(((FafUserDetails) authentication.getPrincipal()).getId()) + .orElseThrow(() -> new ApiException(new Error(TOKEN_INVALID))); } throw new ApiException(new Error(TOKEN_INVALID)); } @@ -306,11 +304,8 @@ public SteamLinkResult linkToSteam(String token, String steamId) { log.debug("linkToSteam requested for steamId '{}' with token: {}", steamId, token); Map attributes = fafTokenService.resolveToken(FafTokenType.LINK_TO_STEAM, token); - User user = userRepository.findOne(Integer.parseInt(attributes.get(KEY_USER_ID))); - - if (user == null) { - throw new ApiException(new Error(ErrorCode.TOKEN_INVALID)); - } + User user = userRepository.findById(Integer.parseInt(attributes.get(KEY_USER_ID))) + .orElseThrow(() -> new ApiException(new Error(ErrorCode.TOKEN_INVALID))); String callbackUrl = attributes.get(KEY_STEAM_LINK_CALLBACK_URL); if (!steamService.ownsForgedAlliance(steamId)) { diff --git a/src/main/java/com/faforever/api/voting/VotingService.java b/src/main/java/com/faforever/api/voting/VotingService.java index 47d8519a1..4cb81126a 100644 --- a/src/main/java/com/faforever/api/voting/VotingService.java +++ b/src/main/java/com/faforever/api/voting/VotingService.java @@ -46,14 +46,13 @@ public void saveVote(Vote vote, Player player) { vote.setVotingAnswers(Collections.emptySet()); } - VotingSubject subject = votingSubjectRepository.findOne(vote.getVotingSubject().getId()); + VotingSubject subject = votingSubjectRepository.findById(vote.getVotingSubject().getId()) + .orElseThrow(() -> new IllegalArgumentException("Subject of vote not found")); vote.getVotingAnswers().forEach(votingAnswer -> { VotingChoice votingChoice = votingAnswer.getVotingChoice(); - VotingChoice one = votingChoiceRepository.findOne(votingChoice.getId()); - if (one == null) { - throw new ApiException(new Error(ErrorCode.VOTING_CHOICE_DOES_NOT_EXIST, votingChoice.getId())); - } + VotingChoice one = votingChoiceRepository.findById(votingChoice.getId()) + .orElseThrow(() -> new ApiException(new Error(ErrorCode.VOTING_CHOICE_DOES_NOT_EXIST, votingChoice.getId()))); votingAnswer.setVotingChoice(one); votingAnswer.setVote(vote); }); @@ -97,10 +96,8 @@ public void saveVote(Vote vote, Player player) { } private List ableToVote(Player player, int votingSubjectId) { - VotingSubject subject = votingSubjectRepository.findOne(votingSubjectId); - if (subject == null) { - throw new ApiException(new Error(ErrorCode.VOTING_SUBJECT_DOES_NOT_EXIST, votingSubjectId)); - } + VotingSubject subject = votingSubjectRepository.findById(votingSubjectId) + .orElseThrow(() -> new ApiException(new Error(ErrorCode.VOTING_SUBJECT_DOES_NOT_EXIST, votingSubjectId))); List errors = new ArrayList<>(); Optional byPlayerAndVotingSubject = voteRepository.findByPlayerAndVotingSubjectId(player, votingSubjectId); diff --git a/src/main/resources/config/application.yml b/src/main/resources/config/application.yml index 9bfe68f68..783bee95f 100644 --- a/src/main/resources/config/application.yml +++ b/src/main/resources/config/application.yml @@ -49,7 +49,7 @@ spring: WRITE_DATES_AS_TIMESTAMPS: false profiles: active: ${API_PROFILE:dev} - http: + servlet: multipart: max-file-size: 300MB max-request-size: 300MB @@ -63,12 +63,12 @@ spring: server: # Mind that this is configured in the docker compose file as well (that is, in the gradle script that generates it) port: ${API_PORT:8010} - context-path: ${CONTEXT_PATH:/} + servlet: + context-path: ${CONTEXT_PATH:/} jetty: max-http-post-size: 314572800 security: - enable-csrf: true oauth2: resource: filter-order: 3 diff --git a/src/test/java/com/faforever/api/clan/ClanServiceTest.java b/src/test/java/com/faforever/api/clan/ClanServiceTest.java index e1f5df9b1..d084a9e66 100644 --- a/src/test/java/com/faforever/api/clan/ClanServiceTest.java +++ b/src/test/java/com/faforever/api/clan/ClanServiceTest.java @@ -173,7 +173,7 @@ public void generatePlayerInvitationTokenFromNonLeader() throws IOException { Clan clan = ClanFactory.builder().leader(leader).build(); - when(clanRepository.findOne(clan.getId())).thenReturn(clan); + when(clanRepository.findById(clan.getId())).thenReturn(Optional.of(clan)); try { instance.generatePlayerInvitationToken(requester, newMember.getId(), clan.getId()); @@ -191,7 +191,7 @@ public void generatePlayerInvitationTokenInvalidPlayer() throws IOException { Clan clan = ClanFactory.builder().leader(requester).build(); - when(clanRepository.findOne(clan.getId())).thenReturn(clan); + when(clanRepository.findById(clan.getId())).thenReturn(Optional.of(clan)); try { instance.generatePlayerInvitationToken(requester, 42, clan.getId()); @@ -214,8 +214,8 @@ public void generatePlayerInvitationToken() throws IOException { FafApiProperties props = new FafApiProperties(); - when(clanRepository.findOne(clan.getId())).thenReturn(clan); - when(playerRepository.findOne(newMember.getId())).thenReturn(newMember); + when(clanRepository.findById(clan.getId())).thenReturn(Optional.of(clan)); + when(playerRepository.findById(newMember.getId())).thenReturn(Optional.of(newMember)); when(fafApiProperties.getClan()).thenReturn(props.getClan()); instance.generatePlayerInvitationToken(requester, newMember.getId(), clan.getId()); @@ -283,7 +283,7 @@ public void acceptPlayerInvitationTokenInvalidPlayer() throws IOException { 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); + when(clanRepository.findById(clan.getId())).thenReturn(Optional.of(clan)); try { instance.acceptPlayerInvitationToken(stringToken, null); @@ -313,8 +313,8 @@ public void acceptPlayerInvitationTokenWrongPlayer() throws IOException { 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); - when(playerRepository.findOne(newMember.getId())).thenReturn(newMember); + when(clanRepository.findById(clan.getId())).thenReturn(Optional.of(clan)); + when(playerRepository.findById(newMember.getId())).thenReturn(Optional.of(newMember)); when(playerService.getPlayer(any())).thenReturn(otherPlayer); try { @@ -344,8 +344,8 @@ public void acceptPlayerInvitationTokenPlayerIAlreadyInAClan() throws IOExceptio 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); - when(playerRepository.findOne(newMember.getId())).thenReturn(newMember); + when(clanRepository.findById(clan.getId())).thenReturn(Optional.of(clan)); + when(playerRepository.findById(newMember.getId())).thenReturn(Optional.of(newMember)); when(playerService.getPlayer(any())).thenReturn(newMember); try { @@ -370,8 +370,8 @@ public void acceptPlayerInvitationToken() throws IOException { 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); - when(playerRepository.findOne(newMember.getId())).thenReturn(newMember); + when(clanRepository.findById(clan.getId())).thenReturn(Optional.of(clan)); + when(playerRepository.findById(newMember.getId())).thenReturn(Optional.of(newMember)); when(playerService.getPlayer(any())).thenReturn(newMember); instance.acceptPlayerInvitationToken(stringToken, null); diff --git a/src/test/java/com/faforever/api/map/MapsControllerTest.java b/src/test/java/com/faforever/api/map/MapsControllerTest.java index 0b0ada092..9cd25f9fd 100644 --- a/src/test/java/com/faforever/api/map/MapsControllerTest.java +++ b/src/test/java/com/faforever/api/map/MapsControllerTest.java @@ -8,7 +8,6 @@ import com.google.common.io.ByteStreams; import org.junit.Test; import org.junit.runner.RunWith; -import org.springframework.boot.actuate.endpoint.PublicMetrics; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.Import; @@ -19,7 +18,6 @@ import javax.inject.Inject; import java.io.InputStream; -import java.util.Collection; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; @@ -43,8 +41,6 @@ public class MapsControllerTest { private PlayerService playerService; @MockBean private ObjectMapper objectMapper; - @MockBean - private Collection publicMetrics; @Inject public void init(MockMvc mvc) { diff --git a/src/test/java/com/faforever/api/security/OAuthClientDetailsServiceTest.java b/src/test/java/com/faforever/api/security/OAuthClientDetailsServiceTest.java index 362cefca9..e265ee3f8 100644 --- a/src/test/java/com/faforever/api/security/OAuthClientDetailsServiceTest.java +++ b/src/test/java/com/faforever/api/security/OAuthClientDetailsServiceTest.java @@ -11,6 +11,8 @@ import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.ClientRegistrationException; +import java.util.Optional; + import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.when; @@ -30,7 +32,7 @@ public void setUp() throws Exception { @Test public void loadClientByClientId() throws Exception { - when(oAuthClientRepository.findOne("123")).thenReturn(new OAuthClient().setDefaultScope("").setRedirectUris("")); + when(oAuthClientRepository.findById("123")).thenReturn(Optional.of(new OAuthClient().setDefaultScope("").setRedirectUris(""))); ClientDetails result = instance.loadClientByClientId("123"); diff --git a/src/test/java/com/faforever/api/user/UserServiceTest.java b/src/test/java/com/faforever/api/user/UserServiceTest.java index dadaf9de6..422f00c67 100644 --- a/src/test/java/com/faforever/api/user/UserServiceTest.java +++ b/src/test/java/com/faforever/api/user/UserServiceTest.java @@ -4,7 +4,6 @@ import com.faforever.api.data.domain.GlobalRating; import com.faforever.api.data.domain.Ladder1v1Rating; import com.faforever.api.data.domain.NameRecord; -import com.faforever.api.data.domain.Player; import com.faforever.api.data.domain.User; import com.faforever.api.email.EmailService; import com.faforever.api.error.ApiExceptionWithCode; @@ -48,7 +47,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; @@ -242,7 +240,6 @@ public void changeEmailInvalidPassword() { @Test public void changeLogin() { User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL); - when(playerRepository.findOne(TEST_USERID)).thenReturn(mock(Player.class)); when(nameRecordRepository.getDaysSinceLastNewRecord(anyInt(), anyInt())).thenReturn(Optional.empty()); instance.changeLogin(TEST_USERNAME_CHANGED, user, "127.0.0.1"); @@ -365,7 +362,7 @@ public void claimPasswordResetToken() throws Exception { KEY_PASSWORD, TEST_NEW_PASSWORD)); User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL); - when(userRepository.findOne(5)).thenReturn(user); + when(userRepository.findById(5)).thenReturn(Optional.of(user)); instance.claimPasswordResetToken(TOKEN_VALUE); @@ -423,7 +420,7 @@ public void linkToSteam() throws Exception { when(steamService.ownsForgedAlliance(any())).thenReturn(true); User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL); - when(userRepository.findOne(5)).thenReturn(user); + when(userRepository.findById(5)).thenReturn(Optional.of(user)); SteamLinkResult result = instance.linkToSteam(TOKEN_VALUE, STEAM_ID); @@ -438,7 +435,7 @@ public void linkToSteamUnknownUser() throws Exception { expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.TOKEN_INVALID)); when(fafTokenService.resolveToken(FafTokenType.LINK_TO_STEAM, TOKEN_VALUE)).thenReturn(ImmutableMap.of(KEY_USER_ID, "5")); - when(userRepository.findOne(5)).thenReturn(null); + when(userRepository.findById(5)).thenReturn(Optional.empty()); instance.linkToSteam(TOKEN_VALUE, STEAM_ID); verifyZeroInteractions(mauticService); @@ -454,7 +451,7 @@ public void linkToSteamNoGame() throws Exception { when(steamService.ownsForgedAlliance(any())).thenReturn(false); User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL); - when(userRepository.findOne(5)).thenReturn(user); + when(userRepository.findById(5)).thenReturn(Optional.of(user)); SteamLinkResult result = instance.linkToSteam(TOKEN_VALUE, STEAM_ID); diff --git a/src/test/java/com/faforever/api/voting/VotingServiceTest.java b/src/test/java/com/faforever/api/voting/VotingServiceTest.java index 31f933be5..40108e59f 100644 --- a/src/test/java/com/faforever/api/voting/VotingServiceTest.java +++ b/src/test/java/com/faforever/api/voting/VotingServiceTest.java @@ -64,7 +64,7 @@ public void saveVoteSuccessful() { Player player = new Player(); when(voteRepository.findByPlayerAndVotingSubjectId(player, votingSubject.getId())).thenReturn(Optional.empty()); - when(votingSubjectRepository.findOne(votingSubject.getId())).thenReturn(votingSubject); + when(votingSubjectRepository.findById(votingSubject.getId())).thenReturn(Optional.of(votingSubject)); instance.saveVote(vote, player); verify(voteRepository).save(vote); @@ -99,7 +99,7 @@ public void notSaveVoteIfUserVotedAlready() { Player player = new Player(); when(voteRepository.findByPlayerAndVotingSubjectId(player, votingSubject.getId())).thenReturn(Optional.of(new Vote())); - when(votingSubjectRepository.findOne(votingSubject.getId())).thenReturn(votingSubject); + when(votingSubjectRepository.findById(votingSubject.getId())).thenReturn(Optional.of(votingSubject)); try { instance.saveVote(vote, player); @@ -142,9 +142,9 @@ public void saveVoteIfAlternativeOrdinalCorrect() { Player player = new Player(); when(voteRepository.findByPlayerAndVotingSubjectId(player, votingSubject.getId())).thenReturn(Optional.empty()); - when(votingSubjectRepository.findOne(votingSubject.getId())).thenReturn(votingSubject); - when(votingChoiceRepository.findOne(votingChoice.getId())).thenReturn(votingChoice); - when(votingChoiceRepository.findOne(votingChoice2.getId())).thenReturn(votingChoice2); + when(votingSubjectRepository.findById(votingSubject.getId())).thenReturn(Optional.of(votingSubject)); + when(votingChoiceRepository.findById(votingChoice.getId())).thenReturn(Optional.of(votingChoice)); + when(votingChoiceRepository.findById(votingChoice2.getId())).thenReturn(Optional.of(votingChoice2)); instance.saveVote(vote, player); verify(voteRepository).save(vote); @@ -177,7 +177,7 @@ public void saveVoteInvalidChoiceId() { Player player = new Player(); when(voteRepository.findByPlayerAndVotingSubjectId(player, votingSubject.getId())).thenReturn(Optional.empty()); - when(votingSubjectRepository.findOne(votingSubject.getId())).thenReturn(votingSubject); + when(votingSubjectRepository.findById(votingSubject.getId())).thenReturn(Optional.of(votingSubject)); expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.VOTING_CHOICE_DOES_NOT_EXIST)); @@ -218,9 +218,9 @@ public void notSaveVoteIfAlternativeOrdinalWrong() { Player player = new Player(); when(voteRepository.findByPlayerAndVotingSubjectId(player, votingSubject.getId())).thenReturn(Optional.empty()); - when(votingSubjectRepository.findOne(votingSubject.getId())).thenReturn(votingSubject); - when(votingChoiceRepository.findOne(votingChoice.getId())).thenReturn(votingChoice); - when(votingChoiceRepository.findOne(votingChoice2.getId())).thenReturn(votingChoice2); + when(votingSubjectRepository.findById(votingSubject.getId())).thenReturn(Optional.of(votingSubject)); + when(votingChoiceRepository.findById(votingChoice.getId())).thenReturn(Optional.of(votingChoice)); + when(votingChoiceRepository.findById(votingChoice2.getId())).thenReturn(Optional.of(votingChoice2)); try { instance.saveVote(vote, player); @@ -261,9 +261,9 @@ public void notSaveVoteOnTooManyAnswers() { Player player = new Player(); when(voteRepository.findByPlayerAndVotingSubjectId(player, votingSubject.getId())).thenReturn(Optional.empty()); - when(votingSubjectRepository.findOne(votingSubject.getId())).thenReturn(votingSubject); - when(votingChoiceRepository.findOne(votingChoice.getId())).thenReturn(votingChoice); - when(votingChoiceRepository.findOne(votingChoice2.getId())).thenReturn(votingChoice2); + when(votingSubjectRepository.findById(votingSubject.getId())).thenReturn(Optional.of(votingSubject)); + when(votingChoiceRepository.findById(votingChoice.getId())).thenReturn(Optional.of(votingChoice)); + when(votingChoiceRepository.findById(votingChoice2.getId())).thenReturn(Optional.of(votingChoice2)); try { instance.saveVote(vote, player); @@ -301,8 +301,8 @@ public void notSaveOnVoteTwiceInOneOption() { Player player = new Player(); when(voteRepository.findByPlayerAndVotingSubjectId(player, votingSubject.getId())).thenReturn(Optional.empty()); - when(votingSubjectRepository.findOne(votingSubject.getId())).thenReturn(votingSubject); - when(votingChoiceRepository.findOne(votingChoice.getId())).thenReturn(votingChoice); + when(votingSubjectRepository.findById(votingSubject.getId())).thenReturn(Optional.of(votingSubject)); + when(votingChoiceRepository.findById(votingChoice.getId())).thenReturn(Optional.of(votingChoice)); try { instance.saveVote(vote, player); } catch (ApiException e) { From ff013551fd9488323ee745943566a28c8f5c3147 Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Tue, 26 Jun 2018 21:02:13 +0200 Subject: [PATCH 04/10] Spring security oauth2 fix --- build.gradle | 3 ++- gradle.properties | 5 +++-- .../java/com/faforever/api/data/AvatarElideTest.java | 9 --------- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/build.gradle b/build.gradle index ce8d221ea..63272ce27 100644 --- a/build.gradle +++ b/build.gradle @@ -291,8 +291,9 @@ dependencies { compile("com.github.FAForever:faf-java-commons:${fafCommonsVersion}") compile("org.kohsuke:github-api:${githubApiVersion}") compile("org.jolokia:jolokia-core") - compile("org.springframework.security:spring-security-jwt:${springSecurityJwtVersion}") + compile("org.springframework.security.oauth.boot:spring-security-oauth2-autoconfigure:${springSecurityOauth2AutoConfigureVersion}") compile("org.springframework.security.oauth:spring-security-oauth2:${springSecurityOauth2Version}") + compile("org.springframework.security:spring-security-jwt:${springSecurityJwtVersion}") compile("org.eclipse.jgit:org.eclipse.jgit:${jgitVersionn}") compile("org.jetbrains:annotations:${jetbrainsAnnotationsVersion}") compile("com.google.guava:guava:${guavaVersion}") diff --git a/gradle.properties b/gradle.properties index 4c327d428..258a6489e 100644 --- a/gradle.properties +++ b/gradle.properties @@ -10,8 +10,9 @@ swaggerVersion=2.6.1 swaggerCoreVersion=1.5.8 coverallsGradlePluginVersion=2.4.0 hikariCpVersion=2.4.6 -springSecurityJwtVersion=1.0.7.RELEASE -springSecurityOauth2Version=2.0.12.RELEASE +springSecurityJwtVersion=1.0.9.RELEASE +springSecurityOauth2AutoConfigureVersion=2.0.1.RELEASE +springSecurityOauth2Version=2.3.3.RELEASE guavaVersion=21.0-rc1 jetbrainsAnnotationsVersion=13.0 springBootAdminClientVersion=2.0.0 diff --git a/src/inttest/java/com/faforever/api/data/AvatarElideTest.java b/src/inttest/java/com/faforever/api/data/AvatarElideTest.java index 696a964b6..0a0441dad 100644 --- a/src/inttest/java/com/faforever/api/data/AvatarElideTest.java +++ b/src/inttest/java/com/faforever/api/data/AvatarElideTest.java @@ -81,15 +81,6 @@ public void moderatorCanAssignAvatar() throws Exception { )) ).andExpect(status().isCreated()); Optional createdAssignment = avatarAssignmentRepository.findOneByAvatarIdAndPlayerId(1, 1); - createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); - createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); - createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); - createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); - createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); - createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); - createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); - createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); - createdAssignment = avatarAssignmentRepository.findOneByAvatarAndPlayer(avatar, player); assertThat(createdAssignment.isPresent(), is(true)); assertThat(createdAssignment.get().getPlayer().getId(), is(player.getId())); assertThat(createdAssignment.get().getAvatar().getId(), is(avatar.getId())); From eddeed7097246a070d4bd1d12e82a345300393ac Mon Sep 17 00:00:00 2001 From: kubo Date: Sat, 21 Jul 2018 23:05:10 +0200 Subject: [PATCH 05/10] #140 Fix elide spring transaction integration and test fixes --- build.gradle | 1 + .../com/faforever/api/data/ClanElideTest.java | 2 +- .../faforever/api/data/VotingElideTest.java | 2 +- src/inttest/resources/config/application.yml | 4 - .../com/faforever/api/config/MvcConfig.java | 11 +- .../api/config/elide/ElideConfig.java | 17 +- .../elide/SpringHibernateDataStore.java | 156 ++++++++++++++++++ .../elide/SpringHibernateTransaction.java | 78 +++++++++ .../api/data/domain/VotingSubject.java | 2 + .../data/listeners/VotingSubjectEnricher.java | 7 - .../VotingSubjectRevealWinnerCheck.java | 25 +++ .../VotingSubjectRevealWinnerValidator.java | 20 +++ 12 files changed, 303 insertions(+), 22 deletions(-) create mode 100644 src/main/java/com/faforever/api/config/elide/SpringHibernateDataStore.java create mode 100644 src/main/java/com/faforever/api/config/elide/SpringHibernateTransaction.java create mode 100644 src/main/java/com/faforever/api/data/validation/VotingSubjectRevealWinnerCheck.java create mode 100644 src/main/java/com/faforever/api/data/validation/VotingSubjectRevealWinnerValidator.java diff --git a/build.gradle b/build.gradle index 63272ce27..31424875a 100644 --- a/build.gradle +++ b/build.gradle @@ -199,6 +199,7 @@ task sendCoverageToCodacy(type: JavaExec, dependsOn: jacocoTestReport) { apply plugin: 'com.bmuschko.docker-remote-api' apply plugin: 'application' +mainClassName = 'com.faforever.api.FafApiApplication' import com.bmuschko.gradle.docker.DockerRemoteApiPlugin import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage diff --git a/src/inttest/java/com/faforever/api/data/ClanElideTest.java b/src/inttest/java/com/faforever/api/data/ClanElideTest.java index 7964c18a1..9be3b696f 100644 --- a/src/inttest/java/com/faforever/api/data/ClanElideTest.java +++ b/src/inttest/java/com/faforever/api/data/ClanElideTest.java @@ -135,7 +135,7 @@ public void cannotTransferLeadershipToNonClanMember() throws Exception { mockMvc.perform( patch("/data/clan/1") .content(generateTransferLeadershipContent(1, 1))) // magic value from prepClanData.sql - .andExpect(status().is5xxServerError()); // TODO: Catch javax.validation.ConstraintViolationException and wrap it into a regular ApiException + .andExpect(status().is4xxClientError()); // TODO: Catch javax.validation.ConstraintViolationException and wrap it into a regular ApiException } @SneakyThrows diff --git a/src/inttest/java/com/faforever/api/data/VotingElideTest.java b/src/inttest/java/com/faforever/api/data/VotingElideTest.java index 0e62612ef..173c501f9 100644 --- a/src/inttest/java/com/faforever/api/data/VotingElideTest.java +++ b/src/inttest/java/com/faforever/api/data/VotingElideTest.java @@ -199,7 +199,7 @@ public void testRevealWinnerOnEndedSubjectWorks() throws Exception { @WithUserDetails(AUTH_MODERATOR) public void testRevealWinnerOnNoneEndedSubjectFails() throws Exception { mockMvc.perform(patch("/data/votingSubject/1").content(PATCH_VOTING_SUBJECT_REVEAL_ID_1)) - .andExpect(status().is(500)); + .andExpect(status().is4xxClientError()); } @Test diff --git a/src/inttest/resources/config/application.yml b/src/inttest/resources/config/application.yml index 6e828e8ad..2cb9f847c 100644 --- a/src/inttest/resources/config/application.yml +++ b/src/inttest/resources/config/application.yml @@ -19,10 +19,6 @@ spring: console: enabled: true -security: - oauth2: - resource: - filter-order: 3 faf-api: jwt: diff --git a/src/main/java/com/faforever/api/config/MvcConfig.java b/src/main/java/com/faforever/api/config/MvcConfig.java index ec0ac8f88..b2da4dfef 100644 --- a/src/main/java/com/faforever/api/config/MvcConfig.java +++ b/src/main/java/com/faforever/api/config/MvcConfig.java @@ -2,15 +2,16 @@ import org.springframework.context.annotation.Configuration; import org.springframework.core.Ordered; +import org.springframework.web.servlet.config.annotation.ContentNegotiationConfigurer; import org.springframework.web.servlet.config.annotation.EnableWebMvc; import org.springframework.web.servlet.config.annotation.PathMatchConfigurer; import org.springframework.web.servlet.config.annotation.ResourceHandlerRegistry; import org.springframework.web.servlet.config.annotation.ViewControllerRegistry; -import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; @EnableWebMvc @Configuration -public class MvcConfig extends WebMvcConfigurerAdapter { +public class MvcConfig implements WebMvcConfigurer { @Override public void addViewControllers(ViewControllerRegistry registry) { @@ -30,4 +31,10 @@ public void addResourceHandlers(ResourceHandlerRegistry registry) { public void configurePathMatch(PathMatchConfigurer configurer) { configurer.setUseRegisteredSuffixPatternMatch(true); } + + @Override + public void configureContentNegotiation(final ContentNegotiationConfigurer configurer) { + // Turn off suffix-based content negotiation + configurer.favorPathExtension(false); + } } diff --git a/src/main/java/com/faforever/api/config/elide/ElideConfig.java b/src/main/java/com/faforever/api/config/elide/ElideConfig.java index bc491fbee..c5ae46189 100644 --- a/src/main/java/com/faforever/api/config/elide/ElideConfig.java +++ b/src/main/java/com/faforever/api/config/elide/ElideConfig.java @@ -14,17 +14,18 @@ import com.yahoo.elide.ElideSettingsBuilder; import com.yahoo.elide.core.EntityDictionary; import com.yahoo.elide.core.filter.dialect.RSQLFilterDialect; -import com.yahoo.elide.datastores.hibernate5.AbstractHibernateStore; import com.yahoo.elide.jsonapi.JsonApiMapper; import com.yahoo.elide.security.checks.Check; import com.yahoo.elide.utils.coerce.CoerceUtil; import org.apache.commons.beanutils.ConvertUtils; import org.apache.commons.beanutils.Converter; -import org.hibernate.SessionFactory; +import org.hibernate.ScrollMode; +import org.springframework.beans.factory.config.AutowireCapableBeanFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.transaction.PlatformTransactionManager; -import javax.persistence.EntityManagerFactory; +import javax.persistence.EntityManager; import java.time.Duration; import java.time.Instant; import java.time.OffsetDateTime; @@ -36,12 +37,12 @@ public class ElideConfig { public static final String DEFAULT_CACHE_NAME = "Elide.defaultCache"; @Bean - public Elide elide(AbstractHibernateStore hibernateStore, ObjectMapper objectMapper, EntityDictionary entityDictionary, ExtendedAuditLogger extendedAuditLogger) { + public Elide elide(SpringHibernateDataStore springHibernateDataStore, ObjectMapper objectMapper, EntityDictionary entityDictionary, ExtendedAuditLogger extendedAuditLogger) { RSQLFilterDialect rsqlFilterDialect = new RSQLFilterDialect(entityDictionary); registerAdditionalConverters(); - return new Elide(new ElideSettingsBuilder(hibernateStore) + return new Elide(new ElideSettingsBuilder(springHibernateDataStore) .withJsonApiMapper(new JsonApiMapper(entityDictionary, objectMapper)) .withAuditLogger(extendedAuditLogger) .withEntityDictionary(entityDictionary) @@ -51,8 +52,10 @@ public Elide elide(AbstractHibernateStore hibernateStore, ObjectMapper objectMap } @Bean - AbstractHibernateStore hibernateStore(EntityManagerFactory entityManagerFactory) { - return new AbstractHibernateStore.Builder(entityManagerFactory.unwrap(SessionFactory.class)).build(); + SpringHibernateDataStore springHibernateDataStore(PlatformTransactionManager txManager, + AutowireCapableBeanFactory beanFactory, + EntityManager entityManager) { + return new SpringHibernateDataStore(txManager, beanFactory, entityManager, false, true, ScrollMode.FORWARD_ONLY); } /** diff --git a/src/main/java/com/faforever/api/config/elide/SpringHibernateDataStore.java b/src/main/java/com/faforever/api/config/elide/SpringHibernateDataStore.java new file mode 100644 index 000000000..3f0bd8eac --- /dev/null +++ b/src/main/java/com/faforever/api/config/elide/SpringHibernateDataStore.java @@ -0,0 +1,156 @@ +/* + * Copyright (c) 2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.faforever.api.config.elide; + +import com.google.common.base.Preconditions; +import com.yahoo.elide.core.DataStore; +import com.yahoo.elide.core.DataStoreTransaction; +import com.yahoo.elide.core.EntityDictionary; +import org.hibernate.ScrollMode; +import org.hibernate.Session; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.config.AutowireCapableBeanFactory; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.TransactionStatus; +import org.springframework.transaction.support.DefaultTransactionDefinition; + +import javax.persistence.EntityManager; +import javax.persistence.metamodel.EntityType; +import java.util.Objects; +import java.util.Set; + +/** + * Spring Hibernate DataStore. + * + * @author olOwOlo + */ +public class SpringHibernateDataStore implements DataStore { + + private static final Logger logger = LoggerFactory.getLogger(SpringHibernateDataStore.class); + + protected final PlatformTransactionManager txManager; + protected final AutowireCapableBeanFactory beanFactory; + protected final EntityManager entityManager; + protected final boolean isSpringDependencyInjection; + protected final boolean isScrollEnabled; + protected final ScrollMode scrollMode; + protected final HibernateTransactionSupplier transactionSupplier; + + /** + * Constructor. + * + * @param txManager Spring PlatformTransactionManager + * @param beanFactory Spring AutowireCapableBeanFactory + * @param entityManager EntityManager + * @param isScrollEnabled Whether or not scrolling is enabled on driver + * @param scrollMode Scroll mode to use for scrolling driver + */ + public SpringHibernateDataStore(PlatformTransactionManager txManager, + AutowireCapableBeanFactory beanFactory, + EntityManager entityManager, + boolean isSpringDependencyInjection, + boolean isScrollEnabled, + ScrollMode scrollMode) { + this(txManager, beanFactory, entityManager, isSpringDependencyInjection, + isScrollEnabled, scrollMode, SpringHibernateTransaction::new); + } + + /** + *

Constructor.

+ * Useful for extending the store and relying on existing code to instantiate custom hibernate transaction. + * + * @param txManager Spring PlatformTransactionManager + * @param beanFactory Spring AutowireCapableBeanFactory + * @param entityManager EntityManager factory + * @param isScrollEnabled Whether or not scrolling is enabled on driver + * @param scrollMode Scroll mode to use for scrolling driver + * @param transactionSupplier Supplier for transaction + */ + protected SpringHibernateDataStore(PlatformTransactionManager txManager, + AutowireCapableBeanFactory beanFactory, + EntityManager entityManager, + boolean isSpringDependencyInjection, + boolean isScrollEnabled, + ScrollMode scrollMode, + HibernateTransactionSupplier transactionSupplier) { + this.txManager = txManager; + this.beanFactory = beanFactory; + this.entityManager = entityManager; + this.isSpringDependencyInjection = isSpringDependencyInjection; + this.isScrollEnabled = isScrollEnabled; + this.scrollMode = scrollMode; + this.transactionSupplier = transactionSupplier; + } + + @Override + public void populateEntityDictionary(EntityDictionary dictionary) { + if (isSpringDependencyInjection) { + logger.info("Spring Dependency Injection is enabled, " + + "each time an object of entityClass type is instantiated by Elide, " + + "Spring will try to inject beans."); + } + Set> entities = entityManager.getMetamodel().getEntities(); + /* bind all entities */ + entities.stream() + .map(EntityType::getJavaType) + .filter(Objects::nonNull) + .forEach(mappedClass -> { + try { + // Ignore this result. + // We are just checking to see if it throws an exception meaning that + // provided class was _not_ an entity. + dictionary.lookupEntityClass(mappedClass); + // Bind if successful + dictionary.bindEntity(mappedClass); + if (isSpringDependencyInjection) { + // Bind Spring Dependency Injection + dictionary.bindInitializer(beanFactory::autowireBean, mappedClass); + } + } catch (IllegalArgumentException e) { + // Ignore this entity + // Turns out that hibernate may include non-entity types in this list when using things + // like envers. Since they are not entities, we do not want to bind them into the entity + // dictionary + } + }); + } + + @Override + public DataStoreTransaction beginTransaction() { + // begin a spring transaction + DefaultTransactionDefinition def = new DefaultTransactionDefinition(); + def.setName("elide transaction"); + def.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED); + TransactionStatus txStatus = txManager.getTransaction(def); + + Session session = entityManager.unwrap(Session.class); + Preconditions.checkNotNull(session); + + return transactionSupplier.get(session, txManager, txStatus, isScrollEnabled, scrollMode); + } + + /** + * Functional interface for describing a method to supply a custom Hibernate transaction. + */ + @FunctionalInterface + public interface HibernateTransactionSupplier { + SpringHibernateTransaction get(Session session, PlatformTransactionManager txManager, + TransactionStatus txStatus, boolean isScrollEnabled, ScrollMode scrollMode); + } +} diff --git a/src/main/java/com/faforever/api/config/elide/SpringHibernateTransaction.java b/src/main/java/com/faforever/api/config/elide/SpringHibernateTransaction.java new file mode 100644 index 000000000..cbfa11dfe --- /dev/null +++ b/src/main/java/com/faforever/api/config/elide/SpringHibernateTransaction.java @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.faforever.api.config.elide; + +import com.yahoo.elide.core.RequestScope; +import com.yahoo.elide.core.exceptions.TransactionException; +import com.yahoo.elide.datastores.hibernate5.HibernateTransaction; +import org.hibernate.ScrollMode; +import org.hibernate.Session; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionStatus; + +import java.io.IOException; + +/** + * Spring Hibernate Transaction. + * + * @author olOwOlo + */ +public class SpringHibernateTransaction extends HibernateTransaction { + + private final Session session; + private final TransactionStatus txStatus; + private final PlatformTransactionManager txManager; + + /** + * Constructor. + * + * @param session Hibernate session + * @param txManager Spring PlatformTransactionManager + * @param txStatus Spring Transaction status + * @param isScrollEnabled Whether or not scrolling is enabled + * @param scrollMode Scroll mode to use if scrolling enabled + */ + protected SpringHibernateTransaction(Session session, + PlatformTransactionManager txManager, + TransactionStatus txStatus, + boolean isScrollEnabled, + ScrollMode scrollMode) { + super(session, isScrollEnabled, scrollMode); + this.session = session; + this.txManager = txManager; + this.txStatus = txStatus; + } + + @Override + public void commit(RequestScope scope) { + try { + flush(scope); + txManager.commit(txStatus); + } catch (org.springframework.transaction.TransactionException e) { + throw new TransactionException(e); + } + } + + @Override + public void close() throws IOException { + if (session.isOpen() && !txStatus.isCompleted()) { + txManager.rollback(txStatus); + throw new IOException("Transaction not closed"); + } + } + +} diff --git a/src/main/java/com/faforever/api/data/domain/VotingSubject.java b/src/main/java/com/faforever/api/data/domain/VotingSubject.java index cfc20c550..64bdb7bb2 100644 --- a/src/main/java/com/faforever/api/data/domain/VotingSubject.java +++ b/src/main/java/com/faforever/api/data/domain/VotingSubject.java @@ -2,6 +2,7 @@ import com.faforever.api.data.checks.permission.IsModerator; import com.faforever.api.data.listeners.VotingSubjectEnricher; +import com.faforever.api.data.validation.VotingSubjectRevealWinnerCheck; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonManagedReference; import com.yahoo.elide.annotation.Audit; @@ -39,6 +40,7 @@ @Audit(action = Action.UPDATE, logStatement = "Updated voting subject with id: {0}", logExpressions = {"${votingSubject.id}"}) @Setter @EntityListeners(VotingSubjectEnricher.class) +@VotingSubjectRevealWinnerCheck public class VotingSubject extends AbstractEntity { public static final String TYPE_NAME = "votingSubject"; diff --git a/src/main/java/com/faforever/api/data/listeners/VotingSubjectEnricher.java b/src/main/java/com/faforever/api/data/listeners/VotingSubjectEnricher.java index b8d81e544..c172946d5 100644 --- a/src/main/java/com/faforever/api/data/listeners/VotingSubjectEnricher.java +++ b/src/main/java/com/faforever/api/data/listeners/VotingSubjectEnricher.java @@ -4,9 +4,6 @@ import com.faforever.api.data.domain.VotingChoice; import com.faforever.api.data.domain.VotingQuestion; import com.faforever.api.data.domain.VotingSubject; -import com.faforever.api.error.ApiException; -import com.faforever.api.error.Error; -import com.faforever.api.error.ErrorCode; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import org.springframework.context.support.MessageSourceAccessor; @@ -40,13 +37,9 @@ public void init(MessageSourceAccessor messageSourceAccessor) { @PreUpdate public void preUpdate(VotingSubject votingSubject) { - if (votingSubject.getEndOfVoteTime().isAfter(OffsetDateTime.now()) && votingSubject.getRevealWinner()) { - throw new ApiException(new Error(ErrorCode.CAN_NOT_REVEAL_RESULTS_WHEN_VOTING_IS_NOT_FINISHED)); - } updateWinners(votingSubject); } - private void updateWinners(VotingSubject votingSubject) { votingSubject.getVotingQuestions().forEach(this::calculateWinners); } diff --git a/src/main/java/com/faforever/api/data/validation/VotingSubjectRevealWinnerCheck.java b/src/main/java/com/faforever/api/data/validation/VotingSubjectRevealWinnerCheck.java new file mode 100644 index 000000000..e44402ecb --- /dev/null +++ b/src/main/java/com/faforever/api/data/validation/VotingSubjectRevealWinnerCheck.java @@ -0,0 +1,25 @@ +package com.faforever.api.data.validation; + + +import javax.validation.Constraint; +import javax.validation.Payload; +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +@Target({TYPE, ANNOTATION_TYPE}) +@Retention(RUNTIME) +@Constraint(validatedBy = VotingSubjectRevealWinnerValidator.class) +@Documented +public @interface VotingSubjectRevealWinnerCheck { + + String message() default "Cannot reveal voting result before end of voting period"; + + Class[] groups() default {}; + + Class[] payload() default {}; +} diff --git a/src/main/java/com/faforever/api/data/validation/VotingSubjectRevealWinnerValidator.java b/src/main/java/com/faforever/api/data/validation/VotingSubjectRevealWinnerValidator.java new file mode 100644 index 000000000..a5ff05f22 --- /dev/null +++ b/src/main/java/com/faforever/api/data/validation/VotingSubjectRevealWinnerValidator.java @@ -0,0 +1,20 @@ +package com.faforever.api.data.validation; + +import com.faforever.api.data.domain.VotingSubject; + +import javax.validation.ConstraintValidator; +import javax.validation.ConstraintValidatorContext; +import java.time.OffsetDateTime; + +public class VotingSubjectRevealWinnerValidator implements ConstraintValidator { + + @Override + public void initialize(VotingSubjectRevealWinnerCheck constraintAnnotation) { + //Comment for codacy to show that this constructor is intentional empty + } + + @Override + public boolean isValid(VotingSubject votingSubject, ConstraintValidatorContext context) { + return votingSubject.getRevealWinner() && votingSubject.getEndOfVoteTime().isBefore(OffsetDateTime.now()); + } +} From ec2c097e56314c41fb078d1d6a682de74ec1f1a0 Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Sun, 22 Jul 2018 01:08:06 +0200 Subject: [PATCH 06/10] Fix vote tests on fast computers --- src/inttest/resources/sql/prepVotingData.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/inttest/resources/sql/prepVotingData.sql b/src/inttest/resources/sql/prepVotingData.sql index 953ce1aaf..5e6ececb2 100644 --- a/src/inttest/resources/sql/prepVotingData.sql +++ b/src/inttest/resources/sql/prepVotingData.sql @@ -7,7 +7,7 @@ DELETE FROM voting_subject; INSERT INTO voting_subject (id, subject_key, begin_of_vote_time, end_of_vote_time, min_games_to_vote, description_key, topic_url) VALUES - (1, 'subject', NOW(), DATE_ADD(NOW(), INTERVAL 1 YEAR), 0, 'des', 'www.google.de'); + (1, 'subject', NOW() - INTERVAL 1 MINUTE, DATE_ADD(NOW(), INTERVAL 1 YEAR), 0, 'des', 'www.google.de'); INSERT INTO voting_subject (id, subject_key, reveal_winner, begin_of_vote_time, end_of_vote_time, min_games_to_vote, description_key, topic_url) VALUES From db5d45ac7c195ead84ba8edb4e9033f3fe876ca0 Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Sun, 22 Jul 2018 01:17:47 +0200 Subject: [PATCH 07/10] Migrate latest entities to Elide 4 --- src/main/java/com/faforever/api/data/domain/Message.java | 2 -- src/main/java/com/faforever/api/data/domain/Tutorial.java | 2 -- .../java/com/faforever/api/data/domain/TutorialCategory.java | 2 -- 3 files changed, 6 deletions(-) diff --git a/src/main/java/com/faforever/api/data/domain/Message.java b/src/main/java/com/faforever/api/data/domain/Message.java index ca359152e..7eb5b926c 100644 --- a/src/main/java/com/faforever/api/data/domain/Message.java +++ b/src/main/java/com/faforever/api/data/domain/Message.java @@ -9,7 +9,6 @@ import com.yahoo.elide.annotation.DeletePermission; import com.yahoo.elide.annotation.Include; import com.yahoo.elide.annotation.ReadPermission; -import com.yahoo.elide.annotation.SharePermission; import com.yahoo.elide.annotation.UpdatePermission; import lombok.Setter; @@ -23,7 +22,6 @@ @Table(name = "messages") @Setter @Include(rootLevel = true) -@SharePermission(expression = IsModerator.EXPRESSION) @DeletePermission(expression = IsModerator.EXPRESSION) @UpdatePermission(expression = IsModerator.EXPRESSION) @CreatePermission(expression = IsModerator.EXPRESSION) diff --git a/src/main/java/com/faforever/api/data/domain/Tutorial.java b/src/main/java/com/faforever/api/data/domain/Tutorial.java index dec8e5ea0..e9890bd49 100644 --- a/src/main/java/com/faforever/api/data/domain/Tutorial.java +++ b/src/main/java/com/faforever/api/data/domain/Tutorial.java @@ -10,7 +10,6 @@ import com.yahoo.elide.annotation.DeletePermission; import com.yahoo.elide.annotation.Include; import com.yahoo.elide.annotation.ReadPermission; -import com.yahoo.elide.annotation.SharePermission; import com.yahoo.elide.annotation.UpdatePermission; import lombok.Setter; import org.jetbrains.annotations.Nullable; @@ -28,7 +27,6 @@ @Table(name = "tutorial") @Setter @Include(rootLevel = true, type = Tutorial.TYPE_NAME) -@SharePermission(expression = IsModerator.EXPRESSION) @DeletePermission(expression = IsModerator.EXPRESSION) @UpdatePermission(expression = IsModerator.EXPRESSION) @CreatePermission(expression = IsModerator.EXPRESSION) diff --git a/src/main/java/com/faforever/api/data/domain/TutorialCategory.java b/src/main/java/com/faforever/api/data/domain/TutorialCategory.java index d3c26af8c..2737b9bec 100644 --- a/src/main/java/com/faforever/api/data/domain/TutorialCategory.java +++ b/src/main/java/com/faforever/api/data/domain/TutorialCategory.java @@ -11,7 +11,6 @@ import com.yahoo.elide.annotation.DeletePermission; import com.yahoo.elide.annotation.Include; import com.yahoo.elide.annotation.ReadPermission; -import com.yahoo.elide.annotation.SharePermission; import com.yahoo.elide.annotation.UpdatePermission; import lombok.Setter; @@ -32,7 +31,6 @@ @Table(name = "tutorial_category") @Setter @Include(rootLevel = true, type = TutorialCategory.TYPE_NAME) -@SharePermission(expression = IsModerator.EXPRESSION) @DeletePermission(expression = IsModerator.EXPRESSION) @UpdatePermission(expression = IsModerator.EXPRESSION) @CreatePermission(expression = IsModerator.EXPRESSION) From f1c7614de391da2e16f05c47b1a4a482aa01843f Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Sun, 22 Jul 2018 01:44:13 +0200 Subject: [PATCH 08/10] Follow codacy advice --- .../faforever/api/config/elide/SpringHibernateDataStore.java | 1 + src/main/java/com/faforever/api/user/UserService.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/faforever/api/config/elide/SpringHibernateDataStore.java b/src/main/java/com/faforever/api/config/elide/SpringHibernateDataStore.java index 3f0bd8eac..d66495016 100644 --- a/src/main/java/com/faforever/api/config/elide/SpringHibernateDataStore.java +++ b/src/main/java/com/faforever/api/config/elide/SpringHibernateDataStore.java @@ -127,6 +127,7 @@ public void populateEntityDictionary(EntityDictionary dictionary) { // Turns out that hibernate may include non-entity types in this list when using things // like envers. Since they are not entities, we do not want to bind them into the entity // dictionary + logger.debug("Ignoring entity", e); } }); } diff --git a/src/main/java/com/faforever/api/user/UserService.java b/src/main/java/com/faforever/api/user/UserService.java index 24974cb3b..9f98109e7 100644 --- a/src/main/java/com/faforever/api/user/UserService.java +++ b/src/main/java/com/faforever/api/user/UserService.java @@ -258,7 +258,7 @@ void claimPasswordResetToken(String token) { int userId = Integer.parseInt(claims.get(KEY_USER_ID)); String newPassword = claims.get(KEY_PASSWORD); User user = userRepository.findById(userId) - .orElseThrow(() -> new ApiException(new Error(ErrorCode.TOKEN_INVALID))); + .orElseThrow(() -> new ApiException(new Error(TOKEN_INVALID))); setPassword(user, newPassword); } @@ -305,7 +305,7 @@ public SteamLinkResult linkToSteam(String token, String steamId) { Map attributes = fafTokenService.resolveToken(FafTokenType.LINK_TO_STEAM, token); User user = userRepository.findById(Integer.parseInt(attributes.get(KEY_USER_ID))) - .orElseThrow(() -> new ApiException(new Error(ErrorCode.TOKEN_INVALID))); + .orElseThrow(() -> new ApiException(new Error(TOKEN_INVALID))); String callbackUrl = attributes.get(KEY_STEAM_LINK_CALLBACK_URL); if (!steamService.ownsForgedAlliance(steamId)) { From 1285338a2a388314397783950fb0d74a65bbac74 Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Sun, 22 Jul 2018 15:28:02 +0200 Subject: [PATCH 09/10] Remove migration leftovers --- build.gradle | 1 - src/main/resources/config/application.yml | 5 ----- 2 files changed, 6 deletions(-) diff --git a/build.gradle b/build.gradle index 31424875a..ba3f63f97 100644 --- a/build.gradle +++ b/build.gradle @@ -277,7 +277,6 @@ dependencyManagement { } dependencies { - runtime("org.springframework.boot:spring-boot-properties-migrator") runtime("io.micrometer:micrometer-registry-prometheus") compile("org.springframework.boot:spring-boot-starter-jdbc") diff --git a/src/main/resources/config/application.yml b/src/main/resources/config/application.yml index 783bee95f..edcc674a5 100644 --- a/src/main/resources/config/application.yml +++ b/src/main/resources/config/application.yml @@ -68,11 +68,6 @@ server: jetty: max-http-post-size: 314572800 -security: - oauth2: - resource: - filter-order: 3 - management: port: 8011 security: From da72a0c73af7c7519e038b31b3d0484030945480 Mon Sep 17 00:00:00 2001 From: kubo Date: Wed, 25 Jul 2018 00:24:07 +0200 Subject: [PATCH 10/10] #140 Fix transaction handling --- src/inttest/resources/sql/prepDefaultUser.sql | 3 ++- src/main/java/com/faforever/api/data/DataController.java | 9 ++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/inttest/resources/sql/prepDefaultUser.sql b/src/inttest/resources/sql/prepDefaultUser.sql index 56f4e1660..7ca4cd66c 100644 --- a/src/inttest/resources/sql/prepDefaultUser.sql +++ b/src/inttest/resources/sql/prepDefaultUser.sql @@ -41,7 +41,8 @@ DELETE FROM login; INSERT INTO oauth_clients (id, name, client_secret, client_type, redirect_uris, default_redirect_uri, default_scope) VALUES - ('test', 'test', 'test', 'public', 'http://localhost https://www.getpostman.com/oauth2/callback ', 'http://localhost', + ('test', 'test', '{noop}test', 'public', 'http://localhost https://www.getpostman.com/oauth2/callback ', + 'http://localhost', 'read_events read_achievements upload_map upload_mod upload_avatar write_account_data vote'); INSERT INTO login (id, login, email, password, steamid) diff --git a/src/main/java/com/faforever/api/data/DataController.java b/src/main/java/com/faforever/api/data/DataController.java index 44a9a09e7..9eda470fb 100644 --- a/src/main/java/com/faforever/api/data/DataController.java +++ b/src/main/java/com/faforever/api/data/DataController.java @@ -8,7 +8,6 @@ import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.Authentication; import org.springframework.stereotype.Component; -import org.springframework.transaction.annotation.Transactional; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; @@ -41,11 +40,11 @@ private static Object getPrincipal(final Authentication authentication) { return authentication != null ? authentication.getPrincipal() : null; } + //!!! No @Transactional - transactions are being handled by Elide @RequestMapping( method = RequestMethod.GET, produces = JSON_API_MEDIA_TYPE, value = {"/{entity}", "/{entity}/{id}/relationships/{entity2}", "/{entity}/{id}/{child}", "/{entity}/{id}"}) - @Transactional(readOnly = true) @Cacheable(cacheResolver = "elideCacheResolver", keyGenerator = GetCacheKeyGenerator.NAME) public ResponseEntity get(@RequestParam final Map allRequestParams, final HttpServletRequest request, @@ -58,11 +57,11 @@ public ResponseEntity get(@RequestParam final Map allReq return wrapResponse(response); } + //!!! No @Transactional - transactions are being handled by Elide @RequestMapping( method = RequestMethod.POST, produces = JSON_API_MEDIA_TYPE, value = {"/{entity}", "/{entity}/{id}/relationships/{entity2}", "/{entity}/{id}/{child}", "/{entity}/{id}"}) - @Transactional @Cacheable(cacheResolver = "elideCacheResolver") @PreAuthorize("hasRole('USER')") public ResponseEntity post(@RequestBody final String body, @@ -76,11 +75,11 @@ public ResponseEntity post(@RequestBody final String body, return wrapResponse(response); } + //!!! No @Transactional - transactions are being handled by Elide @RequestMapping( method = RequestMethod.PATCH, produces = JSON_API_MEDIA_TYPE, value = {"/{entity}/{id}", "/{entity}/{id}/relationships/{entity2}"}) - @Transactional @PreAuthorize("hasRole('USER')") public ResponseEntity patch(@RequestBody final String body, final HttpServletRequest request, @@ -94,11 +93,11 @@ public ResponseEntity patch(@RequestBody final String body, return wrapResponse(response); } + //!!! No @Transactional - transactions are being handled by Elide @RequestMapping( method = RequestMethod.DELETE, produces = JSON_API_MEDIA_TYPE, value = {"/{entity}/{id}", "/{entity}/{id}/relationships/{entity2}"}) - @Transactional @PreAuthorize("hasRole('USER')") public ResponseEntity delete(final HttpServletRequest request, final Authentication authentication) {