microsoftgraph / msgraph-sdk-dotnet

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

Kiota Serialization objects completely unusable #2442

Open stephajn opened 7 months ago

stephajn commented 7 months ago

Describe the bug

I am working with retrieving CustomSecurityAttributes for a Graph User. In version 5.44.0 of the library, the values of these attributes are returned as JsonElements which I can retrieve property data from.

When I updated to 5.48.0, the JsonElements were replaced with UntypeObject values instead.

  1. This is a breaking change!
  2. The UntypedObject class is completely unusable because the dictionary it returns is an IDictionary<string, UntypedNode>. When I retrieve any value from it, All I get back is an UntypedNode object. I can't cast this to UntypedBoolean, UntypedString etc. In addition, just trying to call GetValue() on the value just throws a NotImplementedException rather than actually giving me a value.

I had to revert back to 5.44.0.

Expected behavior

I expect to not have to deal with such breaking changes like this in a minor release version.

I expect to be able to properly retrieve values from the UntypedObject in a much more intuitive way than has been provided.

How to reproduce

Just try to retrieve custom security attributes for a Graph User that has them set.

SDK Version

5.48.0

Latest version known to work for scenario above?

5.44.0

Known Workarounds

Don't upgrade to 5.48.0 and stay on 5.44.0 instead.

Debug output

No response

Configuration

No response

Other information

No response

Bill-Miller-USMA commented 7 months ago

Looks like the Unusable UntypedArray, UntypedObject and UntypeEtc objects from Kiota project started to affect MS.Graph responses from 5.47.0. ver 5.46.0 is the last known good version. Tried to use Kiota object but _value / nodes are protected and Kiota serialization/deserialization requires an advanced degree to use! Dictionary<String,Object> has been working so well. If a big change like this is done, we need some direction on how to get at the data. SDK should make it easier to consume graph OData, not incrementally harder.

Bill-Miller-USMA commented 7 months ago

After some work, we were able to serialize and then deserialize the data from an UntypedArray.

  1. Ensure transitive dependencies for Microsoft.Kiota.Abstractions specify 1.8.3 instead of the default 1.8.0.
  2. Serialize the Kiota object after casting to kiota untyped types (we treat all additionaldata as dictionary<string, object>) ie String sJSON = KiotaJsonSerializer.SerializeAsString((UntypedArray)dsoData.Value);
  3. Deserialize JSON as needed..

The key was to get the right nuget reference since 1.8.0 does not have this. I think 5.47.0+ graph sdk installs should require 1.8.3 Kiota abstractions, or whichever minimum version first provided for UntypedArray. It is still a breaking change that should be documented better in nuget/release notes.

And also not sure how this helps besides keeping up to date; as since first working with Graph SDK, we've had to handle JSONElement and UntypedArray. Plus now UntypedObject, etc? Perhaps we can push Kiota JSON all the way through our data calls, and remove Newtonsoft. Or find a use for System.Text.JSON? I thought ExpandoObject was the clear winner to handle various data objects, at least on the client side!

iphdav commented 7 months ago

This looks related to #2459 which I have just submitted. Surely any complex dotnet graph usage will break until this is resolved? We are sticking with 5.46.

quinterojose commented 7 months ago

This is very unfortunate. I just spent all afternoon troubleshooting this issue. The following code was working fine before the update:

string[]? existingOrganizationNodeIds = [];
if ((existingUser?.AdditionalData.TryGetValue($"extension_{extensionAppId}_organizationNodeIds", out var value) ?? false)
    && value is JsonElement jsonElement)
{
    existingOrganizationNodeIds = jsonElement.EnumerateArray().Select(item => item.ToString()).ToArray();
}

Serializing the UntypedArray object to a string using KiotaJsonSerializer then deserializing back to a string[] using JsonSerializer solves the issue but man!

Here's the new code:

string[]? existingOrganizationNodeIds = [];
if ((existingUser?.AdditionalData.TryGetValue($"extension_{extensionAppId}_organizationNodeIds", out var value) ?? false)
    && value is UntypedArray untypedArray)
{
    var tempJson = KiotaJsonSerializer.SerializeAsString(untypedArray);
    existingOrganizationNodeIds = JsonSerializer.Deserialize<string[]>(tempJson);
}

