From 7f500d27b686f1d316f2655a364c19362d3d7150 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Fri, 26 Jan 2024 13:39:20 +0100 Subject: [PATCH] improved error checking and error messages for ImportOfficeData --- .../hs/office/migration/ImportOfficeData.java | 84 +++++++++++++------ 1 file changed, 58 insertions(+), 26 deletions(-) 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 68d6fe39..b0aa2c9e 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 @@ -136,17 +136,17 @@ public class ImportOfficeData extends ContextBasedTest { @Value("${hsadminng.superuser}") private String rbacSuperuser; - private static NavigableMap contacts = new TreeMap<>(); - private static NavigableMap persons = new TreeMap<>(); - private static NavigableMap partners = new TreeMap<>(); - private static NavigableMap debitors = new TreeMap<>(); - private static NavigableMap memberships = new TreeMap<>(); + private static Map contacts = new WriteOnceMap<>(); + private static Map persons = new WriteOnceMap<>(); + private static Map partners = new WriteOnceMap<>(); + private static Map debitors = new WriteOnceMap<>(); + private static Map memberships = new WriteOnceMap<>(); - private static NavigableMap relationships = new TreeMap<>(); - private static NavigableMap sepaMandates = new TreeMap<>(); - private static NavigableMap bankAccounts = new TreeMap<>(); - private static NavigableMap coopShares = new TreeMap<>(); - private static NavigableMap coopAssets = new TreeMap<>(); + private static Map relationships = new WriteOnceMap<>(); + private static Map sepaMandates = new WriteOnceMap<>(); + private static Map bankAccounts = new WriteOnceMap<>(); + private static Map coopShares = new WriteOnceMap<>(); + private static Map coopAssets = new WriteOnceMap<>(); @PersistenceContext EntityManager em; @@ -521,10 +521,10 @@ public class ImportOfficeData extends ContextBasedTest { private void persist(final Integer id, final HasUuid entity) { try { - System.out.println("persisting #" + entity.hashCode() + ": " + entity); + //System.out.println("persisting #" + entity.hashCode() + ": " + entity); em.persist(entity); - em.flush(); - System.out.println("persisted #" + entity.hashCode() + " as " + entity.getUuid()); + //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); @@ -645,6 +645,7 @@ public class ImportOfficeData extends ContextBasedTest { .personType(HsOfficePersonType.LEGAL_PERSON) .tradeName("Hostsharing eG") .build(); + assertThat(persons.containsKey(1)).describedAs("overwriting " + persons.get(1) + " index " + 1 + " with " + mandant).isFalse(); persons.put(1, mandant); records.stream() @@ -659,7 +660,10 @@ public class ImportOfficeData extends ContextBasedTest { .relAnchor(mandant) .contact(null) // is set during contacts import depending on assigned roles .build(); - relationships.put(relationshipId++, partnerRelationship); + final Integer i3 = relationshipId++; + assertThat(relationships.containsKey(i3)).describedAs("overwriting " + relationships.get(i3) + " index " + i3 + + " with " + partnerRelationship).isFalse(); + relationships.put(i3, partnerRelationship); final var partner = HsOfficePartnerEntity.builder() .partnerNumber(rec.getInteger("member_id")) @@ -668,7 +672,9 @@ public class ImportOfficeData extends ContextBasedTest { .contact(null) // is set during contacts import depending on assigned roles .person(person) .build(); - partners.put(rec.getInteger("bp_id"), partner); + final Integer i2 = rec.getInteger("bp_id"); + assertThat(partners.containsKey(i2)).describedAs("overwriting " + partners.get(i2) + " index " + i2 + " with " + partner).isFalse(); + partners.put(i2, partner); final var debitor = HsOfficeDebitorEntity.builder() .partner(partner) @@ -680,7 +686,9 @@ public class ImportOfficeData extends ContextBasedTest { .vatBusiness("GROSS".equals(rec.getString("indicator_vat"))) // TODO: remove .vatId(rec.getString("uid_vat")) .build(); - debitors.put(rec.getInteger("bp_id"), debitor); + final Integer i1 = rec.getInteger("bp_id"); + assertThat(debitors.containsKey(i1)).describedAs("overwriting " + debitors.get(i1) + " index " + i1 + " with " + debitor).isFalse(); + debitors.put(i1, debitor); if (isNotBlank(rec.getString("member_since"))) { assertThat(rec.getInteger("member_id")).isEqualTo(partner.getPartnerNumber()); @@ -697,7 +705,9 @@ public class ImportOfficeData extends ContextBasedTest { : HsOfficeReasonForTermination.UNKNOWN) .mainDebitor(debitor) .build(); - memberships.put(rec.getInteger("bp_id"), membership); + final Integer i = rec.getInteger("bp_id"); + assertThat(memberships.containsKey(i)).describedAs("overwriting " + memberships.get(i) + " index " + i + " with " + membership).isFalse(); + memberships.put(i, membership); } }); } @@ -726,7 +736,9 @@ public class ImportOfficeData extends ContextBasedTest { .comment( rec.getString("comment")) .build(); - coopShares.put(rec.getInteger("member_share_id"), shareTransaction); + final Integer i = rec.getInteger("member_share_id"); + assertThat(coopShares.containsKey(i)).describedAs("overwriting " + coopShares.get(i) + " index " + i + " with " + shareTransaction).isFalse(); + coopShares.put(i, shareTransaction); }); } @@ -769,7 +781,9 @@ public class ImportOfficeData extends ContextBasedTest { .comment(rec.getString("comment")) .build(); - coopAssets.put(rec.getInteger("member_asset_id"), assetTransaction); + final Integer i = rec.getInteger("member_asset_id"); + assertThat(coopAssets.containsKey(i)).describedAs("overwriting " + coopAssets.get(i) + " index " + i + " with " + assetTransaction).isFalse(); + coopAssets.put(i, assetTransaction); }); } @@ -812,13 +826,14 @@ public class ImportOfficeData extends ContextBasedTest { .map(row -> new Record(columns, row)) .forEach(rec -> { final var contactId = rec.getInteger("contact_id"); + final var bpId = rec.getInteger("bp_id"); if (rec.getString("roles").isBlank()) { fail("empty roles assignment not allowed for contact_id: " + contactId); } - final var partner = partners.get(rec.getInteger("bp_id")); - final var debitor = debitors.get(rec.getInteger("bp_id")); + final var partner = partners.get(bpId); + final var debitor = debitors.get(bpId); final var partnerPerson = partner.getPerson(); if (containsPartnerRole(rec)) { @@ -870,14 +885,16 @@ public class ImportOfficeData extends ContextBasedTest { } private static void optionallyAddMissingContractualRelationships() { + final var contractualMissing = new HashSet(); partners.forEach( (id, partner) -> { final var partnerPerson = partner.getPerson(); if (relationships.values().stream().filter(rel -> rel.getRelHolder() == partnerPerson && rel.getRelType() == HsOfficeRelationshipType.REPRESENTATIVE).findFirst().isEmpty()) { addRelationship(partnerPerson, partnerPerson, partner.getContact(), HsOfficeRelationshipType.REPRESENTATIVE); + contractualMissing.add(partner.getPartnerNumber()); } }); + // assertThat(contractualMissing).isEmpty(); } - private static boolean containsRole(final Record rec, final String role) { final var roles = rec.getString("roles"); return ("," + roles + ",").contains("," + role + ","); @@ -898,7 +915,9 @@ public class ImportOfficeData extends ContextBasedTest { .contact(contact) .relType(representative) .build(); - relationships.put(relationshipId++, rel); + final Integer i = relationshipId++; + assertThat(relationships.containsKey(i)).describedAs("overwriting " + relationships.get(i) + " index " + i + " with " + rel).isFalse(); + relationships.put(i, rel); return rel; } @@ -909,7 +928,9 @@ public class ImportOfficeData extends ContextBasedTest { person.setTradeName(contactRecord.getString("firma")); determinePersonType(person, contactRecord.getString("roles")); - persons.put(contactRecord.getInteger("contact_id"), person); + final Integer i = contactRecord.getInteger("contact_id"); + assertThat(persons.containsKey(i)).describedAs("overwriting " + persons.get(i) + " index " + i + " with " + person).isFalse(); + persons.put(i, person); return person; } @@ -923,7 +944,7 @@ public class ImportOfficeData extends ContextBasedTest { if (roles.contains("contractual") && !roles.contains("partner") && !person.getFamilyName().isBlank() && !person.getGivenName().isBlank()) { person.setPersonType(HsOfficePersonType.NATURAL_PERSON); - } else if ( endsWithWord(person.getTradeName(), "e.K.", "e.G.", "eG", "GmbH", "AG") ) { + } else if ( endsWithWord(person.getTradeName(), "e.K.", "e.G.", "eG", "GmbH", "AG", "KG") ) { person.setPersonType(HsOfficePersonType.LEGAL_PERSON); } else if ( endsWithWord(person.getTradeName(), "OHG") ) { person.setPersonType(HsOfficePersonType.INCORPORATED_FIRM); @@ -964,7 +985,9 @@ public class ImportOfficeData extends ContextBasedTest { contact.setPostalAddress(toAddress(contactRecord)); contact.setPhoneNumbers(toPhoneNumbers(contactRecord)); - contacts.put(contactRecord.getInteger("contact_id"), contact); + final Integer i = contactRecord.getInteger("contact_id"); + assertThat(contacts.containsKey(i)).describedAs("overwriting " + contacts.get(i) + " index " + i + " with " + contact).isFalse(); + contacts.put(i, contact); return contact; } @@ -1160,3 +1183,12 @@ class OrderedDependedTestsExtension implements TestWatcher, BeforeEachCallback { assumeThat(previousTestsPassed).isTrue(); } } + +class WriteOnceMap extends TreeMap { + + @Override + public V put(final K k, final V v) { + assertThat(containsKey(k)).describedAs("overwriting " + get(k) + " index " + k + " with " + v).isFalse(); + return super.put(k, v); + } +}