Support # ended service paths in registrations

fgalan commented 6 years ago

From https://fiware-orion.readthedocs.io/en/master/user/service_path/index.html

While entities belong to services and servicepaths, subscriptions and registrations belong only to the service. The servicepath in subscriptions and registrations doesn't denote sense of belonging, but is the expression of the query associated to the subscription or registration.

Under that ligth, query associated be as wider as possible in the default case, thus /# is the one to use.

We have deteced a part of places in the NGSIv1 registration logic where this doesn't occur. Check for FIXME marks with this issue number along the code

It should be fixed.

EDIT: check also

postRegisterContext.cpp
postNotifyContextAvailability.cpp
postRegistration.cpp

kzangeli commented 6 years ago

The problem here is that no context provider can say all entities E with service path /a/# are mine. If another context provider comes after and registers an entity E with service path /a/b/c/d, then the new registration may not work. It may be shadowed by the first. This is not as easy as it may seem at a first glance ...

kzangeli commented 6 years ago

Another question here ... If recursive service paths are to be allowed, then a vector of service paths also should be allowed, right?

kzangeli commented 6 years ago

One more thing, assuming we implement # for registrations.

If more than one registration (of different context providers) matches an update/query, what should the broker do?

Send forward to all the matching CPrs?
Send only to the first one that mongo returns?
Pick the CPr with the "most narrow match"? (longer/more exact service path: /a wins against /#)

Example of registrations that could give more than one match:

CP1 registers EntityId: 'E', EntityType: 'T', Service Path: '/#' CP2 registers EntityId: 'E', EntityType: 'T', Service Path: '/a' Update/Query for EntityId: 'E', EntityType: 'T', Service Path: '/a'

CP2 is "stronger" ... However, if we only update CP2, CP1 would be incorrect ... Not very good.

Another option would be to check old registrations and if an overlap is detected, to refuse the registration. Can't say I like that option very much.

In my opinion, the best option is to not allow '#' in Service Path of registrations ...

fgalan commented 6 years ago

So, the discussion is not about if /# should be the default for registrations which doesn't explicitely use Fiware-Servicepath header at creation time but about # should be allowed as service path for registrations at all?

Just to be sure of what we are talking here :)

kzangeli commented 6 years ago

Yes, first I believe we need to decide whether to allow recursive service paths or not. This decision defines the default value, / or /#

If we decide to allow recursive service paths, then we need to decide how to tackle the problems that recursive service paths add.

And also, should we allow a vector of service paths?

fgalan commented 6 years ago

In fact, they are independent issues. I mean, the problem of having two possible CPrs candidate for the same entity-attribute can happen even if these CPrs are defined using the same service path. In the past, we were dicussing about it in the original implementation of CPrs, even were dicussing as part of the "advance CPr functionality" that CB would be smart enough to cope with that situation (e.g. using the second CPr as backup of the first if the forwarded request fail or using some scoring algorithm to pick the best CPr). I don't remember exactly, but I think that for the "basic CPr functionality" mongoBackend returns the list of the possible CPr and the service routine pick some of them without any special criteria. And by the moment that is ok.

On the other hand, the case of having the same CPr covering several service paths (even all the service paths) is a common use case. I don't see any reason to precluding it, specially if currently we are working that way in NGSIv1 registrations (to be checked).

fgalan commented 6 years ago

Tested with

--SHELL-INIT--
dbInit CB
dbInit CP1
brokerStart CB
brokerStart CP1

--SHELL--

echo "01. Register E1/A1 on CB  with CP1 as providing application"
echo "==========================================================="
payload='{
  "contextRegistrations": [
  {
    "entities": [
      {
         "type": "T1",
         "isPattern": "false",
         "id": "E1"
      }
    ],
    "attributes": [
      {
        "name": "A1",
        "type": "string",
        "isDomain": "false"
      }
    ],
    "providingApplication": "http://localhost:'${CP1_PORT}'/v1"
    }
 ],
 "duration": "P1M"
}'
orionCurl --url /v1/registry/registerContext --payload "$payload" --servicePath '/#'
echo
echo

echo "02. Update/APPEND E1/A1 on CP1 subserv A"
echo "========================================"
payload='{
  "contextElements": [
    {
      "type": "T1",
      "id":   "E1",
      "attributes": [
        {
          "name": "A1",
          "type": "string",
          "value": "A1 in CP1 subserv A"
        }
      ]
    }
  ],
  "updateAction": "APPEND"
}'
orionCurl --url /v1/updateContext --payload "$payload" --port $CP1_PORT --servicePath '/A'
echo
echo

echo "03. Update/APPEND E1/A1 on CP1 subserv B"
echo "========================================"
payload='{
  "contextElements": [
    {
      "type": "T1",
      "id":   "E1",
      "attributes": [
        {
          "name": "A1",
          "type": "string",
          "value": "A1 in CP1 subserv B"
        }
      ]
    }
  ],
  "updateAction": "APPEND"
}'
orionCurl --url /v1/updateContext --payload "$payload" --port $CP1_PORT --servicePath '/B'
echo
echo

echo "04. Query E1/A1 in CB subserv A"
echo "==============================="
payload='{
  "entities": [
    {
      "id":   "E1",
      "type": "T1"
    }
  ],
  "attributes": [
    "A1"
  ]
}'
orionCurl --url /v1/queryContext --payload "$payload" --servicePath '/A'
echo
echo

echo "05. Query E1/A1 in CB subserv B"
echo "==============================="
payload='{
  "entities": [
    {
      "id":   "E1",
      "type": "T1"
    }
  ],
  "attributes": [
    "A1"
  ]
}'
orionCurl --url /v1/queryContext --payload "$payload" --servicePath '/B'
echo
echo

It doesn't work.

First, due to servicePathCheck() check in postRegisterContext(), which doesn't allow using #. However, even in the case of removing that check, the searchContextProviders() logic is not working, given the way in which it fills the service path for the query to mongo, based on fillQueryServicePath(), which doesn't takes into account /# (in this sense, compare with the logic in addTriggeredSubscriptions_noCache(), which takes this into account).

fgalan commented 6 years ago

Thus, my conclusions in this issue are:

Multi-CPr covering the same service path is not related with the present issue, as it may happen also in the case of using regular service paths. If needed, a separate issue should be created about this.
The case of having the same CPr covering several service paths (even all the service paths) is a common and desirable use case. Nothing precludes it from a conceptual point of view. However, current implementation is limited and doesn't allow it.
This issue is re-scoped so the issue is now about solving that limitation. Issue name will be changed accordinly.
In the current implemetation, it is ok to have / as default for registrations (both for NGSIv1 and NGSIv2 created registrations). Once this issue gets implemented we can consider if keep / as default or using /#, taking into account backward compatibility aspects in that decission.
A note about the limitation should be added at the documentation in https://fiware-orion.readthedocs.org/en/master/

fgalan commented 6 years ago

A note about the limitation should be added at the documentation in https://fiware-orion.readthedocs.org/en/master/

Done in PR https://github.com/telefonicaid/fiware-orion/pull/3092. This PR would be rolled back if this issue is at the end implemented.

ianp1 commented 3 years ago

Are there any plans on implementing this kind of behavior in the future? For me this seems like a common usecase.

fgalan commented 3 years ago

As far as I know, there aren't plans in the short future to address this.

However, the issues discussion above contains useful information (code hints, functional test to be used to assure the functionality, etc.) so if somebody volunteers to implement this, we would be happy to review the PR and eventually merge the contribution :)

ianp1 commented 3 years ago

Hey, thanks for your reply. I might take a look into the issue, but may not have the time to do so, will see. But first of all i am wondering if the discussion above about overlapping context provider registrations was resolved in the meantime? I think this should be adressed first, since an implementation of wildcard registrations would need to adress this, right?

I understand the problem with diverging data when only updating the closest match in context providers. However, this can also happen with the current implementation, right? If an entity is already present within orion, a context provider will not receive the request anymore, hence possibly creating a different state on the provider versus the registered provider.

fgalan commented 3 years ago

I think the discussion was already solved and the conclusions posted in this specific comment: https://github.com/telefonicaid/fiware-orion/issues/3078#issuecomment-361240213, remarking:

Multi-CPr covering the same service path is not related with the present issue, as it may happen also in the case of using regular service paths. If needed, a separate issue should be created about this.

So the same behaviour we have now (whatever it is :), I mean pick the first one, a random one, etc.) should be applied to a new scenario implementing also wildcards.

telefonicaid / fiware-orion

Support # ended service paths in registrations #3078