Skip to content

Fixes deserialization of OData primitive types #135

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.

Sign up for GitHub

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

Jump to bottom
Merged
MIchaelMainer merged 7 commits into from
Oct 6, 2020
Merged

Fixes deserialization of OData primitive types #135

MIchaelMainer merged 7 commits into from
Oct 6, 2020

Conversation

@MIchaelMainer
Copy link
Collaborator

@MIchaelMainer MIchaelMainer commented Sep 22, 2020 •
edited
Loading

Functions and actions that return OData primitive types are wrapped in a JSON object. That encapsulating JSON object is handled for Graph types, but not for OData primitives. This PR fixes deserialization for request builders generated for actions and functions that returned OData primitives. The following response was not being deserialized:

{
    "@odata.context": "https://graph.microsoft.com/beta/$metadata#String",
    "value": "somevalue"
}

This is causing deserialization errors for generated API signatures like this as the response body is not a string

string result = await graphClient.Education
				 .SynchronizationProfiles[""]
				 .UploadUrl()
				 .Request()
				 .GetAsync();

There are some actions like checkMemberObjects that return a collection of OData primitives. These scenarios are accounted for in the generator:

<Action Name="checkMemberGroups" IsBound="true" xmlns="http://docs.oasis-open.org/odata/ns/edm">
  <Parameter Name="bindingParameter" Type="graph.directoryObject" Nullable="false" />
  <Parameter Name="groupIds" Type="Collection(Edm.String)" Nullable="false" Unicode="false" />
  <ReturnType Type="Collection(Edm.String)" Nullable="false" Unicode="false" />
</Action>
IDirectoryObjectCheckMemberGroupsCollectionPage page = await graphClient.Me
									.CheckMemberGroups(new List<string>())
									.Request()
									.PostAsync();

Fixes

Fixes #134
Fixes microsoftgraph/msgraph-sdk-dotnet#549

Discovered bug

microsoftgraph/msgraph-sdk-serviceissues#9

Scenarios

I'm a bit concerned about breaking existing v1.0 scenarios that work contrary to the assumptions here. I've marked the scenarios that I have tested as validated.

v1.0 scenarios addressed by this fix

<!-- Validated -->
<Function Name="count" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="Collection(graph.workbookChart)" />
    <ReturnType Type="Edm.Int32" Nullable="false" />
</Function>

<!-- Validated -->
<Function Name="count" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="Collection(graph.workbookTable)" />
    <ReturnType Type="Edm.Int32" Nullable="false" />
</Function>

<Function Name="count" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="Collection(graph.workbookTableColumn)" />
    <ReturnType Type="Edm.Int32" Nullable="false" />
</Function>
<Function Name="count" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="Collection(graph.workbookTableRow)" />
    <ReturnType Type="Edm.Int32" Nullable="false" />
</Function>
<Function Name="count" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="Collection(graph.workbookChartPoint)" />
    <ReturnType Type="Edm.Int32" Nullable="false" />
</Function>
<Function Name="count" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="Collection(graph.workbookChartSeries)" />
    <ReturnType Type="Edm.Int32" Nullable="false" />
</Function>
<Function Name="count" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="Collection(graph.workbookRangeBorder)" />
    <ReturnType Type="Edm.Int32" Nullable="false" />
</Function>

<!-- Validated -->
<Function Name="image" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="graph.workbookChart" />
    <ReturnType Type="Edm.String" Unicode="false" />
</Function>

<!-- Validated -->
<Function Name="image" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="graph.workbookChart" />
    <Parameter Name="width" Type="Edm.Int32" Nullable="false" />
    <ReturnType Type="Edm.String" Unicode="false" />
</Function>

<Function Name="image" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="graph.workbookChart" />
    <Parameter Name="width" Type="Edm.Int32" Nullable="false" />
    <Parameter Name="height" Type="Edm.Int32" Nullable="false" />
    <ReturnType Type="Edm.String" Unicode="false" />
</Function>
<Function Name="image" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindparameter" Type="graph.workbookChart" />
    <Parameter Name="width" Type="Edm.Int32" Nullable="false" />
    <Parameter Name="height" Type="Edm.Int32" Nullable="false" />
    <Parameter Name="fittingMode" Type="Edm.String" Nullable="false" Unicode="false" />
    <ReturnType Type="Edm.String" Unicode="false" />
</Function>
<Function Name="verifyWindowsEnrollmentAutoDiscovery" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindingParameter" Type="graph.deviceManagement" />
    <Parameter Name="domainName" Type="Edm.String" Unicode="false" />
    <ReturnType Type="Edm.Boolean" Nullable="false" />
</Function>
<Function Name="downloadApplePushNotificationCertificateSigningRequest" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindingParameter" Type="graph.applePushNotificationCertificate" />
    <ReturnType Type="Edm.String" Unicode="false" />
</Function>

<!-- validated that this APIs actual doesn't match CSDL description. Service issue https://github.com/microsoftgraph/msgraph-sdk-serviceissues/issues/9-->
<Action Name="revokeSignInSessions" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindingParameter" Type="graph.user" Nullable="false" />
    <ReturnType Type="Edm.Boolean" />
</Action>

<!-- Validated -->
<Action Name="addGroup" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindingParameter" Type="graph.groupLifecyclePolicy" />
    <Parameter Name="groupId" Type="Edm.String" Nullable="false" Unicode="false" />
    <ReturnType Type="Edm.Boolean" Nullable="false" />
