fix: PR comments + other changes

stellio-hub · Jan 3, 2025 · 06b7ec4 · 06b7ec4
1 parent 98e6891
commit 06b7ec4
Show file tree

Hide file tree

Showing 9 changed files with 125 additions and 22 deletions.
diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/temporal/util/TemporalQueryUtils.kt b/search-service/src/main/kotlin/com/egm/stellio/search/temporal/util/TemporalQueryUtils.kt
@@ -20,6 +20,7 @@ import com.egm.stellio.search.temporal.model.TemporalQuery.Timerel
 import com.egm.stellio.shared.config.ApplicationProperties
 import com.egm.stellio.shared.model.APIException
 import com.egm.stellio.shared.model.BadRequestDataException
+import com.egm.stellio.shared.queryparameter.FormatValue
 import com.egm.stellio.shared.queryparameter.QueryParameter
 import com.egm.stellio.shared.util.QueryParamValue
 import com.egm.stellio.shared.util.hasValueInQueryParam
@@ -42,18 +43,26 @@ fun composeTemporalEntitiesQueryFromGet(
         requestParams,
         contexts
     ).bind()
-
-    if (inQueryEntities)
+    var withTemporalValues = false
+    var withAggregatedValues = false
+    if (inQueryEntities) {
         entitiesQueryFromGet.validateMinimalQueryEntitiesParameters().bind()
+    }
     val optionsParam = Optional.ofNullable(requestParams.getFirst(QueryParameter.OPTIONS.key))
-    val formatParam = Optional.ofNullable(requestParams.getFirst(QueryParameter.FORMAT.key))
-    val withTemporalValues = 
-        hasValueInQueryParam(formatParam, QueryParamValue.TEMPORAL_VALUES) ||
-        hasValueInQueryParam(optionsParam, QueryParamValue.TEMPORAL_VALUES)
-
-        val withAggregatedValues = 
-            hasValueInQueryParam(formatParam, QueryParamValue.TEMPORAL_VALUES) ||
-            hasValueInQueryParam(optionsParam, QueryParamValue.TEMPORAL_VALUES)
+    val formatParam = requestParams.getFirst(QueryParameter.FORMAT.key)
+    when (formatParam) {
+        FormatValue.TEMPORAL_VALUES.value -> withTemporalValues = true
+        FormatValue.AGGREGATED_VALUES.value -> withAggregatedValues = true
+        else -> {
+            val hasTemporal = hasValueInQueryParam(optionsParam, QueryParamValue.TEMPORAL_VALUES)
+            val hasAggregated = hasValueInQueryParam(optionsParam, QueryParamValue.AGGREGATED_VALUES)
+            if (hasTemporal && hasAggregated) {
+                return BadRequestDataException("Only one temporal representation can be present").left()
+            }
+            withTemporalValues = hasTemporal
+            withAggregatedValues = hasAggregated
+        }
+    }
     val withAudit = hasValueInQueryParam(
         Optional.ofNullable(requestParams.getFirst(QueryParameter.OPTIONS.key)),
         QueryParamValue.AUDIT

diff --git a/...h-service/src/test/kotlin/com/egm/stellio/search/temporal/util/TemporalQueryUtilsTests.kt b/...h-service/src/test/kotlin/com/egm/stellio/search/temporal/util/TemporalQueryUtilsTests.kt
@@ -94,6 +94,27 @@ class TemporalQueryUtilsTests {
         }
     }
 
+    @Test
+    fun `it shouldn't validate the temporal query if both temporalValues and aggregatedValues are present`() = runTest {
+        val queryParams = gimmeTemporalEntitiesQueryParams()
+        queryParams.replace("options", listOf("aggregatedValues,temporalValues"))
+        queryParams.add("aggrMethods", "sum")
+        val pagination = mockkClass(ApplicationProperties.Pagination::class)
+        every { pagination.limitDefault } returns 30
+        every { pagination.limitMax } returns 100
+        every { pagination.temporalLimit } returns 100
+
+        composeTemporalEntitiesQueryFromGet(
+            pagination,
+            queryParams,
+            APIC_COMPOUND_CONTEXTS,
+            true
+        ).shouldFail {
+            assertInstanceOf(BadRequestDataException::class.java, it)
+            assertEquals("Only one temporal representation can be present", it.message)
+        }
+    }
+
     @Test
     fun `it should parse a valid temporal query`() = runTest {
         val queryParams = gimmeTemporalEntitiesQueryParams()

diff --git a/...service/src/test/kotlin/com/egm/stellio/search/temporal/web/TemporalEntityHandlerTests.kt b/...service/src/test/kotlin/com/egm/stellio/search/temporal/web/TemporalEntityHandlerTests.kt
@@ -425,6 +425,27 @@ class TemporalEntityHandlerTests : TemporalEntityHandlerTestCommon() {
             )
     }
 