Here's another similar issue reported https://github.com/microsoft/kiota-serialization-json-dotnet/issues/212

petrhollayms commented 6 months ago

Related: #2459

Cezus commented 4 months ago

Any updates?

Kraeuterbuddah commented 2 months ago

Any updates?

Bill-Miller-USMA commented 2 months ago

We have since moved from Newtonsoft.JSON to System.Text.JSON. All GSC updates have maintained the change to Kiota UntypedArray. Here is the primary iteration for Microsoft.Graph.Models.ListItem:

// Primary Iteration from GSC results Dictionary<string, object> dataRow = new Dictionary<string, object>(); foreach (KeyValuePair<String, Object> kvpData in tItem.Fields.AdditionalData) { if (kvpData.Value.GetType().Name == "JsonElement") dataRow.Add(kvpData.Key, myExtensions.Deserialize<Object>(kvpData.Value.ToString())); else if (kvpData.Value.GetType().Name == "String" && kvpData.Value == null) dataRow.Add(kvpData.Key, ""); // Optional, converts null strings to be ""; for GUI since we do not need the concept of null string else if (kvpData.Value.GetType().Name == "UntypedArray") { String sJSON = KiotaJsonSerializer.SerializeAsString((UntypedArray)kvpData.Value); dataRow.Add(kvpData.Key, myExtensions.Deserialize<Object>(sJSON)); } else dataRow.Add(kvpData.Key, kvpData.Value); }

Here are the extensions, mostly used to parse System.Text.JSON ValueKind objects:

` // Extensions public static T Deserialize(string content) { try { if (content == "") return (T)(Object) ""; T oreq = System.Text.Json.JsonSerializer.Deserialize(content); foreach (PropertyInfo pi in oreq.GetType().GetProperties(BindingFlags.Instance|BindingFlags.Public)) { if (pi.PropertyType==typeof(Dictionary<string, object>)) { var lParams = pi.GetValue(oreq, null) as Dictionary<string, object>; var lParamsNew = new Dictionary<string, object>(); foreach (var lParam in lParams) lParamsNew.Add(lParam.Key, myExtensions.GetObjectValue(lParam.Value)); pi.SetValue(oreq, lParamsNew); } } return oreq; } catch (Exception ex) { return default(T); } }

    // Ref: https://stackoverflow.com/questions/77334298/obtain-a-real-value-from-a-valuekind
    public static object? GetObjectValue(object? obj)
    {
        try
        {
            switch (obj)
            {
                case null:
                    return "";
                case JsonElement jsonElement:
                    {
                        var typeOfObject = jsonElement.ValueKind;
                        var rawText = jsonElement.GetRawText(); // Retrieves the raw JSON text for the element.

                        return typeOfObject switch
                        {
                            JsonValueKind.Number => float.Parse(rawText, CultureInfo.InvariantCulture),
                            JsonValueKind.String => obj.ToString(), // Directly gets the string.
                            JsonValueKind.True => true,
                            JsonValueKind.False => false,
                            JsonValueKind.Null => null,
                            JsonValueKind.Undefined => null, // Undefined treated as null.
                            JsonValueKind.Object => rawText, // Returns raw JSON for objects.
                            JsonValueKind.Array => rawText, // Returns raw JSON for arrays.
                            _ => rawText // Fallback to raw text for any other kind.
                        };
                    }
                default:
                    throw new ArgumentException("Expected a JsonElement object", nameof(obj));
            }
        }
        catch (Exception ex)
        {
            return $"Error: {ex.Message}";
        }
    }

`

mariussm commented 1 month ago

Yeah, this was not fun to work with. Here is my workaround: https://goodworkaround.com/2024/10/07/working-around-custom-security-attribute-limitations-in-net-graph-sdk/

ashley-w-brazier-cloudm commented 1 month ago

Any update on this at all?

aalte commented 6 days ago

Surprisingly bad developer experience for being a Microsoft maintained library. I hope this is not a datapoint of where things are going in general. Maybe slow down the release cycles a bit and focus more on quality instead.