diff --git a/common/src/main/java/org/fiware/tmforum/common/repository/NgsiLdBaseRepository.java b/common/src/main/java/org/fiware/tmforum/common/repository/NgsiLdBaseRepository.java index f78418aa..58603612 100644 --- a/common/src/main/java/org/fiware/tmforum/common/repository/NgsiLdBaseRepository.java +++ b/common/src/main/java/org/fiware/tmforum/common/repository/NgsiLdBaseRepository.java @@ -2,6 +2,7 @@ import io.github.wistefan.mapping.EntityVOMapper; import io.github.wistefan.mapping.JavaObjectMapper; +import io.github.wistefan.mapping.ReservedWordHandler; import io.github.wistefan.mapping.annotations.MappingEnabled; import io.micronaut.cache.annotation.CacheInvalidate; import io.micronaut.cache.annotation.CachePut; @@ -12,9 +13,13 @@ import lombok.RequiredArgsConstructor; import org.fiware.ngsi.api.EntitiesApiClient; import org.fiware.ngsi.api.SubscriptionsApiClient; +import org.fiware.ngsi.model.AdditionalPropertyVO; import org.fiware.ngsi.model.EntityFragmentVO; import org.fiware.ngsi.model.EntityListVO; import org.fiware.ngsi.model.EntityVO; +import org.fiware.ngsi.model.GeoPropertyVO; +import org.fiware.ngsi.model.PropertyVO; +import org.fiware.ngsi.model.RelationshipVO; import org.fiware.ngsi.model.SubscriptionVO; import org.fiware.tmforum.common.CommonConstants; import org.fiware.tmforum.common.caching.EntityIdKeyGenerator; @@ -29,8 +34,11 @@ import java.net.URI; import java.util.Arrays; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.function.BiConsumer; import java.util.function.Function; import java.util.stream.Stream; @@ -186,7 +194,17 @@ public Mono updateDomainEntity(String id, T domainEntity) { } private EntityVO mergeForUpdate(EntityVO existing, EntityVO update) { - update.getAdditionalProperties().forEach(existing::setAdditionalProperties); + // `existing` was parsed by EscapeCleaningParser, which strips the tmfEscaped- prefix + // from reserved words it can safely unescape (everything except VO_FIELD_COLLISIONS). + // Writing that back verbatim would put raw JSON-LD keywords (@id/@type/@value/@context) + // on the wire; brokers are free to drop them (Scorpio >= 6.0.0 does), which silently + // destroys nested free-form values such as an expanded ODRL policy. Re-apply the escape + // before the entity goes out again. Only relevant on the replaceOnUpdate path, since + // PATCH /attrs sends the freshly mapped `update` only, which JavaObjectMapper escapes. + reEscapeReservedWords(existing); + if (update.getAdditionalProperties() != null) { + update.getAdditionalProperties().forEach(existing::setAdditionalProperties); + } if (update.getLocation() != null) existing.setLocation(update.getLocation()); if (update.getObservationSpace() != null) existing.setObservationSpace(update.getObservationSpace()); if (update.getOperationSpace() != null) existing.setOperationSpace(update.getOperationSpace()); @@ -195,6 +213,80 @@ private EntityVO mergeForUpdate(EntityVO existing, EntityVO update) { return existing; } + /** + * Re-apply the reserved-word escape to the additional properties of an entity that was read + * back from the broker, recursively. Counterpart of the {@code EscapeCleaningParser} of the + * mapping library, which removes the prefix while parsing. {@code escapeReservedWords} is + * idempotent, so keys that kept their prefix during parsing (the VO field collisions + * {@code id}/{@code type}/{@code value}) are left untouched. + * + * @param entityVO the entity to fix up in place + */ + private void reEscapeReservedWords(EntityVO entityVO) { + reEscapeProperties(entityVO.getAdditionalProperties(), entityVO::setAdditionalProperties); + } + + /** + * Rewrite the keys of an additional-properties map in place and recurse into its values. + * + * @param properties the (potentially null) map to rewrite + * @param setter the {@code setAdditionalProperties} of the owning VO, used to re-insert + */ + private void reEscapeProperties(Map properties, + BiConsumer setter) { + if (properties == null || properties.isEmpty()) { + return; + } + Map reEscaped = new LinkedHashMap<>(); + properties.forEach((key, value) -> reEscaped.put(ReservedWordHandler.escapeReservedWords(key), + reEscapeAdditionalProperty(value))); + properties.clear(); + reEscaped.forEach(setter); + } + + private AdditionalPropertyVO reEscapeAdditionalProperty(AdditionalPropertyVO additionalPropertyVO) { + // the generated VOs do not share an accessor for the additional properties, so every + // concrete type has to be handled explicitly. + if (additionalPropertyVO instanceof PropertyVO propertyVO) { + propertyVO.setValue(reEscapeValue(propertyVO.getValue())); + reEscapeProperties(propertyVO.getAdditionalProperties(), propertyVO::setAdditionalProperties); + } else if (additionalPropertyVO instanceof RelationshipVO relationshipVO) { + reEscapeProperties(relationshipVO.getAdditionalProperties(), relationshipVO::setAdditionalProperties); + } else if (additionalPropertyVO instanceof GeoPropertyVO geoPropertyVO) { + reEscapeProperties(geoPropertyVO.getAdditionalProperties(), geoPropertyVO::setAdditionalProperties); + } else if (additionalPropertyVO instanceof List multiAttribute) { + // multi-attributes: PropertyListVO, RelationshipListVO and GeoPropertyListVO are lists + // of the types handled above. + multiAttribute.stream() + .filter(AdditionalPropertyVO.class::isInstance) + .map(AdditionalPropertyVO.class::cast) + .forEach(this::reEscapeAdditionalProperty); + } + return additionalPropertyVO; + } + + /** + * Free-form property values are parsed into plain maps and lists, with their keys cleaned by + * the same parser, so they need the escape re-applied too. This is what carries e.g. the + * expanded ODRL policy of a product-offering-price and the reason the entity has to be fixed + * up at all. + * + * @param value the value to fix up + * @return the value with all reserved keys escaped again + */ + private Object reEscapeValue(Object value) { + if (value instanceof Map valueMap) { + Map reEscaped = new LinkedHashMap<>(); + valueMap.forEach((key, nestedValue) -> reEscaped + .put(ReservedWordHandler.escapeReservedWords(String.valueOf(key)), reEscapeValue(nestedValue))); + return reEscaped; + } + if (value instanceof List valueList) { + return valueList.stream().map(this::reEscapeValue).toList(); + } + return value; + } + /** * Delete a domain entity * diff --git a/common/src/test/java/org/fiware/tmforum/common/repository/NgsiLdBaseRepositoryUpdateTest.java b/common/src/test/java/org/fiware/tmforum/common/repository/NgsiLdBaseRepositoryUpdateTest.java new file mode 100644 index 00000000..e358e29f --- /dev/null +++ b/common/src/test/java/org/fiware/tmforum/common/repository/NgsiLdBaseRepositoryUpdateTest.java @@ -0,0 +1,184 @@ +package org.fiware.tmforum.common.repository; + +import io.github.wistefan.mapping.JavaObjectMapper; +import io.micronaut.http.HttpResponse; +import org.fiware.ngsi.api.EntitiesApiClient; +import org.fiware.ngsi.model.BatchOperationResultVO; +import org.fiware.ngsi.model.EntityListVO; +import org.fiware.ngsi.model.EntityVO; +import org.fiware.ngsi.model.PropertyListVO; +import org.fiware.ngsi.model.PropertyVO; +import org.fiware.tmforum.common.configuration.GeneralProperties; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import reactor.core.publisher.Mono; + +import java.net.URI; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Tests for the read-merge-write update path ({@code replaceOnUpdate}, required for Scorpio 6.x). + */ +class NgsiLdBaseRepositoryUpdateTest { + + private static final String ENTITY_ID = "urn:ngsi-ld:product-offering-price:8de1a1f4-2e56-4e3e-8d5b-8a3fa4a0e6d1"; + + private GeneralProperties properties; + private EntitiesApiClient entitiesApi; + private JavaObjectMapper javaObjectMapper; + private TmForumRepository repository; + + @BeforeEach + public void setUp() { + properties = new GeneralProperties(); + properties.setReplaceOnUpdate(true); + entitiesApi = mock(EntitiesApiClient.class); + javaObjectMapper = mock(JavaObjectMapper.class); + repository = new TmForumRepository(properties, entitiesApi, null, null, null, javaObjectMapper); + when(entitiesApi.batchEntityUpsert(any(), any())) + .thenReturn(Mono.just(HttpResponse.ok(new BatchOperationResultVO()))); + } + + @Test + public void reservedWordsAreEscapedAgainBeforeTheEntityIsWrittenBack() { + // The EscapeCleaningParser of the mapping library strips the tmfEscaped- prefix from + // reserved words while the existing entity is read from the broker. Writing that back + // verbatim puts raw JSON-LD keywords on the wire, which Scorpio >= 6.0.0 drops - and with + // them the expanded ODRL policy of the offering price, which then fails to map in the EDC. + EntityVO existing = anEntity(); + PropertyVO policy = aProperty(Map.of( + "@type", List.of("http://www.w3.org/ns/odrl/2/Offer"), + "@id", "urn:uuid:1b0f3b8a-6c2a-4b6b-9d0a-2f0f6a1a3c4d")); + existing.setAdditionalProperties("policy", policy); + mockRetrieval(existing); + + repository.updateDomainEntity(ENTITY_ID, new Object()).block(); + + Map writtenPolicy = writtenValueOf("policy"); + assertEquals(Map.of( + "tmfEscaped-@type", List.of("http://www.w3.org/ns/odrl/2/Offer"), + "tmfEscaped-@id", "urn:uuid:1b0f3b8a-6c2a-4b6b-9d0a-2f0f6a1a3c4d"), + writtenPolicy, + "Keywords the parser unescaped on read have to be escaped again on write."); + } + + @Test + public void escapingIsAppliedRecursivelyAndIsIdempotent() { + // Nested objects, lists and the keys the parser deliberately leaves escaped + // (id/type/value collide with VO fields) all have to end up correct. + EntityVO existing = anEntity(); + existing.setAdditionalProperties("policy", aProperty(Map.of( + "@type", "Offer", + "tmfEscaped-id", "already-escaped", + "permission", List.of(Map.of( + "@id", "urn:uuid:permission", + "constraint", Map.of("@value", "5")))))); + mockRetrieval(existing); + + repository.updateDomainEntity(ENTITY_ID, new Object()).block(); + + Map written = writtenValueOf("policy"); + assertEquals("Offer", written.get("tmfEscaped-@type")); + assertEquals("already-escaped", written.get("tmfEscaped-id"), + "Keys that kept their prefix during parsing must not be escaped twice."); + Map permission = (Map) ((List) written.get("permission")).get(0); + assertEquals("urn:uuid:permission", permission.get("tmfEscaped-@id")); + assertEquals(Map.of("tmfEscaped-@value", "5"), permission.get("constraint")); + } + + @Test + public void escapingCoversSubAttributesAndMultiAttributes() { + EntityVO existing = anEntity(); + PropertyVO first = aProperty("first"); + first.setAdditionalProperties("@type", aProperty("sub-attribute")); + PropertyVO second = aProperty(Map.of("@id", "urn:uuid:second")); + PropertyListVO multiAttribute = new PropertyListVO(); + multiAttribute.add(first); + multiAttribute.add(second); + existing.setAdditionalProperties("relatedParty", multiAttribute); + mockRetrieval(existing); + + repository.updateDomainEntity(ENTITY_ID, new Object()).block(); + + PropertyListVO written = (PropertyListVO) writtenEntity().getAdditionalProperties().get("relatedParty"); + assertTrue(written.get(0).getAdditionalProperties().containsKey("tmfEscaped-@type"), + "Sub-attribute names are escaped as well."); + assertFalse(written.get(0).getAdditionalProperties().containsKey("@type")); + assertEquals(Map.of("tmfEscaped-@id", "urn:uuid:second"), written.get(1).getValue()); + } + + @Test + public void anUpdateWithoutAdditionalPropertiesDoesNotFail() { + // e.g. an update that only touches a structural field. The merge used to NPE here. + EntityVO existing = anEntity(); + existing.setAdditionalProperties("name", aProperty("an offering price")); + when(javaObjectMapper.toEntityVO(any())).thenReturn(new EntityVO()); + when(entitiesApi.retrieveEntityById(eq(URI.create(ENTITY_ID)), any(), any(), any(), any(), any())) + .thenReturn(Mono.just(HttpResponse.ok(existing))); + + repository.updateDomainEntity(ENTITY_ID, new Object()).block(); + + assertEquals("an offering price", + ((PropertyVO) writtenEntity().getAdditionalProperties().get("name")).getValue()); + } + + @Test + public void plainAttributesAreLeftAlone() { + EntityVO existing = anEntity(); + existing.setAdditionalProperties("name", aProperty("an offering price")); + mockRetrieval(existing); + + repository.updateDomainEntity(ENTITY_ID, new Object()).block(); + + EntityVO written = writtenEntity(); + assertEquals("an offering price", ((PropertyVO) written.getAdditionalProperties().get("name")).getValue()); + assertEquals("2026-08-05T10:15:30Z", + ((PropertyVO) written.getAdditionalProperties().get("lastUpdate")).getValue(), + "The update still has to be merged on top of the existing entity."); + } + + private EntityVO anEntity() { + EntityVO entityVO = new EntityVO(); + entityVO.setId(URI.create(ENTITY_ID)); + entityVO.setType("product-offering-price"); + return entityVO; + } + + private PropertyVO aProperty(Object value) { + PropertyVO propertyVO = new PropertyVO(); + // the mapping library parses free-form values into mutable maps, mirror that + propertyVO.setValue(value instanceof Map map ? new LinkedHashMap<>(map) : value); + return propertyVO; + } + + private void mockRetrieval(EntityVO existing) { + // what the domain object to be updated maps to - a partial entity carrying just the change + EntityVO update = new EntityVO(); + update.setAdditionalProperties("lastUpdate", aProperty("2026-08-05T10:15:30Z")); + when(javaObjectMapper.toEntityVO(any())).thenReturn(update); + when(entitiesApi.retrieveEntityById(eq(URI.create(ENTITY_ID)), any(), any(), any(), any(), any())) + .thenReturn(Mono.just(HttpResponse.ok(existing))); + } + + private EntityVO writtenEntity() { + ArgumentCaptor captor = ArgumentCaptor.forClass(EntityListVO.class); + verify(entitiesApi).batchEntityUpsert(captor.capture(), eq("replace")); + return captor.getValue().get(0); + } + + private Map writtenValueOf(String attribute) { + return (Map) ((PropertyVO) writtenEntity().getAdditionalProperties().get(attribute)).getValue(); + } +}