</Action>

<Action Name="removeGroup" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindingParameter" Type="graph.groupLifecyclePolicy" />
    <Parameter Name="groupId" Type="Edm.String" Nullable="false" Unicode="false" />
    <ReturnType Type="Edm.Boolean" Nullable="false" />
</Action>
<Action Name="setMobileDeviceManagementAuthority" IsBound="true"
    xmlns="http://docs.oasis-open.org/odata/ns/edm">
    <Parameter Name="bindingParameter" Type="graph.organization" />
    <ReturnType Type="Edm.Int32" Nullable="false" />
</Action>

Tool tip
You can use LinqPAD to query the metadata like this

https://github.com/MIchaelMainer/graph-test-harness/blob/master/src/LINQPad/Query%20-%20Metadata.linq#L56

@MIchaelMainer MIchaelMainer mentioned this pull request Sep 22, 2020
Open
@MIchaelMainer
Use intermediate response object
f87d585
@MIchaelMainer
Add support for OData int64 response (long)
a1a83dc 
	new file:   src/Microsoft.Graph.Core/Models/ODataMethodInt64Response.cs
This was referenced Sep 25, 2020
Merged
Merged
Merged
@MIchaelMainer MIchaelMainer marked this pull request as ready for review September 25, 2020 06:56
baywet
baywet previously approved these changes Sep 25, 2020
View reviewed changes
zengin
zengin previously approved these changes Sep 25, 2020
View reviewed changes
@MIchaelMainer MIchaelMainer dismissed stale reviews from zengin and baywet via edd6f1c September 30, 2020 19:21
zengin
zengin previously approved these changes Sep 30, 2020
View reviewed changes
nikithauc
nikithauc previously approved these changes Oct 2, 2020
View reviewed changes
baywet
baywet reviewed Oct 2, 2020
View reviewed changes
tests/Microsoft.Graph.DotnetCore.Core.Test/Microsoft.Graph.DotnetCore.Core.Test.csproj Outdated
<PackageReference Include="Moq" Version="4.10.1" />
<PackageReference Include="xunit" Version="2.4.1" />
<PackageReference Include="Microsoft.Graph" Version="1.*" />
<PackageReference Include="Microsoft.Graph" Version="3.14.0" />
Copy link
Member

@baywet baywet Oct 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be 3.* instead of locking down the version?

Copy link
Collaborator Author

@MIchaelMainer MIchaelMainer Oct 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Will change.

@MIchaelMainer MIchaelMainer dismissed stale reviews from nikithauc and zengin via 59e630d October 6, 2020 05:44
nikithauc
nikithauc reviewed Oct 6, 2020
View reviewed changes
zengin
zengin previously approved these changes Oct 6, 2020
View reviewed changes
@MIchaelMainer MIchaelMainer merged commit f11798e into dev Oct 6, 2020
MIchaelMainer added a commit that referenced this pull request Oct 6, 2020
@MIchaelMainer
1.22 release (#140)
c68ff79 
* Support native plaform Http handlers for Xamarin.Mac
* Update macOS to use NSUrlSessionHandler in foundation namespace.
* Add support for Xamarin.Mac20
* Compression header handling (#118)
* Copy Http Content headers to the destination stream in compression handler.
* Ensure headers are not empty.
* Ensure that compressed data is the same as decompressed data.
* Add `IBaseRequestBuilder` interface to `BaseRequestBuilder` class (#128)
* Update Microsoft.Graph.Core.csproj (#129)
* Use new service connection (#130)
* Fixes deserialization of OData primitive types (#135)

Adds intermediate response objects for deserialization of OData primitives.

	new file:   src/Microsoft.Graph.Core/Models/ODataMethodInt64Response.cs
	new file:   src/Microsoft.Graph.Core/Models/ODataMethodInt32Response.cs
	new file:   src/Microsoft.Graph.Core/Models/ODataMethodBooleanResponse.cs
	new file:   src/Microsoft.Graph.Core/Models/ODataMethodStringResponse.cs
@MIchaelMainer MIchaelMainer mentioned this pull request Oct 13, 2020
Closed
@andrueastman andrueastman mentioned this pull request Dec 7, 2020
Merged
andrueastman pushed a commit that referenced this pull request Dec 8, 2020
Fixes deserialization of OData primitive types (#135)
1052407 
Adds intermediate response objects for deserialization of OData primitives.

	new file:   src/Microsoft.Graph.Core/Models/ODataMethodInt64Response.cs
	new file:   src/Microsoft.Graph.Core/Models/ODataMethodInt32Response.cs
	new file:   src/Microsoft.Graph.Core/Models/ODataMethodBooleanResponse.cs
	new file:   src/Microsoft.Graph.Core/Models/ODataMethodStringResponse.cs
This was referenced Dec 8, 2020
Merged
Closed
@baywet baywet deleted the fix/deserializeODataPrimitive branch August 15, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@baywet baywet baywet left review comments

@zengin zengin zengin approved these changes

@nikithauc nikithauc nikithauc approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Attempting to deserialize a String type Deserialization exception when adding group to lifecycle policy in Azure Active Directory

5 participants

@MIchaelMainer @zengin @baywet @nikithauc