microsoftgraph / msgraph-sdk-dotnet

Microsoft Graph Client Library for .NET!
https://graph.microsoft.com
Other
696 stars 247 forks source link

BaseDeltaFunctionResponse breaking change introduced in 5.57.0 #2688

Open tbithell opened 1 week ago

tbithell commented 1 week ago

Describe the bug

This code works in 5.56.1 and earlier, but breaks in 5.57 with - System.ArgumentException : The Parsable does not contain a collection property

`var pageIterator = PageIterator<DriveItem, BaseDeltaFunctionResponse>.CreatePageIterator( appGraphServiceClient, deltaGetResponse, (driveItem) => { allDriveItems.Add(driveItem); return true; }, (request) => { deltaToken = DeltaHttpHelper.GetQueryStringValueFromURI(request.URI.AbsoluteUri, Utils.Constants.Token); return request; });

await pageIterator.IterateAsync();`

Expected behavior

My integration tests should continue to work properly as they did before upgraded to 5.57

How to reproduce

After getting DriveItem DeltaQuery results create a pageiterator with the following code in 5.56.1 and then again in 5.57. It will work fine in 5.56.1 and fail in 5.57

`var pageIterator = PageIterator<DriveItem, BaseDeltaFunctionResponse>.CreatePageIterator( appGraphServiceClient, deltaGetResponse, (driveItem) => { allDriveItems.Add(driveItem); return true; }, (request) => { deltaToken = DeltaHttpHelper.GetQueryStringValueFromURI(request.URI.AbsoluteUri, Utils.Constants.Token); return request; });

await pageIterator.IterateAsync();`

SDK Version

5.57

Latest version known to work for scenario above?

5.56.1

Known Workarounds

None

Debug output

Click to expand log ``` ```

Configuration

No response

Other information

No response

andrueastman commented 1 week ago

Thanks for raising this @tbithell

I believe this is caused by this PR - https://github.com/microsoftgraph/msgraph-sdk-dotnet-core/pull/903 that changes the reflection usage in library and looks like this affects BaseDeltaFunctionResponse model as it does not contain a Value property(it is added by derived instances).

https://github.com/microsoftgraph/msgraph-sdk-dotnet/blob/b95df6a1d17c04f5437d97720984d6f6873e3a25/src/Microsoft.Graph/Generated/Models/BaseDeltaFunctionResponse.cs#L13

Ideally, using BaseDeltaFunctionResponse as the type parameter would be incorrect as the collection property of the response will have no type/property information to be deserialized into and would cause problems.

Out of curiosity does this work out for you when you use the DeltaGetResponse as the type parameter instead? Is there a specific reason to use the base class here?

tbithell commented 1 week ago

I'll give that a shot. The reason we use the base is we are using the same method to create a PageIterator for both a DeltaGetResponse and a DeltaWithTokenGetResponse.