From efa53978bf4c24d1c8146346be39c984164cc2c1 Mon Sep 17 00:00:00 2001 From: Sam Xu Date: Wed, 18 Jan 2023 17:34:51 -0800 Subject: [PATCH] fixes #2598, Composite key entities PUT,PATCH URIs generated by OData Client do not follow key order --- .../ALinq/QueryableResourceExpression.cs | 6 +- .../ALinq/ResourceBinder.cs | 65 +++++++++++++++- src/Microsoft.OData.Client/ClientEdmModel.cs | 13 +++- .../Metadata/ODataTypeInfo.cs | 75 ++++++++++++++++--- 4 files changed, 144 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.OData.Client/ALinq/QueryableResourceExpression.cs b/src/Microsoft.OData.Client/ALinq/QueryableResourceExpression.cs index 141ef45799..c2333e9d94 100644 --- a/src/Microsoft.OData.Client/ALinq/QueryableResourceExpression.cs +++ b/src/Microsoft.OData.Client/ALinq/QueryableResourceExpression.cs @@ -507,9 +507,9 @@ internal void SetKeyPredicate(IEnumerable keyValues) /// Gets the key properties from KeyPredicateConjuncts /// /// The key properties. - internal Dictionary GetKeyProperties() + internal IList> GetKeyProperties() { - var keyValues = new Dictionary(EqualityComparer.Default); + var keyValues = new List>(); if (this.keyPredicateConjuncts.Count > 0) { foreach (Expression predicate in this.keyPredicateConjuncts) @@ -518,7 +518,7 @@ internal Dictionary GetKeyProperties() ConstantExpression constantValue; if (ResourceBinder.PatternRules.MatchKeyComparison(predicate, out property, out constantValue)) { - keyValues.Add(property, constantValue); + keyValues.Add(new KeyValuePair(property, constantValue)); } } } diff --git a/src/Microsoft.OData.Client/ALinq/ResourceBinder.cs b/src/Microsoft.OData.Client/ALinq/ResourceBinder.cs index 780459ca57..ab8d604215 100644 --- a/src/Microsoft.OData.Client/ALinq/ResourceBinder.cs +++ b/src/Microsoft.OData.Client/ALinq/ResourceBinder.cs @@ -333,6 +333,7 @@ private static List ExtractKeyPredicate( Debug.Assert(predicates != null, "predicates != null"); Dictionary keyValuesFromPredicates = null; + Dictionary expToPropertyNameMap = null; nonKeyPredicates = null; List keyPredicates = null; @@ -345,6 +346,7 @@ private static List ExtractKeyPredicate( if (keyValuesFromPredicates == null) { keyValuesFromPredicates = new Dictionary(EqualityComparer.Default); + expToPropertyNameMap = new Dictionary(); keyPredicates = new List(); } else if (keyValuesFromPredicates.ContainsKey(property)) @@ -356,6 +358,7 @@ private static List ExtractKeyPredicate( keyValuesFromPredicates.Add(property, constantValue); keyPredicates.Add(predicate); + expToPropertyNameMap.Add(predicate, ClientTypeUtil.GetServerDefinedName(property)); } else { @@ -384,19 +387,79 @@ private static List 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 AdjustKeyExpressionsBasedOnKeyOrders(IEdmStructuralProperty[] edmKeys, + List keyPredicates, Dictionary 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 orderedKeyPredicates = new List(); + 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; } diff --git a/src/Microsoft.OData.Client/ClientEdmModel.cs b/src/Microsoft.OData.Client/ClientEdmModel.cs index ef2b5d431d..49d7d973f4 100644 --- a/src/Microsoft.OData.Client/ClientEdmModel.cs +++ b/src/Microsoft.OData.Client/ClientEdmModel.cs @@ -473,7 +473,18 @@ private EdmTypeCacheValue GetOrCreateEdmTypeInternal(IEdmStructuredType edmBaseT entityType.AddProperty(property); } - entityType.AddKeys(loadedKeyProperties); + // add keys in the same order of "keyProperties" + List orderedloadedKeyProperties = new List(); + 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 diff --git a/src/Microsoft.OData.Client/Metadata/ODataTypeInfo.cs b/src/Microsoft.OData.Client/Metadata/ODataTypeInfo.cs index ea1393574c..fdec93fea7 100644 --- a/src/Microsoft.OData.Client/Metadata/ODataTypeInfo.cs +++ b/src/Microsoft.OData.Client/Metadata/ODataTypeInfo.cs @@ -271,7 +271,7 @@ private IEnumerable GetAllProperties() } private PropertyInfo[] GetKeyProperties() - { + { if (CommonUtil.IsUnsupportedType(type)) { throw new InvalidOperationException(c.Strings.ClientType_UnsupportedType(type)); @@ -281,27 +281,34 @@ private PropertyInfo[] GetKeyProperties() IEnumerable customAttributes = type.GetCustomAttributes(true); bool isEntity = customAttributes.OfType().Any(); KeyAttribute dataServiceKeyAttribute = customAttributes.OfType().FirstOrDefault(); - List keyProperties = new List(); - + + List> keyWithOrders = new List>(); + 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 keyProperties = new List(); + foreach (var item in keyWithOrders) + { + keyProperties.Add(item.Key); + } + Type keyPropertyDeclaringType = null; foreach (PropertyInfo key in keyProperties) { @@ -336,26 +343,54 @@ private PropertyInfo[] GetKeyProperties() return keyProperties.Count > 0 ? keyProperties.ToArray() : (isEntity ? ClientTypeUtil.EmptyPropertyInfoArray : null); } + private static void InserKeyBasedOnOrder(List> keys, PropertyInfo keyPi, int order) + { + var newKeyWithOrder = new KeyValuePair(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); + } + } + /// /// Returns the KeyKind if is declared as a key in or it follows the key naming convention. /// /// Property in question. /// DataServiceKeyAttribute instance. /// Returns the KeyKind if is declared as a key in or it follows the key naming convention. - 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().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)) { @@ -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().FirstOrDefault(); + if (columnAttribute != null) + { + order = columnAttribute.Order; + } + + kind = KeyKind.AttributedKey; + return true; + } } }