+    @Test
+    fun `it should raise a 400 if temporalValues and aggregatedValues exist in options query param`() {
+        webClient.get()
+            .uri(
+                "/ngsi-ld/v1/temporal/entities/urn:ngsi-ld:Entity:01?" +
+                    "timerel=after&timeAt=2020-01-31T07:31:39Z&options=aggregatedValues," +
+                    "temporalValues&aggrPeriodDuration=PXD3N"
+            )
+            .exchange()
+            .expectStatus().isBadRequest
+            .expectBody().json(
+                """
+                {
+                    "type": "https://uri.etsi.org/ngsi-ld/errors/BadRequestData",
+                    "title": "Only one temporal representation can be present",
+                    "detail": "$DEFAULT_DETAIL"
+                } 
+                """
+            )
+    }
+
     @Test
     fun `it should return a 404 if temporal entity attribute does not exist`() {
         coEvery {

diff --git a/shared/src/main/kotlin/com/egm/stellio/shared/model/NgsiLdDataRepresentation.kt b/shared/src/main/kotlin/com/egm/stellio/shared/model/NgsiLdDataRepresentation.kt
@@ -29,11 +29,13 @@ data class NgsiLdDataRepresentation(
             acceptMediaType: MediaType
         ): NgsiLdDataRepresentation {
             val optionsParam = queryParams.getOrDefault(QueryParameter.OPTIONS.key, emptyList())
-            val formatParam = queryParams.getOrDefault(QueryParameter.FORMAT.key, emptyList())
+            val formatParam = queryParams.getFirst(QueryParameter.FORMAT.key)
             val attributeRepresentation = when {
-                formatParam.contains(FormatValue.KEY_VALUES.value) -> AttributeRepresentation.SIMPLIFIED
-                formatParam.contains(FormatValue.NORMALIZED.value) -> AttributeRepresentation.NORMALIZED
-                optionsParam.contains(FormatValue.KEY_VALUES.value) -> AttributeRepresentation.SIMPLIFIED
+                formatParam.equals(FormatValue.KEY_VALUES.value) ||
+                    formatParam.equals(FormatValue.SIMPLIFIED.value) -> AttributeRepresentation.SIMPLIFIED
+                formatParam.equals(FormatValue.NORMALIZED.value) -> AttributeRepresentation.NORMALIZED
+                optionsParam.contains(FormatValue.KEY_VALUES.value) ||
+                    optionsParam.contains(FormatValue.SIMPLIFIED.value) -> AttributeRepresentation.SIMPLIFIED
                 else -> AttributeRepresentation.NORMALIZED
             }
             val includeSysAttrs = optionsParam.contains(OptionsValue.SYS_ATTRS.value)

diff --git a/shared/src/main/kotlin/com/egm/stellio/shared/queryparameter/FormatValue.kt b/shared/src/main/kotlin/com/egm/stellio/shared/queryparameter/FormatValue.kt
@@ -2,6 +2,7 @@ package com.egm.stellio.shared.queryparameter
 
 enum class FormatValue(val value: String) {
     KEY_VALUES("keyValues"),
+    SIMPLIFIED("simplified"),
     NORMALIZED("normalized"),
     TEMPORAL_VALUES("temporalValues"),
     AGGREGATED_VALUES("aggregatedValues")

diff --git a/shared/src/main/kotlin/com/egm/stellio/shared/queryparameter/OptionsValue.kt b/shared/src/main/kotlin/com/egm/stellio/shared/queryparameter/OptionsValue.kt
@@ -2,7 +2,6 @@ package com.egm.stellio.shared.queryparameter
 
 enum class OptionsValue(val value: String) {
     SYS_ATTRS("sysAttrs"),
-    KEY_VALUES("keyValues"),
     NO_OVERWRITE("noOverwrite"),
     UPDATE_MODE("update"),
     REPLACE_MODE("replace")

diff --git a/shared/src/main/kotlin/com/egm/stellio/shared/util/ApiUtils.kt b/shared/src/main/kotlin/com/egm/stellio/shared/util/ApiUtils.kt
@@ -174,8 +174,8 @@ enum class QueryParamValue(val value: String) {
     AGGREGATED_VALUES("aggregatedValues")
 }
 
-fun hasValueInQueryParam(options: Optional<String>, queryParamValue: QueryParamValue): Boolean =
-    options
+fun hasValueInQueryParam(queryParam: Optional<String>, queryParamValue: QueryParamValue): Boolean =
+    queryParam
         .map { it.split(",") }
         .filter { it.any { option -> option == queryParamValue.value } }
         .isPresent

diff --git a/shared/src/test/kotlin/com/egm/stellio/shared/model/NgsiLdDataRepresentationTest.kt b/shared/src/test/kotlin/com/egm/stellio/shared/model/NgsiLdDataRepresentationTest.kt
@@ -0,0 +1,50 @@
+package com.egm.stellio.shared.model
+
+import com.egm.stellio.shared.model.NgsiLdDataRepresentation.Companion.parseRepresentations
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Test
+import org.springframework.http.MediaType
+import org.springframework.test.context.ActiveProfiles
+import org.springframework.util.LinkedMultiValueMap
+
+@ActiveProfiles("test")
+class NgsiLdDataRepresentationTest {
+
+    @Test
+    fun `it should return the attribute representation in the format query param when options exist`() {
+        val queryParams = LinkedMultiValueMap<String, String>()
+        queryParams.add("timerel", "after")
+        queryParams.add("timeAt", "2025-01-03T07:45:24Z")
+        queryParams.add("options", "simplified")
+        queryParams.add("format", "normalized")
+
+        val parsedRepresentation = parseRepresentations(queryParams, MediaType.APPLICATION_JSON)
+
+        assertEquals(AttributeRepresentation.NORMALIZED, parsedRepresentation.attributeRepresentation)
+    }
+
+    @Test
+    fun `it should return the attribute representation in the options query param when no format is given`() {
+        val queryParams = LinkedMultiValueMap<String, String>()
+        queryParams.add("timerel", "after")
+        queryParams.add("timeAt", "2025-01-03T07:45:24Z")
+        queryParams.add("options", "simplified")
+
+        val parsedRepresentation = parseRepresentations(queryParams, MediaType.APPLICATION_JSON)
+
+        assertEquals(AttributeRepresentation.SIMPLIFIED, parsedRepresentation.attributeRepresentation)
+    }
+
+    @Test
+    fun `it should return the attribute representation in the first format query param`() {
+        val queryParams = LinkedMultiValueMap<String, String>()
+        queryParams.add("timerel", "after")
+        queryParams.add("timeAt", "2025-01-03T07:45:24Z")
+        queryParams.add("format", "simplified")
+        queryParams.add("format", "normalized")
+
+        val parsedRepresentation = parseRepresentations(queryParams, MediaType.APPLICATION_JSON)
+
+        assertEquals(AttributeRepresentation.SIMPLIFIED, parsedRepresentation.attributeRepresentation)
+    }
+}
diff --git a/shared/src/test/kotlin/com/egm/stellio/shared/util/ApiUtilsTests.kt b/shared/src/test/kotlin/com/egm/stellio/shared/util/ApiUtilsTests.kt
@@ -32,27 +32,27 @@ class ApiUtilsTests {
     }
 
     @Test
-    fun `it should not find a value if there is no options query param`() {
+    fun `it should not find a value if there is no format query param`() {
         assertFalse(hasValueInQueryParam(Optional.empty(), TEMPORAL_VALUES))
     }
 
     @Test
-    fun `it should not find a value if it is not in a single value options query param`() {
+    fun `it should not find a value if it is not in a single value format query param`() {
         assertFalse(hasValueInQueryParam(Optional.of("one"), TEMPORAL_VALUES))
     }
 
     @Test
-    fun `it should not find a value if it is not in a multi value options query param`() {
+    fun `it should not find a value if it is not in a multi value format query param`() {
         assertFalse(hasValueInQueryParam(Optional.of("one,two"), TEMPORAL_VALUES))
     }
 
     @Test
-    fun `it should find a value if it is in a single value options query param`() {
+    fun `it should find a value if it is in a single value format query param`() {
         assertTrue(hasValueInQueryParam(Optional.of("temporalValues"), TEMPORAL_VALUES))
     }
 
     @Test
-    fun `it should find a value if it is in a multi value options query param`() {
+    fun `it should find a value if it is in a multi value format query param`() {
         assertTrue(hasValueInQueryParam(Optional.of("one,temporalValues"), TEMPORAL_VALUES))
     }