Skip to content

Commit

Permalink
Don't remove default table mapping for TVF return entity types. (#25034)
Browse files Browse the repository at this point in the history 
Preserve null mappings in the model snapshot.
Add non-generic ToSqlQuery overload.
Output Fluent API for ToSqlQuery in the model snapshot.
Fix SetFunctionName setting incorrect annotation.

Fixes #23408
  • Loading branch information
@AndriySvyryd
AndriySvyryd authored Jun 8, 2021
1 parent 7482ed7 commit 6b4ee13
Show file tree
Hide file tree
Showing 10 changed files with 192 additions and 59 deletions.
68 changes: 54 additions & 14 deletions src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -825,28 +825,43 @@ protected virtual void GenerateEntityTypeAnnotations(
|| entityType.BaseType == null)
{
var tableName = (string?)tableNameAnnotation?.Value ?? entityType.GetTableName();
if (tableName != null)
if (tableName != null
|| tableNameAnnotation != null)
{
var schemaAnnotation = annotations.Find(RelationalAnnotationNames.Schema);
stringBuilder
.AppendLine()
.Append(builderName)
.Append(".ToTable(")
.Append(Code.Literal(tableName));
.Append(".ToTable(");

if (tableName == null
&& schemaAnnotation == null)
{
stringBuilder.Append("(string)");
}

stringBuilder.Append(Code.UnknownLiteral(tableName));

if (tableNameAnnotation != null)
{
annotations.Remove(tableNameAnnotation.Name);
}

var schemaAnnotation = annotations.Find(RelationalAnnotationNames.Schema);
if (schemaAnnotation?.Value != null)
var isExcludedAnnotation = annotations.Find(RelationalAnnotationNames.IsTableExcludedFromMigrations);
if (schemaAnnotation != null)
{
stringBuilder
.Append(", ")
.Append(Code.Literal((string)schemaAnnotation.Value));
annotations.Remove(schemaAnnotation.Name);
.Append(", ");

if (schemaAnnotation.Value == null
&& ((bool?)isExcludedAnnotation?.Value) != true)
{
stringBuilder.Append("(string)");
}

stringBuilder.Append(Code.UnknownLiteral(schemaAnnotation.Value));
}

var isExcludedAnnotation = annotations.Find(RelationalAnnotationNames.IsTableExcludedFromMigrations);
if (isExcludedAnnotation != null)
{
if (((bool?)isExcludedAnnotation.Value) == true)
Expand All @@ -870,19 +885,21 @@ protected virtual void GenerateEntityTypeAnnotations(
stringBuilder.AppendLine(");");
}
}
annotations.Remove(RelationalAnnotationNames.Schema);

var viewNameAnnotation = annotations.Find(RelationalAnnotationNames.ViewName);
if (viewNameAnnotation?.Value != null
|| entityType.BaseType == null)
{
var viewName = (string?)viewNameAnnotation?.Value ?? entityType.GetViewName();
if (viewName != null)
if (viewName != null
|| viewNameAnnotation != null)
{
stringBuilder
.AppendLine()
.Append(builderName)
.Append(".ToView(")
.Append(Code.Literal(viewName));
.Append(Code.UnknownLiteral(viewName));
if (viewNameAnnotation != null)
{
annotations.Remove(viewNameAnnotation.Name);
Expand All @@ -900,25 +917,48 @@ protected virtual void GenerateEntityTypeAnnotations(
stringBuilder.AppendLine(");");
}
}
annotations.Remove(RelationalAnnotationNames.ViewSchema);
annotations.Remove(RelationalAnnotationNames.ViewDefinitionSql);

var functionNameAnnotation = annotations.Find(RelationalAnnotationNames.FunctionName);
if (functionNameAnnotation?.Value != null
|| entityType.BaseType == null)
{
var functionName = (string?)functionNameAnnotation?.Value ?? entityType.GetFunctionName();
if (functionName != null)
if (functionName != null
|| functionNameAnnotation != null)
{
stringBuilder
.AppendLine()
.Append(builderName)
.Append(".ToFunction(")
.Append(Code.Literal(functionName));
.Append(Code.UnknownLiteral(functionName))
.AppendLine(");");
if (functionNameAnnotation != null)
{
annotations.Remove(functionNameAnnotation.Name);
}
}
}

stringBuilder.AppendLine(");");
var sqlQueryAnnotation = annotations.Find(RelationalAnnotationNames.SqlQuery);
if (sqlQueryAnnotation?.Value != null
|| entityType.BaseType == null)
{
var sqlQuery = (string?)sqlQueryAnnotation?.Value ?? entityType.GetSqlQuery();
if (sqlQuery != null
|| sqlQueryAnnotation != null)
{
stringBuilder
.AppendLine()
.Append(builderName)
.Append(".ToSqlQuery(")
.Append(Code.UnknownLiteral(sqlQuery))
.AppendLine(");");
if (sqlQueryAnnotation != null)
{
annotations.Remove(sqlQueryAnnotation.Name);
}
}
}

Expand Down
1 change: 1 addition & 0 deletions src/EFCore.Design/Migrations/Design/MigrationsCodeGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ private static IEnumerable<IAnnotatable> GetAnnotatables(IModel model)
private IEnumerable<string> GetAnnotationNamespaces(IEnumerable<IAnnotatable> items)
=> items.SelectMany(
i => Dependencies.AnnotationCodeGenerator.FilterIgnoredAnnotations(i.GetAnnotations())
.Where(a => a.Value != null)
.Select(a => new { Annotatable = i, Annotation = a })
.SelectMany(a => GetProviderType(a.Annotatable, a.Annotation.Value!.GetType()).GetNamespaces()));

Expand Down
14 changes: 7 additions & 7 deletions src/EFCore.Relational/Design/AnnotationCodeGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ public AnnotationCodeGenerator(AnnotationCodeGeneratorDependencies dependencies)
/// <inheritdoc />
public virtual IEnumerable<IAnnotation> FilterIgnoredAnnotations(IEnumerable<IAnnotation> annotations)
=> annotations.Where(
a => !(
a.Value is null
|| CoreAnnotationNames.AllNames.Contains(a.Name)
a => !(CoreAnnotationNames.AllNames.Contains(a.Name)
|| _ignoredRelationalAnnotations.Contains(a.Name)));

/// <inheritdoc />
Expand Down Expand Up @@ -676,12 +674,14 @@ private static void GenerateSimpleFluentApiCall(
string methodName,
List<MethodCallCodeFragment> methodCallCodeFragments)
{
if (annotations.TryGetValue(annotationName, out var annotation)
&& annotation.Value is object annotationValue)
if (annotations.TryGetValue(annotationName, out var annotation))
{
annotations.Remove(annotationName);
methodCallCodeFragments.Add(
new MethodCallCodeFragment(methodName, annotationValue));
if (annotation.Value is object annotationValue)
{
methodCallCodeFragments.Add(
new MethodCallCodeFragment(methodName, annotationValue));
}
}
}

Expand Down
17 changes: 14 additions & 3 deletions src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -691,10 +691,9 @@ public static bool CanSetViewSchema(
/// <param name="entityTypeBuilder"> The builder for the entity type being configured. </param>
/// <param name="query"> The SQL query that will provide the underlying data for the entity type. </param>
/// <returns> The same builder instance so that multiple calls can be chained. </returns>
public static EntityTypeBuilder<TEntity> ToSqlQuery<TEntity>(
this EntityTypeBuilder<TEntity> entityTypeBuilder,
public static EntityTypeBuilder ToSqlQuery(
this EntityTypeBuilder entityTypeBuilder,
string query)
where TEntity : class
{
Check.NotNull(query, nameof(query));

Expand All @@ -703,6 +702,18 @@ public static EntityTypeBuilder<TEntity> ToSqlQuery<TEntity>(
return entityTypeBuilder;
}

/// <summary>
/// Configures a SQL string used to provide data for the entity type.
/// </summary>
/// <param name="entityTypeBuilder"> The builder for the entity type being configured. </param>
/// <param name="query"> The SQL query that will provide the underlying data for the entity type. </param>
/// <returns> The same builder instance so that multiple calls can be chained. </returns>
public static EntityTypeBuilder<TEntity> ToSqlQuery<TEntity>(
this EntityTypeBuilder<TEntity> entityTypeBuilder,
string query)
where TEntity : class
=> (EntityTypeBuilder<TEntity>)ToSqlQuery((EntityTypeBuilder)entityTypeBuilder, query);

/// <summary>
/// Configures a SQL string used to provide data for the entity type.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ public static void SetFunctionName(this IMutableEntityType entityType, string? n
string? name,
bool fromDataAnnotation = false)
=> (string?)entityType.SetAnnotation(
RelationalAnnotationNames.ViewName,
RelationalAnnotationNames.FunctionName,
Check.NullButNotEmpty(name, nameof(name)),
fromDataAnnotation)?.Value;

Expand Down
5 changes: 0 additions & 5 deletions src/EFCore.Relational/Metadata/Conventions/TableValuedDbFunctionConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,6 @@ private void ProcessDbFunctionAdded(

entityType = entityTypeBuilder.Metadata;
}

if (entityType.GetFunctionName() == null)
{
entityTypeBuilder.ToTable(null);
}
}
}
}
60 changes: 52 additions & 8 deletions test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,37 @@ public void Test_new_annotations_handled_for_entity_types()
RelationalAnnotationNames.ViewColumnMappings,
RelationalAnnotationNames.SqlQueryColumnMappings,
RelationalAnnotationNames.FunctionColumnMappings,
RelationalAnnotationNames.DefaultColumnMappings,
RelationalAnnotationNames.TableMappings,
RelationalAnnotationNames.ViewMappings,
RelationalAnnotationNames.FunctionMappings,
RelationalAnnotationNames.SqlQueryMappings,
RelationalAnnotationNames.DefaultMappings,
RelationalAnnotationNames.ForeignKeyMappings,
RelationalAnnotationNames.TableIndexMappings,
RelationalAnnotationNames.UniqueConstraintMappings,
RelationalAnnotationNames.RelationalOverrides,
RelationalAnnotationNames.DefaultValueSql,
RelationalAnnotationNames.ComputedColumnSql,
RelationalAnnotationNames.DefaultValue,
RelationalAnnotationNames.Name,
#pragma warning disable CS0618 // Type or member is obsolete
RelationalAnnotationNames.SequencePrefix,
#pragma warning restore CS0618 // Type or member is obsolete
RelationalAnnotationNames.Sequences,
RelationalAnnotationNames.CheckConstraints,
RelationalAnnotationNames.DefaultSchema,
RelationalAnnotationNames.Filter,
#pragma warning disable CS0618 // Type or member is obsolete
RelationalAnnotationNames.DbFunction,
#pragma warning restore CS0618 // Type or member is obsolete
RelationalAnnotationNames.DbFunctions,
RelationalAnnotationNames.MaxIdentifierLength,
RelationalAnnotationNames.IsFixedLength,
RelationalAnnotationNames.Collation
RelationalAnnotationNames.Collation,
RelationalAnnotationNames.IsStored,
RelationalAnnotationNames.RelationalModel,
RelationalAnnotationNames.ModelDependencies
};

// Add a line here if the code generator is supposed to handle this annotation
Expand Down Expand Up @@ -140,18 +158,18 @@ public void Test_new_annotations_handled_for_entity_types()
},
{
RelationalAnnotationNames.FunctionName,
(null, "")
(null, _nl + "modelBuilder." + nameof(RelationalEntityTypeBuilderExtensions.ToFunction) + @"(null);" + _nl)
},
{
RelationalAnnotationNames.SqlQuery,
(null, "")
(null, _nl + "modelBuilder." + nameof(RelationalEntityTypeBuilderExtensions.ToSqlQuery) + @"(null);" + _nl)
}
};

MissingAnnotationCheck(
b => b.Entity<WithAnnotations>().Metadata,
notForEntityType, forEntityType,
_toTable,
a => _toTable,
(g, m, b) => g.TestGenerateEntityTypeAnnotations("modelBuilder", (IEntityType)m, b));
}

