Merge pull request #148 from servicetitan/fixArrayReverse

Fix critical bug with attribute reordering by destructive Array.Reverse() on memoized array

Author: alex-kulakov

Orm/Xtensive.Orm/IoC/ServiceRegistration.cs

@@ -83,7 +83,7 @@ public static ServiceRegistration[] CreateAll(Type type, bool defaultOnly) =>
         return Array.Empty<ServiceRegistration>();
 
       var attributes = type.GetAttributes<ServiceAttribute>(AttributeSearchOptions.InheritNone);
-      var registrations = new List<ServiceRegistration>(attributes.Length);
+      var registrations = new List<ServiceRegistration>(attributes.Count);
       foreach (var sa in attributes) {
         if (!defaultOnly || sa.Default) {
           registrations.Add(new ServiceRegistration(sa.Type, sa.Name.IsNullOrEmpty() ? null : sa.Name, type, sa.Singleton));

Orm/Xtensive.Orm/Linq/ExpressionWriter.cs

@@ -1,4 +1,4 @@
-// Copyright (C) 2008-2020 Xtensive LLC.
+// Copyright (C) 2008-2021 Xtensive LLC.
 // This code is distributed under MIT license terms.
 // See the License.txt file in the project root for more information.
 // Created by: Alexey Kochetov
@@ -294,12 +294,12 @@ protected virtual string GetTypeName(Type type)
       name = name.Replace('+', '.');
 
       if (name.IndexOf("__DisplayClass", StringComparison.Ordinal) > 0 &&
-        type.GetAttributes<CompilerGeneratedAttribute>(AttributeSearchOptions.InheritNone).Length > 0) {
+        type.GetAttributes<CompilerGeneratedAttribute>(AttributeSearchOptions.InheritNone).Count > 0) {
         return "@";
       }
 
       if (name.IndexOf("__AnonymousType", StringComparison.Ordinal) > 0 &&
-        type.GetAttributes<CompilerGeneratedAttribute>(AttributeSearchOptions.InheritNone).Length > 0) {
+        type.GetAttributes<CompilerGeneratedAttribute>(AttributeSearchOptions.InheritNone).Count > 0) {
         return string.Format("@<{0}>",
           (from pi in type.GetProperties() select pi.Name).ToCommaDelimitedString());
       }
@@ -340,7 +340,7 @@ protected override Expression VisitConstant(ConstantExpression c)
     {
       var type = c.Type;
       if (type.Name.IndexOf("__DisplayClass", StringComparison.Ordinal) > 0 &&
-        type.GetAttributes<CompilerGeneratedAttribute>(AttributeSearchOptions.InheritNone).Length > 0) {
+        type.GetAttributes<CompilerGeneratedAttribute>(AttributeSearchOptions.InheritNone).Count > 0) {
         // A constant of display class type
         Write("@");
       }
@@ -556,7 +556,7 @@ protected override Expression VisitMethodCall(MethodCallExpression mc)
       }
       else {
         // Static method
-        if (mc.Method.GetAttributes<ExtensionAttribute>(AttributeSearchOptions.InheritNone).Length > 0) {
+        if (mc.Method.GetAttributes<ExtensionAttribute>(AttributeSearchOptions.InheritNone).Count > 0) {
           // A special case: extension method
           Visit(mc.Arguments[0]);
           arguments = new System.Collections.ObjectModel.ReadOnlyCollection<Expression>(mc.Arguments.Skip(1).ToList());

Orm/Xtensive.Orm/Orm/Building/Builders/ModelDefBuilder.cs

@@ -1,4 +1,4 @@
-// Copyright (C) 2009-2020 Xtensive LLC.
+// Copyright (C) 2009-2021 Xtensive LLC.
 // This code is distributed under MIT license terms.
 // See the License.txt file in the project root for more information.
 // Created by: Dmitri Maximov
@@ -151,12 +151,12 @@ public void ProcessProperties(TypeDef typeDef, HierarchyDef hierarchyDef)
         }
 
         // FieldAttribute presence is required
-        var fieldAttributes = GetFieldAttributes<FieldAttribute>(propertyInfo);
-        if (fieldAttributes.Length == 0) {
+        var reversedFieldAttributes = GetReversedFieldAttributes<FieldAttribute>(propertyInfo);
+        if (reversedFieldAttributes.Count == 0) {
           continue;
         }
 
-        var field = DefineField(propertyInfo, fieldAttributes);
+        var field = DefineField(propertyInfo, reversedFieldAttributes);
 
         // Declared & inherited fields must be processed for hierarchy root
         if (hierarchyDef != null) {
@@ -284,9 +284,9 @@ public HierarchyDef DefineHierarchy(TypeDef typeDef, HierarchyRootAttribute attr
     }
 
     public FieldDef DefineField(PropertyInfo propertyInfo) =>
-      DefineField(propertyInfo, GetFieldAttributes<FieldAttribute>(propertyInfo));
+      DefineField(propertyInfo, GetReversedFieldAttributes<FieldAttribute>(propertyInfo));
 
-    public FieldDef DefineField(PropertyInfo propertyInfo, FieldAttribute[] fieldAttributes)
+    public FieldDef DefineField(PropertyInfo propertyInfo, IReadOnlyList<FieldAttribute> reversedFieldAttributes)
     {
       // Persistent indexers are not supported
       var indexParameters = propertyInfo.GetIndexParameters();
@@ -297,21 +297,21 @@ public FieldDef DefineField(PropertyInfo propertyInfo, FieldAttribute[] fieldAtt
       var fieldDef = new FieldDef(propertyInfo, context.Validator);
       fieldDef.Name = context.NameBuilder.BuildFieldName(fieldDef);
 
-      if (fieldAttributes.Length > 0) {
-        foreach (var attribute in fieldAttributes) {
-          attributeProcessor.Process(fieldDef, attribute);
+      if (reversedFieldAttributes.Count > 0) {
+        for (int i = reversedFieldAttributes.Count; i-- > 0;) {
+          attributeProcessor.Process(fieldDef, reversedFieldAttributes[i]);
         }
 
         // Association
-        var associationAttributes = GetFieldAttributes<AssociationAttribute>(propertyInfo);
-        foreach (var attribute in associationAttributes) {
-          attributeProcessor.Process(fieldDef, attribute);
+        var reversedAssociationAttributes = GetReversedFieldAttributes<AssociationAttribute>(propertyInfo);
+        for (int i = reversedAssociationAttributes.Count; i-- > 0;) {
+          attributeProcessor.Process(fieldDef, reversedAssociationAttributes[i]);
         }
 
         // Mapping name
-        var mappingAttributes = GetFieldAttributes<FieldMappingAttribute>(propertyInfo);
-        foreach (var attribute in mappingAttributes) {
-          attributeProcessor.Process(fieldDef, attribute);
+        var reversedMappingAttributes = GetReversedFieldAttributes<FieldMappingAttribute>(propertyInfo);
+        for (int i = reversedMappingAttributes.Count; i-- > 0;) {
+          attributeProcessor.Process(fieldDef, reversedMappingAttributes[i]);
         }
 
         // Type discriminator
@@ -370,16 +370,11 @@ private bool ShouldSkipFulltextIndex(TypeDef typeDef)
       return hierarchy == null && !typeDef.IsStructure;
     }
 
-    private static T[] GetFieldAttributes<T>(PropertyInfo property)
-      where T : Attribute
-    {
-      var attributes = property.GetAttributes<T>(AttributeSearchOptions.InheritAll);
-      // Attributes will contain attributes from all inheritance chain
-      // with the most specific type first.
-      // Reverse them for correct processing (i.e. descendants override settings from base).
-      Array.Reverse(attributes);
-      return attributes;
-    }
+    // Attributes will contain attributes from all inheritance chain
+    // with the most specific type first.
+    // Should be enumerated in reversed order for correct processing (i.e. descendants override settings from base).
+    private static IReadOnlyList<T> GetReversedFieldAttributes<T>(PropertyInfo property) where T : Attribute =>
+      property.GetAttributes<T>(AttributeSearchOptions.InheritAll);
 
     private bool IsTypeAvailable(Type type) =>
       context.BuilderConfiguration.ModelFilter.IsTypeAvailable(type)

Orm/Xtensive.Orm/Reflection/AttributeHelper.cs

@@ -36,7 +36,7 @@ private static readonly ConcurrentDictionary<AttributesKey, Attribute[]> attribu
     /// <param name="options">Attribute search options.</param>
     /// <returns>An array of attributes of specified type.</returns>
     ///
-    public static TAttribute[] GetAttributes<TAttribute>(this MemberInfo member, AttributeSearchOptions options = AttributeSearchOptions.InheritNone)
+    public static IReadOnlyList<TAttribute> GetAttributes<TAttribute>(this MemberInfo member, AttributeSearchOptions options = AttributeSearchOptions.InheritNone)
         where TAttribute : Attribute =>
       AttributeDictionary<TAttribute>.Dictionary.GetOrAdd(
         new PerAttributeKey(member, options),
@@ -59,7 +59,7 @@ public static TAttribute GetAttribute<TAttribute>(this MemberInfo member, Attrib
       where TAttribute : Attribute
     {
       var attributes = member.GetAttributes<TAttribute>(options);
-      return attributes.Length switch {
+      return attributes.Count switch {
         0 => null,
         1 => attributes[0],
         _ => throw new InvalidOperationException(string.Format(Strings.ExMultipleAttributesOfTypeXAreNotAllowedHere,
@@ -68,7 +68,7 @@ public static TAttribute GetAttribute<TAttribute>(this MemberInfo member, Attrib
       };
     }
 
-    private static Attribute[] GetAttributes(MemberInfo member, Type attributeType, AttributeSearchOptions options) =>
+    private static IReadOnlyList<Attribute> GetAttributes(MemberInfo member, Type attributeType, AttributeSearchOptions options) =>
       attributesByMemberInfoAndSearchOptions.GetOrAdd(
         new AttributesKey(member, attributeType, options),
         t => ExtractAttributes(t).ToArray()