Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for the format parameter #1299

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from
4 changes: 2 additions & 2 deletions search-service/config/detekt/baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
<ID>LongMethod:AttributeInstanceService.kt$AttributeInstanceService$@Transactional suspend fun create(attributeInstance: AttributeInstance): Either&lt;APIException, Unit&gt;</ID>
<ID>LongMethod:EnabledAuthorizationServiceTests.kt$EnabledAuthorizationServiceTests$@Test fun `it should return serialized access control entities with other rigths if user is owner`()</ID>
<ID>LongMethod:EntityAccessControlHandler.kt$EntityAccessControlHandler$@PostMapping("/{subjectId}/attrs", consumes = [MediaType.APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE]) suspend fun addRightsOnEntities( @RequestHeader httpHeaders: HttpHeaders, @PathVariable subjectId: String, @RequestBody requestBody: Mono&lt;String&gt;, @AllowedParameters @RequestParam queryParams: MultiValueMap&lt;String, String&gt; ): ResponseEntity&lt;*&gt;</ID>
<ID>LongMethod:EntityHandler.kt$EntityHandler$@GetMapping("/{entityId}", produces = [APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE, GEO_JSON_CONTENT_TYPE]) suspend fun getByURI( @RequestHeader httpHeaders: HttpHeaders, @PathVariable entityId: URI, @AllowedParameters( implemented = [ QP.OPTIONS, QP.TYPE, QP.ATTRS, QP.GEOMETRY_PROPERTY, QP.LANG, QP.CONTAINED_BY, QP.JOIN, QP.JOIN_LEVEL, QP.DATASET_ID, ], notImplemented = [QP.FORMAT, QP.PICK, QP.OMIT, QP.ENTITY_MAP, QP.LOCAL, QP.VIA] ) @RequestParam queryParams: MultiValueMap&lt;String, String&gt; ): ResponseEntity&lt;*&gt;</ID>
<ID>LongMethod:EntityHandler.kt$EntityHandler$@GetMapping(produces = [APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE, GEO_JSON_CONTENT_TYPE]) suspend fun getEntities( @RequestHeader httpHeaders: HttpHeaders, @AllowedParameters( implemented = [ QP.OPTIONS, QP.COUNT, QP.OFFSET, QP.LIMIT, QP.ID, QP.TYPE, QP.ID_PATTERN, QP.ATTRS, QP.Q, QP.GEOMETRY, QP.GEOREL, QP.COORDINATES, QP.GEOPROPERTY, QP.GEOMETRY_PROPERTY, QP.LANG, QP.SCOPEQ, QP.CONTAINED_BY, QP.JOIN, QP.JOIN_LEVEL, QP.DATASET_ID, ], notImplemented = [QP.FORMAT, QP.PICK, QP.OMIT, QP.EXPAND_VALUES, QP.CSF, QP.ENTITY_MAP, QP.LOCAL, QP.VIA] ) @RequestParam queryParams: MultiValueMap&lt;String, String&gt; ): ResponseEntity&lt;*&gt;</ID>
<ID>LongMethod:EntityHandler.kt$EntityHandler$@GetMapping("/{entityId}", produces = [APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE, GEO_JSON_CONTENT_TYPE]) suspend fun getByURI( @RequestHeader httpHeaders: HttpHeaders, @PathVariable entityId: URI, @AllowedParameters( implemented = [ QP.OPTIONS, QP.FORMAT, QP.TYPE, QP.ATTRS, QP.GEOMETRY_PROPERTY, QP.LANG, QP.CONTAINED_BY, QP.JOIN, QP.JOIN_LEVEL, QP.DATASET_ID, ], notImplemented = [QP.PICK, QP.OMIT, QP.ENTITY_MAP, QP.LOCAL, QP.VIA] ) @RequestParam queryParams: MultiValueMap&lt;String, String&gt; ): ResponseEntity&lt;*&gt;</ID>
<ID>LongMethod:EntityHandler.kt$EntityHandler$@GetMapping(produces = [APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE, GEO_JSON_CONTENT_TYPE]) suspend fun getEntities( @RequestHeader httpHeaders: HttpHeaders, @AllowedParameters( implemented = [ QP.OPTIONS, QP.FORMAT, QP.COUNT, QP.OFFSET, QP.LIMIT, QP.ID, QP.TYPE, QP.ID_PATTERN, QP.ATTRS, QP.Q, QP.GEOMETRY, QP.GEOREL, QP.COORDINATES, QP.GEOPROPERTY, QP.GEOMETRY_PROPERTY, QP.LANG, QP.SCOPEQ, QP.CONTAINED_BY, QP.JOIN, QP.JOIN_LEVEL, QP.DATASET_ID, ], notImplemented = [QP.PICK, QP.OMIT, QP.EXPAND_VALUES, QP.CSF, QP.ENTITY_MAP, QP.LOCAL, QP.VIA] ) @RequestParam queryParams: MultiValueMap&lt;String, String&gt; ): ResponseEntity&lt;*&gt;</ID>
<ID>LongMethod:LinkedEntityServiceTests.kt$LinkedEntityServiceTests$@Test fun `it should inline entities up to the asked 2nd level`()</ID>
<ID>LongMethod:PatchAttributeTests.kt$PatchAttributeTests.Companion$@JvmStatic fun mergePatchProvider(): Stream&lt;Arguments&gt;</ID>
<ID>LongMethod:PatchAttributeTests.kt$PatchAttributeTests.Companion$@JvmStatic fun partialUpdatePatchProvider(): Stream&lt;Arguments&gt;</ID>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,11 @@ class EntityHandler(
@RequestHeader httpHeaders: HttpHeaders,
@AllowedParameters(
implemented = [
QP.OPTIONS, QP.COUNT, QP.OFFSET, QP.LIMIT, QP.ID, QP.TYPE, QP.ID_PATTERN, QP.ATTRS, QP.Q,
QP.OPTIONS, QP.FORMAT, QP.COUNT, QP.OFFSET, QP.LIMIT, QP.ID, QP.TYPE, QP.ID_PATTERN, QP.ATTRS, QP.Q,
QP.GEOMETRY, QP.GEOREL, QP.COORDINATES, QP.GEOPROPERTY, QP.GEOMETRY_PROPERTY,
QP.LANG, QP.SCOPEQ, QP.CONTAINED_BY, QP.JOIN, QP.JOIN_LEVEL, QP.DATASET_ID,
],
notImplemented = [QP.FORMAT, QP.PICK, QP.OMIT, QP.EXPAND_VALUES, QP.CSF, QP.ENTITY_MAP, QP.LOCAL, QP.VIA]
notImplemented = [QP.PICK, QP.OMIT, QP.EXPAND_VALUES, QP.CSF, QP.ENTITY_MAP, QP.LOCAL, QP.VIA]
)
@RequestParam queryParams: MultiValueMap<String, String>
): ResponseEntity<*> = either {
Expand Down Expand Up @@ -288,10 +288,10 @@ class EntityHandler(
@PathVariable entityId: URI,
@AllowedParameters(
implemented = [
QP.OPTIONS, QP.TYPE, QP.ATTRS, QP.GEOMETRY_PROPERTY,
QP.OPTIONS, QP.FORMAT, QP.TYPE, QP.ATTRS, QP.GEOMETRY_PROPERTY,
QP.LANG, QP.CONTAINED_BY, QP.JOIN, QP.JOIN_LEVEL, QP.DATASET_ID,
],
notImplemented = [QP.FORMAT, QP.PICK, QP.OMIT, QP.ENTITY_MAP, QP.LOCAL, QP.VIA]
notImplemented = [QP.PICK, QP.OMIT, QP.ENTITY_MAP, QP.LOCAL, QP.VIA]
)
@RequestParam queryParams: MultiValueMap<String, String>
): ResponseEntity<*> = either {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ 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.OptionsParamValue
import com.egm.stellio.shared.util.hasValueInOptionsParam
import com.egm.stellio.shared.util.QueryParamValue
import com.egm.stellio.shared.util.hasValueInQueryParam
import com.egm.stellio.shared.util.parseTimeParameter
import org.springframework.util.MultiValueMap
import org.springframework.util.MultiValueMapAdapter
Expand All @@ -42,21 +43,29 @@ fun composeTemporalEntitiesQueryFromGet(
requestParams,
contexts
).bind()

if (inQueryEntities)
var withTemporalValues = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of using these 2 booleans, it will be cleaner to switch to an enum similar to the AttributeRepresentation one

var withAggregatedValues = false
if (inQueryEntities) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you add enclosing { for this one-line expression?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

detekt automatic fix

entitiesQueryFromGet.validateMinimalQueryEntitiesParameters().bind()

val withTemporalValues = hasValueInOptionsParam(
Optional.ofNullable(requestParams.getFirst(QueryParameter.OPTIONS.key)),
OptionsParamValue.TEMPORAL_VALUES
)
val withAudit = hasValueInOptionsParam(
Optional.ofNullable(requestParams.getFirst(QueryParameter.OPTIONS.key)),
OptionsParamValue.AUDIT
)
val withAggregatedValues = hasValueInOptionsParam(
}
val optionsParam = Optional.ofNullable(requestParams.getFirst(QueryParameter.OPTIONS.key))
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)),
OptionsParamValue.AGGREGATED_VALUES
QueryParamValue.AUDIT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should use OptionsValue instead

)
val temporalQuery =
buildTemporalQuery(requestParams, defaultPagination, inQueryEntities, withAggregatedValues).bind()
Expand All @@ -83,17 +92,17 @@ fun composeTemporalEntitiesQueryFromPost(
contexts
).bind()

