From 79d4d8c7f27e762c07252c7483ad780e845d04a5 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Thu, 19 Mar 2026 09:40:14 +0100 Subject: [PATCH] Taiga#457: improve bookingitem resource mapping error message (#218) Co-authored-by: Michael Hoennig Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/218 Reviewed-by: Marc Sandlus --- ...rror-message-for-string-instead-of-json.md | 63 +++++++++++++++++++ doc/PR/README.md | 53 ++++++++++++++++ .../booking/item/HsBookingItemController.java | 2 +- .../item/HsBookingItemEntityPatcher.java | 2 +- .../asset/HsHostingAssetController.java | 2 +- .../asset/HsHostingAssetEntityPatcher.java | 2 +- .../contact/HsOfficeContactEntityPatcher.java | 6 +- .../HsOfficeContactFromResourceConverter.java | 6 +- .../relation/HsOfficeRelationController.java | 6 +- .../hsadminng/mapper/KeyValueMap.java | 5 +- .../HsBookingItemEntityPatcherUnitTest.java | 2 +- .../HsHostingAssetEntityPatcherUnitTest.java | 2 +- .../hsadminng/mapper/KeyValueMapUnitTest.java | 8 ++- 13 files changed, 139 insertions(+), 20 deletions(-) create mode 100644 doc/PR/2026-03-18-PR#218-HOSTING-fix-bad-error-message-for-string-instead-of-json.md create mode 100644 doc/PR/README.md diff --git a/doc/PR/2026-03-18-PR#218-HOSTING-fix-bad-error-message-for-string-instead-of-json.md b/doc/PR/2026-03-18-PR#218-HOSTING-fix-bad-error-message-for-string-instead-of-json.md new file mode 100644 index 00000000..e0de2282 --- /dev/null +++ b/doc/PR/2026-03-18-PR#218-HOSTING-fix-bad-error-message-for-string-instead-of-json.md @@ -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. diff --git a/doc/PR/README.md b/doc/PR/README.md new file mode 100644 index 00000000..a27d65ae --- /dev/null +++ b/doc/PR/README.md @@ -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 ('### ...'). + + + + +``` + + 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 26f012f3..f0b04d16 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 @@ -174,6 +174,6 @@ public class HsBookingItemController implements HsBookingItemsApi { .map(parentItemUuid -> em.find(HsBookingItemRealEntity.class, parentItemUuid)) .ifPresent(entity::setParentItem); entity.setValidity(toPostgresDateRange(LocalDate.now(), resource.getValidTo())); - entity.putResources(KeyValueMap.from(resource.getResources())); + entity.putResources(KeyValueMap.from("resources", resource.getResources())); }; } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntityPatcher.java b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntityPatcher.java index 13d11466..8da3f412 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntityPatcher.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntityPatcher.java @@ -21,7 +21,7 @@ public class HsBookingItemEntityPatcher implements EntityPatcher entity.getResources().patch(KeyValueMap.from(resource.getResources()))); + .ifPresent(r -> entity.getResources().patch(KeyValueMap.from("resources", resource.getResources()))); OptionalFromJson.of(resource.getValidTo()) .ifPresent(entity::setValidTo); } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetController.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetController.java index 3d4a5ec6..f58f65ac 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetController.java @@ -155,7 +155,7 @@ public class HsHostingAssetController implements HsHostingAssetsApi { } final BiConsumer RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> { - entity.putConfig(KeyValueMap.from(resource.getConfig())); + entity.putConfig(KeyValueMap.from("config", resource.getConfig())); if (resource.getBookingItemUuid() != null) { entity.setBookingItem(realBookingItemRepo.findByUuid(resource.getBookingItemUuid()) .orElseThrow(() -> new EntityNotFoundException("ERROR: [400] bookingItemUuid %s not found".formatted( diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcher.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcher.java index 5296a955..f615d661 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcher.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcher.java @@ -24,7 +24,7 @@ public class HsHostingAssetEntityPatcher implements EntityPatcher entity.getConfig().patch(KeyValueMap.from(resource.getConfig()))); + .ifPresent(r -> entity.getConfig().patch(KeyValueMap.from("config", resource.getConfig()))); OptionalFromJson.of(resource.getAlarmContactUuid()) // HOWTO: patch nullable JSON resource uuid to an ntity reference .ifPresent(newValue -> entity.setAlarmContact( diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java index 356aa950..2136fd0a 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcher.java @@ -19,10 +19,10 @@ class HsOfficeContactEntityPatcher implements EntityPatcher entity.getPostalAddress().patch(KeyValueMap.from(resource.getPostalAddress()))); + .ifPresent(r -> entity.getPostalAddress().patch(KeyValueMap.from("postalAddress", resource.getPostalAddress()))); 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()) - .ifPresent(r -> entity.getPhoneNumbers().patch(KeyValueMap.from(resource.getPhoneNumbers()))); + .ifPresent(r -> entity.getPhoneNumbers().patch(KeyValueMap.from("phoneNumbers", resource.getPhoneNumbers()))); } } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactFromResourceConverter.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactFromResourceConverter.java index 8ebcbc88..8c2cf4f7 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactFromResourceConverter.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactFromResourceConverter.java @@ -19,9 +19,9 @@ public class HsOfficeContactFromResourceConverter final var resource = context.getSource(); final var entity = context.getDestinationType().getDeclaredConstructor().newInstance(); entity.setCaption(resource.getCaption()); - entity.putPostalAddress(from(resource.getPostalAddress())); - entity.putEmailAddresses(from(resource.getEmailAddresses())); - entity.putPhoneNumbers(from(resource.getPhoneNumbers())); + entity.putPostalAddress(from("postalAddress", resource.getPostalAddress())); + entity.putEmailAddresses(from("emailAddresses", resource.getEmailAddresses())); + entity.putPhoneNumbers(from("phoneNumbers", resource.getPhoneNumbers())); return entity; } } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationController.java index 6e552f3a..b700051a 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationController.java @@ -184,8 +184,8 @@ public class HsOfficeRelationController implements HsOfficeRelationsApi { @SuppressWarnings("unchecked") final BiConsumer CONTACT_RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> { - entity.putPostalAddress(from(resource.getPostalAddress())); - entity.putEmailAddresses(from(resource.getEmailAddresses())); - entity.putPhoneNumbers(from(resource.getPhoneNumbers())); + entity.putPostalAddress(from("postalAddress", resource.getPostalAddress())); + entity.putEmailAddresses(from("emailAddresses", resource.getEmailAddresses())); + entity.putPhoneNumbers(from("phoneNumbers", resource.getPhoneNumbers())); }; } diff --git a/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java b/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java index 7fded816..bc0fa588 100644 --- a/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java +++ b/src/main/java/net/hostsharing/hsadminng/mapper/KeyValueMap.java @@ -1,14 +1,15 @@ package net.hostsharing.hsadminng.mapper; +import jakarta.validation.ValidationException; import java.util.Map; public class KeyValueMap { @SuppressWarnings("unchecked") - public static Map from(final Object obj) { + public static Map from(final String key, final Object obj) { if (obj == null || obj instanceof Map) { return (Map) obj; } - throw new ClassCastException("Map expected, but got: " + obj); + throw new ValidationException(key + ": Map expected, but got: " + obj); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntityPatcherUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntityPatcherUnitTest.java index 2113166c..2e4c2aa7 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntityPatcherUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntityPatcherUnitTest.java @@ -71,7 +71,7 @@ class HsBookingItemEntityPatcherUnitTest extends PatchUnitTestBase< final var entity = new HsBookingItemRbacEntity(); entity.setUuid(INITIAL_BOOKING_ITEM_UUID); 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.setValidity(Range.closedInfinite(GIVEN_VALID_FROM)); return entity; diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcherUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcherUnitTest.java index 51020b16..5eb86712 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcherUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcherUnitTest.java @@ -71,7 +71,7 @@ class HsHostingAssetEntityPatcherUnitTest extends PatchUnitTestBase< final var entity = new HsHostingAssetRbacEntity(); entity.setUuid(INITIAL_BOOKING_ITEM_UUID); 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.setAlarmContact(givenInitialContact); return entity; diff --git a/src/test/java/net/hostsharing/hsadminng/mapper/KeyValueMapUnitTest.java b/src/test/java/net/hostsharing/hsadminng/mapper/KeyValueMapUnitTest.java index 34f8526a..9d1e73aa 100644 --- a/src/test/java/net/hostsharing/hsadminng/mapper/KeyValueMapUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/mapper/KeyValueMapUnitTest.java @@ -2,6 +2,7 @@ package net.hostsharing.hsadminng.mapper; import org.junit.jupiter.api.Test; +import jakarta.validation.ValidationException; import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; @@ -13,7 +14,7 @@ class KeyValueMapUnitTest { @Test void fromMap() { - final var result = KeyValueMap.from(Map.ofEntries( + final var result = KeyValueMap.from("propName", Map.ofEntries( Map.entry("one", 1), Map.entry("two", 2) )); @@ -24,9 +25,10 @@ class KeyValueMapUnitTest { @Test void fromNonMap() { 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"); } }