Skip to content

Commit

Permalink
fixes #2598, Composite key entities PUT,PATCH URIs generated by OData…
Browse files Browse the repository at this point in the history 
… Client do not follow key order
  • Loading branch information
@xuzhg
xuzhg committed Feb 1, 2023
1 parent 12c18fc commit 0e358b8
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 15 deletions.
6 changes: 3 additions & 3 deletions src/Microsoft.OData.Client/ALinq/QueryableResourceExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -507,9 +507,9 @@ internal void SetKeyPredicate(IEnumerable<Expression> keyValues)
/// Gets the key properties from KeyPredicateConjuncts
/// </summary>
/// <returns>The key properties.</returns>
internal Dictionary<PropertyInfo, ConstantExpression> GetKeyProperties()
internal IList<KeyValuePair<PropertyInfo, ConstantExpression>> GetKeyProperties()
{
var keyValues = new Dictionary<PropertyInfo, ConstantExpression>(EqualityComparer<PropertyInfo>.Default);
var keyValues = new List<KeyValuePair<PropertyInfo, ConstantExpression>>();
if (this.keyPredicateConjuncts.Count > 0)
{
foreach (Expression predicate in this.keyPredicateConjuncts)
Expand All @@ -518,7 +518,7 @@ internal Dictionary<PropertyInfo, ConstantExpression> GetKeyProperties()
ConstantExpression constantValue;
if (ResourceBinder.PatternRules.MatchKeyComparison(predicate, out property, out constantValue))
{
keyValues.Add(property, constantValue);
keyValues.Add(new KeyValuePair<PropertyInfo, ConstantExpression>(property, constantValue));
}
}
}
Expand Down
65 changes: 64 additions & 1 deletion src/Microsoft.OData.Client/ALinq/ResourceBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ private static List<Expression> ExtractKeyPredicate(
Debug.Assert(predicates != null, "predicates != null");

Dictionary<PropertyInfo, ConstantExpression> keyValuesFromPredicates = null;
Dictionary<Expression, string> expToPropertyNameMap = null;
nonKeyPredicates = null;
List<Expression> keyPredicates = null;

Expand All @@ -345,6 +346,7 @@ private static List<Expression> ExtractKeyPredicate(
if (keyValuesFromPredicates == null)
{
keyValuesFromPredicates = new Dictionary<PropertyInfo, ConstantExpression>(EqualityComparer<PropertyInfo>.Default);
expToPropertyNameMap = new Dictionary<Expression, string>();
keyPredicates = new List<Expression>();
}
else if (keyValuesFromPredicates.ContainsKey(property))
Expand All @@ -356,6 +358,7 @@ private static List<Expression> ExtractKeyPredicate(

keyValuesFromPredicates.Add(property, constantValue);
keyPredicates.Add(predicate);
expToPropertyNameMap.Add(predicate, ClientTypeUtil.GetServerDefinedName(property));
}
else
{
Expand Down Expand Up @@ -384,19 +387,79 @@ private static List<Expression> ExtractKeyPredicate(

Debug.Assert(schemaType != null, "SchemaType can not be null.");

var keys = schemaType.Key().ToArray();
// Obtain the key properties from EdmEntityType, and compare them with keys obtained from predicates
// By this time we are sure that keyValuesFromPredicates has unique set (i.e. no duplicates), and has all the keys in it.
// So just comparing count with EDMModel's keys is enough for validation
bool allKeysPresent = schemaType.Key().Count() == keyValuesFromPredicates.Keys.Count;
bool allKeysPresent = keys.Length == keyValuesFromPredicates.Keys.Count;

if (!allKeysPresent)
{
keyValuesFromPredicates = null;
keyPredicates = null;
}

if (allKeysPresent && keys.Length > 1)
{
// OData Client generates a request that should be the canonical URL, based on the order of keys in the CSDL.
keyPredicates = AdjustKeyExpressionsBasedOnKeyOrders(keys, keyPredicates, expToPropertyNameMap);
}
}
}

return keyPredicates;
}

private static List<Expression> AdjustKeyExpressionsBasedOnKeyOrders(IEdmStructuralProperty[] edmKeys,
List<Expression> keyPredicates, Dictionary<Expression, string> expToPropertyNameMap)
{
bool needAdjust = false;
for (int i = 0; i < edmKeys.Length; ++i)
{
IEdmStructuralProperty edmKey = edmKeys[i];
Expression keyExpr = keyPredicates[i];

if (edmKey.Name != expToPropertyNameMap[keyExpr])
{
needAdjust = true;
break;
}
}

if (needAdjust)
{
List<Expression> orderedKeyPredicates = new List<Expression>();
foreach (var key in edmKeys)
{
// find
Expression foundExp = null;
foreach (var exp in keyPredicates)
{
string propertyName = expToPropertyNameMap[exp];
if (key.Name == propertyName)
{
foundExp = exp;
break;
}
}

// if found
if (foundExp != null)
{
keyPredicates.Remove(foundExp);
orderedKeyPredicates.Add(foundExp);
}
}

// append the remainings
foreach (var exp in keyPredicates)
{
orderedKeyPredicates.Add(exp);
}

keyPredicates = orderedKeyPredicates;
}

return keyPredicates;
}

Expand Down
13 changes: 12 additions & 1 deletion src/Microsoft.OData.Client/ClientEdmModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,18 @@ private EdmTypeCacheValue GetOrCreateEdmTypeInternal(IEdmStructuredType edmBaseT
entityType.AddProperty(property);
}

entityType.AddKeys(loadedKeyProperties);
// add keys in the same order of "keyProperties"
List<IEdmStructuralProperty> orderedloadedKeyProperties = new List<IEdmStructuralProperty>();
foreach (var orderedKey in keyProperties)
{
var keyProperty = loadedKeyProperties.FirstOrDefault(k => k.Name == orderedKey.Name);
if (keyProperty != null)
{
orderedloadedKeyProperties.Add(keyProperty);
}
}

entityType.AddKeys(orderedloadedKeyProperties);
};

