search

camaraproject / Commonalities

Repository to describe, develop, document and test the common guidelines and assets for CAMARA APIs
Apache License 2.0
9 stars 24 forks source link

Common proposal to tackle subscription-based open issues. #185

Closed bigludo7 closed 1 month ago

bigludo7 commented 3 months ago

Problem description We have several subscription-related issues for some time. (Issues check list 140 155 153 154 155 150 163 149 176 159) In order to speed up their resolution and with CAMARA first meta-release in front us, it is critical to move forward on these issues. This proposal has been build by @shilpa-padgaonkar @rartych @hdamker @PedroDiez @patrice-conil and myself.

Possible evolution

We have a meeting with to tackle long time open issues related to events & subscriptions and craft a common proposal.

Here is the sum-up of our proposal and then you have the detail below.

Comments are welcome in particular for blocker (as the issues are opened for several weeks). The plan is to craft a Design guideline proposal via a PR.

A TSC presentation of this proposal is also planned.

item1: subscription model https://github.com/camaraproject/Commonalities/issues/140 https://github.com/camaraproject/Commonalities/issues/155

Proposal: align our subscription model with CloudsEvents one

All agreed that this is the 'last' time to do it and better to align with CloudEvents subscription model.

item2: Manage lifecycle of subscriptions issue https://github.com/camaraproject/Commonalities/issues/153

Proposal: Introduce a status attribute in the template as we have fair UCs that can leverage this (Retrieve expired subscriptions for monitoring, deactivate a subscription if an user revoked her/his consent, etc...). API subproject could decide to use it or not. A enumeration status list could be proposed

item3: Allow event consumers to subscribe to more than one event types with a single subscription Issue https://github.com/camaraproject/Commonalities/issues/154

Proposal: Switch to an array in the model but at least for the first meta-release we enforce to have only event type per subscription. After this first meta release decision to handle several event types in one subscription request should be discussed at API sub projet level.

item4: Support filter option for subscriptions https://github.com/camaraproject/Commonalities/issues/155

Proposal: add filters and it's up to API Project to use it. We recommend to be very cautious as it add complexity so it should be keep for very relevant UC. Add initialEvent in config as well to manage request to get event when current situation of a device corresponds to the subscriptions type. initialEvent is not in the template but pre-defined and it's up to each subproject to decide to use or not. We need to expliclty describe this in the guideline.

item5: Change the scope pattern for explicit subscriptions https://github.com/camaraproject/Commonalities/issues/163

Proposal: We keep as it for now but discussion on the issue not close. In particular we're looking for @jlurien feedback as José crafted the scope part. Linked to the Wild Card discussion - @shilpa-padgaonkar craft an issue: https://github.com/camaraproject/Commonalities/issues/184#issuecomment-2058688162

item6: Extension Authcontext for certain event types https://github.com/camaraproject/Commonalities/issues/150

Proposal: Postponed - tag it as backlog (Put the label)

item7: Align Common Design Model for Cloud Event specversion field in APIs Design https://github.com/camaraproject/Commonalities/issues/176

Proposal: As stated by Pedro better to use an enum @PedroDiez will craft the PR then we will create issue when API projet did not follow the pattern

item8:x-correlator in CloudsEvent notification? https://github.com/camaraproject/Commonalities/issues/164

PR to merged (https://github.com/camaraproject/Commonalities/pull/170)

Proposal: @shilpa-padgaonkar / @patrice-conil to take a look and review.

item9:CloudEvent type enum causes code generation problem https://github.com/camaraproject/Commonalities/issues/159

Good news as it works for now so we can close this one and opened it again

DETAILLED ITEM DESCRIPTION & DISCUSSION

item1: subscription model

Some issues like https://github.com/camaraproject/Commonalities/issues/140 or https://github.com/camaraproject/Commonalities/issues/155 raised the point about alignement with cloudEvents subscription model.

We have a good example of the shift in issue 155.

We absolutely need to make a decision on this before meta release 1

Change:

In order to discuss on an existing case, if we try to realign our subscription model to CloudsEvent one we are going from:

{
  "webhook": {
    "notificationUrl": "https://application-server.com",
    "notificationAuthToken": "c8974e592c2fa383d4a3960714"
  },
  "subscriptionDetail": {
    "device": {
      "phoneNumber": "123456789"
      }
    },
    "area": {
       "areaType": "CIRCLE",
      "center": {
        "latitude": 50.735851,
        "longitude": 7.10066
    },
    "type": "org.camaraproject.geofencing.v0.area-entered"
  },
  "subscriptionExpireTime": "2024-06-08T09:42:23.807Z",
  "subscriptionMaxEvents ": "1",
  "initalEvent ": true,
  "subscriptionId": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
  "startsAt": "2024-03-08T09:42:23.807Z",
  "expiresAt": "2024-06-08T09:42:23.807Z"
}
}

to

{
  "id": "3fa85f64-5717-4562-b3fc-2c963f66afa6", 
  "protocol": "HTTP",   **Only HTTP
  "protocolSettings": {},
  "sink": "https://application-server.com",
  "sinkCredential": {  **To be studied
    "credentialType": "OauthToken"
    "credential": "c8974e592c2fa383d4a3960714"
  },
  "types": [  **depending on other discussion if 1 or *
    "org.camaraproject.geofencing.v0.area-entered"
 ], 
"config": { 
    "subscriptionDetail": {
      "device": {
        "phoneNumber": "123456789"
      },
    },
    "area": {
       "areaType": "CIRCLE",
      "center": {
        "latitude": 50.735851,
        "longitude": 7.10066
    },
    "type": "org.camaraproject.geofencing.v0.area-entered"
  },
    "subscriptionExpireTime": "2024-06-08T09:42:23.807Z",
    "subscriptionMaxEvents ": "1",
    "initalEvent ": true,
     "startsAt": "2024-03-08T09:42:23.807Z",
    "expiresAt": "2024-06-08T09:42:23.807Z"
} }

_Important impact on 4 APIs (device location, Device status, SIM Swa, Connectivity Insight)

Meeting discussion: All agreed that this is the 'last' time to do it.

Orange: No issue till we do not have to add filter behavior. DT: Better to switch to this new model now Telefonica: Technically it is good to move forward + agreed to have a template

Positive point for a change:

Next steps: Inform the TSC (next meeting) - Ping for Connectivity Insights (Shilpa will do) Create an issue to drop objection for each subproject.

item2: Manage lifecycle of subscriptions

issue https://github.com/camaraproject/Commonalities/issues/153 No status in CloudEvents subscription model

Async creation as Monitoring of terminated subscription could leverage a status attribute

Is it enough to manage a state engine?

Orange: humm not sure we need it right now DT: we can postpone Telefonica: Does it relevant to show to the consumer expired/terminated subscription?

DT: how we manage if the user revokes the consent associated to the subscription? Pedro: This is a status - the subscription should not be termianted but more 'on hold' mode

Proposal: Introduce a status attribute in the template as we have fair UC (expired, deactivated). Then subproject could decide to use it or not.

item3: Allow event consumers to subscribe to more than one event types with a single subscription

Issue https://github.com/camaraproject/Commonalities/issues/154

Latest proposal:

Do we need to be restrictive on this point? ... if a Telco is able to manage multiple event type in one request while another Telco is able to manage only one is there risk on API adoption ? Proposal: change the eventType as an enum but Telco is free to indicate in the documentation that only one eventType should be provided if this telco has specific reason to offer granularity 1,1

Patrice: de facto is we move to the CloudEvents the attribute should be an array Orange: Technically not an issue DT: As we have close event type this is not an issue to have this multiple types Telefonica: prefer to not have to support multiple events. As of now we have no buisness requirement. At least we must allow Telco to have a restriction to no manage multiple.

Proposal: We can switch to an array but at least for the first meta release we enforce to have only event type per subscription. After this first meta release decision to handle several event type could discussed at sub projet level.

item4: Support filter option for subscriptions

https://github.com/camaraproject/Commonalities/issues/155 If we shift to cloudsEvent structure we open the possibility to use filters.

Filtershas the drawback to require a 'grammar' and server side to implement it.

As long as we do not have clear uc requiring it I will probably prefer to not use it (have filters but not use if) For geofencing or device status probably a initialEvent attribute is more straighforward

Orange : add filters and it's up to API Project to use it. Add initialEvent in config as well. DT: How to manage that a consumer does not want to get the termination subscription? Telefonica: not use for filters as of now. OK for the initialEvent. It has impact for the implementation.

OK for the initialEvent(in the config structure) but not in the template but pre-defined and it's up to each sub project to decide to use or not. We need to explicitly describe this in the guideline.

item5: Change the scope pattern for explicit subscriptions

https://github.com/camaraproject/Commonalities/issues/163 Shilpa crafted PR Not far to a conclusion - need to be discussed with Pedro

Pedro: How we manage a request for all event type for a given APi in one call Related to the discussion on the wild card scope It was left hanging. Nobody has documented. We need a general issue for Wild card scope (Shilpa will create an issue in Commonalities & referes it in Identity & Consent)

Proposal: As of now we strictly follow the pattern for eventType Have feedback from José will be helpful (Pedro will do a follow-up)

item6: Extension Authcontext for certain event types https://github.com/camaraproject/Commonalities/issues/150

CloudsEvent spec specified that this This extension is purely informational and is not intended to secure CloudEvents.

Orange : I'm afraid it will bring more confusion thant solving problem - I prefer to not add this Telefonica: same DT: Same

To be postponed - tag it as backlog (Put the label)

item7: Align Common Design Model for Cloud Event specversion field in APIs Design

https://github.com/camaraproject/Commonalities/issues/176 It has been detected a misaligment regarding the design format used to reflect specversion field in API that manage notifications. Do we use an enum or a open string Proposal: As stated by Pedro better to use an enum - Orange & DT agrees Pedro will craft the PR Then we will create issue when projet did not follow the pattern

item8:x-correlator in CloudsEvent notification? https://github.com/camaraproject/Commonalities/issues/164

PR to merged (https://github.com/camaraproject/Commonalities/pull/170)

@shilpa-padgaonkar / @patrice-conil to take a look and review.

item9:CloudEvent type enum causes code generation problem https://github.com/camaraproject/Commonalities/issues/159

As in camaraproject/DeviceStatus#107 reported using the CloudEvent type identifier as an enum e.g. org.camaraproject.device-status.v0.roaming-status causes problems in the code generation for Java and probably other languages too.

An enum can't contain either a dot or (in Java) lowercase letters.

We have a proposal on the table by Lyle Bertz that need to be assessed quickly.

Good news as it works for now so we can close this one and opened it again

Additional context Presentation to be done in next TSC

shilpa-padgaonkar commented 3 months ago

@bigludo7 and @PedroDiez As we agreed today to have a common template, we probably need to also agree where this template yaml file should reside.

bigludo7 commented 2 months ago

@shilpa-padgaonkar I tend t think here: https://github.com/camaraproject/Commonalities/tree/main/artifacts

I will craft a draft PR to propose this file.

shilpa-padgaonkar commented 2 months ago

@bigludo7 Sounds good to me. Maybe you could create a folder named camara-cloudevents under artifacts, and we use it to place both notifications and subscription yaml templates?

PedroDiez commented 2 months ago

@bigludo7, @shilpa-padgaonkar as part of that template I think it would be also good point to model the POST callback, to have a reference of the model for CAMARA cloudevents profile as well as the traversal case of "subscription-ends"

PedroDiez commented 2 months ago

Hi,

@bigludo7, @shilpa-padgaonkar within Issue 155 context

Proposal: add filters and it's up to API Project to use it. We recommend to be very cautious as it add complexity so it should be keep for very relevant UC.

I understood we do not consider filters from the beginning as we use currently "subscriptionDetail" object as filter criteria, and this can generate complex rules to manage filtering

bigludo7 commented 2 months ago

Hello @PedroDiez @shilpa-padgaonkar Agreed Pedro but I noted that we keep the filters in the model but we recommend to not use till good UC for that as very complex.

If both You and Shilpa prefer to remove it from the model I'm fine.

bigludo7 commented 2 months ago

@PedroDiez @shilpa-padgaonkar We crafted with @patrice-conil the yaml. We took the CloudEvent specification in input but we slightly updated it to follow CAMARA pattern regarding use of polymorphic pattern and class heritage. We keep filters but open to removing it you prefer.

@PedroDiez We also described the callback part.