Expand All @@ -176,20 +194,41 @@ public void Test_new_annotations_handled_for_properties()
CoreAnnotationNames.AmbiguousNavigations,
CoreAnnotationNames.DuplicateServiceProperties,
RelationalAnnotationNames.TableName,
RelationalAnnotationNames.IsTableExcludedFromMigrations,
RelationalAnnotationNames.ViewName,
RelationalAnnotationNames.Schema,
RelationalAnnotationNames.ViewSchema,
RelationalAnnotationNames.ViewDefinitionSql,
RelationalAnnotationNames.FunctionName,
RelationalAnnotationNames.SqlQuery,
RelationalAnnotationNames.DefaultSchema,
RelationalAnnotationNames.DefaultMappings,
RelationalAnnotationNames.TableColumnMappings,
RelationalAnnotationNames.ViewColumnMappings,
RelationalAnnotationNames.SqlQueryColumnMappings,
RelationalAnnotationNames.FunctionColumnMappings,
RelationalAnnotationNames.DefaultColumnMappings,
RelationalAnnotationNames.TableMappings,
RelationalAnnotationNames.ViewMappings,
RelationalAnnotationNames.FunctionMappings,
RelationalAnnotationNames.SqlQueryMappings,
RelationalAnnotationNames.ForeignKeyMappings,
RelationalAnnotationNames.TableIndexMappings,
RelationalAnnotationNames.UniqueConstraintMappings,
RelationalAnnotationNames.Name,
RelationalAnnotationNames.Sequences,
#pragma warning disable CS0618 // Type or member is obsolete
RelationalAnnotationNames.SequencePrefix,
#pragma warning restore CS0618 // Type or member is obsolete
RelationalAnnotationNames.CheckConstraints,
RelationalAnnotationNames.Filter,
#pragma warning disable CS0618 // Type or member is obsolete
RelationalAnnotationNames.DbFunction,
#pragma warning restore CS0618 // Type or member is obsolete
RelationalAnnotationNames.DbFunctions,
RelationalAnnotationNames.MaxIdentifierLength
RelationalAnnotationNames.MaxIdentifierLength,
RelationalAnnotationNames.RelationalModel,
RelationalAnnotationNames.ModelDependencies
};

