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

Address issues serializing a delta response payload from a function #1360

Open
mikepizzo opened this issue Nov 26, 2024 · 0 comments
Open

Address issues serializing a delta response payload from a function #1360

mikepizzo opened this issue Nov 26, 2024 · 0 comments
Assignees
Labels
bug Something isn't working P1

Comments

@mikepizzo
Copy link
Member

Assemblies affected
ASP.NET Core OData 8.x

Describe the bug
OData has traditionally supported writing delta response payloads in two scenarios:

  1. The response to a request to a delta link
  2. The response to a PATCH request to a collection (aka bulk operation)
    Recently, OData added a third scenario for returning a delta payload:
  3. The result of a function whose returntype is annotated with the isDelta attribute.

Reproduce steps
The simplest set of steps to reproduce the issue. If possible, reference a commit that demonstrates the issue.

Download and run the DeltaFunctions Sample

Data Model
Please share your Data model, for example, your C# class.

EDM (CSDL) Model

<edmx:Edmx xmlns:edmx="http://docs.oasis-open.org/odata/ns/edmx" Version="4.0">
    <edmx:DataServices>
        <Schema xmlns="http://docs.oasis-open.org/odata/ns/edm" Namespace="microsoft.graph">
            <EntityType Name="recoveryPreviewJob">
                <Key>
                    <PropertyRef Name="id"/>
                </Key>
                <Property Name="id" Type="Edm.String"/>
            </EntityType>
            <EntityType Name="recoveryChangeObject">
                <Key>
                    <PropertyRef Name="id"/>
                </Key>
                <Property Name="id" Type="Edm.String"/>
                <NavigationProperty Name="currentState" Type="Edm.EntityType" Nullable="false"/>
                <NavigationProperty Name="deltaFromCurrent" Type="Edm.EntityType" Nullable="false"/>
            </EntityType>
            <EntityType Name="directoryObject">
                <Key>
                    <PropertyRef Name="id"/>
                </Key>
                <Property Name="id" Type="Edm.String"/>
                <Property Name="displayName" Type="Edm.String"/>
                </EntityType>
            <EntityType Name="user" BaseType="microsoft.graph.directoryObject">
                <Property Name="emailName" Type="Edm.String"/>
            </EntityType>
            <EntityType Name="group" BaseType="microsoft.graph.directoryObject">
                <NavigationProperty Name="members" Type="Collection(microsoft.graph.directoryObject)"/>
            </EntityType>
            <Function Name="getChanges" IsBound="true" IsComposable="true">
                <Parameter Name="recoveryPreviewJob" Type="microsoft.graph.recoveryPreviewJob" Nullable="false"/>
                <ReturnType Type="Collection(microsoft.graph.recoveryChangeObject)" Nullable="false"/>
            </Function>
            <EntityContainer Name="graphService">
                <EntitySet Name="recoveryPreviewJobs" EntityType="microsoft.graph.recoveryPreviewJob"/>
                <EntitySet Name="recoveryChangeObjects" EntityType="microsoft.graph.recoveryChangeObject"/>
            </EntityContainer>
        </Schema>
    </edmx:DataServices>
</edmx:Edmx>

Request/Response
GET: http://localhost:5219/recoveryPreviewJobs/1/getChanges()

{
    "@context": "http://localhost:5219/$metadata#recoveryChangeObjects/$delta",
    "value": [
        {
            "id": null,
            "currentState": {
                "@type": "#microsoft.graph.user",
                "id": "user1",
                "displayName": "William",
                "emailName": null   <=email should not be written; it was not set in the delta change object
            },
            "deltaFromCurrent": {
                "@type": "#microsoft.graph.user",
                "id": "user1",
                "displayName": "Bill",
                "emailName": null    <= email should not be written; it was not set in the delta change object
            }
        },
        {
            "id": "2",
            "currentState": {
                "@type": "#microsoft.graph.group",
                "id": "group1",
                "displayName": null,
                "members@delta": [                    <=this should be "members", not "members@delta -- it is the full collection
                    {
                        "@type": "#microsoft.graph.user",
                        "id": "user7",
                        "displayName": null,            <= displayName was not set and should not be written
                        "emailName": null                <=emailName was not set and should not be written
                    }
                ]
            },
            "deltaFromCurrent": {
                "@type": "#microsoft.graph.group",
                "id": "group1",
                "displayName": null,
                "members@delta": [
                    {
                        "@type": "#microsoft.graph.user",
                        "id": "user3",
                        "displayName": null,            <=displayName was not set and should not be written
                        "emailName": null                <=emailName was not set and should not be written
                    },
                    {
                        "@removed": {
                            "reason": "deleted"
                        },
                        "@id": "https://graph.microsoft.com/v1.0/users/4"    <= the deleted resource set other fields, like id, that should have been written
                    }
                ]
            }
        }
    ]
}

Expected behavior

  1. Only properties of the change objects that were set should be written
  2. Nested ODataResourceSets within the delta payload should be serialized using ODataDeltaResourceSetSerializer, not DeltaResourceSetSerializer
  3. Properties of deleted resources should be serialized
  4. Other annotations, etc. of deleted and changed resources should be written just as would be for a regular resource (not shown)
{
    "@context": "http://localhost:5219/$metadata#recoveryChangeObjects/$delta",
    "value": [
        {
            "id": null,
            "currentState": {
                "@type": "#microsoft.graph.user",
                "id": "user1",
                "displayName": "William"
            },
            "deltaFromCurrent": {
                "@type": "#microsoft.graph.user",
                "id": "user1",
                "displayName": "Bill"
            }
        },
        {
            "id": "2",
            "currentState": {
                "@type": "#microsoft.graph.group",
                "id": "group1",
                "members": [
                    {
                        "@type": "#microsoft.graph.user",
                        "id": "user7"
                    }
                ]
            },
            "deltaFromCurrent": {
                "@type": "#microsoft.graph.group",
                "id": "group1",
                "members@delta": [
                    {
                        "@type": "#microsoft.graph.user",
                        "id": "user3"
                    },
                    {
                        "@removed": {
                            "reason": "deleted"
                        },
                        "@type": "#microsoft.graph.user",
                        "@id": "https://graph.microsoft.com/v1.0/users/4",
                        "id": "user4"
                    }
                ]
            }
        }
    ]
}

Additional context
Attempting to implement the third pattern revealed some issues with how we serialize a delta payload. Many of these are there due to how the code evolved; originally delta payloads were completely separate from regular payloads, and deleted resources were completely different than regular resources. However, the 4.01 JSON delta format supported in 1), and exclusively in 2) and 3) makes delta payloads and regular payloads more similar than different, and deleted resources the same as regular resources with an additional removed control information at the beginning (and the requirement to include key fields or @id).

For these reasons, we should refactor the delta serializers to be more fully functioned and to share implementation with the non-delta serializers.

@mikepizzo mikepizzo added the bug Something isn't working label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1
Projects
None yet
Development

No branches or pull requests

2 participants