Cleanup for UnicodeAttribute PR

dotnet · Oct 27, 2020 · a302bac · a302bac
1 parent bd6af77
commit a302bac
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 36 deletions.
diff --git a/src/EFCore.Abstractions/UnicodeAttribute.cs b/src/EFCore.Abstractions/UnicodeAttribute.cs
@@ -11,18 +11,18 @@ namespace Microsoft.EntityFrameworkCore
     [AttributeUsage(AttributeTargets.Property | AttributeTargets.Field, AllowMultiple = false)]
     public sealed class UnicodeAttribute : Attribute
     {
-        /// <summary>
-        ///     A value indicating whether the property can contain unicode characters or not.
-        /// </summary>
-        public bool IsUnicode { get; }
-
         /// <summary>
         ///     Initializes a new instance of the <see cref="UnicodeAttribute" /> class.
         /// </summary>
-        /// <param name="isUnicode">A value indicating whether the property can contain unicode characters or not.</param>
-        public UnicodeAttribute(bool isUnicode = true)
+        /// <param name="unicode"> A value indicating whether the property can contain unicode characters or not. </param>
+        public UnicodeAttribute(bool unicode = true)
         {
-            IsUnicode = isUnicode;
+            IsUnicode = unicode;
         }
+
+        /// <summary>
+        ///     A value indicating whether the property can contain unicode characters or not.
+        /// </summary>
+        public bool IsUnicode { get; }
     }
 }
diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs
@@ -373,18 +373,19 @@ private void GenerateMaxLengthAttribute(IProperty property)
 
         private void GenerateUnicodeAttribute(IProperty property)
         {
-            CoreTypeMapping typeMapping = property.FindTypeMapping();
-            if (property.ClrType != typeof(string) && typeMapping.Converter?.ProviderClrType != typeof(string))
+            if (property.ClrType != typeof(string))
             {
                 return;
             }
 
-            var isUnicode = property.IsUnicode();
-
-            if (isUnicode.HasValue && !isUnicode.Value)
+            var unicode = property.IsUnicode();
+            if (unicode.HasValue)
             {
                 var unicodeAttribute = new AttributeWriter(nameof(UnicodeAttribute));
-                unicodeAttribute.AddParameter(_code.Literal(false));
+                if (!unicode.Value)
+                {
+                    unicodeAttribute.AddParameter(_code.Literal(false));
+                }
                 _sb.AppendLine(unicodeAttribute.ToString());
             }
         }

diff --git a/src/EFCore/Metadata/Conventions/UnicodeAttributeConvention.cs b/src/EFCore/Metadata/Conventions/UnicodeAttributeConvention.cs
@@ -35,7 +35,7 @@ protected override void ProcessPropertyAdded(
             MemberInfo clrMember,
             IConventionContext context)
         {
-            propertyBuilder.IsUnicode(unicode: attribute.IsUnicode, fromDataAnnotation: true);
+            propertyBuilder.IsUnicode(attribute.IsUnicode, fromDataAnnotation: true);
         }
     }
 }
diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs
@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Linq;
 using Microsoft.EntityFrameworkCore.Internal;
 using Xunit;
@@ -859,8 +860,6 @@ public void UnicodeAttribute_is_generated_for_property()
                             x.Property<string>("A").HasMaxLength(34).IsUnicode();
                             x.Property<string>("B").HasMaxLength(34).IsUnicode(false);
                             x.Property<string>("C").HasMaxLength(34);
-                            x.Property<System.DateTimeOffset>("D").HasConversion<string>();
-                            x.Property<System.DateTimeOffset>("E").HasConversion<string>().IsUnicode(false);
                         }),
                 new ModelCodeGenerationOptions { UseDataAnnotations = true },
                 code =>
@@ -881,15 +880,13 @@ public partial class Entity
         [Key]
         public int Id { get; set; }
         [StringLength(34)]
+        [Unicode]
         public string A { get; set; }
         [StringLength(34)]
         [Unicode(false)]
         public string B { get; set; }
         [StringLength(34)]
         public string C { get; set; }
-        public DateTimeOffset D { get; set; }
-        [Unicode(false)]
-        public DateTimeOffset E { get; set; }
     }
 }
 ",
@@ -901,8 +898,6 @@ public partial class Entity
                     Assert.True(entitType.GetProperty("A").IsUnicode());
                     Assert.False(entitType.GetProperty("B").IsUnicode());
                     Assert.Null(entitType.GetProperty("C").IsUnicode());
-                    Assert.Null(entitType.GetProperty("D").IsUnicode());
-                    Assert.False(entitType.GetProperty("E").IsUnicode());
                 });
         }
 

diff --git a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs
@@ -2379,29 +2379,22 @@ public virtual void TimestampAttribute_throws_if_value_in_database_changed()
         }
 
         [ConditionalFact]
-        public virtual void Unicode_annotation_is_enabled()
+        public virtual void UnicodeAttribute_sets_unicode_for_properties_and_fields()
         {
             var modelBuilder = CreateModelBuilder();
 
-            modelBuilder.Entity<UnicodeAnnotationClass>();
+            modelBuilder.Entity<UnicodeAnnotationClass>(b =>
+            {
+                b.Property(e => e.PersonMiddleName);
+                b.Property(e => e.PersonAddress);
+            });
 
             Validate(modelBuilder);
 
             Assert.True(GetProperty<UnicodeAnnotationClass>(modelBuilder, "PersonFirstName").IsUnicode());
             Assert.False(GetProperty<UnicodeAnnotationClass>(modelBuilder, "PersonLastName").IsUnicode());
-        }
-
-        [ConditionalFact]
-        public virtual void Unicode_is_configured_explicitly_for_fields()
-        {
-            var modelBuilder = CreateModelBuilder();
-
-            modelBuilder.Entity<UnicodeAnnotationClass>().Property(n => n.PersonMiddleName);
-            modelBuilder.Entity<UnicodeAnnotationClass>().Property(n => n.PersonAddress);
 
-            Validate(modelBuilder);
-
-            Assert.Null(GetProperty<UnicodeAnnotationClass>(modelBuilder, "PersonMiddleName").IsUnicode());
+            Assert.True(GetProperty<UnicodeAnnotationClass>(modelBuilder, "PersonMiddleName").IsUnicode());
             Assert.False(GetProperty<UnicodeAnnotationClass>(modelBuilder, "PersonAddress").IsUnicode());
         }
 
@@ -2415,6 +2408,7 @@ protected class UnicodeAnnotationClass
             [Unicode(false)]
             public string PersonLastName { get; set; }
 
+            [Unicode]
             public string PersonMiddleName;
 
             [Unicode(false)]