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

Issue with type changes on Graph Sdk model classes #2386

Open
jsweiler opened this issue Mar 14, 2024 · 4 comments
Open

Issue with type changes on Graph Sdk model classes #2386

jsweiler opened this issue Mar 14, 2024 · 4 comments

Comments

@jsweiler
Copy link

In one of the recent Graph sdk updates (Microsoft.Graph.Communications.Calls package) many of the models were moved from the Microsoft.Graph namespace to Microsoft.Graph.Models namespace.

However some of the notification updates that our app gets do not have the .models namespace in the json. Depending on how you deserialize the json you will get a null result. One example is when an audio prompt is completed playing currently with the latest sdk (version 1.2.0.10115) you get a notification like this

[ { "@odata.type": "#microsoft.graph.mediaPrompt", "mediaInfo": { "@odata.type": "#microsoft.graph.mediaInfo", "uri": "uriStrippedFor Privacy", "resourceId": "resourceId stripped for privacy" } } ]

(This is the prompts array which shows prompts which just completed and is inside the PlayPromptOperation type). But notice both of the odata.types do not match up with the current .models namespace.

Another example is when handling the CallsOnIncoming event. In the incoming call it contains this info about the resource id of the call.

{ "@odata.type": "#microsoft.graph.identity", "id": "369ebe19-93ba-46da-aa2b-c473034c9a64", "tenantId": "1aad987e-533b-4c16-83c1-13359661d26d", "identityProvider": "AAD" }

Again notice that the type is microsoft.graph.identity and the actual type in the sdk is now microsoft.graph.models.identity. I'm wondering if this was intended or just overlooked?

I realize there are ways to deserialize that can handler either way, but also depending on how the application handles it this breaks it.

@andrueastman
Copy link
Member

Thanks for raising this @jsweiler

Any chance you can share more details on the deserialization mechanism used that uses the naming in the "@odata.type with the naming in the namespace?

@jsweiler
Copy link
Author

@andrueastman using an older sdk this kind of code worked to get an applicationInstance

var appInstanceData = call.Resource.Targets.FirstOrDefault(c => c.Identity.AdditionalData != null);
if (appInstanceData.Identity.AdditionalData.TryGetValue("applicationInstance", out object value))
{
    var appInstance = value as Microsoft.Graph.Identity;
    if (appInstance != null)
    {
        // do something with appInstance
    }
}

That does not anymore.

Now using Newtonsoft.Json I am able to correctly deserialize it.

My bigger concern would be that the api doesn't change beneath the sdk before we are able to get our newer deserialization code into prod.

@andrueastman
Copy link
Member

Taking a look at the mapping values in the Identity model type for example, you'll see the mapping values do follow the correct naming to enable polymorphic deserialization. Therefore, the existence of the source in the Microsoft.Graph.Models shouldn't really be an issue given the discriminator values.

"#microsoft.graph.azureCommunicationServicesUserIdentity" => new AzureCommunicationServicesUserIdentity(),

@jsweiler Any chance you can confirm what the type of the call property is in your sample? I suspect it's not a model in the SDK package. We may have to follow up with the Microsoft.Graph.Communications.Calls package if the model is owned there to understand this better.

I also notice the pulling of the applicationInstance property from the additionalData when it is defined in the CommunicationsIdentitySet type meaning this should be directly accessible with something like this

if (appInstanceData.Identity is CommunicationsIdentitySet communicationsIdentitySet)
{
    var appInstance = communicationsIdentitySet.ApplicationInstance;
}

In general, models generated by Kiota are meant to be serialized using the kiota serialization libraries as do not use serialization library annotations/mechanisms for System.Text.Json/Newtonsoft and should be deserialized with the KiotaJsonSerializer

var call = KiotaJsonSerializer.Deserialize<Call>(responseStream);

@jsweiler
Copy link
Author

@andrueastman The type of the call object in my code is ICall in the Microsoft.Graph.Communications.Calls namespace. The call object has an AdditionalData property which is of type IDictionary<string, object> and that is where I get the Identity object.

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

No branches or pull requests

3 participants