add problem with Too Many Business-Rules Implemented in Controllers to README

README.md

@@ -575,7 +575,7 @@ that and creates too many (grant- and role-) rows and too even tables which coul
 
 The basic idea is always to always have a fixed set of ordered role-types which apply for all DB-tables under RBAC,
 e.g. OWNER>ADMIN>AGENT\[>PROXY?\]>TENENT>REFERRER.
-Grants between these for the same DB-row would be implicit by order comparision.
+Grants between these for the same DB-row would be implicit by order comparison.
 This way we would get rid of all explicit grants within the same DB-row
 and would not need the `rbac.role` table anymore.
 We would also reduce the depth of the expensive recursive CTE-query.
@@ -591,6 +591,12 @@ E.g. the uuid of the target main object is often taken from an uuid of a sub-sub
 (For now, use `StrictMapper` to avoid this, for the case it happens.)
 
 
+### Too Many Business-Rules Implemented in Controllers
+
+Some REST-Controllers implement too much code for business-roles.
+This should be extracted to services.
+
+
 ## How To ...
 
 ### How to Configure .pgpass for the Default PostgreSQL Database?

src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionController.java

@@ -8,7 +8,8 @@ import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeCoopAs
 import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeCoopAssetsTransactionTypeResource;
 import net.hostsharing.hsadminng.hs.office.membership.HsOfficeMembershipEntity;
 import net.hostsharing.hsadminng.hs.office.membership.HsOfficeMembershipRepository;
-import net.hostsharing.hsadminng.mapper.StandardMapper;
+import net.hostsharing.hsadminng.mapper.StrictMapper;
+import net.hostsharing.hsadminng.persistence.EntityManagerWrapper;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.format.annotation.DateTimeFormat;
 import org.springframework.format.annotation.DateTimeFormat.ISO;
@@ -18,6 +19,7 @@ import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder;
 
 import jakarta.persistence.EntityNotFoundException;
+import jakarta.validation.ValidationException;
 import java.time.LocalDate;
 import java.util.ArrayList;
 import java.util.List;
@@ -37,7 +39,10 @@ public class HsOfficeCoopAssetsTransactionController implements HsOfficeCoopAsse
     private Context context;
 
     @Autowired
-    private StandardMapper mapper;
+    private StrictMapper mapper;
+
+    @Autowired
+    private EntityManagerWrapper emw;
 
     @Autowired
     private HsOfficeCoopAssetsTransactionRepository coopAssetsTransactionRepo;
@@ -60,7 +65,7 @@ public class HsOfficeCoopAssetsTransactionController implements HsOfficeCoopAsse
                 fromValueDate,
                 toValueDate);
 
-        final var resources = mapper.mapList(entities, HsOfficeCoopAssetsTransactionResource.class);
+        final var resources = mapper.mapList(entities, HsOfficeCoopAssetsTransactionResource.class, ENTITY_TO_RESOURCE_POSTMAPPER);
         return ResponseEntity.ok(resources);
     }
 
@@ -85,7 +90,7 @@ public class HsOfficeCoopAssetsTransactionController implements HsOfficeCoopAsse
                         .path("/api/hs/office/coopassetstransactions/{id}")
                         .buildAndExpand(saved.getUuid())
                         .toUri();
-        final var mapped = mapper.map(saved, HsOfficeCoopAssetsTransactionResource.class);
+        final var mapped = mapper.map(saved, HsOfficeCoopAssetsTransactionResource.class, ENTITY_TO_RESOURCE_POSTMAPPER);
         return ResponseEntity.created(uri).body(mapped);
     }
 
@@ -141,7 +146,25 @@ public class HsOfficeCoopAssetsTransactionController implements HsOfficeCoopAsse
         }
     }
 
