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

Add guidance for the use of collections of entity types #482

Open
wants to merge 24 commits into
base: vNext
Choose a base branch
from

Conversation

corranrogue9
Copy link
Contributor

@corranrogue9 corranrogue9 commented Jul 21, 2023

I'm adding guidance to suggest that collections should be entity types instead of complex types because they can be modified piecewise, while collections of complex types must always be replaced, which can result in data loss and/or race conditions.

Please ignore the commit comments, I will perform a squash merge to complete the PR.

Copy link
Contributor

@mikepizzo mikepizzo left a comment

Choose a reason for hiding this comment

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

It will surprise no-one to hear that I'm glad to get these guidelines published... ;-)

Looks good in general; a few comments below...

## Adding individual elements to a collection

For both `foos` and `bars`, the OData standard specifies that elements can be added to the collection using a `POST` request
1. [Complex Types](https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_UpdateaCollectionProperty):
Copy link
Contributor

Choose a reason for hiding this comment

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

The protocol supports this, but it's a bit asymmetric to support adding but not removing, so I'm not sure it's a pattern we want to encourage on ms graph. at least we should all out that it's an uncommon pattern due to this asymmetry.

{
"value": [
{
"someProperty": "an original value"
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason that your complex collection example starts with one value, and your entity collection sample starts with 2? it makes the sample payloads look more different than they are.

]
}
```

Copy link
Contributor

Choose a reason for hiding this comment

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

also, you can have other objects reference an entity within a collection, which you cannot do with a complex type:

<EntityType Name="baz">
      <Property Name="Description" Type="Edm.String"/>
      <NaviationProperty Name="bar" Type="self.bar"/>
</EntityType>
GET interestingData/baz
200 OK
{
    "Description": "my favorite bar",
    "bar": 
    {
        "id": "firstBarId",
        "differentProperty": 15
     }
}

The [OData standard](https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_UpdateanEntity) specifies that clients can replace all elements of a collection of complex types or all elements of a collection of entity types using a `PATCH` request:
> Collection properties...provided in the payload corresponding to updatable properties MUST replace the value of the corresponding property in the entity or complex type.

The standard also specifies that collections of entities can be updated in a relative fashion using the [delta syntax](https://docs.oasis-open.org/odata/odata-json-format/v4.01/odata-json-format-v4.01.html#_Toc38457777) in a `PATCH` request:
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe move this to after the replace request and just before the delta example. Also call out (importantly) that this is not possible with complex types.

The [OData standard](https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_UpdateanEntity) specifies that clients can replace all elements of a collection of complex types or all elements of a collection of entity types using a `PATCH` request:
> Collection properties...provided in the payload corresponding to updatable properties MUST replace the value of the corresponding property in the entity or complex type.

The standard also specifies that collections of entities can be updated in a relative fashion using the [delta syntax](https://docs.oasis-open.org/odata/odata-json-format/v4.01/odata-json-format-v4.01.html#_Toc38457777) in a `PATCH` request:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure "relative fashion" will resonate as strongly with what the user is trying to do here. maybe something like "The standard allows updating a collection of entities, including adding, removing, or modifying individual existing entities in a single PATCH request. This is not possible with complex types because there is no way to identity them individually within the collection."

3. Individual elements can be updated
4. Several elements within the collection may be added, removed, or updated in a single request

Further, to remove or update elements in a collection of complex types, the entire collection must be replaced with a `PATCH` request.
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest: "within"

4. Several elements within the collection may be added, removed, or updated in a single request

Further, to remove or update elements in a collection of complex types, the entire collection must be replaced with a `PATCH` request.
This has the potential to result in data loss for clients who accidentally don't include all of the current elements of the collection, or encounter a race condition where two clients are attempting to `PATCH` the same collection.
Copy link
Contributor

Choose a reason for hiding this comment

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

here I would just use "update"; the key is that they are updating the collection, not what verb they are using.

2. Individual elements can be removed
3. Individual elements can be updated
4. Several elements within the collection may be added, removed, or updated in a single request

Copy link
Contributor

Choose a reason for hiding this comment

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

and individual entities within a collection can be referenced, i.e., by other entities.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes - this came up recently in a Defender API review - they wanted to reference an item in another collection, but that collection used complex types :(

@@ -182,7 +182,9 @@ Another way to avoid this is to use JSON batch as described in the [Microsoft Gr

You can model structured resources for your APIs by using the OData entity type or complex type. The main difference between these types is that an entity type declares a key property to uniquely identify its objects, and a complex type does not. In Microsoft Graph, this key property is called `id` for server-created key values. If there is a natural name for the key property, then the workload can use that.

Because objects of complex types in Microsoft Graph don’t have unique identifiers, they are not directly addressable via URIs. Therefore, you must use entity types to model addressable resources such as individually addressable items within a collection. For more information, see the [Microsoft REST API Guidelines collection URL patterns](https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#93-collection-url-patterns). Complex types are better suited to represent composite properties of API entities.
Because objects of complex types in Microsoft Graph don’t have unique identifiers, they are not directly addressable via URIs. Therefore, you must use entity types to model addressable resources such as individually addressable items within a collection. For more information, see the [Microsoft REST API Guidelines collection URL patterns](https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#93-collection-url-patterns).
Copy link
Contributor

Choose a reason for hiding this comment

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

small nit, and i realize this is previous wording, but "must" immediately makes me think that you're making me do something that i wouldn't otherwise want to do. i realize "you get to use entity types..." also doesn't quite hit the mark. maybe something more neutral like "Entity types enable you to model addressable resources..."

Copy link
Collaborator

Choose a reason for hiding this comment

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

This year we committed to merge MS REST guidelines with Graph guidelines, since Azure had already diverged completely from the baseline guidelines. Thus, I think we should delete the reference to the baseline guidelines and create a comprehensive pattern for when and how to use collections. We should also move all the details we need to this pattern and leave the standard OData query parameters to the OData specification, with a reference to it.

@@ -182,7 +182,9 @@ Another way to avoid this is to use JSON batch as described in the [Microsoft Gr

You can model structured resources for your APIs by using the OData entity type or complex type. The main difference between these types is that an entity type declares a key property to uniquely identify its objects, and a complex type does not. In Microsoft Graph, this key property is called `id` for server-created key values. If there is a natural name for the key property, then the workload can use that.

Because objects of complex types in Microsoft Graph don’t have unique identifiers, they are not directly addressable via URIs. Therefore, you must use entity types to model addressable resources such as individually addressable items within a collection. For more information, see the [Microsoft REST API Guidelines collection URL patterns](https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#93-collection-url-patterns). Complex types are better suited to represent composite properties of API entities.
Because objects of complex types in Microsoft Graph don’t have unique identifiers, they are not directly addressable via URIs. Therefore, you must use entity types to model addressable resources such as individually addressable items within a collection. For more information, see the [Microsoft REST API Guidelines collection URL patterns](https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#93-collection-url-patterns).
This is also why [collection properties](./collection-properties.md) should almost always be modeled using an entity type rather than a complex type.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: "The inability to directly address an individual complex type within a collection means that..."

>
> OData services support querying collections of entities, complex type instances, and primitive values.

### Complex Type
Copy link
Collaborator

@OlgaPodo OlgaPodo Jul 25, 2023

Choose a reason for hiding this comment

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

As i understood your suggestion was to prefer collection of entity types when there is a need to update individual elements therefore i would remove references to complex types and show difference only when it's absolutely essential so that the pattern would be relatively short. IMO if you show differences for GET then the POST doesn't add much value. While PATCH is important.

@OlgaPodo OlgaPodo added Graph Guidelines This should be reviewed by Microsoft Grap team. Microsoft Graph This should be reviewed by Microsoft Grap team. labels Jul 27, 2023
@@ -0,0 +1,402 @@
OData services treat collections of complex types differently than they treat collections of entity types.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this missing a title?

@@ -0,0 +1,402 @@
OData services treat collections of complex types differently than they treat collections of entity types.
This is due to the nature of entity types being "individually addressable" (they have some key which uniquely identifies them within the collection) while complex types are not individually addressable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anywhere in this topic were we want to say something about collections of primitive types?

Let's use the following CSDL as an example:

```xml
<EntityType Name="interestingData">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have something a little more realistic?
I think we can do better than using foo and bar everywhere...

Let's use the following CSDL as an example:

```xml
<EntityType Name="interestingData">
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intended to be a singleton? We might want to mention that, as earlier we point out that entity types have keys :)

OlgaPodo and others added 2 commits December 1, 2023 11:25
Co-authored-by: Dan Kershaw [MSFT] <dkershaw10@users.noreply.github.com>
Co-authored-by: Dan Kershaw [MSFT] <dkershaw10@users.noreply.github.com>
## Conclusion

Collections of entity types have several behaviors that are not available for collections of complex types:
1. Individual elements can be retrieved
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that the corollary of this is that items in the collection can be referenced by other navigation properties

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graph Guidelines This should be reviewed by Microsoft Grap team. Microsoft Graph This should be reviewed by Microsoft Grap team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants