Open baywet opened 8 months ago
is this fixed?
No, this still needs to be implemented. is this something you'd like to submit a pull request for provided some guidance?
Yes I would like to fix this but it would be helpful to know what to do
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.
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?
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:
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.
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: