microsoftgraph / microsoftgraph/msgraph-sdk-dotnet
[Client bug]: Optional parameter to ignore `null` values on set
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 789
- Forks
- 264
- Avg merge
- 15h 17m
- Merged PRs (30d)
- 3
Description
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
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the BackingStore behavior used by WipePostRequestBody and how property assignments are tracked before PostAsync sends the request. Compare the proposed constructor and post-initialization approaches, then define the general behavior for ignored null or default values and verify that intentional value removal remains possible.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- api, developer-experience
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100