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

Re-introduce GeoJSON conversion under a flag #1020

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

jason-fox
Copy link
Contributor

@jason-fox jason-fox commented Apr 12, 2021

Reverts #1014 and #1018. Adds autocastGeoJSON to the config for use with NGSI-v2 legacy systems.

  • Updated code
  • Added unit test
  • Updated Docs

@mapedraza
Copy link
Collaborator

Pending to define Group provision and device provision JSON payload how to enable those casting functionalities instead of having a global env flag

@jason-fox jason-fox marked this pull request as draft April 14, 2021 10:11
@jason-fox jason-fox marked this pull request as ready for review April 14, 2021 14:45
@jason-fox
Copy link
Contributor Author

Pending to define Group provision and device provision JSON payload how to enable those casting functionalities instead of having a global env flag

@mapedraza - the global flag has been removed and autocastGeoJSON = true added to the attribute metadata. I think this is now ready for review.

Comment on lines 118 to 122
{
"name": "observationSpace",
"type": "Polygon",
"autocastGeoJSON" : "true"
}
Copy link
Member

Choose a reason for hiding this comment

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

I think the following syntax would be better:

        {
            "name": "observationSpace",
            "type": "Polygon",
            "autocast" : true
        }

So the way in which the autocast is done depends on the type. This way we can extend the functionality in the future to autocast other types. E.g. to autocast to boolean (e.g. "true" -> true, 0 -> false):

        {
            "name": "observationSpaceEnabled",
            "type": "Boolean",
            "autocast" : true
        }

Maybe it would even better to have a autocastType to cover cases in which the NGSI type need to be customized by the user, eg:

        {
            "name": "observationSpace",
            "type": "MyCustomType",
            "autocastType": "Polygon",
            "autocast" : true
        }

The autocastType could be optional and, in the case it is ommited, type is used.

With **NGSI-v2**, for backwards compatibility reasons, automatic GeoJSON conversion for types other than `geo:json` is turned off by default.
Add the `autocastGeoJSON` configuration to the attribute to enable GeoJSON conversion. Each GeoJSON attribute can be provisioned as shown:

```json
Copy link
Member

@fgalan fgalan Apr 27, 2021

Choose a reason for hiding this comment

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

This example in documentation is for a configuration group. However, the templates modified in this PR are only about devices:

imagen

The behavior should be as follow:

  • It can be enabled/disabled at configuration group level
  • It can be enabled/disabled at device level. If not defined at device level, the config group setting is used (and if not defined at config group level, then assume false by default)

# Conflicts:
#	CHANGES_NEXT_RELEASE
#	doc/advanced-topics.md
#	test/unit/ngsiv2/ngsiService/geoproperties-test.js
@@ -70,10 +70,10 @@ curl http://${KEYSTONE_HOST}/v3/OS-TRUST/trusts \
Apart from the generation of the trust, the use of secured Context Brokers should be transparent to the user of the IoT
Copy link
Member

Choose a reason for hiding this comment

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

I understand this PR provides "the definitive fix" mentioned in #1012 (comment)

Thus, an entry in CHANGES_NEXT_RELEASE relating issue #1012 should be included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants