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

Support # ended service paths in registrations #3078

Open
fgalan opened this Issue Jan 17, 2018 · 9 comments

Comments

Projects
None yet
2 participants
@fgalan
Copy link
Member

fgalan commented Jan 17, 2018

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

This comment has been minimized.

Copy link
Member

kzangeli commented Jan 19, 2018

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

This comment has been minimized.

Copy link
Member

kzangeli commented Jan 19, 2018

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

@kzangeli

This comment has been minimized.

Copy link
Member

kzangeli commented Jan 20, 2018

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

This comment has been minimized.

Copy link
Member Author

fgalan commented Jan 24, 2018

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

This comment has been minimized.

Copy link
Member

kzangeli commented Jan 24, 2018

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

This comment has been minimized.

Copy link
Member Author

fgalan commented Jan 29, 2018

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

This comment has been minimized.

Copy link
Member Author

fgalan commented Jan 29, 2018

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

This comment has been minimized.

Copy link
Member Author

fgalan commented Jan 29, 2018

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 fgalan added techdebt nicetohave and removed bug labels Jan 29, 2018

@fgalan fgalan changed the title Default service path in registrations NGSIv1 should be /# Support # ended service paths in registrations Jan 29, 2018

@fgalan

This comment has been minimized.

Copy link
Member Author

fgalan commented Jan 29, 2018

Done in PR #3092. This PR would be rolled back if this issue is at the end implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment