fix: incorrect behaviors in Batch Entity Upsert endpoint (#1290)

- noOverwrite option is not handled whereas it should be passed when in update mode
- replace mode was only replacing the attributes of the entity

search-service/src/main/kotlin/com/egm/stellio/search/entity/service/EntityOperationService.kt

@@ -4,6 +4,7 @@ import arrow.core.Either
 import arrow.core.left
 import arrow.core.raise.either
 import arrow.core.right
+import com.egm.stellio.search.entity.model.EMPTY_UPDATE_RESULT
 import com.egm.stellio.search.entity.model.UpdateResult
 import com.egm.stellio.search.entity.web.BatchEntityError
 import com.egm.stellio.search.entity.web.BatchEntitySuccess
@@ -23,8 +24,7 @@ import java.net.URI
 @Component
 class EntityOperationService(
     private val entityService: EntityService,
-    private val entityQueryService: EntityQueryService,
-    private val entityAttributeService: EntityAttributeService,
+    private val entityQueryService: EntityQueryService
 ) {
 
     /**
@@ -150,7 +150,8 @@ class EntityOperationService(
     @Transactional
     suspend fun upsert(
         entities: List<JsonLdNgsiLdEntity>,
-        options: String?,
+        disallowOverwrite: Boolean,
+        updateMode: Boolean,
         sub: Sub?
     ): Pair<BatchOperationResult, List<URI>> {
         val (existingEntities, newEntities) = splitEntitiesByExistence(entities)
@@ -168,7 +169,7 @@ class EntityOperationService(
 
         if (existingOrDuplicatedEntities.isNotEmpty()) {
             val updateOperationResult =
-                if (options == "update") update(existingOrDuplicatedEntities, false, sub)
+                if (updateMode) update(existingOrDuplicatedEntities, disallowOverwrite, sub)
                 else replace(existingOrDuplicatedEntities, sub)
 
             batchOperationResult.errors.addAll(updateOperationResult.errors)
@@ -208,8 +209,7 @@ class EntityOperationService(
         entities: List<JsonLdNgsiLdEntity>,
         disallowOverwrite: Boolean = false,
         sub: Sub?,
-        processor:
-        suspend (JsonLdNgsiLdEntity, Boolean, Sub?) -> Either<APIException, UpdateResult>
+        processor: suspend (JsonLdNgsiLdEntity, Boolean, Sub?) -> Either<APIException, UpdateResult>
     ): BatchOperationResult =
         entities.map {
             processEntity(it, disallowOverwrite, sub, processor)
@@ -227,8 +227,7 @@ class EntityOperationService(
         entity: JsonLdNgsiLdEntity,
         disallowOverwrite: Boolean = false,
         sub: Sub?,
-        processor:
-        suspend (JsonLdNgsiLdEntity, Boolean, Sub?) -> Either<APIException, UpdateResult>
+        processor: suspend (JsonLdNgsiLdEntity, Boolean, Sub?) -> Either<APIException, UpdateResult>
     ): Either<BatchEntityError, BatchEntitySuccess> =
         kotlin.runCatching {
             either {
@@ -250,19 +249,16 @@ class EntityOperationService(
             onSuccess = { it }
         )
 
+    @SuppressWarnings("UnusedParameter")
     suspend fun replaceEntity(
         entity: JsonLdNgsiLdEntity,
         disallowOverwrite: Boolean,
         sub: Sub?
     ): Either<APIException, UpdateResult> = either {
         val (jsonLdEntity, ngsiLdEntity) = entity
-        entityAttributeService.deleteAttributes(ngsiLdEntity.id).bind()
-        entityService.appendAttributes(
-            ngsiLdEntity.id,
-            jsonLdEntity.getModifiableMembers(),
-            disallowOverwrite,
-            sub
-        ).bind()
+        entityService.replaceEntity(ngsiLdEntity.id, ngsiLdEntity, jsonLdEntity, sub).map {
+            EMPTY_UPDATE_RESULT
+        }.bind()
     }
 
     suspend fun updateEntity(

search-service/src/main/kotlin/com/egm/stellio/search/entity/web/BatchAPIResponses.kt

@@ -27,16 +27,6 @@ data class BatchOperationResult(
         entities.forEach {
             errors.add(BatchEntityError(it.first.toUri(), arrayListOf(it.second.message)))
         }
-
-    @JsonIgnore
-    fun addEntitiesToErrors(entities: List<NgsiLdEntity>, errorMessage: String) =
-        addIdsToErrors(entities.map { it.id }, errorMessage)
-
-    @JsonIgnore
-    fun addIdsToErrors(entitiesIds: List<URI>, errorMessage: String) =
-        errors.addAll(
-            entitiesIds.map { BatchEntityError(it, arrayListOf(errorMessage)) }
-        )
 }
 
 data class BatchEntitySuccess(
@@ -53,7 +43,6 @@ data class BatchEntityError(
 
 typealias JsonLdNgsiLdEntity = Pair<ExpandedEntity, NgsiLdEntity>
 
-fun List<JsonLdNgsiLdEntity>.extractNgsiLdEntities(): List<NgsiLdEntity> = 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

search-service/src/main/kotlin/com/egm/stellio/search/entity/web/EntityOperationHandler.kt

@@ -109,7 +109,9 @@ class EntityOperationHandler(
         )
         @RequestParam queryParams: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
-        val options = queryParams.getFirst(QP.OPTIONS.key)
+        val options = queryParams.getFirst(QP.OPTIONS.key)?.split(",")
+        val disallowOverwrite = options?.any { it == OptionsValue.NO_OVERWRITE.value } == true
+        val updateMode = options?.any { it == OptionsValue.UPDATE_MODE.value } == true
         val sub = getSubFromSecurityContext()
 
         val (parsedEntities, unparsableEntities) = prepareEntitiesFromRequestBody(requestBody, httpHeaders).bind()
@@ -121,7 +123,8 @@ class EntityOperationHandler(
         val newUniqueEntities = if (parsedEntities.isNotEmpty()) {
             val (updateOperationResult, newUniqueEntities) = entityOperationService.upsert(
                 parsedEntities,
-                options,
+                disallowOverwrite,
+                updateMode,
                 sub.getOrNull()
             )
 
@@ -155,12 +158,12 @@ class EntityOperationHandler(
         @RequestParam queryParams: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
         val options = queryParams.getFirst(QP.OPTIONS.key)
+        val disallowOverwrite = options?.let { it == OptionsValue.NO_OVERWRITE.value } == true
+
         val sub = getSubFromSecurityContext()
 
         val (parsedEntities, unparsableEntities) = prepareEntitiesFromRequestBody(requestBody, httpHeaders).bind()
 
-        val disallowOverwrite = options?.let { it == OptionsValue.NO_OVERWRITE.value } ?: false
-
         val batchOperationResult = BatchOperationResult().apply {
             addEntitiesToErrors(unparsableEntities)
         }
@@ -201,8 +204,7 @@ class EntityOperationHandler(
         }
 
         if (parsedEntities.isNotEmpty()) {
-            val mergeOperationResult =
-                entityOperationService.merge(parsedEntities, sub.getOrNull())
+            val mergeOperationResult = entityOperationService.merge(parsedEntities, sub.getOrNull())
             batchOperationResult.errors.addAll(mergeOperationResult.errors)
             batchOperationResult.success.addAll(mergeOperationResult.success)
         }

search-service/src/test/kotlin/com/egm/stellio/search/entity/service/EntityOperationServiceTests.kt

@@ -40,9 +40,6 @@ class EntityOperationServiceTests {
     @MockkBean
     private lateinit var entityService: EntityService
 
-    @MockkBean(relaxed = true)
-    private lateinit var entityAttributeService: EntityAttributeService
-
     @MockkBean
     private lateinit var entityQueryService: EntityQueryService
 
@@ -282,11 +279,8 @@ class EntityOperationServiceTests {
     @Test
     fun `batch replace should ask to replace entities`() = runTest {
         coEvery {
-            entityAttributeService.deleteAttributes(any())
+            entityService.replaceEntity(any(), any(), any(), any())
         } returns Unit.right()
-        coEvery {
-            entityService.appendAttributes(any(), any(), any(), any())
-        } returns EMPTY_UPDATE_RESULT.right()
 
         val batchOperationResult = entityOperationService.replace(
             listOf(
@@ -302,14 +296,7 @@ class EntityOperationServiceTests {
         )
         assertTrue(batchOperationResult.errors.isEmpty())
 
-        coVerify { entityAttributeService.deleteAttributes(firstEntityURI) }
-        coVerify { entityAttributeService.deleteAttributes(secondEntityURI) }
-        coVerify {
-            entityService.appendAttributes(eq(firstEntityURI), any(), false, sub)
-        }
-        coVerify {
-            entityService.appendAttributes(eq(secondEntityURI), any(), false, sub)
-        }
+        coVerify { entityService.replaceEntity(firstEntityURI, any(), any(), any()) }
     }
 
     @Test
@@ -457,7 +444,8 @@ class EntityOperationServiceTests {
                 firstExpandedEntity to firstEntity,
                 secondExpandedEntity to secondEntity
             ),
-            null,
+            disallowOverwrite = false,
+            updateMode = false,
             sub
         )
 
@@ -484,7 +472,8 @@ class EntityOperationServiceTests {
                 firstExpandedEntity to firstEntity,
                 secondExpandedEntity to secondEntity
             ),
-            "update",
+            disallowOverwrite = false,
+            updateMode = true,
             sub
         )
 
@@ -523,7 +512,8 @@ class EntityOperationServiceTests {
                 firstExpandedEntity to firstEntity,
                 secondExpandedEntity to secondEntity
             ),
-            "update",
+            disallowOverwrite = false,
+            updateMode = true,
             sub
         )
 
@@ -567,7 +557,8 @@ class EntityOperationServiceTests {
                 firstExpandedEntity to firstEntity,
                 secondExpandedEntity to secondEntity
             ),
-            null,
+            disallowOverwrite = false,
+            updateMode = false,
             sub
         )
 

search-service/src/test/kotlin/com/egm/stellio/search/entity/web/EntityOperationHandlerTests.kt

@@ -112,6 +112,8 @@ class EntityOperationHandlerTests {
 
     private val batchCreateEndpoint = "/ngsi-ld/v1/entityOperations/create"
     private val batchUpsertWithUpdateEndpoint = "/ngsi-ld/v1/entityOperations/upsert?options=update"
+    private val batchUpsertWithUpdateAndNoOverwriteEndpoint =
+        "/ngsi-ld/v1/entityOperations/upsert?options=update,noOverwrite"
     private val batchUpdateEndpoint = "/ngsi-ld/v1/entityOperations/update"
     private val batchUpdateEndpointWithNoOverwriteOption = "/ngsi-ld/v1/entityOperations/update?options=noOverwrite"
     private val batchDeleteEndpoint = "/ngsi-ld/v1/entityOperations/delete"
@@ -324,7 +326,7 @@ class EntityOperationHandlerTests {
             updatedEntitiesIds.map { BatchEntitySuccess(it, mockkClass(UpdateResult::class)) }.toMutableList()
         )
 
-        coEvery { entityOperationService.upsert(any(), any(), any()) } returns (
+        coEvery { entityOperationService.upsert(any(), any(), any(), any()) } returns (
             updatedBatchResult to createdEntitiesIds
             )
 
@@ -341,7 +343,7 @@ class EntityOperationHandlerTests {
     fun `upsert batch entity should return a 204 if it has only updated existing entities`() = runTest {
         val jsonLdFile = validJsonFile
         coEvery {
-            entityOperationService.upsert(any(), any(), any())
+            entityOperationService.upsert(any(), any(), any(), any())
         } returns (BatchOperationResult(success = mutableListOf(), errors = mutableListOf()) to emptyList())
 
         webClient.post()
@@ -360,7 +362,7 @@ class EntityOperationHandlerTests {
             BatchEntityError(dissolvedOxygenSensorUri, arrayListOf("Update unexpectedly failed."))
         )
 
-        coEvery { entityOperationService.upsert(any(), any(), any()) } returns (
+        coEvery { entityOperationService.upsert(any(), any(), any(), any()) } returns (
             BatchOperationResult(
                 arrayListOf(BatchEntitySuccess(deviceUri, mockkClass(UpdateResult::class))),
                 errors
@@ -391,6 +393,24 @@ class EntityOperationHandlerTests {
             )
     }
 
+    @Test
+    fun `upsert batch entity should update entities without overwriting if update and noOverwrite options`() = runTest {
+        coEvery {
+            entityOperationService.upsert(any(), any(), any(), any())
+        } returns (BatchOperationResult(success = mutableListOf(), errors = mutableListOf()) to emptyList())
+
+        webClient.post()
+            .uri(batchUpsertWithUpdateAndNoOverwriteEndpoint)
+            .bodyValue(validJsonFile)
+            .exchange()
+            .expectStatus().isNoContent
+            .expectBody().isEmpty
+
+        coVerify(exactly = 1) {
+            entityOperationService.upsert(any(), true, true, any())
+        }
+    }
+
     @Test
     fun `upsert batch entity should return a 400 if JSON-LD payload is not correct`() {
         shouldReturn400WithBadPayload("upsert")

shared/src/main/kotlin/com/egm/stellio/shared/queryparameter/OptionsValue.kt

@@ -3,5 +3,7 @@ package com.egm.stellio.shared.queryparameter
 enum class OptionsValue(val value: String) {
     SYS_ATTRS("sysAttrs"),
     KEY_VALUES("keyValues"),
-    NO_OVERWRITE("noOverwrite")
+    NO_OVERWRITE("noOverwrite"),
+    UPDATE_MODE("update"),
+    REPLACE_MODE("replace")
 }