Conversation
| } | ||
|
|
||
| if (value is IDelta) | ||
| if (value is IDelta || value is IDeltaSet) |
There was a problem hiding this comment.
Can we use or pattern matching here:
| if (value is IDelta || value is IDeltaSet) | |
| if (value is IDelta or IDeltaSet) |
There was a problem hiding this comment.
That's C# 9 feature and it seems we have to update the csproj to enable C# 9 language.
I don't want to introduce that changes in this PR. Thanks.
|
|
||
| Contract.Assert(deltaNestedResource != null, "deltaNestedResource != null"); | ||
|
|
||
| //If DeltaSet collection, we are handling delta collections so the value will be that itself and no need to get instance value |
There was a problem hiding this comment.
| //If DeltaSet collection, we are handling delta collections so the value will be that itself and no need to get instance value | |
| // If DeltaSet collection, we are handling delta collections so the value will be that itself and no need to get instance value |
|
|
||
| if (deltaNestedResource is IDeltaSet) | ||
| { | ||
| // TODO: That's the bulk insert OData Path handler feature, |
There was a problem hiding this comment.
Instead of leaving a TODO in the code, I'd suggest adding a link to an issue in github documenting this feature.
| FieldInfo field = deltaNestedResource.GetType().GetField("_instance", BindingFlags.NonPublic | BindingFlags.Instance); | ||
| Contract.Assert(field != null, "field != null"); | ||
| cacheHit.SetValue(_instance, field.GetValue(deltaNestedResource)); | ||
| if (!(deltaNestedResource is IDeltaSet)) |
There was a problem hiding this comment.
| if (!(deltaNestedResource is IDeltaSet)) | |
| if (deltaNestedResource is not IDeltaSet) |
There was a problem hiding this comment.
Same as the above one. That's C# 9 feature and I 'd skip it in this PR. Thanks.
| { | ||
| PropertyAccessor<T> cacheHit = _allProperties[name]; | ||
| // Get the Delta<{NestedResourceType}>._instance using Reflection. | ||
| FieldInfo field = deltaNestedResource.GetType().GetField("_instance", BindingFlags.NonPublic | BindingFlags.Instance); |
There was a problem hiding this comment.
I know this was already like this in the previous code, but why do we use reflection on a type that we own? Or am I missing something here?
There was a problem hiding this comment.
I think deltaNestedResource is a generic type instance, DeltaSet< T >. We don't know the real type until running.
| /// <param name="model">The Edm model.</param> | ||
| /// <param name="structuredType">The given structured type.</param> | ||
| /// <returns>All property names.</returns> | ||
| public static IList<string> GetAllProperties(this IEdmModel model, IEdmStructuredType structuredType) |
There was a problem hiding this comment.
We don't seem to require index access to this collection, so I'd suggest ICollection instead of IList.
Having said that, can we use either a readonly or an immutable variant?
Alternatively, this could be an iterator method which would not require allocating a collection and would lead to simpler code as well.
There was a problem hiding this comment.
Changed as suggested. Thanks.
| IDeltaSet set = value as IDeltaSet; | ||
| if (set != null) |
There was a problem hiding this comment.
| IDeltaSet set = value as IDeltaSet; | |
| if (set != null) | |
| if (value is IDeltaSet set) |
vcantor
left a comment
vcantor
left a comment
There was a problem hiding this comment.
Looks good to me. The deltaset can be obtained using TryGetPropertyValue. Thank you very much!
3 participants
Issue #743
Enable the deep update for the nested delta collection.
Request Header:
Patch to single entity with nested delta collection:
{ "name":"anothertest", "@id": "Order(2)", "items@delta":[ { "@id": "OrderItems(2, 1)", "price":0 }, { "@removed":{ "reason":"deleted" }, "@id": "OrderItems(2, 2)" } ] }Patch to single entity with nested collection:
{ "name":"anothertest", "@id": "Order(2)", "items":[ { "price":0 }, { "price":1 } ] }