+    final BiConsumer<HsOfficeCoopAssetsTransactionEntity, HsOfficeCoopAssetsTransactionResource> ENTITY_TO_RESOURCE_POSTMAPPER = (entity, resource) -> {
+        if (resource.getReversalAssetTx() != null) {
+            resource.getReversalAssetTx().setRevertedAssetTxUuid(entity.getUuid());
+        }
+        if (resource.getRevertedAssetTx() != null) {
+            resource.getRevertedAssetTx().setReversalAssetTxUuid(entity.getUuid());
+        }
+        if (resource.getAdoptionAssetTx() != null) {
+            resource.getAdoptionAssetTx().setTransferAssetTxUuid(entity.getUuid());
+        }
+        if (resource.getTransferAssetTx() != null) {
+            resource.getTransferAssetTx().setAdoptionAssetTxUuid(entity.getUuid());
+        }
+    };
+
     final BiConsumer<HsOfficeCoopAssetsTransactionInsertResource, HsOfficeCoopAssetsTransactionEntity> RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> {
+        if (resource.getMembershipUuid() != null) {
+            entity.setMembership(emw.getReference(HsOfficeMembershipEntity.class, resource.getMembershipUuid()));
+        }
         if (resource.getRevertedAssetTxUuid() != null) {
             entity.setRevertedAssetTx(coopAssetsTransactionRepo.findByUuid(resource.getRevertedAssetTxUuid())
                     .orElseThrow(() -> new EntityNotFoundException("ERROR: [400] revertedEntityUuid %s not found".formatted(
@@ -150,16 +173,7 @@ public class HsOfficeCoopAssetsTransactionController implements HsOfficeCoopAsse
 
         final var adoptingMembership = determineAdoptingMembership(resource);
         if (adoptingMembership != null) {
-            final var adoptingAssetTx = coopAssetsTransactionRepo.save(
+            final var adoptingAssetTx = coopAssetsTransactionRepo.save(createAdoptingAssetTx(entity, adoptingMembership));
-                    HsOfficeCoopAssetsTransactionEntity.builder()
-                            .membership(adoptingMembership)
-                            .transactionType(HsOfficeCoopAssetsTransactionType.ADOPTION)
-                            .assetTransferTx(entity)
-                            .assetValue(entity.getAssetValue().negate())
-                            .comment(entity.getComment())
-                            .reference(entity.getReference())
-                            .valueDate(entity.getValueDate())
-                            .build());
             entity.setAssetAdoptionAssetTx(adoptingAssetTx);
         }
     };
@@ -169,14 +183,19 @@ public class HsOfficeCoopAssetsTransactionController implements HsOfficeCoopAsse
         final var adoptingMembershipMemberNumber = resource.getAdoptingMembershipMemberNumber();
         if (adoptingMembershipUuid != null && adoptingMembershipMemberNumber != null) {
             throw new IllegalArgumentException(
-                    "[400] either adoptingMembership.uuid or adoptingMembership.memberNumber can be given, not both");
+                // @formatter:off
+                resource.getTransactionType() == TRANSFER
+                    ? "[400] either adoptingMembership.uuid or adoptingMembership.memberNumber can be given, not both"
+                    : "[400] adoptingMembership.uuid and adoptingMembership.memberNumber must not be given for transactionType="
+                            + resource.getTransactionType());
+                // @formatter:on
         }
 
         if (adoptingMembershipUuid != null) {
             final var adoptingMembership = membershipRepo.findByUuid(adoptingMembershipUuid);
             return adoptingMembership.orElseThrow(() ->
-                    new IllegalArgumentException(
+                    new ValidationException(
-                            "[400] adoptingMembership.uuid='" + adoptingMembershipUuid + "' not found or not accessible"));
+                            "adoptingMembership.uuid='" + adoptingMembershipUuid + "' not found or not accessible"));
         }
 
         if (adoptingMembershipMemberNumber != null) {
@@ -185,15 +204,30 @@ public class HsOfficeCoopAssetsTransactionController implements HsOfficeCoopAsse
             if (adoptingMembership != null) {
                 return adoptingMembership;
             }
-            throw new IllegalArgumentException("[400] adoptingMembership.memberNumber='" + adoptingMembershipMemberNumber
+            throw new ValidationException("adoptingMembership.memberNumber='" + adoptingMembershipMemberNumber
                     + "' not found or not accessible");
         }
 
         if (resource.getTransactionType() == TRANSFER) {
-            throw new IllegalArgumentException(
+            throw new ValidationException(
-                    "[400] either adoptingMembership.uuid or adoptingMembership.memberNumber must be given for " + TRANSFER);
+                    "either adoptingMembership.uuid or adoptingMembership.memberNumber must be given for transactionType="
+                            + TRANSFER);
         }
 
         return null;
     }
-};
+
+    private HsOfficeCoopAssetsTransactionEntity createAdoptingAssetTx(
+            final HsOfficeCoopAssetsTransactionEntity transferAssetTxEntity,
+            final HsOfficeMembershipEntity adoptingMembership) {
+        return HsOfficeCoopAssetsTransactionEntity.builder()
+                .membership(adoptingMembership)
+                .transactionType(HsOfficeCoopAssetsTransactionType.ADOPTION)
+                .assetTransferTx(transferAssetTxEntity)
+                .assetValue(transferAssetTxEntity.getAssetValue().negate())
+                .comment(transferAssetTxEntity.getComment())
+                .reference(transferAssetTxEntity.getReference())
+                .valueDate(transferAssetTxEntity.getValueDate())
+                .build();
+    }
+}

src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java

@@ -51,7 +51,7 @@ public class HsOfficeCoopAssetsTransactionEntity implements Stringifyable, BaseE
             .withProp(HsOfficeCoopAssetsTransactionEntity::getReference)
             .withProp(HsOfficeCoopAssetsTransactionEntity::getComment)
             .withProp(HsOfficeCoopAssetsTransactionEntity::getRevertedAssetTx)
-            .withProp(HsOfficeCoopAssetsTransactionEntity::getAssetReversalTx)
+            .withProp(HsOfficeCoopAssetsTransactionEntity::getReversalAssetTx)
             .withProp(HsOfficeCoopAssetsTransactionEntity::getAssetAdoptionAssetTx)
             .withProp(HsOfficeCoopAssetsTransactionEntity::getAssetTransferTx)
             .quotedValues(false);
@@ -104,7 +104,7 @@ public class HsOfficeCoopAssetsTransactionEntity implements Stringifyable, BaseE
 
     // and the other way around
     @OneToOne(mappedBy = "revertedAssetTx")
-    private HsOfficeCoopAssetsTransactionEntity assetReversalTx;
+    private HsOfficeCoopAssetsTransactionEntity reversalAssetTx;
 
     // Optionally, the UUID of the corresponding transaction for a transfer transaction.
     @OneToOne(cascade = CascadeType.PERSIST) // TODO.impl: can probably be removed after office data migration

src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerRestTest.java

@@ -25,6 +25,7 @@ class HsOfficeBankAccountControllerRestTest {
     Context contextMock;
 
     @MockBean
+    @SuppressWarnings("unused") // not used in test, but in controller class
     StandardMapper mapper;
 
     @MockBean

src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionControllerRestTest.java

@@ -1,37 +1,67 @@
 package net.hostsharing.hsadminng.hs.office.coopassets;
 
+import net.hostsharing.hsadminng.config.JsonObjectMapperConfiguration;
 import net.hostsharing.hsadminng.context.Context;
+import net.hostsharing.hsadminng.hs.office.membership.HsOfficeMembershipEntity;
 import net.hostsharing.hsadminng.hs.office.membership.HsOfficeMembershipRepository;
-import net.hostsharing.hsadminng.mapper.StandardMapper;
+import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerEntity;
+import net.hostsharing.hsadminng.mapper.StrictMapper;
+import net.hostsharing.hsadminng.persistence.EntityManagerWrapper;
 import net.hostsharing.hsadminng.rbac.test.JsonBuilder;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.EnumSource;
+import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
 import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.context.annotation.Import;
 import org.springframework.http.MediaType;
+import org.springframework.test.context.junit4.SpringRunner;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
 
+import java.util.Optional;
 import java.util.UUID;
 import java.util.function.Function;
 
 import static net.hostsharing.hsadminng.rbac.test.JsonBuilder.jsonObject;
 import static org.hamcrest.Matchers.is;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.when;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 @WebMvcTest(HsOfficeCoopAssetsTransactionController.class)
+@Import({ StrictMapper.class, JsonObjectMapperConfiguration.class })
+@RunWith(SpringRunner.class)
 class HsOfficeCoopAssetsTransactionControllerRestTest {
 
+    private static final UUID AVAILABLE_MEMBERSHIP_UUID = UUID.randomUUID();
+    public static final HsOfficeMembershipEntity AVAILABLE_MEMBER_ENTITY = HsOfficeMembershipEntity.builder()
+            .uuid(AVAILABLE_MEMBERSHIP_UUID)
+            .partner(HsOfficePartnerEntity.builder()
+                    .partnerNumber(12345)
+                    .build())
+            .memberNumberSuffix("00")
+            .build();
+    private static final UUID UNAVAILABLE_MEMBERSHIP_UUID = UUID.randomUUID();
+    private static final String UNAVAILABLE_MEMBER_NUMBER = "M-1234699";
+    private static final String AVAILABLE_MEMBER_NUMBER = "M-1234600";
+
     @Autowired
     MockMvc mockMvc;
 
     @MockBean
     Context contextMock;
 
+    @Autowired
+    @SuppressWarnings("unused") // not used in test, but in controller class
+    StrictMapper mapper;
+
     @MockBean
-    StandardMapper mapper;
+    @SuppressWarnings("unused") // not used in test, but in base-class of StrictMapper
+    EntityManagerWrapper em;
 
     @MockBean
     HsOfficeCoopAssetsTransactionRepository coopAssetsTransactionRepo;
@@ -46,7 +76,9 @@ class HsOfficeCoopAssetsTransactionControllerRestTest {
                "assetValue": 128.00,
                "valueDate": "2022-10-13",
                "reference": "valid reference",
-               "comment": "valid comment"
+               "comment": "valid comment",
+               "adoptingMembership.uuid": null,
+               "adoptingMembership.memberNumber": null
             }
             """.formatted(UUID.randomUUID());
 
@@ -69,8 +101,6 @@ class HsOfficeCoopAssetsTransactionControllerRestTest {
                         .with("assetValue", -64.00),
                 "[for DEPOSIT, assetValue must be positive but is \"-64.00\"]"),
 
-        //TODO: other transaction types
-
         ASSETS_VALUE_FOR_DISBURSAL_MUST_BE_NEGATIVE(
                 requestBody -> requestBody
                         .with("transactionType", "DISBURSAL")
@@ -79,6 +109,20 @@ class HsOfficeCoopAssetsTransactionControllerRestTest {
 
         //TODO: other transaction types
 
+        ADOPTING_MEMBERSHIP_NUMBER_FOR_TRANSFER_MUST_BE_GIVEN_AND_AVAILABLE(
+                requestBody -> requestBody
+                        .with("transactionType", "TRANSFER")
+                        .with("assetValue", -64.00)
+                        .with("adoptingMembership.memberNumber", UNAVAILABLE_MEMBER_NUMBER),
+                "adoptingMembership.memberNumber='M-1234699' not found or not accessible"),
+
+        ADOPTING_MEMBERSHIP_UUID_FOR_TRANSFER_MUST_BE_GIVEN_AND_AVAILABLE(
+                requestBody -> requestBody
+                        .with("transactionType", "TRANSFER")
+                        .with("assetValue", -64.00)
+                        .with("adoptingMembership.uuid", UNAVAILABLE_MEMBERSHIP_UUID.toString()),
+                "adoptingMembership.uuid='" + UNAVAILABLE_MEMBERSHIP_UUID + "' not found or not accessible"),
+
         ASSETS_VALUE_MUST_NOT_BE_NULL(
                 requestBody -> requestBody
                         .with("transactionType", "REVERSAL")
@@ -115,6 +159,7 @@ class HsOfficeCoopAssetsTransactionControllerRestTest {
     @ParameterizedTest
     @EnumSource(BadRequestTestCases.class)
     void respondWithBadRequest(final BadRequestTestCases testCase) throws Exception {
+//        assumeThat(testCase == ADOPTING_MEMBERSHIP_NUMBER_FOR_TRANSFER_MUST_BE_GIVEN_AND_AVAILABLE).isTrue();
 
         // when
         mockMvc.perform(MockMvcRequestBuilders
@@ -131,4 +176,68 @@ class HsOfficeCoopAssetsTransactionControllerRestTest {
                 .andExpect(status().is4xxClientError());
     }
 
+    enum SuccessfullyCreatedTestCases {
+
+        ADOPTING_MEMBERSHIP_NUMBER_FOR_TRANSFER_MUST_BE_GIVEN_AND_AVAILABLE(
+                requestBody -> requestBody
+                        .with("transactionType", "TRANSFER")
+                        .with("assetValue", -64.00)
+                        .with("adoptingMembership.memberNumber", AVAILABLE_MEMBER_NUMBER),
+                "adoptingMembership.memberNumber='M-1234699' not found or not accessible"),
+
+        ADOPTING_MEMBERSHIP_UUID_FOR_TRANSFER_MUST_BE_GIVEN_AND_AVAILABLE(
+                requestBody -> requestBody
+                        .with("transactionType", "TRANSFER")
+                        .with("assetValue", -64.00)
+                        .with("adoptingMembership.uuid", AVAILABLE_MEMBERSHIP_UUID.toString()),
+                "adoptingMembership.uuid='" + UNAVAILABLE_MEMBERSHIP_UUID + "' not found or not accessible");
+
+        private final Function<JsonBuilder, JsonBuilder> givenBodyTransformation;
+        private final String expectedErrorMessage;
+
+        SuccessfullyCreatedTestCases(
+                final Function<JsonBuilder, JsonBuilder> givenBodyTransformation,
+                final String expectedErrorMessage) {
+            this.givenBodyTransformation = givenBodyTransformation;
+            this.expectedErrorMessage = expectedErrorMessage;
+        }
+
+        String givenRequestBody() {
+            return givenBodyTransformation.apply(jsonObject(VALID_INSERT_REQUEST_BODY)).toString();
+        }
+    }
+
+    @ParameterizedTest
+    @EnumSource(SuccessfullyCreatedTestCases.class)
+    void respondWithSuccessfullyCreated(final SuccessfullyCreatedTestCases testCase) throws Exception {
+
+        // when
+        mockMvc.perform(MockMvcRequestBuilders
+                        .post("/api/hs/office/coopassetstransactions")
+                        .header("current-subject", "superuser-alex@hostsharing.net")
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .content(testCase.givenRequestBody())
+                        .accept(MediaType.APPLICATION_JSON))
+
+                // then
+                //FIXME.andExpect(jsonPath("message", is("ERROR: [400] " + testCase.expectedErrorMessage)))
+                //FIXME.andExpect(jsonPath("statusPhrase", is("Bad Request")))
+                .andExpect(status().is2xxSuccessful());
+    }
+
+    @BeforeEach
+    void initMocks() {
+        final var availableMemberNumber = Integer.valueOf(AVAILABLE_MEMBER_NUMBER.substring("M-".length()));
+        when(membershipRepo.findMembershipByMemberNumber(availableMemberNumber)).thenReturn(AVAILABLE_MEMBER_ENTITY);
+        when(membershipRepo.findByUuid(AVAILABLE_MEMBERSHIP_UUID)).thenReturn(Optional.of(AVAILABLE_MEMBER_ENTITY));
+        when(coopAssetsTransactionRepo.save(any(HsOfficeCoopAssetsTransactionEntity.class)))
+                .thenAnswer(invocation -> {
+                            final var entity = (HsOfficeCoopAssetsTransactionEntity) invocation.getArgument(0);
+                            if (entity.getUuid() == null) {
+                                entity.setUuid(UUID.randomUUID());
+                            }
+                            return entity;
+                        }
+                );
+    }
 }

src/test/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionControllerRestTest.java

@@ -30,6 +30,7 @@ class HsOfficeCoopSharesTransactionControllerRestTest {
     Context contextMock;
 
     @MockBean
+    @SuppressWarnings("unused") // not used in test, but in controller class
     StandardMapper mapper;
 
     @MockBean

src/test/java/net/hostsharing/hsadminng/hs/office/scenarios/HsOfficeScenarioTests.java

@@ -427,6 +427,8 @@ class HsOfficeScenarioTests extends ScenarioTest {
                 .doRun();
     }
 
+    // FIXME: implement revert for an asset TRANSFER tx
+
     @Test
     @Order(4900)
     @Requires("Membership: M-3101000 - Test AG")