From 693569ceea850a0c563a6b88c4eea70cda62d193 Mon Sep 17 00:00:00 2001 From: Benoit Orihuela Date: Wed, 29 Nov 2023 07:40:28 +0100 Subject: [PATCH 1/3] refactor(WIP): inclusive error handling in batch operations --- .../search/service/EntityOperationService.kt | 124 ++++++++--------- .../stellio/search/web/BatchAPIResponses.kt | 19 +++ .../search/web/EntityOperationHandler.kt | 131 ++++++++++-------- .../service/EntityOperationServiceTests.kt | 53 +++++-- .../search/web/EntityOperationHandlerTests.kt | 71 +++++++--- .../egm/stellio/shared/util/JsonLdUtils.kt | 25 ++-- 6 files changed, 252 insertions(+), 171 deletions(-) diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/service/EntityOperationService.kt b/search-service/src/main/kotlin/com/egm/stellio/search/service/EntityOperationService.kt index b703086fe..56c87f45e 100644 --- a/search-service/src/main/kotlin/com/egm/stellio/search/service/EntityOperationService.kt +++ b/search-service/src/main/kotlin/com/egm/stellio/search/service/EntityOperationService.kt @@ -6,12 +6,8 @@ import arrow.core.raise.either import arrow.core.right import com.egm.stellio.search.authorization.AuthorizationService import com.egm.stellio.search.model.UpdateResult -import com.egm.stellio.search.web.BatchEntityError -import com.egm.stellio.search.web.BatchEntitySuccess -import com.egm.stellio.search.web.BatchOperationResult +import com.egm.stellio.search.web.* import com.egm.stellio.shared.model.BadRequestDataException -import com.egm.stellio.shared.model.JsonLdEntity -import com.egm.stellio.shared.model.NgsiLdEntity import com.egm.stellio.shared.util.Sub import org.springframework.stereotype.Component import org.springframework.transaction.annotation.Transactional @@ -30,8 +26,10 @@ class EntityOperationService( /** * Splits [entities] by their existence in the DB. */ - suspend fun splitEntitiesByExistence(entities: List): Pair, List> { - val extractIdFunc: (NgsiLdEntity) -> URI = { it.id } + suspend fun splitEntitiesByExistence( + entities: List + ): Pair, List> { + val extractIdFunc: (JsonLdNgsiLdEntity) -> URI = { it.entityId() } return splitEntitiesByExistenceGeneric(entities, extractIdFunc) } @@ -58,20 +56,16 @@ class EntityOperationService( * @return a [BatchOperationResult] */ suspend fun create( - entities: List, - jsonLdEntities: List, + entities: List, sub: Sub? ): BatchOperationResult { - val creationResults = entities.map { ngsiLdEntity -> + val creationResults = entities.map { jsonLdNgsiLdEntity -> either { - val jsonLdEntity = jsonLdEntities.find { jsonLdEntity -> - ngsiLdEntity.id.toString() == jsonLdEntity.id - }!! - entityPayloadService.createEntity(ngsiLdEntity, jsonLdEntity, sub) + entityPayloadService.createEntity(jsonLdNgsiLdEntity.second, jsonLdNgsiLdEntity.first, sub) .map { - BatchEntitySuccess(ngsiLdEntity.id) + BatchEntitySuccess(jsonLdNgsiLdEntity.entityId()) }.mapLeft { apiException -> - BatchEntityError(ngsiLdEntity.id, arrayListOf(apiException.message)) + BatchEntityError(jsonLdNgsiLdEntity.entityId(), arrayListOf(apiException.message)) }.bind() } }.fold( @@ -124,7 +118,7 @@ class EntityOperationService( * @return a [BatchOperationResult] with list of replaced ids and list of errors. */ @Transactional - suspend fun replace(entities: List>, sub: Sub?): BatchOperationResult = + suspend fun replace(entities: List, sub: Sub?): BatchOperationResult = processEntities(entities, false, sub, ::replaceEntity) /** @@ -136,18 +130,18 @@ class EntityOperationService( */ @Transactional suspend fun update( - entities: List>, + entities: List, disallowOverwrite: Boolean = false, sub: Sub? ): BatchOperationResult = processEntities(entities, disallowOverwrite, sub, ::updateEntity) private suspend fun processEntities( - entities: List>, + entities: List, disallowOverwrite: Boolean = false, sub: Sub?, processor: - suspend (Pair, Boolean, Sub?) -> Either + suspend (JsonLdNgsiLdEntity, Boolean, Sub?) -> Either ): BatchOperationResult = entities.map { processEntity(it, disallowOverwrite, sub, processor) @@ -162,16 +156,16 @@ class EntityOperationService( ) private suspend fun processEntity( - entity: Pair, + entity: JsonLdNgsiLdEntity, disallowOverwrite: Boolean = false, sub: Sub?, processor: - suspend (Pair, Boolean, Sub?) -> Either + suspend (JsonLdNgsiLdEntity, Boolean, Sub?) -> Either ): Either = kotlin.runCatching { processor(entity, disallowOverwrite, sub) }.fold( - onFailure = { BatchEntityError(entity.first.id, arrayListOf(it.message!!)).left() }, + onFailure = { BatchEntityError(entity.entityId(), arrayListOf(it.message!!)).left() }, onSuccess = { it } ) @@ -181,57 +175,55 @@ class EntityOperationService( @Transactional(rollbackFor = [BadRequestDataException::class]) @Throws(BadRequestDataException::class) suspend fun replaceEntity( - entity: Pair, + entity: JsonLdNgsiLdEntity, disallowOverwrite: Boolean, sub: Sub? - ): Either = - either { - val (ngsiLdEntity, jsonLdEntity) = entity - temporalEntityAttributeService.deleteTemporalAttributesOfEntity(ngsiLdEntity.id) - val updateResult = entityPayloadService.appendAttributes( - ngsiLdEntity.id, - jsonLdEntity.getAttributes(), - disallowOverwrite, - sub - ).bind() - - if (updateResult.notUpdated.isNotEmpty()) - BadRequestDataException( - updateResult.notUpdated.joinToString(", ") { it.attributeName + " : " + it.reason } - ).left().bind() - else updateResult.right().bind() - }.map { - BatchEntitySuccess(entity.first.id) - }.mapLeft { - BatchEntityError(entity.first.id, arrayListOf(it.message)) - } + ): Either = either { + val (jsonLdEntity, ngsiLdEntity) = entity + temporalEntityAttributeService.deleteTemporalAttributesOfEntity(ngsiLdEntity.id) + val updateResult = entityPayloadService.appendAttributes( + ngsiLdEntity.id, + jsonLdEntity.getAttributes(), + disallowOverwrite, + sub + ).bind() + + if (updateResult.notUpdated.isNotEmpty()) + BadRequestDataException( + updateResult.notUpdated.joinToString(", ") { it.attributeName + " : " + it.reason } + ).left().bind() + else updateResult.right().bind() + }.map { + BatchEntitySuccess(entity.entityId()) + }.mapLeft { + BatchEntityError(entity.entityId(), arrayListOf(it.message)) + } /* * Transactional because it should not replace entity attributes if new ones could not be replaced. */ @Transactional suspend fun updateEntity( - entity: Pair, + entity: JsonLdNgsiLdEntity, disallowOverwrite: Boolean, sub: Sub? - ): Either = - either { - val (ngsiLdEntity, jsonLdEntity) = entity - val updateResult = entityPayloadService.appendAttributes( - ngsiLdEntity.id, - jsonLdEntity.getAttributes(), - disallowOverwrite, - sub - ).bind() - if (updateResult.notUpdated.isEmpty()) - updateResult.right().bind() - else - BadRequestDataException( - ArrayList(updateResult.notUpdated.map { it.attributeName + " : " + it.reason }).joinToString() - ).left().bind() - }.map { - BatchEntitySuccess(entity.first.id, it) - }.mapLeft { - BatchEntityError(entity.first.id, arrayListOf(it.message)) - } + ): Either = either { + val (jsonLdEntity, ngsiLdEntity) = entity + val updateResult = entityPayloadService.appendAttributes( + ngsiLdEntity.id, + jsonLdEntity.getAttributes(), + disallowOverwrite, + sub + ).bind() + if (updateResult.notUpdated.isEmpty()) + updateResult.right().bind() + else + BadRequestDataException( + ArrayList(updateResult.notUpdated.map { it.attributeName + " : " + it.reason }).joinToString() + ).left().bind() + }.map { + BatchEntitySuccess(entity.entityId(), it) + }.mapLeft { + BatchEntityError(entity.entityId(), arrayListOf(it.message)) + } } diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/web/BatchAPIResponses.kt b/search-service/src/main/kotlin/com/egm/stellio/search/web/BatchAPIResponses.kt index b292ae419..3d5f6cf9b 100644 --- a/search-service/src/main/kotlin/com/egm/stellio/search/web/BatchAPIResponses.kt +++ b/search-service/src/main/kotlin/com/egm/stellio/search/web/BatchAPIResponses.kt @@ -1,7 +1,10 @@ package com.egm.stellio.search.web import com.egm.stellio.search.model.UpdateResult +import com.egm.stellio.shared.model.APIException +import com.egm.stellio.shared.model.JsonLdEntity import com.egm.stellio.shared.model.NgsiLdEntity +import com.egm.stellio.shared.util.toUri import com.fasterxml.jackson.annotation.JsonIgnore import com.fasterxml.jackson.annotation.JsonValue import java.net.URI @@ -19,6 +22,12 @@ data class BatchOperationResult( @JsonIgnore fun getSuccessfulEntitiesIds() = success.map { it.entityId } + @JsonIgnore + fun addEntitiesToErrors(entities: List>) = + entities.forEach { + errors.add(BatchEntityError(it.first.toUri(), arrayListOf(it.second.message))) + } + @JsonIgnore fun addEntitiesToErrors(entities: List, errorMessage: String) = addIdsToErrors(entities.map { it.id }, errorMessage) @@ -41,3 +50,13 @@ data class BatchEntityError( val entityId: URI, val error: MutableList ) + +typealias JsonLdNgsiLdEntity = Pair + +fun List.extractNgsiLdEntities(): List = this.map { it.second } +fun JsonLdNgsiLdEntity.entityId(): URI = this.second.id + +data class BatchEntityPreparation( + val success: List = emptyList(), + val errors: List> = emptyList() +) diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/web/EntityOperationHandler.kt b/search-service/src/main/kotlin/com/egm/stellio/search/web/EntityOperationHandler.kt index cb8a05aed..8be1fe676 100644 --- a/search-service/src/main/kotlin/com/egm/stellio/search/web/EntityOperationHandler.kt +++ b/search-service/src/main/kotlin/com/egm/stellio/search/web/EntityOperationHandler.kt @@ -15,7 +15,9 @@ import com.egm.stellio.search.util.validateMinimalQueryEntitiesParameters import com.egm.stellio.shared.config.ApplicationProperties import com.egm.stellio.shared.model.* import com.egm.stellio.shared.util.* -import com.egm.stellio.shared.util.JsonLdUtils.expandJsonLdEntities +import com.egm.stellio.shared.util.JsonLdUtils.JSONLD_ID_TERM +import com.egm.stellio.shared.util.JsonLdUtils.NGSILD_CORE_CONTEXT +import com.egm.stellio.shared.util.JsonLdUtils.expandJsonLdEntityF import com.egm.stellio.shared.util.JsonLdUtils.filterJsonLdEntitiesOnAttributes import com.egm.stellio.shared.util.JsonUtils.deserializeAsList import kotlinx.coroutines.reactive.awaitFirst @@ -56,18 +58,19 @@ class EntityOperationHandler( checkBatchRequestBody(body).bind() checkContext(httpHeaders, body).bind() val context = getContextFromLinkHeader(httpHeaders.getOrEmpty(HttpHeaders.LINK)).bind() - val (expandedEntities, ngsiLdEntities) = - expandAndPrepareBatchOfEntities(body, context, httpHeaders.contentType).bind() - val (existingEntities, newEntities) = entityOperationService.splitEntitiesByExistence(ngsiLdEntities) + val (parsedEntities, unparsableEntities) = + expandAndPrepareBatchOfEntities(body, context, httpHeaders.contentType) + val (existingEntities, newEntities) = entityOperationService.splitEntitiesByExistence(parsedEntities) val (unauthorizedEntities, authorizedEntities) = newEntities.partition { authorizationService.userCanCreateEntities(sub).isLeft() } val batchOperationResult = BatchOperationResult().apply { - addEntitiesToErrors(existingEntities, ENTITY_ALREADY_EXISTS_MESSAGE) - addEntitiesToErrors(unauthorizedEntities, ENTITIY_CREATION_FORBIDDEN_MESSAGE) + addEntitiesToErrors(unparsableEntities) + addEntitiesToErrors(existingEntities.extractNgsiLdEntities(), ENTITY_ALREADY_EXISTS_MESSAGE) + addEntitiesToErrors(unauthorizedEntities.extractNgsiLdEntities(), ENTITIY_CREATION_FORBIDDEN_MESSAGE) } - doBatchCreation(authorizedEntities, expandedEntities, batchOperationResult, sub) + doBatchCreation(authorizedEntities, batchOperationResult, sub) if (batchOperationResult.errors.isEmpty()) ResponseEntity.status(HttpStatus.CREATED).body(batchOperationResult.getSuccessfulEntitiesIds()) @@ -96,43 +99,44 @@ class EntityOperationHandler( checkContext(httpHeaders, body).bind() val context = getContextFromLinkHeader(httpHeaders.getOrEmpty(HttpHeaders.LINK)).bind() - val (expandedEntities, ngsiLdEntities) = - expandAndPrepareBatchOfEntities(body, context, httpHeaders.contentType).bind() - val (existingEntities, newEntities) = entityOperationService.splitEntitiesByExistence(ngsiLdEntities) + val (parsedEntities, unparsableEntities) = + expandAndPrepareBatchOfEntities(body, context, httpHeaders.contentType) + val (existingEntities, newEntities) = entityOperationService.splitEntitiesByExistence(parsedEntities) val (newUnauthorizedEntities, newAuthorizedEntities) = newEntities.partition { authorizationService.userCanCreateEntities(sub).isLeft() } val batchOperationResult = BatchOperationResult().apply { - addEntitiesToErrors(newUnauthorizedEntities, ENTITIY_CREATION_FORBIDDEN_MESSAGE) + addEntitiesToErrors(unparsableEntities) + addEntitiesToErrors(newUnauthorizedEntities.extractNgsiLdEntities(), ENTITIY_CREATION_FORBIDDEN_MESSAGE) } - doBatchCreation(newAuthorizedEntities, expandedEntities, batchOperationResult, sub) + doBatchCreation(newAuthorizedEntities, batchOperationResult, sub) val (existingEntitiesUnauthorized, existingEntitiesAuthorized) = - existingEntities.partition { authorizationService.userCanUpdateEntity(it.id, sub).isLeft() } - batchOperationResult.addEntitiesToErrors(existingEntitiesUnauthorized, ENTITY_UPDATE_FORBIDDEN_MESSAGE) + existingEntities.partition { authorizationService.userCanUpdateEntity(it.entityId(), sub).isLeft() } + batchOperationResult.addEntitiesToErrors( + existingEntitiesUnauthorized.extractNgsiLdEntities(), + ENTITY_UPDATE_FORBIDDEN_MESSAGE + ) if (existingEntitiesAuthorized.isNotEmpty()) { - val entitiesToUpdate = existingEntitiesAuthorized.map { ngsiLdEntity -> - Pair(ngsiLdEntity, expandedEntities.find { ngsiLdEntity.id.toString() == it.id }!!) - } val updateOperationResult = when (options) { - "update" -> entityOperationService.update(entitiesToUpdate, false, sub.getOrNull()) - else -> entityOperationService.replace(entitiesToUpdate, sub.getOrNull()) + "update" -> entityOperationService.update(existingEntitiesAuthorized, false, sub.getOrNull()) + else -> entityOperationService.replace(existingEntitiesAuthorized, sub.getOrNull()) } if (options == "update") - publishUpdateEvents(sub.getOrNull(), updateOperationResult, expandedEntities, ngsiLdEntities) + publishUpdateEvents(sub.getOrNull(), updateOperationResult, parsedEntities) else - publishReplaceEvents(sub.getOrNull(), updateOperationResult, ngsiLdEntities) + publishReplaceEvents(sub.getOrNull(), updateOperationResult, parsedEntities) batchOperationResult.errors.addAll(updateOperationResult.errors) batchOperationResult.success.addAll(updateOperationResult.success) } if (batchOperationResult.errors.isEmpty() && newEntities.isNotEmpty()) - ResponseEntity.status(HttpStatus.CREATED).body(newEntities.map { it.id }) + ResponseEntity.status(HttpStatus.CREATED).body(newEntities.map { it.entityId() }) else if (batchOperationResult.errors.isEmpty()) ResponseEntity.status(HttpStatus.NO_CONTENT).build() else @@ -161,26 +165,24 @@ class EntityOperationHandler( val context = getContextFromLinkHeader(httpHeaders.getOrEmpty(HttpHeaders.LINK)).bind() val disallowOverwrite = options.map { it == QUERY_PARAM_OPTIONS_NOOVERWRITE_VALUE }.orElse(false) - val (expandedEntities, ngsiLdEntities) = - expandAndPrepareBatchOfEntities(body, context, httpHeaders.contentType).bind() - val (existingEntities, newEntities) = entityOperationService.splitEntitiesByExistence(ngsiLdEntities) + val (parsedEntities, unparsableEntities) = + expandAndPrepareBatchOfEntities(body, context, httpHeaders.contentType) + val (existingEntities, newEntities) = entityOperationService.splitEntitiesByExistence(parsedEntities) val (existingEntitiesUnauthorized, existingEntitiesAuthorized) = - existingEntities.partition { authorizationService.userCanUpdateEntity(it.id, sub).isLeft() } + existingEntities.partition { authorizationService.userCanUpdateEntity(it.entityId(), sub).isLeft() } val batchOperationResult = BatchOperationResult().apply { - addEntitiesToErrors(newEntities, ENTITY_DOES_NOT_EXIST_MESSAGE) - addEntitiesToErrors(existingEntitiesUnauthorized, ENTITY_UPDATE_FORBIDDEN_MESSAGE) + addEntitiesToErrors(unparsableEntities) + addEntitiesToErrors(newEntities.extractNgsiLdEntities(), ENTITY_DOES_NOT_EXIST_MESSAGE) + addEntitiesToErrors(existingEntitiesUnauthorized.extractNgsiLdEntities(), ENTITY_UPDATE_FORBIDDEN_MESSAGE) } if (existingEntitiesAuthorized.isNotEmpty()) { - val entitiesToUpdate = existingEntitiesAuthorized.map { ngsiLdEntity -> - Pair(ngsiLdEntity, expandedEntities.find { ngsiLdEntity.id.toString() == it.id }!!) - } val updateOperationResult = - entityOperationService.update(entitiesToUpdate, disallowOverwrite, sub.getOrNull()) + entityOperationService.update(existingEntitiesAuthorized, disallowOverwrite, sub.getOrNull()) - publishUpdateEvents(sub.getOrNull(), updateOperationResult, expandedEntities, ngsiLdEntities) + publishUpdateEvents(sub.getOrNull(), updateOperationResult, parsedEntities) batchOperationResult.errors.addAll(updateOperationResult.errors) batchOperationResult.success.addAll(updateOperationResult.success) @@ -305,36 +307,47 @@ class EntityOperationHandler( payload: List>, context: String?, contentType: MediaType? - ): Either, List>> = either { - payload - .let { - if (contentType == JSON_LD_MEDIA_TYPE) - expandJsonLdEntities(it) - else - expandJsonLdEntities(it, listOf(context ?: JsonLdUtils.NGSILD_CORE_CONTEXT)) + ): BatchEntityPreparation = + payload.map { + if (contentType == JSON_LD_MEDIA_TYPE) + expandJsonLdEntityF(it).mapLeft { apiException -> Pair(it[JSONLD_ID_TERM] as String, apiException) } + else + expandJsonLdEntityF(it, listOf(context ?: NGSILD_CORE_CONTEXT)) + .mapLeft { apiException -> Pair(it[JSONLD_ID_TERM] as String, apiException) } + }.map { + when (it) { + is Either.Left -> it.value.left() + is Either.Right -> { + when (val result = it.value.toNgsiLdEntity()) { + is Either.Left -> Pair(it.value.id, result.value).left() + is Either.Right -> Pair(it.value, result.value).right() + } + } } - .let { jsonLdEntities -> - Pair(jsonLdEntities, jsonLdEntities.map { it.toNgsiLdEntity().bind() }) + }.fold(BatchEntityPreparation()) { acc, entry -> + when (entry) { + is Either.Left -> acc.copy(errors = acc.errors.plus(entry.value)) + is Either.Right -> acc.copy(success = acc.success.plus(entry.value)) } - } + } private suspend fun doBatchCreation( - entitiesToCreate: List, - jsonLdEntities: List, + entitiesToCreate: List, batchOperationResult: BatchOperationResult, sub: Option ) { if (entitiesToCreate.isNotEmpty()) { - val createOperationResult = entityOperationService.create(entitiesToCreate, jsonLdEntities, sub.getOrNull()) + val createOperationResult = entityOperationService.create(entitiesToCreate, sub.getOrNull()) authorizationService.createAdminRights(createOperationResult.getSuccessfulEntitiesIds(), sub) entitiesToCreate - .filter { it.id in createOperationResult.getSuccessfulEntitiesIds() } + .filter { it.second.id in createOperationResult.getSuccessfulEntitiesIds() } .forEach { + val ngsiLdEntity = it.second entityEventService.publishEntityCreateEvent( sub.getOrNull(), - it.id, - it.types, - it.contexts + ngsiLdEntity.id, + ngsiLdEntity.types, + ngsiLdEntity.contexts ) } batchOperationResult.errors.addAll(createOperationResult.errors) @@ -345,26 +358,26 @@ class EntityOperationHandler( private suspend fun publishReplaceEvents( sub: String?, updateBatchOperationResult: BatchOperationResult, - ngsiLdEntities: List - ) = ngsiLdEntities.filter { it.id in updateBatchOperationResult.getSuccessfulEntitiesIds() } + jsonLdNgsiLdEntities: List + ) = jsonLdNgsiLdEntities.filter { it.entityId() in updateBatchOperationResult.getSuccessfulEntitiesIds() } .forEach { entityEventService.publishEntityReplaceEvent( sub, - it.id, - it.types, - it.contexts + it.entityId(), + it.second.types, + it.second.contexts ) } private suspend fun publishUpdateEvents( sub: String?, updateBatchOperationResult: BatchOperationResult, - jsonLdEntities: List, - ngsiLdEntities: List + jsonLdNgsiLdEntities: List ) { updateBatchOperationResult.success.forEach { - val jsonLdEntity = jsonLdEntities.find { jsonLdEntity -> jsonLdEntity.id.toUri() == it.entityId }!! - val ngsiLdEntity = ngsiLdEntities.find { ngsiLdEntity -> ngsiLdEntity.id == it.entityId }!! + val (jsonLdEntity, ngsiLdEntity) = jsonLdNgsiLdEntities.find { jsonLdNgsiLdEntity -> + jsonLdNgsiLdEntity.entityId() == it.entityId + }!! entityEventService.publishAttributeChangeEvents( sub, it.entityId, diff --git a/search-service/src/test/kotlin/com/egm/stellio/search/service/EntityOperationServiceTests.kt b/search-service/src/test/kotlin/com/egm/stellio/search/service/EntityOperationServiceTests.kt index 673bc054a..df5985f15 100644 --- a/search-service/src/test/kotlin/com/egm/stellio/search/service/EntityOperationServiceTests.kt +++ b/search-service/src/test/kotlin/com/egm/stellio/search/service/EntityOperationServiceTests.kt @@ -77,10 +77,15 @@ class EntityOperationServiceTests { entityPayloadService.filterExistingEntitiesAsIds(listOf(firstEntityURI, secondEntityURI)) } returns listOf(firstEntityURI) - val (exist, doNotExist) = entityOperationService.splitEntitiesByExistence(listOf(firstEntity, secondEntity)) + val (exist, doNotExist) = entityOperationService.splitEntitiesByExistence( + listOf( + Pair(firstJsonLdEntity, firstEntity), + Pair(secondJsonLdEntity, secondEntity) + ) + ) - assertEquals(listOf(firstEntity), exist) - assertEquals(listOf(secondEntity), doNotExist) + assertEquals(listOf(Pair(firstJsonLdEntity, firstEntity)), exist) + assertEquals(listOf(Pair(secondJsonLdEntity, secondEntity)), doNotExist) } @Test @@ -101,8 +106,10 @@ class EntityOperationServiceTests { coEvery { entityPayloadService.createEntity(any(), any(), any()) } returns Unit.right() val batchOperationResult = entityOperationService.create( - listOf(firstEntity, secondEntity), - listOf(firstJsonLdEntity, secondJsonLdEntity), + listOf( + Pair(firstJsonLdEntity, firstEntity), + Pair(secondJsonLdEntity, secondEntity) + ), sub ) @@ -128,8 +135,10 @@ class EntityOperationServiceTests { } returns BadRequestDataException("Invalid entity").left() val batchOperationResult = entityOperationService.create( - listOf(firstEntity, secondEntity), - listOf(firstJsonLdEntity, secondJsonLdEntity), + listOf( + Pair(firstJsonLdEntity, firstEntity), + Pair(secondJsonLdEntity, secondEntity) + ), sub ) @@ -149,7 +158,10 @@ class EntityOperationServiceTests { } returns EMPTY_UPDATE_RESULT.right() val batchOperationResult = entityOperationService.update( - listOf(Pair(firstEntity, firstJsonLdEntity), Pair(secondEntity, secondJsonLdEntity)), + listOf( + Pair(firstJsonLdEntity, firstEntity), + Pair(secondJsonLdEntity, secondEntity) + ), false, sub ) @@ -178,7 +190,10 @@ class EntityOperationServiceTests { val batchOperationResult = entityOperationService.update( - listOf(Pair(firstEntity, firstJsonLdEntity), Pair(secondEntity, secondJsonLdEntity)), + listOf( + Pair(firstJsonLdEntity, firstEntity), + Pair(secondJsonLdEntity, secondEntity) + ), false, sub ) @@ -210,7 +225,10 @@ class EntityOperationServiceTests { } returns updateResult.right() val batchOperationResult = entityOperationService.update( - listOf(Pair(firstEntity, firstJsonLdEntity), Pair(secondEntity, secondJsonLdEntity)), + listOf( + Pair(firstJsonLdEntity, firstEntity), + Pair(secondJsonLdEntity, secondEntity) + ), false, sub ) @@ -237,7 +255,10 @@ class EntityOperationServiceTests { } returns EMPTY_UPDATE_RESULT.right() val batchOperationResult = entityOperationService.replace( - listOf(Pair(firstEntity, firstJsonLdEntity), Pair(secondEntity, secondJsonLdEntity)), + listOf( + Pair(firstJsonLdEntity, firstEntity), + Pair(secondJsonLdEntity, secondEntity) + ), sub ) @@ -268,7 +289,10 @@ class EntityOperationServiceTests { } returns BadRequestDataException("error").left() val batchOperationResult = entityOperationService.replace( - listOf(Pair(firstEntity, firstJsonLdEntity), Pair(secondEntity, secondJsonLdEntity)), + listOf( + Pair(firstJsonLdEntity, firstEntity), + Pair(secondJsonLdEntity, secondEntity) + ), sub ) @@ -295,7 +319,10 @@ class EntityOperationServiceTests { ).right() val batchOperationResult = entityOperationService.replace( - listOf(Pair(firstEntity, firstJsonLdEntity), Pair(secondEntity, secondJsonLdEntity)), + listOf( + Pair(firstJsonLdEntity, firstEntity), + Pair(secondJsonLdEntity, secondEntity) + ), sub ) diff --git a/search-service/src/test/kotlin/com/egm/stellio/search/web/EntityOperationHandlerTests.kt b/search-service/src/test/kotlin/com/egm/stellio/search/web/EntityOperationHandlerTests.kt index e49eebde2..d5d4512ca 100644 --- a/search-service/src/test/kotlin/com/egm/stellio/search/web/EntityOperationHandlerTests.kt +++ b/search-service/src/test/kotlin/com/egm/stellio/search/web/EntityOperationHandlerTests.kt @@ -134,7 +134,11 @@ class EntityOperationHandlerTests { val jsonLdFile = ClassPathResource("/ngsild/hcmr/HCMR_test_file.json") coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( - listOf(mockedTemperatureSensorEntity, mockedDissolvedOxygenSensorEntity, mockedDeviceEntity), + listOf( + Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity), + Pair(mockedDissolvedOxygenSensorJsonLdEntity, mockedDissolvedOxygenSensorEntity), + Pair(mockedDeviceJsonLdEntity, mockedDeviceEntity) + ), emptyList() ) coEvery { authorizationService.userCanUpdateEntity(any(), sub) } returns Unit.right() @@ -163,7 +167,10 @@ class EntityOperationHandlerTests { ) coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( - listOf(mockedTemperatureSensorEntity, mockedDissolvedOxygenSensorEntity), + listOf( + Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity), + Pair(mockedDissolvedOxygenSensorJsonLdEntity, mockedDissolvedOxygenSensorEntity), + ), emptyList() ) coEvery { authorizationService.userCanUpdateEntity(any(), sub) } returns Unit.right() @@ -206,7 +213,10 @@ class EntityOperationHandlerTests { val jsonLdFile = ClassPathResource("/ngsild/hcmr/HCMR_test_file.json") coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( - listOf(mockedTemperatureSensorEntity, mockedDissolvedOxygenSensorEntity), + listOf( + Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity), + Pair(mockedDissolvedOxygenSensorJsonLdEntity, mockedDissolvedOxygenSensorEntity), + ), emptyList() ) coEvery { authorizationService.userCanUpdateEntity(any(), sub) } returns Unit.right() @@ -231,8 +241,8 @@ class EntityOperationHandlerTests { val jsonLdFile = ClassPathResource("/ngsild/hcmr/HCMR_test_file.json") coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( - listOf(mockedTemperatureSensorEntity), - listOf(mockedDeviceEntity) + listOf(Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity)), + listOf(Pair(mockedDeviceJsonLdEntity, mockedDeviceEntity)) ) coEvery { authorizationService.userCanUpdateEntity(any(), sub) } returns Unit.right() coEvery { entityOperationService.update(any(), any(), any()) } returns BatchOperationResult( @@ -263,7 +273,7 @@ class EntityOperationHandlerTests { @Test fun `create batch entity should return a 201 if JSON-LD payload is correct`() = runTest { val jsonLdFile = ClassPathResource("/ngsild/hcmr/HCMR_test_file.json") - val capturedExpandedEntities = slot>() + val capturedExpandedEntities = slot>() val capturedEntitiesIds = mutableListOf() val capturedEntityTypes = slot>() @@ -271,7 +281,7 @@ class EntityOperationHandlerTests { entityOperationService.splitEntitiesByExistence(capture(capturedExpandedEntities)) } answers { Pair(emptyList(), capturedExpandedEntities.captured) } coEvery { authorizationService.userCanCreateEntities(sub) } returns Unit.right() - coEvery { entityOperationService.create(any(), any(), any()) } returns BatchOperationResult( + coEvery { entityOperationService.create(any(), any()) } returns BatchOperationResult( allEntitiesUris.map { BatchEntitySuccess(it) }.toMutableList(), arrayListOf() ) @@ -294,7 +304,7 @@ class EntityOperationHandlerTests { .jsonPath("$").isArray .jsonPath("$[*]").isEqualTo(allEntitiesUris.map { it.toString() }) - assertEquals(allEntitiesUris, capturedExpandedEntities.captured.map { it.id }) + assertEquals(allEntitiesUris, capturedExpandedEntities.captured.map { it.entityId() }) coVerify { authorizationService.createAdminRights(allEntitiesUris, sub) } coVerify(timeout = 1000, exactly = 3) { @@ -315,12 +325,15 @@ class EntityOperationHandlerTests { entityOperationService.splitEntitiesByExistence(any()) } answers { Pair( - listOf(mockedTemperatureSensorEntity), - listOf(mockedDissolvedOxygenSensorEntity, mockedDeviceEntity) + listOf(Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity)), + listOf( + Pair(mockedDissolvedOxygenSensorJsonLdEntity, mockedDissolvedOxygenSensorEntity), + Pair(mockedDeviceJsonLdEntity, mockedDeviceEntity) + ) ) } coEvery { authorizationService.userCanCreateEntities(sub) } returns Unit.right() - coEvery { entityOperationService.create(any(), any(), any()) } returns BatchOperationResult( + coEvery { entityOperationService.create(any(), any()) } returns BatchOperationResult( createdEntitiesIds.map { BatchEntitySuccess(it) }.toMutableList(), arrayListOf() ) @@ -370,7 +383,7 @@ class EntityOperationHandlerTests { coEvery { entityOperationService.splitEntitiesByExistence(any()) - } returns Pair(emptyList(), listOf(mockedDeviceEntity)) + } returns Pair(emptyList(), listOf(Pair(mockedDeviceJsonLdEntity, mockedDeviceEntity))) coEvery { authorizationService.userCanCreateEntities(sub) } returns AccessDeniedException(ENTITIY_CREATION_FORBIDDEN_MESSAGE).left() @@ -436,11 +449,14 @@ class EntityOperationHandlerTests { ) coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( - listOf(mockedDissolvedOxygenSensorEntity, mockedDeviceEntity), - listOf(mockedTemperatureSensorEntity) + listOf( + Pair(mockedDissolvedOxygenSensorJsonLdEntity, mockedDissolvedOxygenSensorEntity), + Pair(mockedDeviceJsonLdEntity, mockedDeviceEntity) + ), + listOf(Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity)) ) coEvery { authorizationService.userCanCreateEntities(any()) } returns Unit.right() - coEvery { entityOperationService.create(any(), any(), any()) } returns createdBatchResult + coEvery { entityOperationService.create(any(), any()) } returns createdBatchResult coEvery { authorizationService.createAdminRights(any(), eq(sub)) } returns Unit.right() coEvery { entityEventService.publishEntityCreateEvent(any(), any(), any(), any()) } returns Job() @@ -484,7 +500,7 @@ class EntityOperationHandlerTests { val jsonLdFile = ClassPathResource("/ngsild/hcmr/HCMR_test_file.json") coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( - listOf(mockedTemperatureSensorEntity), + listOf(Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity)), emptyList() ) coEvery { authorizationService.userCanUpdateEntity(any(), sub) } returns Unit.right() @@ -512,11 +528,14 @@ class EntityOperationHandlerTests { ) coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( - listOf(mockedDissolvedOxygenSensorEntity, mockedTemperatureSensorEntity), - listOf(mockedDeviceEntity) + listOf( + Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity), + Pair(mockedDissolvedOxygenSensorJsonLdEntity, mockedDissolvedOxygenSensorEntity) + ), + listOf(Pair(mockedDeviceJsonLdEntity, mockedDeviceEntity)) ) coEvery { authorizationService.userCanCreateEntities(sub) } returns Unit.right() - coEvery { entityOperationService.create(any(), any(), any()) } returns BatchOperationResult( + coEvery { entityOperationService.create(any(), any()) } returns BatchOperationResult( arrayListOf(BatchEntitySuccess(deviceUri, mockkClass(UpdateResult::class))), arrayListOf() ) @@ -561,7 +580,10 @@ class EntityOperationHandlerTests { val entitiesIds = arrayListOf(temperatureSensorUri, dissolvedOxygenSensorUri) coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( - listOf(mockedDissolvedOxygenSensorEntity, mockedTemperatureSensorEntity), + listOf( + Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity), + Pair(mockedDissolvedOxygenSensorJsonLdEntity, mockedDissolvedOxygenSensorEntity) + ), emptyList() ) coEvery { authorizationService.userCanUpdateEntity(any(), sub) } returns Unit.right() @@ -577,7 +599,7 @@ class EntityOperationHandlerTests { .exchange() .expectStatus().isNoContent - coVerify { entityOperationService.create(any(), any(), any()) wasNot Called } + coVerify { entityOperationService.create(any(), any()) wasNot Called } coVerify { entityOperationService.replace(any(), sub.getOrNull()) } coVerify { entityOperationService.update(any(), any(), any()) wasNot Called } coVerify(timeout = 1000, exactly = 2) { @@ -596,7 +618,7 @@ class EntityOperationHandlerTests { coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( emptyList(), - listOf(mockedTemperatureSensorEntity) + listOf(Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity)) ) coEvery { authorizationService.userCanCreateEntities(sub) @@ -630,7 +652,10 @@ class EntityOperationHandlerTests { val jsonLdFile = ClassPathResource("/ngsild/hcmr/HCMR_test_file.json") coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( - listOf(mockedTemperatureSensorEntity, mockedDissolvedOxygenSensorEntity), + listOf( + Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity), + Pair(mockedDissolvedOxygenSensorJsonLdEntity, mockedDissolvedOxygenSensorEntity) + ), emptyList() ) coEvery { diff --git a/shared/src/main/kotlin/com/egm/stellio/shared/util/JsonLdUtils.kt b/shared/src/main/kotlin/com/egm/stellio/shared/util/JsonLdUtils.kt index cad467492..91031de9b 100644 --- a/shared/src/main/kotlin/com/egm/stellio/shared/util/JsonLdUtils.kt +++ b/shared/src/main/kotlin/com/egm/stellio/shared/util/JsonLdUtils.kt @@ -136,6 +136,21 @@ object JsonLdUtils { ): Map = doJsonLdExpansion(deserializedPayload, contexts) + suspend fun expandJsonLdEntityF( + input: Map, + contexts: List + ): Either = + runCatching { + doJsonLdExpansion(input, contexts) + }.fold({ + JsonLdEntity(it, contexts).right() + }, { + it.toAPIException().left() + }) + + suspend fun expandJsonLdEntityF(input: Map): Either = + expandJsonLdEntityF(input, extractContextFromInput(input)) + suspend fun expandJsonLdEntity(input: Map, contexts: List): JsonLdEntity = JsonLdEntity(doJsonLdExpansion(input, contexts), contexts) @@ -147,16 +162,6 @@ object JsonLdUtils { return expandJsonLdEntity(jsonInput, extractContextFromInput(jsonInput)) } - suspend fun expandJsonLdEntities(entities: List>): List = - entities.map { - expandJsonLdEntity(it, extractContextFromInput(it)) - } - - suspend fun expandJsonLdEntities(entities: List>, contexts: List): List = - entities.map { - expandJsonLdEntity(it, contexts) - } - fun expandJsonLdTerms(terms: List, contexts: List): List = terms.map { expandJsonLdTerm(it, contexts) From 6790ca290afd3ac703325404f8bb3c79aa024a89 Mon Sep 17 00:00:00 2001 From: Benoit Orihuela Date: Thu, 30 Nov 2023 09:42:51 +0100 Subject: [PATCH 2/3] feat: add minimal test --- .../search/web/EntityOperationHandlerTests.kt | 37 +++++++++++++++++++ .../ngsild/two_sensors_one_invalid.jsonld | 15 ++++++++ 2 files changed, 52 insertions(+) create mode 100644 search-service/src/test/resources/ngsild/two_sensors_one_invalid.jsonld diff --git a/search-service/src/test/kotlin/com/egm/stellio/search/web/EntityOperationHandlerTests.kt b/search-service/src/test/kotlin/com/egm/stellio/search/web/EntityOperationHandlerTests.kt index d5d4512ca..2359225eb 100644 --- a/search-service/src/test/kotlin/com/egm/stellio/search/web/EntityOperationHandlerTests.kt +++ b/search-service/src/test/kotlin/com/egm/stellio/search/web/EntityOperationHandlerTests.kt @@ -4,6 +4,7 @@ import arrow.core.left import arrow.core.right import com.egm.stellio.search.authorization.AuthorizationService import com.egm.stellio.search.config.SearchProperties +import com.egm.stellio.search.model.EMPTY_UPDATE_RESULT import com.egm.stellio.search.model.EntityPayload import com.egm.stellio.search.model.UpdateResult import com.egm.stellio.search.service.EntityEventService @@ -203,6 +204,42 @@ class EntityOperationHandlerTests { ) } + @Test + fun `update batch entity should return a 207 if one entity is an invalid NGSI-LD payload`() = runTest { + val jsonLdFile = ClassPathResource("/ngsild/two_sensors_one_invalid.jsonld") + + coEvery { entityOperationService.splitEntitiesByExistence(any()) } returns Pair( + listOf(Pair(mockedTemperatureSensorJsonLdEntity, mockedTemperatureSensorEntity)), + emptyList() + ) + coEvery { authorizationService.userCanUpdateEntity(any(), sub) } returns Unit.right() + coEvery { entityOperationService.update(any(), any(), any()) } returns BatchOperationResult( + mutableListOf(BatchEntitySuccess(temperatureSensorUri, EMPTY_UPDATE_RESULT)), + mutableListOf() + ) + + webClient.post() + .uri(batchUpdateEndpoint) + .bodyValue(jsonLdFile) + .exchange() + .expectStatus().isEqualTo(HttpStatus.MULTI_STATUS) + .expectBody().json( + """ + { + "errors": [ + { + "entityId": "urn:ngsi-ld:Sensor:HCMR-AQUABOX2temperature", + "error": [ "Unable to expand input payload" ] + } + ], + "success": [ + "urn:ngsi-ld:Sensor:HCMR-AQUABOX1temperature" + ] + } + """.trimIndent() + ) + } + @Test fun `update batch entity should return a 400 if JSON-LD payload is not correct`() { shouldReturn400WithBadPayload("update") diff --git a/search-service/src/test/resources/ngsild/two_sensors_one_invalid.jsonld b/search-service/src/test/resources/ngsild/two_sensors_one_invalid.jsonld new file mode 100644 index 000000000..43b7a6771 --- /dev/null +++ b/search-service/src/test/resources/ngsild/two_sensors_one_invalid.jsonld @@ -0,0 +1,15 @@ +[ + { + "id": "urn:ngsi-ld:Sensor:HCMR-AQUABOX1temperature", + "type": "Sensor", + "@context": [ + "https://raw.githubusercontent.com/easy-global-market/ngsild-api-data-models/master/aquac/jsonld-contexts/aquac-compound.jsonld" + ] + }, + { + "id": "urn:ngsi-ld:Sensor:HCMR-AQUABOX2temperature", + "@context": [ + "https://raw.githubusercontent.com/easy-global-market/ngsild-api-data-models/master/aquac/jsonld-contexts/aquac-compound.jsonld" + ] + } +] From 389ff4042d36ccc17d2da538b813d5082d6b11af Mon Sep 17 00:00:00 2001 From: Benoit Orihuela Date: Thu, 30 Nov 2023 12:10:02 +0100 Subject: [PATCH 3/3] small refactoring and documentation --- .../stellio/search/web/BatchAPIResponses.kt | 2 ++ .../search/web/EntityOperationHandler.kt | 30 ++++++++----------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/web/BatchAPIResponses.kt b/search-service/src/main/kotlin/com/egm/stellio/search/web/BatchAPIResponses.kt index 3d5f6cf9b..552b52639 100644 --- a/search-service/src/main/kotlin/com/egm/stellio/search/web/BatchAPIResponses.kt +++ b/search-service/src/main/kotlin/com/egm/stellio/search/web/BatchAPIResponses.kt @@ -56,6 +56,8 @@ typealias JsonLdNgsiLdEntity = Pair fun List.extractNgsiLdEntities(): List = this.map { it.second } fun JsonLdNgsiLdEntity.entityId(): URI = this.second.id +// a temporary data class to hold the result of deserializing, expanding and transforming to NGSI-LD entities +// the entities received in a batch operation data class BatchEntityPreparation( val success: List = emptyList(), val errors: List> = emptyList() diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/web/EntityOperationHandler.kt b/search-service/src/main/kotlin/com/egm/stellio/search/web/EntityOperationHandler.kt index 8be1fe676..3b167748a 100644 --- a/search-service/src/main/kotlin/com/egm/stellio/search/web/EntityOperationHandler.kt +++ b/search-service/src/main/kotlin/com/egm/stellio/search/web/EntityOperationHandler.kt @@ -1,10 +1,7 @@ package com.egm.stellio.search.web -import arrow.core.Either -import arrow.core.Option -import arrow.core.left +import arrow.core.* import arrow.core.raise.either -import arrow.core.right import com.egm.stellio.search.authorization.AuthorizationService import com.egm.stellio.search.service.EntityEventService import com.egm.stellio.search.service.EntityOperationService @@ -309,21 +306,18 @@ class EntityOperationHandler( contentType: MediaType? ): BatchEntityPreparation = payload.map { - if (contentType == JSON_LD_MEDIA_TYPE) - expandJsonLdEntityF(it).mapLeft { apiException -> Pair(it[JSONLD_ID_TERM] as String, apiException) } - else - expandJsonLdEntityF(it, listOf(context ?: NGSILD_CORE_CONTEXT)) - .mapLeft { apiException -> Pair(it[JSONLD_ID_TERM] as String, apiException) } - }.map { - when (it) { - is Either.Left -> it.value.left() - is Either.Right -> { - when (val result = it.value.toNgsiLdEntity()) { - is Either.Left -> Pair(it.value.id, result.value).left() - is Either.Right -> Pair(it.value, result.value).right() - } + val jsonLdExpansionResult = + if (contentType == JSON_LD_MEDIA_TYPE) + expandJsonLdEntityF(it) + else + expandJsonLdEntityF(it, listOf(context ?: NGSILD_CORE_CONTEXT)) + jsonLdExpansionResult + .mapLeft { apiException -> Pair(it[JSONLD_ID_TERM] as String, apiException) } + .flatMap { jsonLdEntity -> + jsonLdEntity.toNgsiLdEntity() + .mapLeft { apiException -> Pair(it[JSONLD_ID_TERM] as String, apiException) } + .map { ngsiLdEntity -> Pair(jsonLdEntity, ngsiLdEntity) } } - } }.fold(BatchEntityPreparation()) { acc, entry -> when (entry) { is Either.Left -> acc.copy(errors = acc.errors.plus(entry.value))