1
0

Taiga#457: improve bookingitem resource mapping error message (#218)

Co-authored-by: Michael Hoennig <michael@hoennig.de>
Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/218
Reviewed-by: Marc Sandlus <hsh-marcsandlus@noreply.dev.hostsharing.net>
This commit is contained in:
Michael Hoennig
2026-03-19 09:40:14 +01:00
parent 69e2fc09a6
commit 79d4d8c7f2
13 changed files with 139 additions and 20 deletions
@@ -0,0 +1,63 @@
# PR#218: Bad error message for string instead of JSON
This PR contains an unrelated quickfix, see [Additional Changes](#additional-changes) below.
## The Problem
This request:
```
$ curl --no-progress-meter -X 'POST' \
'https://backend.ngdev.hostsharing.net/api/hs/booking/items' \
-H 'Accept: application/json' \
-H "Authorization: Bearer $BEARER" \
-H "Origin: http://127.0.0.1:3001" \
-H 'Content-Type: application/json' \
-d '{
"project.uuid": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
"parentItem.uuid": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
"type": "PRIVATE_CLOUD",
"caption": "string",
"validTo": "2026-03-18",
"resources": "string",
"hostingAsset": {
"parentAsset.uuid": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
"assignedToAsset.uuid": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
"type": "CLOUD_SERVER",
"identifier": "string",
"caption": "string",
"alarmContact.uuid": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
"config": "string",
"subHostingAssets": [
{
"type": "CLOUD_SERVER",
"identifier": "string",
"caption": "string",
"assignedToAsset.uuid": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
"alarmContact.uuid": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
"config": "string"
}
]
}
}' \
|jq|less
```
responses with this unclear error message:
```json
{
"timestamp": "2026-03-18 03:59:56",
"path": "",
"statusCode": 500,
"statusPhrase": "Internal Server Error",
"message": "ERROR: [500] Map expected, but got: string"
}
```
## The Solution
Fortunately, in this case, it's a union (anyIf) at the API level (OpenAPI specification).
Thus, it cannot be parsed by the Spring Framework, but we parse it ourselves,
and I was able to simply pass the related property name and improve the error message.
+53
View File
@@ -0,0 +1,53 @@
# Pull-Request Documentations in doc/PR
This directory contains documentation for each pull request (PR).
## Naming Convention
1. Date of the PR in the format `YYYY-MM-DD`
2. followed '-PR#' followed by the number of the pull request in GitEA,
3. a short description of the PR with dashes between the words,
4. '.md'
Yes, for this you need to open a pull request,
but initially prefix its title with "WIP: " to mark it as a work in progress until it is ready for review.
## Guidelines
- Documentations must be written in Markdown format.
- Use Englisch, it's a public open source project.
- Use clear and concise language and keep it short.
- Include relevant details and context.
- Do not reference to Taiga or any other tool that is not public.
- If necessary, copy important parts of the ticket description.
- One sentence or statement per line to make diffs easier to read.
## Structure
```Markdown
## The Problem
Here you describe what this PR is supposed to solve.
## The Solution
Here you describe the changes you made and why you made them.
If necessary, you can link to an ADR (Architecture Decision Record).
## Additional Changes
Here you list any additional changes you made, e.g. "fixed formatting in ..." or "fixed some naming issues".
## Attachments
Here you can add any longer sections that would interrupt the reading flow in the previous sections.
Put each attachment on a level-3 heading ('### ...').
```
@@ -174,6 +174,6 @@ public class HsBookingItemController implements HsBookingItemsApi {
.map(parentItemUuid -> em.find(HsBookingItemRealEntity.class, parentItemUuid)) .map(parentItemUuid -> em.find(HsBookingItemRealEntity.class, parentItemUuid))
.ifPresent(entity::setParentItem); .ifPresent(entity::setParentItem);
entity.setValidity(toPostgresDateRange(LocalDate.now(), resource.getValidTo())); entity.setValidity(toPostgresDateRange(LocalDate.now(), resource.getValidTo()));
entity.putResources(KeyValueMap.from(resource.getResources())); entity.putResources(KeyValueMap.from("resources", resource.getResources()));
}; };
} }
@@ -21,7 +21,7 @@ public class HsBookingItemEntityPatcher implements EntityPatcher<HsBookingItemPa
OptionalFromJson.of(resource.getCaption()) OptionalFromJson.of(resource.getCaption())
.ifPresent(entity::setCaption); .ifPresent(entity::setCaption);
Optional.ofNullable(resource.getResources()) Optional.ofNullable(resource.getResources())
.ifPresent(r -> entity.getResources().patch(KeyValueMap.from(resource.getResources()))); .ifPresent(r -> entity.getResources().patch(KeyValueMap.from("resources", resource.getResources())));
OptionalFromJson.of(resource.getValidTo()) OptionalFromJson.of(resource.getValidTo())
.ifPresent(entity::setValidTo); .ifPresent(entity::setValidTo);
} }
@@ -155,7 +155,7 @@ public class HsHostingAssetController implements HsHostingAssetsApi {
} }
final BiConsumer<HsHostingAssetInsertResource, HsHostingAssetRbacEntity> RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> { final BiConsumer<HsHostingAssetInsertResource, HsHostingAssetRbacEntity> RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> {
entity.putConfig(KeyValueMap.from(resource.getConfig())); entity.putConfig(KeyValueMap.from("config", resource.getConfig()));
if (resource.getBookingItemUuid() != null) { if (resource.getBookingItemUuid() != null) {
entity.setBookingItem(realBookingItemRepo.findByUuid(resource.getBookingItemUuid()) entity.setBookingItem(realBookingItemRepo.findByUuid(resource.getBookingItemUuid())
.orElseThrow(() -> new EntityNotFoundException("ERROR: [400] bookingItemUuid %s not found".formatted( .orElseThrow(() -> new EntityNotFoundException("ERROR: [400] bookingItemUuid %s not found".formatted(
@@ -24,7 +24,7 @@ public class HsHostingAssetEntityPatcher implements EntityPatcher<HsHostingAsset
OptionalFromJson.of(resource.getCaption()) OptionalFromJson.of(resource.getCaption())
.ifPresent(entity::setCaption); .ifPresent(entity::setCaption);
Optional.ofNullable(resource.getConfig()) Optional.ofNullable(resource.getConfig())
.ifPresent(r -> entity.getConfig().patch(KeyValueMap.from(resource.getConfig()))); .ifPresent(r -> entity.getConfig().patch(KeyValueMap.from("config", resource.getConfig())));
OptionalFromJson.of(resource.getAlarmContactUuid()) OptionalFromJson.of(resource.getAlarmContactUuid())
// HOWTO: patch nullable JSON resource uuid to an ntity reference // HOWTO: patch nullable JSON resource uuid to an ntity reference
.ifPresent(newValue -> entity.setAlarmContact( .ifPresent(newValue -> entity.setAlarmContact(
@@ -19,10 +19,10 @@ class HsOfficeContactEntityPatcher implements EntityPatcher<HsOfficeContactPatch
public void apply(final HsOfficeContactPatchResource resource) { public void apply(final HsOfficeContactPatchResource resource) {
OptionalFromJson.of(resource.getCaption()).ifPresent(entity::setCaption); OptionalFromJson.of(resource.getCaption()).ifPresent(entity::setCaption);
Optional.ofNullable(resource.getPostalAddress()) Optional.ofNullable(resource.getPostalAddress())
.ifPresent(r -> entity.getPostalAddress().patch(KeyValueMap.from(resource.getPostalAddress()))); .ifPresent(r -> entity.getPostalAddress().patch(KeyValueMap.from("postalAddress", resource.getPostalAddress())));
Optional.ofNullable(resource.getEmailAddresses()) Optional.ofNullable(resource.getEmailAddresses())
.ifPresent(r -> entity.getEmailAddresses().patch(KeyValueMap.from(resource.getEmailAddresses()))); .ifPresent(r -> entity.getEmailAddresses().patch(KeyValueMap.from("emailAddresses", resource.getEmailAddresses())));
Optional.ofNullable(resource.getPhoneNumbers()) Optional.ofNullable(resource.getPhoneNumbers())
.ifPresent(r -> entity.getPhoneNumbers().patch(KeyValueMap.from(resource.getPhoneNumbers()))); .ifPresent(r -> entity.getPhoneNumbers().patch(KeyValueMap.from("phoneNumbers", resource.getPhoneNumbers())));
} }
} }
@@ -19,9 +19,9 @@ public class HsOfficeContactFromResourceConverter<E extends HsOfficeContact>
final var resource = context.getSource(); final var resource = context.getSource();
final var entity = context.getDestinationType().getDeclaredConstructor().newInstance(); final var entity = context.getDestinationType().getDeclaredConstructor().newInstance();
entity.setCaption(resource.getCaption()); entity.setCaption(resource.getCaption());
entity.putPostalAddress(from(resource.getPostalAddress())); entity.putPostalAddress(from("postalAddress", resource.getPostalAddress()));
entity.putEmailAddresses(from(resource.getEmailAddresses())); entity.putEmailAddresses(from("emailAddresses", resource.getEmailAddresses()));
entity.putPhoneNumbers(from(resource.getPhoneNumbers())); entity.putPhoneNumbers(from("phoneNumbers", resource.getPhoneNumbers()));
return entity; return entity;
} }
} }
@@ -184,8 +184,8 @@ public class HsOfficeRelationController implements HsOfficeRelationsApi {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
final BiConsumer<HsOfficeContactInsertResource, HsOfficeContactRealEntity> CONTACT_RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> { final BiConsumer<HsOfficeContactInsertResource, HsOfficeContactRealEntity> CONTACT_RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> {
entity.putPostalAddress(from(resource.getPostalAddress())); entity.putPostalAddress(from("postalAddress", resource.getPostalAddress()));
entity.putEmailAddresses(from(resource.getEmailAddresses())); entity.putEmailAddresses(from("emailAddresses", resource.getEmailAddresses()));
entity.putPhoneNumbers(from(resource.getPhoneNumbers())); entity.putPhoneNumbers(from("phoneNumbers", resource.getPhoneNumbers()));
}; };
} }
@@ -1,14 +1,15 @@
package net.hostsharing.hsadminng.mapper; package net.hostsharing.hsadminng.mapper;
import jakarta.validation.ValidationException;
import java.util.Map; import java.util.Map;
public class KeyValueMap { public class KeyValueMap {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
public static <T> Map<String, T> from(final Object obj) { public static <T> Map<String, T> from(final String key, final Object obj) {
if (obj == null || obj instanceof Map<?, ?>) { if (obj == null || obj instanceof Map<?, ?>) {
return (Map<String, T>) obj; return (Map<String, T>) obj;
} }
throw new ClassCastException("Map expected, but got: " + obj); throw new ValidationException(key + ": Map expected, but got: " + obj);
} }
} }
@@ -71,7 +71,7 @@ class HsBookingItemEntityPatcherUnitTest extends PatchUnitTestBase<
final var entity = new HsBookingItemRbacEntity(); final var entity = new HsBookingItemRbacEntity();
entity.setUuid(INITIAL_BOOKING_ITEM_UUID); entity.setUuid(INITIAL_BOOKING_ITEM_UUID);
entity.setProject(PROJECT_TEST_ENTITY); entity.setProject(PROJECT_TEST_ENTITY);
entity.getResources().putAll(KeyValueMap.from(INITIAL_RESOURCES)); entity.getResources().putAll(KeyValueMap.from("resources", INITIAL_RESOURCES));
entity.setCaption(INITIAL_CAPTION); entity.setCaption(INITIAL_CAPTION);
entity.setValidity(Range.closedInfinite(GIVEN_VALID_FROM)); entity.setValidity(Range.closedInfinite(GIVEN_VALID_FROM));
return entity; return entity;
@@ -71,7 +71,7 @@ class HsHostingAssetEntityPatcherUnitTest extends PatchUnitTestBase<
final var entity = new HsHostingAssetRbacEntity(); final var entity = new HsHostingAssetRbacEntity();
entity.setUuid(INITIAL_BOOKING_ITEM_UUID); entity.setUuid(INITIAL_BOOKING_ITEM_UUID);
entity.setBookingItem(CLOUD_SERVER_BOOKING_ITEM_REAL_ENTITY); entity.setBookingItem(CLOUD_SERVER_BOOKING_ITEM_REAL_ENTITY);
entity.getConfig().putAll(KeyValueMap.from(INITIAL_CONFIG)); entity.getConfig().putAll(KeyValueMap.from("config", INITIAL_CONFIG));
entity.setCaption(INITIAL_CAPTION); entity.setCaption(INITIAL_CAPTION);
entity.setAlarmContact(givenInitialContact); entity.setAlarmContact(givenInitialContact);
return entity; return entity;
@@ -2,6 +2,7 @@ package net.hostsharing.hsadminng.mapper;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import jakarta.validation.ValidationException;
import java.util.Map; import java.util.Map;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
@@ -13,7 +14,7 @@ class KeyValueMapUnitTest {
@Test @Test
void fromMap() { void fromMap() {
final var result = KeyValueMap.from(Map.ofEntries( final var result = KeyValueMap.from("propName", Map.ofEntries(
Map.entry("one", 1), Map.entry("one", 1),
Map.entry("two", 2) Map.entry("two", 2)
)); ));
@@ -24,9 +25,10 @@ class KeyValueMapUnitTest {
@Test @Test
void fromNonMap() { void fromNonMap() {
final var exception = catchThrowable( () -> final var exception = catchThrowable( () ->
KeyValueMap.from("not a map") KeyValueMap.from("propMap", "not a map")
); );
assertThat(exception).isInstanceOf(ClassCastException.class); assertThat(exception).isInstanceOf(ValidationException.class);
assertThat(exception.getMessage()).isEqualTo("propMap: Map expected, but got: not a map");
} }
} }