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

Kiota Serialization objects completely unusable #2442

Open
stephajn opened this issue Apr 16, 2024 · 12 comments
Open

Kiota Serialization objects completely unusable #2442

stephajn opened this issue Apr 16, 2024 · 12 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

@stephajn
Copy link

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

@stephajn stephajn added status:waiting-for-triage An issue that is yet to be reviewed or assigned type:bug A broken experience labels Apr 16, 2024
@Bill-Miller-USMA
Copy link

Bill-Miller-USMA commented Apr 18, 2024

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
Copy link

Bill-Miller-USMA commented Apr 18, 2024

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
Copy link

iphdav commented Apr 26, 2024

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
Copy link

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 microsoft/kiota-serialization-json-dotnet#212

@petrhollayms
Copy link

Related: #2459

@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
@CloudCees
Copy link

Any updates?

1 similar comment
@Kraeuterbuddah
Copy link

Any updates?

@Bill-Miller-USMA
Copy link

Bill-Miller-USMA commented Sep 12, 2024

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
Copy link

mariussm commented Oct 7, 2024

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

Any update on this at all?

@aalte
Copy link

aalte commented Nov 27, 2024

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.

@MartinM85
Copy link
Contributor

MartinM85 commented Dec 12, 2024

Do you have any examples how are your custom security attributes defined?

At least me, I'm able to read the value of CSA.

image

SDK code:

image

var user = await client.Users["{user_id}"].GetAsync((requestConfiguration) =>
{
    requestConfiguration.QueryParameters.Select = ["customSecurityAttributes"];
});
var attributeSet = user.CustomSecurityAttributes.AdditionalData["Engineering"];
if (attributeSet is UntypedObject untypedObject)
{
    var attributeValue = untypedObject.GetValue();
    var attributeName = attributeValue["Level"];
    if (attributeName is UntypedString untypedString)
    {
        var value = untypedString.GetValue();
    }
}

If the CSA has multiple values (of int type):

var attributeSet = user.CustomSecurityAttributes.AdditionalData["Test"];
if (attributeSet is UntypedObject untypedObject)
{
    var attributeValue = untypedObject.GetValue();
    var attributeName = attributeValue["Attribute1"];
    if (attributeName is UntypedArray untypedArray)
    {
        var values = untypedArray.GetValue();
        foreach (var value in values)
        {
            if (value is UntypedInteger untypedInteger)
            {
                var intValue = untypedInteger.GetValue();
            }
        }
    }
}

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