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 transitioning from a structural collection to a navigation collection #551

Open
wants to merge 22 commits into
base: vNext
Choose a base branch
from
Open
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 145 additions & 0 deletions graph/articles/collections.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,3 +316,148 @@ Content-Type: application/json
"value": []
}
```

## 11. Collections of structural types (complex types or primitive types)
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved

Collections of entity types are generally preferable to collections of structual types because collections of structural types must be updated as a single unit, meaning that they are overwritten entirely by new contents, rather than be updated relative to the existing contents.
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved
Sometimes, structural collection properties are added to a type and then scenarios are discovered later that require a collection of entity types.
Take the following model with an entity type `foo` that has a collection of `bar`s:
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved

```xml
<EntityType Name="foo">
<Key>
<PropertyRef Name="id" />
</Key>
<Property Name="id" Type="Edm.String" Nullable="false" />
<Property Name="bars" Type="Collection(self.bar)" />
</EntityType>

<ComplexType Name="bar">
<Property Name="prop1" Type="Edm.String" />
<Property Name="prop2" Type="Edm.String" />
</ComplexType>
```
and a scenario arises that requires, for example, to remove individual `bar`s from the collection.
There are two options forward: //// TODO do we want to offer both options, or just one?
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved

### 11.1 Side-by-side collection properties

The model can be updated to have two collections side-by-side:
```diff
<EntityType Name="foo">
<Key>
<PropertyRef Name="id" />
</Key>
<Property Name="id" Type="Edm.String" Nullable="false" />
<Property Name="bars" Type="Collection(self.bar)" />
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved
+ <NavigationProperty Name="barsAsEntities" Type="Collection(self.barAsEntity)" ContainsTarget="true" />
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved
</EntityType>

<ComplexType Name="bar">
<Property Name="prop1" Type="Edm.String" />
<Property Name="prop2" Type="Edm.String" />
</ComplexType>

+<EntityType Name="barAsEntity">
+ <Key>
+ <PropertyRef Name="prop1" />
+ </Key>
+ <Property Name="prop1" Type="Edm.String" />
+ <Property Name="prop2" Type="Edm.String" />
+</EntityType>
```
Clients will now be able to refer to individual `bar`s using `prop1` as a key, and they can now remove those `bar`s using `DELETE` requests:
```http
DELETE /foos/{fooId}/bars/{some_prop1}
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved
```
```http
HTTP/1.1 204 No Content
```
The expectation is that `bars` and `barsAsEntities` are treated as two "views" into the same data.
To meet this expectation, workloads must:
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved
1. Keep the properties consistent between `bar` and `barAsEntity`.
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved
Any changes to one type must be reflected in the other type.
2. Reject requests that update both collections at the same time.
A request that adds an item to `barsAsEntities` while replacing the content of `bars` must rejected with a `400`, for example:
```http
PATCH /foos/{fooId}
{
"bars": [
{
"prop1": "some value",
"prop2": "another value"
}
],
"barsAsEntities@delta": [
{
"prop1": "a key value",
"prop2": "some new value"
}
]
}
```
```http
HTTP/1.1 400 Bad Request
{
"error": {
"code": "badRequest",
"message": "'bars' and 'barsAsEntities' cannot be updated in the same request.",
}
```

TODO should this be a 409 conflict instead?
TODO implement this in WebApi

### 11.2 `$select` overloading
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved

The model can be updated to simply switch the complex type for an entity type:
```diff
<EntityType Name="foo">
<Key>
<PropertyRef Name="id" />
</Key>
<Property Name="id" Type="Edm.String" Nullable="false" />
- <Property Name="bars" Type="Collection(self.bar)" />
+ <NavigationProperty Name="bars" Type="Collection(self.bar)" ContainsTarget="true" />
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved
</EntityType>

- <ComplexType Name="bar">
+ <EntityType Name="bar">
+ <Key>
+ <PropertyRef Name="prop1" />
+ </Key>
<Property Name="prop1" Type="Edm.String" />
<Property Name="prop2" Type="Edm.String" />
-</ComplexType>
+</EntityType>
```
To maintain backwards compatibility **and** compliance with the OData standard, there are several semantic changes that the workload must address:
1. Existing clients would have been able to `$select` the `bars` property.
Now that `bars` is a navigation property, the [OData standard](https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#_Toc31361040) specifies that its navigation link be returned when it is `$selected`:

> If the select item is a navigation property, then the corresponding navigation link is represented in the response.

Because the previous behavior for `$select=bars` was to include the collection in the response, and because the standard dictates that the navigation link be included in the response, the new behavior is to include both:

```http
GET /foos/{fooId}?$select=bars
```
```http
200 OK
{
"id": "{fooId}",
"bars": [
{
"prop1": "some value",
"prop2": "another value"
},
...
]
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved
"[email protected]": "/foos('{fooId}')/bars"
}
```

2. The default behavior for structural collections is to include them in the response payload for their containing entity. If this was the behavior of `foo` before, it must be preserved by **auto-expanding** the `bars` property now that it is a navigation property (because the default behavior for navigation properties is to **not** expand them).
3. Structural collections are updated using `PATCH` requests to replace the entire contents of the collection. The new navigation property must preserve this behavior.
corranrogue9 marked this conversation as resolved.
Show resolved Hide resolved

TODO implement this in webapi