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 7 commits into
base: develop
Choose a base branch
from

Conversation

ranim-n
Copy link
Contributor

@ranim-n ranim-n commented Dec 24, 2024

No description provided.

@ranim-n ranim-n added feature New feature or request common Relates to common behaviors ngsild-1.8.1 labels Dec 24, 2024
@ranim-n ranim-n self-assigned this Dec 24, 2024
@ranim-n ranim-n linked an issue Dec 24, 2024 that may be closed by this pull request
Copy link
Contributor

github-actions bot commented Dec 24, 2024

Test Results

   70 files  +1     70 suites  +1   1m 21s ⏱️ -1s
1 116 tests +6  1 116 ✅ +6  0 💤 ±0  0 ❌ ±0 
1 155 runs  +6  1 155 ✅ +6  0 💤 ±0  0 ❌ ±0 

Results for commit e89b116. ± Comparison against base commit a76f793.

This pull request removes 194 and adds 44 tests. Note that renamed tests count towards both.
                                    { "id":…, withTemporalValues=true, withAudit=false, expectation={
                      "@id": "https://uri…
                      "@type": "@json",
                      …
                    "@value": "/A/B"
                    "@value": "/C/D"
                    "@value": 20
                    "…
                    {
                  "@type": "https://uri.etsi.org/ngsi-ld/DateTime",
…
com.egm.stellio.search.entity.util.PatchAttributeTests ‑ [1] source={
    "attribute": {
        "type": "Property",
        "value": 12.0,
        "observedAt": "2024-04-14T12:34:56Z"
    }
}, target={
    "attribute": {
        "type": "Property",
        "value": 12.2,
        "unitCode": "GRM"
    }
}, expected={
    "attribute": {
        "type": "Property",
        "value": 12.2,
        "unitCode": "GRM",
        "observedAt": "2024-04-14T12:34:56Z"
    }
}
com.egm.stellio.search.entity.util.PatchAttributeTests ‑ [1] source={
    "attribute": {
        "type": "Property",
        "value": 12.0,
        "observedAt": "2024-04-14T12:34:56Z",
        "subAttribute": {
            "type": "Property",
            "value": "subAttribute"
        }
    }
}, target={
    "attribute": {
        "type": "Property",
        "value": 12.2,
        "unitCode": "GRM",
        "subAttribute": {
            "type": "Property",
            "value": "newSubAttributeValue"
        }
    }
}, expected={
    "attribute": {
        "type": "Property",
        "value": 12.2,
        "unitCode": "GRM",
        "observedAt": "2024-04-14T12:34:56Z",
        "subAttribute": {
            "type": "Property",
            "value": "newSubAttributeValue"
        }
    }
}
com.egm.stellio.search.entity.util.PatchAttributeTests ‑ [2] source={
    "attribute": {
        "type": "Property",
        "value": { "en": "car", "fr": "voiture" }
    }
}, target={
    "attribute": {
        "type": "Property",
        "value": { "fr": "vélo", "es": "bicicleta" }
    }
}, expected={
    "attribute": {
        "type": "Property",
        "value": { "en": "car", "fr": "vélo", "es": "bicicleta" }
    }
}
com.egm.stellio.search.entity.util.PatchAttributeTests ‑ [2] source={
    "incoming": {
        "type": "JsonProperty",
        "json": { "id": 1, "b": null, "c": 12.4 },
        "observedAt": "2022-12-24T14:01:22.066Z",
        "subAttribute": {
            "type": "Property",
            "value": "subAttribute"
        }
    }
}, target={
    "incoming": {
        "type": "JsonProperty",
        "json": { "id": 2, "b": "something" },
        "observedAt": "2023-12-24T14:01:22.066Z"
    }
}, expected={
    "incoming": {
        "type": "JsonProperty",
        "json": { "id": 2, "b": "something" },
        "observedAt": "2023-12-24T14:01:22.066Z",
        "subAttribute": {
            "type": "Property",
            "value": "subAttribute"
        }
    }
}
com.egm.stellio.search.entity.util.PatchAttributeTests ‑ [3] source={
    "attribute": {
        "type": "Property",
        "value": [ "car", "voiture" ]
    }
}, target={
    "attribute": {
        "type": "Property",
        "value": [ "vélo", "bicicleta" ]
    }
}, expected={
    "attribute": {
        "type": "Property",
        "value": [ "vélo", "bicicleta" ]
    }
}
com.egm.stellio.search.entity.util.PatchAttributeTests ‑ [3] source={
    "attribute": {
        "type": "VocabProperty",
        "vocab": "stellio"
    }
}, target={
    "attribute": {
        "type": "VocabProperty",
        "vocab": "egm"
    }
}, expected={
    "attribute": {
        "type": "VocabProperty",
        "vocab": "egm"
    }
}
com.egm.stellio.search.entity.util.PatchAttributeTests ‑ [4] source={
    "attribute": {
        "type": "Relationship",
        "object": "urn:ngsi-ld:Entity:01"
    }
}, target={
    "attribute": {
        "type": "Relationship",
        "object": "urn:ngsi-ld:Entity:02"
    }
}, expected={
    "attribute": {
        "type": "Relationship",
        "object": "urn:ngsi-ld:Entity:02"
    }
}
com.egm.stellio.search.entity.util.PatchAttributeTests ‑ [5] source={
    "attribute": {
        "type": "LanguageProperty",
        "languageMap": { "en": "train", "fr": "train" }
    }
}, target={
    "attribute": {
        "type": "LanguageProperty",
        "languageMap": { "fr": "TGV", "es": "tren" }
    }
}, expected={
    "attribute": {
        "type": "LanguageProperty",
        "languageMap": { "en": "train", "fr": "TGV", "es": "tren" }
    }
}
com.egm.stellio.search.entity.util.PatchAttributeTests ‑ [6] source={
    "incoming": {
        "type": "JsonProperty",
        "json": { "a": 1, "b": "thing" }
    }
}, target={
    "incoming": {
        "type": "JsonProperty",
        "json": { "a": 2, "c": "other thing" }
    }
}, expected={
    "incoming": {
        "type": "JsonProperty",
        "json": { "a": 2, "b": "thing", "c": "other thing" }
    }
}
com.egm.stellio.search.entity.util.PatchAttributeTests ‑ [7] source={
    "attribute": {
        "type": "VocabProperty",
        "vocab": "stellio"
    }
}, target={
    "attribute": {
        "type": "VocabProperty",
        "vocab": "egm"
    }
}, expected={
    "attribute": {
        "type": "VocabProperty",
        "vocab": "egm"
    }
}
…

♻️ This comment has been updated with latest results.

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

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

@@ -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

@@ -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)

@@ -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

@@ -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

@bobeal
Copy link
Member

bobeal commented Jan 4, 2025

A note about the behavior of the MultiValueMap holding the query parameters:

  • if you pass such a query string to the endpoint: ?type=Beehive&options=sysAttrs,audit&options=keyValues
  • the map of the query parameters received in the handler will be this one:
image

In other words:

  • it takes care of "accumulating" the values of query parameters with the same name
  • it does not do the split of a query parameter which has a comma separated list of values

So this has to be handled in the code.

@@ -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

…/web/TemporalEntityHandlerTests.kt

Co-authored-by: Benoit Orihuela <[email protected]>
Comment on lines +194 to +213
fun extractTemporalRepresentation(requestParams: MultiValueMap<String, String>):
Either<APIException, TemporalRepresentation> = either {
val optionsParam = Optional.ofNullable(requestParams.getFirst(QueryParameter.OPTIONS.key))
val formatParam = requestParams.getFirst(QueryParameter.FORMAT.key)
return when (formatParam) {
FormatValue.TEMPORAL_VALUES.value -> TemporalRepresentation.TEMPORAL_VALUES.right()
FormatValue.AGGREGATED_VALUES.value -> TemporalRepresentation.AGGREGATED_VALUES.right()
else -> {
val hasTemporal = hasValueInQueryParam(optionsParam, QueryParamValue.TEMPORAL_VALUES)
val hasAggregated = hasValueInQueryParam(optionsParam, QueryParamValue.AGGREGATED_VALUES)
when {
hasTemporal && hasAggregated ->
return BadRequestDataException("Only one temporal representation can be present").left()
hasTemporal -> TemporalRepresentation.TEMPORAL_VALUES.right()
hasAggregated -> TemporalRepresentation.AGGREGATED_VALUES.right()
else -> TemporalRepresentation.NONE.right()
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be simplified. By adding a fromString function to the TemporalRepresentation enum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Relates to common behaviors feature New feature or request ngsild-1.8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for the format parameter
3 participants