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 index 5855d2193..7a5caf046 100644 --- 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 @@ -190,7 +190,7 @@ fun buildTimerelAndTime( "'timerel' and 'time' must be used in conjunction".left() } -fun extractTemporalRepresentation( +private fun extractTemporalRepresentation( queryParams: MultiValueMap ): Either = either { val optionsParam = Optional.ofNullable(queryParams.getFirst(QueryParameter.OPTIONS.key)) @@ -198,18 +198,21 @@ fun extractTemporalRepresentation( if (formatParam != null) { return TemporalRepresentation.fromString(formatParam) } else { - if (!optionsParam.isEmpty) { + return if (!optionsParam.isEmpty) { val hasTemporal = hasValueInOptionsParam(optionsParam, OptionsValue.TEMPORAL_VALUES).bind() val hasAggregated = hasValueInOptionsParam(optionsParam, OptionsValue.AGGREGATED_VALUES).bind() when { hasTemporal && hasAggregated -> - return BadRequestDataException("Only one temporal representation can be present").left() - hasTemporal -> return TemporalRepresentation.TEMPORAL_VALUES.right() - hasAggregated -> return TemporalRepresentation.AGGREGATED_VALUES.right() - else -> return TemporalRepresentation.NORMALIZED.right() + BadRequestDataException( + "Found different temporal representations in options query parameter," + + " only one can be provided" + ).left() + hasTemporal -> TemporalRepresentation.TEMPORAL_VALUES.right() + hasAggregated -> TemporalRepresentation.AGGREGATED_VALUES.right() + else -> TemporalRepresentation.NORMALIZED.right() } } else - return TemporalRepresentation.NORMALIZED.right() + TemporalRepresentation.NORMALIZED.right() } } diff --git a/search-service/src/test/kotlin/com/egm/stellio/search/temporal/service/AttributeInstanceServiceTests.kt b/search-service/src/test/kotlin/com/egm/stellio/search/temporal/service/AttributeInstanceServiceTests.kt index 02083fb4b..e2277aafb 100644 --- a/search-service/src/test/kotlin/com/egm/stellio/search/temporal/service/AttributeInstanceServiceTests.kt +++ b/search-service/src/test/kotlin/com/egm/stellio/search/temporal/service/AttributeInstanceServiceTests.kt @@ -200,8 +200,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = now.minusHours(1) - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) .shouldSucceedWith { @@ -226,8 +225,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer timerel = Timerel.AFTER, timeAt = now.minusHours(1), timeproperty = AttributeInstance.TemporalProperty.CREATED_AT - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) .shouldSucceedWith { @@ -252,8 +250,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer timerel = Timerel.AFTER, timeAt = now.minusHours(1), timeproperty = AttributeInstance.TemporalProperty.CREATED_AT - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) .shouldSucceedWith { @@ -278,8 +275,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer timerel = Timerel.AFTER, timeAt = now.minusHours(1), timeproperty = AttributeInstance.TemporalProperty.MODIFIED_AT - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) .shouldSucceedWith { @@ -298,8 +294,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = now.minusHours(1) - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) .shouldSucceedWith { @@ -315,7 +310,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer } attributeInstanceService.search( - gimmeTemporalEntitiesQuery(buildDefaultTestTemporalQuery(), TemporalRepresentation.NORMALIZED), + gimmeTemporalEntitiesQuery(buildDefaultTestTemporalQuery()), incomingAttribute ) .shouldSucceedWith { @@ -415,8 +410,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer timerel = Timerel.AFTER, timeAt = ZonedDateTime.parse("2022-07-01T00:00:00Z"), instanceLimit = 5 - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) @@ -442,8 +436,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer timeAt = ZonedDateTime.parse("2022-07-01T00:00:00Z"), endTimeAt = ZonedDateTime.parse("2022-07-05T00:00:00Z"), instanceLimit = 5 - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) @@ -470,8 +463,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer timeAt = now.minusHours(1), instanceLimit = 5, lastN = 5 - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) .shouldSucceedWith { @@ -533,8 +525,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = now.minusHours(1) - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) .shouldSucceedWith { results -> @@ -556,8 +547,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = now.minusHours(1) - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search( temporalEntitiesQuery, @@ -578,8 +568,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = now.plusHours(1) - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) .shouldSucceedWith { @@ -820,8 +809,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = ZonedDateTime.parse("1970-01-01T00:00:00Z") - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.modifyAttributeInstance( @@ -859,8 +847,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = ZonedDateTime.parse("1970-01-01T00:00:00Z") - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.modifyAttributeInstance( @@ -899,8 +886,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = ZonedDateTime.parse("1970-01-01T00:00:00Z") - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.modifyAttributeInstance( @@ -944,8 +930,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = ZonedDateTime.parse("1970-01-01T00:00:00Z") - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.modifyAttributeInstance( @@ -984,7 +969,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer ).shouldSucceed() attributeInstanceService.search( - gimmeTemporalEntitiesQuery(buildDefaultTestTemporalQuery(), TemporalRepresentation.NORMALIZED), + gimmeTemporalEntitiesQuery(buildDefaultTestTemporalQuery()), incomingAttribute ) .shouldSucceedWith { @@ -1058,8 +1043,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = now.minusHours(1) - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, incomingAttribute) .shouldSucceedWith { assertThat(it).isEmpty() } @@ -1069,8 +1053,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer timerel = Timerel.AFTER, timeAt = now.minusHours(1), timeproperty = AttributeInstance.TemporalProperty.CREATED_AT - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesAuditQuery, incomingAttribute) @@ -1096,8 +1079,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = now.minusHours(1) - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, outgoingAttribute) .shouldSucceedWith { assertThat(it).hasSize(5) } @@ -1124,8 +1106,7 @@ class AttributeInstanceServiceTests : WithTimescaleContainer, WithKafkaContainer buildDefaultTestTemporalQuery( timerel = Timerel.AFTER, timeAt = now.minusHours(1) - ), - TemporalRepresentation.NORMALIZED + ) ) attributeInstanceService.search(temporalEntitiesQuery, outgoingAttribute) .shouldSucceedWith { assertThat(it).hasSize(5) } diff --git a/search-service/src/test/kotlin/com/egm/stellio/search/temporal/util/TemporalQueryUtilsTests.kt b/search-service/src/test/kotlin/com/egm/stellio/search/temporal/util/TemporalQueryUtilsTests.kt index 977aa8ebc..7008d1897 100644 --- a/search-service/src/test/kotlin/com/egm/stellio/search/temporal/util/TemporalQueryUtilsTests.kt +++ b/search-service/src/test/kotlin/com/egm/stellio/search/temporal/util/TemporalQueryUtilsTests.kt @@ -111,7 +111,10 @@ class TemporalQueryUtilsTests { true ).shouldFail { assertInstanceOf(BadRequestDataException::class.java, it) - assertEquals("Only one temporal representation can be present", it.message) + assertEquals( + "Found different temporal representations in options query parameter, only one can be provided", + it.message + ) } } @@ -151,7 +154,7 @@ class TemporalQueryUtilsTests { true ).shouldFail { assertInstanceOf(InvalidRequestException::class.java, it) - assertEquals("'invalidOptions' is not a valid options value", it.message) + assertEquals("'invalidOptions' is not a valid value for the options query parameter", it.message) } } diff --git a/search-service/src/test/kotlin/com/egm/stellio/search/temporal/web/TemporalEntityHandlerTests.kt b/search-service/src/test/kotlin/com/egm/stellio/search/temporal/web/TemporalEntityHandlerTests.kt index ed1d3c901..a4d811227 100644 --- a/search-service/src/test/kotlin/com/egm/stellio/search/temporal/web/TemporalEntityHandlerTests.kt +++ b/search-service/src/test/kotlin/com/egm/stellio/search/temporal/web/TemporalEntityHandlerTests.kt @@ -440,7 +440,7 @@ class TemporalEntityHandlerTests : TemporalEntityHandlerTestCommon() { """ { "type": "https://uri.etsi.org/ngsi-ld/errors/BadRequestData", - "title": "Only one temporal representation can be present", + "title": "Found different temporal representations in options query parameter, only one can be provided", "detail": "$DEFAULT_DETAIL" } """ @@ -482,7 +482,7 @@ class TemporalEntityHandlerTests : TemporalEntityHandlerTestCommon() { """ { "type": "https://uri.etsi.org/ngsi-ld/errors/InvalidRequest", - "title": "'invalidOptions' is not a valid options value", + "title": "'invalidOptions' is not a valid value for the options query parameter", "detail": "$DEFAULT_DETAIL" } """ 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 index 8ae1a3363..21b49457a 100644 --- a/shared/src/main/kotlin/com/egm/stellio/shared/model/NgsiLdDataRepresentation.kt +++ b/shared/src/main/kotlin/com/egm/stellio/shared/model/NgsiLdDataRepresentation.kt @@ -1,7 +1,6 @@ package com.egm.stellio.shared.model import arrow.core.Either -import arrow.core.left import arrow.core.raise.either import arrow.core.right import com.egm.stellio.shared.queryparameter.FormatValue @@ -34,23 +33,20 @@ data class NgsiLdDataRepresentation( ): Either = either { val optionsParam = queryParams.getOrDefault(QueryParameter.OPTIONS.key, emptyList()) .flatMap { it.split(",") } - val formatParam = queryParams.getFirst(QueryParameter.FORMAT.key) - if (formatParam != null && FormatValue.fromString(formatParam) == null) { - return InvalidRequestException("'$formatParam' is not a valid format value").left() - } - - optionsParam.forEach { option -> - OptionsValue.fromString(option).bind() + .map { OptionsValue.fromString(it).bind() } + val formatParam = queryParams.getFirst(QueryParameter.FORMAT.key)?.let { + FormatValue.fromString(it).bind() } val attributeRepresentation = when { - formatParam == FormatValue.KEY_VALUES.value || - formatParam == FormatValue.SIMPLIFIED.value -> AttributeRepresentation.SIMPLIFIED - formatParam == FormatValue.NORMALIZED.value -> AttributeRepresentation.NORMALIZED - optionsParam.contains(FormatValue.KEY_VALUES.value) || - optionsParam.contains(FormatValue.SIMPLIFIED.value) -> AttributeRepresentation.SIMPLIFIED + formatParam == FormatValue.KEY_VALUES || formatParam == FormatValue.SIMPLIFIED -> + AttributeRepresentation.SIMPLIFIED + formatParam == FormatValue.NORMALIZED -> + AttributeRepresentation.NORMALIZED + optionsParam.contains(OptionsValue.KEY_VALUES) || optionsParam.contains(OptionsValue.SIMPLIFIED) -> + AttributeRepresentation.SIMPLIFIED else -> AttributeRepresentation.NORMALIZED } - val includeSysAttrs = optionsParam.contains(OptionsValue.SYS_ATTRS.value) + val includeSysAttrs = optionsParam.contains(OptionsValue.SYS_ATTRS) val languageFilter = queryParams.getFirst(QueryParameter.LANG.key) val entityRepresentation = EntityRepresentation.forMediaType(acceptMediaType) val geometryProperty = 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 index 0565a3c61..17af1cee9 100644 --- a/shared/src/main/kotlin/com/egm/stellio/shared/queryparameter/FormatValue.kt +++ b/shared/src/main/kotlin/com/egm/stellio/shared/queryparameter/FormatValue.kt @@ -1,5 +1,11 @@ package com.egm.stellio.shared.queryparameter +import arrow.core.Either +import arrow.core.left +import arrow.core.raise.either +import com.egm.stellio.shared.model.APIException +import com.egm.stellio.shared.model.InvalidRequestException + enum class FormatValue(val value: String) { KEY_VALUES("keyValues"), SIMPLIFIED("simplified"), @@ -7,7 +13,9 @@ enum class FormatValue(val value: String) { TEMPORAL_VALUES("temporalValues"), AGGREGATED_VALUES("aggregatedValues"); companion object { - fun fromString(key: String): FormatValue? = + fun fromString(key: String): Either = either { FormatValue.entries.find { it.value == key } + ?: return InvalidRequestException("'$key' is not a valid value for the format query parameter").left() + } } } 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 index 25fa030b7..3b551152a 100644 --- a/shared/src/main/kotlin/com/egm/stellio/shared/queryparameter/OptionsValue.kt +++ b/shared/src/main/kotlin/com/egm/stellio/shared/queryparameter/OptionsValue.kt @@ -20,7 +20,7 @@ enum class OptionsValue(val value: String) { companion object { fun fromString(key: String): Either = either { OptionsValue.entries.find { it.value == key } - ?: return InvalidRequestException("'$key' is not a valid options value").left() + ?: return InvalidRequestException("'$key' is not a valid value for the options query parameter").left() } } } 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 index 37c48968f..73fb41df4 100644 --- a/shared/src/main/kotlin/com/egm/stellio/shared/util/ApiUtils.kt +++ b/shared/src/main/kotlin/com/egm/stellio/shared/util/ApiUtils.kt @@ -170,17 +170,14 @@ internal fun canExpandJsonLdKeyFromCore(contexts: List): Boolean { } fun hasValueInOptionsParam( - queryParam: Optional, - optionsParamValue: OptionsValue + optionsParam: Optional, + optionsValue: OptionsValue ): Either = either { - val optionsValue = queryParam + optionsParam .map { it.split(",") } .orElse(emptyList()) - - optionsValue.forEach { option -> - OptionsValue.fromString(option).bind() - } - optionsValue.any { option -> option == optionsParamValue.value } + .map { OptionsValue.fromString(it).bind() } + .any { it == optionsValue } } fun parseQueryParameter(queryParam: String?): Set = diff --git a/shared/src/test/kotlin/com/egm/stellio/shared/model/NgsiLdDataRepresentationTests.kt b/shared/src/test/kotlin/com/egm/stellio/shared/model/NgsiLdDataRepresentationTests.kt index 52dcaeb62..f95a7831f 100644 --- a/shared/src/test/kotlin/com/egm/stellio/shared/model/NgsiLdDataRepresentationTests.kt +++ b/shared/src/test/kotlin/com/egm/stellio/shared/model/NgsiLdDataRepresentationTests.kt @@ -15,7 +15,7 @@ import org.springframework.util.LinkedMultiValueMap class NgsiLdDataRepresentationTests { @Test - fun `it should return the attribute representation from the format query param when both format and options exist`() { + fun `it should return the attribute representation from the format when both format and options exist`() { val queryParams = LinkedMultiValueMap() queryParams.add("timerel", "after") queryParams.add("timeAt", "2025-01-03T07:45:24Z") @@ -78,7 +78,7 @@ class NgsiLdDataRepresentationTests { parseRepresentations(queryParams, MediaType.APPLICATION_JSON).shouldFail { assertInstanceOf(InvalidRequestException::class.java, it) - assertEquals("'invalid' is not a valid format value", it.message) + assertEquals("'invalid' is not a valid value for the format query parameter", it.message) } } @@ -91,7 +91,7 @@ class NgsiLdDataRepresentationTests { parseRepresentations(queryParams, MediaType.APPLICATION_JSON).shouldFail { assertInstanceOf(InvalidRequestException::class.java, it) - assertEquals("'invalidOptions' is not a valid options value", it.message) + assertEquals("'invalidOptions' is not a valid value for the options query parameter", it.message) } } @@ -104,7 +104,7 @@ class NgsiLdDataRepresentationTests { parseRepresentations(queryParams, MediaType.APPLICATION_JSON).shouldFail { assertInstanceOf(InvalidRequestException::class.java, it) - assertEquals("'invalidOptions' is not a valid options value", it.message) + assertEquals("'invalidOptions' is not a valid value for the options query parameter", it.message) } } } 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 index fce6ec97c..767b07f42 100644 --- a/shared/src/test/kotlin/com/egm/stellio/shared/util/ApiUtilsTests.kt +++ b/shared/src/test/kotlin/com/egm/stellio/shared/util/ApiUtilsTests.kt @@ -42,7 +42,7 @@ class ApiUtilsTests { fun `it should return an exception if it is given an invalid options query param`() { hasValueInOptionsParam(Optional.of("one"), TEMPORAL_VALUES).shouldFail { assertInstanceOf(InvalidRequestException::class.java, it) - assertEquals("'one' is not a valid options value", it.message) + assertEquals("'one' is not a valid value for the options query parameter", it.message) } } @@ -50,7 +50,7 @@ class ApiUtilsTests { fun `it should return an exception if it is given an invalid multi value options query param`() { hasValueInOptionsParam(Optional.of("one,two"), TEMPORAL_VALUES).shouldFail { assertInstanceOf(InvalidRequestException::class.java, it) - assertEquals("'one' is not a valid options value", it.message) + assertEquals("'one' is not a valid value for the options query parameter", it.message) } } @@ -64,7 +64,7 @@ class ApiUtilsTests { fun `it should return an exception if it is given at least one invalid value in options query param`() { hasValueInOptionsParam(Optional.of("one,temporalValues"), TEMPORAL_VALUES).shouldFail { assertInstanceOf(InvalidRequestException::class.java, it) - assertEquals("'one' is not a valid options value", it.message) + assertEquals("'one' is not a valid value for the options query parameter", it.message) } }