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

NGSIv2 based forwarding for CPrs #3068

Closed
fgalan opened this issue Jan 9, 2018 · 15 comments
Closed

NGSIv2 based forwarding for CPrs #3068

fgalan opened this issue Jan 9, 2018 · 15 comments
Milestone

Comments

@fgalan
Copy link
Member

fgalan commented Jan 9, 2018

NGSIv2 based forwarding (i.e. forward NGSIv2 queries/updates from CB to CPr, based in existing registrations) has not been yet defined.

First task with regard to this issue is specificaiton of the mechanism. In order to do so, I have recovered some previous discussion on PR #2990 comments that may be useful.

@jmcanterafonseca comment:

First of all a context provider in theory should be just as any other context broker i.e. supporting the same set of APIs. Having said that I have to acknowledge the fact that Orion Broker main use case for context providers is the integration of external datasources and in that case we can not force developers to implement all the stack, and as a result my proposal is:

The operations that would have to be supported by an NGSIv2 context provider are:

/v2/entities?type=XX,JJ&id=YY,ZZ&attrs=at1,atn

I assume that at this stage we don't want to forward full query requests i.e. q parameter,

We need essentially to support the attribute PATCH method so that we can forward update requests >properly i.e. currently we have many many flavours for operations in charge of changing an attribute >but the essential one is PATCH, so that's why this is proposed.

/v2/entities/<entityId>/attrs

What do you think?

@fgalan comment:

Regarding the operations I guess you mean (in your previous comment verb is missing):

GET /v2/entities?type=XX,JJ&id=YY,ZZ&attrs=at1,atn
PATCH /v2/entities/<entityId>/attrs

I think it may sense. Next step in this line would be to include that "CPr requirement" in the
specification in this PR along with a functional explanation on how forwarding works (paying special attention to "multi forwarding" case, e.g. a query on entity E ends in a forwared query for E-A1, E-A2 and E-A3 as E-A{1|2|3} belong to different CPrs).

This specification could be either in the .apib (if we agree forwarding is part of the NGSIv2 API) or specific .md in the Orion manual (if we agree on the opposite), which I see by your last comment #2990 (comment) is yet a matter of discussion.

PD. I agree ?q shouldn't be a requirement for CPrs.

Second task regarding this issue is to implement the mechanism in CB.

CC: @dcalvoalonso as the specified mechanism is relevant in the context of the IOTAs NGSIv2 implementation in which you are involved (pasive attributes and commands).

@kzangeli
Copy link
Member

Two important things, major drawbacks in v1 forwards:

  1. At entity creation, if the entity is covered by a registration, then the creation should fail.
    This is tricky, depends on attributes, service path etc.
    However, the infamous "shadowing" problem of APIv1 forwards is solved with this.
  2. If more than one context provider is found for an update/query, all of them (not just the first one
    that mongo returns) must be forwarded to.
    Also not easy, if we have a query to forward to 6 CPrs, and all of them have a different value for an
    attribute, which one do we choose? Using timestamps perhaps ...

@jaimeventura
Copy link

Any update on this?
Im trying to implement a CP in python, using connexion and flask, but i cant find the ngsi v1 openapi/swagger spec to do it easily (only NGSI v2 is available).

@fgalan
Copy link
Member Author

fgalan commented Mar 1, 2019

We have been recently talking about this @dcalvoalonso @jmcanterafonseca and me so your comment is very timely :)

