From c1490e5901c3258bd1ca6f99e3076df3e4130edf Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Thu, 1 Feb 2024 09:05:16 +0100 Subject: [PATCH] amendments according to self-review --- .../hs/office/partner/HsOfficePartnerController.java | 3 ++- .../hsadminng/rbac/rbacrole/RbacRoleRepository.java | 4 ++-- src/main/resources/db/changelog/050-rbac-base.sql | 2 +- .../hsadminng/hs/office/migration/ImportOfficeData.java | 7 ++++--- .../partner/HsOfficePartnerControllerAcceptanceTest.java | 3 ++- .../hs/office/test/ContextBasedTestWithCleanup.java | 2 +- 6 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerController.java index 7bcaeb2a..88186664 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerController.java @@ -110,6 +110,7 @@ public class HsOfficePartnerController implements HsOfficePartnersApi { } if (partnerRepo.deleteByUuid(partnerUuid) != 1 || + // TODO: move to after delete trigger in partner relationshipRepo.deleteByUuid(partnerToDelete.get().getPartnerRole().getUuid()) != 1 ) { ResponseEntity.internalServerError().build(); } @@ -159,7 +160,7 @@ public class HsOfficePartnerController implements HsOfficePartnersApi { private E ref(final Class entityClass, final UUID uuid) { try { final var e = em.getReference(entityClass, uuid); - em.contains(e); + em.contains(e); // TODO: check if this is really needed to force an exception if not existing return e; } catch (final Throwable exc) { throw new ReferenceNotFoundException(entityClass, uuid, exc); diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepository.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepository.java index 747bb119..94633d7c 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepository.java @@ -10,10 +10,10 @@ public interface RbacRoleRepository extends Repository { /** * @return the number of persistent RbacRoleEntity instances, mostly for testing purposes. */ - long count(); + long count(); // TODO: move to test sources /** - * @return all persistent RbacRoleEntity instances, mostly for testing purposes. + * @return all persistent RbacRoleEntity instances, assigned to the current subject (user or assumed roles) */ List findAll(); diff --git a/src/main/resources/db/changelog/050-rbac-base.sql b/src/main/resources/db/changelog/050-rbac-base.sql index fc9506df..aab14b95 100644 --- a/src/main/resources/db/changelog/050-rbac-base.sql +++ b/src/main/resources/db/changelog/050-rbac-base.sql @@ -120,7 +120,7 @@ $$; create table RbacObject ( uuid uuid primary key default uuid_generate_v4(), - serialId serial, + serialId serial, -- TODO: we might want to remove this once test data deletion works properly objectTable varchar(64) not null, unique (objectTable, uuid) ); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/migration/ImportOfficeData.java b/src/test/java/net/hostsharing/hsadminng/hs/office/migration/ImportOfficeData.java index 8b200f59..a0d37f30 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/migration/ImportOfficeData.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/migration/ImportOfficeData.java @@ -524,8 +524,9 @@ public class ImportOfficeData extends ContextBasedTest { try { //System.out.println("persisting #" + entity.hashCode() + ": " + entity); em.persist(entity); - //em.flush(); - //System.out.println("persisted #" + entity.hashCode() + " as " + entity.getUuid()); + // uncomment for debugging purposes + // em.flush(); + // System.out.println("persisted #" + entity.hashCode() + " as " + entity.getUuid()); } catch (Exception exc) { System.err.println("failed to persist #" + entity.hashCode() + ": " + entity); System.err.println(exc); @@ -896,7 +897,7 @@ public class ImportOfficeData extends ContextBasedTest { contractualMissing.add(partner.getPartnerNumber()); } }); - // assertThat(contractualMissing).isEmpty(); + // assertThat(contractualMissing).isEmpty(); uncomment if we don't want allow missing contractual contact } private static boolean containsRole(final Record rec, final String role) { final var roles = rec.getString("roles"); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java index 4987c21f..76afd85e 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java @@ -491,8 +491,9 @@ class HsOfficePartnerControllerAcceptanceTest extends ContextBasedTestWithCleanu @AfterEach void cleanup() { - cleanupAllNew(HsOfficePartnerDetailsEntity.class); // TODO: should not be necessary cleanupAllNew(HsOfficePartnerEntity.class); + + // TODO: should not be necessary anymore, once it's deleted via after delete trigger cleanupAllNew(HsOfficeRelationshipEntity.class); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/test/ContextBasedTestWithCleanup.java b/src/test/java/net/hostsharing/hsadminng/hs/office/test/ContextBasedTestWithCleanup.java index 4a203e92..7bc34a44 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/test/ContextBasedTestWithCleanup.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/test/ContextBasedTestWithCleanup.java @@ -24,7 +24,7 @@ import static java.util.stream.Collectors.toSet; import static org.apache.commons.collections4.SetUtils.difference; import static org.assertj.core.api.Assertions.assertThat; -//@DirtiesContext +// TODO: cleanup the whole class public abstract class ContextBasedTestWithCleanup extends ContextBasedTest { private static final boolean DETAILED_BUT_SLOW_CHECK = true;