1
0

add micrometer @Timing annotations to Controllers+Repositories + ArchTest (#128)

Co-authored-by: Michael Hoennig <michael@hoennig.de>
Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/128
Reviewed-by: Timotheus Pokorra <timotheus.pokorra@hostsharing.net>
This commit is contained in:
Michael Hoennig
2024-12-05 10:32:34 +01:00
parent 0832c90c82
commit ddba946d72
88 changed files with 461 additions and 136 deletions

View File

@@ -10,6 +10,7 @@ import com.tngtech.archunit.lang.ArchCondition;
import com.tngtech.archunit.lang.ArchRule;
import com.tngtech.archunit.lang.ConditionEvents;
import com.tngtech.archunit.lang.SimpleConditionEvent;
import io.micrometer.core.annotation.Timed;
import net.hostsharing.hsadminng.HsadminNgApplication;
import net.hostsharing.hsadminng.hs.booking.item.HsBookingItem;
import net.hostsharing.hsadminng.hs.hosting.asset.HsHostingAssetRbacEntity;
@@ -20,7 +21,9 @@ import org.springframework.web.bind.annotation.RestController;
import jakarta.persistence.Table;
import static com.tngtech.archunit.core.domain.JavaModifier.ABSTRACT;
import java.lang.annotation.Annotation;
import static com.tngtech.archunit.core.domain.JavaModifier.PUBLIC;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.*;
import static com.tngtech.archunit.library.dependencies.SlicesRuleDefinition.slices;
import static java.lang.String.format;
@@ -98,7 +101,7 @@ public class ArchitectureTest {
@SuppressWarnings("unused")
public static final ArchRule testClassesAreProperlyNamed = classes()
.that().haveSimpleNameEndingWith("Test")
.and().doNotHaveModifier(ABSTRACT)
.and().doNotHaveModifier(JavaModifier.ABSTRACT)
.should().haveNameMatching(".*(UnitTest|RestTest|IntegrationTest|AcceptanceTest|ScenarioTest|ArchitectureTest)$");
@ArchTest
@@ -346,11 +349,25 @@ public class ArchitectureTest {
static final ArchRule restControllerNaming =
classes().that().areAnnotatedWith(RestController.class).should().haveSimpleNameEndingWith("Controller");
@ArchTest
@SuppressWarnings("unused")
static final ArchRule restControllerMethods = classes()
.that().areAnnotatedWith(RestController.class)
.and().resideOutsideOfPackages("net.hostsharing.hsadminng.rbac.test", "net.hostsharing.hsadminng.rbac.test.*")
.should(haveAllPublicMethodsAnnotatedWith(Timed.class));
@ArchTest
@SuppressWarnings("unused")
static final ArchRule repositoryNaming =
classes().that().areAssignableTo(Repository.class).should().haveSimpleNameEndingWith("Repository");
@ArchTest
@SuppressWarnings("unused")
static final ArchRule repositoryMethods = classes()
.that().areAssignableTo(Repository.class)
.and().resideOutsideOfPackages("net.hostsharing.hsadminng.rbac.test", "net.hostsharing.hsadminng.rbac.test.*")
.should(haveAllPublicMethodsAnnotatedWith(Timed.class));
@ArchTest
@SuppressWarnings("unused")
static final ArchRule tableNamesOfRbacEntitiesShouldEndWith_rv =
@@ -389,4 +406,35 @@ public class ArchitectureTest {
}
};
}
private static ArchCondition<JavaClass> haveAllPublicMethodsAnnotatedWith(Class<? extends Annotation> annotation) {
return new ArchCondition<>("have all public methods annotated with @" + annotation.getSimpleName()) {
@Override
public void check(final JavaClass item, final ConditionEvents events) {
for (JavaMethod method : item.getMethods()) {
if (method.isAnnotatedWith(annotation)) {
continue;
}
if (isGeneratedSpringRepositoryMethod(item, method)) {
continue;
}
if (item.isAnnotatedWith(RestController.class) && !method.getModifiers().contains(PUBLIC)) {
continue;
}
final var message = String.format(
"Method %s in class %s is not annotated with @%s",
method.getName(),
item.getName(),
annotation.getSimpleName()
);
events.add(SimpleConditionEvent.violated(method, message));
}
}
};
}
private static boolean isGeneratedSpringRepositoryMethod(final JavaClass item, final JavaMethod method) {
// this is a heuristic, ideally we can determine all methods with generated database calls
return item.isAssignableTo(Repository.class) && !method.getModifiers().contains(JavaModifier.ABSTRACT);
}
}

View File

