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

add support for emitting webhooks #508

Open
baywet opened this issue Mar 19, 2024 · 6 comments
Open

add support for emitting webhooks #508

baywet opened this issue Mar 19, 2024 · 6 comments
Labels
priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days type:enhancement Enhancement request targeting an existing experience.
Milestone

Comments

@baywet
Copy link
Member

baywet commented Mar 19, 2024

Microsoft Graph has types that are only used when the service calls to the client's APIs: change notifications.
In the Microsoft Graph pipeline, those types are being trimmed by kiota because they are not used anywhere (any inbound endpoint).
We should:

  1. come up with an annotation that means "used for a webhook"
  2. add this annotation to the metadata via XOD
  3. leverage that annotation here so a webhook definition can be added to the description
  4. have kiota project the types from webhooks components, and not trim them.
@baywet baywet added type:enhancement Enhancement request targeting an existing experience. priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days labels Mar 19, 2024
@baywet baywet added this to the OData: 1.7 milestone Mar 19, 2024
@Tushar1337
Copy link

is this fixed?

@baywet
Copy link
Member Author

baywet commented Nov 9, 2024

No, this still needs to be implemented.
is this something you'd like to submit a pull request for provided some guidance?

@Tushar1337
Copy link

Yes I would like to fix this but it would be helpful to know what to do

@baywet
Copy link
Member Author

baywet commented Nov 9, 2024

The first step is more of a design exercise that can simply be discussed here.

We already have the type definition in the metadata and it looks something like this.

<ComplexType Name="changeNotification">
        <Property Name="changeType" Type="graph.changeType" Nullable="false">
          <Annotation Term="Org.OData.Core.V1.Description" String="Indicates the type of change that will raise the change notification. The supported values are: created, updated, deleted. Required." />
        </Property>
        <Property Name="clientState" Type="Edm.String">
          <Annotation Term="Org.OData.Core.V1.Description" String="Value of the clientState property sent in the subscription request (if any). The maximum length is 255 characters. The client can check whether the change notification came from the service by comparing the values of the clientState property. The value of the clientState property sent with the subscription is compared with the value of the clientState property received with each change notification. Optional." />
        </Property>
        <Property Name="encryptedContent" Type="graph.changeNotificationEncryptedContent">
          <Annotation Term="Org.OData.Core.V1.Description" String="(Preview) Encrypted content attached with the change notification. Only provided if encryptionCertificate and includeResourceData were defined during the subscription request and if the resource supports it. Optional." />
        </Property>
        <Property Name="id" Type="Edm.String">
          <Annotation Term="Org.OData.Core.V1.Description" String="Unique ID for the notification. Optional." />
        </Property>
        <Property Name="lifecycleEvent" Type="graph.lifecycleEventType">
          <Annotation Term="Org.OData.Core.V1.Description" String="The type of lifecycle notification if the current notification is a lifecycle notification. Optional. Supported values are missed, subscriptionRemoved, reauthorizationRequired. Optional." />
        </Property>
        <Property Name="resource" Type="Edm.String" Nullable="false">
          <Annotation Term="Org.OData.Core.V1.Description" String="The URI of the resource that emitted the change notification relative to https://graph.microsoft.com. Required." />
        </Property>
        <Property Name="resourceData" Type="graph.resourceData">
          <Annotation Term="Org.OData.Core.V1.Description" String="The content of this property depends on the type of resource being subscribed to. Optional." />
        </Property>
        <Property Name="subscriptionExpirationDateTime" Type="Edm.DateTimeOffset" Nullable="false">
          <Annotation Term="Org.OData.Core.V1.Description" String="The expiration time for the subscription. Required." />
        </Property>
        <Property Name="subscriptionId" Type="Edm.Guid" Nullable="false">
          <Annotation Term="Org.OData.Core.V1.Description" String="The unique identifier of the subscription that generated the notification.Required." />
        </Property>
        <Property Name="tenantId" Type="Edm.Guid" Nullable="false">
          <Annotation Term="Org.OData.Core.V1.Description" String="The unique identifier of the tenant from which the change notification originated. Required." />
        </Property>
      </ComplexType>

We need to come up with a new annotation like this

<Annotation Term="Org.OData.Core.V1.Description" String="The unique identifier of the tenant from which the change notification originated. Required." />

Which would roughly mean "this type is used as the main body for webhook ID X and for method POST".

Once we have a good design, the first step would be to submit a pull request to this list of vocabularies so it's accepted "as a common vocabulary".

Once the annotation is accepted, we can implement parsing it here and translating it to a webhook entry

@mikepizzo might be able to help with the annotation design.

Let us know if you have any additional comments or questions.

@mikepizzo
Copy link

mikepizzo commented Nov 11, 2024

I'm having a little trouble envisioning what general OData/OASIS annotation we would define for this. Basically, it seems to be saying "I know this type is not part of an endpoint, but generate a type for it anyway".

I think a more basic question may be, what is the set of types that kiota is currently omitting because they are not exposed through an endpoint, and why are they part of graph schema? I understand why these types are not exposed through an endpoint, and that we do want them to be part of the generated types -- what other types are defined in the schema, not exposed through an endpoint, and that we do NOT want to generate types for?

@baywet
Copy link
Member Author

baywet commented Nov 13, 2024

Thanks for chiming in @mikepizzo

Kiota ignores all types that are not "referenced" by any operations whether directly or indirectly (inheritance, properties, etc...), As far as I know that only includes:

  • Orphan types that were created by workloads but not actually used.
  • Change notifications related types.

It was actually me who added those change notification types to the metadata back when I was a PM for that team. The engineering group was assuming "the SDKs have the types" which was false. They were also planning "random additions" to the types, Adding those definitions in the metadata was kind of a forcing functions to have them go in front of API review, document things, etc...

I think there's value in introducing a notion of callbacks/webhooks for any OData API so clients know "this API can call you back". I don't know how much work that'd require from a standardization perspective to be honest. Short of that an annotation which says "I know this isn't referenced anywhere, but trust me, we need it" would be helpful.

If that doesn't make sense for OData/CSDL, we might instead explore adding a setting in kiota somewhere to convey that information of "not trimming" specific types from the description even though they are not referenced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days type:enhancement Enhancement request targeting an existing experience.
Projects
None yet
Development

No branches or pull requests

3 participants