val withTemporalValues = hasValueInOptionsParam(
val withTemporalValues = hasValueInQueryParam(
Optional.ofNullable(requestParams.getFirst(QueryParameter.OPTIONS.key)),
OptionsParamValue.TEMPORAL_VALUES
QueryParamValue.TEMPORAL_VALUES
)
val withAudit = hasValueInOptionsParam(
val withAudit = hasValueInQueryParam(
Optional.ofNullable(requestParams.getFirst(QueryParameter.OPTIONS.key)),
OptionsParamValue.AUDIT
QueryParamValue.AUDIT
)
val withAggregatedValues = hasValueInOptionsParam(
val withAggregatedValues = hasValueInQueryParam(
Optional.ofNullable(requestParams.getFirst(QueryParameter.OPTIONS.key)),
OptionsParamValue.AGGREGATED_VALUES
QueryParamValue.AGGREGATED_VALUES
)

val temporalParams = mapOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,11 @@ class TemporalEntityHandler(
@RequestHeader httpHeaders: HttpHeaders,
@AllowedParameters(
implemented = [
QP.OPTIONS, QP.COUNT, QP.OFFSET, QP.LIMIT, QP.ID, QP.TYPE, QP.ID_PATTERN, QP.ATTRS, QP.Q,
QP.OPTIONS, QP.FORMAT, QP.COUNT, QP.OFFSET, QP.LIMIT, QP.ID, QP.TYPE, QP.ID_PATTERN, QP.ATTRS, QP.Q,
QP.GEOMETRY, QP.GEOREL, QP.COORDINATES, QP.GEOPROPERTY, QP.TIMEPROPERTY, QP.TIMEREL, QP.TIMEAT,
QP.ENDTIMEAT, QP.LASTN, QP.LANG, QP.AGGRMETHODS, QP.AGGRPERIODDURATION, QP.SCOPEQ, QP.DATASET_ID
],
notImplemented = [QP.FORMAT, QP.LOCAL, QP.VIA, QP.PICK, QP.OMIT, QP.EXPAND_VALUES, QP.CSF]
notImplemented = [QP.LOCAL, QP.VIA, QP.PICK, QP.OMIT, QP.EXPAND_VALUES, QP.CSF]
)
@RequestParam queryParams: MultiValueMap<String, String>
): ResponseEntity<*> = either {
Expand Down Expand Up @@ -189,10 +189,10 @@ class TemporalEntityHandler(
@PathVariable entityId: URI,
@AllowedParameters(
implemented = [
QP.OPTIONS, QP.ATTRS, QP.TIMEPROPERTY, QP.TIMEREL, QP.TIMEAT, QP.ENDTIMEAT, QP.LASTN,
QP.OPTIONS, QP.FORMAT, QP.ATTRS, QP.TIMEPROPERTY, QP.TIMEREL, QP.TIMEAT, QP.ENDTIMEAT, QP.LASTN,
QP.LANG, QP.AGGRMETHODS, QP.AGGRPERIODDURATION, QP.DATASET_ID
],
notImplemented = [QP.FORMAT, QP.LOCAL, QP.VIA, QP.PICK, QP.OMIT]
notImplemented = [QP.LOCAL, QP.VIA, QP.PICK, QP.OMIT]
)
@RequestParam queryParams: MultiValueMap<String, String>
): ResponseEntity<*> = either {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ class EntitiesQueryUtilsTests {
requestParams.add("count", "true")
requestParams.add("offset", "1")
requestParams.add("limit", "10")
requestParams.add("options", "keyValues")
requestParams.add("format", "keyValues")
requestParams.add("containedBy", "urn:ngsi-ld:Beekeper:A,urn:ngsi-ld:Beekeeper:B")
requestParams.add("join", "inline")
requestParams.add("joinLevel", "1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,47 @@ class EntityHandlerTests {
)
}

@Test
fun `get entity by id should correctly return the representation asked in the format parameter`() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better add one or more tests on the parseRepresentations function in NgsiLdDataRepresentation

initializeRetrieveEntityMocks()
coEvery { entityQueryService.queryEntity(any(), MOCK_USER_SUB) } returns ExpandedEntity(
mapOf(
"@id" to beehiveId.toString(),
"@type" to listOf("Beehive"),
"https://uri.etsi.org/ngsi-ld/default-context/prop1" to mapOf(
JSONLD_TYPE to NGSILD_PROPERTY_TYPE.uri,
NGSILD_PROPERTY_VALUE to mapOf(
JSONLD_VALUE to "some value"
)
),
"https://uri.etsi.org/ngsi-ld/default-context/rel1" to mapOf(
JSONLD_TYPE to NGSILD_RELATIONSHIP_TYPE.uri,
NGSILD_RELATIONSHIP_OBJECT to mapOf(
JSONLD_ID to "urn:ngsi-ld:Entity:1234"
)
)
)
).right()

webClient.get()
.uri("/ngsi-ld/v1/entities/$beehiveId?format=keyValues&options=normalized")
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)
.exchange()
.expectStatus().isOk
.expectBody()
.json(
"""
{
"id": "$beehiveId",
"type": "Beehive",
"prop1": "some value",
"rel1": "urn:ngsi-ld:Entity:1234",
"@context": "${applicationProperties.contexts.core}"
}
""".trimIndent()
)
}

@Test
fun `get entity by id should return 404 if the entity has none of the requested attributes`() {
initializeRetrieveEntityMocks()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
ranim-n marked this conversation as resolved.
Show resolved Hide resolved
)
.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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.egm.stellio.shared.model

import com.egm.stellio.shared.queryparameter.FormatValue
import com.egm.stellio.shared.queryparameter.OptionsValue
import com.egm.stellio.shared.queryparameter.QueryParameter
import com.egm.stellio.shared.util.GEO_JSON_MEDIA_TYPE
Expand Down Expand Up @@ -28,9 +29,16 @@ data class NgsiLdDataRepresentation(
acceptMediaType: MediaType
): NgsiLdDataRepresentation {
val optionsParam = queryParams.getOrDefault(QueryParameter.OPTIONS.key, emptyList())
val formatParam = queryParams.getFirst(QueryParameter.FORMAT.key)
val attributeRepresentation = when {
bobeal marked this conversation as resolved.
Show resolved Hide resolved
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)
val attributeRepresentation = optionsParam.contains(OptionsValue.KEY_VALUES.value)
.let { if (it) AttributeRepresentation.SIMPLIFIED else AttributeRepresentation.NORMALIZED }
val languageFilter = queryParams.getFirst(QueryParameter.LANG.key)
val entityRepresentation = EntityRepresentation.forMediaType(acceptMediaType)
val geometryProperty =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.egm.stellio.shared.queryparameter

enum class FormatValue(val value: String) {
KEY_VALUES("keyValues"),
bobeal marked this conversation as resolved.
Show resolved Hide resolved
SIMPLIFIED("simplified"),
NORMALIZED("normalized"),
TEMPORAL_VALUES("temporalValues"),
AGGREGATED_VALUES("aggregatedValues")
bobeal marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ enum class QueryParameter(
JOIN("join"),
JOIN_LEVEL("joinLevel"),
OPTIONS("options"),
FORMAT("format"),
OBSERVED_AT("observedAt"),

// geoQuery
Expand All @@ -43,7 +44,6 @@ enum class QueryParameter(
DELETE_ALL("deleteAll"),

// not implemented yet
FORMAT("format"),
PICK("pick"),
OMIT("omit"),
EXPAND_VALUES("expandValues"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,16 +168,16 @@ internal fun canExpandJsonLdKeyFromCore(contexts: List<String>): Boolean {
return expandedType == NGSILD_DATASET_ID_PROPERTY
}

enum class OptionsParamValue(val value: String) {
enum class QueryParamValue(val value: String) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TEMPORAL_VALUES and AGGREGATED_VALUES should be moved / merged in FormatValue. AUDIT should be moved / merged in OptionsValue

TEMPORAL_VALUES("temporalValues"),
AUDIT("audit"),
AGGREGATED_VALUES("aggregatedValues")
}

fun hasValueInOptionsParam(options: Optional<String>, optionValue: OptionsParamValue): Boolean =
options
fun hasValueInQueryParam(queryParam: Optional<String>, queryParamValue: QueryParamValue): Boolean =
queryParam
.map { it.split(",") }
.filter { it.any { option -> option == optionValue.value } }
.filter { it.any { option -> option == queryParamValue.value } }
.isPresent

fun parseQueryParameter(queryParam: String?): Set<String> =
Expand Down
Loading
Loading