@@ -139,10 +139,10 @@ class HsBookingItemControllerAcceptanceTest extends ContextBasedTestWithCleanup
@Nested
@Order(3)
class AddBookingItem {
class PostNewBookingItem {
@Test
void globalAdmin_canAddBookingItem() {
void globalAdmin_canPostNewBookingItem() {
context.define("superuser-alex@hostsharing.net");
final var givenProject = findDefaultProjectOfDebitorNumber(1000111);
@@ -349,7 +349,7 @@ class HsBookingItemControllerAcceptanceTest extends ContextBasedTestWithCleanup
}
@Test
void projectAgent_canAddBookingItemEvenIfHostingAssetCreationFails() {
void projectAgent_canPostNewBookingItemEvenIfHostingAssetCreationFails() {
context.define("superuser-alex@hostsharing.net", "hs_booking.project#D-1000111-D-1000111defaultproject:AGENT");
final var givenProject = findDefaultProjectOfDebitorNumber(1000111);

View File

@@ -84,7 +84,7 @@ class HsBookingItemControllerRestTest {
}
@Nested
class AddBookingItem {
class PostNewBookingItem {
@Test
void globalAdmin_canAddValidBookingItem() throws Exception {

View File

@@ -79,10 +79,10 @@ class HsBookingProjectControllerAcceptanceTest extends ContextBasedTestWithClean
}
@Nested
class AddBookingProject {
class PostNewBookingProject {
@Test
void globalAdmin_canAddBookingProject() {
void globalAdmin_canPostNewBookingProject() {
context.define("superuser-alex@hostsharing.net");
final var givenDebitor = debitorRepo.findByDebitorNumber(1000111).stream()

View File

@@ -76,7 +76,7 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup
@Nested
@Order(2)
class ListAssets {
class GetListOfHostingAssets {
@Test
void globalAdmin_canViewAllAssetsOfArbitraryDebitor() {
@@ -146,7 +146,7 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup
@Nested
@Order(3)
class AddAsset {
class PostNewHostingAsset {
@Test
void globalAdmin_canAddBookedAsset() {
@@ -481,7 +481,7 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup
@Nested
@Order(4)
class PatchAsset {
class PatchHostingAsset {
@Test
void globalAdmin_canPatchAllUpdatablePropertiesOfAsset() {

View File

@@ -583,7 +583,7 @@ public class HsHostingAssetControllerRestTest {
@ParameterizedTest
@EnumSource(HsHostingAssetControllerRestTest.ListTestCases.class)
void shouldListAssets(final HsHostingAssetControllerRestTest.ListTestCases testCase) throws Exception {
void shouldGetListOfHostingAssets(final HsHostingAssetControllerRestTest.ListTestCases testCase) throws Exception {
// given
when(rbacAssetRepo.findAllByCriteria(null, null, testCase.assetType))
.thenReturn(testCase.givenHostingAssetsOfType);
@@ -607,7 +607,7 @@ public class HsHostingAssetControllerRestTest {
}
@Test
void shouldPatchAsset() throws Exception {
void shouldPatchHostingAsset() throws Exception {
// given
final var givenDomainSetup = HsHostingAssetRealEntity.builder()
.type(HsHostingAssetType.DOMAIN_SETUP)

View File

@@ -50,7 +50,7 @@ class HsOfficeBankAccountControllerAcceptanceTest extends ContextBasedTestWithCl
EntityManager em;
@Nested
class ListBankAccounts {
class GetListOfBankAccounts {
@Test
void globalAdmin_withoutAssumedRoles_canViewAllBankAccounts_ifNoCriteriaGiven() throws JSONException {
@@ -117,7 +117,7 @@ class HsOfficeBankAccountControllerAcceptanceTest extends ContextBasedTestWithCl
class CreateBankAccount {
@Test
void globalAdmin_withoutAssumedRole_canAddBankAccount() {
void globalAdmin_withoutAssumedRole_canPostNewBankAccount() {
context.define("superuser-alex@hostsharing.net");

View File

@@ -128,7 +128,7 @@ class HsOfficeBankAccountRepositoryIntegrationTest extends ContextBasedTestWithC
}
@Nested
class ListBankAccounts {
class GetListOfBankAccounts {
@Test
public void globalAdmin_canViewAllBankAccounts() {

View File

@@ -58,7 +58,7 @@ class HsOfficePartnerControllerAcceptanceTest extends ContextBasedTestWithCleanu
@Nested
@Transactional
class ListPartners {
class GetListOfPartners {
@Test
void globalAdmin_withoutAssumedRoles_canViewAllPartners_ifNoCriteriaGiven() {
@@ -87,10 +87,10 @@ class HsOfficePartnerControllerAcceptanceTest extends ContextBasedTestWithCleanu
@Nested
@Transactional
class AddPartner {
class PostNewPartner {
@Test
void globalAdmin_withoutAssumedRole_canAddPartner() {
void globalAdmin_withoutAssumedRole_canPostNewPartner() {
context.define("superuser-alex@hostsharing.net");
final var givenMandantPerson = personRepo.findPersonByOptionalNameLike("Hostsharing eG").stream().findFirst().orElseThrow();
@@ -150,7 +150,7 @@ class HsOfficePartnerControllerAcceptanceTest extends ContextBasedTestWithCleanu
}
@Test
void globalAdmin_canNotAddPartner_ifContactDoesNotExist() {
void globalAdmin_canNotPostNewPartner_ifContactDoesNotExist() {
context.define("superuser-alex@hostsharing.net");
final var givenMandantPerson = personRepo.findPersonByOptionalNameLike("Hostsharing eG").get(0);
@@ -188,7 +188,7 @@ class HsOfficePartnerControllerAcceptanceTest extends ContextBasedTestWithCleanu
}
@Test
void globalAdmin_canNotAddPartner_ifPersonDoesNotExist() {
void globalAdmin_canNotPostNewPartner_ifPersonDoesNotExist() {
context.define("superuser-alex@hostsharing.net");
final var mandantPerson = personRepo.findPersonByOptionalNameLike("Hostsharing eG").get(0);

View File

@@ -91,7 +91,7 @@ class HsOfficePartnerControllerRestTest {
}
@Nested
class AddPartner {
class PostNewPartner {
@Test
void respondBadRequest_ifPersonUuidIsInvalid() throws Exception {

View File

@@ -52,7 +52,7 @@ class HsOfficePersonControllerAcceptanceTest extends ContextBasedTestWithCleanup
EntityManager em;
@Nested
class ListPersons {
class GetListOfPersons {
@Test
void globalAdmin_withoutAssumedRoles_canViewAllPersons_ifNoCriteriaGiven() {

View File

@@ -54,7 +54,7 @@ class HsOfficeRelationControllerAcceptanceTest extends ContextBasedTestWithClean
JpaAttempt jpaAttempt;
@Nested
class ListRelations {
class GetListOfRelations {
@Test
void globalAdmin_withoutAssumedRoles_canViewAllRelationsOfGivenPersonAndType() {

View File

@@ -132,10 +132,10 @@ class HsOfficeSepaMandateControllerAcceptanceTest extends ContextBasedTestWithCl
}
@Nested
class AddSepaMandate {
class PostNewSepaMandate {
@Test
void globalAdmin_canAddSepaMandate() {
void globalAdmin_canPostNewSepaMandate() {
context.define("superuser-alex@hostsharing.net");
final var givenDebitor = debitorRepo.findDebitorByOptionalNameLike("Third").get(0);
@@ -177,7 +177,7 @@ class HsOfficeSepaMandateControllerAcceptanceTest extends ContextBasedTestWithCl
// TODO.test: move validation tests to a ...WebMvcTest
@Test
void globalAdmin_canNotAddSepaMandateWhenDebitorUuidIsMissing() {
void globalAdmin_canNotPostNewSepaMandateWhenDebitorUuidIsMissing() {
context.define("superuser-alex@hostsharing.net");
final var givenDebitor = debitorRepo.findDebitorByOptionalNameLike("Third").get(0);
@@ -202,7 +202,7 @@ class HsOfficeSepaMandateControllerAcceptanceTest extends ContextBasedTestWithCl
}
@Test
void globalAdmin_canNotAddSepaMandate_ifBankAccountDoesNotExist() {
void globalAdmin_canNotPostNewSepaMandate_ifBankAccountDoesNotExist() {
context.define("superuser-alex@hostsharing.net");
final var givenDebitor = debitorRepo.findDebitorByOptionalNameLike("Third").get(0);
@@ -232,7 +232,7 @@ class HsOfficeSepaMandateControllerAcceptanceTest extends ContextBasedTestWithCl
}
@Test
void globalAdmin_canNotAddSepaMandate_ifPersonDoesNotExist() {
void globalAdmin_canNotPostNewSepaMandate_ifPersonDoesNotExist() {
context.define("superuser-alex@hostsharing.net");
final var givenDebitorUuid = UUID.fromString("00000000-0000-0000-0000-000000000000");

View File

@@ -161,7 +161,7 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest {
}
@Nested
class GetGrantById {
class GetListOfGrantsByUuid {
@Test
void customerAdmin_withAssumedPacketAdminRole_canReadPacketAdminsGrantById() {
@@ -171,7 +171,7 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest {
final var givenGrantedRole = getRbacRoleByName("rbactest.package#xxx00:ADMIN");
// when
final var grant = givencurrentSubjectAsPackageAdmin.getGrantById()
final var grant = givencurrentSubjectAsPackageAdmin.getListOfGrantsByUuid()
.forGrantedRole(givenGrantedRole).toGranteeUser(givenGranteeUser);
// then
@@ -190,7 +190,7 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest {
final var givenGrantedRole = getRbacRoleByName("rbactest.package#xxx00:ADMIN");
// when
final var grant = givencurrentSubjectAsPackageAdmin.getGrantById()
final var grant = givencurrentSubjectAsPackageAdmin.getListOfGrantsByUuid()
.forGrantedRole(givenGrantedRole).toGranteeUser(givenGranteeUser);
// then
@@ -211,7 +211,7 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest {
final var givenGrantedRole = getRbacRoleByName("rbactest.package#xxx00:ADMIN");
// when
final var grant = givencurrentSubjectAsPackageAdmin.getGrantById()
final var grant = givencurrentSubjectAsPackageAdmin.getListOfGrantsByUuid()
.forGrantedRole(givenGrantedRole).toGranteeUser(givenGranteeUser);
// then
@@ -231,7 +231,7 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest {
"rbactest.package#xxx00:TENANT");
final var givenGranteeUser = findRbacSubjectByName("pac-admin-xxx00@xxx.example.com");
final var givenGrantedRole = getRbacRoleByName("rbactest.package#xxx00:ADMIN");
final var grant = givencurrentSubjectAsPackageAdmin.getGrantById()
final var grant = givencurrentSubjectAsPackageAdmin.getListOfGrantsByUuid()
.forGrantedRole(givenGrantedRole).toGranteeUser(givenGranteeUser);
// then
@@ -360,8 +360,8 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest {
return new RevokeFixture(givenOwnPackageAdminRole);
}
GetGrantByIdFixture getGrantById() {
return new GetGrantByIdFixture();
GetListOfGrantsByUuidFixture getListOfGrantsByUuid() {
return new GetListOfGrantsByUuidFixture();
}
class GrantFixture {
@@ -443,12 +443,12 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest {
}
}
private class GetGrantByIdFixture {
private class GetListOfGrantsByUuidFixture {
private Subject currentSubject = Subject.this;
private RbacRoleEntity grantedRole;
GetGrantByIdFixture forGrantedRole(final RbacRoleEntity grantedRole) {
GetListOfGrantsByUuidFixture forGrantedRole(final RbacRoleEntity grantedRole) {
this.grantedRole = grantedRole;
return this;
}

View File

@@ -1,5 +1,6 @@
package net.hostsharing.hsadminng.rbac.role;
import io.micrometer.core.annotation.Timed;
import org.springframework.data.repository.Repository;
import java.util.List;
@@ -7,5 +8,6 @@ import java.util.UUID;
public interface RawRbacObjectRepository extends Repository<RawRbacObjectEntity, UUID> {
@Timed("app.rbac.objects.repo.findAll.real")
List<RawRbacObjectEntity> findAll();
}

View File

@@ -1,5 +1,6 @@
package net.hostsharing.hsadminng.rbac.role;
import io.micrometer.core.annotation.Timed;
import org.springframework.data.repository.Repository;
import java.util.List;
@@ -7,5 +8,6 @@ import java.util.UUID;
public interface RawRbacRoleRepository extends Repository<RawRbacRoleEntity, UUID> {
@Timed("app.rbac.roles.repo.findAll.real")
List<RawRbacRoleEntity> findAll();
}

View File

@@ -45,7 +45,7 @@ class RbacSubjectControllerRestTest {
@Test
void createSubjectUsesGivenUuid() throws Exception {
void postNewSubjectUsesGivenUuid() throws Exception {
// given
final var givenUuid = UUID.randomUUID();
@@ -69,7 +69,7 @@ class RbacSubjectControllerRestTest {
}
@Test
void createSubjectGeneratesRandomUuidIfNotGiven() throws Exception {
void postNewSubjectGeneratesRandomUuidIfNotGiven() throws Exception {
// when
mockMvc.perform(MockMvcRequestBuilders
.post("/api/rbac/subjects")

View File

@@ -41,7 +41,7 @@ class RbacSubjectRepositoryIntegrationTest extends ContextBasedTest {
HttpServletRequest request;
@Nested
class CreateSubject {
class PostNewSubject {
@Test
@Transactional(propagation = Propagation.NEVER)
@@ -178,7 +178,7 @@ class RbacSubjectRepositoryIntegrationTest extends ContextBasedTest {
}
@Nested
class ListSubjectPermissions {
class GetListOfSubjectPermissions {
private static final String[] ALL_USER_PERMISSIONS = Array.of(
// @formatter:off