msgraph-sdk-dotnet icon indicating copy to clipboard operation
msgraph-sdk-dotnet copied to clipboard

[Client bug]: Optional parameter to ignore `null` values on set

Open olivermue opened this issue 2 years ago • 0 comments

Describe the bug One of the big changes from v4 to v5 was the introduction of the BackingStore. And this approach of a stateful object that only returns the changed values including null values works very well in scenarios like get an object via graph client; make some changes; set the same object via graph client. Unfortunately there are also some other (IMHO common) usage scenarios that won't work well in conjunction with the current model. In our case we have our own DTOs that will be mapped to graph objects and then be sent via graph client. These simple DTOs come from somewhere and will then be mapped onto the graph objects like this:

var wipeBody= new WipePostRequestBody
{
    KeepEnrollmentData = ourDto.KeepEnrollmentData,
    KeepUserData = ourDto.KeepUserData,
    MacOsUnlockCode = ourDto.MacOsUnlockCode,
}

var baseRequest = serviceClient.DeviceManagement.ManagedDevices[request.DeviceId].Wipe.PostAsync(wipeBody);

Unfortunately if the device to wipe is not an MacOS device and the value in ourDto.MacOsUnlockCode is null, this value will be send to graph and produces an error message. To avoid this error, we had to rewrite the above code as follows:

var wipeBody= new WipePostRequestBody
{
    KeepEnrollmentData = ourDto.KeepEnrollmentData,
    KeepUserData = ourDto.KeepUserData,
}

if (!string.isNullOrEmpty(ourDto.MacOsUnlockCode)
    MacOsUnlockCode = ourDto.MacOsUnlockCode,

var baseRequest = serviceClient.DeviceManagement.ManagedDevices[request.DeviceId].Wipe.PostAsync(wipeBody);

This approach is not very intuitive and depending on the project structure or usage of libraries like AutoMapper, this can really hard to takle down or fix it.

Expected behavior To probably make it easier to initialize graph objects from own DTOs it would be great if we could somehow interfere the behaviour when setting values to null or default. Maybe by applying an optional ctor parameter or calling a method after initialization:

var wipeBody= new WipePostRequestBody(ignoreDefaultValues: true)
{
    KeepEnrollmentData = ourDto.KeepEnrollmentData,
    KeepUserData = ourDto.KeepUserData,
    MacOsUnlockCode = ourDto.MacOsUnlockCode,
}

var wipeBody= new WipePostRequestBody
{
    KeepEnrollmentData = ourDto.KeepEnrollmentData,
    KeepUserData = ourDto.KeepUserData,
    MacOsUnlockCode = ourDto.MacOsUnlockCode,
}
wipeBody.BackingStore.RemoveAppliedDefaultValues();

I would guess the first approach is more visible to developers when setup a graph object by hand, while the second approach would be easier to use in libraries like AutoMapper.

Additional context While this concrete example explicitly leads to an error on the server side, we have the same issue also on other places, which don't throw an exception and can lead to subtle problems (e.g. deleting the description of a group or an user). So we need a general solution for this problem and not an explicit on this example object.

Client version v5.11

olivermue avatar May 19 '23 07:05 olivermue