// Creating an entity type
Expand Down
75 changes: 65 additions & 10 deletions src/Microsoft.OData.Client/Metadata/ODataTypeInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ private IEnumerable<PropertyInfo> GetAllProperties()
}

private PropertyInfo[] GetKeyProperties()
{
{
if (CommonUtil.IsUnsupportedType(type))
{
throw new InvalidOperationException(c.Strings.ClientType_UnsupportedType(type));
Expand All @@ -281,27 +281,34 @@ private PropertyInfo[] GetKeyProperties()
IEnumerable<object> customAttributes = type.GetCustomAttributes(true);
bool isEntity = customAttributes.OfType<EntityTypeAttribute>().Any();
KeyAttribute dataServiceKeyAttribute = customAttributes.OfType<KeyAttribute>().FirstOrDefault();
List<PropertyInfo> keyProperties = new List<PropertyInfo>();


List<KeyValuePair<PropertyInfo, int>> keyWithOrders = new List<KeyValuePair<PropertyInfo, int>>();

KeyKind currentKeyKind = KeyKind.NotKey;
KeyKind newKeyKind = KeyKind.NotKey;
foreach (PropertyInfo propertyInfo in Properties)
{
if ((newKeyKind = IsKeyProperty(propertyInfo, dataServiceKeyAttribute)) != KeyKind.NotKey)
if ((newKeyKind = IsKeyProperty(propertyInfo, dataServiceKeyAttribute, out int order)) != KeyKind.NotKey)
{
if (newKeyKind > currentKeyKind)
{
keyProperties.Clear();
keyWithOrders.Clear();
currentKeyKind = newKeyKind;
keyProperties.Add(propertyInfo);
InserKeyBasedOnOrder(keyWithOrders, propertyInfo, order);
}
else if (newKeyKind == currentKeyKind)
{
keyProperties.Add(propertyInfo);
InserKeyBasedOnOrder(keyWithOrders, propertyInfo, order);
}
}
}

List<PropertyInfo> keyProperties = new List<PropertyInfo>();
foreach (var item in keyWithOrders)
{
keyProperties.Add(item.Key);
}

Type keyPropertyDeclaringType = null;
foreach (PropertyInfo key in keyProperties)
{
Expand Down Expand Up @@ -336,26 +343,54 @@ private PropertyInfo[] GetKeyProperties()
return keyProperties.Count > 0 ? keyProperties.ToArray() : (isEntity ? ClientTypeUtil.EmptyPropertyInfoArray : null);
}

private static void InserKeyBasedOnOrder(List<KeyValuePair<PropertyInfo, int>> keys, PropertyInfo keyPi, int order)
{
var newKeyWithOrder = new KeyValuePair<PropertyInfo, int>(keyPi, order);
if (order < 0)
{
// order < 0 means there's no order value setting, append this key at the end of list.
keys.Add(newKeyWithOrder);
}
else
{
int index = 0;
foreach (var key in keys)
{
if (key.Value < 0 || key.Value > order)
{
// Insert the new key before the first negative order or first order bigger than new order.
// If the new order value is same as one item in the list , move to next.
break;
}
}

keys.Insert(index, newKeyWithOrder);
}
}

/// <summary>
/// Returns the KeyKind if <paramref name="propertyInfo"/> is declared as a key in <paramref name="dataServiceKeyAttribute"/> or it follows the key naming convention.
/// </summary>
/// <param name="propertyInfo">Property in question.</param>
/// <param name="dataServiceKeyAttribute">DataServiceKeyAttribute instance.</param>
/// <returns>Returns the KeyKind if <paramref name="propertyInfo"/> is declared as a key in <paramref name="dataServiceKeyAttribute"/> or it follows the key naming convention.</returns>
private static KeyKind IsKeyProperty(PropertyInfo propertyInfo, KeyAttribute dataServiceKeyAttribute)
private static KeyKind IsKeyProperty(PropertyInfo propertyInfo, KeyAttribute dataServiceKeyAttribute, out int order)
{
Debug.Assert(propertyInfo != null, "propertyInfo != null");

string propertyName = ClientTypeUtil.GetServerDefinedName(propertyInfo);

order = -1;
KeyKind keyKind = KeyKind.NotKey;
if (dataServiceKeyAttribute != null && dataServiceKeyAttribute.KeyNames.Contains(propertyName))
{
order = dataServiceKeyAttribute.KeyNames.IndexOf(propertyName);
keyKind = KeyKind.AttributedKey;
}
else if (propertyInfo.GetCustomAttributes().OfType<System.ComponentModel.DataAnnotations.KeyAttribute>().Any())
else if (IsDataAnnotationsKeyProperty(propertyInfo, out KeyKind newKind, out int newOrder))
{
keyKind = KeyKind.AttributedKey;
order = newOrder;
keyKind = newKind;
}
else if (propertyName.EndsWith("ID", StringComparison.Ordinal))
{
Expand All @@ -374,5 +409,25 @@ private static KeyKind IsKeyProperty(PropertyInfo propertyInfo, KeyAttribute dat

return keyKind;
}

private static bool IsDataAnnotationsKeyProperty(PropertyInfo propertyInfo, out KeyKind kind, out int order)
{
order = -1;
kind = KeyKind.NotKey;
var attributes = propertyInfo.GetCustomAttributes();
if(!attributes.Any(a => a is System.ComponentModel.DataAnnotations.KeyAttribute))
{
return false;
}

var columnAttribute = attributes.OfType<System.ComponentModel.DataAnnotations.Schema.ColumnAttribute>().FirstOrDefault();
if (columnAttribute != null)
{
order = columnAttribute.Order;
}

kind = KeyKind.AttributedKey;
return true;
}
}
}

0 comments on commit 0e358b8

Please sign in to comment.