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

There is a bug in versions >= SDK 5.47 (Kiota change?) when fetching a multivalued list item from sharepoint #2459

Open
iphdav opened this issue Apr 26, 2024 · 3 comments
Assignees
Labels
priority:p1 High priority/Major issue but not blocking or Big percentage of customers affected.Bug SLA <=7days type:bug A broken experience

Comments

@iphdav
Copy link

iphdav commented Apr 26, 2024

Describe the bug

We fetch items from a sharepoint list like this:

var graphItems = graphClient.Sites[siteId].Lists[listId].Items;
var listItems = await graphItems.GetAsync(o => o.QueryParameters.Expand = new[] { "Fields" }); 

Then serialize and print one item in the list:

var dic = item.Fields.AdditionalData;
var json = JsonSerializer.Serialize(dic);

In SDK versions prior to 5.47, the output would look like this (I am only showing the property that has the issue):

{"RenewalStatus":[{"LookupId":6,"LookupValue":"Dead - see general status"},{"LookupId":21,"LookupValue":"Removed at CPA"}]}

In SDK versions from 5.47 onwards (tested to 5.50) the output to the same item is this:

{"RenewalStatus":{}}

When I look the AdditionalData dictionary, the type of the property in question was previously an array, but from 5.57 onwards it is Microsoft.Kiota.Abstractions.Serialization.UntypedArray.

Expected behavior

I believe I have described the steps to reproduce in the question, but in the sharepoint list the field is a "Lookup" field with allow multiple values enabled.

How to reproduce

As per the code in my description.

SDK Version

5.47 to latest (5.50)

Latest version known to work for scenario above?

5.46 and before

Known Workarounds

I am fixing our projects to Graph 5.46 until this is fixed or until it is clearer why this information is not being returned in the last few versions of the SDK. This is clearly an SDK change and not a sharepoint change given reverting to the older SDK resolves the issue.

Debug output

Click to expand log ```
</details>


### Configuration

_No response_

### Other information

_No response_
@iphdav iphdav added status:waiting-for-triage An issue that is yet to be reviewed or assigned type:bug A broken experience labels Apr 26, 2024
@iphdav
Copy link
Author

iphdav commented Apr 26, 2024

Thanks to @Bill-Miller-USMA I have figured out a backwards and forwards compatible work around (hoping Microsoft fixed this and my code keeps working on updated SDKs). But this is just awful and I beg Microsoft to improve this situation. People will expect JsonSerializer will serialize the information in the dictionary. The work around is I serialize the information using Kiota and then deserialize into array using JsonSerializer as shown below on the dictionary entry "RenewalStatus":

if (dic["RenewalStatus"] is Microsoft.Kiota.Abstractions.Serialization.UntypedArray)
{
    string tempJson = KiotaJsonSerializer.SerializeAsString((UntypedArray)dic
["RenewalStatus"]);
    dic["RenewalStatus"] = JsonSerializer.Deserialize<SharepointLookup[]>(tempJson);
}

If anyone is wondering, here is my object holding the information:

public record SharepointLookup
{
    public int LookupId { get; set; }
    public string LookupValue { get; set; }
}

Rather than just closing this issue I would like to see the SDK revert or something that will not cause almost any developer using graph and sharepoint to need to spend 3-4 hours on this issue with a minor point release. Thanks all and good day!

@darrelmiller
Copy link
Contributor

Hey @iphdav Apologies for the annoyance caused by this change. Unfortunately, it was never by explicit design that the contents of AdditionalData would serialize as JSON. It was more of a coincidence that may or may not have worked when dealing with more complex data types like DateTime and Duration.

The change was made to enable us to have first class support for untyped data whatever the underlying media type. Your usecase is a valid one, and one we want to be able to support properly and we believe we can do that by creating specialized JsonUntyped* properties in the Kiota.Serialization.Json library. That will then give us the ability to provide serialize/deserialize support from System.Text.Json, when the media type is actually JSON. Which, for your scenario, it is.

We work closely with Graph API teams to ensure that they schematize all the content that they return, so that we can provide guarantees on the API that we will not introduce breaking changes. AdditionalData is the bucket where we put the properties that are not schematized and therefore we cannot make those guarantees for. I recognize that changing the way we deliver additional data objects is in itself a breaking change, however, I just wanted to call out that there are risks to be aware of when relying on the data in the AdditionalData property.

@iphdav
Copy link
Author

iphdav commented Apr 30, 2024

@darrelmiller Thank you for the reply and additional context. If you follow your strategy, I agree that should allow correct and full serialization by System.Text.Json and would have solved this issue for me and likely others in the future.

To give you some additional context: I was upgrading some old usage of sharepoint lists to the graph API, where most of the data is stored in custom fields/columns added to the sharepoint list. Pre-graph sharepoint has a /_api/ endpoint which has strong OData support (better than graph), and the old infrastructure would generate a strongly typed client library to interacting with this sharepoint list data.

Microsoft recommends graph for new developments, but all these custom shareopoint fields end up in the "AdditionalData" dictionary, even though they are "schematized" within sharepoint - or at least you can pull down a full EDM model using the older /_api/ endpoints.

I did not want my client code to be dealing with a Dictionary<string,object> (AdditionalData), so was serializing that to JSON, then deserializing it into a strongly typed object. I have solved my immediately problem, but going through this process I realize that graph has other very lacking areas. One is that it just cannot translate as flexible IQueryable predicates to OData, compared to the previous APIs. Ironically graph is somewhat driving me away from sharepoint now as I think the outcome is we just get that data out of a sharepoint list and put in the effort moving it to a SQL backend or Cosmos, etc. I understand it is hard to focus on something that mainly benefits C#/.NET developers where the graph/kiota teams are really trying to target the lowest common denominator so all languages can be equally supported.

@petrhollayms petrhollayms added priority:p1 High priority/Major issue but not blocking or Big percentage of customers affected.Bug SLA <=7days and removed status:waiting-for-triage An issue that is yet to be reviewed or assigned labels May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p1 High priority/Major issue but not blocking or Big percentage of customers affected.Bug SLA <=7days type:bug A broken experience
Projects
None yet
Development

No branches or pull requests

4 participants