hostsharing/hs.hsadmin.ng
1
0
Fork 0
Code Activity

produce client-error for unspecified-properties (#166)

Browse Source 
Co-authored-by: Michael Hoennig <michael@hoennig.de>
Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/166
Reviewed-by: Marc Sandlus <marc.sandlus@hostsharing.net>
This commit is contained in:
Michael Hoennig
2025-03-20 12:04:57 +01:00
parent 4994bac101
commit e6b32eda88
21 changed files with 148 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
src/main/java/net/hostsharing/hsadminng/config/JsonObjectMapperConfiguration.java
View File

@@ -27,7 +27,8 @@ public class JsonObjectMapperConfiguration {
.modules(new JsonNullableModule(), new JavaTimeModule())
.featuresToEnable(
JsonParser.Feature.ALLOW_COMMENTS,
DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS
DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS,
DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES
)
.featuresToDisable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
}

6
src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntityPatcher.java
View File

@@ -25,6 +25,12 @@ class HsOfficePartnerEntityPatcher implements EntityPatcher<HsOfficePartnerPatch
@Override
public void apply(final HsOfficePartnerPatchResource resource) {
// HOWTO: allow properties from the GET request to be passed to PATCH, but only if unchanged.
// These properties have to be specified in the OpenAPI PATCH resource specification,
// ídeally with a comment in the description field, that the value mist be unchanged, if given at all.
ignoreUnchangedPropertyValue("uuid", resource.getUuid(), entity.getUuid());
ignoreUnchangedPropertyValue("partnerNumber", resource.getPartnerNumber(), entity.getPartnerNumber().toString());
if (resource.getPartnerRel() != null) {
new HsOfficeRelationPatcher(mapper, em, entity.getPartnerRel()).apply(resource.getPartnerRel());
}

13
src/main/java/net/hostsharing/hsadminng/mapper/EntityPatcher.java
View File

@@ -1,6 +1,19 @@
package net.hostsharing.hsadminng.mapper;
import org.openapitools.jackson.nullable.JsonNullable;
import jakarta.validation.ValidationException;
import java.util.Optional;
public interface EntityPatcher<R> {
void apply(R resource);
default <T> void ignoreUnchangedPropertyValue(final String propertyName, final JsonNullable<T> resourcePropertyValue, final T entityPropertyValue) {
Optional.ofNullable(resourcePropertyValue).ifPresent(value -> {
if (!value.get().equals(entityPropertyValue) ) {
throw new ValidationException(propertyName + " cannot be changed, either leave empty or leave unchanged as " + entityPropertyValue);
}
});
}
}

1
src/main/resources/api-definition/hs-booking/hs-booking-item-schemas.yaml
View File

@@ -48,6 +48,7 @@ components:
nullable: true
resources:
$ref: '#/components/schemas/BookingResources'
additionalProperties: false
HsBookingItemInsert:
type: object

1
src/main/resources/api-definition/hs-booking/hs-booking-project-schemas.yaml
View File

@@ -21,6 +21,7 @@ components:
caption:
type: string
nullable: true
additionalProperties: false
HsBookingProjectInsert:
type: object

1
src/main/resources/api-definition/hs-hosting/hs-hosting-asset-schemas.yaml
View File

@@ -60,6 +60,7 @@ components:
nullable: true
config:
$ref: '#/components/schemas/HsHostingAssetConfiguration'
additionalProperties: false
HsHostingAssetInsert:
type: object

1
src/main/resources/api-definition/hs-office/hs-office-bankaccount-schemas.yaml
View File

@@ -29,3 +29,4 @@ components:
- holder
- iban
- bic
additionalProperties: false

2
src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml
View File

@@ -31,6 +31,7 @@ components:
$ref: '#/components/schemas/HsOfficeContactPhoneNumbers'
required:
- caption
additionalProperties: false
HsOfficeContactPatch:
type: object
@@ -44,6 +45,7 @@ components:
$ref: '#/components/schemas/HsOfficeContactEmailAddresses'
phoneNumbers:
$ref: '#/components/schemas/HsOfficeContactPhoneNumbers'
additionalProperties: false
HsOfficeContactPostalAddress:
# forces generating a java.lang.Object containing a Map, instead of a class with fixed properties

2
src/main/resources/api-definition/hs-office/hs-office-debitor-schemas.yaml
View File

@@ -67,6 +67,7 @@ components:
type: string
pattern: '^[a-z0-9]{3}$'
nullable: true
additionalProperties: false
HsOfficeDebitorInsert:
type: object
@@ -101,3 +102,4 @@ components:
- debitorNumberSuffix
- defaultPrefix
- billable
additionalProperties: false

14
src/main/resources/api-definition/hs-office/hs-office-partner-schemas.yaml
View File

@@ -48,6 +48,16 @@ components:
HsOfficePartnerPatch:
type: object
properties:
uuid:
type: string
format: uuid
nullable: true
description: if given (e.g. taken from a GET request), it must be identical to the patched entities uuid
partnerNumber:
type: string
pattern: 'P-[0-9]{5}'
nullable: true
description: if given (e.g. taken from a GET request), it must be identical to the patched entities partnerNumber
partnerRel:
$ref: 'hs-office-relation-schemas.yaml#/components/schemas/HsOfficeRelationPatch'
details:
@@ -78,6 +88,7 @@ components:
type: string
format: date
nullable: true
additionalProperties: false
HsOfficePartnerInsert:
type: object
@@ -93,6 +104,7 @@ components:
- partnerNumber
- partnerRel
- details
additionalProperties: false
HsOfficePartnerRelInsert:
type: object
@@ -111,6 +123,7 @@ components:
- anchor.uuid
- holder.uuid
- relContact.uuid
additionalProperties: false
HsOfficePartnerDetailsInsert:
type: object
@@ -136,3 +149,4 @@ components:
type: string
format: date
nullable: true
additionalProperties: false

3
src/main/resources/api-definition/hs-office/hs-office-person-schemas.yaml
View File

@@ -50,12 +50,12 @@ components:
type: string
required:
- personType
additionalProperties: false
HsOfficePersonPatch:
type: object
properties:
personType:
nullable: true
$ref: '#/components/schemas/HsOfficePersonType'
tradeName:
type: string
@@ -72,3 +72,4 @@ components:
familyName:
type: string
nullable: true
additionalProperties: false

3
src/main/resources/api-definition/hs-office/hs-office-relation-schemas.yaml
View File

@@ -41,6 +41,7 @@ components:
type: string
format: uuid
nullable: true
additionalProperties: false
HsOfficeRelationPatch:
type: object
@@ -95,6 +96,7 @@ components:
description:
Additionally to `type` and `anchor.uuid`, either `anchor.uuid` or `anchor`
and either `contact` or `contact.uuid` need to be given.
additionalProperties: false
# relation created as a sub-element with implicitly known type
HsOfficeRelationSubInsert:
@@ -116,3 +118,4 @@ components:
- anchor.uuid
- holder.uuid
- contact.uuid
additionalProperties: false

1
src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemControllerAcceptanceTest.java
View File

@@ -555,7 +555,6 @@ class HsBookingItemControllerAcceptanceTest extends ContextBasedTestWithCleanup
.contentType(ContentType.JSON)
.body("""
{
"validFrom": "2020-06-05",
"validTo": "2022-12-31",
"resources": {
"Traffic": 500,

25
src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemControllerRestTest.java
View File

@@ -2,6 +2,7 @@ package net.hostsharing.hsadminng.hs.booking.item;
import net.hostsharing.hsadminng.config.JsonObjectMapperConfiguration;
import net.hostsharing.hsadminng.context.Context;
import net.hostsharing.hsadminng.hs.booking.generated.api.v1.model.HsBookingItemInsertResource;
import net.hostsharing.hsadminng.hs.booking.project.HsBookingProjectRealEntity;
import net.hostsharing.hsadminng.hs.booking.project.HsBookingProjectRealRepository;
import net.hostsharing.hsadminng.mapper.StrictMapper;
@@ -35,6 +36,7 @@ import static org.hamcrest.Matchers.matchesRegex;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.*;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@@ -112,7 +114,6 @@ class HsBookingItemControllerRestTest {
"type": "MANAGED_SERVER",
"caption": "some new booking",
"validTo": "{validTo}",
"garbage": "should not be accepted",
"resources": { "CPU": 12, "RAM": 4, "SSD": 100, "Traffic": 250 }
}
"""
@@ -120,6 +121,7 @@ class HsBookingItemControllerRestTest {
.replace("{validTo}", LocalDate.now().plusMonths(1).toString())
)
.accept(MediaType.APPLICATION_JSON))
.andDo(print())
// then
.andExpect(status().isCreated())
@@ -161,7 +163,7 @@ class HsBookingItemControllerRestTest {
"project.uuid": "{projectUuid}",
"type": "MANAGED_SERVER",
"caption": "some new booking",
"validFrom": "{validFrom}",
"validFrom": "{validFrom}", // not specified => not accepted
"resources": { "CPU": 12, "RAM": 4, "SSD": 100, "Traffic": 250 }
}
"""
@@ -169,24 +171,15 @@ class HsBookingItemControllerRestTest {
.replace("{validFrom}", LocalDate.now().plusMonths(1).toString())
)
.accept(MediaType.APPLICATION_JSON))
.andDo(print())
// then
// TODO.test: MockMvc does not seem to validate additionalProperties=false
// .andExpect(status().is4xxClientError())
.andExpect(status().isCreated())
.andExpect(status().is4xxClientError())
.andExpect(jsonPath("$", lenientlyEquals("""
{
"type": "MANAGED_SERVER",
"caption": "some new booking",
"validFrom": "{today}",
"validTo": null,
"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/[^/]*")));
"message": "ERROR: [400] JSON parse error: Unrecognized field \\"validFrom\\" (class ${resourceClass}), not marked as ignorable"
}
""".replace("${resourceClass}", HsBookingItemInsertResource.class.getName()))));
}
}
}

6
src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetControllerRestTest.java
View File

@@ -50,6 +50,7 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.*;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@@ -667,10 +668,8 @@ public class HsHostingAssetControllerRestTest {
.contentType(MediaType.APPLICATION_JSON)
.content("""
{
"type": "DOMAIN_HTTP_SETUP",
"identifier": "updated example.org|HTTP",
"caption": "some updated fake Domain-HTTP-Setup",
"alarmContact": null,
"alarmContact.uuid": null,
"config": {
"autoconfig": true,
"multiviews": true,
@@ -682,6 +681,7 @@ public class HsHostingAssetControllerRestTest {
}
""")
.accept(MediaType.APPLICATION_JSON))
.andDo(print())
// then
.andExpect(status().is2xxSuccessful())

3
src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java
View File

@@ -627,13 +627,12 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu
.contentType(ContentType.JSON)
.body("""
{
"contactUuid": "%s",
"vatId": "VAT222222",
"vatCountryCode": "AA",
"vatBusiness": true,
"defaultPrefix": "for"
}
""".formatted(givenContact.getUuid()))
""")
.port(port)
.when()
.patch("http://localhost/api/hs/office/debitors/" + givenDebitor.getUuid())

7
src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java
View File

@@ -157,7 +157,7 @@ class HsOfficePartnerControllerAcceptanceTest extends ContextBasedTestWithCleanu
final var givenMandantPerson = personRealRepo.findPersonByOptionalNameLike("Hostsharing eG").get(0);
final var givenPerson = personRealRepo.findPersonByOptionalNameLike("Third").get(0);
final var location = RestAssured // @formatter:off
RestAssured // @formatter:off
.given()
.header("Authorization", "Bearer superuser-alex@hostsharing.net")
.contentType(ContentType.JSON)
@@ -169,8 +169,6 @@ class HsOfficePartnerControllerAcceptanceTest extends ContextBasedTestWithCleanu
"holder.uuid": "%s",
"contact.uuid": "%s"
},
"person.uuid": "%s",
"contact.uuid": "%s",
"details": {}
}
""".formatted(
@@ -207,8 +205,6 @@ class HsOfficePartnerControllerAcceptanceTest extends ContextBasedTestWithCleanu
"holder.uuid": "%s",
"contact.uuid": "%s"
},
"person.uuid": "%s",
"contact.uuid": "%s",
"details": {}
}
""".formatted(
@@ -324,7 +320,6 @@ class HsOfficePartnerControllerAcceptanceTest extends ContextBasedTestWithCleanu
.contentType(ContentType.JSON)
.body("""
{
"partnerNumber": "P-20011",
"partnerRel": {
"holder.uuid": "%s"
},

94
src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntityPatcherUnitTest.java
View File

@@ -10,28 +10,28 @@ import net.hostsharing.hsadminng.mapper.StrictMapper;
import net.hostsharing.hsadminng.persistence.EntityManagerWrapper;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.openapitools.jackson.nullable.JsonNullable;
import jakarta.validation.ValidationException;
import java.time.LocalDate;
import java.util.UUID;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
import static org.assertj.core.api.Assertions.catchThrowable;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.lenient;
@TestInstance(PER_CLASS)
@ExtendWith(MockitoExtension.class)
// This test class does not subclass PatchUnitTestBase because it has no directly patchable properties.
// But the factory-structure is kept, so PatchUnitTestBase could easily be plugged back in if needed.
class HsOfficePartnerEntityPatcherUnitTest {
private static final UUID INITIAL_PARTNER_UUID = UUID.randomUUID();
private static final int INITIAL_PARTNER_NUMBER = 12345;
private static final UUID INITIAL_CONTACT_UUID = UUID.randomUUID();
private static final UUID INITIAL_PARTNER_PERSON_UUID = UUID.randomUUID();
private static final UUID INITIAL_DETAILS_UUID = UUID.randomUUID();
@@ -50,7 +50,9 @@ class HsOfficePartnerEntityPatcherUnitTest {
@Mock
private EntityManagerWrapper emw;
private StrictMapper mapper = new StrictMapper(emw);
private final StrictMapper mapper = new StrictMapper(emw);
private final HsOfficePartnerPatchResource patchResource = newPatchResource();
private final HsOfficePartnerRbacEntity entity = newInitialEntity();
@BeforeEach
void initMocks() {
@@ -60,14 +62,88 @@ class HsOfficePartnerEntityPatcherUnitTest {
HsOfficeContactRealEntity.builder().uuid(invocation.getArgument(1)).build());
}
@Test
void ignorePartnerUuidIfNotGiven() {
// given
patchResource.setUuid(null);
// when
createPatcher(entity).apply(patchResource);
// then
assertThat(entity.getUuid()).isEqualTo(INITIAL_PARTNER_UUID);
}
@Test
void ignoreUnchangedPartnerUuid() {
// given
patchResource.setUuid(JsonNullable.of(INITIAL_PARTNER_UUID));
// when
createPatcher(entity).apply(patchResource);
// then
assertThat(entity.getUuid()).isEqualTo(INITIAL_PARTNER_UUID);
}
@Test
void rejectChangingThePartnerUuid() {
// given
patchResource.setUuid(JsonNullable.of(UUID.randomUUID()));
// when
final var exception = catchThrowable(() -> createPatcher(entity).apply(patchResource));
// then
assertThat(exception).isInstanceOf(ValidationException.class).hasMessageContaining(
"uuid cannot be changed, either leave empty or leave unchanged as " + INITIAL_PARTNER_UUID
);
}
@Test
void ignorePartnerNumberIfNotGiven() {
// given
patchResource.setPartnerNumber(null);
// when
createPatcher(entity).apply(patchResource);
// then
assertThat(entity.getPartnerNumber()).isEqualTo(INITIAL_PARTNER_NUMBER);
}
@Test
void ignoreUnchangedPartnerNumber() {
// given
patchResource.setPartnerNumber(JsonNullable.of(String.valueOf(INITIAL_PARTNER_NUMBER)));
// when
createPatcher(entity).apply(patchResource);
// then
assertThat(entity.getPartnerNumber()).isEqualTo(INITIAL_PARTNER_NUMBER);
}
@Test
void rejectChangingThePartnerNumber() {
// given
patchResource.setPartnerNumber(JsonNullable.of("99999"));
// when
final var exception = catchThrowable(() -> createPatcher(entity).apply(patchResource));
// then
assertThat(exception).isInstanceOf(ValidationException.class).hasMessageContaining(
"partnerNumber cannot be changed, either leave empty or leave unchanged as " + INITIAL_PARTNER_NUMBER
);
}
@Test
void patchPartnerPerson() {
// given
final var patchResource = newPatchResource();
final var newHolderUuid = UUID.randomUUID();
patchResource.setPartnerRel(new HsOfficeRelationPatchResource());
patchResource.getPartnerRel().setHolderUuid(JsonNullable.of(newHolderUuid));
final var entity = newInitialEntity();
// when
createPatcher(entity).apply(patchResource);
@@ -79,11 +155,9 @@ class HsOfficePartnerEntityPatcherUnitTest {
@Test
void patchPartnerContact() {
// given
final var patchResource = newPatchResource();
final var newContactUuid = UUID.randomUUID();
patchResource.setPartnerRel(new HsOfficeRelationPatchResource());
patchResource.getPartnerRel().setContactUuid(JsonNullable.of(newContactUuid));
final var entity = newInitialEntity();
// when
createPatcher(entity).apply(patchResource);
@@ -95,11 +169,9 @@ class HsOfficePartnerEntityPatcherUnitTest {
@Test
void patchPartnerDetails() {
// given
final var patchResource = newPatchResource();
final var newDateOfBirth = LocalDate.now();
patchResource.setDetails(new HsOfficePartnerDetailsPatchResource());
patchResource.getDetails().setDateOfDeath(JsonNullable.of(newDateOfBirth));
final var entity = newInitialEntity();
// when
createPatcher(entity).apply(patchResource);
@@ -111,7 +183,7 @@ class HsOfficePartnerEntityPatcherUnitTest {
protected HsOfficePartnerRbacEntity newInitialEntity() {
final var entity = HsOfficePartnerRbacEntity.builder()
.uuid(INITIAL_PARTNER_UUID)
.partnerNumber(12345)
.partnerNumber(INITIAL_PARTNER_NUMBER)
.partnerRel(HsOfficeRelationRealEntity.builder()
.holder(givenInitialPartnerPerson)
.contact(givenInitialContact)

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

@@ -398,7 +398,7 @@ class HsOfficeScenarioTests extends ScenarioTest {
void shouldInvalidateSepaMandateForDebitor() {
new InvalidateSepaMandateForDebitor(scenarioTest)
.given("bankAccountIBAN", "DE02701500000000594937")
.given("mandateValidUntil", "2025-09-30")
.given("mandateValidTo", "2025-09-30")
.doRun();
}

2
src/test/java/net/hostsharing/hsadminng/hs/office/scenarios/debitor/InvalidateSepaMandateForDebitor.java
View File

@@ -25,7 +25,7 @@ public class InvalidateSepaMandateForDebitor extends UseCase<InvalidateSepaManda
return withTitle("Patch the End of the Mandate into the SEPA-Mandate", () ->
httpPatch("/api/hs/office/sepamandates/&{SEPA-Mandate: %{bankAccountIBAN}}", usingJsonBody("""
{
"validUntil": ${mandateValidUntil}
"validTo": ${mandateValidTo}
}
"""))
.expecting(OK).expecting(JSON)

5
src/test/java/net/hostsharing/hsadminng/hs/office/scenarios/partner/ReplaceDeceasedPartnerWithCommunityOfHeirs.java
View File

@@ -34,18 +34,15 @@ public class ReplaceDeceasedPartnerWithCommunityOfHeirs extends UseCase<ReplaceD
() -> httpPatch("/api/hs/office/partners/%{Partner: %{partnerNumber}}",
usingJsonBody("""
{
"wrong1": false,
"partnerRel": {
"wrong2": false,
"holder": {
"personType": "UNINCORPORATED_FIRM",
"tradeName": "Erbengemeinschaft %{givenNameOfDeceasedPerson} %{familyNameOfDeceasedPerson}",
},
"contact": {
"wrong3": false,
"caption": "Erbengemeinschaft %{givenNameOfDeceasedPerson} %{familyNameOfDeceasedPerson}",
"postalAddress": {
"wrong4": false,
"whatever": "any key is allowed here",
"name": "Erbengemeinschaft %{givenNameOfDeceasedPerson} %{familyNameOfDeceasedPerson}",
"co": "%{representativeGivenName} %{representativeFamilyName}",
%{communityOfHeirsPostalAddress}