Merge pull request #1059 from stellio-hub/fix/ignore-existing-attrs-i…

…n-update-operation fix(core): non existing attrs in append or update operations should be ignored
stellio-hub · Dec 18, 2023 · 577ad56 · 577ad56
2 parents 3faa8e7 + eb1d51a
commit 577ad56
Show file tree

Hide file tree

Showing 11 changed files with 61 additions and 73 deletions.
diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/config/WebConfig.kt b/search-service/src/main/kotlin/com/egm/stellio/search/config/WebConfig.kt
@@ -3,7 +3,6 @@ package com.egm.stellio.search.config
 import org.springframework.context.annotation.Configuration
 import org.springframework.http.codec.ServerCodecConfigurer
 import org.springframework.web.reactive.config.EnableWebFlux
-import org.springframework.web.reactive.config.PathMatchConfigurer
 import org.springframework.web.reactive.config.WebFluxConfigurer
 
 @Configuration
@@ -14,8 +13,4 @@ class WebConfig(private val searchProperties: SearchProperties) : WebFluxConfigu
         configurer.defaultCodecs().enableLoggingRequestDetails(true)
         configurer.defaultCodecs().maxInMemorySize(searchProperties.payloadMaxBodySize)
     }
-
-    override fun configurePathMatching(configurer: PathMatchConfigurer) {
-        configurer.setUseTrailingSlashMatch(true)
-    }
 }
diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/model/UpdateResult.kt b/search-service/src/main/kotlin/com/egm/stellio/search/model/UpdateResult.kt
@@ -20,6 +20,10 @@ data class UpdateResult(
             updated = this.updated.plus(other.updated),
             notUpdated = this.notUpdated.plus(other.notUpdated)
         )
+
+    @JsonIgnore
+    fun hasSuccessfulUpdate(): Boolean =
+        this.updated.isNotEmpty()
 }
 
 val EMPTY_UPDATE_RESULT: UpdateResult = UpdateResult(emptyList(), emptyList())
@@ -48,7 +52,8 @@ data class UpdateAttributeResult(
         this.updateOperationResult in listOf(
             UpdateOperationResult.APPENDED,
             UpdateOperationResult.REPLACED,
-            UpdateOperationResult.UPDATED
+            UpdateOperationResult.UPDATED,
+            UpdateOperationResult.IGNORED
         )
 }
 
@@ -62,9 +67,6 @@ enum class UpdateOperationResult {
     fun isSuccessResult(): Boolean = listOf(APPENDED, REPLACED, UPDATED).contains(this)
 }
 
