Bugfix: properly handle invalid membership with empty validity - and other empty ranges (#169)

Co-authored-by: Michael Hoennig <michael@hoennig.de>
Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/169
Reviewed-by: Marc Sandlus <marc.sandlus@hostsharing.net>

src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemController.java

@@ -157,9 +157,9 @@ public class HsBookingItemController implements HsBookingItemsApi {
         }
     }
 
-    final BiConsumer<HsBookingItem, HsBookingItemResource> ITEM_TO_RESOURCE_POSTMAPPER = (entity, resource) -> {
+    final static BiConsumer<HsBookingItem, HsBookingItemResource> 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));
         }
     };

src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipController.java

@@ -162,7 +162,7 @@ public class HsOfficeMembershipController implements HsOfficeMembershipsApi {
     final BiConsumer<HsOfficeMembershipEntity, HsOfficeMembershipResource> 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?

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<HsOfficeSepaMandateEntity, HsOfficeSepaMandateResource> 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));

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()
+                    HsBookingProjectRealEntity.builder()
-                                    .uuid(invocation.getArgument(1))
+                            .uuid(invocation.getArgument(1))
-                                    .build()
+                            .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}",
+                                        "project.uuid": "{projectUuid}",
-                                "type": "MANAGED_SERVER",
+                                        "type": "MANAGED_SERVER",
-                                "caption": "some new booking",
+                                        "caption": "some new booking",
-                                "validTo": "{validTo}",
+                                        "validTo": "{validTo}",
-                                "resources": { "CPU": 12, "RAM": 4, "SSD": 100, "Traffic": 250 }
+                                        "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("""
+                    .andExpect(jsonPath(
-                            {
+                            "$", lenientlyEquals("""
-                                "type": "MANAGED_SERVER",
+                                    {
-                                "caption": "some new booking",
+                                        "type": "MANAGED_SERVER",
-                                "validFrom": "{today}",
+                                        "caption": "some new booking",
-                                "validTo": "{todayPlus1Month}",
+                                        "validFrom": "{today}",
-                                "resources": { "CPU": 12, "SSD": 100, "Traffic": 250 }
+                                        "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}",
+                                        "project.uuid": "{projectUuid}",
-                                "type": "MANAGED_SERVER",
+                                        "type": "MANAGED_SERVER",
-                                "caption": "some new booking",
+                                        "caption": "some new booking",
-                                "validFrom": "{validFrom}", // not specified => not accepted
+                                        "validFrom": "{validFrom}", // not specified => not accepted
-                                "resources": { "CPU": 12, "RAM": 4, "SSD": 100, "Traffic": 250 }
+                                        "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("""
+                    .andExpect(jsonPath(
-                            {
+                            "$", lenientlyEquals("""
-                                "message": "ERROR: [400] JSON parse error: Unrecognized field \\"validFrom\\" (class ${resourceClass}), not marked as ignorable"
+                                    {
-                            }
+                                        "message": "ERROR: [400] JSON parse error: Unrecognized field \\"validFrom\\" (class ${resourceClass}), not marked as ignorable"
-                            """.replace("${resourceClass}", HsBookingItemInsertResource.class.getName()))));
+                                    }
+                                    """.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();
         }
     }
 }

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
+                              }
+                            ]
+                            """)));
         }
     }
 

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
+                              }
+                            ]
+                            """)));
+        }
+    }
+}