only use @Transactional from Spring (not JavaX) and use readOnly=true where possible

This commit is contained in:
Michael Hoennig 2022-08-17 16:38:03 +02:00
parent 6be5cfd923
commit 787400c089
9 changed files with 37 additions and 23 deletions

View File

@ -90,7 +90,8 @@ openapiProcessor {
} }
} }
sourceSets.main.java.srcDir 'build/generated/sources/openapi' sourceSets.main.java.srcDir 'build/generated/sources/openapi'
compileJava.dependsOn('processSpring') project.tasks.processResources.dependsOn('processSpring')
project.tasks.compileJava.dependsOn('processSpring')
spotless { spotless {
java { java {

View File

@ -5,10 +5,10 @@ import net.hostsharing.hsadminng.generated.api.v1.api.CustomersApi;
import net.hostsharing.hsadminng.generated.api.v1.model.CustomerResource; import net.hostsharing.hsadminng.generated.api.v1.model.CustomerResource;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.bind.annotation.RestController; import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder; import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder;
import javax.transaction.Transactional;
import java.util.List; import java.util.List;
import java.util.UUID; import java.util.UUID;
@ -26,7 +26,7 @@ public class CustomerController implements CustomersApi {
private CustomerRepository customerRepository; private CustomerRepository customerRepository;
@Override @Override
@Transactional @Transactional(readOnly = true)
public ResponseEntity<List<CustomerResource>> listCustomers( public ResponseEntity<List<CustomerResource>> listCustomers(
String userName, String userName,
String assumedRoles, String assumedRoles,

View File

@ -7,9 +7,9 @@ import net.hostsharing.hsadminng.generated.api.v1.model.PackageResource;
import net.hostsharing.hsadminng.generated.api.v1.model.PackageUpdateResource; import net.hostsharing.hsadminng.generated.api.v1.model.PackageUpdateResource;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.bind.annotation.RestController; import org.springframework.web.bind.annotation.RestController;
import javax.transaction.Transactional;
import java.util.List; import java.util.List;
import java.util.UUID; import java.util.UUID;
@ -26,7 +26,7 @@ public class PackageController implements PackagesApi {
private PackageRepository packageRepository; private PackageRepository packageRepository;
@Override @Override
@Transactional @Transactional(readOnly = true)
public ResponseEntity<List<PackageResource>> listPackages( public ResponseEntity<List<PackageResource>> listPackages(
String userName, String userName,
String assumedRoles, String assumedRoles,

View File

@ -2,16 +2,15 @@ package net.hostsharing.hsadminng.rbac.rbacgrant;
import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.context.Context;
import net.hostsharing.hsadminng.generated.api.v1.api.RbacgrantsApi; import net.hostsharing.hsadminng.generated.api.v1.api.RbacgrantsApi;
import net.hostsharing.hsadminng.generated.api.v1.api.RbacrolesApi;
import net.hostsharing.hsadminng.generated.api.v1.model.RbacGrantResource; import net.hostsharing.hsadminng.generated.api.v1.model.RbacGrantResource;
import net.hostsharing.hsadminng.generated.api.v1.model.RbacRoleResource;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.bind.annotation.RestController; import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder; import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder;
import javax.transaction.Transactional;
import java.util.List; import java.util.List;
import java.util.UUID;
import static net.hostsharing.hsadminng.Mapper.map; import static net.hostsharing.hsadminng.Mapper.map;
import static net.hostsharing.hsadminng.Mapper.mapList; import static net.hostsharing.hsadminng.Mapper.mapList;
@ -27,7 +26,7 @@ public class RbacGrantController implements RbacgrantsApi {
private RbacGrantRepository rbacGrantRepository; private RbacGrantRepository rbacGrantRepository;
@Override @Override
@Transactional @Transactional(readOnly = true)
public ResponseEntity<List<RbacGrantResource>> listUserGrants( public ResponseEntity<List<RbacGrantResource>> listUserGrants(
final String currentUser, final String currentUser,
final String assumedRoles) { final String assumedRoles) {

View File

@ -5,9 +5,9 @@ import net.hostsharing.hsadminng.generated.api.v1.api.RbacrolesApi;
import net.hostsharing.hsadminng.generated.api.v1.model.RbacRoleResource; import net.hostsharing.hsadminng.generated.api.v1.model.RbacRoleResource;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.bind.annotation.RestController; import org.springframework.web.bind.annotation.RestController;
import javax.transaction.Transactional;
import java.util.List; import java.util.List;
import static net.hostsharing.hsadminng.Mapper.mapList; import static net.hostsharing.hsadminng.Mapper.mapList;
@ -23,7 +23,7 @@ public class RbacRoleController implements RbacrolesApi {
private RbacRoleRepository rbacRoleRepository; private RbacRoleRepository rbacRoleRepository;
@Override @Override
@Transactional @Transactional(readOnly = true)
public ResponseEntity<List<RbacRoleResource>> listRoles( public ResponseEntity<List<RbacRoleResource>> listRoles(
final String currentUser, final String currentUser,
final String assumedRoles) { final String assumedRoles) {

View File

@ -6,11 +6,11 @@ import net.hostsharing.hsadminng.generated.api.v1.model.RbacUserPermissionResour
import net.hostsharing.hsadminng.generated.api.v1.model.RbacUserResource; import net.hostsharing.hsadminng.generated.api.v1.model.RbacUserResource;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.bind.annotation.*; import org.springframework.web.bind.annotation.*;
import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder; import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder;
import javax.persistence.EntityManager; import javax.persistence.EntityManager;
import javax.transaction.Transactional;
import java.util.List; import java.util.List;
import java.util.UUID; import java.util.UUID;
@ -48,7 +48,7 @@ public class RbacUserController implements RbacusersApi {
} }
@Override @Override
@Transactional @Transactional(readOnly=true)
public ResponseEntity<List<RbacUserResource>> listUsers( public ResponseEntity<List<RbacUserResource>> listUsers(
@RequestHeader(name = "current-user") final String currentUserName, @RequestHeader(name = "current-user") final String currentUserName,
@RequestHeader(name = "assumed-roles", required = false) final String assumedRoles, @RequestHeader(name = "assumed-roles", required = false) final String assumedRoles,
@ -62,7 +62,7 @@ public class RbacUserController implements RbacusersApi {
} }
@Override @Override
@Transactional @Transactional(readOnly=true)
public ResponseEntity<List<RbacUserPermissionResource>> listUserPermissions( public ResponseEntity<List<RbacUserPermissionResource>> listUserPermissions(
@RequestHeader(name = "current-user") final String currentUserName, @RequestHeader(name = "current-user") final String currentUserName,
@RequestHeader(name = "assumed-roles", required = false) final String assumedRoles, @RequestHeader(name = "assumed-roles", required = false) final String assumedRoles,

View File

@ -8,8 +8,7 @@ import org.junit.jupiter.api.Test;
import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.web.bind.annotation.RestController; import org.springframework.web.bind.annotation.RestController;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.*;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.methods;
import static com.tngtech.archunit.library.dependencies.SlicesRuleDefinition.slices; import static com.tngtech.archunit.library.dependencies.SlicesRuleDefinition.slices;
@AnalyzeClasses(packages = ArchUnitTest.NET_HOSTSHARING_HSADMINNG) @AnalyzeClasses(packages = ArchUnitTest.NET_HOSTSHARING_HSADMINNG)
@ -66,6 +65,22 @@ public class ArchUnitTest {
.should().haveSimpleNameEndingWith("AcceptanceTest") .should().haveSimpleNameEndingWith("AcceptanceTest")
.orShould().haveSimpleNameNotContaining("AcceptanceTest$"); .orShould().haveSimpleNameNotContaining("AcceptanceTest$");
@ArchTest
@SuppressWarnings("unused")
public static final ArchRule doNotUseJavaxTransactionAnnotationAtClassLevel = noClasses()
.should().beAnnotatedWith(javax.transaction.Transactional.class.getName())
.as("Use @%s instead of @%s." .formatted(
org.springframework.transaction.annotation.Transactional.class.getName(),
javax.transaction.Transactional.class));
@ArchTest
@SuppressWarnings("unused")
public static final ArchRule doNotUseJavaxTransactionAnnotationAtMethodLevel = noMethods()
.should().beAnnotatedWith(javax.transaction.Transactional.class)
.as("Use @%s instead of @%s." .formatted(
org.springframework.transaction.annotation.Transactional.class.getName(),
javax.transaction.Transactional.class.getName()));
@Test @Test
public void everythingShouldBeFreeOfCycles() { public void everythingShouldBeFreeOfCycles() {
slices().matching("net.hostsharing.hsadminng.(*)..").should().beFreeOfCycles(); slices().matching("net.hostsharing.hsadminng.(*)..").should().beFreeOfCycles();

View File

@ -24,6 +24,7 @@ import java.util.List;
import java.util.UUID; import java.util.UUID;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assumptions.assumeThat;
import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.containsString;
@SpringBootTest( @SpringBootTest(
@ -31,7 +32,7 @@ import static org.hamcrest.CoreMatchers.containsString;
classes = { HsadminNgApplication.class, JpaAttempt.class } classes = { HsadminNgApplication.class, JpaAttempt.class }
) )
@Accepts({ "GRT:S(Schema)" }) @Accepts({ "GRT:S(Schema)" })
@Transactional(propagation = Propagation.NEVER) @Transactional(readOnly = true, propagation = Propagation.NEVER)
class RbacGrantControllerAcceptanceTest { class RbacGrantControllerAcceptanceTest {
@LocalServerPort @LocalServerPort
@ -64,8 +65,8 @@ class RbacGrantControllerAcceptanceTest {
// given // given
final var givenNewUserName = "test-user-" + RandomStringUtils.randomAlphabetic(8) + "@example.com"; final var givenNewUserName = "test-user-" + RandomStringUtils.randomAlphabetic(8) + "@example.com";
final String givenCurrentUserPackageAdmin = "aaa00@aaa.example.com"; final var givenCurrentUserPackageAdmin = "aaa00@aaa.example.com";
final String givenAssumedRole = "package#aaa00.admin"; final var givenAssumedRole = "package#aaa00.admin";
final var givenOwnPackageAdminRole = "package#aaa00.admin"; final var givenOwnPackageAdminRole = "package#aaa00.admin";
// when // when
@ -105,8 +106,8 @@ class RbacGrantControllerAcceptanceTest {
// given // given
final var givenNewUserName = "test-user-" + RandomStringUtils.randomAlphabetic(8) + "@example.com"; final var givenNewUserName = "test-user-" + RandomStringUtils.randomAlphabetic(8) + "@example.com";
final String givenCurrentUserPackageAdmin = "aaa00@aaa.example.com"; final var givenCurrentUserPackageAdmin = "aaa00@aaa.example.com";
final String givenAssumedRole = "package#aaa00.admin"; final var givenAssumedRole = "package#aaa00.admin";
final var givenAlienPackageAdminRole = "package#aab00.admin"; final var givenAlienPackageAdminRole = "package#aab00.admin";
// when // when

View File

@ -9,7 +9,6 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.ComponentScan;
import org.springframework.orm.jpa.JpaSystemException; import org.springframework.orm.jpa.JpaSystemException;
import org.springframework.test.annotation.Commit;
import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.annotation.DirtiesContext;
import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.annotation.Transactional;
@ -59,7 +58,6 @@ class RbacUserRepositoryIntegrationTest {
} }
@Test @Test
@Commit
@Transactional(propagation = Propagation.NEVER) @Transactional(propagation = Propagation.NEVER)
void anyoneCanCreateTheirOwnUser_committed() { void anyoneCanCreateTheirOwnUser_committed() {