From a7d30726a67fe3bf0863b0a9a23cbe95d6ff20fb Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Mon, 29 Apr 2024 15:32:39 +0200 Subject: [PATCH 1/8] extract PatchableMapWrapper.of for reusability / less boilerplate code --- .../hs/booking/item/HsBookingItemEntity.java | 24 +++++-------- .../hosting/asset/HsHostingAssetEntity.java | 16 +++------ .../hsadminng/mapper/PatchableMapWrapper.java | 34 ++++++++++++------- 3 files changed, 36 insertions(+), 38 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java index 8bdb5c8b..a0574bce 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java @@ -98,7 +98,15 @@ public class HsBookingItemEntity implements Stringifyable, RbacObject { private Map resources = new HashMap<>(); @Transient - private PatchableMapWrapper resourcesWrapper; + private PatchableMapWrapper resourcesWrapper; + + public PatchableMapWrapper getResources() { + return PatchableMapWrapper.of(resourcesWrapper, (newWrapper) -> {resourcesWrapper = newWrapper; }, resources ); + } + + public void putResources(Map newResources) { + getResources().assign(newResources); + } public void setValidFrom(final LocalDate validFrom) { setValidity(toPostgresDateRange(validFrom, getValidTo())); @@ -116,20 +124,6 @@ public class HsBookingItemEntity implements Stringifyable, RbacObject { return upperInclusiveFromPostgresDateRange(getValidity()); } - public PatchableMapWrapper getResources() { - if ( resourcesWrapper == null ) { - resourcesWrapper = new PatchableMapWrapper(resources); - } - return resourcesWrapper; - } - - public void putResources(Map entries) { - if ( resourcesWrapper == null ) { - resourcesWrapper = new PatchableMapWrapper(resources); - } - resourcesWrapper.assign(entries); - } - @Override public String toString() { return stringify.apply(this); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java index 199e0e7b..462ccd2c 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java @@ -105,20 +105,14 @@ public class HsHostingAssetEntity implements Stringifyable, RbacObject { private Map config = new HashMap<>(); @Transient - private PatchableMapWrapper configWrapper; + private PatchableMapWrapper configWrapper; - public PatchableMapWrapper getConfig() { - if ( configWrapper == null ) { - configWrapper = new PatchableMapWrapper(config); - } - return configWrapper; + public PatchableMapWrapper getConfig() { + return PatchableMapWrapper.of(configWrapper, (newWrapper) -> {configWrapper = newWrapper; }, config ); } - public void putConfig(Map entries) { - if ( configWrapper == null ) { - configWrapper = new PatchableMapWrapper(config); - } - configWrapper.assign(entries); + public void putConfig(Map newConfg) { + PatchableMapWrapper.of(configWrapper, (newWrapper) -> {configWrapper = newWrapper; }, config).assign(newConfg); } @Override diff --git a/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java b/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java index 678a68cd..a3d2687e 100644 --- a/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java +++ b/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java @@ -6,31 +6,41 @@ import jakarta.validation.constraints.NotNull; import java.util.Collection; import java.util.Map; import java.util.Set; +import java.util.function.Consumer; +import static java.util.Optional.ofNullable; import static java.util.stream.Collectors.joining; /** This class wraps another (usually persistent) map and * supports applying `PatchMap` as well as a toString method with stable entry order. */ -public class PatchableMapWrapper implements Map { +public class PatchableMapWrapper implements Map { - private final Map delegate; + private final Map delegate; - public PatchableMapWrapper(final Map map) { + private PatchableMapWrapper(final Map map) { delegate = map; } + public static PatchableMapWrapper of(final PatchableMapWrapper currentWrapper, final Consumer> setWrapper, final Map target) { + return ofNullable(currentWrapper).orElseGet(() -> { + final var newWrapper = new PatchableMapWrapper(target); + setWrapper.accept(newWrapper); + return newWrapper; + }); + } + @NotNull - public static ImmutablePair entry(final String key, final Object value) { + public static ImmutablePair entry(final String key, final E value) { return new ImmutablePair<>(key, value); } - public void assign(final Map entries) { + public void assign(final Map entries) { delegate.clear(); delegate.putAll(entries); } - public void patch(final Map patch) { + public void patch(final Map patch) { patch.forEach((key, value) -> { if (value == null) { remove(key); @@ -73,22 +83,22 @@ public class PatchableMapWrapper implements Map { } @Override - public Object get(final Object key) { + public T get(final Object key) { return delegate.get(key); } @Override - public Object put(final String key, final Object value) { + public T put(final String key, final T value) { return delegate.put(key, value); } @Override - public Object remove(final Object key) { + public T remove(final Object key) { return delegate.remove(key); } @Override - public void putAll(final Map m) { + public void putAll(final @NotNull Map m) { delegate.putAll(m); } @@ -103,12 +113,12 @@ public class PatchableMapWrapper implements Map { } @Override - public Collection values() { + public Collection values() { return delegate.values(); } @Override - public Set> entrySet() { + public Set> entrySet() { return delegate.entrySet(); } } -- 2.39.5 From d0d9883e49203924bf37167247bb2cbcdf6eec61 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Mon, 29 Apr 2024 17:32:42 +0200 Subject: [PATCH 2/8] contact.emailaddresses from TEXT to JSON --- .../contact/HsOfficeContactController.java | 12 +++++-- .../office/contact/HsOfficeContactEntity.java | 27 ++++++++++++--- ...java => HsOfficeContactEntityPatcher.java} | 10 ++++-- .../hsadminng/mapper/KeyValueMap.java | 4 +-- .../hsadminng/mapper/PatchMap.java | 8 ++--- .../hsadminng/stringify/Stringify.java | 10 ++++++ .../hs-office/hs-office-contact-schemas.yaml | 13 +++++--- .../501-contact/5010-hs-office-contact.sql | 4 +-- .../5018-hs-office-contact-test-data.sql | 18 ++++++---- ...OfficeContactControllerAcceptanceTest.java | 33 ++++++++++++------- .../HsOfficeContactEntityPatcherUnitTest.java | 32 ++++++++++++++---- .../office/contact/TestHsOfficeContact.java | 3 +- ...OfficeDebitorControllerAcceptanceTest.java | 28 ++++++++++------ 13 files changed, 144 insertions(+), 58 deletions(-) rename src/main/java/net/hostsharing/hsadminng/hs/office/contact/{HsOfficeContactEntityPatch.java => HsOfficeContactEntityPatcher.java} (62%) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java index 073587f2..25695c92 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java @@ -14,6 +14,9 @@ import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBui import java.util.List; import java.util.UUID; +import java.util.function.BiConsumer; + +import static net.hostsharing.hsadminng.mapper.KeyValueMap.from; @RestController @@ -51,7 +54,7 @@ public class HsOfficeContactController implements HsOfficeContactsApi { context.define(currentUser, assumedRoles); - final var entityToSave = mapper.map(body, HsOfficeContactEntity.class); + final var entityToSave = mapper.map(body, HsOfficeContactEntity.class, RESOURCE_TO_ENTITY_POSTMAPPER); final var saved = contactRepo.save(entityToSave); @@ -108,10 +111,15 @@ public class HsOfficeContactController implements HsOfficeContactsApi { final var current = contactRepo.findByUuid(contactUuid).orElseThrow(); - new HsOfficeContactEntityPatch(current).apply(body); + new HsOfficeContactEntityPatcher(current).apply(body); final var saved = contactRepo.save(current); final var mapped = mapper.map(saved, HsOfficeContactResource.class); return ResponseEntity.ok(mapped); } + + @SuppressWarnings("unchecked") + final BiConsumer RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> { + entity.putEmailAdresses(from(resource.getEmailAddresses())); + }; } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java index e09d0044..dabcba8a 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java @@ -1,17 +1,22 @@ package net.hostsharing.hsadminng.hs.office.contact; +import io.hypersistence.utils.hibernate.type.json.JsonType; import lombok.*; import lombok.experimental.FieldNameConstants; import net.hostsharing.hsadminng.errors.DisplayName; +import net.hostsharing.hsadminng.mapper.PatchableMapWrapper; import net.hostsharing.hsadminng.rbac.rbacobject.RbacObject; import net.hostsharing.hsadminng.rbac.rbacdef.RbacView; import net.hostsharing.hsadminng.rbac.rbacdef.RbacView.SQL; import net.hostsharing.hsadminng.stringify.Stringify; import net.hostsharing.hsadminng.stringify.Stringifyable; import org.hibernate.annotations.GenericGenerator; +import org.hibernate.annotations.Type; import jakarta.persistence.*; import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import java.util.UUID; import static net.hostsharing.hsadminng.rbac.rbacdef.RbacView.GLOBAL; @@ -48,13 +53,27 @@ public class HsOfficeContactEntity implements Stringifyable, RbacObject { private String label; @Column(name = "postaladdress") - private String postalAddress; // TODO.spec: check if we really want multiple, if so: JSON-Array or Postgres-Array? + private String postalAddress; // multiline free-format text - @Column(name = "emailaddresses", columnDefinition = "json") - private String emailAddresses; // TODO.spec: check if we can really add multiple. format: ["eins@...", "zwei@..."] + @Builder.Default + @Setter(AccessLevel.NONE) + @Type(JsonType.class) + @Column(name = "emailaddresses") + private Map emailAddresses = new HashMap<>(); + + @Transient + private PatchableMapWrapper emailAddressesWrapper; + + public PatchableMapWrapper getEmailAdresses() { + return PatchableMapWrapper.of(emailAddressesWrapper, (newWrapper) -> {emailAddressesWrapper = newWrapper; }, emailAddresses ); + } + + public void putEmailAdresses(Map newEmailAddresses) { + getEmailAdresses().assign(newEmailAddresses); + } @Column(name = "phonenumbers", columnDefinition = "json") - private String phoneNumbers; // TODO.spec: check if we can really add multiple. format: { "office": "+49 40 12345-10", "fax": "+49 40 12345-05" } + private String phoneNumbers; @Override public String toString() { diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatch.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java similarity index 62% rename from src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatch.java rename to src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java index af6cfbc6..4e13ede9 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatch.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java @@ -1,14 +1,17 @@ package net.hostsharing.hsadminng.hs.office.contact; import net.hostsharing.hsadminng.mapper.EntityPatcher; +import net.hostsharing.hsadminng.mapper.KeyValueMap; import net.hostsharing.hsadminng.mapper.OptionalFromJson; import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeContactPatchResource; -class HsOfficeContactEntityPatch implements EntityPatcher { +import java.util.Optional; + +class HsOfficeContactEntityPatcher implements EntityPatcher { private final HsOfficeContactEntity entity; - HsOfficeContactEntityPatch(final HsOfficeContactEntity entity) { + HsOfficeContactEntityPatcher(final HsOfficeContactEntity entity) { this.entity = entity; } @@ -16,7 +19,8 @@ class HsOfficeContactEntityPatch implements EntityPatcher entity.getEmailAdresses().patch(KeyValueMap.from(resource.getEmailAddresses()))); OptionalFromJson.of(resource.getPhoneNumbers()).ifPresent(entity::setPhoneNumbers); } } diff --git a/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java b/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java index 5a8cff2f..a17eab25 100644 --- a/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java +++ b/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java @@ -5,9 +5,9 @@ import java.util.Map; public class KeyValueMap { @SuppressWarnings("unchecked") - public static Map from(final Object obj) { + public static Map from(final Object obj) { if (obj instanceof Map) { - return (Map) obj; + return (Map) obj; } throw new ClassCastException("Map expected, but got: " + obj); } diff --git a/src/main/java/net/hostsharing/hsadminng/mapper/PatchMap.java b/src/main/java/net/hostsharing/hsadminng/mapper/PatchMap.java index 74a36bfa..6fd843c9 100644 --- a/src/main/java/net/hostsharing/hsadminng/mapper/PatchMap.java +++ b/src/main/java/net/hostsharing/hsadminng/mapper/PatchMap.java @@ -12,19 +12,19 @@ import static java.util.Arrays.stream; * This is a map which can take key-value-pairs where the value can be null * thus JSON nullable object structures from HTTP PATCH can be represented. */ -public class PatchMap extends TreeMap { +public class PatchMap extends TreeMap { - public PatchMap(final ImmutablePair[] entries) { + public PatchMap(final ImmutablePair[] entries) { stream(entries).forEach(r -> put(r.getKey(), r.getValue())); } @SafeVarargs - public static Map patchMap(final ImmutablePair... entries) { + public static Map patchMap(final ImmutablePair... entries) { return new PatchMap(entries); } @NotNull - public static ImmutablePair entry(final String key, final Object value) { + public static ImmutablePair entry(final String key, final T value) { return new ImmutablePair<>(key, value); } } diff --git a/src/main/java/net/hostsharing/hsadminng/stringify/Stringify.java b/src/main/java/net/hostsharing/hsadminng/stringify/Stringify.java index 8cdf433b..ffdb7a5a 100644 --- a/src/main/java/net/hostsharing/hsadminng/stringify/Stringify.java +++ b/src/main/java/net/hostsharing/hsadminng/stringify/Stringify.java @@ -4,7 +4,9 @@ import net.hostsharing.hsadminng.errors.DisplayName; import jakarta.validation.constraints.NotNull; import java.util.ArrayList; +import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.function.Function; @@ -62,6 +64,7 @@ public final class Stringify { final var propValues = props.stream() .map(prop -> PropertyValue.of(prop, prop.getter.apply(object))) .filter(Objects::nonNull) + .filter(PropertyValue::nonEmpty) .map(propVal -> { if (propVal.rawValue instanceof Stringifyable stringifyable) { return new PropertyValue<>(propVal.prop, propVal.rawValue, stringifyable.toShortString()); @@ -110,5 +113,12 @@ public final class Stringify { static PropertyValue of(Property prop, Object rawValue) { return rawValue != null ? new PropertyValue<>(prop, rawValue, rawValue.toString()) : null; } + + boolean nonEmpty() { + return rawValue != null && + (!(rawValue instanceof Collection c) || !c.isEmpty()) && + (!(rawValue instanceof Map m) || !m.isEmpty()) && + (!(rawValue instanceof String s) || !s.isEmpty()); + } } } diff --git a/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml b/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml index 9d8dc76a..507211b6 100644 --- a/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml +++ b/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml @@ -14,7 +14,7 @@ components: postalAddress: type: string emailAddresses: - type: string + $ref: '#/components/schemas/HsOfficeContactEmailAddresses' phoneNumbers: type: string @@ -26,7 +26,7 @@ components: postalAddress: type: string emailAddresses: - type: string + $ref: '#/components/schemas/HsOfficeContactEmailAddresses' phoneNumbers: type: string required: @@ -42,8 +42,13 @@ components: type: string nullable: true emailAddresses: - type: string - nullable: true + $ref: '#/components/schemas/HsOfficeContactEmailAddresses' phoneNumbers: type: string nullable: true + + HsOfficeContactEmailAddresses: + # forces generating a java.lang.Object containing a Map, instead of class HsOfficeContactEmailAddresses + anyOf: + - type: object + additionalProperties: true diff --git a/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql b/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql index d6428651..8e45f583 100644 --- a/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql +++ b/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql @@ -10,8 +10,8 @@ create table if not exists hs_office_contact version int not null default 0, label varchar(128) not null, postalAddress text, - emailAddresses text, -- TODO.feat: change to json - phoneNumbers text -- TODO.feat: change to json + emailAddresses jsonb not null, + phoneNumbers text ); --// diff --git a/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql b/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql index 7970e0f6..996f97a0 100644 --- a/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql +++ b/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql @@ -11,8 +11,9 @@ create or replace procedure createHsOfficeContactTestData(contLabel varchar) language plpgsql as $$ declare - currentTask varchar; - emailAddr varchar; + currentTask varchar; + postalAddr varchar; + emailAddr varchar; begin currentTask = 'creating contact test-data ' || contLabel; execute format('set local hsadminng.currentTask to %L', currentTask); @@ -22,14 +23,17 @@ begin perform createRbacUser(emailAddr); call defineContext(currentTask, null, emailAddr); + postalAddr := E'Vorname Nachname\nStraße Hnr\nPLZ Stadt'; + raise notice 'creating test contact: %', contLabel; insert into hs_office_contact (label, postaladdress, emailaddresses, phonenumbers) - values (contLabel, $address$ -Vorname Nachname -Straße Hnr -PLZ Stadt -$address$, emailAddr, '+49 123 1234567'); + values ( + contLabel, + postalAddr, + ('{ "main": "' || emailAddr || '" }')::jsonb, + '+49 123 1234567' + ); end; $$; --// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java index c7833c9c..328148a6 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java @@ -19,6 +19,7 @@ import org.springframework.transaction.annotation.Transactional; import jakarta.persistence.EntityManager; import jakarta.persistence.PersistenceContext; +import java.util.Map; import java.util.UUID; import static net.hostsharing.hsadminng.rbac.test.IsValidUuidMatcher.isUuidValid; @@ -103,7 +104,9 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .body(""" { "label": "Temp Contact", - "emailAddresses": "test@example.org" + "emailAddresses": { + "main": "test@example.org" + } } """) .port(port) @@ -114,7 +117,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .contentType(ContentType.JSON) .body("uuid", isUuidValid()) .body("label", is("Temp Contact")) - .body("emailAddresses", is("test@example.org")) + .body("emailAddresses", is(Map.of("main", "test@example.org"))) .header("Location", startsWith("http://localhost")) .extract().header("Location"); // @formatter:on @@ -180,9 +183,11 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .contentType("application/json") .body("", lenientlyEquals(""" { - "label": "first contact", - "emailAddresses": "contact-admin@firstcontact.example.com", - "phoneNumbers": "+49 123 1234567" + "label": "first contact", + "emailAddresses": { + "main": "contact-admin@firstcontact.example.com" + }, + "phoneNumbers": "+49 123 1234567" } """)); // @formatter:on } @@ -204,7 +209,9 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .body(""" { "label": "Temp patched contact", - "emailAddresses": "patched@example.org", + "emailAddresses": { + "main": "patched@example.org" + }, "postalAddress": "Patched Address", "phoneNumbers": "+01 100 123456" } @@ -217,7 +224,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .contentType(ContentType.JSON) .body("uuid", isUuidValid()) .body("label", is("Temp patched contact")) - .body("emailAddresses", is("patched@example.org")) + .body("emailAddresses", is(Map.of("main", "patched@example.org"))) .body("postalAddress", is("Patched Address")) .body("phoneNumbers", is("+01 100 123456")); // @formatter:on @@ -227,7 +234,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu assertThat(contactRepo.findByUuid(givenContact.getUuid())).isPresent().get() .matches(person -> { assertThat(person.getLabel()).isEqualTo("Temp patched contact"); - assertThat(person.getEmailAddresses()).isEqualTo("patched@example.org"); + assertThat(person.getEmailAddresses()).isEqualTo(Map.of("main", "patched@example.org")); assertThat(person.getPostalAddress()).isEqualTo("Patched Address"); assertThat(person.getPhoneNumbers()).isEqualTo("+01 100 123456"); return true; @@ -246,7 +253,9 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .contentType(ContentType.JSON) .body(""" { - "emailAddresses": "patched@example.org", + "emailAddresses": { + "main": "patched@example.org" + }, "phoneNumbers": "+01 100 123456" } """) @@ -258,7 +267,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .contentType(ContentType.JSON) .body("uuid", isUuidValid()) .body("label", is(givenContact.getLabel())) - .body("emailAddresses", is("patched@example.org")) + .body("emailAddresses", is(Map.of("main", "patched@example.org"))) .body("postalAddress", is(givenContact.getPostalAddress())) .body("phoneNumbers", is("+01 100 123456")); // @formatter:on @@ -267,7 +276,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu assertThat(contactRepo.findByUuid(givenContact.getUuid())).isPresent().get() .matches(person -> { assertThat(person.getLabel()).isEqualTo(givenContact.getLabel()); - assertThat(person.getEmailAddresses()).isEqualTo("patched@example.org"); + assertThat(person.getEmailAddresses()).isEqualTo(Map.of("main", "patched@example.org")); assertThat(person.getPostalAddress()).isEqualTo(givenContact.getPostalAddress()); assertThat(person.getPhoneNumbers()).isEqualTo("+01 100 123456"); return true; @@ -340,7 +349,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu final var newContact = HsOfficeContactEntity.builder() .uuid(UUID.randomUUID()) .label("Temp from " + Context.getCallerMethodNameFromStackFrame(1) ) - .emailAddresses(RandomStringUtils.randomAlphabetic(10) + "@example.org") + .emailAddresses(Map.of("main", RandomStringUtils.randomAlphabetic(10) + "@example.org")) .postalAddress("Postal Address " + RandomStringUtils.randomAlphabetic(10)) .phoneNumbers("+01 200 " + RandomStringUtils.randomNumeric(8)) .build(); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java index 31a5ca02..9e2d6d8f 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java @@ -4,9 +4,12 @@ import net.hostsharing.hsadminng.rbac.test.PatchUnitTestBase; import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeContactPatchResource; import org.junit.jupiter.api.TestInstance; +import java.util.Map; import java.util.UUID; import java.util.stream.Stream; +import static net.hostsharing.hsadminng.mapper.PatchMap.entry; +import static net.hostsharing.hsadminng.mapper.PatchMap.patchMap; import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; @TestInstance(PER_CLASS) @@ -16,13 +19,26 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< > { private static final UUID INITIAL_CONTACT_UUID = UUID.randomUUID(); + private static final Map PATCH_EMAIL_ADDRESSES = patchMap( + entry("main", "patched@example.com"), + entry("paul", null), + entry("suse", "suse@example.com") + ); + private static final Map PATCHED_EMAIL_ADDRESSES = patchMap( + entry("main", "patched@example.com"), + entry("suse", "suse@example.com"), + entry("mila", "mila@example.com") + ); @Override protected HsOfficeContactEntity newInitialEntity() { final var entity = new HsOfficeContactEntity(); entity.setUuid(INITIAL_CONTACT_UUID); entity.setLabel("initial label"); - entity.setEmailAddresses("initial@example.org"); + entity.putEmailAdresses(Map.ofEntries( + entry("main", "initial@example.org"), + entry("paul", "paul@example.com"), + entry("mila", "mila@example.com"))); entity.setPhoneNumbers("initial postal address"); entity.setPostalAddress("+01 100 123456789"); return entity; @@ -34,8 +50,8 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< } @Override - protected HsOfficeContactEntityPatch createPatcher(final HsOfficeContactEntity entity) { - return new HsOfficeContactEntityPatch(entity); + protected HsOfficeContactEntityPatcher createPatcher(final HsOfficeContactEntity entity) { + return new HsOfficeContactEntityPatcher(entity); } @Override @@ -46,11 +62,13 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< HsOfficeContactPatchResource::setLabel, "patched label", HsOfficeContactEntity::setLabel), - new JsonNullableProperty<>( - "emailAddresses", + new SimpleProperty<>( + "resources", HsOfficeContactPatchResource::setEmailAddresses, - "patched trade name", - HsOfficeContactEntity::setEmailAddresses), + PATCH_EMAIL_ADDRESSES, + HsOfficeContactEntity::putEmailAdresses, + PATCHED_EMAIL_ADDRESSES) + .notNullable(), new JsonNullableProperty<>( "phoneNumbers", HsOfficeContactPatchResource::setPhoneNumbers, diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/TestHsOfficeContact.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/TestHsOfficeContact.java index b42ef8e5..9256084f 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/TestHsOfficeContact.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/TestHsOfficeContact.java @@ -1,5 +1,6 @@ package net.hostsharing.hsadminng.hs.office.contact; +import java.util.Map; public class TestHsOfficeContact { @@ -9,7 +10,7 @@ public class TestHsOfficeContact { return HsOfficeContactEntity.builder() .label(label) .postalAddress("address of " + label) - .emailAddresses(emailAddr) + .emailAddresses(Map.of("main", emailAddr)) .build(); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java index 9b3638c4..a18f23ec 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java @@ -107,7 +107,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "mark": null, "contact": { "label": "first contact", - "emailAddresses": "contact-admin@firstcontact.example.com", + "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, "phoneNumbers": "+49 123 1234567" } }, @@ -132,7 +132,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "mark": null, "contact": { "label": "first contact", - "emailAddresses": "contact-admin@firstcontact.example.com", + "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, "phoneNumbers": "+49 123 1234567" } }, @@ -162,7 +162,9 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "anchor": {"tradeName": "Second e.K."}, "holder": {"tradeName": "Second e.K."}, "type": "DEBITOR", - "contact": {"emailAddresses": "contact-admin@secondcontact.example.com"} + "contact": { + "emailAddresses": { "main": "contact-admin@secondcontact.example.com" } + } }, "debitorNumber": 1000212, "debitorNumberSuffix": 12, @@ -172,7 +174,9 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "anchor": {"tradeName": "Hostsharing eG"}, "holder": {"tradeName": "Second e.K."}, "type": "PARTNER", - "contact": {"emailAddresses": "contact-admin@secondcontact.example.com"} + "contact": { + "emailAddresses": { "main": "contact-admin@secondcontact.example.com" } + } }, "details": { "registrationOffice": "Hamburg", @@ -192,7 +196,9 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "anchor": {"tradeName": "Third OHG"}, "holder": {"tradeName": "Third OHG"}, "type": "DEBITOR", - "contact": {"emailAddresses": "contact-admin@thirdcontact.example.com"} + "contact": { + "emailAddresses": { "main": "contact-admin@thirdcontact.example.com" } + } }, "debitorNumber": 1000313, "debitorNumberSuffix": 13, @@ -202,7 +208,9 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "anchor": {"tradeName": "Hostsharing eG"}, "holder": {"tradeName": "Third OHG"}, "type": "PARTNER", - "contact": {"emailAddresses": "contact-admin@thirdcontact.example.com"} + "contact": { + "emailAddresses": { "main": "contact-admin@thirdcontact.example.com" } + } }, "details": { "registrationOffice": "Hamburg", @@ -456,8 +464,8 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "type": "DEBITOR", "contact": { "label": "first contact", - "postalAddress": "\\nVorname Nachname\\nStraße Hnr\\nPLZ Stadt\\n", - "emailAddresses": "contact-admin@firstcontact.example.com", + "postalAddress": "Vorname Nachname\\nStraße Hnr\\nPLZ Stadt", + "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, "phoneNumbers": "+49 123 1234567" } }, @@ -472,8 +480,8 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "mark": null, "contact": { "label": "first contact", - "postalAddress": "\\nVorname Nachname\\nStraße Hnr\\nPLZ Stadt\\n", - "emailAddresses": "contact-admin@firstcontact.example.com", + "postalAddress": "Vorname Nachname\\nStraße Hnr\\nPLZ Stadt", + "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, "phoneNumbers": "+49 123 1234567" } }, -- 2.39.5 From e7df0bfd4d575a8844fec303056d4759b8c32a52 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 30 Apr 2024 09:26:34 +0200 Subject: [PATCH 3/8] fix subgraph order --- .../test/pac/TestPackageEntityUnitTest.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/test/pac/TestPackageEntityUnitTest.java b/src/test/java/net/hostsharing/hsadminng/rbac/test/pac/TestPackageEntityUnitTest.java index 660ad955..824bb1bb 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/test/pac/TestPackageEntityUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/test/pac/TestPackageEntityUnitTest.java @@ -13,6 +13,19 @@ class TestPackageEntityUnitTest { assertThat(rbacFlowchart).isEqualTo(""" %%{init:{'flowchart':{'htmlLabels':false}}}%% flowchart TB + + subgraph customer["`**customer**`"] + direction TB + style customer fill:#99bcdb,stroke:#274d6e,stroke-width:8px + + subgraph customer:roles[ ] + style customer:roles fill:#99bcdb,stroke:white + + role:customer:OWNER[[customer:OWNER]] + role:customer:ADMIN[[customer:ADMIN]] + role:customer:TENANT[[customer:TENANT]] + end + end subgraph package["`**package**`"] direction TB @@ -36,19 +49,6 @@ class TestPackageEntityUnitTest { end end - subgraph customer["`**customer**`"] - direction TB - style customer fill:#99bcdb,stroke:#274d6e,stroke-width:8px - - subgraph customer:roles[ ] - style customer:roles fill:#99bcdb,stroke:white - - role:customer:OWNER[[customer:OWNER]] - role:customer:ADMIN[[customer:ADMIN]] - role:customer:TENANT[[customer:TENANT]] - end - end - %% granting roles to roles role:global:ADMIN -.->|XX| role:customer:OWNER role:customer:OWNER -.-> role:customer:ADMIN -- 2.39.5 From 5798a998859df3260711d40450acd9eee20ed80e Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 30 Apr 2024 09:28:00 +0200 Subject: [PATCH 4/8] cleanup, clarify, re-categorize and fix some TODO.spec issues --- .../hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java | 2 +- .../hs/office/relation/HsOfficeRelationEntity.java | 2 +- .../resources/db/changelog/1-rbac/1051-rbac-user-grant.sql | 2 +- .../db/changelog/1-rbac/1057-rbac-role-builder.sql | 2 +- .../5-hs-office/506-debitor/5060-hs-office-debitor.sql | 2 +- .../hsadminng/hs/office/migration/ImportOfficeData.java | 6 +++--- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java index 33e6f2e8..9cf134c9 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java @@ -170,7 +170,7 @@ public class HsOfficeDebitorEntity implements RbacObject, Stringifyable { "vatCountryCode", "vatBusiness", "vatReverseCharge", - "defaultPrefix" /* TODO.spec: do we want that updatable? */) + "defaultPrefix") .toRole("global", ADMIN).grantPermission(INSERT) .importRootEntityAliasProxy("debitorRel", HsOfficeRelationEntity.class, usingCase(DEBITOR), diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java index 581e6bb7..7c8cd78e 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java @@ -142,7 +142,7 @@ public class HsOfficeRelationEntity implements RbacObject, Stringifyable { with.permission(UPDATE); }) .createSubRole(AGENT, (with) -> { - // TODO.spec: we need relation:PROXY, to allow changing the relation contact. + // TODO.rbac: we need relation:PROXY, to allow changing the relation contact. // the alternative would be to move this to the relation:ADMIN role, // but then the partner holder person could update the partner relation itself, // see partner entity. diff --git a/src/main/resources/db/changelog/1-rbac/1051-rbac-user-grant.sql b/src/main/resources/db/changelog/1-rbac/1051-rbac-user-grant.sql index 17645ca3..fc74a6de 100644 --- a/src/main/resources/db/changelog/1-rbac/1051-rbac-user-grant.sql +++ b/src/main/resources/db/changelog/1-rbac/1051-rbac-user-grant.sql @@ -63,7 +63,7 @@ begin insert into RbacGrants (grantedByRoleUuid, ascendantUuid, descendantUuid, assumed) values (grantedByRoleUuid, userUuid, grantedRoleUuid, doAssume); - -- TODO.spec: What should happen on mupltiple grants? What if options (doAssume) are not the same? + -- TODO.impl: What should happen on mupltiple grants? What if options (doAssume) are not the same? -- Most powerful or latest grant wins? What about managed? -- on conflict do nothing; -- allow granting multiple times end; $$; diff --git a/src/main/resources/db/changelog/1-rbac/1057-rbac-role-builder.sql b/src/main/resources/db/changelog/1-rbac/1057-rbac-role-builder.sql index 1e9bd2bc..cb20bbbc 100644 --- a/src/main/resources/db/changelog/1-rbac/1057-rbac-role-builder.sql +++ b/src/main/resources/db/changelog/1-rbac/1057-rbac-role-builder.sql @@ -52,7 +52,7 @@ begin if cardinality(userUuids) > 0 then -- direct grants to users need a grantedByRole which can revoke the grant if grantedByRole is null then - userGrantsByRoleUuid := roleUuid; -- TODO.spec: or do we want to require an explicit userGrantsByRoleUuid? + userGrantsByRoleUuid := roleUuid; -- TODO.impl: or do we want to require an explicit userGrantsByRoleUuid? else userGrantsByRoleUuid := getRoleId(grantedByRole); end if; diff --git a/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql b/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql index 39db61e2..bbf72543 100644 --- a/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql +++ b/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql @@ -11,7 +11,7 @@ create table hs_office_debitor debitorNumberSuffix char(2) not null check (debitorNumberSuffix::text ~ '^[0-9][0-9]$'), debitorRelUuid uuid not null references hs_office_relation(uuid), billable boolean not null default true, - vatId varchar(24), -- TODO.spec: here or in person? + vatId varchar(24), vatCountryCode varchar(2), vatBusiness boolean not null, vatReverseCharge boolean not null, 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 3bc64cd1..5e1b96b0 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 @@ -810,7 +810,7 @@ public class ImportOfficeData extends ContextBasedTest { ) .shareCount(rec.getInteger("quantity")) .comment( rec.getString("comment")) - .reference("legacy data import") // TODO.spec: or use value from comment column? + .reference(member.getMemberNumber().toString()) .build(); if (shareTransaction.getTransactionType() == HsOfficeCoopSharesTransactionType.ADJUSTMENT) { @@ -867,7 +867,7 @@ public class ImportOfficeData extends ContextBasedTest { .transactionType(assetTypeMapping.get(rec.getString("action"))) .assetValue(rec.getBigDecimal("amount")) .comment(rec.getString("comment")) - .reference("legacy data import") // TODO.spec: or use value from comment column? + .reference("legacy data import") // TODO.: Kundennummer .build(); if (assetTransaction.getTransactionType() == HsOfficeCoopAssetsTransactionType.ADJUSTMENT) { @@ -1092,7 +1092,7 @@ public class ImportOfficeData extends ContextBasedTest { contactRecord.getString("first_name"), contactRecord.getString("last_name"), contactRecord.getString("firma"))); - contact.setEmailAddresses(contactRecord.getString("email")); + contact.putEmailAdresses( Map.of("main", contactRecord.getString("email"))); contact.setPostalAddress(toAddress(contactRecord)); contact.setPhoneNumbers(toPhoneNumbers(contactRecord)); -- 2.39.5 From 5954a87b360e0a2e8b8142e2b630d5a5830d5b02 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 30 Apr 2024 09:33:32 +0200 Subject: [PATCH 5/8] amend assertions in ImportOfficeData according to JSON email-addresses and empty properties --- .../hs/office/migration/ImportOfficeData.java | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 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 5e1b96b0..08d5cbcb 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 @@ -240,30 +240,30 @@ public class ImportOfficeData extends ContextBasedTest { """); assertThat(toFormattedString(contacts)).isEqualToIgnoringWhitespace(""" { - 1101=contact(label='Herr Michael Mellies ', emailAddresses='mih@example.org'), - 1200=contact(label='JM e.K.', emailAddresses='jm-ex-partner@example.org'), - 1201=contact(label='Frau Dr. Jenny Meyer-Billing , JM GmbH', emailAddresses='jm-billing@example.org'), - 1202=contact(label='Herr Andrew Meyer-Operation , JM GmbH', emailAddresses='am-operation@example.org'), - 1203=contact(label='Herr Philip Meyer-Contract , JM GmbH', emailAddresses='pm-partner@example.org'), - 1204=contact(label='Frau Tammy Meyer-VIP , JM GmbH', emailAddresses='tm-vip@example.org'), - 1301=contact(label='Petra Schmidt , Test PS', emailAddresses='ps@example.com'), - 1401=contact(label='Frau Frauke Fanninga ', emailAddresses='ff@example.org'), - 1501=contact(label='Frau Cecilia Camus ', emailAddresses='cc@example.org') - } + 1101=contact(label='Herr Michael Mellies ', emailAddresses='{main=mih@example.org}'), + 1200=contact(label='JM e.K.', emailAddresses='{main=jm-ex-partner@example.org}'), + 1201=contact(label='Frau Dr. Jenny Meyer-Billing , JM GmbH', emailAddresses='{main=jm-billing@example.org}'), + 1202=contact(label='Herr Andrew Meyer-Operation , JM GmbH', emailAddresses='{main=am-operation@example.org}'), + 1203=contact(label='Herr Philip Meyer-Contract , JM GmbH', emailAddresses='{main=pm-partner@example.org}'), + 1204=contact(label='Frau Tammy Meyer-VIP , JM GmbH', emailAddresses='{main=tm-vip@example.org}'), + 1301=contact(label='Petra Schmidt , Test PS', emailAddresses='{main=ps@example.com}'), + 1401=contact(label='Frau Frauke Fanninga ', emailAddresses='{main=ff@example.org}'), + 1501=contact(label='Frau Cecilia Camus ', emailAddresses='{main=cc@example.org}') + } """); assertThat(toFormattedString(persons)).isEqualToIgnoringWhitespace(""" { 1=person(personType='LP', tradeName='Hostsharing eG'), - 1101=person(personType='NP', tradeName='', familyName='Mellies', givenName='Michael'), - 1200=person(personType='LP', tradeName='JM e.K.', familyName='', givenName=''), + 1101=person(personType='NP', familyName='Mellies', givenName='Michael'), + 1200=person(personType='LP', tradeName='JM e.K.'), 1201=person(personType='LP', tradeName='JM GmbH', familyName='Meyer-Billing', givenName='Jenny'), 1202=person(personType='LP', tradeName='JM GmbH', familyName='Meyer-Operation', givenName='Andrew'), 1203=person(personType='LP', tradeName='JM GmbH', familyName='Meyer-Contract', givenName='Philip'), 1204=person(personType='LP', tradeName='JM GmbH', familyName='Meyer-VIP', givenName='Tammy'), 1301=person(personType='??', tradeName='Test PS', familyName='Schmidt', givenName='Petra'), - 1401=person(personType='NP', tradeName='', familyName='Fanninga', givenName='Frauke'), - 1501=person(personType='NP', tradeName='', familyName='Camus', givenName='Cecilia') - } + 1401=person(personType='NP', familyName='Fanninga', givenName='Frauke'), + 1501=person(personType='NP', familyName='Camus', givenName='Cecilia') + } """); assertThat(toFormattedString(debitors)).isEqualToIgnoringWhitespace(""" { @@ -363,11 +363,11 @@ public class ImportOfficeData extends ContextBasedTest { assertThat(toFormattedString(coopShares)).isEqualToIgnoringWhitespace(""" { - 33443=CoopShareTransaction(M-1001700: 2000-12-06, SUBSCRIPTION, 20, legacy data import, initial share subscription), - 33451=CoopShareTransaction(M-1002000: 2000-12-06, SUBSCRIPTION, 2, legacy data import, initial share subscription), - 33701=CoopShareTransaction(M-1001700: 2005-01-10, SUBSCRIPTION, 40, legacy data import, increase), - 33810=CoopShareTransaction(M-1002000: 2016-12-31, CANCELLATION, 22, legacy data import, membership ended) - } + 33443=CoopShareTransaction(M-1001700: 2000-12-06, SUBSCRIPTION, 20, 1001700, initial share subscription), + 33451=CoopShareTransaction(M-1002000: 2000-12-06, SUBSCRIPTION, 2, 1002000, initial share subscription), + 33701=CoopShareTransaction(M-1001700: 2005-01-10, SUBSCRIPTION, 40, 1001700, increase), + 33810=CoopShareTransaction(M-1002000: 2016-12-31, CANCELLATION, 22, 1002000, membership ended) + } """); } -- 2.39.5 From 90c21e7ec13dea8925df9d756c3e7e9147b160e8 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 30 Apr 2024 11:14:34 +0200 Subject: [PATCH 6/8] contact.phonenumbers from TEXT to JSON --- .../contact/HsOfficeContactController.java | 1 + .../office/contact/HsOfficeContactEntity.java | 24 ++++++++++--- .../contact/HsOfficeContactEntityPatcher.java | 5 +-- .../hsadminng/mapper/KeyValueMap.java | 2 +- .../hsadminng/mapper/PatchableMapWrapper.java | 6 ++-- .../hs-office/hs-office-contact-schemas.yaml | 13 ++++--- .../5018-hs-office-contact-test-data.sql | 2 +- ...OfficeContactControllerAcceptanceTest.java | 26 ++++++++------ .../HsOfficeContactEntityPatcherUnitTest.java | 28 +++++++++++---- ...OfficeDebitorControllerAcceptanceTest.java | 8 ++--- .../hs/office/migration/ImportOfficeData.java | 36 +++++++++---------- 11 files changed, 98 insertions(+), 53 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java index 25695c92..028e8398 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java @@ -121,5 +121,6 @@ public class HsOfficeContactController implements HsOfficeContactsApi { @SuppressWarnings("unchecked") final BiConsumer RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> { entity.putEmailAdresses(from(resource.getEmailAddresses())); + entity.putPhoneNumbers(from(resource.getPhoneNumbers())); }; } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java index dabcba8a..a33ddf5d 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java @@ -49,7 +49,7 @@ public class HsOfficeContactEntity implements Stringifyable, RbacObject { @Version private int version; - @Column(name = "label") + @Column(name = "label") // TODO.impl: rename to caption private String label; @Column(name = "postaladdress") @@ -64,16 +64,30 @@ public class HsOfficeContactEntity implements Stringifyable, RbacObject { @Transient private PatchableMapWrapper emailAddressesWrapper; - public PatchableMapWrapper getEmailAdresses() { + @Builder.Default + @Setter(AccessLevel.NONE) + @Type(JsonType.class) + @Column(name = "phonenumbers") + private Map phoneNumbers = new HashMap<>(); + + @Transient + private PatchableMapWrapper phoneNumbersWrapper; + + public PatchableMapWrapper getEmailAddresses() { return PatchableMapWrapper.of(emailAddressesWrapper, (newWrapper) -> {emailAddressesWrapper = newWrapper; }, emailAddresses ); } public void putEmailAdresses(Map newEmailAddresses) { - getEmailAdresses().assign(newEmailAddresses); + getEmailAddresses().assign(newEmailAddresses); } - @Column(name = "phonenumbers", columnDefinition = "json") - private String phoneNumbers; + public PatchableMapWrapper getPhoneNumbers() { + return PatchableMapWrapper.of(phoneNumbersWrapper, (newWrapper) -> {phoneNumbersWrapper = newWrapper; }, phoneNumbers ); + } + + public void putPhoneNumbers(Map newPhoneNumbers) { + getPhoneNumbers().assign(newPhoneNumbers); + } @Override public String toString() { diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java index 4e13ede9..edefb8f3 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java @@ -20,7 +20,8 @@ class HsOfficeContactEntityPatcher implements EntityPatcher entity.getEmailAdresses().patch(KeyValueMap.from(resource.getEmailAddresses()))); - OptionalFromJson.of(resource.getPhoneNumbers()).ifPresent(entity::setPhoneNumbers); + .ifPresent(r -> entity.getEmailAddresses().patch(KeyValueMap.from(resource.getEmailAddresses()))); + Optional.ofNullable(resource.getPhoneNumbers()) + .ifPresent(r -> entity.getPhoneNumbers().patch(KeyValueMap.from(resource.getPhoneNumbers()))); } } diff --git a/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java b/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java index a17eab25..7fded816 100644 --- a/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java +++ b/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java @@ -6,7 +6,7 @@ public class KeyValueMap { @SuppressWarnings("unchecked") public static Map from(final Object obj) { - if (obj instanceof Map) { + if (obj == null || obj instanceof Map) { return (Map) obj; } throw new ClassCastException("Map expected, but got: " + obj); diff --git a/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java b/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java index a3d2687e..4962ac8d 100644 --- a/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java +++ b/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java @@ -36,8 +36,10 @@ public class PatchableMapWrapper implements Map { } public void assign(final Map entries) { - delegate.clear(); - delegate.putAll(entries); + if (entries != null ) { + delegate.clear(); + delegate.putAll(entries); + } } public void patch(final Map patch) { diff --git a/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml b/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml index 507211b6..6d9f2a46 100644 --- a/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml +++ b/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml @@ -16,7 +16,7 @@ components: emailAddresses: $ref: '#/components/schemas/HsOfficeContactEmailAddresses' phoneNumbers: - type: string + $ref: '#/components/schemas/HsOfficeContactPhoneNumbers' HsOfficeContactInsert: type: object @@ -28,7 +28,7 @@ components: emailAddresses: $ref: '#/components/schemas/HsOfficeContactEmailAddresses' phoneNumbers: - type: string + $ref: '#/components/schemas/HsOfficeContactPhoneNumbers' required: - label @@ -44,11 +44,16 @@ components: emailAddresses: $ref: '#/components/schemas/HsOfficeContactEmailAddresses' phoneNumbers: - type: string - nullable: true + $ref: '#/components/schemas/HsOfficeContactPhoneNumbers' HsOfficeContactEmailAddresses: # forces generating a java.lang.Object containing a Map, instead of class HsOfficeContactEmailAddresses anyOf: - type: object additionalProperties: true + + HsOfficeContactPhoneNumbers: + # forces generating a java.lang.Object containing a Map, instead of class HsOfficeContactEmailAddresses + anyOf: + - type: object + additionalProperties: true diff --git a/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql b/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql index 996f97a0..2a989593 100644 --- a/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql +++ b/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql @@ -32,7 +32,7 @@ begin contLabel, postalAddr, ('{ "main": "' || emailAddr || '" }')::jsonb, - '+49 123 1234567' + ('{ "office": "+49 123 1234567" }')::jsonb ); end; $$; --// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java index 328148a6..0b53d0b1 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java @@ -187,7 +187,9 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": "+49 123 1234567" + "phoneNumbers": { + "office": "+49 123 1234567" + } } """)); // @formatter:on } @@ -213,7 +215,9 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu "main": "patched@example.org" }, "postalAddress": "Patched Address", - "phoneNumbers": "+01 100 123456" + "phoneNumbers": { + "office": "+01 100 123456" + } } """) .port(port) @@ -226,7 +230,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .body("label", is("Temp patched contact")) .body("emailAddresses", is(Map.of("main", "patched@example.org"))) .body("postalAddress", is("Patched Address")) - .body("phoneNumbers", is("+01 100 123456")); + .body("phoneNumbers", is(Map.of("office", "+01 100 123456"))); // @formatter:on // finally, the contact is actually updated @@ -234,9 +238,9 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu assertThat(contactRepo.findByUuid(givenContact.getUuid())).isPresent().get() .matches(person -> { assertThat(person.getLabel()).isEqualTo("Temp patched contact"); - assertThat(person.getEmailAddresses()).isEqualTo(Map.of("main", "patched@example.org")); + assertThat(person.getEmailAddresses()).containsExactlyEntriesOf(Map.of("main", "patched@example.org")); assertThat(person.getPostalAddress()).isEqualTo("Patched Address"); - assertThat(person.getPhoneNumbers()).isEqualTo("+01 100 123456"); + assertThat(person.getPhoneNumbers()).containsExactlyEntriesOf(Map.of("office", "+01 100 123456")); return true; }); } @@ -256,7 +260,9 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu "emailAddresses": { "main": "patched@example.org" }, - "phoneNumbers": "+01 100 123456" + "phoneNumbers": { + "office": "+01 100 123456" + } } """) .port(port) @@ -269,16 +275,16 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .body("label", is(givenContact.getLabel())) .body("emailAddresses", is(Map.of("main", "patched@example.org"))) .body("postalAddress", is(givenContact.getPostalAddress())) - .body("phoneNumbers", is("+01 100 123456")); + .body("phoneNumbers", is(Map.of("office", "+01 100 123456"))); // @formatter:on // finally, the contact is actually updated assertThat(contactRepo.findByUuid(givenContact.getUuid())).isPresent().get() .matches(person -> { assertThat(person.getLabel()).isEqualTo(givenContact.getLabel()); - assertThat(person.getEmailAddresses()).isEqualTo(Map.of("main", "patched@example.org")); + assertThat(person.getEmailAddresses()).containsExactlyEntriesOf(Map.of("main", "patched@example.org")); assertThat(person.getPostalAddress()).isEqualTo(givenContact.getPostalAddress()); - assertThat(person.getPhoneNumbers()).isEqualTo("+01 100 123456"); + assertThat(person.getPhoneNumbers()).containsExactlyEntriesOf(Map.of("office", "+01 100 123456")); return true; }); } @@ -351,7 +357,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .label("Temp from " + Context.getCallerMethodNameFromStackFrame(1) ) .emailAddresses(Map.of("main", RandomStringUtils.randomAlphabetic(10) + "@example.org")) .postalAddress("Postal Address " + RandomStringUtils.randomAlphabetic(10)) - .phoneNumbers("+01 200 " + RandomStringUtils.randomNumeric(8)) + .phoneNumbers(Map.of("office", "+01 200 " + RandomStringUtils.randomNumeric(8))) .build(); return contactRepo.save(newContact); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java index 9e2d6d8f..ffd5dd4b 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java @@ -30,6 +30,17 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< entry("mila", "mila@example.com") ); + private static final Map PATCH_PHONE_NUMBERS = patchMap( + entry("mobile", null), + entry("private", "+49 40 987654321"), + entry("fax", "+49 40 12345-99") + ); + private static final Map PATCHED_PHONE_NUMBERS = patchMap( + entry("office", "+49 40 12345-00"), + entry("private", "+49 40 987654321"), + entry("fax", "+49 40 12345-99") + ); + @Override protected HsOfficeContactEntity newInitialEntity() { final var entity = new HsOfficeContactEntity(); @@ -39,8 +50,11 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< entry("main", "initial@example.org"), entry("paul", "paul@example.com"), entry("mila", "mila@example.com"))); - entity.setPhoneNumbers("initial postal address"); - entity.setPostalAddress("+01 100 123456789"); + entity.putPhoneNumbers(Map.ofEntries( + entry("office", "+49 40 12345-00"), + entry("mobile", "+49 1555 1234567"), + entry("fax", "+49 40 12345-90"))); + entity.setPostalAddress("Initialstraße 50\n20000 Hamburg"); return entity; } @@ -69,11 +83,13 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< HsOfficeContactEntity::putEmailAdresses, PATCHED_EMAIL_ADDRESSES) .notNullable(), - new JsonNullableProperty<>( - "phoneNumbers", + new SimpleProperty<>( + "resources", HsOfficeContactPatchResource::setPhoneNumbers, - "patched family name", - HsOfficeContactEntity::setPhoneNumbers), + PATCH_PHONE_NUMBERS, + HsOfficeContactEntity::putPhoneNumbers, + PATCHED_PHONE_NUMBERS) + .notNullable(), new JsonNullableProperty<>( "patched given name", HsOfficeContactPatchResource::setPostalAddress, diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java index a18f23ec..71505f38 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java @@ -108,7 +108,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "contact": { "label": "first contact", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": "+49 123 1234567" + "phoneNumbers": { "office": "+49 123 1234567" } } }, "debitorNumber": 1000111, @@ -133,7 +133,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "contact": { "label": "first contact", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": "+49 123 1234567" + "phoneNumbers": { "office": "+49 123 1234567" } } }, "details": { @@ -466,7 +466,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "label": "first contact", "postalAddress": "Vorname Nachname\\nStraße Hnr\\nPLZ Stadt", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": "+49 123 1234567" + "phoneNumbers": { "office": "+49 123 1234567" } } }, "debitorNumber": 1000111, @@ -482,7 +482,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "label": "first contact", "postalAddress": "Vorname Nachname\\nStraße Hnr\\nPLZ Stadt", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": "+49 123 1234567" + "phoneNumbers": { "office": "+49 123 1234567" } } }, "details": { 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 08d5cbcb..c42e0629 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 @@ -240,16 +240,16 @@ public class ImportOfficeData extends ContextBasedTest { """); assertThat(toFormattedString(contacts)).isEqualToIgnoringWhitespace(""" { - 1101=contact(label='Herr Michael Mellies ', emailAddresses='{main=mih@example.org}'), - 1200=contact(label='JM e.K.', emailAddresses='{main=jm-ex-partner@example.org}'), - 1201=contact(label='Frau Dr. Jenny Meyer-Billing , JM GmbH', emailAddresses='{main=jm-billing@example.org}'), - 1202=contact(label='Herr Andrew Meyer-Operation , JM GmbH', emailAddresses='{main=am-operation@example.org}'), - 1203=contact(label='Herr Philip Meyer-Contract , JM GmbH', emailAddresses='{main=pm-partner@example.org}'), - 1204=contact(label='Frau Tammy Meyer-VIP , JM GmbH', emailAddresses='{main=tm-vip@example.org}'), - 1301=contact(label='Petra Schmidt , Test PS', emailAddresses='{main=ps@example.com}'), - 1401=contact(label='Frau Frauke Fanninga ', emailAddresses='{main=ff@example.org}'), - 1501=contact(label='Frau Cecilia Camus ', emailAddresses='{main=cc@example.org}') - } + 1101=contact(label='Herr Michael Mellies ', emailAddresses='{ main: mih@example.org }'), + 1200=contact(label='JM e.K.', emailAddresses='{ main: jm-ex-partner@example.org }'), + 1201=contact(label='Frau Dr. Jenny Meyer-Billing , JM GmbH', emailAddresses='{ main: jm-billing@example.org }'), + 1202=contact(label='Herr Andrew Meyer-Operation , JM GmbH', emailAddresses='{ main: am-operation@example.org }'), + 1203=contact(label='Herr Philip Meyer-Contract , JM GmbH', emailAddresses='{ main: pm-partner@example.org }'), + 1204=contact(label='Frau Tammy Meyer-VIP , JM GmbH', emailAddresses='{ main: tm-vip@example.org }'), + 1301=contact(label='Petra Schmidt , Test PS', emailAddresses='{ main: ps@example.com }'), + 1401=contact(label='Frau Frauke Fanninga ', emailAddresses='{ main: ff@example.org }'), + 1501=contact(label='Frau Cecilia Camus ', emailAddresses='{ main: cc@example.org }') + } """); assertThat(toFormattedString(persons)).isEqualToIgnoringWhitespace(""" { @@ -1094,7 +1094,7 @@ public class ImportOfficeData extends ContextBasedTest { contactRecord.getString("firma"))); contact.putEmailAdresses( Map.of("main", contactRecord.getString("email"))); contact.setPostalAddress(toAddress(contactRecord)); - contact.setPhoneNumbers(toPhoneNumbers(contactRecord)); + contact.putPhoneNumbers(toPhoneNumbers(contactRecord)); contacts.put(contactRecord.getInteger("contact_id"), contact); return contact; @@ -1120,17 +1120,17 @@ public class ImportOfficeData extends ContextBasedTest { return record; } - private String toPhoneNumbers(final Record rec) { - final var result = new StringBuilder("{\n"); + private Map toPhoneNumbers(final Record rec) { + final var phoneNumbers = new LinkedHashMap(); if (isNotBlank(rec.getString("phone_private"))) - result.append(" \"private\": " + "\"" + rec.getString("phone_private") + "\",\n"); + phoneNumbers.put("privat", rec.getString("phone_private")); if (isNotBlank(rec.getString("phone_office"))) - result.append(" \"office\": " + "\"" + rec.getString("phone_office") + "\",\n"); + phoneNumbers.put("geschäftlich", rec.getString("phone_office")); if (isNotBlank(rec.getString("phone_mobile"))) - result.append(" \"mobile\": " + "\"" + rec.getString("phone_mobile") + "\",\n"); + phoneNumbers.put("mobil", rec.getString("phone_mobile")); if (isNotBlank(rec.getString("fax"))) - result.append(" \"fax\": " + "\"" + rec.getString("fax") + "\",\n"); - return (result + "}").replace("\",\n}", "\"\n}"); + phoneNumbers.put("Fax", rec.getString("fax")); + return phoneNumbers; } private String toAddress(final Record rec) { -- 2.39.5 From fed712bf0286958c2a961fdd1e6444fe9fad2e0a Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 30 Apr 2024 11:23:24 +0200 Subject: [PATCH 7/8] fix putEmailAdresses -> putEmailAddresses --- .../hs/office/contact/HsOfficeContactController.java | 2 +- .../hsadminng/hs/office/contact/HsOfficeContactEntity.java | 2 +- .../office/contact/HsOfficeContactEntityPatcherUnitTest.java | 4 ++-- .../hsadminng/hs/office/migration/ImportOfficeData.java | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java index 028e8398..90449ce7 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactController.java @@ -120,7 +120,7 @@ public class HsOfficeContactController implements HsOfficeContactsApi { @SuppressWarnings("unchecked") final BiConsumer RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> { - entity.putEmailAdresses(from(resource.getEmailAddresses())); + entity.putEmailAddresses(from(resource.getEmailAddresses())); entity.putPhoneNumbers(from(resource.getPhoneNumbers())); }; } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java index a33ddf5d..87caacfe 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java @@ -77,7 +77,7 @@ public class HsOfficeContactEntity implements Stringifyable, RbacObject { return PatchableMapWrapper.of(emailAddressesWrapper, (newWrapper) -> {emailAddressesWrapper = newWrapper; }, emailAddresses ); } - public void putEmailAdresses(Map newEmailAddresses) { + public void putEmailAddresses(Map newEmailAddresses) { getEmailAddresses().assign(newEmailAddresses); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java index ffd5dd4b..ce565cc7 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java @@ -46,7 +46,7 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< final var entity = new HsOfficeContactEntity(); entity.setUuid(INITIAL_CONTACT_UUID); entity.setLabel("initial label"); - entity.putEmailAdresses(Map.ofEntries( + entity.putEmailAddresses(Map.ofEntries( entry("main", "initial@example.org"), entry("paul", "paul@example.com"), entry("mila", "mila@example.com"))); @@ -80,7 +80,7 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< "resources", HsOfficeContactPatchResource::setEmailAddresses, PATCH_EMAIL_ADDRESSES, - HsOfficeContactEntity::putEmailAdresses, + HsOfficeContactEntity::putEmailAddresses, PATCHED_EMAIL_ADDRESSES) .notNullable(), new SimpleProperty<>( 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 c42e0629..5b3ee744 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 @@ -1092,7 +1092,7 @@ public class ImportOfficeData extends ContextBasedTest { contactRecord.getString("first_name"), contactRecord.getString("last_name"), contactRecord.getString("firma"))); - contact.putEmailAdresses( Map.of("main", contactRecord.getString("email"))); + contact.putEmailAddresses( Map.of("main", contactRecord.getString("email"))); contact.setPostalAddress(toAddress(contactRecord)); contact.putPhoneNumbers(toPhoneNumbers(contactRecord)); -- 2.39.5 From 7347b2a5fd5984e7f3410b60fb471fe479f52651 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 30 Apr 2024 12:05:42 +0200 Subject: [PATCH 8/8] amendmends according to code-review --- .../hs-office/hs-office-contact-schemas.yaml | 15 +++++++++- .../501-contact/5010-hs-office-contact.sql | 2 +- .../5018-hs-office-contact-test-data.sql | 2 +- ...OfficeContactControllerAcceptanceTest.java | 16 +++++----- .../HsOfficeContactEntityPatcherUnitTest.java | 12 ++++---- ...OfficeDebitorControllerAcceptanceTest.java | 10 +++---- .../hs/office/migration/ImportOfficeData.java | 30 +++++++++---------- 7 files changed, 50 insertions(+), 37 deletions(-) diff --git a/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml b/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml index 6d9f2a46..5905bdf4 100644 --- a/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml +++ b/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml @@ -56,4 +56,17 @@ components: # forces generating a java.lang.Object containing a Map, instead of class HsOfficeContactEmailAddresses anyOf: - type: object - additionalProperties: true + properties: + phone_office: + type: string + nullable: true + phone_private: + type: string + nullable: true + phone_mobile: + type: string + nullable: true + fax: + type: string + nullable: true + additionalProperties: false diff --git a/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql b/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql index 8e45f583..ca875a89 100644 --- a/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql +++ b/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql @@ -11,7 +11,7 @@ create table if not exists hs_office_contact label varchar(128) not null, postalAddress text, emailAddresses jsonb not null, - phoneNumbers text + phoneNumbers jsonb not null ); --// diff --git a/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql b/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql index 2a989593..e9e7a9e0 100644 --- a/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql +++ b/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql @@ -32,7 +32,7 @@ begin contLabel, postalAddr, ('{ "main": "' || emailAddr || '" }')::jsonb, - ('{ "office": "+49 123 1234567" }')::jsonb + ('{ "phone_office": "+49 123 1234567" }')::jsonb ); end; $$; --// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java index 0b53d0b1..1b209737 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java @@ -188,7 +188,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu "main": "contact-admin@firstcontact.example.com" }, "phoneNumbers": { - "office": "+49 123 1234567" + "phone_office": "+49 123 1234567" } } """)); // @formatter:on @@ -216,7 +216,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "postalAddress": "Patched Address", "phoneNumbers": { - "office": "+01 100 123456" + "phone_office": "+01 100 123456" } } """) @@ -230,7 +230,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .body("label", is("Temp patched contact")) .body("emailAddresses", is(Map.of("main", "patched@example.org"))) .body("postalAddress", is("Patched Address")) - .body("phoneNumbers", is(Map.of("office", "+01 100 123456"))); + .body("phoneNumbers", is(Map.of("phone_office", "+01 100 123456"))); // @formatter:on // finally, the contact is actually updated @@ -240,7 +240,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu assertThat(person.getLabel()).isEqualTo("Temp patched contact"); assertThat(person.getEmailAddresses()).containsExactlyEntriesOf(Map.of("main", "patched@example.org")); assertThat(person.getPostalAddress()).isEqualTo("Patched Address"); - assertThat(person.getPhoneNumbers()).containsExactlyEntriesOf(Map.of("office", "+01 100 123456")); + assertThat(person.getPhoneNumbers()).containsExactlyEntriesOf(Map.of("phone_office", "+01 100 123456")); return true; }); } @@ -261,7 +261,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu "main": "patched@example.org" }, "phoneNumbers": { - "office": "+01 100 123456" + "phone_office": "+01 100 123456" } } """) @@ -275,7 +275,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .body("label", is(givenContact.getLabel())) .body("emailAddresses", is(Map.of("main", "patched@example.org"))) .body("postalAddress", is(givenContact.getPostalAddress())) - .body("phoneNumbers", is(Map.of("office", "+01 100 123456"))); + .body("phoneNumbers", is(Map.of("phone_office", "+01 100 123456"))); // @formatter:on // finally, the contact is actually updated @@ -284,7 +284,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu assertThat(person.getLabel()).isEqualTo(givenContact.getLabel()); assertThat(person.getEmailAddresses()).containsExactlyEntriesOf(Map.of("main", "patched@example.org")); assertThat(person.getPostalAddress()).isEqualTo(givenContact.getPostalAddress()); - assertThat(person.getPhoneNumbers()).containsExactlyEntriesOf(Map.of("office", "+01 100 123456")); + assertThat(person.getPhoneNumbers()).containsExactlyEntriesOf(Map.of("phone_office", "+01 100 123456")); return true; }); } @@ -357,7 +357,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .label("Temp from " + Context.getCallerMethodNameFromStackFrame(1) ) .emailAddresses(Map.of("main", RandomStringUtils.randomAlphabetic(10) + "@example.org")) .postalAddress("Postal Address " + RandomStringUtils.randomAlphabetic(10)) - .phoneNumbers(Map.of("office", "+01 200 " + RandomStringUtils.randomNumeric(8))) + .phoneNumbers(Map.of("phone_office", "+01 200 " + RandomStringUtils.randomNumeric(8))) .build(); return contactRepo.save(newContact); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java index ce565cc7..a9c20958 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java @@ -31,13 +31,13 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< ); private static final Map PATCH_PHONE_NUMBERS = patchMap( - entry("mobile", null), - entry("private", "+49 40 987654321"), + entry("phone_mobile", null), + entry("phone_private", "+49 40 987654321"), entry("fax", "+49 40 12345-99") ); private static final Map PATCHED_PHONE_NUMBERS = patchMap( - entry("office", "+49 40 12345-00"), - entry("private", "+49 40 987654321"), + entry("phone_office", "+49 40 12345-00"), + entry("phone_private", "+49 40 987654321"), entry("fax", "+49 40 12345-99") ); @@ -51,8 +51,8 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< entry("paul", "paul@example.com"), entry("mila", "mila@example.com"))); entity.putPhoneNumbers(Map.ofEntries( - entry("office", "+49 40 12345-00"), - entry("mobile", "+49 1555 1234567"), + entry("phone_office", "+49 40 12345-00"), + entry("phone_mobile", "+49 1555 1234567"), entry("fax", "+49 40 12345-90"))); entity.setPostalAddress("Initialstraße 50\n20000 Hamburg"); return entity; diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java index 71505f38..9bda7ec4 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java @@ -108,7 +108,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "contact": { "label": "first contact", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": { "office": "+49 123 1234567" } + "phoneNumbers": { "phone_office": "+49 123 1234567" } } }, "debitorNumber": 1000111, @@ -133,7 +133,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "contact": { "label": "first contact", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": { "office": "+49 123 1234567" } + "phoneNumbers": { "phone_office": "+49 123 1234567" } } }, "details": { @@ -231,7 +231,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu } @Test - void globalAdmin_withoutAssumedRoles_canFindDebitorDebitorByDebitorNumber() throws JSONException { + void globalAdmin_withoutAssumedRoles_canFindDebitorDebitorByDebitorNumber() { RestAssured // @formatter:off .given() @@ -466,7 +466,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "label": "first contact", "postalAddress": "Vorname Nachname\\nStraße Hnr\\nPLZ Stadt", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": { "office": "+49 123 1234567" } + "phoneNumbers": { "phone_office": "+49 123 1234567" } } }, "debitorNumber": 1000111, @@ -482,7 +482,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "label": "first contact", "postalAddress": "Vorname Nachname\\nStraße Hnr\\nPLZ Stadt", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": { "office": "+49 123 1234567" } + "phoneNumbers": { "phone_office": "+49 123 1234567" } } }, "details": { 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 5b3ee744..9bb44f3d 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 @@ -390,16 +390,16 @@ public class ImportOfficeData extends ContextBasedTest { assertThat(toFormattedString(coopAssets)).isEqualToIgnoringWhitespace(""" { - 30000=CoopAssetsTransaction(M-1001700: 2000-12-06, DEPOSIT, 1280.00, legacy data import, for subscription A), - 31000=CoopAssetsTransaction(M-1002000: 2000-12-06, DEPOSIT, 128.00, legacy data import, for subscription B), - 32000=CoopAssetsTransaction(M-1001700: 2005-01-10, DEPOSIT, 2560.00, legacy data import, for subscription C), - 33001=CoopAssetsTransaction(M-1001700: 2005-01-10, TRANSFER, -512.00, legacy data import, for transfer to 10), - 33002=CoopAssetsTransaction(M-1002000: 2005-01-10, ADOPTION, 512.00, legacy data import, for transfer from 7), - 34001=CoopAssetsTransaction(M-1002000: 2016-12-31, CLEARING, -8.00, legacy data import, for cancellation D), - 34002=CoopAssetsTransaction(M-1002000: 2016-12-31, DISBURSAL, -100.00, legacy data import, for cancellation D), - 34003=CoopAssetsTransaction(M-1002000: 2016-12-31, LOSS, -20.00, legacy data import, for cancellation D), - 35001=CoopAssetsTransaction(M-1909000: 2024-01-15, DEPOSIT, 128.00, legacy data import, for subscription E), - 35002=CoopAssetsTransaction(M-1909000: 2024-01-20, ADJUSTMENT, -128.00, legacy data import, chargeback for subscription E, M-1909000:DEP:+128.00) + 30000=CoopAssetsTransaction(M-1001700: 2000-12-06, DEPOSIT, 1280.00, 1001700, for subscription A), + 31000=CoopAssetsTransaction(M-1002000: 2000-12-06, DEPOSIT, 128.00, 1002000, for subscription B), + 32000=CoopAssetsTransaction(M-1001700: 2005-01-10, DEPOSIT, 2560.00, 1001700, for subscription C), + 33001=CoopAssetsTransaction(M-1001700: 2005-01-10, TRANSFER, -512.00, 1001700, for transfer to 10), + 33002=CoopAssetsTransaction(M-1002000: 2005-01-10, ADOPTION, 512.00, 1002000, for transfer from 7), + 34001=CoopAssetsTransaction(M-1002000: 2016-12-31, CLEARING, -8.00, 1002000, for cancellation D), + 34002=CoopAssetsTransaction(M-1002000: 2016-12-31, DISBURSAL, -100.00, 1002000, for cancellation D), + 34003=CoopAssetsTransaction(M-1002000: 2016-12-31, LOSS, -20.00, 1002000, for cancellation D), + 35001=CoopAssetsTransaction(M-1909000: 2024-01-15, DEPOSIT, 128.00, 1909000, for subscription E), + 35002=CoopAssetsTransaction(M-1909000: 2024-01-20, ADJUSTMENT, -128.00, 1909000, chargeback for subscription E, M-1909000:DEP:+128.00) } """); } @@ -867,7 +867,7 @@ public class ImportOfficeData extends ContextBasedTest { .transactionType(assetTypeMapping.get(rec.getString("action"))) .assetValue(rec.getBigDecimal("amount")) .comment(rec.getString("comment")) - .reference("legacy data import") // TODO.: Kundennummer + .reference(member.getMemberNumber().toString()) .build(); if (assetTransaction.getTransactionType() == HsOfficeCoopAssetsTransactionType.ADJUSTMENT) { @@ -1123,13 +1123,13 @@ public class ImportOfficeData extends ContextBasedTest { private Map toPhoneNumbers(final Record rec) { final var phoneNumbers = new LinkedHashMap(); if (isNotBlank(rec.getString("phone_private"))) - phoneNumbers.put("privat", rec.getString("phone_private")); + phoneNumbers.put("phone_private", rec.getString("phone_private")); if (isNotBlank(rec.getString("phone_office"))) - phoneNumbers.put("geschäftlich", rec.getString("phone_office")); + phoneNumbers.put("phone_office", rec.getString("phone_office")); if (isNotBlank(rec.getString("phone_mobile"))) - phoneNumbers.put("mobil", rec.getString("phone_mobile")); + phoneNumbers.put("phone_mobile", rec.getString("phone_mobile")); if (isNotBlank(rec.getString("fax"))) - phoneNumbers.put("Fax", rec.getString("fax")); + phoneNumbers.put("fax", rec.getString("fax")); return phoneNumbers; } -- 2.39.5