var columnMapping =
Expand All @@ -201,6 +240,7 @@ public void Test_new_annotations_handled_for_properties()
{
{ CoreAnnotationNames.MaxLength, (256, $@"{_nl}.{nameof(PropertyBuilder.HasMaxLength)}(256){columnMapping}") },
{ CoreAnnotationNames.Precision, (4, $@"{_nl}.{nameof(PropertyBuilder.HasPrecision)}(4){columnMapping}") },
{ CoreAnnotationNames.Scale, (null, $@"{columnMapping}") },
{ CoreAnnotationNames.Unicode, (false, $@"{_nl}.{nameof(PropertyBuilder.IsUnicode)}(false){columnMapping}") },
{
CoreAnnotationNames.ValueConverter, (new ValueConverter<int, long>(v => v, v => (int)v),
Expand Down Expand Up @@ -242,21 +282,25 @@ public void Test_new_annotations_handled_for_properties()
RelationalAnnotationNames.Collation,
("Some Collation",
$@"{columnMapping}{_nl}.{nameof(RelationalPropertyBuilderExtensions.UseCollation)}(""Some Collation"")")
},
{
RelationalAnnotationNames.IsStored,
(null, $@"{columnMapping}{_nl}.HasAnnotation(""{RelationalAnnotationNames.IsStored}"", null)")
}
};

MissingAnnotationCheck(
b => b.Entity<WithAnnotations>().Property(e => e.Id).Metadata,
notForProperty, forProperty,
$"{columnMapping}",
a => $"{columnMapping}",
(g, m, b) => g.TestGeneratePropertyAnnotations((IProperty)m, b));
}

private static void MissingAnnotationCheck(
Func<ModelBuilder, IMutableAnnotatable> createMetadataItem,
HashSet<string> invalidAnnotations,
Dictionary<string, (object Value, string Expected)> validAnnotations,
string generationDefault,
Func<string, string> generationDefault,
Action<TestCSharpSnapshotGenerator, IMutableAnnotatable, IndentedStringBuilder> test)
{
var sqlServerTypeMappingSource = new SqlServerTypeMappingSource(
Expand Down Expand Up @@ -314,7 +358,7 @@ private static void MissingAnnotationCheck(
Assert.Equal(
validAnnotations.ContainsKey(annotationName)
? validAnnotations[annotationName].Expected
: generationDefault,
: generationDefault(annotationName),
sb.ToString());
}
catch (Exception e)
Expand Down
Loading

0 comments on commit 6b4ee13

Please sign in to comment.