-fun UpdateResult.hasSuccessfulUpdate(): Boolean =
-    this.updated.isNotEmpty()
-
 fun updateResultFromDetailedResult(updateStatuses: List<UpdateAttributeResult>): UpdateResult {
     val updated = updateStatuses.filter { it.isSuccessfullyUpdated() }
         .map { UpdatedDetails(it.attributeName, it.datasetId, it.updateOperationResult) }

diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/service/EntityPayloadService.kt b/search-service/src/main/kotlin/com/egm/stellio/search/service/EntityPayloadService.kt
@@ -492,33 +492,32 @@ class EntityPayloadService(
         expandedAttributes: ExpandedAttributes,
         disallowOverwrite: Boolean,
         sub: Sub?
-    ): Either<APIException, UpdateResult> =
-        either {
-            val (coreAttrs, otherAttrs) =
-                expandedAttributes.toList().partition { JSONLD_EXPANDED_ENTITY_SPECIFIC_MEMBERS.contains(it.first) }
-            val createdAt = ZonedDateTime.now(ZoneOffset.UTC)
+    ): Either<APIException, UpdateResult> = either {
+        val (coreAttrs, otherAttrs) =
+            expandedAttributes.toList().partition { JSONLD_EXPANDED_ENTITY_SPECIFIC_MEMBERS.contains(it.first) }
+        val createdAt = ZonedDateTime.now(ZoneOffset.UTC)
 
-            val operationType =
-                if (disallowOverwrite) APPEND_ATTRIBUTES
-                else APPEND_ATTRIBUTES_OVERWRITE_ALLOWED
-            val coreUpdateResult = updateCoreAttributes(entityUri, coreAttrs, createdAt, operationType).bind()
-            val attrsUpdateResult = temporalEntityAttributeService.appendEntityAttributes(
-                entityUri,
-                otherAttrs.toMap().toNgsiLdAttributes().bind(),
-                expandedAttributes,
-                disallowOverwrite,
-                createdAt,
-                sub
-            ).bind()
+        val operationType =
+            if (disallowOverwrite) APPEND_ATTRIBUTES
+            else APPEND_ATTRIBUTES_OVERWRITE_ALLOWED
+        val coreUpdateResult = updateCoreAttributes(entityUri, coreAttrs, createdAt, operationType).bind()
+        val attrsUpdateResult = temporalEntityAttributeService.appendEntityAttributes(
+            entityUri,
+            otherAttrs.toMap().toNgsiLdAttributes().bind(),
+            expandedAttributes,
+            disallowOverwrite,
+            createdAt,
+            sub
+        ).bind()
 
-            val updateResult = coreUpdateResult.mergeWith(attrsUpdateResult)
-            // update modifiedAt in entity if at least one attribute has been added
-            if (updateResult.hasSuccessfulUpdate()) {
-                val teas = temporalEntityAttributeService.getForEntity(entityUri, emptySet())
-                updateState(entityUri, createdAt, teas).bind()
-            }
-            updateResult
+        val updateResult = coreUpdateResult.mergeWith(attrsUpdateResult)
+        // update modifiedAt in entity if at least one attribute has been added
+        if (updateResult.hasSuccessfulUpdate()) {
+            val teas = temporalEntityAttributeService.getForEntity(entityUri, emptySet())
+            updateState(entityUri, createdAt, teas).bind()
         }
+        updateResult
+    }
 
     @Transactional
     suspend fun updateAttributes(

diff --git a/...-service/src/main/kotlin/com/egm/stellio/search/service/TemporalEntityAttributeService.kt b/...-service/src/main/kotlin/com/egm/stellio/search/service/TemporalEntityAttributeService.kt
@@ -689,7 +689,7 @@ class TemporalEntityAttributeService(
                 UpdateAttributeResult(
                     attributeName,
                     datasetId,
-                    UpdateOperationResult.IGNORED,
+                    UpdateOperationResult.FAILED,
                     "Unknown attribute $attributeName with datasetId $datasetId in entity $entityId"
                 )
             }
@@ -819,7 +819,7 @@ class TemporalEntityAttributeService(
                 UpdateAttributeResult(
                     attributeName,
                     datasetId,
-                    UpdateOperationResult.IGNORED,
+                    UpdateOperationResult.FAILED,
                     "Unknown attribute $attributeName with datasetId $datasetId in entity $entityId"
                 )
             } else {

diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/web/EntityHandler.kt b/search-service/src/main/kotlin/com/egm/stellio/search/web/EntityHandler.kt
@@ -5,7 +5,6 @@ import arrow.core.left
 import arrow.core.raise.either
 import arrow.core.right
 import com.egm.stellio.search.authorization.AuthorizationService
-import com.egm.stellio.search.model.hasSuccessfulUpdate
 import com.egm.stellio.search.service.EntityEventService
 import com.egm.stellio.search.service.EntityPayloadService
 import com.egm.stellio.search.service.QueryService
@@ -288,10 +287,6 @@ class EntityHandler(
         { it }
     )
 
-    @DeleteMapping("/", "")
-    fun handleMissingEntityIdOnDelete(): ResponseEntity<*> =
-        missingPathErrorResponse("Missing entity id when trying to delete an entity")
-
     /**
      * Implements 6.6.3.1 - Append Entity Attributes
      *
@@ -489,7 +484,7 @@ class EntityHandler(
         { it }
     )
 
-    @DeleteMapping("/attrs/{attrId}", "/{entityId}/attrs")
+    @DeleteMapping("/attrs/{attrId}")
     fun handleMissingEntityIdOrAttributeOnDeleteAttribute(): ResponseEntity<*> =
         missingPathErrorResponse("Missing entity id or attribute id when trying to delete an attribute")
 

diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/web/TemporalEntityHandler.kt b/search-service/src/main/kotlin/com/egm/stellio/search/web/TemporalEntityHandler.kt
@@ -247,14 +247,11 @@ class TemporalEntityHandler(
 
     @PatchMapping(
         "/attrs/{attrId}/{instanceId}",
-        "/{entityId}/attrs/{instanceId}",
         "/attrs/{instanceId}",
-        "/{entityId}/attrs",
-        "/attrs"
     )
     fun handleMissingParametersOnModifyInstanceTemporal(): ResponseEntity<*> =
         missingPathErrorResponse(
-            "Missing some parameter(entity id, attribute id, instance id) when trying to modify temporal entity"
+            "Missing some parameter (entity id, attribute id, instance id) when trying to modify temporal entity"
         )
 
     /**
@@ -277,10 +274,6 @@ class TemporalEntityHandler(
         { it }
     )
 
-    @DeleteMapping("/", "")
-    fun handleMissingEntityIdOnDeleteTemporalEntity(): ResponseEntity<*> =
-        missingPathErrorResponse("Missing entity id when trying to delete temporal entity")
-
     /**
      * Implements 6.21.3.1 - Delete Attribute from Temporal Representation of an Entity
      */

diff --git a/search-service/src/test/kotlin/com/egm/stellio/search/web/TemporalEntityHandlerTests.kt b/search-service/src/test/kotlin/com/egm/stellio/search/web/TemporalEntityHandlerTests.kt
@@ -1256,20 +1256,11 @@ class TemporalEntityHandlerTests {
     }
 
     @Test
-    fun `delete temporal entity should return a 400 if entity id is missing`() {
+    fun `delete temporal entity should return a 405 if entity id is missing`() {
         webClient.delete()
             .uri("/ngsi-ld/v1/temporal/entities/")
             .exchange()
-            .expectStatus().isBadRequest
-            .expectBody().json(
-                """
-                {
-                    "type":"https://uri.etsi.org/ngsi-ld/errors/BadRequestData",
-                    "title":"The request includes input data which does not meet the requirements of the operation",
-                    "detail":"Missing entity id when trying to delete temporal entity"
-                }
-                """.trimIndent()
-            )
+            .expectStatus().isEqualTo(HttpStatus.METHOD_NOT_ALLOWED)
     }
 
     @Test

diff --git a/shared/src/main/kotlin/com/egm/stellio/shared/config/TrailingSlashRedirectFilter.kt b/shared/src/main/kotlin/com/egm/stellio/shared/config/TrailingSlashRedirectFilter.kt
@@ -0,0 +1,23 @@
+package com.egm.stellio.shared.config
+
+import org.springframework.http.server.reactive.ServerHttpRequest
+import org.springframework.stereotype.Component
+import org.springframework.web.server.ServerWebExchange
+import org.springframework.web.server.WebFilter
+import org.springframework.web.server.WebFilterChain
+import reactor.core.publisher.Mono
+
+@Component
+class TrailingSlashRedirectFilter : WebFilter {
+
+    override fun filter(exchange: ServerWebExchange, chain: WebFilterChain): Mono<Void> {
+        val request: ServerHttpRequest = exchange.request
+        val path: String = request.path.value()
+        if (path.endsWith("/")) {
+            val newPath = path.removeSuffix("/")
+            val newRequest: ServerHttpRequest = request.mutate().path(newPath).build()
+            return chain.filter(exchange.mutate().request(newRequest).build())
+        }
+        return chain.filter(exchange)
+    }
+}
diff --git a/shared/src/main/kotlin/com/egm/stellio/shared/web/ExceptionHandler.kt b/shared/src/main/kotlin/com/egm/stellio/shared/web/ExceptionHandler.kt
@@ -10,6 +10,7 @@ import org.springframework.http.ProblemDetail
 import org.springframework.http.ResponseEntity
 import org.springframework.web.bind.annotation.ExceptionHandler
 import org.springframework.web.bind.annotation.RestControllerAdvice
+import org.springframework.web.server.MethodNotAllowedException
 import org.springframework.web.server.NotAcceptableStatusException
 import org.springframework.web.server.UnsupportedMediaTypeStatusException
 
@@ -65,6 +66,8 @@ class ExceptionHandler {
                 HttpStatus.NOT_ACCEPTABLE,
                 NotAcceptableResponse(cause.message)
             )
+            is MethodNotAllowedException ->
+                ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED).body(cause.body)
             is NonexistentTenantException -> generateErrorResponse(
                 HttpStatus.NOT_FOUND,
                 NonexistentTenantResponse(cause.message)

diff --git a/subscription-service/src/main/kotlin/com/egm/stellio/subscription/config/WebConfig.kt b/subscription-service/src/main/kotlin/com/egm/stellio/subscription/config/WebConfig.kt
@@ -3,7 +3,6 @@ package com.egm.stellio.subscription.config
 import org.springframework.context.annotation.Configuration
 import org.springframework.http.codec.ServerCodecConfigurer
 import org.springframework.web.reactive.config.EnableWebFlux
-import org.springframework.web.reactive.config.PathMatchConfigurer
 import org.springframework.web.reactive.config.WebFluxConfigurer
 
 @Configuration
@@ -13,8 +12,4 @@ class WebConfig : WebFluxConfigurer {
     override fun configureHttpMessageCodecs(configurer: ServerCodecConfigurer) {
         configurer.defaultCodecs().enableLoggingRequestDetails(true)
     }
-
-    override fun configurePathMatching(configurer: PathMatchConfigurer) {
-        configurer.setUseTrailingSlashMatch(true)
-    }
 }
diff --git a/subscription-service/src/main/kotlin/com/egm/stellio/subscription/web/SubscriptionHandler.kt b/subscription-service/src/main/kotlin/com/egm/stellio/subscription/web/SubscriptionHandler.kt
@@ -164,10 +164,6 @@ class SubscriptionHandler(
         { it }
     )
 
-    @PatchMapping("/", "")
-    fun handleMissingIdOnUpdate(): ResponseEntity<*> =
-        missingPathErrorResponse("Missing id when trying to update a subscription")
-
     /**
      * Implements 6.11.3.3 - Delete Subscription
      */
@@ -185,10 +181,6 @@ class SubscriptionHandler(
         { it }
     )
 
-    @DeleteMapping("/", "")
-    fun handleMissingIdOnDelete(): ResponseEntity<*> =
-        missingPathErrorResponse("Missing id when trying to delete a subscription")
-
     private suspend fun checkSubscriptionExists(subscriptionId: URI): Either<APIException, Unit> =
         subscriptionService.exists(subscriptionId)
             .flatMap {