Fix scaffolding type mapper FixedLength bug

Fixes #14160 Superceeds #14162 ScafoldingTypeMapper should have been passing `false` when looking for the default mapping, since, non-fixed length is the default. (Compare this to Unicode, where we pass `true` because Unicode is the default.) Also included is a fix for test, which wasn't Asserting for fixed length, as found by Smit. Also fixed the code in RelationalTypeMappingInfo that was for some reason using parsed-length to drive the fixed-length flag, although this was a secondary issue.
dotnet · Dec 19, 2018 · 2f07f47 · 2f07f47
1 parent af13b52
commit 2f07f47
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 14 deletions.
diff --git a/src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs b/src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs
@@ -72,7 +72,7 @@ public virtual TypeScaffoldingInfo FindMapping(
                         keyOrIndex,
                         rowVersion: rowVersion,
                         size: mapping.Size,
-                        fixedLength: true);
+                        fixedLength: false);
 
                     scaffoldFixedLength = fixedLengthMapping.IsFixedLength != byteArrayMapping.IsFixedLength ? (bool?)byteArrayMapping.IsFixedLength : null;
 
@@ -120,7 +120,7 @@ public virtual TypeScaffoldingInfo FindMapping(
                         keyOrIndex,
                         unicode: mapping.IsUnicode,
                         size: mapping.Size,
-                        fixedLength: true);
+                        fixedLength: false);
 
                     scaffoldFixedLength = fixedLengthMapping.IsFixedLength != stringMapping.IsFixedLength ? (bool?)stringMapping.IsFixedLength : null;
 

diff --git a/src/EFCore.Relational/Storage/RelationalTypeMappingInfo.cs b/src/EFCore.Relational/Storage/RelationalTypeMappingInfo.cs
@@ -44,7 +44,7 @@ public RelationalTypeMappingInfo([NotNull] IReadOnlyList<IProperty> principals)
             _coreTypeMappingInfo = new TypeMappingInfo(principals);
 
             string storeTypeName = null;
-            var fixedLength = false;
+            bool? fixedLength = null;
             for (var i = 0; i < principals.Count; i++)
             {
                 var principal = principals[i];
@@ -57,13 +57,9 @@ public RelationalTypeMappingInfo([NotNull] IReadOnlyList<IProperty> principals)
                     }
                 }
 
-                if (!fixedLength)
+                if (fixedLength == null)
                 {
-                    var isFixedLength = principal.Relational().IsFixedLength;
-                    if (isFixedLength)
-                    {
-                        fixedLength = true;
-                    }
+                    fixedLength = principal.Relational().IsFixedLength;
                 }
             }
 
@@ -81,7 +77,7 @@ public RelationalTypeMappingInfo([NotNull] Type type)
             _coreTypeMappingInfo = new TypeMappingInfo(type);
             StoreTypeName = null;
             StoreTypeNameBase = null;
-            IsFixedLength = false;
+            IsFixedLength = null;
             _parsedSize = null;
             _parsedPrecision = null;
             _parsedScale = null;
@@ -100,7 +96,7 @@ public RelationalTypeMappingInfo([NotNull] string storeTypeName)
             _coreTypeMappingInfo = new TypeMappingInfo();
             StoreTypeName = storeTypeName;
             StoreTypeNameBase = ParseStoreTypeName(storeTypeName, out _parsedSize, out _parsedPrecision, out _parsedScale, out _isMax);
-            IsFixedLength = _parsedSize != null;
+            IsFixedLength = null;
         }
 
         /// <summary>
@@ -130,7 +126,7 @@ public RelationalTypeMappingInfo([NotNull] MemberInfo member)
                 _isMax = false;
             }
 
-            IsFixedLength = _parsedSize != null;
+            IsFixedLength = null;
         }
 
         /// <summary>

diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqlServerTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqlServerTest.cs
@@ -295,7 +295,7 @@ public void Maps_key_nchar_max_column()
         {
             var mapping = CreateMapper().FindMapping("nchar(max)", keyOrIndex: true, rowVersion: false);
 
-            AssertMapping<string>(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: true);
+            AssertMapping<string>(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: null);
         }
 
         [Fact]
@@ -311,7 +311,7 @@ public void Maps_key_char_max_column()
         {
             var mapping = CreateMapper().FindMapping("char(max)", keyOrIndex: true, rowVersion: false);
 
-            AssertMapping<string>(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: true);
+            AssertMapping<string>(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: null);
         }
 
         [Fact]
@@ -352,6 +352,7 @@ private static void AssertMapping<T>(TypeScaffoldingInfo mapping, bool inferred,
             Assert.Equal(inferred, mapping.IsInferred);
             Assert.Equal(maxLength, mapping.ScaffoldMaxLength);
             Assert.Equal(unicode, mapping.ScaffoldUnicode);
+            Assert.Equal(fixedLength, mapping.ScaffoldFixedLength);
         }
 
         private static ScaffoldingTypeMapper CreateMapper()