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 1 commit into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -21,8 +21,8 @@ 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.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 @@ -45,18 +45,22 @@ fun composeTemporalEntitiesQueryFromGet(

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 = when {
Copy link
Member

Choose a reason for hiding this comment

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

you could take this opportunity to return an APIException if both temporalValues and aggregatedValues are provided

hasValueInQueryParam(formatParam, QueryParamValue.TEMPORAL_VALUES) -> true
hasValueInQueryParam(optionsParam, QueryParamValue.TEMPORAL_VALUES) -> true
else -> false
}

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 withAggregatedValues = when {
hasValueInQueryParam(formatParam, QueryParamValue.AGGREGATED_VALUES) -> true
hasValueInQueryParam(optionsParam, QueryParamValue.AGGREGATED_VALUES) -> true
else -> false
}
val withAudit = hasValueInQueryParam(
Optional.ofNullable(requestParams.getFirst(QueryParameter.OPTIONS.key)),
OptionsParamValue.AGGREGATED_VALUES
QueryParamValue.AUDIT
)
val temporalQuery =
buildTemporalQuery(requestParams, defaultPagination, inQueryEntities, withAggregatedValues).bind()
Expand All @@ -83,17 +87,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
@@ -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,14 @@ data class NgsiLdDataRepresentation(
acceptMediaType: MediaType
): NgsiLdDataRepresentation {
val optionsParam = queryParams.getOrDefault(QueryParameter.OPTIONS.key, emptyList())
val formatParam = queryParams.getOrDefault(QueryParameter.FORMAT.key, emptyList())
val attributeRepresentation = when {
Copy link
Member

Choose a reason for hiding this comment

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

since you are in it and since it is very related, also add support for the simplified value for the format query param (synonym of keyValues)

formatParam.contains(FormatValue.KEY_VALUES.value) -> AttributeRepresentation.SIMPLIFIED
formatParam.contains(FormatValue.NORMALIZED.value) -> AttributeRepresentation.NORMALIZED
optionsParam.contains(FormatValue.KEY_VALUES.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,8 @@
package com.egm.stellio.shared.queryparameter

enum class FormatValue(val value: String) {
KEY_VALUES("keyValues"),
Copy link
Member

Choose a reason for hiding this comment

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

don't forget to remove it in the OptionsValues enum

NORMALIZED("normalized"),
TEMPORAL_VALUES("temporalValues"),
AGGREGATED_VALUES("aggregatedValues")
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 are unused, this is problematic

}
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 =
fun hasValueInQueryParam(options: Optional<String>, queryParamValue: QueryParamValue): Boolean =
Copy link
Member

Choose a reason for hiding this comment

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

options is no longer a good name for the 1st parameter

options
.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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package com.egm.stellio.shared.util

import com.egm.stellio.shared.config.ApplicationProperties
import com.egm.stellio.shared.model.BadRequestDataException
import com.egm.stellio.shared.util.OptionsParamValue.TEMPORAL_VALUES
import com.egm.stellio.shared.util.QueryParamValue.TEMPORAL_VALUES
import com.egm.stellio.shared.web.CustomWebFilter
import io.mockk.every
import io.mockk.mockk
Expand Down Expand Up @@ -33,27 +33,27 @@ class ApiUtilsTests {

@Test
fun `it should not find a value if there is no options query param`() {
Copy link
Member

Choose a reason for hiding this comment

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

also update the names of the test functions (options is no longer valid)

assertFalse(hasValueInOptionsParam(Optional.empty(), TEMPORAL_VALUES))
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`() {
assertFalse(hasValueInOptionsParam(Optional.of("one"), TEMPORAL_VALUES))
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`() {
assertFalse(hasValueInOptionsParam(Optional.of("one,two"), TEMPORAL_VALUES))
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`() {
assertTrue(hasValueInOptionsParam(Optional.of("temporalValues"), TEMPORAL_VALUES))
assertTrue(hasValueInQueryParam(Optional.of("temporalValues"), TEMPORAL_VALUES))
}

@Test
fun `it should find a value if it is in a multi value options query param`() {
assertTrue(hasValueInOptionsParam(Optional.of("one,temporalValues"), TEMPORAL_VALUES))
assertTrue(hasValueInQueryParam(Optional.of("one,temporalValues"), TEMPORAL_VALUES))
}

@Test
Expand Down
Loading