There is a proposal for forwarding in NGSIv2 (described at FIWARE/specifications#2) but it is too complex to be implemented at once. Thus, we are going to opt to implement something simpler in the first step but compatible with the specification if at the end we decide to do a full implementation.

In particular, NGSIv1 forwarding is based in the following operations:

  • POST /v1/updateContext
  • POST /v1/queryContext

So in a first step we are going just to change there for their NGSIv2 counteparts:

  • POST /v2/op/update
  • GET /v2/entities?<parameters>

Note that they are the same operations described in the full proposal (https://github.com/Fiware/specifications/blob/5a83a7117bb5cb952546c385b900055dbb0a0417/ContextSource_Forwarding/contextsource_forwarding_spec.md#context-source-interface) so they should be compatible in the future.

@jaimeventura maybe you want to help in this (given that you benefit from it in your CPr implementation)? The task of implementing the new forwarding is not very complex and I can guide during the process. Do you have background in C/C++?

@fgalan
Copy link
Member Author

fgalan commented Mar 6, 2019

More detail on the operations to implement:

  • POST /v2/op/update will use "update" as actionType (to be consistent with NGSIv1)
  • With regards to the <parameters> in GET /v2/entities?<parameters>, according to NGSIv2 specification it allows 15 query parameters, but not all them make sense in the context of query forwarding. A preliminary list of parameters to implement (to be confirmed upon implementation) is the following:
    • id, to specify a single entity or list to get
    • type, to specify the type of entities to get
    • attrs, to specify a single attribute or list to get

@kzangeli
Copy link
Member

kzangeli commented Mar 8, 2019

My first doubt about forwarding is the following:
We will have V1 and V2 forwarding co-existing for some time, so how decide whether to send the forwarded requests as V2 instead of V1 ?

Different possibilities:

  • All registrations created with V2 will get forwards using V2 (we'd need a new field for regs in mongo)
  • The field http:url of the registration could be used to indicate the protocol version
  • We invent a new field for the registration, indicating V1/V2 for forwards
  • ...

I always prefer the method that gives the most liberty to the user, in this case, the third option, where the "registrator" decides what protocol to use for the forwarded messages.

@jmcanterafonseca
Copy link
Contributor

jmcanterafonseca commented Mar 8, 2019 via email

@kzangeli
Copy link
Member

kzangeli commented Mar 8, 2019

Ah, great. Totally forgot about "legacyForwarding".
Good, the "registrator" decides.

@fgalan
Copy link
Member Author

fgalan commented Mar 8, 2019

legacyForwarding: false will indicate that v2 should be used, right?

Correct. In addition, omitting legacyForwarding field also indicates that v2 should be used.

@fgalan
Copy link
Member Author

fgalan commented Apr 4, 2019

PR #3467 implements NGSIv2 based forwarding for queries with basic .test coverage.

What is pending after that PR is:

@fgalan
Copy link
Member Author

fgalan commented Apr 5, 2019

NGSIv2 based forwarding for queries => updates?

Correct. It was a typo. Now fixed above.

@dcalvoalonso
Copy link

As part of telefonicaid/iotagent-node-lib#762, I am doing some integrations tests of this PR with some IOTAs. In particular, I am working with the LWM2M IoT Agent. You can check the WIP PR in telefonicaid/lightweightm2m-iotagent#186.

I think that there is a problem with entities containing attributes with spaces in the name.

For instance, using in the registration:

{
    "url": "http://localhost:1026/v2/registrations",
    "method": "POST",
    "json": {
        "dataProvided": {
            "entities": [
                {
                    "type": "Light",
                    "id": "TestClient:Light"
                }
            ],
            "attrs": [
                "luminescence"
            ]
        },
        "provider": {
            "http": {
                "url": "http://localhost:4041/v2"
            }
        },
        "expires": "2019-05-10T09:44:02.907Z"
    },
    "headers": {
        "fiware-service": "smartgondor",
        "fiware-servicepath": "/gardens"
    }
}

Then I can do:

curl -X GET \
  http://localhost:1026/v2/entities/TestClient:Light \
  -H 'fiware-service: smartgondor' \
  -H 'fiware-servicepath: /gardens'

And the endpoint in the IoT Agent is correctly called.

However, if I use luminescence att in the registration, then calling the CB to query the entity's attributes does not cause any call to the IOTA endpoint

@fgalan
Copy link
Member Author

fgalan commented Apr 10, 2019

Spaces are not allowed in NGSIv2 attribute names (along with other identifiers, please have a look to "Field syntax restrictions" section in the NGSIv2 specification). In fact, you shouldn't be able even to register an attribute like that (CB failing to detect it would be a bug).

Could you confirm the response you get when you try to register an registration with luminescence att, please?

@dcalvoalonso
Copy link

dcalvoalonso commented Apr 10, 2019

Spaces are not allowed in NGSIv2 attribute names (along with other identifiers, please have a look to "Field syntax restrictions" section in the NGSIv2 specification). In fact, you shouldn't be able even to register an attribute like that (CB failing to detect it would be a bug).

Sorry I forgot this! :(

Could you confirm the response you get when you try to register a registration with luminescence att, please?

The response is 201 Created. You can see in the screenshots below the Postman request and the entry in the database.
orion_registration_postman

orion_registration

@fgalan
Copy link
Member Author

fgalan commented Apr 10, 2019

The issue reported by @dcalvoalonso has been spinned out to #3468

@fgalan
Copy link
Member Author

fgalan commented Nov 5, 2019

Basic NGSIv2 forwarding implemented in Orion 2.3.0. Possible continuation of the work in issue #3485. The present issue gets closed.

@fgalan fgalan closed this as completed Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants