fix: align event subscription/notification to Commonalities r3.2#84
Conversation
benhepworth
left a comment
There was a problem hiding this comment.
Thanks for the PR @mohdfarhanakram - great progress aligning with Commonalities r3.2. Most everything is pretty solid - just a few recommendations that I added in-line. Summary of these recommendations:
- suggest renaming BadRequest400 to CreateSessionBadRequest400 and then creating a new Generic400 for the other calls since the sink specific errors only apply when creating a session (see model QoD uses for reference)
- question about uri for http/mqtt - http is a URI but not technically for mqtt, which is why the original was a string - needs more discussion with the group, will address on the next call.
- made
statusrequired in SessionBase since there should always be a current state of the subscription (equivalent of QoD qosStatus, which is required in it's SessionInfo).
| $ref: "#/components/schemas/Protocol" | ||
| sink: | ||
| type: string | ||
| format: uri |
There was a problem hiding this comment.
we should discuss this more - the reason it was originally a string is because a MQTT topic is not technically a URI by definition.
There was a problem hiding this comment.
will look up in Commonalities to align with that
There was a problem hiding this comment.
we should leave this as a string for the generic SessionBase.sink since both HTTP and MQTT share it -- and add format:uri to HTTPSessionRequest.sink
There was a problem hiding this comment.
Updated - removed format: URI from SessionBase.sink so it stays as a plain string (since MQTT topics aren't URIs). HTTPSessionRequest.sink retains format: URI as expected.
- Align sink and sinkCredential definitions to CAMARA Event Guide - Add SinkCredential, AccessTokenCredential, PlainCredential, RefreshTokenCredential schemas - Add sink URI pattern validation and 400 INVALID_SINK error response - Rename subscription-ends to subscription-ended with terminationReason enum - Add subscription-started and subscription-updated lifecycle events - Rename EventType to SubscriptionEventType - Fix types/subscribedEventTypes property name mismatch in SessionRequest - Add SubscriptionStatus enum to SessionBase - Rename startTime to startsAt per r3.2 convention - Remove sinkCredential from response schemas (security compliance)
777f0d1 to
6119f83
Compare
What type of PR is this?
correction
What this PR does / why we need it:
Aligns the Session Insights API event subscription and notification model to the CAMARA Commonalities r3.2 Event Subscription and Notification Guide.
sinkdefinition with URI format, HTTPS pattern validation, and examplesinkCredentialwith properSinkCredentialobject schema (withAccessTokenCredential,PlainCredential,RefreshTokenCredentialsubtypes)400 INVALID_SINKerror response (along withINVALID_CREDENTIAL,INVALID_TOKEN,INVALID_ARGUMENT) with examplessubscription-ends→subscription-endedwithterminationReasonenum (NETWORK_TERMINATED,SUBSCRIPTION_EXPIRED,MAX_EVENTS_REACHED,ACCESS_TOKEN_EXPIRED,SUBSCRIPTION_DELETED)subscription-startedlifecycle event withinitiationReasonsubscription-updatedlifecycle event withupdateReasonEventType→SubscriptionEventTypewith descriptiontypes/subscribedEventTypesproperty name mismatch inSessionRequest(required field referenced non-existent property)SubscriptionStatusenum (ACTIVATION_REQUESTED,ACTIVE,INACTIVE,EXPIRED,DELETED)startTime→startsAtper r3.2 conventionsinkCredentialfrom response schemas (must not be returned per security guidelines)Which issue(s) this PR fixes:
Fixes #70
Special notes for reviewers:
SinkCredential,AccessTokenCredential, etc.) follow the exact same pattern used in QualityOnDemandsubscribedEventTypes→typesfix also corrects a bug whereSessionRequest.requiredreferenced a non-existent property nameChangelog input
Additional documentation