Skip to content

Delta does not allow UpdatableProperties to be changed #2483

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

Merged
merged 2 commits into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
47 changes: 23 additions & 24 deletions src/Microsoft.AspNet.OData.Shared/DeltaOfTStructuralType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.Contracts;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
using Microsoft.AspNet.OData.Builder.Conventions.Attributes;
using Microsoft.AspNet.OData.Common;
using Microsoft.AspNet.OData.Formatter;

Expand All @@ -24,11 +22,11 @@ namespace Microsoft.AspNet.OData
public class Delta<TStructuralType> : TypedDelta, IDelta where TStructuralType : class
{
// cache property accessors for this type and all its derived types.
private static ConcurrentDictionary<Type, Dictionary<string, PropertyAccessor<TStructuralType>>> _propertyCache
private static readonly ConcurrentDictionary<Type, Dictionary<string, PropertyAccessor<TStructuralType>>> _propertyCache
= new ConcurrentDictionary<Type, Dictionary<string, PropertyAccessor<TStructuralType>>>();

private Dictionary<string, PropertyAccessor<TStructuralType>> _allProperties;
private HashSet<string> _updatableProperties;
private List<string> _updatableProperties;

private HashSet<string> _changedProperties;

Expand All @@ -38,7 +36,7 @@ private static ConcurrentDictionary<Type, Dictionary<string, PropertyAccessor<TS
private TStructuralType _instance;
private Type _structuredType;

private PropertyInfo _dynamicDictionaryPropertyinfo;
private readonly PropertyInfo _dynamicDictionaryPropertyinfo;
private HashSet<string> _changedDynamicProperties;
private IDictionary<string, object> _dynamicDictionaryCache;

Expand Down Expand Up @@ -94,18 +92,19 @@ public Delta(Type structuralType, IEnumerable<string> updatableProperties,

/// <inheritdoc/>
public override Type StructuredType
{
get
{
return _structuredType;
}
}
=> _structuredType;

/// <inheritdoc/>
public override Type ExpectedClrType
{
get { return typeof(TStructuralType); }
}
=> typeof(TStructuralType);

/// <summary>
/// The list of property names that can be updated.
/// </summary>
/// <remarks>When the list is modified, any modified properties that were removed from the list are no longer
/// considered to be changed.</remarks>
public IList<string> UpdatableProperties
=> _updatableProperties;

/// <inheritdoc/>
public override void Clear()
Expand All @@ -116,7 +115,7 @@ public override void Clear()
/// <inheritdoc/>
public override bool TrySetPropertyValue(string name, object value)
{
if (string.IsNullOrWhiteSpace(name))
if (String.IsNullOrWhiteSpace(name))
{
throw Error.ArgumentNull("name");
}
Expand Down Expand Up @@ -171,7 +170,7 @@ public override bool TryGetPropertyValue(string name, out object value)
}
}

if (this._deltaNestedResources.ContainsKey(name))
if (_deltaNestedResources.ContainsKey(name))
{
// If this is a nested resource, get the value from the dictionary of nested resources.
object deltaNestedResource = _deltaNestedResources[name];
Expand Down Expand Up @@ -259,7 +258,7 @@ public TStructuralType GetInstance()
/// </summary>
public override IEnumerable<string> GetChangedPropertyNames()
{
return _changedProperties.Concat(_deltaNestedResources.Keys);
return _changedProperties.Intersect(_updatableProperties).Concat(_deltaNestedResources.Keys);
}

/// <summary>
Expand All @@ -269,7 +268,8 @@ public override IEnumerable<string> GetChangedPropertyNames()
/// </summary>
public override IEnumerable<string> GetUnchangedPropertyNames()
{
return _updatableProperties.Except(GetChangedPropertyNames());
// UpdatableProperties could include arbitrary strings, filter by _allProperties
return _updatableProperties.Intersect(_allProperties.Keys).Except(GetChangedPropertyNames());
}

/// <summary>
Expand All @@ -295,7 +295,7 @@ public void CopyChangedValues(TStructuralType original)

// For regular non-structural properties at current level.
PropertyAccessor<TStructuralType>[] propertiesToCopy =
this._changedProperties.Select(s => _allProperties[s]).ToArray();
_changedProperties.Intersect(_updatableProperties).Select(s => _allProperties[s]).ToArray();
foreach (PropertyAccessor<TStructuralType> propertyToCopy in propertiesToCopy)
{
propertyToCopy.Copy(_instance, original);
Expand Down Expand Up @@ -505,12 +505,11 @@ private void InitializeProperties(IEnumerable<string> updatableProperties)

if (updatableProperties != null)
{
_updatableProperties = new HashSet<string>(updatableProperties);
_updatableProperties.IntersectWith(_allProperties.Keys);
_updatableProperties = updatableProperties.Intersect(_allProperties.Keys).ToList();
}
else
{
_updatableProperties = new HashSet<string>(_allProperties.Keys);
_updatableProperties = new List<string>(_allProperties.Keys);
}

if (_dynamicDictionaryPropertyinfo != null)
Expand Down Expand Up @@ -625,7 +624,7 @@ private bool TrySetPropertyValueInternal(string name, object value)
throw Error.ArgumentNull("name");
}

if (!_updatableProperties.Contains(name))
if (!(_allProperties.ContainsKey(name) && _updatableProperties.Contains(name)))
{
return false;
}
Expand Down Expand Up @@ -655,7 +654,7 @@ private bool TrySetNestedResourceInternal(string name, object deltaNestedResourc
throw Error.ArgumentNull("name");
}

if (!_updatableProperties.Contains(name))
if (!(_allProperties.ContainsKey(name) && _updatableProperties.Contains(name)))
{
return false;
}
Expand Down
Loading