From 399512bd98c94d65e82338f358235a6935dead18 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 2 Apr 2025 12:09:08 +0200 Subject: [PATCH] Bugfix: properly handle invalid membership with empty validity - and other empty ranges (#169) Co-authored-by: Michael Hoennig Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/169 Reviewed-by: Marc Sandlus --- .../booking/item/HsBookingItemController.java | 4 +- .../HsOfficeMembershipController.java | 2 +- .../HsOfficeSepaMandateController.java | 7 +- .../item/HsBookingItemControllerRestTest.java | 111 +++++++++++------ .../HsOfficeMembershipControllerRestTest.java | 57 +++++++-- ...HsOfficeSepaMandateControllerRestTest.java | 117 ++++++++++++++++++ 6 files changed, 241 insertions(+), 57 deletions(-) create mode 100644 src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerRestTest.java diff --git a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemController.java b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemController.java index 24b61ad2..ddd8a281 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemController.java @@ -157,9 +157,9 @@ public class HsBookingItemController implements HsBookingItemsApi { } } - final BiConsumer ITEM_TO_RESOURCE_POSTMAPPER = (entity, resource) -> { + final static BiConsumer ITEM_TO_RESOURCE_POSTMAPPER = (entity, resource) -> { resource.setValidFrom(entity.getValidity().lower()); - if (entity.getValidity().hasUpperBound()) { + if (entity.getValidity().upper() != null) { resource.setValidTo(entity.getValidity().upper().minusDays(1)); } }; diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipController.java index 97042c1b..72bac844 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipController.java @@ -162,7 +162,7 @@ public class HsOfficeMembershipController implements HsOfficeMembershipsApi { final BiConsumer SEPA_MANDATE_ENTITY_TO_RESOURCE_POSTMAPPER = (entity, resource) -> { resource.setMemberNumber(entity.getTaggedMemberNumber()); resource.setValidFrom(entity.getValidity().lower()); - if (entity.getValidity().hasUpperBound()) { + if (entity.getValidity().upper() != null) { resource.setValidTo(entity.getValidity().upper().minusDays(1)); } resource.getPartner().setPartnerNumber(entity.getPartner().getTaggedPartnerNumber()); // TODO.refa: use partner mapper? diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateController.java index 5456274e..3b59ff3d 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateController.java @@ -17,8 +17,6 @@ import org.springframework.transaction.annotation.Transactional; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder; -import jakarta.persistence.EntityManager; -import jakarta.persistence.PersistenceContext; import jakarta.validation.ValidationException; import java.util.List; import java.util.UUID; @@ -45,9 +43,6 @@ public class HsOfficeSepaMandateController implements HsOfficeSepaMandatesApi { @Autowired private HsOfficeSepaMandateRepository sepaMandateRepo; - @PersistenceContext - private EntityManager em; - @Override @Transactional(readOnly = true) @Timed("app.office.sepaMandates.api.getListOfSepaMandates") @@ -140,7 +135,7 @@ public class HsOfficeSepaMandateController implements HsOfficeSepaMandatesApi { final BiConsumer SEPA_MANDATE_ENTITY_TO_RESOURCE_POSTMAPPER = (entity, resource) -> { resource.setValidFrom(entity.getValidity().lower()); - if (entity.getValidity().hasUpperBound()) { + if (entity.getValidity().upper() != null) { resource.setValidTo(entity.getValidity().upper().minusDays(1)); } resource.setDebitor(mapper.map(entity.getDebitor(), HsOfficeDebitorResource.class)); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemControllerRestTest.java b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemControllerRestTest.java index ee226814..17558d42 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemControllerRestTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemControllerRestTest.java @@ -1,9 +1,11 @@ package net.hostsharing.hsadminng.hs.booking.item; +import io.hypersistence.utils.hibernate.type.range.Range; import net.hostsharing.hsadminng.config.JsonObjectMapperConfiguration; import net.hostsharing.hsadminng.config.MessageTranslator; import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.hs.booking.generated.api.v1.model.HsBookingItemInsertResource; +import net.hostsharing.hsadminng.hs.booking.generated.api.v1.model.HsBookingItemResource; import net.hostsharing.hsadminng.hs.booking.project.HsBookingProjectRealEntity; import net.hostsharing.hsadminng.hs.booking.project.HsBookingProjectRealRepository; import net.hostsharing.hsadminng.mapper.StrictMapper; @@ -33,6 +35,7 @@ import java.util.Map; import java.util.UUID; import static net.hostsharing.hsadminng.test.JsonMatcher.lenientlyEquals; +import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.matchesRegex; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -98,10 +101,10 @@ class HsBookingItemControllerRestTest { // given when(em.find(HsBookingProjectRealEntity.class, givenProjectUuid)).thenAnswer(invocation -> - HsBookingProjectRealEntity.builder() - .uuid(invocation.getArgument(1)) - .build() - ); + HsBookingProjectRealEntity.builder() + .uuid(invocation.getArgument(1)) + .build() + ); when(rbacBookingItemRepo.save(any())).thenAnswer(invocation -> invocation.getArgument(0)); // when @@ -110,34 +113,35 @@ class HsBookingItemControllerRestTest { .header("Authorization", "Bearer superuser-alex@hostsharing.net") .contentType(MediaType.APPLICATION_JSON) .content(""" - { - "project.uuid": "{projectUuid}", - "type": "MANAGED_SERVER", - "caption": "some new booking", - "validTo": "{validTo}", - "resources": { "CPU": 12, "RAM": 4, "SSD": 100, "Traffic": 250 } - } - """ + { + "project.uuid": "{projectUuid}", + "type": "MANAGED_SERVER", + "caption": "some new booking", + "validTo": "{validTo}", + "resources": { "CPU": 12, "RAM": 4, "SSD": 100, "Traffic": 250 } + } + """ .replace("{projectUuid}", givenProjectUuid.toString()) .replace("{validTo}", LocalDate.now().plusMonths(1).toString()) ) .accept(MediaType.APPLICATION_JSON)) - .andDo(print()) + .andDo(print()) // then .andExpect(status().isCreated()) - .andExpect(jsonPath("$", lenientlyEquals(""" - { - "type": "MANAGED_SERVER", - "caption": "some new booking", - "validFrom": "{today}", - "validTo": "{todayPlus1Month}", - "resources": { "CPU": 12, "SSD": 100, "Traffic": 250 } - } - """ + .andExpect(jsonPath( + "$", lenientlyEquals(""" + { + "type": "MANAGED_SERVER", + "caption": "some new booking", + "validFrom": "{today}", + "validTo": "{todayPlus1Month}", + "resources": { "CPU": 12, "SSD": 100, "Traffic": 250 } + } + """ .replace("{today}", LocalDate.now().toString()) .replace("{todayPlus1Month}", LocalDate.now().plusMonths(1).toString())) - )) + )) .andExpect(header().string("Location", matchesRegex("http://localhost/api/hs/booking/items/[^/]*"))); } @@ -160,27 +164,60 @@ class HsBookingItemControllerRestTest { .header("Authorization", "Bearer superuser-alex@hostsharing.net") .contentType(MediaType.APPLICATION_JSON) .content(""" - { - "project.uuid": "{projectUuid}", - "type": "MANAGED_SERVER", - "caption": "some new booking", - "validFrom": "{validFrom}", // not specified => not accepted - "resources": { "CPU": 12, "RAM": 4, "SSD": 100, "Traffic": 250 } - } - """ + { + "project.uuid": "{projectUuid}", + "type": "MANAGED_SERVER", + "caption": "some new booking", + "validFrom": "{validFrom}", // not specified => not accepted + "resources": { "CPU": 12, "RAM": 4, "SSD": 100, "Traffic": 250 } + } + """ .replace("{projectUuid}", givenProjectUuid.toString()) .replace("{validFrom}", LocalDate.now().plusMonths(1).toString()) ) .accept(MediaType.APPLICATION_JSON)) - .andDo(print()) + .andDo(print()) // then .andExpect(status().is4xxClientError()) - .andExpect(jsonPath("$", lenientlyEquals(""" - { - "message": "ERROR: [400] JSON parse error: Unrecognized field \\"validFrom\\" (class ${resourceClass}), not marked as ignorable" - } - """.replace("${resourceClass}", HsBookingItemInsertResource.class.getName())))); + .andExpect(jsonPath( + "$", lenientlyEquals(""" + { + "message": "ERROR: [400] JSON parse error: Unrecognized field \\"validFrom\\" (class ${resourceClass}), not marked as ignorable" + } + """.replace("${resourceClass}", HsBookingItemInsertResource.class.getName())))); + } + } + + @Nested + class itemToResourcePostmapper { + + @Test + void canConvertEmptyValidity() { + + // given + final var givenProject = HsBookingProjectRealEntity.builder() + .uuid(UUID.randomUUID()) + .build(); + when(em.find(HsBookingProjectRealEntity.class, givenProject.getUuid())).thenAnswer(invocation -> + HsBookingProjectRealEntity.builder() + .uuid(invocation.getArgument(1)) + .build() + ); + final var givenBookingItem = HsBookingItemRbacEntity.builder() + .uuid(UUID.randomUUID()) + .project(givenProject) + .validity(Range.emptyRange(LocalDate.class)) + .build(); + final var givenBookingResource = new HsBookingItemResource(); + + // when + HsBookingItemController.ITEM_TO_RESOURCE_POSTMAPPER.accept( + givenBookingItem, givenBookingResource); + + // then + assertThat(givenBookingResource.getValidFrom()).isNull(); + assertThat(givenBookingResource.getValidTo()).isNull(); } } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipControllerRestTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipControllerRestTest.java index 302d2c34..7914dacd 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipControllerRestTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipControllerRestTest.java @@ -1,5 +1,6 @@ package net.hostsharing.hsadminng.hs.office.membership; +import io.hypersistence.utils.hibernate.type.range.Range; import net.hostsharing.hsadminng.config.MessageTranslator; import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.hs.office.coopassets.HsOfficeCoopAssetsTransactionRepository; @@ -22,6 +23,7 @@ import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; +import java.time.LocalDate; import java.util.List; import java.util.Optional; import java.util.UUID; @@ -34,6 +36,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.startsWith; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -45,16 +48,24 @@ public class HsOfficeMembershipControllerRestTest { private static final HsOfficePartnerRealEntity PARTNER_12345 = HsOfficePartnerRealEntity.builder() .partnerNumber(12345) .build(); + public static final HsOfficeMembershipEntity MEMBERSHIP_1234502 = HsOfficeMembershipEntity.builder() + .partner(PARTNER_12345) + .memberNumberSuffix("02") + .validity(Range.emptyRange(LocalDate.class)) + .status(HsOfficeMembershipStatus.INVALID) + .build(); public static final HsOfficeMembershipEntity MEMBERSHIP_1234501 = HsOfficeMembershipEntity.builder() .partner(PARTNER_12345) .memberNumberSuffix("01") .validity(localDateRange("[2013-10-01,]")) + .membershipFeeBillable(false) .status(HsOfficeMembershipStatus.ACTIVE) .build(); public static final HsOfficeMembershipEntity MEMBERSHIP_1234500 = HsOfficeMembershipEntity.builder() .partner(PARTNER_12345) .memberNumberSuffix("00") .validity(localDateRange("[2011-04-01,2016-12-31]")) + .membershipFeeBillable(true) .status(HsOfficeMembershipStatus.CANCELLED) .build(); public static final String MEMBERSHIP_1234501_JSON = """ @@ -101,15 +112,6 @@ public class HsOfficeMembershipControllerRestTest { mockMvc.perform(MockMvcRequestBuilders .get("/api/hs/office/memberships?partnerNumber=P-12345") .header("Authorization", "Bearer superuser-alex@hostsharing.net") - .contentType(MediaType.APPLICATION_JSON) - .content(""" - { - "partner.uuid": null, - "memberNumberSuffix": "01", - "validFrom": "2022-10-13", - "membershipFeeBillable": "true" - } - """) .accept(MediaType.APPLICATION_JSON)) // then @@ -124,7 +126,8 @@ public class HsOfficeMembershipControllerRestTest { when(membershipRepo.findMembershipsByPartnerNumber(12345)) .thenReturn(List.of( MEMBERSHIP_1234500, - MEMBERSHIP_1234501 + MEMBERSHIP_1234501, + MEMBERSHIP_1234502 )); // when @@ -143,8 +146,40 @@ public class HsOfficeMembershipControllerRestTest { .accept(MediaType.APPLICATION_JSON)) // then + .andDo(print()) .andExpect(status().is2xxSuccessful()) - .andExpect(jsonPath("$", hasSize(2))); + .andExpect(jsonPath("$", hasSize(3))) + .andExpect(jsonPath("$", lenientlyEquals(""" + [ + { + "partner": { "partnerNumber": "P-12345" }, + "memberNumber": "M-1234500", + "memberNumberSuffix": "00", + "validFrom": "2011-04-01", + "validTo": "2016-12-30", + "status": "CANCELLED", + "membershipFeeBillable": true + }, + { + "partner": { "partnerNumber": "P-12345" }, + "memberNumber": "M-1234501", + "memberNumberSuffix": "01", + "validFrom": "2013-10-01", + "validTo": null, + "status": "ACTIVE", + "membershipFeeBillable": false + }, + { + "partner": { "partnerNumber": "P-12345" }, + "memberNumber": "M-1234502", + "memberNumberSuffix": "02", + "validFrom": null, + "validTo": null, + "status": "INVALID", + "membershipFeeBillable": null + } + ] + """))); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerRestTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerRestTest.java new file mode 100644 index 00000000..55f02269 --- /dev/null +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerRestTest.java @@ -0,0 +1,117 @@ +package net.hostsharing.hsadminng.hs.office.sepamandate; + +import io.hypersistence.utils.hibernate.type.range.Range; +import net.hostsharing.hsadminng.config.DisableSecurityConfig; +import net.hostsharing.hsadminng.config.MessageTranslator; +import net.hostsharing.hsadminng.context.Context; +import net.hostsharing.hsadminng.hs.office.bankaccount.HsOfficeBankAccountRepository; +import net.hostsharing.hsadminng.hs.office.debitor.HsOfficeDebitorEntity; +import net.hostsharing.hsadminng.hs.office.debitor.HsOfficeDebitorRepository; +import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerRealEntity; +import net.hostsharing.hsadminng.mapper.StrictMapper; +import net.hostsharing.hsadminng.persistence.EntityManagerWrapper; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.context.annotation.Import; +import org.springframework.http.MediaType; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; + +import java.time.LocalDate; +import java.util.List; + +import static net.hostsharing.hsadminng.test.JsonMatcher.lenientlyEquals; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@WebMvcTest(HsOfficeSepaMandateController.class) +@Import({ StrictMapper.class, DisableSecurityConfig.class, MessageTranslator.class}) +@ActiveProfiles("test") +class HsOfficeSepaMandateControllerRestTest { + + @MockitoBean + HsOfficeDebitorRepository debitorRepo; + + @MockitoBean + HsOfficeBankAccountRepository bankAccountRepo; + + @MockitoBean + HsOfficeSepaMandateRepository sepaMandateRepo; + + @Autowired + MockMvc mockMvc; + + @MockitoBean + Context contextMock; + + @MockitoBean + EntityManagerWrapper em; + + @Nested + class GetListOfSepaMandates { + + @Test + void mapsSepaMandatesToResource() throws Exception { + + // given + final var givenPartner = HsOfficePartnerRealEntity.builder().partnerNumber(12345).build(); + final var givenDebitor = HsOfficeDebitorEntity.builder() + .partner(givenPartner) + .debitorNumberSuffix("00") + .build(); + when(sepaMandateRepo.findSepaMandateByOptionalIban("DE123")).thenReturn( + List.of( + HsOfficeSepaMandateEntity.builder() + .agreement(LocalDate.of(2024, 10, 15)) + .debitor(givenDebitor) + .reference("ref-xyz") + .validity(Range.emptyRange(LocalDate.class)) + .build(), + HsOfficeSepaMandateEntity.builder() + .agreement(LocalDate.of(2024, 11, 1)) + .debitor(givenDebitor) + .reference("ref-abc") + .validity(Range.localDateRange("[2024-11-01,)")) + .build() + ) + ); + + // when + mockMvc.perform(MockMvcRequestBuilders + .get("/api/hs/office/sepamandates?iban=DE123") + .header("Authorization", "Bearer superuser-alex@hostsharing.net") + .contentType(MediaType.APPLICATION_JSON) + .accept(MediaType.APPLICATION_JSON)) + + // then + .andDo(print()) + .andExpect(status().is2xxSuccessful()) + .andExpect(jsonPath("$", lenientlyEquals(""" + [ + { + "debitor": { "debitorNumber": "D-1234500" }, + "bankAccount": null, + "reference": "ref-xyz", + "agreement": "2024-10-15", + "validFrom": null, + "validTo": null + }, + { + "debitor": { "debitorNumber": "D-1234500" }, + "bankAccount": null, + "reference": "ref-abc", + "agreement": "2024-11-01", + "validFrom": "2024-11-01", + "validTo": null + } + ] + """))); + } + } +}